[PATCH] crypto: Add driver for Aspeed HACE

Simon Glass sjg at chromium.org
Wed May 19 17:34:04 CEST 2021


Hi Cédric,

On Tue, 18 May 2021 at 03:02, Cédric Le Goater <clg at kaod.org> wrote:
>
> On 5/18/21 9:09 AM, Joel Stanley wrote:
> > The HACE supports MD5, SHA1 and SHA2 family hash functions. This driver
> > uses it in a polling mode to perform hash calculations over buffers
> > placed in DRAM.
> >
> > It registers a driver to allow the hardware's clocking to be enabled
> > using the clock framework.
> >
> > Co-developed-by: Klaus Heinrich Kiwi <klaus at linux.vnet.ibm.com>
> > Signed-off-by: Klaus Heinrich Kiwi <klaus at linux.vnet.ibm.com>
> > Signed-off-by: Joel Stanley <joel at jms.id.au>
> > ---
> >  configs/evb-ast2600_defconfig |   1 +
> >  drivers/crypto/Kconfig        |  19 +++
> >  drivers/crypto/Makefile       |   1 +
> >  drivers/crypto/aspeed_hace.c  | 256 ++++++++++++++++++++++++++++++++++
> >  4 files changed, 277 insertions(+)
> >  create mode 100644 drivers/crypto/aspeed_hace.c

At some point we need to fix up this interface to use its own uclass, not misc.

+Alexandru Gagniuc who is looking at something similar

> >
> > diff --git a/configs/evb-ast2600_defconfig b/configs/evb-ast2600_defconfig
> > index 91518dbe3589..52ae8e423bfd 100644
> > --- a/configs/evb-ast2600_defconfig
> > +++ b/configs/evb-ast2600_defconfig
> > @@ -45,6 +45,7 @@ CONFIG_REGMAP=y
> >  CONFIG_SPL_OF_TRANSLATE=y
> >  CONFIG_CLK=y
> >  CONFIG_SPL_CLK=y
> > +CONFIG_ASPEED_HACE=y
> >  CONFIG_DM_I2C=y
> >  CONFIG_MISC=y
> >  CONFIG_SPL_MISC=y
> > diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> > index 1ea116be7503..e92037d88906 100644
> > --- a/drivers/crypto/Kconfig
> > +++ b/drivers/crypto/Kconfig
> > @@ -2,4 +2,23 @@ menu "Hardware crypto devices"
> >
> >  source drivers/crypto/fsl/Kconfig
> >
> > +config ASPEED_HACE
> > +     bool "ASPEED Hash and Crypto Engine"
> > +     depends on ASPEED_AST2600
> > +     imply SHA_HW_ACCEL
> > +     imply SHA_PROG_HW_ACCEL
> > +     imply SHA512
> > +     imply SHA512_ALGO
> > +     imply CMD_HASH
> > +     help
> > +      Select this option to enable a driver for using the SHA engine in
> > +      the ASPEED BMC SoCs.
> > +
> > +      Enabling this allows the use of SHA operations in hardware without requiring the
> > +      SHA software implementations, saving code size.
> > +
> > +      Due to hardware limitations it cannot be used with a FIT placed in SPI
> > +      FLASH. Data can only be hashed if it is in SDRAM, making this relevant
> > +      for MMC and network boot only.
> > +
> >  endmenu
> > diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
> > index efbd1d3fca05..ac93b1295954 100644
> > --- a/drivers/crypto/Makefile
> > +++ b/drivers/crypto/Makefile
> > @@ -4,5 +4,6 @@
> >  #    http://www.samsung.com
> >
> >  obj-$(CONFIG_EXYNOS_ACE_SHA) += ace_sha.o
> > +obj-$(CONFIG_ASPEED_HACE)    += aspeed_hace.o
> >  obj-y += rsa_mod_exp/
> >  obj-y += fsl/
> > diff --git a/drivers/crypto/aspeed_hace.c b/drivers/crypto/aspeed_hace.c
> > new file mode 100644
> > index 000000000000..941c1555fe5c
> > --- /dev/null
> > +++ b/drivers/crypto/aspeed_hace.c
> > @@ -0,0 +1,256 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +//
> > +// (C) Copyright ASPEED Technology Inc.
> > +// Copyright 2021 IBM Corp.
> > +
> > +#include <common.h>
> > +#include <clk.h>
> > +
> > +#include <log.h>
> > +#include <asm/io.h>
> > +#include <malloc.h>
> > +#include <hash.h>
> > +
> > +#include <dm/device.h>
> > +#include <dm/fdtaddr.h>
> > +
> > +#include <linux/bitops.h>
> > +#include <linux/delay.h>
> > +#include <linux/kernel.h>
> > +#include <linux/iopoll.h>
> > +
> > +#define ASPEED_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 ASPEED_HACE_HASH_SRC         0x20
> > +#define ASPEED_HACE_HASH_DIGEST_BUFF 0x24
> > +#define ASPEED_HACE_HASH_KEY_BUFF    0x28
> > +#define ASPEED_HACE_HASH_DATA_LEN    0x2C
> > +#define  HACE_SG_LAST                        BIT(31)
> > +#define ASPEED_HACE_HASH_CMD         0x30
> > +#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 ASPEED_MAX_SG                        32
>
> It would nice to have a struct aspeed_hace describing the register layout.
>
> > +struct aspeed_sg {
> > +     u32 len;
> > +     u32 addr;
> > +};
> > +
> > +struct aspeed_hash_ctx {

Needs a comment...what is this struct for?

> > +     u32 method;
> > +     u32 digest_size;
> > +     u32 len;
> > +     u32 count;
> > +     struct aspeed_sg list[ASPEED_MAX_SG]; /* Must be 8 byte aligned */

comments for other struct members

> > +};
>
>
> Shouldn't we use  __aligned(8); ?
>
>
> > +struct aspeed_hace {
> > +     struct clk clk;
> > +};
> > +
> > +static phys_addr_t base;
> > +
> > +static int aspeed_hace_wait_completion(u32 reg, u32 flag, int timeout_us)
> > +{
> > +     u32 val;
> > +
> > +     return readl_poll_timeout(reg, val, (val & flag) == flag, timeout_us);
> > +}
> > +
> > +static int digest_object(const void *src, unsigned int length, void *digest, u32 method)

