u-boot.lists.denx.de archive mirror
 help / color / mirror / Atom feed
From: "Pali Rohár" <pali@kernel.org>
To: Samuel Holland <samuel@sholland.org>
Cc: u-boot@lists.denx.de, Jagan Teki <jagan@amarulasolutions.com>,
	Andre Przywara <andre.przywara@arm.com>,
	Alexandru Gagniuc <mr.nuke.me@gmail.com>,
	Chris Packham <judge.packham@gmail.com>,
	"NXP i.MX U-Boot Team" <uboot-imx@nxp.com>,
	Naoki Hayama <naoki.hayama@lineo.co.jp>,
	Joel Stanley <joel@jms.id.au>
Subject: Re: [PATCH v3 1/4] tools: Separate image types which depend on OpenSSL
Date: Fri, 15 Oct 2021 13:47:35 +0200	[thread overview]
Message-ID: <20211015114735.rig3e4cuc7mn6a7e@pali> (raw)
In-Reply-To: <20211015031916.44461-2-samuel@sholland.org>

Hello! I did more investigation for this topic during working on kwboot
improvements, which resulted in this patch series:
https://lore.kernel.org/u-boot/20210924210716.29752-1-kabel@kernel.org/

The result is that kwbimage is used only for: Kirkwood, Dove, A370, AXP,
A375, A38x, A39x and MSYS. This information is now also in manpage:
https://lore.kernel.org/u-boot/20210924210716.29752-39-kabel@kernel.org/

So, Orion, Discovery, A3700, A7k, A8k and CN913x does *not* use kwbimage
and so they do do not need to select libcrypto.

Therefore you can drop selection of libcrypto from ARCH_ORION5X.

ARCH_KIRKWOOD includes only Kirkwood (which required kwbimage), but does
not use any crypto. So selecting unused crypto seems suspicious for
anybody who look at it... And it is there only for transitional kwbimage
dependency, which is very unintuitive.

With ARCH_MVEBU it is quite complicated as it is used for both 32-bit
SoCs (XP, 375, 38x) and also 64-bit SoCs (3700, 7k, 8k). kwbimage is
used only for 32-bit mvebu SoCs.

I do not know how to express this correctly.

But I would rather suggest to introduce a new option which specify that
tools/kwbimage.o is required and this dependency would selects its
direct (crypto) dependency. Otherwise nobody would remember in future
why non-crypto platforms are selecting crypto functionality, or why
64-bit platforms are enabling crypto functionality which is not used at
all.

On Thursday 14 October 2021 22:19:13 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.
> 
> When building for platforms which use those image types, automatically
> select TOOLS_LIBCRYPTO, since it is required for the build to complete.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
> 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
> 
>  arch/arm/Kconfig              |  3 +++
>  arch/arm/mach-imx/mxs/Kconfig |  2 ++
>  scripts/config_whitelist.txt  |  1 -
>  tools/Makefile                | 19 +++++--------------
>  tools/mxsimage.c              |  3 ---
>  5 files changed, 10 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index d8c041a877..380ad4f670 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -566,6 +566,7 @@ config ARCH_KIRKWOOD
>  	select BOARD_EARLY_INIT_F
>  	select CPU_ARM926EJS
>  	select GPIO_EXTRA_HEADER
> +	select TOOLS_LIBCRYPTO
>  
>  config ARCH_MVEBU
>  	bool "Marvell MVEBU family (Armada XP/375/38x/3700/7K/8K)"
> @@ -580,12 +581,14 @@ config ARCH_MVEBU
>  	select OF_CONTROL
>  	select OF_SEPARATE
>  	select SPI
> +	select TOOLS_LIBCRYPTO
>  	imply CMD_DM
>  
>  config ARCH_ORION5X
>  	bool "Marvell Orion"
>  	select CPU_ARM926EJS
>  	select GPIO_EXTRA_HEADER
> +	select TOOLS_LIBCRYPTO
>  
>  config TARGET_STV0991
>  	bool "Support stv0991"
> diff --git a/arch/arm/mach-imx/mxs/Kconfig b/arch/arm/mach-imx/mxs/Kconfig
> index b2026a3758..6f138d25e9 100644
> --- a/arch/arm/mach-imx/mxs/Kconfig
> +++ b/arch/arm/mach-imx/mxs/Kconfig
> @@ -3,6 +3,7 @@ if ARCH_MX23
>  config MX23
>  	bool
>  	default y
> +	select TOOLS_LIBCRYPTO
>  
>  choice
>  	prompt "MX23 board select"
> @@ -34,6 +35,7 @@ if ARCH_MX28
>  config MX28
>  	bool
>  	default y
> +	select TOOLS_LIBCRYPTO
>  
>  choice
>  	prompt "MX28 board select"
> diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
> index 3a6865dc70..bea6b6f83b 100644
> --- a/scripts/config_whitelist.txt
> +++ b/scripts/config_whitelist.txt
> @@ -838,7 +838,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
> 

  reply	other threads:[~2021-10-15 11:47 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-15  3:19 [PATCH v3 0/4] sunxi: TOC0 image type support Samuel Holland
2021-10-15  3:19 ` [PATCH v3 1/4] tools: Separate image types which depend on OpenSSL Samuel Holland
2021-10-15 11:47   ` Pali Rohár [this message]
2021-10-18 14:09   ` Alex G.
2021-10-19 10:41     ` Andre Przywara
2021-10-19 13:28       ` Samuel Holland
2021-10-19 14:43         ` Andre Przywara
2021-10-15  3:19 ` [PATCH v3 2/4] tools: mkimage: Add Allwinner TOC0 support Samuel Holland
2021-10-15  3:19 ` [PATCH v3 3/4] sunxi: Support SPL in both eGON and TOC0 images Samuel Holland
2021-10-15  3:19 ` [PATCH v3 4/4] sunxi: Support building a SPL as a TOC0 image Samuel Holland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211015114735.rig3e4cuc7mn6a7e@pali \
    --to=pali@kernel.org \
    --cc=andre.przywara@arm.com \
    --cc=jagan@amarulasolutions.com \
    --cc=joel@jms.id.au \
    --cc=judge.packham@gmail.com \
    --cc=mr.nuke.me@gmail.com \
    --cc=naoki.hayama@lineo.co.jp \
    --cc=samuel@sholland.org \
    --cc=u-boot@lists.denx.de \
    --cc=uboot-imx@nxp.com \
    --subject='Re: [PATCH v3 1/4] tools: Separate image types which depend on OpenSSL' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).