[U-Boot] [PATCH 8/9] [v4] hash: Add function to find hash_algo struct with progressive hash

[Ruchika Gupta <ruchika.gupta@freescale.com>]

Hi Simon,

> -----Original Message-----
> From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
> Sent: Saturday, January 03, 2015 3:54 AM
> To: Gupta Ruchika-R66431
> Cc: U-Boot Mailing List; Sun York-R58495; Wolfgang Denk
> Subject: Re: [PATCH 8/9] [v4] hash: Add function to find hash_algo struct
> with progressive hash
> 
> Hi Ruchika,
> 
> On 30 December 2014 at 02:30, Ruchika Gupta <ruchika.gupta@freescale.com>
> wrote:
> > The hash_algo structure has some implementations in which progressive
> > hash API's are not defined. These are basically the hardware based
> > implementations of SHA. An API is added to find the algo which has
> > progressive hash API's defined. This can then be integrated with RSA
> > checksum library which uses Progressive Hash API's.
> >
> > Signed-off-by: Ruchika Gupta <ruchika.gupta@freescale.com>
> > CC: Simon Glass <sjg@chromium.org>
> > ---
> > Changes in v4:
> > Few cosmetic changes. Currently I have not replaced CONFIG_SHA1  with
> CONFIG_CMD_SHA1SUM.
> > Waiting for reply from Simon and Denx for the same.
> 
> ./tools/buildman/buildman -b try-rsa MPC8308RDB boards.cfg is up to date.
> Nothing to do.
> Building 10 commits for 1 boards (1 thread, 32 jobs per thread)
>    10    0    0 /10     MPC8308RDB
> (try-rsa=458103: asc) u> ./tools/buildman/buildman -b try-rsa MPC8308RDB -sS
> boards.cfg is up to date. Nothing to do.
> Summary of 10 commits for 1 boards (1 thread, 32 jobs per thread)
> 01: Merge branch 'master' of git://git.denx.de/u-boot-usb
> 02: rsa: Split the rsa-verify to separate the modular exponentiation
> 03: FIT: Modify option FIT_SIGNATURE in Kconfig
> 04: DM: crypto/rsa: Add rsa Modular Exponentiation DM driver
> 05: configs: Move CONFIG_FIT_SIGNATURE to defconfig
> 06: lib/rsa: Modify rsa to use DM driver
> 07: DM: crypto/fsl - Add Freescale rsa DM driver
> 08: lib/rsa: Add Kconfig for devices supporting RSA Modular Exponentiation
> 09: hash: Add function to find hash_algo struct with progressive hash
>    powerpc: (for 1/1 boards)  all +208.0  data +28.0  text +180.0
> 10: rsa: Use checksum algorithms from struct hash_algo (no errors to report)
> (try-rsa=458103: asc) u>
> 
> So this adds 180 bytes of code space. I really don't think that is a big
> problem, so I think this patch is fine. I'll Wolfgang chime in if he
> disagrees.
> 
> >
> > Changes in v3 :
> > Corrected ifdef for SHA1
> >
> > Changes in v2 :
> > Added commit message
> >
> >  common/hash.c  | 33 ++++++++++++++++++++++++---------  include/hash.h
> > | 14 ++++++++++++++
> >  2 files changed, 38 insertions(+), 9 deletions(-)
> >
> > diff --git a/common/hash.c b/common/hash.c index 12d6759..ea1ec60
> > 100644
> > --- a/common/hash.c
> > +++ b/common/hash.c
> > @@ -20,7 +20,7 @@
> >  #include <asm/io.h>
> >  #include <asm/errno.h>
> >
> > -#ifdef CONFIG_CMD_SHA1SUM
> > +#ifdef CONFIG_SHA1
> >  static int hash_init_sha1(struct hash_algo *algo, void **ctxp)  {
> >         sha1_context *ctx = malloc(sizeof(sha1_context)); @@ -125,12
> > +125,7 @@ static struct hash_algo hash_algo[] = {
> >                 CHUNKSZ_SHA256,
> >         },
> >  #endif
> > -       /*
> > -        * This is CONFIG_CMD_SHA1SUM instead of CONFIG_SHA1 since
> otherwise
> > -        * it bloats the code for boards which use SHA1 but not the 'hash'
> > -        * or 'sha1sum' commands.
> > -        */
> > -#ifdef CONFIG_CMD_SHA1SUM
> > +#ifdef CONFIG_SHA1
> >         {
> >                 "sha1",
> >                 SHA1_SUM_LEN,
> > @@ -140,7 +135,6 @@ static struct hash_algo hash_algo[] = {
> >                 hash_update_sha1,
> >                 hash_finish_sha1,
> >         },
> > -#define MULTI_HASH
> >  #endif
> >  #ifdef CONFIG_SHA256
> >         {
> > @@ -152,7 +146,6 @@ static struct hash_algo hash_algo[] = {
> >                 hash_update_sha256,
> >                 hash_finish_sha256,
> >         },
> > -#define MULTI_HASH
> >  #endif
> >         {
> >                 "crc32",
> > @@ -165,6 +158,10 @@ static struct hash_algo hash_algo[] = {
> >         },
> >  };
> >
> > +#if defined(CONFIG_SHA256) || defined(CONFIG_CMD_SHA1SUM) #define
> > +MULTI_HASH #endif
> > +
> >  #if defined(CONFIG_HASH_VERIFY) || defined(CONFIG_CMD_HASH)  #define
> > MULTI_HASH  #endif @@ -311,6 +308,24 @@ int hash_lookup_algo(const
> > char *algo_name, struct hash_algo **algop)
> >         return -EPROTONOSUPPORT;
> >  }
> >
> > +int hash_progressive_lookup_algo(const char *algo_name,
> > +                                struct hash_algo **algop) {
> > +       int i;
> > +
> > +       for (i = 0; i < ARRAY_SIZE(hash_algo); i++) {
> > +               if (!strcmp(algo_name, hash_algo[i].name)) {
> > +                       if (hash_algo[i].hash_init) {
> > +                               *algop = &hash_algo[i];
> > +                               return 0;
> > +                       }
> > +               }
> > +       }
> 
> Can't you just call hash_lookup_algo() and then check for hash_init?
Hash_lookup_algo always searches from the start of hash_algo[]. So what you have suggested above wouldn't work.

> 
> > +
> > +       debug("Unknown hash algorithm '%s'\n", algo_name);
> > +       return -EPROTONOSUPPORT;
> > +}
> > +
> >  void hash_show(struct hash_algo *algo, ulong addr, ulong len, uint8_t
> > *output)  {
> >         int i;
> > diff --git a/include/hash.h b/include/hash.h index d8ec4f0..c0a7ebc
> > 100644
> > --- a/include/hash.h
> > +++ b/include/hash.h
> > @@ -128,6 +128,20 @@ int hash_block(const char *algo_name, const void
> > *data, unsigned int len,  int hash_lookup_algo(const char *algo_name,
> > struct hash_algo **algop);
> >
> >  /**
> > + * hash_progressive_lookup_algo() - Look up hash_algo for prog. hash
> > +support
> > + *
> > + * The function returns the pointer to the struct or -EPROTONOSUPPORT
> > +if the
> > + * algorithm is not available with progressive hash support.
> > + *
> > + * @algo_name: Hash algorithm to look up
> > + * @algop: Pointer to the hash_algo struct if found
> > + *
> > + * @return 0 if ok, -EPROTONOSUPPORT for an unknown algorithm.
> > + */
> > +int hash_progressive_lookup_algo(const char *algo_name,
> > +                                struct hash_algo **algop);
> > +
> > +/**
> >   * hash_show() - Print out a hash algorithm and value
> >   *
> >   * You will get a message like this (without a newline at the end):
> > --
> > 1.8.1.4
> >
> 
> Regards,
> Simon

Regards,
Ruchika