diff mbox series

[v3,1/3] hw: Model ASPEED's Hash and Crypto Engine

Message ID 20210312105711.551423-2-joel@jms.id.au (mailing list archive)
State New, archived
Headers show
Series hw/misc: Model ASPEED hash and crypto engine | expand

Commit Message

Joel Stanley March 12, 2021, 10:57 a.m. UTC 
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>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
v3:
 - rebase on upstream to fix meson.build conflict
v2:
 - reorder register defines
 - mask src/dest/len registers according to hardware
---
 include/hw/misc/aspeed_hace.h |  33 ++++
 hw/misc/aspeed_hace.c         | 312 ++++++++++++++++++++++++++++++++++
 hw/misc/meson.build           |   1 +
 3 files changed, 346 insertions(+)
 create mode 100644 include/hw/misc/aspeed_hace.h
 create mode 100644 hw/misc/aspeed_hace.c

Comments

Andrew Jeffery March 17, 2021, 12:02 a.m. UTC | #1 
On Fri, 12 Mar 2021, at 21:27, 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>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> v3:
>  - rebase on upstream to fix meson.build conflict
> v2:
>  - reorder register defines
>  - mask src/dest/len registers according to hardware
> ---
>  include/hw/misc/aspeed_hace.h |  33 ++++
>  hw/misc/aspeed_hace.c         | 312 ++++++++++++++++++++++++++++++++++
>  hw/misc/meson.build           |   1 +
>  3 files changed, 346 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..e1fce670ef9e
> --- /dev/null
> +++ b/include/hw/misc/aspeed_hace.h
> @@ -0,0 +1,33 @@
> +/*
> + * 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 ASPEED_HACE(obj) OBJECT_CHECK(AspeedHACEState, (obj), 
> TYPE_ASPEED_HACE)
> +
> +#define ASPEED_HACE_NR_REGS (0x64 >> 2)
> +
> +typedef struct AspeedHACEState {
> +    /* <private> */
> +    SysBusDevice parent;
> +
> +    /*< public >*/
> +    MemoryRegion iomem;
> +    qemu_irq irq;
> +
> +    uint32_t regs[ASPEED_HACE_NR_REGS];
> +
> +    MemoryRegion *dram_mr;
> +    AddressSpace dram_as;
> +} AspeedHACEState;
> +
> +#endif /* _ASPEED_HACE_H_ */
> diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
> new file mode 100644
> index 000000000000..3d02fae2dd62
> --- /dev/null
> +++ b/hw/misc/aspeed_hace.c
> @@ -0,0 +1,312 @@
> +/*
> + * 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 algorithim 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 algorithim 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))
> +/* Cascscaed 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 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 = 0x80000000 | s->regs[R_HASH_SRC];

Tricky. Also doesn't work on the AST2400 where SDRAM is based at 
0x40000000?

Other than that it looks good to me.

Andrew
Cédric Le Goater March 17, 2021, 7:24 a.m. UTC | #2 
On 3/17/21 1:02 AM, Andrew Jeffery wrote:
> 
> 
> On Fri, 12 Mar 2021, at 21:27, 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>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> v3:
>>  - rebase on upstream to fix meson.build conflict
>> v2:
>>  - reorder register defines
>>  - mask src/dest/len registers according to hardware
>> ---
>>  include/hw/misc/aspeed_hace.h |  33 ++++
>>  hw/misc/aspeed_hace.c         | 312 ++++++++++++++++++++++++++++++++++
>>  hw/misc/meson.build           |   1 +
>>  3 files changed, 346 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..e1fce670ef9e
>> --- /dev/null
>> +++ b/include/hw/misc/aspeed_hace.h
>> @@ -0,0 +1,33 @@
>> +/*
>> + * 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 ASPEED_HACE(obj) OBJECT_CHECK(AspeedHACEState, (obj), 
>> TYPE_ASPEED_HACE)
>> +
>> +#define ASPEED_HACE_NR_REGS (0x64 >> 2)
>> +
>> +typedef struct AspeedHACEState {
>> +    /* <private> */
>> +    SysBusDevice parent;
>> +
>> +    /*< public >*/
>> +    MemoryRegion iomem;
>> +    qemu_irq irq;
>> +
>> +    uint32_t regs[ASPEED_HACE_NR_REGS];
>> +
>> +    MemoryRegion *dram_mr;
>> +    AddressSpace dram_as;
>> +} AspeedHACEState;
>> +
>> +#endif /* _ASPEED_HACE_H_ */
>> diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
>> new file mode 100644
>> index 000000000000..3d02fae2dd62
>> --- /dev/null
>> +++ b/hw/misc/aspeed_hace.c
>> @@ -0,0 +1,312 @@
>> +/*
>> + * 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 algorithim 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 algorithim 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))
>> +/* Cascscaed 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 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 = 0x80000000 | s->regs[R_HASH_SRC];
> 
> Tricky. Also doesn't work on the AST2400 where SDRAM is based at 
> 0x40000000?

We need a "sdram-base" property. See the Aspeed SMC model for DMAs, 
it's very similar.

Thanks,

C. 


> 
> Other than that it looks good to me.
> 
> Andrew
>
Cédric Le Goater March 17, 2021, 8:13 a.m. UTC | #3 
On 3/12/21 11:57 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>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> v3:
>  - rebase on upstream to fix meson.build conflict
> v2:
>  - reorder register defines
>  - mask src/dest/len registers according to hardware
> ---
>  include/hw/misc/aspeed_hace.h |  33 ++++
>  hw/misc/aspeed_hace.c         | 312 ++++++++++++++++++++++++++++++++++
>  hw/misc/meson.build           |   1 +
>  3 files changed, 346 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..e1fce670ef9e
> --- /dev/null
> +++ b/include/hw/misc/aspeed_hace.h
> @@ -0,0 +1,33 @@
> +/*
> + * 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 ASPEED_HACE(obj) OBJECT_CHECK(AspeedHACEState, (obj), TYPE_ASPEED_HACE)
> +
> +#define ASPEED_HACE_NR_REGS (0x64 >> 2)
> +
> +typedef struct AspeedHACEState {
> +    /* <private> */
> +    SysBusDevice parent;
> +
> +    /*< public >*/
> +    MemoryRegion iomem;
> +    qemu_irq irq;
> +
> +    uint32_t regs[ASPEED_HACE_NR_REGS];
> +
> +    MemoryRegion *dram_mr;
> +    AddressSpace dram_as;
> +} AspeedHACEState;
> +
> +#endif /* _ASPEED_HACE_H_ */
> diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
> new file mode 100644
> index 000000000000..3d02fae2dd62
> --- /dev/null
> +++ b/hw/misc/aspeed_hace.c
> @@ -0,0 +1,312 @@
> +/*
> + * 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 algorithim selection */

