[PATCH v2 16/50] image: Add Kconfig options for FIT in the host build

[Simon Glass]

Hi Alex,

On Tue, 11 May 2021 at 13:57, Alex G. <mr.nuke.me at gmail.com> wrote:
>
> On 5/6/21 9:24 AM, Simon Glass wrote:
> > In preparation for enabling CONFIG_IS_ENABLED() on the host build, add
> > some options to enable the various FIT options expected in these tools.
> > This will ensure that the code builds correctly when CONFIG_HOST_xxx
> > is distinct from CONFIG_xxx.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
>
> Reviewed-by: Alexandru Gagniuc <mr.nuke.me at gmail.com>
>
> This makes me wonder whether we should just always enable host features.
> Right now, each defconfig can have a different mkimage config. So we
> should really have mkimage-imx8, mkimage-stm32mp, etc, which support
> different feature sets. This doesn't make much sense.

My intent is that we enable all features in host tools. For
distributions they build the tools-only config and make things work
that way. But perhaps we could avoid building the tools, or do it
separately, if there were no different between boards.

>
> The alternative is to get rid of all these configs and always enable
> mkimage features. The disadvantage is that we'd require openssl for
> building target code.
>
> A second alternative is to have a mkimage-nossl that gets built and used
> when openssl isn't available. It's really just openssl that causes such
> a schism.

Why is openssl such a problem?

>
> Alex
>
> > ---
> >
> > (no changes since v1)
> >
> >   common/image-fit-sig.c |  3 ++-
> >   common/image-fit.c     |  4 ++--
> >   tools/Kconfig          | 25 +++++++++++++++++++++++++
> >   tools/Makefile         | 18 +++++++++---------
> >   4 files changed, 38 insertions(+), 12 deletions(-)
> >
> > diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c
> > index 55ddf1879ed..12a6745c642 100644
> > --- a/common/image-fit-sig.c
> > +++ b/common/image-fit-sig.c
> > @@ -72,11 +72,12 @@ static int fit_image_setup_verify(struct image_sign_info *info,
> >       char *algo_name;
> >       const char *padding_name;
> >
> > +#ifndef USE_HOSTCC
> >       if (fdt_totalsize(fit) > CONFIG_FIT_SIGNATURE_MAX_SIZE) {
> >               *err_msgp = "Total size too large";
> >               return 1;
> >       }
> > -
> > +#endif
> >       if (fit_image_hash_get_algo(fit, noffset, &algo_name)) {
> >               *err_msgp = "Can't get hash algo property";
> >               return -1;
> > diff --git a/common/image-fit.c b/common/image-fit.c
> > index e614643fe39..a16e2dd54a5 100644
> > --- a/common/image-fit.c
> > +++ b/common/image-fit.c
> > @@ -165,7 +165,7 @@ int fit_get_subimage_count(const void *fit, int images_noffset)
> >       return count;
> >   }
> >
> > -#if CONFIG_IS_ENABLED(FIT_PRINT) || CONFIG_IS_ENABLED(SPL_FIT_PRINT)
> > +#if CONFIG_IS_ENABLED(FIT_PRINT)
> >   /**
> >    * fit_image_print_data() - prints out the hash node details
> >    * @fit: pointer to the FIT format image header
> > @@ -573,7 +573,7 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)
> >   #else
> >   void fit_print_contents(const void *fit) { }
> >   void fit_image_print(const void *fit, int image_noffset, const char *p) { }
> > -#endif /* CONFIG_IS_ENABLED(FIR_PRINT) || CONFIG_IS_ENABLED(SPL_FIT_PRINT) */
> > +#endif /* CONFIG_IS_ENABLED(FIT_PRINT) */
> >
> >   /**
> >    * fit_get_desc - get node description property
> > diff --git a/tools/Kconfig b/tools/Kconfig
> > index b2f5012240c..f00ab661135 100644
> > --- a/tools/Kconfig
> > +++ b/tools/Kconfig
> > @@ -9,4 +9,29 @@ config MKIMAGE_DTC_PATH
> >         some cases the system dtc may not support all required features
> >         and the path to a different version should be given here.
> >
> > +config HOST_FIT
> > +     def_bool y
> > +     help
> > +       Enable FIT support in the host build.
>
> Don't we always want to enable this for mkimage?

Yes, that's right.

>
> > +
> > +config HOST_FIT_FULL_CHECK
> > +     def_bool y
> > +     help
> > +       Do a full check of the FIT before using it in the host build
>
> How would this be used? FIT vs FIT_FULL is mostly an SPL distinction. I
> don't think we should have it in host tools.

It allows us to use CONFIG_IS_ENABLED() everywhere, including in code
built as part of host tools. In fact that is the main purpose of this
series.

>
> > +
> > +config HOST_FIT_PRINT
> > +     def_bool y
> > +     help
> > +       Print the content of the FIT verbosely in the host build
>
> This option also doesn't make sense.This seems to do what 'mkimage -l'
> already supports.

Are you sure you have looked at the goal of the CONFIG_IS_ENABLED()
changes? This is here purely to avoid #ifdefs in the share code.

>
> > +
> > +config HOST_FIT_SIGNATURE
> > +     def_bool y
> > +     help
> > +       Enable signature verification of FIT uImages in the host build
>
> s/verification/signing and verification/

OK, yes it does control that in the tools, by virtue of tools/Makefile

>
> > +
> > +config HOST_FIT_SIGNATURE_MAX_SIZE
> > +     hex
> > +     depends on HOST_FIT_SIGNATURE
> > +     default 0x10000000
> > +
>
> The only use of FIT_SIGNATURE_MAX_SIZE is under "#ifndef USE_HOSTCC". So
> this wouldn't make any sense for the host.

This is used in fit_image_setup_verify() which is build by tools.

[..]

Regards,
Simon