qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/28] Fix incorrect hash results on AST2700
@ 2025-05-15  8:09 Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 01/28] hw/misc/aspeed_hace: Remove unused code for better readability Jamin Lin via
                   ` (31 more replies)
  0 siblings, 32 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

v1:
 1. Added support for 64-bit DMA in the HACE model
 2. Refactored the do_hash operation in the HACE model
 3. Fixed a crash caused by out-of-bound memory access in HACE
 4. Added more trace events and implemented dumping of source hash data and
    resulting digests to improve debugging
 5. Refactored the HACE QTest framework to support both AST1030 and AST2700
 6. Added a test case for SHA384

v2:
  1. Create new helper functions
     hash_get_source_addr
     hash_prepare_direct_iov
     hash_prepare_sg_iov
     hash_get_digest_addr
     hash_write_digest_and_unmap_iov
     hash_execute_non_acc_mode
     hash_execute_acc_mode
  2. Refactor do_hash_operation
  3. Fix review issue
  4. Revise trace-events
  5. Move register size to instance class and dynamically allocate regs

v3:
  1. Split patch to introduce these routines one by one : 
       hash_prepare_sg_iov
       hash_prepare_direct_iov
       hash_execute_acc_mode
       hash_execute_non_acc_mode
       hash_write_digest_and_unmap_iov
  2. Fix run qtest failed  
 
This patchset resolves incorrect hash results reported on the AST2700 platform.
This update addresses the following kernel warnings and test failures related to
the crypto self-test framework:

aspeed-hmac-sha512 test failed (incorrect result)
aspeed-hmac-sha384 test failed (incorrect result)
aspeed-sha512 test failed (incorrect result)
aspeed-sha384 test failed (incorrect result)
aspeed-hmac-sha256 test failed (incorrect result)
aspeed-hmac-sha224 test failed (incorrect result)
aspeed-hmac-sha1 test failed (incorrect result)
aspeed-sha224 test failed (incorrect result)
aspeed-sha256 test failed (incorrect result)
aspeed-sha1 test failed (incorrect result)

How to test it

Use the following command to dump information about the supported digest methods
via the ast_crypto_engine hardware engine:

root@ast2700-default:~# openssl engine -pre DUMP_INFO ast_crypto_engine

Digest SHA1, NID=64, AF_ALG info: name=sha1ALG_ERR: , driver=aspeed-sha1 (hw accelerated)
Digest SHA224, NID=675, AF_ALG info: name=sha224ALG_ERR: , driver=aspeed-sha224 (hw accelerated)
Digest SHA256, NID=672, AF_ALG info: name=sha256ALG_ERR: , driver=aspeed-sha256 (hw accelerated)
Digest SHA384, NID=673, AF_ALG info: name=sha384ALG_ERR: , driver=aspeed-sha384 (hw accelerated)
Digest SHA512, NID=674, AF_ALG info: name=sha512ALG_ERR: , driver=aspeed-sha512 (hw accelerated)

The status of SHA1, SHA224, SHA256, SHA384, and SHA512 should be marked as
hw accelerated, indicating that these algorithms are supported by hardware
acceleration via the aspeed drivers.

Create a test file on the host machine and compute its HASH value as the
expected result

Create a 256MB test file

$ dd if=/dev/random of=/tmp/256M bs=1M count=256
Generate Hash Values Using SHA1, SHA224, SHA256, SHA384, and SHA512

Use the following commands to generate HASH values for a 256MB file using
different SHA algorithms:

$ sha1sum /tmp/256M
7fc628811a31ab87b0502dab3ed8d3ef07565885  /tmp/256M

$ sha224sum /tmp/256M
2d261c11ba05b3a62e0efeab51c307d9933426c7e18204683ef3da54  /tmp/256M

$ sha256sum /tmp/256M
5716d1700ee35c92ca5ca5b466639e9c36eed3f1447c1aec27f16d0fe113f94d  /tmp/256M

$ sha384sum /tmp/256M
fb6bc62afa1096dcd3b870e7d2546b7a5a177b5f2bbd5c9759218182454709e0c504a2d9c26404e04aa8010a291b7f1c  /tmp/256M

$ sha512sum /tmp/256M
fbceda7be34836fe857781656318ecd5b457a833a24c8736d5b8ef8d07e1950eebcdb140eebe4f12b5ff59586f7eb1c64fa95869c63dd9e4703d91261093c5c9  /tmp/256M

Generate HASH Values Using the Hardware Engine

Use the following commands to generate HASH values for a 256MB file using
various SHA algorithms with the ast_crypto_engine hardware engine:

root@ast2700-default:~# openssl dgst -sha1 -engine ast_crypto_engine /tmp/256M
Engine "ast_crypto_engine" set.
SHA1(/tmp/256M)= 7fc628811a31ab87b0502dab3ed8d3ef07565885

root@ast2700-default:~# openssl dgst -sha224 -engine ast_crypto_engine /tmp/256M
Engine "ast_crypto_engine" set.
SHA2-224(/tmp/256M)= 2d261c11ba05b3a62e0efeab51c307d9933426c7e18204683ef3da54

root@ast2700-default:~# openssl dgst -sha256 -engine ast_crypto_engine /tmp/256M
Engine "ast_crypto_engine" set.
SHA2-256(/tmp/256M)= 5716d1700ee35c92ca5ca5b466639e9c36eed3f1447c1aec27f16d0fe113f94d

root@ast2700-default:~# openssl dgst -sha384 -engine ast_crypto_engine /tmp/256M
Engine "ast_crypto_engine" set.
SHA2-384(/tmp/256M)= fb6bc62afa1096dcd3b870e7d2546b7a5a177b5f2bbd5c9759218182454709e0c504a2d9c26404e04aa8010a291b7f1c

root@ast2700-default:~# openssl dgst -sha512 -engine ast_crypto_engine /tmp/256M
Engine "ast_crypto_engine" set.
SHA2-512(/tmp/256M)= fbceda7be34836fe857781656318ecd5b457a833a24c8736d5b8ef8d07e1950eebcdb140eebe4f12b5ff59586f7eb1c64fa95869c63dd9e4703d91261093c5c9

The HASH values generated here should exactly match those computed on the host
machine using sha shell commands, verifying both the correctness of the
hardware-accelerated results and the functionality of the ast_crypto_engine.

Jamin Lin (28):
  hw/misc/aspeed_hace: Remove unused code for better readability
  hw/misc/aspeed_hace: Improve readability and consistency in variable
    naming
  hw/misc/aspeed_hace: Ensure HASH_IRQ is always set to prevent firmware
    hang
  hw/misc/aspeed_hace: Extract direct mode hash buffer setup into helper
    function
  hw/misc/aspeed_hace: Extract SG-mode hash buffer setup into helper
    function
  hw/misc/aspeed_hace: Extract digest write and iov unmap into helper
    function
  hw/misc/aspeed_hace: Extract non-accumulation hash execution into
    helper function
  hw/misc/aspeed_hace: Extract accumulation-mode hash execution into
    helper function
  hw/misc/aspeed_hace: Introduce 64-bit hash source address helper
    function
  hw/misc/aspeed_hace: Rename R_HASH_DEST to R_HASH_DIGEST and introduce
    64-bit hash digest address helper
  hw/misc/aspeed_hace: Support accumulative mode for direct access mode
  hw/misc/aspeed_hace: Move register size to instance class and
    dynamically allocate regs
  hw/misc/aspeed_hace: Add support for source, digest, key buffer 64 bit
    addresses
  hw/misc/aspeed_hace: Support DMA 64 bits dram address
  hw/misc/aspeed_hace: Add trace-events for better debugging
  hw/misc/aspeed_hace: Support to dump plaintext and digest for better
    debugging
  tests/qtest: Reorder aspeed test list
  test/qtest: Introduce a new aspeed-hace-utils.c to place common
    testcases
  test/qtest/hace: Specify explicit array sizes for test vectors and
    hash results
  test/qtest/hace: Adjust test address range for AST1030 due to SRAM
    limitations
  test/qtest/hace: Add SHA-384 test cases for ASPEED HACE model
  test/qtest/hace: Add SHA-384 tests for AST2600
  test/qtest/hace: Add tests for AST1030
  test/qtest/hace: Update source data and digest data type to 64-bit
  test/qtest/hace: Support 64-bit source and digest addresses for
    AST2700
  test/qtest/hace: Support to test upper 32 bits of digest and source
    addresses
  test/qtest/hace: Support to validate 64-bit hmac key buffer addresses
  test/qtest/hace: Add tests for AST2700

 include/hw/misc/aspeed_hace.h   |  11 +-
 tests/qtest/aspeed-hace-utils.h |  84 +++++
 hw/misc/aspeed_hace.c           | 479 +++++++++++++++--------
 tests/qtest/aspeed-hace-utils.c | 646 ++++++++++++++++++++++++++++++++
 tests/qtest/aspeed_hace-test.c  | 577 +++++-----------------------
 tests/qtest/ast2700-hace-test.c |  98 +++++
 hw/misc/trace-events            |   8 +
 tests/qtest/meson.build         |  13 +-
 8 files changed, 1279 insertions(+), 637 deletions(-)
 create mode 100644 tests/qtest/aspeed-hace-utils.h
 create mode 100644 tests/qtest/aspeed-hace-utils.c
 create mode 100644 tests/qtest/ast2700-hace-test.c

-- 
2.43.0



^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCH v3 01/28] hw/misc/aspeed_hace: Remove unused code for better readability
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 02/28] hw/misc/aspeed_hace: Improve readability and consistency in variable naming Jamin Lin via
                   ` (30 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

In the previous design of the hash framework, accumulative hashing was not
supported. To work around this limitation, commit 5cd7d85 introduced an
iov_cache array to store all the hash data from firmware.
Once the ASPEED HACE model collected all the data, it passed the iov_cache to
the hash API to calculate the final digest.

However, with commit e3c0752, the hash framework now supports accumulative
hashing. This allows us to refactor the ASPEED HACE model, removing redundant
logic and simplifying the implementation for better readability and
maintainability.

As a result, the iov_count variable is no longer needed—it was previously used
to track how many cached entries were used for hashing.
To maintain VMSTATE compatibility after removing this field, the VMSTATE_VERSION
is bumped to 2

This cleanup follows significant changes in commit 4c1d0af4a28d, making the
model more readable.

- Deleted "iov_cache" and "iov_count" from "AspeedHACEState".
- Removed "reconstruct_iov" function and related logic.
- Simplified "do_hash_operation" by eliminating redundant checks.

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

diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h
index 5d4aa19cfe..b69a038d35 100644
--- a/include/hw/misc/aspeed_hace.h
+++ b/include/hw/misc/aspeed_hace.h
@@ -31,10 +31,8 @@ struct AspeedHACEState {
     MemoryRegion iomem;
     qemu_irq irq;
 
-    struct iovec iov_cache[ASPEED_HACE_MAX_SG];
     uint32_t regs[ASPEED_HACE_NR_REGS];
     uint32_t total_req_len;
-    uint32_t iov_count;
 
     MemoryRegion *dram_mr;
     AddressSpace dram_as;
diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index f4bff32a00..9263739ea6 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -142,25 +142,6 @@ static bool has_padding(AspeedHACEState *s, struct iovec *iov,
     return false;
 }
 
-static int reconstruct_iov(AspeedHACEState *s, struct iovec *iov, int id,
-                           uint32_t *pad_offset)
-{
-    int i, iov_count;
-    if (*pad_offset != 0) {
-        s->iov_cache[s->iov_count].iov_base = iov[id].iov_base;
-        s->iov_cache[s->iov_count].iov_len = *pad_offset;
-        ++s->iov_count;
-    }
-    for (i = 0; i < s->iov_count; i++) {
-        iov[i].iov_base = s->iov_cache[i].iov_base;
-        iov[i].iov_len = s->iov_cache[i].iov_len;
-    }
-    iov_count = s->iov_count;
-    s->iov_count = 0;
-    s->total_req_len = 0;
-    return iov_count;
-}
-
 static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
                               bool acc_mode)
 {
@@ -242,19 +223,6 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
         iov[0].iov_base = haddr;
         iov[0].iov_len = len;
         i = 1;
-
-        if (s->iov_count) {
-            /*
-             * In aspeed sdk kernel driver, sg_mode is disabled in hash_final().
-             * Thus if we received a request with sg_mode disabled, it is
-             * required to check whether cache is empty. If no, we should
-             * combine cached iov and the current iov.
-             */
-            s->total_req_len += len;
-            if (has_padding(s, iov, len, &total_msg_len, &pad_offset)) {
-                i = reconstruct_iov(s, iov, 0, &pad_offset);
-            }
-        }
     }
 
     if (acc_mode) {
@@ -278,7 +246,6 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
             qcrypto_hash_free(s->hash_ctx);
 
             s->hash_ctx = NULL;
-            s->iov_count = 0;
             s->total_req_len = 0;
         }
     } else if (qcrypto_hash_bytesv(algo, iov, i, &digest_buf,
@@ -437,7 +404,6 @@ static void aspeed_hace_reset(DeviceState *dev)
     }
 
     memset(s->regs, 0, sizeof(s->regs));
-    s->iov_count = 0;
     s->total_req_len = 0;
 }
 
@@ -469,12 +435,11 @@ static const Property aspeed_hace_properties[] = {
 
 static const VMStateDescription vmstate_aspeed_hace = {
     .name = TYPE_ASPEED_HACE,
-    .version_id = 1,
-    .minimum_version_id = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
     .fields = (const VMStateField[]) {
         VMSTATE_UINT32_ARRAY(regs, AspeedHACEState, ASPEED_HACE_NR_REGS),
         VMSTATE_UINT32(total_req_len, AspeedHACEState),
-        VMSTATE_UINT32(iov_count, AspeedHACEState),
         VMSTATE_END_OF_LIST(),
     }
 };
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 02/28] hw/misc/aspeed_hace: Improve readability and consistency in variable naming
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 01/28] hw/misc/aspeed_hace: Remove unused code for better readability Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 03/28] hw/misc/aspeed_hace: Ensure HASH_IRQ is always set to prevent firmware hang Jamin Lin via
                   ` (29 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

Currently, users define multiple local variables within different if-statements.
To improve readability and maintain consistency in variable naming, rename the
variables accordingly.
Introduced "sg_addr" to clearly indicate the scatter-gather mode buffer address.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/misc/aspeed_hace.c | 67 +++++++++++++++++++++----------------------
 1 file changed, 33 insertions(+), 34 deletions(-)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 9263739ea6..6be94963bc 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -145,15 +145,19 @@ static bool has_padding(AspeedHACEState *s, struct iovec *iov,
 static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
                               bool acc_mode)
 {
+    g_autofree uint8_t *digest_buf = NULL;
     struct iovec iov[ASPEED_HACE_MAX_SG];
+    bool acc_final_request = false;
+    Error *local_err = NULL;
     uint32_t total_msg_len;
-    uint32_t pad_offset;
-    g_autofree uint8_t *digest_buf = NULL;
     size_t digest_len = 0;
-    bool sg_acc_mode_final_request = false;
-    int i;
+    uint32_t sg_addr = 0;
+    uint32_t pad_offset;
+    int iov_idx = 0;
+    uint32_t len = 0;
+    uint32_t src = 0;
     void *haddr;
-    Error *local_err = NULL;
+    hwaddr plen;
 
     if (acc_mode && s->hash_ctx == NULL) {
         s->hash_ctx = qcrypto_hash_new(algo, &local_err);
@@ -166,74 +170,69 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
     }
 
     if (sg_mode) {
-        uint32_t len = 0;
-
-        for (i = 0; !(len & SG_LIST_LEN_LAST); i++) {
-            uint32_t addr, src;
-            hwaddr plen;
-
-            if (i == ASPEED_HACE_MAX_SG) {
+        for (iov_idx = 0; !(len & SG_LIST_LEN_LAST); iov_idx++) {
+            if (iov_idx == ASPEED_HACE_MAX_SG) {
                 qemu_log_mask(LOG_GUEST_ERROR,
                         "aspeed_hace: guest failed to set end of sg list marker\n");
                 break;
             }
 
-            src = s->regs[R_HASH_SRC] + (i * SG_LIST_ENTRY_SIZE);
+            src = s->regs[R_HASH_SRC] + (iov_idx * SG_LIST_ENTRY_SIZE);
 
             len = address_space_ldl_le(&s->dram_as, src,
                                        MEMTXATTRS_UNSPECIFIED, NULL);
 
-            addr = address_space_ldl_le(&s->dram_as, src + SG_LIST_LEN_SIZE,
-                                        MEMTXATTRS_UNSPECIFIED, NULL);
-            addr &= SG_LIST_ADDR_MASK;
+            sg_addr = address_space_ldl_le(&s->dram_as, src + SG_LIST_LEN_SIZE,
+                                           MEMTXATTRS_UNSPECIFIED, NULL);
+            sg_addr &= SG_LIST_ADDR_MASK;
 
             plen = len & SG_LIST_LEN_MASK;
-            haddr = address_space_map(&s->dram_as, addr, &plen, false,
+            haddr = address_space_map(&s->dram_as, sg_addr, &plen, false,
                                       MEMTXATTRS_UNSPECIFIED);
             if (haddr == NULL) {
                 qemu_log_mask(LOG_GUEST_ERROR,
                               "%s: qcrypto failed\n", __func__);
                 return;
             }
-            iov[i].iov_base = haddr;
+            iov[iov_idx].iov_base = haddr;
             if (acc_mode) {
                 s->total_req_len += plen;
 
-                if (has_padding(s, &iov[i], plen, &total_msg_len,
+                if (has_padding(s, &iov[iov_idx], plen, &total_msg_len,
                                 &pad_offset)) {
                     /* Padding being present indicates the final request */
-                    sg_acc_mode_final_request = true;
-                    iov[i].iov_len = pad_offset;
+                    acc_final_request = true;
+                    iov[iov_idx].iov_len = pad_offset;
                 } else {
-                    iov[i].iov_len = plen;
+                    iov[iov_idx].iov_len = plen;
                 }
             } else {
-                iov[i].iov_len = plen;
+                iov[iov_idx].iov_len = plen;
             }
         }
     } else {
-        hwaddr len = s->regs[R_HASH_SRC_LEN];
+        plen = s->regs[R_HASH_SRC_LEN];
 
         haddr = address_space_map(&s->dram_as, s->regs[R_HASH_SRC],
-                                  &len, false, MEMTXATTRS_UNSPECIFIED);
+                                  &plen, false, MEMTXATTRS_UNSPECIFIED);
         if (haddr == NULL) {
             qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto failed\n", __func__);
             return;
         }
         iov[0].iov_base = haddr;
-        iov[0].iov_len = len;
-        i = 1;
+        iov[0].iov_len = plen;
+        iov_idx = 1;
     }
 
     if (acc_mode) {
-        if (qcrypto_hash_updatev(s->hash_ctx, iov, i, &local_err) < 0) {
+        if (qcrypto_hash_updatev(s->hash_ctx, iov, iov_idx, &local_err) < 0) {
             qemu_log_mask(LOG_GUEST_ERROR, "qcrypto hash update failed : %s",
                           error_get_pretty(local_err));
             error_free(local_err);
             return;
         }
 
-        if (sg_acc_mode_final_request) {
+        if (acc_final_request) {
             if (qcrypto_hash_finalize_bytes(s->hash_ctx, &digest_buf,
                                             &digest_len, &local_err)) {
                 qemu_log_mask(LOG_GUEST_ERROR,
@@ -248,7 +247,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
             s->hash_ctx = NULL;
             s->total_req_len = 0;
         }
-    } else if (qcrypto_hash_bytesv(algo, iov, i, &digest_buf,
+    } else if (qcrypto_hash_bytesv(algo, iov, iov_idx, &digest_buf,
                                    &digest_len, &local_err) < 0) {
         qemu_log_mask(LOG_GUEST_ERROR, "qcrypto hash bytesv failed : %s",
                       error_get_pretty(local_err));
@@ -263,10 +262,10 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
                       "aspeed_hace: address space write failed\n");
     }
 
-    for (; i > 0; i--) {
-        address_space_unmap(&s->dram_as, iov[i - 1].iov_base,
-                            iov[i - 1].iov_len, false,
-                            iov[i - 1].iov_len);
+    for (; iov_idx > 0; iov_idx--) {
+        address_space_unmap(&s->dram_as, iov[iov_idx - 1].iov_base,
+                            iov[iov_idx - 1].iov_len, false,
+                            iov[iov_idx - 1].iov_len);
     }
 
     /*
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 03/28] hw/misc/aspeed_hace: Ensure HASH_IRQ is always set to prevent firmware hang
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 01/28] hw/misc/aspeed_hace: Remove unused code for better readability Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 02/28] hw/misc/aspeed_hace: Improve readability and consistency in variable naming Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 04/28] hw/misc/aspeed_hace: Extract direct mode hash buffer setup into helper function Jamin Lin via
                   ` (28 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

Currently, if the program encounters an unsupported algorithm, it does not set
the HASH_IRQ bit in the status register and send an interrupt to indicate
command completion. As a result, the FW gets stuck waiting for a completion
signal from the HACE module.

Additionally, in do_hash_operation, if an error occurs within the conditional
statement, the HASH_IRQ bit is not set in the status register. This causes the
firmware to continuously send HASH commands, as it is unaware that the HACE
model has completed processing the command.

To fix this, the HASH_IRQ bit in the status register must always be set to
ensure that the firmware receives an interrupt from the HACE module, preventing
it from getting stuck or repeatedly sending HASH commands.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Fixes: c5475b3 ("hw: Model ASPEED's Hash and Crypto Engine")
---
 hw/misc/aspeed_hace.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 6be94963bc..1256926d22 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -267,12 +267,6 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
                             iov[iov_idx - 1].iov_len, false,
                             iov[iov_idx - 1].iov_len);
     }
-
-    /*
-     * Set status bits to indicate completion. Testing shows hardware sets
-     * these irrespective of HASH_IRQ_EN.
-     */
-    s->regs[R_STATUS] |= HASH_IRQ;
 }
 
 static uint64_t aspeed_hace_read(void *opaque, hwaddr addr, unsigned int size)
@@ -356,10 +350,16 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
                 qemu_log_mask(LOG_GUEST_ERROR,
                         "%s: Invalid hash algorithm selection 0x%"PRIx64"\n",
                         __func__, data & ahc->hash_mask);
-                break;
+        } else {
+            do_hash_operation(s, algo, data & HASH_SG_EN,
+                    ((data & HASH_HMAC_MASK) == HASH_DIGEST_ACCUM));
         }
-        do_hash_operation(s, algo, data & HASH_SG_EN,
-                ((data & HASH_HMAC_MASK) == HASH_DIGEST_ACCUM));
+
+        /*
+         * Set status bits to indicate completion. Testing shows hardware sets
+         * these irrespective of HASH_IRQ_EN.
+         */
+        s->regs[R_STATUS] |= HASH_IRQ;
 
         if (data & HASH_IRQ_EN) {
             qemu_irq_raise(s->irq);
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 04/28] hw/misc/aspeed_hace: Extract direct mode hash buffer setup into helper function
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (2 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 03/28] hw/misc/aspeed_hace: Ensure HASH_IRQ is always set to prevent firmware hang Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 05/28] hw/misc/aspeed_hace: Extract SG-mode " Jamin Lin via
                   ` (27 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

To improve code readability and maintainability of do_hash_operation(), this
commit introduces a new helper function: hash_prepare_direct_iov().
This function encapsulates the logic for setting up the I/O vector (iov)
in direct mode (non-scatter-gather).

No functional changes are introduced.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/misc/aspeed_hace.c | 42 ++++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 1256926d22..42c6f29f82 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -142,6 +142,31 @@ static bool has_padding(AspeedHACEState *s, struct iovec *iov,
     return false;
 }
 
+static int hash_prepare_direct_iov(AspeedHACEState *s, struct iovec *iov)
+{
+    uint32_t src;
+    void *haddr;
+    hwaddr plen;
+    int iov_idx;
+
+    plen = s->regs[R_HASH_SRC_LEN];
+    src = s->regs[R_HASH_SRC];
+    haddr = address_space_map(&s->dram_as, src, &plen, false,
+                              MEMTXATTRS_UNSPECIFIED);
+    if (haddr == NULL) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Unable to map address, addr=0x%x, "
+                      "plen=0x%" HWADDR_PRIx "\n",
+                      __func__, src, plen);
+        return -1;
+    }
+
+    iov[0].iov_base = haddr;
+    iov[0].iov_len = plen;
+    iov_idx = 1;
+
+    return iov_idx;
+}
 static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
                               bool acc_mode)
 {
@@ -169,6 +194,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
         }
     }
 
+    /* Prepares the iov for hashing operations based on the selected mode */
     if (sg_mode) {
         for (iov_idx = 0; !(len & SG_LIST_LEN_LAST); iov_idx++) {
             if (iov_idx == ASPEED_HACE_MAX_SG) {
@@ -211,17 +237,13 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
             }
         }
     } else {
-        plen = s->regs[R_HASH_SRC_LEN];
+        iov_idx = hash_prepare_direct_iov(s, iov);
+    }
 
-        haddr = address_space_map(&s->dram_as, s->regs[R_HASH_SRC],
-                                  &plen, false, MEMTXATTRS_UNSPECIFIED);
-        if (haddr == NULL) {
-            qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto failed\n", __func__);
-            return;
-        }
-        iov[0].iov_base = haddr;
-        iov[0].iov_len = plen;
-        iov_idx = 1;
+    if (iov_idx <= 0) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Failed to prepare iov\n", __func__);
+         return;
     }
 
     if (acc_mode) {
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 05/28] hw/misc/aspeed_hace: Extract SG-mode hash buffer setup into helper function
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (3 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 04/28] hw/misc/aspeed_hace: Extract direct mode hash buffer setup into helper function Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 06/28] hw/misc/aspeed_hace: Extract digest write and iov unmap " Jamin Lin via
                   ` (26 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

To improve code readability and maintainability of do_hash_operation(), this
commit introduces a new helper function: hash_prepare_sg_iov().

This function handles scatter-gather (SG) mode setup, including SG list
parsing, address mapping, and optional accumulation mode support with
padding detection.

No functional changes are introduced.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/misc/aspeed_hace.c | 111 ++++++++++++++++++++++++------------------
 1 file changed, 63 insertions(+), 48 deletions(-)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 42c6f29f82..22eea62693 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -167,6 +167,67 @@ static int hash_prepare_direct_iov(AspeedHACEState *s, struct iovec *iov)
 
     return iov_idx;
 }
