On 10/15/21 3:30 PM, Pali Rohár wrote:
On Friday 15 October 2021 09:35:43 Alex G. wrote:
On 10/15/21 6:34 AM, Pali Rohár wrote:
On Wednesday 06 October 2021 17:05:24 Alex G. wrote:
Hi Jernej,

On 10/6/21 4:27 PM, Jernej Škrabec wrote:
Hi everyone!

Commit cb9faa6f98ae ("tools: Use a single target-independent config to enable
OpenSSL") recently introduced option to disable usage of OpenSSL via
CONFIG_TOOLS_LIBCRYPTO. However, just a bit later, another commit b4f3cc2c42d9
("tools: kwbimage: Do not hide usage of secure header under
CONFIG_ARMADA_38X") made U-Boot tools hard dependent on OpenSSL. That totally
defeats the purpose of first commit. I suggest that it gets reverted.

I would like disable OpenSSL for my usage, since it gives me troubles when
cross-compiling U-Boot inside LibreELEC build system. It's not needed for our
case anyway.

Best regards,


Can you please give the following diff a try, and if it works for you, submit 
as patch?

This change is incorrect and will break mvebu builds. mvebu requires
kwbimage for building boot images and so you cannot disable it or make
it optional.


If kwbimage is required and missing the CI builds and tests don't catch
that. I ran buildman with the change, and nothing broke. Sounds like that
needs to be addressed.

It is possible that tests do not covert all scenarios.

That being said, I'm not okay with making everyone a slave to OpenSSL
because of any given platform.

I propose to revert commit b4f3cc2c42d9 ("tools: kwbimage: Do not hide usage
of secure header under CONFIG_ARMADA_38X"), and rework it such that it
doesn't force libcrypto on everyone. And we very likely need a CI test
against libcrypto linkage when TOOLS_LIBCRYPTO is not set.

Reverting that commit is not a solution as it can lead to broken
kwbimage (when crypto stuff is not enabled). Plus there is lot of other
changes and fixes in kwboot and kwbimage...

There are lots of CI tests that do not build usable images. I caution against conflating testability with usability, as anyone who intends to run images on hardware will likely have done their diligence and enabled TOOLS_LIBCRYPTO.

Alex



Some information with another approach how to solve build issues are in
this email:
https://lore.kernel.org/u-boot/20211015114735.rig3e4cuc7mn6a7e@pali/

Alex


diff --git a/tools/Makefile b/tools/Makefile
index 4a86321f64..7f72ff9645 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -96,7 +96,8 @@ AES_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/aes/, \

   # Cryptographic helpers that depend on openssl/libcrypto
   LIBCRYPTO_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/, \
-                                       fdt-libcrypto.o)
+                                       fdt-libcrypto.o) \
+                                       kwbimage.o

   ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o

@@ -117,7 +118,6 @@ dumpimage-mkimage-objs := aisimage.o \
                        imximage.o \
                        imx8image.o \
                        imx8mimage.o \
-                       kwbimage.o \
                        lib/md5.o \
                        lpc32xximage.o \
                        mxsimage.o \
@@ -169,8 +169,8 @@ 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),)
+# MXSImage needs LibSSL <- Nope! Read the frogging notice at the top
+ifneq ($(CONFIG_TOOLS_LIBCRYPTO),)
   HOSTCFLAGS_kwbimage.o += \
        $(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
   HOSTLDLIBS_mkimage += \

Reply via email to