Re: [PATCH u-boot-marvell v2 18/20] tools: kwbimage/kwboot: Check ext field for non-zero value

Stefan Roese
2022-01-13T06:46:11Z

On 1/12/22 18:20, Pali Rohár wrote:
> Despite the official specification, BootROM does not look at the lowest bit
> of ext field but rather checks if ext field is non-zero.
>
> Moreover original Marvell doimage tool puts into the mhdr->ext field the
> number of extended headers, so basically it sets ext filed to non-zero
> value if some extended header is present.
>
> Fix U-Boot dumpimage and kwboot tools to parse correctly also kwbimage
> files created by Marvell doimage tool, in the same way as the BootROM is
> doing it when booting these images.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Marek Behún <marek.behun@nic.cz>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
> tools/kwbimage.c | 2 +-
> tools/kwbimage.h | 6 +++---
> tools/kwboot.c | 4 ++--
> 3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tools/kwbimage.c b/tools/kwbimage.c
> index de7e9acf7fe5..92d163b6050e 100644
> --- a/tools/kwbimage.c
> +++ b/tools/kwbimage.c
> @@ -1948,7 +1948,7 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size,
> if (kwbimage_version(ptr) == 0) {
> struct main_hdr_v0 *mhdr = (struct main_hdr_v0 *)ptr;
>
> - if (mhdr->ext & 0x1) {
> + if (mhdr->ext) {
> struct ext_hdr_v0 *ext_hdr = (void *)(mhdr + 1);
>
> csum = image_checksum8(ext_hdr, sizeof(*ext_hdr) - 1);
> diff --git a/tools/kwbimage.h b/tools/kwbimage.h
> index c000cba4b8d1..9ebc7d72d363 100644
> --- a/tools/kwbimage.h
> +++ b/tools/kwbimage.h
> @@ -208,7 +208,7 @@ static inline size_t kwbheader_size(const void *header)
> const struct main_hdr_v0 *hdr = header;
>
> return sizeof(*hdr) +
> - (hdr->ext & 0x1) ? sizeof(struct ext_hdr_v0) : 0;
> + hdr->ext ? sizeof(struct ext_hdr_v0) : 0;
> } else {
> const struct main_hdr_v1 *hdr = header;
>
> @@ -252,7 +252,7 @@ static inline struct opt_hdr_v1 *opt_hdr_v1_first(void *img) {
> return NULL;
>
> mhdr = img;
> - if (mhdr->ext & 0x1)
> + if (mhdr->ext)
> return (struct opt_hdr_v1 *)(mhdr + 1);
> else
> return NULL;
> @@ -272,7 +272,7 @@ static inline struct opt_hdr_v1 *_opt_hdr_v1_next(struct opt_hdr_v1 *cur)
>
> static inline struct opt_hdr_v1 *opt_hdr_v1_next(struct opt_hdr_v1 *cur)
> {
> - if (*opt_hdr_v1_ext(cur) & 0x1)
> + if (*opt_hdr_v1_ext(cur))
> return _opt_hdr_v1_next(cur);
> else
> return NULL;
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index d22e6ea96a5c..c3d8ab654417 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -1398,7 +1398,7 @@ kwboot_add_bin_ohdr_v1(void *img, size_t *size, uint32_t binsz)
> uint32_t ohdrsz;
> uint8_t *prev_ext;
>
> - if (hdr->ext & 0x1) {
> + if (hdr->ext) {
> for_each_opt_hdr_v1 (ohdr, img)
> if (opt_hdr_v1_next(ohdr) == NULL)
> break;
> @@ -1422,7 +1422,7 @@ kwboot_add_bin_ohdr_v1(void *img, size_t *size, uint32_t binsz)
> ohdrsz = sizeof(*ohdr) + 4 + 4 * num_args + binsz + 4;
> kwboot_img_grow_hdr(hdr, size, ohdrsz);
>
> - *prev_ext |= 1;
> + *prev_ext = 1;
>
> ohdr->headertype = OPT_HDR_V1_BINARY_TYPE;
> ohdr->headersz_msb = ohdrsz >> 16;
>

Viele Grüße,
Stefan Roese

--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de