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

2021-10-19 Thread Andre Przywara
On Tue, 19 Oct 2021 08:28:44 -0500
Samuel Holland  wrote:

Hi Samuel,

> On 10/19/21 5:41 AM, Andre Przywara wrote:
> > On Mon, 18 Oct 2021 09:09:04 -0500
> > "Alex G."  wrote:
> > 
> > Hi,
> >   
> >> On 10/14/21 10:19 PM, 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 
> >>
> >> NAK.
> >>
> >> The intent, as detailed in tools/Makefile, is to _NOT_ to conflate 
> >> target options with tools options.  
> > 
> > I am a bit undecided, because I think the intent was more for *just*
> > building mkimage (tools-only_defconfig, for the u-boot-tools distro
> > package, for instance). (Which doesn't seem to work, btw, with or without
> > this patch.)  
> 
> TOOLS_LIBCRYPTO=n works for me with this patch, and I just
> double-checked and verified that mkimage is compiled/linked without OpenSSL.

For tools-only_defconfig and "make tools"?
I see that the SSL linking errors vanish, but I still get this instead:
$ make -s tools
/usr/bin/ld: tools/image-host.o: in function `fit_image_setup_sig':
image-host.c:(.text+0xec): undefined reference to `image_get_checksum_algo'
/usr/bin/ld: image-host.c:(.text+0xfc): undefined reference to 
`image_get_crypto_algo'
/usr/bin/ld: image-host.c:(.text+0x108): undefined reference to 
`image_get_padding_algo'
/usr/bin/ld: tools/image-host.o: in function `fit_config_add_verification_data':
image-host.c:(.text+0x7b0): undefined reference to `fit_region_make_list'
/usr/bin/ld: tools/image-host.o: in function `fit_check_sign':
image-host.c:(.text+0x1548): undefined reference to `fit_config_verify'
/usr/bin/ld: tools/common/image-fit.o: in function `fit_image_verify_with_data':
image-fit.c:(.text+0x17c0): undefined reference to 
`fit_image_verify_required_sigs'
/usr/bin/ld: image-fit.c:(.text+0x19e0): undefined reference to 
`fit_image_check_sig'
/usr/bin/ld: tools/common/image-fit.o: in function `fit_image_load':
image-fit.c:(.text+0x27ec): undefined reference to `fit_config_verify'
collect2: error: ld returned 1 exit status
make[1]: *** [scripts/Makefile.host:104: tools/dumpimage] Error 1
make: *** [Makefile:1800: tools] Error 2
(On Ubuntu 20.04 arm64)

> 
> > However just building mkimage because it's needed to create a certain
> > board firmware is a different story, I think, and including OpenSSL (if
> > the platform requires that) is hardly a user's choice at this point.
> > 
> > But anyway: Samuel, what is the actual problem this patch is solving?  
> 
> The actual problem is that TOOLS_LIBCRYPTO=n is broken, and would be
> further broken by adding sunxi_toc0.o to dumpimage-mkimage-objs. Fixing
> that requires moving objects that depend on OpenSSL to LIBCRYPTO_OBJS,
> and adding sunxi_toc0.o there in patch 2.

Right, I was just confused because it is still broken for me (see above).

> > TOOLS_LIBCRYPTO is default y, so normally (make foo_defconfig; make)
> > everything should be fine? And it only breaks if a user deliberately and
> > manually deselects it, between "make foo_defconfig" and "make"?
> > 
> > So this patch is somewhat optional, at least for the purpose of TOC0
> > support?  
> 
> The Makefile changes are needed for TOC0 support. The Kconfig changes
> are not.

Right, I see.

> And I think the only controversial part of this patch is the
> "select TOOLS_LIBCRYPTO" lines. So I suggest omitting all of the Kconfig
> changes from this patch (and removing those lines from the commit
> message). I can send v4 or you can fix it up.

You should probably send a v4, so that we get Alex' OK for that.
I need to test on actual non-secure hardware tonight, to verify that it
still works (which I strongly assume now, from looking at the code). Then
I would be eager to merge it, since it booted my Remix Mini PC beautifully.

Cheers,
Andre

> >   
> >> Disabling openssl libs is purely at the user's discretion. If platforms 
> >> can't build a usable image, I suggest just printing a loud warning 
> >> instead of overriding the user.
> >>
> >> Alex
> >>  
> >>> ---
> >>>
> >>> 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 

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

2021-10-19 Thread Samuel Holland
On 10/19/21 5:41 AM, Andre Przywara wrote:
> On Mon, 18 Oct 2021 09:09:04 -0500
> "Alex G."  wrote:
> 
> Hi,
> 
>> On 10/14/21 10:19 PM, 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   
>>
>> NAK.
>>
>> The intent, as detailed in tools/Makefile, is to _NOT_ to conflate 
>> target options with tools options.
> 
> I am a bit undecided, because I think the intent was more for *just*
> building mkimage (tools-only_defconfig, for the u-boot-tools distro
> package, for instance). (Which doesn't seem to work, btw, with or without
> this patch.)

TOOLS_LIBCRYPTO=n works for me with this patch, and I just
double-checked and verified that mkimage is compiled/linked without OpenSSL.

> However just building mkimage because it's needed to create a certain
> board firmware is a different story, I think, and including OpenSSL (if
> the platform requires that) is hardly a user's choice at this point.
> 
> But anyway: Samuel, what is the actual problem this patch is solving?

The actual problem is that TOOLS_LIBCRYPTO=n is broken, and would be
further broken by adding sunxi_toc0.o to dumpimage-mkimage-objs. Fixing
that requires moving objects that depend on OpenSSL to LIBCRYPTO_OBJS,
and adding sunxi_toc0.o there in patch 2.

> TOOLS_LIBCRYPTO is default y, so normally (make foo_defconfig; make)
> everything should be fine? And it only breaks if a user deliberately and
> manually deselects it, between "make foo_defconfig" and "make"?
> 
> So this patch is somewhat optional, at least for the purpose of TOC0
> support?

The Makefile changes are needed for TOC0 support. The Kconfig changes
are not. And I think the only controversial part of this patch is the
"select TOOLS_LIBCRYPTO" lines. So I suggest omitting all of the Kconfig
changes from this patch (and removing those lines from the commit
message). I can send v4 or you can fix it up.

Regards,
Samuel

> Cheers,
> Andre
> 
>> Disabling openssl libs is purely at the user's discretion. If platforms 
>> can't build a usable image, I suggest just printing a loud warning 
>> instead of overriding the user.
>>
>> Alex
>>
>>> ---
>>>
>>> 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 
>>> 

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

2021-10-19 Thread Andre Przywara
On Mon, 18 Oct 2021 09:09:04 -0500
"Alex G."  wrote:

Hi,

> On 10/14/21 10:19 PM, 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   
> 
> NAK.
> 
> The intent, as detailed in tools/Makefile, is to _NOT_ to conflate 
> target options with tools options.

I am a bit undecided, because I think the intent was more for *just*
building mkimage (tools-only_defconfig, for the u-boot-tools distro
package, for instance). (Which doesn't seem to work, btw, with or without
this patch.)
However just building mkimage because it's needed to create a certain
board firmware is a different story, I think, and including OpenSSL (if
the platform requires that) is hardly a user's choice at this point.

But anyway: Samuel, what is the actual problem this patch is solving?
TOOLS_LIBCRYPTO is default y, so normally (make foo_defconfig; make)
everything should be fine? And it only breaks if a user deliberately and
manually deselects it, between "make foo_defconfig" and "make"?

So this patch is somewhat optional, at least for the purpose of TOC0
support?

Cheers,
Andre

> Disabling openssl libs is purely at the user's discretion. If platforms 
> can't build a usable image, I suggest just printing a loud warning 
> instead of overriding the user.
> 
> Alex
> 
> > ---
> > 
> > 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 \
> >  

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

2021-10-18 Thread Alex G.

On 10/14/21 10:19 PM, 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 


NAK.

The intent, as detailed in tools/Makefile, is to _NOT_ to conflate 
target options with tools options.


Disabling openssl libs is purely at the user's discretion. If platforms 
can't build a usable image, I suggest just printing a loud warning 
instead of overriding the user.


Alex


---

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=0x
  HOST_EXTRACFLAGS  += -DCONFIG_FIT_CIPHER
-endif
  
-# MXSImage needs LibSSL

-ifneq 
($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_TOOLS_LIBCRYPTO),)
  

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

2021-10-15 Thread Pali Rohár
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-ka...@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-ka...@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 
> ---
> 
> 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 \
>

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

2021-10-14 Thread Samuel Holland
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 
---

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=0x
 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 
  */
 
-#ifdef CONFIG_MXS
-
 #include 
 #include 
 #include 
@@ -2363,4 +2361,3 @@