All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] arm: mvebu: Fix usage of BIN header arguments
@ 2021-10-21 14:46 Pali Rohár
  2021-10-21 14:46 ` [PATCH 1/4] tools: kwboot: Align UART baudrate change code in BIN header to 128-bit boundary Pali Rohár
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Pali Rohár @ 2021-10-21 14:46 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behún, u-boot

BIN header arguments are used only for aligning ARM executable code
to 128-bit boundary. This patch series document this behavior and fix
kwbimage and kwboot code to properly generate BIN headers.

Pali Rohár (4):
  tools: kwboot: Align UART baudrate change code in BIN header to
    128-bit boundary
  tools: kwbimage: Align BIN header executable code to 128-bit boundary
  arm: mvebu: Add documentation for save_boot_params() function
  arm: mvebu: Remove dummy BIN header arguments for SPL binary

 arch/arm/mach-mvebu/kwbimage.cfg.in |  2 +-
 arch/arm/mach-mvebu/lowlevel_spl.S  |  9 +++++
 tools/kwbimage.c                    | 51 +++++++++++++++++++----------
 tools/kwboot.c                      | 22 ++++++++++---
 4 files changed, 61 insertions(+), 23 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] tools: kwboot: Align UART baudrate change code in BIN header to 128-bit boundary
  2021-10-21 14:46 [PATCH 0/4] arm: mvebu: Fix usage of BIN header arguments Pali Rohár
@ 2021-10-21 14:46 ` Pali Rohár
  2021-10-22  6:47   ` Stefan Roese
  2021-10-21 14:46 ` [PATCH 2/4] tools: kwbimage: Align BIN header executable code " Pali Rohár
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Pali Rohár @ 2021-10-21 14:46 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behún, u-boot

ARM executable code inside the BIN header on some mvebu platforms
(e.g. A370, AXP) must always be aligned with the 128-bit boundary. This
requirement can be met by inserting dummy arguments into BIN header.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwboot.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index 6a1a030308e7..36d0cab29d1f 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -255,7 +255,7 @@ static unsigned char kwboot_baud_code[] = {
 };
 
 #define KWBOOT_BAUDRATE_BIN_HEADER_SZ (sizeof(kwboot_baud_code) + \
-				       sizeof(struct opt_hdr_v1) + 8)
+				       sizeof(struct opt_hdr_v1) + 8 + 16)
 
 static const char kwb_baud_magic[16] = "$baudratechange";
 
@@ -1328,11 +1328,10 @@ kwboot_add_bin_ohdr_v1(void *img, size_t *size, uint32_t binsz)
 {
 	struct main_hdr_v1 *hdr = img;
 	struct opt_hdr_v1 *ohdr;
+	uint32_t num_args;
+	uint32_t offset;
 	uint32_t ohdrsz;
 
-	ohdrsz = binsz + 8 + sizeof(*ohdr);
-	kwboot_img_grow_hdr(img, size, ohdrsz);
-
 	if (hdr->ext & 0x1) {
 		for_each_opt_hdr_v1 (ohdr, img)
 			if (opt_hdr_v1_next(ohdr) == NULL)
@@ -1345,13 +1344,26 @@ kwboot_add_bin_ohdr_v1(void *img, size_t *size, uint32_t binsz)
 		ohdr = (void *)(hdr + 1);
 	}
 
+	/*
+	 * ARM executable code inside the BIN header on some mvebu platforms
+	 * (e.g. A370, AXP) must always be aligned with the 128-bit boundary.
+	 * This requirement can be met by inserting dummy arguments into
+	 * BIN header, if needed.
+	 */
+	offset = &ohdr->data[4] - (char *)img;
+	num_args = ((16 - offset % 16) % 16) / sizeof(uint32_t);
+
+	ohdrsz = sizeof(*ohdr) + 4 + 4 * num_args + binsz + 4;
+	kwboot_img_grow_hdr(hdr, size, ohdrsz);
+
 	ohdr->headertype = OPT_HDR_V1_BINARY_TYPE;
 	ohdr->headersz_msb = ohdrsz >> 16;
 	ohdr->headersz_lsb = cpu_to_le16(ohdrsz & 0xffff);
 
 	memset(&ohdr->data[0], 0, ohdrsz - sizeof(*ohdr));
+	*(uint32_t *)&ohdr->data[0] = cpu_to_le32(num_args);
 
-	return &ohdr->data[4];
+	return &ohdr->data[4 + 4 * num_args];
 }
 
 static void
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] tools: kwbimage: Align BIN header executable code to 128-bit boundary
  2021-10-21 14:46 [PATCH 0/4] arm: mvebu: Fix usage of BIN header arguments Pali Rohár
  2021-10-21 14:46 ` [PATCH 1/4] tools: kwboot: Align UART baudrate change code in BIN header to 128-bit boundary Pali Rohár
@ 2021-10-21 14:46 ` Pali Rohár
  2021-10-22  6:47   ` Stefan Roese
  2021-10-21 14:46 ` [PATCH 3/4] arm: mvebu: Add documentation for save_boot_params() function Pali Rohár
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Pali Rohár @ 2021-10-21 14:46 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behún, u-boot

