All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/3] hw/misc: Model ASPEED hash and crypto engine
@ 2021-03-24  7:09 Joel Stanley
  2021-03-24  7:09 ` [PATCH v4 1/3] hw: Model ASPEED's Hash and Crypto Engine Joel Stanley
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Joel Stanley @ 2021-03-24  7:09 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Cédric Le Goater
  Cc: Andrew Jeffery, Peter Maydell, qemu-arm, qemu-devel

v4: Rebase on Philippe's memory region cleanup series [1]
    Address feedback from Cédric
    Rework qtest to run on ast2400, ast2500 and ast2600
v3: Rework qtest to not use libqtest-single.h, rebase to avoid LPC
conflicts.
v2: Address review from Andrew and Philippe. Adds a qtest.

[1] https://lore.kernel.org/qemu-devel/20210312182851.1922972-1-f4bug@amsat.org/

This adds a model for the ASPEED hash and crypto engine (HACE) found on
all supported ASPEED SoCs.

The model uses Qemu's gcrypto API to perform the SHA and MD5 hashing
directly in the machine's emulated memory space, which I found a neat
use of Qemu's features.

It has been tested using u-boot and from Linux userspace, and adds a
qtest for the model running as part of the ast2600-evb, ast2500-evb and
palmetto-bmc (to test ast2400) machines.

Note that the tests will fail without Philippe's series.

Joel Stanley (3):
  hw: Model ASPEED's Hash and Crypto Engine
  aspeed: Integrate HACE
  tests/qtest: Add test for Aspeed HACE

 docs/system/arm/aspeed.rst     |   2 +-
 include/hw/arm/aspeed_soc.h    |   3 +
 include/hw/misc/aspeed_hace.h  |  43 ++++
 hw/arm/aspeed_ast2600.c        |  15 ++
 hw/arm/aspeed_soc.c            |  16 ++
 hw/misc/aspeed_hace.c          | 358 +++++++++++++++++++++++++++++++++
 tests/qtest/aspeed_hace-test.c | 313 ++++++++++++++++++++++++++++
 MAINTAINERS                    |   1 +
 hw/misc/meson.build            |   1 +
 tests/qtest/meson.build        |   3 +
 10 files changed, 754 insertions(+), 1 deletion(-)
 create mode 100644 include/hw/misc/aspeed_hace.h
 create mode 100644 hw/misc/aspeed_hace.c
 create mode 100644 tests/qtest/aspeed_hace-test.c

-- 
2.30.2



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 1/3] hw: Model ASPEED's Hash and Crypto Engine
  2021-03-24  7:09 [PATCH v4 0/3] hw/misc: Model ASPEED hash and crypto engine Joel Stanley
@ 2021-03-24  7:09 ` Joel Stanley
  2021-03-24  7:26   ` Cédric Le Goater
  2021-03-24  9:46   ` Philippe Mathieu-Daudé
  2021-03-24  7:09 ` [PATCH v4 2/3] aspeed: Integrate HACE Joel Stanley
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 11+ messages in thread
From: Joel Stanley @ 2021-03-24  7:09 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Cédric Le Goater
  Cc: Andrew Jeffery, Peter Maydell, qemu-arm, qemu-devel

The HACE (Hash and Crypto Engine) is a device that offloads MD5, SHA1,
SHA2, RSA and other cryptographic algorithms.

This initial model implements a subset of the device's functionality;
currently only direct access (non-scatter gather) hashing.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
v3:
 - rebase on upstream to fix meson.build conflict
v2:
 - reorder register defines
 - mask src/dest/len registers according to hardware
v4:
 - Fix typos in comments
 - Remove sdram base address; new memory region fixes mean this is not
   required
 - Use PRIx64
 - Add Object Classes for soc familiy specific features
 - Convert big switch statement to a lookup in a struct
---
 include/hw/misc/aspeed_hace.h |  43 ++++
 hw/misc/aspeed_hace.c         | 358 ++++++++++++++++++++++++++++++++++
 hw/misc/meson.build           |   1 +
 3 files changed, 402 insertions(+)
 create mode 100644 include/hw/misc/aspeed_hace.h
 create mode 100644 hw/misc/aspeed_hace.c

diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h
new file mode 100644
index 000000000000..94d5ada95fa2
--- /dev/null
+++ b/include/hw/misc/aspeed_hace.h
@@ -0,0 +1,43 @@
+/*
+ * ASPEED Hash and Crypto Engine
+ *
+ * Copyright (C) 2021 IBM Corp.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef ASPEED_HACE_H
+#define ASPEED_HACE_H
+
+#include "hw/sysbus.h"
+
+#define TYPE_ASPEED_HACE "aspeed.hace"
+#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"
+OBJECT_DECLARE_TYPE(AspeedHACEState, AspeedHACEClass, ASPEED_HACE)
+
+#define ASPEED_HACE_NR_REGS (0x64 >> 2)
+
+struct AspeedHACEState {
+    SysBusDevice parent;
+
+    MemoryRegion iomem;
+    qemu_irq irq;
+
+    uint32_t regs[ASPEED_HACE_NR_REGS];
+
+    MemoryRegion *dram_mr;
+    AddressSpace dram_as;
+};
+
+
+struct AspeedHACEClass {
+    SysBusDeviceClass parent_class;
+
+    uint32_t src_mask;
+    uint32_t dest_mask;
+    uint32_t hash_mask;
+};
+
+#endif /* _ASPEED_HACE_H_ */
diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
new file mode 100644
index 000000000000..a962ccea90e8
--- /dev/null
+++ b/hw/misc/aspeed_hace.c
@@ -0,0 +1,358 @@
+/*
+ * ASPEED Hash and Crypto Engine
+ *
+ * Copyright (C) 2021 IBM Corp.
+ *
+ * Joel Stanley <joel@jms.id.au>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/error-report.h"
+#include "hw/misc/aspeed_hace.h"
+#include "qapi/error.h"
+#include "migration/vmstate.h"
+#include "crypto/hash.h"
+#include "hw/qdev-properties.h"
+#include "hw/irq.h"
+
+#define R_CRYPT_CMD     (0x10 / 4)
+
+#define R_STATUS        (0x1c / 4)
+#define HASH_IRQ        BIT(9)
+#define CRYPT_IRQ       BIT(12)
+#define TAG_IRQ         BIT(15)
+
+#define R_HASH_SRC      (0x20 / 4)
+#define R_HASH_DEST     (0x24 / 4)
+#define R_HASH_SRC_LEN  (0x2c / 4)
+
+#define R_HASH_CMD      (0x30 / 4)
+/* Hash algorithm selection */
+#define  HASH_ALGO_MASK                 (BIT(4) | BIT(5) | BIT(6))
+#define  HASH_ALGO_MD5                  0
+#define  HASH_ALGO_SHA1                 BIT(5)
+#define  HASH_ALGO_SHA224               BIT(6)
+#define  HASH_ALGO_SHA256               (BIT(4) | BIT(6))
+#define  HASH_ALGO_SHA512_SERIES        (BIT(5) | BIT(6))
+/* SHA512 algorithm selection */
+#define  SHA512_HASH_ALGO_MASK          (BIT(10) | BIT(11) | BIT(12))
+#define  HASH_ALGO_SHA512_SHA512        0
+#define  HASH_ALGO_SHA512_SHA384        BIT(10)
+#define  HASH_ALGO_SHA512_SHA256        BIT(11)
+#define  HASH_ALGO_SHA512_SHA224        (BIT(10) | BIT(11))
+/* HMAC modes */
+#define  HASH_HMAC_MASK                 (BIT(7) | BIT(8))
+#define  HASH_DIGEST                    0
+#define  HASH_DIGEST_HMAC               BIT(7)
+#define  HASH_DIGEST_ACCUM              BIT(8)
+#define  HASH_HMAC_KEY                  (BIT(7) | BIT(8))
+/* Cascaded operation modes */
+#define  HASH_ONLY                      0
+#define  HASH_ONLY2                     BIT(0)
+#define  HASH_CRYPT_THEN_HASH           BIT(1)
+#define  HASH_HASH_THEN_CRYPT           (BIT(0) | BIT(1))
+/* Other cmd bits */
+#define  HASH_IRQ_EN                    BIT(9)
+#define  HASH_SG_EN                     BIT(18)
+
+static const struct {
+    uint32_t mask;
+    QCryptoHashAlgorithm algo;
+} hash_algo_map[] = {
+    { HASH_ALGO_MD5, QCRYPTO_HASH_ALG_MD5 },
+    { HASH_ALGO_SHA1, QCRYPTO_HASH_ALG_SHA1 },
+    { HASH_ALGO_SHA224, QCRYPTO_HASH_ALG_SHA224 },
+    { HASH_ALGO_SHA256, QCRYPTO_HASH_ALG_SHA256 },
+    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA512, QCRYPTO_HASH_ALG_SHA512 },
+    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA384, QCRYPTO_HASH_ALG_SHA384 },
+    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA256, QCRYPTO_HASH_ALG_SHA256 },
+};
+
+static int hash_algo_lookup(uint32_t mask)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(hash_algo_map); i++) {
+        if (mask == hash_algo_map[i].mask)
+            return hash_algo_map[i].algo;
+    }
+
+    return -1;
+}
+
+static int do_hash_operation(AspeedHACEState *s, int algo)
+{
+    hwaddr src, len, dest;
+    uint8_t *digest_buf = NULL;
+    size_t digest_len = 0;
+    char *src_buf;
+    int rc;
+
+    src = s->regs[R_HASH_SRC];
+    len = s->regs[R_HASH_SRC_LEN];
+    dest = s->regs[R_HASH_DEST];
+
+    src_buf = address_space_map(&s->dram_as, src, &len, false,
+                                MEMTXATTRS_UNSPECIFIED);
+    if (!src_buf) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map dram\n", __func__);
+        return -EACCES;
+    }
+
+    rc = qcrypto_hash_bytes(algo, src_buf, len, &digest_buf, &digest_len,
+                            &error_fatal);
+    if (rc < 0) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto failed\n", __func__);
+        return rc;
+    }
+
+    rc = address_space_write(&s->dram_as, dest, MEMTXATTRS_UNSPECIFIED,
+                             digest_buf, digest_len);
+    if (rc) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: address space write failed\n", __func__);
+    }
+    g_free(digest_buf);
+
+    address_space_unmap(&s->dram_as, src_buf, len, false, len);
+
+    /*
+     * Set status bits to indicate completion. Testing shows hardware sets
+     * these irrespective of HASH_IRQ_EN.
+     */
+    s->regs[R_STATUS] |= HASH_IRQ;
+
+    return 0;
+}
+
+
+static uint64_t aspeed_hace_read(void *opaque, hwaddr addr, unsigned int size)
+{
+    AspeedHACEState *s = ASPEED_HACE(opaque);
+
+    addr >>= 2;
+
+    if (addr >= ASPEED_HACE_NR_REGS) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Out-of-bounds read at offset 0x%" HWADDR_PRIx "\n",
+                      __func__, addr << 2);
+        return 0;
+    }
+
+    return s->regs[addr];
+}
+
+static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
+                              unsigned int size)
+{
+    AspeedHACEState *s = ASPEED_HACE(opaque);
+    AspeedHACEClass *ahc = ASPEED_HACE_GET_CLASS(s);
+
+    addr >>= 2;
+
+    if (addr >= ASPEED_HACE_NR_REGS) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Out-of-bounds write at offset 0x%" HWADDR_PRIx "\n",
+                      __func__, addr << 2);
+        return;
+    }
+
+    switch (addr) {
+    case R_STATUS:
+        if (data & HASH_IRQ) {
+            data &= ~HASH_IRQ;
+
+            if (s->regs[addr] & HASH_IRQ) {
+                qemu_irq_lower(s->irq);
+            }
+        }
+        break;
+    case R_HASH_SRC:
+        data &= ahc->src_mask;
+        break;
+    case R_HASH_DEST:
+        data &= ahc->dest_mask;
+        break;
+    case R_HASH_SRC_LEN:
+        data &= 0x0FFFFFFF;
+        break;
+    case R_HASH_CMD: {
+        int algo = -1;
+        if ((data & HASH_HMAC_MASK)) {
+            qemu_log_mask(LOG_UNIMP,
+                          "%s: HMAC engine command mode %"PRIx64" not implemented",
+                          __func__, (data & HASH_HMAC_MASK) >> 8);
+        }
+        if (data & HASH_SG_EN) {
+            qemu_log_mask(LOG_UNIMP,
+                          "%s: Hash scatter gather mode not implemented",
+                          __func__);
+        }
+        if (data & BIT(1)) {
+            qemu_log_mask(LOG_UNIMP,
+                          "%s: Cascaded mode not implemented",
+                          __func__);
+        }
+        algo = hash_algo_lookup(data & ahc->hash_mask);
+        if (algo < 0) {
+                qemu_log_mask(LOG_GUEST_ERROR,
+                        "%s: Invalid hash algorithm selection 0x%"PRIx64"\n",
+                        __func__, data & ahc->hash_mask);
+                break;
+        }
+        do_hash_operation(s, algo);
+
+        if (data & HASH_IRQ_EN) {
+            qemu_irq_raise(s->irq);
+        }
+        break;
+    }
+    case R_CRYPT_CMD:
+        qemu_log_mask(LOG_UNIMP, "%s: Crypt commands not implemented\n",
+                       __func__);
+        break;
+    default:
+        break;
+    }
+
+    s->regs[addr] = data;
+}
+
+static const MemoryRegionOps aspeed_hace_ops = {
+    .read = aspeed_hace_read,
+    .write = aspeed_hace_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 4,
+    },
+};
+
+static void aspeed_hace_reset(DeviceState *dev)
+{
+    struct AspeedHACEState *s = ASPEED_HACE(dev);
+
+    memset(s->regs, 0, sizeof(s->regs));
+}
+
+static void aspeed_hace_realize(DeviceState *dev, Error **errp)
+{
+    AspeedHACEState *s = ASPEED_HACE(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+
+    sysbus_init_irq(sbd, &s->irq);
+
+    memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_hace_ops, s,
+            TYPE_ASPEED_HACE, 0x1000);
+
+    if (!s->dram_mr) {
+        error_setg(errp, TYPE_ASPEED_HACE ": 'dram' link not set");
+        return;
+    }
+
+    address_space_init(&s->dram_as, s->dram_mr, "dram");
+
+    sysbus_init_mmio(sbd, &s->iomem);
+}
+
+static Property aspeed_hace_properties[] = {
+    DEFINE_PROP_LINK("dram", AspeedHACEState, dram_mr,
+                     TYPE_MEMORY_REGION, MemoryRegion *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+
+static const VMStateDescription vmstate_aspeed_hace = {
+    .name = TYPE_ASPEED_HACE,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(regs, AspeedHACEState, ASPEED_HACE_NR_REGS),
+        VMSTATE_END_OF_LIST(),
+    }
+};
+
+static void aspeed_hace_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = aspeed_hace_realize;
+    dc->reset = aspeed_hace_reset;
+    device_class_set_props(dc, aspeed_hace_properties);
+    dc->vmsd = &vmstate_aspeed_hace;
+}
+
+static const TypeInfo aspeed_hace_info = {
+    .name = TYPE_ASPEED_HACE,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(AspeedHACEState),
+    .class_init = aspeed_hace_class_init,
+    .class_size = sizeof(AspeedHACEClass)
+};
+
+static void aspeed_ast2400_hace_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    AspeedHACEClass *ahc = ASPEED_HACE_CLASS(klass);
+
+    dc->desc = "AST2400 Hash and Crypto Engine";
+
+    ahc->src_mask = 0x0FFFFFFF;
+    ahc->dest_mask = 0x0FFFFFF8;
+    ahc->hash_mask = HASH_ALGO_MASK;
+}
+
+static const TypeInfo aspeed_ast2400_hace_info = {
+    .name = TYPE_ASPEED_AST2400_HACE,
+    .parent = TYPE_ASPEED_HACE,
+    .class_init = aspeed_ast2400_hace_class_init,
+};
+
+static void aspeed_ast2500_hace_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    AspeedHACEClass *ahc = ASPEED_HACE_CLASS(klass);
+
+    dc->desc = "AST2500 Hash and Crypto Engine";
+
+    ahc->src_mask = 0x3fffffff;
+    ahc->dest_mask = 0x3ffffff8;
+    ahc->hash_mask = HASH_ALGO_MASK;
+}
+
+static const TypeInfo aspeed_ast2500_hace_info = {
+    .name = TYPE_ASPEED_AST2500_HACE,
+    .parent = TYPE_ASPEED_HACE,
+    .class_init = aspeed_ast2500_hace_class_init,
+};
+
+static void aspeed_ast2600_hace_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    AspeedHACEClass *ahc = ASPEED_HACE_CLASS(klass);
+
+    dc->desc = "AST2600 Hash and Crypto Engine";
+
+    ahc->src_mask = 0x7FFFFFFF;
+    ahc->dest_mask = 0x7FFFFFF8;
+    ahc->hash_mask = HASH_ALGO_MASK | SHA512_HASH_ALGO_MASK;
+}
+
+static const TypeInfo aspeed_ast2600_hace_info = {
+    .name = TYPE_ASPEED_AST2600_HACE,
+    .parent = TYPE_ASPEED_HACE,
+    .class_init = aspeed_ast2600_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_hace_info);
+}
+
+type_init(aspeed_hace_register_types);
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 21034dc60a81..1e7b8b064bd1 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -109,6 +109,7 @@ softmmu_ss.add(when: 'CONFIG_PVPANIC_ISA', if_true: files('pvpanic-isa.c'))
 softmmu_ss.add(when: 'CONFIG_PVPANIC_PCI', if_true: files('pvpanic-pci.c'))
 softmmu_ss.add(when: 'CONFIG_AUX', if_true: files('auxbus.c'))
 softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files(
+  'aspeed_hace.c',
   'aspeed_lpc.c',
   'aspeed_scu.c',
   'aspeed_sdmc.c',
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 2/3] aspeed: Integrate HACE
  2021-03-24  7:09 [PATCH v4 0/3] hw/misc: Model ASPEED hash and crypto engine Joel Stanley
  2021-03-24  7:09 ` [PATCH v4 1/3] hw: Model ASPEED's Hash and Crypto Engine Joel Stanley
