[PATCH v1 0/1] aspeed/hace: Support AST1030 HACE

2022-05-02 Thread Steven Lee
Per ast1030_v7.pdf, AST1030 HACE engine is identical to AST2600's HACE
engine.

Please help to review.

Thanks,
Steven

Based-on: 20220426021120.28255-3-steven_...@aspeedtech.com
([v6,2/3] aspeed/hace: Support AST2600 HACE)

Steven Lee (1):
  aspeed/hace: Support AST1030 HACE

 hw/misc/aspeed_hace.c | 20 
 include/hw/misc/aspeed_hace.h |  2 ++
 2 files changed, 22 insertions(+)

-- 
2.17.1




[PATCH v1 1/1] aspeed/hace: Support AST1030 HACE

2022-05-02 Thread Steven Lee
Per ast1030_v7.pdf, AST1030 HACE engine is identical to AST2600's HACE
engine.

Signed-off-by: Steven Lee 
---
 hw/misc/aspeed_hace.c | 20 
 include/hw/misc/aspeed_hace.h |  2 ++
 2 files changed, 22 insertions(+)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 10f00e65f4..91f3c0b208 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -378,11 +378,31 @@ static const TypeInfo aspeed_ast2600_hace_info = {
 .class_init = aspeed_ast2600_hace_class_init,
 };
 
+static void aspeed_ast1030_hace_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+AspeedHACEClass *ahc = ASPEED_HACE_CLASS(klass);
+
+dc->desc = "AST1030 Hash and Crypto Engine";
+
+ahc->src_mask = 0x7FFF;
+ahc->dest_mask = 0x7FF8;
+ahc->key_mask = 0x7FF8;
+ahc->hash_mask = 0x00147FFF;
+}
+
+static const TypeInfo aspeed_ast1030_hace_info = {
+.name = TYPE_ASPEED_AST1030_HACE,
+.parent = TYPE_ASPEED_HACE,
+.class_init = aspeed_ast1030_hace_class_init,
+};
+
 static void aspeed_hace_register_types(void)
 {
 type_register_static(_ast2400_hace_info);
 type_register_static(_ast2500_hace_info);
 type_register_static(_ast2600_hace_info);
+type_register_static(_ast1030_hace_info);
 type_register_static(_hace_info);
 }
 
diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h
index 94d5ada95f..e9d3563a05 100644
--- a/include/hw/misc/aspeed_hace.h
+++ b/include/hw/misc/aspeed_hace.h
@@ -15,6 +15,8 @@
 #define TYPE_ASPEED_AST2400_HACE TYPE_ASPEED_HACE "-ast2400"
 #define TYPE_ASPEED_AST2500_HACE TYPE_ASPEED_HACE "-ast2500"
 #define TYPE_ASPEED_AST2600_HACE TYPE_ASPEED_HACE "-ast2600"
+#define TYPE_ASPEED_AST1030_HACE TYPE_ASPEED_HACE "-ast1030"
+
 OBJECT_DECLARE_TYPE(AspeedHACEState, AspeedHACEClass, ASPEED_HACE)
 
 #define ASPEED_HACE_NR_REGS (0x64 >> 2)
-- 
2.17.1




[PATCH v6 1/3] aspeed/hace: Support HMAC Key Buffer register.

2022-04-25 Thread Steven Lee
Support HACE28: Hash HMAC Key Buffer Base Address Register.

Signed-off-by: Troy Lee 
Signed-off-by: Steven Lee 
Reviewed-by: Cédric Le Goater 
---
 hw/misc/aspeed_hace.c | 7 +++
 include/hw/misc/aspeed_hace.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 10f00e65f4..59fe5bfca2 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -27,6 +27,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)
@@ -210,6 +211,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 &= 0x0FFF;
 break;
@@ -333,6 +337,7 @@ static void aspeed_ast2400_hace_class_init(ObjectClass 
*klass, void *data)
 
 ahc->src_mask = 0x0FFF;
 ahc->dest_mask = 0x0FF8;
+ahc->key_mask = 0x0FC0;
 ahc->hash_mask = 0x03ff; /* No SG or SHA512 modes */
 }
 
@@ -351,6 +356,7 @@ static void aspeed_ast2500_hace_class_init(ObjectClass 
*klass, void *data)
 
 ahc->src_mask = 0x3fff;
 ahc->dest_mask = 0x3ff8;
+ahc->key_mask = 0x3FC0;
 ahc->hash_mask = 0x03ff; /* No SG or SHA512 modes */
 }
 
@@ -369,6 +375,7 @@ static void aspeed_ast2600_hace_class_init(ObjectClass 
*klass, void *data)
 
 ahc->src_mask = 0x7FFF;
 ahc->dest_mask = 0x7FF8;
+ahc->key_mask = 0x7FF8;
 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




[PATCH v6 2/3] aspeed/hace: Support AST2600 HACE

2022-04-25 Thread Steven Lee
The aspeed ast2600 accumulative mode is described in datasheet
ast2600v10.pdf section 25.6.4:
 1. Allocating 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 set 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 
Signed-off-by: Steven Lee 
---
 hw/misc/aspeed_hace.c | 132 --
 include/hw/misc/aspeed_hace.h |   4 ++
 2 files changed, 131 insertions(+), 5 deletions(-)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 59fe5bfca2..0f4059e6df 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -65,7 +65,6 @@
 #define SG_LIST_ADDR_SIZE   4
 #define SG_LIST_ADDR_MASK   0x7FFF
 #define SG_LIST_ENTRY_SIZE  (SG_LIST_LEN_SIZE + SG_LIST_ADDR_SIZE)
-#define ASPEED_HACE_MAX_SG  256/* max number of entries */
 
 static const struct {
 uint32_t mask;
@@ -95,11 +94,104 @@ static int hash_algo_lookup(uint32_t reg)
 return -1;
 }
 
