| Message ID | 20210916063912.21546-1-chiawei_wang@aspeedtech.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [next] lib: hash-checksum: Use DM_HASH if supported | expand |
Hi Chia-Wei, On Thu, 16 Sept 2021 at 00:39, Chia-Wei Wang <chiawei_wang@aspeedtech.com> wrote: > > Use DM_HASH to perform hashing operations if supported. > Thus either SW or HW-assisted hashing could be leveraged. This is missing a full motivation. Please can you explain why this code is needed on a board, rather than just the host? As of recently, this has become host-only code. Regards, Simon
Hi Simon, > From: Simon Glass <sjg@chromium.org> > Sent: Wednesday, October 6, 2021 10:10 PM > > Hi Chia-Wei, > > On Thu, 16 Sept 2021 at 00:39, Chia-Wei Wang > <chiawei_wang@aspeedtech.com> wrote: > > > > Use DM_HASH to perform hashing operations if supported. > > Thus either SW or HW-assisted hashing could be leveraged. > > This is missing a full motivation. Please can you explain why this code is > needed on a board, rather than just the host? > > As of recently, this has become host-only code. The entry to non-DM hash function for U-Boot is kind of inconsistent. When a FIT image is verified by a hash digest: hash-1 { algo = "sha256"; }; The hash is calculated by calculate_hash() in image-fit.c. fit_image_verify_with_data() -> fit_image_check_hash() -> calculate_hash() However, when a FIT image is verified by a checksum signature: signature { algo = "sha256,rsa2048"; key-name-hint = "dev"; }; The hash comes from hash_calculate() in hash-checksum.c. fit_image_verify_with_data() -> fit_image_setup_verify() -> image_get_checksum_algo() -> hash_calculate() I checked the master and next branches. It seems that the logic still exists. (correct me if I am wrong) This patch is like a temporary solution to make the DM_HASH work smoothly. I believe a patch to refactor hash calculation of U-boot itself and the host tools is needed in the future. Regards, Chiawei
Hi ChiaWei, +Alexandru Gagniuc too On Wed, 6 Oct 2021 at 20:07, ChiaWei Wang <chiawei_wang@aspeedtech.com> wrote: > > Hi Simon, > > > From: Simon Glass <sjg@chromium.org> > > Sent: Wednesday, October 6, 2021 10:10 PM > > > > Hi Chia-Wei, > > > > On Thu, 16 Sept 2021 at 00:39, Chia-Wei Wang > > <chiawei_wang@aspeedtech.com> wrote: > > > > > > Use DM_HASH to perform hashing operations if supported. > > > Thus either SW or HW-assisted hashing could be leveraged. > > > > This is missing a full motivation. Please can you explain why this code is > > needed on a board, rather than just the host? > > > > As of recently, this has become host-only code. > > The entry to non-DM hash function for U-Boot is kind of inconsistent. > > When a FIT image is verified by a hash digest: > hash-1 { > algo = "sha256"; > }; > > The hash is calculated by calculate_hash() in image-fit.c. > fit_image_verify_with_data() -> fit_image_check_hash() -> calculate_hash() > > However, when a FIT image is verified by a checksum signature: > signature { > algo = "sha256,rsa2048"; > key-name-hint = "dev"; > }; > > The hash comes from hash_calculate() in hash-checksum.c. > fit_image_verify_with_data() -> fit_image_setup_verify() -> image_get_checksum_algo() -> hash_calculate() > > I checked the master and next branches. It seems that the logic still exists. (correct me if I am wrong) > This patch is like a temporary solution to make the DM_HASH work smoothly. > I believe a patch to refactor hash calculation of U-boot itself and the host tools is needed in the future. Yes I see. We should move this code into common then, I suppose. You patch looks reasonable to me. Alex, can you comment on this? Regards, Simon
diff --git a/lib/hash-checksum.c b/lib/hash-checksum.c index d732ecc38f..f30873db38 100644 --- a/lib/hash-checksum.c +++ b/lib/hash-checksum.c @@ -10,6 +10,10 @@ #include <linux/errno.h> #include <asm/unaligned.h> #include <hash.h> +#if defined(CONFIG_DM_HASH) +#include <dm.h> +#include <u-boot/hash.h> +#endif #else #include "fdt_host.h" #endif @@ -20,6 +24,38 @@ int hash_calculate(const char *name, const struct image_region region[], int region_count, uint8_t *checksum) { +#if !defined(USE_HOSTCC) && defined(CONFIG_DM_HASH) + int i, rc; + enum HASH_ALGO hash_algo; + struct udevice *dev; + void *ctx; + + rc = uclass_get_device(UCLASS_HASH, 0, &dev); + if (rc) { + debug("failed to get hash device, rc=%d\n", rc); + return -1; + } + + hash_algo = hash_algo_lookup_by_name(name); + if (hash_algo == HASH_ALGO_INVALID) { + debug("Unsupported hash algorithm\n"); + return -1; + }; + + rc = hash_init(dev, hash_algo, &ctx); + if (rc) + return rc; + + for (i = 0; i < region_count; i++) { + rc = hash_update(dev, ctx, region[i].data, region[i].size); + if (rc) + return rc; + } + + rc = hash_finish(dev, ctx, checksum); + if (rc) + return rc; +#else struct hash_algo *algo; int ret = 0; void *ctx; @@ -47,6 +83,7 @@ int hash_calculate(const char *name, ret = algo->hash_finish(algo, ctx, checksum, algo->digest_size); if (ret) return ret; +#endif return 0; }
Use DM_HASH to perform hashing operations if supported. Thus either SW or HW-assisted hashing could be leveraged. Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com> --- lib/hash-checksum.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)