@ 2021-03-24  7:09 ` Joel Stanley
  2021-03-24  7:21   ` Cédric Le Goater
  2021-03-24  9:47   ` Philippe Mathieu-Daudé
  2021-03-24  7:09 ` [PATCH v4 3/3] tests/qtest: Add test for Aspeed HACE Joel Stanley
  2021-03-24  7:30 ` [PATCH v4 0/3] hw/misc: Model ASPEED hash and crypto engine no-reply
  3 siblings, 2 replies; 11+ messages in thread
From: Joel Stanley @ 2021-03-24  7:09 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Cédric Le Goater
  Cc: Andrew Jeffery, Peter Maydell, qemu-arm, qemu-devel

Add the hash and crypto engine model to the Aspeed socs.

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
v3: Rebase on upstream
v4: Update integration for soc-specific hace objects
---
 docs/system/arm/aspeed.rst  |  2 +-
 include/hw/arm/aspeed_soc.h |  3 +++
 hw/arm/aspeed_ast2600.c     | 15 +++++++++++++++
 hw/arm/aspeed_soc.c         | 16 ++++++++++++++++
 4 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
index d1fb8f25b39c..f9466e6d8245 100644
--- a/docs/system/arm/aspeed.rst
+++ b/docs/system/arm/aspeed.rst
@@ -49,6 +49,7 @@ Supported devices
  * Ethernet controllers
  * Front LEDs (PCA9552 on I2C bus)
  * LPC Peripheral Controller (a subset of subdevices are supported)
+ * Hash/Crypto Engine (HACE) - Hash support only, no scatter-gather
 
 
 Missing devices
@@ -59,7 +60,6 @@ Missing devices
  * PWM and Fan Controller
  * Slave GPIO Controller
  * Super I/O Controller
- * Hash/Crypto Engine
  * PCI-Express 1 Controller
  * Graphic Display Controller
  * PECI Controller
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 9359d6da336d..d9161d26d645 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -21,6 +21,7 @@
 #include "hw/rtc/aspeed_rtc.h"
 #include "hw/i2c/aspeed_i2c.h"
 #include "hw/ssi/aspeed_smc.h"
+#include "hw/misc/aspeed_hace.h"
 #include "hw/watchdog/wdt_aspeed.h"
 #include "hw/net/ftgmac100.h"
 #include "target/arm/cpu.h"
@@ -50,6 +51,7 @@ struct AspeedSoCState {
     AspeedTimerCtrlState timerctrl;
     AspeedI2CState i2c;
     AspeedSCUState scu;
+    AspeedHACEState hace;
     AspeedXDMAState xdma;
     AspeedSMCState fmc;
     AspeedSMCState spi[ASPEED_SPIS_NUM];
@@ -133,6 +135,7 @@ enum {
     ASPEED_DEV_XDMA,
     ASPEED_DEV_EMMC,
     ASPEED_DEV_KCS,
+    ASPEED_DEV_HACE,
 };
 
 #endif /* ASPEED_SOC_H */
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index bc87e754a3cc..c149936e0b28 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -42,6 +42,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
     [ASPEED_DEV_ETH2]      = 0x1E680000,
     [ASPEED_DEV_ETH4]      = 0x1E690000,
     [ASPEED_DEV_VIC]       = 0x1E6C0000,
+    [ASPEED_DEV_HACE]      = 0x1E6D0000,
     [ASPEED_DEV_SDMC]      = 0x1E6E0000,
     [ASPEED_DEV_SCU]       = 0x1E6E2000,
     [ASPEED_DEV_XDMA]      = 0x1E6E7000,
@@ -102,6 +103,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
     [ASPEED_DEV_I2C]       = 110,   /* 110 -> 125 */
     [ASPEED_DEV_ETH1]      = 2,
     [ASPEED_DEV_ETH2]      = 3,
+    [ASPEED_DEV_HACE]      = 4,
     [ASPEED_DEV_ETH3]      = 32,
     [ASPEED_DEV_ETH4]      = 33,
     [ASPEED_DEV_KCS]       = 138,   /* 138 -> 142 */
@@ -213,6 +215,9 @@ static void aspeed_soc_ast2600_init(Object *obj)
                             TYPE_SYSBUS_SDHCI);
 
     object_initialize_child(obj, "lpc", &s->lpc, TYPE_ASPEED_LPC);
+
+    snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
+    object_initialize_child(obj, "hace", &s->hace, typename);
 }
 
 /*
@@ -498,6 +503,16 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_4,
                        qdev_get_gpio_in(DEVICE(&s->a7mpcore),
                                 sc->irqmap[ASPEED_DEV_KCS] + aspeed_lpc_kcs_4));
+
+    /* 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;
+    }
+    sysbus_mmio_map(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));
 }
 
 static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 057d053c8478..c8c3bd233233 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -34,6 +34,7 @@ static const hwaddr aspeed_soc_ast2400_memmap[] = {
     [ASPEED_DEV_VIC]    = 0x1E6C0000,
     [ASPEED_DEV_SDMC]   = 0x1E6E0000,
     [ASPEED_DEV_SCU]    = 0x1E6E2000,
+    [ASPEED_DEV_HACE]   = 0x1E6E3000,
     [ASPEED_DEV_XDMA]   = 0x1E6E7000,
     [ASPEED_DEV_VIDEO]  = 0x1E700000,
     [ASPEED_DEV_ADC]    = 0x1E6E9000,
@@ -65,6 +66,7 @@ static const hwaddr aspeed_soc_ast2500_memmap[] = {
     [ASPEED_DEV_VIC]    = 0x1E6C0000,
     [ASPEED_DEV_SDMC]   = 0x1E6E0000,
     [ASPEED_DEV_SCU]    = 0x1E6E2000,
+    [ASPEED_DEV_HACE]   = 0x1E6E3000,
     [ASPEED_DEV_XDMA]   = 0x1E6E7000,
     [ASPEED_DEV_ADC]    = 0x1E6E9000,
     [ASPEED_DEV_VIDEO]  = 0x1E700000,
@@ -117,6 +119,7 @@ static const int aspeed_soc_ast2400_irqmap[] = {
     [ASPEED_DEV_ETH2]   = 3,
     [ASPEED_DEV_XDMA]   = 6,
     [ASPEED_DEV_SDHCI]  = 26,
+    [ASPEED_DEV_HACE]   = 4,
 };
 
 #define aspeed_soc_ast2500_irqmap aspeed_soc_ast2400_irqmap
@@ -212,6 +215,9 @@ static void aspeed_soc_init(Object *obj)
     }
 
     object_initialize_child(obj, "lpc", &s->lpc, TYPE_ASPEED_LPC);
+
+    snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
+    object_initialize_child(obj, "hace", &s->hace, typename);
 }
 
 static void aspeed_soc_realize(DeviceState *dev, Error **errp)
@@ -425,6 +431,16 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
 
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_4,
                        qdev_get_gpio_in(DEVICE(&s->lpc), aspeed_lpc_kcs_4));
+
+    /* 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;
+    }
+    sysbus_mmio_map(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));
 }
 static Property aspeed_soc_properties[] = {
     DEFINE_PROP_LINK("dram", AspeedSoCState, dram_mr, TYPE_MEMORY_REGION,
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 3/3] tests/qtest: Add test for Aspeed HACE
  2021-03-24  7:09 [PATCH v4 0/3] hw/misc: Model ASPEED hash and crypto engine Joel Stanley
  2021-03-24  7:09 ` [PATCH v4 1/3] hw: Model ASPEED's Hash and Crypto Engine Joel Stanley
  2021-03-24  7:09 ` [PATCH v4 2/3] aspeed: Integrate HACE Joel Stanley
@ 2021-03-24  7:09 ` Joel Stanley
  2021-03-24  7:21   ` Cédric Le Goater
  2021-03-24  7:30 ` [PATCH v4 0/3] hw/misc: Model ASPEED hash and crypto engine no-reply
  3 siblings, 1 reply; 11+ messages in thread
