mkimage_fit_atf.sh: not found

Thu Jan 6 20:05:07 CET 2022

Hello Tim,

> -----Original Message-----
> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Tim Harvey
> Sent: Thursday, January 6, 2022 6:10 PM
> To: ZHIZHIKIN Andrey <andrey.zhizhikin at leica-geosystems.com>
> Cc: u-boot <u-boot at lists.denx.de>; Stefano Babic <sbabic at denx.de>; Fabio Estevam
> <festevam at gmail.com>; Schrempf Frieder <frieder.schrempf at kontron.de>; Adam Ford
> <aford173 at gmail.com>; Marcel Ziswiler <marcel at ziswiler.com>; Jagan Teki
> <jagan at amarulasolutions.com>; Oliver Graute <oliver.graute at kococonnector.com>
> Subject: Re: mkimage_fit_atf.sh: not found
> 
> On Thu, Jan 6, 2022 at 2:07 AM ZHIZHIKIN Andrey
> <andrey.zhizhikin at leica-geosystems.com> wrote:
> >
> > Hello Tim,
> >
> > > -----Original Message-----
> > > From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Tim Harvey
> > > Sent: Wednesday, January 5, 2022 8:08 PM
> > > To: ZHIZHIKIN Andrey <andrey.zhizhikin at leica-geosystems.com>
> > > Cc: u-boot <u-boot at lists.denx.de>; Stefano Babic <sbabic at denx.de>; Fabio
> Estevam
> > > <festevam at gmail.com>; Schrempf Frieder <frieder.schrempf at kontron.de>; Adam
> Ford
> > > <aford173 at gmail.com>; Marcel Ziswiler <marcel at ziswiler.com>; Jagan Teki
> > > <jagan at amarulasolutions.com>
> > > Subject: Re: mkimage_fit_atf.sh: not found
> > >
> > > On Wed, Jan 5, 2022 at 3:34 AM ZHIZHIKIN Andrey
> > > <andrey.zhizhikin at leica-geosystems.com> wrote:
> > > >
> > > > Hello Tim,
> > > >
> > > > > -----Original Message-----
> > > > > From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Tim Harvey
> > > > > Sent: Tuesday, January 4, 2022 11:48 PM
> > > > > To: u-boot <u-boot at lists.denx.de>; Stefano Babic <sbabic at denx.de>; Fabio
> > > Estevam
> > > > > <festevam at gmail.com>
> > > > > Cc: Schrempf Frieder <frieder.schrempf at kontron.de>; Adam Ford
> > > > > <aford173 at gmail.com>; Marcel Ziswiler <marcel at ziswiler.com>; Jagan Teki
> > > > > <jagan at amarulasolutions.com>
> > > > > Subject: mkimage_fit_atf.sh: not found
> > > > >
> > > > > Stefano and Fabio,
> > > > >
> > > > > I'm seeing the imx8mm_venice_defconfig target failing to build on
> > > > > master due to mkimage_fit_atf.sh not found:
> > > > > ./"arch/arm/mach-imx/mkimage_fit_atf.sh" \
> > > > > arch/arm/dts/imx8mm-venice-gw71xx-0x.dtb
> > > > > arch/arm/dts/imx8mm-venice-gw72xx-0x.dtb
> > > > > arch/arm/dts/imx8mm-venice-gw73xx-0x.dtb
> > > > > arch/arm/dts/imx8mm-venice-gw7901.dtb
> > > > > arch/arm/dts/imx8mm-venice-gw7902.dtb > u-boot.its
> > > > > /bin/sh: 1: ./arch/arm/mach-imx/mkimage_fit_atf.sh: not found
> > > > >
> > > >
> > > > This has been dropped in d9a6f0eed6 ("tree: imx: remove old fit generator
> > > script")
> > >
> > > So why was that merged when it breaks several boards that are not
> > > switched to binman because of the CI issue?
> >
> > Because the FIT generator script has been broken after commit 3f04db891a
> > ("image: Check for unit addresses in FITs"), which has covered CVE-2021-27138.
> > You can see the reasoning of merging d9a6f0eed6 ("tree: imx: remove old fit
> > generator script") in the commit message.
> >
> > In addition, d9a6f0eed6 ("tree: imx: remove old fit generator script") has
> > been introduced as a part of discussion stemmed from [1], where it has been
> > pointed out that certain boards are still using old FIT generator which is
> > not working, and I've listed those board config files for maintainers to react.
> >
> 
> Andrey,
> 
> I agree removing the old fit generator is the right path but I am very
> surprised it was merged when it broke boards that had not moved to
> binman yet. There has been a warning to migrate to binman but there
> was never a deadline.

I totally agree that there was no deadline set for this, hence I submitted
the patch as RFC in a good faith that board maintainers (which were all
included in the patch submission mail) would take a look and comment on
this removal. Since nobody complained about that removal at the submission
time - the patch has been accepted.

If you take a look at the discussion that originated this removal patch -
you would see more background and insights on why it has been introduced,
and I saw no objections there either. I've listed those board configs there
which would be affected by the removal, and even included Oliver as I did
not see any binman conversion series from him at that time. Even that did
not trigger any red flag at the time...

This all may come from the fact that board maintainers (like you) had already
patches prepared to move to binman, and I do believe that at the time of
submission there has been no checks in CI about those missing binaries,
hence the submission came clean.

> 
> > Binman CI missing binaries verification came later I believe, that is the
> > reason we are seeing those conversions pending. But this is unrelated to the
> > FIT generator script removal, which was broken anyways.
> >
> 
> I have not followed the discussion about what was wrong with the FIT
> generator or what was specifically 'broken' but boards at least built
> and booted with it and now they do not build. I do know there was a
> time where I needed a patch that dealt with '@' symbols and I'm not
> sure if that ever got merged before the FIT generator was simply
> removed. Again, I hadn't thought much of it because I had binman
> conversion patches in flight not realizing they would get stuck
> because of CI.

This is exactly the point: those board builds were broken *way before*
this removal due to fact that FIT generator has been producing ITS with
"@" symbols which were not accepted by updated mkimage.

> 
> I don't really know anything about U-Boot CI but I'm also surprised it
> doesn't point out the fact that several boards won't currently build.

Honestly, I'm in the same position here with the imx8mq_evk conversion
patch: I know nothing about CI, and only deducted from the comments on
conversion patch that this might be an issue, with Heiko's patch solving
it...

> 
> > >
> > > >
> > > > > As far as I can tell the other boards that are still using
> > > > > SPL_FIT_GENERATOR also fail due to this (ie imx8mm_beacon_defconfig,
> > > > > imx8mq_evk_defconfig, imx8mm-icore-mx8mm-edimm2.2_defconfig, etc).
> > > >
> > > > imx8mq_evk is already converted and I've sent a patch for it, see [1].
> > > >
> > > > >
> > > > > What is the state of the binman conversion? I submitted a series to
> > > > > convert my boards to binman and it has just been sitting without any
> > > > > response for months now [1].
> > > >
> > > > I believe that the reason for your series sitting in the queue is the same
> as
> > > > for imx8mq_evk: missing binary blobs (ATF and DDR) are failing CI builds.
> > > >
> > >
> > > Right, so imx8mq_evk (and others) are completely broken for the
> > > pending release correct?
> >
> > That is correct, and there are patches to address this.
> >
> > There is also a patch series from Heiko to address the "missing binary" CI
> issue, see [2].
> >
> 
> Again personally I did not pay attention to d9a6f0eed6 ("tree: imx:
> remove old fit generator script") because I already had submitted a
> series to move my board to binman (other boards still had not done
> this however and would be broken without a change). I see that Stefano
> applied d9a6f0eed6 ("tree: imx: remove old fit generator script") on
> Oct 7 and he must not have realized it broke boards that had not moved
> to binman. Several of us have submitted patches to move to binman and
> thus also likely did not notice but those patches never got merged
> because of this CI issue.
> 
> I see Heiko has a v4 of his patch here
> https://patchwork.ozlabs.org/project/uboot/list/?series=279595. Can
> that get merged?
> 
> It looks to me like the following boards would be currently broken as
> they all use CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-imx/mkimage_fit_atf.sh":
> cgtqmx8_defconfig
> imx8mm-icore-mx8mm-ctouch2_defconfig
> imx8mm-icore-mx8mm-edimm2.2_defconfig
> imx8mm_beacon_defconfig
> imx8mm_venice_defconfig
> imx8mn_beacon_2g_defconfig
> imx8mn_beacon_defconfig
> imx8mq_evk_defconfig
> imx8mq_phanbell_defconfig
> imx8qm_rom7720_a1_4G_defconfig
> pico-imx8mq_defconfig
> 
> I don't know exactly how the U-Boot release schedule works and when
> 2021.01 is released but in order to save these boards from being
> broken the following would have to occur:
> apply Heiko's [PATCH v4] binman: add support for creating dummy files
> for external blobs
> (https://patchwork.ozlabs.org/project/uboot/list/?series=279595)
> apply the conversion to binman series that has been pending because of
> the CI breakage for these boards:
> - Adam Ford: imx8mm_beacon* boards
> - Tim Harvey: imx8mm_venice boards
> - Andrey Zhizhikin: imx8mq_evk
> - Peng Fan: imx8mq_phanbell and pico-imx8mq

That sounds like a plan to me, and this is what I was expecting to happen.
Hopefully, this would make it into 2022.01...

> 
> That would still leave the following boards broken as nobody has
> submitted binman patches for them. These are both maintained by Oliver
> Graute (added to cc):
> imx8qm_rom7720_a1_4G_defconfig
> cgtqmx8_defconfig

I guess Oliver would need to step up here to have those boards converted.

> 
> I believe Stefano is not currently available and I'm not sure when he
> is back to merge the pending binman conversion series. Additionally
> some of these series may need to be rebased (I know I had to do that
> twice because of things being moved to Kconfig)
> 
> Best regards,
> 
> Tim
> 
> > >
> > > Sounds like we need to revert d9a6f0eed6 ("tree: imx: remove old fit
> > > generator script")
> >
> > No, because reverting this patch would bring the FIT generator which
> > does not produce correct images due to reasons stated above.
> >
> > If there are plans to revert it - then it should also be complemented with
> > the patch that fixes the FIT generator. I'm not sure if this is a desired
> > behavior to bring back old functionality and fix it for the sake of removing
> > it at the later stage.
> >
> > >
> > > Tim
> > >
> > > > >
> > > > > I'm not sure when the above breakage occurred but the conversion to
> > > > > binman resolves it and other things.
> > > > >
> > > > > Please let me know what you expect me to do to resolve this as there
> > > > > is a release pending.
> > > > >
> > > > > Best regards,
> > > > >
> > > > > Tim
> > > > > [1] https://patchwork.ozlabs.org/project/uboot/patch/20211006201700.3018-
> 1-
> > > > > tharvey at gateworks.com/
> > > >
> > > > -- andrey
> > > >
> > > > Link: [1]:
> > > http://patchwork.ozlabs.org/project/uboot/patch/20211203161802.12699-1-
> > > andrey.zhizhikin at leica-geosystems.com/
> >
> > -- andrey
> >
> > Link: [1]: https://lore.kernel.org/u-
> boot/AM6PR06MB46912C898E0D6EA797D1EB5EA6C49 at AM6PR06MB4691.eurprd06.prod.outlook.c
> om/
> > Link: [2]: https://lore.kernel.org/u-boot/20220105125816.197045-1-
> heiko.thiery at gmail.com/
> >

Cheers,
Andrey