-static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode)
+/**
+ * Check whether the request contains padding message.
+ *
+ * @param s aspeed hace state object
+ * @param iov   iov of current request
+ * @param req_len   length of the current request
+ * @param total_msg_len length of all acc_mode requests(excluding padding msg)
+ * @param pad_offsetstart offset of padding message
+ */
+static bool has_padding(AspeedHACEState *s, struct iovec *iov,
+hwaddr req_len, uint32_t *total_msg_len,
+uint32_t *pad_offset)
+{
+*total_msg_len = (uint32_t)(ldq_be_p(iov->iov_base + req_len - 8) / 8);
+/*
+ * SG_LIST_LEN_LAST asserted in the request length doesn't mean it is the
+ * last request. The last request should contain padding message.
+ * We check whether message contains padding by
+ *   1. Get total message length. If the current message contains
+ *  padding, the last 8 bytes are total message length.
+ *   2. Check whether the total message length is valid.
+ *  If it is valid, the value should less than or equal to
+ *  total_req_len.
+ *   3. Current request len - padding_size to get padding offset.
+ *  The padding message's first byte should be 0x80
+ */
+if (*total_msg_len <= s->total_req_len) {
+uint32_t padding_size = s->total_req_len - *total_msg_len;
+uint8_t *padding = iov->iov_base;
+*pad_offset = req_len - padding_size;
+if (padding[*pad_offset] == 0x80) {
+return true;
+}
+}
+
+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;
+}
+
+/**
+ * Generate iov for accumulative mode.
+ *
+ * @param s aspeed hace state object
+ * @param iov   iov of the current request
+ * @param idindex of the current iov
+ * @param req_len   length of the current request
+ *
+ * @return count of iov
+ */
+static int gen_acc_mode_iov(AspeedHACEState *s, struct iovec *iov, int id,
+hwaddr *req_len)
+{
+uint32_t pad_offset;
+uint32_t total_msg_len;
+s->total_req_len += *req_len;
+
+if (has_padding(s, [id], *req_len, _msg_len, _offset)) {
+if (s->iov_count) {
+return reconstruct_iov(s, iov, id, _offset);
+}
+
+*req_len -= s->total_req_len - total_msg_len;
+s->total_req_len = 0;
+iov[id].iov_len = *req_len;
+} else {
+s->iov_cache[s->iov_count].iov_base = iov->iov_base;
+s->iov_cache[s->iov_count].iov_len = *req_len;
+++s->iov_count;
+}
+
+return id + 

[PATCH v6 0/3] aspeed/hace: Support AST2600 HACE

2022-04-25 Thread 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 v6:
- Bug fixes in reconstruct_iov().

Steven Lee (3):
  aspeed/hace: Support HMAC Key Buffer register.
  aspeed/hace: Support AST2600 HACE
  tests/qtest: Add test for Aspeed HACE accumulative mode

 hw/misc/aspeed_hace.c  | 139 +--
 include/hw/misc/aspeed_hace.h  |   5 ++
 tests/qtest/aspeed_hace-test.c | 145 +
 3 files changed, 284 insertions(+), 5 deletions(-)

-- 
2.17.1




[PATCH v6 3/3] tests/qtest: Add test for Aspeed HACE accumulative mode

2022-04-25 Thread 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 
Signed-off-by: Steven Lee 
Acked-by: Thomas Huth 
Reviewed-by: Joel Stanley 
---
 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 58aa22014d..85d705ec50 100644
--- a/tests/qtest/aspeed_hace-test.c
+++ b/tests/qtest/aspeed_hace-test.c
@@ -20,6 +20,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_ISRBIT(13)
@@ -95,6 +96,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)
@@ -307,6 +359,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 + 0x100;
+const uint32_t digest_addr = src_addr + 0x400;
+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);
+
+/* Che

Re: [PATCH v5 2/3] aspeed/hace: Support AST2600 HACE

2022-04-25 Thread Steven Lee
The 04/25/2022 21:46, Cédric Le Goater wrote:
> On 4/22/22 07:19, Steven Lee wrote:
> > +/**
> > + * Check whether the request contains padding message.
> > + *
> > + * @param s aspeed hace state object
> > + * @param iov   iov of current request
> > + * @param req_len   length of the current request
> > + * @param total_msg_len length of all acc_mode requests(excluding padding 
> > msg)
> > + * @param pad_offsetstart offset of padding message
> > + */
> > +static bool has_padding(AspeedHACEState *s, struct iovec *iov,
> > +hwaddr req_len, uint32_t *total_msg_len,
> > +uint32_t *pad_offset)
> > +{
> > +*total_msg_len = (uint32_t)(ldq_be_p(iov->iov_base + req_len - 8) / 8);
> > +/*
> > + * SG_LIST_LEN_LAST asserted in the request length doesn't mean it is 
> > the
> > + * last request. The last request should contain padding message.
> > + * We check whether message contains padding by
> > + *   1. Get total message length. If the current message contains
> > + *  padding, the last 8 bytes are total message length.
> > + *   2. Check whether the total message length is valid.
> > + *  If it is valid, the value should less than or equal to
> > + *  total_req_len.
> > + *   3. Current request len - padding_size to get padding offset.
> > + *  The padding message's first byte should be 0x80
> > + */
> > +if (*total_msg_len <= s->total_req_len) {
> > +uint32_t padding_size = s->total_req_len - *total_msg_len;
> > +uint8_t *padding = iov->iov_base;
> > +*pad_offset = req_len - padding_size;
> > +if (padding[*pad_offset] == 0x80) {
> > +return true;
> > +}
> > +}
> > +
> > +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) {
> 
> pad_offset can not be NULL. or may be you meant *pad_offset ?
> 
> Thanks,
> 
> C.
> 
> 

Yes, it should be *pad_offset, I will fix it.

Thanks,
Steven

> > +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;
> > +}
> > +
> 



[PATCH v5 0/3] aspeed/hace: Support AST2600 HACE

2022-04-21 Thread 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 v5:
- Move iov cache related variables into AspeedHACEState.
- Update vmstate and reset_handler for new added attributes in
  AspeedHACEState.

Steven Lee (3):
  aspeed/hace: Support HMAC Key Buffer register.
  aspeed/hace: Support AST2600 HACE
  tests/qtest: Add test for Aspeed HACE accumulative mode

 hw/misc/aspeed_hace.c  | 139 +--
 include/hw/misc/aspeed_hace.h  |   5 ++
 tests/qtest/aspeed_hace-test.c | 145 +
 3 files changed, 284 insertions(+), 5 deletions(-)

-- 
2.17.1




[PATCH v5 1/3] aspeed/hace: Support HMAC Key Buffer register.

2022-04-21 Thread Steven Lee
Support HACE28: Hash HMAC Key Buffer Base Address Register.

Signed-off-by: Troy Lee 
Signed-off-by: Steven Lee 
Reviewed-by: Cédric Le Goater 
---
 hw/misc/aspeed_hace.c | 7 +++
 include/hw/misc/aspeed_hace.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 10f00e65f4..59fe5bfca2 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -27,6 +27,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)