From: Joel Stanley @ 2021-03-24  7:09 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Cédric Le Goater
  Cc: Andrew Jeffery, Peter Maydell, qemu-arm, qemu-devel

This adds a test for the Aspeed Hash and Crypto (HACE) engine. It tests
the currently implemented behavior of the hash functionality.

The tests are similar, but are cut/pasted instead of broken out into a
common function so the assert machinery produces useful output when a
test fails.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
v3: Write test without libqtest-single.h
v4: Run tests on all aspeed machines
---
 tests/qtest/aspeed_hace-test.c | 313 +++++++++++++++++++++++++++++++++
 MAINTAINERS                    |   1 +
 tests/qtest/meson.build        |   3 +
 3 files changed, 317 insertions(+)
 create mode 100644 tests/qtest/aspeed_hace-test.c

diff --git a/tests/qtest/aspeed_hace-test.c b/tests/qtest/aspeed_hace-test.c
new file mode 100644
index 000000000000..2b624b6b099b
--- /dev/null
+++ b/tests/qtest/aspeed_hace-test.c
@@ -0,0 +1,313 @@
+/*
+ * QTest testcase for the ASPEED Hash and Crypto Engine
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright 2021 IBM Corp.
+ */
+
+#include "qemu/osdep.h"
+
+#include "libqos/libqtest.h"
+#include "qemu-common.h"
+#include "qemu/bitops.h"
+
+#define HACE_CMD                 0x10
+#define  HACE_SHA_BE_EN          BIT(3)
+#define  HACE_MD5_LE_EN          BIT(2)
+#define  HACE_ALGO_MD5           0
+#define  HACE_ALGO_SHA1          BIT(5)
+#define  HACE_ALGO_SHA224        BIT(6)
+#define  HACE_ALGO_SHA256        (BIT(4) | BIT(6))
+#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_STS                 0x1c
+#define  HACE_RSA_ISR            BIT(13)
+#define  HACE_CRYPTO_ISR         BIT(12)
+#define  HACE_HASH_ISR           BIT(9)
+#define  HACE_RSA_BUSY           BIT(2)
+#define  HACE_CRYPTO_BUSY        BIT(1)
+#define  HACE_HASH_BUSY          BIT(0)
+#define HACE_HASH_SRC            0x20
+#define HACE_HASH_DIGEST         0x24
+#define HACE_HASH_KEY_BUFF       0x28
+#define HACE_HASH_DATA_LEN       0x2c
+#define HACE_HASH_CMD            0x30
+
+/*
+ * Test vector is the ascii "abc"
+ *
+ * Expected results were generated using command line utitiles:
+ *
+ *  echo -n -e 'abc' | dd of=/tmp/test
+ *  for hash in sha512sum sha256sum md5sum; do $hash /tmp/test; done
+ *
+ */
+static const uint8_t test_vector[] = {0x61, 0x62, 0x63};
+
+static const uint8_t test_result_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_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 const uint8_t test_result_md5[] = {
+    0x90, 0x01, 0x50, 0x98, 0x3c, 0xd2, 0x4f, 0xb0, 0xd6, 0x96, 0x3f, 0x7d,
+    0x28, 0xe1, 0x7f, 0x72};
+
+
+static void write_regs(QTestState *s, uint32_t base, uint32_t src,
+                       uint32_t length, uint32_t out, uint32_t method)
+{
+        qtest_writel(s, base + HACE_HASH_SRC, src);
+        qtest_writel(s, base + HACE_HASH_DIGEST, out);
+        qtest_writel(s, base + HACE_HASH_DATA_LEN, length);
+        qtest_writel(s, base + HACE_HASH_CMD, HACE_SHA_BE_EN | method);
+}
+
+static void test_md5(const char *machine, const uint32_t base,
+                     const uint32_t src_addr)
+
+{
+    QTestState *s = qtest_init(machine);
+
+    uint32_t digest_addr = src_addr + 0x01000000;
+    uint8_t digest[16] = {0};
+
+    /* 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, src_addr, test_vector, sizeof(test_vector));
+
+    write_regs(s, base, src_addr, sizeof(test_vector), digest_addr, HACE_ALGO_MD5);
+
+    /* Check hash IRQ status is asserted */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
+
+    /* Clear IRQ status and check status is deasserted */
+    qtest_writel(s, base + HACE_STS, 0x00000200);
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Read computed digest from memory */
+    qtest_memread(s, digest_addr, digest, sizeof(digest));
+
+    /* Check result of computation */
+    g_assert_cmpmem(digest, sizeof(digest),
+                    test_result_md5, sizeof(digest));
+}
+
+static void test_sha256(const char *machine, const uint32_t base,
+                        const uint32_t src_addr)
+{
+    QTestState *s = qtest_init(machine);
+
+    const uint32_t digest_addr = src_addr + 0x1000000;
+    uint8_t digest[32] = {0};
+
+    /* 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, src_addr, test_vector, sizeof(test_vector));
+
+    write_regs(s, base, src_addr, sizeof(test_vector), digest_addr, HACE_ALGO_SHA256);
+
+    /* Check hash IRQ status is asserted */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
+
+    /* Clear IRQ status and check status is deasserted */
+    qtest_writel(s, base + HACE_STS, 0x00000200);
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Read computed digest from memory */
+    qtest_memread(s, digest_addr, digest, sizeof(digest));
+
+    /* Check result of computation */
+    g_assert_cmpmem(digest, sizeof(digest),
+                    test_result_sha256, sizeof(digest));
+}
+
+static void test_sha512(const char *machine, const uint32_t base,
+                        const uint32_t src_addr)
+{
+    QTestState *s = qtest_init(machine);
+
+    const uint32_t digest_addr = src_addr + 0x1000000;
+    uint8_t digest[64] = {0};
+
+    /* 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, src_addr, test_vector, sizeof(test_vector));
+
+    write_regs(s, base, src_addr, sizeof(test_vector), digest_addr, HACE_ALGO_SHA512);
+
+    /* Check hash IRQ status is asserted */
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
+
+    /* Clear IRQ status and check status is deasserted */
+    qtest_writel(s, base + HACE_STS, 0x00000200);
+    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
+
+    /* Read computed digest from memory */
+    qtest_memread(s, digest_addr, digest, sizeof(digest));
+
+    /* Check result of computation */
+    g_assert_cmpmem(digest, sizeof(digest),
+                    test_result_sha512, sizeof(digest));
+}
+
+struct masks {
+    uint32_t src;
+    uint32_t dest;
+    uint32_t len;
+};
+
+static const struct masks ast2600_masks = {
+    .src  = 0x7fffffff,
+    .dest = 0x7ffffff8,
+    .len  = 0x0fffffff,
+};
+
+static const struct masks ast2500_masks = {
+    .src  = 0x3fffffff,
+    .dest = 0x3ffffff8,
+    .len  = 0x0fffffff,
+};
+
+static const struct masks ast2400_masks = {
+    .src  = 0x0fffffff,
+    .dest = 0x0ffffff8,
+    .len  = 0x0fffffff,
+};
+
+static void test_addresses(const char *machine, const uint32_t base,
+                           const struct masks *expected)
+{
+    QTestState *s = qtest_init(machine);
+
+    /*
+     * Check command mode is zero, meaning engine is in direct access mode,
+     * as this affects the masking behavior of the HASH_SRC register.
+     */
+    g_assert_cmphex(qtest_readl(s, base + HACE_CMD), ==, 0);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC), ==, 0);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST), ==, 0);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==, 0);
+
+
+    /* Check that the address masking is correct */
+    qtest_writel(s, base + HACE_HASH_SRC, 0xffffffff);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC), ==, expected->src);
+
+    qtest_writel(s, base + HACE_HASH_DIGEST, 0xffffffff);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST), ==, expected->dest);
+
+    qtest_writel(s, base + HACE_HASH_DATA_LEN, 0xffffffff);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==, expected->len);
+
+    /* Reset to zero */
+    qtest_writel(s, base + HACE_HASH_SRC, 0);
+    qtest_writel(s, base + HACE_HASH_DIGEST, 0);
+    qtest_writel(s, base + HACE_HASH_DATA_LEN, 0);
+
+    /* Check that all bits are now zero */
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC), ==, 0);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST), ==, 0);
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==, 0);
+}
+
+/* ast2600 */
+static void test_md5_ast2600(void)
+{
+    test_md5("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
+}
+
+static void test_sha256_ast2600(void)
+{
+    test_sha256("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
+}
+
+static void test_sha512_ast2600(void)
+{
+    test_sha512("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
+}
+
+static void test_addresses_ast2600(void)
+{
+    test_addresses("-machine ast2600-evb", 0x1e6d0000, &ast2600_masks);
+}
+
+/* ast2500 */
+static void test_md5_ast2500(void)
+{
+    test_md5("-machine ast2500-evb", 0x1e6e3000, 0x80000000);
+}
+
+static void test_sha256_ast2500(void)
+{
+    test_sha256("-machine ast2500-evb", 0x1e6e3000, 0x80000000);
+}
+
+static void test_sha512_ast2500(void)
+{
+    test_sha512("-machine ast2500-evb", 0x1e6e3000, 0x80000000);
+}
+
+static void test_addresses_ast2500(void)
+{
+    test_addresses("-machine ast2500-evb", 0x1e6e3000, &ast2500_masks);
+}
+
+/* ast2400 */
+static void test_md5_ast2400(void)
+{
+    test_md5("-machine palmetto-bmc", 0x1e6e3000, 0x40000000);
+}
+
+static void test_sha256_ast2400(void)
+{
+    test_sha256("-machine palmetto-bmc", 0x1e6e3000, 0x40000000);
+}
+
+static void test_sha512_ast2400(void)
+{
+    test_sha512("-machine palmetto-bmc", 0x1e6e3000, 0x40000000);
+}
+
+static void test_addresses_ast2400(void)
+{
+    test_addresses("-machine palmetto-bmc", 0x1e6e3000, &ast2400_masks);
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+
+    qtest_add_func("ast2600/hace/addresses", test_addresses_ast2600);
+    qtest_add_func("ast2600/hace/sha512", test_sha512_ast2600);
+    qtest_add_func("ast2600/hace/sha256", test_sha256_ast2600);
+    qtest_add_func("ast2600/hace/md5", test_md5_ast2600);
+
+    qtest_add_func("ast2500/hace/addresses", test_addresses_ast2500);
+    qtest_add_func("ast2500/hace/sha512", test_sha512_ast2500);
+    qtest_add_func("ast2500/hace/sha256", test_sha256_ast2500);
+    qtest_add_func("ast2500/hace/md5", test_md5_ast2500);
+
+    qtest_add_func("ast2400/hace/addresses", test_addresses_ast2400);
+    qtest_add_func("ast2400/hace/sha512", test_sha512_ast2400);
+    qtest_add_func("ast2400/hace/sha256", test_sha256_ast2400);
+    qtest_add_func("ast2400/hace/md5", test_md5_ast2400);
+
+    return g_test_run();
+}
diff --git a/MAINTAINERS b/MAINTAINERS
index 9147e9a429a0..62f44456d809 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1026,6 +1026,7 @@ F: include/hw/misc/pca9552*.h
 F: hw/net/ftgmac100.c
 F: include/hw/net/ftgmac100.h
 F: docs/system/arm/aspeed.rst
+F: tests/qtest/*aspeed*
 
 NRF51
 M: Joel Stanley <joel@jms.id.au>
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 902cfef7cb2f..84b3219c15c6 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -163,12 +163,15 @@ qtests_npcm7xx = \
    'npcm7xx_timer-test',
    'npcm7xx_watchdog_timer-test'] + \
    (slirp.found() ? ['npcm7xx_emc-test'] : [])
+qtests_aspeed = \
+  ['aspeed_hace-test']
 qtests_arm = \
   (config_all_devices.has_key('CONFIG_MPS2') ? ['sse-timer-test'] : []) + \
   (config_all_devices.has_key('CONFIG_CMSDK_APB_DUALTIMER') ? ['cmsdk-apb-dualtimer-test'] : []) + \
   (config_all_devices.has_key('CONFIG_CMSDK_APB_TIMER') ? ['cmsdk-apb-timer-test'] : []) + \
   (config_all_devices.has_key('CONFIG_CMSDK_APB_WATCHDOG') ? ['cmsdk-apb-watchdog-test'] : []) + \
   (config_all_devices.has_key('CONFIG_PFLASH_CFI02') ? ['pflash-cfi02-test'] : []) +         \
+  (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed : []) + \
   (config_all_devices.has_key('CONFIG_NPCM7XX') ? qtests_npcm7xx : []) + \
   ['arm-cpu-features',
    'microbit-test',
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 3/3] tests/qtest: Add test for Aspeed HACE
  2021-03-24  7:09 ` [PATCH v4 3/3] tests/qtest: Add test for Aspeed HACE Joel Stanley
@ 2021-03-24  7:21   ` Cédric Le Goater
  2021-03-24  9:05     ` Thomas Huth
  0 siblings, 1 reply; 11+ messages in thread
From: Cédric Le Goater @ 2021-03-24  7:21 UTC (permalink / raw)
  To: Joel Stanley, Philippe Mathieu-Daudé
  Cc: Andrew Jeffery, Peter Maydell, Thomas Huth, qemu-arm, qemu-devel

On 3/24/21 8:09 AM, Joel Stanley wrote:
> This adds a test for the Aspeed Hash and Crypto (HACE) engine. It tests
> the currently implemented behavior of the hash functionality.
> 
> The tests are similar, but are cut/pasted instead of broken out into a
> common function so the assert machinery produces useful output when a
> test fails.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thomas, 

Can we keep your Acked-by ?

Thanks,

C.

> ---
> v3: Write test without libqtest-single.h
> v4: Run tests on all aspeed machines
> ---
>  tests/qtest/aspeed_hace-test.c | 313 +++++++++++++++++++++++++++++++++
>  MAINTAINERS                    |   1 +
>  tests/qtest/meson.build        |   3 +
>  3 files changed, 317 insertions(+)
>  create mode 100644 tests/qtest/aspeed_hace-test.c
> 
> diff --git a/tests/qtest/aspeed_hace-test.c b/tests/qtest/aspeed_hace-test.c
> new file mode 100644
> index 000000000000..2b624b6b099b
> --- /dev/null
> +++ b/tests/qtest/aspeed_hace-test.c
> @@ -0,0 +1,313 @@
> +/*
> + * QTest testcase for the ASPEED Hash and Crypto Engine
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright 2021 IBM Corp.
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "libqos/libqtest.h"
> +#include "qemu-common.h"
> +#include "qemu/bitops.h"
> +
> +#define HACE_CMD                 0x10
> +#define  HACE_SHA_BE_EN          BIT(3)
> +#define  HACE_MD5_LE_EN          BIT(2)
> +#define  HACE_ALGO_MD5           0
> +#define  HACE_ALGO_SHA1          BIT(5)
> +#define  HACE_ALGO_SHA224        BIT(6)
> +#define  HACE_ALGO_SHA256        (BIT(4) | BIT(6))
> +#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_STS                 0x1c
> +#define  HACE_RSA_ISR            BIT(13)
> +#define  HACE_CRYPTO_ISR         BIT(12)
> +#define  HACE_HASH_ISR           BIT(9)
> +#define  HACE_RSA_BUSY           BIT(2)
> +#define  HACE_CRYPTO_BUSY        BIT(1)
> +#define  HACE_HASH_BUSY          BIT(0)
> +#define HACE_HASH_SRC            0x20
> +#define HACE_HASH_DIGEST         0x24
> +#define HACE_HASH_KEY_BUFF       0x28
> +#define HACE_HASH_DATA_LEN       0x2c
> +#define HACE_HASH_CMD            0x30
> +
> +/*
> + * Test vector is the ascii "abc"
> + *
> + * Expected results were generated using command line utitiles:
> + *
> + *  echo -n -e 'abc' | dd of=/tmp/test
> + *  for hash in sha512sum sha256sum md5sum; do $hash /tmp/test; done
> + *
> + */
> +static const uint8_t test_vector[] = {0x61, 0x62, 0x63};
> +
> +static const uint8_t test_result_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_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 const uint8_t test_result_md5[] = {
> +    0x90, 0x01, 0x50, 0x98, 0x3c, 0xd2, 0x4f, 0xb0, 0xd6, 0x96, 0x3f, 0x7d,
> +    0x28, 0xe1, 0x7f, 0x72};
> +
> +
> +static void write_regs(QTestState *s, uint32_t base, uint32_t src,
> +                       uint32_t length, uint32_t out, uint32_t method)
> +{
> +        qtest_writel(s, base + HACE_HASH_SRC, src);
> +        qtest_writel(s, base + HACE_HASH_DIGEST, out);
> +        qtest_writel(s, base + HACE_HASH_DATA_LEN, length);
> +        qtest_writel(s, base + HACE_HASH_CMD, HACE_SHA_BE_EN | method);
> +}
> +
> +static void test_md5(const char *machine, const uint32_t base,
> +                     const uint32_t src_addr)
> +
> +{
> +    QTestState *s = qtest_init(machine);
> +
> +    uint32_t digest_addr = src_addr + 0x01000000;
> +    uint8_t digest[16] = {0};
> +
> +    /* 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, src_addr, test_vector, sizeof(test_vector));
> +
> +    write_regs(s, base, src_addr, sizeof(test_vector), digest_addr, HACE_ALGO_MD5);
> +
> +    /* Check hash IRQ status is asserted */
> +    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
> +
> +    /* Clear IRQ status and check status is deasserted */
> +    qtest_writel(s, base + HACE_STS, 0x00000200);
> +    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
> +
> +    /* Read computed digest from memory */
> +    qtest_memread(s, digest_addr, digest, sizeof(digest));
> +
> +    /* Check result of computation */
> +    g_assert_cmpmem(digest, sizeof(digest),
> +                    test_result_md5, sizeof(digest));
> +}
> +
> +static void test_sha256(const char *machine, const uint32_t base,
> +                        const uint32_t src_addr)
> +{
> +    QTestState *s = qtest_init(machine);
> +
> +    const uint32_t digest_addr = src_addr + 0x1000000;
> +    uint8_t digest[32] = {0};
> +
> +    /* 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, src_addr, test_vector, sizeof(test_vector));
> +
> +    write_regs(s, base, src_addr, sizeof(test_vector), digest_addr, HACE_ALGO_SHA256);
> +
> +    /* Check hash IRQ status is asserted */
> +    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
> +
> +    /* Clear IRQ status and check status is deasserted */
> +    qtest_writel(s, base + HACE_STS, 0x00000200);
> +    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
> +
> +    /* Read computed digest from memory */
> +    qtest_memread(s, digest_addr, digest, sizeof(digest));
> +
> +    /* Check result of computation */
> +    g_assert_cmpmem(digest, sizeof(digest),
> +                    test_result_sha256, sizeof(digest));
> +}
> +
> +static void test_sha512(const char *machine, const uint32_t base,
> +                        const uint32_t src_addr)
> +{
> +    QTestState *s = qtest_init(machine);
> +
> +    const uint32_t digest_addr = src_addr + 0x1000000;
> +    uint8_t digest[64] = {0};
> +
> +    /* 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, src_addr, test_vector, sizeof(test_vector));
> +
> +    write_regs(s, base, src_addr, sizeof(test_vector), digest_addr, HACE_ALGO_SHA512);
> +
> +    /* Check hash IRQ status is asserted */
> +    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0x00000200);
> +
> +    /* Clear IRQ status and check status is deasserted */
> +    qtest_writel(s, base + HACE_STS, 0x00000200);
> +    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
> +
> +    /* Read computed digest from memory */
> +    qtest_memread(s, digest_addr, digest, sizeof(digest));
> +
> +    /* Check result of computation */
> +    g_assert_cmpmem(digest, sizeof(digest),
> +                    test_result_sha512, sizeof(digest));
> +}
> +
> +struct masks {
> +    uint32_t src;
> +    uint32_t dest;
> +    uint32_t len;
> +};
> +
> +static const struct masks ast2600_masks = {
> +    .src  = 0x7fffffff,
> +    .dest = 0x7ffffff8,
> +    .len  = 0x0fffffff,
> +};
> +
> +static const struct masks ast2500_masks = {
> +    .src  = 0x3fffffff,
> +    .dest = 0x3ffffff8,
> +    .len  = 0x0fffffff,
> +};
> +
> +static const struct masks ast2400_masks = {
> +    .src  = 0x0fffffff,
> +    .dest = 0x0ffffff8,
> +    .len  = 0x0fffffff,
> +};
> +
> +static void test_addresses(const char *machine, const uint32_t base,
> +                           const struct masks *expected)
> +{
> +    QTestState *s = qtest_init(machine);
> +
> +    /*
> +     * Check command mode is zero, meaning engine is in direct access mode,
> +     * as this affects the masking behavior of the HASH_SRC register.
> +     */
> +    g_assert_cmphex(qtest_readl(s, base + HACE_CMD), ==, 0);
> +    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC), ==, 0);
> +    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST), ==, 0);
> +    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==, 0);
> +
> +
> +    /* Check that the address masking is correct */
> +    qtest_writel(s, base + HACE_HASH_SRC, 0xffffffff);
> +    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC), ==, expected->src);
> +
> +    qtest_writel(s, base + HACE_HASH_DIGEST, 0xffffffff);
> +    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST), ==, expected->dest);
> +
> +    qtest_writel(s, base + HACE_HASH_DATA_LEN, 0xffffffff);
> +    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==, expected->len);
> +
> +    /* Reset to zero */
> +    qtest_writel(s, base + HACE_HASH_SRC, 0);
> +    qtest_writel(s, base + HACE_HASH_DIGEST, 0);
> +    qtest_writel(s, base + HACE_HASH_DATA_LEN, 0);
> +
> +    /* Check that all bits are now zero */
> +    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_SRC), ==, 0);
> +    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST), ==, 0);
> +    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==, 0);
> +}
> +
> +/* ast2600 */
> +static void test_md5_ast2600(void)
> +{
> +    test_md5("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
> +}
> +
> +static void test_sha256_ast2600(void)
> +{
> +    test_sha256("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
> +}
> +
> +static void test_sha512_ast2600(void)
> +{
> +    test_sha512("-machine ast2600-evb", 0x1e6d0000, 0x80000000);
> +}
> +
> +static void test_addresses_ast2600(void)
> +{
> +    test_addresses("-machine ast2600-evb", 0x1e6d0000, &ast2600_masks);
> +}
> +
> +/* ast2500 */
> +static void test_md5_ast2500(void)
> +{
> +    test_md5("-machine ast2500-evb", 0x1e6e3000, 0x80000000);
> +}
> +
> +static void test_sha256_ast2500(void)
> +{
> +    test_sha256("-machine ast2500-evb", 0x1e6e3000, 0x80000000);
> +}
> +
> +static void test_sha512_ast2500(void)
> +{
> +    test_sha512("-machine ast2500-evb", 0x1e6e3000, 0x80000000);
> +}
> +
> +static void test_addresses_ast2500(void)
> +{
> +    test_addresses("-machine ast2500-evb", 0x1e6e3000, &ast2500_masks);
> +}
> +
> +/* ast2400 */
> +static void test_md5_ast2400(void)
> +{
> +    test_md5("-machine palmetto-bmc", 0x1e6e3000, 0x40000000);
> +}
> +
> +static void test_sha256_ast2400(void)
> +{
> +    test_sha256("-machine palmetto-bmc", 0x1e6e3000, 0x40000000);
> +}
> +
> +static void test_sha512_ast2400(void)
> +{
> +    test_sha512("-machine palmetto-bmc", 0x1e6e3000, 0x40000000);
> +}
> +
> +static void test_addresses_ast2400(void)
> +{
> +    test_addresses("-machine palmetto-bmc", 0x1e6e3000, &ast2400_masks);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    g_test_init(&argc, &argv, NULL);
> +
> +    qtest_add_func("ast2600/hace/addresses", test_addresses_ast2600);
> +    qtest_add_func("ast2600/hace/sha512", test_sha512_ast2600);
> +    qtest_add_func("ast2600/hace/sha256", test_sha256_ast2600);
> +    qtest_add_func("ast2600/hace/md5", test_md5_ast2600);
> +
> +    qtest_add_func("ast2500/hace/addresses", test_addresses_ast2500);
> +    qtest_add_func("ast2500/hace/sha512", test_sha512_ast2500);
> +    qtest_add_func("ast2500/hace/sha256", test_sha256_ast2500);
> +    qtest_add_func("ast2500/hace/md5", test_md5_ast2500);
> +
> +    qtest_add_func("ast2400/hace/addresses", test_addresses_ast2400);
> +    qtest_add_func("ast2400/hace/sha512", test_sha512_ast2400);
> +    qtest_add_func("ast2400/hace/sha256", test_sha256_ast2400);
> +    qtest_add_func("ast2400/hace/md5", test_md5_ast2400);
> +
> +    return g_test_run();
> +}
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9147e9a429a0..62f44456d809 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1026,6 +1026,7 @@ F: include/hw/misc/pca9552*.h
>  F: hw/net/ftgmac100.c
>  F: include/hw/net/ftgmac100.h
>  F: docs/system/arm/aspeed.rst
> +F: tests/qtest/*aspeed*
>  
>  NRF51
>  M: Joel Stanley <joel@jms.id.au>
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 902cfef7cb2f..84b3219c15c6 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -163,12 +163,15 @@ qtests_npcm7xx = \
>     'npcm7xx_timer-test',
>     'npcm7xx_watchdog_timer-test'] + \
>     (slirp.found() ? ['npcm7xx_emc-test'] : [])
> +qtests_aspeed = \
> +  ['aspeed_hace-test']
>  qtests_arm = \
>    (config_all_devices.has_key('CONFIG_MPS2') ? ['sse-timer-test'] : []) + \
>    (config_all_devices.has_key('CONFIG_CMSDK_APB_DUALTIMER') ? ['cmsdk-apb-dualtimer-test'] : []) + \
>    (config_all_devices.has_key('CONFIG_CMSDK_APB_TIMER') ? ['cmsdk-apb-timer-test'] : []) + \
>    (config_all_devices.has_key('CONFIG_CMSDK_APB_WATCHDOG') ? ['cmsdk-apb-watchdog-test'] : []) + \
>    (config_all_devices.has_key('CONFIG_PFLASH_CFI02') ? ['pflash-cfi02-test'] : []) +         \
> +  (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed : []) + \
>    (config_all_devices.has_key('CONFIG_NPCM7XX') ? qtests_npcm7xx : []) + \
>    ['arm-cpu-features',
>     'microbit-test',
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/3] aspeed: Integrate HACE
  2021-03-24  7:09 ` [PATCH v4 2/3] aspeed: Integrate HACE Joel Stanley
@ 2021-03-24  7:21   ` Cédric Le Goater
  2021-03-24  9:47   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 11+ messages in thread
From: Cédric Le Goater @ 2021-03-24  7:21 UTC (permalink / raw)
  To: Joel Stanley, Philippe Mathieu-Daudé
  Cc: Andrew Jeffery, Peter Maydell, qemu-arm, qemu-devel

On 3/24/21 8:09 AM, Joel Stanley wrote:
> Add the hash and crypto engine model to the Aspeed socs.
> 
> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> Signed-off-by: Joel Stanley <joel@jms.id.au>

Reviewed-by: Cédric Le Goater <clg@kaod.org>


> ---
> v3: Rebase on upstream
> v4: Update integration for soc-specific hace objects
> ---
>  docs/system/arm/aspeed.rst  |  2 +-
>  include/hw/arm/aspeed_soc.h |  3 +++
>  hw/arm/aspeed_ast2600.c     | 15 +++++++++++++++
>  hw/arm/aspeed_soc.c         | 16 ++++++++++++++++
>  4 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
> index d1fb8f25b39c..f9466e6d8245 100644
> --- a/docs/system/arm/aspeed.rst
> +++ b/docs/system/arm/aspeed.rst
> @@ -49,6 +49,7 @@ Supported devices
>   * Ethernet controllers
>   * Front LEDs (PCA9552 on I2C bus)
>   * LPC Peripheral Controller (a subset of subdevices are supported)
> + * Hash/Crypto Engine (HACE) - Hash support only, no scatter-gather
>  
>  
>  Missing devices
> @@ -59,7 +60,6 @@ Missing devices
>   * PWM and Fan Controller
>   * Slave GPIO Controller
>   * Super I/O Controller
> - * Hash/Crypto Engine
>   * PCI-Express 1 Controller
>   * Graphic Display Controller
>   * PECI Controller
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 9359d6da336d..d9161d26d645 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -21,6 +21,7 @@
>  #include "hw/rtc/aspeed_rtc.h"
>  #include "hw/i2c/aspeed_i2c.h"
>  #include "hw/ssi/aspeed_smc.h"
> +#include "hw/misc/aspeed_hace.h"
>  #include "hw/watchdog/wdt_aspeed.h"
>  #include "hw/net/ftgmac100.h"
>  #include "target/arm/cpu.h"
> @@ -50,6 +51,7 @@ struct AspeedSoCState {
>      AspeedTimerCtrlState timerctrl;
>      AspeedI2CState i2c;
>      AspeedSCUState scu;
> +    AspeedHACEState hace;
>      AspeedXDMAState xdma;
>      AspeedSMCState fmc;
>      AspeedSMCState spi[ASPEED_SPIS_NUM];
> @@ -133,6 +135,7 @@ enum {
>      ASPEED_DEV_XDMA,
>      ASPEED_DEV_EMMC,
>      ASPEED_DEV_KCS,
> +    ASPEED_DEV_HACE,
>  };
>  
>  #endif /* ASPEED_SOC_H */
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index bc87e754a3cc..c149936e0b28 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -42,6 +42,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
>      [ASPEED_DEV_ETH2]      = 0x1E680000,
>      [ASPEED_DEV_ETH4]      = 0x1E690000,
>      [ASPEED_DEV_VIC]       = 0x1E6C0000,
> +    [ASPEED_DEV_HACE]      = 0x1E6D0000,
>      [ASPEED_DEV_SDMC]      = 0x1E6E0000,
>      [ASPEED_DEV_SCU]       = 0x1E6E2000,
>      [ASPEED_DEV_XDMA]      = 0x1E6E7000,
> @@ -102,6 +103,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
>      [ASPEED_DEV_I2C]       = 110,   /* 110 -> 125 */
>      [ASPEED_DEV_ETH1]      = 2,
>      [ASPEED_DEV_ETH2]      = 3,
> +    [ASPEED_DEV_HACE]      = 4,
>      [ASPEED_DEV_ETH3]      = 32,
>      [ASPEED_DEV_ETH4]      = 33,
>      [ASPEED_DEV_KCS]       = 138,   /* 138 -> 142 */
> @@ -213,6 +215,9 @@ static void aspeed_soc_ast2600_init(Object *obj)
>                              TYPE_SYSBUS_SDHCI);
>  
>      object_initialize_child(obj, "lpc", &s->lpc, TYPE_ASPEED_LPC);
> +
> +    snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
> +    object_initialize_child(obj, "hace", &s->hace, typename);
>  }
>  
>  /*
> @@ -498,6 +503,16 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_4,
>                         qdev_get_gpio_in(DEVICE(&s->a7mpcore),
>                                  sc->irqmap[ASPEED_DEV_KCS] + aspeed_lpc_kcs_4));
> +
> +    /* 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;
> +    }
> +    sysbus_mmio_map(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));
>  }
>  
>  static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index 057d053c8478..c8c3bd233233 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -34,6 +34,7 @@ static const hwaddr aspeed_soc_ast2400_memmap[] = {
>      [ASPEED_DEV_VIC]    = 0x1E6C0000,
>      [ASPEED_DEV_SDMC]   = 0x1E6E0000,
>      [ASPEED_DEV_SCU]    = 0x1E6E2000,
> +    [ASPEED_DEV_HACE]   = 0x1E6E3000,
>      [ASPEED_DEV_XDMA]   = 0x1E6E7000,
>      [ASPEED_DEV_VIDEO]  = 0x1E700000,
>      [ASPEED_DEV_ADC]    = 0x1E6E9000,
> @@ -65,6 +66,7 @@ static const hwaddr aspeed_soc_ast2500_memmap[] = {
>      [ASPEED_DEV_VIC]    = 0x1E6C0000,
>      [ASPEED_DEV_SDMC]   = 0x1E6E0000,
>      [ASPEED_DEV_SCU]    = 0x1E6E2000,
> +    [ASPEED_DEV_HACE]   = 0x1E6E3000,
>      [ASPEED_DEV_XDMA]   = 0x1E6E7000,
>      [ASPEED_DEV_ADC]    = 0x1E6E9000,
>      [ASPEED_DEV_VIDEO]  = 0x1E700000,
> @@ -117,6 +119,7 @@ static const int aspeed_soc_ast2400_irqmap[] = {
>      [ASPEED_DEV_ETH2]   = 3,
>      [ASPEED_DEV_XDMA]   = 6,
>      [ASPEED_DEV_SDHCI]  = 26,
> +    [ASPEED_DEV_HACE]   = 4,
>  };
>  
>  #define aspeed_soc_ast2500_irqmap aspeed_soc_ast2400_irqmap
> @@ -212,6 +215,9 @@ static void aspeed_soc_init(Object *obj)
>      }
>  
>      object_initialize_child(obj, "lpc", &s->lpc, TYPE_ASPEED_LPC);
> +
> +    snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
> +    object_initialize_child(obj, "hace", &s->hace, typename);
>  }
>  
>  static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> @@ -425,6 +431,16 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>  
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_4,
>                         qdev_get_gpio_in(DEVICE(&s->lpc), aspeed_lpc_kcs_4));
> +
> +    /* 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;
> +    }
> +    sysbus_mmio_map(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));
>  }
>  static Property aspeed_soc_properties[] = {
>      DEFINE_PROP_LINK("dram", AspeedSoCState, dram_mr, TYPE_MEMORY_REGION,
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/3] hw: Model ASPEED's Hash and Crypto Engine
  2021-03-24  7:09 ` [PATCH v4 1/3] hw: Model ASPEED's Hash and Crypto Engine Joel Stanley
@ 2021-03-24  7:26   ` Cédric Le Goater
  2021-03-24  9:46   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 11+ messages in thread
From: Cédric Le Goater @ 2021-03-24  7:26 UTC (permalink / raw)
  To: Joel Stanley, Philippe Mathieu-Daudé
  Cc: Andrew Jeffery, Peter Maydell, qemu-arm, qemu-devel

On 3/24/21 8:09 AM, Joel Stanley wrote:
> The HACE (Hash and Crypto Engine) is a device that offloads MD5, SHA1,
> SHA2, RSA and other cryptographic algorithms.
> 
> This initial model implements a subset of the device's functionality;
> currently only direct access (non-scatter gather) hashing.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
> v3:
>  - rebase on upstream to fix meson.build conflict
> v2:
>  - reorder register defines
>  - mask src/dest/len registers according to hardware
> v4:
>  - Fix typos in comments
>  - Remove sdram base address; new memory region fixes mean this is not
>    required
>  - Use PRIx64
>  - Add Object Classes for soc familiy specific features
>  - Convert big switch statement to a lookup in a struct
> ---
>  include/hw/misc/aspeed_hace.h |  43 ++++
>  hw/misc/aspeed_hace.c         | 358 ++++++++++++++++++++++++++++++++++
>  hw/misc/meson.build           |   1 +
>  3 files changed, 402 insertions(+)
>  create mode 100644 include/hw/misc/aspeed_hace.h
>  create mode 100644 hw/misc/aspeed_hace.c
> 
> diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h
> new file mode 100644
> index 000000000000..94d5ada95fa2
> --- /dev/null
> +++ b/include/hw/misc/aspeed_hace.h
> @@ -0,0 +1,43 @@
> +/*
> + * ASPEED Hash and Crypto Engine
> + *
> + * Copyright (C) 2021 IBM Corp.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef ASPEED_HACE_H
> +#define ASPEED_HACE_H
> +
> +#include "hw/sysbus.h"
> +
> +#define TYPE_ASPEED_HACE "aspeed.hace"
> +#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"
> +OBJECT_DECLARE_TYPE(AspeedHACEState, AspeedHACEClass, ASPEED_HACE)
> +
> +#define ASPEED_HACE_NR_REGS (0x64 >> 2)
> +
> +struct AspeedHACEState {
> +    SysBusDevice parent;
> +
> +    MemoryRegion iomem;
> +    qemu_irq irq;
> +
> +    uint32_t regs[ASPEED_HACE_NR_REGS];
> +
> +    MemoryRegion *dram_mr;
> +    AddressSpace dram_as;
> +};
> +
> +
> +struct AspeedHACEClass {
> +    SysBusDeviceClass parent_class;
> +
> +    uint32_t src_mask;
> +    uint32_t dest_mask;
> +    uint32_t hash_mask;
> +};
> +
> +#endif /* _ASPEED_HACE_H_ */
> diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
> new file mode 100644
> index 000000000000..a962ccea90e8
> --- /dev/null
> +++ b/hw/misc/aspeed_hace.c
> @@ -0,0 +1,358 @@
> +/*
> + * ASPEED Hash and Crypto Engine
> + *
> + * Copyright (C) 2021 IBM Corp.
> + *
> + * Joel Stanley <joel@jms.id.au>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qemu/error-report.h"
> +#include "hw/misc/aspeed_hace.h"
> +#include "qapi/error.h"
> +#include "migration/vmstate.h"
> +#include "crypto/hash.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/irq.h"
> +
> +#define R_CRYPT_CMD     (0x10 / 4)
> +
> +#define R_STATUS        (0x1c / 4)
> +#define HASH_IRQ        BIT(9)
> +#define CRYPT_IRQ       BIT(12)
> +#define TAG_IRQ         BIT(15)
> +
> +#define R_HASH_SRC      (0x20 / 4)
> +#define R_HASH_DEST     (0x24 / 4)
> +#define R_HASH_SRC_LEN  (0x2c / 4)
> +
> +#define R_HASH_CMD      (0x30 / 4)
> +/* Hash algorithm selection */
> +#define  HASH_ALGO_MASK                 (BIT(4) | BIT(5) | BIT(6))
> +#define  HASH_ALGO_MD5                  0
> +#define  HASH_ALGO_SHA1                 BIT(5)
> +#define  HASH_ALGO_SHA224               BIT(6)
> +#define  HASH_ALGO_SHA256               (BIT(4) | BIT(6))
> +#define  HASH_ALGO_SHA512_SERIES        (BIT(5) | BIT(6))
> +/* SHA512 algorithm selection */
> +#define  SHA512_HASH_ALGO_MASK          (BIT(10) | BIT(11) | BIT(12))
> +#define  HASH_ALGO_SHA512_SHA512        0
> +#define  HASH_ALGO_SHA512_SHA384        BIT(10)
> +#define  HASH_ALGO_SHA512_SHA256        BIT(11)
> +#define  HASH_ALGO_SHA512_SHA224        (BIT(10) | BIT(11))
> +/* HMAC modes */
> +#define  HASH_HMAC_MASK                 (BIT(7) | BIT(8))
> +#define  HASH_DIGEST                    0
> +#define  HASH_DIGEST_HMAC               BIT(7)
> +#define  HASH_DIGEST_ACCUM              BIT(8)
> +#define  HASH_HMAC_KEY                  (BIT(7) | BIT(8))
> +/* Cascaded operation modes */
> +#define  HASH_ONLY                      0
> +#define  HASH_ONLY2                     BIT(0)
> +#define  HASH_CRYPT_THEN_HASH           BIT(1)
> +#define  HASH_HASH_THEN_CRYPT           (BIT(0) | BIT(1))
> +/* Other cmd bits */
> +#define  HASH_IRQ_EN                    BIT(9)
> +#define  HASH_SG_EN                     BIT(18)
> +
> +static const struct {
> +    uint32_t mask;
> +    QCryptoHashAlgorithm algo;
> +} hash_algo_map[] = {
> +    { HASH_ALGO_MD5, QCRYPTO_HASH_ALG_MD5 },
> +    { HASH_ALGO_SHA1, QCRYPTO_HASH_ALG_SHA1 },
> +    { HASH_ALGO_SHA224, QCRYPTO_HASH_ALG_SHA224 },
> +    { HASH_ALGO_SHA256, QCRYPTO_HASH_ALG_SHA256 },
> +    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA512, QCRYPTO_HASH_ALG_SHA512 },
> +    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA384, QCRYPTO_HASH_ALG_SHA384 },
> +    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA256, QCRYPTO_HASH_ALG_SHA256 },
> +};
> +
> +static int hash_algo_lookup(uint32_t mask)
> +{
> +    int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(hash_algo_map); i++) {
> +        if (mask == hash_algo_map[i].mask)
> +            return hash_algo_map[i].algo;
> +    }
> +
> +    return -1;
> +}
> +
> +static int do_hash_operation(AspeedHACEState *s, int algo)
> +{
> +    hwaddr src, len, dest;
> +    uint8_t *digest_buf = NULL;
> +    size_t digest_len = 0;
> +    char *src_buf;
> +    int rc;
> +
> +    src = s->regs[R_HASH_SRC];
> +    len = s->regs[R_HASH_SRC_LEN];
> +    dest = s->regs[R_HASH_DEST];
> +
> +    src_buf = address_space_map(&s->dram_as, src, &len, false,
> +                                MEMTXATTRS_UNSPECIFIED);
> +    if (!src_buf) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map dram\n", __func__);
> +        return -EACCES;
> +    }
> +
> +    rc = qcrypto_hash_bytes(algo, src_buf, len, &digest_buf, &digest_len,
> +                            &error_fatal);
> +    if (rc < 0) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto failed\n", __func__);
> +        return rc;
> +    }
> +
> +    rc = address_space_write(&s->dram_as, dest, MEMTXATTRS_UNSPECIFIED,
> +                             digest_buf, digest_len);
> +    if (rc) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: address space write failed\n", __func__);
> +    }
> +    g_free(digest_buf);
> +
> +    address_space_unmap(&s->dram_as, src_buf, len, false, len);
> +
> +    /*
> +     * Set status bits to indicate completion. Testing shows hardware sets
> +     * these irrespective of HASH_IRQ_EN.
> +     */
> +    s->regs[R_STATUS] |= HASH_IRQ;
> +
> +    return 0;
> +}
> +
> +
> +static uint64_t aspeed_hace_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> +    AspeedHACEState *s = ASPEED_HACE(opaque);
> +
> +    addr >>= 2;
> +
> +    if (addr >= ASPEED_HACE_NR_REGS) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Out-of-bounds read at offset 0x%" HWADDR_PRIx "\n",
> +                      __func__, addr << 2);
> +        return 0;
> +    }
> +
> +    return s->regs[addr];
> +}
> +
> +static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
> +                              unsigned int size)
> +{
> +    AspeedHACEState *s = ASPEED_HACE(opaque);
> +    AspeedHACEClass *ahc = ASPEED_HACE_GET_CLASS(s);
> +
> +    addr >>= 2;
> +
> +    if (addr >= ASPEED_HACE_NR_REGS) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Out-of-bounds write at offset 0x%" HWADDR_PRIx "\n",
> +                      __func__, addr << 2);
> +        return;
> +    }
> +
> +    switch (addr) {
> +    case R_STATUS:
> +        if (data & HASH_IRQ) {
> +            data &= ~HASH_IRQ;
> +
> +            if (s->regs[addr] & HASH_IRQ) {
> +                qemu_irq_lower(s->irq);
> +            }
> +        }
> +        break;
> +    case R_HASH_SRC:
> +        data &= ahc->src_mask;
> +        break;
> +    case R_HASH_DEST:
> +        data &= ahc->dest_mask;
> +        break;
> +    case R_HASH_SRC_LEN:
> +        data &= 0x0FFFFFFF;
> +        break;
> +    case R_HASH_CMD: {
> +        int algo = -1;
> +        if ((data & HASH_HMAC_MASK)) {
> +            qemu_log_mask(LOG_UNIMP,
> +                          "%s: HMAC engine command mode %"PRIx64" not implemented",
> +                          __func__, (data & HASH_HMAC_MASK) >> 8);
> +        }
> +        if (data & HASH_SG_EN) {
> +            qemu_log_mask(LOG_UNIMP,
> +                          "%s: Hash scatter gather mode not implemented",
> +                          __func__);
> +        }
> +        if (data & BIT(1)) {
> +            qemu_log_mask(LOG_UNIMP,
> +                          "%s: Cascaded mode not implemented",
> +                          __func__);
> +        }
> +        algo = hash_algo_lookup(data & ahc->hash_mask);
> +        if (algo < 0) {
> +                qemu_log_mask(LOG_GUEST_ERROR,
> +                        "%s: Invalid hash algorithm selection 0x%"PRIx64"\n",
> +                        __func__, data & ahc->hash_mask);
> +                break;
> +        }
> +        do_hash_operation(s, algo);
> +
> +        if (data & HASH_IRQ_EN) {
> +            qemu_irq_raise(s->irq);
> +        }
> +        break;
> +    }
> +    case R_CRYPT_CMD:
> +        qemu_log_mask(LOG_UNIMP, "%s: Crypt commands not implemented\n",
> +                       __func__);
> +        break;
> +    default:
> +        break;
> +    }
> +
> +    s->regs[addr] = data;
> +}
> +
> +static const MemoryRegionOps aspeed_hace_ops = {
> +    .read = aspeed_hace_read,
> +    .write = aspeed_hace_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 1,
> +        .max_access_size = 4,
> +    },
> +};
> +
> +static void aspeed_hace_reset(DeviceState *dev)
> +{
> +    struct AspeedHACEState *s = ASPEED_HACE(dev);
> +
> +    memset(s->regs, 0, sizeof(s->regs));
> +}
> +
> +static void aspeed_hace_realize(DeviceState *dev, Error **errp)
> +{
> +    AspeedHACEState *s = ASPEED_HACE(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +
> +    sysbus_init_irq(sbd, &s->irq);
> +
> +    memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_hace_ops, s,
> +            TYPE_ASPEED_HACE, 0x1000);
> +
> +    if (!s->dram_mr) {
> +        error_setg(errp, TYPE_ASPEED_HACE ": 'dram' link not set");
> +        return;
> +    }
> +
> +    address_space_init(&s->dram_as, s->dram_mr, "dram");
> +
> +    sysbus_init_mmio(sbd, &s->iomem);
> +}
> +
> +static Property aspeed_hace_properties[] = {
> +    DEFINE_PROP_LINK("dram", AspeedHACEState, dram_mr,
> +                     TYPE_MEMORY_REGION, MemoryRegion *),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +
> +static const VMStateDescription vmstate_aspeed_hace = {
> +    .name = TYPE_ASPEED_HACE,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32_ARRAY(regs, AspeedHACEState, ASPEED_HACE_NR_REGS),
> +        VMSTATE_END_OF_LIST(),
> +    }
> +};
> +
> +static void aspeed_hace_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = aspeed_hace_realize;
> +    dc->reset = aspeed_hace_reset;
> +    device_class_set_props(dc, aspeed_hace_properties);
> +    dc->vmsd = &vmstate_aspeed_hace;
> +}
> +
> +static const TypeInfo aspeed_hace_info = {
> +    .name = TYPE_ASPEED_HACE,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(AspeedHACEState),
> +    .class_init = aspeed_hace_class_init,
> +    .class_size = sizeof(AspeedHACEClass)
> +};
> +
> +static void aspeed_ast2400_hace_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    AspeedHACEClass *ahc = ASPEED_HACE_CLASS(klass);
> +
> +    dc->desc = "AST2400 Hash and Crypto Engine";
> +
> +    ahc->src_mask = 0x0FFFFFFF;
> +    ahc->dest_mask = 0x0FFFFFF8;
> +    ahc->hash_mask = HASH_ALGO_MASK;
> +}
> +
> +static const TypeInfo aspeed_ast2400_hace_info = {
> +    .name = TYPE_ASPEED_AST2400_HACE,
> +    .parent = TYPE_ASPEED_HACE,
> +    .class_init = aspeed_ast2400_hace_class_init,
> +};
> +
> +static void aspeed_ast2500_hace_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    AspeedHACEClass *ahc = ASPEED_HACE_CLASS(klass);
> +
> +    dc->desc = "AST2500 Hash and Crypto Engine";
> +
> +    ahc->src_mask = 0x3fffffff;
> +    ahc->dest_mask = 0x3ffffff8;
> +    ahc->hash_mask = HASH_ALGO_MASK;
> +}
> +
> +static const TypeInfo aspeed_ast2500_hace_info = {
> +    .name = TYPE_ASPEED_AST2500_HACE,
> +    .parent = TYPE_ASPEED_HACE,
> +    .class_init = aspeed_ast2500_hace_class_init,
> +};
> +
> +static void aspeed_ast2600_hace_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    AspeedHACEClass *ahc = ASPEED_HACE_CLASS(klass);
> +
> +    dc->desc = "AST2600 Hash and Crypto Engine";
> +
> +    ahc->src_mask = 0x7FFFFFFF;
> +    ahc->dest_mask = 0x7FFFFFF8;
> +    ahc->hash_mask = HASH_ALGO_MASK | SHA512_HASH_ALGO_MASK;
> +}
> +
> +static const TypeInfo aspeed_ast2600_hace_info = {
> +    .name = TYPE_ASPEED_AST2600_HACE,
> +    .parent = TYPE_ASPEED_HACE,
> +    .class_init = aspeed_ast2600_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_hace_info);
> +}
> +
> +type_init(aspeed_hace_register_types);
> diff --git a/hw/misc/meson.build b/hw/misc/meson.build
> index 21034dc60a81..1e7b8b064bd1 100644
> --- a/hw/misc/meson.build
> +++ b/hw/misc/meson.build
> @@ -109,6 +109,7 @@ softmmu_ss.add(when: 'CONFIG_PVPANIC_ISA', if_true: files('pvpanic-isa.c'))
>  softmmu_ss.add(when: 'CONFIG_PVPANIC_PCI', if_true: files('pvpanic-pci.c'))
>  softmmu_ss.add(when: 'CONFIG_AUX', if_true: files('auxbus.c'))
>  softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files(
> +  'aspeed_hace.c',
>    'aspeed_lpc.c',
>    'aspeed_scu.c',
>    'aspeed_sdmc.c',
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 0/3] hw/misc: Model ASPEED hash and crypto engine
  2021-03-24  7:09 [PATCH v4 0/3] hw/misc: Model ASPEED hash and crypto engine Joel Stanley
                   ` (2 preceding siblings ...)
  2021-03-24  7:09 ` [PATCH v4 3/3] tests/qtest: Add test for Aspeed HACE Joel Stanley
