| Message ID | 1502743087-27316-1-git-send-email-trini@konsulko.com |
|---|---|
| State | Accepted |
| Commit | 78eda89e9a25810d58dc6df86dbe5d8902cbb90c |
| Delegated to: | Tom Rini |
| Headers | show |
> -----Original Message----- > From: Tom Rini [mailto:trini@konsulko.com] > Sent: Tuesday, August 15, 2017 2:08 AM > To: u-boot@lists.denx.de > Cc: Simon Glass <sjg@chromium.org>; Sumit Garg <sumit.garg@nxp.com> > Subject: [PATCH v2] hash: Compile only hardware or software versions of SHA > algorithms > > Commit 089df18bfe9d ("lib: move hash CONFIG options to Kconfig") moved > CONFIG_SHA1, CONFIG_SHA256, CONFIG_SHA_HW_ACCEL, and > CONFIG_SHA_PROG_HW_ACCEL config options to Kconfig. So in the case of > SPL, CONFIG_SPL_HASH_SUPPORT enables CONFIG_SHA1 and > CONFIG_SHA256 which enables SHA SW library by default. But in the case of > platforms with SHA HW library support, SHA SW library becomes redundant and > increases size of SPL by approx 18K. Rework the code so that we have named > members and only have either software or hardware versions of the algorithm, > depending on the relevant config options. Update the comment around > hash_algo to reflect this as well. > > Reported-by: Sumit Garg <sumit.garg@nxp.com> > Cc: Sumit Garg <sumit.garg@nxp.com> > Signed-off-by: Tom Rini <trini@konsulko.com> > --- > Changes in v2: > - Rework to switch the struct to use named members, and have only one > instance of sha1 and sha256. Re-order chunk_size in the listing so > it's not in between #ifdefs. > - Take author since the patch is almost entirely different. Sumit, if > you feel this is unfair, please feel free to speak-up and I'll re-post > with you as Author. Your commit message was quite good, so I've > reused that almost entirely. > --- > common/hash.c | 95 +++++++++++++++++++++++++++--------------------------- > ----- > 1 file changed, 44 insertions(+), 51 deletions(-) > With this patch code looks much cleaner. No issues from my side with you as Author. Reviewed-by: Sumit Garg <sumit.garg@nxp.com> Sumit
On Mon, Aug 14, 2017 at 04:38:07PM -0400, Tom Rini wrote: > Commit 089df18bfe9d ("lib: move hash CONFIG options to Kconfig") moved > CONFIG_SHA1, CONFIG_SHA256, CONFIG_SHA_HW_ACCEL, and > CONFIG_SHA_PROG_HW_ACCEL config options to Kconfig. So in the case of > SPL, CONFIG_SPL_HASH_SUPPORT enables CONFIG_SHA1 and CONFIG_SHA256 which > enables SHA SW library by default. But in the case of platforms with > SHA HW library support, SHA SW library becomes redundant and increases > size of SPL by approx 18K. Rework the code so that we have named > members and only have either software or hardware versions of the > algorithm, depending on the relevant config options. Update the comment > around hash_algo to reflect this as well. > > Reported-by: Sumit Garg <sumit.garg@nxp.com> > Cc: Sumit Garg <sumit.garg@nxp.com> > Signed-off-by: Tom Rini <trini@konsulko.com> > Reviewed-by: Sumit Garg <sumit.garg@nxp.com> Applied to u-boot/master, thanks!
diff --git a/common/hash.c b/common/hash.c index 771d8fa87f94..b60c028da355 100644 --- a/common/hash.c +++ b/common/hash.c @@ -30,7 +30,7 @@ #include <u-boot/sha256.h> #include <u-boot/md5.h> -#ifdef CONFIG_SHA1 +#if defined(CONFIG_SHA1) && !defined(CONFIG_SHA_PROG_HW_ACCEL) static int hash_init_sha1(struct hash_algo *algo, void **ctxp) { sha1_context *ctx = malloc(sizeof(sha1_context)); @@ -58,7 +58,7 @@ static int hash_finish_sha1(struct hash_algo *algo, void *ctx, void *dest_buf, } #endif -#ifdef CONFIG_SHA256 +#if defined(CONFIG_SHA256) && !defined(CONFIG_SHA_PROG_HW_ACCEL) static int hash_init_sha256(struct hash_algo *algo, void **ctxp) { sha256_context *ctx = malloc(sizeof(sha256_context)); @@ -113,68 +113,61 @@ static int hash_finish_crc32(struct hash_algo *algo, void *ctx, void *dest_buf, } /* - * These are the hash algorithms we support. Chips which support accelerated - * crypto could perhaps add named version of these algorithms here. Note that - * algorithm names must be in lower case. + * These are the hash algorithms we support. If we have hardware acceleration + * is enable we will use that, otherwise a software version of the algorithm. + * Note that algorithm names must be in lower case. */ static struct hash_algo hash_algo[] = { - /* - * CONFIG_SHA_HW_ACCEL is defined if hardware acceleration is - * available. - */ -#ifdef CONFIG_SHA_HW_ACCEL +#ifdef CONFIG_SHA1 { - "sha1", - SHA1_SUM_LEN, - hw_sha1, - CHUNKSZ_SHA1, -#ifdef CONFIG_SHA_PROG_HW_ACCEL - hw_sha_init, - hw_sha_update, - hw_sha_finish, + .name = "sha1", + .digest_size = SHA1_SUM_LEN, + .chunk_size = CHUNKSZ_SHA1, +#ifdef CONFIG_SHA_HW_ACCEL + .hash_func_ws = hw_sha1, +#else + .hash_func_ws = sha1_csum_wd, #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 - }, + .hash_init = hw_sha_init, + .hash_update = hw_sha_update, + .hash_finish = hw_sha_finish, +#else + .hash_init = hash_init_sha1, + .hash_update = hash_update_sha1, + .hash_finish = hash_finish_sha1, #endif -#ifdef CONFIG_SHA1 - { - "sha1", - SHA1_SUM_LEN, - sha1_csum_wd, - CHUNKSZ_SHA1, - hash_init_sha1, - hash_update_sha1, - hash_finish_sha1, }, #endif #ifdef CONFIG_SHA256 { - "sha256", - SHA256_SUM_LEN, - sha256_csum_wd, - CHUNKSZ_SHA256, - hash_init_sha256, - hash_update_sha256, - hash_finish_sha256, + .name = "sha256", + .digest_size = SHA256_SUM_LEN, + .chunk_size = CHUNKSZ_SHA256, +#ifdef CONFIG_SHA_HW_ACCEL + .hash_func_ws = hw_sha256, +#else + .hash_func_ws = sha256_csum_wd, +#endif +#ifdef CONFIG_SHA_PROG_HW_ACCEL + .hash_init = hw_sha_init, + .hash_update = hw_sha_update, + .hash_finish = hw_sha_finish, +#else + .hash_init = hash_init_sha256, + .hash_update = hash_update_sha256, + .hash_finish = hash_finish_sha256, +#endif }, #endif { - "crc32", - 4, - crc32_wd_buf, - CHUNKSZ_CRC32, - hash_init_crc32, - hash_update_crc32, - hash_finish_crc32, + .name = "crc32", + .digest_size = 4, + .chunk_size = CHUNKSZ_CRC32, + .hash_func_ws = crc32_wd_buf, + .hash_init = hash_init_crc32, + .hash_update = hash_update_crc32, + .hash_finish = hash_finish_crc32, }, };
Commit 089df18bfe9d ("lib: move hash CONFIG options to Kconfig") moved CONFIG_SHA1, CONFIG_SHA256, CONFIG_SHA_HW_ACCEL, and CONFIG_SHA_PROG_HW_ACCEL config options to Kconfig. So in the case of SPL, CONFIG_SPL_HASH_SUPPORT enables CONFIG_SHA1 and CONFIG_SHA256 which enables SHA SW library by default. But in the case of platforms with SHA HW library support, SHA SW library becomes redundant and increases size of SPL by approx 18K. Rework the code so that we have named members and only have either software or hardware versions of the algorithm, depending on the relevant config options. Update the comment around hash_algo to reflect this as well. Reported-by: Sumit Garg <sumit.garg@nxp.com> Cc: Sumit Garg <sumit.garg@nxp.com> Signed-off-by: Tom Rini <trini@konsulko.com> --- Changes in v2: - Rework to switch the struct to use named members, and have only one instance of sha1 and sha256. Re-order chunk_size in the listing so it's not in between #ifdefs. - Take author since the patch is almost entirely different. Sumit, if you feel this is unfair, please feel free to speak-up and I'll re-post with you as Author. Your commit message was quite good, so I've reused that almost entirely. --- common/hash.c | 95 +++++++++++++++++++++++++++-------------------------------- 1 file changed, 44 insertions(+), 51 deletions(-)