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

Priyanka Jain
2022-02-02T09:06:46Z

>-----Original Message-----
>From: Tom Rini <trini@konsulko.com>
>Sent: Saturday, January 22, 2022 10:05 PM
>To: Pali Rohár <pali@kernel.org>; Alison Wang <alison.wang@nxp.com>;
>Priyanka Jain <priyanka.jain@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>;
>Rajesh Bhagat <rajesh.bhagat@nxp.com>
>Cc: Simon Glass <sjg@chromium.org>; Alexandru Gagniuc
><mr.nuke.me@gmail.com>; Yann Dirson <yann@blade-group.com>; Stefan
>Roese <sr@denx.de>; Marek Behún <marek.behun@nic.cz>; u-
>boot@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@kernel.org>
>> > > > > Reviewed-by: Stefan Roese <sr@denx.de>
>> > > > > Reviewed-by: Simon Glass <sjg@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