algorithm

> +#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 algorithim selection */

same

> +#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))
> +/* Cascscaed operation modes */

Cascaded.

> +#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 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 = 0x80000000 | s->regs[R_HASH_SRC];
> +    len = s->regs[R_HASH_SRC_LEN];
> +    dest = 0x80000000 | s->regs[R_HASH_DEST];

The model needs a "sdram-base" property. See aspeed-smc.

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

Instead of mapping the machine memory, we could allocate a buffer 
to load the data with address_space_ldl_le routines. I think this
would be safe for endianess, or we don't care ? 

Also, HACE30 has byte swapping control bits in 3:2. How do they work ? 


> +    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);
> +
> +    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 &= 0x7FFFFFFF;
> +        break;
> +    case R_HASH_DEST:
> +        data &= 0x7FFFFFF8;
> +        break;

These mask values depend on the SoC revision since the SDRAM address space
has a different size. I would add an Object Class for these values.

> +    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 %ld not implemented",

%ld will surely raise a compile error on windows.

> +                          __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__);
> +        }
> +        switch (data & HASH_ALGO_MASK) {
> +        case HASH_ALGO_MD5:
> +            algo = QCRYPTO_HASH_ALG_MD5;
> +            break;
> +        case HASH_ALGO_SHA1:
> +            algo = QCRYPTO_HASH_ALG_SHA1;
> +            break;
> +        case HASH_ALGO_SHA224:
> +            algo = QCRYPTO_HASH_ALG_SHA224;
> +            break;
> +        case HASH_ALGO_SHA256:
> +            algo = QCRYPTO_HASH_ALG_SHA256;
> +            break;
> +        case HASH_ALGO_SHA512_SERIES:
> +            switch (data & SHA512_HASH_ALGO_MASK) {
> +            case HASH_ALGO_SHA512_SHA512:
> +                algo = QCRYPTO_HASH_ALG_SHA512;
> +                break;
> +            case HASH_ALGO_SHA512_SHA384:
> +                algo = QCRYPTO_HASH_ALG_SHA384;
> +                break;
> +            case HASH_ALGO_SHA512_SHA256:
> +                algo = QCRYPTO_HASH_ALG_SHA256;
> +                break;
> +            case HASH_ALGO_SHA512_SHA224:
> +                algo = QCRYPTO_HASH_ALG_SHA224;
> +                break;

Couldn't we use an array of 

   struct { 
	uint64_t mask; 
	int	 algo;
   };

and a lookup routine ? 

> +            default:
> +                qemu_log_mask(LOG_GUEST_ERROR,
> +                        "%s: Invalid sha512 hash algorithm selection 0x%03lx\n",
> +                        __func__, data & SHA512_HASH_ALGO_MASK);

PRIx64 should be used.

> +                break;
> +            }
> +            break;
> +        default:
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Invalid hash algorithm selection 0x%03lx\n",

also here.

> +                      __func__, data & HASH_ALGO_MASK);
> +            break;
> +        }
> +        if (algo >= 0) {
> +            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->desc = "Aspeed Hash and Crypto Engine",
> +    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,
> +};
> +
> +static void aspeed_hace_register_types(void)
> +{
> +    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 00356cf12ec7..23b61d55f62e 100644
> --- a/hw/misc/meson.build
> +++ b/hw/misc/meson.build
> @@ -104,6 +104,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',
>
Philippe Mathieu-Daudé March 17, 2021, 11:47 a.m. UTC | #4 
On 3/12/21 11:57 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>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> v3:
>  - rebase on upstream to fix meson.build conflict
> v2:
>  - reorder register defines
>  - mask src/dest/len registers according to hardware
> ---
>  include/hw/misc/aspeed_hace.h |  33 ++++
>  hw/misc/aspeed_hace.c         | 312 ++++++++++++++++++++++++++++++++++
>  hw/misc/meson.build           |   1 +
>  3 files changed, 346 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..e1fce670ef9e
> --- /dev/null
> +++ b/include/hw/misc/aspeed_hace.h
> @@ -0,0 +1,33 @@
> +/*
> + * 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 ASPEED_HACE(obj) OBJECT_CHECK(AspeedHACEState, (obj), TYPE_ASPEED_HACE)
> +
> +#define ASPEED_HACE_NR_REGS (0x64 >> 2)
> +
> +typedef struct AspeedHACEState {
> +    /* <private> */
> +    SysBusDevice parent;
> +
> +    /*< public >*/
> +    MemoryRegion iomem;
> +    qemu_irq irq;
> +
> +    uint32_t regs[ASPEED_HACE_NR_REGS];
> +
> +    MemoryRegion *dram_mr;
> +    AddressSpace dram_as;
> +} AspeedHACEState;
> +
> +#endif /* _ASPEED_HACE_H_ */
> diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
> new file mode 100644
> index 000000000000..3d02fae2dd62
> --- /dev/null
> +++ b/hw/misc/aspeed_hace.c
> @@ -0,0 +1,312 @@
> +/*
> + * 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 algorithim 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 algorithim 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))
> +/* Cascscaed 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 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 = 0x80000000 | s->regs[R_HASH_SRC];
> +    len = s->regs[R_HASH_SRC_LEN];
> +    dest = 0x80000000 | s->regs[R_HASH_DEST];
> +
> +    src_buf = address_space_map(&s->dram_as, src, &len, false,
> +                                MEMTXATTRS_UNSPECIFIED);

It seems the Aspeed machines aren't using correctly the AS API...
This device shouldn't worry about where it is physically mapped.
IOW its AS is too wide.

I'm trying to fix this in a series:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg791085.html
Cédric Le Goater March 17, 2021, 12:36 p.m. UTC | #5 
On 3/17/21 12:47 PM, Philippe Mathieu-Daudé wrote:
> On 3/12/21 11:57 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>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> v3:
>>  - rebase on upstream to fix meson.build conflict
>> v2:
>>  - reorder register defines
>>  - mask src/dest/len registers according to hardware
>> ---
>>  include/hw/misc/aspeed_hace.h |  33 ++++
>>  hw/misc/aspeed_hace.c         | 312 ++++++++++++++++++++++++++++++++++
>>  hw/misc/meson.build           |   1 +
>>  3 files changed, 346 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..e1fce670ef9e
>> --- /dev/null
>> +++ b/include/hw/misc/aspeed_hace.h
>> @@ -0,0 +1,33 @@
>> +/*
>> + * 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 ASPEED_HACE(obj) OBJECT_CHECK(AspeedHACEState, (obj), TYPE_ASPEED_HACE)
>> +
>> +#define ASPEED_HACE_NR_REGS (0x64 >> 2)
>> +
>> +typedef struct AspeedHACEState {
>> +    /* <private> */
>> +    SysBusDevice parent;
>> +
>> +    /*< public >*/
>> +    MemoryRegion iomem;
>> +    qemu_irq irq;
>> +
>> +    uint32_t regs[ASPEED_HACE_NR_REGS];
>> +
>> +    MemoryRegion *dram_mr;
>> +    AddressSpace dram_as;
>> +} AspeedHACEState;
>> +
>> +#endif /* _ASPEED_HACE_H_ */
>> diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
>> new file mode 100644
>> index 000000000000..3d02fae2dd62
>> --- /dev/null
>> +++ b/hw/misc/aspeed_hace.c
>> @@ -0,0 +1,312 @@
>> +/*
>> + * 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 algorithim 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 algorithim 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))
>> +/* Cascscaed 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 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 = 0x80000000 | s->regs[R_HASH_SRC];
>> +    len = s->regs[R_HASH_SRC_LEN];
>> +    dest = 0x80000000 | s->regs[R_HASH_DEST];
>> +
>> +    src_buf = address_space_map(&s->dram_as, src, &len, false,
>> +                                MEMTXATTRS_UNSPECIFIED);
> 
> It seems the Aspeed machines aren't using correctly the AS API...
> This device shouldn't worry about where it is physically mapped.
> IOW its AS is too wide.
> 
> I'm trying to fix this in a series:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg791085.html

The buffers can be anywhere in DRAM which is mapped at 0x80000000
on the AST2600. This is correct and very similar to what we do 
in the Aspeed SMC DMA models.

I would prefer if we did loads though.

Cheers,

C.
Philippe Mathieu-Daudé March 17, 2021, 1:13 p.m. UTC | #6 
On 3/17/21 1:36 PM, Cédric Le Goater wrote:
> On 3/17/21 12:47 PM, Philippe Mathieu-Daudé wrote:
>> On 3/12/21 11:57 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>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>> v3:
>>>  - rebase on upstream to fix meson.build conflict
>>> v2:
>>>  - reorder register defines
>>>  - mask src/dest/len registers according to hardware
>>> ---
>>>  include/hw/misc/aspeed_hace.h |  33 ++++
>>>  hw/misc/aspeed_hace.c         | 312 ++++++++++++++++++++++++++++++++++
>>>  hw/misc/meson.build           |   1 +
>>>  3 files changed, 346 insertions(+)
>>>  create mode 100644 include/hw/misc/aspeed_hace.h
>>>  create mode 100644 hw/misc/aspeed_hace.c

>>> +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 = 0x80000000 | s->regs[R_HASH_SRC];
>>> +    len = s->regs[R_HASH_SRC_LEN];
>>> +    dest = 0x80000000 | s->regs[R_HASH_DEST];
>>> +
>>> +    src_buf = address_space_map(&s->dram_as, src, &len, false,
>>> +                                MEMTXATTRS_UNSPECIFIED);
>>
>> It seems the Aspeed machines aren't using correctly the AS API...
>> This device shouldn't worry about where it is physically mapped.
>> IOW its AS is too wide.
>>
>> I'm trying to fix this in a series:
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg791085.html
> 
> The buffers can be anywhere in DRAM

Exactly. This device only requires direct access to DRAM, not the
full bus. Regardless the amount of DRAM available, only the bus
aperture to the DRAM should be passed to this device (MR link),
then this device use an AS view on it, zero-based.

Your device becomes independent to where the DRAM is physically
mapped on the SoC.

> which is mapped at 0x80000000
> on the AST2600. This is correct and very similar to what we do 
> in the Aspeed SMC DMA models.
> 
> I would prefer if we did loads though.
> 
> Cheers,
> 
> C.
> 
>
Cédric Le Goater March 17, 2021, 5:03 p.m. UTC | #7 
On 3/17/21 2:13 PM, Philippe Mathieu-Daudé wrote:
> 
> 
> On 3/17/21 1:36 PM, Cédric Le Goater wrote:
>> On 3/17/21 12:47 PM, Philippe Mathieu-Daudé wrote:
>>> On 3/12/21 11:57 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>
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>> ---
>>>> v3:
>>>>  - rebase on upstream to fix meson.build conflict
>>>> v2:
>>>>  - reorder register defines
>>>>  - mask src/dest/len registers according to hardware
>>>> ---
>>>>  include/hw/misc/aspeed_hace.h |  33 ++++
>>>>  hw/misc/aspeed_hace.c         | 312 ++++++++++++++++++++++++++++++++++
>>>>  hw/misc/meson.build           |   1 +
>>>>  3 files changed, 346 insertions(+)
>>>>  create mode 100644 include/hw/misc/aspeed_hace.h
>>>>  create mode 100644 hw/misc/aspeed_hace.c
> 
>>>> +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 = 0x80000000 | s->regs[R_HASH_SRC];
>>>> +    len = s->regs[R_HASH_SRC_LEN];
>>>> +    dest = 0x80000000 | s->regs[R_HASH_DEST];
>>>> +
>>>> +    src_buf = address_space_map(&s->dram_as, src, &len, false,
>>>> +                                MEMTXATTRS_UNSPECIFIED);
>>>
>>> It seems the Aspeed machines aren't using correctly the AS API...
>>> This device shouldn't worry about where it is physically mapped.
>>> IOW its AS is too wide.
>>>
>>> I'm trying to fix this in a series:
>>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg791085.html
>>
>> The buffers can be anywhere in DRAM
> 
> Exactly. This device only requires direct access to DRAM, not the
> full bus. Regardless the amount of DRAM available, only the bus
> aperture to the DRAM should be passed to this device (MR link),
> then this device use an AS view on it, zero-based.
> 
> Your device becomes independent to where the DRAM is physically
> mapped on the SoC.

ok.

The initial problem was that the RAM was not initialized before 
the SoC was realized and we couldn't use it. things have changed
and we can now pass directly machine->ram to sub models instead of 
the ram_container. That should address your comments on the HACE 
model and on the SMC model. 

I will check what we can do for the mmio_flash address space.

C. 


> 
>> which is mapped at 0x80000000
>> on the AST2600. This is correct and very similar to what we do 
>> in the Aspeed SMC DMA models.
>>
>> I would prefer if we did loads though.
>>
>> Cheers,
>>
>> C.
>>
>>
diff mbox series

Patch

diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h
new file mode 100644
index 000000000000..e1fce670ef9e
--- /dev/null
+++ b/include/hw/misc/aspeed_hace.h
@@ -0,0 +1,33 @@ 
+/*
+ * 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 ASPEED_HACE(obj) OBJECT_CHECK(AspeedHACEState, (obj), TYPE_ASPEED_HACE)
+
+#define ASPEED_HACE_NR_REGS (0x64 >> 2)
+
+typedef struct AspeedHACEState {
+    /* <private> */
+    SysBusDevice parent;
+
+    /*< public >*/
+    MemoryRegion iomem;
+    qemu_irq irq;
+
+    uint32_t regs[ASPEED_HACE_NR_REGS];
+
+    MemoryRegion *dram_mr;
+    AddressSpace dram_as;
+} AspeedHACEState;
+
+#endif /* _ASPEED_HACE_H_ */
diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
new file mode 100644
index 000000000000..3d02fae2dd62
--- /dev/null
+++ b/hw/misc/aspeed_hace.c
@@ -0,0 +1,312 @@ 
+/*
+ * 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 algorithim 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 algorithim 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))
+/* Cascscaed 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 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 = 0x80000000 | s->regs[R_HASH_SRC];
+    len = s->regs[R_HASH_SRC_LEN];
+    dest = 0x80000000 | 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);
+
+    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 &= 0x7FFFFFFF;
+        break;
+    case R_HASH_DEST:
+        data &= 0x7FFFFFF8;
+        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 %ld 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__);
+        }
+        switch (data & HASH_ALGO_MASK) {
+        case HASH_ALGO_MD5:
+            algo = QCRYPTO_HASH_ALG_MD5;
+            break;
+        case HASH_ALGO_SHA1:
+            algo = QCRYPTO_HASH_ALG_SHA1;
+            break;
+        case HASH_ALGO_SHA224:
+            algo = QCRYPTO_HASH_ALG_SHA224;
+            break;
+        case HASH_ALGO_SHA256:
+            algo = QCRYPTO_HASH_ALG_SHA256;
+            break;
+        case HASH_ALGO_SHA512_SERIES:
+            switch (data & SHA512_HASH_ALGO_MASK) {
+            case HASH_ALGO_SHA512_SHA512:
+                algo = QCRYPTO_HASH_ALG_SHA512;
+                break;
+            case HASH_ALGO_SHA512_SHA384:
+                algo = QCRYPTO_HASH_ALG_SHA384;
+                break;
+            case HASH_ALGO_SHA512_SHA256:
+                algo = QCRYPTO_HASH_ALG_SHA256;
+                break;
+            case HASH_ALGO_SHA512_SHA224:
+                algo = QCRYPTO_HASH_ALG_SHA224;
+                break;
+            default:
+                qemu_log_mask(LOG_GUEST_ERROR,
+                        "%s: Invalid sha512 hash algorithm selection 0x%03lx\n",
+                        __func__, data & SHA512_HASH_ALGO_MASK);
+                break;
+            }
+            break;
+        default:
+            qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Invalid hash algorithm selection 0x%03lx\n",
+                      __func__, data & HASH_ALGO_MASK);
+            break;
+        }
+        if (algo >= 0) {
+            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->desc = "Aspeed Hash and Crypto Engine",
+    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,
+};
+
+static void aspeed_hace_register_types(void)
+{
+    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 00356cf12ec7..23b61d55f62e 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -104,6 +104,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',