Re: [U-Boot] [PATCH 8/9] [v3] hash: Add function to find hash_algo struct with progressive hash
Hi Ruchika,
On 23 December 2014 at 04:32, Ruchika Gupta 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
> CC: Simon Glass
> ---
> Changes in v3 :
> Corrected ifdef for SHA1
>
> Changes in v2 :
> Added commit message
>
> common/hash.c | 33 -
> include/hash.h | 15 +++
> 2 files changed, 39 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
> #include
>
> -#ifdef CONFIG_CMD_SHA1SUM
> +#ifdef CONFIG_SHA1
I'm still not sure about this. I suspect this will bloat the code for
boards that use CONFIG_SHA1 (most) but not CONFIG_CMD_SHA1SUM. You
could check that, but I went through some contortions to make sure
that the hash API was not compiled in when not needed.
> 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.
> -*/
This is the comment referring to the above.
Is it possible to leave this logic as it is?
> -#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;
> + }
> + }
> + }
> +
> + 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..059f84e 100644
> --- a/include/hash.h
> +++ b/include/hash.h
> @@ -128,6 +128,21 @@ 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 the hash_algo struct with
> progressive
> + * hash support for an algorithm
Try to get that on one line if you can.
> + *
> + * 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 8/9] [v3] hash: Add function to find hash_algo struct with progressive hash
Hi Simon,
> -Original Message-
> From: s...@google.com [mailto:s...@google.com] On Behalf Of Simon Glass
> Sent: Wednesday, December 24, 2014 6:20 AM
> To: Gupta Ruchika-R66431
> Cc: U-Boot Mailing List; Sun York-R58495
> Subject: Re: [PATCH 8/9] [v3] hash: Add function to find hash_algo struct
> with progressive hash
>
> Hi Ruchika,
>
> On 23 December 2014 at 04:32, Ruchika Gupta
> 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
> > CC: Simon Glass
> > ---
> > Changes in v3 :
> > Corrected ifdef for SHA1
> >
> > Changes in v2 :
> > Added commit message
> >
> > common/hash.c | 33 - include/hash.h
> > | 15 +++
> > 2 files changed, 39 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
> > #include
> >
> > -#ifdef CONFIG_CMD_SHA1SUM
> > +#ifdef CONFIG_SHA1
>
> I'm still not sure about this. I suspect this will bloat the code for boards
> that use CONFIG_SHA1 (most) but not CONFIG_CMD_SHA1SUM. You could check that,
> but I went through some contortions to make sure that the hash API was not
> compiled in when not needed.
Since we will be using this API now in RSA checksum, defining CONFIG_SHA1
should allow the compilation of this structure. Asking user to enable
CONFIG_CMD_SHA1SUM for using rsa-checksum doesn’t look right. Please suggest.
>
> > 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.
> > -*/
>
> This is the comment referring to the above.
>
> Is it possible to leave this logic as it is?
>
> > -#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;
> > + }
> > + }
> > + }
> > +
> > + 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..059f84e
> > 100644
> > --- a/include/hash.h
> > +++ b/include/hash.h
> > @@ -128,6 +128,21 @@ 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 the hash_algo struct with
> progressive
> > + * hash support for an algorithm
>
> Try to get that on one line if you can.
>
> > + *
> > + * 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
> > + *
> > + * @re
Re: [U-Boot] [PATCH 8/9] [v3] hash: Add function to find hash_algo struct with progressive hash
+Wolfgang Hi Ruchika, On 29 December 2014 at 00:07, Ruchika Gupta wrote: > Hi Simon, > >> -Original Message- >> From: s...@google.com [mailto:s...@google.com] On Behalf Of Simon Glass >> Sent: Wednesday, December 24, 2014 6:20 AM >> To: Gupta Ruchika-R66431 >> Cc: U-Boot Mailing List; Sun York-R58495 >> Subject: Re: [PATCH 8/9] [v3] hash: Add function to find hash_algo struct >> with progressive hash >> >> Hi Ruchika, >> >> On 23 December 2014 at 04:32, Ruchika Gupta >> 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 >> > CC: Simon Glass >> > --- >> > Changes in v3 : >> > Corrected ifdef for SHA1 >> > >> > Changes in v2 : >> > Added commit message >> > >> > common/hash.c | 33 - include/hash.h >> > | 15 +++ >> > 2 files changed, 39 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 >> > #include >> > >> > -#ifdef CONFIG_CMD_SHA1SUM >> > +#ifdef CONFIG_SHA1 >> >> I'm still not sure about this. I suspect this will bloat the code for boards >> that use CONFIG_SHA1 (most) but not CONFIG_CMD_SHA1SUM. You could check that, >> but I went through some contortions to make sure that the hash API was not >> compiled in when not needed. > > Since we will be using this API now in RSA checksum, defining CONFIG_SHA1 > should allow the compilation of this structure. Asking user to enable > CONFIG_CMD_SHA1SUM for using rsa-checksum doesn’t look right. Please suggest. Agreed it doesn't, it was just a code size hack. Wolfgang might be able to chime in with thoughts here (+Cc). But still, do you need to change it? After all, CONFIG_CMD_SHA1SUM should be a superest for CONFIG_SHA1. [snip] Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 8/9] [v3] hash: Add function to find hash_algo struct with progressive hash
Hi Simon, > -Original Message- > From: s...@google.com [mailto:s...@google.com] On Behalf Of Simon Glass > Sent: Tuesday, December 30, 2014 2:44 AM > To: Gupta Ruchika-R66431 > Cc: U-Boot Mailing List; Sun York-R58495; Wolfgang Denk > Subject: Re: [PATCH 8/9] [v3] hash: Add function to find hash_algo struct > with progressive hash > > +Wolfgang > > Hi Ruchika, > > On 29 December 2014 at 00:07, Ruchika Gupta > wrote: > > Hi Simon, > > > >> -Original Message- > >> From: s...@google.com [mailto:s...@google.com] On Behalf Of Simon Glass > >> Sent: Wednesday, December 24, 2014 6:20 AM > >> To: Gupta Ruchika-R66431 > >> Cc: U-Boot Mailing List; Sun York-R58495 > >> Subject: Re: [PATCH 8/9] [v3] hash: Add function to find hash_algo > >> struct with progressive hash > >> > >> Hi Ruchika, > >> > >> On 23 December 2014 at 04:32, Ruchika Gupta > >> > >> 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 > >> > CC: Simon Glass > >> > --- > >> > Changes in v3 : > >> > Corrected ifdef for SHA1 > >> > > >> > Changes in v2 : > >> > Added commit message > >> > > >> > common/hash.c | 33 - > >> > include/hash.h > >> > | 15 +++ > >> > 2 files changed, 39 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 > >> > #include > >> > > >> > -#ifdef CONFIG_CMD_SHA1SUM > >> > +#ifdef CONFIG_SHA1 > >> > >> I'm still not sure about this. I suspect this will bloat the code for > >> boards that use CONFIG_SHA1 (most) but not CONFIG_CMD_SHA1SUM. You > >> could check that, but I went through some contortions to make sure > >> that the hash API was not compiled in when not needed. > > > > Since we will be using this API now in RSA checksum, defining CONFIG_SHA1 > should allow the compilation of this structure. Asking user to enable > CONFIG_CMD_SHA1SUM for using rsa-checksum doesn’t look right. Please suggest. > > Agreed it doesn't, it was just a code size hack. Wolfgang might be able to > chime in with thoughts here (+Cc). > > But still, do you need to change it? After all, CONFIG_CMD_SHA1SUM should be > a superest for CONFIG_SHA1. With CONFIG_FIT_SIGNATURE, CONFIG_SHA1 and CONFIG_SHA256 get automatically defined in include/image.h. I need to use the structure hash_algos to find the functions to be used for algo SHA1. If I leave this as it is, it would mean that I will have to modify include/image.h to define CONFIG_CMD_SHA1SUM for FIT signatures. I am not sure whether that would be the right thing to do. > > [snip] > > Regards, > Simon Regards, Ruchika ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
