[PATCH v2] tools: mkimage: Call verify_header after writing image to disk

[Z.Q. Hou]

> -----Original Message-----
> From: Pali Rohár <pali at kernel.org>
> Sent: 2022年2月16日 3:52
> To: Priyanka Jain <priyanka.jain at nxp.com>
> Cc: Tom Rini <trini at konsulko.com>; Alison Wang <alison.wang at nxp.com>;
> Mingkai Hu <mingkai.hu at nxp.com>; Rajesh Bhagat
> <rajesh.bhagat at nxp.com>; Jiafei Pan <jiafei.pan at nxp.com>; Simon Glass
> <sjg at chromium.org>; Alexandru Gagniuc <mr.nuke.me at gmail.com>; Yann
> Dirson <yann at blade-group.com>; Stefan Roese <sr at denx.de>; Marek
> Behún <marek.behun at nic.cz>; u-boot at lists.denx.de
> Subject: Re: [PATCH v2] tools: mkimage: Call verify_header after writing
> image to disk
> 
> On Wednesday 02 February 2022 09:06:30 Priyanka Jain wrote:
> > >-----Original Message-----
> > >From: Tom Rini <trini at konsulko.com>
> > >Sent: Saturday, January 22, 2022 10:05 PM
> > >To: Pali Rohár <pali at kernel.org>; Alison Wang <alison.wang at nxp.com>;
> > >Priyanka Jain <priyanka.jain at nxp.com>; Mingkai Hu
> > ><mingkai.hu at nxp.com>; Rajesh Bhagat <rajesh.bhagat at nxp.com>
> > >Cc: Simon Glass <sjg at chromium.org>; Alexandru Gagniuc
> > ><mr.nuke.me at gmail.com>; Yann Dirson <yann at blade-group.com>;
> Stefan
> > >Roese <sr at denx.de>; Marek Behún <marek.behun at nic.cz>; u-
> > >boot at lists.denx.de
> > >Subject: Re: [PATCH v2] tools: mkimage: Call verify_header after
> > >writing image to disk
> > >
> > >On Sat, Jan 22, 2022 at 05:31:18PM +0100, Pali Rohár wrote:
> > >> On Friday 21 January 2022 21:15:43 Tom Rini wrote:
> > >> > On Sat, Jan 22, 2022 at 02:44:22AM +0100, Pali Rohár wrote:
> > >> > > On Friday 21 January 2022 16:21:33 Tom Rini wrote:
> > >> > > > On Fri, Jan 14, 2022 at 06:34:43PM +0100, Pali Rohár wrote:
> > >> > > >
> > >> > > > > If image backend provides verify_header callback then call
> > >> > > > > it after writing image to disk. This ensures that written image is
> correct.
> > >> > > > >
> > >> > > > > Signed-off-by: Pali Rohár <pali at kernel.org>
> > >> > > > > Reviewed-by: Stefan Roese <sr at denx.de>
> > >> > > > > Reviewed-by: Simon Glass <sjg at chromium.org>
> > >> > > > > ---
> > >> > > > >  tools/mkimage.c | 41
> > >> > > > > +++++++++++++++++++++++++++++++++++++++++
> > >> > > > >  1 file changed, 41 insertions(+)
> > >> > > >
> > >> > > > This breaks a number of platforms such as
> > >> > > > ls1021atwr_sdcard_qspi and it's not clear to me why exactly.
> > >> > >
> > >> > > Maybe they were already broken and this patch just detected it?
> > >> > > Or verify_header callback for particular image type is reject
> > >> > > valid image?
> > >> > >
> > >> > > Do you have some pointers to failed build logs?
> > >> >
> > >> > Try building for ls1021atwr_sdcard_qspi with your patch applied,
> > >> > the only new thing that's shown in the logs is the error message.
> > >>
> > >> So... I have tried following without this patch:
> > >>
> > >> $ make ls1021atwr_sdcard_qspi_defconfig $ make
> > >> CROSS_COMPILE=arm-linux-gnueabi- -j8
> > >>
> > >> It generated file spl/u-boot-spl.pbl without error. Now I called -l
> > >> on this generated file for type pblimage and I got following output:
> > >>
> > >> $ ./tools/dumpimage -T pblimage -l spl/u-boot-spl.pbl GP Header:
> > >> Size
> > >> aa55aa55 LoadAddr 1ee0100
> > >>
> > >> $ ./tools/mkimage -T pblimage -l spl/u-boot-spl.pbl GP Header: Size
> > >> aa55aa55 LoadAddr 1ee0100
> > >>
> > >> "GP Header:" line is from the TI OMAP image backend type gpimage or
> > >> type omapimage (implemented in file gpimage-common.c).
> > >>
> > >> So it means that files generated by ls1021atwr_sdcard_qspi are
> > >> already broken and my patch just detected it. Or it is also
> > >> possible that validation code in pblimage.c file is incorrect and broken.
> > >>
> > >> What to do with it now?
> > >
> > >Thanks for digging.  This is a problem for a number of the ls1021,
> > >ls1043 and ls1046 platforms, so lets add some maintainers there.
> > >
> > >--
> > >Tom
> >
> >
> > I will ask NXP-platform owners to check on this.
> >
> > Thanks
> > Priyanka
> 
> Hello! Any news on this?

Indeed it is a historical problem and exposed by this patch.
The problem is the RCW headers are different on PPC and ARM platforms, while ls1021a/ls1043a/ls1046a leveraged the PPC tool to generate PBL image, but didn’t handle this differentially, this result in the image header verification failed.
I'll submit a patch to fix it.

Thanks,
Zhiqiang