ARM executable code inside the BIN header on some mvebu platforms
(e.g. A370, AXP) must always be aligned with the 128-bit boundary. This
requirement can be met by inserting dummy arguments into BIN header.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/kwbimage.c | 51 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 34 insertions(+), 17 deletions(-)

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index 77bf4dd8865e..abc88d01b9d8 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -932,6 +932,12 @@ static size_t image_headersz_v1(int *hasext)
 	 */
 	headersz = sizeof(struct main_hdr_v1);
 
+	if (image_get_csk_index() >= 0) {
+		headersz += sizeof(struct secure_hdr_v1);
+		if (hasext)
+			*hasext = 1;
+	}
+
 	count = image_count_options(IMAGE_CFG_DATA);
 	if (count > 0)
 		headersz += sizeof(struct register_set_hdr_v1) + 8 * count + 4;
@@ -963,15 +969,10 @@ static size_t image_headersz_v1(int *hasext)
 			return 0;
 		}
 
-		headersz += sizeof(struct opt_hdr_v1) +
-			ALIGN(s.st_size, 4) +
-			(binarye->binary.nargs + 2) * sizeof(uint32_t);
-		if (hasext)
-			*hasext = 1;
-	}
-
-	if (image_get_csk_index() >= 0) {
-		headersz += sizeof(struct secure_hdr_v1);
+		headersz += sizeof(struct opt_hdr_v1) + sizeof(uint32_t) +
+			(binarye->binary.nargs) * sizeof(uint32_t);
+		headersz = ALIGN(headersz, 16);
+		headersz += ALIGN(s.st_size, 4) + sizeof(uint32_t);
 		if (hasext)
 			*hasext = 1;
 	}