@@ -210,6 +211,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 &= 0x0FFF;
 break;
@@ -333,6 +337,7 @@ static void aspeed_ast2400_hace_class_init(ObjectClass 
*klass, void *data)
 
 ahc->src_mask = 0x0FFF;
 ahc->dest_mask = 0x0FF8;
+ahc->key_mask = 0x0FC0;
 ahc->hash_mask = 0x03ff; /* No SG or SHA512 modes */
 }
 
@@ -351,6 +356,7 @@ static void aspeed_ast2500_hace_class_init(ObjectClass 
*klass, void *data)
 
 ahc->src_mask = 0x3fff;
 ahc->dest_mask = 0x3ff8;
+ahc->key_mask = 0x3FC0;
 ahc->hash_mask = 0x03ff; /* No SG or SHA512 modes */
 }
 
@@ -369,6 +375,7 @@ static void aspeed_ast2600_hace_class_init(ObjectClass 
*klass, void *data)
 
 ahc->src_mask = 0x7FFF;
 ahc->dest_mask = 0x7FF8;
+ahc->key_mask = 0x7FF8;
 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




[PATCH v5 2/3] aspeed/hace: Support AST2600 HACE

2022-04-21 Thread Steven Lee
The aspeed ast2600 accumulative mode is described in datasheet
ast2600v10.pdf section 25.6.4:
 1. Allocating 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 set 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 
Signed-off-by: Steven Lee 
---
 hw/misc/aspeed_hace.c | 132 --
 include/hw/misc/aspeed_hace.h |   4 ++
 2 files changed, 131 insertions(+), 5 deletions(-)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 59fe5bfca2..3164f6 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -65,7 +65,6 @@
 #define SG_LIST_ADDR_SIZE   4
 #define SG_LIST_ADDR_MASK   0x7FFF
 #define SG_LIST_ENTRY_SIZE  (SG_LIST_LEN_SIZE + SG_LIST_ADDR_SIZE)
-#define ASPEED_HACE_MAX_SG  256/* max number of entries */
 
 static const struct {
 uint32_t mask;
@@ -95,11 +94,104 @@ static int hash_algo_lookup(uint32_t reg)
 return -1;
 }
 
