On 5/18/21 5:36 AM, Joel Stanley wrote:
On Fri, 14 May 2021 at 09:12, Heinrich Schuchardt <[email protected]> wrote:

Commit a479f103dc1c ("hash: Allow for SHA512 hardware implementations")
defined function definitions for hardware accelerated SHA384 and SHA512.
If CONFIG_SHA_HW_ACCEL=y, these functions are used.

We already have boards using CONFIG_SHA_HW_ACCEL=y but none implements the
new functions hw_sha384() and hw_sha512().

For implementing the EFI TCG2 protocol we need SHA384 and SHA512. The
missing hardware acceleration functions lead to build errors on boards like
peach-pi_defconfig.

Introduce a new Kconfig symbol CONFIG_SHA512_HW_ACCEL to control if the
functions hw_sha384() and hw_sha512() shall be used to implement the SHA384
and SHA512 algorithms.

Fixes: a479f103dc1c ("hash: Allow for SHA512 hardware implementations")
Signed-off-by: Heinrich Schuchardt <[email protected]>
---
This patch replaces
hash: revert Allow for SHA512 hardware implementations
https://lists.denx.de/pipermail/u-boot/2021-May/449648.html
https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/

This looks to be a good compromise. Thanks for fixing it.

Reviewed-by: Joel Stanley <[email protected]>

The CONFIG maze that is the hash acceleration code could do with some
overarching cleanup at some point.


--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -389,21 +389,32 @@ config SHA384
           (digest).

  config SHA_HW_ACCEL
-       bool "Enable hashing using hardware"
+       bool "Enable hardware acceleration for SHA hash functions"
         help
-         This option enables hardware acceleration for SHA hashing.
-         This affects the 'hash' command and also the hash_lookup_algo()
-         function.
+         This option enables hardware acceleration for the SHA1 and SHA256
+         hashing algorithms. This affects the 'hash' command and also the
+         hash_lookup_algo() function.
+
+if SHA_HW_ACCEL
+
+config SHA512_HW_ACCEL
+       bool "Enable hardware acceleration for SHA512"
+       depends on SHA512_ALGO

This dependency is new, does it make sense? We don't have an
equivalent one for SHA_HW_ACCEL, but perhaps we should introduce one?

It does not make sense to show SHA512_HW_ACCEL is we cannot have SHA512
or SHA384.


+       help
+         This option enables hardware acceleration for the SHA384 andSHA512

Nit: there's a missing space after "and" here.

I will add it.


Perhaps you could add to the help text that this option should be
disabled if the platform requires SHA512 support but does not have
hardware acceleration.

The current text already says that it "enables hardware acceleration".

It would provide little value to add to all Kconfig options:
"Don't enable this option if you don't have a hardware for it.".

Best regards

Heinrich


+         hashing algorithms. This affects the 'hash' command and also the
+         hash_lookup_algo() function.

  config SHA_PROG_HW_ACCEL
         bool "Enable Progressive hashing support using hardware"
-       depends on SHA_HW_ACCEL
         help
           This option enables hardware-acceleration for SHA progressive
           hashing.
           Data can be streamed in a block at a time and the hashing is
           performed in hardware.

+endif
+
  config MD5
         bool "Support MD5 algorithm"
         help
--
2.30.2


Reply via email to