Re: [PATCH] Revert "tree: imx: remove old fit generator script"

7 Jan 2022

On Thu, Jan 6, 2022 at 12:27 PM Tim Harvey tharvey@gateworks.com wrote:
...
On Thu, Jan 6, 2022 at 11:18 AM ZHIZHIKIN Andrey
andrey.zhizhikin@leica-geosystems.com wrote:
...
Hello Tom,
...
-----Original Message-----
From: U-Boot u-boot-bounces@lists.denx.de On Behalf Of Tom Rini
Sent: Thursday, January 6, 2022 7:52 PM
To: u-boot@lists.denx.de
Cc: Tim Harvey tharvey@gateworks.com
Subject: [PATCH] Revert "tree: imx: remove old fit generator script"
This reverts commit d9a6f0eed66a39206b13513ec914f14084c3bb73.
For right now, it's too close to the release to merge the series that
allows for binman to be used to generate the final images, and also not
break CI, and then also merge all of the series that convert currently
broken platforms to use binman instead.  So, bring back this script now
and remove it again for real after the release.
Please note that this might not work, as the FIT generator script would
generate ITS with '@' symbols which are not compatible with mkimage due
to CVE-2021-27138. This revert should be complemented with the fix to
remove those '@' symbols as well.
Correct, the revert is not enough anymore:
  MKIMAGE u-boot.itb
u-boot.its:7.11-15.5: Warning (unit_address_vs_reg): /images/uboot@1:
node has a unit name, but no reg property
u-boot.its:16.9-21.5: Warning (unit_address_vs_reg): /images/fdt@1:
node has a unit name, but no reg property
u-boot.its:22.9-27.5: Warning (unit_address_vs_reg): /images/fdt@2:
node has a unit name, but no reg property
u-boot.its:28.9-33.5: Warning (unit_address_vs_reg): /images/fdt@3:
node has a unit name, but no reg property
u-boot.its:34.9-39.5: Warning (unit_address_vs_reg): /images/fdt@4:
node has a unit name, but no reg property
u-boot.its:40.9-45.5: Warning (unit_address_vs_reg): /images/fdt@5:
node has a unit name, but no reg property
u-boot.its:46.9-55.5: Warning (unit_address_vs_reg): /images/atf@1:
node has a unit name, but no reg property
u-boot.its:60.12-65.5: Warning (unit_address_vs_reg):
/configurations/config@1: node has a unit name, but no reg property
u-boot.its:66.12-71.5: Warning (unit_address_vs_reg):
/configurations/config@2: node has a unit name, but no reg property
u-boot.its:72.12-77.5: Warning (unit_address_vs_reg):
/configurations/config@3: node has a unit name, but no reg property
u-boot.its:78.12-83.5: Warning (unit_address_vs_reg):
/configurations/config@4: node has a unit name, but no reg property
u-boot.its:84.12-89.5: Warning (unit_address_vs_reg):
/configurations/config@5: node has a unit name, but no reg property
./tools/mkimage: verify_header failed for FIT Image support with exit code 1
Makefile:1433: recipe for target 'u-boot.itb' failed
make: *** [u-boot.itb] Error 1
make: *** Deleting file 'u-boot.itb'
make: *** Waiting for unfinished jobs....
I don't know what had changed to cause this or when (again, I stopped
worrying about it because I thought we were moving to binman for this
release). There was a patch that resolved this from Oliver at
https://lists.denx.de/pipermail/u-boot/2021-August/457997.html but I
don't think that fully solves anything 'at this point' either.
Even with that applied to current master I then end up with:
  MKIMAGE flash.bin
