Re: [PATCH u-boot-marvell 09/11] tools: kwbimage: Fill the real header size into the main header

From: "Pali Rohár" <pali@kernel.org>
To: Pierre Bourdon <delroth@gmail.com>
Cc: "Marek Behún" <kabel@kernel.org>, "Stefan Roese" <sr@denx.de>,
	u-boot@lists.denx.de, "Marek Behún" <marek.behun@nic.cz>
Subject: Re: [PATCH u-boot-marvell 09/11] tools: kwbimage: Fill the real header size into the main header
Date: Sat, 25 Dec 2021 18:57:10 +0100	[thread overview]
Message-ID: <20211225175710.vtr35zodwypfgp2u@pali> (raw)
In-Reply-To: <CA+V6dmj6O-iXbBPVCM-1BYih3pHzMqCOwjF023CbR5cd=u+dYQ@mail.gmail.com>

On Saturday 25 December 2021 18:48:22 Pierre Bourdon wrote:
> Hi Marek, Pali,
> 
> On Mon, Nov 8, 2021 at 6:14 PM Marek Behún <kabel@kernel.org> wrote:
> > Fill the real header size without padding into the main header
> >
> > This allows to reduce final image when converting image to another format
> > which does not need additional padding.
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> 
> This patch seems to cause mkimage to generate v1 images with invalid
> checksums (which fail to verify with kwboot or mkimage -l). Could you
> double check whether you can reproduce on the latest u-boot master?

Hello! I'm using this patch for more than month and I have not seen any
boot issue related to this patch A385 board.

> I don't really understand how this patch is supposed to work
> (headersz_lsb/headersz_msb get updated *after* csum8 has already been
> computed!).

Ou. Now I see, this is of course wrong in this patch! Checksum in
main_hdr->checksum must be filled *after* updating main_hdr->headersz_*
fields.

Just moving "main_hdr->checksum = image_checksum8(main_hdr, headersz);"
after the "main_hdr->headersz_msb = (headersz & 0xFFFF0000) >> 16;"
should be enough. Are you going to prepare a fixup for master branch?

I'm not sure how it could happen... but probably "real header size" is
the same as it was before and therefore checksum did not change.

> $ tools/mkimage -n ./board/qnap/qsw-98dx3236/kwbimage.cfg -T kwbimage
> -a 0x00800000 -e 0x00800000 -d u-boot.bin u-boot.kwb
> Image Type:   MVEBU Boot from nand Image
> Image version:1
> BIN Hdr Size: 76224 Bytes = 74.44 KiB = 0.07 MiB
> Data Size:    735764 Bytes = 718.52 KiB = 0.70 MiB
> Load Address: 00800000
> Entry Point:  00800000
> 
> $ sudo tools/kwboot -a -b u-boot.kwb -t -B 115200 /dev/ttyUSB0
> kwboot version 2022.01-rc4-00075-g3bd6c62cf4-dirty
> u-boot.kwb: Invalid image.
> 
> (The specific kwbimage.cfg/board is still a WIP, I can try to provide
> some more repro if somehow there is something specific to my setup,
> but I doubt it.)
> 
> Best,
> 
> -- 
> Pierre Bourdon <delroth@gmail.com>
> Software Engineer @ Zürich, Switzerland
> https://delroth.net/