[U-Boot] [PATCH][v2] crypto/fsl - Add progressive hashing support using hardware acceleration.

Simon Glass sjg at chromium.org
Fri Feb 6 06:45:08 CET 2015


Hi,

On 5 February 2015 at 07:29, Gaurav Rana <gaurav.rana at freescale.com> wrote:
> Currently only normal hashing is supported using hardware acceleration.
> Added support for progressinve hashing using h/w.
>
> Signed-off-by: Ruchika Gupta <ruchika.gupta at freescale.com>
> Signed-off-by: Gaurav Rana <gaurav.rana at freescale.com>
> CC: Simon Glass <sjg at chromium.org>
> ---
> Changes in v2:
> Merge to common functions for SHA1 and SHA256.
> Incorporate comments.

Normally it is a good idea to list the comments you addressed.

This patch looks good to me - I just have some nits below.

>
>  README                        |   4 ++
>  common/hash.c                 |  10 +++
>  drivers/crypto/fsl/fsl_hash.c | 137 +++++++++++++++++++++++++++++++++++++++++-
>  drivers/crypto/fsl/fsl_hash.h |  33 ++++++++++
>  include/fsl_sec.h             |  26 ++++++++
>  include/hw_sha.h              |  39 ++++++++++++
>  6 files changed, 248 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/crypto/fsl/fsl_hash.h
>
> diff --git a/README b/README
> index cac7978..4c72baa 100644
> --- a/README
> +++ b/README
> @@ -3151,6 +3151,10 @@ CBFS (Coreboot Filesystem) support
>
>                 CONFIG_SHA1 - support SHA1 hashing
>                 CONFIG_SHA256 - support SHA256 hashing
> +               CONFIG_SHA_HW_ACCEL - support SHA1 and SHA256 hashing using
> +               hardware acceleration
> +               CONFIG_SHA_PROG_HW_ACCEL - support SHA1 and SHA256 progressive
> +               hashing using hw acceleration

Can these go in Kconfig?

