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

[Stefan Roese <sr@denx.de>]

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