On 5/9/25 09:04, Jamin Lin wrote:
Hi Cédric

Subject: Re: [PATCH v1 08/22] hw/misc/aspeed_hace: Support DMA 64 bits
dram address.

On 3/21/25 10:26, Jamin Lin wrote:
According to the AST2700 design, the data source address is 64-bit,
with R_HASH_SRC_HI storing bits [63:32] and R_HASH_SRC storing bits
[31:0].

Similarly, the digest address is 64-bit, with R_HASH_DEST_HI storing
bits [63:32] and R_HASH_DEST storing bits [31:0].

Ideally, sg_addr should be 64-bit for the AST2700, using the following
program to obtain the 64-bit sg_addr and convert it to a DRAM offset:

```
sg_addr = deposit64(sg_addr, 32, 32,
                      address_space_ldl_le(&s->dram_as, src +
SG_LIST_ADDR_SIZE,

MEMTXATTRS_UNSPECIFIED,
NULL); sg_addr -= 0x400000000; ```

I don't think the code extract above is useful.

To maintain compatibility with older SoCs such as the AST2600, the
AST2700 HW HACE controllers automatically set bit 34 of the 64-bit sg_addr.

I suppose that's what bits [30:28] of the first word of the scatter-gather entry
are for ?

As a result,
the firmware only needs to provide a 32-bit sg_addr containing bits [31:0].
This is sufficient for the AST2700, as it uses a DRAM offset rather
than a DRAM address.

yes the HACE model can use a relative address because the DRAM memory
region is directly available. There is no need to construct a physical address.

Introduce a has_dma64 class attribute and set it to true for the AST2700.

Signed-off-by: Jamin Lin <jamin_...@aspeedtech.com>
---
   include/hw/misc/aspeed_hace.h |  1 +
   hw/misc/aspeed_hace.c         | 27 ++++++++++++++++++++++++++-
   2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/include/hw/misc/aspeed_hace.h
b/include/hw/misc/aspeed_hace.h index a4479bd383..58fb66009a 100644
--- a/include/hw/misc/aspeed_hace.h
+++ b/include/hw/misc/aspeed_hace.h
@@ -52,6 +52,7 @@ struct AspeedHACEClass {
       uint32_t src_hi_mask;
       uint32_t dest_hi_mask;
       uint32_t key_hi_mask;
+    bool has_dma64;
   };

   #endif /* ASPEED_HACE_H */
diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c index
51c6523fab..8f333fc97e 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -148,6 +148,7 @@ static bool has_padding(AspeedHACEState *s, struct
iovec *iov,
   static void do_hash_operation(AspeedHACEState *s, int algo, bool
sg_mode,
                                 bool acc_mode)
   {
+    AspeedHACEClass *ahc = ASPEED_HACE_GET_CLASS(s);
       bool sg_acc_mode_final_request = false;
       g_autofree uint8_t *digest_buf = NULL;
       struct iovec iov[ASPEED_HACE_MAX_SG]; @@ -182,6 +183,9 @@
static
void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
               }

               src = deposit64(src, 0, 32, s->regs[R_HASH_SRC]);
+            if (ahc->has_dma64) {
+                src = deposit64(src, 32, 32, s->regs[R_HASH_SRC_HI]);
+            }

That's where a little helper would be nice to have.

I add hash_get_source_addr help function.
Please see patch 6 comments.

               src += i * SG_LIST_ENTRY_SIZE;

               len = address_space_ldl_le(&s->dram_as, src, @@ -190,6
+194,21 @@ static void do_hash_operation(AspeedHACEState *s, int algo,
bool sg_mode,
               sg_addr = address_space_ldl_le(&s->dram_as, src +
SG_LIST_LEN_SIZE,

MEMTXATTRS_UNSPECIFIED, NULL);
               sg_addr &= SG_LIST_ADDR_MASK;
+            /*
+             * Ideally, sg_addr should be 64-bit for the AST2700, using
the
+             * following program to obtain the 64-bit sg_addr and
convert it
+             * to a DRAM offset:
+             * sg_addr = deposit64(sg_addr, 32, 32,
+             *      address_space_ldl_le(&s->dram_as, src +
SG_ADDR_LEN_SIZE,
+             *
MEMTXATTRS_UNSPECIFIED, NULL);
+             * sg_addr -= 0x400000000;
+             *

I don't think the above comment is useful. Please keep the one below.

+             * To maintain compatibility with older SoCs such as the
AST2600,
+             * the AST2700 HW automatically set bit 34 of the 64-bit
sg_addr.
+             * As a result, the firmware only needs to provide a 32-bit
sg_addr
+             * containing bits [31:0]. This is sufficient for the AST2700,
as
+             * it uses a DRAM offset rather than a DRAM address.
+             */


Thanks for suggestion will update them.

The SG_LIST_ADDR_MASK needs an update though. AFAICT, it's bigger on
AST2700.

The value of SG_LIST_ADDR_MASK was wrong for AST2700, AST2600 and AST1030.
The correct value should be 0x7FFFFFF8.
Will create a new patch to fix it.
Please see patch 4 comments.
By the way, AST2500 do not support SG mode.

Should we introduce a class attribute then ?

Thanks,

C.

Reply via email to