* [PATCH v3 0/2] Supporting AST2600 HACE engine accumulative mode
@ 2022-03-25 3:58 Steven Lee
2022-03-25 3:58 ` [PATCH v3 1/2] aspeed/hace: Support AST2600 HACE Steven Lee
2022-03-25 3:58 ` [PATCH v3 2/2] tests/qtest: Add test for Aspeed HACE accumulative mode Steven Lee
0 siblings, 2 replies; 5+ messages in thread
From: Steven Lee @ 2022-03-25 3:58 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell, Andrew Jeffery,
Joel Stanley, Thomas Huth, Laurent Vivier, Paolo Bonzini,
open list:ASPEED BMCs, open list:All patches CC here
Cc: jamin_lin, troy_lee, steven_lee
This patch series implements ast2600 hace engine with accumulative mode
and unit test against to it.
Verified with following models
- AST2600 with OpenBmc VERSION_ID=2.12.0-dev-660-g4c7b3e692-dirty
- check hash verification in uboot and check whether qemu crashed
during openbmc web gui login.
- AST1030 with ASPEED zephyr SDK v1.04
- run `hash sha256` command in zephyr shell to verify aspeed hace.
Please help to review
Thanks,
Steven
Changes in v3:
- Refactor acc_mode message handling flow.
Steven Lee (2):
aspeed/hace: Support AST2600 HACE
tests/qtest: Add test for Aspeed HACE accumulative mode
hw/misc/aspeed_hace.c | 113 +++++++++++++++++++++++--
include/hw/misc/aspeed_hace.h | 1 +
tests/qtest/aspeed_hace-test.c | 145 +++++++++++++++++++++++++++++++++
3 files changed, 250 insertions(+), 9 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 1/2] aspeed/hace: Support AST2600 HACE
2022-03-25 3:58 [PATCH v3 0/2] Supporting AST2600 HACE engine accumulative mode Steven Lee
@ 2022-03-25 3:58 ` Steven Lee
2022-03-25 7:08 ` Joel Stanley
2022-03-25 3:58 ` [PATCH v3 2/2] tests/qtest: Add test for Aspeed HACE accumulative mode Steven Lee
1 sibling, 1 reply; 5+ messages in thread
From: Steven Lee @ 2022-03-25 3:58 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell, Andrew Jeffery,
Joel Stanley, Thomas Huth, Laurent Vivier, Paolo Bonzini,
open list:ASPEED BMCs, open list:All patches CC here
Cc: jamin_lin, troy_lee, steven_lee
The aspeed ast2600 acculumative mode is described in datasheet
ast2600v10.pdf section 25.6.4:
1. Allocationg and initiating accumulative hash digest write buffer
with initial state.
* Since QEMU crypto/hash api doesn't provide the API to set initial
state of hash library, and the initial state is already setted by
crypto library (gcrypt/glib/...), so skip this step.
2. Calculating accumulative hash digest.
(a) When receiving the last accumulative data, software need to add
padding message at the end of the accumulative data. Padding
message described in specific of MD5, SHA-1, SHA224, SHA256,
SHA512, SHA512/224, SHA512/256.
* Since the crypto library (gcrypt/glib) already pad the
padding message internally.
* This patch is to remove the padding message which fed byguest
machine driver.
Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
---
hw/misc/aspeed_hace.c | 113 +++++++++++++++++++++++++++++++---
include/hw/misc/aspeed_hace.h | 1 +
2 files changed, 105 insertions(+), 9 deletions(-)
diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 10f00e65f4..a883625e92 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -11,6 +11,7 @@
#include "qemu/osdep.h"
#include "qemu/log.h"
#include "qemu/error-report.h"
+#include "qemu/bswap.h"
#include "hw/misc/aspeed_hace.h"
#include "qapi/error.h"
#include "migration/vmstate.h"
@@ -27,6 +28,7 @@
#define R_HASH_SRC (0x20 / 4)
#define R_HASH_DEST (0x24 / 4)
+#define R_HASH_KEY_BUFF (0x28 / 4)
#define R_HASH_SRC_LEN (0x2c / 4)
#define R_HASH_CMD (0x30 / 4)
@@ -94,12 +96,17 @@ static int hash_algo_lookup(uint32_t reg)
return -1;
}
-static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode)
+static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
+ bool acc_mode)
{
struct iovec iov[ASPEED_HACE_MAX_SG];
g_autofree uint8_t *digest_buf;
size_t digest_len = 0;
- int i;
+ int i, j;
+ static struct iovec iov_cache[ASPEED_HACE_MAX_SG];
+ static int cnt;
+ static bool has_cache;
+ static uint32_t total_len;
if (sg_mode) {
uint32_t len = 0;
@@ -123,12 +130,93 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode)
MEMTXATTRS_UNSPECIFIED, NULL);
addr &= SG_LIST_ADDR_MASK;
- iov[i].iov_len = len & SG_LIST_LEN_MASK;
- plen = iov[i].iov_len;
+ plen = len & SG_LIST_LEN_MASK;
iov[i].iov_base = address_space_map(&s->dram_as, addr, &plen, false,
MEMTXATTRS_UNSPECIFIED);
+
+ if (acc_mode) {
+ total_len += plen;
+
+ if (len & SG_LIST_LEN_LAST) {
+ /*
+ * In the padding message, the last 64/128 bit represents
+ * the total length of bitstream in big endian.
+ * SHA-224, SHA-256 are 64 bit
+ * SHA-384, SHA-512, SHA-512/224, SHA-512/256 are 128 bit
+ * However, we would not process such a huge bit stream.
+ */
+ uint8_t *padding = iov[i].iov_base;
+ uint32_t llen = (uint32_t)(ldq_be_p(iov[i].iov_base + plen - 8) / 8);
+ uint32_t pad_start_off = 0;
+
+ if (llen <= total_len) {
+ uint32_t padding_size = total_len - llen;
+ pad_start_off = plen - padding_size;
+ }
+
+ /*
+ * FIXME:
+ * length with sg_last_bit doesn't mean the message
+ * contains padding.
+ * Need to find a better way to check whether the current payload
+ * contains padding message.
+ * Current solution is to check:
+ * - padding start byte is 0x80
+ * - message length should less than total length(msg + padding)
+ */
+ if (llen > total_len || padding[pad_start_off] != 0x80) {
+ has_cache = true;
+ iov_cache[cnt].iov_base = iov[i].iov_base;
+ iov_cache[cnt].iov_len = plen;
+ cnt++;
+ } else {
+ if (has_cache) {
+ has_cache = false;
+ if (pad_start_off != 0) {
+ iov_cache[cnt].iov_base = iov[i].iov_base;
+ iov_cache[cnt].iov_len = pad_start_off;
+ cnt++;
+ }
+ for (j = 0; j < cnt; j++) {
+ iov[j].iov_base = iov_cache[j].iov_base;
+ iov[j].iov_len = iov_cache[j].iov_len;
+ }
+ /*
+ * This should be the last message as it contains
+ * padding message.
+ * store cnt(count of iov), clear cache and break
+ * the loop.
+ */
+ i = cnt;
+ cnt = 0;
+ total_len = 0;
+ break;
+ }
+ plen -= total_len - llen;
+ cnt = 0;
+ total_len = 0;
+ }
+ }
+ }
+ iov[i].iov_len = plen;
}
} else {
+ /*
+ * FIXME:
+ * Driver sends hash_update() and hash_final() with
+ * sg_mode enable in aspeed ast2600 u-boot and ast1030 zephyr sdk.
+ * Clear cache if driver trigger hash_update() with sg_mode enable
+ * but trigger hash_final() without sg mode for preventing iov_cache
+ * overflow.
+ */
+ if (cnt || total_len) {
+ cnt = 0;
+ total_len = 0;
+ qemu_log_mask(LOG_UNIMP,
+ "hash update with sg_mode and hash_final() without"
+ "sg mode is not yet implemented\n");
+ }
+
hwaddr len = s->regs[R_HASH_SRC_LEN];
iov[0].iov_len = len;
@@ -210,6 +298,9 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
case R_HASH_DEST:
data &= ahc->dest_mask;
break;
+ case R_HASH_KEY_BUFF:
+ data &= ahc->key_mask;
+ break;
case R_HASH_SRC_LEN:
data &= 0x0FFFFFFF;
break;
@@ -229,12 +320,13 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
}
algo = hash_algo_lookup(data);
if (algo < 0) {
- qemu_log_mask(LOG_GUEST_ERROR,
- "%s: Invalid hash algorithm selection 0x%"PRIx64"\n",
- __func__, data & ahc->hash_mask);
- break;
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Invalid hash algorithm selection 0x%"PRIx64"\n",
+ __func__, data & ahc->hash_mask);
+ break;
}
- do_hash_operation(s, algo, data & HASH_SG_EN);
+ do_hash_operation(s, algo, data & HASH_SG_EN,
+ ((data & HASH_HMAC_MASK) == HASH_DIGEST_ACCUM));
if (data & HASH_IRQ_EN) {
qemu_irq_raise(s->irq);
@@ -333,6 +425,7 @@ static void aspeed_ast2400_hace_class_init(ObjectClass *klass, void *data)
ahc->src_mask = 0x0FFFFFFF;
ahc->dest_mask = 0x0FFFFFF8;
+ ahc->key_mask = 0x0FFFFFC0;
ahc->hash_mask = 0x000003ff; /* No SG or SHA512 modes */
}
@@ -351,6 +444,7 @@ static void aspeed_ast2500_hace_class_init(ObjectClass *klass, void *data)
ahc->src_mask = 0x3fffffff;
ahc->dest_mask = 0x3ffffff8;
+ ahc->key_mask = 0x3FFFFFC0;
ahc->hash_mask = 0x000003ff; /* No SG or SHA512 modes */
}
@@ -369,6 +463,7 @@ static void aspeed_ast2600_hace_class_init(ObjectClass *klass, void *data)
ahc->src_mask = 0x7FFFFFFF;
ahc->dest_mask = 0x7FFFFFF8;
+ ahc->key_mask = 0x7FFFFFF8;
ahc->hash_mask = 0x00147FFF;
}
diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h
index 94d5ada95f..2242945eb4 100644
--- a/include/hw/misc/aspeed_hace.h
+++ b/include/hw/misc/aspeed_hace.h
@@ -37,6 +37,7 @@ struct AspeedHACEClass {
uint32_t src_mask;
uint32_t dest_mask;
+ uint32_t key_mask;
uint32_t hash_mask;
};
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v3 2/2] tests/qtest: Add test for Aspeed HACE accumulative mode
2022-03-25 3:58 [PATCH v3 0/2] Supporting AST2600 HACE engine accumulative mode Steven Lee
2022-03-25 3:58 ` [PATCH v3 1/2] aspeed/hace: Support AST2600 HACE Steven Lee
@ 2022-03-25 3:58 ` Steven Lee
1 sibling, 0 replies; 5+ messages in thread
From: Steven Lee @ 2022-03-25 3:58 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell, Andrew Jeffery,
Joel Stanley, Thomas Huth, Laurent Vivier, Paolo Bonzini,
open list:ASPEED BMCs, open list:All patches CC here
Cc: jamin_lin, troy_lee, steven_lee
This add two addition test cases for accumulative mode under sg enabled.
The input vector was manually craft with "abc" + bit 1 + padding zeros + L.
The padding length depends on algorithm, i.e. SHA512 (1024 bit),
SHA256 (512 bit).
The result was calculated by command line sha512sum/sha256sum utilities
without padding, i.e. only "abc" ascii text.
Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
---
tests/qtest/aspeed_hace-test.c | 145 +++++++++++++++++++++++++++++++++
1 file changed, 145 insertions(+)
diff --git a/tests/qtest/aspeed_hace-test.c b/tests/qtest/aspeed_hace-test.c
index 09ee31545e..6a2f404b93 100644
--- a/tests/qtest/aspeed_hace-test.c
+++ b/tests/qtest/aspeed_hace-test.c
@@ -21,6 +21,7 @@
#define HACE_ALGO_SHA512 (BIT(5) | BIT(6))
#define HACE_ALGO_SHA384 (BIT(5) | BIT(6) | BIT(10))
#define HACE_SG_EN BIT(18)
+#define HACE_ACCUM_EN BIT(8)
#define HACE_STS 0x1c
#define HACE_RSA_ISR BIT(13)
@@ -96,6 +97,57 @@ static const uint8_t test_result_sg_sha256[] = {
0x55, 0x1e, 0x1e, 0xc5, 0x80, 0xdd, 0x6d, 0x5a, 0x6e, 0xcd, 0xe9, 0xf3,
0xd3, 0x5e, 0x6e, 0x4a, 0x71, 0x7f, 0xbd, 0xe4};
+/*
+ * The accumulative mode requires firmware to provide internal initial state
+ * and message padding (including length L at the end of padding).
+ *
+ * This test vector is a ascii text "abc" with padding message.
+ *
+ * Expected results were generated using command line utitiles:
+ *
+ * echo -n -e 'abc' | dd of=/tmp/test
+ * for hash in sha512sum sha256sum; do $hash /tmp/test; done
+ */
+static const uint8_t test_vector_accum_512[] = {
+ 0x61, 0x62, 0x63, 0x80, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18};
+
+static const uint8_t test_vector_accum_256[] = {
+ 0x61, 0x62, 0x63, 0x80, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18};
+
+static const uint8_t test_result_accum_sha512[] = {
+ 0xdd, 0xaf, 0x35, 0xa1, 0x93, 0x61, 0x7a, 0xba, 0xcc, 0x41, 0x73, 0x49,
+ 0xae, 0x20, 0x41, 0x31, 0x12, 0xe6, 0xfa, 0x4e, 0x89, 0xa9, 0x7e, 0xa2,
+ 0x0a, 0x9e, 0xee, 0xe6, 0x4b, 0x55, 0xd3, 0x9a, 0x21, 0x92, 0x99, 0x2a,
+ 0x27, 0x4f, 0xc1, 0xa8, 0x36, 0xba, 0x3c, 0x23, 0xa3, 0xfe, 0xeb, 0xbd,
+ 0x45, 0x4d, 0x44, 0x23, 0x64, 0x3c, 0xe8, 0x0e, 0x2a, 0x9a, 0xc9, 0x4f,
+ 0xa5, 0x4c, 0xa4, 0x9f};
+
+static const uint8_t test_result_accum_sha256[] = {
+ 0xba, 0x78, 0x16, 0xbf, 0x8f, 0x01, 0xcf, 0xea, 0x41, 0x41, 0x40, 0xde,
+ 0x5d, 0xae, 0x22, 0x23, 0xb0, 0x03, 0x61, 0xa3, 0x96, 0x17, 0x7a, 0x9c,
+ 0xb4, 0x10, 0xff, 0x61, 0xf2, 0x00, 0x15, 0xad};
static void write_regs(QTestState *s, uint32_t base, uint32_t src,
uint32_t length, uint32_t out, uint32_t method)
@@ -308,6 +360,86 @@ static void test_sha512_sg(const char *machine, const uint32_t base,
qtest_quit(s);
}
+static void test_sha256_accum(const char *machine, const uint32_t base,
+ const uint32_t src_addr)
+{
+ QTestState *s = qtest_init(machine);
+
+ const uint32_t buffer_addr = src_addr + 0x1000000;
+ const uint32_t digest_addr = src_addr + 0x4000000;
+ uint8_t digest[32] = {0};
+ struct AspeedSgList array[] = {
+ { cpu_to_le32(sizeof(test_vector_accum_256) | SG_LIST_LEN_LAST),
+ cpu_to_le32(buffer_addr) },
+ };
+
+ /* Check engine is idle, no busy or irq bits set */
+ g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+ /* Write test vector into memory */
+ qtest_memwrite(s, buffer_addr, test_vector_accum_256, sizeof(test_vector_accum_256));
+ qtest_memwrite(s, src_addr, array, sizeof(array));
+
+ write_regs(s, base, src_addr, sizeof(test_vector_accum_256),
+ digest_addr, HACE_ALGO_SHA256 | HACE_SG_EN | HACE_ACCUM_EN);
+
+ /* Check hash IRQ status is asserted */
+ g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
+
+ /* Clear IRQ status and check status is deasserted */
+ qtest_writel(s, base + HACE_STS, 0x00000200);
+ g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+ /* Read computed digest from memory */
+ qtest_memread(s, digest_addr, digest, sizeof(digest));
+
+ /* Check result of computation */
+ g_assert_cmpmem(digest, sizeof(digest),
+ test_result_accum_sha256, sizeof(digest));
+
+ qtest_quit(s);
+}
+
+static void test_sha512_accum(const char *machine, const uint32_t base,
+ const uint32_t src_addr)
+{
+ QTestState *s = qtest_init(machine);
+
+ const uint32_t buffer_addr = src_addr + 0x1000000;
+ const uint32_t digest_addr = src_addr + 0x4000000;
+ uint8_t digest[64] = {0};
+ struct AspeedSgList array[] = {
+ { cpu_to_le32(sizeof(test_vector_accum_512) | SG_LIST_LEN_LAST),
+ cpu_to_le32(buffer_addr) },
+ };
+
+ /* Check engine is idle, no busy or irq bits set */
+ g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+ /* Write test vector into memory */
+ qtest_memwrite(s, buffer_addr, test_vector_accum_512, sizeof(test_vector_accum_512));
+ qtest_memwrite(s, src_addr, array, sizeof(array));
+
+ write_regs(s, base, src_addr, sizeof(test_vector_accum_512),
+ digest_addr, HACE_ALGO_SHA512 | HACE_SG_EN | HACE_ACCUM_EN);
+
+ /* Check hash IRQ status is asserted */
+ g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
+
+ /* Clear IRQ status and check status is deasserted */
+ qtest_writel(s, base + HACE_STS, 0x00000200);
+ g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+ /* Read computed digest from memory */
+ qtest_memread(s, digest_addr, digest, sizeof(digest));
+
+ /* Check result of computation */
+ g_assert_cmpmem(digest, sizeof(digest),
+ test_result_accum_sha512, sizeof(digest));
+
+ qtest_quit(s);
+}
+
struct masks {
uint32_t src;
uint32_t dest;
@@ -396,6 +528,16 @@ static void test_sha512_sg_ast2600(void)
test_sha512_sg("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
}
+static void test_sha256_accum_ast2600(void)
+{
+ test_sha256_accum("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
+}
+
+static void test_sha512_accum_ast2600(void)
+{
+ test_sha512_accum("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
+}
+
static void test_addresses_ast2600(void)
{
test_addresses("-machine ast2600-evb", 0x1e6d0000, &ast2600_masks);
@@ -455,6 +597,9 @@ int main(int argc, char **argv)
qtest_add_func("ast2600/hace/sha512_sg", test_sha512_sg_ast2600);
qtest_add_func("ast2600/hace/sha256_sg", test_sha256_sg_ast2600);
+ qtest_add_func("ast2600/hace/sha512_accum", test_sha512_accum_ast2600);
+ qtest_add_func("ast2600/hace/sha256_accum", test_sha256_accum_ast2600);
+
qtest_add_func("ast2500/hace/addresses", test_addresses_ast2500);
qtest_add_func("ast2500/hace/sha512", test_sha512_ast2500);
qtest_add_func("ast2500/hace/sha256", test_sha256_ast2500);
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] aspeed/hace: Support AST2600 HACE
2022-03-25 3:58 ` [PATCH v3 1/2] aspeed/hace: Support AST2600 HACE Steven Lee
@ 2022-03-25 7:08 ` Joel Stanley
2022-03-29 7:11 ` Steven Lee
0 siblings, 1 reply; 5+ messages in thread
From: Joel Stanley @ 2022-03-25 7:08 UTC (permalink / raw)
To: Steven Lee
Cc: Laurent Vivier, Peter Maydell, Thomas Huth, Jamin Lin,
Andrew Jeffery, Troy Lee, open list:All patches CC here,
open list:ASPEED BMCs, Cédric Le Goater, Paolo Bonzini
Hi Steven,
I've pointed out some small things like spelling fixes, and made a
suggestion to split the change into multiple patches.
Aside from that, I need your help to understand what your change is
trying to do.
On Fri, 25 Mar 2022 at 03:58, Steven Lee <steven_lee@aspeedtech.com> wrote:
>
> The aspeed ast2600 acculumative mode is described in datasheet
accumulative
> ast2600v10.pdf section 25.6.4:
> 1. Allocationg and initiating accumulative hash digest write buffer
allocating
> with initial state.
> * Since QEMU crypto/hash api doesn't provide the API to set initial
> state of hash library, and the initial state is already setted by
> crypto library (gcrypt/glib/...), so skip this step.
> 2. Calculating accumulative hash digest.
> (a) When receiving the last accumulative data, software need to add
> padding message at the end of the accumulative data. Padding
> message described in specific of MD5, SHA-1, SHA224, SHA256,
> SHA512, SHA512/224, SHA512/256.
> * Since the crypto library (gcrypt/glib) already pad the
> padding message internally.
> * This patch is to remove the padding message which fed byguest
> machine driver.
>
> Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
> ---
> hw/misc/aspeed_hace.c | 113 +++++++++++++++++++++++++++++++---
> include/hw/misc/aspeed_hace.h | 1 +
> 2 files changed, 105 insertions(+), 9 deletions(-)
>
> diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
> index 10f00e65f4..a883625e92 100644
> --- a/hw/misc/aspeed_hace.c
> +++ b/hw/misc/aspeed_hace.c
> @@ -11,6 +11,7 @@
> #include "qemu/osdep.h"
> #include "qemu/log.h"
> #include "qemu/error-report.h"
> +#include "qemu/bswap.h"
> #include "hw/misc/aspeed_hace.h"
> #include "qapi/error.h"
> #include "migration/vmstate.h"
> @@ -27,6 +28,7 @@
>
> #define R_HASH_SRC (0x20 / 4)
> #define R_HASH_DEST (0x24 / 4)
> +#define R_HASH_KEY_BUFF (0x28 / 4)
> #define R_HASH_SRC_LEN (0x2c / 4)
>
> #define R_HASH_CMD (0x30 / 4)
> @@ -94,12 +96,17 @@ static int hash_algo_lookup(uint32_t reg)
> return -1;
> }
>
> -static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode)
> +static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
> + bool acc_mode)
> {
> struct iovec iov[ASPEED_HACE_MAX_SG];
> g_autofree uint8_t *digest_buf;
> size_t digest_len = 0;
> - int i;
> + int i, j;
> + static struct iovec iov_cache[ASPEED_HACE_MAX_SG];
> + static int cnt;
Do you mean count? Please call it "count" if that's what you mean.
> + static bool has_cache;
> + static uint32_t total_len;
>
> if (sg_mode) {
> uint32_t len = 0;
> @@ -123,12 +130,93 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode)
> MEMTXATTRS_UNSPECIFIED, NULL);
> addr &= SG_LIST_ADDR_MASK;
>
> - iov[i].iov_len = len & SG_LIST_LEN_MASK;
> - plen = iov[i].iov_len;
> + plen = len & SG_LIST_LEN_MASK;
> iov[i].iov_base = address_space_map(&s->dram_as, addr, &plen, false,
> MEMTXATTRS_UNSPECIFIED);
> +
> + if (acc_mode) {
This function is getting large. We should try to refactor it, instead
of attempting to support all three cases in the one function.
> + total_len += plen;
> +
> + if (len & SG_LIST_LEN_LAST) {
> + /*
> + * In the padding message, the last 64/128 bit represents
> + * the total length of bitstream in big endian.
> + * SHA-224, SHA-256 are 64 bit
> + * SHA-384, SHA-512, SHA-512/224, SHA-512/256 are 128 bit
> + * However, we would not process such a huge bit stream.
> + */
> + uint8_t *padding = iov[i].iov_base;
> + uint32_t llen = (uint32_t)(ldq_be_p(iov[i].iov_base + plen - 8) / 8);
What is llen?
> + uint32_t pad_start_off = 0;
> +
> + if (llen <= total_len) {
> + uint32_t padding_size = total_len - llen;
> + pad_start_off = plen - padding_size;
> + }
I find it hard to follow this code. I tried to look at the u-boot
driver to understand it, and it's equally hard to understand.
Can you try to provide an overview?
> +
> + /*
> + * FIXME:
> + * length with sg_last_bit doesn't mean the message
> + * contains padding.
> + * Need to find a better way to check whether the current payload
> + * contains padding message.
> + * Current solution is to check:
> + * - padding start byte is 0x80
> + * - message length should less than total length(msg + padding)
> + */
> + if (llen > total_len || padding[pad_start_off] != 0x80) {
> + has_cache = true;
> + iov_cache[cnt].iov_base = iov[i].iov_base;
> + iov_cache[cnt].iov_len = plen;
> + cnt++;
> + } else {
> + if (has_cache) {
> + has_cache = false;
> + if (pad_start_off != 0) {
> + iov_cache[cnt].iov_base = iov[i].iov_base;
> + iov_cache[cnt].iov_len = pad_start_off;
> + cnt++;
> + }
> + for (j = 0; j < cnt; j++) {
> + iov[j].iov_base = iov_cache[j].iov_base;
> + iov[j].iov_len = iov_cache[j].iov_len;
Can you explain why you've needed to add iov_cache?
> + }
> + /*
> + * This should be the last message as it contains
> + * padding message.
> + * store cnt(count of iov), clear cache and break
> + * the loop.
> + */
> + i = cnt;
> + cnt = 0;
> + total_len = 0;
> + break;
> + }
> + plen -= total_len - llen;
> + cnt = 0;
> + total_len = 0;
> + }
> + }
> + }
> + iov[i].iov_len = plen;
> }
> } else {
> + /*
> + * FIXME:
> + * Driver sends hash_update() and hash_final() with
> + * sg_mode enable in aspeed ast2600 u-boot and ast1030 zephyr sdk.
> + * Clear cache if driver trigger hash_update() with sg_mode enable
> + * but trigger hash_final() without sg mode for preventing iov_cache
> + * overflow.
I don't follow your explanation. If we have sg_mode enabled then we
won't enter this condition.
This looks like a fix for a separate issue. If it is, then put it in a
different patch so we can review it there.
> + */
> + if (cnt || total_len) {
> + cnt = 0;
> + total_len = 0;
> + qemu_log_mask(LOG_UNIMP,
> + "hash update with sg_mode and hash_final() without"
> + "sg mode is not yet implemented\n");
> + }
> +
> hwaddr len = s->regs[R_HASH_SRC_LEN];
>
> iov[0].iov_len = len;
> @@ -210,6 +298,9 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
> case R_HASH_DEST:
> data &= ahc->dest_mask;
> break;
> + case R_HASH_KEY_BUFF:
> + data &= ahc->key_mask;
> + break;
You could make the key_mask/KEY_BUFF change a seperate patch.
> case R_HASH_SRC_LEN:
> data &= 0x0FFFFFFF;
> break;
> @@ -229,12 +320,13 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
> }
> algo = hash_algo_lookup(data);
> if (algo < 0) {
> - qemu_log_mask(LOG_GUEST_ERROR,
> - "%s: Invalid hash algorithm selection 0x%"PRIx64"\n",
> - __func__, data & ahc->hash_mask);
> - break;
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Invalid hash algorithm selection 0x%"PRIx64"\n",
> + __func__, data & ahc->hash_mask);
> + break;
This whitespace change looks unrelated.
> }
> - do_hash_operation(s, algo, data & HASH_SG_EN);
> + do_hash_operation(s, algo, data & HASH_SG_EN,
> + ((data & HASH_HMAC_MASK) == HASH_DIGEST_ACCUM));
>
> if (data & HASH_IRQ_EN) {
> qemu_irq_raise(s->irq);
> @@ -333,6 +425,7 @@ static void aspeed_ast2400_hace_class_init(ObjectClass *klass, void *data)
>
> ahc->src_mask = 0x0FFFFFFF;
> ahc->dest_mask = 0x0FFFFFF8;
> + ahc->key_mask = 0x0FFFFFC0;
> ahc->hash_mask = 0x000003ff; /* No SG or SHA512 modes */
> }
>
> @@ -351,6 +444,7 @@ static void aspeed_ast2500_hace_class_init(ObjectClass *klass, void *data)
>
> ahc->src_mask = 0x3fffffff;
> ahc->dest_mask = 0x3ffffff8;
> + ahc->key_mask = 0x3FFFFFC0;
> ahc->hash_mask = 0x000003ff; /* No SG or SHA512 modes */
> }
>
> @@ -369,6 +463,7 @@ static void aspeed_ast2600_hace_class_init(ObjectClass *klass, void *data)
>
> ahc->src_mask = 0x7FFFFFFF;
> ahc->dest_mask = 0x7FFFFFF8;
> + ahc->key_mask = 0x7FFFFFF8;
> ahc->hash_mask = 0x00147FFF;
> }
>
> diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h
> index 94d5ada95f..2242945eb4 100644
> --- a/include/hw/misc/aspeed_hace.h
> +++ b/include/hw/misc/aspeed_hace.h
> @@ -37,6 +37,7 @@ struct AspeedHACEClass {
>
> uint32_t src_mask;
> uint32_t dest_mask;
> + uint32_t key_mask;
> uint32_t hash_mask;
> };
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] aspeed/hace: Support AST2600 HACE
2022-03-25 7:08 ` Joel Stanley
@ 2022-03-29 7:11 ` Steven Lee
0 siblings, 0 replies; 5+ messages in thread
From: Steven Lee @ 2022-03-29 7:11 UTC (permalink / raw)
To: Joel Stanley
Cc: Laurent Vivier, Peter Maydell, Thomas Huth, Jamin Lin,
Andrew Jeffery, Troy Lee, open list:All patches CC here,
open list:ASPEED BMCs, Cédric Le Goater, Paolo Bonzini
The 03/25/2022 15:08, Joel Stanley wrote:
> Hi Steven,
>
> I've pointed out some small things like spelling fixes, and made a
> suggestion to split the change into multiple patches.
>
> Aside from that, I need your help to understand what your change is
> trying to do.
>
Hi Joel,
Thanks for the review and sorry for late reply, I was taking Monday off.
I added some examples to describe the driver behavior below, hope it helps.
> On Fri, 25 Mar 2022 at 03:58, Steven Lee <steven_lee@aspeedtech.com> wrote:
> >
> > The aspeed ast2600 acculumative mode is described in datasheet
>
> accumulative
>
will fix it.
> > ast2600v10.pdf section 25.6.4:
> > 1. Allocationg and initiating accumulative hash digest write buffer
>
> allocating
>
will fix it.
> > with initial state.
> > * Since QEMU crypto/hash api doesn't provide the API to set initial
> > state of hash library, and the initial state is already setted by
> > crypto library (gcrypt/glib/...), so skip this step.
> > 2. Calculating accumulative hash digest.
> > (a) When receiving the last accumulative data, software need to add
> > padding message at the end of the accumulative data. Padding
> > message described in specific of MD5, SHA-1, SHA224, SHA256,
> > SHA512, SHA512/224, SHA512/256.
> > * Since the crypto library (gcrypt/glib) already pad the
> > padding message internally.
> > * This patch is to remove the padding message which fed byguest
> > machine driver.
> >
> > Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
> > Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
> > ---
> > hw/misc/aspeed_hace.c | 113 +++++++++++++++++++++++++++++++---
> > include/hw/misc/aspeed_hace.h | 1 +
> > 2 files changed, 105 insertions(+), 9 deletions(-)
> >
> > diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
> > index 10f00e65f4..a883625e92 100644
> > --- a/hw/misc/aspeed_hace.c
> > +++ b/hw/misc/aspeed_hace.c
> > @@ -11,6 +11,7 @@
> > #include "qemu/osdep.h"
> > #include "qemu/log.h"
> > #include "qemu/error-report.h"
> > +#include "qemu/bswap.h"
> > #include "hw/misc/aspeed_hace.h"
> > #include "qapi/error.h"
> > #include "migration/vmstate.h"
> > @@ -27,6 +28,7 @@
> >
> > #define R_HASH_SRC (0x20 / 4)
> > #define R_HASH_DEST (0x24 / 4)
> > +#define R_HASH_KEY_BUFF (0x28 / 4)
> > #define R_HASH_SRC_LEN (0x2c / 4)
> >
> > #define R_HASH_CMD (0x30 / 4)
> > @@ -94,12 +96,17 @@ static int hash_algo_lookup(uint32_t reg)
> > return -1;
> > }
> >
> > -static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode)
> > +static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
> > + bool acc_mode)
> > {
> > struct iovec iov[ASPEED_HACE_MAX_SG];
> > g_autofree uint8_t *digest_buf;
> > size_t digest_len = 0;
> > - int i;
> > + int i, j;
> > + static struct iovec iov_cache[ASPEED_HACE_MAX_SG];
> > + static int cnt;
>
> Do you mean count? Please call it "count" if that's what you mean.
>
Yes, I will rename it to "count".
> > + static bool has_cache;
> > + static uint32_t total_len;
> >
> > if (sg_mode) {
> > uint32_t len = 0;
> > @@ -123,12 +130,93 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode)
> > MEMTXATTRS_UNSPECIFIED, NULL);
> > addr &= SG_LIST_ADDR_MASK;
> >
> > - iov[i].iov_len = len & SG_LIST_LEN_MASK;
> > - plen = iov[i].iov_len;
> > + plen = len & SG_LIST_LEN_MASK;
> > iov[i].iov_base = address_space_map(&s->dram_as, addr, &plen, false,
> > MEMTXATTRS_UNSPECIFIED);
> > +
> > + if (acc_mode) {
>
> This function is getting large. We should try to refactor it, instead
> of attempting to support all three cases in the one function.
>
will refactor it.
> > + total_len += plen;
> > +
> > + if (len & SG_LIST_LEN_LAST) {
> > + /*
> > + * In the padding message, the last 64/128 bit represents
> > + * the total length of bitstream in big endian.
> > + * SHA-224, SHA-256 are 64 bit
> > + * SHA-384, SHA-512, SHA-512/224, SHA-512/256 are 128 bit
> > + * However, we would not process such a huge bit stream.
> > + */
> > + uint8_t *padding = iov[i].iov_base;
> > + uint32_t llen = (uint32_t)(ldq_be_p(iov[i].iov_base + plen - 8) / 8);
>
> What is llen?
>
message length in the last messge.
Is rename "llen" to "message_len" ok?
> > + uint32_t pad_start_off = 0;
> > +
> > + if (llen <= total_len) {
> > + uint32_t padding_size = total_len - llen;
> > + pad_start_off = plen - padding_size;
> > + }
>
> I find it hard to follow this code. I tried to look at the u-boot
> driver to understand it, and it's equally hard to understand.
>
> Can you try to provide an overview?
>
aspeed_hace of aspeed u-boot and ast1030 zephyr sdk have the following behavior
1. Driver calls hash_final() or sha_digest() to trigger 1 hash operation request.
the data is put in iov[0].iov_base.
Example 1-a. request is "abc", driver adds padding to make the
message length to be multiples of 64.
iov[0].iov_base =
61 62 63 80 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 18
padding start with 0x80, the last 8 bytes are length of request
length = 0x18 / 8 = 3
Example 1-b. request length is 56.
In this case, because of the minimum size of padding is 9(0x80(1 byte) + message_len(8 bytes)).
request size + minimum padding size = 56 + 9 = 65, it is not multiples of 64,
driver adds padding to make the size of message as 128.
iov[0].iov_base =
61 62 63 64 62 63 64 65 63 64 65 66 64 65 66 67
65 66 67 68 66 67 68 69 67 68 69 6a 68 69 6a 6b
69 6a 6b 6c 6a 6b 6c 6d 6b 6c 6d 6e 6c 6d 6e 6f
6d 6e 6f 70 6e 6f 70 71 80 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 c0
Example 1-c. Driver put all data in a request and trigger hash operation .
https://github.com/AspeedTech-BMC/u-boot/blob/v00.04.09/drivers/crypto/aspeed_hace.c#L273
iov[0].iov_base =
0e 02 a0 06 0d 02 f2 6c 88 cd 01 f0 4e 01 18 be
0f 02 7d 09 e4 13 93 1a 40 3b ba 0d 91 1a 0d 02
...
iov[1].iov_base =
80 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 56 78
2. Driver calls hash_update() to trigger one or multiple hash operation requests, then
call hash_final() to trigger the last hash operation request.
Example 2-a. Driver call hash_update() with the following request:
iov[0].iov_base =
41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f 50
51 52 53 54 55 56 57 58 59 5a 61 62 63 64 65 66
67 68 69 6a 6b 6c 6d 6e 6f 70 71 72 73 74 75 76
77 78 79 7a 30 31 32 33 34 35 36 37 38 39 2b 2d
Driver call hash_final() with padding as request length is multiples of 64:
iov[0].iov_base =
80 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 02 00
length = 200 / 8 = 64
Example 2-b. Driver call hash_update() with the request that
length is not multiples of 64:
iov[0].iov_base =
08 9f 13 aa 41 d8 4c e3 7a 11 85 1c b3 27 be 55
ec 60 f7 8e 02 99 30 c7 3b d2 69 00 74 0b a2 16
ad 44 db 4f e6 7d 14 88 1f b6 2a c1 58 ef 63 fa
91 05 9c 33 ca 3e d5 6c 03 77 0e a5 19 b0 47 de
...
Driver call hash_final() with the request data and padding
iov[0].iov_base =
0d a4 18 af 46 dd 51 e8 7f 16 8a 21 b8 2c c3 5a
f1 65 fc 93 07 9e 35 cc 40 d7 6e 05 79 10 a7 1b
b2 49 e0 54 eb 82 19 8d 24 bb 2f c6 5d f4 68 ff
96 0a a1 38 cf 43 da 71 08 7c 13 aa 1e b5 4c 80
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 1f f8
length = 1ff8 / 8 = 1023
> > +
> > + /*
> > + * FIXME:
> > + * length with sg_last_bit doesn't mean the message
> > + * contains padding.
> > + * Need to find a better way to check whether the current payload
> > + * contains padding message.
> > + * Current solution is to check:
> > + * - padding start byte is 0x80
> > + * - message length should less than total length(msg + padding)
> > + */
> > + if (llen > total_len || padding[pad_start_off] != 0x80) {
> > + has_cache = true;
> > + iov_cache[cnt].iov_base = iov[i].iov_base;
> > + iov_cache[cnt].iov_len = plen;
> > + cnt++;
> > + } else {
> > + if (has_cache) {
> > + has_cache = false;
> > + if (pad_start_off != 0) {
> > + iov_cache[cnt].iov_base = iov[i].iov_base;
> > + iov_cache[cnt].iov_len = pad_start_off;
> > + cnt++;
> > + }
> > + for (j = 0; j < cnt; j++) {
> > + iov[j].iov_base = iov_cache[j].iov_base;
> > + iov[j].iov_len = iov_cache[j].iov_len;
>
> Can you explain why you've needed to add iov_cache?
>
iov_cache is for example 2-a and 2-b mentioned above.
> > + }
> > + /*
> > + * This should be the last message as it contains
> > + * padding message.
> > + * store cnt(count of iov), clear cache and break
> > + * the loop.
> > + */
> > + i = cnt;
> > + cnt = 0;
> > + total_len = 0;
> > + break;
> > + }
> > + plen -= total_len - llen;
> > + cnt = 0;
> > + total_len = 0;
> > + }
> > + }
> > + }
> > + iov[i].iov_len = plen;
> > }
> > } else {
> > + /*
> > + * FIXME:
> > + * Driver sends hash_update() and hash_final() with
> > + * sg_mode enable in aspeed ast2600 u-boot and ast1030 zephyr sdk.
> > + * Clear cache if driver trigger hash_update() with sg_mode enable
> > + * but trigger hash_final() without sg mode for preventing iov_cache
> > + * overflow.
>
> I don't follow your explanation. If we have sg_mode enabled then we
> won't enter this condition.
>
Yes, if driver enable sg_mode in both hash_update() and hash_final(), then the implementation
is not necessary.
For instance, aspeed u-boot stores sg_enable in ctx->method.
https://github.com/AspeedTech-BMC/u-boot/blob/v00.04.09/drivers/crypto/aspeed_hace.c#L165
hash_update() and hash_finish() pass the the same ctx to qemu aspeed
hace model.
https://github.com/AspeedTech-BMC/u-boot/blob/v00.04.09/drivers/crypto/aspeed_hace.c#L237
https://github.com/AspeedTech-BMC/u-boot/blob/v00.04.09/drivers/crypto/aspeed_hace.c#L264
However, I found aspeed kernel driver only enable sg_mode in hash_update.
It enable sg_mode during hash_update()
https://github.com/AspeedTech-BMC/linux/blob/v00.05.00/drivers/crypto/aspeed/aspeed-hace-hash.c#L369
then clear sg_mode after trigger hash_update().
https://github.com/AspeedTech-BMC/linux/blob/v00.05.00/drivers/crypto/aspeed/aspeed-hace-hash.c#L314
It means sg_mode is not enable in hash_final() below.
https://github.com/AspeedTech-BMC/linux/blob/v00.05.00/drivers/crypto/aspeed/aspeed-hace-hash.c#L377
Thus if we don't clear the cache for this case, the next sg_en hash_update() will get
wrong result as there is a previous request in the cache.
> This looks like a fix for a separate issue. If it is, then put it in a
> different patch so we can review it there.
>
>
>
will fix it.
> > + */
> > + if (cnt || total_len) {
> > + cnt = 0;
> > + total_len = 0;
> > + qemu_log_mask(LOG_UNIMP,
> > + "hash update with sg_mode and hash_final() without"
> > + "sg mode is not yet implemented\n");
> > + }
> > +
> > hwaddr len = s->regs[R_HASH_SRC_LEN];
> >
> > iov[0].iov_len = len;
> > @@ -210,6 +298,9 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
> > case R_HASH_DEST:
> > data &= ahc->dest_mask;
> > break;
> > + case R_HASH_KEY_BUFF:
> > + data &= ahc->key_mask;
> > + break;
>
> You could make the key_mask/KEY_BUFF change a seperate patch.
>
will fix it.
> > case R_HASH_SRC_LEN:
> > data &= 0x0FFFFFFF;
> > break;
> > @@ -229,12 +320,13 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
> > }
> > algo = hash_algo_lookup(data);
> > if (algo < 0) {
> > - qemu_log_mask(LOG_GUEST_ERROR,
> > - "%s: Invalid hash algorithm selection 0x%"PRIx64"\n",
> > - __func__, data & ahc->hash_mask);
> > - break;
> > + qemu_log_mask(LOG_GUEST_ERROR,
> > + "%s: Invalid hash algorithm selection 0x%"PRIx64"\n",
> > + __func__, data & ahc->hash_mask);
> > + break;
>
> This whitespace change looks unrelated.
>
> > }
will fix it
> > - do_hash_operation(s, algo, data & HASH_SG_EN);
> > + do_hash_operation(s, algo, data & HASH_SG_EN,
> > + ((data & HASH_HMAC_MASK) == HASH_DIGEST_ACCUM));
> >
> > if (data & HASH_IRQ_EN) {
> > qemu_irq_raise(s->irq);
> > @@ -333,6 +425,7 @@ static void aspeed_ast2400_hace_class_init(ObjectClass *klass, void *data)
> >
> > ahc->src_mask = 0x0FFFFFFF;
> > ahc->dest_mask = 0x0FFFFFF8;
> > + ahc->key_mask = 0x0FFFFFC0;
> > ahc->hash_mask = 0x000003ff; /* No SG or SHA512 modes */
> > }
> >
> > @@ -351,6 +444,7 @@ static void aspeed_ast2500_hace_class_init(ObjectClass *klass, void *data)
> >
> > ahc->src_mask = 0x3fffffff;
> > ahc->dest_mask = 0x3ffffff8;
> > + ahc->key_mask = 0x3FFFFFC0;
> > ahc->hash_mask = 0x000003ff; /* No SG or SHA512 modes */
> > }
> >
> > @@ -369,6 +463,7 @@ static void aspeed_ast2600_hace_class_init(ObjectClass *klass, void *data)
> >
> > ahc->src_mask = 0x7FFFFFFF;
> > ahc->dest_mask = 0x7FFFFFF8;
> > + ahc->key_mask = 0x7FFFFFF8;
> > ahc->hash_mask = 0x00147FFF;
> > }
> >
> > diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h
> > index 94d5ada95f..2242945eb4 100644
> > --- a/include/hw/misc/aspeed_hace.h
> > +++ b/include/hw/misc/aspeed_hace.h
> > @@ -37,6 +37,7 @@ struct AspeedHACEClass {
> >
> > uint32_t src_mask;
> > uint32_t dest_mask;
> > + uint32_t key_mask;
> > uint32_t hash_mask;
> > };
> >
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-03-29 7:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-25 3:58 [PATCH v3 0/2] Supporting AST2600 HACE engine accumulative mode Steven Lee
2022-03-25 3:58 ` [PATCH v3 1/2] aspeed/hace: Support AST2600 HACE Steven Lee
2022-03-25 7:08 ` Joel Stanley
2022-03-29 7:11 ` Steven Lee
2022-03-25 3:58 ` [PATCH v3 2/2] tests/qtest: Add test for Aspeed HACE accumulative mode Steven Lee
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.