+
+static int hash_prepare_sg_iov(AspeedHACEState *s, struct iovec *iov,
+                               bool acc_mode, bool *acc_final_request)
+{
+    uint32_t total_msg_len;
+    uint32_t pad_offset;
+    uint32_t len = 0;
+    uint32_t sg_addr;
+    uint32_t src;
+    int iov_idx;
+    hwaddr plen;
+    void *haddr;
+
+    for (iov_idx = 0; !(len & SG_LIST_LEN_LAST); iov_idx++) {
+        if (iov_idx == ASPEED_HACE_MAX_SG) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: Failed to set end of sg list marker\n",
+                          __func__);
+            return -1;
+        }
+
+        src = s->regs[R_HASH_SRC] + (iov_idx * SG_LIST_ENTRY_SIZE);
+
+        len = address_space_ldl_le(&s->dram_as, src,
+                                   MEMTXATTRS_UNSPECIFIED, NULL);
+        sg_addr = address_space_ldl_le(&s->dram_as, src + SG_LIST_LEN_SIZE,
+                                       MEMTXATTRS_UNSPECIFIED, NULL);
+        sg_addr &= SG_LIST_ADDR_MASK;
+
+        plen = len & SG_LIST_LEN_MASK;
+        haddr = address_space_map(&s->dram_as, sg_addr, &plen, false,
+                                  MEMTXATTRS_UNSPECIFIED);
+
+        if (haddr == NULL) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: Unable to map address, sg_addr=0x%x, "
+                          "plen=0x%" HWADDR_PRIx "\n",
+                          __func__, sg_addr, plen);
+            return -1;
+        }
+
+        iov[iov_idx].iov_base = haddr;
+        if (acc_mode) {
+            s->total_req_len += plen;
+
+            if (has_padding(s, &iov[iov_idx], plen, &total_msg_len,
+                            &pad_offset)) {
+                /* Padding being present indicates the final request */
+                *acc_final_request = true;
+                iov[iov_idx].iov_len = pad_offset;
+            } else {
+                iov[iov_idx].iov_len = plen;
+            }
+        } else {
+            iov[iov_idx].iov_len = plen;
+        }
+    }
+
+    return iov_idx;
+}
+
 static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
                               bool acc_mode)
 {
@@ -174,15 +235,8 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
     struct iovec iov[ASPEED_HACE_MAX_SG];
     bool acc_final_request = false;
     Error *local_err = NULL;
-    uint32_t total_msg_len;
     size_t digest_len = 0;
-    uint32_t sg_addr = 0;
-    uint32_t pad_offset;
-    int iov_idx = 0;
-    uint32_t len = 0;
-    uint32_t src = 0;
-    void *haddr;
-    hwaddr plen;
+    int iov_idx = -1;
 
     if (acc_mode && s->hash_ctx == NULL) {
         s->hash_ctx = qcrypto_hash_new(algo, &local_err);
@@ -196,46 +250,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
 
     /* Prepares the iov for hashing operations based on the selected mode */
     if (sg_mode) {
-        for (iov_idx = 0; !(len & SG_LIST_LEN_LAST); iov_idx++) {
-            if (iov_idx == ASPEED_HACE_MAX_SG) {
-                qemu_log_mask(LOG_GUEST_ERROR,
-                        "aspeed_hace: guest failed to set end of sg list marker\n");
-                break;
-            }
-
-            src = s->regs[R_HASH_SRC] + (iov_idx * SG_LIST_ENTRY_SIZE);
-
-            len = address_space_ldl_le(&s->dram_as, src,
-                                       MEMTXATTRS_UNSPECIFIED, NULL);
-
-            sg_addr = address_space_ldl_le(&s->dram_as, src + SG_LIST_LEN_SIZE,
-                                           MEMTXATTRS_UNSPECIFIED, NULL);
-            sg_addr &= SG_LIST_ADDR_MASK;
-
-            plen = len & SG_LIST_LEN_MASK;
-            haddr = address_space_map(&s->dram_as, sg_addr, &plen, false,
-                                      MEMTXATTRS_UNSPECIFIED);
-            if (haddr == NULL) {
-                qemu_log_mask(LOG_GUEST_ERROR,
-                              "%s: qcrypto failed\n", __func__);
-                return;
-            }
-            iov[iov_idx].iov_base = haddr;
-            if (acc_mode) {
-                s->total_req_len += plen;
-
-                if (has_padding(s, &iov[iov_idx], plen, &total_msg_len,
-                                &pad_offset)) {
-                    /* Padding being present indicates the final request */
-                    acc_final_request = true;
-                    iov[iov_idx].iov_len = pad_offset;
-                } else {
-                    iov[iov_idx].iov_len = plen;
-                }
-            } else {
-                iov[iov_idx].iov_len = plen;
-            }
-        }
+        iov_idx = hash_prepare_sg_iov(s, iov, acc_mode, &acc_final_request);
     } else {
         iov_idx = hash_prepare_direct_iov(s, iov);
     }
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 06/28] hw/misc/aspeed_hace: Extract digest write and iov unmap into helper function
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (4 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 05/28] hw/misc/aspeed_hace: Extract SG-mode " Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 07/28] hw/misc/aspeed_hace: Extract non-accumulation hash execution " Jamin Lin via
                   ` (25 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

To improve code readability and maintainability of do_hash_operation(), this
commit introduces a new helper function: hash_write_digest_and_unmap_iov().

The helper consolidates the final digest writeback and subsequent unmapping of
the I/O vectors into a single routine.

No functional changes are introduced.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/misc/aspeed_hace.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 22eea62693..7da781f864 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -228,6 +228,26 @@ static int hash_prepare_sg_iov(AspeedHACEState *s, struct iovec *iov,
     return iov_idx;
 }
 
+static void hash_write_digest_and_unmap_iov(AspeedHACEState *s,
+                                            struct iovec *iov,
+                                            int iov_idx,
+                                            uint8_t *digest_buf,
+                                            size_t digest_len)
+{
+    if (address_space_write(&s->dram_as, s->regs[R_HASH_DEST],
+                            MEMTXATTRS_UNSPECIFIED, digest_buf, digest_len)) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Failed to write digest to 0x%x\n",
+                      __func__, s->regs[R_HASH_DEST]);
+    }
+
+    for (; iov_idx > 0; iov_idx--) {
+        address_space_unmap(&s->dram_as, iov[iov_idx - 1].iov_base,
+                            iov[iov_idx - 1].iov_len, false,
+                            iov[iov_idx - 1].iov_len);
+    }
+}
+
 static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
                               bool acc_mode)
 {
@@ -292,18 +312,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
         return;
     }
 
-    if (address_space_write(&s->dram_as, s->regs[R_HASH_DEST],
-                            MEMTXATTRS_UNSPECIFIED,
-                            digest_buf, digest_len)) {
-        qemu_log_mask(LOG_GUEST_ERROR,
-                      "aspeed_hace: address space write failed\n");
-    }
-
-    for (; iov_idx > 0; iov_idx--) {
-        address_space_unmap(&s->dram_as, iov[iov_idx - 1].iov_base,
-                            iov[iov_idx - 1].iov_len, false,
-                            iov[iov_idx - 1].iov_len);
-    }
+    hash_write_digest_and_unmap_iov(s, iov, iov_idx, digest_buf, digest_len);
 }
 
 static uint64_t aspeed_hace_read(void *opaque, hwaddr addr, unsigned int size)
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 07/28] hw/misc/aspeed_hace: Extract non-accumulation hash execution into helper function
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (5 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 06/28] hw/misc/aspeed_hace: Extract digest write and iov unmap " Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 08/28] hw/misc/aspeed_hace: Extract accumulation-mode " Jamin Lin via
                   ` (24 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

To improve code readability and maintainability of do_hash_operation(), this
commit introduces a new helper function: hash_execute_non_acc_mode().

The helper encapsulate the hashing logic for non-accumulation mode.

No functional changes are introduced.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/misc/aspeed_hace.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 7da781f864..c50e228cdf 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -248,6 +248,25 @@ static void hash_write_digest_and_unmap_iov(AspeedHACEState *s,
     }
 }
 
+static void hash_execute_non_acc_mode(AspeedHACEState *s, int algo,
+                                      struct iovec *iov, int iov_idx)
+{
+    g_autofree uint8_t *digest_buf = NULL;
+    Error *local_err = NULL;
+    size_t digest_len = 0;
+
+    if (qcrypto_hash_bytesv(algo, iov, iov_idx, &digest_buf,
+                            &digest_len, &local_err) < 0) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: qcrypto hash bytesv failed : %s",
+                      __func__, error_get_pretty(local_err));
+        error_free(local_err);
+        return;
+    }
+
+    hash_write_digest_and_unmap_iov(s, iov, iov_idx, digest_buf, digest_len);
+}
+
 static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
                               bool acc_mode)
 {
@@ -304,15 +323,12 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
             s->hash_ctx = NULL;
             s->total_req_len = 0;
         }
-    } else if (qcrypto_hash_bytesv(algo, iov, iov_idx, &digest_buf,
-                                   &digest_len, &local_err) < 0) {
-        qemu_log_mask(LOG_GUEST_ERROR, "qcrypto hash bytesv failed : %s",
-                      error_get_pretty(local_err));
-        error_free(local_err);
-        return;
-    }
 
