Re: [PATCH v4 1/4] tools: Separate image types which depend on OpenSSL

Pali Rohár
2021-10-20T07:29:39Z

On Tuesday 19 October 2021 21:44:51 Samuel Holland wrote:
> Some image types (kwbimage and mxsimage) always depend on OpenSSL, so
> they can only be included in mkimage when TOOLS_LIBCRYPTO is selected.
> Use Makefile logic to conditionally link the files.
>
> Signed-off-by: Samuel Holland <samuel@sholland.org>

NAK.

As explained in previous email [1], kwbimage is required for building
Kirkwood, Dove, A370, AXP, A375, A38x, A39x and MSYS platforms.
Therefore it cannot be disabled or hidden behind some user config
options for these platforms (and it does not matter if it is crypto
option or any other option). kwbimage must be unconditionally enabled on
these platforms like it was before this change, as it is crucial part of
build.

[1] - https://lore.kernel.org/u-boot/20211015114735.rig3e4cuc7mn6a7e@pali/

> ---
>
> Changes in v4:
> - Do not select TOOLS_LIBCRYPTO anywhere
>
> Changes in v3:
> - Selected TOOLS_LIBCRYPTO on all platforms that use kwbimage (as best
> as I can tell, using the suggestions from Pali Rohár)
>
> Changes in v2:
> - Refactored the first patch on top of TOOLS_LIBCRYPTO
>
> scripts/config_whitelist.txt | 1 -
> tools/Makefile | 19 +++++--------------
> tools/mxsimage.c | 3 ---
> 3 files changed, 5 insertions(+), 18 deletions(-)
>
> diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
> index cd94b5777a..affae6875d 100644
> --- a/scripts/config_whitelist.txt
> +++ b/scripts/config_whitelist.txt
> @@ -828,7 +828,6 @@ CONFIG_MXC_UART_BASE
> CONFIG_MXC_USB_FLAGS
> CONFIG_MXC_USB_PORT
> CONFIG_MXC_USB_PORTSC
> -CONFIG_MXS
> CONFIG_MXS_AUART
> CONFIG_MXS_AUART_BASE
> CONFIG_MXS_OCOTP
> diff --git a/tools/Makefile b/tools/Makefile
> index 999fd46531..a9b3d982d8 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -94,9 +94,11 @@ ECDSA_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/ecdsa/, ecdsa-libcrypto.
> AES_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/aes/, \
> aes-encrypt.o aes-decrypt.o)
>
> -# Cryptographic helpers that depend on openssl/libcrypto
> -LIBCRYPTO_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/, \
> - fdt-libcrypto.o)
> +# Cryptographic helpers and image types that depend on openssl/libcrypto
> +LIBCRYPTO_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := \
> + lib/fdt-libcrypto.o \
> + kwbimage.o \
> + mxsimage.o
>
> ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o
>
> @@ -118,10 +120,8 @@ dumpimage-mkimage-objs := aisimage.o \
> imximage.o \
> imx8image.o \
> imx8mimage.o \
> - kwbimage.o \
> lib/md5.o \
> lpc32xximage.o \
> - mxsimage.o \
> omapimage.o \
> os_support.o \
> pblimage.o \
> @@ -156,22 +156,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_TOOLS_LIBCRYPTO),)
> -# 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_TOOLS_LIBCRYPTO
> # This affects include/image.h, but including the board config file
> # is tricky, so manually define this options here.
> HOST_EXTRACFLAGS += -DCONFIG_FIT_SIGNATURE
> HOST_EXTRACFLAGS += -DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff
> HOST_EXTRACFLAGS += -DCONFIG_FIT_CIPHER
> -endif
>
> -# MXSImage needs LibSSL
> -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_TOOLS_LIBCRYPTO),)
> HOSTCFLAGS_kwbimage.o += \
> $(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
> HOSTLDLIBS_mkimage += \
> diff --git a/tools/mxsimage.c b/tools/mxsimage.c
> index 002f4b525a..2bfbb421eb 100644
> --- a/tools/mxsimage.c
> +++ b/tools/mxsimage.c
> @@ -5,8 +5,6 @@
> * Copyright (C) 2012-2013 Marek Vasut <marex@denx.de>
> */
>
> -#ifdef CONFIG_MXS
> -
> #include <errno.h>
> #include <fcntl.h>
> #include <stdio.h>
> @@ -2363,4 +2361,3 @@ U_BOOT_IMAGE_TYPE(
> NULL,
> mxsimage_generate
> );
> -#endif
> --
> 2.32.0
>