-static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode)
+/**
+ * Check whether the request contains padding message.
+ *
+ * @param s aspeed hace state object
+ * @param iov   iov of current request
+ * @param req_len   length of the current request
+ * @param total_msg_len length of all acc_mode requests(excluding padding msg)
+ * @param pad_offsetstart offset of padding message
+ */
+static bool has_padding(AspeedHACEState *s, struct iovec *iov,
+hwaddr req_len, uint32_t *total_msg_len,
+uint32_t *pad_offset)
+{
+*total_msg_len = (uint32_t)(ldq_be_p(iov->iov_base + req_len - 8) / 8);
+/*
+ * SG_LIST_LEN_LAST asserted in the request length doesn't mean it is the
+ * last request. The last request should contain padding message.
+ * We check whether message contains padding by
+ *   1. Get total message length. If the current message contains
+ *  padding, the last 8 bytes are total message length.
+ *   2. Check whether the total message length is valid.
+ *  If it is valid, the value should less than or equal to
+ *  total_req_len.
+ *   3. Current request len - padding_size to get padding offset.
+ *  The padding message's first byte should be 0x80
+ */
+if (*total_msg_len <= s->total_req_len) {
+uint32_t padding_size = s->total_req_len - *total_msg_len;
+uint8_t *padding = iov->iov_base;
+*pad_offset = req_len - padding_size;
+if (padding[*pad_offset] == 0x80) {
+return true;
+}
+}
+
+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;
+}
+
+/**
+ * Generate iov for accumulative mode.
+ *
+ * @param s aspeed hace state object
+ * @param iov   iov of the current request
+ * @param idindex of the current iov
+ * @param req_len   length of the current request
+ *
+ * @return count of iov
+ */
+static int gen_acc_mode_iov(AspeedHACEState *s, struct iovec *iov, int id,
+hwaddr *req_len)
+{
+uint32_t pad_offset;
+uint32_t total_msg_len;
+s->total_req_len += *req_len;
+
+if (has_padding(s, [id], *req_len, _msg_len, _offset)) {
+if (s->iov_count) {
+return reconstruct_iov(s, iov, id, _offset);
+}
+
+*req_len -= s->total_req_len - total_msg_len;
+s->total_req_len = 0;
+iov[id].iov_len = *req_len;
+} else {
+s->iov_cache[s->iov_count].iov_base = iov->iov_base;
+s->iov_cache[s->iov_count].iov_len = *req_len;
+++s->iov_count;
+}
+
+return id + 

[PATCH v5 3/3] tests/qtest: Add test for Aspeed HACE accumulative mode

2022-04-21 Thread 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 
Signed-off-by: Steven Lee 
Acked-by: Thomas Huth 
Reviewed-by: Joel Stanley 
---
 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 58aa22014d..85d705ec50 100644
--- a/tests/qtest/aspeed_hace-test.c
+++ b/tests/qtest/aspeed_hace-test.c
@@ -20,6 +20,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_ISRBIT(13)
@@ -95,6 +96,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)
@@ -307,6 +359,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 + 0x100;
+const uint32_t digest_addr = src_addr + 0x400;
+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);
+
+/* Che

Re: [PATCH v4 2/3] aspeed/hace: Support AST2600 HACE

2022-04-20 Thread Steven Lee
The 04/20/2022 20:53, Cédric Le Goater wrote:
> On 3/31/22 09:48, Steven Lee wrote:
> > The aspeed ast2600 accumulative mode is described in datasheet
> > ast2600v10.pdf section 25.6.4:
> >   1. Allocating 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
> 
> s/setted/set/
> 

will fix it.

> >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 
> > Signed-off-by: Steven Lee 
> > ---
> >   hw/misc/aspeed_hace.c | 140 --
> >   1 file changed, 136 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
> > index 59fe5bfca2..5a7a144602 100644
> > --- a/hw/misc/aspeed_hace.c
> > +++ b/hw/misc/aspeed_hace.c
> > @@ -95,12 +95,115 @@ static int hash_algo_lookup(uint32_t reg)
> >   return -1;
> >   }
> >   
> > -static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode)
> > +/**
> > + * Check whether the request contains padding message.
> > + *
> > + * @param iov   iov of current request
> > + * @param idindex of iov of current request
> > + * @param total_req_len length of all acc_mode requests(including padding 
> > msg)
> > + * @param req_len   length of the current request
> > + * @param total_msg_len length of all acc_mode requests(excluding padding 
> > msg)
> > + * @param pad_offsetstart offset of padding message
> > + */
> > +static bool has_padding(struct iovec *iov, uint32_t total_req_len,
> > +hwaddr req_len, uint32_t *total_msg_len,
> > +uint32_t *pad_offset)
> > +{
> > +*total_msg_len = (uint32_t)(ldq_be_p(iov->iov_base + req_len - 8) / 8);
> > +/*
> > + * SG_LIST_LEN_LAST asserted in the request length doesn't mean it is 
> > the
> > + * last request. The last request should contain padding message.
> > + * We check whether message contains padding by
> > + *   1. Get total message length. If the current message contains
> > + *  padding, the last 8 bytes are total message length.
> > + *   2. Check whether the total message length is valid.
> > + *  If it is valid, the value should less than or eaual to
> 
> s/eaual/equal/
> 

will fix it.

> > + *  total_req_len.
> > + *   3. Current request len - padding_size to get padding offset.
> > + *  The padding message's first byte should be 0x80
> > + */
> > +if (*total_msg_len <= total_req_len) {
> > +uint32_t padding_size = total_req_len - *total_msg_len;
> > +uint8_t *padding = iov->iov_base;
> > +*pad_offset = req_len - padding_size;
> > +if (padding[*pad_offset] == 0x80) {
> > +return true;
> > +}
> > +}
> > +
> > +return false;
> > +}
> > +
> > +static int reconstruct_iov(struct iovec *cache, struct iovec *iov, int id,
> > +   uint32_t *total_req_len,
> > +   uint32_t *pad_offset,
> > +   int *count)
> > +{
> > +int i, iov_count;
> > +if (pad_offset != 0) {
> > +(cache + *count)->iov_base = (iov + id)->iov_base;
> 
> I would prefer the array notation iov[i], like elsewhere in this file..
> 

will use iov[i] instead of (iov + i).

> > +(cache + *count)->iov_len = *pad_offset;
> > +++*count;
> > +}
> > +for (i = 0; i < *count; i++) {
> > +(iov + i)->iov_base = (cache + i)->iov_base;
> > +(iov + i)->iov_len = (cache + i)->iov_len;
> 
> ditto.
> 

will use iov[i] instead of (iov + i).

> > +}
> > +iov_count = *count;
> > +*count = 0;
> > + 

[PATCH v4 3/3] tests/qtest: Add test for Aspeed HACE accumulative mode

2022-03-31 Thread 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 
Signed-off-by: Steven Lee 
---
 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_ISRBIT(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 + 0x100;
+const uint32_t digest_addr = src_addr + 0x400;
+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(

[PATCH v4 0/3] aspeed/hace: Support AST2600 HACE

2022-03-31 Thread 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 v4:
- Separate HACE28 support to another patch.
- Refactor acc_mode message handling flow.

Steven Lee (3):
  aspeed/hace: Support HMAC Key Buffer register.
  aspeed/hace: Support AST2600 HACE
  tests/qtest: Add test for Aspeed HACE accumulative mode

 hw/misc/aspeed_hace.c  | 147 -
 include/hw/misc/aspeed_hace.h  |   1 +
 tests/qtest/aspeed_hace-test.c | 145 
 3 files changed, 289 insertions(+), 4 deletions(-)

-- 
2.17.1




[PATCH v4 2/3] aspeed/hace: Support AST2600 HACE

2022-03-31 Thread Steven Lee
The aspeed ast2600 accumulative mode is described in datasheet
ast2600v10.pdf section 25.6.4:
 1. Allocating 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 
Signed-off-by: Steven Lee 
---
 hw/misc/aspeed_hace.c | 140 --
 1 file changed, 136 insertions(+), 4 deletions(-)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 59fe5bfca2..5a7a144602 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -95,12 +95,115 @@ static int hash_algo_lookup(uint32_t reg)
 return -1;
 }
 
-static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode)
+/**
+ * Check whether the request contains padding message.
+ *
+ * @param iov   iov of current request
+ * @param idindex of iov of current request
+ * @param total_req_len length of all acc_mode requests(including padding msg)
+ * @param req_len   length of the current request
+ * @param total_msg_len length of all acc_mode requests(excluding padding msg)
+ * @param pad_offsetstart offset of padding message
+ */
+static bool has_padding(struct iovec *iov, uint32_t total_req_len,
+hwaddr req_len, uint32_t *total_msg_len,
+uint32_t *pad_offset)
+{
+*total_msg_len = (uint32_t)(ldq_be_p(iov->iov_base + req_len - 8) / 8);
+/*
+ * SG_LIST_LEN_LAST asserted in the request length doesn't mean it is the
+ * last request. The last request should contain padding message.
+ * We check whether message contains padding by
+ *   1. Get total message length. If the current message contains
+ *  padding, the last 8 bytes are total message length.
+ *   2. Check whether the total message length is valid.
+ *  If it is valid, the value should less than or eaual to
+ *  total_req_len.
+ *   3. Current request len - padding_size to get padding offset.
+ *  The padding message's first byte should be 0x80
+ */
+if (*total_msg_len <= total_req_len) {
+uint32_t padding_size = total_req_len - *total_msg_len;
+uint8_t *padding = iov->iov_base;
+*pad_offset = req_len - padding_size;
+if (padding[*pad_offset] == 0x80) {
+return true;
+}
+}
+
+return false;
+}
+
+static int reconstruct_iov(struct iovec *cache, struct iovec *iov, int id,
+   uint32_t *total_req_len,
+   uint32_t *pad_offset,
+   int *count)
+{
+int i, iov_count;
+if (pad_offset != 0) {
+(cache + *count)->iov_base = (iov + id)->iov_base;
+(cache + *count)->iov_len = *pad_offset;
+++*count;
+}
+for (i = 0; i < *count; i++) {
+(iov + i)->iov_base = (cache + i)->iov_base;
+(iov + i)->iov_len = (cache + i)->iov_len;
+}
+iov_count = *count;
+*count = 0;
+*total_req_len = 0;
+return iov_count;
+}
+
+/**
+ * Generate iov for accumulative mode.
+ *
+ * @param cache cached iov
+ * @param iov   iov of current request
+ * @param idindex of iov of current request
+ * @param total_req_len total length of the request(including padding)
+ * @param req_len   length of the current request
+ * @param count count of cached iov
+ */
+static int gen_acc_mode_iov(struct iovec *cache, struct iovec *iov, int id,
+uint32_t *total_req_len, hwaddr *req_len,
+int *count)
+{
+uint32_t pad_offset;
+uint32_t total_msg_len;
+*total_req_len += *req_len;
+
+if (has_padding([id], *total_req_len, *req_len, _msg_len,
+_offset)) {
+if (*count) {
+return reconstruct_iov(cache, iov, id, total_req_len,
+_offset, count);
+}
+
+*req_len -= *total_req_len - total_msg_len;
+*total_req_len = 0;
+(iov + id)->iov_len = *req_len;
+return id + 1;
+} else {
+(cache + *count)->iov_base = iov->iov_base;
+(cache + *count)->iov_len = *req_len;
+++*count;
+}
+
+return 0;
+}
+
+static void do_hash_operati

[PATCH v4 1/3] aspeed/hace: Support HMAC Key Buffer register.

2022-03-31 Thread Steven Lee
Support HACE28: Hash HMAC Key Buffer Base Address Register.

Signed-off-by: Troy Lee 
Signed-off-by: Steven Lee 
---
 hw/misc/aspeed_hace.c | 7 +++
 include/hw/misc/aspeed_hace.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 10f00e65f4..59fe5bfca2 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -27,6 +27,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)
