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

Hi,

On 28 January 2015 at 03:51, Gaurav Rana gaurav.rana@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@freescale.com Signed-off-by: Gaurav Rana gaurav.rana@freescale.com CC: Simon Glass sjg@chromium.org

---

This patch is dependent on following series of 10 patches. https://patchwork.ozlabs.org/patch/432126/ . .

Now applied to mainline.

.

https://patchwork.ozlabs.org/patch/432135/ README | 4 ++ common/hash.c | 10 +++ drivers/crypto/fsl/fsl_hash.c | 141 ++++++++++++++++++++++++++++++++++++++++++ drivers/crypto/fsl/fsl_hash.h | 32 ++++++++++ include/fsl_sec.h | 30 +++++++++ include/hw_sha.h | 77 +++++++++++++++++++++++ 6 files changed, 294 insertions(+) create mode 100644 drivers/crypto/fsl/fsl_hash.h

diff --git a/README b/README index cac7978..98aa31f 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 hw
  

s/hw/hardware/

•           acceleration
  

•           CONFIG_SHA_PROG_HW_ACCEL - support SHA1 and SHA256 progressive
  

•           hashing using hw acceleration
  
            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..d4becd3 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_sha1_init,
  

•           hw_sha1_update,
  

•           hw_sha1_finish,
  

+#endif }, { "sha256", SHA256_SUM_LEN, hw_sha256, CHUNKSZ_SHA256, +#ifdef CONFIG_SHA_PROG_HW_ACCEL

•           hw_sha256_init,
  

•           hw_sha256_update,
  

•           hw_sha256_finish,
  

+#endif }, #endif #ifdef CONFIG_SHA1 diff --git a/drivers/crypto/fsl/fsl_hash.c b/drivers/crypto/fsl/fsl_hash.c index d77f257..1681705 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,111 @@ 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, -1 on error
• */

+static int caam_hash_init(void **ctxp, enum caam_hash_algos caam_algo) +{

•   struct sha_ctx *ctx = malloc(sizeof(struct sha_ctx));
  

Please check return value and return -ENOMEM. Also you can use calloc() to zero it.

•   memset(ctx, 0, sizeof(struct sha_ctx));
  

•   *ctxp = ctx;
  

•   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

Shouldn't this be handled in finish()?

• • @caam_algo: Enum for SHA1 or SHA256
• • @return 0 if ok, -1 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 = (struct sha_ctx *)hash_ctx;
  

I don't think you need the cast.

•   if (ctx->sg_num >= MAX_SG) {
  

•           free(ctx);
  

•           return -1;
  

•   }
  

+#ifdef CONFIG_PHYS_64BIT

•   ctx->sg_tbl[ctx->sg_num].addr_hi = addr >> 32;
  

Is this needed, or will addr >> 32 be 0 in this case anyway?

+#else

•   ctx->sg_tbl[ctx->sg_num].addr_hi = 0x0;
  

+#endif

•   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);
  

•   }
  

Can you move this block to the finish() method?

•   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, -1 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 = (struct sha_ctx *)hash_ctx;
  

•   int i = 0, ret = 0;
  

•   if (size < driver_hash[caam_algo].digestsize) {
  

•           free(ctx);
  

•           return -1;
  

-ENOSPC

•   }
  

•   for (i = 0; i < ctx->sg_num; i++)
  

