[U-Boot] static var mem alignment issue/question

[Måns Rullgård]

Tim Harvey <tharvey at gateworks.com> writes:

> Greetings,
>
> In debugging an issue with a rather old branch of U-Boot (2015-04) I
> found that the static assignment of a data buffer was not 32-bit
> aligned which caused data aborts. However I find that current U-boot
> master does not suffer this issue and no matter what I declare static
> before the particular variable its always 32-bit aligned. I don't see
> any code changes that would cause this and in both cases I'm building
> with the same gcc5.2.0 toolchain.
>
> The code in question is this this from cmd/fdt.c:
>
>         } else if (argv[1][0] == 's') {
>                 char *pathp;            /* path */
>                 char *prop;             /* property */
>                 int  nodeoffset;        /* node offset from libfdt */
>                 static char data[SCRATCHPAD];   /* storage for the property */
>                 int  len;               /* new length of the property */
>                 int  ret;
>
> What guarantee's that 'data' above is 32-bit aligned in master that is
> missing from the older U-Boot branch I'm using? Perhaps there is some
> arg in a Makefile that I'm missing?

There are no such guarantees.  Things often end up 4-byte aligned by
chance simply because most of them are a multiple of 4 bytes in size.
The code should be fixed, either by explicitly aligning the buffer or by
using the put_unaligned_b32() accessor in fdt_parse_prop().  Here's an
untested patch:

>From 714f6a16062cc43e9aadfba29b295365fd4926de Mon Sep 17 00:00:00 2001
From: Mans Rullgard <mans at mansr.com>
Date: Tue, 29 Nov 2016 11:59:37 +0000
Subject: [PATCH] cmd/fdt: fix unaligned memory access

The buffer passed to fdt_parse_prop() might be unaligned.  Use the
proper access helper when writing 32-bit values.

Signed-off-by: Mans Rullgard <mans at mansr.com>
---
 cmd/fdt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cmd/fdt.c b/cmd/fdt.c
index b503357dc3a8..560b8cc9df61 100644
--- a/cmd/fdt.c
+++ b/cmd/fdt.c
@@ -17,6 +17,7 @@
 #include <fdt_support.h>
 #include <mapmem.h>
 #include <asm/io.h>
+#include <asm/unaligned.h>
 
 #define MAX_LEVEL	32		/* how deeply nested we will go */
 #define SCRATCHPAD	1024		/* bytes of scratchpad memory */
@@ -766,7 +767,7 @@ static int fdt_parse_prop(char * const *newval, int count, char *data, int *len)
 
 			cp = newp;
 			tmp = simple_strtoul(cp, &newp, 0);
-			*(__be32 *)data = __cpu_to_be32(tmp);
+			put_unaligned_be32(tmp, data);
 			data  += 4;
 			*len += 4;
 
-- 
2.10.2

-- 
Måns Rullgård