@@ -210,6 +211,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 &= 0x0FFF;
 break;
@@ -333,6 +337,7 @@ static void aspeed_ast2400_hace_class_init(ObjectClass 
*klass, void *data)
 
 ahc->src_mask = 0x0FFF;
 ahc->dest_mask = 0x0FF8;
+ahc->key_mask = 0x0FC0;
 ahc->hash_mask = 0x03ff; /* No SG or SHA512 modes */
 }
 
@@ -351,6 +356,7 @@ static void aspeed_ast2500_hace_class_init(ObjectClass 
*klass, void *data)
 
 ahc->src_mask = 0x3fff;
 ahc->dest_mask = 0x3ff8;
+ahc->key_mask = 0x3FC0;
 ahc->hash_mask = 0x03ff; /* No SG or SHA512 modes */
 }
 
@@ -369,6 +375,7 @@ static void aspeed_ast2600_hace_class_init(ObjectClass 
*klass, void *data)
 
 ahc->src_mask = 0x7FFF;
 ahc->dest_mask = 0x7FF8;
+ahc->key_mask = 0x7FF8;
 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




Re: [PATCH v3 1/2] aspeed/hace: Support AST2600 HACE

2022-03-29 Thread Steven Lee
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  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 
> > Signed-off-by: Steven Lee 
> > ---
> >  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(>dram_as, addr, , 
> > 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
> > +

[PATCH v3 1/2] aspeed/hace: Support AST2600 HACE

2022-03-24 Thread 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 
Signed-off-by: Steven Lee 
---
 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(>dram_as, addr, , 
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++;
+}
+ 

[PATCH v3 0/2] Supporting AST2600 HACE engine accumulative mode

2022-03-24 Thread 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




[PATCH v3 2/2] tests/qtest: Add test for Aspeed HACE accumulative mode

2022-03-24 Thread 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 
Signed-off-by: Steven Lee 
---
 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_ISRBIT(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 + 0x100;
+const uint32_t digest_addr = src_addr + 0x400;
+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(

[PATCH v2 0/2] hw: aspeed_scu: Add AST2600 hpll calculation function

2022-03-15 Thread Steven Lee
AST2600's HPLL register offset and bit definition are different from AST2500.
The patch series adds a hpll calculation function for ast2600 and modify apb 
frequency
calculation function based on SCU200 register description and note in 
ast2600v11.pdf.

Changes since v1:
- introduce ast2400 and ast2600 get_apb_freq class handlers.
- introduce clkin_25Mhz attribute.

Please help to review.

Thanks,
Steven

Steven Lee (2):
  hw: aspeed_scu: Add AST2600 apb_freq and hpll calculation function
  hw: aspeed_scu: Introduce clkin_25Mhz attribute

 hw/misc/aspeed_scu.c | 45 ++--
 include/hw/misc/aspeed_scu.h | 19 +++
 2 files changed, 62 insertions(+), 2 deletions(-)

-- 
2.17.1




[PATCH v2 2/2] hw: aspeed_scu: Introduce clkin_25Mhz attribute

2022-03-15 Thread Steven Lee
AST2600 clkin is always 25MHz, introduce clkin_25Mhz attribute
for aspeed_scu_get_clkin() to return the correct clkin for ast2600.

Signed-off-by: Steven Lee 
---
 hw/misc/aspeed_scu.c | 6 +-
 include/hw/misc/aspeed_scu.h | 1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index d65f86df3d..150567f98a 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -371,7 +371,8 @@ static const MemoryRegionOps aspeed_ast2500_scu_ops = {
 
 static uint32_t aspeed_scu_get_clkin(AspeedSCUState *s)
 {
-if (s->hw_strap1 & SCU_HW_STRAP_CLK_25M_IN) {
+if (s->hw_strap1 & SCU_HW_STRAP_CLK_25M_IN ||
+ASPEED_SCU_GET_CLASS(s)->clkin_25Mhz) {
 return 2500;
 } else if (s->hw_strap1 & SCU_HW_STRAP_CLK_48M_IN) {
 return 4800;
@@ -562,6 +563,7 @@ static void aspeed_2400_scu_class_init(ObjectClass *klass, 
void *data)
 asc->get_apb = aspeed_2400_scu_get_apb_freq;
 asc->apb_divider = 2;
 asc->nr_regs = ASPEED_SCU_NR_REGS;
+asc->clkin_25Mhz = false;
 asc->ops = _ast2400_scu_ops;
 }
 
@@ -583,6 +585,7 @@ static void aspeed_2500_scu_class_init(ObjectClass *klass, 
void *data)
 asc->get_apb = aspeed_2400_scu_get_apb_freq;
 asc->apb_divider = 4;
 asc->nr_regs = ASPEED_SCU_NR_REGS;
+asc->clkin_25Mhz = false;
 asc->ops = _ast2500_scu_ops;
 }
 
@@ -756,6 +759,7 @@ static void aspeed_2600_scu_class_init(ObjectClass *klass, 
void *data)
 asc->get_apb = aspeed_2600_scu_get_apb_freq;
 asc->apb_divider = 4;
 asc->nr_regs = ASPEED_AST2600_SCU_NR_REGS;
+asc->clkin_25Mhz = true;
 asc->ops = _ast2600_scu_ops;
 }
 
diff --git a/include/hw/misc/aspeed_scu.h b/include/hw/misc/aspeed_scu.h
index 6bf67589e8..fdc721846c 100644
--- a/include/hw/misc/aspeed_scu.h
+++ b/include/hw/misc/aspeed_scu.h
@@ -59,6 +59,7 @@ struct AspeedSCUClass {
 uint32_t (*get_apb)(AspeedSCUState *s);
 uint32_t apb_divider;
 uint32_t nr_regs;
+bool clkin_25Mhz;
 const MemoryRegionOps *ops;
 };
 
-- 
2.17.1




[PATCH v2 1/2] hw: aspeed_scu: Add AST2600 apb_freq and hpll calculation function

2022-03-15 Thread Steven Lee
AST2600's HPLL register offset and bit definition are different from
AST2500. Add a hpll calculation function and an apb frequency calculation
function based on SCU200 register description in ast2600v11.pdf.

Signed-off-by: Steven Lee 
---
 hw/misc/aspeed_scu.c | 39 +++-
 include/hw/misc/aspeed_scu.h | 18 +
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index d06e179a6e..d65f86df3d 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -213,6 +213,11 @@ static uint32_t aspeed_scu_get_random(void)
 }
 
 uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s)
+{
+return ASPEED_SCU_GET_CLASS(s)->get_apb(s);
+}
+
+static uint32_t aspeed_2400_scu_get_apb_freq(AspeedSCUState *s)
 {
 AspeedSCUClass *asc = ASPEED_SCU_GET_CLASS(s);
 uint32_t hpll = asc->calc_hpll(s, s->regs[HPLL_PARAM]);
@@ -221,6 +226,15 @@ uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s)
 / asc->apb_divider;
 }
 
+static uint32_t aspeed_2600_scu_get_apb_freq(AspeedSCUState *s)
+{
+AspeedSCUClass *asc = ASPEED_SCU_GET_CLASS(s);
+uint32_t hpll = asc->calc_hpll(s, s->regs[AST2600_HPLL_PARAM]);
+
+return hpll / (SCU_CLK_GET_PCLK_DIV(s->regs[AST2600_CLK_SEL]) + 1)
+/ asc->apb_divider;
+}
+
 static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size)
 {
 AspeedSCUState *s = ASPEED_SCU(opaque);
@@ -426,6 +440,26 @@ static uint32_t aspeed_2500_scu_calc_hpll(AspeedSCUState 
*s, uint32_t hpll_reg)
 return clkin * multiplier;
 }
 
+static uint32_t aspeed_2600_scu_calc_hpll(AspeedSCUState *s, uint32_t hpll_reg)
+{
+uint32_t multiplier = 1;
+uint32_t clkin = aspeed_scu_get_clkin(s);
+
+if (hpll_reg & SCU_AST2600_H_PLL_OFF) {
+return 0;
+}
+
+if (!(hpll_reg & SCU_AST2600_H_PLL_BYPASS_EN)) {
+uint32_t p = (hpll_reg >> 19) & 0xf;
+uint32_t n = (hpll_reg >> 13) & 0x3f;
+uint32_t m = hpll_reg & 0x1fff;
+
+multiplier = ((m + 1) / (n + 1)) / (p + 1);
+}
+
+return clkin * multiplier;
+}
+
 static void aspeed_scu_reset(DeviceState *dev)
 {
 AspeedSCUState *s = ASPEED_SCU(dev);
@@ -525,6 +559,7 @@ static void aspeed_2400_scu_class_init(ObjectClass *klass, 
void *data)
 dc->desc = "ASPEED 2400 System Control Unit";
 asc->resets = ast2400_a0_resets;
 asc->calc_hpll = aspeed_2400_scu_calc_hpll;
+asc->get_apb = aspeed_2400_scu_get_apb_freq;
 asc->apb_divider = 2;
 asc->nr_regs = ASPEED_SCU_NR_REGS;
 asc->ops = _ast2400_scu_ops;
@@ -545,6 +580,7 @@ static void aspeed_2500_scu_class_init(ObjectClass *klass, 
void *data)
 dc->desc = "ASPEED 2500 System Control Unit";
 asc->resets = ast2500_a1_resets;
 asc->calc_hpll = aspeed_2500_scu_calc_hpll;
+asc->get_apb = aspeed_2400_scu_get_apb_freq;
 asc->apb_divider = 4;
 asc->nr_regs = ASPEED_SCU_NR_REGS;
 asc->ops = _ast2500_scu_ops;
@@ -716,7 +752,8 @@ static void aspeed_2600_scu_class_init(ObjectClass *klass, 
void *data)
 dc->desc = "ASPEED 2600 System Control Unit";
 dc->reset = aspeed_ast2600_scu_reset;
 asc->resets = ast2600_a3_resets;
-asc->calc_hpll = aspeed_2500_scu_calc_hpll; /* No change since AST2500 */
+asc->calc_hpll = aspeed_2600_scu_calc_hpll;
+asc->get_apb = aspeed_2600_scu_get_apb_freq;
 asc->apb_divider = 4;
 asc->nr_regs = ASPEED_AST2600_SCU_NR_REGS;
 asc->ops = _ast2600_scu_ops;
diff --git a/include/hw/misc/aspeed_scu.h b/include/hw/misc/aspeed_scu.h
index c14aff2bcb..6bf67589e8 100644
--- a/include/hw/misc/aspeed_scu.h
+++ b/include/hw/misc/aspeed_scu.h
@@ -56,6 +56,7 @@ struct AspeedSCUClass {
 
 const uint32_t *resets;
 uint32_t (*calc_hpll)(AspeedSCUState *s, uint32_t hpll_reg);
+uint32_t (*get_apb)(AspeedSCUState *s);
 uint32_t apb_divider;
 uint32_t nr_regs;
 const MemoryRegionOps *ops;
@@ -316,4 +317,21 @@ uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s);
 SCU_HW_STRAP_VGA_SIZE_SET(VGA_16M_DRAM) |   \
 SCU_AST2500_HW_STRAP_RESERVED1)
 
