[U-Boot] [PATCH 1/4] [RFC] [v2] rsa: Split the rsa-verify

[Simon Glass]

Hi Ruchika,

On 18 December 2014 at 04:22, Ruchika Gupta <ruchika.gupta at freescale.com> wrote:
>
> Public exponentiation which is required in rsa verify
> functionality is currently tightly integrated with
> verification code in rsa_verify.c. Currently this
> implementation is software based. Some platforms
> having support of the exponentiation in hardware.
> To enable the rsa verify functionality to use the
> Modular exponentiation if present in hardware, the
> patch-set splits the file into two files:
>
> 1. rsa-verify.c
> - The file parses device tree keys node to fill a keyprop
> structure. The keyprop structure can then be converted
> to implementation specific formal (struct rsa_pub_key
> for sw implementation).
> - The parsed device tree node is then passed to a generic
> rsa_mod_exp function.
>
> 2. rsa-mod-exp.c
> Move the software specific functions related to exponentiation
> from rsa-verify.c to this file. The file is compiled if
> "CONFIG_RSA_MOD_EXP_SW" is defined. In general if both
> CONFIG_FIT_SIGNATURE and CONFIG_RSA are defined,
> CONFIG_RSA_MOD_EXP_SW gets automatically defined.
>
> Platforms having hardware implementation for rsa_mod_exp can
> add a define "CONFIG_RSA_MOD_EXP_HW" to their config files.
> Adding this defined, undefs the CONFIG_RSA_MOD_EXP_SW and
> hardware implementation of mod_exp gets compiled.
>
> Signed-off-by: Ruchika Gupta <ruchika.gupta at freescale.com>
> CC: Simon Glass <sjg at chromium.org>
> ---
> Changes in v2:
> Addressed few of Simon Glass's comments:
> - Kconfig option added for RSA
> - Comments added for new keyprop struct
>
>  Kconfig                      |   2 +
>  include/config_fallbacks.h   |   8 ++
>  include/u-boot/rsa-mod-exp.h |  34 +++++
>  lib/Kconfig                  |  15 +++
>  lib/rsa/Makefile             |   1 +
>  lib/rsa/rsa-mod-exp.c        | 308 +++++++++++++++++++++++++++++++++++++++++++
>  lib/rsa/rsa-verify.c         | 307 +++++-------------------------------------
>  tools/Makefile               |   2 +-
>  8 files changed, 399 insertions(+), 278 deletions(-)
>  create mode 100644 include/u-boot/rsa-mod-exp.h
>  create mode 100644 lib/rsa/rsa-mod-exp.c
>

Here are my comments on the whole series. In general the approach
looks fine provided you move it to driver model in a follow-up series.
See the 'thermal' uclass for a very simple example.

- Check your multi-line comment style
- Make sure you add a full help paragraph for new Kconfigs
- Avoid double blank lines
- First patch should just move code out of lib/rsa/rsa-verify.c
- Add a separate patch to move CONFIG_FIT_SIGNATURE to Kconfig for
existing boards
- Make sure functions have comments describing their purpose,
arguments, return value. Normally these go in the header file except
for static functions
- For your header file guard #ifdefs, use a symbol that relates to the filename
- Word-wrap your commit messages to >70 chars (e.g. 76 or so)
- Make sure you check return values of functions that return errors
- Make sure the vboot tests still pass
   make O=sandbox sandbox_defconfig all
   O=b/sandbox ./test/vboot/vboot_test.sh
- Check your hash.c changes, I think you have the #ifdefs wrong.

Please then send a v3 without RFC and we'll try to get final things resolved.

Regards,
Simon