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

7 Jan 2022


      On Fri, Jan 7, 2022 at 2:39 PM Tom Rini trini@konsulko.com wrote:
...
On Fri, Jan 07, 2022 at 12:37:52PM -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 will submit a v3 for you that does this.
Great, thanks!
I just sent a patch for the beacon_imx8mn_2g board against your WIP branch
with the rest of the binman stuff already applied.  Do you need me to
rebase that patch on master and squash it with the other nano?  I wasn't
sure if the WIP would get merged as-is with updates.
adam
...
--
Tom