+/* SCU200   H-PLL Parameter Register (for Aspeed AST2600 SOC)
+ *
+ *  28:26  H-PLL Parameters
+ *  25 Enable H-PLL reset
+ *  24 Enable H-PLL bypass mode
+ *  23 Turn off H-PLL
+ *  22:19  H-PLL Post Divider (P)
+ *  18:13  H-PLL Numerator (M)
+ *  12:0   H-PLL Denumerator (N)
+ *
+ *  (Output frequency) = CLKIN(25MHz) * [(M+1) / (N+1)] / (P+1)
+ *
+ * The default frequency is 1200Mhz when CLKIN = 25MHz
+ */
+#define SCU_AST2600_H_PLL_BYPASS_EN(0x1 << 24)
+#define SCU_AST2600_H_PLL_OFF  (0x1 << 23)
+
 #endif /* ASPEED_SCU_H */
-- 
2.17.1




Re: [PATCH v1 1/1] hw: aspeed_scu: Add AST2600 hpll calculation function

2022-03-14 Thread Steven Lee
The 03/14/2022 20:21, Cédric Le Goater wrote:
> Hello Steven,
> 
> On 3/14/22 10:54, Steven Lee wrote:
> > AST2600's HPLL register offset and bit definition are different from
> > AST2500. Add a hpll calculation function for ast2600 and modify apb 
> > frequency
> > calculation function based on SCU200 register description in ast2600v11.pdf.
> 
> It looks good. A few minor comments on the modeling.
>   
> > Signed-off-by: Steven Lee 
> > ---
> >   hw/misc/aspeed_scu.c | 43 
> >   include/hw/misc/aspeed_scu.h | 17 ++
> >   2 files changed, 56 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> > index d06e179a6e..3b11e98d66 100644
> > --- a/hw/misc/aspeed_scu.c
> > +++ b/hw/misc/aspeed_scu.c
> > @@ -205,6 +205,8 @@ static const uint32_t 
> > ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
> >[BMC_DEV_ID]  = 0x2402U
> >   };
> >   
> > +static uint32_t aspeed_2600_scu_calc_hpll(AspeedSCUState *s, uint32_t 
> > hpll_reg);
> > +
> >   static uint32_t aspeed_scu_get_random(void)
> >   {
> >   uint32_t num;
> > @@ -215,9 +217,19 @@ static uint32_t aspeed_scu_get_random(void)
> >   uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s)
> >   {
> >   AspeedSCUClass *asc = ASPEED_SCU_GET_CLASS(s);
> > -uint32_t hpll = asc->calc_hpll(s, s->regs[HPLL_PARAM]);
> > +uint32_t hpll, hpll_reg, clk_sel_reg;
> > +
> > +if (asc->calc_hpll == aspeed_2600_scu_calc_hpll) {
> 
> That's indeed one way to distinguish the AST2600 from the previous SoCs.
> I would prefer to introduce a new APB freq class handler to deal with
> the differences in the AST2600. aspeed_scu_get_apb_freq() would become :
> 
>   uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s)
>   {
>   return ASPEED_SCU_GET_CLASS(s)->get_apb(s);
>   }
> 
> The current aspeed_scu_get_apb_freq() would become the AST2400 and AST2500
> handler and you would have to introduce a new one for the AST2600.
> 

Hi Cédric,

Thanks for the review.
I was wondering if the following implementation is good to you.

1 Modify aspeed_scu_get_apb_freq() as below
uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s)
{
return ASPEED_SCU_GET_CLASS(s)->get_apb(s);
}

2. Introduce 2 APB class handlers: aspeed_2400_scu_get_apb_freq() and 
aspeed_2600_scu_get_apb_freq()

3. Add new attribute get_apb in AspeedSCUClass.

4. In aspeed_2400_scu_class_init() and aspeed_2500_scu_class_init()
asc->get_apb = aspeed_2400_scu_get_apb_freq;

   In aspeed_2600_scu_class_init()
asc->get_apb = aspeed_2600_scu_get_apb_freq;