./tools/mkimage: Can't open spl/u-boot-spl-ddr.bin: No such file or directory
arch/arm/mach-imx/Makefile:167: recipe for target 'flash.bin' failed
make[1]: *** [flash.bin] Error 1
make[1]: *** Deleting file 'flash.bin'
Makefile:1526: recipe for target 'flash.bin' failed
At some point over the past couple of months that patch resolved the
building issue when using the FIT generator but I also don't know what
else has changed that now causes that to not work.
As Tom pointed out in another thread these build failures did not get
caught by CI apparently because CI does a 'make all' which did not
include the FIT images (that was accomplished with the 'flash.bin'
target prior to binman conversion).
Is it too late to apply the CI fix and the pending binman conversions?
I know that my series has been reviewed by Marcel [1] and as far as I
know didn't get merged simply because of the CI issue. It still
applies and produces a valid flash.bin image.
I was also able to merge Peng's series [2] which converts
imx8mq_evk/imx8mq_phanbell/pico-imx8mq to binman and was able to build
flash.bin images for them
I tried to merge Adam's series that moves imx8mm_beacon to binman [3]
and imx8mn_beacon to binman [4] but they no longer apply due to
defconfig/Kconfig changes
That still leaves the following unbuildable with
CONFIG_SPL_FIT_GENERATOR = "arch/arm/mach-imx/mkimage_fit_atf.sh":
configs/cgtqmx8_defconfig:CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-imx/mkimage_fit_atf.sh"
configs/imx8mm-icore-mx8mm-ctouch2_defconfig:CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-imx/mkimage_fit_atf.sh"
configs/imx8mm-icore-mx8mm-edimm2.2_defconfig:CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-imx/mkimage_fit_atf.sh"
configs/imx8mm_beacon_defconfig:CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-imx/mkimage_fit_atf.sh"
configs/imx8mn_beacon_2g_defconfig:CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-imx/mkimage_fit_atf.sh"
configs/imx8mn_beacon_defconfig:CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-imx/mkimage_fit_atf.sh"
configs/imx8qm_rom7720_a1_4G_defconfig:CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-imx/mkimage_fit_atf.sh"
Tim
[1] https://patchwork.ozlabs.org/project/uboot/list/?series=265765
[2] https://patchwork.ozlabs.org/project/uboot/list/?series=268380
[3] https://patchwork.ozlabs.org/project/uboot/list/?series=261640
[4] https://patchwork.ozlabs.org/project/uboot/list/?series=261822
Tom,
I'm not familiar with the U-boot CI tool. Is it a show-stopper that it
does not build for boards using binman for release? From what you
mentioned in another thread it was never building the flash.bin target
for the boards using the FIT generator anyway.
Best regards,
Tim
...
...
...
Suggested-by: Tim Harvey tharvey@gateworks.com
Signed-off-by: Tom Rini trini@konsulko.com

Makefile                             |   3 +
 arch/arm/mach-imx/mkimage_fit_atf.sh | 143 +++++++++++++++++++++++++++
 2 files changed, 146 insertions(+)
 create mode 100755 arch/arm/mach-imx/mkimage_fit_atf.sh

