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

[Cédric Le Goater]

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.