[U-Boot,v2] fdt: Fix alignment issue when reading 64-bits properties from fdt
diff mbox series

Message ID 20191022080522.2113-1-jjhiblot@ti.com
State Accepted
Commit d60ae4c59df55c08dc96202ff58fed21ab3afb7d
Delegated to: Simon Glass
Headers show
Series
  • [U-Boot,v2] fdt: Fix alignment issue when reading 64-bits properties from fdt
Related show

Commit Message

Jean-Jacques Hiblot Oct. 22, 2019, 8:05 a.m. UTC
The FDT specification [0] gives a requirement of aligning properties on
32-bits. Make sure that the compiler is aware of this constraint when
accessing 64-bits properties.

[0]: https://github.com/devicetree-org/devicetree-specification/blob/master/source/flattened-format.rst

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---

Here is a portion of the disassembly of ofnode_read_u64(). It show the effect
of the patch on ARM64.

with the classic fdt64_t type. GCC emits LDR (load register)
  34:   54000109        b.ls    54 <ofnode_read_u64+0x54>  // b.plast
  38:   f9400000        ldr     x0, [x0]
  3c:   dac00c00        rev     x0, x0

with the unaligned_fdt64_t type. GCC emits LDP (Load Pair of registers)
  34:   54000129        b.ls    58 <ofnode_read_u64+0x58>  // b.plast
  38:   29400001        ldp     w1, w0, [x0]
  3c:   aa008020        orr     x0, x1, x0, lsl #32
  40:   dac00c00        rev     x0, x0
  

 common/fdt_support.c       | 2 +-
 drivers/core/ofnode.c      | 2 +-
 include/linux/libfdt_env.h | 1 +
 lib/fdtdec.c               | 2 +-
 4 files changed, 4 insertions(+), 3 deletions(-)

Comments

Simon Glass Oct. 27, 2019, 4:32 p.m. UTC | #1
On Tue, 22 Oct 2019 at 02:05, Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
>
> The FDT specification [0] gives a requirement of aligning properties on
> 32-bits. Make sure that the compiler is aware of this constraint when
> accessing 64-bits properties.
>
> [0]: https://github.com/devicetree-org/devicetree-specification/blob/master/source/flattened-format.rst
>
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> ---
>
> Here is a portion of the disassembly of ofnode_read_u64(). It show the effect
> of the patch on ARM64.
>
> with the classic fdt64_t type. GCC emits LDR (load register)
>   34:   54000109        b.ls    54 <ofnode_read_u64+0x54>  // b.plast
>   38:   f9400000        ldr     x0, [x0]
>   3c:   dac00c00        rev     x0, x0
>
> with the unaligned_fdt64_t type. GCC emits LDP (Load Pair of registers)
>   34:   54000129        b.ls    58 <ofnode_read_u64+0x58>  // b.plast
>   38:   29400001        ldp     w1, w0, [x0]
>   3c:   aa008020        orr     x0, x1, x0, lsl #32
>   40:   dac00c00        rev     x0, x0
>
>
>  common/fdt_support.c       | 2 +-
>  drivers/core/ofnode.c      | 2 +-
>  include/linux/libfdt_env.h | 1 +
>  lib/fdtdec.c               | 2 +-
>  4 files changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

No change log?
Simon Glass Oct. 29, 2019, 11:21 p.m. UTC | #2
On Tue, 22 Oct 2019 at 02:05, Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
>
> The FDT specification [0] gives a requirement of aligning properties on
> 32-bits. Make sure that the compiler is aware of this constraint when
> accessing 64-bits properties.
>
> [0]: https://github.com/devicetree-org/devicetree-specification/blob/master/source/flattened-format.rst
>
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> ---
>
> Here is a portion of the disassembly of ofnode_read_u64(). It show the effect
> of the patch on ARM64.
>
> with the classic fdt64_t type. GCC emits LDR (load register)
>   34:   54000109        b.ls    54 <ofnode_read_u64+0x54>  // b.plast
>   38:   f9400000        ldr     x0, [x0]
>   3c:   dac00c00        rev     x0, x0
>
> with the unaligned_fdt64_t type. GCC emits LDP (Load Pair of registers)
>   34:   54000129        b.ls    58 <ofnode_read_u64+0x58>  // b.plast
>   38:   29400001        ldp     w1, w0, [x0]
>   3c:   aa008020        orr     x0, x1, x0, lsl #32
>   40:   dac00c00        rev     x0, x0
>
>
>  common/fdt_support.c       | 2 +-
>  drivers/core/ofnode.c      | 2 +-
>  include/linux/libfdt_env.h | 1 +
>  lib/fdtdec.c               | 2 +-
>  4 files changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

No change log?

Applied to u-boot-dm, thanks!

Patch
diff mbox series

diff --git a/common/fdt_support.c b/common/fdt_support.c
index baf7924ff6..6834399039 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -1566,7 +1566,7 @@  static int fdt_read_prop(const fdt32_t *prop, int prop_len, int cell_off,
 			 uint64_t *val, int cells)
 {
 	const fdt32_t *prop32 = &prop[cell_off];
-	const fdt64_t *prop64 = (const fdt64_t *)&prop[cell_off];
+	const unaligned_fdt64_t *prop64 = (const fdt64_t *)&prop[cell_off];
 
 	if ((cell_off + cells) > prop_len)
 		return -FDT_ERR_NOSPACE;
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 297f0a0c7c..8f0eab2ca6 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -57,7 +57,7 @@  int ofnode_read_s32_default(ofnode node, const char *propname, s32 def)
 
 int ofnode_read_u64(ofnode node, const char *propname, u64 *outp)
 {
-	const fdt64_t *cell;
+	const unaligned_fdt64_t *cell;
 	int len;
 
 	assert(ofnode_valid(node));
diff --git a/include/linux/libfdt_env.h b/include/linux/libfdt_env.h
index e2bf79c7ee..e49fcd72bd 100644
--- a/include/linux/libfdt_env.h
+++ b/include/linux/libfdt_env.h
@@ -16,6 +16,7 @@ 
 typedef __be16 fdt16_t;
 typedef __be32 fdt32_t;
 typedef __be64 fdt64_t;
+typedef __be64 unaligned_fdt64_t __aligned(4);
 
 #define fdt32_to_cpu(x) be32_to_cpu(x)
 #define cpu_to_fdt32(x) cpu_to_be32(x)
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 17736ce665..125d9dbf26 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -242,7 +242,7 @@  int fdtdec_get_pci_bar32(struct udevice *dev, struct fdt_pci_addr *addr,
 uint64_t fdtdec_get_uint64(const void *blob, int node, const char *prop_name,
 			   uint64_t default_val)
 {
-	const uint64_t *cell64;
+	const unaligned_fdt64_t *cell64;
 	int length;
 
 	cell64 = fdt_getprop(blob, node, prop_name, &length);