@ 2021-03-24  7:30 ` no-reply
  3 siblings, 0 replies; 11+ messages in thread
From: no-reply @ 2021-03-24  7:30 UTC (permalink / raw)
  To: joel; +Cc: peter.maydell, andrew, qemu-devel, f4bug, qemu-arm, clg

Patchew URL: https://patchew.org/QEMU/20210324070955.125941-1-joel@jms.id.au/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210324070955.125941-1-joel@jms.id.au
Subject: [PATCH v4 0/3] hw/misc: Model ASPEED hash and crypto engine

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210324070955.125941-1-joel@jms.id.au -> patchew/20210324070955.125941-1-joel@jms.id.au
Switched to a new branch 'test'
5ebc2c5 tests/qtest: Add test for Aspeed HACE
6fc1946 aspeed: Integrate HACE
fa3bbae hw: Model ASPEED's Hash and Crypto Engine

=== OUTPUT BEGIN ===
1/3 Checking commit fa3bbae71dd5 (hw: Model ASPEED's Hash and Crypto Engine)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#22: 
new file mode 100644

WARNING: line over 80 characters
#95: FILE: hw/misc/aspeed_hace.c:69:
+    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA512, QCRYPTO_HASH_ALG_SHA512 },

WARNING: line over 80 characters
#96: FILE: hw/misc/aspeed_hace.c:70:
+    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA384, QCRYPTO_HASH_ALG_SHA384 },

WARNING: line over 80 characters
#97: FILE: hw/misc/aspeed_hace.c:71:
+    { HASH_ALGO_SHA512_SERIES | HASH_ALGO_SHA512_SHA256, QCRYPTO_HASH_ALG_SHA256 },

ERROR: braces {} are necessary for all arms of this statement
#105: FILE: hw/misc/aspeed_hace.c:79:
+        if (mask == hash_algo_map[i].mask)
[...]

WARNING: line over 80 characters
#212: FILE: hw/misc/aspeed_hace.c:186:
+                          "%s: HMAC engine command mode %"PRIx64" not implemented",

total: 1 errors, 5 warnings, 408 lines checked

Patch 1/3 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/3 Checking commit 6fc194601e1b (aspeed: Integrate HACE)
3/3 Checking commit 5ebc2c5642da (tests/qtest: Add test for Aspeed HACE)
WARNING: line over 80 characters
#130: FILE: tests/qtest/aspeed_hace-test.c:91:
+    write_regs(s, base, src_addr, sizeof(test_vector), digest_addr, HACE_ALGO_MD5);

WARNING: line over 80 characters
#161: FILE: tests/qtest/aspeed_hace-test.c:122:
+    write_regs(s, base, src_addr, sizeof(test_vector), digest_addr, HACE_ALGO_SHA256);

WARNING: line over 80 characters
#192: FILE: tests/qtest/aspeed_hace-test.c:153:
+    write_regs(s, base, src_addr, sizeof(test_vector), digest_addr, HACE_ALGO_SHA512);

WARNING: line over 80 characters
#253: FILE: tests/qtest/aspeed_hace-test.c:214:
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DIGEST), ==, expected->dest);

WARNING: line over 80 characters
#256: FILE: tests/qtest/aspeed_hace-test.c:217:
+    g_assert_cmphex(qtest_readl(s, base + HACE_HASH_DATA_LEN), ==, expected->len);

total: 0 errors, 5 warnings, 335 lines checked

Patch 3/3 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210324070955.125941-1-joel@jms.id.au/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 3/3] tests/qtest: Add test for Aspeed HACE
  2021-03-24  7:21   ` Cédric Le Goater
