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

[Alex G. <mr.nuke.me@gmail.com>]

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@chromium.org>

Reviewed-by: Alexandru Gagniuc <mr.nuke.me@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.

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.

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?

> +
> +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.

> +
> +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.

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

s/verification/signing and verification/

> +
> +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.

>   endmenu
> diff --git a/tools/Makefile b/tools/Makefile
> index 2b4bc547abd..d143198f7c9 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -53,12 +53,12 @@ hostprogs-y += mkenvimage
>   mkenvimage-objs := mkenvimage.o os_support.o lib/crc32.o
>   
>   hostprogs-y += dumpimage mkimage
> -hostprogs-$(CONFIG_FIT_SIGNATURE) += fit_info fit_check_sign
> +hostprogs-$(CONFIG_HOST_FIT_SIGNATURE) += fit_info fit_check_sign
>   
>   hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
>   
> -FIT_OBJS-$(CONFIG_FIT) := fit_common.o fit_image.o image-host.o common/image-fit.o
> -FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := common/image-sig.o common/image-fit-sig.o
> +FIT_OBJS-$(CONFIG_HOST_FIT) := fit_common.o fit_image.o image-host.o common/image-fit.o
> +FIT_SIG_OBJS-$(CONFIG_HOST_FIT_SIGNATURE) := common/image-sig.o common/image-fit-sig.o
>   FIT_CIPHER_OBJS-$(CONFIG_FIT_CIPHER) := common/image-cipher.o
>   
>   # The following files are synced with upstream DTC.
> @@ -66,17 +66,17 @@ FIT_CIPHER_OBJS-$(CONFIG_FIT_CIPHER) := common/image-cipher.o
>   LIBFDT_OBJS := $(addprefix libfdt/, fdt.o fdt_ro.o fdt_wip.o fdt_sw.o fdt_rw.o \
>   		fdt_strerror.o fdt_empty_tree.o fdt_addresses.o fdt_overlay.o)
>   
> -RSA_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/rsa/, \
> +RSA_OBJS-$(CONFIG_HOST_FIT_SIGNATURE) := $(addprefix lib/rsa/, \
>   					rsa-sign.o rsa-verify.o \
>   					rsa-mod-exp.o)
>   
> -ECDSA_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/ecdsa/, ecdsa-libcrypto.o)
> +ECDSA_OBJS-$(CONFIG_HOST_FIT_SIGNATURE) := $(addprefix lib/ecdsa/, ecdsa-libcrypto.o)
>   
>   AES_OBJS-$(CONFIG_FIT_CIPHER) := $(addprefix lib/aes/, \
>   					aes-encrypt.o aes-decrypt.o)
>   
>   # Cryptographic helpers that depend on openssl/libcrypto
> -LIBCRYPTO_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/, \
> +LIBCRYPTO_OBJS-$(CONFIG_HOST_FIT_SIGNATURE) := $(addprefix lib/, \
>   					fdt-libcrypto.o)
>   
>   ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o
> @@ -137,13 +137,13 @@ fit_info-objs   := $(dumpimage-mkimage-objs) fit_info.o
>   fit_check_sign-objs   := $(dumpimage-mkimage-objs) fit_check_sign.o
>   file2include-objs := file2include.o
>   
> -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_FIT_SIGNATURE),)
> +ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_HOST_FIT_SIGNATURE),)
>   # Add CONFIG_MXS into host CFLAGS, so we can check whether or not register
>   # the mxsimage support within tools/mxsimage.c .
>   HOSTCFLAGS_mxsimage.o += -DCONFIG_MXS
>   endif
>   
> -ifdef CONFIG_FIT_SIGNATURE
> +ifdef CONFIG_HOST_FIT_SIGNATURE
>   # This affects include/image.h, but including the board config file
>   # is tricky, so manually define this options here.
>   HOST_EXTRACFLAGS	+= -DCONFIG_FIT_SIGNATURE
> @@ -165,7 +165,7 @@ HOSTCFLAGS_kwbimage.o += -DCONFIG_KWB_SECURE
>   endif
>   
>   # MXSImage needs LibSSL
> -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_FIT_SIGNATURE)$(CONFIG_FIT_CIPHER),)
> +ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_HOST_FIT_SIGNATURE)$(CONFIG_FIT_CIPHER),)
>   HOSTCFLAGS_kwbimage.o += \
>   	$(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
>   HOSTLDLIBS_mkimage += \
>