[PATCH] cmd: bcb: fix bcb struct alignment issue

[PATCH] cmd: bcb: fix bcb struct alignment issue

[Gary Bisson]

Without this patch the bcb struct could be located at an odd address
which resulted in data not being copied to the buffer.

Here was the repro steps (from Mattijs):
=> mmc dev 1
=> bcb load 1 misc
=> bcb dump command
00000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=> part start mmc 1 misc misc_start
=> mmc read ${loadaddr} ${misc_start} 4
=> bcb load 1 misc
=> bcb dump command
00000000: 62 6f 6f 74 6f 6e 63 65 2d 62 6f 6f 74 6c 6f 61
00000010: 64 65 72 00 00 00 00 00 00 00 00 00 00 00 00 00

This behavior was observed on an Amlogic A311D (ARM64) platform with a
recent GCC toolchain (11.2.0) but is most likely affecting other
platforms.

To avoid issues the structure is aligned on DMA minimum alignment value
as it is passed directly to the read function.

Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>
---
 cmd/bcb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cmd/bcb.c b/cmd/bcb.c
index 6b6f1e9a2f1..92f4d27990d 100644
--- a/cmd/bcb.c
+++ b/cmd/bcb.c
@@ -12,6 +12,7 @@
 #include <log.h>
 #include <part.h>
 #include <malloc.h>
+#include <memalign.h>
 
 enum bcb_cmd {
 	BCB_CMD_LOAD,
@@ -24,7 +25,7 @@ enum bcb_cmd {
 
 static int bcb_dev = -1;
 static int bcb_part = -1;
-static struct bootloader_message bcb = { { 0 } };
+static struct bootloader_message bcb __aligned(ARCH_DMA_MINALIGN) = { { 0 } };
 
 static int bcb_cmd_get(char *cmd)
 {
-- 
2.34.1

Re: [PATCH] cmd: bcb: fix bcb struct alignment issue

[Mattijs Korpershoek]

Hi Gary,

Thank you for your patch.

Gary Bisson <gary.bisson@boundarydevices.com> writes:

> Without this patch the bcb struct could be located at an odd address
> which resulted in data not being copied to the buffer.
>
> Here was the repro steps (from Mattijs):
> => mmc dev 1
> => bcb load 1 misc
> => bcb dump command
> 00000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> => part start mmc 1 misc misc_start
> => mmc read ${loadaddr} ${misc_start} 4
> => bcb load 1 misc
> => bcb dump command
> 00000000: 62 6f 6f 74 6f 6e 63 65 2d 62 6f 6f 74 6c 6f 61
> 00000010: 64 65 72 00 00 00 00 00 00 00 00 00 00 00 00 00
>
> This behavior was observed on an Amlogic A311D (ARM64) platform with a
> recent GCC toolchain (11.2.0) but is most likely affecting other
> platforms.
>
> To avoid issues the structure is aligned on DMA minimum alignment value
> as it is passed directly to the read function.
>
> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>
> ---
>  cmd/bcb.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
Tested-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> # on khadas vim3

>
> diff --git a/cmd/bcb.c b/cmd/bcb.c
> index 6b6f1e9a2f1..92f4d27990d 100644
> --- a/cmd/bcb.c
> +++ b/cmd/bcb.c
> @@ -12,6 +12,7 @@
>  #include <log.h>
>  #include <part.h>
>  #include <malloc.h>
> +#include <memalign.h>
>  
>  enum bcb_cmd {
>  	BCB_CMD_LOAD,
> @@ -24,7 +25,7 @@ enum bcb_cmd {
>  
>  static int bcb_dev = -1;
>  static int bcb_part = -1;
> -static struct bootloader_message bcb = { { 0 } };
> +static struct bootloader_message bcb __aligned(ARCH_DMA_MINALIGN) = { { 0 } };
>  
>  static int bcb_cmd_get(char *cmd)
>  {
> -- 
> 2.34.1

[PATCH] cmd: bcb: fix bcb struct alignment issue

[sbabic]

> Without this patch the bcb struct could be located at an odd address
> which resulted in data not being copied to the buffer.
> Here was the repro steps (from Mattijs):
> => mmc dev 1
> => bcb load 1 misc
> => bcb dump command
> 00000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> => part start mmc 1 misc misc_start
> => mmc read ${loadaddr} ${misc_start} 4
> => bcb load 1 misc
> => bcb dump command
> 00000000: 62 6f 6f 74 6f 6e 63 65 2d 62 6f 6f 74 6c 6f 61
> 00000010: 64 65 72 00 00 00 00 00 00 00 00 00 00 00 00 00
> This behavior was observed on an Amlogic A311D (ARM64) platform with a
> recent GCC toolchain (11.2.0) but is most likely affecting other
> platforms.
> To avoid issues the structure is aligned on DMA minimum alignment value
> as it is passed directly to the read function.
> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>
> Tested-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> # on khadas vim3
Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

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