Re: [PATCH u-boot-mvebu 5/5] tools: kwboot: Workaround A38x BootROM bug for images with a gap

Stefan Roese
2023-03-24T08:25:00Z

On 3/23/23 20:57, Pali Rohár wrote:
> A38x BootROM has a bug which cause that BootROM loads data part of UART
> image into RAM target address increased by one byte when source address
> and header size stored in the image header are not same.
>
> Workaround this bug by completely removing a gap between header and data
> part of the UART image. Without gap, this BootROM bug is not triggered.
>
> This gap can be present in SDIO or SATA image types which have aligned
> start of the data part to the media sector size. With this workaround
> kwboot should be able to convert and send SDIO or SATA images for UART
> booting.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>

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

Thanks,
Stefan

> ---
> tools/kwboot.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 1cf78dda6755..2b92966919da 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -78,6 +78,17 @@
> *
> * - IBR_HDR_UART_ID (0x69):
> * UART image can be transfered via xmodem protocol over first UART.
> + * Unlike all other image types, header size stored in the image must be
> + * multiply of the 128 bytes (for all other image types it can be any size)
> + * and data part of the image does not have to contain 32-bit checksum
> + * (all other image types must have valid 32-bit checksum in its data part).
> + * And data size stored in the image is ignored. A38x BootROM determinates
> + * size of the data part implicitly by the end of the xmodem transfer.
> + * A38x BootROM has a bug which cause that BootROM loads data part of UART
> + * image into RAM target address increased by one byte when source address
> + * and header size stored in the image header are not same. So UART image
> + * should be constructed in a way that there is no gap between header and
> + * data part.
> *
> * - IBR_HDR_I2C_ID (0x4D):
> * It is unknown for what kind of storage is used this image. It is not
> @@ -2188,6 +2199,18 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
> }
> }
>
> + /* Header size and source address must be same for UART type due to A38x BootROM bug */
> + if (hdrsz != le32_to_cpu(hdr->srcaddr)) {
> + if (is_secure) {
> + fprintf(stderr, "Cannot align image with secure header\n");
> + goto err;
> + }
> +
> + kwboot_printv("Removing gap between image header and data\n");
> + memmove(img + hdrsz, img + le32_to_cpu(hdr->srcaddr), le32_to_cpu(hdr->blocksize));
> + hdr->srcaddr = cpu_to_le32(hdrsz);
> + }
> +
> hdr->checksum = kwboot_hdr_csum8(hdr) - csum;
>
> *size = le32_to_cpu(hdr->srcaddr) + le32_to_cpu(hdr->blocksize);

Viele Grüße,
Stefan Roese

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