On Fri, Jan 07, 2022 at 09:27:05AM -0800, Tim Harvey wrote:
> On Thu, Jan 6, 2022 at 12:27 PM Tim Harvey <thar...@gateworks.com> wrote:
> >
> > On Thu, Jan 6, 2022 at 11:18 AM ZHIZHIKIN Andrey
> > <andrey.zhizhi...@leica-geosystems.com> wrote:
> > >
> > > Hello Tom,
> > >
> > > > -----Original Message-----
> > > > From: U-Boot <u-boot-boun...@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 <thar...@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.

It's at https://u-boot.readthedocs.io/en/latest/develop/ci_testing.html
(and in-tree under doc/develop/ci_testing.rst).

> 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.

Yes, breaking CI is a ship-stopper.  That all of these boards were not
previously building the final image as part of "all" is a problem.

So, here's what I'm at right now.  I've grabbed Heiko's patch.
Everything is currently visible at
https://patchwork.ozlabs.org/bundle/trini/2022-01-07-imx8-and-binman-updates/
and with this, we get some boards building and complaining as expected:
   aarch64:  w+   imx8mn_evk
+(imx8mn_evk) Image 'main-section' is missing external blobs and is 
non-functional: blob-ext@1
blob-ext@2 blob-ext@3 blob-ext@4
+(imx8mn_evk) Image 'main-section' is missing external blobs and is 
non-functional: blob-ext
+(imx8mn_evk)
+(imx8mn_evk) Some images are invalid

But others are:
   aarch64:  +   imx8mn_beacon_2g
+(imx8mn_beacon_2g) ===================== WARNING ======================
+(imx8mn_beacon_2g) This board uses CONFIG_SPL_FIT_GENERATOR. Please migrate
+(imx8mn_beacon_2g) to binman instead, to avoid the proliferation of
+(imx8mn_beacon_2g) arch-specific scripts with no tests.
+(imx8mn_beacon_2g) ====================================================
+(imx8mn_beacon_2g) Image 'main-section' is missing external blobs and is 
non-functional: blob-
ext@1 blob-ext@2 blob-ext@3 blob-ext@4
+(imx8mn_beacon_2g) binman: Error 1 running 'mkimage -d ./mkimage.spl.mkimage 
-n spl/u-boot-spl
.cfgout -T imx8mimage -e 0x912000 ./mkimage-out.spl.mkimage': 
spl/u-boot-spl-ddr.bin: Can't ope
n: No such file or directory
+(imx8mn_beacon_2g)
+(imx8mn_beacon_2g) make[1]: *** [Makefile:1088: all] Error 1
+(imx8mn_beacon_2g) make: *** [Makefile:177: sub-make] Error 2

and that's the problem to fix still, that ddr bin being faked out, in
CI.  I think I see how to do that however, and I'm going to test that
out locally and then shoot this at CI and see what happens.

-- 
Tom

Attachment: signature.asc
Description: PGP signature

Reply via email to