@ 2021-03-24  9:05     ` Thomas Huth
  0 siblings, 0 replies; 11+ messages in thread
From: Thomas Huth @ 2021-03-24  9:05 UTC (permalink / raw)
  To: Cédric Le Goater, Joel Stanley, Philippe Mathieu-Daudé
  Cc: Andrew Jeffery, Peter Maydell, qemu-arm, qemu-devel

On 24/03/2021 08.21, Cédric Le Goater wrote:
> On 3/24/21 8:09 AM, Joel Stanley wrote:
>> This adds a test for the Aspeed Hash and Crypto (HACE) engine. It tests
>> the currently implemented behavior of the hash functionality.
>>
>> The tests are similar, but are cut/pasted instead of broken out into a
>> common function so the assert machinery produces useful output when a
>> test fails.
>>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> 
> Thomas,
> 
> Can we keep your Acked-by ?

Yes:

Acked-by: Thomas Huth <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/3] hw: Model ASPEED's Hash and Crypto Engine
  2021-03-24  7:09 ` [PATCH v4 1/3] hw: Model ASPEED's Hash and Crypto Engine Joel Stanley
  2021-03-24  7:26   ` Cédric Le Goater
@ 2021-03-24  9:46   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-24  9:46 UTC (permalink / raw)
  To: Joel Stanley, Cédric Le Goater
  Cc: Andrew Jeffery, Peter Maydell, qemu-arm, qemu-devel

On 3/24/21 8:09 AM, Joel Stanley wrote:
> The HACE (Hash and Crypto Engine) is a device that offloads MD5, SHA1,
> SHA2, RSA and other cryptographic algorithms.
> 
> This initial model implements a subset of the device's functionality;
> currently only direct access (non-scatter gather) hashing.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> v3:
>  - rebase on upstream to fix meson.build conflict
> v2:
>  - reorder register defines
>  - mask src/dest/len registers according to hardware
> v4:
>  - Fix typos in comments
>  - Remove sdram base address; new memory region fixes mean this is not
>    required
>  - Use PRIx64
>  - Add Object Classes for soc familiy specific features
>  - Convert big switch statement to a lookup in a struct
> ---
>  include/hw/misc/aspeed_hace.h |  43 ++++
>  hw/misc/aspeed_hace.c         | 358 ++++++++++++++++++++++++++++++++++
>  hw/misc/meson.build           |   1 +
>  3 files changed, 402 insertions(+)
>  create mode 100644 include/hw/misc/aspeed_hace.h
>  create mode 100644 hw/misc/aspeed_hace.c

> +static int hash_algo_lookup(uint32_t mask)
> +{
> +    int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(hash_algo_map); i++) {
> +        if (mask == hash_algo_map[i].mask)

{

> +            return hash_algo_map[i].algo;

}

> +    }
> +
> +    return -1;
> +}
> +
> +static int do_hash_operation(AspeedHACEState *s, int algo)
> +{
> +    hwaddr src, len, dest;
> +    uint8_t *digest_buf = NULL;

Eventually g_autofree,

> +    size_t digest_len = 0;
> +    char *src_buf;
> +    int rc;
> +
> +    src = s->regs[R_HASH_SRC];
> +    len = s->regs[R_HASH_SRC_LEN];
> +    dest = s->regs[R_HASH_DEST];
> +
> +    src_buf = address_space_map(&s->dram_as, src, &len, false,
> +                                MEMTXATTRS_UNSPECIFIED);
> +    if (!src_buf) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map dram\n", __func__);
> +        return -EACCES;
> +    }
> +
> +    rc = qcrypto_hash_bytes(algo, src_buf, len, &digest_buf, &digest_len,
> +                            &error_fatal);
> +    if (rc < 0) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto failed\n", __func__);
> +        return rc;
> +    }
> +
> +    rc = address_space_write(&s->dram_as, dest, MEMTXATTRS_UNSPECIFIED,
> +                             digest_buf, digest_len);
> +    if (rc) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: address space write failed\n", __func__);
> +    }
> +    g_free(digest_buf);

removing g_free().

> +
> +    address_space_unmap(&s->dram_as, src_buf, len, false, len);
> +
> +    /*
> +     * Set status bits to indicate completion. Testing shows hardware sets
> +     * these irrespective of HASH_IRQ_EN.
> +     */
> +    s->regs[R_STATUS] |= HASH_IRQ;
> +
> +    return 0;
> +}

Generic model LGTM.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/3] aspeed: Integrate HACE
  2021-03-24  7:09 ` [PATCH v4 2/3] aspeed: Integrate HACE Joel Stanley
  2021-03-24  7:21   ` Cédric Le Goater
@ 2021-03-24  9:47   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-24  9:47 UTC (permalink / raw)
  To: Joel Stanley, Cédric Le Goater
  Cc: Andrew Jeffery, Peter Maydell, qemu-arm, qemu-devel

On 3/24/21 8:09 AM, Joel Stanley wrote:
> Add the hash and crypto engine model to the Aspeed socs.
> 
> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> v3: Rebase on upstream
> v4: Update integration for soc-specific hace objects
> ---
>  docs/system/arm/aspeed.rst  |  2 +-
>  include/hw/arm/aspeed_soc.h |  3 +++
>  hw/arm/aspeed_ast2600.c     | 15 +++++++++++++++
>  hw/arm/aspeed_soc.c         | 16 ++++++++++++++++
>  4 files changed, 35 insertions(+), 1 deletion(-)

LGTM.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-03-24  9:49 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-24  7:09 [PATCH v4 0/3] hw/misc: Model ASPEED hash and crypto engine Joel Stanley
2021-03-24  7:09 ` [PATCH v4 1/3] hw: Model ASPEED's Hash and Crypto Engine Joel Stanley
2021-03-24  7:26   ` Cédric Le Goater
2021-03-24  9:46   ` Philippe Mathieu-Daudé
2021-03-24  7:09 ` [PATCH v4 2/3] aspeed: Integrate HACE Joel Stanley
2021-03-24  7:21   ` Cédric Le Goater
2021-03-24  9:47   ` Philippe Mathieu-Daudé
2021-03-24  7:09 ` [PATCH v4 3/3] tests/qtest: Add test for Aspeed HACE Joel Stanley
2021-03-24  7:21   ` Cédric Le Goater
2021-03-24  9:05     ` Thomas Huth
2021-03-24  7:30 ` [PATCH v4 0/3] hw/misc: Model ASPEED hash and crypto engine no-reply

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.