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_lin@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.
> 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.
> + */
The SG_LIST_ADDR_MASK needs an update though. AFAICT, it's bigger on AST2700.
Thanks,
C.
> plen = len & SG_LIST_LEN_MASK;
> haddr = address_space_map(&s->dram_as, sg_addr, &plen, false,
> @@ -218,7 +237,9 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
> } else {
> plen = s->regs[R_HASH_SRC_LEN];
> 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]);
> + }
> haddr = address_space_map(&s->dram_as, src,
> &plen, false, MEMTXATTRS_UNSPECIFIED);
> if (haddr == NULL) {
> @@ -275,6 +296,9 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
> }
>
> digest_addr = deposit64(digest_addr, 0, 32, s->regs[R_HASH_DEST]);
> + if (ahc->has_dma64) {
> + digest_addr = deposit64(digest_addr, 32, 32, s->regs[R_HASH_DEST_HI]);
> + }
> if (address_space_write(&s->dram_as, digest_addr,
> MEMTXATTRS_UNSPECIFIED,
> digest_buf, digest_len)) {
> @@ -601,6 +625,7 @@ static void aspeed_ast2700_hace_class_init(ObjectClass *klass, void *data)
> * has completed. It is a temporary workaround.
> */
> ahc->raise_crypt_interrupt_workaround = true;
> + ahc->has_dma64 = true;
> }
>
> static const TypeInfo aspeed_ast2700_hace_info = {