@@ -984,9 +985,12 @@ static size_t image_headersz_v1(int *hasext)
 }
 
 int add_binary_header_v1(uint8_t **cur, uint8_t **next_ext,
-			 struct image_cfg_element *binarye)
+			 struct image_cfg_element *binarye,
+			 struct main_hdr_v1 *main_hdr)
 {
 	struct opt_hdr_v1 *hdr = (struct opt_hdr_v1 *)*cur;
+	uint32_t add_args;
+	uint32_t offset;
 	uint32_t *args;
 	size_t binhdrsz;
 	struct stat s;
@@ -1009,12 +1013,6 @@ int add_binary_header_v1(uint8_t **cur, uint8_t **next_ext,
 		goto err_close;
 	}
 
-	binhdrsz = sizeof(struct opt_hdr_v1) +
-		(binarye->binary.nargs + 2) * sizeof(uint32_t) +
-		ALIGN(s.st_size, 4);
-	hdr->headersz_lsb = cpu_to_le16(binhdrsz & 0xFFFF);
-	hdr->headersz_msb = (binhdrsz & 0xFFFF0000) >> 16;
-
 	*cur += sizeof(struct opt_hdr_v1);
 
 	args = (uint32_t *)*cur;
@@ -1025,6 +1023,19 @@ int add_binary_header_v1(uint8_t **cur, uint8_t **next_ext,
 
 	*cur += (binarye->binary.nargs + 1) * sizeof(uint32_t);
 
+	/*
+	 * ARM executable code inside the BIN header on some mvebu platforms
+	 * (e.g. A370, AXP) must always be aligned with the 128-bit boundary.
+	 * This requirement can be met by inserting dummy arguments into
+	 * BIN header, if needed.
+	 */
+	offset = *cur - (uint8_t *)main_hdr;
+	add_args = ((16 - offset % 16) % 16) / sizeof(uint32_t);
+	if (add_args) {
+		*(args - 1) = cpu_to_le32(binarye->binary.nargs + add_args);
+		*cur += add_args * sizeof(uint32_t);
+	}
+
 	ret = fread(*cur, s.st_size, 1, bin);
 	if (ret != 1) {
 		fprintf(stderr,
@@ -1043,6 +1054,12 @@ int add_binary_header_v1(uint8_t **cur, uint8_t **next_ext,
 
 	*cur += sizeof(uint32_t);
 
+	binhdrsz = sizeof(struct opt_hdr_v1) +
+		(binarye->binary.nargs + add_args + 2) * sizeof(uint32_t) +
+		ALIGN(s.st_size, 4);
+	hdr->headersz_lsb = cpu_to_le16(binhdrsz & 0xFFFF);
+	hdr->headersz_msb = (binhdrsz & 0xFFFF0000) >> 16;
+
 	return 0;
 
 err_close:
@@ -1299,7 +1316,7 @@ static void *image_create_v1(size_t *imagesz, struct image_tool_params *params,
 		if (e->type != IMAGE_CFG_BINARY)
 			continue;
 
-		if (add_binary_header_v1(&cur, &next_ext, e))
+		if (add_binary_header_v1(&cur, &next_ext, e, main_hdr))
 			return NULL;
 	}
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] arm: mvebu: Add documentation for save_boot_params() function
  2021-10-21 14:46 [PATCH 0/4] arm: mvebu: Fix usage of BIN header arguments Pali Rohár
  2021-10-21 14:46 ` [PATCH 1/4] tools: kwboot: Align UART baudrate change code in BIN header to 128-bit boundary Pali Rohár
  2021-10-21 14:46 ` [PATCH 2/4] tools: kwbimage: Align BIN header executable code " Pali Rohár
@ 2021-10-21 14:46 ` Pali Rohár
  2021-10-22  6:47   ` Stefan Roese
  2021-10-21 14:46 ` [PATCH 4/4] arm: mvebu: Remove dummy BIN header arguments for SPL binary Pali Rohár
  2021-10-28 10:48 ` [PATCH 0/4] arm: mvebu: Fix usage of BIN header arguments Stefan Roese
  4 siblings, 1 reply; 10+ messages in thread
From: Pali Rohár @ 2021-10-21 14:46 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behún, u-boot

Important detail is availability of kwbimage BIN header arguments passed
via r0 and r1 registers by BootROM.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/arm/mach-mvebu/lowlevel_spl.S | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/mach-mvebu/lowlevel_spl.S b/arch/arm/mach-mvebu/lowlevel_spl.S
index dde77b765214..39d42912c49f 100644
--- a/arch/arm/mach-mvebu/lowlevel_spl.S
+++ b/arch/arm/mach-mvebu/lowlevel_spl.S
@@ -3,6 +3,15 @@
 #include <config.h>
 #include <linux/linkage.h>
 
+/*
+ * BootROM loads the header part of kwbimage into L2 cache. BIN header usually
+ * contains U-Boot SPL, optionally it can also contain additional arguments.
+ * The number of these arguments is in r0, pointer to the argument array in r1.
+ * BootROM expects executable BIN header code to return to address stored in lr.
+ * Other registers (r2 - r12) must be preserved. We save all registers to
+ * CONFIG_SPL_BOOTROM_SAVE address. BIN header arguments (passed via r0 and r1)
+ * are currently not used by U-Boot SPL binary.
+ */
 ENTRY(save_boot_params)
 	stmfd	sp!, {r0 - r12, lr}	/* @ save registers on stack */
 	ldr	r12, =CONFIG_SPL_BOOTROM_SAVE
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] arm: mvebu: Remove dummy BIN header arguments for SPL binary
  2021-10-21 14:46 [PATCH 0/4] arm: mvebu: Fix usage of BIN header arguments Pali Rohár
                   ` (2 preceding siblings ...)
  2021-10-21 14:46 ` [PATCH 3/4] arm: mvebu: Add documentation for save_boot_params() function Pali Rohár
@ 2021-10-21 14:46 ` Pali Rohár
  2021-10-22  6:48   ` Stefan Roese
  2021-10-28 10:48 ` [PATCH 0/4] arm: mvebu: Fix usage of BIN header arguments Stefan Roese
  4 siblings, 1 reply; 10+ messages in thread
From: Pali Rohár @ 2021-10-21 14:46 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behún, u-boot

U-Boot SPL binary does not read BIN header arguments, so passing some dummy
values 0000005b and 00000068 has no effect for U-Boot SPL code.

Probably these two values comes from old Marvell DDR training code which
was separated from U-Boot and used it for some configuration.

Seems that two 32-bit values were specified here to ensure SPL code
alignment to 128-bit boundary as it is required e.g. for A370 or AXP
processors. Main kwbimage header is 64-byte long which is aligned to
128-bit boundary. Optional kwbheader is 32-bit long, number of BIN header
arguments is stored in 32-bit number. So for alignment to 128-bit boundary
is needed 64-bit padding which exactly these two 32-bit dummy arguments
provided.

Now when mkimage correctly aligns start of executable code in BIN header to
128-bit boundary, there is no requirement to put dummy argument values into
kwbimage. So remove them.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/arm/mach-mvebu/kwbimage.cfg.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/kwbimage.cfg.in b/arch/arm/mach-mvebu/kwbimage.cfg.in
index 72e67d75c325..049d23c6ef08 100644
--- a/arch/arm/mach-mvebu/kwbimage.cfg.in
+++ b/arch/arm/mach-mvebu/kwbimage.cfg.in
@@ -9,4 +9,4 @@ VERSION		1
 #@BOOT_FROM
 
 # Binary Header (bin_hdr) with DDR3 training code
-BINARY spl/u-boot-spl.bin 0000005b 00000068
+BINARY spl/u-boot-spl.bin
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] tools: kwboot: Align UART baudrate change code in BIN header to 128-bit boundary
  2021-10-21 14:46 ` [PATCH 1/4] tools: kwboot: Align UART baudrate change code in BIN header to 128-bit boundary Pali Rohár
@ 2021-10-22  6:47   ` Stefan Roese
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Roese @ 2021-10-22  6:47 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Marek Behún, u-boot

On 21.10.21 16:46, Pali Rohár wrote:
> ARM executable code inside the BIN header on some mvebu platforms
> (e.g. A370, AXP) must always be aligned with the 128-bit boundary. This
> requirement can be met by inserting dummy arguments into BIN header.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

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

Thanks,
Stefan

> ---
>   tools/kwboot.c | 22 +++++++++++++++++-----
>   1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 6a1a030308e7..36d0cab29d1f 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -255,7 +255,7 @@ static unsigned char kwboot_baud_code[] = {
>   };
>   
>   #define KWBOOT_BAUDRATE_BIN_HEADER_SZ (sizeof(kwboot_baud_code) + \
> -				       sizeof(struct opt_hdr_v1) + 8)
> +				       sizeof(struct opt_hdr_v1) + 8 + 16)
>   
>   static const char kwb_baud_magic[16] = "$baudratechange";
>   
> @@ -1328,11 +1328,10 @@ kwboot_add_bin_ohdr_v1(void *img, size_t *size, uint32_t binsz)
>   {
>   	struct main_hdr_v1 *hdr = img;
>   	struct opt_hdr_v1 *ohdr;
> +	uint32_t num_args;
> +	uint32_t offset;
>   	uint32_t ohdrsz;
>   
> -	ohdrsz = binsz + 8 + sizeof(*ohdr);
> -	kwboot_img_grow_hdr(img, size, ohdrsz);
> -
>   	if (hdr->ext & 0x1) {
>   		for_each_opt_hdr_v1 (ohdr, img)
>   			if (opt_hdr_v1_next(ohdr) == NULL)
> @@ -1345,13 +1344,26 @@ kwboot_add_bin_ohdr_v1(void *img, size_t *size, uint32_t binsz)
>   		ohdr = (void *)(hdr + 1);
>   	}
>   
> +	/*
> +	 * ARM executable code inside the BIN header on some mvebu platforms
> +	 * (e.g. A370, AXP) must always be aligned with the 128-bit boundary.
> +	 * This requirement can be met by inserting dummy arguments into
> +	 * BIN header, if needed.
> +	 */
> +	offset = &ohdr->data[4] - (char *)img;
> +	num_args = ((16 - offset % 16) % 16) / sizeof(uint32_t);
> +
> +	ohdrsz = sizeof(*ohdr) + 4 + 4 * num_args + binsz + 4;
> +	kwboot_img_grow_hdr(hdr, size, ohdrsz);
> +
>   	ohdr->headertype = OPT_HDR_V1_BINARY_TYPE;
>   	ohdr->headersz_msb = ohdrsz >> 16;
>   	ohdr->headersz_lsb = cpu_to_le16(ohdrsz & 0xffff);
>   
>   	memset(&ohdr->data[0], 0, ohdrsz - sizeof(*ohdr));
> +	*(uint32_t *)&ohdr->data[0] = cpu_to_le32(num_args);
>   
> -	return &ohdr->data[4];
> +	return &ohdr->data[4 + 4 * num_args];
>   }
>   
>   static void
> 


Viele Grüße,
Stefan

-- 
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] tools: kwbimage: Align BIN header executable code to 128-bit boundary
  2021-10-21 14:46 ` [PATCH 2/4] tools: kwbimage: Align BIN header executable code " Pali Rohár
@ 2021-10-22  6:47   ` Stefan Roese
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Roese @ 2021-10-22  6:47 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Marek Behún, u-boot

On 21.10.21 16:46, Pali Rohár wrote:
> ARM executable code inside the BIN header on some mvebu platforms
> (e.g. A370, AXP) must always be aligned with the 128-bit boundary. This
> requirement can be met by inserting dummy arguments into BIN header.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

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

Thanks,
Stefan

> ---
>   tools/kwbimage.c | 51 ++++++++++++++++++++++++++++++++----------------
>   1 file changed, 34 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/kwbimage.c b/tools/kwbimage.c
> index 77bf4dd8865e..abc88d01b9d8 100644
> --- a/tools/kwbimage.c
> +++ b/tools/kwbimage.c
> @@ -932,6 +932,12 @@ static size_t image_headersz_v1(int *hasext)
>   	 */
>   	headersz = sizeof(struct main_hdr_v1);
>   
> +	if (image_get_csk_index() >= 0) {
> +		headersz += sizeof(struct secure_hdr_v1);
> +		if (hasext)
> +			*hasext = 1;
> +	}
> +
>   	count = image_count_options(IMAGE_CFG_DATA);
>   	if (count > 0)
>   		headersz += sizeof(struct register_set_hdr_v1) + 8 * count + 4;
> @@ -963,15 +969,10 @@ static size_t image_headersz_v1(int *hasext)
>   			return 0;
>   		}
>   
> -		headersz += sizeof(struct opt_hdr_v1) +
> -			ALIGN(s.st_size, 4) +
> -			(binarye->binary.nargs + 2) * sizeof(uint32_t);
> -		if (hasext)
> -			*hasext = 1;
> -	}
> -
> -	if (image_get_csk_index() >= 0) {
> -		headersz += sizeof(struct secure_hdr_v1);
> +		headersz += sizeof(struct opt_hdr_v1) + sizeof(uint32_t) +
> +			(binarye->binary.nargs) * sizeof(uint32_t);
> +		headersz = ALIGN(headersz, 16);
> +		headersz += ALIGN(s.st_size, 4) + sizeof(uint32_t);
>   		if (hasext)
>   			*hasext = 1;
>   	}
> @@ -984,9 +985,12 @@ static size_t image_headersz_v1(int *hasext)
>   }
>   
>   int add_binary_header_v1(uint8_t **cur, uint8_t **next_ext,
> -			 struct image_cfg_element *binarye)
> +			 struct image_cfg_element *binarye,
> +			 struct main_hdr_v1 *main_hdr)
>   {
>   	struct opt_hdr_v1 *hdr = (struct opt_hdr_v1 *)*cur;
> +	uint32_t add_args;
> +	uint32_t offset;
>   	uint32_t *args;
>   	size_t binhdrsz;
>   	struct stat s;
> @@ -1009,12 +1013,6 @@ int add_binary_header_v1(uint8_t **cur, uint8_t **next_ext,
>   		goto err_close;
>   	}
>   
> -	binhdrsz = sizeof(struct opt_hdr_v1) +
> -		(binarye->binary.nargs + 2) * sizeof(uint32_t) +
> -		ALIGN(s.st_size, 4);
> -	hdr->headersz_lsb = cpu_to_le16(binhdrsz & 0xFFFF);
> -	hdr->headersz_msb = (binhdrsz & 0xFFFF0000) >> 16;
> -
>   	*cur += sizeof(struct opt_hdr_v1);
>   
>   	args = (uint32_t *)*cur;
> @@ -1025,6 +1023,19 @@ int add_binary_header_v1(uint8_t **cur, uint8_t **next_ext,
>   
>   	*cur += (binarye->binary.nargs + 1) * sizeof(uint32_t);
>   
> +	/*
> +	 * ARM executable code inside the BIN header on some mvebu platforms
> +	 * (e.g. A370, AXP) must always be aligned with the 128-bit boundary.
> +	 * This requirement can be met by inserting dummy arguments into
> +	 * BIN header, if needed.
> +	 */
> +	offset = *cur - (uint8_t *)main_hdr;
> +	add_args = ((16 - offset % 16) % 16) / sizeof(uint32_t);
> +	if (add_args) {
> +		*(args - 1) = cpu_to_le32(binarye->binary.nargs + add_args);
> +		*cur += add_args * sizeof(uint32_t);
> +	}
> +
>   	ret = fread(*cur, s.st_size, 1, bin);
>   	if (ret != 1) {
>   		fprintf(stderr,
> @@ -1043,6 +1054,12 @@ int add_binary_header_v1(uint8_t **cur, uint8_t **next_ext,
>   
>   	*cur += sizeof(uint32_t);
>   
> +	binhdrsz = sizeof(struct opt_hdr_v1) +
> +		(binarye->binary.nargs + add_args + 2) * sizeof(uint32_t) +
> +		ALIGN(s.st_size, 4);
> +	hdr->headersz_lsb = cpu_to_le16(binhdrsz & 0xFFFF);
> +	hdr->headersz_msb = (binhdrsz & 0xFFFF0000) >> 16;
> +
>   	return 0;
>   
>   err_close:
> @@ -1299,7 +1316,7 @@ static void *image_create_v1(size_t *imagesz, struct image_tool_params *params,
>   		if (e->type != IMAGE_CFG_BINARY)
>   			continue;
>   
> -		if (add_binary_header_v1(&cur, &next_ext, e))
> +		if (add_binary_header_v1(&cur, &next_ext, e, main_hdr))
>   			return NULL;
>   	}
>   
> 


Viele Grüße,
Stefan

-- 
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] arm: mvebu: Add documentation for save_boot_params() function
  2021-10-21 14:46 ` [PATCH 3/4] arm: mvebu: Add documentation for save_boot_params() function Pali Rohár
@ 2021-10-22  6:47   ` Stefan Roese
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Roese @ 2021-10-22  6:47 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Marek Behún, u-boot

On 21.10.21 16:46, Pali Rohár wrote:
> Important detail is availability of kwbimage BIN header arguments passed
> via r0 and r1 registers by BootROM.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

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

Thanks,
Stefan

> ---
>   arch/arm/mach-mvebu/lowlevel_spl.S | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm/mach-mvebu/lowlevel_spl.S b/arch/arm/mach-mvebu/lowlevel_spl.S
> index dde77b765214..39d42912c49f 100644
> --- a/arch/arm/mach-mvebu/lowlevel_spl.S
> +++ b/arch/arm/mach-mvebu/lowlevel_spl.S
> @@ -3,6 +3,15 @@
>   #include <config.h>
>   #include <linux/linkage.h>
>   
> +/*
> + * BootROM loads the header part of kwbimage into L2 cache. BIN header usually
> + * contains U-Boot SPL, optionally it can also contain additional arguments.
> + * The number of these arguments is in r0, pointer to the argument array in r1.
> + * BootROM expects executable BIN header code to return to address stored in lr.
> + * Other registers (r2 - r12) must be preserved. We save all registers to
> + * CONFIG_SPL_BOOTROM_SAVE address. BIN header arguments (passed via r0 and r1)
> + * are currently not used by U-Boot SPL binary.
> + */
>   ENTRY(save_boot_params)
>   	stmfd	sp!, {r0 - r12, lr}	/* @ save registers on stack */
>   	ldr	r12, =CONFIG_SPL_BOOTROM_SAVE
> 


Viele Grüße,
Stefan

-- 
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] arm: mvebu: Remove dummy BIN header arguments for SPL binary
  2021-10-21 14:46 ` [PATCH 4/4] arm: mvebu: Remove dummy BIN header arguments for SPL binary Pali Rohár
@ 2021-10-22  6:48   ` Stefan Roese
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Roese @ 2021-10-22  6:48 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Marek Behún, u-boot

On 21.10.21 16:46, Pali Rohár wrote:
> U-Boot SPL binary does not read BIN header arguments, so passing some dummy
> values 0000005b and 00000068 has no effect for U-Boot SPL code.
> 
> Probably these two values comes from old Marvell DDR training code which
> was separated from U-Boot and used it for some configuration.
> 
> Seems that two 32-bit values were specified here to ensure SPL code
> alignment to 128-bit boundary as it is required e.g. for A370 or AXP
> processors. Main kwbimage header is 64-byte long which is aligned to
> 128-bit boundary. Optional kwbheader is 32-bit long, number of BIN header
> arguments is stored in 32-bit number. So for alignment to 128-bit boundary
> is needed 64-bit padding which exactly these two 32-bit dummy arguments
> provided.
> 
> Now when mkimage correctly aligns start of executable code in BIN header to
> 128-bit boundary, there is no requirement to put dummy argument values into
> kwbimage. So remove them.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

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

Thanks,
Stefan

> ---
>   arch/arm/mach-mvebu/kwbimage.cfg.in | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-mvebu/kwbimage.cfg.in b/arch/arm/mach-mvebu/kwbimage.cfg.in
> index 72e67d75c325..049d23c6ef08 100644
> --- a/arch/arm/mach-mvebu/kwbimage.cfg.in
> +++ b/arch/arm/mach-mvebu/kwbimage.cfg.in
> @@ -9,4 +9,4 @@ VERSION		1
>   #@BOOT_FROM
>   
>   # Binary Header (bin_hdr) with DDR3 training code
> -BINARY spl/u-boot-spl.bin 0000005b 00000068
> +BINARY spl/u-boot-spl.bin
> 


Viele Grüße,
Stefan

-- 
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/4] arm: mvebu: Fix usage of BIN header arguments
  2021-10-21 14:46 [PATCH 0/4] arm: mvebu: Fix usage of BIN header arguments Pali Rohár
                   ` (3 preceding siblings ...)
  2021-10-21 14:46 ` [PATCH 4/4] arm: mvebu: Remove dummy BIN header arguments for SPL binary Pali Rohár
@ 2021-10-28 10:48 ` Stefan Roese
  4 siblings, 0 replies; 10+ messages in thread
From: Stefan Roese @ 2021-10-28 10:48 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Marek Behún, u-boot

On 21.10.21 16:46, Pali Rohár wrote:
> BIN header arguments are used only for aligning ARM executable code
> to 128-bit boundary. This patch series document this behavior and fix
> kwbimage and kwboot code to properly generate BIN headers.
> 
> Pali Rohár (4):
>    tools: kwboot: Align UART baudrate change code in BIN header to
>      128-bit boundary
>    tools: kwbimage: Align BIN header executable code to 128-bit boundary
>    arm: mvebu: Add documentation for save_boot_params() function
>    arm: mvebu: Remove dummy BIN header arguments for SPL binary
> 
>   arch/arm/mach-mvebu/kwbimage.cfg.in |  2 +-
>   arch/arm/mach-mvebu/lowlevel_spl.S  |  9 +++++
>   tools/kwbimage.c                    | 51 +++++++++++++++++++----------
>   tools/kwboot.c                      | 22 ++++++++++---
>   4 files changed, 61 insertions(+), 23 deletions(-)
> 

Applied to u-boot-marvell/master

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-10-28 10:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-21 14:46 [PATCH 0/4] arm: mvebu: Fix usage of BIN header arguments Pali Rohár
2021-10-21 14:46 ` [PATCH 1/4] tools: kwboot: Align UART baudrate change code in BIN header to 128-bit boundary Pali Rohár
2021-10-22  6:47   ` Stefan Roese
2021-10-21 14:46 ` [PATCH 2/4] tools: kwbimage: Align BIN header executable code " Pali Rohár
2021-10-22  6:47   ` Stefan Roese
2021-10-21 14:46 ` [PATCH 3/4] arm: mvebu: Add documentation for save_boot_params() function Pali Rohár
2021-10-22  6:47   ` Stefan Roese
2021-10-21 14:46 ` [PATCH 4/4] arm: mvebu: Remove dummy BIN header arguments for SPL binary Pali Rohár
2021-10-22  6:48   ` Stefan Roese
2021-10-28 10:48 ` [PATCH 0/4] arm: mvebu: Fix usage of BIN header arguments Stefan Roese

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.