| Message ID | 20210720063839.1518-7-chiawei_wang@aspeedtech.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On Tue, Jul 20, 2021 at 02:38:31PM +0800, Chia-Wei Wang wrote: > From: Joel Stanley <joel@jms.id.au> > > Currently the FIT verification calls directly into > SW implemented functions to get a CRC/SHA/MD5 hash. > > This patch removes duplcated algorithm lookup and use > hash_lookup_algo to get the hashing function with HW > accelearation supported if configured. > > The MD5 direct call remains as it is not included in > the hash lookup table of hash.c. > > Signed-off-by: Joel Stanley <joel@jms.id.au> > Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com> While this is a good idea, there's some required prep work. At least the following platforms don't compile due to this patch: ls1046ardb_qspi imx8mm_beacon imx8mn_beacon imx8mn_beacon_2g imx8mm-icore-mx8mm-ctouch2 imx8mm-icore-mx8mm-edimm2.2 imx8mm_evk imx8mn_ddr4_evk imx8mn_evk imx8mp_evk imx8mq_evk imx8mm_venice imx8mq_phanbell phycore-imx8mm phycore-imx8mp pico-imx8mq verdin-imx8mm mt8183_pumpkin mt8516_pumpkin mscc_jr2 mscc_luton mscc_ocelot mscc_serval mscc_servalt mt7620_mt7530_rfb mt7620_rfb mt7628_rfb Which is likely due to cases where HASH or SPL_HASH_SUPPORT are not being selected as it was not previously required. -- Tom
Hi Tom, > From: Tom Rini <trini@konsulko.com> > Sent: Saturday, July 24, 2021 8:57 PM > > On Tue, Jul 20, 2021 at 02:38:31PM +0800, Chia-Wei Wang wrote: > > > From: Joel Stanley <joel@jms.id.au> > > > > Currently the FIT verification calls directly into SW implemented > > functions to get a CRC/SHA/MD5 hash. > > > > This patch removes duplcated algorithm lookup and use hash_lookup_algo > > to get the hashing function with HW accelearation supported if > > configured. > > > > The MD5 direct call remains as it is not included in the hash lookup > > table of hash.c. > > > > Signed-off-by: Joel Stanley <joel@jms.id.au> > > Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com> > > While this is a good idea, there's some required prep work. At least the > following platforms don't compile due to this patch: > ls1046ardb_qspi imx8mm_beacon imx8mn_beacon imx8mn_beacon_2g > imx8mm-icore-mx8mm-ctouch2 imx8mm-icore-mx8mm-edimm2.2 > imx8mm_evk imx8mn_ddr4_evk imx8mn_evk imx8mp_evk imx8mq_evk > imx8mm_venice imx8mq_phanbell phycore-imx8mm phycore-imx8mp > pico-imx8mq verdin-imx8mm mt8183_pumpkin mt8516_pumpkin mscc_jr2 > mscc_luton mscc_ocelot mscc_serval mscc_servalt mt7620_mt7530_rfb > mt7620_rfb mt7628_rfb > > Which is likely due to cases where HASH or SPL_HASH_SUPPORT are not being > selected as it was not previously required. > Thanks for the notification of this error. I will examine the code flow to figure out the root cause on these platforms. Meanwhile, Simon also suggested the need to add a new UCLASS_HASH to refactor the hash structure. http://patchwork.ozlabs.org/project/uboot/patch/20210720063839.1518-4-chiawei_wang@aspeedtech.com/ I was wondering if I can prepare another leading patch for UCLASS_HASH and also to make sure the current codebase works fine? After that, we can restart this patch series for Aspeed FIT booting. Regards, Chiawei
On Mon, Jul 26, 2021 at 12:06:28AM +0000, ChiaWei Wang wrote: > Hi Tom, > > > From: Tom Rini <trini@konsulko.com> > > Sent: Saturday, July 24, 2021 8:57 PM > > > > On Tue, Jul 20, 2021 at 02:38:31PM +0800, Chia-Wei Wang wrote: > > > > > From: Joel Stanley <joel@jms.id.au> > > > > > > Currently the FIT verification calls directly into SW implemented > > > functions to get a CRC/SHA/MD5 hash. > > > > > > This patch removes duplcated algorithm lookup and use hash_lookup_algo > > > to get the hashing function with HW accelearation supported if > > > configured. > > > > > > The MD5 direct call remains as it is not included in the hash lookup > > > table of hash.c. > > > > > > Signed-off-by: Joel Stanley <joel@jms.id.au> > > > Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com> > > > > While this is a good idea, there's some required prep work. At least the > > following platforms don't compile due to this patch: > > ls1046ardb_qspi imx8mm_beacon imx8mn_beacon imx8mn_beacon_2g > > imx8mm-icore-mx8mm-ctouch2 imx8mm-icore-mx8mm-edimm2.2 > > imx8mm_evk imx8mn_ddr4_evk imx8mn_evk imx8mp_evk imx8mq_evk > > imx8mm_venice imx8mq_phanbell phycore-imx8mm phycore-imx8mp > > pico-imx8mq verdin-imx8mm mt8183_pumpkin mt8516_pumpkin mscc_jr2 > > mscc_luton mscc_ocelot mscc_serval mscc_servalt mt7620_mt7530_rfb > > mt7620_rfb mt7628_rfb > > > > Which is likely due to cases where HASH or SPL_HASH_SUPPORT are not being > > selected as it was not previously required. > > > > Thanks for the notification of this error. I will examine the code flow to figure out the root cause on these platforms. > > Meanwhile, Simon also suggested the need to add a new UCLASS_HASH to refactor the hash structure. > http://patchwork.ozlabs.org/project/uboot/patch/20210720063839.1518-4-chiawei_wang@aspeedtech.com/ > > I was wondering if I can prepare another leading patch for UCLASS_HASH and also to make sure the current codebase works fine? > After that, we can restart this patch series for Aspeed FIT booting. OK, sounds like a good plan, thanks! -- Tom
diff --git a/common/image-fit.c b/common/image-fit.c index 0c5a05948d..e52ff47bc3 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -1196,7 +1196,7 @@ int fit_set_timestamp(void *fit, int noffset, time_t timestamp) * calculate_hash - calculate and return hash for provided input data * @data: pointer to the input data * @data_len: data length - * @algo: requested hash algorithm + * @algo_name: requested hash algorithm * @value: pointer to the char, will hold hash value data (caller must * allocate enough free space) * value_len: length of the calculated hash @@ -1210,37 +1210,22 @@ int fit_set_timestamp(void *fit, int noffset, time_t timestamp) * 0, on success * -1, when algo is unsupported */ -int calculate_hash(const void *data, int data_len, const char *algo, - uint8_t *value, int *value_len) +int calculate_hash(const void *data, int data_len, const char *algo_name, + uint8_t *value, int *value_len) { - if (IMAGE_ENABLE_CRC32 && strcmp(algo, "crc32") == 0) { - *((uint32_t *)value) = crc32_wd(0, data, data_len, - CHUNKSZ_CRC32); - *((uint32_t *)value) = cpu_to_uimage(*((uint32_t *)value)); - *value_len = 4; - } else if (IMAGE_ENABLE_SHA1 && strcmp(algo, "sha1") == 0) { - sha1_csum_wd((unsigned char *)data, data_len, - (unsigned char *)value, CHUNKSZ_SHA1); - *value_len = 20; - } else if (IMAGE_ENABLE_SHA256 && strcmp(algo, "sha256") == 0) { - sha256_csum_wd((unsigned char *)data, data_len, - (unsigned char *)value, CHUNKSZ_SHA256); - *value_len = SHA256_SUM_LEN; - } else if (IMAGE_ENABLE_SHA384 && strcmp(algo, "sha384") == 0) { - sha384_csum_wd((unsigned char *)data, data_len, - (unsigned char *)value, CHUNKSZ_SHA384); - *value_len = SHA384_SUM_LEN; - } else if (IMAGE_ENABLE_SHA512 && strcmp(algo, "sha512") == 0) { - sha512_csum_wd((unsigned char *)data, data_len, - (unsigned char *)value, CHUNKSZ_SHA512); - *value_len = SHA512_SUM_LEN; - } else if (IMAGE_ENABLE_MD5 && strcmp(algo, "md5") == 0) { + struct hash_algo *algo; + + if (IMAGE_ENABLE_MD5 && strcmp(algo_name, "md5") == 0) { md5_wd((unsigned char *)data, data_len, value, CHUNKSZ_MD5); *value_len = 16; + } else if (hash_lookup_algo(algo_name, &algo) == 0) { + algo->hash_func_ws(data, data_len, value, algo->chunk_size); + *value_len = algo->digest_size; } else { debug("Unsupported hash alogrithm\n"); return -1; } + return 0; }