* [PULL 01/39] tests/qtest/ast2700-smc-test: Fix leak
2025-05-26 8:04 [PULL 00/39] aspeed queue Cédric Le Goater
@ 2025-05-26 8:04 ` Cédric Le Goater
2025-05-26 8:04 ` [PULL 02/39] tests/qtest/aspeed_smc-test: Fix memory leaks Cédric Le Goater
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Cédric Le Goater @ 2025-05-26 8:04 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Fabiano Rosas, Jamin Lin, Cédric Le Goater
From: Fabiano Rosas <farosas@suse.de>
ASAN spotted a leak of the memory used to hold the tmp_path:
Direct leak of 35 byte(s) in 1 object(s) allocated from:
#0 0x55e29aa96da9 in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:69:3
#1 0x7fe0cfb26518 in g_malloc ../glib/gmem.c:106
#2 0x7fe0cfb4146e in g_strconcat ../glib/gstrfuncs.c:629
#3 0x7fe0cfb0a78f in g_get_tmp_name ../glib/gfileutils.c:1742
#4 0x7fe0cfb0b00b in g_file_open_tmp ../glib/gfileutils.c:1802
#5 0x55e29ab53961 in test_ast2700_evb ../tests/qtest/ast2700-smc-test.c:20:10
#6 0x55e29ab53803 in main ../tests/qtest/ast2700-smc-test.c:65:5
#7 0x7fe0cf7bd24c in __libc_start_main ../csu/libc-start.c:308
#8 0x55e29a9f7759 in _start ../sysdeps/x86_64/start.S:120
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Jamin Lin <jamin_lin@aspeedtech.com>
Message-ID: <20250509175047.26066-1-farosas@suse.de>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
tests/qtest/ast2700-smc-test.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/qtest/ast2700-smc-test.c b/tests/qtest/ast2700-smc-test.c
index d1c485630744..62d538d8a3a7 100644
--- a/tests/qtest/ast2700-smc-test.c
+++ b/tests/qtest/ast2700-smc-test.c
@@ -67,5 +67,6 @@ int main(int argc, char **argv)
qtest_quit(ast2700_evb_data.s);
unlink(ast2700_evb_data.tmp_path);
+ g_free(ast2700_evb_data.tmp_path);
return ret;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 02/39] tests/qtest/aspeed_smc-test: Fix memory leaks
2025-05-26 8:04 [PULL 00/39] aspeed queue Cédric Le Goater
2025-05-26 8:04 ` [PULL 01/39] tests/qtest/ast2700-smc-test: Fix leak Cédric Le Goater
@ 2025-05-26 8:04 ` Cédric Le Goater
2025-05-26 8:04 ` [PULL 03/39] hw/misc/aspeed_hace: Remove unused code for better readability Cédric Le Goater
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Cédric Le Goater @ 2025-05-26 8:04 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Laurent Vivier, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
Link: https://patchwork.kernel.org/project/qemu-devel/patch/20250509175047.26066-1-farosas@suse.de/
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20250513080806.1005996-1-jamin_lin@aspeedtech.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
tests/qtest/aspeed_smc-test.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
index 4e1389385d85..52a00e6f0a7e 100644
--- a/tests/qtest/aspeed_smc-test.c
+++ b/tests/qtest/aspeed_smc-test.c
@@ -228,5 +228,10 @@ int main(int argc, char **argv)
unlink(ast2500_evb_data.tmp_path);
unlink(ast2600_evb_data.tmp_path);
unlink(ast1030_evb_data.tmp_path);
+ g_free(palmetto_data.tmp_path);
+ g_free(ast2500_evb_data.tmp_path);
+ g_free(ast2600_evb_data.tmp_path);
+ g_free(ast1030_evb_data.tmp_path);
+
return ret;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 03/39] hw/misc/aspeed_hace: Remove unused code for better readability
2025-05-26 8:04 [PULL 00/39] aspeed queue Cédric Le Goater
2025-05-26 8:04 ` [PULL 01/39] tests/qtest/ast2700-smc-test: Fix leak Cédric Le Goater
2025-05-26 8:04 ` [PULL 02/39] tests/qtest/aspeed_smc-test: Fix memory leaks Cédric Le Goater
@ 2025-05-26 8:04 ` Cédric Le Goater
2025-05-26 8:04 ` [PULL 04/39] hw/misc/aspeed_hace: Improve readability and consistency in variable naming Cédric Le Goater
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Cédric Le Goater @ 2025-05-26 8:04 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
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>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20250515081008.583578-2-jamin_lin@aspeedtech.com
Signed-off-by: Cédric Le Goater <clg@redhat.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 5d4aa19cfecb..b69a038d3540 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 f4bff32a0041..9263739ea634 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.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 04/39] hw/misc/aspeed_hace: Improve readability and consistency in variable naming
2025-05-26 8:04 [PULL 00/39] aspeed queue Cédric Le Goater
` (2 preceding siblings ...)
2025-05-26 8:04 ` [PULL 03/39] hw/misc/aspeed_hace: Remove unused code for better readability Cédric Le Goater
@ 2025-05-26 8:04 ` Cédric Le Goater
2025-05-26 8:04 ` [PULL 05/39] hw/misc/aspeed_hace: Ensure HASH_IRQ is always set to prevent firmware hang Cédric Le Goater
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Cédric Le Goater @ 2025-05-26 8:04 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
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>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20250515081008.583578-3-jamin_lin@aspeedtech.com
Signed-off-by: Cédric Le Goater <clg@redhat.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 9263739ea634..6be94963bce7 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.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 05/39] hw/misc/aspeed_hace: Ensure HASH_IRQ is always set to prevent firmware hang
2025-05-26 8:04 [PULL 00/39] aspeed queue Cédric Le Goater
` (3 preceding siblings ...)
2025-05-26 8:04 ` [PULL 04/39] hw/misc/aspeed_hace: Improve readability and consistency in variable naming Cédric Le Goater
@ 2025-05-26 8:04 ` Cédric Le Goater
2025-05-26 8:04 ` [PULL 06/39] hw/misc/aspeed_hace: Extract direct mode hash buffer setup into helper function Cédric Le Goater
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Cédric Le Goater @ 2025-05-26 8:04 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
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")
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20250515081008.583578-4-jamin_lin@aspeedtech.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
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 6be94963bce7..1256926d2276 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.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 06/39] hw/misc/aspeed_hace: Extract direct mode hash buffer setup into helper function
2025-05-26 8:04 [PULL 00/39] aspeed queue Cédric Le Goater
` (4 preceding siblings ...)
2025-05-26 8:04 ` [PULL 05/39] hw/misc/aspeed_hace: Ensure HASH_IRQ is always set to prevent firmware hang Cédric Le Goater
@ 2025-05-26 8:04 ` Cédric Le Goater
2025-05-26 8:04 ` [PULL 07/39] hw/misc/aspeed_hace: Extract SG-mode " Cédric Le Goater
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Cédric Le Goater @ 2025-05-26 8:04 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
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>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20250515081008.583578-5-jamin_lin@aspeedtech.com
Signed-off-by: Cédric Le Goater <clg@redhat.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 1256926d2276..42c6f29f8262 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.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 07/39] hw/misc/aspeed_hace: Extract SG-mode hash buffer setup into helper function
2025-05-26 8:04 [PULL 00/39] aspeed queue Cédric Le Goater
` (5 preceding siblings ...)
2025-05-26 8:04 ` [PULL 06/39] hw/misc/aspeed_hace: Extract direct mode hash buffer setup into helper function Cédric Le Goater
@ 2025-05-26 8:04 ` Cédric Le Goater
2025-05-26 8:04 ` [PULL 08/39] hw/misc/aspeed_hace: Extract digest write and iov unmap " Cédric Le Goater
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Cédric Le Goater @ 2025-05-26 8:04 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
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>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20250515081008.583578-6-jamin_lin@aspeedtech.com
Signed-off-by: Cédric Le Goater <clg@redhat.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 42c6f29f8262..22eea62693c7 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.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 08/39] hw/misc/aspeed_hace: Extract digest write and iov unmap into helper function
2025-05-26 8:04 [PULL 00/39] aspeed queue Cédric Le Goater
` (6 preceding siblings ...)
2025-05-26 8:04 ` [PULL 07/39] hw/misc/aspeed_hace: Extract SG-mode " Cédric Le Goater
@ 2025-05-26 8:04 ` Cédric Le Goater
2025-05-26 8:04 ` [PULL 09/39] hw/misc/aspeed_hace: Extract non-accumulation hash execution " Cédric Le Goater
2025-05-26 18:05 ` [PULL 00/39] aspeed queue Stefan Hajnoczi
9 siblings, 0 replies; 11+ messages in thread
From: Cédric Le Goater @ 2025-05-26 8:04 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
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>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20250515081008.583578-7-jamin_lin@aspeedtech.com
Signed-off-by: Cédric Le Goater <clg@redhat.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 22eea62693c7..7da781f8649e 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.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 09/39] hw/misc/aspeed_hace: Extract non-accumulation hash execution into helper function
2025-05-26 8:04 [PULL 00/39] aspeed queue Cédric Le Goater
` (7 preceding siblings ...)
2025-05-26 8:04 ` [PULL 08/39] hw/misc/aspeed_hace: Extract digest write and iov unmap " Cédric Le Goater
@ 2025-05-26 8:04 ` Cédric Le Goater
2025-05-26 18:05 ` [PULL 00/39] aspeed queue Stefan Hajnoczi
9 siblings, 0 replies; 11+ messages in thread
From: Cédric Le Goater @ 2025-05-26 8:04 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Jamin Lin, Cédric Le Goater
From: Jamin Lin <jamin_lin@aspeedtech.com>
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>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20250515081008.583578-8-jamin_lin@aspeedtech.com
Signed-off-by: Cédric Le Goater <clg@redhat.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 7da781f8649e..c50e228cdf95 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.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PULL 00/39] aspeed queue
2025-05-26 8:04 [PULL 00/39] aspeed queue Cédric Le Goater
` (8 preceding siblings ...)
2025-05-26 8:04 ` [PULL 09/39] hw/misc/aspeed_hace: Extract non-accumulation hash execution " Cédric Le Goater
@ 2025-05-26 18:05 ` Stefan Hajnoczi
9 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2025-05-26 18:05 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-arm, qemu-devel, Cédric Le Goater
[-- Attachment #1: Type: text/plain, Size: 116 bytes --]
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/10.1 for any user-visible changes.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread