[PATCH v4 1/4] hw/misc/aspeed_hace: Fix coding style

[PATCH v4 1/4] hw/misc/aspeed_hace: Fix coding style

[cy_liu via]

From: Jamin Lin <jamin_lin@aspeedtech.com>

Fix coding style issues from checkpatch.pl.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
 hw/misc/aspeed_hace.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index e3f7df2e86..18b85081c7 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -75,9 +75,12 @@ static const struct {
     { HASH_ALGO_SHA1, QCRYPTO_HASH_ALGO_SHA1 },
     { HASH_ALGO_SHA224, QCRYPTO_HASH_ALGO_SHA224 },
     { HASH_ALGO_SHA256, QCRYPTO_HASH_ALGO_SHA256 },
-    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA512, QCRYPTO_HASH_ALGO_SHA512 },
-    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA384, QCRYPTO_HASH_ALGO_SHA384 },
-    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA256, QCRYPTO_HASH_ALGO_SHA256 },
+    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA512,
+      QCRYPTO_HASH_ALGO_SHA512 },
+    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA384,
+      QCRYPTO_HASH_ALGO_SHA384 },
+    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA256,
+      QCRYPTO_HASH_ALGO_SHA256 },
 };
 
 static int hash_algo_lookup(uint32_t reg)
@@ -201,7 +204,8 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
             haddr = address_space_map(&s->dram_as, addr, &plen, false,
                                       MEMTXATTRS_UNSPECIFIED);
             if (haddr == NULL) {
-                qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto failed\n", __func__);
+                qemu_log_mask(LOG_GUEST_ERROR,
+                              "%s: qcrypto failed\n", __func__);
                 return;
             }
             iov[i].iov_base = haddr;
-- 
2.34.1

[PATCH v4 0/4] Support HACE to AST2700 (resend)

[Jamin Lin via]

This patch series is from https://patchwork.kernel.org/project/qemu-devel/cover/20250213033531.3367697-1-jamin_lin@aspeedtech.com/. 
To expedite the review process, I have separated the HACE patches portion from
the https://patchwork.kernel.org/project/qemu-devel/cover/20250213033531.3367697-1-jamin_lin@aspeedtech.com/ patch series into this new patch series.

v4: Support HACE to AST2700

Jamin Lin (4):
  hw/misc/aspeed_hace: Fix coding style
  hw/misc/aspeed_hace: Add AST2700 support
  hw/arm/aspeed_ast27x0: Add HACE support for AST2700
  hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test

 include/hw/misc/aspeed_hace.h |  2 ++
 hw/arm/aspeed_ast27x0.c       | 15 ++++++++++
 hw/misc/aspeed_hace.c         | 55 ++++++++++++++++++++++++++++++++---
 3 files changed, 68 insertions(+), 4 deletions(-)

-- 
2.34.1

[PATCH v4 1/4] hw/misc/aspeed_hace: Fix coding style

[Jamin Lin via]

Fix coding style issues from checkpatch.pl.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
 hw/misc/aspeed_hace.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index e3f7df2e86..18b85081c7 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -75,9 +75,12 @@ static const struct {
     { HASH_ALGO_SHA1, QCRYPTO_HASH_ALGO_SHA1 },
     { HASH_ALGO_SHA224, QCRYPTO_HASH_ALGO_SHA224 },
     { HASH_ALGO_SHA256, QCRYPTO_HASH_ALGO_SHA256 },
-    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA512, QCRYPTO_HASH_ALGO_SHA512 },
-    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA384, QCRYPTO_HASH_ALGO_SHA384 },
-    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA256, QCRYPTO_HASH_ALGO_SHA256 },
+    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA512,
+      QCRYPTO_HASH_ALGO_SHA512 },
+    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA384,
+      QCRYPTO_HASH_ALGO_SHA384 },
+    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA256,
+      QCRYPTO_HASH_ALGO_SHA256 },
 };
 
 static int hash_algo_lookup(uint32_t reg)
@@ -201,7 +204,8 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
             haddr = address_space_map(&s->dram_as, addr, &plen, false,
                                       MEMTXATTRS_UNSPECIFIED);
             if (haddr == NULL) {
-                qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto failed\n", __func__);
+                qemu_log_mask(LOG_GUEST_ERROR,
+                              "%s: qcrypto failed\n", __func__);
                 return;
             }
             iov[i].iov_base = haddr;
-- 
2.34.1

[PATCH v4 2/4] hw/misc/aspeed_hace: Add AST2700 support

[Jamin Lin via]

Introduce a new ast2700 class to support AST2700.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Andrew Jeffery <andrew@codeconstruct.com.au>
---
 include/hw/misc/aspeed_hace.h |  1 +
 hw/misc/aspeed_hace.c         | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h
index 4af9919195..d13fd3da07 100644
--- a/include/hw/misc/aspeed_hace.h
+++ b/include/hw/misc/aspeed_hace.h
@@ -18,6 +18,7 @@
 #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"
+#define TYPE_ASPEED_AST2700_HACE TYPE_ASPEED_HACE "-ast2700"
 
 OBJECT_DECLARE_TYPE(AspeedHACEState, AspeedHACEClass, ASPEED_HACE)
 
diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 18b85081c7..86422cb3be 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -552,12 +552,32 @@ static const TypeInfo aspeed_ast1030_hace_info = {
     .class_init = aspeed_ast1030_hace_class_init,
 };
 
+static void aspeed_ast2700_hace_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    AspeedHACEClass *ahc = ASPEED_HACE_CLASS(klass);
+
+    dc->desc = "AST2700 Hash and Crypto Engine";
+
+    ahc->src_mask = 0x7FFFFFFF;
+    ahc->dest_mask = 0x7FFFFFF8;
+    ahc->key_mask = 0x7FFFFFF8;
+    ahc->hash_mask = 0x00147FFF;
+}
+
+static const TypeInfo aspeed_ast2700_hace_info = {
+    .name = TYPE_ASPEED_AST2700_HACE,
+    .parent = TYPE_ASPEED_HACE,
+    .class_init = aspeed_ast2700_hace_class_init,
+};
+
 static void aspeed_hace_register_types(void)
 {
     type_register_static(&aspeed_ast2400_hace_info);
     type_register_static(&aspeed_ast2500_hace_info);
     type_register_static(&aspeed_ast2600_hace_info);
     type_register_static(&aspeed_ast1030_hace_info);
+    type_register_static(&aspeed_ast2700_hace_info);
     type_register_static(&aspeed_hace_info);
 }
 
-- 
2.34.1

[PATCH v4 3/4] hw/arm/aspeed_ast27x0: Add HACE support for AST2700

[Jamin Lin via]

The HACE controller between AST2600 and AST2700 are almost identical.
The HACE controller registers base address starts at 0x1207_0000 and
its alarm interrupt is connected to GICINT4.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Andrew Jeffery <andrew@codeconstruct.com.au>
---
 hw/arm/aspeed_ast27x0.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index 2d0c99f159..a48f47b74e 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -67,6 +67,7 @@ static const hwaddr aspeed_soc_ast2700_memmap[] = {
     [ASPEED_DEV_RTC]       =  0x12C0F000,
     [ASPEED_DEV_SDHCI]     =  0x14080000,
     [ASPEED_DEV_TIMER1]    =  0x12C10000,
+    [ASPEED_DEV_HACE]      =  0x12070000,
 };
 
 #define AST2700_MAX_IRQ 256
@@ -401,6 +402,9 @@ static void aspeed_soc_ast2700_init(Object *obj)
 
     snprintf(typename, sizeof(typename), "aspeed.timer-%s", socname);
     object_initialize_child(obj, "timerctrl", &s->timerctrl, typename);
+
+    snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
+    object_initialize_child(obj, "hace", &s->hace, typename);
 }
 
 /*
@@ -733,6 +737,17 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->timerctrl), i, irq);
     }
 
+    /* HACE */
+    object_property_set_link(OBJECT(&s->hace), "dram", OBJECT(s->dram_mr),
+                             &error_abort);
+    if (!sysbus_realize(SYS_BUS_DEVICE(&s->hace), errp)) {
+        return;
+    }
+    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->hace), 0,
+                    sc->memmap[ASPEED_DEV_HACE]);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->hace), 0,
+                       aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
+
     create_unimplemented_device("ast2700.dpmcu", 0x11000000, 0x40000);
     create_unimplemented_device("ast2700.iomem0", 0x12000000, 0x01000000);
     create_unimplemented_device("ast2700.iomem1", 0x14000000, 0x01000000);
-- 
2.34.1

[PATCH v4 4/4] hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test

[Jamin Lin via]

Currently, it does not support the CRYPT command. Instead, it only sends an
interrupt to notify the firmware that the crypt command has completed.
It is a temporary workaround to resolve the boot issue in the Crypto Manager
Self Test.

Introduce a new "use_crypt_workaround" class attribute and set it to true in
the AST2700 HACE model to enable this workaround by default for AST2700.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/misc/aspeed_hace.h |  1 +
 hw/misc/aspeed_hace.c         | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h
index d13fd3da07..5d4aa19cfe 100644
--- a/include/hw/misc/aspeed_hace.h
+++ b/include/hw/misc/aspeed_hace.h
@@ -50,6 +50,7 @@ struct AspeedHACEClass {
     uint32_t dest_mask;
     uint32_t key_mask;
     uint32_t hash_mask;
+    bool raise_crypt_interrupt_workaround;
 };
 
 #endif /* ASPEED_HACE_H */
diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 86422cb3be..32a5dbded3 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -59,6 +59,7 @@
 /* Other cmd bits */
 #define  HASH_IRQ_EN                    BIT(9)
 #define  HASH_SG_EN                     BIT(18)
+#define  CRYPT_IRQ_EN                   BIT(12)
 /* Scatter-gather data list */
 #define SG_LIST_LEN_SIZE                4
 #define SG_LIST_LEN_MASK                0x0FFFFFFF
@@ -343,6 +344,15 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
                 qemu_irq_lower(s->irq);
             }
         }
+        if (ahc->raise_crypt_interrupt_workaround) {
+            if (data & CRYPT_IRQ) {
+                data &= ~CRYPT_IRQ;
+
+                if (s->regs[addr] & CRYPT_IRQ) {
+                    qemu_irq_lower(s->irq);
+                }
+            }
+        }
         break;
     case R_HASH_SRC:
         data &= ahc->src_mask;
@@ -388,6 +398,12 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
     case R_CRYPT_CMD:
         qemu_log_mask(LOG_UNIMP, "%s: Crypt commands not implemented\n",
                        __func__);
+        if (ahc->raise_crypt_interrupt_workaround) {
+            s->regs[R_STATUS] |= CRYPT_IRQ;
+            if (data & CRYPT_IRQ_EN) {
+                qemu_irq_raise(s->irq);
+            }
+        }
         break;
     default:
         break;
@@ -563,6 +579,13 @@ static void aspeed_ast2700_hace_class_init(ObjectClass *klass, void *data)
     ahc->dest_mask = 0x7FFFFFF8;
     ahc->key_mask = 0x7FFFFFF8;
     ahc->hash_mask = 0x00147FFF;
+
+    /*
+     * Currently, it does not support the CRYPT command. Instead, it only
+     * sends an interrupt to notify the firmware that the crypt command
+     * has completed. It is a temporary workaround.
+     */
+    ahc->raise_crypt_interrupt_workaround = true;
 }
 
 static const TypeInfo aspeed_ast2700_hace_info = {
-- 
2.34.1

Re: [PATCH v4 4/4] hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test

[Cédric Le Goater]

On 2/25/25 08:56, Jamin Lin wrote:
> Currently, it does not support the CRYPT command. Instead, it only sends an
> interrupt to notify the firmware that the crypt command has completed.
> It is a temporary workaround to resolve the boot issue in the Crypto Manager
> Self Test.
> 
> Introduce a new "use_crypt_workaround" class attribute and set it to true in
> the AST2700 HACE model to enable this workaround by default for AST2700.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   include/hw/misc/aspeed_hace.h |  1 +
>   hw/misc/aspeed_hace.c         | 23 +++++++++++++++++++++++
>   2 files changed, 24 insertions(+)
> 
> diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h
> index d13fd3da07..5d4aa19cfe 100644
> --- a/include/hw/misc/aspeed_hace.h
> +++ b/include/hw/misc/aspeed_hace.h
> @@ -50,6 +50,7 @@ struct AspeedHACEClass {
>       uint32_t dest_mask;
>       uint32_t key_mask;
>       uint32_t hash_mask;
> +    bool raise_crypt_interrupt_workaround;
>   };
>   
>   #endif /* ASPEED_HACE_H */
> diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
> index 86422cb3be..32a5dbded3 100644
> --- a/hw/misc/aspeed_hace.c
> +++ b/hw/misc/aspeed_hace.c
> @@ -59,6 +59,7 @@
>   /* Other cmd bits */
>   #define  HASH_IRQ_EN                    BIT(9)
>   #define  HASH_SG_EN                     BIT(18)
> +#define  CRYPT_IRQ_EN                   BIT(12)
>   /* Scatter-gather data list */
>   #define SG_LIST_LEN_SIZE                4
>   #define SG_LIST_LEN_MASK                0x0FFFFFFF
> @@ -343,6 +344,15 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
>                   qemu_irq_lower(s->irq);
>               }
>           }
> +        if (ahc->raise_crypt_interrupt_workaround) {
> +            if (data & CRYPT_IRQ) {
> +                data &= ~CRYPT_IRQ;
> +
> +                if (s->regs[addr] & CRYPT_IRQ) {
> +                    qemu_irq_lower(s->irq);
> +                }
> +            }
> +        }
>           break;
>       case R_HASH_SRC:
>           data &= ahc->src_mask;
> @@ -388,6 +398,12 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
>       case R_CRYPT_CMD:
>           qemu_log_mask(LOG_UNIMP, "%s: Crypt commands not implemented\n",
>                          __func__);
> +        if (ahc->raise_crypt_interrupt_workaround) {
> +            s->regs[R_STATUS] |= CRYPT_IRQ;
> +            if (data & CRYPT_IRQ_EN) {
> +                qemu_irq_raise(s->irq);
> +            }
> +        }
>           break;
>       default:
>           break;
> @@ -563,6 +579,13 @@ static void aspeed_ast2700_hace_class_init(ObjectClass *klass, void *data)
>       ahc->dest_mask = 0x7FFFFFF8;
>       ahc->key_mask = 0x7FFFFFF8;
>       ahc->hash_mask = 0x00147FFF;
> +
> +    /*
> +     * Currently, it does not support the CRYPT command. Instead, it only
> +     * sends an interrupt to notify the firmware that the crypt command
> +     * has completed. It is a temporary workaround.
> +     */
> +    ahc->raise_crypt_interrupt_workaround = true;
>   }
>   
>   static const TypeInfo aspeed_ast2700_hace_info = {

Re: [PATCH v4 0/4] Support HACE to AST2700 (resend)

[Cédric Le Goater]

On 2/25/25 08:56, Jamin Lin wrote:
> This patch series is from https://patchwork.kernel.org/project/qemu-devel/cover/20250213033531.3367697-1-jamin_lin@aspeedtech.com/.
> To expedite the review process, I have separated the HACE patches portion from
> the https://patchwork.kernel.org/project/qemu-devel/cover/20250213033531.3367697-1-jamin_lin@aspeedtech.com/ patch series into this new patch series.
> 
> v4: Support HACE to AST2700
> 
> Jamin Lin (4):
>    hw/misc/aspeed_hace: Fix coding style
>    hw/misc/aspeed_hace: Add AST2700 support
>    hw/arm/aspeed_ast27x0: Add HACE support for AST2700
>    hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test
> 
>   include/hw/misc/aspeed_hace.h |  2 ++
>   hw/arm/aspeed_ast27x0.c       | 15 ++++++++++
>   hw/misc/aspeed_hace.c         | 55 ++++++++++++++++++++++++++++++++---
>   3 files changed, 68 insertions(+), 4 deletions(-)
> 


Applied to aspeed-next.

Thanks,

C.

Re: [PATCH v4 0/4] Support HACE to AST2700 (resend)

[Cédric Le Goater]

Jamin,

On 2/25/25 08:56, Jamin Lin wrote:
> This patch series is from https://patchwork.kernel.org/project/qemu-devel/cover/20250213033531.3367697-1-jamin_lin@aspeedtech.com/.
> To expedite the review process, I have separated the HACE patches portion from
> the https://patchwork.kernel.org/project/qemu-devel/cover/20250213033531.3367697-1-jamin_lin@aspeedtech.com/ patch series into this new patch series.
> 
> v4: Support HACE to AST2700
> 
> Jamin Lin (4):
>    hw/misc/aspeed_hace: Fix coding style
>    hw/misc/aspeed_hace: Add AST2700 support
>    hw/arm/aspeed_ast27x0: Add HACE support for AST2700
>    hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test
> 
>   include/hw/misc/aspeed_hace.h |  2 ++
>   hw/arm/aspeed_ast27x0.c       | 15 ++++++++++
>   hw/misc/aspeed_hace.c         | 55 ++++++++++++++++++++++++++++++++---
>   3 files changed, 68 insertions(+), 4 deletions(-)
> 

The kernel reports a lot of errors when running the crypto tests on a
AST2700 A1 Soc. See the functional test logs :
     
     $ grep alg: build/tests/functional/aarch64/test_aarch64_aspeed.AST2x00MachineSDK.test_aarch64_ast2700a1_evb_sdk_v09_05/console.log

     2025-03-12 10:10:42,705: [    2.206831] alg: skcipher: aspeed-ctr-tdes encryption test failed (wrong result) on test vector 0, cfg="in-place (one sglist)"
     2025-03-12 10:10:42,706: [    2.207328] alg: self-tests for ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
     2025-03-12 10:10:42,707: [    2.207894] alg: self-tests for ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
     2025-03-12 10:10:42,725: [    2.219342] alg: skcipher: aspeed-ctr-des encryption test failed (wrong result) on test vector 0, cfg="in-place (one sglist)"
     2025-03-12 10:10:42,725: [    2.219511] alg: self-tests for ctr(des) using aspeed-ctr-des failed (rc=-22)
     2025-03-12 10:10:42,725: [    2.219651] alg: self-tests for ctr(des) using aspeed-ctr-des failed (rc=-22)
     2025-03-12 10:10:42,733: [    2.224344] alg: skcipher: aspeed-ctr-aes encryption test failed (wrong result) on test vector 0, cfg="in-place (one sglist)"
     2025-03-12 10:10:42,734: [    2.224775] alg: self-tests for ctr(aes) using aspeed-ctr-aes failed (rc=-22)
     2025-03-12 10:10:42,734: [    2.225015] alg: self-tests for ctr(aes) using aspeed-ctr-aes failed (rc=-22)
     ...

What is something missing in the models ?

Thanks,

C.

RE: [PATCH v4 0/4] Support HACE to AST2700 (resend)

[Jamin Lin]

Hi Cedric,

> -----Original Message-----
> From: Cédric Le Goater <clg@kaod.org>
> Sent: Wednesday, March 12, 2025 5:25 PM
> To: Jamin Lin <jamin_lin@aspeedtech.com>; Peter Maydell
> <peter.maydell@linaro.org>; Steven Lee <steven_lee@aspeedtech.com>; Troy
> Lee <leetroy@gmail.com>; Andrew Jeffery <andrew@codeconstruct.com.au>;
> Joel Stanley <joel@jms.id.au>; open list:ASPEED BMCs
> <qemu-arm@nongnu.org>; open list:All patches CC here
> <qemu-devel@nongnu.org>
> Cc: Troy Lee <troy_lee@aspeedtech.com>
> Subject: Re: [PATCH v4 0/4] Support HACE to AST2700 (resend)
> 
> Jamin,
> 
> On 2/25/25 08:56, Jamin Lin wrote:
> > This patch series is from
> https://patchwork.kernel.org/project/qemu-devel/cover/20250213033531.336
> 7697-1-jamin_lin@aspeedtech.com/.
> > To expedite the review process, I have separated the HACE patches
> > portion from the
> https://patchwork.kernel.org/project/qemu-devel/cover/20250213033531.336
> 7697-1-jamin_lin@aspeedtech.com/ patch series into this new patch series.
> >
> > v4: Support HACE to AST2700
> >
> > Jamin Lin (4):
> >    hw/misc/aspeed_hace: Fix coding style
> >    hw/misc/aspeed_hace: Add AST2700 support
> >    hw/arm/aspeed_ast27x0: Add HACE support for AST2700
> >    hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test
> >
> >   include/hw/misc/aspeed_hace.h |  2 ++
> >   hw/arm/aspeed_ast27x0.c       | 15 ++++++++++
> >   hw/misc/aspeed_hace.c         | 55
> ++++++++++++++++++++++++++++++++---
> >   3 files changed, 68 insertions(+), 4 deletions(-)
> >
> 
> The kernel reports a lot of errors when running the crypto tests on a
> AST2700 A1 Soc. See the functional test logs :
> 
>      $ grep alg:
> build/tests/functional/aarch64/test_aarch64_aspeed.AST2x00MachineSDK.test
> _aarch64_ast2700a1_evb_sdk_v09_05/console.log
> 
>      2025-03-12 10:10:42,705: [    2.206831] alg: skcipher: aspeed-ctr-tdes
> encryption test failed (wrong result) on test vector 0, cfg="in-place (one sglist)"
>      2025-03-12 10:10:42,706: [    2.207328] alg: self-tests for
> ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
>      2025-03-12 10:10:42,707: [    2.207894] alg: self-tests for
> ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
>      2025-03-12 10:10:42,725: [    2.219342] alg: skcipher: aspeed-ctr-des
> encryption test failed (wrong result) on test vector 0, cfg="in-place (one sglist)"
>      2025-03-12 10:10:42,725: [    2.219511] alg: self-tests for ctr(des)
> using aspeed-ctr-des failed (rc=-22)
>      2025-03-12 10:10:42,725: [    2.219651] alg: self-tests for ctr(des)
> using aspeed-ctr-des failed (rc=-22)
>      2025-03-12 10:10:42,733: [    2.224344] alg: skcipher: aspeed-ctr-aes
> encryption test failed (wrong result) on test vector 0, cfg="in-place (one sglist)"
>      2025-03-12 10:10:42,734: [    2.224775] alg: self-tests for ctr(aes)
> using aspeed-ctr-aes failed (rc=-22)
>      2025-03-12 10:10:42,734: [    2.225015] alg: self-tests for ctr(aes)
> using aspeed-ctr-aes failed (rc=-22)
>      ...
> 
> What is something missing in the models ?
> 

Currently, we do not implement HACE/CRYPT in aspeed_hace.c. 
https://patchwork.kernel.org/project/qemu-devel/patch/20250225075622.305515-5-jamin_lin@aspeedtech.com/

Known Issue:
The HACE crypto and hash engine is enable by default since AST2700 A1.
However, aspeed_hace.c(HACE model) currently does not support the CRYPTO command.
To boot AST2700 A1, created a patch which temporarily resolves the
issue by sending an interrupt to notify the firmware that the cryptographic
command has completed. It is a temporary workaround to resolve the boot issue
in the Crypto Manager SelfTest.

As a result, you will encounter the following kernel warning due to the
Crypto Manager test failure.

```
alg: skcipher: aspeed-ctr-tdes encryption test failed (wrong result) on test vector 0, cfg="in-place (one sglist)"
[    5.035966] alg: self-tests for ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
[    5.036139] ------------[ cut here ]------------
[    5.037188] alg: self-tests for ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
[    5.037312] WARNING: CPU: 2 PID: 109 at /crypto/testmgr.c:5936 alg_test+0x42c/0x548
[    5.038049] Modules linked in:
[    5.038302] CPU: 2 PID: 109 Comm: cryptomgr_test Tainted: G        W          6.6.52-v00.06.04-gf52a0cf7c475 #1
[    5.038787] Hardware name: AST2700-EVB (DT)
[    5.038988] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    5.039315] pc : alg_test+0x42c/0x548
[    5.039505] lr : alg_test+0x42c/0x548
[    5.039697] sp : ffffffc0825e3d50
[    5.039862] x29: ffffffc0825e3df0 x28: 0000000000000004 x27: 0000000000000000
[    5.040226] x26: ffffffc080bcada0 x25: ffffffc081dac9d0 x24: 0000000000000004
[    5.040700] x23: 0000000000001285 x22: ffffff8003ded280 x21: ffffff8003ded200
[    5.041458] x20: 00000000ffffffff x19: 00000000ffffffea x18: ffffffffffffffff
[    5.041915] x17: 282064656c696166 x16: 20736564742d7274 x15: 00000000fffffffe
[    5.042287] x14: 0000000000000000 x13: ffffffc081ba555c x12: 65742d666c657320
[    5.042684] x11: 00000000fffeffff x10: ffffffc0818ff048 x9 : ffffffc0800a36e4
[    5.043077] x8 : 000000000017ffe8 x7 : c0000000fffeffff x6 : 000000000057ffa8
[    5.043461] x5 : 000000000000ffff x4 : 0000000000000000 x3 : 0000000000000000
[    5.043751] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffffff800415e740
[    5.045544] Call trace:
[    5.045693]  alg_test+0x42c/0x548
[    5.045878]  cryptomgr_test+0x28/0x48
[    5.046052]  kthread+0x114/0x120
[    5.046226]  ret_from_fork+0x10/0x20
[    5.046413] ---[ end trace 0000000000000000 ]---
[    5.071510] alg: skcipher: aspeed-ctr-des encryption test failed (wrong result) on test vector 0, cfg="in-place (one sglist)"
[    5.072145] alg: self-tests for ctr(des) using aspeed-ctr-des failed (rc=-22)
```

Thanks-Jamin

> Thanks,
> 
> C.
> 

Re: RE: [PATCH v4 0/4] Support HACE to AST2700 (resend)

[Cédric Le Goater]

Hello Jamin,

On 3/12/25 10:32, Jamin Lin wrote:
> Hi Cedric,
> 
>> -----Original Message-----
>> From: Cédric Le Goater <clg@kaod.org>
>> Sent: Wednesday, March 12, 2025 5:25 PM
>> To: Jamin Lin <jamin_lin@aspeedtech.com>; Peter Maydell
>> <peter.maydell@linaro.org>; Steven Lee <steven_lee@aspeedtech.com>; Troy
>> Lee <leetroy@gmail.com>; Andrew Jeffery <andrew@codeconstruct.com.au>;
>> Joel Stanley <joel@jms.id.au>; open list:ASPEED BMCs
>> <qemu-arm@nongnu.org>; open list:All patches CC here
>> <qemu-devel@nongnu.org>
>> Cc: Troy Lee <troy_lee@aspeedtech.com>
>> Subject: Re: [PATCH v4 0/4] Support HACE to AST2700 (resend)
>>
>> Jamin,
>>
>> On 2/25/25 08:56, Jamin Lin wrote:
>>> This patch series is from
>> https://patchwork.kernel.org/project/qemu-devel/cover/20250213033531.336
>> 7697-1-jamin_lin@aspeedtech.com/.
>>> To expedite the review process, I have separated the HACE patches
>>> portion from the
>> https://patchwork.kernel.org/project/qemu-devel/cover/20250213033531.336
>> 7697-1-jamin_lin@aspeedtech.com/ patch series into this new patch series.
>>>
>>> v4: Support HACE to AST2700
>>>
>>> Jamin Lin (4):
>>>     hw/misc/aspeed_hace: Fix coding style
>>>     hw/misc/aspeed_hace: Add AST2700 support
>>>     hw/arm/aspeed_ast27x0: Add HACE support for AST2700
>>>     hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test
>>>
>>>    include/hw/misc/aspeed_hace.h |  2 ++
>>>    hw/arm/aspeed_ast27x0.c       | 15 ++++++++++
>>>    hw/misc/aspeed_hace.c         | 55
>> ++++++++++++++++++++++++++++++++---
>>>    3 files changed, 68 insertions(+), 4 deletions(-)
>>>
>>
>> The kernel reports a lot of errors when running the crypto tests on a
>> AST2700 A1 Soc. See the functional test logs :
>>
>>       $ grep alg:
>> build/tests/functional/aarch64/test_aarch64_aspeed.AST2x00MachineSDK.test
>> _aarch64_ast2700a1_evb_sdk_v09_05/console.log
>>
>>       2025-03-12 10:10:42,705: [    2.206831] alg: skcipher: aspeed-ctr-tdes
>> encryption test failed (wrong result) on test vector 0, cfg="in-place (one sglist)"
>>       2025-03-12 10:10:42,706: [    2.207328] alg: self-tests for
>> ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
>>       2025-03-12 10:10:42,707: [    2.207894] alg: self-tests for
>> ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
>>       2025-03-12 10:10:42,725: [    2.219342] alg: skcipher: aspeed-ctr-des
>> encryption test failed (wrong result) on test vector 0, cfg="in-place (one sglist)"
>>       2025-03-12 10:10:42,725: [    2.219511] alg: self-tests for ctr(des)
>> using aspeed-ctr-des failed (rc=-22)
>>       2025-03-12 10:10:42,725: [    2.219651] alg: self-tests for ctr(des)
>> using aspeed-ctr-des failed (rc=-22)
>>       2025-03-12 10:10:42,733: [    2.224344] alg: skcipher: aspeed-ctr-aes
>> encryption test failed (wrong result) on test vector 0, cfg="in-place (one sglist)"
>>       2025-03-12 10:10:42,734: [    2.224775] alg: self-tests for ctr(aes)
>> using aspeed-ctr-aes failed (rc=-22)
>>       2025-03-12 10:10:42,734: [    2.225015] alg: self-tests for ctr(aes)
>> using aspeed-ctr-aes failed (rc=-22)
>>       ...
>>
>> What is something missing in the models ?
>>
> 
> Currently, we do not implement HACE/CRYPT in aspeed_hace.c.
> https://patchwork.kernel.org/project/qemu-devel/patch/20250225075622.305515-5-jamin_lin@aspeedtech.com/
> 
> Known Issue:
> The HACE crypto and hash engine is enable by default since AST2700 A1.
> However, aspeed_hace.c(HACE model) currently does not support the CRYPTO command.
> To boot AST2700 A1, created a patch which temporarily resolves the
> issue by sending an interrupt to notify the firmware that the cryptographic
> command has completed. It is a temporary workaround to resolve the boot issue
> in the Crypto Manager SelfTest.
> 
> As a result, you will encounter the following kernel warning due to the
> Crypto Manager test failure.
> 
> ```
> alg: skcipher: aspeed-ctr-tdes encryption test failed (wrong result) on test vector 0, cfg="in-place (one sglist)"
> [    5.035966] alg: self-tests for ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
> [    5.036139] ------------[ cut here ]------------
> [    5.037188] alg: self-tests for ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
> [    5.037312] WARNING: CPU: 2 PID: 109 at /crypto/testmgr.c:5936 alg_test+0x42c/0x548
> [    5.038049] Modules linked in:
> [    5.038302] CPU: 2 PID: 109 Comm: cryptomgr_test Tainted: G        W          6.6.52-v00.06.04-gf52a0cf7c475 #1
> [    5.038787] Hardware name: AST2700-EVB (DT)
> [    5.038988] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    5.039315] pc : alg_test+0x42c/0x548
> [    5.039505] lr : alg_test+0x42c/0x548
> [    5.039697] sp : ffffffc0825e3d50
> [    5.039862] x29: ffffffc0825e3df0 x28: 0000000000000004 x27: 0000000000000000
> [    5.040226] x26: ffffffc080bcada0 x25: ffffffc081dac9d0 x24: 0000000000000004
> [    5.040700] x23: 0000000000001285 x22: ffffff8003ded280 x21: ffffff8003ded200
> [    5.041458] x20: 00000000ffffffff x19: 00000000ffffffea x18: ffffffffffffffff
> [    5.041915] x17: 282064656c696166 x16: 20736564742d7274 x15: 00000000fffffffe
> [    5.042287] x14: 0000000000000000 x13: ffffffc081ba555c x12: 65742d666c657320
> [    5.042684] x11: 00000000fffeffff x10: ffffffc0818ff048 x9 : ffffffc0800a36e4
> [    5.043077] x8 : 000000000017ffe8 x7 : c0000000fffeffff x6 : 000000000057ffa8
> [    5.043461] x5 : 000000000000ffff x4 : 0000000000000000 x3 : 0000000000000000
> [    5.043751] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffffff800415e740
> [    5.045544] Call trace:
> [    5.045693]  alg_test+0x42c/0x548
> [    5.045878]  cryptomgr_test+0x28/0x48
> [    5.046052]  kthread+0x114/0x120
> [    5.046226]  ret_from_fork+0x10/0x20
> [    5.046413] ---[ end trace 0000000000000000 ]---
> [    5.071510] alg: skcipher: aspeed-ctr-des encryption test failed (wrong result) on test vector 0, cfg="in-place (one sglist)"
> [    5.072145] alg: self-tests for ctr(des) using aspeed-ctr-des failed (rc=-22)
> ```

This is a bit confusing for people running the tests, since it
might be considered as a regression in the models. I understand
it is interesting for real HW and so, it would good to complete
the implementation of the QEMU models too or, may be, we could
deactivate the tests from the kernel command line ? if possible.

Thanks,

C.

RE: RE: [PATCH v4 0/4] Support HACE to AST2700 (resend)

[Jamin Lin]

+ Kane

Hi Cedric,

> Subject: Re: RE: [PATCH v4 0/4] Support HACE to AST2700 (resend)
> 
> Hello Jamin,
> 
> On 3/12/25 10:32, Jamin Lin wrote:
> > Hi Cedric,
> >
> >> -----Original Message-----
> >> From: Cédric Le Goater <clg@kaod.org>
> >> Sent: Wednesday, March 12, 2025 5:25 PM
> >> To: Jamin Lin <jamin_lin@aspeedtech.com>; Peter Maydell
> >> <peter.maydell@linaro.org>; Steven Lee <steven_lee@aspeedtech.com>;
> >> Troy Lee <leetroy@gmail.com>; Andrew Jeffery
> >> <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>; open
> >> list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC here
> >> <qemu-devel@nongnu.org>
> >> Cc: Troy Lee <troy_lee@aspeedtech.com>
> >> Subject: Re: [PATCH v4 0/4] Support HACE to AST2700 (resend)
> >>
> >> Jamin,
> >>
> >> On 2/25/25 08:56, Jamin Lin wrote:
> >>> This patch series is from
> >> https://patchwork.kernel.org/project/qemu-devel/cover/20250213033531.
> >> 336
> >> 7697-1-jamin_lin@aspeedtech.com/.
> >>> To expedite the review process, I have separated the HACE patches
> >>> portion from the
> >> https://patchwork.kernel.org/project/qemu-devel/cover/20250213033531.
> >> 336 7697-1-jamin_lin@aspeedtech.com/ patch series into this new patch
> >> series.
> >>>
> >>> v4: Support HACE to AST2700
> >>>
> >>> Jamin Lin (4):
> >>>     hw/misc/aspeed_hace: Fix coding style
> >>>     hw/misc/aspeed_hace: Add AST2700 support
> >>>     hw/arm/aspeed_ast27x0: Add HACE support for AST2700
> >>>     hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self
> >>> Test
> >>>
> >>>    include/hw/misc/aspeed_hace.h |  2 ++
> >>>    hw/arm/aspeed_ast27x0.c       | 15 ++++++++++
> >>>    hw/misc/aspeed_hace.c         | 55
> >> ++++++++++++++++++++++++++++++++---
> >>>    3 files changed, 68 insertions(+), 4 deletions(-)
> >>>
> >>
> >> The kernel reports a lot of errors when running the crypto tests on a
> >> AST2700 A1 Soc. See the functional test logs :
> >>
> >>       $ grep alg:
> >> build/tests/functional/aarch64/test_aarch64_aspeed.AST2x00MachineSDK.
> >> test _aarch64_ast2700a1_evb_sdk_v09_05/console.log
> >>
> >>       2025-03-12 10:10:42,705: [    2.206831] alg: skcipher:
> aspeed-ctr-tdes
> >> encryption test failed (wrong result) on test vector 0, cfg="in-place (one
> sglist)"
> >>       2025-03-12 10:10:42,706: [    2.207328] alg: self-tests for
> >> ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
> >>       2025-03-12 10:10:42,707: [    2.207894] alg: self-tests for
> >> ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
> >>       2025-03-12 10:10:42,725: [    2.219342] alg: skcipher:
> aspeed-ctr-des
> >> encryption test failed (wrong result) on test vector 0, cfg="in-place (one
> sglist)"
> >>       2025-03-12 10:10:42,725: [    2.219511] alg: self-tests for ctr(des)
> >> using aspeed-ctr-des failed (rc=-22)
> >>       2025-03-12 10:10:42,725: [    2.219651] alg: self-tests for ctr(des)
> >> using aspeed-ctr-des failed (rc=-22)
> >>       2025-03-12 10:10:42,733: [    2.224344] alg: skcipher:
> aspeed-ctr-aes
> >> encryption test failed (wrong result) on test vector 0, cfg="in-place (one
> sglist)"
> >>       2025-03-12 10:10:42,734: [    2.224775] alg: self-tests for ctr(aes)
> >> using aspeed-ctr-aes failed (rc=-22)
> >>       2025-03-12 10:10:42,734: [    2.225015] alg: self-tests for ctr(aes)
> >> using aspeed-ctr-aes failed (rc=-22)
> >>       ...
> >>
> >> What is something missing in the models ?
> >>
> >
> > Currently, we do not implement HACE/CRYPT in aspeed_hace.c.
> > https://patchwork.kernel.org/project/qemu-devel/patch/20250225075622.3
> > 05515-5-jamin_lin@aspeedtech.com/
> >
> > Known Issue:
> > The HACE crypto and hash engine is enable by default since AST2700 A1.
> > However, aspeed_hace.c(HACE model) currently does not support the
> CRYPTO command.
> > To boot AST2700 A1, created a patch which temporarily resolves the
> > issue by sending an interrupt to notify the firmware that the
> > cryptographic command has completed. It is a temporary workaround to
> > resolve the boot issue in the Crypto Manager SelfTest.
> >
> > As a result, you will encounter the following kernel warning due to
> > the Crypto Manager test failure.
> >
> > ```
> > alg: skcipher: aspeed-ctr-tdes encryption test failed (wrong result) on test
> vector 0, cfg="in-place (one sglist)"
> > [    5.035966] alg: self-tests for ctr(des3_ede) using aspeed-ctr-tdes failed
> (rc=-22)
> > [    5.036139] ------------[ cut here ]------------
> > [    5.037188] alg: self-tests for ctr(des3_ede) using aspeed-ctr-tdes failed
> (rc=-22)
> > [    5.037312] WARNING: CPU: 2 PID: 109 at /crypto/testmgr.c:5936
> alg_test+0x42c/0x548
> > [    5.038049] Modules linked in:
> > [    5.038302] CPU: 2 PID: 109 Comm: cryptomgr_test Tainted: G
> W          6.6.52-v00.06.04-gf52a0cf7c475 #1
> > [    5.038787] Hardware name: AST2700-EVB (DT)
> > [    5.038988] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS
> BTYPE=--)
> > [    5.039315] pc : alg_test+0x42c/0x548
> > [    5.039505] lr : alg_test+0x42c/0x548
> > [    5.039697] sp : ffffffc0825e3d50
> > [    5.039862] x29: ffffffc0825e3df0 x28: 0000000000000004 x27:
> 0000000000000000
> > [    5.040226] x26: ffffffc080bcada0 x25: ffffffc081dac9d0 x24:
> 0000000000000004
> > [    5.040700] x23: 0000000000001285 x22: ffffff8003ded280 x21:
> ffffff8003ded200
> > [    5.041458] x20: 00000000ffffffff x19: 00000000ffffffea x18: ffffffffffffffff
> > [    5.041915] x17: 282064656c696166 x16: 20736564742d7274 x15:
> 00000000fffffffe
> > [    5.042287] x14: 0000000000000000 x13: ffffffc081ba555c x12:
> 65742d666c657320
> > [    5.042684] x11: 00000000fffeffff x10: ffffffc0818ff048 x9 :
> ffffffc0800a36e4
> > [    5.043077] x8 : 000000000017ffe8 x7 : c0000000fffeffff x6 :
> 000000000057ffa8
> > [    5.043461] x5 : 000000000000ffff x4 : 0000000000000000 x3 :
> 0000000000000000
> > [    5.043751] x2 : 0000000000000000 x1 : 0000000000000000 x0 :
> ffffff800415e740
> > [    5.045544] Call trace:
> > [    5.045693]  alg_test+0x42c/0x548
> > [    5.045878]  cryptomgr_test+0x28/0x48
> > [    5.046052]  kthread+0x114/0x120
> > [    5.046226]  ret_from_fork+0x10/0x20
> > [    5.046413] ---[ end trace 0000000000000000 ]---
> > [    5.071510] alg: skcipher: aspeed-ctr-des encryption test failed (wrong
> result) on test vector 0, cfg="in-place (one sglist)"
> > [    5.072145] alg: self-tests for ctr(des) using aspeed-ctr-des failed
> (rc=-22)
> > ```
> 
> This is a bit confusing for people running the tests, since it might be considered
> as a regression in the models. I understand it is interesting for real HW and so,
> it would good to complete the implementation of the QEMU models too or,
> may be, we could deactivate the tests from the kernel command line ? if
> possible.
> 
> Thanks,
> 

We plan to implement the "HACE/CRYPT commands", which should resolve these kernel warnings.
However, according to the kernel self-test report, the kernel attempts to test several crypto modes
"that are currently not supported by QEMU crypto/cipher.c such as CFB (Cipher Feedback) and OFB(Output Feedback)"

root@ast2700-default:~# dmesg | grep "self-tests"
[    5.312232] alg: self-tests for ctr(des) using aspeed-ctr-des failed (rc=-22)
[    5.314913] alg: self-tests for ctr(des) using aspeed-ctr-des failed (rc=-22)
[    5.316347] alg: self-tests for ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
[    5.321167] alg: self-tests for ecb(des) using aspeed-ecb-des failed (rc=-22)
[    5.321622] alg: self-tests for ecb(des) using aspeed-ecb-des failed (rc=-22)
[    5.343443] alg: self-tests for ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
[    5.358788] alg: self-tests for ecb(des3_ede) using aspeed-ecb-tdes failed (rc=-22)
[    5.359267] alg: self-tests for ecb(des3_ede) using aspeed-ecb-tdes failed (rc=-22)
[    5.541477] alg: self-tests for cbc(des) using aspeed-cbc-des failed (rc=-22)
[    5.541888] alg: self-tests for cbc(des) using aspeed-cbc-des failed (rc=-22)
[    5.561193] alg: self-tests for cbc(des3_ede) using aspeed-cbc-tdes failed (rc=-22)
[    5.561645] alg: self-tests for cbc(des3_ede) using aspeed-cbc-tdes failed (rc=-22)
[    5.629708] alg: self-tests for cbc(aes) using aspeed-cbc-aes failed (rc=-22)
[    5.630234] alg: self-tests for cbc(aes) using aspeed-cbc-aes failed (rc=-22)
[    5.640356] alg: self-tests for ctr(aes) using aspeed-ctr-aes failed (rc=-22)
[    5.641544] alg: self-tests for ctr(aes) using aspeed-ctr-aes failed (rc=-22)
[    5.653529] alg: self-tests for ecb(aes) using aspeed-ecb-aes failed (rc=-22)
[    5.654212] alg: self-tests for ecb(aes) using aspeed-ecb-aes failed (rc=-22)

I asked our BSP team several times if they could disable the crypto self-tests, but they rejected the request. 
Their reason is that they want to validate our hardware crypto engine during kernel startup in the ASPEED SDK.
If you prefer not to see these kernel warnings, I can add the following kernel boot parameter in the functional test environment:
bootargs=console=ttyS12,115200n8 root=/dev/ram rw earlycon cryptomgr.notests=1

Thanks,
Jamin

> C.
> 

Re: [PATCH v4 0/4] Support HACE to AST2700 (resend)

[Cédric Le Goater]

On 3/13/26 02:35, Jamin Lin wrote:
> + Kane
> 
> Hi Cedric,
> 
>> Subject: Re: RE: [PATCH v4 0/4] Support HACE to AST2700 (resend)
>>
>> Hello Jamin,
>>
>> On 3/12/25 10:32, Jamin Lin wrote:
>>> Hi Cedric,
>>>
>>>> -----Original Message-----
>>>> From: Cédric Le Goater <clg@kaod.org>
>>>> Sent: Wednesday, March 12, 2025 5:25 PM
>>>> To: Jamin Lin <jamin_lin@aspeedtech.com>; Peter Maydell
>>>> <peter.maydell@linaro.org>; Steven Lee <steven_lee@aspeedtech.com>;
>>>> Troy Lee <leetroy@gmail.com>; Andrew Jeffery
>>>> <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>; open
>>>> list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC here
>>>> <qemu-devel@nongnu.org>
>>>> Cc: Troy Lee <troy_lee@aspeedtech.com>
>>>> Subject: Re: [PATCH v4 0/4] Support HACE to AST2700 (resend)
>>>>
>>>> Jamin,
>>>>
>>>> On 2/25/25 08:56, Jamin Lin wrote:
>>>>> This patch series is from
>>>> https://patchwork.kernel.org/project/qemu-devel/cover/20250213033531.
>>>> 336
>>>> 7697-1-jamin_lin@aspeedtech.com/.
>>>>> To expedite the review process, I have separated the HACE patches
>>>>> portion from the
>>>> https://patchwork.kernel.org/project/qemu-devel/cover/20250213033531.
>>>> 336 7697-1-jamin_lin@aspeedtech.com/ patch series into this new patch
>>>> series.
>>>>>
>>>>> v4: Support HACE to AST2700
>>>>>
>>>>> Jamin Lin (4):
>>>>>      hw/misc/aspeed_hace: Fix coding style
>>>>>      hw/misc/aspeed_hace: Add AST2700 support
>>>>>      hw/arm/aspeed_ast27x0: Add HACE support for AST2700
>>>>>      hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self
>>>>> Test
>>>>>
>>>>>     include/hw/misc/aspeed_hace.h |  2 ++
>>>>>     hw/arm/aspeed_ast27x0.c       | 15 ++++++++++
>>>>>     hw/misc/aspeed_hace.c         | 55
>>>> ++++++++++++++++++++++++++++++++---
>>>>>     3 files changed, 68 insertions(+), 4 deletions(-)
>>>>>
>>>>
>>>> The kernel reports a lot of errors when running the crypto tests on a
>>>> AST2700 A1 Soc. See the functional test logs :
>>>>
>>>>        $ grep alg:
>>>> build/tests/functional/aarch64/test_aarch64_aspeed.AST2x00MachineSDK.
>>>> test _aarch64_ast2700a1_evb_sdk_v09_05/console.log
>>>>
>>>>        2025-03-12 10:10:42,705: [    2.206831] alg: skcipher:
>> aspeed-ctr-tdes
>>>> encryption test failed (wrong result) on test vector 0, cfg="in-place (one
>> sglist)"
>>>>        2025-03-12 10:10:42,706: [    2.207328] alg: self-tests for
>>>> ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
>>>>        2025-03-12 10:10:42,707: [    2.207894] alg: self-tests for
>>>> ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
>>>>        2025-03-12 10:10:42,725: [    2.219342] alg: skcipher:
>> aspeed-ctr-des
>>>> encryption test failed (wrong result) on test vector 0, cfg="in-place (one
>> sglist)"
>>>>        2025-03-12 10:10:42,725: [    2.219511] alg: self-tests for ctr(des)
>>>> using aspeed-ctr-des failed (rc=-22)
>>>>        2025-03-12 10:10:42,725: [    2.219651] alg: self-tests for ctr(des)
>>>> using aspeed-ctr-des failed (rc=-22)
>>>>        2025-03-12 10:10:42,733: [    2.224344] alg: skcipher:
>> aspeed-ctr-aes
>>>> encryption test failed (wrong result) on test vector 0, cfg="in-place (one
>> sglist)"
>>>>        2025-03-12 10:10:42,734: [    2.224775] alg: self-tests for ctr(aes)
>>>> using aspeed-ctr-aes failed (rc=-22)
>>>>        2025-03-12 10:10:42,734: [    2.225015] alg: self-tests for ctr(aes)
>>>> using aspeed-ctr-aes failed (rc=-22)
>>>>        ...
>>>>
>>>> What is something missing in the models ?
>>>>
>>>
>>> Currently, we do not implement HACE/CRYPT in aspeed_hace.c.
>>> https://patchwork.kernel.org/project/qemu-devel/patch/20250225075622.3
>>> 05515-5-jamin_lin@aspeedtech.com/
>>>
>>> Known Issue:
>>> The HACE crypto and hash engine is enable by default since AST2700 A1.
>>> However, aspeed_hace.c(HACE model) currently does not support the
>> CRYPTO command.
>>> To boot AST2700 A1, created a patch which temporarily resolves the
>>> issue by sending an interrupt to notify the firmware that the
>>> cryptographic command has completed. It is a temporary workaround to
>>> resolve the boot issue in the Crypto Manager SelfTest.
>>>
>>> As a result, you will encounter the following kernel warning due to
>>> the Crypto Manager test failure.
>>>
>>> ```
>>> alg: skcipher: aspeed-ctr-tdes encryption test failed (wrong result) on test
>> vector 0, cfg="in-place (one sglist)"
>>> [    5.035966] alg: self-tests for ctr(des3_ede) using aspeed-ctr-tdes failed
>> (rc=-22)
>>> [    5.036139] ------------[ cut here ]------------
>>> [    5.037188] alg: self-tests for ctr(des3_ede) using aspeed-ctr-tdes failed
>> (rc=-22)
>>> [    5.037312] WARNING: CPU: 2 PID: 109 at /crypto/testmgr.c:5936
>> alg_test+0x42c/0x548
>>> [    5.038049] Modules linked in:
>>> [    5.038302] CPU: 2 PID: 109 Comm: cryptomgr_test Tainted: G
>> W          6.6.52-v00.06.04-gf52a0cf7c475 #1
>>> [    5.038787] Hardware name: AST2700-EVB (DT)
>>> [    5.038988] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS
>> BTYPE=--)
>>> [    5.039315] pc : alg_test+0x42c/0x548
>>> [    5.039505] lr : alg_test+0x42c/0x548
>>> [    5.039697] sp : ffffffc0825e3d50
>>> [    5.039862] x29: ffffffc0825e3df0 x28: 0000000000000004 x27:
>> 0000000000000000
>>> [    5.040226] x26: ffffffc080bcada0 x25: ffffffc081dac9d0 x24:
>> 0000000000000004
>>> [    5.040700] x23: 0000000000001285 x22: ffffff8003ded280 x21:
>> ffffff8003ded200
>>> [    5.041458] x20: 00000000ffffffff x19: 00000000ffffffea x18: ffffffffffffffff
>>> [    5.041915] x17: 282064656c696166 x16: 20736564742d7274 x15:
>> 00000000fffffffe
>>> [    5.042287] x14: 0000000000000000 x13: ffffffc081ba555c x12:
>> 65742d666c657320
>>> [    5.042684] x11: 00000000fffeffff x10: ffffffc0818ff048 x9 :
>> ffffffc0800a36e4
>>> [    5.043077] x8 : 000000000017ffe8 x7 : c0000000fffeffff x6 :
>> 000000000057ffa8
>>> [    5.043461] x5 : 000000000000ffff x4 : 0000000000000000 x3 :
>> 0000000000000000
>>> [    5.043751] x2 : 0000000000000000 x1 : 0000000000000000 x0 :
>> ffffff800415e740
>>> [    5.045544] Call trace:
>>> [    5.045693]  alg_test+0x42c/0x548
>>> [    5.045878]  cryptomgr_test+0x28/0x48
>>> [    5.046052]  kthread+0x114/0x120
>>> [    5.046226]  ret_from_fork+0x10/0x20
>>> [    5.046413] ---[ end trace 0000000000000000 ]---
>>> [    5.071510] alg: skcipher: aspeed-ctr-des encryption test failed (wrong
>> result) on test vector 0, cfg="in-place (one sglist)"
>>> [    5.072145] alg: self-tests for ctr(des) using aspeed-ctr-des failed
>> (rc=-22)
>>> ```
>>
>> This is a bit confusing for people running the tests, since it might be considered
>> as a regression in the models. I understand it is interesting for real HW and so,
>> it would good to complete the implementation of the QEMU models too or,
>> may be, we could deactivate the tests from the kernel command line ? if
>> possible.
>>
>> Thanks,
>>
> 
> We plan to implement the "HACE/CRYPT commands", which should resolve these kernel warnings.
> However, according to the kernel self-test report, the kernel attempts to test several crypto modes
> "that are currently not supported by QEMU crypto/cipher.c such as CFB (Cipher Feedback) and OFB(Output Feedback)"
> 
> root@ast2700-default:~# dmesg | grep "self-tests"
> [    5.312232] alg: self-tests for ctr(des) using aspeed-ctr-des failed (rc=-22)
> [    5.314913] alg: self-tests for ctr(des) using aspeed-ctr-des failed (rc=-22)
> [    5.316347] alg: self-tests for ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
> [    5.321167] alg: self-tests for ecb(des) using aspeed-ecb-des failed (rc=-22)
> [    5.321622] alg: self-tests for ecb(des) using aspeed-ecb-des failed (rc=-22)
> [    5.343443] alg: self-tests for ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
> [    5.358788] alg: self-tests for ecb(des3_ede) using aspeed-ecb-tdes failed (rc=-22)
> [    5.359267] alg: self-tests for ecb(des3_ede) using aspeed-ecb-tdes failed (rc=-22)
> [    5.541477] alg: self-tests for cbc(des) using aspeed-cbc-des failed (rc=-22)
> [    5.541888] alg: self-tests for cbc(des) using aspeed-cbc-des failed (rc=-22)
> [    5.561193] alg: self-tests for cbc(des3_ede) using aspeed-cbc-tdes failed (rc=-22)
> [    5.561645] alg: self-tests for cbc(des3_ede) using aspeed-cbc-tdes failed (rc=-22)
> [    5.629708] alg: self-tests for cbc(aes) using aspeed-cbc-aes failed (rc=-22)
> [    5.630234] alg: self-tests for cbc(aes) using aspeed-cbc-aes failed (rc=-22)
> [    5.640356] alg: self-tests for ctr(aes) using aspeed-ctr-aes failed (rc=-22)
> [    5.641544] alg: self-tests for ctr(aes) using aspeed-ctr-aes failed (rc=-22)
> [    5.653529] alg: self-tests for ecb(aes) using aspeed-ecb-aes failed (rc=-22)
> [    5.654212] alg: self-tests for ecb(aes) using aspeed-ecb-aes failed (rc=-22)
> 
> I asked our BSP team several times if they could disable the crypto self-tests, but they rejected the request.
> Their reason is that they want to validate our hardware crypto engine during kernel startup in the ASPEED SDK.

yes it makes sense.

> If you prefer not to see these kernel warnings, I can add the following kernel boot parameter in the functional test environment:
> bootargs=console=ttyS12,115200n8 root=/dev/ram rw earlycon cryptomgr.notests=1

Given the number of warnings in the kernel, I think we should rely
on the unit tests tests/qtest/ast2700-hace-test.c only and drop
the kernel tests to reduce confusion.

Once the QEMU (Aspeed HACE and crypto) implementation has made
progress, we should reassess.

Thanks,

C.