•           len +=  (sec_in32(&ctx->sg_tbl[i].len_flag) &
  

Double space

{} around this I think

•                    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) { @@ -69,9 +176,43 @@ void hw_sha256(const unsigned char *pbuf, unsigned int buf_len, printf("CAAM was not setup properly or it is faulty\n"); }

+int hw_sha256_init(struct hash_algo *algo, void **ctxp) +{

•   return caam_hash_init(ctxp, SHA256);
  

+}

+int hw_sha256_update(struct hash_algo *algo, void *ctx, const void *buf,

•                       unsigned int size, int is_last)
  

+{

•   return caam_hash_update(ctx, buf, size, is_last, SHA256);
  

+}

+int hw_sha256_finish(struct hash_algo *algo, void *ctx, void *dest_buf,

•                int size)
  

+{

•   return caam_hash_finish(ctx, dest_buf, size, SHA256);
  

+}

void hw_sha1(const unsigned char *pbuf, unsigned int buf_len, unsigned char *pout, unsigned int chunk_size) { if (caam_hash(pbuf, buf_len, pout, SHA1)) printf("CAAM was not setup properly or it is faulty\n"); }

+int hw_sha1_init(struct hash_algo *algo, void **ctxp) +{

•   return caam_hash_init(ctxp, SHA1);
  

+}

+int hw_sha1_update(struct hash_algo *algo, void *ctx, const void *buf,

•                       unsigned int size, int is_last)
  

+{

•   return caam_hash_update(ctx, buf, size, is_last, SHA1);
  

+}

+int hw_sha1_finish(struct hash_algo *algo, void *ctx, void *dest_buf,

•                int size)
  

+{

•   return caam_hash_finish(ctx, dest_buf, size, SHA1);
  

+} diff --git a/drivers/crypto/fsl/fsl_hash.h b/drivers/crypto/fsl/fsl_hash.h new file mode 100644 index 0000000..7f07ea4 --- /dev/null +++ b/drivers/crypto/fsl/fsl_hash.h @@ -0,0 +1,32 @@ +/*

• • Copyright 2014 Freescale Semiconductor, Inc.
• • SPDX-License-Identifier: GPL-2.0+
• */

+#ifndef _SHA_H +#define _SHA_H

+#include <fsl_sec.h> +#include "jr.h"

+#define MAX_SG 16

+/*

• • Hash context contain the following fields

contains

• • Sha Descriptor

Please can you use the standard format?

@sha_desc: SHA descriptor @sg_num: number of entries in sg table etc.

• • number of entries in sg table
• • total length of buffer
• • sg entry table
• • 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];
  

Do you need to include <hash.h> in this header to get this symbol?

+};

+#endif diff --git a/include/fsl_sec.h b/include/fsl_sec.h index aa850a3..cece820 100644 --- a/include/fsl_sec.h +++ b/include/fsl_sec.h @@ -175,6 +175,36 @@ 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 the start of the
  

•                            * buffer - hi
  

•                            */
  

Please put this comment on one line before the addr_hi parameter. Or use the @addr_hi structure-commenting approach before the structure. But don't split comments like this.

•   uint16_t reserved_zero;
  

+#else

•   uint16_t reserved_zero;
  

•   uint16_t addr_hi;       /* Memory Address of the start of the
  

•                            * buffer - hi
  

•                            */
  

here too.

•   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..6b040d2 100644 --- a/include/hw_sha.h +++ b/include/hw_sha.h @@ -22,6 +22,44 @@ void hw_sha256(const uchar * in_addr, uint buflen, uchar * out_addr, uint chunk_size);

+/*

• • Create the context for sha256 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, -1 on error
• */

+int hw_sha256_init(struct hash_algo *algo, void **ctxp);

+/*

• • Update buffer for sha256 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, -1 on error

-ve on error, please fix globally.

• */

+int hw_sha256_update(struct hash_algo *algo, void *ctx, const void *buf,

•                unsigned int size, int is_last);
  

+/*

• • Copy sha256 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, -1 on error
• */

+int hw_sha256_finish(struct hash_algo *algo, void *ctx, void *dest_buf,

•                int size);
  

/**

• Computes hash value of input pbuf using h/w acceleration

@@ -34,4 +72,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 sha1 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, -1 on error
• */

+int hw_sha1_init(struct hash_algo *algo, void **ctxp);

+/*

• • Update buffer for sha1 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, -1 on error
• */

+int hw_sha1_update(struct hash_algo *algo, void *ctx, const void *buf,

•                unsigned int size, int is_last);
  

+/*

• • Copy sha1 hash result at destination location
• • The context is freed after completion of hash operation.
• • @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, -1 on error
• */

+int hw_sha1_finish(struct hash_algo *algo, void *ctx, void *dest_buf,

•                int size);
  

The duplication is grim, but maybe we can move this to driver model later.

But still, since you are passed in the algo, why don't you have common functions for both SHA1 and SHA256?

#endif

1.8.1.4

Regards, Simon