> > +hpll_reg = s->regs[AST2600_HPLL_PARAM];
> > +clk_sel_reg = s->regs[AST2600_CLK_SEL];
> > +} else {
> > +hpll_reg = s->regs[HPLL_PARAM];
> > +clk_sel_reg = s->regs[CLK_SEL];
> > +}
> > +
> > +hpll = asc->calc_hpll(s, hpll_reg);
> >   
> > -return hpll / (SCU_CLK_GET_PCLK_DIV(s->regs[CLK_SEL]) + 1)
> > +return hpll / (SCU_CLK_GET_PCLK_DIV(clk_sel_reg) + 1)
> >   / asc->apb_divider;>   }
> >   
> > @@ -357,7 +369,10 @@ static const MemoryRegionOps aspeed_ast2500_scu_ops = {
> >   
> >   static uint32_t aspeed_scu_get_clkin(AspeedSCUState *s)
> >   {
> > -if (s->hw_strap1 & SCU_HW_STRAP_CLK_25M_IN) {
> > +AspeedSCUClass *asc = ASPEED_SCU_GET_CLASS(s);
> > +
> > +if (s->hw_strap1 & SCU_HW_STRAP_CLK_25M_IN ||
> > +asc->calc_hpll == aspeed_2600_scu_calc_hpll) {
> 
> Indeed, the AST2600 CLKIN is always 25Mhz. Instead of testing ->calc_hpll,
> I would introduce a class attribute, something like 'bool is_25Mhz'.
> 
> This change should be in a second patch though.
> 

will add a new attribute for clkin in the second patch.

Thanks,
Steven

> Thanks,
> 
> C.
> 
> >   return 2500;
> >   } else if (s->hw_strap1 & SCU_HW_STRAP_CLK_48M_IN) {
> >   return 4800;
> > @@ -426,6 +441,26 @@ static uint32_t 
> > aspeed_2500_scu_calc_hpll(AspeedSCUState *s, uint32_t hpll_reg)
> >   return clkin * multiplier;
> >   }
> >   
> > +static uint32_t aspeed_2600_scu_calc_hpll(AspeedSCUState *s, uint32_t 
> > hpll_reg)
> > +{
> > +uint32_t multiplier = 1;
> > +uint32_t clkin = aspeed_scu_get_clkin(s);
> > +
> > +if (hpll_reg & SCU_AST2600_H_PLL_OFF) {
> > +return 0;
> > +}
> > +
> &g

[PATCH v1 0/1] hw: aspeed_scu: Add AST2600 hpll calculation function

2022-03-14 Thread Steven Lee
AST2600's HPLL register offset and bit definition are different from AST2500.
The patch series adds a hpll calculation function for ast2600 and modify apb 
frequency
calculation function based on SCU200 register description and note in 
ast2600v11.pdf.

Please help to review.

Thanks,
Steven

Steven Lee (1):
  hw: aspeed_scu: Add AST2600 hpll calculation function

 hw/misc/aspeed_scu.c | 43 
 include/hw/misc/aspeed_scu.h | 17 ++
 2 files changed, 56 insertions(+), 4 deletions(-)

-- 
2.17.1




[PATCH v1 1/1] hw: aspeed_scu: Add AST2600 hpll calculation function

2022-03-14 Thread Steven Lee
AST2600's HPLL register offset and bit definition are different from
AST2500. Add a hpll calculation function for ast2600 and modify apb frequency
calculation function based on SCU200 register description in ast2600v11.pdf.

Signed-off-by: Steven Lee 
---
 hw/misc/aspeed_scu.c | 43 
 include/hw/misc/aspeed_scu.h | 17 ++
 2 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index d06e179a6e..3b11e98d66 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -205,6 +205,8 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] 
= {
  [BMC_DEV_ID]  = 0x2402U
 };
 
+static uint32_t aspeed_2600_scu_calc_hpll(AspeedSCUState *s, uint32_t 
hpll_reg);
+
 static uint32_t aspeed_scu_get_random(void)
 {
 uint32_t num;
@@ -215,9 +217,19 @@ static uint32_t aspeed_scu_get_random(void)
 uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s)
 {
 AspeedSCUClass *asc = ASPEED_SCU_GET_CLASS(s);
-uint32_t hpll = asc->calc_hpll(s, s->regs[HPLL_PARAM]);
+uint32_t hpll, hpll_reg, clk_sel_reg;
+
+if (asc->calc_hpll == aspeed_2600_scu_calc_hpll) {
+hpll_reg = s->regs[AST2600_HPLL_PARAM];
+clk_sel_reg = s->regs[AST2600_CLK_SEL];
+} else {
+hpll_reg = s->regs[HPLL_PARAM];
+clk_sel_reg = s->regs[CLK_SEL];
+}
+
+hpll = asc->calc_hpll(s, hpll_reg);
 
-return hpll / (SCU_CLK_GET_PCLK_DIV(s->regs[CLK_SEL]) + 1)
+return hpll / (SCU_CLK_GET_PCLK_DIV(clk_sel_reg) + 1)
 / asc->apb_divider;
 }
 
@@ -357,7 +369,10 @@ static const MemoryRegionOps aspeed_ast2500_scu_ops = {
 
 static uint32_t aspeed_scu_get_clkin(AspeedSCUState *s)
 {
-if (s->hw_strap1 & SCU_HW_STRAP_CLK_25M_IN) {
+AspeedSCUClass *asc = ASPEED_SCU_GET_CLASS(s);
+
+if (s->hw_strap1 & SCU_HW_STRAP_CLK_25M_IN ||
+asc->calc_hpll == aspeed_2600_scu_calc_hpll) {
 return 2500;
 } else if (s->hw_strap1 & SCU_HW_STRAP_CLK_48M_IN) {
 return 4800;
@@ -426,6 +441,26 @@ static uint32_t aspeed_2500_scu_calc_hpll(AspeedSCUState 
*s, uint32_t hpll_reg)
 return clkin * multiplier;
 }
 
+static uint32_t aspeed_2600_scu_calc_hpll(AspeedSCUState *s, uint32_t hpll_reg)
+{
+uint32_t multiplier = 1;
+uint32_t clkin = aspeed_scu_get_clkin(s);
+
+if (hpll_reg & SCU_AST2600_H_PLL_OFF) {
+return 0;
+}
+
+if (!(hpll_reg & SCU_H_PLL_BYPASS_EN)) {
+uint32_t p = (hpll_reg >> 19) & 0xf;
+uint32_t n = (hpll_reg >> 13) & 0x3f;
+uint32_t m = hpll_reg & 0x1fff;
+
+multiplier = ((m + 1) / (n + 1)) / (p + 1);
+}
+
+return clkin * multiplier;
+}
+
 static void aspeed_scu_reset(DeviceState *dev)
 {
 AspeedSCUState *s = ASPEED_SCU(dev);
@@ -716,7 +751,7 @@ static void aspeed_2600_scu_class_init(ObjectClass *klass, 
void *data)
 dc->desc = "ASPEED 2600 System Control Unit";
 dc->reset = aspeed_ast2600_scu_reset;
 asc->resets = ast2600_a3_resets;
-asc->calc_hpll = aspeed_2500_scu_calc_hpll; /* No change since AST2500 */
+asc->calc_hpll = aspeed_2600_scu_calc_hpll;
 asc->apb_divider = 4;
 asc->nr_regs = ASPEED_AST2600_SCU_NR_REGS;
 asc->ops = _ast2600_scu_ops;
diff --git a/include/hw/misc/aspeed_scu.h b/include/hw/misc/aspeed_scu.h
index c14aff2bcb..91c500c5bc 100644
--- a/include/hw/misc/aspeed_scu.h
+++ b/include/hw/misc/aspeed_scu.h
@@ -316,4 +316,21 @@ uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s);
 SCU_HW_STRAP_VGA_SIZE_SET(VGA_16M_DRAM) |   \
 SCU_AST2500_HW_STRAP_RESERVED1)
 
+/* SCU200   H-PLL Parameter Register (for Aspeed AST2600 SOC)
+ *
+ *  28:26  H-PLL Parameters
+ *  25 Enable H-PLL reset
+ *  24 Enable H-PLL bypass mode
+ *  23 Turn off H-PLL
+ *  22:19  H-PLL Post Divider (P)
+ *  18:13   H-PLL Numerator (M)
+ *  12:0H-PLL Denumerator (N)
+ *
+ *  (Output frequency) = CLKIN(25MHz) * [(M+1) / (N+1)] / (P+1)
+ *
+ * The default frequency is 1200Mhz when CLKIN = 25MHz
+ */
+#define SCU_AST2600_H_PLL_BYPASS_EN(0x1 << 24)
+#define SCU_AST2600_H_PLL_OFF  (0x1 << 23)
+
 #endif /* ASPEED_SCU_H */
-- 
2.17.1