You should document the args and return values, including alignment constraints.

> > +{
> > +     if (!((u32)src & BIT(31))) {

Can you use ulong instead of u32, so it will build on 64-bit machines?

> > +             debug("HACE src out of bounds: can only copy from SDRAM\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     if ((u32)digest & 0x7) {
> > +             debug("HACE dest alignment incorrect: %p\n", digest);
> > +             return -EINVAL;
> > +     }
> > +
> > +     if (readl(base + ASPEED_HACE_STS) & HACE_HASH_BUSY) {
> > +             debug("HACE error: engine busy\n");
> > +             return -EBUSY;
> > +     }
> > +
> > +     /* Clear pending completion status */
> > +     writel(HACE_HASH_ISR, base + ASPEED_HACE_STS);
> > +
> > +     writel((u32)src, base + ASPEED_HACE_HASH_SRC);
> > +     writel((u32)digest, base + ASPEED_HACE_HASH_DIGEST_BUFF);
> > +     writel(length, base + ASPEED_HACE_HASH_DATA_LEN);
> > +     writel(HACE_SHA_BE_EN | method, base + ASPEED_HACE_HASH_CMD);
> > +
> > +     /* SHA512 hashing appears to have a througput of about 12MB/s */

spelling

> > +     return aspeed_hace_wait_completion(base + ASPEED_HACE_STS,
> > +                     HACE_HASH_ISR,
> > +                     1000 + (length >> 3));
> > +}
> > +
> > +void hw_sha1(const unsigned char *pbuf, unsigned int buf_len,
> > +          unsigned char *pout, unsigned int chunk_size)
> > +{
> > +     int rc;
> > +
> > +     rc = digest_object(pbuf, buf_len, pout, HACE_ALGO_SHA1);
> > +     if (rc)
> > +             debug("HACE failure: %d\n", rc);
> > +}
> > +
> > +void hw_sha256(const unsigned char *pbuf, unsigned int buf_len,
> > +            unsigned char *pout, unsigned int chunk_size)
> > +{
> > +     int rc;
> > +
> > +     rc = digest_object(pbuf, buf_len, pout, HACE_ALGO_SHA256);
> > +     if (rc)
> > +             debug("HACE failure: %d\n", rc);
> > +}
> > +
> > +void hw_sha512(const unsigned char *pbuf, unsigned int buf_len,
> > +            unsigned char *pout, unsigned int chunk_size)
> > +{
> > +     int rc;
> > +
> > +     rc = digest_object(pbuf, buf_len, pout, HACE_ALGO_SHA512);
> > +     if (rc)
> > +             debug("HACE failure: %d\n", rc);
> > +}
> > +
> > +int hw_sha_init(struct hash_algo *algo, void **ctxp)
> > +{
> > +     struct aspeed_hash_ctx *ctx;
> > +     u32 method;
> > +
> > +     if (!strcmp(algo->name, "sha1"))
> > +             method = HACE_ALGO_SHA1;
> > +     else if (!strcmp(algo->name, "sha256"))
> > +             method = HACE_ALGO_SHA256;
> > +     else if (!strcmp(algo->name, "sha512"))
> > +             method = HACE_ALGO_SHA512;
> > +     else
> > +             return -ENOTSUPP;
> > +     }
> > +
> > +     ctx = memalign(8, sizeof(*ctx));
>
> ARCH_DMA_MINALIGN ?

If you use the priv_auto thing you can use the DM_FLAG_ALLOC_PRIV_DMA flag.

>
> > +     if (!ctx) {
> > +             debug("HACE error: Cannot allocate memory for context\n");
> > +             return -ENOMEM;
> > +     }
> > +
> > +     memset(ctx, '\0', sizeof(*ctx));
> > +
> > +     if (((uintptr_t)ctx->list & 0x3) != 0) {

Drop != 0

> > +             printf("HACE error: Invalid alignment for input data\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     ctx->method = method | HACE_SG_EN;
> > +     ctx->digest_size = algo->digest_size;
> > +     *ctxp = ctx;
> > +
> > +     return 0;
> > +}
> > +
> > +int hw_sha_update(struct hash_algo *algo, void *hash_ctx, const void *buf,
> > +               unsigned int size, int is_last)
> > +{
> > +     struct aspeed_hash_ctx *ctx = hash_ctx;
> > +     struct aspeed_sg *sg = &ctx->list[ctx->count];
> > +
> > +     if (ctx->count >= ARRAY_SIZE(ctx->list)) {
> > +             debug("HACE error: Reached maximum number of hash segments\n");
> > +             free(ctx);
> > +             return -EINVAL;
> > +     }
> > +
> > +     sg->addr = (u32)buf;
> > +     sg->len = size;
> > +     if (is_last)
> > +             sg->len |= HACE_SG_LAST;
> > +
> > +     ctx->count++;
> > +     ctx->len += size;
> > +
> > +     return 0;
> > +}
> > +
> > +int hw_sha_finish(struct hash_algo *algo, void *hash_ctx, void *dest_buf, int size)
> > +{
> > +     struct aspeed_hash_ctx *ctx = hash_ctx;
> > +     int rc;
> > +
> > +     if (size < ctx->digest_size) {
> > +             debug("HACE error: insufficient size on destination buffer\n");
> > +             free(ctx);
> > +             return -EINVAL;
> > +     }
> > +
> > +     rc = digest_object(ctx->list, ctx->len, dest_buf, ctx->method);
> > +     if (rc)
> > +             debug("HACE Scatter-Gather failure\n");
> > +
> > +     free(ctx);
> > +
> > +     return rc;
> > +}
> > +
> > +static int aspeed_hace_probe(struct udevice *dev)
> > +{
> > +     struct aspeed_hace *hace = dev_get_priv(dev);
> > +     int ret;
> > +
> > +     ret = clk_get_by_index(dev, 0, &hace->clk);
> > +     if (ret < 0) {
> > +             debug("Can't get clock for %s: %d\n", dev->name, ret);
> > +             return ret;
> > +     }
> > +
> > +     ret = clk_enable(&hace->clk);
> > +     if (ret) {
> > +             debug("Failed to enable fsi clock (%d)\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     /* As the crypto code does not pass us any driver state */
> > +     base = devfdt_get_addr(dev);
> > +
> > +     return ret;
> > +}
> > +
> > +static int aspeed_hace_remove(struct udevice *dev)
> > +{
> > +     struct aspeed_hace *hace = dev_get_priv(dev);
> > +
> > +     clk_disable(&hace->clk);
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct udevice_id aspeed_hace_ids[] = {
> > +     { .compatible = "aspeed,ast2600-hace" },
> > +     { }
> > +};
> > +
> > +U_BOOT_DRIVER(aspeed_hace) = {
> > +     .name           = "aspeed_hace",
> > +     .id             = UCLASS_MISC,
> > +     .of_match       = aspeed_hace_ids,
> > +     .probe          = aspeed_hace_probe,
> > +     .remove         = aspeed_hace_remove,
> > +     .priv_auto      = sizeof(struct aspeed_hace),
> > +};
> >
>

Regards,
Simon


More information about the U-Boot mailing list