Re: [PATCH] fdt: Fix fdt_pack_reg() on 64-bit platforms
On Fri, Mar 29, 2024 at 07:55:53PM -0500, Sam Protsenko wrote:
> When "memory" node is being processed in fdt_pack_reg() on ARM64
> platforms, an unaligned bus access might happen, which leads to
> "synchronous abort" CPU exception. Consider next dts example:
>
> / {
> #address-cells = <2>;
> #size-cells = <1>;
>
> memory@8000 {
> device_type = "memory";
> reg = <0x0 0x8000 0x3ab0>,
> <0x0 0xc000 0x4000>,
> <0x8 0x8000 0x8000>;
> };
> };
>
> After fdt_pack_reg() reads the first addr/size entry from such memory
> node, the "p" pointer becomes 12 bytes shifted from its original value
> (8 bytes for two address cells + 4 bytes for one size cell). So now it's
> not 64-bit aligned, and an attempt to do 64-bit bus access to that
> address will cause an abort like this:
>
> "Synchronous Abort" handler, esr 0x9621, far 0xba235efc
>
> This issue was originally reported by David Virag [1] who observed it
> happening on Samsung Exynos7885 SoC (ARM64), and later the same issue
> was observed on Samsung Exynos850 (ARM64).
>
> Fix the issue by using put_unaligned_be64() helper, which takes care of
> possible unaligned 64-bit accesses. That solution was proposed by Simon
> Glass in the original thread [1].
>
> [1] https://lists.denx.de/pipermail/u-boot/2023-July/522074.html
>
> Fixes: 739a01ed8e02 ("fdt_support: fix an endian bug of
> fdt_fixup_memory_banks")
> Suggested-by: Simon Glass
> Reported-by: David Virag
> Closes: https://lists.denx.de/pipermail/u-boot/2023-July/522074.html
> Signed-off-by: Sam Protsenko
> Reviewed-by: Heinrich Schuchardt
Applied to u-boot/master, thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH] fdt: Fix fdt_pack_reg() on 64-bit platforms
On 3/30/24 01:55, Sam Protsenko wrote:
When "memory" node is being processed in fdt_pack_reg() on ARM64
platforms, an unaligned bus access might happen, which leads to
"synchronous abort" CPU exception. Consider next dts example:
/ {
#address-cells = <2>;
#size-cells = <1>;
memory@8000 {
device_type = "memory";
reg = <0x0 0x8000 0x3ab0>,
<0x0 0xc000 0x4000>,
<0x8 0x8000 0x8000>;
};
};
After fdt_pack_reg() reads the first addr/size entry from such memory
node, the "p" pointer becomes 12 bytes shifted from its original value
(8 bytes for two address cells + 4 bytes for one size cell). So now it's
not 64-bit aligned, and an attempt to do 64-bit bus access to that
address will cause an abort like this:
"Synchronous Abort" handler, esr 0x9621, far 0xba235efc
This issue was originally reported by David Virag [1] who observed it
happening on Samsung Exynos7885 SoC (ARM64), and later the same issue
was observed on Samsung Exynos850 (ARM64).
Fix the issue by using put_unaligned_be64() helper, which takes care of
possible unaligned 64-bit accesses. That solution was proposed by Simon
Glass in the original thread [1].
[1] https://lists.denx.de/pipermail/u-boot/2023-July/522074.html
Fixes: 739a01ed8e02 ("fdt_support: fix an endian bug of fdt_fixup_memory_banks")
Suggested-by: Simon Glass
Reported-by: David Virag
Closes: https://lists.denx.de/pipermail/u-boot/2023-July/522074.html
Signed-off-by: Sam Protsenko
Reviewed-by: Heinrich Schuchardt
---
boot/fdt_support.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/boot/fdt_support.c b/boot/fdt_support.c
index 090d82ee80a5..9844c70be806 100644
--- a/boot/fdt_support.c
+++ b/boot/fdt_support.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -421,13 +422,13 @@ static int fdt_pack_reg(const void *fdt, void *buf, u64
*address, u64 *size,
for (i = 0; i < n; i++) {
if (address_cells == 2)
- *(fdt64_t *)p = cpu_to_fdt64(address[i]);
+ put_unaligned_be64(address[i], p);
else
*(fdt32_t *)p = cpu_to_fdt32(address[i]);
p += 4 * address_cells;
if (size_cells == 2)
- *(fdt64_t *)p = cpu_to_fdt64(size[i]);
+ put_unaligned_be64(size[i], p);
else
*(fdt32_t *)p = cpu_to_fdt32(size[i]);
p += 4 * size_cells;
[PATCH] fdt: Fix fdt_pack_reg() on 64-bit platforms
When "memory" node is being processed in fdt_pack_reg() on ARM64
platforms, an unaligned bus access might happen, which leads to
"synchronous abort" CPU exception. Consider next dts example:
/ {
#address-cells = <2>;
#size-cells = <1>;
memory@8000 {
device_type = "memory";
reg = <0x0 0x8000 0x3ab0>,
<0x0 0xc000 0x4000>,
<0x8 0x8000 0x8000>;
};
};
After fdt_pack_reg() reads the first addr/size entry from such memory
node, the "p" pointer becomes 12 bytes shifted from its original value
(8 bytes for two address cells + 4 bytes for one size cell). So now it's
not 64-bit aligned, and an attempt to do 64-bit bus access to that
address will cause an abort like this:
"Synchronous Abort" handler, esr 0x9621, far 0xba235efc
This issue was originally reported by David Virag [1] who observed it
happening on Samsung Exynos7885 SoC (ARM64), and later the same issue
was observed on Samsung Exynos850 (ARM64).
Fix the issue by using put_unaligned_be64() helper, which takes care of
possible unaligned 64-bit accesses. That solution was proposed by Simon
Glass in the original thread [1].
[1] https://lists.denx.de/pipermail/u-boot/2023-July/522074.html
Fixes: 739a01ed8e02 ("fdt_support: fix an endian bug of fdt_fixup_memory_banks")
Suggested-by: Simon Glass
Reported-by: David Virag
Closes: https://lists.denx.de/pipermail/u-boot/2023-July/522074.html
Signed-off-by: Sam Protsenko
---
boot/fdt_support.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/boot/fdt_support.c b/boot/fdt_support.c
index 090d82ee80a5..9844c70be806 100644
--- a/boot/fdt_support.c
+++ b/boot/fdt_support.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -421,13 +422,13 @@ static int fdt_pack_reg(const void *fdt, void *buf, u64
*address, u64 *size,
for (i = 0; i < n; i++) {
if (address_cells == 2)
- *(fdt64_t *)p = cpu_to_fdt64(address[i]);
+ put_unaligned_be64(address[i], p);
else
*(fdt32_t *)p = cpu_to_fdt32(address[i]);
p += 4 * address_cells;
if (size_cells == 2)
- *(fdt64_t *)p = cpu_to_fdt64(size[i]);
+ put_unaligned_be64(size[i], p);
else
*(fdt32_t *)p = cpu_to_fdt32(size[i]);
p += 4 * size_cells;
--
2.39.2
