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

[Simon Glass]

Hi Ruchika,

On 5 February 2015 at 04:20, Ruchika Gupta <ruchika.gupta at freescale.com> wrote:
> Hi Simon,
>
[snip]

>> > 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.
> Ok
>>
>> > +       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()?
> The interface as defined in hash.h has is_last in the hash_update function. We have defined this function on similar line. Already existing function pointer as available in include/hash.h is pasted below for reference.

OK I see. It seems wrong, but maybe I misunderstand it. Anyway as you
say it has nothing to do with your patch.

> /*
>          * hash_update: Perform hashing on the given buffer
>          *
>          * 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 (*hash_update)(struct hash_algo *algo, void *ctx, const void *buf,
>                            unsigned int size, int is_last);
>
> Are you suggesting that we change the above function pointer in include/hash.h also ?
>

No let's leave it.

[snip]

Regards,
Simon