-    hash_write_digest_and_unmap_iov(s, iov, iov_idx, digest_buf, digest_len);
+        hash_write_digest_and_unmap_iov(s, iov, iov_idx, digest_buf,
+                                        digest_len);
+    } else {
+        hash_execute_non_acc_mode(s, algo, iov, iov_idx);
+    }
 }
 
 static uint64_t aspeed_hace_read(void *opaque, hwaddr addr, unsigned int size)
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 08/28] hw/misc/aspeed_hace: Extract accumulation-mode hash execution into helper function
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (6 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 07/28] hw/misc/aspeed_hace: Extract non-accumulation hash execution " Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 09/28] hw/misc/aspeed_hace: Introduce 64-bit hash source address " Jamin Lin via
                   ` (23 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

To improve code readability and maintainability of do_hash_operation(), this
commit introduces a new helper function: hash_execute_acc_mode().

This function encapsulates the full flow for accumulation mode, including
context initialization, update, conditional finalization, and digest writeback
with I/O vector unmapping.

No functional changes are introduced.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/misc/aspeed_hace.c | 74 ++++++++++++++++++++++++-------------------
 1 file changed, 41 insertions(+), 33 deletions(-)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index c50e228cdf..33e13974fe 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -267,26 +267,57 @@ static void hash_execute_non_acc_mode(AspeedHACEState *s, int algo,
     hash_write_digest_and_unmap_iov(s, iov, iov_idx, digest_buf, digest_len);
 }
 
-static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
-                              bool acc_mode)
+static void hash_execute_acc_mode(AspeedHACEState *s, int algo,
+                                  struct iovec *iov, int iov_idx,
+                                  bool final_request)
 {
     g_autofree uint8_t *digest_buf = NULL;
-    struct iovec iov[ASPEED_HACE_MAX_SG];
-    bool acc_final_request = false;
     Error *local_err = NULL;
     size_t digest_len = 0;
-    int iov_idx = -1;
 
-    if (acc_mode && s->hash_ctx == NULL) {
+    if (s->hash_ctx == NULL) {
         s->hash_ctx = qcrypto_hash_new(algo, &local_err);
         if (s->hash_ctx == NULL) {
-            qemu_log_mask(LOG_GUEST_ERROR, "qcrypto hash failed : %s",
-                          error_get_pretty(local_err));
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto hash new failed : %s",
+                          __func__, error_get_pretty(local_err));
             error_free(local_err);
             return;
         }
     }
 
+    if (qcrypto_hash_updatev(s->hash_ctx, iov, iov_idx, &local_err) < 0) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto hash updatev failed : %s",
+                      __func__, error_get_pretty(local_err));
+        error_free(local_err);
+        return;
+    }
+
+    if (final_request) {
+        if (qcrypto_hash_finalize_bytes(s->hash_ctx, &digest_buf,
+                                        &digest_len, &local_err)) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: qcrypto hash finalize bytes failed : %s",
+                          __func__, error_get_pretty(local_err));
+            error_free(local_err);
+            local_err = NULL;
+        }
+
+        qcrypto_hash_free(s->hash_ctx);
+
+        s->hash_ctx = NULL;
+        s->total_req_len = 0;
+    }
+
+    hash_write_digest_and_unmap_iov(s, iov, iov_idx, digest_buf, digest_len);
+}
+
+static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
+                              bool acc_mode)
+{
+    struct iovec iov[ASPEED_HACE_MAX_SG];
+    bool acc_final_request = false;
+    int iov_idx = -1;
+
     /* Prepares the iov for hashing operations based on the selected mode */
     if (sg_mode) {
         iov_idx = hash_prepare_sg_iov(s, iov, acc_mode, &acc_final_request);
@@ -300,32 +331,9 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
          return;
     }
 
+    /* Executes the hash operation */
     if (acc_mode) {
-        if (qcrypto_hash_updatev(s->hash_ctx, iov, iov_idx, &local_err) < 0) {
-            qemu_log_mask(LOG_GUEST_ERROR, "qcrypto hash update failed : %s",
-                          error_get_pretty(local_err));
-            error_free(local_err);
-            return;
-        }
-
-        if (acc_final_request) {
-            if (qcrypto_hash_finalize_bytes(s->hash_ctx, &digest_buf,
-                                            &digest_len, &local_err)) {
-                qemu_log_mask(LOG_GUEST_ERROR,
-                              "qcrypto hash finalize failed : %s",
-                              error_get_pretty(local_err));
-                error_free(local_err);
-                local_err = NULL;
-            }
-
-            qcrypto_hash_free(s->hash_ctx);
-
-            s->hash_ctx = NULL;
-            s->total_req_len = 0;
-        }
-
-        hash_write_digest_and_unmap_iov(s, iov, iov_idx, digest_buf,
-                                        digest_len);
+        hash_execute_acc_mode(s, algo, iov, iov_idx, acc_final_request);
     } else {
         hash_execute_non_acc_mode(s, algo, iov, iov_idx);
     }
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 09/28] hw/misc/aspeed_hace: Introduce 64-bit hash source address helper function
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (7 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 08/28] hw/misc/aspeed_hace: Extract accumulation-mode " Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 10/28] hw/misc/aspeed_hace: Rename R_HASH_DEST to R_HASH_DIGEST and introduce 64-bit hash digest address helper Jamin Lin via
                   ` (22 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

The AST2700 CPU, based on the Cortex-A35, is a 64-bit processor, and its DRAM
address space is also 64-bit. To support future AST2700 updates, the source
hash buffer address data type is being updated to 64-bit.

Introduces the "hash_get_source_addr()" helper function to extract the source hash
buffer address.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/misc/aspeed_hace.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 33e13974fe..b3c3af51fa 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -142,21 +142,30 @@ static bool has_padding(AspeedHACEState *s, struct iovec *iov,
     return false;
 }
 
+static uint64_t hash_get_source_addr(AspeedHACEState *s)
+{
+    uint64_t src_addr = 0;
+
+    src_addr = deposit64(src_addr, 0, 32, s->regs[R_HASH_SRC]);
+
+    return src_addr;
+}
+
 static int hash_prepare_direct_iov(AspeedHACEState *s, struct iovec *iov)
 {
-    uint32_t src;
+    uint64_t src;
     void *haddr;
     hwaddr plen;
     int iov_idx;
 
     plen = s->regs[R_HASH_SRC_LEN];
-    src = s->regs[R_HASH_SRC];
+    src = hash_get_source_addr(s);
     haddr = address_space_map(&s->dram_as, src, &plen, false,
                               MEMTXATTRS_UNSPECIFIED);
     if (haddr == NULL) {
         qemu_log_mask(LOG_GUEST_ERROR,
-                      "%s: Unable to map address, addr=0x%x, "
-                      "plen=0x%" HWADDR_PRIx "\n",
+                      "%s: Unable to map address, addr=0x%" HWADDR_PRIx
+                      " ,plen=0x%" HWADDR_PRIx "\n",
                       __func__, src, plen);
         return -1;
     }
@@ -175,11 +184,12 @@ static int hash_prepare_sg_iov(AspeedHACEState *s, struct iovec *iov,
     uint32_t pad_offset;
     uint32_t len = 0;
     uint32_t sg_addr;
-    uint32_t src;
+    uint64_t src;
     int iov_idx;
     hwaddr plen;
     void *haddr;
 
+    src = hash_get_source_addr(s);
     for (iov_idx = 0; !(len & SG_LIST_LEN_LAST); iov_idx++) {
         if (iov_idx == ASPEED_HACE_MAX_SG) {
             qemu_log_mask(LOG_GUEST_ERROR,
@@ -188,8 +198,6 @@ static int hash_prepare_sg_iov(AspeedHACEState *s, struct iovec *iov,
             return -1;
         }
 
-        src = s->regs[R_HASH_SRC] + (iov_idx * SG_LIST_ENTRY_SIZE);
-
         len = address_space_ldl_le(&s->dram_as, src,
                                    MEMTXATTRS_UNSPECIFIED, NULL);
         sg_addr = address_space_ldl_le(&s->dram_as, src + SG_LIST_LEN_SIZE,
@@ -208,6 +216,8 @@ static int hash_prepare_sg_iov(AspeedHACEState *s, struct iovec *iov,
             return -1;
         }
 
+        src += SG_LIST_ENTRY_SIZE;
+
         iov[iov_idx].iov_base = haddr;
         if (acc_mode) {
             s->total_req_len += plen;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 10/28] hw/misc/aspeed_hace: Rename R_HASH_DEST to R_HASH_DIGEST and introduce 64-bit hash digest address helper
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (8 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 09/28] hw/misc/aspeed_hace: Introduce 64-bit hash source address " Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 11/28] hw/misc/aspeed_hace: Support accumulative mode for direct access mode Jamin Lin via
                   ` (21 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

Renaming R_HASH_DEST to R_HASH_DIGEST for better semantic clarity.

The AST2700 CPU, based on the Cortex-A35, features a 64-bit DRAM address space.
To prepare for future AST2700 support, this change introduces a new helper
function hash_get_digest_addr() to encapsulate digest address extraction logic
and improve code readability.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/misc/aspeed_hace.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index b3c3af51fa..62649b5b27 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -27,7 +27,7 @@
 #define TAG_IRQ         BIT(15)
 
 #define R_HASH_SRC      (0x20 / 4)
-#define R_HASH_DEST     (0x24 / 4)
+#define R_HASH_DIGEST   (0x24 / 4)
 #define R_HASH_KEY_BUFF (0x28 / 4)
 #define R_HASH_SRC_LEN  (0x2c / 4)
 
@@ -238,17 +238,30 @@ static int hash_prepare_sg_iov(AspeedHACEState *s, struct iovec *iov,
     return iov_idx;
 }
 
+static uint64_t hash_get_digest_addr(AspeedHACEState *s)
+{
+    uint64_t digest_addr = 0;
+
+    digest_addr = deposit64(digest_addr, 0, 32, s->regs[R_HASH_DIGEST]);
+
+    return digest_addr;
+}
+
 static void hash_write_digest_and_unmap_iov(AspeedHACEState *s,
                                             struct iovec *iov,
                                             int iov_idx,
                                             uint8_t *digest_buf,
                                             size_t digest_len)
 {
-    if (address_space_write(&s->dram_as, s->regs[R_HASH_DEST],
-                            MEMTXATTRS_UNSPECIFIED, digest_buf, digest_len)) {
+    uint64_t digest_addr = 0;
+
+    digest_addr = hash_get_digest_addr(s);
+    if (address_space_write(&s->dram_as, digest_addr,
+                            MEMTXATTRS_UNSPECIFIED,
+                            digest_buf, digest_len)) {
         qemu_log_mask(LOG_GUEST_ERROR,
-                      "%s: Failed to write digest to 0x%x\n",
-                      __func__, s->regs[R_HASH_DEST]);
+                      "%s: Failed to write digest to 0x%" HWADDR_PRIx "\n",
+                      __func__, digest_addr);
     }
 
     for (; iov_idx > 0; iov_idx--) {
@@ -402,7 +415,7 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
     case R_HASH_SRC:
         data &= ahc->src_mask;
         break;
-    case R_HASH_DEST:
+    case R_HASH_DIGEST:
         data &= ahc->dest_mask;
         break;
     case R_HASH_KEY_BUFF:
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 11/28] hw/misc/aspeed_hace: Support accumulative mode for direct access mode
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (9 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 10/28] hw/misc/aspeed_hace: Rename R_HASH_DEST to R_HASH_DIGEST and introduce 64-bit hash digest address helper Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 12/28] hw/misc/aspeed_hace: Move register size to instance class and dynamically allocate regs Jamin Lin via
                   ` (20 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

Enable accumulative mode for direct access mode operations. In direct access
mode, only a single source buffer is used, so the "iovec" count is set to 1.
If "acc_mode" is enabled:
1. Accumulate "total_req_len" with the current request length ("plen").
2. Check for padding and determine whether this is the final request.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/misc/aspeed_hace.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 62649b5b27..049f732f99 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -151,8 +151,11 @@ static uint64_t hash_get_source_addr(AspeedHACEState *s)
     return src_addr;
 }
 
-static int hash_prepare_direct_iov(AspeedHACEState *s, struct iovec *iov)
+static int hash_prepare_direct_iov(AspeedHACEState *s, struct iovec *iov,
+                                   bool acc_mode, bool *acc_final_request)
 {
+    uint32_t total_msg_len;
+    uint32_t pad_offset;
     uint64_t src;
     void *haddr;
     hwaddr plen;
@@ -171,9 +174,23 @@ static int hash_prepare_direct_iov(AspeedHACEState *s, struct iovec *iov)
     }
 
     iov[0].iov_base = haddr;
-    iov[0].iov_len = plen;
     iov_idx = 1;
 
+    if (acc_mode) {
+        s->total_req_len += plen;
+
+        if (has_padding(s, &iov[0], plen, &total_msg_len,
+                        &pad_offset)) {
+            /* Padding being present indicates the final request */
+            *acc_final_request = true;
+            iov[0].iov_len = pad_offset;
+        } else {
+            iov[0].iov_len = plen;
+        }
+    } else {
+        iov[0].iov_len = plen;
+    }
+
     return iov_idx;
 }
 
@@ -345,7 +362,8 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
     if (sg_mode) {
         iov_idx = hash_prepare_sg_iov(s, iov, acc_mode, &acc_final_request);
     } else {
-        iov_idx = hash_prepare_direct_iov(s, iov);
+        iov_idx = hash_prepare_direct_iov(s, iov, acc_mode,
+                                          &acc_final_request);
     }
 
     if (iov_idx <= 0) {
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 12/28] hw/misc/aspeed_hace: Move register size to instance class and dynamically allocate regs
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (10 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 11/28] hw/misc/aspeed_hace: Support accumulative mode for direct access mode Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 13/28] hw/misc/aspeed_hace: Add support for source, digest, key buffer 64 bit addresses Jamin Lin via
                   ` (19 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

Dynamically allocate the register array by removing the hardcoded
ASPEED_HACE_NR_REGS macro.

To support different register sizes across SoC variants, introduce a new
"nr_regs" class attribute and replace the static "regs" array with dynamically
allocated memory.

Add a new "aspeed_hace_unrealize" function to properly free the allocated "regs"
memory during device cleanup.

Remove the bounds checking in the MMIO read/write handlers since the
MemoryRegion size now matches the (register array size << 2).

This commit updates the VMState fields accordingly. The VMState version was
already bumped in a previous patch of this series, so no further version change
is needed.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/misc/aspeed_hace.h |  5 +++--
 hw/misc/aspeed_hace.c         | 36 ++++++++++++++++++-----------------
 2 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h
index b69a038d35..f30d606559 100644
--- a/include/hw/misc/aspeed_hace.h
+++ b/include/hw/misc/aspeed_hace.h
@@ -22,7 +22,6 @@
 
 OBJECT_DECLARE_TYPE(AspeedHACEState, AspeedHACEClass, ASPEED_HACE)
 
-#define ASPEED_HACE_NR_REGS (0x64 >> 2)
 #define ASPEED_HACE_MAX_SG  256 /* max number of entries */
 
 struct AspeedHACEState {
@@ -31,7 +30,7 @@ struct AspeedHACEState {
     MemoryRegion iomem;
     qemu_irq irq;
 
-    uint32_t regs[ASPEED_HACE_NR_REGS];
+    uint32_t *regs;
     uint32_t total_req_len;
 
     MemoryRegion *dram_mr;
@@ -44,10 +43,12 @@ struct AspeedHACEState {
 struct AspeedHACEClass {
     SysBusDeviceClass parent_class;
 
+    const MemoryRegionOps *reg_ops;
     uint32_t src_mask;
     uint32_t dest_mask;
     uint32_t key_mask;
     uint32_t hash_mask;
+    uint64_t nr_regs;
     bool raise_crypt_interrupt_workaround;
 };
 
diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 049f732f99..fef63eb488 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -386,13 +386,6 @@ static uint64_t aspeed_hace_read(void *opaque, hwaddr addr, unsigned int size)
 
     addr >>= 2;
 
-    if (addr >= ASPEED_HACE_NR_REGS) {
-        qemu_log_mask(LOG_GUEST_ERROR,
-                      "%s: Out-of-bounds read at offset 0x%" HWADDR_PRIx "\n",
-                      __func__, addr << 2);
-        return 0;
-    }
-
     return s->regs[addr];
 }
 
@@ -404,13 +397,6 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
 
     addr >>= 2;
 
-    if (addr >= ASPEED_HACE_NR_REGS) {
-        qemu_log_mask(LOG_GUEST_ERROR,
-                      "%s: Out-of-bounds write at offset 0x%" HWADDR_PRIx "\n",
-                      __func__, addr << 2);
-        return;
-    }
-
     switch (addr) {
     case R_STATUS:
         if (data & HASH_IRQ) {
@@ -507,13 +493,14 @@ static const MemoryRegionOps aspeed_hace_ops = {
 static void aspeed_hace_reset(DeviceState *dev)
 {
     struct AspeedHACEState *s = ASPEED_HACE(dev);
+    AspeedHACEClass *ahc = ASPEED_HACE_GET_CLASS(s);
 
     if (s->hash_ctx != NULL) {
         qcrypto_hash_free(s->hash_ctx);
         s->hash_ctx = NULL;
     }
 
-    memset(s->regs, 0, sizeof(s->regs));
+    memset(s->regs, 0, ahc->nr_regs << 2);
     s->total_req_len = 0;
 }
 
@@ -521,11 +508,13 @@ static void aspeed_hace_realize(DeviceState *dev, Error **errp)
 {
     AspeedHACEState *s = ASPEED_HACE(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    AspeedHACEClass *ahc = ASPEED_HACE_GET_CLASS(s);
 
     sysbus_init_irq(sbd, &s->irq);
 
+    s->regs = g_new(uint32_t, ahc->nr_regs);
     memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_hace_ops, s,
-            TYPE_ASPEED_HACE, 0x1000);
+                          TYPE_ASPEED_HACE, ahc->nr_regs << 2);
 
     if (!s->dram_mr) {
         error_setg(errp, TYPE_ASPEED_HACE ": 'dram' link not set");
@@ -548,17 +537,25 @@ static const VMStateDescription vmstate_aspeed_hace = {
     .version_id = 2,
     .minimum_version_id = 2,
     .fields = (const VMStateField[]) {
-        VMSTATE_UINT32_ARRAY(regs, AspeedHACEState, ASPEED_HACE_NR_REGS),
         VMSTATE_UINT32(total_req_len, AspeedHACEState),
         VMSTATE_END_OF_LIST(),
     }
 };
 
+static void aspeed_hace_unrealize(DeviceState *dev)
+{
+    AspeedHACEState *s = ASPEED_HACE(dev);
+
+    g_free(s->regs);
+    s->regs = NULL;
+}
+
 static void aspeed_hace_class_init(ObjectClass *klass, const void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = aspeed_hace_realize;
+    dc->unrealize = aspeed_hace_unrealize;
     device_class_set_legacy_reset(dc, aspeed_hace_reset);
     device_class_set_props(dc, aspeed_hace_properties);
     dc->vmsd = &vmstate_aspeed_hace;
@@ -579,6 +576,7 @@ static void aspeed_ast2400_hace_class_init(ObjectClass *klass, const void *data)
 
     dc->desc = "AST2400 Hash and Crypto Engine";
 
+    ahc->nr_regs = 0x64 >> 2;
     ahc->src_mask = 0x0FFFFFFF;
     ahc->dest_mask = 0x0FFFFFF8;
     ahc->key_mask = 0x0FFFFFC0;
@@ -598,6 +596,7 @@ static void aspeed_ast2500_hace_class_init(ObjectClass *klass, const void *data)
 
     dc->desc = "AST2500 Hash and Crypto Engine";
 
+    ahc->nr_regs = 0x64 >> 2;
     ahc->src_mask = 0x3fffffff;
     ahc->dest_mask = 0x3ffffff8;
     ahc->key_mask = 0x3FFFFFC0;
@@ -617,6 +616,7 @@ static void aspeed_ast2600_hace_class_init(ObjectClass *klass, const void *data)
 
     dc->desc = "AST2600 Hash and Crypto Engine";
 
+    ahc->nr_regs = 0x64 >> 2;
     ahc->src_mask = 0x7FFFFFFF;
     ahc->dest_mask = 0x7FFFFFF8;
     ahc->key_mask = 0x7FFFFFF8;
@@ -636,6 +636,7 @@ static void aspeed_ast1030_hace_class_init(ObjectClass *klass, const void *data)
 
     dc->desc = "AST1030 Hash and Crypto Engine";
 
+    ahc->nr_regs = 0x64 >> 2;
     ahc->src_mask = 0x7FFFFFFF;
     ahc->dest_mask = 0x7FFFFFF8;
     ahc->key_mask = 0x7FFFFFF8;
@@ -655,6 +656,7 @@ static void aspeed_ast2700_hace_class_init(ObjectClass *klass, const void *data)
 
     dc->desc = "AST2700 Hash and Crypto Engine";
 
+    ahc->nr_regs = 0x64 >> 2;
     ahc->src_mask = 0x7FFFFFFF;
     ahc->dest_mask = 0x7FFFFFF8;
     ahc->key_mask = 0x7FFFFFF8;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 13/28] hw/misc/aspeed_hace: Add support for source, digest, key buffer 64 bit addresses
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (11 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 12/28] hw/misc/aspeed_hace: Move register size to instance class and dynamically allocate regs Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 14/28] hw/misc/aspeed_hace: Support DMA 64 bits dram address Jamin Lin via
                   ` (18 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

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_DIGEST_HI storing bits
[63:32] and R_HASH_DIGEST storing bits [31:0]. The HMAC key buffer address is also
64-bit, with R_HASH_KEY_BUFF_HI storing bits [63:32] and R_HASH_KEY_BUFF storing
bits [31:0].

The AST2700 supports a maximum DRAM size of 8 GB, with a DRAM addressable range
from 0x0_0000_0000 to 0x1_FFFF_FFFF. Since this range fits within 34 bits, only
bits [33:0] are needed to store the DRAM offset. To optimize address storage,
the high physical address bits [1:0] of the source, digest and key buffer
addresses are stored as dram_offset bits [33:32].

To achieve this, a src_hi_mask with a mask value of 0x3 is introduced, ensuring
that src_addr_hi consists of bits [1:0]. The final src_addr is computed as
(src_addr_hi[1:0] << 32) | src_addr[31:0], representing the DRAM offset within
bits [33:0].

Similarly, a dest_hi_mask with a mask value of 0x3 is introduced to ensure that
dest_addr_hi consists of bits [1:0]. The final dest_addr is calculated as
(dest_addr_hi[1:0] << 32) | dest_addr[31:0], representing the DRAM offset within
bits [33:0].

Additionally, a key_hi_mask with a mask value of 0x3 is introduced to ensure
that key_buf_addr_hi consists of bits [1:0]. The final key_buf_addr is
determined as (key_buf_addr_hi[1:0] << 32) | key_buf_addr[31:0], representing
the DRAM offset within bits [33:0].

This approach eliminates the need to reduce the high part of the DRAM physical
address for DMA operations. Previously, this was calculated as
(high physical address bits [7:0] - 4), since the DRAM start address is
0x4_00000000, making the high part address [7:0] - 4.

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

diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h
index f30d606559..9945b61863 100644
--- a/include/hw/misc/aspeed_hace.h
+++ b/include/hw/misc/aspeed_hace.h
@@ -50,6 +50,9 @@ struct AspeedHACEClass {
     uint32_t hash_mask;
     uint64_t nr_regs;
     bool raise_crypt_interrupt_workaround;
+    uint32_t src_hi_mask;
+    uint32_t dest_hi_mask;
+    uint32_t key_hi_mask;
 };
 
 #endif /* ASPEED_HACE_H */
diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index fef63eb488..d58645cabd 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -30,6 +30,9 @@
 #define R_HASH_DIGEST   (0x24 / 4)
 #define R_HASH_KEY_BUFF (0x28 / 4)
 #define R_HASH_SRC_LEN  (0x2c / 4)
+#define R_HASH_SRC_HI       (0x90 / 4)
+#define R_HASH_DIGEST_HI    (0x94 / 4)
+#define R_HASH_KEY_BUFF_HI  (0x98 / 4)
 
 #define R_HASH_CMD      (0x30 / 4)
 /* Hash algorithm selection */
@@ -473,6 +476,15 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
             }
         }
         break;
+    case R_HASH_SRC_HI:
+        data &= ahc->src_hi_mask;
+        break;
+    case R_HASH_DIGEST_HI:
+        data &= ahc->dest_hi_mask;
+        break;
+    case R_HASH_KEY_BUFF_HI:
+        data &= ahc->key_hi_mask;
+        break;
     default:
         break;
     }
@@ -656,12 +668,29 @@ static void aspeed_ast2700_hace_class_init(ObjectClass *klass, const void *data)
 
     dc->desc = "AST2700 Hash and Crypto Engine";
 
-    ahc->nr_regs = 0x64 >> 2;
+    ahc->nr_regs = 0x9C >> 2;
     ahc->src_mask = 0x7FFFFFFF;
     ahc->dest_mask = 0x7FFFFFF8;
     ahc->key_mask = 0x7FFFFFF8;
     ahc->hash_mask = 0x00147FFF;
 
+    /*
+     * The AST2700 supports a maximum DRAM size of 8 GB, with a DRAM
+     * addressable range from 0x0_0000_0000 to 0x1_FFFF_FFFF. Since this range
+     * fits within 34 bits, only bits [33:0] are needed to store the DRAM
+     * offset. To optimize address storage, the high physical address bits
+     * [1:0] of the source, digest and key buffer addresses are stored as
+     * dram_offset bits [33:32].
+     *
+     * This approach eliminates the need to reduce the high part of the DRAM
+     * physical address for DMA operations. Previously, this was calculated as
+     * (high physical address bits [7:0] - 4), since the DRAM start address is
+     * 0x4_00000000, making the high part address [7:0] - 4.
+     */
+    ahc->src_hi_mask = 0x00000003;
+    ahc->dest_hi_mask = 0x00000003;
+    ahc->key_hi_mask = 0x00000003;
+
     /*
      * Currently, it does not support the CRYPT command. Instead, it only
      * sends an interrupt to notify the firmware that the crypt command
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 14/28] hw/misc/aspeed_hace: Support DMA 64 bits dram address
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (12 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 13/28] hw/misc/aspeed_hace: Add support for source, digest, key buffer 64 bit addresses Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 15/28] hw/misc/aspeed_hace: Add trace-events for better debugging Jamin Lin via
                   ` (17 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

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].

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.

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         | 17 ++++++++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h
index 9945b61863..d5d07c6c02 100644
--- a/include/hw/misc/aspeed_hace.h
+++ b/include/hw/misc/aspeed_hace.h
@@ -53,6 +53,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 d58645cabd..764408716e 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -147,9 +147,13 @@ static bool has_padding(AspeedHACEState *s, struct iovec *iov,
 
 static uint64_t hash_get_source_addr(AspeedHACEState *s)
 {
+    AspeedHACEClass *ahc = ASPEED_HACE_GET_CLASS(s);
     uint64_t src_addr = 0;
 
     src_addr = deposit64(src_addr, 0, 32, s->regs[R_HASH_SRC]);
+    if (ahc->has_dma64) {
+        src_addr = deposit64(src_addr, 32, 32, s->regs[R_HASH_SRC_HI]);
+    }
 
     return src_addr;
 }
@@ -223,7 +227,13 @@ static int hash_prepare_sg_iov(AspeedHACEState *s, struct iovec *iov,
         sg_addr = address_space_ldl_le(&s->dram_as, src + SG_LIST_LEN_SIZE,
                                        MEMTXATTRS_UNSPECIFIED, NULL);
         sg_addr &= SG_LIST_ADDR_MASK;
-
+        /*
+         * 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.
+         */
         plen = len & SG_LIST_LEN_MASK;
         haddr = address_space_map(&s->dram_as, sg_addr, &plen, false,
                                   MEMTXATTRS_UNSPECIFIED);
@@ -260,9 +270,13 @@ static int hash_prepare_sg_iov(AspeedHACEState *s, struct iovec *iov,
 
 static uint64_t hash_get_digest_addr(AspeedHACEState *s)
 {
+    AspeedHACEClass *ahc = ASPEED_HACE_GET_CLASS(s);
     uint64_t digest_addr = 0;
 
     digest_addr = deposit64(digest_addr, 0, 32, s->regs[R_HASH_DIGEST]);
+    if (ahc->has_dma64) {
+        digest_addr = deposit64(digest_addr, 32, 32, s->regs[R_HASH_DIGEST_HI]);
+    }
 
     return digest_addr;
 }
@@ -697,6 +711,7 @@ static void aspeed_ast2700_hace_class_init(ObjectClass *klass, const 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 = {
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 15/28] hw/misc/aspeed_hace: Add trace-events for better debugging
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (13 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 14/28] hw/misc/aspeed_hace: Support DMA 64 bits dram address Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 16/28] hw/misc/aspeed_hace: Support to dump plaintext and digest " Jamin Lin via
                   ` (16 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

Introduced "trace_aspeed_hace_hash_addr", "trace_aspeed_hace_hash_sg",
"trace_aspeed_hace_read", "trace_aspeed_hace_hash_execute_acc_mode",
and "trace_aspeed_hace_write" trace events.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/misc/aspeed_hace.c | 10 ++++++++++
 hw/misc/trace-events  |  7 +++++++
 2 files changed, 17 insertions(+)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 764408716e..ee1d9ab58f 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -18,6 +18,7 @@
 #include "crypto/hash.h"
 #include "hw/qdev-properties.h"
 #include "hw/irq.h"
+#include "trace.h"
 
 #define R_CRYPT_CMD     (0x10 / 4)
 
@@ -170,6 +171,7 @@ static int hash_prepare_direct_iov(AspeedHACEState *s, struct iovec *iov,
 
     plen = s->regs[R_HASH_SRC_LEN];
     src = hash_get_source_addr(s);
+    trace_aspeed_hace_hash_addr("src", src);
     haddr = address_space_map(&s->dram_as, src, &plen, false,
                               MEMTXATTRS_UNSPECIFIED);
     if (haddr == NULL) {
@@ -227,6 +229,7 @@ static int hash_prepare_sg_iov(AspeedHACEState *s, struct iovec *iov,
         sg_addr = address_space_ldl_le(&s->dram_as, src + SG_LIST_LEN_SIZE,
                                        MEMTXATTRS_UNSPECIFIED, NULL);
         sg_addr &= SG_LIST_ADDR_MASK;
+        trace_aspeed_hace_hash_sg(iov_idx, src, sg_addr, len);
         /*
          * To maintain compatibility with older SoCs such as the AST2600,
          * the AST2700 HW automatically set bit 34 of the 64-bit sg_addr.
@@ -290,6 +293,7 @@ static void hash_write_digest_and_unmap_iov(AspeedHACEState *s,
     uint64_t digest_addr = 0;
 
     digest_addr = hash_get_digest_addr(s);
+    trace_aspeed_hace_hash_addr("digest", digest_addr);
     if (address_space_write(&s->dram_as, digest_addr,
                             MEMTXATTRS_UNSPECIFIED,
                             digest_buf, digest_len)) {
@@ -332,6 +336,8 @@ static void hash_execute_acc_mode(AspeedHACEState *s, int algo,
     Error *local_err = NULL;
     size_t digest_len = 0;
 
+    trace_aspeed_hace_hash_execute_acc_mode(final_request);
+
     if (s->hash_ctx == NULL) {
         s->hash_ctx = qcrypto_hash_new(algo, &local_err);
         if (s->hash_ctx == NULL) {
@@ -403,6 +409,8 @@ static uint64_t aspeed_hace_read(void *opaque, hwaddr addr, unsigned int size)
 
     addr >>= 2;
 
+    trace_aspeed_hace_read(addr << 2, s->regs[addr]);
+
     return s->regs[addr];
 }
 
@@ -414,6 +422,8 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
 
     addr >>= 2;
 
+    trace_aspeed_hace_write(addr << 2, data);
+
     switch (addr) {
     case R_STATUS:
         if (data & HASH_IRQ) {
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 4383808d7a..b980d7fdd3 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -302,6 +302,13 @@ aspeed_peci_read(uint64_t offset, uint64_t data) "offset 0x%" PRIx64 " data 0x%"
 aspeed_peci_write(uint64_t offset, uint64_t data) "offset 0x%" PRIx64 " data 0x%" PRIx64
 aspeed_peci_raise_interrupt(uint32_t ctrl, uint32_t status) "ctrl 0x%" PRIx32 " status 0x%" PRIx32
 
+# aspeed_hace.c
+aspeed_hace_read(uint64_t offset, uint64_t data) "offset 0x%" PRIx64 " data 0x%" PRIx64
+aspeed_hace_write(uint64_t offset, uint64_t data) "offset 0x%" PRIx64 " data 0x%" PRIx64
+aspeed_hace_hash_sg(int index, uint64_t list_addr, uint64_t buf_addr, uint32_t len) "%d: list_addr 0x%" PRIx64 " buf_addr 0x%" PRIx64 " len 0x%" PRIx32
+aspeed_hace_hash_addr(const char *s, uint64_t addr) "%s: 0x%" PRIx64
+aspeed_hace_hash_execute_acc_mode(bool final_request) "final request: %d"
+
 # bcm2835_property.c
 bcm2835_mbox_property(uint32_t tag, uint32_t bufsize, size_t resplen) "mbox property tag:0x%08x in_sz:%u out_sz:%zu"
 
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 16/28] hw/misc/aspeed_hace: Support to dump plaintext and digest for better debugging
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (14 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 15/28] hw/misc/aspeed_hace: Add trace-events for better debugging Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 17/28] tests/qtest: Reorder aspeed test list Jamin Lin via
                   ` (15 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

1. Added "hace_hexdump()" to dump a contiguous buffer using qemu_hexdump.
2. Added "hace_iov_hexdump()" to flatten and dump scatter-gather source vectors.
3. Introduced a new trace event: "aspeed_hace_hexdump".

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/misc/aspeed_hace.c | 46 +++++++++++++++++++++++++++++++++++++++++++
 hw/misc/trace-events  |  1 +
 2 files changed, 47 insertions(+)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index ee1d9ab58f..8924a30eff 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -10,8 +10,10 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "qemu/log.h"
 #include "qemu/error-report.h"
+#include "qemu/iov.h"
 #include "hw/misc/aspeed_hace.h"
 #include "qapi/error.h"
 #include "migration/vmstate.h"
@@ -88,6 +90,42 @@ static const struct {
       QCRYPTO_HASH_ALGO_SHA256 },
 };
 
+static void hace_hexdump(const char *desc, const char *buf, size_t size)
+{
+    g_autoptr(GString) str = g_string_sized_new(64);
+    size_t len;
+    size_t i;
+
+    for (i = 0; i < size; i += len) {
+        len = MIN(16, size - i);
+        g_string_truncate(str, 0);
+        qemu_hexdump_line(str, buf + i, len, 1, 4);
+        trace_aspeed_hace_hexdump(desc, i, str->str);
+    }
+}
+
+static void hace_iov_hexdump(const char *desc, const struct iovec *iov,
+                             const unsigned int iov_cnt)
+{
+    size_t size = 0;
+    char *buf;
+    int i;
+
+    for (i = 0; i < iov_cnt; i++) {
+        size += iov[i].iov_len;
+    }
+
+    buf = g_malloc(size);
+
+    if (!buf) {
+        return;
+    }
+
+    iov_to_buf(iov, iov_cnt, 0, buf, size);
+    hace_hexdump(desc, buf, size);
+    g_free(buf);
+}
+
 static int hash_algo_lookup(uint32_t reg)
 {
     int i;
@@ -302,6 +340,10 @@ static void hash_write_digest_and_unmap_iov(AspeedHACEState *s,
                       __func__, digest_addr);
     }
 
+    if (trace_event_get_state_backends(TRACE_ASPEED_HACE_HEXDUMP)) {
+        hace_hexdump("digest", (char *)digest_buf, digest_len);
+    }
+
     for (; iov_idx > 0; iov_idx--) {
         address_space_unmap(&s->dram_as, iov[iov_idx - 1].iov_base,
                             iov[iov_idx - 1].iov_len, false,
@@ -395,6 +437,10 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
          return;
     }
 
+    if (trace_event_get_state_backends(TRACE_ASPEED_HACE_HEXDUMP)) {
+        hace_iov_hexdump("plaintext", iov, iov_idx);
+    }
+
     /* Executes the hash operation */
     if (acc_mode) {
         hash_execute_acc_mode(s, algo, iov, iov_idx, acc_final_request);
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index b980d7fdd3..e3f64c0ff6 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -308,6 +308,7 @@ aspeed_hace_write(uint64_t offset, uint64_t data) "offset 0x%" PRIx64 " data 0x%
 aspeed_hace_hash_sg(int index, uint64_t list_addr, uint64_t buf_addr, uint32_t len) "%d: list_addr 0x%" PRIx64 " buf_addr 0x%" PRIx64 " len 0x%" PRIx32
 aspeed_hace_hash_addr(const char *s, uint64_t addr) "%s: 0x%" PRIx64
 aspeed_hace_hash_execute_acc_mode(bool final_request) "final request: %d"
+aspeed_hace_hexdump(const char *desc, uint32_t offset, char *s) "%s: 0x%08x: %s"
 
 # bcm2835_property.c
 bcm2835_mbox_property(uint32_t tag, uint32_t bufsize, size_t resplen) "mbox property tag:0x%08x in_sz:%u out_sz:%zu"
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 17/28] tests/qtest: Reorder aspeed test list
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (15 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 16/28] hw/misc/aspeed_hace: Support to dump plaintext and digest " Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 18/28] test/qtest: Introduce a new aspeed-hace-utils.c to place common testcases Jamin Lin via
                   ` (14 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

Reordered the aspeed test list to keep the alphabetical order.
No functional changes in test behavior.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 tests/qtest/meson.build | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 3136d15e0f..6b5161a643 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -211,9 +211,9 @@ qtests_npcm7xx = \
    'npcm_gmac-test'] + \
    (slirp.found() ? ['npcm7xx_emc-test'] : [])
 qtests_aspeed = \
-  ['aspeed_hace-test',
-   'aspeed_smc-test',
-   'aspeed_gpio-test']
+  ['aspeed_gpio-test',
+   'aspeed_hace-test',
+   'aspeed_smc-test']
 qtests_aspeed64 = \
   ['ast2700-gpio-test',
    'ast2700-smc-test']
@@ -360,6 +360,8 @@ if gnutls.found()
 endif
 
 qtests = {
+  'aspeed_smc-test': files('aspeed-smc-utils.c', 'aspeed_smc-test.c'),
+  'ast2700-smc-test': files('aspeed-smc-utils.c', 'ast2700-smc-test.c'),
   'bios-tables-test': [io, 'boot-sector.c', 'acpi-utils.c', 'tpm-emu.c'],
   'cdrom-test': files('boot-sector.c'),
   'dbus-vmstate-test': files('migration/migration-qmp.c',
@@ -381,8 +383,6 @@ qtests = {
   'virtio-net-failover': migration_files,
   'vmgenid-test': files('boot-sector.c', 'acpi-utils.c'),
   'netdev-socket': files('netdev-socket.c', '../unit/socket-helpers.c'),
-  'aspeed_smc-test': files('aspeed-smc-utils.c', 'aspeed_smc-test.c'),
-  'ast2700-smc-test': files('aspeed-smc-utils.c', 'ast2700-smc-test.c'),
 }
 
 if vnc.found()
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 18/28] test/qtest: Introduce a new aspeed-hace-utils.c to place common testcases
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (16 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 17/28] tests/qtest: Reorder aspeed test list Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 19/28] test/qtest/hace: Specify explicit array sizes for test vectors and hash results Jamin Lin via
                   ` (13 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee, Cédric Le Goater

The test cases for the ASPEED HACE model were originally placed in
aspeed_hace-test.c. However, this test file only supports ARM32. To enable
compatibility with all ASPEED SoCs, including the AST2700, which uses the
AArch64 architecture, this update introduces a new source file,
aspeed-hace-utils.c.

All common APIs and test cases have been moved from aspeed_hace-test.c to
aspeed-hace-utils.c to facilitate reuse across different ASPEED SoCs.
As a result, these test cases can now be reused for AST2700 and future ASPEED
SoC testing.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
 tests/qtest/aspeed-hace-utils.h |  71 +++++
 tests/qtest/aspeed-hace-utils.c | 455 ++++++++++++++++++++++++++++
 tests/qtest/aspeed_hace-test.c  | 515 ++------------------------------
 tests/qtest/meson.build         |   1 +
 4 files changed, 547 insertions(+), 495 deletions(-)
 create mode 100644 tests/qtest/aspeed-hace-utils.h
 create mode 100644 tests/qtest/aspeed-hace-utils.c

diff --git a/tests/qtest/aspeed-hace-utils.h b/tests/qtest/aspeed-hace-utils.h
new file mode 100644
index 0000000000..598577c69b
--- /dev/null
+++ b/tests/qtest/aspeed-hace-utils.h
@@ -0,0 +1,71 @@
+/*
+ * QTest testcase for the ASPEED Hash and Crypto Engine
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright 2021 IBM Corp.
+ */
+
+#ifndef TESTS_ASPEED_HACE_UTILS_H
+#define TESTS_ASPEED_HACE_UTILS_H
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "qemu/bitops.h"
+
+#define HACE_CMD                 0x10
+#define  HACE_SHA_BE_EN          BIT(3)
+#define  HACE_MD5_LE_EN          BIT(2)
+#define  HACE_ALGO_MD5           0
+#define  HACE_ALGO_SHA1          BIT(5)
+#define  HACE_ALGO_SHA224        BIT(6)
+#define  HACE_ALGO_SHA256        (BIT(4) | BIT(6))
+#define  HACE_ALGO_SHA512        (BIT(5) | BIT(6))
+#define  HACE_ALGO_SHA384        (BIT(5) | BIT(6) | BIT(10))
+#define  HACE_SG_EN              BIT(18)
+#define  HACE_ACCUM_EN           BIT(8)
+
+#define HACE_STS                 0x1c
+#define  HACE_RSA_ISR            BIT(13)
+#define  HACE_CRYPTO_ISR         BIT(12)
+#define  HACE_HASH_ISR           BIT(9)
+#define  HACE_RSA_BUSY           BIT(2)
+#define  HACE_CRYPTO_BUSY        BIT(1)
+#define  HACE_HASH_BUSY          BIT(0)
+#define HACE_HASH_SRC            0x20
+#define HACE_HASH_DIGEST         0x24
+#define HACE_HASH_KEY_BUFF       0x28
+#define HACE_HASH_DATA_LEN       0x2c
+#define HACE_HASH_CMD            0x30
+
+/* Scatter-Gather Hash */
+#define SG_LIST_LEN_LAST         BIT(31)
+struct AspeedSgList {
+        uint32_t len;
+        uint32_t addr;
+} __attribute__ ((__packed__));
+
+struct AspeedMasks {
+    uint32_t src;
+    uint32_t dest;
+    uint32_t len;
+};
+
+void aspeed_test_md5(const char *machine, const uint32_t base,
+                     const uint32_t src_addr);
+void aspeed_test_sha256(const char *machine, const uint32_t base,
+                        const uint32_t src_addr);
+void aspeed_test_sha512(const char *machine, const uint32_t base,
+                        const uint32_t src_addr);
+void aspeed_test_sha256_sg(const char *machine, const uint32_t base,
+                           const uint32_t src_addr);
+void aspeed_test_sha512_sg(const char *machine, const uint32_t base,
+                           const uint32_t src_addr);
+void aspeed_test_sha256_accum(const char *machine, const uint32_t base,
+                              const uint32_t src_addr);
+void aspeed_test_sha512_accum(const char *machine, const uint32_t base,
+                              const uint32_t src_addr);
+void aspeed_test_addresses(const char *machine, const uint32_t base,
+                           const struct AspeedMasks *expected);
+
+#endif /* TESTS_ASPEED_HACE_UTILS_H */
+
diff --git a/tests/qtest/aspeed-hace-utils.c b/tests/qtest/aspeed-hace-utils.c
new file mode 100644
index 0000000000..8582847945
--- /dev/null
+++ b/tests/qtest/aspeed-hace-utils.c
@@ -0,0 +1,455 @@
+/*
+ * QTest testcase for the ASPEED Hash and Crypto Engine
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright 2021 IBM Corp.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "qemu/bitops.h"
+#include "aspeed-hace-utils.h"
+
+/*
+ * Test vector is the ascii "abc"
+ *
+ * Expected results were generated using command line utitiles:
+ *
+ *  echo -n -e 'abc' | dd of=/tmp/test
+ *  for hash in sha512sum sha256sum md5sum; do $hash /tmp/test; done
+ *
+ */
+static const uint8_t test_vector[] = {0x61, 0x62, 0x63};
+
+static const uint8_t test_result_sha512[] = {
+    0xdd, 0xaf, 0x35, 0xa1, 0x93, 0x61, 0x7a, 0xba, 0xcc, 0x41, 0x73, 0x49,
+    0xae, 0x20, 0x41, 0x31, 0x12, 0xe6, 0xfa, 0x4e, 0x89, 0xa9, 0x7e, 0xa2,
+    0x0a, 0x9e, 0xee, 0xe6, 0x4b, 0x55, 0xd3, 0x9a, 0x21, 0x92, 0x99, 0x2a,
+    0x27, 0x4f, 0xc1, 0xa8, 0x36, 0xba, 0x3c, 0x23, 0xa3, 0xfe, 0xeb, 0xbd,
+    0x45, 0x4d, 0x44, 0x23, 0x64, 0x3c, 0xe8, 0x0e, 0x2a, 0x9a, 0xc9, 0x4f,
+    0xa5, 0x4c, 0xa4, 0x9f};
+
+static const uint8_t test_result_sha256[] = {
+    0xba, 0x78, 0x16, 0xbf, 0x8f, 0x01, 0xcf, 0xea, 0x41, 0x41, 0x40, 0xde,
+    0x5d, 0xae, 0x22, 0x23, 0xb0, 0x03, 0x61, 0xa3, 0x96, 0x17, 0x7a, 0x9c,
+    0xb4, 0x10, 0xff, 0x61, 0xf2, 0x00, 0x15, 0xad};
+
+static const uint8_t test_result_md5[] = {
+    0x90, 0x01, 0x50, 0x98, 0x3c, 0xd2, 0x4f, 0xb0, 0xd6, 0x96, 0x3f, 0x7d,
+    0x28, 0xe1, 0x7f, 0x72};
+
+/*
+ * The Scatter-Gather Test vector is the ascii "abc" "def" "ghi", broken
+ * into blocks of 3 characters as shown
+ *
+ * Expected results were generated using command line utitiles:
+ *
+ *  echo -n -e 'abcdefghijkl' | dd of=/tmp/test
+ *  for hash in sha512sum sha256sum; do $hash /tmp/test; done
+ *
+ */
+static const uint8_t test_vector_sg1[] = {0x61, 0x62, 0x63, 0x64, 0x65, 0x66};
+static const uint8_t test_vector_sg2[] = {0x67, 0x68, 0x69};
+static const uint8_t test_vector_sg3[] = {0x6a, 0x6b, 0x6c};
+
+static const uint8_t test_result_sg_sha512[] = {
+    0x17, 0x80, 0x7c, 0x72, 0x8e, 0xe3, 0xba, 0x35, 0xe7, 0xcf, 0x7a, 0xf8,
+    0x23, 0x11, 0x6d, 0x26, 0xe4, 0x1e, 0x5d, 0x4d, 0x6c, 0x2f, 0xf1, 0xf3,
+    0x72, 0x0d, 0x3d, 0x96, 0xaa, 0xcb, 0x6f, 0x69, 0xde, 0x64, 0x2e, 0x63,
+    0xd5, 0xb7, 0x3f, 0xc3, 0x96, 0xc1, 0x2b, 0xe3, 0x8b, 0x2b, 0xd5, 0xd8,
+    0x84, 0x25, 0x7c, 0x32, 0xc8, 0xf6, 0xd0, 0x85, 0x4a, 0xe6, 0xb5, 0x40,
+    0xf8, 0x6d, 0xda, 0x2e};
+
+static const uint8_t test_result_sg_sha256[] = {
+    0xd6, 0x82, 0xed, 0x4c, 0xa4, 0xd9, 0x89, 0xc1, 0x34, 0xec, 0x94, 0xf1,
+    0x55, 0x1e, 0x1e, 0xc5, 0x80, 0xdd, 0x6d, 0x5a, 0x6e, 0xcd, 0xe9, 0xf3,
+    0xd3, 0x5e, 0x6e, 0x4a, 0x71, 0x7f, 0xbd, 0xe4};
+
+/*
+ * The accumulative mode requires firmware to provide internal initial state
+ * and message padding (including length L at the end of padding).
+ *
+ * This test vector is a ascii text "abc" with padding message.
+ *
+ * Expected results were generated using command line utitiles:
+ *
+ *  echo -n -e 'abc' | dd of=/tmp/test
+ *  for hash in sha512sum sha256sum; do $hash /tmp/test; done
+ */
+static const uint8_t test_vector_accum_512[] = {
+    0x61, 0x62, 0x63, 0x80, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18};
+
+static const uint8_t test_vector_accum_256[] = {
+    0x61, 0x62, 0x63, 0x80, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18};
+
+static const uint8_t test_result_accum_sha512[] = {
+    0xdd, 0xaf, 0x35, 0xa1, 0x93, 0x61, 0x7a, 0xba, 0xcc, 0x41, 0x73, 0x49,
+    0xae, 0x20, 0x41, 0x31, 0x12, 0xe6, 0xfa, 0x4e, 0x89, 0xa9, 0x7e, 0xa2,
+    0x0a, 0x9e, 0xee, 0xe6, 0x4b, 0x55, 0xd3, 0x9a, 0x21, 0x92, 0x99, 0x2a,
+    0x27, 0x4f, 0xc1, 0xa8, 0x36, 0xba, 0x3c, 0x23, 0xa3, 0xfe, 0xeb, 0xbd,
+    0x45, 0x4d, 0x44, 0x23, 0x64, 0x3c, 0xe8, 0x0e, 0x2a, 0x9a, 0xc9, 0x4f,
+    0xa5, 0x4c, 0xa4, 0x9f};
+
+static const uint8_t test_result_accum_sha256[] = {
+    0xba, 0x78, 0x16, 0xbf, 0x8f, 0x01, 0xcf, 0xea, 0x41, 0x41, 0x40, 0xde,
+    0x5d, 0xae, 0x22, 0x23, 0xb0, 0x03, 0x61, 0xa3, 0x96, 0x17, 0x7a, 0x9c,
+    0xb4, 0x10, 0xff, 0x61, 0xf2, 0x00, 0x15, 0xad};
+
+static void write_regs(QTestState *s, uint32_t base, uint32_t src,
+                       uint32_t length, uint32_t out, uint32_t method)
+{
+        qtest_writel(s, base + HACE_HASH_SRC, src);
+        qtest_writel(s, base + HACE_HASH_DIGEST, out);
+        qtest_writel(s, base + HACE_HASH_DATA_LEN, length);
+        qtest_writel(s, base + HACE_HASH_CMD, HACE_SHA_BE_EN | method);
+}
+
+void aspeed_test_md5(const char *machine, const uint32_t base,
+                     const uint32_t src_addr)
+
+{
+    QTestState *s = qtest_init(machine);
+
+    uint32_t digest_addr = src_addr + 0x01000000;
+    uint8_t digest[16] = {0};
+
+    /* Check engine is idle, no busy or irq bits set */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Write test vector into memory */
+    qtest_memwrite(s, src_addr, test_vector, sizeof(test_vector));
+
+    write_regs(s, base, src_addr, sizeof(test_vector),
+               digest_addr, HACE_ALGO_MD5);
+
+    /* Check hash IRQ status is asserted */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
+
+    /* Clear IRQ status and check status is deasserted */
+    qtest_writel(s, base + HACE_STS, 0x00000200);
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Read computed digest from memory */
+    qtest_memread(s, digest_addr, digest, sizeof(digest));
+
+    /* Check result of computation */
+    g_assert_cmpmem(digest, sizeof(digest),
+                    test_result_md5, sizeof(digest));
+
+    qtest_quit(s);
+}
+
+void aspeed_test_sha256(const char *machine, const uint32_t base,
+                        const uint32_t src_addr)
+{
+    QTestState *s = qtest_init(machine);
+
+    const uint32_t digest_addr = src_addr + 0x1000000;
+    uint8_t digest[32] = {0};
+
+    /* Check engine is idle, no busy or irq bits set */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Write test vector into memory */
+    qtest_memwrite(s, src_addr, test_vector, sizeof(test_vector));
+
+    write_regs(s, base, src_addr, sizeof(test_vector), digest_addr,
+               HACE_ALGO_SHA256);
+
+    /* Check hash IRQ status is asserted */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
+
+    /* Clear IRQ status and check status is deasserted */
+    qtest_writel(s, base + HACE_STS, 0x00000200);
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Read computed digest from memory */
+    qtest_memread(s, digest_addr, digest, sizeof(digest));
+
+    /* Check result of computation */
+    g_assert_cmpmem(digest, sizeof(digest),
+                    test_result_sha256, sizeof(digest));
+
+    qtest_quit(s);
+}
+
+void aspeed_test_sha512(const char *machine, const uint32_t base,
+                        const uint32_t src_addr)
+{
+    QTestState *s = qtest_init(machine);
+
+    const uint32_t digest_addr = src_addr + 0x1000000;
+    uint8_t digest[64] = {0};
+
+    /* Check engine is idle, no busy or irq bits set */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Write test vector into memory */
+    qtest_memwrite(s, src_addr, test_vector, sizeof(test_vector));
+
+    write_regs(s, base, src_addr, sizeof(test_vector), digest_addr,
+               HACE_ALGO_SHA512);
+
+    /* Check hash IRQ status is asserted */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
+
+    /* Clear IRQ status and check status is deasserted */
+    qtest_writel(s, base + HACE_STS, 0x00000200);
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Read computed digest from memory */
+    qtest_memread(s, digest_addr, digest, sizeof(digest));
+
+    /* Check result of computation */
+    g_assert_cmpmem(digest, sizeof(digest),
+                    test_result_sha512, sizeof(digest));
+
+    qtest_quit(s);
+}
+
+void aspeed_test_sha256_sg(const char *machine, const uint32_t base,
+                           const uint32_t src_addr)
+{
+    QTestState *s = qtest_init(machine);
+
+    const uint32_t src_addr_1 = src_addr + 0x1000000;
+    const uint32_t src_addr_2 = src_addr + 0x2000000;
+    const uint32_t src_addr_3 = src_addr + 0x3000000;
+    const uint32_t digest_addr = src_addr + 0x4000000;
+    uint8_t digest[32] = {0};
+    struct AspeedSgList array[] = {
+        {  cpu_to_le32(sizeof(test_vector_sg1)),
+           cpu_to_le32(src_addr_1) },
+        {  cpu_to_le32(sizeof(test_vector_sg2)),
+           cpu_to_le32(src_addr_2) },
+        {  cpu_to_le32(sizeof(test_vector_sg3) | SG_LIST_LEN_LAST),
+           cpu_to_le32(src_addr_3) },
+    };
+
+    /* Check engine is idle, no busy or irq bits set */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Write test vector into memory */
+    qtest_memwrite(s, src_addr_1, test_vector_sg1, sizeof(test_vector_sg1));
+    qtest_memwrite(s, src_addr_2, test_vector_sg2, sizeof(test_vector_sg2));
+    qtest_memwrite(s, src_addr_3, test_vector_sg3, sizeof(test_vector_sg3));
+    qtest_memwrite(s, src_addr, array, sizeof(array));
+
+    write_regs(s, base, src_addr,
+               (sizeof(test_vector_sg1)
+                + sizeof(test_vector_sg2)
+                + sizeof(test_vector_sg3)),
+               digest_addr, HACE_ALGO_SHA256 | HACE_SG_EN);
+
+    /* Check hash IRQ status is asserted */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
+
+    /* Clear IRQ status and check status is deasserted */
+    qtest_writel(s, base + HACE_STS, 0x00000200);
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Read computed digest from memory */
+    qtest_memread(s, digest_addr, digest, sizeof(digest));
+
+    /* Check result of computation */
+    g_assert_cmpmem(digest, sizeof(digest),
+                    test_result_sg_sha256, sizeof(digest));
+
+    qtest_quit(s);
+}
+
+void aspeed_test_sha512_sg(const char *machine, const uint32_t base,
+                           const uint32_t src_addr)
+{
+    QTestState *s = qtest_init(machine);
+
+    const uint32_t src_addr_1 = src_addr + 0x1000000;
+    const uint32_t src_addr_2 = src_addr + 0x2000000;
+    const uint32_t src_addr_3 = src_addr + 0x3000000;
+    const uint32_t digest_addr = src_addr + 0x4000000;
+    uint8_t digest[64] = {0};
+    struct AspeedSgList array[] = {
+        {  cpu_to_le32(sizeof(test_vector_sg1)),
+           cpu_to_le32(src_addr_1) },
+        {  cpu_to_le32(sizeof(test_vector_sg2)),
+           cpu_to_le32(src_addr_2) },
+        {  cpu_to_le32(sizeof(test_vector_sg3) | SG_LIST_LEN_LAST),
+           cpu_to_le32(src_addr_3) },
+    };
+
+    /* Check engine is idle, no busy or irq bits set */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Write test vector into memory */
+    qtest_memwrite(s, src_addr_1, test_vector_sg1, sizeof(test_vector_sg1));
+    qtest_memwrite(s, src_addr_2, test_vector_sg2, sizeof(test_vector_sg2));
+    qtest_memwrite(s, src_addr_3, test_vector_sg3, sizeof(test_vector_sg3));
+    qtest_memwrite(s, src_addr, array, sizeof(array));
+
+    write_regs(s, base, src_addr,
+               (sizeof(test_vector_sg1)
+                + sizeof(test_vector_sg2)
+                + sizeof(test_vector_sg3)),
+               digest_addr, HACE_ALGO_SHA512 | HACE_SG_EN);
+
+    /* Check hash IRQ status is asserted */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
+
+    /* Clear IRQ status and check status is deasserted */
+    qtest_writel(s, base + HACE_STS, 0x00000200);
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Read computed digest from memory */
+    qtest_memread(s, digest_addr, digest, sizeof(digest));
+
+    /* Check result of computation */
+    g_assert_cmpmem(digest, sizeof(digest),
+                    test_result_sg_sha512, sizeof(digest));
+
+    qtest_quit(s);
+}
+
+void aspeed_test_sha256_accum(const char *machine, const uint32_t base,
+                              const uint32_t src_addr)
+{
+    QTestState *s = qtest_init(machine);
+
+    const uint32_t buffer_addr = src_addr + 0x1000000;
+    const uint32_t digest_addr = src_addr + 0x4000000;
+    uint8_t digest[32] = {0};
+    struct AspeedSgList array[] = {
+        {  cpu_to_le32(sizeof(test_vector_accum_256) | SG_LIST_LEN_LAST),
+           cpu_to_le32(buffer_addr) },
+    };
+
+    /* Check engine is idle, no busy or irq bits set */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Write test vector into memory */
+    qtest_memwrite(s, buffer_addr, test_vector_accum_256,
+                   sizeof(test_vector_accum_256));
+    qtest_memwrite(s, src_addr, array, sizeof(array));
+
+    write_regs(s, base, src_addr, sizeof(test_vector_accum_256),
+               digest_addr, HACE_ALGO_SHA256 | HACE_SG_EN | HACE_ACCUM_EN);
+
+    /* Check hash IRQ status is asserted */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
+
+    /* Clear IRQ status and check status is deasserted */
+    qtest_writel(s, base + HACE_STS, 0x00000200);
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Read computed digest from memory */
+    qtest_memread(s, digest_addr, digest, sizeof(digest));
+
+    /* Check result of computation */
+    g_assert_cmpmem(digest, sizeof(digest),
+                    test_result_accum_sha256, sizeof(digest));
+
+    qtest_quit(s);
+}
+
+void aspeed_test_sha512_accum(const char *machine, const uint32_t base,
+                              const uint32_t src_addr)
+{
+    QTestState *s = qtest_init(machine);
+
+    const uint32_t buffer_addr = src_addr + 0x1000000;
+    const uint32_t digest_addr = src_addr + 0x4000000;
+    uint8_t digest[64] = {0};
+    struct AspeedSgList array[] = {
+        {  cpu_to_le32(sizeof(test_vector_accum_512) | SG_LIST_LEN_LAST),
+           cpu_to_le32(buffer_addr) },
+    };
+
+    /* Check engine is idle, no busy or irq bits set */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Write test vector into memory */
+    qtest_memwrite(s, buffer_addr, test_vector_accum_512,
+                   sizeof(test_vector_accum_512));
+    qtest_memwrite(s, src_addr, array, sizeof(array));
+
+    write_regs(s, base, src_addr, sizeof(test_vector_accum_512),
+               digest_addr, HACE_ALGO_SHA512 | HACE_SG_EN | HACE_ACCUM_EN);
+
+    /* Check hash IRQ status is asserted */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
+
+    /* Clear IRQ status and check status is deasserted */
+    qtest_writel(s, base + HACE_STS, 0x00000200);
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Read computed digest from memory */
+    qtest_memread(s, digest_addr, digest, sizeof(digest));
+
+    /* Check result of computation */
+    g_assert_cmpmem(digest, sizeof(digest),
+                    test_result_accum_sha512, sizeof(digest));
+
+    qtest_quit(s);
+}
+
+void aspeed_test_addresses(const char *machine, const uint32_t base,
+                           const struct AspeedMasks *expected)
+{
+    QTestState *s = qtest_init(machine);
+
+    /*
+     * Check command mode is zero, meaning engine is in direct access mode,
+     * as this affects the masking behavior of the HASH_SRC register.
+     */
+    g_assert_cmphex(qtest_readl(s, base + HACE_CMD), ==, 0);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC), ==, 0);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST), ==, 0);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==, 0);
+
+
+    /* Check that the address masking is correct */
+    qtest_writel(s, base + HACE_HASH_SRC, 0xffffffff);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC), ==, expected->src);
+
+    qtest_writel(s, base + HACE_HASH_DIGEST, 0xffffffff);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST), ==,
+                    expected->dest);
+
+    qtest_writel(s, base + HACE_HASH_DATA_LEN, 0xffffffff);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==,
+                    expected->len);
+
+    /* Reset to zero */
+    qtest_writel(s, base + HACE_HASH_SRC, 0);
+    qtest_writel(s, base + HACE_HASH_DIGEST, 0);
+    qtest_writel(s, base + HACE_HASH_DATA_LEN, 0);
+
+    /* Check that all bits are now zero */
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC), ==, 0);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST), ==, 0);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==, 0);
+
+    qtest_quit(s);
+}
+
diff --git a/tests/qtest/aspeed_hace-test.c b/tests/qtest/aspeed_hace-test.c
index ce86a44672..42a306af2a 100644
--- a/tests/qtest/aspeed_hace-test.c
+++ b/tests/qtest/aspeed_hace-test.c
@@ -6,584 +6,109 @@
  */
 
 #include "qemu/osdep.h"
