[PATCH] tools: kwboot: Fix sending Kirkwood v0 images

[PATCH] tools: kwboot: Fix sending Kirkwood v0 images

[Pali Rohár]

Properly calculate and align image header size to xmodem block size.

Kirkwood v0 images do not have stored total size of header in header
structure itself like it is for v1 images. So kwbheader_size() calculates
size by traversing image structure itself. Aligning is done in kwboot by
putting zero padding bytes between the header and data part.

Signed-off-by: Pali Rohár <pali@kernel.org>
Tested-by: Tony Dinh <mibodhi@gmail.com>
---
 tools/kwboot.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index bacca1530110..4e4d544efd3f 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -1073,6 +1073,14 @@ kwboot_xmodem(int tty, const void *_img, size_t size, int baudrate)
 
 	hdrsz = kwbheader_size(img);
 
+	/*
+	 * If header size is not aligned to xmodem block size (which applies
+	 * for all images in kwbimage v0 format) then we have to ensure that
+	 * the last xmodem block of header contains beginning of the data
+	 * followed by the header. So align header size to xmodem block size.
+	 */
+	hdrsz += (KWBOOT_XM_BLKSZ - hdrsz % KWBOOT_XM_BLKSZ) % KWBOOT_XM_BLKSZ;
+
 	kwboot_printv("Waiting 2s and flushing tty\n");
 	sleep(2); /* flush isn't effective without it */
 	tcflush(tty, TCIOFLUSH);
@@ -1083,12 +1091,17 @@ kwboot_xmodem(int tty, const void *_img, size_t size, int baudrate)
 	if (rc)
 		return rc;
 
-	img += hdrsz;
-	size -= hdrsz;
-
-	rc = kwboot_xmodem_one(tty, &pnum, 0, img, size, 0);
-	if (rc)
-		return rc;
+	/*
+	 * If we have already sent image data as a part of the last
+	 * xmodem header block then we have nothing more to send.
+	 */
+	if (hdrsz < size) {
+		img += hdrsz;
+		size -= hdrsz;
+		rc = kwboot_xmodem_one(tty, &pnum, 0, img, size, 0);
+		if (rc)
+			return rc;
+	}
 
 	rc = kwboot_xm_finish(tty);
 	if (rc)
-- 
2.20.1

Re: [PATCH] tools: kwboot: Fix sending Kirkwood v0 images

[Stefan Roese]

On 05.11.21 23:29, Pali Rohár wrote:
> Properly calculate and align image header size to xmodem block size.
> 
> Kirkwood v0 images do not have stored total size of header in header
> structure itself like it is for v1 images. So kwbheader_size() calculates
> size by traversing image structure itself. Aligning is done in kwboot by
> putting zero padding bytes between the header and data part.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Tested-by: Tony Dinh <mibodhi@gmail.com>

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

Thanks,
Stefan

> ---
>   tools/kwboot.c | 25 +++++++++++++++++++------
>   1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index bacca1530110..4e4d544efd3f 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -1073,6 +1073,14 @@ kwboot_xmodem(int tty, const void *_img, size_t size, int baudrate)
>   
>   	hdrsz = kwbheader_size(img);
>   
> +	/*
> +	 * If header size is not aligned to xmodem block size (which applies
> +	 * for all images in kwbimage v0 format) then we have to ensure that
> +	 * the last xmodem block of header contains beginning of the data
> +	 * followed by the header. So align header size to xmodem block size.
> +	 */
> +	hdrsz += (KWBOOT_XM_BLKSZ - hdrsz % KWBOOT_XM_BLKSZ) % KWBOOT_XM_BLKSZ;
> +
>   	kwboot_printv("Waiting 2s and flushing tty\n");
>   	sleep(2); /* flush isn't effective without it */
>   	tcflush(tty, TCIOFLUSH);
> @@ -1083,12 +1091,17 @@ kwboot_xmodem(int tty, const void *_img, size_t size, int baudrate)
>   	if (rc)
>   		return rc;
>   
> -	img += hdrsz;
> -	size -= hdrsz;
> -
> -	rc = kwboot_xmodem_one(tty, &pnum, 0, img, size, 0);
> -	if (rc)
> -		return rc;
> +	/*
> +	 * If we have already sent image data as a part of the last
> +	 * xmodem header block then we have nothing more to send.
> +	 */
> +	if (hdrsz < size) {
> +		img += hdrsz;
> +		size -= hdrsz;
> +		rc = kwboot_xmodem_one(tty, &pnum, 0, img, size, 0);
> +		if (rc)
> +			return rc;
> +	}
>   
>   	rc = kwboot_xm_finish(tty);
>   	if (rc)
> 

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

Re: [PATCH] tools: kwboot: Fix sending Kirkwood v0 images

[Stefan Roese]

On 05.11.21 23:29, Pali Rohár wrote:
> Properly calculate and align image header size to xmodem block size.
> 
> Kirkwood v0 images do not have stored total size of header in header
> structure itself like it is for v1 images. So kwbheader_size() calculates
> size by traversing image structure itself. Aligning is done in kwboot by
> putting zero padding bytes between the header and data part.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Tested-by: Tony Dinh <mibodhi@gmail.com>

Applied to u-boot-marvell/master

Thanks,
Stefan

> ---
>   tools/kwboot.c | 25 +++++++++++++++++++------
>   1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index bacca1530110..4e4d544efd3f 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -1073,6 +1073,14 @@ kwboot_xmodem(int tty, const void *_img, size_t size, int baudrate)
>   
>   	hdrsz = kwbheader_size(img);
>   
> +	/*
> +	 * If header size is not aligned to xmodem block size (which applies
> +	 * for all images in kwbimage v0 format) then we have to ensure that
> +	 * the last xmodem block of header contains beginning of the data
> +	 * followed by the header. So align header size to xmodem block size.
> +	 */
> +	hdrsz += (KWBOOT_XM_BLKSZ - hdrsz % KWBOOT_XM_BLKSZ) % KWBOOT_XM_BLKSZ;
> +
>   	kwboot_printv("Waiting 2s and flushing tty\n");
>   	sleep(2); /* flush isn't effective without it */
>   	tcflush(tty, TCIOFLUSH);
> @@ -1083,12 +1091,17 @@ kwboot_xmodem(int tty, const void *_img, size_t size, int baudrate)
>   	if (rc)
>   		return rc;
>   
> -	img += hdrsz;
> -	size -= hdrsz;
> -
> -	rc = kwboot_xmodem_one(tty, &pnum, 0, img, size, 0);
> -	if (rc)
> -		return rc;
> +	/*
> +	 * If we have already sent image data as a part of the last
> +	 * xmodem header block then we have nothing more to send.
> +	 */
> +	if (hdrsz < size) {
> +		img += hdrsz;
> +		size -= hdrsz;
> +		rc = kwboot_xmodem_one(tty, &pnum, 0, img, size, 0);
> +		if (rc)
> +			return rc;
> +	}
>   
>   	rc = kwboot_xm_finish(tty);
>   	if (rc)
> 

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