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

7 Jan 2022


      On Fri, Jan 7, 2022 at 4:42 PM Tom Rini trini@konsulko.com wrote:
...
On Fri, Jan 07, 2022 at 11:26:49PM +0100, Marcel Ziswiler wrote:
...
Hi Tim and Tom
On Fri, 2022-01-07 at 12:37 -0800, Tim Harvey wrote:
...
On Fri, Jan 7, 2022 at 12:27 PM Tom Rini trini@konsulko.com wrote:
...
On Fri, Jan 07, 2022 at 12:24:57PM -0800, Tim Harvey wrote:
...
On Fri, Jan 7, 2022 at 12:08 PM Adam Ford aford173@gmail.com
wrote:
...
...
...
...
...
On Fri, Jan 7, 2022 at 12:25 PM Tom Rini trini@konsulko.com
wrote:
...
...
...
...
...
>
> On Fri, Jan 07, 2022 at 12:21:18PM -0600, Adam Ford wrote:
> > On Fri, Jan 7, 2022 at 11:38 AM Tom Rini trini@konsulko.com
wrote:
...
...
...
...
...
> >
> > > On Fri, Jan 07, 2022 at 09:27:05AM -0800, Tim Harvey wrote:
> > > > 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.
> > >
> > > 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
...
...
...
...
...
> > >
> >
> > The beacon boards are mine.  I can work on this one today.
Do I just grab
...
...
...
...
...
> > the binman updates and apply it to master and fix it from
that starting
...
...
...
...
...
> > point?
>
> So, yes, grabbing the series from the above link and applying
it on top
...
...
...
...
...
> of master, and making builds work with fake binaries would be
very
...
...
...
...
...
> helpful.  As best I can tell there's still something missing
with making
...
...
...
...
...
> fake blobs link and not fail.  For example, imx8mm_venice is
part of the
...
...
...
...
...
> series above but still fails with 'make
BINMAN_FAKE_EXT_BLOBS=1 ...':
...
...
...
...
...
> Image 'main-section:u-boot-spl-ddr' has faked external blobs
and is non-functional:
...
...
...
...
...
> lpddr4_pmu_train_1d_imem.bin lpddr4_pmu_train_1d_dmem.bin
lpddr4_pmu_train_2d_imem.bin
...
...
...
...
...
> lpddr4_pmu_train_2d_dmem.bin
> Image 'main-section' is missing external blobs and is
non-functional: atf_blob blob-ext
...
...
...
...
...
> Wrote map file './imx-boot.map' to show errors
> binman: Node '/binman/imx-boot/blob-ext@1': Offset 0x0 (0)
overlaps with previous entry '/binman/imx-
...
...
...
...
...
> boot/uboot' ending at 0x1aee08 (1764872)
>
From what I can tell looking at the imx8mm_venice board is that
the imx8mm-u-boot.dtsi has the binman
...
...
...
...
...
nodes to build the image, but
arch/arm/dts/imx8mm-venice-u-boot.dtsi duplicates that work, so there are
...
...
...
...
...
two copies trying to occupy the same space.  Deleting the
&binman node from the venice-u-boot.dtsi file
...
...
...
...
...
appears to make the problem go away, and binman is still making
the image.
...
...
...
...
...
Adam,
I don't quite follow. I don't see any binman nodes in
arch/arm/dts/imx8mm-u-boot.dtsi.
I know that Marcel submitted a patch that added them (and I'm not
sure
...
...
...
...
if he added them yet in a way that was compatible with multiple
fdt's)
...
...
...
...
but this hasn't been merged yet so I don't see any duplicate binman
nodes for venice.
Right, so this is with
https://patchwork.ozlabs.org/bundle/trini/2022-01-07-imx8-and-binman-updates...
...
...
...
applied and I did have to manually apply Marcel's 4/7 and 7/7, so
maybe
...
...
...
I mismerged something there too?  The tree is also at
https://github.com/trini/u-boot/tree/WIP/2022-01-07-imx8-and-buildman-update...
...
...
...
Tom,
Thank you for helping with this. During the merge window we had a lot
of collisions because of things moving to defconfig that I think
caused a lot of imx patches to have to be rebased. Then as most of us
were submitting binman patches this CI issue popped up and as Stefano
was unavailable I think it all got left hanging. In the middle of that
Marcel's patch moved binman nodes to a common place so some rebasing
is needed there as well. Fun times!
Yes, I couldn't get all from your patchwork bundle to apply - I didn't
try as hard as you to manually merge :)
I pulled your tree and see Marcel's patches there now. With the binman
node removed from arch/arm/dts/imx8mm-venice-u-boot.dtsi venice works.
I also quickly double checked everything and indeed somehow the removal
of the binman stuff from
...
arch/arm/dts/imx8mm-venice-u-boot.dtsi which I had in my 4th patch
"[PATCH v3 4/7] arm64: dts: imx8mm: use
...
common binman configuration" got lost. Otherwise it all looks good.
Thanks!
I must have messed it up, sorry.  I'm doing one more CI run right now of
Adam's versions of the patches (just to be extra safe) and then I'll
merge the branch.  Thanks all!
Thanks for your work and helping us get this merged while the imx
maintainer is out.
adam
...
--
Tom