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_...@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); > > > } > > > } > > >