>
>                 Note: There is also a sha1sum command, which should perhaps
>                 be deprecated in favour of 'hash sha1'.
> diff --git a/common/hash.c b/common/hash.c
> index d154d02..9e9f84b 100644
> --- a/common/hash.c
> +++ b/common/hash.c
> @@ -127,11 +127,21 @@ static struct hash_algo hash_algo[] = {
>                 SHA1_SUM_LEN,
>                 hw_sha1,
>                 CHUNKSZ_SHA1,
> +#ifdef CONFIG_SHA_PROG_HW_ACCEL
> +               hw_sha_init,
> +               hw_sha_update,
> +               hw_sha_finish,
> +#endif
>         }, {
>                 "sha256",
>                 SHA256_SUM_LEN,
>                 hw_sha256,
>                 CHUNKSZ_SHA256,
> +#ifdef CONFIG_SHA_PROG_HW_ACCEL
> +               hw_sha_init,
> +               hw_sha_update,
> +               hw_sha_finish,
> +#endif
>         },
>  #endif
>  #ifdef CONFIG_SHA1
> diff --git a/drivers/crypto/fsl/fsl_hash.c b/drivers/crypto/fsl/fsl_hash.c
> index d77f257..65c35d7 100644
> --- a/drivers/crypto/fsl/fsl_hash.c
> +++ b/drivers/crypto/fsl/fsl_hash.c
> @@ -10,6 +10,8 @@
>  #include "jobdesc.h"
>  #include "desc.h"
>  #include "jr.h"
> +#include "fsl_hash.h"
> +#include <hw_sha.h>
>
>  #define CRYPTO_MAX_ALG_NAME    80
>  #define SHA1_DIGEST_SIZE        20
> @@ -39,6 +41,113 @@ static struct caam_hash_template driver_hash[] = {
>         },
>  };
>
> +/* Create the context for progressive hashing using h/w acceleration.
> + *
> + * @ctxp: Pointer to the pointer of the context for hashing
> + * @caam_algo: Enum for SHA1 or SHA256
> + * @return 0 if ok, -ENOMEM on error
> + */
> +static int caam_hash_init(void **ctxp, enum caam_hash_algos caam_algo)
> +{
> +       *ctxp = calloc(1, sizeof(struct sha_ctx));
> +       if (*ctxp == NULL) {
> +               debug("Cannot allocate memory for context\n");
> +               return -ENOMEM;
> +       }
> +       return 0;
> +}
> +
> +/*
> + * Update sg table for progressive hashing using h/w acceleration
> + *
> + * The context is freed by this function if an error occurs.
> + *
> + * @hash_ctx: Pointer to the context for hashing
> + * @buf: Pointer to the buffer being hashed
> + * @size: Size of the buffer being hashed
> + * @is_last: 1 if this is the last update; 0 otherwise
> + * @caam_algo: Enum for SHA1 or SHA256
> + * @return 0 if ok, -EINVAL on error
> + */
> +static int caam_hash_update(void *hash_ctx, const void *buf,
> +                           unsigned int size, int is_last,
> +                           enum caam_hash_algos caam_algo)
> +{
> +       uint32_t final = 0;
> +       dma_addr_t addr = virt_to_phys((void *)buf);
> +       struct sha_ctx *ctx = hash_ctx;
> +
> +       if (ctx->sg_num >= MAX_SG) {
> +               free(ctx);
> +               return -EINVAL;
> +       }
> +
> +#ifdef CONFIG_PHYS_64BIT
> +       ctx->sg_tbl[ctx->sg_num].addr_hi = addr >> 32;
> +#else
> +       ctx->sg_tbl[ctx->sg_num].addr_hi = 0x0;
> +#endif

To clarify my comment on this. If we are on a 32-bit machine, will not
addr >> 32 be zero anyway? If so, can we avoid this #ifdef and just
have the first line? Or does the type change?

> +       ctx->sg_tbl[ctx->sg_num].addr_lo = addr;
> +
> +       sec_out32(&ctx->sg_tbl[ctx->sg_num].len_flag,
> +                 (size & SG_ENTRY_LENGTH_MASK));
> +
> +       ctx->sg_num++;
> +
> +       if (is_last) {
> +               final = sec_in32(&ctx->sg_tbl[ctx->sg_num - 1].len_flag) |
> +                       SG_ENTRY_FINAL_BIT;
> +               sec_out32(&ctx->sg_tbl[ctx->sg_num - 1].len_flag, final);
> +       }
> +
> +       return 0;
> +}
> +
> +/*
> + * Perform progressive hashing on the given buffer and copy hash at
> + * destination buffer
> + *
> + * The context is freed after completion of hash operation.
> + *
> + * @hash_ctx: Pointer to the context for hashing
> + * @dest_buf: Pointer to the destination buffer where hash is to be copied
> + * @size: Size of the buffer being hashed
> + * @caam_algo: Enum for SHA1 or SHA256
> + * @return 0 if ok, -EINVAL on error
> + */
> +static int caam_hash_finish(void *hash_ctx, void *dest_buf,
> +                           int size, enum caam_hash_algos caam_algo)
> +{
> +       uint32_t len = 0;
> +       struct sha_ctx *ctx = hash_ctx;
> +       int i = 0, ret = 0;
> +
> +       if (size < driver_hash[caam_algo].digestsize) {
> +               free(ctx);
> +               return -EINVAL;
> +       }
> +
> +       for (i = 0; i < ctx->sg_num; i++)
> +               len += (sec_in32(&ctx->sg_tbl[i].len_flag) &
> +                       SG_ENTRY_LENGTH_MASK);
> +
> +       inline_cnstr_jobdesc_hash(ctx->sha_desc, (uint8_t *)ctx->sg_tbl, len,
> +                                 ctx->hash,
> +                                 driver_hash[caam_algo].alg_type,
> +                                 driver_hash[caam_algo].digestsize,
> +                                 1);
> +
> +       ret = run_descriptor_jr(ctx->sha_desc);
> +
> +       if (ret)
> +               debug("Error %x\n", ret);
> +       else
> +               memcpy(dest_buf, ctx->hash, sizeof(ctx->hash));
> +
> +       free(ctx);
> +       return ret;
> +}
> +
>  int caam_hash(const unsigned char *pbuf, unsigned int buf_len,
>               unsigned char *pout, enum caam_hash_algos algo)
>  {
> @@ -48,7 +157,7 @@ int caam_hash(const unsigned char *pbuf, unsigned int buf_len,
>         desc = malloc(sizeof(int) * MAX_CAAM_DESCSIZE);
>         if (!desc) {
>                 debug("Not enough memory for descriptor allocation\n");
> -               return -1;
> +               return -ENOMEM;
>         }
>
>         inline_cnstr_jobdesc_hash(desc, pbuf, buf_len, pout,
> @@ -75,3 +184,29 @@ void hw_sha1(const unsigned char *pbuf, unsigned int buf_len,
>         if (caam_hash(pbuf, buf_len, pout, SHA1))
>                 printf("CAAM was not setup properly or it is faulty\n");
>  }
> +
> +int hw_sha_init(struct hash_algo *algo, void **ctxp)
> +{
> +       if (!strcmp(algo->name, driver_hash[SHA1].name))
> +               return caam_hash_init(ctxp, SHA1);
> +       else
> +               return caam_hash_init(ctxp, SHA256);

Ick, please try to avoid duplicating code. You could have

static enum caam_hash_algos algo_to_hash(struct hash_algo *algo)
{
   return ...
}

and then have in hw_sha_init():

              return caam_hash_init(ctxp, algo_to_hash(algo))

Similarly with the below:

> +}
> +
> +int hw_sha_update(struct hash_algo *algo, void *ctx, const void *buf,
> +                           unsigned int size, int is_last)
> +{
> +       if (!strcmp(algo->name, driver_hash[SHA1].name))
> +               return caam_hash_update(ctx, buf, size, is_last, SHA1);
> +       else
> +               return caam_hash_update(ctx, buf, size, is_last, SHA256);
> +}
> +
> +int hw_sha_finish(struct hash_algo *algo, void *ctx, void *dest_buf,
> +                    int size)
> +{
> +       if (!strcmp(algo->name, driver_hash[SHA1].name))
> +               return caam_hash_finish(ctx, dest_buf, size, SHA1);
> +       else
> +               return caam_hash_finish(ctx, dest_buf, size, SHA256);
> +}
> diff --git a/drivers/crypto/fsl/fsl_hash.h b/drivers/crypto/fsl/fsl_hash.h
> new file mode 100644
> index 0000000..4e840e6
> --- /dev/null
> +++ b/drivers/crypto/fsl/fsl_hash.h
> @@ -0,0 +1,33 @@
> +/*
> + * Copyright 2014 Freescale Semiconductor, Inc.
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + *
> + */
> +
> +#ifndef _SHA_H
> +#define _SHA_H
> +
> +#include <fsl_sec.h>
> +#include <hash.h>
> +#include "jr.h"
> +
> +#define MAX_SG         16

I think this is max scatter gather so you should either make this
longer and self-documenting or add a comment.

> +
> +/*
> + * Hash context contains the following fields
> + * @sha_desc: Sha Descriptor
> + * @sg_num: number of entries in sg table
> + * @len: total length of buffer
> + * @sg_tbl: sg entry table
> + * @hash: index to the hash calculated
> + */
> +struct sha_ctx {
> +       uint32_t sha_desc[64];
> +       uint32_t sg_num;
> +       uint32_t len;
> +       struct sg_entry sg_tbl[MAX_SG];
> +       u8 hash[HASH_MAX_DIGEST_SIZE];
> +};
> +
> +#endif
> diff --git a/include/fsl_sec.h b/include/fsl_sec.h
> index aa850a3..b6e6f04 100644
> --- a/include/fsl_sec.h
> +++ b/include/fsl_sec.h
> @@ -175,6 +175,32 @@ struct jr_regs {
>         u32 jrcr;
>  };
>
> +/*
> + * Scatter Gather Entry - Specifies the the Scatter Gather Format
> + * related information
> + */
> +struct sg_entry {
> +#ifdef CONFIG_SYS_FSL_SEC_LE
> +       uint32_t addr_lo;       /* Memory Address - lo */
> +       uint16_t addr_hi;       /* Memory Address of start of buffer - hi */
> +       uint16_t reserved_zero;
> +#else
> +       uint16_t reserved_zero;
> +       uint16_t addr_hi;       /* Memory Address of start of buffer - hi */
> +       uint32_t addr_lo;       /* Memory Address - lo */
> +#endif
> +
> +       uint32_t len_flag;      /* Length of the data in the frame */
> +#define SG_ENTRY_LENGTH_MASK   0x3FFFFFFF
> +#define SG_ENTRY_EXTENSION_BIT 0x80000000
> +#define SG_ENTRY_FINAL_BIT     0x40000000
> +       uint32_t bpid_offset;
> +#define SG_ENTRY_BPID_MASK     0x00FF0000
> +#define SG_ENTRY_BPID_SHIFT    16
> +#define SG_ENTRY_OFFSET_MASK   0x00001FFF
> +#define SG_ENTRY_OFFSET_SHIFT  0
> +};
> +
>  int sec_init(void);
>  #endif
>
> diff --git a/include/hw_sha.h b/include/hw_sha.h
> index 783350d..3d8213e 100644
> --- a/include/hw_sha.h
> +++ b/include/hw_sha.h
> @@ -34,4 +34,43 @@ void hw_sha256(const uchar * in_addr, uint buflen,
>   */
>  void hw_sha1(const uchar * in_addr, uint buflen,
>                         uchar * out_addr, uint chunk_size);
> +
> +/*
> + * Create the context for sha progressive hashing using h/w acceleration
> + *
> + * @algo: Pointer to the hash_algo struct
> + * @ctxp: Pointer to the pointer of the context for hashing
> + * @return 0 if ok, -ve on error
> + */
> +int hw_sha_init(struct hash_algo *algo, void **ctxp);
> +
> +/*
> + * Update buffer for sha progressive hashing using h/w acceleration
> + *
> + * The context is freed by this function if an error occurs.
> + *
> + * @algo: Pointer to the hash_algo struct
> + * @ctx: Pointer to the context for hashing
> + * @buf: Pointer to the buffer being hashed
> + * @size: Size of the buffer being hashed
> + * @is_last: 1 if this is the last update; 0 otherwise
> + * @return 0 if ok, -ve on error
> + */
> +int hw_sha_update(struct hash_algo *algo, void *ctx, const void *buf,
> +                    unsigned int size, int is_last);
> +
> +/*
> + * Copy sha hash result at destination location
> + *
> + * The context is freed after completion of hash operation or after an error.
> + *
> + * @algo: Pointer to the hash_algo struct
> + * @ctx: Pointer to the context for hashing
> + * @dest_buf: Pointer to the destination buffer where hash is to be copied
> + * @size: Size of the buffer being hashed
> + * @return 0 if ok, -ve on error
> + */
> +int hw_sha_finish(struct hash_algo *algo, void *ctx, void *dest_buf,
> +                    int size);
> +
>  #endif
> --
> 1.8.1.4
>

Regards,
Simon


More information about the U-Boot mailing list