diff --git a/Makefile b/Makefile
index ae9bfab91acf..0c478645c3a7 100644
--- a/Makefile
+++ b/Makefile
@@ -1343,6 +1343,9 @@ $(U_BOOT_ITS): $(subst ",,$(CONFIG_SPL_FIT_SOURCE))
 else
 ifneq ($(CONFIG_USE_SPL_FIT_GENERATOR),)
 U_BOOT_ITS := u-boot.its
+ifeq ($(CONFIG_SPL_FIT_GENERATOR),"arch/arm/mach-imx/mkimage_fit_atf.sh")
+U_BOOT_ITS_DEPS += u-boot-nodtb.bin
+endif
 ifeq ($(CONFIG_SPL_FIT_GENERATOR),"arch/arm/mach-rockchip/make_fit_atf.py")
 U_BOOT_ITS_DEPS += u-boot
 endif
diff --git a/arch/arm/mach-imx/mkimage_fit_atf.sh b/arch/arm/mach-
imx/mkimage_fit_atf.sh
new file mode 100755
index 000000000000..2a17968794c1
--- /dev/null
+++ b/arch/arm/mach-imx/mkimage_fit_atf.sh
@@ -0,0 +1,143 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0+
+#
+# script to generate FIT image source for i.MX8MQ boards with
+# ARM Trusted Firmware and multiple device trees (given on the command line)
+#
+# usage: $0 <dt_name> [<dt_name> [<dt_name] ...]



+[ -z "$BL31" ] && BL31="bl31.bin"
+[ -z "$TEE_LOAD_ADDR" ] && TEE_LOAD_ADDR="0xfe000000"
+[ -z "$ATF_LOAD_ADDR" ] && ATF_LOAD_ADDR="0x00910000"
+[ -z "$BL33_LOAD_ADDR" ] && BL33_LOAD_ADDR="0x40200000"



+if [ ! -f $BL31 ]; then

echo "ERROR: BL31 file $BL31 NOT found" >&2


exit 0



+else

echo "$BL31 size: " >&2


stat -c %s $BL31 >&2



+fi



+BL32="tee.bin"



+if [ ! -f $BL32 ]; then

BL32=/dev/null



+else

echo "Building with TEE support, make sure your $BL31 is compiled with



spd. If you do not want tee, please delete $BL31" >&2

echo "$BL32 size: " >&2


stat -c %s $BL32 >&2



+fi



+BL33="u-boot-nodtb.bin"



+if [ ! -f $BL33 ]; then

echo "ERROR: $BL33 file NOT found" >&2


exit 0



+else

echo "u-boot-nodtb.bin size: " >&2


stat -c %s u-boot-nodtb.bin >&2



+fi



+for dtname in $*
+do

echo "$dtname size: " >&2


stat -c %s $dtname >&2



+done




+cat << __HEADER_EOF
+/dts-v1/;



+/ {

description = "Configuration to load ATF before U-Boot";



images {


        uboot@1 {



This (and all other similar places) would be rejected by mkimage... :(
...

                description = "U-Boot (64-bit)";


                os = "u-boot";


                data = /incbin/("$BL33");


                type = "standalone";


                arch = "arm64";


                compression = "none";


                load = <$BL33_LOAD_ADDR>;


        };



+__HEADER_EOF



+cnt=1
+for dtname in $*
+do

cat << __FDT_IMAGE_EOF


        fdt@$cnt {


                description = "$(basename $dtname .dtb)";


                data = /incbin/("$dtname");


                type = "flat_dt";


                compression = "none";


        };



+__FDT_IMAGE_EOF
+cnt=$((cnt+1))
+done



+cat << __HEADER_EOF

        atf@1 {


                description = "ARM Trusted Firmware";


                os = "arm-trusted-firmware";


                data = /incbin/("$BL31");


                type = "firmware";


                arch = "arm64";


                compression = "none";


                load = <$ATF_LOAD_ADDR>;


                entry = <$ATF_LOAD_ADDR>;


        };



+__HEADER_EOF



+if [ -f $BL32 ]; then
+cat << __HEADER_EOF

        tee@1 {


                description = "TEE firmware";


                data = /incbin/("$BL32");


                type = "firmware";


                arch = "arm64";


                compression = "none";


                load = <$TEE_LOAD_ADDR>;


                entry = <$TEE_LOAD_ADDR>;


        };



+__HEADER_EOF
+fi



+cat << __CONF_HEADER_EOF

};


configurations {


        default = "config@1";




+__CONF_HEADER_EOF



+cnt=1
+for dtname in $*
+do
+if [ -f $BL32 ]; then
+cat << __CONF_SECTION_EOF

        config@$cnt {


                description = "$(basename $dtname .dtb)";


                firmware = "uboot@1";


                loadables = "atf@1", "tee@1";


                fdt = "fdt@$cnt";


        };



+__CONF_SECTION_EOF
+else
+cat << __CONF_SECTION1_EOF

        config@$cnt {


                description = "$(basename $dtname .dtb)";


                firmware = "uboot@1";


                loadables = "atf@1";


                fdt = "fdt@$cnt";


        };



+__CONF_SECTION1_EOF
+fi
+cnt=$((cnt+1))
+done



+cat << __ITS_EOF

};



+};
+__ITS_EOF
2.25.1
-- andrey

    