[U-Boot] [PATCH][v5] crypto/fsl - Add progressive hashing support using hardware acceleration.

[Simon Glass]

Hi Gaurav,

On 12 February 2015 at 02:43, Gaurav Rana <gaurav.rana at freescale.com> wrote:
> Currently only normal hashing is supported using hardware acceleration.
> Added support for progressive hashing using hardware.
>
> Signed-off-by: Ruchika Gupta <ruchika.gupta at freescale.com>
> Signed-off-by: Gaurav Rana <gaurav.rana at freescale.com>
> CC: Simon Glass <sjg at chromium.org>

Acked-by: Simon Glass <sjg at chromium.org>

I have some requested edits on the Kconfig help below. Can you please
respin with these and my Ack?

York do you plan to pick this up>? If not I can bring it through u-boot-dm.

> ---
> Changes in v5:
> Modify description for CONFIG_SHA256, CONFIG_SHA256.
>
> Changes in v4:
> Add CONFIG_SHA256, CONFIG_SHA256, CONFIG_SHA_PROG_HW_ACCEL, CONFIG_SHA_HW_ACCEL to Kconfig.
> Modify README for these configs descriptions.
>
> Changes in v3:
> Remove duplication of code and create function gen_hash_type.
> Modify MAX_SG to MAX_SG_32
>
> Changes in v2:
> Merge to common functions for SHA1 and SHA256.
> Replace malloc and memset with calloc.
> Remove cast conversions for void* pointers.
> Replace hardcoded errors with Macros.
> Corrected comments.
>
>  Kconfig                       |   4 +-
>  README                        |  23 +++++--
>  common/hash.c                 |  10 +++
>  drivers/crypto/fsl/fsl_hash.c | 138 +++++++++++++++++++++++++++++++++++++++++-
>  drivers/crypto/fsl/fsl_hash.h |  34 +++++++++++
>  include/fsl_sec.h             |  26 ++++++++
>  include/hw_sha.h              |  41 ++++++++++++-
>  lib/Kconfig                   |  38 ++++++++++++
>  8 files changed, 307 insertions(+), 7 deletions(-)
>  create mode 100644 drivers/crypto/fsl/fsl_hash.h
>
> diff --git a/Kconfig b/Kconfig
> index fed488f..c4afb82 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -121,7 +121,9 @@ config FIT_SIGNATURE
>         select RSA
>         help
>           This option enables signature verification of FIT uImages,
> -         using a hash signed and verified using RSA.
> +         using a hash signed and verified using RSA. If
> +         CONFIG_SHA_PROG_HW_ACCEL is defined, i.e support for progressive
> +         hashing is available using hardware, RSA library will be using it.

s/will be using it/will use it/

>           See doc/uImage.FIT/signature.txt for more details.
>
>  config SYS_EXTRA_OPTIONS
> diff --git a/README b/README
> index cac7978..7a3f1e7 100644
> --- a/README
> +++ b/README
> @@ -3149,8 +3149,21 @@ CBFS (Coreboot Filesystem) support
>                 Enable the hash verify command (hash -v). This adds to code
>                 size a little.
>
> -               CONFIG_SHA1 - support SHA1 hashing
> -               CONFIG_SHA256 - support SHA256 hashing
> +               CONFIG_SHA1 - This option enables support of hashing using SHA1
> +               algorithm. The hash is calculated in software.
> +               CONFIG_SHA256 - This option enables support of hashing using
> +               SHA256 algorithm. The hash is calculated in software.
> +               CONFIG_SHA_HW_ACCEL - This option enables calculation of hash
> +               using SHA1/SHA256 algorithm in hardware. The files using
> +               hash_lookup_algo function would get pointer to structure having
> +               hardware accelerated SHA support. The hash command would
> +               automatically use hardware support if this option is enabled.
> +               CONFIG_SHA_PROG_HW_ACCEL - This option enables SHA1 or SHA256
> +               progressive hashing using hardware acceleration. The
> +               hash_progressive_lookup_algo function would return pointer to
> +               structure having support for progressive hashing in hardware.
> +               FIT_SIGNATURE which uses this function would automatically
> +               use hardware support if this option is enabled.

The use of tense here bothers me - here are some suggestions. Can you
please try to incorporate this to simplify the message?

CONFIG_SHA_HW_ACCEL - This option enables hardware acceleration for
SHA1/SHA256 hashing.

This affects the 'hash' command and also hash_lookup_algo().

CONFIG_SHA_PROG_HW_ACCEL - This option enables hardware-acceleration
for SHA1/SHA256 progressive hashing. Data can be streamed in a block
at a time and the hashing is performed in hardware.

This affects hash_progressive_lookup_algo() and also verified boot.

> @@ -3444,8 +3457,10 @@ FIT uImage format:
>
>                 CONFIG_FIT_SIGNATURE
>                 This option enables signature verification of FIT uImages,
> -               using a hash signed and verified using RSA. See
> -               doc/uImage.FIT/signature.txt for more details.
> +               using a hash signed and verified using RSA. If
> +               CONFIG_SHA_PROG_HW_ACCEL is defined, i.e support for progressive
> +               hashing is available using hardware, RSA library will be using it.

s/be using it/use it/

> +               See doc/uImage.FIT/signature.txt for more details.
>
>                 WARNING: When relying on signed FIT images with required
>                 signature check the legacy image format is default

[snip]

> diff --git a/lib/Kconfig b/lib/Kconfig
> index a1f30a2..e3dd7ad 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -29,4 +29,42 @@ config SYS_HZ
>
>  source lib/rsa/Kconfig
>
> +menu "Hashing Support"
> +
> +config SHA1
> +       bool "Enable SHA1 support"
> +       help
> +         This option enables support of hashing using SHA1 algorithm.
> +         The hash is calculated in software.
> +         The SHA1 algorithm produces a 160bits (20-byte) hash value
> +         termed as digest size.

How about "a 160-bit (20-byte) hash value (digest)"

> +
> +config SHA256
> +       bool "Enable SHA256 support"
> +       help
> +         This option enables support of hashing using SHA256 algorithm.
> +         The hash is calculated in software.
> +         The SHA256 algorithm produces a 256bits (32-byte) hash value
> +         termed as digest size.

How about "a 256-bit (32-byte) hash value (digest)"

> +
> +config SHA_HW_ACCEL
> +       bool "Enable hashing using hardware"
> +       help
> +         This option enables calculation of hash using SHA1/SHA256 algorithm
> +         in hardware. The files using hash_lookup_algo function would get
> +         pointer to structure having hardware accelerated SHA support. The
> +         hash command would automatically use hardware support if this option
> +         is enabled.

Again please try to remove 'would'., and below.

> +
> +config SHA_PROG_HW_ACCEL
> +       bool "Enable Progressive hashing support using hardware"
> +       depends on SHA_HW_ACCEL
> +       help
> +         This option enables SHA1 or SHA256 progressive hashing using hardware
> +         acceleration. The hash_progressive_lookup_algo function would return
> +         pointer to structure having support for progressive hashing in
> +         hardware. FIT_SIGNATURE which uses this function would automatically
> +         use hardware support if this option is enabled.
> +endmenu
> +
>  endmenu
> --
> 1.8.1.4
>

Regards,
Simon