Refactor "do_hash_operation()" by extracting hash execution and result handling
into dedicated helper functions:
- "hash_write_digest_and_unmap_iov()": Writes the digest result to memory and
unmaps IOVs after processing.
- "hash_execute_non_acc_mode()": Handles one-shot (non-accumulated) hash
operations.
- "hash_execute_acc_mode()": Handles accumulated hashing, including update and
finalize logic.
No functional changes introduced.
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
hw/misc/aspeed_hace.c | 133 ++++++++++++++++++++++++++----------------
1 file changed, 83 insertions(+), 50 deletions(-)
diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 22eea62693..eb513ba00f 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -228,26 +228,96 @@ static int hash_prepare_sg_iov(AspeedHACEState *s, struct iovec *iov,
return iov_idx;
}
-static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
- bool acc_mode)
+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 hash_execute_non_acc_mode(AspeedHACEState *s, int algo,
+ struct iovec *iov, int iov_idx)
{
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;
+ size_t digest_len;
+
+ 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 hash_execute_acc_mode(AspeedHACEState *s, int algo,
+ struct iovec *iov, int iov_idx,
+ bool final_request)
+{
+ g_autofree uint8_t *digest_buf = NULL;
+ Error *local_err = NULL;
+ size_t digest_len;
- 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);
@@ -261,48 +331,11 @@ 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;
- }
- } 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;
- }
-
- 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_execute_acc_mode(s, algo, iov, iov_idx, acc_final_request);
+ } else {
+ hash_execute_non_acc_mode(s, algo, iov, iov_idx);
}
}
--
2.43.0Hello Jamin
On 5/13/25 08:28, Jamin Lin wrote:
> Refactor "do_hash_operation()" by extracting hash execution and result handling
> into dedicated helper functions:
>
> - "hash_write_digest_and_unmap_iov()": Writes the digest result to memory and
> unmaps IOVs after processing.
> - "hash_execute_non_acc_mode()": Handles one-shot (non-accumulated) hash
> operations.
> - "hash_execute_acc_mode()": Handles accumulated hashing, including update and
> finalize logic.
>
> No functional changes introduced.
This patch is breaking 'check-qtest-arm'
stderr:
**
ERROR:../tests/qtest/aspeed_hace-test.c:254:test_sha512: assertion failed (digest == test_result_sha512)
Thanks,
C.
>
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
> hw/misc/aspeed_hace.c | 133 ++++++++++++++++++++++++++----------------
> 1 file changed, 83 insertions(+), 50 deletions(-)
>
> diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
> index 22eea62693..eb513ba00f 100644
> --- a/hw/misc/aspeed_hace.c
> +++ b/hw/misc/aspeed_hace.c
> @@ -228,26 +228,96 @@ static int hash_prepare_sg_iov(AspeedHACEState *s, struct iovec *iov,
> return iov_idx;
> }
>
> -static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
> - bool acc_mode)
> +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 hash_execute_non_acc_mode(AspeedHACEState *s, int algo,
> + struct iovec *iov, int iov_idx)
> {
> 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;
> + size_t digest_len;
> +
> + 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 hash_execute_acc_mode(AspeedHACEState *s, int algo,
> + struct iovec *iov, int iov_idx,
> + bool final_request)
> +{
> + g_autofree uint8_t *digest_buf = NULL;
> + Error *local_err = NULL;
> + size_t digest_len;
>
> - 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);
> @@ -261,48 +331,11 @@ 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;
> - }
> - } 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;
> - }
> -
> - 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_execute_acc_mode(s, algo, iov, iov_idx, acc_final_request);
> + } else {
> + hash_execute_non_acc_mode(s, algo, iov, iov_idx);
> }
> }
>
Hi Cédric
> Subject: Re: [PATCH v2 05/25] hw/misc/aspeed_hace: Split hash execution into
> helper functions for clarity
>
> Hello Jamin
>
> On 5/13/25 08:28, Jamin Lin wrote:
> > Refactor "do_hash_operation()" by extracting hash execution and result
> > handling into dedicated helper functions:
> >
> > - "hash_write_digest_and_unmap_iov()": Writes the digest result to memory
> and
> > unmaps IOVs after processing.
> > - "hash_execute_non_acc_mode()": Handles one-shot (non-accumulated)
> hash
> > operations.
> > - "hash_execute_acc_mode()": Handles accumulated hashing, including
> update and
> > finalize logic.
> >
> > No functional changes introduced.
>
> This patch is breaking 'check-qtest-arm'
>
>
> stderr:
> **
> ERROR:../tests/qtest/aspeed_hace-test.c:254:test_sha512: assertion failed
> (digest == test_result_sha512)
>
I cannot reproduce it and my test steps as following.
Download this patch series
wget https://patchew.org/QEMU/20250513062901.2256865-1-jamin._5Flin@aspeedtech.com/mbox
Git clone QEMU, Build and Test
version: 69ee0189d7977cfbb1b2c7a27393d8b9fb661b20
1. git clone https://github.com/qemu/qemu.git
2. cd qemu
3. git am ../mbox
4. git log --oneline
ef01d966fb hw/misc/aspeed_hace: Split hash execution into helper functions for clarity
5. git rebase --interactive ef01d966fb^
6. git log | head
commit ef01d966fb27e9f9f7da16900f2d7b25de06fb32
Author: Jamin Lin via <qemu-devel@nongnu.org>
Date: Tue May 13 14:28:35 2025 +0800
hw/misc/aspeed_hace: Split hash execution into helper functions for clarity
7. cd ..
8. mkdir build
9. cd build
../qemu/configure --target-list=arm-linux-user,arm-softmmu,aarch64-softmmu,aarch64-linux-user --enable-slirp --disable-gcrypt --disable-nettle
Crypto
TLS priority : NORMAL
GNUTLS support : NO
libgcrypt : NO
nettle : NO
SM4 ALG support : NO
SM3 ALG support : NO
AF_ALG support : NO
rng-none : NO
Linux keyring : YES
Linux keyutils : NO
10. Build
make -j32
11. Run check-qtest-arm
make check-qtest-arm
jamin_lin@aspeed-fw02:~/commit/build$ make check-qtest-arm
[1/8] Generating qemu-version.h with a custom command (wrapped by meson to capture output)
/home/jamin_lin/commit/build/pyvenv/bin/meson test --no-rebuild -t 1 --num-processes 1 --print-errorlogs --suite qtest-arm
1/41 qemu:qtest+qtest-arm / qtest-arm/qom-test OK 232.04s 79 subtests passed
2/41 qemu:qtest+qtest-arm / qtest-arm/device-introspect-test OK 5.39s 6 subtests passed
3/41 qemu:qtest+qtest-arm / qtest-arm/cdrom-test SKIP 0.01s
4/41 qemu:qtest+qtest-arm / qtest-arm/stm32l4x5_usart-test OK 1.00s 7 subtests passed
5/41 qemu:qtest+qtest-arm / qtest-arm/aspeed_smc-test OK 89.73s 34 subtests passed
6/41 qemu:qtest+qtest-arm / qtest-arm/boot-serial-test OK 0.72s 3 subtests passed
7/41 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_pwm-test OK 5.44s 3 subtests passed
8/41 qemu:qtest+qtest-arm / qtest-arm/test-hmp OK 47.27s 80 subtests passed
9/41 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_watchdog_timer-test OK 5.16s 15 subtests passed
10/41 qemu:qtest+qtest-arm / qtest-arm/qmp-cmd-test OK 8.01s 62 subtests passed
11/41 qemu:qtest+qtest-arm / qtest-arm/qos-test OK 21.41s 97 subtests passed
12/41 qemu:qtest+qtest-arm / qtest-arm/sse-timer-test OK 0.24s 3 subtests passed
13/41 qemu:qtest+qtest-arm / qtest-arm/cmsdk-apb-dualtimer-test OK 0.14s 2 subtests passed
14/41 qemu:qtest+qtest-arm / qtest-arm/cmsdk-apb-timer-test OK 0.14s 1 subtests passed
15/41 qemu:qtest+qtest-arm / qtest-arm/cmsdk-apb-watchdog-test OK 1.02s 7 subtests passed
16/41 qemu:qtest+qtest-arm / qtest-arm/pflash-cfi02-test OK 1.62s 4 subtests passed
17/41 qemu:qtest+qtest-arm / qtest-arm/aspeed_hace-test OK 5.18s 16 subtests passed
18/41 qemu:qtest+qtest-arm / qtest-arm/aspeed_gpio-test OK 0.35s 2 subtests passed
19/41 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_adc-test OK 2.13s 6 subtests passed
20/41 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_gpio-test OK 0.19s 18 subtests passed
21/41 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_rng-test OK 0.18s 2 subtests passed
22/41 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_sdhci-test OK 0.74s 3 subtests passed
23/41 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_smbus-test OK 6.85s 40 subtests passed
24/41 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_timer-test OK 0.31s 180 subtests passed
25/41 qemu:qtest+qtest-arm / qtest-arm/npcm_gmac-test OK 0.35s 2 subtests passed
26/41 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_emc-test OK 1.21s 6 subtests passed
27/41 qemu:qtest+qtest-arm / qtest-arm/hexloader-test OK 0.22s 1 subtests passed
28/41 qemu:qtest+qtest-arm / qtest-arm/tpm-tis-i2c-test OK 0.58s 6 subtests passed
29/41 qemu:qtest+qtest-arm / qtest-arm/test-arm-mptimer OK 0.16s 61 subtests passed
30/41 qemu:qtest+qtest-arm / qtest-arm/microbit-test OK 3.87s 6 subtests passed
31/41 qemu:qtest+qtest-arm / qtest-arm/stm32l4x5_exti-test OK 0.15s 9 subtests passed
32/41 qemu:qtest+qtest-arm / qtest-arm/stm32l4x5_syscfg-test OK 0.15s 10 subtests passed
33/41 qemu:qtest+qtest-arm / qtest-arm/stm32l4x5_rcc-test OK 0.14s 5 subtests passed
34/41 qemu:qtest+qtest-arm / qtest-arm/stm32l4x5_gpio-test OK 0.17s 14 subtests passed
35/41 qemu:qtest+qtest-arm / qtest-arm/aspeed_fsi-test OK 0.30s 4 subtests passed
36/41 qemu:qtest+qtest-arm / qtest-arm/dm163-test OK 0.45s 3 subtests passed
37/41 qemu:qtest+qtest-arm / qtest-arm/arm-cpu-features OK 0.65s 1 subtests passed
38/41 qemu:qtest+qtest-arm / qtest-arm/machine-none-test OK 0.13s 1 subtests passed
39/41 qemu:qtest+qtest-arm / qtest-arm/qmp-test OK 0.51s 4 subtests passed
40/41 qemu:qtest+qtest-arm / qtest-arm/readconfig-test OK 0.13s 1 subtests passed
41/41 qemu:qtest+qtest-arm / qtest-arm/netdev-socket OK 3.03s 9 subtests passed
Ok: 40
Expected Fail: 0
Fail: 0
Unexpected Pass: 0
Skipped: 1
Timeout: 0
12. Run aspeed_hace-test
$ QTEST_QEMU_BINARY=${PWD}/qemu-system-arm tests/qtest/aspeed_hace-test
TAP version 13
# random seed: R02Sc6431b2cd4efa9bf268be49b109f1240
1..16
# Start of arm tests
# Start of ast2600 tests
# Start of hace tests
# starting QEMU: exec /home/jamin_lin/commit/build/qemu-system-arm -qtest unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine ast2600-evb -accel qtest
ok 1 /arm/ast2600/hace/addresses
# starting QEMU: exec /home/jamin_lin/commit/build/qemu-system-arm -qtest unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine ast2600-evb -accel qtest
ok 2 /arm/ast2600/hace/sha512
# starting QEMU: exec /home/jamin_lin/commit/build/qemu-system-arm -qtest unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine ast2600-evb -accel qtest
ok 3 /arm/ast2600/hace/sha256
# starting QEMU: exec /home/jamin_lin/commit/build/qemu-system-arm -qtest unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine ast2600-evb -accel qtest
ok 4 /arm/ast2600/hace/md5
# starting QEMU: exec /home/jamin_lin/commit/build/qemu-system-arm -qtest unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine ast2600-evb -accel qtest
ok 5 /arm/ast2600/hace/sha512_sg
# slow test /arm/ast2600/hace/sha512_sg executed in 0.58 secs
# starting QEMU: exec /home/jamin_lin/commit/build/qemu-system-arm -qtest unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine ast2600-evb -accel qtest
ok 6 /arm/ast2600/hace/sha256_sg
# slow test /arm/ast2600/hace/sha256_sg executed in 0.58 secs
# starting QEMU: exec /home/jamin_lin/commit/build/qemu-system-arm -qtest unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine ast2600-evb -accel qtest
ok 7 /arm/ast2600/hace/sha512_accum
# starting QEMU: exec /home/jamin_lin/commit/build/qemu-system-arm -qtest unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine ast2600-evb -accel qtest
ok 8 /arm/ast2600/hace/sha256_accum
# End of hace tests
# End of ast2600 tests
# Start of ast2500 tests
# Start of hace tests
# starting QEMU: exec /home/jamin_lin/commit/build/qemu-system-arm -qtest unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine ast2500-evb -accel qtest
ok 9 /arm/ast2500/hace/addresses
# starting QEMU: exec /home/jamin_lin/commit/build/qemu-system-arm -qtest unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine ast2500-evb -accel qtest
ok 10 /arm/ast2500/hace/sha512
# starting QEMU: exec /home/jamin_lin/commit/build/qemu-system-arm -qtest unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine ast2500-evb -accel qtest
ok 11 /arm/ast2500/hace/sha256
# starting QEMU: exec /home/jamin_lin/commit/build/qemu-system-arm -qtest unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine ast2500-evb -accel qtest
ok 12 /arm/ast2500/hace/md5
# End of hace tests
# End of ast2500 tests
# Start of ast2400 tests
# Start of hace tests
# starting QEMU: exec /home/jamin_lin/commit/build/qemu-system-arm -qtest unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine palmetto-bmc -accel qtest
ok 13 /arm/ast2400/hace/addresses
# starting QEMU: exec /home/jamin_lin/commit/build/qemu-system-arm -qtest unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine palmetto-bmc -accel qtest
ok 14 /arm/ast2400/hace/sha512
# starting QEMU: exec /home/jamin_lin/commit/build/qemu-system-arm -qtest unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine palmetto-bmc -accel qtest
ok 15 /arm/ast2400/hace/sha256
# starting QEMU: exec /home/jamin_lin/commit/build/qemu-system-arm -qtest unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine palmetto-bmc -accel qtest
ok 16 /arm/ast2400/hace/md5
# End of hace tests
# End of ast2400 tests
# End of arm tests
Thanks-Jamin
>
> Thanks,
>
> C.
>
>
>
> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> > hw/misc/aspeed_hace.c | 133
> ++++++++++++++++++++++++++----------------
> > 1 file changed, 83 insertions(+), 50 deletions(-)
> >
> > diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c index
> > 22eea62693..eb513ba00f 100644
> > --- a/hw/misc/aspeed_hace.c
> > +++ b/hw/misc/aspeed_hace.c
> > @@ -228,26 +228,96 @@ static int hash_prepare_sg_iov(AspeedHACEState
> *s, struct iovec *iov,
> > return iov_idx;
> > }
> >
> > -static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
> > - bool acc_mode)
> > +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 hash_execute_non_acc_mode(AspeedHACEState *s, int algo,
> > + struct iovec *iov, int iov_idx)
> > {
> > 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;
> > + size_t digest_len;
> > +
> > + 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 hash_execute_acc_mode(AspeedHACEState *s, int algo,
> > + struct iovec *iov, int iov_idx,
> > + bool final_request) {
> > + g_autofree uint8_t *digest_buf = NULL;
> > + Error *local_err = NULL;
> > + size_t digest_len;
> >
> > - 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); @@ -261,48 +331,11 @@ 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;
> > - }
> > - } 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;
> > - }
> > -
> > - 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_execute_acc_mode(s, algo, iov, iov_idx, acc_final_request);
> > + } else {
> > + hash_execute_non_acc_mode(s, algo, iov, iov_idx);
> > }
> > }
> >
Hi Cédric,
> Subject: RE: [PATCH v2 05/25] hw/misc/aspeed_hace: Split hash execution into
> helper functions for clarity
>
> Hi Cédric
>
> > Subject: Re: [PATCH v2 05/25] hw/misc/aspeed_hace: Split hash
> > execution into helper functions for clarity
> >
> > Hello Jamin
> >
> > On 5/13/25 08:28, Jamin Lin wrote:
> > > Refactor "do_hash_operation()" by extracting hash execution and
> > > result handling into dedicated helper functions:
> > >
> > > - "hash_write_digest_and_unmap_iov()": Writes the digest result to
> > > memory
> > and
> > > unmaps IOVs after processing.
> > > - "hash_execute_non_acc_mode()": Handles one-shot (non-accumulated)
> > hash
> > > operations.
> > > - "hash_execute_acc_mode()": Handles accumulated hashing, including
> > update and
> > > finalize logic.
> > >
> > > No functional changes introduced.
> >
> > This patch is breaking 'check-qtest-arm'
> >
> >
> > stderr:
> > **
> > ERROR:../tests/qtest/aspeed_hace-test.c:254:test_sha512: assertion
> > failed (digest == test_result_sha512)
> >
>
I’d like to provide some additional information:
My test environment is Ubuntu 24.04, and the glibc version is 2.39.
I previously encountered issues where very very older versions of glibc (such as those on Ubuntu 18.04) did not support SHA-512 properly.
In those cases, I switched to using libgcrypt to perform SHA-512 testing instead.
If you're still experiencing failures when testing SHA-512, please try building with either the "--enable-gcrypt" or "--enable-nettle" option enabled.
Jamin
> I cannot reproduce it and my test steps as following.
>
> Download this patch series
> wget
> https://patchew.org/QEMU/20250513062901.2256865-1-jamin._5Flin@aspeed
> tech.com/mbox
>
> Git clone QEMU, Build and Test
> version: 69ee0189d7977cfbb1b2c7a27393d8b9fb661b20
> 1. git clone https://github.com/qemu/qemu.git 2. cd qemu 3. git am ../mbox 4.
> git log --oneline ef01d966fb hw/misc/aspeed_hace: Split hash execution into
> helper functions for clarity 5. git rebase --interactive ef01d966fb^ 6. git log |
> head commit ef01d966fb27e9f9f7da16900f2d7b25de06fb32
> Author: Jamin Lin via <qemu-devel@nongnu.org>
> Date: Tue May 13 14:28:35 2025 +0800
>
> hw/misc/aspeed_hace: Split hash execution into helper functions for
> clarity
>
> 7. cd ..
> 8. mkdir build
> 9. cd build
> ../qemu/configure
> --target-list=arm-linux-user,arm-softmmu,aarch64-softmmu,aarch64-linux-user
> --enable-slirp --disable-gcrypt --disable-nettle
>
> Crypto
> TLS priority : NORMAL
> GNUTLS support : NO
> libgcrypt : NO
> nettle : NO
> SM4 ALG support : NO
> SM3 ALG support : NO
> AF_ALG support : NO
> rng-none : NO
> Linux keyring : YES
> Linux keyutils : NO
>
> 10. Build
> make -j32
> 11. Run check-qtest-arm
> make check-qtest-arm
> jamin_lin@aspeed-fw02:~/commit/build$ make check-qtest-arm [1/8]
> Generating qemu-version.h with a custom command (wrapped by meson to
> capture output) /home/jamin_lin/commit/build/pyvenv/bin/meson test
> --no-rebuild -t 1 --num-processes 1 --print-errorlogs --suite qtest-arm
> 1/41 qemu:qtest+qtest-arm / qtest-arm/qom-test
> OK 232.04s 79 subtests passed
> 2/41 qemu:qtest+qtest-arm / qtest-arm/device-introspect-test
> OK 5.39s 6 subtests passed
> 3/41 qemu:qtest+qtest-arm / qtest-arm/cdrom-test
> SKIP 0.01s
> 4/41 qemu:qtest+qtest-arm / qtest-arm/stm32l4x5_usart-test
> OK 1.00s 7 subtests passed
> 5/41 qemu:qtest+qtest-arm / qtest-arm/aspeed_smc-test
> OK 89.73s 34 subtests passed
> 6/41 qemu:qtest+qtest-arm / qtest-arm/boot-serial-test
> OK 0.72s 3 subtests passed
> 7/41 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_pwm-test
> OK 5.44s 3 subtests passed
> 8/41 qemu:qtest+qtest-arm / qtest-arm/test-hmp
> OK 47.27s 80 subtests passed
> 9/41 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_watchdog_timer-test
> OK 5.16s 15 subtests passed
> 10/41 qemu:qtest+qtest-arm / qtest-arm/qmp-cmd-test
> OK 8.01s 62 subtests passed
> 11/41 qemu:qtest+qtest-arm / qtest-arm/qos-test
> OK 21.41s 97 subtests passed
> 12/41 qemu:qtest+qtest-arm / qtest-arm/sse-timer-test
> OK 0.24s 3 subtests passed
> 13/41 qemu:qtest+qtest-arm / qtest-arm/cmsdk-apb-dualtimer-test
> OK 0.14s 2 subtests passed
> 14/41 qemu:qtest+qtest-arm / qtest-arm/cmsdk-apb-timer-test
> OK 0.14s 1 subtests passed
> 15/41 qemu:qtest+qtest-arm / qtest-arm/cmsdk-apb-watchdog-test
> OK 1.02s 7 subtests passed
> 16/41 qemu:qtest+qtest-arm / qtest-arm/pflash-cfi02-test
> OK 1.62s 4 subtests passed
> 17/41 qemu:qtest+qtest-arm / qtest-arm/aspeed_hace-test
> OK 5.18s 16 subtests passed
> 18/41 qemu:qtest+qtest-arm / qtest-arm/aspeed_gpio-test
> OK 0.35s 2 subtests passed
> 19/41 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_adc-test
> OK 2.13s 6 subtests passed
> 20/41 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_gpio-test
> OK 0.19s 18 subtests passed
> 21/41 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_rng-test
> OK 0.18s 2 subtests passed
> 22/41 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_sdhci-test
> OK 0.74s 3 subtests passed
> 23/41 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_smbus-test
> OK 6.85s 40 subtests passed
> 24/41 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_timer-test
> OK 0.31s 180 subtests passed
> 25/41 qemu:qtest+qtest-arm / qtest-arm/npcm_gmac-test
> OK 0.35s 2 subtests passed
> 26/41 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_emc-test
> OK 1.21s 6 subtests passed
> 27/41 qemu:qtest+qtest-arm / qtest-arm/hexloader-test
> OK 0.22s 1 subtests passed
> 28/41 qemu:qtest+qtest-arm / qtest-arm/tpm-tis-i2c-test
> OK 0.58s 6 subtests passed
> 29/41 qemu:qtest+qtest-arm / qtest-arm/test-arm-mptimer
> OK 0.16s 61 subtests passed
> 30/41 qemu:qtest+qtest-arm / qtest-arm/microbit-test
> OK 3.87s 6 subtests passed
> 31/41 qemu:qtest+qtest-arm / qtest-arm/stm32l4x5_exti-test
> OK 0.15s 9 subtests passed
> 32/41 qemu:qtest+qtest-arm / qtest-arm/stm32l4x5_syscfg-test
> OK 0.15s 10 subtests passed
> 33/41 qemu:qtest+qtest-arm / qtest-arm/stm32l4x5_rcc-test
> OK 0.14s 5 subtests passed
> 34/41 qemu:qtest+qtest-arm / qtest-arm/stm32l4x5_gpio-test
> OK 0.17s 14 subtests passed
> 35/41 qemu:qtest+qtest-arm / qtest-arm/aspeed_fsi-test
> OK 0.30s 4 subtests passed
> 36/41 qemu:qtest+qtest-arm / qtest-arm/dm163-test
> OK 0.45s 3 subtests passed
> 37/41 qemu:qtest+qtest-arm / qtest-arm/arm-cpu-features
> OK 0.65s 1 subtests passed
> 38/41 qemu:qtest+qtest-arm / qtest-arm/machine-none-test
> OK 0.13s 1 subtests passed
> 39/41 qemu:qtest+qtest-arm / qtest-arm/qmp-test
> OK 0.51s 4 subtests passed
> 40/41 qemu:qtest+qtest-arm / qtest-arm/readconfig-test
> OK 0.13s 1 subtests passed
> 41/41 qemu:qtest+qtest-arm / qtest-arm/netdev-socket
> OK 3.03s 9 subtests passed
>
> Ok: 40
> Expected Fail: 0
> Fail: 0
> Unexpected Pass: 0
> Skipped: 1
> Timeout: 0
>
> 12. Run aspeed_hace-test
> $ QTEST_QEMU_BINARY=${PWD}/qemu-system-arm
> tests/qtest/aspeed_hace-test TAP version 13 # random seed:
> R02Sc6431b2cd4efa9bf268be49b109f1240
> 1..16
> # Start of arm tests
> # Start of ast2600 tests
> # Start of hace tests
> # starting QEMU: exec /home/jamin_lin/commit/build/qemu-system-arm
> -qtest unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon
> chardev=char0,mode=control -display none -audio none -machine ast2600-evb
> -accel qtest ok 1 /arm/ast2600/hace/addresses # starting QEMU: exec
> /home/jamin_lin/commit/build/qemu-system-arm -qtest
> unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon
> chardev=char0,mode=control -display none -audio none -machine ast2600-evb
> -accel qtest ok 2 /arm/ast2600/hace/sha512 # starting QEMU: exec
> /home/jamin_lin/commit/build/qemu-system-arm -qtest
> unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon
> chardev=char0,mode=control -display none -audio none -machine ast2600-evb
> -accel qtest ok 3 /arm/ast2600/hace/sha256 # starting QEMU: exec
> /home/jamin_lin/commit/build/qemu-system-arm -qtest
> unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon
> chardev=char0,mode=control -display none -audio none -machine ast2600-evb
> -accel qtest ok 4 /arm/ast2600/hace/md5 # starting QEMU: exec
> /home/jamin_lin/commit/build/qemu-system-arm -qtest
> unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon
> chardev=char0,mode=control -display none -audio none -machine ast2600-evb
> -accel qtest ok 5 /arm/ast2600/hace/sha512_sg # slow test
> /arm/ast2600/hace/sha512_sg executed in 0.58 secs # starting QEMU: exec
> /home/jamin_lin/commit/build/qemu-system-arm -qtest
> unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon
> chardev=char0,mode=control -display none -audio none -machine ast2600-evb
> -accel qtest ok 6 /arm/ast2600/hace/sha256_sg # slow test
> /arm/ast2600/hace/sha256_sg executed in 0.58 secs # starting QEMU: exec
> /home/jamin_lin/commit/build/qemu-system-arm -qtest
> unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon
> chardev=char0,mode=control -display none -audio none -machine ast2600-evb
> -accel qtest ok 7 /arm/ast2600/hace/sha512_accum # starting QEMU: exec
> /home/jamin_lin/commit/build/qemu-system-arm -qtest
> unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon
> chardev=char0,mode=control -display none -audio none -machine ast2600-evb
> -accel qtest ok 8 /arm/ast2600/hace/sha256_accum # End of hace tests # End
> of ast2600 tests # Start of ast2500 tests # Start of hace tests # starting QEMU:
> exec /home/jamin_lin/commit/build/qemu-system-arm -qtest
> unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon
> chardev=char0,mode=control -display none -audio none -machine ast2500-evb
> -accel qtest ok 9 /arm/ast2500/hace/addresses # starting QEMU: exec
> /home/jamin_lin/commit/build/qemu-system-arm -qtest
> unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon
> chardev=char0,mode=control -display none -audio none -machine ast2500-evb
> -accel qtest ok 10 /arm/ast2500/hace/sha512 # starting QEMU: exec
> /home/jamin_lin/commit/build/qemu-system-arm -qtest
> unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon
> chardev=char0,mode=control -display none -audio none -machine ast2500-evb
> -accel qtest ok 11 /arm/ast2500/hace/sha256 # starting QEMU: exec
> /home/jamin_lin/commit/build/qemu-system-arm -qtest
> unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon
> chardev=char0,mode=control -display none -audio none -machine ast2500-evb
> -accel qtest ok 12 /arm/ast2500/hace/md5 # End of hace tests # End of
> ast2500 tests # Start of ast2400 tests # Start of hace tests # starting QEMU:
> exec /home/jamin_lin/commit/build/qemu-system-arm -qtest
> unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon
> chardev=char0,mode=control -display none -audio none -machine
> palmetto-bmc -accel qtest ok 13 /arm/ast2400/hace/addresses # starting
> QEMU: exec /home/jamin_lin/commit/build/qemu-system-arm -qtest
> unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon
> chardev=char0,mode=control -display none -audio none -machine
> palmetto-bmc -accel qtest ok 14 /arm/ast2400/hace/sha512 # starting QEMU:
> exec /home/jamin_lin/commit/build/qemu-system-arm -qtest
> unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon
> chardev=char0,mode=control -display none -audio none -machine
> palmetto-bmc -accel qtest ok 15 /arm/ast2400/hace/sha256 # starting QEMU:
> exec /home/jamin_lin/commit/build/qemu-system-arm -qtest
> unix:/tmp/qtest-3741778.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-3741778.qmp,id=char0 -mon
> chardev=char0,mode=control -display none -audio none -machine
> palmetto-bmc -accel qtest ok 16 /arm/ast2400/hace/md5 # End of hace tests
> # End of ast2400 tests # End of arm tests
>
> Thanks-Jamin
>
> >
> > Thanks,
> >
> > C.
> >
> >
> >
> > >
> > > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > > ---
> > > hw/misc/aspeed_hace.c | 133
> > ++++++++++++++++++++++++++----------------
> > > 1 file changed, 83 insertions(+), 50 deletions(-)
> > >
> > > diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c index
> > > 22eea62693..eb513ba00f 100644
> > > --- a/hw/misc/aspeed_hace.c
> > > +++ b/hw/misc/aspeed_hace.c
> > > @@ -228,26 +228,96 @@ static int
> hash_prepare_sg_iov(AspeedHACEState
> > *s, struct iovec *iov,
> > > return iov_idx;
> > > }
> > >
> > > -static void do_hash_operation(AspeedHACEState *s, int algo, bool
> sg_mode,
> > > - bool acc_mode)
> > > +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 hash_execute_non_acc_mode(AspeedHACEState *s, int algo,
> > > + struct iovec *iov, int
> > > +iov_idx)
> > > {
> > > 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;
> > > + size_t digest_len;
> > > +
> > > + 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 hash_execute_acc_mode(AspeedHACEState *s, int algo,
> > > + struct iovec *iov, int iov_idx,
> > > + bool final_request) {
> > > + g_autofree uint8_t *digest_buf = NULL;
> > > + Error *local_err = NULL;
> > > + size_t digest_len;
> > >
> > > - 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); @@ -261,48 +331,11 @@ 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;
> > > - }
> > > - } 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;
> > > - }
> > > -
> > > - 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_execute_acc_mode(s, algo, iov, iov_idx,
> acc_final_request);
> > > + } else {
> > > + hash_execute_non_acc_mode(s, algo, iov, iov_idx);
> > > }
> > > }
> > >
Hello Jamin, On 5/14/25 04:48, Jamin Lin wrote: > Hi Cédric, > >> Subject: RE: [PATCH v2 05/25] hw/misc/aspeed_hace: Split hash execution into >> helper functions for clarity >> >> Hi Cédric >> >>> Subject: Re: [PATCH v2 05/25] hw/misc/aspeed_hace: Split hash >>> execution into helper functions for clarity >>> >>> Hello Jamin >>> >>> On 5/13/25 08:28, Jamin Lin wrote: >>>> Refactor "do_hash_operation()" by extracting hash execution and >>>> result handling into dedicated helper functions: >>>> >>>> - "hash_write_digest_and_unmap_iov()": Writes the digest result to >>>> memory >>> and >>>> unmaps IOVs after processing. >>>> - "hash_execute_non_acc_mode()": Handles one-shot (non-accumulated) >>> hash >>>> operations. >>>> - "hash_execute_acc_mode()": Handles accumulated hashing, including >>> update and >>>> finalize logic. >>>> >>>> No functional changes introduced. >>> >>> This patch is breaking 'check-qtest-arm' >>> >>> >>> stderr: >>> ** >>> ERROR:../tests/qtest/aspeed_hace-test.c:254:test_sha512: assertion >>> failed (digest == test_result_sha512) >>> >> > > I’d like to provide some additional information: > > My test environment is Ubuntu 24.04, and the glibc version is 2.39. > I previously encountered issues where very very older versions of glibc (such as those on Ubuntu 18.04) did not support SHA-512 properly. > In those cases, I switched to using libgcrypt to perform SHA-512 testing instead. > > If you're still experiencing failures when testing SHA-512, please try building with either the "--enable-gcrypt" or "--enable-nettle" option enabled. The test fails on RHEL9 and Ubuntu 22.04 and it doesn't on F42. Most likely, it is a compiler behavior difference. The problem is due to the local 'size_t digest_len' variable not being initialized to zero. With that fixed, the test passes on all distros. Please split patch 5 to introduce these routines one by one : hash_prepare_sg_iov hash_prepare_direct_iov hash_execute_acc_mode hash_execute_non_acc_mode Thanks, C.
Hi Cédric > Subject: Re: [PATCH v2 05/25] hw/misc/aspeed_hace: Split hash execution into > helper functions for clarity > > Hello Jamin, > > On 5/14/25 04:48, Jamin Lin wrote: > > Hi Cédric, > > > >> Subject: RE: [PATCH v2 05/25] hw/misc/aspeed_hace: Split hash > >> execution into helper functions for clarity > >> > >> Hi Cédric > >> > >>> Subject: Re: [PATCH v2 05/25] hw/misc/aspeed_hace: Split hash > >>> execution into helper functions for clarity > >>> > >>> Hello Jamin > >>> > >>> On 5/13/25 08:28, Jamin Lin wrote: > >>>> Refactor "do_hash_operation()" by extracting hash execution and > >>>> result handling into dedicated helper functions: > >>>> > >>>> - "hash_write_digest_and_unmap_iov()": Writes the digest result to > >>>> memory > >>> and > >>>> unmaps IOVs after processing. > >>>> - "hash_execute_non_acc_mode()": Handles one-shot (non-accumulated) > >>> hash > >>>> operations. > >>>> - "hash_execute_acc_mode()": Handles accumulated hashing, including > >>> update and > >>>> finalize logic. > >>>> > >>>> No functional changes introduced. > >>> > >>> This patch is breaking 'check-qtest-arm' > >>> > >>> > >>> stderr: > >>> ** > >>> ERROR:../tests/qtest/aspeed_hace-test.c:254:test_sha512: > >>> assertion failed (digest == test_result_sha512) > >>> > >> > > > > I’d like to provide some additional information: > > > > My test environment is Ubuntu 24.04, and the glibc version is 2.39. > > I previously encountered issues where very very older versions of glibc (such > as those on Ubuntu 18.04) did not support SHA-512 properly. > > In those cases, I switched to using libgcrypt to perform SHA-512 testing > instead. > > > > If you're still experiencing failures when testing SHA-512, please try building > with either the "--enable-gcrypt" or "--enable-nettle" option enabled. > > The test fails on RHEL9 and Ubuntu 22.04 and it doesn't on F42. Most likely, it > is a compiler behavior difference. > I will test is on Ubuntu 22.04. Will create a new patch to set local size_t digest_len 0. > > The problem is due to the local 'size_t digest_len' variable not being initialized > to zero. With that fixed, the test passes on all distros. > > Please split patch 5 to introduce these routines one by one : > > hash_prepare_sg_iov > hash_prepare_direct_iov > hash_execute_acc_mode > hash_execute_non_acc_mode > Will do Thanks-Jamin > Thanks, > > C. >
© 2016 - 2025 Red Hat, Inc.