diff mbox series

[v3,06/14] common: fit: Use hash.c to call CRC/SHA function

Message ID 20210720063839.1518-7-chiawei_wang@aspeedtech.com
State New
Headers show
Series None | expand

Commit Message

ChiaWei Wang July 20, 2021, 6:38 a.m. UTC
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>

---
 common/image-fit.c | 35 ++++++++++-------------------------
 1 file changed, 10 insertions(+), 25 deletions(-)

-- 
2.17.1

Comments

Tom Rini July 24, 2021, 12:57 p.m. UTC | #1
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
ChiaWei Wang July 26, 2021, 12:06 a.m. UTC | #2
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
Tom Rini July 26, 2021, 2:01 a.m. UTC | #3
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 mbox series

Patch

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;
 }