-
 #include "libqtest.h"
 #include "qemu/bitops.h"
+#include "aspeed-hace-utils.h"
 
-#define HACE_CMD                 0x10
-#define  HACE_SHA_BE_EN          BIT(3)
-#define  HACE_MD5_LE_EN          BIT(2)
-#define  HACE_ALGO_MD5           0
-#define  HACE_ALGO_SHA1          BIT(5)
-#define  HACE_ALGO_SHA224        BIT(6)
-#define  HACE_ALGO_SHA256        (BIT(4) | BIT(6))
-#define  HACE_ALGO_SHA512        (BIT(5) | BIT(6))
-#define  HACE_ALGO_SHA384        (BIT(5) | BIT(6) | BIT(10))
-#define  HACE_SG_EN              BIT(18)
-#define  HACE_ACCUM_EN           BIT(8)
-
-#define HACE_STS                 0x1c
-#define  HACE_RSA_ISR            BIT(13)
-#define  HACE_CRYPTO_ISR         BIT(12)
-#define  HACE_HASH_ISR           BIT(9)
-#define  HACE_RSA_BUSY           BIT(2)
-#define  HACE_CRYPTO_BUSY        BIT(1)
-#define  HACE_HASH_BUSY          BIT(0)
-#define HACE_HASH_SRC            0x20
-#define HACE_HASH_DIGEST         0x24
-#define HACE_HASH_KEY_BUFF       0x28
-#define HACE_HASH_DATA_LEN       0x2c
-#define HACE_HASH_CMD            0x30
-/* Scatter-Gather Hash */
-#define SG_LIST_LEN_LAST         BIT(31)
-struct AspeedSgList {
-        uint32_t len;
-        uint32_t addr;
-} __attribute__ ((__packed__));
-
-/*
- * Test vector is the ascii "abc"
- *
- * Expected results were generated using command line utitiles:
- *
- *  echo -n -e 'abc' | dd of=/tmp/test
- *  for hash in sha512sum sha256sum md5sum; do $hash /tmp/test; done
- *
- */
-static const uint8_t test_vector[] = {0x61, 0x62, 0x63};
-
-static const uint8_t test_result_sha512[] = {
-    0xdd, 0xaf, 0x35, 0xa1, 0x93, 0x61, 0x7a, 0xba, 0xcc, 0x41, 0x73, 0x49,
-    0xae, 0x20, 0x41, 0x31, 0x12, 0xe6, 0xfa, 0x4e, 0x89, 0xa9, 0x7e, 0xa2,
-    0x0a, 0x9e, 0xee, 0xe6, 0x4b, 0x55, 0xd3, 0x9a, 0x21, 0x92, 0x99, 0x2a,
-    0x27, 0x4f, 0xc1, 0xa8, 0x36, 0xba, 0x3c, 0x23, 0xa3, 0xfe, 0xeb, 0xbd,
-    0x45, 0x4d, 0x44, 0x23, 0x64, 0x3c, 0xe8, 0x0e, 0x2a, 0x9a, 0xc9, 0x4f,
-    0xa5, 0x4c, 0xa4, 0x9f};
-
-static const uint8_t test_result_sha256[] = {
-    0xba, 0x78, 0x16, 0xbf, 0x8f, 0x01, 0xcf, 0xea, 0x41, 0x41, 0x40, 0xde,
-    0x5d, 0xae, 0x22, 0x23, 0xb0, 0x03, 0x61, 0xa3, 0x96, 0x17, 0x7a, 0x9c,
-    0xb4, 0x10, 0xff, 0x61, 0xf2, 0x00, 0x15, 0xad};
-
-static const uint8_t test_result_md5[] = {
-    0x90, 0x01, 0x50, 0x98, 0x3c, 0xd2, 0x4f, 0xb0, 0xd6, 0x96, 0x3f, 0x7d,
-    0x28, 0xe1, 0x7f, 0x72};
-
-/*
- * The Scatter-Gather Test vector is the ascii "abc" "def" "ghi", broken
- * into blocks of 3 characters as shown
- *
- * Expected results were generated using command line utitiles:
- *
- *  echo -n -e 'abcdefghijkl' | dd of=/tmp/test
- *  for hash in sha512sum sha256sum; do $hash /tmp/test; done
- *
- */
-static const uint8_t test_vector_sg1[] = {0x61, 0x62, 0x63, 0x64, 0x65, 0x66};
-static const uint8_t test_vector_sg2[] = {0x67, 0x68, 0x69};
-static const uint8_t test_vector_sg3[] = {0x6a, 0x6b, 0x6c};
-
-static const uint8_t test_result_sg_sha512[] = {
-    0x17, 0x80, 0x7c, 0x72, 0x8e, 0xe3, 0xba, 0x35, 0xe7, 0xcf, 0x7a, 0xf8,
-    0x23, 0x11, 0x6d, 0x26, 0xe4, 0x1e, 0x5d, 0x4d, 0x6c, 0x2f, 0xf1, 0xf3,
-    0x72, 0x0d, 0x3d, 0x96, 0xaa, 0xcb, 0x6f, 0x69, 0xde, 0x64, 0x2e, 0x63,
-    0xd5, 0xb7, 0x3f, 0xc3, 0x96, 0xc1, 0x2b, 0xe3, 0x8b, 0x2b, 0xd5, 0xd8,
-    0x84, 0x25, 0x7c, 0x32, 0xc8, 0xf6, 0xd0, 0x85, 0x4a, 0xe6, 0xb5, 0x40,
-    0xf8, 0x6d, 0xda, 0x2e};
-
-static const uint8_t test_result_sg_sha256[] = {
-    0xd6, 0x82, 0xed, 0x4c, 0xa4, 0xd9, 0x89, 0xc1, 0x34, 0xec, 0x94, 0xf1,
-    0x55, 0x1e, 0x1e, 0xc5, 0x80, 0xdd, 0x6d, 0x5a, 0x6e, 0xcd, 0xe9, 0xf3,
-    0xd3, 0x5e, 0x6e, 0x4a, 0x71, 0x7f, 0xbd, 0xe4};
-
-/*
- * The accumulative mode requires firmware to provide internal initial state
- * and message padding (including length L at the end of padding).
- *
- * This test vector is a ascii text "abc" with padding message.
- *
- * Expected results were generated using command line utitiles:
- *
- *  echo -n -e 'abc' | dd of=/tmp/test
- *  for hash in sha512sum sha256sum; do $hash /tmp/test; done
- */
-static const uint8_t test_vector_accum_512[] = {
-    0x61, 0x62, 0x63, 0x80, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18};
-
-static const uint8_t test_vector_accum_256[] = {
-    0x61, 0x62, 0x63, 0x80, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18};
-
-static const uint8_t test_result_accum_sha512[] = {
-    0xdd, 0xaf, 0x35, 0xa1, 0x93, 0x61, 0x7a, 0xba, 0xcc, 0x41, 0x73, 0x49,
-    0xae, 0x20, 0x41, 0x31, 0x12, 0xe6, 0xfa, 0x4e, 0x89, 0xa9, 0x7e, 0xa2,
-    0x0a, 0x9e, 0xee, 0xe6, 0x4b, 0x55, 0xd3, 0x9a, 0x21, 0x92, 0x99, 0x2a,
-    0x27, 0x4f, 0xc1, 0xa8, 0x36, 0xba, 0x3c, 0x23, 0xa3, 0xfe, 0xeb, 0xbd,
-    0x45, 0x4d, 0x44, 0x23, 0x64, 0x3c, 0xe8, 0x0e, 0x2a, 0x9a, 0xc9, 0x4f,
-    0xa5, 0x4c, 0xa4, 0x9f};
-
-static const uint8_t test_result_accum_sha256[] = {
-    0xba, 0x78, 0x16, 0xbf, 0x8f, 0x01, 0xcf, 0xea, 0x41, 0x41, 0x40, 0xde,
-    0x5d, 0xae, 0x22, 0x23, 0xb0, 0x03, 0x61, 0xa3, 0x96, 0x17, 0x7a, 0x9c,
-    0xb4, 0x10, 0xff, 0x61, 0xf2, 0x00, 0x15, 0xad};
-
-static void write_regs(QTestState *s, uint32_t base, uint32_t src,
-                       uint32_t length, uint32_t out, uint32_t method)
-{
-        qtest_writel(s, base + HACE_HASH_SRC, src);
-        qtest_writel(s, base + HACE_HASH_DIGEST, out);
-        qtest_writel(s, base + HACE_HASH_DATA_LEN, length);
-        qtest_writel(s, base + HACE_HASH_CMD, HACE_SHA_BE_EN | method);
-}
-
-static void test_md5(const char *machine, const uint32_t base,
-                     const uint32_t src_addr)
-
-{
-    QTestState *s = qtest_init(machine);
-
-    uint32_t digest_addr = src_addr + 0x01000000;
-    uint8_t digest[16] = {0};
-
-    /* Check engine is idle, no busy or irq bits set */
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
-
-    /* Write test vector into memory */
-    qtest_memwrite(s, src_addr, test_vector, sizeof(test_vector));
-
-    write_regs(s, base, src_addr, sizeof(test_vector), digest_addr, HACE_ALGO_MD5);
-
-    /* Check hash IRQ status is asserted */
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
-
-    /* Clear IRQ status and check status is deasserted */
-    qtest_writel(s, base + HACE_STS, 0x00000200);
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
-
-    /* Read computed digest from memory */
-    qtest_memread(s, digest_addr, digest, sizeof(digest));
-
-    /* Check result of computation */
-    g_assert_cmpmem(digest, sizeof(digest),
-                    test_result_md5, sizeof(digest));
-
-    qtest_quit(s);
-}
-
-static void test_sha256(const char *machine, const uint32_t base,
-                        const uint32_t src_addr)
-{
-    QTestState *s = qtest_init(machine);
-
-    const uint32_t digest_addr = src_addr + 0x1000000;
-    uint8_t digest[32] = {0};
-
-    /* Check engine is idle, no busy or irq bits set */
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
-
-    /* Write test vector into memory */
-    qtest_memwrite(s, src_addr, test_vector, sizeof(test_vector));
-
-    write_regs(s, base, src_addr, sizeof(test_vector), digest_addr, HACE_ALGO_SHA256);
-
-    /* Check hash IRQ status is asserted */
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
-
-    /* Clear IRQ status and check status is deasserted */
-    qtest_writel(s, base + HACE_STS, 0x00000200);
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
-
-    /* Read computed digest from memory */
-    qtest_memread(s, digest_addr, digest, sizeof(digest));
-
-    /* Check result of computation */
-    g_assert_cmpmem(digest, sizeof(digest),
-                    test_result_sha256, sizeof(digest));
-
-    qtest_quit(s);
-}
-
-static void test_sha512(const char *machine, const uint32_t base,
-                        const uint32_t src_addr)
-{
-    QTestState *s = qtest_init(machine);
-
-    const uint32_t digest_addr = src_addr + 0x1000000;
-    uint8_t digest[64] = {0};
-
-    /* Check engine is idle, no busy or irq bits set */
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
-
-    /* Write test vector into memory */
-    qtest_memwrite(s, src_addr, test_vector, sizeof(test_vector));
-
-    write_regs(s, base, src_addr, sizeof(test_vector), digest_addr, HACE_ALGO_SHA512);
-
-    /* Check hash IRQ status is asserted */
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
-
-    /* Clear IRQ status and check status is deasserted */
-    qtest_writel(s, base + HACE_STS, 0x00000200);
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
-
-    /* Read computed digest from memory */
-    qtest_memread(s, digest_addr, digest, sizeof(digest));
-
-    /* Check result of computation */
-    g_assert_cmpmem(digest, sizeof(digest),
-                    test_result_sha512, sizeof(digest));
-
-    qtest_quit(s);
-}
-
-static void test_sha256_sg(const char *machine, const uint32_t base,
-                        const uint32_t src_addr)
-{
-    QTestState *s = qtest_init(machine);
-
-    const uint32_t src_addr_1 = src_addr + 0x1000000;
-    const uint32_t src_addr_2 = src_addr + 0x2000000;
-    const uint32_t src_addr_3 = src_addr + 0x3000000;
-    const uint32_t digest_addr = src_addr + 0x4000000;
-    uint8_t digest[32] = {0};
-    struct AspeedSgList array[] = {
-        {  cpu_to_le32(sizeof(test_vector_sg1)),
-           cpu_to_le32(src_addr_1) },
-        {  cpu_to_le32(sizeof(test_vector_sg2)),
-           cpu_to_le32(src_addr_2) },
-        {  cpu_to_le32(sizeof(test_vector_sg3) | SG_LIST_LEN_LAST),
-           cpu_to_le32(src_addr_3) },
-    };
-
-    /* Check engine is idle, no busy or irq bits set */
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
-
-    /* Write test vector into memory */
-    qtest_memwrite(s, src_addr_1, test_vector_sg1, sizeof(test_vector_sg1));
-    qtest_memwrite(s, src_addr_2, test_vector_sg2, sizeof(test_vector_sg2));
-    qtest_memwrite(s, src_addr_3, test_vector_sg3, sizeof(test_vector_sg3));
-    qtest_memwrite(s, src_addr, array, sizeof(array));
-
-    write_regs(s, base, src_addr,
-               (sizeof(test_vector_sg1)
-                + sizeof(test_vector_sg2)
-                + sizeof(test_vector_sg3)),
-               digest_addr, HACE_ALGO_SHA256 | HACE_SG_EN);
-
-    /* Check hash IRQ status is asserted */
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
-
-    /* Clear IRQ status and check status is deasserted */
-    qtest_writel(s, base + HACE_STS, 0x00000200);
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
-
-    /* Read computed digest from memory */
-    qtest_memread(s, digest_addr, digest, sizeof(digest));
-
-    /* Check result of computation */
-    g_assert_cmpmem(digest, sizeof(digest),
-                    test_result_sg_sha256, sizeof(digest));
-
-    qtest_quit(s);
-}
-
-static void test_sha512_sg(const char *machine, const uint32_t base,
-                        const uint32_t src_addr)
-{
-    QTestState *s = qtest_init(machine);
-
-    const uint32_t src_addr_1 = src_addr + 0x1000000;
-    const uint32_t src_addr_2 = src_addr + 0x2000000;
-    const uint32_t src_addr_3 = src_addr + 0x3000000;
-    const uint32_t digest_addr = src_addr + 0x4000000;
-    uint8_t digest[64] = {0};
-    struct AspeedSgList array[] = {
-        {  cpu_to_le32(sizeof(test_vector_sg1)),
-           cpu_to_le32(src_addr_1) },
-        {  cpu_to_le32(sizeof(test_vector_sg2)),
-           cpu_to_le32(src_addr_2) },
-        {  cpu_to_le32(sizeof(test_vector_sg3) | SG_LIST_LEN_LAST),
-           cpu_to_le32(src_addr_3) },
-    };
-
-    /* Check engine is idle, no busy or irq bits set */
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
-
-    /* Write test vector into memory */
-    qtest_memwrite(s, src_addr_1, test_vector_sg1, sizeof(test_vector_sg1));
-    qtest_memwrite(s, src_addr_2, test_vector_sg2, sizeof(test_vector_sg2));
-    qtest_memwrite(s, src_addr_3, test_vector_sg3, sizeof(test_vector_sg3));
-    qtest_memwrite(s, src_addr, array, sizeof(array));
-
-    write_regs(s, base, src_addr,
-               (sizeof(test_vector_sg1)
-                + sizeof(test_vector_sg2)
-                + sizeof(test_vector_sg3)),
-               digest_addr, HACE_ALGO_SHA512 | HACE_SG_EN);
-
-    /* Check hash IRQ status is asserted */
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
-
-    /* Clear IRQ status and check status is deasserted */
-    qtest_writel(s, base + HACE_STS, 0x00000200);
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
-
-    /* Read computed digest from memory */
-    qtest_memread(s, digest_addr, digest, sizeof(digest));
-
-    /* Check result of computation */
-    g_assert_cmpmem(digest, sizeof(digest),
-                    test_result_sg_sha512, sizeof(digest));
-
-    qtest_quit(s);
-}
-
-static void test_sha256_accum(const char *machine, const uint32_t base,
-                        const uint32_t src_addr)
-{
-    QTestState *s = qtest_init(machine);
-
-    const uint32_t buffer_addr = src_addr + 0x1000000;
-    const uint32_t digest_addr = src_addr + 0x4000000;
-    uint8_t digest[32] = {0};
-    struct AspeedSgList array[] = {
-        {  cpu_to_le32(sizeof(test_vector_accum_256) | SG_LIST_LEN_LAST),
-           cpu_to_le32(buffer_addr) },
-    };
-
-    /* Check engine is idle, no busy or irq bits set */
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
-
-    /* Write test vector into memory */
-    qtest_memwrite(s, buffer_addr, test_vector_accum_256,
-                   sizeof(test_vector_accum_256));
-    qtest_memwrite(s, src_addr, array, sizeof(array));
-
-    write_regs(s, base, src_addr, sizeof(test_vector_accum_256),
-               digest_addr, HACE_ALGO_SHA256 | HACE_SG_EN | HACE_ACCUM_EN);
-
-    /* Check hash IRQ status is asserted */
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
-
-    /* Clear IRQ status and check status is deasserted */
-    qtest_writel(s, base + HACE_STS, 0x00000200);
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
-
-    /* Read computed digest from memory */
-    qtest_memread(s, digest_addr, digest, sizeof(digest));
-
-    /* Check result of computation */
-    g_assert_cmpmem(digest, sizeof(digest),
-                    test_result_accum_sha256, sizeof(digest));
-
-    qtest_quit(s);
-}
-
-static void test_sha512_accum(const char *machine, const uint32_t base,
-                        const uint32_t src_addr)
-{
-    QTestState *s = qtest_init(machine);
-
-    const uint32_t buffer_addr = src_addr + 0x1000000;
-    const uint32_t digest_addr = src_addr + 0x4000000;
-    uint8_t digest[64] = {0};
-    struct AspeedSgList array[] = {
-        {  cpu_to_le32(sizeof(test_vector_accum_512) | SG_LIST_LEN_LAST),
-           cpu_to_le32(buffer_addr) },
-    };
-
-    /* Check engine is idle, no busy or irq bits set */
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
-
-    /* Write test vector into memory */
-    qtest_memwrite(s, buffer_addr, test_vector_accum_512,
-                   sizeof(test_vector_accum_512));
-    qtest_memwrite(s, src_addr, array, sizeof(array));
-
-    write_regs(s, base, src_addr, sizeof(test_vector_accum_512),
-               digest_addr, HACE_ALGO_SHA512 | HACE_SG_EN | HACE_ACCUM_EN);
-
-    /* Check hash IRQ status is asserted */
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
-
-    /* Clear IRQ status and check status is deasserted */
-    qtest_writel(s, base + HACE_STS, 0x00000200);
-    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
-
-    /* Read computed digest from memory */
-    qtest_memread(s, digest_addr, digest, sizeof(digest));
-
-    /* Check result of computation */
-    g_assert_cmpmem(digest, sizeof(digest),
-                    test_result_accum_sha512, sizeof(digest));
-
-    qtest_quit(s);
-}
-
-struct masks {
-    uint32_t src;
-    uint32_t dest;
-    uint32_t len;
-};
-
-static const struct masks ast2600_masks = {
+static const struct AspeedMasks ast2600_masks = {
     .src  = 0x7fffffff,
     .dest = 0x7ffffff8,
     .len  = 0x0fffffff,
 };
 
-static const struct masks ast2500_masks = {
+static const struct AspeedMasks ast2500_masks = {
     .src  = 0x3fffffff,
     .dest = 0x3ffffff8,
     .len  = 0x0fffffff,
 };
 
-static const struct masks ast2400_masks = {
+static const struct AspeedMasks ast2400_masks = {
     .src  = 0x0fffffff,
     .dest = 0x0ffffff8,
     .len  = 0x0fffffff,
 };
 
-static void test_addresses(const char *machine, const uint32_t base,
-                           const struct masks *expected)
-{
-    QTestState *s = qtest_init(machine);
-
-    /*
-     * Check command mode is zero, meaning engine is in direct access mode,
-     * as this affects the masking behavior of the HASH_SRC register.
-     */
-    g_assert_cmphex(qtest_readl(s, base + HACE_CMD), ==, 0);
-    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC), ==, 0);
-    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST), ==, 0);
-    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==, 0);
-
-
-    /* Check that the address masking is correct */
-    qtest_writel(s, base + HACE_HASH_SRC, 0xffffffff);
-    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC), ==, expected->src);
-
-    qtest_writel(s, base + HACE_HASH_DIGEST, 0xffffffff);
-    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST), ==, expected->dest);
-
-    qtest_writel(s, base + HACE_HASH_DATA_LEN, 0xffffffff);
-    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==, expected->len);
-
-    /* Reset to zero */
-    qtest_writel(s, base + HACE_HASH_SRC, 0);
-    qtest_writel(s, base + HACE_HASH_DIGEST, 0);
-    qtest_writel(s, base + HACE_HASH_DATA_LEN, 0);
-
-    /* Check that all bits are now zero */
-    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC), ==, 0);
-    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST), ==, 0);
-    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==, 0);
-
-    qtest_quit(s);
-}
-
 /* ast2600 */
 static void test_md5_ast2600(void)
 {
-    test_md5("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
+    aspeed_test_md5("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
 }
 
 static void test_sha256_ast2600(void)
 {
-    test_sha256("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
+    aspeed_test_sha256("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
 }
 
 static void test_sha256_sg_ast2600(void)
 {
-    test_sha256_sg("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
+    aspeed_test_sha256_sg("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
 }
 
 static void test_sha512_ast2600(void)
 {
-    test_sha512("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
+    aspeed_test_sha512("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
 }
 
 static void test_sha512_sg_ast2600(void)
 {
-    test_sha512_sg("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
+    aspeed_test_sha512_sg("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
 }
 
 static void test_sha256_accum_ast2600(void)
 {
-    test_sha256_accum("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
+    aspeed_test_sha256_accum("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
 }
 
 static void test_sha512_accum_ast2600(void)
 {
-    test_sha512_accum("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
+    aspeed_test_sha512_accum("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
 }
 
 static void test_addresses_ast2600(void)
 {
-    test_addresses("-machine ast2600-evb", 0x1e6d0000, &ast2600_masks);
+    aspeed_test_addresses("-machine ast2600-evb", 0x1e6d0000, &ast2600_masks);
 }
 
 /* ast2500 */
 static void test_md5_ast2500(void)
 {
-    test_md5("-machine ast2500-evb", 0x1e6e3000, 0x80000000);
+    aspeed_test_md5("-machine ast2500-evb", 0x1e6e3000, 0x80000000);
 }
 
 static void test_sha256_ast2500(void)
 {
-    test_sha256("-machine ast2500-evb", 0x1e6e3000, 0x80000000);
+    aspeed_test_sha256("-machine ast2500-evb", 0x1e6e3000, 0x80000000);
 }
 
 static void test_sha512_ast2500(void)
 {
-    test_sha512("-machine ast2500-evb", 0x1e6e3000, 0x80000000);
+    aspeed_test_sha512("-machine ast2500-evb", 0x1e6e3000, 0x80000000);
 }
 
 static void test_addresses_ast2500(void)
 {
-    test_addresses("-machine ast2500-evb", 0x1e6e3000, &ast2500_masks);
+    aspeed_test_addresses("-machine ast2500-evb", 0x1e6e3000, &ast2500_masks);
 }
 
 /* ast2400 */
 static void test_md5_ast2400(void)
 {
-    test_md5("-machine palmetto-bmc", 0x1e6e3000, 0x40000000);
+    aspeed_test_md5("-machine palmetto-bmc", 0x1e6e3000, 0x40000000);
 }
 
 static void test_sha256_ast2400(void)
 {
-    test_sha256("-machine palmetto-bmc", 0x1e6e3000, 0x40000000);
+    aspeed_test_sha256("-machine palmetto-bmc", 0x1e6e3000, 0x40000000);
 }
 
 static void test_sha512_ast2400(void)
 {
-    test_sha512("-machine palmetto-bmc", 0x1e6e3000, 0x40000000);
+    aspeed_test_sha512("-machine palmetto-bmc", 0x1e6e3000, 0x40000000);
 }
 
 static void test_addresses_ast2400(void)
 {
-    test_addresses("-machine palmetto-bmc", 0x1e6e3000, &ast2400_masks);
+    aspeed_test_addresses("-machine palmetto-bmc", 0x1e6e3000, &ast2400_masks);
 }
 
 int main(int argc, char **argv)
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 6b5161a643..bb14a22ebe 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -360,6 +360,7 @@ if gnutls.found()
 endif
 
 qtests = {
+  'aspeed_hace-test': files('aspeed-hace-utils.c', 'aspeed_hace-test.c'),
   'aspeed_smc-test': files('aspeed-smc-utils.c', 'aspeed_smc-test.c'),
   'ast2700-smc-test': files('aspeed-smc-utils.c', 'ast2700-smc-test.c'),
   'bios-tables-test': [io, 'boot-sector.c', 'acpi-utils.c', 'tpm-emu.c'],
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 19/28] test/qtest/hace: Specify explicit array sizes for test vectors and hash results
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (17 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 18/28] test/qtest: Introduce a new aspeed-hace-utils.c to place common testcases Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 20/28] test/qtest/hace: Adjust test address range for AST1030 due to SRAM limitations Jamin Lin via
                   ` (12 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

To enhance code readability and prevent potential buffer overflows or unintended
size assumptions, this commit updates all fixed-size array declarations to use
explicit array sizes.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 tests/qtest/aspeed-hace-utils.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/tests/qtest/aspeed-hace-utils.c b/tests/qtest/aspeed-hace-utils.c
index 8582847945..777fa5b986 100644
--- a/tests/qtest/aspeed-hace-utils.c
+++ b/tests/qtest/aspeed-hace-utils.c
@@ -19,9 +19,9 @@
  *  for hash in sha512sum sha256sum md5sum; do $hash /tmp/test; done
  *
  */
-static const uint8_t test_vector[] = {0x61, 0x62, 0x63};
+static const uint8_t test_vector[3] = {0x61, 0x62, 0x63};
 
-static const uint8_t test_result_sha512[] = {
+static const uint8_t test_result_sha512[64] = {
     0xdd, 0xaf, 0x35, 0xa1, 0x93, 0x61, 0x7a, 0xba, 0xcc, 0x41, 0x73, 0x49,
     0xae, 0x20, 0x41, 0x31, 0x12, 0xe6, 0xfa, 0x4e, 0x89, 0xa9, 0x7e, 0xa2,
     0x0a, 0x9e, 0xee, 0xe6, 0x4b, 0x55, 0xd3, 0x9a, 0x21, 0x92, 0x99, 0x2a,
@@ -29,12 +29,12 @@ static const uint8_t test_result_sha512[] = {
     0x45, 0x4d, 0x44, 0x23, 0x64, 0x3c, 0xe8, 0x0e, 0x2a, 0x9a, 0xc9, 0x4f,
     0xa5, 0x4c, 0xa4, 0x9f};
 
-static const uint8_t test_result_sha256[] = {
+static const uint8_t test_result_sha256[32] = {
     0xba, 0x78, 0x16, 0xbf, 0x8f, 0x01, 0xcf, 0xea, 0x41, 0x41, 0x40, 0xde,
     0x5d, 0xae, 0x22, 0x23, 0xb0, 0x03, 0x61, 0xa3, 0x96, 0x17, 0x7a, 0x9c,
     0xb4, 0x10, 0xff, 0x61, 0xf2, 0x00, 0x15, 0xad};
 
-static const uint8_t test_result_md5[] = {
+static const uint8_t test_result_md5[16] = {
     0x90, 0x01, 0x50, 0x98, 0x3c, 0xd2, 0x4f, 0xb0, 0xd6, 0x96, 0x3f, 0x7d,
     0x28, 0xe1, 0x7f, 0x72};
 
@@ -48,11 +48,11 @@ static const uint8_t test_result_md5[] = {
  *  for hash in sha512sum sha256sum; do $hash /tmp/test; done
  *
  */
-static const uint8_t test_vector_sg1[] = {0x61, 0x62, 0x63, 0x64, 0x65, 0x66};
-static const uint8_t test_vector_sg2[] = {0x67, 0x68, 0x69};
-static const uint8_t test_vector_sg3[] = {0x6a, 0x6b, 0x6c};
+static const uint8_t test_vector_sg1[6] = {0x61, 0x62, 0x63, 0x64, 0x65, 0x66};
+static const uint8_t test_vector_sg2[3] = {0x67, 0x68, 0x69};
+static const uint8_t test_vector_sg3[3] = {0x6a, 0x6b, 0x6c};
 
-static const uint8_t test_result_sg_sha512[] = {
+static const uint8_t test_result_sg_sha512[64] = {
     0x17, 0x80, 0x7c, 0x72, 0x8e, 0xe3, 0xba, 0x35, 0xe7, 0xcf, 0x7a, 0xf8,
     0x23, 0x11, 0x6d, 0x26, 0xe4, 0x1e, 0x5d, 0x4d, 0x6c, 0x2f, 0xf1, 0xf3,
     0x72, 0x0d, 0x3d, 0x96, 0xaa, 0xcb, 0x6f, 0x69, 0xde, 0x64, 0x2e, 0x63,
@@ -60,7 +60,7 @@ static const uint8_t test_result_sg_sha512[] = {
     0x84, 0x25, 0x7c, 0x32, 0xc8, 0xf6, 0xd0, 0x85, 0x4a, 0xe6, 0xb5, 0x40,
     0xf8, 0x6d, 0xda, 0x2e};
 
-static const uint8_t test_result_sg_sha256[] = {
+static const uint8_t test_result_sg_sha256[32] = {
     0xd6, 0x82, 0xed, 0x4c, 0xa4, 0xd9, 0x89, 0xc1, 0x34, 0xec, 0x94, 0xf1,
     0x55, 0x1e, 0x1e, 0xc5, 0x80, 0xdd, 0x6d, 0x5a, 0x6e, 0xcd, 0xe9, 0xf3,
     0xd3, 0x5e, 0x6e, 0x4a, 0x71, 0x7f, 0xbd, 0xe4};
@@ -76,7 +76,7 @@ static const uint8_t test_result_sg_sha256[] = {
  *  echo -n -e 'abc' | dd of=/tmp/test
  *  for hash in sha512sum sha256sum; do $hash /tmp/test; done
  */
-static const uint8_t test_vector_accum_512[] = {
+static const uint8_t test_vector_accum_512[128] = {
     0x61, 0x62, 0x63, 0x80, 0x00, 0x00, 0x00, 0x00,
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
@@ -94,7 +94,7 @@ static const uint8_t test_vector_accum_512[] = {
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18};
 
-static const uint8_t test_vector_accum_256[] = {
+static const uint8_t test_vector_accum_256[64] = {
     0x61, 0x62, 0x63, 0x80, 0x00, 0x00, 0x00, 0x00,
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
@@ -104,7 +104,7 @@ static const uint8_t test_vector_accum_256[] = {
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18};
 
-static const uint8_t test_result_accum_sha512[] = {
+static const uint8_t test_result_accum_sha512[64] = {
     0xdd, 0xaf, 0x35, 0xa1, 0x93, 0x61, 0x7a, 0xba, 0xcc, 0x41, 0x73, 0x49,
     0xae, 0x20, 0x41, 0x31, 0x12, 0xe6, 0xfa, 0x4e, 0x89, 0xa9, 0x7e, 0xa2,
     0x0a, 0x9e, 0xee, 0xe6, 0x4b, 0x55, 0xd3, 0x9a, 0x21, 0x92, 0x99, 0x2a,
@@ -112,7 +112,7 @@ static const uint8_t test_result_accum_sha512[] = {
     0x45, 0x4d, 0x44, 0x23, 0x64, 0x3c, 0xe8, 0x0e, 0x2a, 0x9a, 0xc9, 0x4f,
     0xa5, 0x4c, 0xa4, 0x9f};
 
-static const uint8_t test_result_accum_sha256[] = {
+static const uint8_t test_result_accum_sha256[32] = {
     0xba, 0x78, 0x16, 0xbf, 0x8f, 0x01, 0xcf, 0xea, 0x41, 0x41, 0x40, 0xde,
     0x5d, 0xae, 0x22, 0x23, 0xb0, 0x03, 0x61, 0xa3, 0x96, 0x17, 0x7a, 0x9c,
     0xb4, 0x10, 0xff, 0x61, 0xf2, 0x00, 0x15, 0xad};
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 20/28] test/qtest/hace: Adjust test address range for AST1030 due to SRAM limitations
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (18 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 19/28] test/qtest/hace: Specify explicit array sizes for test vectors and hash results Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 21/28] test/qtest/hace: Add SHA-384 test cases for ASPEED HACE model Jamin Lin via
                   ` (11 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

The digest_addr is set to "src_addr + 0x1000000", where src_addr is the DRAM
base address. However, the value 0x1000000 (16MB) is too large because the
AST1030 does not support DRAM, and its SRAM size is only 768KB.

A range size of 0x10000 (64KB) is sufficient for HACE test cases, as the test
vector size does not exceed 64KB.

Updates:
1. Direct Access Mode
Update digest_addr to "src_addr + 0x10000" in the following functions:
aspeed_test_md5
aspeed_test_sha256
aspeed_test_sha512

2. Scatter-Gather (SG) Mode
Update source address for different SG buffer addresses in the following
functions:
src_addr1 = src_addr + 0x10000
src_addr2 = src_addr + 0x20000
src_addr3 = src_addr + 0x30000
digest_addr = src_addr + 0x40000

aspeed_test_sha256_sg
aspeed_test_sha512_sg

3. ACC Mode Update
Update the SG List start address: src_addr + 0x10000
Update the SG List buffer size to 0x30000 (192KB).

buffer_addr = src_addr + 0x10000
digest_addr = src_addr + 0x40000

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 tests/qtest/aspeed-hace-utils.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/tests/qtest/aspeed-hace-utils.c b/tests/qtest/aspeed-hace-utils.c
index 777fa5b986..539d06e4f8 100644
--- a/tests/qtest/aspeed-hace-utils.c
+++ b/tests/qtest/aspeed-hace-utils.c
@@ -132,7 +132,7 @@ void aspeed_test_md5(const char *machine, const uint32_t base,
 {
     QTestState *s = qtest_init(machine);
 
-    uint32_t digest_addr = src_addr + 0x01000000;
+    uint32_t digest_addr = src_addr + 0x010000;
     uint8_t digest[16] = {0};
 
     /* Check engine is idle, no busy or irq bits set */
@@ -166,7 +166,7 @@ void aspeed_test_sha256(const char *machine, const uint32_t base,
 {
     QTestState *s = qtest_init(machine);
 
-    const uint32_t digest_addr = src_addr + 0x1000000;
+    const uint32_t digest_addr = src_addr + 0x10000;
     uint8_t digest[32] = {0};
 
     /* Check engine is idle, no busy or irq bits set */
@@ -200,7 +200,7 @@ void aspeed_test_sha512(const char *machine, const uint32_t base,
 {
     QTestState *s = qtest_init(machine);
 
-    const uint32_t digest_addr = src_addr + 0x1000000;
+    const uint32_t digest_addr = src_addr + 0x10000;
     uint8_t digest[64] = {0};
 
     /* Check engine is idle, no busy or irq bits set */
@@ -234,10 +234,10 @@ void aspeed_test_sha256_sg(const char *machine, const uint32_t base,
 {
     QTestState *s = qtest_init(machine);
 
-    const uint32_t src_addr_1 = src_addr + 0x1000000;
-    const uint32_t src_addr_2 = src_addr + 0x2000000;
-    const uint32_t src_addr_3 = src_addr + 0x3000000;
-    const uint32_t digest_addr = src_addr + 0x4000000;
+    const uint32_t src_addr_1 = src_addr + 0x10000;
+    const uint32_t src_addr_2 = src_addr + 0x20000;
+    const uint32_t src_addr_3 = src_addr + 0x30000;
+    const uint32_t digest_addr = src_addr + 0x40000;
     uint8_t digest[32] = {0};
     struct AspeedSgList array[] = {
         {  cpu_to_le32(sizeof(test_vector_sg1)),
@@ -285,10 +285,10 @@ void aspeed_test_sha512_sg(const char *machine, const uint32_t base,
 {
     QTestState *s = qtest_init(machine);
 
-    const uint32_t src_addr_1 = src_addr + 0x1000000;
-    const uint32_t src_addr_2 = src_addr + 0x2000000;
-    const uint32_t src_addr_3 = src_addr + 0x3000000;
-    const uint32_t digest_addr = src_addr + 0x4000000;
+    const uint32_t src_addr_1 = src_addr + 0x10000;
+    const uint32_t src_addr_2 = src_addr + 0x20000;
+    const uint32_t src_addr_3 = src_addr + 0x30000;
+    const uint32_t digest_addr = src_addr + 0x40000;
     uint8_t digest[64] = {0};
     struct AspeedSgList array[] = {
         {  cpu_to_le32(sizeof(test_vector_sg1)),
@@ -336,8 +336,8 @@ void aspeed_test_sha256_accum(const char *machine, const uint32_t base,
 {
     QTestState *s = qtest_init(machine);
 
-    const uint32_t buffer_addr = src_addr + 0x1000000;
-    const uint32_t digest_addr = src_addr + 0x4000000;
+    const uint32_t buffer_addr = src_addr + 0x10000;
+    const uint32_t digest_addr = src_addr + 0x40000;
     uint8_t digest[32] = {0};
     struct AspeedSgList array[] = {
         {  cpu_to_le32(sizeof(test_vector_accum_256) | SG_LIST_LEN_LAST),
@@ -377,8 +377,8 @@ void aspeed_test_sha512_accum(const char *machine, const uint32_t base,
 {
     QTestState *s = qtest_init(machine);
 
-    const uint32_t buffer_addr = src_addr + 0x1000000;
-    const uint32_t digest_addr = src_addr + 0x4000000;
+    const uint32_t buffer_addr = src_addr + 0x10000;
+    const uint32_t digest_addr = src_addr + 0x40000;
     uint8_t digest[64] = {0};
     struct AspeedSgList array[] = {
         {  cpu_to_le32(sizeof(test_vector_accum_512) | SG_LIST_LEN_LAST),
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 21/28] test/qtest/hace: Add SHA-384 test cases for ASPEED HACE model
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (19 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 20/28] test/qtest/hace: Adjust test address range for AST1030 due to SRAM limitations Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 22/28] test/qtest/hace: Add SHA-384 tests for AST2600 Jamin Lin via
                   ` (10 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

Introduced SHA-384 test functions to verify hashing operations.
Extended support for scatter-gather ("_sg") and accumulation ("_accum") tests.
Updated test result vectors for SHA-384 validation.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 tests/qtest/aspeed-hace-utils.h |   6 ++
 tests/qtest/aspeed-hace-utils.c | 168 +++++++++++++++++++++++++++++++-
 2 files changed, 171 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/aspeed-hace-utils.h b/tests/qtest/aspeed-hace-utils.h
index 598577c69b..f4440561de 100644
--- a/tests/qtest/aspeed-hace-utils.h
+++ b/tests/qtest/aspeed-hace-utils.h
@@ -54,14 +54,20 @@ void aspeed_test_md5(const char *machine, const uint32_t base,
                      const uint32_t src_addr);
 void aspeed_test_sha256(const char *machine, const uint32_t base,
                         const uint32_t src_addr);
+void aspeed_test_sha384(const char *machine, const uint32_t base,
+                        const uint32_t src_addr);
 void aspeed_test_sha512(const char *machine, const uint32_t base,
                         const uint32_t src_addr);
 void aspeed_test_sha256_sg(const char *machine, const uint32_t base,
                            const uint32_t src_addr);
+void aspeed_test_sha384_sg(const char *machine, const uint32_t base,
+                           const uint32_t src_addr);
 void aspeed_test_sha512_sg(const char *machine, const uint32_t base,
                            const uint32_t src_addr);
 void aspeed_test_sha256_accum(const char *machine, const uint32_t base,
                               const uint32_t src_addr);
+void aspeed_test_sha384_accum(const char *machine, const uint32_t base,
+                              const uint32_t src_addr);
 void aspeed_test_sha512_accum(const char *machine, const uint32_t base,
                               const uint32_t src_addr);
 void aspeed_test_addresses(const char *machine, const uint32_t base,
diff --git a/tests/qtest/aspeed-hace-utils.c b/tests/qtest/aspeed-hace-utils.c
index 539d06e4f8..dad90ee81c 100644
--- a/tests/qtest/aspeed-hace-utils.c
+++ b/tests/qtest/aspeed-hace-utils.c
@@ -16,7 +16,7 @@
  * Expected results were generated using command line utitiles:
  *
  *  echo -n -e 'abc' | dd of=/tmp/test
- *  for hash in sha512sum sha256sum md5sum; do $hash /tmp/test; done
+ *  for hash in sha512sum sha384sum sha256sum md5sum; do $hash /tmp/test; done
  *
  */
 static const uint8_t test_vector[3] = {0x61, 0x62, 0x63};
@@ -29,6 +29,12 @@ static const uint8_t test_result_sha512[64] = {
     0x45, 0x4d, 0x44, 0x23, 0x64, 0x3c, 0xe8, 0x0e, 0x2a, 0x9a, 0xc9, 0x4f,
     0xa5, 0x4c, 0xa4, 0x9f};
 
+static const uint8_t test_result_sha384[48] = {
+    0xcb, 0x00, 0x75, 0x3f, 0x45, 0xa3, 0x5e, 0x8b, 0xb5, 0xa0, 0x3d, 0x69,
+    0x9a, 0xc6, 0x50, 0x07, 0x27, 0x2c, 0x32, 0xab, 0x0e, 0xde, 0xd1, 0x63,
+    0x1a, 0x8b, 0x60, 0x5a, 0x43, 0xff, 0x5b, 0xed, 0x80, 0x86, 0x07, 0x2b,
+    0xa1, 0xe7, 0xcc, 0x23, 0x58, 0xba, 0xec, 0xa1, 0x34, 0xc8, 0x25, 0xa7};
+
 static const uint8_t test_result_sha256[32] = {
     0xba, 0x78, 0x16, 0xbf, 0x8f, 0x01, 0xcf, 0xea, 0x41, 0x41, 0x40, 0xde,
     0x5d, 0xae, 0x22, 0x23, 0xb0, 0x03, 0x61, 0xa3, 0x96, 0x17, 0x7a, 0x9c,
@@ -45,7 +51,7 @@ static const uint8_t test_result_md5[16] = {
  * Expected results were generated using command line utitiles:
  *
  *  echo -n -e 'abcdefghijkl' | dd of=/tmp/test
- *  for hash in sha512sum sha256sum; do $hash /tmp/test; done
+ *  for hash in sha512sum sha384sum sha256sum; do $hash /tmp/test; done
  *
  */
 static const uint8_t test_vector_sg1[6] = {0x61, 0x62, 0x63, 0x64, 0x65, 0x66};
@@ -60,6 +66,12 @@ static const uint8_t test_result_sg_sha512[64] = {
     0x84, 0x25, 0x7c, 0x32, 0xc8, 0xf6, 0xd0, 0x85, 0x4a, 0xe6, 0xb5, 0x40,
     0xf8, 0x6d, 0xda, 0x2e};
 
+static const uint8_t test_result_sg_sha384[48] = {
+    0x10, 0x3c, 0xa9, 0x6c, 0x06, 0xa1, 0xce, 0x79, 0x8f, 0x08, 0xf8, 0xef,
+    0xf0, 0xdf, 0xb0, 0xcc, 0xdb, 0x56, 0x7d, 0x48, 0xb2, 0x85, 0xb2, 0x3d,
+    0x0c, 0xd7, 0x73, 0x45, 0x46, 0x67, 0xa3, 0xc2, 0xfa, 0x5f, 0x1b, 0x58,
+    0xd9, 0xcd, 0xf2, 0x32, 0x9b, 0xd9, 0x97, 0x97, 0x30, 0xbf, 0xaa, 0xff};
+
 static const uint8_t test_result_sg_sha256[32] = {
     0xd6, 0x82, 0xed, 0x4c, 0xa4, 0xd9, 0x89, 0xc1, 0x34, 0xec, 0x94, 0xf1,
     0x55, 0x1e, 0x1e, 0xc5, 0x80, 0xdd, 0x6d, 0x5a, 0x6e, 0xcd, 0xe9, 0xf3,
@@ -74,7 +86,7 @@ static const uint8_t test_result_sg_sha256[32] = {
  * Expected results were generated using command line utitiles:
  *
  *  echo -n -e 'abc' | dd of=/tmp/test
- *  for hash in sha512sum sha256sum; do $hash /tmp/test; done
+ *  for hash in sha512sum sha384sum sha256sum; do $hash /tmp/test; done
  */
 static const uint8_t test_vector_accum_512[128] = {
     0x61, 0x62, 0x63, 0x80, 0x00, 0x00, 0x00, 0x00,
@@ -94,6 +106,24 @@ static const uint8_t test_vector_accum_512[128] = {
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18};
 
+static const uint8_t test_vector_accum_384[128] = {
+    0x61, 0x62, 0x63, 0x80, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18};
+
 static const uint8_t test_vector_accum_256[64] = {
     0x61, 0x62, 0x63, 0x80, 0x00, 0x00, 0x00, 0x00,
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
@@ -112,6 +142,12 @@ static const uint8_t test_result_accum_sha512[64] = {
     0x45, 0x4d, 0x44, 0x23, 0x64, 0x3c, 0xe8, 0x0e, 0x2a, 0x9a, 0xc9, 0x4f,
     0xa5, 0x4c, 0xa4, 0x9f};
 
+static const uint8_t test_result_accum_sha384[48] = {
+    0xcb, 0x00, 0x75, 0x3f, 0x45, 0xa3, 0x5e, 0x8b, 0xb5, 0xa0, 0x3d, 0x69,
+    0x9a, 0xc6, 0x50, 0x07, 0x27, 0x2c, 0x32, 0xab, 0x0e, 0xde, 0xd1, 0x63,
+    0x1a, 0x8b, 0x60, 0x5a, 0x43, 0xff, 0x5b, 0xed, 0x80, 0x86, 0x07, 0x2b,
+    0xa1, 0xe7, 0xcc, 0x23, 0x58, 0xba, 0xec, 0xa1, 0x34, 0xc8, 0x25, 0xa7};
+
 static const uint8_t test_result_accum_sha256[32] = {
     0xba, 0x78, 0x16, 0xbf, 0x8f, 0x01, 0xcf, 0xea, 0x41, 0x41, 0x40, 0xde,
     0x5d, 0xae, 0x22, 0x23, 0xb0, 0x03, 0x61, 0xa3, 0x96, 0x17, 0x7a, 0x9c,
@@ -195,6 +231,40 @@ void aspeed_test_sha256(const char *machine, const uint32_t base,
     qtest_quit(s);
 }
 
+void aspeed_test_sha384(const char *machine, const uint32_t base,
+                        const uint32_t src_addr)
+{
+    QTestState *s = qtest_init(machine);
+
+    const uint32_t digest_addr = src_addr + 0x10000;
+    uint8_t digest[48] = {0};
+
+    /* Check engine is idle, no busy or irq bits set */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Write test vector into memory */
+    qtest_memwrite(s, src_addr, test_vector, sizeof(test_vector));
+
+    write_regs(s, base, src_addr, sizeof(test_vector), digest_addr,
+               HACE_ALGO_SHA384);
+
+    /* Check hash IRQ status is asserted */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
+
+    /* Clear IRQ status and check status is deasserted */
+    qtest_writel(s, base + HACE_STS, 0x00000200);
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Read computed digest from memory */
+    qtest_memread(s, digest_addr, digest, sizeof(digest));
+
+    /* Check result of computation */
+    g_assert_cmpmem(digest, sizeof(digest),
+                    test_result_sha384, sizeof(digest));
+
+    qtest_quit(s);
+}
+
 void aspeed_test_sha512(const char *machine, const uint32_t base,
                         const uint32_t src_addr)
 {
@@ -280,6 +350,57 @@ void aspeed_test_sha256_sg(const char *machine, const uint32_t base,
     qtest_quit(s);
 }
 
+void aspeed_test_sha384_sg(const char *machine, const uint32_t base,
+                           const uint32_t src_addr)
+{
+    QTestState *s = qtest_init(machine);
+
+    const uint32_t src_addr_1 = src_addr + 0x10000;
+    const uint32_t src_addr_2 = src_addr + 0x20000;
+    const uint32_t src_addr_3 = src_addr + 0x30000;
+    const uint32_t digest_addr = src_addr + 0x40000;
+    uint8_t digest[48] = {0};
+    struct AspeedSgList array[] = {
+        {  cpu_to_le32(sizeof(test_vector_sg1)),
+           cpu_to_le32(src_addr_1) },
+        {  cpu_to_le32(sizeof(test_vector_sg2)),
+           cpu_to_le32(src_addr_2) },
+        {  cpu_to_le32(sizeof(test_vector_sg3) | SG_LIST_LEN_LAST),
+           cpu_to_le32(src_addr_3) },
+    };
+
+    /* Check engine is idle, no busy or irq bits set */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Write test vector into memory */
+    qtest_memwrite(s, src_addr_1, test_vector_sg1, sizeof(test_vector_sg1));
+    qtest_memwrite(s, src_addr_2, test_vector_sg2, sizeof(test_vector_sg2));
+    qtest_memwrite(s, src_addr_3, test_vector_sg3, sizeof(test_vector_sg3));
+    qtest_memwrite(s, src_addr, array, sizeof(array));
+
+    write_regs(s, base, src_addr,
+               (sizeof(test_vector_sg1)
+                + sizeof(test_vector_sg2)
+                + sizeof(test_vector_sg3)),
+               digest_addr, HACE_ALGO_SHA384 | HACE_SG_EN);
+
+    /* Check hash IRQ status is asserted */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
+
+    /* Clear IRQ status and check status is deasserted */
+    qtest_writel(s, base + HACE_STS, 0x00000200);
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Read computed digest from memory */
+    qtest_memread(s, digest_addr, digest, sizeof(digest));
+
+    /* Check result of computation */
+    g_assert_cmpmem(digest, sizeof(digest),
+                    test_result_sg_sha384, sizeof(digest));
+
+    qtest_quit(s);
+}
+
 void aspeed_test_sha512_sg(const char *machine, const uint32_t base,
                            const uint32_t src_addr)
 {
@@ -372,6 +493,47 @@ void aspeed_test_sha256_accum(const char *machine, const uint32_t base,
     qtest_quit(s);
 }
 
+void aspeed_test_sha384_accum(const char *machine, const uint32_t base,
+                              const uint32_t src_addr)
+{
+    QTestState *s = qtest_init(machine);
+
+    const uint32_t buffer_addr = src_addr + 0x10000;
+    const uint32_t digest_addr = src_addr + 0x40000;
+    uint8_t digest[48] = {0};
+    struct AspeedSgList array[] = {
+        {  cpu_to_le32(sizeof(test_vector_accum_384) | SG_LIST_LEN_LAST),
+           cpu_to_le32(buffer_addr) },
+    };
+
+    /* Check engine is idle, no busy or irq bits set */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Write test vector into memory */
+    qtest_memwrite(s, buffer_addr, test_vector_accum_384,
+                   sizeof(test_vector_accum_384));
+    qtest_memwrite(s, src_addr, array, sizeof(array));
+
+    write_regs(s, base, src_addr, sizeof(test_vector_accum_384),
+               digest_addr, HACE_ALGO_SHA384 | HACE_SG_EN | HACE_ACCUM_EN);
+
+    /* Check hash IRQ status is asserted */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
+
+    /* Clear IRQ status and check status is deasserted */
+    qtest_writel(s, base + HACE_STS, 0x00000200);
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Read computed digest from memory */
+    qtest_memread(s, digest_addr, digest, sizeof(digest));
+
+    /* Check result of computation */
+    g_assert_cmpmem(digest, sizeof(digest),
+                    test_result_accum_sha384, sizeof(digest));
+
+    qtest_quit(s);
+}
+
 void aspeed_test_sha512_accum(const char *machine, const uint32_t base,
                               const uint32_t src_addr)
 {
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 22/28] test/qtest/hace: Add SHA-384 tests for AST2600
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (20 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 21/28] test/qtest/hace: Add SHA-384 test cases for ASPEED HACE model Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 23/28] test/qtest/hace: Add tests for AST1030 Jamin Lin via
                   ` (9 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee, Cédric Le Goater

Introduced "test_sha384_ast2600" to validate SHA-384 hashing.
Added "test_sha384_sg_ast2600" for scatter-gather SHA-384 verification.
Implemented "test_sha384_accum_ast2600" to test SHA-384 accumulation.
Registered new test cases in "main" to ensure execution.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
 tests/qtest/aspeed_hace-test.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tests/qtest/aspeed_hace-test.c b/tests/qtest/aspeed_hace-test.c
index 42a306af2a..ab0c98330e 100644
--- a/tests/qtest/aspeed_hace-test.c
+++ b/tests/qtest/aspeed_hace-test.c
@@ -44,6 +44,16 @@ static void test_sha256_sg_ast2600(void)
     aspeed_test_sha256_sg("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
 }
 
+static void test_sha384_ast2600(void)
+{
+    aspeed_test_sha384("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
+}
+
+static void test_sha384_sg_ast2600(void)
+{
+    aspeed_test_sha384_sg("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
+}
+
 static void test_sha512_ast2600(void)
 {
     aspeed_test_sha512("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
@@ -59,6 +69,11 @@ static void test_sha256_accum_ast2600(void)
     aspeed_test_sha256_accum("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
 }
 
+static void test_sha384_accum_ast2600(void)
+{
+    aspeed_test_sha384_accum("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
+}
+
 static void test_sha512_accum_ast2600(void)
 {
     aspeed_test_sha512_accum("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
@@ -117,13 +132,16 @@ int main(int argc, char **argv)
 
     qtest_add_func("ast2600/hace/addresses", test_addresses_ast2600);
     qtest_add_func("ast2600/hace/sha512", test_sha512_ast2600);
+    qtest_add_func("ast2600/hace/sha384", test_sha384_ast2600);
     qtest_add_func("ast2600/hace/sha256", test_sha256_ast2600);
     qtest_add_func("ast2600/hace/md5", test_md5_ast2600);
 
     qtest_add_func("ast2600/hace/sha512_sg", test_sha512_sg_ast2600);
+    qtest_add_func("ast2600/hace/sha384_sg", test_sha384_sg_ast2600);
     qtest_add_func("ast2600/hace/sha256_sg", test_sha256_sg_ast2600);
 
     qtest_add_func("ast2600/hace/sha512_accum", test_sha512_accum_ast2600);
+    qtest_add_func("ast2600/hace/sha384_accum", test_sha384_accum_ast2600);
     qtest_add_func("ast2600/hace/sha256_accum", test_sha256_accum_ast2600);
 
     qtest_add_func("ast2500/hace/addresses", test_addresses_ast2500);
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 23/28] test/qtest/hace: Add tests for AST1030
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (21 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 22/28] test/qtest/hace: Add SHA-384 tests for AST2600 Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 24/28] test/qtest/hace: Update source data and digest data type to 64-bit Jamin Lin via
                   ` (8 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee, Cédric Le Goater

The HACE model in AST2600 and AST1030 is identical. Referencing the AST2600
test cases, new tests have been created for AST1030.

Implemented test functions for SHA-256, SHA-384, SHA-512, and MD5.
Added scatter-gather and accumulation test variants.
For AST1030, the HACE controller base address starts at "0x7e6d0000", and the
SDRAM start address is "0x0".

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
 tests/qtest/aspeed_hace-test.c | 76 ++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/tests/qtest/aspeed_hace-test.c b/tests/qtest/aspeed_hace-test.c
index ab0c98330e..31890d574e 100644
--- a/tests/qtest/aspeed_hace-test.c
+++ b/tests/qtest/aspeed_hace-test.c
@@ -10,6 +10,12 @@
 #include "qemu/bitops.h"
 #include "aspeed-hace-utils.h"
 
+static const struct AspeedMasks ast1030_masks = {
+    .src  = 0x7fffffff,
+    .dest = 0x7ffffff8,
+    .len  = 0x0fffffff,
+};
+
 static const struct AspeedMasks ast2600_masks = {
     .src  = 0x7fffffff,
     .dest = 0x7ffffff8,
@@ -28,6 +34,62 @@ static const struct AspeedMasks ast2400_masks = {
     .len  = 0x0fffffff,
 };
 
+/* ast1030 */
+static void test_md5_ast1030(void)
+{
+    aspeed_test_md5("-machine ast1030-evb", 0x7e6d0000, 0x00000000);
+}
+
+static void test_sha256_ast1030(void)
+{
+    aspeed_test_sha256("-machine ast1030-evb", 0x7e6d0000, 0x00000000);
+}
+
+static void test_sha256_sg_ast1030(void)
+{
+    aspeed_test_sha256_sg("-machine ast1030-evb", 0x7e6d0000, 0x00000000);
+}
+
+static void test_sha384_ast1030(void)
+{
+    aspeed_test_sha384("-machine ast1030-evb", 0x7e6d0000, 0x00000000);
+}
+
+static void test_sha384_sg_ast1030(void)
+{
+    aspeed_test_sha384_sg("-machine ast1030-evb", 0x7e6d0000, 0x00000000);
+}
+
+static void test_sha512_ast1030(void)
+{
+    aspeed_test_sha512("-machine ast1030-evb", 0x7e6d0000, 0x00000000);
+}
+
+static void test_sha512_sg_ast1030(void)
+{
+    aspeed_test_sha512_sg("-machine ast1030-evb", 0x7e6d0000, 0x00000000);
+}
+
+static void test_sha256_accum_ast1030(void)
+{
+    aspeed_test_sha256_accum("-machine ast1030-evb", 0x7e6d0000, 0x00000000);
+}
+
+static void test_sha384_accum_ast1030(void)
+{
+    aspeed_test_sha384_accum("-machine ast1030-evb", 0x7e6d0000, 0x00000000);
+}
+
+static void test_sha512_accum_ast1030(void)
+{
+    aspeed_test_sha512_accum("-machine ast1030-evb", 0x7e6d0000, 0x00000000);
+}
+
+static void test_addresses_ast1030(void)
+{
+    aspeed_test_addresses("-machine ast1030-evb", 0x7e6d0000, &ast1030_masks);
+}
+
 /* ast2600 */
 static void test_md5_ast2600(void)
 {
@@ -130,6 +192,20 @@ int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
 
+    qtest_add_func("ast1030/hace/addresses", test_addresses_ast1030);
+    qtest_add_func("ast1030/hace/sha512", test_sha512_ast1030);
+    qtest_add_func("ast1030/hace/sha384", test_sha384_ast1030);
+    qtest_add_func("ast1030/hace/sha256", test_sha256_ast1030);
+    qtest_add_func("ast1030/hace/md5", test_md5_ast1030);
+
+    qtest_add_func("ast1030/hace/sha512_sg", test_sha512_sg_ast1030);
+    qtest_add_func("ast1030/hace/sha384_sg", test_sha384_sg_ast1030);
+    qtest_add_func("ast1030/hace/sha256_sg", test_sha256_sg_ast1030);
+
+    qtest_add_func("ast1030/hace/sha512_accum", test_sha512_accum_ast1030);
+    qtest_add_func("ast1030/hace/sha384_accum", test_sha384_accum_ast1030);
+    qtest_add_func("ast1030/hace/sha256_accum", test_sha256_accum_ast1030);
+
     qtest_add_func("ast2600/hace/addresses", test_addresses_ast2600);
     qtest_add_func("ast2600/hace/sha512", test_sha512_ast2600);
     qtest_add_func("ast2600/hace/sha384", test_sha384_ast2600);
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 24/28] test/qtest/hace: Update source data and digest data type to 64-bit
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (22 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 23/28] test/qtest/hace: Add tests for AST1030 Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 25/28] test/qtest/hace: Support 64-bit source and digest addresses for AST2700 Jamin Lin via
                   ` (7 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

Currently, the hash data source and digest result buffer addresses are set to
32-bit. However, the AST2700 CPU is a 64-bit Cortex-A35 architecture, and its
DRAM base address is also 64-bit.

To support AST2700, update the hash data source address and digest result buffer
address to use 64-bit addressing.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 tests/qtest/aspeed-hace-utils.h | 20 ++++-----
 tests/qtest/aspeed-hace-utils.c | 72 ++++++++++++++++-----------------
 2 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/tests/qtest/aspeed-hace-utils.h b/tests/qtest/aspeed-hace-utils.h
index f4440561de..0382570fa2 100644
--- a/tests/qtest/aspeed-hace-utils.h
+++ b/tests/qtest/aspeed-hace-utils.h
@@ -51,25 +51,25 @@ struct AspeedMasks {
 };
 
 void aspeed_test_md5(const char *machine, const uint32_t base,
-                     const uint32_t src_addr);
+                     const uint64_t src_addr);
 void aspeed_test_sha256(const char *machine, const uint32_t base,
-                        const uint32_t src_addr);
+                        const uint64_t src_addr);
 void aspeed_test_sha384(const char *machine, const uint32_t base,
-                        const uint32_t src_addr);
+                        const uint64_t src_addr);
 void aspeed_test_sha512(const char *machine, const uint32_t base,
-                        const uint32_t src_addr);
+                        const uint64_t src_addr);
 void aspeed_test_sha256_sg(const char *machine, const uint32_t base,
-                           const uint32_t src_addr);
+                           const uint64_t src_addr);
 void aspeed_test_sha384_sg(const char *machine, const uint32_t base,
-                           const uint32_t src_addr);
+                           const uint64_t src_addr);
 void aspeed_test_sha512_sg(const char *machine, const uint32_t base,
-                           const uint32_t src_addr);
+                           const uint64_t src_addr);
 void aspeed_test_sha256_accum(const char *machine, const uint32_t base,
-                              const uint32_t src_addr);
+                              const uint64_t src_addr);
 void aspeed_test_sha384_accum(const char *machine, const uint32_t base,
-                              const uint32_t src_addr);
+                              const uint64_t src_addr);
 void aspeed_test_sha512_accum(const char *machine, const uint32_t base,
-                              const uint32_t src_addr);
+                              const uint64_t src_addr);
 void aspeed_test_addresses(const char *machine, const uint32_t base,
                            const struct AspeedMasks *expected);
 
diff --git a/tests/qtest/aspeed-hace-utils.c b/tests/qtest/aspeed-hace-utils.c
index dad90ee81c..1b54870dd4 100644
--- a/tests/qtest/aspeed-hace-utils.c
+++ b/tests/qtest/aspeed-hace-utils.c
@@ -153,22 +153,22 @@ static const uint8_t test_result_accum_sha256[32] = {
     0x5d, 0xae, 0x22, 0x23, 0xb0, 0x03, 0x61, 0xa3, 0x96, 0x17, 0x7a, 0x9c,
     0xb4, 0x10, 0xff, 0x61, 0xf2, 0x00, 0x15, 0xad};
 
-static void write_regs(QTestState *s, uint32_t base, uint32_t src,
-                       uint32_t length, uint32_t out, uint32_t method)
+static void write_regs(QTestState *s, uint32_t base, uint64_t src,
+                       uint32_t length, uint64_t out, uint32_t method)
 {
-        qtest_writel(s, base + HACE_HASH_SRC, src);
-        qtest_writel(s, base + HACE_HASH_DIGEST, out);
+        qtest_writel(s, base + HACE_HASH_SRC, extract64(src, 0, 32));
+        qtest_writel(s, base + HACE_HASH_DIGEST, extract64(out, 0, 32));
         qtest_writel(s, base + HACE_HASH_DATA_LEN, length);
         qtest_writel(s, base + HACE_HASH_CMD, HACE_SHA_BE_EN | method);
 }
 
 void aspeed_test_md5(const char *machine, const uint32_t base,
-                     const uint32_t src_addr)
+                     const uint64_t src_addr)
 
 {
     QTestState *s = qtest_init(machine);
 
-    uint32_t digest_addr = src_addr + 0x010000;
+    uint64_t digest_addr = src_addr + 0x010000;
     uint8_t digest[16] = {0};
 
     /* Check engine is idle, no busy or irq bits set */
@@ -198,11 +198,11 @@ void aspeed_test_md5(const char *machine, const uint32_t base,
 }
 
 void aspeed_test_sha256(const char *machine, const uint32_t base,
-                        const uint32_t src_addr)
+                        const uint64_t src_addr)
 {
     QTestState *s = qtest_init(machine);
 
-    const uint32_t digest_addr = src_addr + 0x10000;
+    const uint64_t digest_addr = src_addr + 0x10000;
     uint8_t digest[32] = {0};
 
     /* Check engine is idle, no busy or irq bits set */
@@ -232,11 +232,11 @@ void aspeed_test_sha256(const char *machine, const uint32_t base,
 }
 
 void aspeed_test_sha384(const char *machine, const uint32_t base,
-                        const uint32_t src_addr)
+                        const uint64_t src_addr)
 {
     QTestState *s = qtest_init(machine);
 
-    const uint32_t digest_addr = src_addr + 0x10000;
+    const uint64_t digest_addr = src_addr + 0x10000;
     uint8_t digest[48] = {0};
 
     /* Check engine is idle, no busy or irq bits set */
@@ -266,11 +266,11 @@ void aspeed_test_sha384(const char *machine, const uint32_t base,
 }
 
 void aspeed_test_sha512(const char *machine, const uint32_t base,
-                        const uint32_t src_addr)
+                        const uint64_t src_addr)
 {
     QTestState *s = qtest_init(machine);
 
-    const uint32_t digest_addr = src_addr + 0x10000;
+    const uint64_t digest_addr = src_addr + 0x10000;
     uint8_t digest[64] = {0};
 
     /* Check engine is idle, no busy or irq bits set */
@@ -300,14 +300,14 @@ void aspeed_test_sha512(const char *machine, const uint32_t base,
 }
 
 void aspeed_test_sha256_sg(const char *machine, const uint32_t base,
-                           const uint32_t src_addr)
+                           const uint64_t src_addr)
 {
     QTestState *s = qtest_init(machine);
 
-    const uint32_t src_addr_1 = src_addr + 0x10000;
-    const uint32_t src_addr_2 = src_addr + 0x20000;
-    const uint32_t src_addr_3 = src_addr + 0x30000;
-    const uint32_t digest_addr = src_addr + 0x40000;
+    const uint64_t src_addr_1 = src_addr + 0x10000;
+    const uint64_t src_addr_2 = src_addr + 0x20000;
+    const uint64_t src_addr_3 = src_addr + 0x30000;
+    const uint64_t digest_addr = src_addr + 0x40000;
     uint8_t digest[32] = {0};
     struct AspeedSgList array[] = {
         {  cpu_to_le32(sizeof(test_vector_sg1)),
@@ -351,14 +351,14 @@ void aspeed_test_sha256_sg(const char *machine, const uint32_t base,
 }
 
 void aspeed_test_sha384_sg(const char *machine, const uint32_t base,
-                           const uint32_t src_addr)
+                           const uint64_t src_addr)
 {
     QTestState *s = qtest_init(machine);
 
-    const uint32_t src_addr_1 = src_addr + 0x10000;
-    const uint32_t src_addr_2 = src_addr + 0x20000;
-    const uint32_t src_addr_3 = src_addr + 0x30000;
-    const uint32_t digest_addr = src_addr + 0x40000;
+    const uint64_t src_addr_1 = src_addr + 0x10000;
+    const uint64_t src_addr_2 = src_addr + 0x20000;
+    const uint64_t src_addr_3 = src_addr + 0x30000;
+    const uint64_t digest_addr = src_addr + 0x40000;
     uint8_t digest[48] = {0};
     struct AspeedSgList array[] = {
         {  cpu_to_le32(sizeof(test_vector_sg1)),
@@ -402,14 +402,14 @@ void aspeed_test_sha384_sg(const char *machine, const uint32_t base,
 }
 
 void aspeed_test_sha512_sg(const char *machine, const uint32_t base,
-                           const uint32_t src_addr)
+                           const uint64_t src_addr)
 {
     QTestState *s = qtest_init(machine);
 
-    const uint32_t src_addr_1 = src_addr + 0x10000;
-    const uint32_t src_addr_2 = src_addr + 0x20000;
-    const uint32_t src_addr_3 = src_addr + 0x30000;
-    const uint32_t digest_addr = src_addr + 0x40000;
+    const uint64_t src_addr_1 = src_addr + 0x10000;
+    const uint64_t src_addr_2 = src_addr + 0x20000;
+    const uint64_t src_addr_3 = src_addr + 0x30000;
+    const uint64_t digest_addr = src_addr + 0x40000;
     uint8_t digest[64] = {0};
     struct AspeedSgList array[] = {
         {  cpu_to_le32(sizeof(test_vector_sg1)),
@@ -453,12 +453,12 @@ void aspeed_test_sha512_sg(const char *machine, const uint32_t base,
 }
 
 void aspeed_test_sha256_accum(const char *machine, const uint32_t base,
-                              const uint32_t src_addr)
+                              const uint64_t src_addr)
 {
     QTestState *s = qtest_init(machine);
 
-    const uint32_t buffer_addr = src_addr + 0x10000;
-    const uint32_t digest_addr = src_addr + 0x40000;
+    const uint64_t buffer_addr = src_addr + 0x10000;
+    const uint64_t digest_addr = src_addr + 0x40000;
     uint8_t digest[32] = {0};
     struct AspeedSgList array[] = {
         {  cpu_to_le32(sizeof(test_vector_accum_256) | SG_LIST_LEN_LAST),
@@ -494,12 +494,12 @@ void aspeed_test_sha256_accum(const char *machine, const uint32_t base,
 }
 
 void aspeed_test_sha384_accum(const char *machine, const uint32_t base,
-                              const uint32_t src_addr)
+                              const uint64_t src_addr)
 {
     QTestState *s = qtest_init(machine);
 
-    const uint32_t buffer_addr = src_addr + 0x10000;
-    const uint32_t digest_addr = src_addr + 0x40000;
+    const uint64_t buffer_addr = src_addr + 0x10000;
+    const uint64_t digest_addr = src_addr + 0x40000;
     uint8_t digest[48] = {0};
     struct AspeedSgList array[] = {
         {  cpu_to_le32(sizeof(test_vector_accum_384) | SG_LIST_LEN_LAST),
@@ -535,12 +535,12 @@ void aspeed_test_sha384_accum(const char *machine, const uint32_t base,
 }
 
 void aspeed_test_sha512_accum(const char *machine, const uint32_t base,
-                              const uint32_t src_addr)
+                              const uint64_t src_addr)
 {
     QTestState *s = qtest_init(machine);
 
-    const uint32_t buffer_addr = src_addr + 0x10000;
-    const uint32_t digest_addr = src_addr + 0x40000;
+    const uint64_t buffer_addr = src_addr + 0x10000;
+    const uint64_t digest_addr = src_addr + 0x40000;
     uint8_t digest[64] = {0};
     struct AspeedSgList array[] = {
         {  cpu_to_le32(sizeof(test_vector_accum_512) | SG_LIST_LEN_LAST),
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 25/28] test/qtest/hace: Support 64-bit source and digest addresses for AST2700
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (23 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 24/28] test/qtest/hace: Update source data and digest data type to 64-bit Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 26/28] test/qtest/hace: Support to test upper 32 bits of digest and source addresses Jamin Lin via
                   ` (6 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee, Cédric Le Goater

Added "HACE_HASH_SRC_HI" and "HACE_HASH_DIGEST_HI", "HACE_HASH_KEY_BUFF_HI"
registers to store upper 32 bits.
Updated "write_regs" to handle 64-bit source and digest addresses.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
 tests/qtest/aspeed-hace-utils.h | 3 +++
 tests/qtest/aspeed-hace-utils.c | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/tests/qtest/aspeed-hace-utils.h b/tests/qtest/aspeed-hace-utils.h
index 0382570fa2..d8684d3f83 100644
--- a/tests/qtest/aspeed-hace-utils.h
+++ b/tests/qtest/aspeed-hace-utils.h
@@ -36,6 +36,9 @@
 #define HACE_HASH_KEY_BUFF       0x28
 #define HACE_HASH_DATA_LEN       0x2c
 #define HACE_HASH_CMD            0x30
+#define HACE_HASH_SRC_HI         0x90
+#define HACE_HASH_DIGEST_HI      0x94
+#define HACE_HASH_KEY_BUFF_HI    0x98
 
 /* Scatter-Gather Hash */
 #define SG_LIST_LEN_LAST         BIT(31)
diff --git a/tests/qtest/aspeed-hace-utils.c b/tests/qtest/aspeed-hace-utils.c
index 1b54870dd4..842bf5630d 100644
--- a/tests/qtest/aspeed-hace-utils.c
+++ b/tests/qtest/aspeed-hace-utils.c
@@ -157,7 +157,9 @@ static void write_regs(QTestState *s, uint32_t base, uint64_t src,
                        uint32_t length, uint64_t out, uint32_t method)
 {
         qtest_writel(s, base + HACE_HASH_SRC, extract64(src, 0, 32));
+        qtest_writel(s, base + HACE_HASH_SRC_HI, extract64(src, 32, 32));
         qtest_writel(s, base + HACE_HASH_DIGEST, extract64(out, 0, 32));
+        qtest_writel(s, base + HACE_HASH_DIGEST_HI, extract64(out, 32, 32));
         qtest_writel(s, base + HACE_HASH_DATA_LEN, length);
         qtest_writel(s, base + HACE_HASH_CMD, HACE_SHA_BE_EN | method);
 }
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 26/28] test/qtest/hace: Support to test upper 32 bits of digest and source addresses
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (24 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 25/28] test/qtest/hace: Support 64-bit source and digest addresses for AST2700 Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:09 ` [PATCH v3 27/28] test/qtest/hace: Support to validate 64-bit hmac key buffer addresses Jamin Lin via
                   ` (5 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee, Cédric Le Goater

Added "src_hi" and "dest_hi" fields to "AspeedMasks" for 64-bit addresses test.
Updated "aspeed_test_addresses" to validate "HACE_HASH_SRC_HI" and
"HACE_HASH_DIGEST_HI".
Ensured correct masking of 64-bit addresses by checking both lower and upper
32-bit registers.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
 tests/qtest/aspeed-hace-utils.h |  2 ++
 tests/qtest/aspeed-hace-utils.c | 15 ++++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/aspeed-hace-utils.h b/tests/qtest/aspeed-hace-utils.h
index d8684d3f83..de8055a1db 100644
--- a/tests/qtest/aspeed-hace-utils.h
+++ b/tests/qtest/aspeed-hace-utils.h
@@ -51,6 +51,8 @@ struct AspeedMasks {
     uint32_t src;
     uint32_t dest;
     uint32_t len;
+    uint32_t src_hi;
+    uint32_t dest_hi;
 };
 
 void aspeed_test_md5(const char *machine, const uint32_t base,
diff --git a/tests/qtest/aspeed-hace-utils.c b/tests/qtest/aspeed-hace-utils.c
index 842bf5630d..cb78f18117 100644
--- a/tests/qtest/aspeed-hace-utils.c
+++ b/tests/qtest/aspeed-hace-utils.c
@@ -588,30 +588,43 @@ void aspeed_test_addresses(const char *machine, const uint32_t base,
      */
     g_assert_cmphex(qtest_readl(s, base + HACE_CMD), ==, 0);
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC), ==, 0);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC_HI), ==, 0);
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST), ==, 0);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST_HI), ==, 0);
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==, 0);
 
-
     /* Check that the address masking is correct */
     qtest_writel(s, base + HACE_HASH_SRC, 0xffffffff);
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC), ==, expected->src);
 
+    qtest_writel(s, base + HACE_HASH_SRC_HI, 0xffffffff);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC_HI),
+                    ==, expected->src_hi);
+
     qtest_writel(s, base + HACE_HASH_DIGEST, 0xffffffff);
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST), ==,
                     expected->dest);
 
+    qtest_writel(s, base + HACE_HASH_DIGEST_HI, 0xffffffff);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST_HI), ==,
+                    expected->dest_hi);
+
     qtest_writel(s, base + HACE_HASH_DATA_LEN, 0xffffffff);
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==,
                     expected->len);
 
     /* Reset to zero */
     qtest_writel(s, base + HACE_HASH_SRC, 0);
+    qtest_writel(s, base + HACE_HASH_SRC_HI, 0);
     qtest_writel(s, base + HACE_HASH_DIGEST, 0);
+    qtest_writel(s, base + HACE_HASH_DIGEST_HI, 0);
     qtest_writel(s, base + HACE_HASH_DATA_LEN, 0);
 
     /* Check that all bits are now zero */
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC), ==, 0);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC_HI), ==, 0);
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST), ==, 0);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST_HI), ==, 0);
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==, 0);
 
     qtest_quit(s);
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 27/28] test/qtest/hace: Support to validate 64-bit hmac key buffer addresses
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (25 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 26/28] test/qtest/hace: Support to test upper 32 bits of digest and source addresses Jamin Lin via
@ 2025-05-15  8:09 ` Jamin Lin via
  2025-05-15  8:10 ` [PATCH v3 28/28] test/qtest/hace: Add tests for AST2700 Jamin Lin via
                   ` (4 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:09 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee, Cédric Le Goater

Added "key" and "key_hi" fields to "AspeedMasks" for 64-bit addresses test.
Updated "aspeed_test_addresses" to validate "HACE_HASH_KEY_BUFF" and
"HACE_HASH_KEY_BUFF_HI".
Ensured correct masking of 64-bit addresses by checking both lower and upper
32-bit registers.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
 tests/qtest/aspeed-hace-utils.h |  2 ++
 tests/qtest/aspeed-hace-utils.c | 14 ++++++++++++++
 tests/qtest/aspeed_hace-test.c  |  4 ++++
 3 files changed, 20 insertions(+)

diff --git a/tests/qtest/aspeed-hace-utils.h b/tests/qtest/aspeed-hace-utils.h
index de8055a1db..c8b2ec45af 100644
--- a/tests/qtest/aspeed-hace-utils.h
+++ b/tests/qtest/aspeed-hace-utils.h
@@ -50,9 +50,11 @@ struct AspeedSgList {
 struct AspeedMasks {
     uint32_t src;
     uint32_t dest;
+    uint32_t key;
     uint32_t len;
     uint32_t src_hi;
     uint32_t dest_hi;
+    uint32_t key_hi;
 };
 
 void aspeed_test_md5(const char *machine, const uint32_t base,
diff --git a/tests/qtest/aspeed-hace-utils.c b/tests/qtest/aspeed-hace-utils.c
index cb78f18117..0f7f911e5e 100644
--- a/tests/qtest/aspeed-hace-utils.c
+++ b/tests/qtest/aspeed-hace-utils.c
@@ -591,6 +591,8 @@ void aspeed_test_addresses(const char *machine, const uint32_t base,
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC_HI), ==, 0);
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST), ==, 0);
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST_HI), ==, 0);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_KEY_BUFF), ==, 0);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_KEY_BUFF_HI), ==, 0);
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==, 0);
 
     /* Check that the address masking is correct */
@@ -609,6 +611,14 @@ void aspeed_test_addresses(const char *machine, const uint32_t base,
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST_HI), ==,
                     expected->dest_hi);
 
+    qtest_writel(s, base + HACE_HASH_KEY_BUFF, 0xffffffff);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_KEY_BUFF), ==,
+                    expected->key);
+
+    qtest_writel(s, base + HACE_HASH_KEY_BUFF_HI, 0xffffffff);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_KEY_BUFF_HI), ==,
+                    expected->key_hi);
+
     qtest_writel(s, base + HACE_HASH_DATA_LEN, 0xffffffff);
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==,
                     expected->len);
@@ -618,6 +628,8 @@ void aspeed_test_addresses(const char *machine, const uint32_t base,
     qtest_writel(s, base + HACE_HASH_SRC_HI, 0);
     qtest_writel(s, base + HACE_HASH_DIGEST, 0);
     qtest_writel(s, base + HACE_HASH_DIGEST_HI, 0);
+    qtest_writel(s, base + HACE_HASH_KEY_BUFF, 0);
+    qtest_writel(s, base + HACE_HASH_KEY_BUFF_HI, 0);
     qtest_writel(s, base + HACE_HASH_DATA_LEN, 0);
 
     /* Check that all bits are now zero */
@@ -625,6 +637,8 @@ void aspeed_test_addresses(const char *machine, const uint32_t base,
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC_HI), ==, 0);
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST), ==, 0);
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST_HI), ==, 0);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_KEY_BUFF), ==, 0);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_KEY_BUFF_HI), ==, 0);
     g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==, 0);
 
     qtest_quit(s);
diff --git a/tests/qtest/aspeed_hace-test.c b/tests/qtest/aspeed_hace-test.c
index 31890d574e..38777020ca 100644
--- a/tests/qtest/aspeed_hace-test.c
+++ b/tests/qtest/aspeed_hace-test.c
@@ -13,24 +13,28 @@
 static const struct AspeedMasks ast1030_masks = {
     .src  = 0x7fffffff,
     .dest = 0x7ffffff8,
+    .key = 0x7ffffff8,
     .len  = 0x0fffffff,
 };
 
 static const struct AspeedMasks ast2600_masks = {
     .src  = 0x7fffffff,
     .dest = 0x7ffffff8,
+    .key = 0x7ffffff8,
     .len  = 0x0fffffff,
 };
 
 static const struct AspeedMasks ast2500_masks = {
     .src  = 0x3fffffff,
     .dest = 0x3ffffff8,
+    .key = 0x3fffffc0,
     .len  = 0x0fffffff,
 };
 
 static const struct AspeedMasks ast2400_masks = {
     .src  = 0x0fffffff,
     .dest = 0x0ffffff8,
+    .key = 0x0fffffc0,
     .len  = 0x0fffffff,
 };
 
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 28/28] test/qtest/hace: Add tests for AST2700
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (26 preceding siblings ...)
  2025-05-15  8:09 ` [PATCH v3 27/28] test/qtest/hace: Support to validate 64-bit hmac key buffer addresses Jamin Lin via
@ 2025-05-15  8:10 ` Jamin Lin via
  2025-05-20 14:58 ` [PATCH v3 00/28] Fix incorrect hash results on AST2700 Fabiano Rosas
                   ` (3 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Jamin Lin via @ 2025-05-15  8:10 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee, Cédric Le Goater

The HACE models in AST2600 and AST2700 are nearly identical. Based on the
AST2600 test cases, new tests have been added for AST2700.

Implemented test functions for SHA-256, SHA-384, SHA-512, and MD5.
Added scatter-gather and accumulation test variants.
For AST2700, the HACE controller base address starts at "0x12070000", and
the DRAM start address is "0x4_00000000".

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
 tests/qtest/ast2700-hace-test.c | 98 +++++++++++++++++++++++++++++++++
 tests/qtest/meson.build         |  2 +
 2 files changed, 100 insertions(+)
 create mode 100644 tests/qtest/ast2700-hace-test.c

diff --git a/tests/qtest/ast2700-hace-test.c b/tests/qtest/ast2700-hace-test.c
new file mode 100644
index 0000000000..a400e2962b
--- /dev/null
+++ b/tests/qtest/ast2700-hace-test.c
@@ -0,0 +1,98 @@
+/*
+ * QTest testcase for the ASPEED Hash and Crypto Engine
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2025 ASPEED Technology Inc.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "qemu/bitops.h"
+#include "aspeed-hace-utils.h"
+
+static const struct AspeedMasks as2700_masks = {
+    .src  = 0x7fffffff,
+    .dest = 0x7ffffff8,
+    .key = 0x7ffffff8,
+    .len  = 0x0fffffff,
+    .src_hi  = 0x00000003,
+    .dest_hi = 0x00000003,
+    .key_hi = 0x00000003,
+};
+
+/* ast2700 */
+static void test_md5_ast2700(void)
+{
+    aspeed_test_md5("-machine ast2700a1-evb", 0x12070000, 0x400000000);
+}
+
+static void test_sha256_ast2700(void)
+{
+    aspeed_test_sha256("-machine ast2700a1-evb", 0x12070000, 0x400000000);
+}
+
+static void test_sha256_sg_ast2700(void)
+{
+    aspeed_test_sha256_sg("-machine ast2700a1-evb", 0x12070000, 0x400000000);
+}
+
+static void test_sha384_ast2700(void)
+{
+    aspeed_test_sha384("-machine ast2700a1-evb", 0x12070000, 0x400000000);
+}
+
+static void test_sha384_sg_ast2700(void)
+{
+    aspeed_test_sha384_sg("-machine ast2700a1-evb", 0x12070000, 0x400000000);
+}
+
+static void test_sha512_ast2700(void)
+{
+    aspeed_test_sha512("-machine ast2700a1-evb", 0x12070000, 0x400000000);
+}
+
+static void test_sha512_sg_ast2700(void)
+{
+    aspeed_test_sha512_sg("-machine ast2700a1-evb", 0x12070000, 0x400000000);
+}
+
+static void test_sha256_accum_ast2700(void)
+{
+    aspeed_test_sha256_accum("-machine ast2700a1-evb", 0x12070000, 0x400000000);
+}
+
+static void test_sha384_accum_ast2700(void)
+{
+    aspeed_test_sha384_accum("-machine ast2700a1-evb", 0x12070000, 0x400000000);
+}
+
+static void test_sha512_accum_ast2700(void)
+{
+    aspeed_test_sha512_accum("-machine ast2700a1-evb", 0x12070000, 0x400000000);
+}
+
+static void test_addresses_ast2700(void)
+{
+    aspeed_test_addresses("-machine ast2700a1-evb", 0x12070000, &as2700_masks);
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+
+    qtest_add_func("ast2700/hace/addresses", test_addresses_ast2700);
+    qtest_add_func("ast2700/hace/sha512", test_sha512_ast2700);
+    qtest_add_func("ast2700/hace/sha384", test_sha384_ast2700);
+    qtest_add_func("ast2700/hace/sha256", test_sha256_ast2700);
+    qtest_add_func("ast2700/hace/md5", test_md5_ast2700);
+
+    qtest_add_func("ast2700/hace/sha512_sg", test_sha512_sg_ast2700);
+    qtest_add_func("ast2700/hace/sha384_sg", test_sha384_sg_ast2700);
+    qtest_add_func("ast2700/hace/sha256_sg", test_sha256_sg_ast2700);
+
+    qtest_add_func("ast2700/hace/sha512_accum", test_sha512_accum_ast2700);
+    qtest_add_func("ast2700/hace/sha384_accum", test_sha384_accum_ast2700);
+    qtest_add_func("ast2700/hace/sha256_accum", test_sha256_accum_ast2700);
+
+    return g_test_run();
+}
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index bb14a22ebe..c587688f67 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -216,6 +216,7 @@ qtests_aspeed = \
    'aspeed_smc-test']
 qtests_aspeed64 = \
   ['ast2700-gpio-test',
+   'ast2700-hace-test',
    'ast2700-smc-test']
 
 qtests_stm32l4x5 = \
@@ -362,6 +363,7 @@ endif
 qtests = {
   'aspeed_hace-test': files('aspeed-hace-utils.c', 'aspeed_hace-test.c'),
   'aspeed_smc-test': files('aspeed-smc-utils.c', 'aspeed_smc-test.c'),
+  'ast2700-hace-test': files('aspeed-hace-utils.c', 'ast2700-hace-test.c'),
   'ast2700-smc-test': files('aspeed-smc-utils.c', 'ast2700-smc-test.c'),
   'bios-tables-test': [io, 'boot-sector.c', 'acpi-utils.c', 'tpm-emu.c'],
   'cdrom-test': files('boot-sector.c'),
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 00/28] Fix incorrect hash results on AST2700
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (27 preceding siblings ...)
  2025-05-15  8:10 ` [PATCH v3 28/28] test/qtest/hace: Add tests for AST2700 Jamin Lin via
@ 2025-05-20 14:58 ` Fabiano Rosas
  2025-05-23  7:17 ` Cédric Le Goater
                   ` (2 subsequent siblings)
  31 siblings, 0 replies; 37+ messages in thread
From: Fabiano Rosas @ 2025-05-20 14:58 UTC (permalink / raw)
  To: Jamin Lin via, Cédric Le Goater, Peter Maydell, Steven Lee,
	Troy Lee, Andrew Jeffery, Joel Stanley, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: jamin_lin, troy_lee

Jamin Lin via <qemu-devel@nongnu.org> writes:

> v1:
>  1. Added support for 64-bit DMA in the HACE model
>  2. Refactored the do_hash operation in the HACE model
>  3. Fixed a crash caused by out-of-bound memory access in HACE
>  4. Added more trace events and implemented dumping of source hash data and
>     resulting digests to improve debugging
>  5. Refactored the HACE QTest framework to support both AST1030 and AST2700
>  6. Added a test case for SHA384
>
> v2:
>   1. Create new helper functions
>      hash_get_source_addr
>      hash_prepare_direct_iov
>      hash_prepare_sg_iov
>      hash_get_digest_addr
>      hash_write_digest_and_unmap_iov
>      hash_execute_non_acc_mode
>      hash_execute_acc_mode
>   2. Refactor do_hash_operation
>   3. Fix review issue
>   4. Revise trace-events
>   5. Move register size to instance class and dynamically allocate regs
>
> v3:
>   1. Split patch to introduce these routines one by one : 
>        hash_prepare_sg_iov
>        hash_prepare_direct_iov
>        hash_execute_acc_mode
>        hash_execute_non_acc_mode
>        hash_write_digest_and_unmap_iov
>   2. Fix run qtest failed  
>  
> This patchset resolves incorrect hash results reported on the AST2700 platform.
> This update addresses the following kernel warnings and test failures related to
> the crypto self-test framework:
>
> aspeed-hmac-sha512 test failed (incorrect result)
> aspeed-hmac-sha384 test failed (incorrect result)
> aspeed-sha512 test failed (incorrect result)
> aspeed-sha384 test failed (incorrect result)
> aspeed-hmac-sha256 test failed (incorrect result)
> aspeed-hmac-sha224 test failed (incorrect result)
> aspeed-hmac-sha1 test failed (incorrect result)
> aspeed-sha224 test failed (incorrect result)
> aspeed-sha256 test failed (incorrect result)
> aspeed-sha1 test failed (incorrect result)
>
> How to test it
>
> Use the following command to dump information about the supported digest methods
> via the ast_crypto_engine hardware engine:
>
> root@ast2700-default:~# openssl engine -pre DUMP_INFO ast_crypto_engine
>
> Digest SHA1, NID=64, AF_ALG info: name=sha1ALG_ERR: , driver=aspeed-sha1 (hw accelerated)
> Digest SHA224, NID=675, AF_ALG info: name=sha224ALG_ERR: , driver=aspeed-sha224 (hw accelerated)
> Digest SHA256, NID=672, AF_ALG info: name=sha256ALG_ERR: , driver=aspeed-sha256 (hw accelerated)
> Digest SHA384, NID=673, AF_ALG info: name=sha384ALG_ERR: , driver=aspeed-sha384 (hw accelerated)
> Digest SHA512, NID=674, AF_ALG info: name=sha512ALG_ERR: , driver=aspeed-sha512 (hw accelerated)
>
> The status of SHA1, SHA224, SHA256, SHA384, and SHA512 should be marked as
> hw accelerated, indicating that these algorithms are supported by hardware
> acceleration via the aspeed drivers.
>
> Create a test file on the host machine and compute its HASH value as the
> expected result
>
> Create a 256MB test file
>
> $ dd if=/dev/random of=/tmp/256M bs=1M count=256
> Generate Hash Values Using SHA1, SHA224, SHA256, SHA384, and SHA512
>
> Use the following commands to generate HASH values for a 256MB file using
> different SHA algorithms:
>
> $ sha1sum /tmp/256M
> 7fc628811a31ab87b0502dab3ed8d3ef07565885  /tmp/256M
>
> $ sha224sum /tmp/256M
> 2d261c11ba05b3a62e0efeab51c307d9933426c7e18204683ef3da54  /tmp/256M
>
> $ sha256sum /tmp/256M
> 5716d1700ee35c92ca5ca5b466639e9c36eed3f1447c1aec27f16d0fe113f94d  /tmp/256M
>
> $ sha384sum /tmp/256M
> fb6bc62afa1096dcd3b870e7d2546b7a5a177b5f2bbd5c9759218182454709e0c504a2d9c26404e04aa8010a291b7f1c  /tmp/256M
>
> $ sha512sum /tmp/256M
> fbceda7be34836fe857781656318ecd5b457a833a24c8736d5b8ef8d07e1950eebcdb140eebe4f12b5ff59586f7eb1c64fa95869c63dd9e4703d91261093c5c9  /tmp/256M
>
> Generate HASH Values Using the Hardware Engine
>
> Use the following commands to generate HASH values for a 256MB file using
> various SHA algorithms with the ast_crypto_engine hardware engine:
>
> root@ast2700-default:~# openssl dgst -sha1 -engine ast_crypto_engine /tmp/256M
> Engine "ast_crypto_engine" set.
> SHA1(/tmp/256M)= 7fc628811a31ab87b0502dab3ed8d3ef07565885
>
> root@ast2700-default:~# openssl dgst -sha224 -engine ast_crypto_engine /tmp/256M
> Engine "ast_crypto_engine" set.
> SHA2-224(/tmp/256M)= 2d261c11ba05b3a62e0efeab51c307d9933426c7e18204683ef3da54
>
> root@ast2700-default:~# openssl dgst -sha256 -engine ast_crypto_engine /tmp/256M
> Engine "ast_crypto_engine" set.
> SHA2-256(/tmp/256M)= 5716d1700ee35c92ca5ca5b466639e9c36eed3f1447c1aec27f16d0fe113f94d
>
> root@ast2700-default:~# openssl dgst -sha384 -engine ast_crypto_engine /tmp/256M
> Engine "ast_crypto_engine" set.
> SHA2-384(/tmp/256M)= fb6bc62afa1096dcd3b870e7d2546b7a5a177b5f2bbd5c9759218182454709e0c504a2d9c26404e04aa8010a291b7f1c
>
> root@ast2700-default:~# openssl dgst -sha512 -engine ast_crypto_engine /tmp/256M
> Engine "ast_crypto_engine" set.
> SHA2-512(/tmp/256M)= fbceda7be34836fe857781656318ecd5b457a833a24c8736d5b8ef8d07e1950eebcdb140eebe4f12b5ff59586f7eb1c64fa95869c63dd9e4703d91261093c5c9
>
> The HASH values generated here should exactly match those computed on the host
> machine using sha shell commands, verifying both the correctness of the
> hardware-accelerated results and the functionality of the ast_crypto_engine.
>
> Jamin Lin (28):
>   hw/misc/aspeed_hace: Remove unused code for better readability
>   hw/misc/aspeed_hace: Improve readability and consistency in variable
>     naming
>   hw/misc/aspeed_hace: Ensure HASH_IRQ is always set to prevent firmware
>     hang
>   hw/misc/aspeed_hace: Extract direct mode hash buffer setup into helper
>     function
>   hw/misc/aspeed_hace: Extract SG-mode hash buffer setup into helper
>     function
>   hw/misc/aspeed_hace: Extract digest write and iov unmap into helper
>     function
>   hw/misc/aspeed_hace: Extract non-accumulation hash execution into
>     helper function
>   hw/misc/aspeed_hace: Extract accumulation-mode hash execution into
>     helper function
>   hw/misc/aspeed_hace: Introduce 64-bit hash source address helper
>     function
>   hw/misc/aspeed_hace: Rename R_HASH_DEST to R_HASH_DIGEST and introduce
>     64-bit hash digest address helper
>   hw/misc/aspeed_hace: Support accumulative mode for direct access mode
>   hw/misc/aspeed_hace: Move register size to instance class and
>     dynamically allocate regs
>   hw/misc/aspeed_hace: Add support for source, digest, key buffer 64 bit
>     addresses
>   hw/misc/aspeed_hace: Support DMA 64 bits dram address
>   hw/misc/aspeed_hace: Add trace-events for better debugging
>   hw/misc/aspeed_hace: Support to dump plaintext and digest for better
>     debugging
>   tests/qtest: Reorder aspeed test list
>   test/qtest: Introduce a new aspeed-hace-utils.c to place common
>     testcases
>   test/qtest/hace: Specify explicit array sizes for test vectors and
>     hash results
>   test/qtest/hace: Adjust test address range for AST1030 due to SRAM
>     limitations
>   test/qtest/hace: Add SHA-384 test cases for ASPEED HACE model
>   test/qtest/hace: Add SHA-384 tests for AST2600
>   test/qtest/hace: Add tests for AST1030
>   test/qtest/hace: Update source data and digest data type to 64-bit
>   test/qtest/hace: Support 64-bit source and digest addresses for
>     AST2700
>   test/qtest/hace: Support to test upper 32 bits of digest and source
>     addresses
>   test/qtest/hace: Support to validate 64-bit hmac key buffer addresses
>   test/qtest/hace: Add tests for AST2700
>
>  include/hw/misc/aspeed_hace.h   |  11 +-
>  tests/qtest/aspeed-hace-utils.h |  84 +++++
>  hw/misc/aspeed_hace.c           | 479 +++++++++++++++--------
>  tests/qtest/aspeed-hace-utils.c | 646 ++++++++++++++++++++++++++++++++
>  tests/qtest/aspeed_hace-test.c  | 577 +++++-----------------------
>  tests/qtest/ast2700-hace-test.c |  98 +++++
>  hw/misc/trace-events            |   8 +
>  tests/qtest/meson.build         |  13 +-
>  8 files changed, 1279 insertions(+), 637 deletions(-)
>  create mode 100644 tests/qtest/aspeed-hace-utils.h
>  create mode 100644 tests/qtest/aspeed-hace-utils.c
>  create mode 100644 tests/qtest/ast2700-hace-test.c

For qtest:

Acked-by: Fabiano Rosas <farosas@suse.de>


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 00/28] Fix incorrect hash results on AST2700
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (28 preceding siblings ...)
  2025-05-20 14:58 ` [PATCH v3 00/28] Fix incorrect hash results on AST2700 Fabiano Rosas
@ 2025-05-23  7:17 ` Cédric Le Goater
  2025-05-23  8:10 ` Cédric Le Goater
  2025-05-29  7:29 ` Michael Tokarev
  31 siblings, 0 replies; 37+ messages in thread
From: Cédric Le Goater @ 2025-05-23  7:17 UTC (permalink / raw)
  To: Jamin Lin, Peter Maydell, Steven Lee, Troy Lee, Andrew Jeffery,
	Joel Stanley, Fabiano Rosas, Laurent Vivier, Paolo Bonzini,
	open list:ASPEED BMCs, open list:All patches CC here
  Cc: troy_lee

On 5/15/25 10:09, Jamin Lin wrote:
> v1:
>   1. Added support for 64-bit DMA in the HACE model
>   2. Refactored the do_hash operation in the HACE model
>   3. Fixed a crash caused by out-of-bound memory access in HACE
>   4. Added more trace events and implemented dumping of source hash data and
>      resulting digests to improve debugging
>   5. Refactored the HACE QTest framework to support both AST1030 and AST2700
>   6. Added a test case for SHA384
> 
> v2:
>    1. Create new helper functions
>       hash_get_source_addr
>       hash_prepare_direct_iov
>       hash_prepare_sg_iov
>       hash_get_digest_addr
>       hash_write_digest_and_unmap_iov
>       hash_execute_non_acc_mode
>       hash_execute_acc_mode
>    2. Refactor do_hash_operation
>    3. Fix review issue
>    4. Revise trace-events
>    5. Move register size to instance class and dynamically allocate regs
> 
> v3:
>    1. Split patch to introduce these routines one by one :
>         hash_prepare_sg_iov
>         hash_prepare_direct_iov
>         hash_execute_acc_mode
>         hash_execute_non_acc_mode
>         hash_write_digest_and_unmap_iov
>    2. Fix run qtest failed
>   
> This patchset resolves incorrect hash results reported on the AST2700 platform.
> This update addresses the following kernel warnings and test failures related to
> the crypto self-test framework:
> 
> aspeed-hmac-sha512 test failed (incorrect result)
> aspeed-hmac-sha384 test failed (incorrect result)
> aspeed-sha512 test failed (incorrect result)
> aspeed-sha384 test failed (incorrect result)
> aspeed-hmac-sha256 test failed (incorrect result)
> aspeed-hmac-sha224 test failed (incorrect result)
> aspeed-hmac-sha1 test failed (incorrect result)
> aspeed-sha224 test failed (incorrect result)
> aspeed-sha256 test failed (incorrect result)
> aspeed-sha1 test failed (incorrect result)
> 
> How to test it
> 
> Use the following command to dump information about the supported digest methods
> via the ast_crypto_engine hardware engine:
> 
> root@ast2700-default:~# openssl engine -pre DUMP_INFO ast_crypto_engine
> 
> Digest SHA1, NID=64, AF_ALG info: name=sha1ALG_ERR: , driver=aspeed-sha1 (hw accelerated)
> Digest SHA224, NID=675, AF_ALG info: name=sha224ALG_ERR: , driver=aspeed-sha224 (hw accelerated)
> Digest SHA256, NID=672, AF_ALG info: name=sha256ALG_ERR: , driver=aspeed-sha256 (hw accelerated)
> Digest SHA384, NID=673, AF_ALG info: name=sha384ALG_ERR: , driver=aspeed-sha384 (hw accelerated)
> Digest SHA512, NID=674, AF_ALG info: name=sha512ALG_ERR: , driver=aspeed-sha512 (hw accelerated)
> 
> The status of SHA1, SHA224, SHA256, SHA384, and SHA512 should be marked as
> hw accelerated, indicating that these algorithms are supported by hardware
> acceleration via the aspeed drivers.
> 
> Create a test file on the host machine and compute its HASH value as the
> expected result
> 
> Create a 256MB test file
> 
> $ dd if=/dev/random of=/tmp/256M bs=1M count=256
> Generate Hash Values Using SHA1, SHA224, SHA256, SHA384, and SHA512
> 
> Use the following commands to generate HASH values for a 256MB file using
> different SHA algorithms:
> 
> $ sha1sum /tmp/256M
> 7fc628811a31ab87b0502dab3ed8d3ef07565885  /tmp/256M
> 
> $ sha224sum /tmp/256M
> 2d261c11ba05b3a62e0efeab51c307d9933426c7e18204683ef3da54  /tmp/256M
> 
> $ sha256sum /tmp/256M
> 5716d1700ee35c92ca5ca5b466639e9c36eed3f1447c1aec27f16d0fe113f94d  /tmp/256M
> 
> $ sha384sum /tmp/256M
> fb6bc62afa1096dcd3b870e7d2546b7a5a177b5f2bbd5c9759218182454709e0c504a2d9c26404e04aa8010a291b7f1c  /tmp/256M
> 
> $ sha512sum /tmp/256M
> fbceda7be34836fe857781656318ecd5b457a833a24c8736d5b8ef8d07e1950eebcdb140eebe4f12b5ff59586f7eb1c64fa95869c63dd9e4703d91261093c5c9  /tmp/256M
> 
> Generate HASH Values Using the Hardware Engine
> 
> Use the following commands to generate HASH values for a 256MB file using
> various SHA algorithms with the ast_crypto_engine hardware engine:
> 
> root@ast2700-default:~# openssl dgst -sha1 -engine ast_crypto_engine /tmp/256M
> Engine "ast_crypto_engine" set.
> SHA1(/tmp/256M)= 7fc628811a31ab87b0502dab3ed8d3ef07565885
> 
> root@ast2700-default:~# openssl dgst -sha224 -engine ast_crypto_engine /tmp/256M
> Engine "ast_crypto_engine" set.
> SHA2-224(/tmp/256M)= 2d261c11ba05b3a62e0efeab51c307d9933426c7e18204683ef3da54
> 
> root@ast2700-default:~# openssl dgst -sha256 -engine ast_crypto_engine /tmp/256M
> Engine "ast_crypto_engine" set.
> SHA2-256(/tmp/256M)= 5716d1700ee35c92ca5ca5b466639e9c36eed3f1447c1aec27f16d0fe113f94d
> 
> root@ast2700-default:~# openssl dgst -sha384 -engine ast_crypto_engine /tmp/256M
> Engine "ast_crypto_engine" set.
> SHA2-384(/tmp/256M)= fb6bc62afa1096dcd3b870e7d2546b7a5a177b5f2bbd5c9759218182454709e0c504a2d9c26404e04aa8010a291b7f1c
> 
> root@ast2700-default:~# openssl dgst -sha512 -engine ast_crypto_engine /tmp/256M
> Engine "ast_crypto_engine" set.
> SHA2-512(/tmp/256M)= fbceda7be34836fe857781656318ecd5b457a833a24c8736d5b8ef8d07e1950eebcdb140eebe4f12b5ff59586f7eb1c64fa95869c63dd9e4703d91261093c5c9
> 
> The HASH values generated here should exactly match those computed on the host
> machine using sha shell commands, verifying both the correctness of the
> hardware-accelerated results and the functionality of the ast_crypto_engine.
> 
> Jamin Lin (28):
>    hw/misc/aspeed_hace: Remove unused code for better readability
>    hw/misc/aspeed_hace: Improve readability and consistency in variable
>      naming
>    hw/misc/aspeed_hace: Ensure HASH_IRQ is always set to prevent firmware
>      hang
>    hw/misc/aspeed_hace: Extract direct mode hash buffer setup into helper
>      function
>    hw/misc/aspeed_hace: Extract SG-mode hash buffer setup into helper
>      function
>    hw/misc/aspeed_hace: Extract digest write and iov unmap into helper
>      function
>    hw/misc/aspeed_hace: Extract non-accumulation hash execution into
>      helper function
>    hw/misc/aspeed_hace: Extract accumulation-mode hash execution into
>      helper function
>    hw/misc/aspeed_hace: Introduce 64-bit hash source address helper
>      function
>    hw/misc/aspeed_hace: Rename R_HASH_DEST to R_HASH_DIGEST and introduce
>      64-bit hash digest address helper
>    hw/misc/aspeed_hace: Support accumulative mode for direct access mode
>    hw/misc/aspeed_hace: Move register size to instance class and
>      dynamically allocate regs
>    hw/misc/aspeed_hace: Add support for source, digest, key buffer 64 bit
>      addresses
>    hw/misc/aspeed_hace: Support DMA 64 bits dram address
>    hw/misc/aspeed_hace: Add trace-events for better debugging
>    hw/misc/aspeed_hace: Support to dump plaintext and digest for better
>      debugging
>    tests/qtest: Reorder aspeed test list
>    test/qtest: Introduce a new aspeed-hace-utils.c to place common
>      testcases
>    test/qtest/hace: Specify explicit array sizes for test vectors and
>      hash results
>    test/qtest/hace: Adjust test address range for AST1030 due to SRAM
>      limitations
>    test/qtest/hace: Add SHA-384 test cases for ASPEED HACE model
>    test/qtest/hace: Add SHA-384 tests for AST2600
>    test/qtest/hace: Add tests for AST1030
>    test/qtest/hace: Update source data and digest data type to 64-bit
>    test/qtest/hace: Support 64-bit source and digest addresses for
>      AST2700
>    test/qtest/hace: Support to test upper 32 bits of digest and source
>      addresses
>    test/qtest/hace: Support to validate 64-bit hmac key buffer addresses
>    test/qtest/hace: Add tests for AST2700
> 
>   include/hw/misc/aspeed_hace.h   |  11 +-
>   tests/qtest/aspeed-hace-utils.h |  84 +++++
>   hw/misc/aspeed_hace.c           | 479 +++++++++++++++--------
>   tests/qtest/aspeed-hace-utils.c | 646 ++++++++++++++++++++++++++++++++
>   tests/qtest/aspeed_hace-test.c  | 577 +++++-----------------------
>   tests/qtest/ast2700-hace-test.c |  98 +++++
>   hw/misc/trace-events            |   8 +
>   tests/qtest/meson.build         |  13 +-
>   8 files changed, 1279 insertions(+), 637 deletions(-)
>   create mode 100644 tests/qtest/aspeed-hace-utils.h
>   create mode 100644 tests/qtest/aspeed-hace-utils.c
>   create mode 100644 tests/qtest/ast2700-hace-test.c
> 

For the whole series,

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 00/28] Fix incorrect hash results on AST2700
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (29 preceding siblings ...)
  2025-05-23  7:17 ` Cédric Le Goater
@ 2025-05-23  8:10 ` Cédric Le Goater
  2025-05-29  7:29 ` Michael Tokarev
  31 siblings, 0 replies; 37+ messages in thread
From: Cédric Le Goater @ 2025-05-23  8:10 UTC (permalink / raw)
  To: Jamin Lin, Peter Maydell, Steven Lee, Troy Lee, Andrew Jeffery,
	Joel Stanley, Fabiano Rosas, Laurent Vivier, Paolo Bonzini,
	open list:ASPEED BMCs, open list:All patches CC here
  Cc: troy_lee

On 5/15/25 10:09, Jamin Lin wrote:
> v1:
>   1. Added support for 64-bit DMA in the HACE model
>   2. Refactored the do_hash operation in the HACE model
>   3. Fixed a crash caused by out-of-bound memory access in HACE
>   4. Added more trace events and implemented dumping of source hash data and
>      resulting digests to improve debugging
>   5. Refactored the HACE QTest framework to support both AST1030 and AST2700
>   6. Added a test case for SHA384
> 
> v2:
>    1. Create new helper functions
>       hash_get_source_addr
>       hash_prepare_direct_iov
>       hash_prepare_sg_iov
>       hash_get_digest_addr
>       hash_write_digest_and_unmap_iov
>       hash_execute_non_acc_mode
>       hash_execute_acc_mode
>    2. Refactor do_hash_operation
>    3. Fix review issue
>    4. Revise trace-events
>    5. Move register size to instance class and dynamically allocate regs
> 
> v3:
>    1. Split patch to introduce these routines one by one :
>         hash_prepare_sg_iov
>         hash_prepare_direct_iov
>         hash_execute_acc_mode
>         hash_execute_non_acc_mode
>         hash_write_digest_and_unmap_iov
>    2. Fix run qtest failed
>   
> This patchset resolves incorrect hash results reported on the AST2700 platform.
> This update addresses the following kernel warnings and test failures related to
> the crypto self-test framework:
> 
> aspeed-hmac-sha512 test failed (incorrect result)
> aspeed-hmac-sha384 test failed (incorrect result)
> aspeed-sha512 test failed (incorrect result)
> aspeed-sha384 test failed (incorrect result)
> aspeed-hmac-sha256 test failed (incorrect result)
> aspeed-hmac-sha224 test failed (incorrect result)
> aspeed-hmac-sha1 test failed (incorrect result)
> aspeed-sha224 test failed (incorrect result)
> aspeed-sha256 test failed (incorrect result)
> aspeed-sha1 test failed (incorrect result)
> 
> How to test it
> 
> Use the following command to dump information about the supported digest methods
> via the ast_crypto_engine hardware engine:
> 
> root@ast2700-default:~# openssl engine -pre DUMP_INFO ast_crypto_engine
> 
> Digest SHA1, NID=64, AF_ALG info: name=sha1ALG_ERR: , driver=aspeed-sha1 (hw accelerated)
> Digest SHA224, NID=675, AF_ALG info: name=sha224ALG_ERR: , driver=aspeed-sha224 (hw accelerated)
> Digest SHA256, NID=672, AF_ALG info: name=sha256ALG_ERR: , driver=aspeed-sha256 (hw accelerated)
> Digest SHA384, NID=673, AF_ALG info: name=sha384ALG_ERR: , driver=aspeed-sha384 (hw accelerated)
> Digest SHA512, NID=674, AF_ALG info: name=sha512ALG_ERR: , driver=aspeed-sha512 (hw accelerated)
> 
> The status of SHA1, SHA224, SHA256, SHA384, and SHA512 should be marked as
> hw accelerated, indicating that these algorithms are supported by hardware
> acceleration via the aspeed drivers.
> 
> Create a test file on the host machine and compute its HASH value as the
> expected result
> 
> Create a 256MB test file
> 
> $ dd if=/dev/random of=/tmp/256M bs=1M count=256
> Generate Hash Values Using SHA1, SHA224, SHA256, SHA384, and SHA512
> 
> Use the following commands to generate HASH values for a 256MB file using
> different SHA algorithms:
> 
> $ sha1sum /tmp/256M
> 7fc628811a31ab87b0502dab3ed8d3ef07565885  /tmp/256M
> 
> $ sha224sum /tmp/256M
> 2d261c11ba05b3a62e0efeab51c307d9933426c7e18204683ef3da54  /tmp/256M
> 
> $ sha256sum /tmp/256M
> 5716d1700ee35c92ca5ca5b466639e9c36eed3f1447c1aec27f16d0fe113f94d  /tmp/256M
> 
> $ sha384sum /tmp/256M
> fb6bc62afa1096dcd3b870e7d2546b7a5a177b5f2bbd5c9759218182454709e0c504a2d9c26404e04aa8010a291b7f1c  /tmp/256M
> 
> $ sha512sum /tmp/256M
> fbceda7be34836fe857781656318ecd5b457a833a24c8736d5b8ef8d07e1950eebcdb140eebe4f12b5ff59586f7eb1c64fa95869c63dd9e4703d91261093c5c9  /tmp/256M
> 
> Generate HASH Values Using the Hardware Engine
> 
> Use the following commands to generate HASH values for a 256MB file using
> various SHA algorithms with the ast_crypto_engine hardware engine:
> 
> root@ast2700-default:~# openssl dgst -sha1 -engine ast_crypto_engine /tmp/256M
> Engine "ast_crypto_engine" set.
> SHA1(/tmp/256M)= 7fc628811a31ab87b0502dab3ed8d3ef07565885
> 
> root@ast2700-default:~# openssl dgst -sha224 -engine ast_crypto_engine /tmp/256M
> Engine "ast_crypto_engine" set.
> SHA2-224(/tmp/256M)= 2d261c11ba05b3a62e0efeab51c307d9933426c7e18204683ef3da54
> 
> root@ast2700-default:~# openssl dgst -sha256 -engine ast_crypto_engine /tmp/256M
> Engine "ast_crypto_engine" set.
> SHA2-256(/tmp/256M)= 5716d1700ee35c92ca5ca5b466639e9c36eed3f1447c1aec27f16d0fe113f94d
> 
> root@ast2700-default:~# openssl dgst -sha384 -engine ast_crypto_engine /tmp/256M
> Engine "ast_crypto_engine" set.
> SHA2-384(/tmp/256M)= fb6bc62afa1096dcd3b870e7d2546b7a5a177b5f2bbd5c9759218182454709e0c504a2d9c26404e04aa8010a291b7f1c
> 
> root@ast2700-default:~# openssl dgst -sha512 -engine ast_crypto_engine /tmp/256M
> Engine "ast_crypto_engine" set.
> SHA2-512(/tmp/256M)= fbceda7be34836fe857781656318ecd5b457a833a24c8736d5b8ef8d07e1950eebcdb140eebe4f12b5ff59586f7eb1c64fa95869c63dd9e4703d91261093c5c9
> 
> The HASH values generated here should exactly match those computed on the host
> machine using sha shell commands, verifying both the correctness of the
> hardware-accelerated results and the functionality of the ast_crypto_engine.
> 
> Jamin Lin (28):
>    hw/misc/aspeed_hace: Remove unused code for better readability
>    hw/misc/aspeed_hace: Improve readability and consistency in variable
>      naming
>    hw/misc/aspeed_hace: Ensure HASH_IRQ is always set to prevent firmware
>      hang
>    hw/misc/aspeed_hace: Extract direct mode hash buffer setup into helper
>      function
>    hw/misc/aspeed_hace: Extract SG-mode hash buffer setup into helper
>      function
>    hw/misc/aspeed_hace: Extract digest write and iov unmap into helper
>      function
>    hw/misc/aspeed_hace: Extract non-accumulation hash execution into
>      helper function
>    hw/misc/aspeed_hace: Extract accumulation-mode hash execution into
>      helper function
>    hw/misc/aspeed_hace: Introduce 64-bit hash source address helper
>      function
>    hw/misc/aspeed_hace: Rename R_HASH_DEST to R_HASH_DIGEST and introduce
>      64-bit hash digest address helper
>    hw/misc/aspeed_hace: Support accumulative mode for direct access mode
>    hw/misc/aspeed_hace: Move register size to instance class and
>      dynamically allocate regs
>    hw/misc/aspeed_hace: Add support for source, digest, key buffer 64 bit
>      addresses
>    hw/misc/aspeed_hace: Support DMA 64 bits dram address
>    hw/misc/aspeed_hace: Add trace-events for better debugging
>    hw/misc/aspeed_hace: Support to dump plaintext and digest for better
>      debugging
>    tests/qtest: Reorder aspeed test list
>    test/qtest: Introduce a new aspeed-hace-utils.c to place common
>      testcases
>    test/qtest/hace: Specify explicit array sizes for test vectors and
>      hash results
>    test/qtest/hace: Adjust test address range for AST1030 due to SRAM
>      limitations
>    test/qtest/hace: Add SHA-384 test cases for ASPEED HACE model
>    test/qtest/hace: Add SHA-384 tests for AST2600
>    test/qtest/hace: Add tests for AST1030
>    test/qtest/hace: Update source data and digest data type to 64-bit
>    test/qtest/hace: Support 64-bit source and digest addresses for
>      AST2700
>    test/qtest/hace: Support to test upper 32 bits of digest and source
>      addresses
>    test/qtest/hace: Support to validate 64-bit hmac key buffer addresses
>    test/qtest/hace: Add tests for AST2700
> 
>   include/hw/misc/aspeed_hace.h   |  11 +-
>   tests/qtest/aspeed-hace-utils.h |  84 +++++
>   hw/misc/aspeed_hace.c           | 479 +++++++++++++++--------
>   tests/qtest/aspeed-hace-utils.c | 646 ++++++++++++++++++++++++++++++++
>   tests/qtest/aspeed_hace-test.c  | 577 +++++-----------------------
>   tests/qtest/ast2700-hace-test.c |  98 +++++
>   hw/misc/trace-events            |   8 +
>   tests/qtest/meson.build         |  13 +-
>   8 files changed, 1279 insertions(+), 637 deletions(-)
>   create mode 100644 tests/qtest/aspeed-hace-utils.h
>   create mode 100644 tests/qtest/aspeed-hace-utils.c
>   create mode 100644 tests/qtest/ast2700-hace-test.c
> 



Applied to aspeed-next.

Thanks,

C.




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 00/28] Fix incorrect hash results on AST2700
  2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
                   ` (30 preceding siblings ...)
  2025-05-23  8:10 ` Cédric Le Goater
@ 2025-05-29  7:29 ` Michael Tokarev
  2025-05-29  7:38   ` Cédric Le Goater
  31 siblings, 1 reply; 37+ messages in thread
From: Michael Tokarev @ 2025-05-29  7:29 UTC (permalink / raw)
  To: Jamin Lin, Cédric Le Goater, Peter Maydell, Steven Lee,
	Troy Lee, Andrew Jeffery, Joel Stanley, Fabiano Rosas,
	Laurent Vivier, Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: troy_lee

On 15.05.2025 11:09, Jamin Lin via wrote:

> This patchset resolves incorrect hash results reported on the AST2700 platform.
> This update addresses the following kernel warnings and test failures related to
> the crypto self-test framework:
..
> Jamin Lin (28):
>    hw/misc/aspeed_hace: Remove unused code for better readability
>    hw/misc/aspeed_hace: Improve readability and consistency in variable
>      naming
>    hw/misc/aspeed_hace: Ensure HASH_IRQ is always set to prevent firmware
>      hang
>    hw/misc/aspeed_hace: Extract direct mode hash buffer setup into helper
>      function
>    hw/misc/aspeed_hace: Extract SG-mode hash buffer setup into helper
>      function
>    hw/misc/aspeed_hace: Extract digest write and iov unmap into helper
>      function
>    hw/misc/aspeed_hace: Extract non-accumulation hash execution into
>      helper function
>    hw/misc/aspeed_hace: Extract accumulation-mode hash execution into
>      helper function
>    hw/misc/aspeed_hace: Introduce 64-bit hash source address helper
>      function
>    hw/misc/aspeed_hace: Rename R_HASH_DEST to R_HASH_DIGEST and introduce
>      64-bit hash digest address helper
>    hw/misc/aspeed_hace: Support accumulative mode for direct access mode
>    hw/misc/aspeed_hace: Move register size to instance class and
>      dynamically allocate regs
>    hw/misc/aspeed_hace: Add support for source, digest, key buffer 64 bit
>      addresses
>    hw/misc/aspeed_hace: Support DMA 64 bits dram address
>    hw/misc/aspeed_hace: Add trace-events for better debugging
>    hw/misc/aspeed_hace: Support to dump plaintext and digest for better
>      debugging
>    tests/qtest: Reorder aspeed test list
>    test/qtest: Introduce a new aspeed-hace-utils.c to place common
>      testcases
>    test/qtest/hace: Specify explicit array sizes for test vectors and
>      hash results
>    test/qtest/hace: Adjust test address range for AST1030 due to SRAM
>      limitations
>    test/qtest/hace: Add SHA-384 test cases for ASPEED HACE model
>    test/qtest/hace: Add SHA-384 tests for AST2600
>    test/qtest/hace: Add tests for AST1030
>    test/qtest/hace: Update source data and digest data type to 64-bit
>    test/qtest/hace: Support 64-bit source and digest addresses for
>      AST2700
>    test/qtest/hace: Support to test upper 32 bits of digest and source
>      addresses
>    test/qtest/hace: Support to validate 64-bit hmac key buffer addresses
>    test/qtest/hace: Add tests for AST2700

Is there anything here which is worth to apply to qemu-stable
(10.0.x is supposed to be LTS series)?

Thanks,

/mjt


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 00/28] Fix incorrect hash results on AST2700
  2025-05-29  7:29 ` Michael Tokarev
@ 2025-05-29  7:38   ` Cédric Le Goater
  2025-05-29  7:40     ` Jamin Lin
  0 siblings, 1 reply; 37+ messages in thread
From: Cédric Le Goater @ 2025-05-29  7:38 UTC (permalink / raw)
  To: Michael Tokarev, Jamin Lin, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: troy_lee

Hello Michael

On 5/29/25 09:29, Michael Tokarev wrote:
> On 15.05.2025 11:09, Jamin Lin via wrote:
> 
>> This patchset resolves incorrect hash results reported on the AST2700 platform.
>> This update addresses the following kernel warnings and test failures related to
>> the crypto self-test framework:
> ..
>> Jamin Lin (28):
>>    hw/misc/aspeed_hace: Remove unused code for better readability
>>    hw/misc/aspeed_hace: Improve readability and consistency in variable
>>      naming
>>    hw/misc/aspeed_hace: Ensure HASH_IRQ is always set to prevent firmware
>>      hang
>>    hw/misc/aspeed_hace: Extract direct mode hash buffer setup into helper
>>      function
>>    hw/misc/aspeed_hace: Extract SG-mode hash buffer setup into helper
>>      function
>>    hw/misc/aspeed_hace: Extract digest write and iov unmap into helper
>>      function
>>    hw/misc/aspeed_hace: Extract non-accumulation hash execution into
>>      helper function
>>    hw/misc/aspeed_hace: Extract accumulation-mode hash execution into
>>      helper function
>>    hw/misc/aspeed_hace: Introduce 64-bit hash source address helper
>>      function
>>    hw/misc/aspeed_hace: Rename R_HASH_DEST to R_HASH_DIGEST and introduce
>>      64-bit hash digest address helper
>>    hw/misc/aspeed_hace: Support accumulative mode for direct access mode
>>    hw/misc/aspeed_hace: Move register size to instance class and
>>      dynamically allocate regs
>>    hw/misc/aspeed_hace: Add support for source, digest, key buffer 64 bit
>>      addresses
>>    hw/misc/aspeed_hace: Support DMA 64 bits dram address
>>    hw/misc/aspeed_hace: Add trace-events for better debugging
>>    hw/misc/aspeed_hace: Support to dump plaintext and digest for better
>>      debugging
>>    tests/qtest: Reorder aspeed test list
>>    test/qtest: Introduce a new aspeed-hace-utils.c to place common
>>      testcases
>>    test/qtest/hace: Specify explicit array sizes for test vectors and
>>      hash results
>>    test/qtest/hace: Adjust test address range for AST1030 due to SRAM
>>      limitations
>>    test/qtest/hace: Add SHA-384 test cases for ASPEED HACE model
>>    test/qtest/hace: Add SHA-384 tests for AST2600
>>    test/qtest/hace: Add tests for AST1030
>>    test/qtest/hace: Update source data and digest data type to 64-bit
>>    test/qtest/hace: Support 64-bit source and digest addresses for
>>      AST2700
>>    test/qtest/hace: Support to test upper 32 bits of digest and source
>>      addresses
>>    test/qtest/hace: Support to validate 64-bit hmac key buffer addresses
>>    test/qtest/hace: Add tests for AST2700
> 
> Is there anything here which is worth to apply to qemu-stable
> (10.0.x is supposed to be LTS series)?


The candidates would be these two :

   hw/misc/aspeed_hace: Ensure HASH_IRQ is always set to
   hw/arm/aspeed_ast27x0: Fix RAM size detection failure on

Jamin,

Do you agree ?

Thanks,

C.




^ permalink raw reply	[flat|nested] 37+ messages in thread

* RE: [PATCH v3 00/28] Fix incorrect hash results on AST2700
  2025-05-29  7:38   ` Cédric Le Goater
@ 2025-05-29  7:40     ` Jamin Lin
  2025-05-29  7:45       ` Michael Tokarev
  0 siblings, 1 reply; 37+ messages in thread
From: Jamin Lin @ 2025-05-29  7:40 UTC (permalink / raw)
  To: Cédric Le Goater, Michael Tokarev, Peter Maydell, Steven Lee,
	Troy Lee, Andrew Jeffery, Joel Stanley, Fabiano Rosas,
	Laurent Vivier, Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: Troy Lee

Hi Cédric

> Subject: Re: [PATCH v3 00/28] Fix incorrect hash results on AST2700
> 
> Hello Michael
> 
> On 5/29/25 09:29, Michael Tokarev wrote:
> > On 15.05.2025 11:09, Jamin Lin via wrote:
> >
> >> This patchset resolves incorrect hash results reported on the AST2700
> platform.
> >> This update addresses the following kernel warnings and test failures
> >> related to the crypto self-test framework:
> > ..
> >> Jamin Lin (28):
> >>    hw/misc/aspeed_hace: Remove unused code for better readability
> >>    hw/misc/aspeed_hace: Improve readability and consistency in
> >> variable
> >>      naming
> >>    hw/misc/aspeed_hace: Ensure HASH_IRQ is always set to prevent
> >> firmware
> >>      hang
> >>    hw/misc/aspeed_hace: Extract direct mode hash buffer setup into
> >> helper
> >>      function
> >>    hw/misc/aspeed_hace: Extract SG-mode hash buffer setup into helper
> >>      function
> >>    hw/misc/aspeed_hace: Extract digest write and iov unmap into
> >> helper
> >>      function
> >>    hw/misc/aspeed_hace: Extract non-accumulation hash execution into
> >>      helper function
> >>    hw/misc/aspeed_hace: Extract accumulation-mode hash execution
> into
> >>      helper function
> >>    hw/misc/aspeed_hace: Introduce 64-bit hash source address helper
> >>      function
> >>    hw/misc/aspeed_hace: Rename R_HASH_DEST to R_HASH_DIGEST
> and
> >> introduce
> >>      64-bit hash digest address helper
> >>    hw/misc/aspeed_hace: Support accumulative mode for direct access
> >> mode
> >>    hw/misc/aspeed_hace: Move register size to instance class and
> >>      dynamically allocate regs
> >>    hw/misc/aspeed_hace: Add support for source, digest, key buffer 64
> >> bit
> >>      addresses
> >>    hw/misc/aspeed_hace: Support DMA 64 bits dram address
> >>    hw/misc/aspeed_hace: Add trace-events for better debugging
> >>    hw/misc/aspeed_hace: Support to dump plaintext and digest for
> >> better
> >>      debugging
> >>    tests/qtest: Reorder aspeed test list
> >>    test/qtest: Introduce a new aspeed-hace-utils.c to place common
> >>      testcases
> >>    test/qtest/hace: Specify explicit array sizes for test vectors and
> >>      hash results
> >>    test/qtest/hace: Adjust test address range for AST1030 due to SRAM
> >>      limitations
> >>    test/qtest/hace: Add SHA-384 test cases for ASPEED HACE model
> >>    test/qtest/hace: Add SHA-384 tests for AST2600
> >>    test/qtest/hace: Add tests for AST1030
> >>    test/qtest/hace: Update source data and digest data type to 64-bit
> >>    test/qtest/hace: Support 64-bit source and digest addresses for
> >>      AST2700
> >>    test/qtest/hace: Support to test upper 32 bits of digest and
> >> source
> >>      addresses
> >>    test/qtest/hace: Support to validate 64-bit hmac key buffer
> >> addresses
> >>    test/qtest/hace: Add tests for AST2700
> >
> > Is there anything here which is worth to apply to qemu-stable (10.0.x
> > is supposed to be LTS series)?
> 
> 
> The candidates would be these two :
> 
>    hw/misc/aspeed_hace: Ensure HASH_IRQ is always set to
>    hw/arm/aspeed_ast27x0: Fix RAM size detection failure on
> 
> Jamin,
> 
> Do you agree ?
> 

Agree
Thanks-Jamin

> Thanks,
> 
> C.
> 


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 00/28] Fix incorrect hash results on AST2700
  2025-05-29  7:40     ` Jamin Lin
@ 2025-05-29  7:45       ` Michael Tokarev
  2025-05-29 12:17         ` Cédric Le Goater
  0 siblings, 1 reply; 37+ messages in thread
From: Michael Tokarev @ 2025-05-29  7:45 UTC (permalink / raw)
  To: Jamin Lin, Cédric Le Goater, Peter Maydell, Steven Lee,
	Troy Lee, Andrew Jeffery, Joel Stanley, Fabiano Rosas,
	Laurent Vivier, Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: Troy Lee

On 29.05.2025 10:40, Jamin Lin wrote:
..
>>> Is there anything here which is worth to apply to qemu-stable (10.0.x
>>> is supposed to be LTS series)?
>>
>> The candidates would be these two :
>>
>>     hw/misc/aspeed_hace: Ensure HASH_IRQ is always set to
>>     hw/arm/aspeed_ast27x0: Fix RAM size detection failure on
>>
>> Jamin,
>>
>> Do you agree ?
>>
> 
> Agree
> Thanks-Jamin

That's what I picked up too, yes.

Thank you!

Please keep Cc: qemu-stable@ for anything else which is worth fixing :)

/mjt


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 00/28] Fix incorrect hash results on AST2700
  2025-05-29  7:45       ` Michael Tokarev
@ 2025-05-29 12:17         ` Cédric Le Goater
  0 siblings, 0 replies; 37+ messages in thread
From: Cédric Le Goater @ 2025-05-29 12:17 UTC (permalink / raw)
  To: Michael Tokarev, Jamin Lin, Peter Maydell, Steven Lee, Troy Lee,
	Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
	Paolo Bonzini, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: Troy Lee

On 5/29/25 09:45, Michael Tokarev wrote:
> On 29.05.2025 10:40, Jamin Lin wrote:
> ..
>>>> Is there anything here which is worth to apply to qemu-stable (10.0.x
>>>> is supposed to be LTS series)?
>>>
>>> The candidates would be these two :
>>>
>>>     hw/misc/aspeed_hace: Ensure HASH_IRQ is always set to
>>>     hw/arm/aspeed_ast27x0: Fix RAM size detection failure on
>>>
>>> Jamin,
>>>
>>> Do you agree ?
>>>
>>
>> Agree
>> Thanks-Jamin
> 
> That's what I picked up too, yes.
> 
> Thank you!
> 
> Please keep Cc: qemu-stable@ for anything else which is worth fixing :)
> 
Yeah. My patch flow is still a bit manual for fixes. I should probably
add a 'Cc: qemu-stable@' trailer to patches before sending a PR.


Thanks,

C.




^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2025-05-29 12:18 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-15  8:09 [PATCH v3 00/28] Fix incorrect hash results on AST2700 Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 01/28] hw/misc/aspeed_hace: Remove unused code for better readability Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 02/28] hw/misc/aspeed_hace: Improve readability and consistency in variable naming Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 03/28] hw/misc/aspeed_hace: Ensure HASH_IRQ is always set to prevent firmware hang Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 04/28] hw/misc/aspeed_hace: Extract direct mode hash buffer setup into helper function Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 05/28] hw/misc/aspeed_hace: Extract SG-mode " Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 06/28] hw/misc/aspeed_hace: Extract digest write and iov unmap " Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 07/28] hw/misc/aspeed_hace: Extract non-accumulation hash execution " Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 08/28] hw/misc/aspeed_hace: Extract accumulation-mode " Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 09/28] hw/misc/aspeed_hace: Introduce 64-bit hash source address " Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 10/28] hw/misc/aspeed_hace: Rename R_HASH_DEST to R_HASH_DIGEST and introduce 64-bit hash digest address helper Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 11/28] hw/misc/aspeed_hace: Support accumulative mode for direct access mode Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 12/28] hw/misc/aspeed_hace: Move register size to instance class and dynamically allocate regs Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 13/28] hw/misc/aspeed_hace: Add support for source, digest, key buffer 64 bit addresses Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 14/28] hw/misc/aspeed_hace: Support DMA 64 bits dram address Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 15/28] hw/misc/aspeed_hace: Add trace-events for better debugging Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 16/28] hw/misc/aspeed_hace: Support to dump plaintext and digest " Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 17/28] tests/qtest: Reorder aspeed test list Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 18/28] test/qtest: Introduce a new aspeed-hace-utils.c to place common testcases Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 19/28] test/qtest/hace: Specify explicit array sizes for test vectors and hash results Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 20/28] test/qtest/hace: Adjust test address range for AST1030 due to SRAM limitations Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 21/28] test/qtest/hace: Add SHA-384 test cases for ASPEED HACE model Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 22/28] test/qtest/hace: Add SHA-384 tests for AST2600 Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 23/28] test/qtest/hace: Add tests for AST1030 Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 24/28] test/qtest/hace: Update source data and digest data type to 64-bit Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 25/28] test/qtest/hace: Support 64-bit source and digest addresses for AST2700 Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 26/28] test/qtest/hace: Support to test upper 32 bits of digest and source addresses Jamin Lin via
2025-05-15  8:09 ` [PATCH v3 27/28] test/qtest/hace: Support to validate 64-bit hmac key buffer addresses Jamin Lin via
2025-05-15  8:10 ` [PATCH v3 28/28] test/qtest/hace: Add tests for AST2700 Jamin Lin via
2025-05-20 14:58 ` [PATCH v3 00/28] Fix incorrect hash results on AST2700 Fabiano Rosas
2025-05-23  7:17 ` Cédric Le Goater
2025-05-23  8:10 ` Cédric Le Goater
2025-05-29  7:29 ` Michael Tokarev
2025-05-29  7:38   ` Cédric Le Goater
2025-05-29  7:40     ` Jamin Lin
2025-05-29  7:45       ` Michael Tokarev
2025-05-29 12:17         ` Cédric Le Goater

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).