[PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[Marek Vasut]

Newer versions of libfdt strictly check whether the FDT blob
passed to them is at 8-byte aligned offset, if it is not, then
the library fails checks with -FDT_ERR_ALIGNMENT . Currently,
android_image_print_dtb_contents() passed FDT directly mapped
from abootimg to libfdt, and this FDT is not always aligned to
8-byte offset. Specifically, the FDTs are somewhat packed in
the abootimg, therefore if the first FDT blob is e.g. 0xfd bytes
long, then the next FDT blob ends up at 0xfd offset, which is
not 8-byte aligned.

Fix this by first extracting the header into 8-byte aligned buffer,
checking only the header for validity, and then by copying the
entire FDT into newly allocated 8-byte aligned buffer. While this
is not efficient, it is the correct way to handle DTs, which must
be at 8-byte aligned offsets. Mitigate the inefficiency for the
common case by checking whether the DT might be 8-byte aligned and
if it is, map it directly.

Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Aaron Kling <webgeek1234@gmail.com>
Cc: Eddie Kovsky <ekovsky@redhat.com>
Cc: George Chan <gchan9527@gmail.com>
Cc: Julien Masson <jmasson@baylibre.com>
Cc: Mattijs Korpershoek <mkorpershoek@kernel.org>
Cc: Nicolas Belin <nbelin@baylibre.com>
Cc: Sam Day <me@samcday.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: u-boot@lists.denx.de
---
 boot/image-android.c | 46 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/boot/image-android.c b/boot/image-android.c
index e46dee0d9b3..5ff2bf664e5 100644
--- a/boot/image-android.c
+++ b/boot/image-android.c
@@ -721,17 +721,19 @@ bool android_image_get_dtb_by_index(ulong hdr_addr, ulong vendor_boot_img,
 	dtb_addr = dtb_img_addr;
 	while (dtb_addr < dtb_img_addr + dtb_img_size) {
 		const struct fdt_header *fdt;
+		struct fdt_header fdth __aligned(8);
 		u32 dtb_size;
 
 		fdt = map_sysmem(dtb_addr, sizeof(*fdt));
-		if (fdt_check_header(fdt) != 0) {
-			unmap_sysmem(fdt);
+		memcpy(&fdth, fdt, sizeof(*fdt));
+		unmap_sysmem(fdt);
+
+		if (fdt_check_header(&fdth) != 0) {
 			printf("Error: Invalid FDT header for index %u\n", i);
 			return false;
 		}
 
-		dtb_size = fdt_totalsize(fdt);
-		unmap_sysmem(fdt);
+		dtb_size = fdt_totalsize(&fdth);
 
 		if (i == index) {
 			if (size)
@@ -885,23 +887,45 @@ bool android_image_print_dtb_contents(ulong hdr_addr)
 	dtb_addr = dtb_img_addr;
 	while (dtb_addr < dtb_img_addr + dtb_img_size) {
 		const struct fdt_header *fdt;
+		struct fdt_header *fulldt;
+		struct fdt_header fdth __aligned(8);
 		u32 dtb_size;
 
 		fdt = map_sysmem(dtb_addr, sizeof(*fdt));
-		if (fdt_check_header(fdt) != 0) {
-			unmap_sysmem(fdt);
+		memcpy(&fdth, fdt, sizeof(*fdt));
+		unmap_sysmem(fdt);
+
+		if (fdt_check_header(&fdth) != 0) {
 			printf("Error: Invalid FDT header for index %u\n", i);
 			return false;
 		}
 
-		res = android_image_print_dtb_info(fdt, i);
-		if (!res) {
+		dtb_size = fdt_totalsize(&fdth);
+
+		/* The device tree must be at an 8-byte aligned address */
+		if ((uintptr_t)fdt & 7) {
+			fulldt = memalign(8, dtb_size);
+			if (!fulldt)
+				return false;
+
+			fdt = map_sysmem(dtb_addr, dtb_size);
+			memcpy(fulldt, fdt, dtb_size);
 			unmap_sysmem(fdt);
-			return false;
+		} else {
+			fulldt = map_sysmem(dtb_addr, dtb_size);
 		}
 
-		dtb_size = fdt_totalsize(fdt);
-		unmap_sysmem(fdt);
+		res = android_image_print_dtb_info(fulldt, i);
+
+		/* The device tree must be at an 8-byte aligned address */
+		if ((uintptr_t)fdt & 7)
+			free(fulldt);
+		else
+			unmap_sysmem(fulldt);
+
+		if (!res)
+			return false;
+
 		dtb_addr += dtb_size;
 		++i;
 	}
-- 
2.51.0

[PATCH 2/2] test/py: android: Point fdt command to aligned addresses

[Marek Vasut]

Newer versions of libfdt strictly check whether the FDT blob
passed to them is at 8-byte aligned offset, if it is not, then
the library fails checks with -FDT_ERR_ALIGNMENT . Currently,
'abootimg get dtb --index=1 addr size' may return non 8-byte
aligned FDT address which points directly into the abootimg.
Copy the result into temporary location before validation to
avoid FDT alignment check failure.

Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Aaron Kling <webgeek1234@gmail.com>
Cc: Eddie Kovsky <ekovsky@redhat.com>
Cc: George Chan <gchan9527@gmail.com>
Cc: Julien Masson <jmasson@baylibre.com>
Cc: Mattijs Korpershoek <mkorpershoek@kernel.org>
Cc: Nicolas Belin <nbelin@baylibre.com>
Cc: Sam Day <me@samcday.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: u-boot@lists.denx.de
---
 test/py/tests/test_android/test_abootimg.py | 28 ++++++++++++++++-----
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/test/py/tests/test_android/test_abootimg.py b/test/py/tests/test_android/test_abootimg.py
index 2aadb692b30..daa87643e93 100644
--- a/test/py/tests/test_android/test_abootimg.py
+++ b/test/py/tests/test_android/test_abootimg.py
@@ -93,10 +93,16 @@ dtb_dump_resp="""## DTB area contents (concat format):
 loadaddr = 0x1000
 # Address in RAM where to load the vendor boot image ('abootimg' looks in $vloadaddr)
 vloadaddr= 0x10000
+# Expected DTB #1 size
+dtb1_size = 0x7d
+# Expected DTB #2 size
+dtb2_size = 0x7d
 # Expected DTB #1 offset from the boot image start address
-dtb1_offset = 0x187d
+dtb1_offset = 0x1800 + dtb1_size
 # Expected DTB offset from the vendor boot image start address
-dtb2_offset = 0x207d
+dtb2_offset = 0x2000 + dtb2_size
+# Expected DTB aligned offset
+dtba_offset = 0x4000
 # DTB #1 start address in RAM
 dtb1_addr = loadaddr + dtb1_offset
 # DTB #2 start address in RAM
@@ -214,11 +220,16 @@ def test_abootimg(abootimg_disk_image, ubman):
     assert response == 'a=11f00000'
 
     ubman.log.action('Testing \'abootimg get dtb --index\'...')
-    ubman.run_command('abootimg get dtb --index=1 dtb1_start')
+    ubman.run_command('abootimg get dtb --index=1 dtb1_start dtb1_size')
     response = ubman.run_command('env print dtb1_start')
     correct_str = "dtb1_start=%x" % (dtb1_addr)
     assert response == correct_str
-    ubman.run_command('fdt addr $dtb1_start')
+    response = ubman.run_command('env print dtb1_size')
+    correct_str = "dtb1_size=%x" % (dtb1_size)
+    assert response == correct_str
+    ubman.run_command('setenv dtbaaddr 0x%x' % (dtba_offset))
+    ubman.run_command('cp.b $dtb1_start $dtbaaddr $dtb1_size')
+    ubman.run_command('fdt addr $dtbaaddr')
     ubman.run_command('fdt get value v / model')
     response = ubman.run_command('env print v')
     assert response == 'v=x2'
@@ -257,12 +268,17 @@ def test_abootimgv4(abootimgv4_disk_image_vboot, abootimgv4_disk_image_boot, ubm
     assert response == 'a=11f00000'
 
     ubman.log.action('Testing \'abootimg get dtb --index\'...')
-    ubman.run_command('abootimg get dtb --index=1 dtb2_start')
+    ubman.run_command('abootimg get dtb --index=1 dtb2_start dtb2_size')
     response = ubman.run_command('env print dtb2_start')
     correct_str = "dtb2_start=%x" % (dtb2_addr)
     assert response == correct_str
+    response = ubman.run_command('env print dtb2_size')
+    correct_str = "dtb2_size=%x" % (dtb2_size)
+    assert response == correct_str
 
-    ubman.run_command('fdt addr $dtb2_start')
+    ubman.run_command('setenv dtbaaddr 0x%x' % (dtba_offset))
+    ubman.run_command('cp.b $dtb2_start $dtbaaddr $dtb2_size')
+    ubman.run_command('fdt addr $dtbaaddr')
     ubman.run_command('fdt get value v / model')
     response = ubman.run_command('env print v')
     assert response == 'v=x2'
-- 
2.51.0

Re: [PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[Simon Glass]

Hi Marek,

On Tue, 11 Nov 2025 at 18:17, Marek Vasut
<marek.vasut+renesas@mailbox.org> wrote:
>
> Newer versions of libfdt strictly check whether the FDT blob
> passed to them is at 8-byte aligned offset, if it is not, then
> the library fails checks with -FDT_ERR_ALIGNMENT . Currently,
> android_image_print_dtb_contents() passed FDT directly mapped
> from abootimg to libfdt, and this FDT is not always aligned to
> 8-byte offset. Specifically, the FDTs are somewhat packed in
> the abootimg, therefore if the first FDT blob is e.g. 0xfd bytes
> long, then the next FDT blob ends up at 0xfd offset, which is
> not 8-byte aligned.
>
> Fix this by first extracting the header into 8-byte aligned buffer,
> checking only the header for validity, and then by copying the
> entire FDT into newly allocated 8-byte aligned buffer. While this
> is not efficient, it is the correct way to handle DTs, which must
> be at 8-byte aligned offsets. Mitigate the inefficiency for the
> common case by checking whether the DT might be 8-byte aligned and
> if it is, map it directly.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: Aaron Kling <webgeek1234@gmail.com>
> Cc: Eddie Kovsky <ekovsky@redhat.com>
> Cc: George Chan <gchan9527@gmail.com>
> Cc: Julien Masson <jmasson@baylibre.com>
> Cc: Mattijs Korpershoek <mkorpershoek@kernel.org>
> Cc: Nicolas Belin <nbelin@baylibre.com>
> Cc: Sam Day <me@samcday.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: u-boot@lists.denx.de
> ---
>  boot/image-android.c | 46 +++++++++++++++++++++++++++++++++-----------
>  1 file changed, 35 insertions(+), 11 deletions(-)
>

OK, we need to adjust mkimage etc. to handle this. Until then I think
we should have a way to disable the 8-byte-alignment check as it seems
to have an enormous blast radius :-)

Regards,
Simon

Re: [PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2542 bytes --]

On Thu, Nov 13, 2025 at 12:33:24PM -0700, Simon Glass wrote:
> Hi Marek,
> 
> On Tue, 11 Nov 2025 at 18:17, Marek Vasut
> <marek.vasut+renesas@mailbox.org> wrote:
> >
> > Newer versions of libfdt strictly check whether the FDT blob
> > passed to them is at 8-byte aligned offset, if it is not, then
> > the library fails checks with -FDT_ERR_ALIGNMENT . Currently,
> > android_image_print_dtb_contents() passed FDT directly mapped
> > from abootimg to libfdt, and this FDT is not always aligned to
> > 8-byte offset. Specifically, the FDTs are somewhat packed in
> > the abootimg, therefore if the first FDT blob is e.g. 0xfd bytes
> > long, then the next FDT blob ends up at 0xfd offset, which is
> > not 8-byte aligned.
> >
> > Fix this by first extracting the header into 8-byte aligned buffer,
> > checking only the header for validity, and then by copying the
> > entire FDT into newly allocated 8-byte aligned buffer. While this
> > is not efficient, it is the correct way to handle DTs, which must
> > be at 8-byte aligned offsets. Mitigate the inefficiency for the
> > common case by checking whether the DT might be 8-byte aligned and
> > if it is, map it directly.
> >
> > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > ---
> > Cc: Aaron Kling <webgeek1234@gmail.com>
> > Cc: Eddie Kovsky <ekovsky@redhat.com>
> > Cc: George Chan <gchan9527@gmail.com>
> > Cc: Julien Masson <jmasson@baylibre.com>
> > Cc: Mattijs Korpershoek <mkorpershoek@kernel.org>
> > Cc: Nicolas Belin <nbelin@baylibre.com>
> > Cc: Sam Day <me@samcday.com>
> > Cc: Simon Glass <sjg@chromium.org>
> > Cc: Tom Rini <trini@konsulko.com>
> > Cc: u-boot@lists.denx.de
> > ---
> >  boot/image-android.c | 46 +++++++++++++++++++++++++++++++++-----------
> >  1 file changed, 35 insertions(+), 11 deletions(-)
> >
> 
> OK, we need to adjust mkimage etc. to handle this. Until then I think
> we should have a way to disable the 8-byte-alignment check as it seems
> to have an enormous blast radius :-)

We can't. We really can't. Passing misaligned device trees results in,
and has already resulted in for years:
- Silent Linux crashes
- When Linux doesn't crash so many of the read functions go wrong the
  result is incomprehensible.

It's something that has and continues to waste untold engineering time.
The only reason it's not worse is that we've killed off *most* of the
platforms that disabled device tree relocation and that's how
misalignment happens downstream most often.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[Simon Glass]

Hi Tom,

On Thu, 13 Nov 2025 at 12:38, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Nov 13, 2025 at 12:33:24PM -0700, Simon Glass wrote:
> > Hi Marek,
> >
> > On Tue, 11 Nov 2025 at 18:17, Marek Vasut
> > <marek.vasut+renesas@mailbox.org> wrote:
> > >
> > > Newer versions of libfdt strictly check whether the FDT blob
> > > passed to them is at 8-byte aligned offset, if it is not, then
> > > the library fails checks with -FDT_ERR_ALIGNMENT . Currently,
> > > android_image_print_dtb_contents() passed FDT directly mapped
> > > from abootimg to libfdt, and this FDT is not always aligned to
> > > 8-byte offset. Specifically, the FDTs are somewhat packed in
> > > the abootimg, therefore if the first FDT blob is e.g. 0xfd bytes
> > > long, then the next FDT blob ends up at 0xfd offset, which is
> > > not 8-byte aligned.
> > >
> > > Fix this by first extracting the header into 8-byte aligned buffer,
> > > checking only the header for validity, and then by copying the
> > > entire FDT into newly allocated 8-byte aligned buffer. While this
> > > is not efficient, it is the correct way to handle DTs, which must
> > > be at 8-byte aligned offsets. Mitigate the inefficiency for the
> > > common case by checking whether the DT might be 8-byte aligned and
> > > if it is, map it directly.
> > >
> > > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > > ---
> > > Cc: Aaron Kling <webgeek1234@gmail.com>
> > > Cc: Eddie Kovsky <ekovsky@redhat.com>
> > > Cc: George Chan <gchan9527@gmail.com>
> > > Cc: Julien Masson <jmasson@baylibre.com>
> > > Cc: Mattijs Korpershoek <mkorpershoek@kernel.org>
> > > Cc: Nicolas Belin <nbelin@baylibre.com>
> > > Cc: Sam Day <me@samcday.com>
> > > Cc: Simon Glass <sjg@chromium.org>
> > > Cc: Tom Rini <trini@konsulko.com>
> > > Cc: u-boot@lists.denx.de
> > > ---
> > >  boot/image-android.c | 46 +++++++++++++++++++++++++++++++++-----------
> > >  1 file changed, 35 insertions(+), 11 deletions(-)
> > >
> >
> > OK, we need to adjust mkimage etc. to handle this. Until then I think
> > we should have a way to disable the 8-byte-alignment check as it seems
> > to have an enormous blast radius :-)
>
> We can't. We really can't. Passing misaligned device trees results in,
> and has already resulted in for years:
> - Silent Linux crashes
> - When Linux doesn't crash so many of the read functions go wrong the
>   result is incomprehensible.
>
> It's something that has and continues to waste untold engineering time.
> The only reason it's not worse is that we've killed off *most* of the
> platforms that disabled device tree relocation and that's how
> misalignment happens downstream most often.

That's fine, but making the devicetree aligned for Linux is pretty
simple. We generally relocate it before booting anyway.

But that's not what I'm seeing here. It seems to be updating U-Boot to
deal with a libfdt change. That is also a worthy goal, but we need to
consider code size and where best in the stack the changes should be
made. Doing a memalign() at the last minute is just not a good idea.

Regards,
Simon

Re: [PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3805 bytes --]

On Thu, Nov 13, 2025 at 12:57:30PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 13 Nov 2025 at 12:38, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Nov 13, 2025 at 12:33:24PM -0700, Simon Glass wrote:
> > > Hi Marek,
> > >
> > > On Tue, 11 Nov 2025 at 18:17, Marek Vasut
> > > <marek.vasut+renesas@mailbox.org> wrote:
> > > >
> > > > Newer versions of libfdt strictly check whether the FDT blob
> > > > passed to them is at 8-byte aligned offset, if it is not, then
> > > > the library fails checks with -FDT_ERR_ALIGNMENT . Currently,
> > > > android_image_print_dtb_contents() passed FDT directly mapped
> > > > from abootimg to libfdt, and this FDT is not always aligned to
> > > > 8-byte offset. Specifically, the FDTs are somewhat packed in
> > > > the abootimg, therefore if the first FDT blob is e.g. 0xfd bytes
> > > > long, then the next FDT blob ends up at 0xfd offset, which is
> > > > not 8-byte aligned.
> > > >
> > > > Fix this by first extracting the header into 8-byte aligned buffer,
> > > > checking only the header for validity, and then by copying the
> > > > entire FDT into newly allocated 8-byte aligned buffer. While this
> > > > is not efficient, it is the correct way to handle DTs, which must
> > > > be at 8-byte aligned offsets. Mitigate the inefficiency for the
> > > > common case by checking whether the DT might be 8-byte aligned and
> > > > if it is, map it directly.
> > > >
> > > > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > > > ---
> > > > Cc: Aaron Kling <webgeek1234@gmail.com>
> > > > Cc: Eddie Kovsky <ekovsky@redhat.com>
> > > > Cc: George Chan <gchan9527@gmail.com>
> > > > Cc: Julien Masson <jmasson@baylibre.com>
> > > > Cc: Mattijs Korpershoek <mkorpershoek@kernel.org>
> > > > Cc: Nicolas Belin <nbelin@baylibre.com>
> > > > Cc: Sam Day <me@samcday.com>
> > > > Cc: Simon Glass <sjg@chromium.org>
> > > > Cc: Tom Rini <trini@konsulko.com>
> > > > Cc: u-boot@lists.denx.de
> > > > ---
> > > >  boot/image-android.c | 46 +++++++++++++++++++++++++++++++++-----------
> > > >  1 file changed, 35 insertions(+), 11 deletions(-)
> > > >
> > >
> > > OK, we need to adjust mkimage etc. to handle this. Until then I think
> > > we should have a way to disable the 8-byte-alignment check as it seems
> > > to have an enormous blast radius :-)
> >
> > We can't. We really can't. Passing misaligned device trees results in,
> > and has already resulted in for years:
> > - Silent Linux crashes
> > - When Linux doesn't crash so many of the read functions go wrong the
> >   result is incomprehensible.
> >
> > It's something that has and continues to waste untold engineering time.
> > The only reason it's not worse is that we've killed off *most* of the
> > platforms that disabled device tree relocation and that's how
> > misalignment happens downstream most often.
> 
> That's fine, but making the devicetree aligned for Linux is pretty
> simple. We generally relocate it before booting anyway.
> 
> But that's not what I'm seeing here. It seems to be updating U-Boot to
> deal with a libfdt change. That is also a worthy goal, but we need to
> consider code size and where best in the stack the changes should be
> made. Doing a memalign() at the last minute is just not a good idea.

Yes, the device tree specification, not the OS, says that it must be 8
byte aligned. We've needed to do that for ages. Not doing that is not a
good thing, for U-Boot, internally. The same APIs that blow up in Linux
(or anything else) with misaligned device trees will blow up in us too.
We need to make sure we pass ourselves something aligned correctly.

Which is different from disagreeing with your specific feedback about
how we get there, to be clear.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[Simon Glass]

Hi Tom,

On Thu, 13 Nov 2025 at 13:05, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Nov 13, 2025 at 12:57:30PM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 13 Nov 2025 at 12:38, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Nov 13, 2025 at 12:33:24PM -0700, Simon Glass wrote:
> > > > Hi Marek,
> > > >
> > > > On Tue, 11 Nov 2025 at 18:17, Marek Vasut
> > > > <marek.vasut+renesas@mailbox.org> wrote:
> > > > >
> > > > > Newer versions of libfdt strictly check whether the FDT blob
> > > > > passed to them is at 8-byte aligned offset, if it is not, then
> > > > > the library fails checks with -FDT_ERR_ALIGNMENT . Currently,
> > > > > android_image_print_dtb_contents() passed FDT directly mapped
> > > > > from abootimg to libfdt, and this FDT is not always aligned to
> > > > > 8-byte offset. Specifically, the FDTs are somewhat packed in
> > > > > the abootimg, therefore if the first FDT blob is e.g. 0xfd bytes
> > > > > long, then the next FDT blob ends up at 0xfd offset, which is
> > > > > not 8-byte aligned.
> > > > >
> > > > > Fix this by first extracting the header into 8-byte aligned buffer,
> > > > > checking only the header for validity, and then by copying the
> > > > > entire FDT into newly allocated 8-byte aligned buffer. While this
> > > > > is not efficient, it is the correct way to handle DTs, which must
> > > > > be at 8-byte aligned offsets. Mitigate the inefficiency for the
> > > > > common case by checking whether the DT might be 8-byte aligned and
> > > > > if it is, map it directly.
> > > > >
> > > > > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > > > > ---
> > > > > Cc: Aaron Kling <webgeek1234@gmail.com>
> > > > > Cc: Eddie Kovsky <ekovsky@redhat.com>
> > > > > Cc: George Chan <gchan9527@gmail.com>
> > > > > Cc: Julien Masson <jmasson@baylibre.com>
> > > > > Cc: Mattijs Korpershoek <mkorpershoek@kernel.org>
> > > > > Cc: Nicolas Belin <nbelin@baylibre.com>
> > > > > Cc: Sam Day <me@samcday.com>
> > > > > Cc: Simon Glass <sjg@chromium.org>
> > > > > Cc: Tom Rini <trini@konsulko.com>
> > > > > Cc: u-boot@lists.denx.de
> > > > > ---
> > > > >  boot/image-android.c | 46 +++++++++++++++++++++++++++++++++-----------
> > > > >  1 file changed, 35 insertions(+), 11 deletions(-)
> > > > >
> > > >
> > > > OK, we need to adjust mkimage etc. to handle this. Until then I think
> > > > we should have a way to disable the 8-byte-alignment check as it seems
> > > > to have an enormous blast radius :-)
> > >
> > > We can't. We really can't. Passing misaligned device trees results in,
> > > and has already resulted in for years:
> > > - Silent Linux crashes
> > > - When Linux doesn't crash so many of the read functions go wrong the
> > >   result is incomprehensible.
> > >
> > > It's something that has and continues to waste untold engineering time.
> > > The only reason it's not worse is that we've killed off *most* of the
> > > platforms that disabled device tree relocation and that's how
> > > misalignment happens downstream most often.
> >
> > That's fine, but making the devicetree aligned for Linux is pretty
> > simple. We generally relocate it before booting anyway.
> >
> > But that's not what I'm seeing here. It seems to be updating U-Boot to
> > deal with a libfdt change. That is also a worthy goal, but we need to
> > consider code size and where best in the stack the changes should be
> > made. Doing a memalign() at the last minute is just not a good idea.
>
> Yes, the device tree specification, not the OS, says that it must be 8
> byte aligned. We've needed to do that for ages. Not doing that is not a
> good thing, for U-Boot, internally. The same APIs that blow up in Linux
> (or anything else) with misaligned device trees will blow up in us too.
> We need to make sure we pass ourselves something aligned correctly.

Yes, agreed. It has been sort-of half done for a while.

> Which is different from disagreeing with your specific feedback about
> how we get there, to be clear.

Regards,
Simon

Re: [PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4529 bytes --]

On Thu, Nov 13, 2025 at 01:28:56PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 13 Nov 2025 at 13:05, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Nov 13, 2025 at 12:57:30PM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Thu, 13 Nov 2025 at 12:38, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Thu, Nov 13, 2025 at 12:33:24PM -0700, Simon Glass wrote:
> > > > > Hi Marek,
> > > > >
> > > > > On Tue, 11 Nov 2025 at 18:17, Marek Vasut
> > > > > <marek.vasut+renesas@mailbox.org> wrote:
> > > > > >
> > > > > > Newer versions of libfdt strictly check whether the FDT blob
> > > > > > passed to them is at 8-byte aligned offset, if it is not, then
> > > > > > the library fails checks with -FDT_ERR_ALIGNMENT . Currently,
> > > > > > android_image_print_dtb_contents() passed FDT directly mapped
> > > > > > from abootimg to libfdt, and this FDT is not always aligned to
> > > > > > 8-byte offset. Specifically, the FDTs are somewhat packed in
> > > > > > the abootimg, therefore if the first FDT blob is e.g. 0xfd bytes
> > > > > > long, then the next FDT blob ends up at 0xfd offset, which is
> > > > > > not 8-byte aligned.
> > > > > >
> > > > > > Fix this by first extracting the header into 8-byte aligned buffer,
> > > > > > checking only the header for validity, and then by copying the
> > > > > > entire FDT into newly allocated 8-byte aligned buffer. While this
> > > > > > is not efficient, it is the correct way to handle DTs, which must
> > > > > > be at 8-byte aligned offsets. Mitigate the inefficiency for the
> > > > > > common case by checking whether the DT might be 8-byte aligned and
> > > > > > if it is, map it directly.
> > > > > >
> > > > > > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > > > > > ---
> > > > > > Cc: Aaron Kling <webgeek1234@gmail.com>
> > > > > > Cc: Eddie Kovsky <ekovsky@redhat.com>
> > > > > > Cc: George Chan <gchan9527@gmail.com>
> > > > > > Cc: Julien Masson <jmasson@baylibre.com>
> > > > > > Cc: Mattijs Korpershoek <mkorpershoek@kernel.org>
> > > > > > Cc: Nicolas Belin <nbelin@baylibre.com>
> > > > > > Cc: Sam Day <me@samcday.com>
> > > > > > Cc: Simon Glass <sjg@chromium.org>
> > > > > > Cc: Tom Rini <trini@konsulko.com>
> > > > > > Cc: u-boot@lists.denx.de
> > > > > > ---
> > > > > >  boot/image-android.c | 46 +++++++++++++++++++++++++++++++++-----------
> > > > > >  1 file changed, 35 insertions(+), 11 deletions(-)
> > > > > >
> > > > >
> > > > > OK, we need to adjust mkimage etc. to handle this. Until then I think
> > > > > we should have a way to disable the 8-byte-alignment check as it seems
> > > > > to have an enormous blast radius :-)
> > > >
> > > > We can't. We really can't. Passing misaligned device trees results in,
> > > > and has already resulted in for years:
> > > > - Silent Linux crashes
> > > > - When Linux doesn't crash so many of the read functions go wrong the
> > > >   result is incomprehensible.
> > > >
> > > > It's something that has and continues to waste untold engineering time.
> > > > The only reason it's not worse is that we've killed off *most* of the
> > > > platforms that disabled device tree relocation and that's how
> > > > misalignment happens downstream most often.
> > >
> > > That's fine, but making the devicetree aligned for Linux is pretty
> > > simple. We generally relocate it before booting anyway.
> > >
> > > But that's not what I'm seeing here. It seems to be updating U-Boot to
> > > deal with a libfdt change. That is also a worthy goal, but we need to
> > > consider code size and where best in the stack the changes should be
> > > made. Doing a memalign() at the last minute is just not a good idea.
> >
> > Yes, the device tree specification, not the OS, says that it must be 8
> > byte aligned. We've needed to do that for ages. Not doing that is not a
> > good thing, for U-Boot, internally. The same APIs that blow up in Linux
> > (or anything else) with misaligned device trees will blow up in us too.
> > We need to make sure we pass ourselves something aligned correctly.
> 
> Yes, agreed. It has been sort-of half done for a while.

Right.

> > Which is different from disagreeing with your specific feedback about
> > how we get there, to be clear.

And again, since your feedback to this patch was "Don't?", I'm saying we
need to. But the rest of your feedback was structural on moving towards
resolving it and so I assume Marek will respond.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[Marek Vasut]

On 11/13/25 9:36 PM, Tom Rini wrote:

Hello everyone,

>>> Which is different from disagreeing with your specific feedback about
>>> how we get there, to be clear.
> 
> And again, since your feedback to this patch was "Don't?", I'm saying we
> need to. But the rest of your feedback was structural on moving towards
> resolving it and so I assume Marek will respond.

The "blast radius" are these patches, that's all that tripped the tests:

  - boot: android: Always use 8-byte aligned DT with libfdt
  - test/py: android: Point fdt command to aligned addresses
  - test/py: Use aligned address for overlays in 'extension' test
  - sandbox: Fix DT compiler address warnings in sandbox DTs
  - sandbox: Fix DT compiler pin warnings in sandbox DTs
  - boot: Assure FDT is always at 8-byte aligned address
  - arm: qemu: Eliminate fdt_high and initrd_high misuse
  - efi_loader: Assure fitImage from capsule is used from 8-byte aligned 
address
  - MIPS: Assure end of U-Boot is at 8-byte aligned offset

Regarding last minute alignment, the problem with this android image 
seems to be in the android image itself, which packs in badly aligned 
FDT. We therefore have to copy it out and realign.

-- 
Best regards,
Marek Vasut

Re: [PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[Simon Glass]

Hi Marek,

On Thu, 13 Nov 2025 at 14:57, Marek Vasut <marek.vasut@mailbox.org> wrote:
>
> On 11/13/25 9:36 PM, Tom Rini wrote:
>
> Hello everyone,
>
> >>> Which is different from disagreeing with your specific feedback about
> >>> how we get there, to be clear.
> >
> > And again, since your feedback to this patch was "Don't?", I'm saying we
> > need to. But the rest of your feedback was structural on moving towards
> > resolving it and so I assume Marek will respond.
>
> The "blast radius" are these patches, that's all that tripped the tests:
>
>   - boot: android: Always use 8-byte aligned DT with libfdt
>   - test/py: android: Point fdt command to aligned addresses
>   - test/py: Use aligned address for overlays in 'extension' test
>   - sandbox: Fix DT compiler address warnings in sandbox DTs
>   - sandbox: Fix DT compiler pin warnings in sandbox DTs
>   - boot: Assure FDT is always at 8-byte aligned address
>   - arm: qemu: Eliminate fdt_high and initrd_high misuse
>   - efi_loader: Assure fitImage from capsule is used from 8-byte aligned
> address
>   - MIPS: Assure end of U-Boot is at 8-byte aligned offset
>
> Regarding last minute alignment, the problem with this android image
> seems to be in the android image itself, which packs in badly aligned
> FDT. We therefore have to copy it out and realign.

My request is to implement these checks as part of the boot flow
(bootm, etc.) rather than adding memory allocations in leaf function.
We already support copying the FDT to a different address so we can
expand it and add things. Can we make use of that code?

Regards,
SImon

Re: [PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2067 bytes --]

On Fri, Nov 14, 2025 at 05:45:32AM -0700, Simon Glass wrote:
> Hi Marek,
> 
> On Thu, 13 Nov 2025 at 14:57, Marek Vasut <marek.vasut@mailbox.org> wrote:
> >
> > On 11/13/25 9:36 PM, Tom Rini wrote:
> >
> > Hello everyone,
> >
> > >>> Which is different from disagreeing with your specific feedback about
> > >>> how we get there, to be clear.
> > >
> > > And again, since your feedback to this patch was "Don't?", I'm saying we
> > > need to. But the rest of your feedback was structural on moving towards
> > > resolving it and so I assume Marek will respond.
> >
> > The "blast radius" are these patches, that's all that tripped the tests:
> >
> >   - boot: android: Always use 8-byte aligned DT with libfdt
> >   - test/py: android: Point fdt command to aligned addresses
> >   - test/py: Use aligned address for overlays in 'extension' test
> >   - sandbox: Fix DT compiler address warnings in sandbox DTs
> >   - sandbox: Fix DT compiler pin warnings in sandbox DTs
> >   - boot: Assure FDT is always at 8-byte aligned address
> >   - arm: qemu: Eliminate fdt_high and initrd_high misuse
> >   - efi_loader: Assure fitImage from capsule is used from 8-byte aligned
> > address
> >   - MIPS: Assure end of U-Boot is at 8-byte aligned offset
> >
> > Regarding last minute alignment, the problem with this android image
> > seems to be in the android image itself, which packs in badly aligned
> > FDT. We therefore have to copy it out and realign.
> 
> My request is to implement these checks as part of the boot flow
> (bootm, etc.) rather than adding memory allocations in leaf function.
> We already support copying the FDT to a different address so we can
> expand it and add things. Can we make use of that code?

There are likely some cases where we should do the check-and-move
elsewhere, but others such as MIPS where we shouldn't get it wrong to
start with. I believe we've had a few other patches that also fix the
other cases where we don't ensure correct alignment to start with, and
really must.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[Simon Glass]

Hi,

On Fri, 14 Nov 2025 at 07:24, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Nov 14, 2025 at 05:45:32AM -0700, Simon Glass wrote:
> > Hi Marek,
> >
> > On Thu, 13 Nov 2025 at 14:57, Marek Vasut <marek.vasut@mailbox.org> wrote:
> > >
> > > On 11/13/25 9:36 PM, Tom Rini wrote:
> > >
> > > Hello everyone,
> > >
> > > >>> Which is different from disagreeing with your specific feedback about
> > > >>> how we get there, to be clear.
> > > >
> > > > And again, since your feedback to this patch was "Don't?", I'm saying we
> > > > need to. But the rest of your feedback was structural on moving towards
> > > > resolving it and so I assume Marek will respond.
> > >
> > > The "blast radius" are these patches, that's all that tripped the tests:
> > >
> > >   - boot: android: Always use 8-byte aligned DT with libfdt
> > >   - test/py: android: Point fdt command to aligned addresses
> > >   - test/py: Use aligned address for overlays in 'extension' test
> > >   - sandbox: Fix DT compiler address warnings in sandbox DTs
> > >   - sandbox: Fix DT compiler pin warnings in sandbox DTs
> > >   - boot: Assure FDT is always at 8-byte aligned address
> > >   - arm: qemu: Eliminate fdt_high and initrd_high misuse
> > >   - efi_loader: Assure fitImage from capsule is used from 8-byte aligned
> > > address
> > >   - MIPS: Assure end of U-Boot is at 8-byte aligned offset
> > >
> > > Regarding last minute alignment, the problem with this android image
> > > seems to be in the android image itself, which packs in badly aligned
> > > FDT. We therefore have to copy it out and realign.
> >
> > My request is to implement these checks as part of the boot flow
> > (bootm, etc.) rather than adding memory allocations in leaf function.
> > We already support copying the FDT to a different address so we can
> > expand it and add things. Can we make use of that code?
>
> There are likely some cases where we should do the check-and-move
> elsewhere, but others such as MIPS where we shouldn't get it wrong to
> start with. I believe we've had a few other patches that also fix the
> other cases where we don't ensure correct alignment to start with, and
> really must.

I'm going to leave this to you and Marek.

Regards,
Simon

Re: [PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[Marek Vasut]

On 11/14/25 1:45 PM, Simon Glass wrote:

Hello Simon,

>>>>> Which is different from disagreeing with your specific feedback about
>>>>> how we get there, to be clear.
>>>
>>> And again, since your feedback to this patch was "Don't?", I'm saying we
>>> need to. But the rest of your feedback was structural on moving towards
>>> resolving it and so I assume Marek will respond.
>>
>> The "blast radius" are these patches, that's all that tripped the tests:
>>
>>    - boot: android: Always use 8-byte aligned DT with libfdt
>>    - test/py: android: Point fdt command to aligned addresses
>>    - test/py: Use aligned address for overlays in 'extension' test
>>    - sandbox: Fix DT compiler address warnings in sandbox DTs
>>    - sandbox: Fix DT compiler pin warnings in sandbox DTs
>>    - boot: Assure FDT is always at 8-byte aligned address
>>    - arm: qemu: Eliminate fdt_high and initrd_high misuse
>>    - efi_loader: Assure fitImage from capsule is used from 8-byte aligned
>> address
>>    - MIPS: Assure end of U-Boot is at 8-byte aligned offset
>>
>> Regarding last minute alignment, the problem with this android image
>> seems to be in the android image itself, which packs in badly aligned
>> FDT. We therefore have to copy it out and realign.
> 
> My request is to implement these checks as part of the boot flow
> (bootm, etc.) rather than adding memory allocations in leaf function.
> We already support copying the FDT to a different address so we can
> expand it and add things. Can we make use of that code?
It seems the 'abootomg' command is extracting DTB from a container where 
the DTB can be at 4-byte aligned address. Thus far, the command 
internally used that possibly 4-byte aligned address, which is wrong. It 
also returned that address which was used by further U-Boot commands 
as-is, which is also wrong.

This DTB usage here has nothing to do with any boot flow, this is 
incorrect DTB alignment during manipulation, which is not part of boot.

What exactly do you propose should be changed with this patch ?

Re: [PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[Simon Glass]

Hi Marek,

On Sat, 15 Nov 2025 at 17:20, Marek Vasut <marek.vasut@mailbox.org> wrote:
>
> On 11/14/25 1:45 PM, Simon Glass wrote:
>
> Hello Simon,
>
> >>>>> Which is different from disagreeing with your specific feedback about
> >>>>> how we get there, to be clear.
> >>>
> >>> And again, since your feedback to this patch was "Don't?", I'm saying we
> >>> need to. But the rest of your feedback was structural on moving towards
> >>> resolving it and so I assume Marek will respond.
> >>
> >> The "blast radius" are these patches, that's all that tripped the tests:
> >>
> >>    - boot: android: Always use 8-byte aligned DT with libfdt
> >>    - test/py: android: Point fdt command to aligned addresses
> >>    - test/py: Use aligned address for overlays in 'extension' test
> >>    - sandbox: Fix DT compiler address warnings in sandbox DTs
> >>    - sandbox: Fix DT compiler pin warnings in sandbox DTs
> >>    - boot: Assure FDT is always at 8-byte aligned address
> >>    - arm: qemu: Eliminate fdt_high and initrd_high misuse
> >>    - efi_loader: Assure fitImage from capsule is used from 8-byte aligned
> >> address
> >>    - MIPS: Assure end of U-Boot is at 8-byte aligned offset
> >>
> >> Regarding last minute alignment, the problem with this android image
> >> seems to be in the android image itself, which packs in badly aligned
> >> FDT. We therefore have to copy it out and realign.
> >
> > My request is to implement these checks as part of the boot flow
> > (bootm, etc.) rather than adding memory allocations in leaf function.
> > We already support copying the FDT to a different address so we can
> > expand it and add things. Can we make use of that code?
> It seems the 'abootomg' command is extracting DTB from a container where
> the DTB can be at 4-byte aligned address. Thus far, the command
> internally used that possibly 4-byte aligned address, which is wrong. It
> also returned that address which was used by further U-Boot commands
> as-is, which is also wrong.
>
> This DTB usage here has nothing to do with any boot flow, this is
> incorrect DTB alignment during manipulation, which is not part of boot.
>
> What exactly do you propose should be changed with this patch ?

Actually this patch seems to adjust code which is just handling a
command. How about creating a common function which can put an FDT
into an abuf, either just using it in place, or allocating memory and
copying it? An abuf is ideal for this. E.g. something like (untested):

/**
 * fdt_ensure_aligned() - Obtain an abuf with a devicetree, aligning if needed
 *
 * @buf: Returns abuf containing FDT on success; caller must abuf_uninit(buf)
 * @addr: Address of FDT
 * Return: 0 on success, -EINVAL if not an FDT, -ENOMEM if out of memory
 */
int fdt_ensure_aligned(struct abuf *buf, void *fdt)
{
   struct fdt_header fdth __aligned(8);

   memcpy(&fdth, fdt, sizeof(struct fdt_header));
   if (fdt_check_header(&fdth) != 0)
      return -EINVAL;

   abuf_init_const(buf, fdt, fdt_totalsize(&fdth));

   /* The device tree must be at an 8-byte aligned address */
   if ((uintptr_t)fdt & 7) {
      void *fulldt;

      // a function like abuf_realloc_align(struct abuf *buf, int
align) would help
      fulldt = memalign(8, buf->size);
      if (!fulldt)
         return -ENOMEM;

      memcpy(fulldt, fdt, buf->size);
      buf->data = fulldt;
      buf->alloced = true;
   }

   return 0;
}

Then you can use this in various places as needed.

Regards,
Simon

Re: [PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3966 bytes --]

On Sun, Nov 16, 2025 at 09:07:19AM -0700, Simon Glass wrote:
> Hi Marek,
> 
> On Sat, 15 Nov 2025 at 17:20, Marek Vasut <marek.vasut@mailbox.org> wrote:
> >
> > On 11/14/25 1:45 PM, Simon Glass wrote:
> >
> > Hello Simon,
> >
> > >>>>> Which is different from disagreeing with your specific feedback about
> > >>>>> how we get there, to be clear.
> > >>>
> > >>> And again, since your feedback to this patch was "Don't?", I'm saying we
> > >>> need to. But the rest of your feedback was structural on moving towards
> > >>> resolving it and so I assume Marek will respond.
> > >>
> > >> The "blast radius" are these patches, that's all that tripped the tests:
> > >>
> > >>    - boot: android: Always use 8-byte aligned DT with libfdt
> > >>    - test/py: android: Point fdt command to aligned addresses
> > >>    - test/py: Use aligned address for overlays in 'extension' test
> > >>    - sandbox: Fix DT compiler address warnings in sandbox DTs
> > >>    - sandbox: Fix DT compiler pin warnings in sandbox DTs
> > >>    - boot: Assure FDT is always at 8-byte aligned address
> > >>    - arm: qemu: Eliminate fdt_high and initrd_high misuse
> > >>    - efi_loader: Assure fitImage from capsule is used from 8-byte aligned
> > >> address
> > >>    - MIPS: Assure end of U-Boot is at 8-byte aligned offset
> > >>
> > >> Regarding last minute alignment, the problem with this android image
> > >> seems to be in the android image itself, which packs in badly aligned
> > >> FDT. We therefore have to copy it out and realign.
> > >
> > > My request is to implement these checks as part of the boot flow
> > > (bootm, etc.) rather than adding memory allocations in leaf function.
> > > We already support copying the FDT to a different address so we can
> > > expand it and add things. Can we make use of that code?
> > It seems the 'abootomg' command is extracting DTB from a container where
> > the DTB can be at 4-byte aligned address. Thus far, the command
> > internally used that possibly 4-byte aligned address, which is wrong. It
> > also returned that address which was used by further U-Boot commands
> > as-is, which is also wrong.
> >
> > This DTB usage here has nothing to do with any boot flow, this is
> > incorrect DTB alignment during manipulation, which is not part of boot.
> >
> > What exactly do you propose should be changed with this patch ?
> 
> Actually this patch seems to adjust code which is just handling a
> command. How about creating a common function which can put an FDT
> into an abuf, either just using it in place, or allocating memory and
> copying it? An abuf is ideal for this. E.g. something like (untested):
> 
> /**
>  * fdt_ensure_aligned() - Obtain an abuf with a devicetree, aligning if needed
>  *
>  * @buf: Returns abuf containing FDT on success; caller must abuf_uninit(buf)
>  * @addr: Address of FDT
>  * Return: 0 on success, -EINVAL if not an FDT, -ENOMEM if out of memory
>  */
> int fdt_ensure_aligned(struct abuf *buf, void *fdt)
> {
>    struct fdt_header fdth __aligned(8);
> 
>    memcpy(&fdth, fdt, sizeof(struct fdt_header));
>    if (fdt_check_header(&fdth) != 0)
>       return -EINVAL;
> 
>    abuf_init_const(buf, fdt, fdt_totalsize(&fdth));
> 
>    /* The device tree must be at an 8-byte aligned address */
>    if ((uintptr_t)fdt & 7) {
>       void *fulldt;
> 
>       // a function like abuf_realloc_align(struct abuf *buf, int
> align) would help
>       fulldt = memalign(8, buf->size);
>       if (!fulldt)
>          return -ENOMEM;
> 
>       memcpy(fulldt, fdt, buf->size);
>       buf->data = fulldt;
>       buf->alloced = true;
>    }
> 
>    return 0;
> }
> 
> Then you can use this in various places as needed.

That looks like massively more code than is needed to just ensure that
the thing we deal with today is also correctly aligned? We don't need to
pull abuf into everything.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[Simon Glass]

Hi Tom,

On Mon, 17 Nov 2025 at 07:21, Tom Rini <trini@konsulko.com> wrote:
>
> On Sun, Nov 16, 2025 at 09:07:19AM -0700, Simon Glass wrote:
> > Hi Marek,
> >
> > On Sat, 15 Nov 2025 at 17:20, Marek Vasut <marek.vasut@mailbox.org> wrote:
> > >
> > > On 11/14/25 1:45 PM, Simon Glass wrote:
> > >
> > > Hello Simon,
> > >
> > > >>>>> Which is different from disagreeing with your specific feedback about
> > > >>>>> how we get there, to be clear.
> > > >>>
> > > >>> And again, since your feedback to this patch was "Don't?", I'm saying we
> > > >>> need to. But the rest of your feedback was structural on moving towards
> > > >>> resolving it and so I assume Marek will respond.
> > > >>
> > > >> The "blast radius" are these patches, that's all that tripped the tests:
> > > >>
> > > >>    - boot: android: Always use 8-byte aligned DT with libfdt
> > > >>    - test/py: android: Point fdt command to aligned addresses
> > > >>    - test/py: Use aligned address for overlays in 'extension' test
> > > >>    - sandbox: Fix DT compiler address warnings in sandbox DTs
> > > >>    - sandbox: Fix DT compiler pin warnings in sandbox DTs
> > > >>    - boot: Assure FDT is always at 8-byte aligned address
> > > >>    - arm: qemu: Eliminate fdt_high and initrd_high misuse
> > > >>    - efi_loader: Assure fitImage from capsule is used from 8-byte aligned
> > > >> address
> > > >>    - MIPS: Assure end of U-Boot is at 8-byte aligned offset
> > > >>
> > > >> Regarding last minute alignment, the problem with this android image
> > > >> seems to be in the android image itself, which packs in badly aligned
> > > >> FDT. We therefore have to copy it out and realign.
> > > >
> > > > My request is to implement these checks as part of the boot flow
> > > > (bootm, etc.) rather than adding memory allocations in leaf function.
> > > > We already support copying the FDT to a different address so we can
> > > > expand it and add things. Can we make use of that code?
> > > It seems the 'abootomg' command is extracting DTB from a container where
> > > the DTB can be at 4-byte aligned address. Thus far, the command
> > > internally used that possibly 4-byte aligned address, which is wrong. It
> > > also returned that address which was used by further U-Boot commands
> > > as-is, which is also wrong.
> > >
> > > This DTB usage here has nothing to do with any boot flow, this is
> > > incorrect DTB alignment during manipulation, which is not part of boot.
> > >
> > > What exactly do you propose should be changed with this patch ?
> >
> > Actually this patch seems to adjust code which is just handling a
> > command. How about creating a common function which can put an FDT
> > into an abuf, either just using it in place, or allocating memory and
> > copying it? An abuf is ideal for this. E.g. something like (untested):
> >
> > /**
> >  * fdt_ensure_aligned() - Obtain an abuf with a devicetree, aligning if needed
> >  *
> >  * @buf: Returns abuf containing FDT on success; caller must abuf_uninit(buf)
> >  * @addr: Address of FDT
> >  * Return: 0 on success, -EINVAL if not an FDT, -ENOMEM if out of memory
> >  */
> > int fdt_ensure_aligned(struct abuf *buf, void *fdt)
> > {
> >    struct fdt_header fdth __aligned(8);
> >
> >    memcpy(&fdth, fdt, sizeof(struct fdt_header));
> >    if (fdt_check_header(&fdth) != 0)
> >       return -EINVAL;
> >
> >    abuf_init_const(buf, fdt, fdt_totalsize(&fdth));
> >
> >    /* The device tree must be at an 8-byte aligned address */
> >    if ((uintptr_t)fdt & 7) {
> >       void *fulldt;
> >
> >       // a function like abuf_realloc_align(struct abuf *buf, int
> > align) would help
> >       fulldt = memalign(8, buf->size);
> >       if (!fulldt)
> >          return -ENOMEM;
> >
> >       memcpy(fulldt, fdt, buf->size);
> >       buf->data = fulldt;
> >       buf->alloced = true;
> >    }
> >
> >    return 0;
> > }
> >
> > Then you can use this in various places as needed.
>
> That looks like massively more code than is needed to just ensure that
> the thing we deal with today is also correctly aligned? We don't need to
> pull abuf into everything.

It is 27 lines of code, less than Marek's delta. Plus by the sounds of
it, we can call it from more than one place. I think it is better to
have a common function than to open-code this stuff in multiple
places. I also think separating out the map_sysmem() stuff would make
sense, since it is a bit convoluted at present.

Re abuf I believe it is a useful abstraction and I wish I had thought
of it some years ago. It is particularly good when you may or may not
need to allocate something. Should we discuss abuf in one of the
U-Boot meetings?

Regards,
Simon

Re: [PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 5136 bytes --]

On Mon, Nov 17, 2025 at 10:04:00AM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 17 Nov 2025 at 07:21, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sun, Nov 16, 2025 at 09:07:19AM -0700, Simon Glass wrote:
> > > Hi Marek,
> > >
> > > On Sat, 15 Nov 2025 at 17:20, Marek Vasut <marek.vasut@mailbox.org> wrote:
> > > >
> > > > On 11/14/25 1:45 PM, Simon Glass wrote:
> > > >
> > > > Hello Simon,
> > > >
> > > > >>>>> Which is different from disagreeing with your specific feedback about
> > > > >>>>> how we get there, to be clear.
> > > > >>>
> > > > >>> And again, since your feedback to this patch was "Don't?", I'm saying we
> > > > >>> need to. But the rest of your feedback was structural on moving towards
> > > > >>> resolving it and so I assume Marek will respond.
> > > > >>
> > > > >> The "blast radius" are these patches, that's all that tripped the tests:
> > > > >>
> > > > >>    - boot: android: Always use 8-byte aligned DT with libfdt
> > > > >>    - test/py: android: Point fdt command to aligned addresses
> > > > >>    - test/py: Use aligned address for overlays in 'extension' test
> > > > >>    - sandbox: Fix DT compiler address warnings in sandbox DTs
> > > > >>    - sandbox: Fix DT compiler pin warnings in sandbox DTs
> > > > >>    - boot: Assure FDT is always at 8-byte aligned address
> > > > >>    - arm: qemu: Eliminate fdt_high and initrd_high misuse
> > > > >>    - efi_loader: Assure fitImage from capsule is used from 8-byte aligned
> > > > >> address
> > > > >>    - MIPS: Assure end of U-Boot is at 8-byte aligned offset
> > > > >>
> > > > >> Regarding last minute alignment, the problem with this android image
> > > > >> seems to be in the android image itself, which packs in badly aligned
> > > > >> FDT. We therefore have to copy it out and realign.
> > > > >
> > > > > My request is to implement these checks as part of the boot flow
> > > > > (bootm, etc.) rather than adding memory allocations in leaf function.
> > > > > We already support copying the FDT to a different address so we can
> > > > > expand it and add things. Can we make use of that code?
> > > > It seems the 'abootomg' command is extracting DTB from a container where
> > > > the DTB can be at 4-byte aligned address. Thus far, the command
> > > > internally used that possibly 4-byte aligned address, which is wrong. It
> > > > also returned that address which was used by further U-Boot commands
> > > > as-is, which is also wrong.
> > > >
> > > > This DTB usage here has nothing to do with any boot flow, this is
> > > > incorrect DTB alignment during manipulation, which is not part of boot.
> > > >
> > > > What exactly do you propose should be changed with this patch ?
> > >
> > > Actually this patch seems to adjust code which is just handling a
> > > command. How about creating a common function which can put an FDT
> > > into an abuf, either just using it in place, or allocating memory and
> > > copying it? An abuf is ideal for this. E.g. something like (untested):
> > >
> > > /**
> > >  * fdt_ensure_aligned() - Obtain an abuf with a devicetree, aligning if needed
> > >  *
> > >  * @buf: Returns abuf containing FDT on success; caller must abuf_uninit(buf)
> > >  * @addr: Address of FDT
> > >  * Return: 0 on success, -EINVAL if not an FDT, -ENOMEM if out of memory
> > >  */
> > > int fdt_ensure_aligned(struct abuf *buf, void *fdt)
> > > {
> > >    struct fdt_header fdth __aligned(8);
> > >
> > >    memcpy(&fdth, fdt, sizeof(struct fdt_header));
> > >    if (fdt_check_header(&fdth) != 0)
> > >       return -EINVAL;
> > >
> > >    abuf_init_const(buf, fdt, fdt_totalsize(&fdth));
> > >
> > >    /* The device tree must be at an 8-byte aligned address */
> > >    if ((uintptr_t)fdt & 7) {
> > >       void *fulldt;
> > >
> > >       // a function like abuf_realloc_align(struct abuf *buf, int
> > > align) would help
> > >       fulldt = memalign(8, buf->size);
> > >       if (!fulldt)
> > >          return -ENOMEM;
> > >
> > >       memcpy(fulldt, fdt, buf->size);
> > >       buf->data = fulldt;
> > >       buf->alloced = true;
> > >    }
> > >
> > >    return 0;
> > > }
> > >
> > > Then you can use this in various places as needed.
> >
> > That looks like massively more code than is needed to just ensure that
> > the thing we deal with today is also correctly aligned? We don't need to
> > pull abuf into everything.
> 
> It is 27 lines of code, less than Marek's delta. Plus by the sounds of
> it, we can call it from more than one place. I think it is better to
> have a common function than to open-code this stuff in multiple
> places. I also think separating out the map_sysmem() stuff would make
> sense, since it is a bit convoluted at present.
> 
> Re abuf I believe it is a useful abstraction and I wish I had thought
> of it some years ago. It is particularly good when you may or may not
> need to allocate something. Should we discuss abuf in one of the
> U-Boot meetings?

No, I think you've lost the thread on some of the problems
unfortunately.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[Simon Glass]

Hi Tom,


On Mon, 17 Nov 2025 at 11:02, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Nov 17, 2025 at 10:04:00AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 17 Nov 2025 at 07:21, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sun, Nov 16, 2025 at 09:07:19AM -0700, Simon Glass wrote:
> > > > Hi Marek,
> > > >
> > > > On Sat, 15 Nov 2025 at 17:20, Marek Vasut <marek.vasut@mailbox.org> wrote:
> > > > >
> > > > > On 11/14/25 1:45 PM, Simon Glass wrote:
> > > > >
> > > > > Hello Simon,
> > > > >
> > > > > >>>>> Which is different from disagreeing with your specific feedback about
> > > > > >>>>> how we get there, to be clear.
> > > > > >>>
> > > > > >>> And again, since your feedback to this patch was "Don't?", I'm saying we
> > > > > >>> need to. But the rest of your feedback was structural on moving towards
> > > > > >>> resolving it and so I assume Marek will respond.
> > > > > >>
> > > > > >> The "blast radius" are these patches, that's all that tripped the tests:
> > > > > >>
> > > > > >>    - boot: android: Always use 8-byte aligned DT with libfdt
> > > > > >>    - test/py: android: Point fdt command to aligned addresses
> > > > > >>    - test/py: Use aligned address for overlays in 'extension' test
> > > > > >>    - sandbox: Fix DT compiler address warnings in sandbox DTs
> > > > > >>    - sandbox: Fix DT compiler pin warnings in sandbox DTs
> > > > > >>    - boot: Assure FDT is always at 8-byte aligned address
> > > > > >>    - arm: qemu: Eliminate fdt_high and initrd_high misuse
> > > > > >>    - efi_loader: Assure fitImage from capsule is used from 8-byte aligned
> > > > > >> address
> > > > > >>    - MIPS: Assure end of U-Boot is at 8-byte aligned offset
> > > > > >>
> > > > > >> Regarding last minute alignment, the problem with this android image
> > > > > >> seems to be in the android image itself, which packs in badly aligned
> > > > > >> FDT. We therefore have to copy it out and realign.
> > > > > >
> > > > > > My request is to implement these checks as part of the boot flow
> > > > > > (bootm, etc.) rather than adding memory allocations in leaf function.
> > > > > > We already support copying the FDT to a different address so we can
> > > > > > expand it and add things. Can we make use of that code?
> > > > > It seems the 'abootomg' command is extracting DTB from a container where
> > > > > the DTB can be at 4-byte aligned address. Thus far, the command
> > > > > internally used that possibly 4-byte aligned address, which is wrong. It
> > > > > also returned that address which was used by further U-Boot commands
> > > > > as-is, which is also wrong.
> > > > >
> > > > > This DTB usage here has nothing to do with any boot flow, this is
> > > > > incorrect DTB alignment during manipulation, which is not part of boot.
> > > > >
> > > > > What exactly do you propose should be changed with this patch ?
> > > >
> > > > Actually this patch seems to adjust code which is just handling a
> > > > command. How about creating a common function which can put an FDT
> > > > into an abuf, either just using it in place, or allocating memory and
> > > > copying it? An abuf is ideal for this. E.g. something like (untested):
> > > >
> > > > /**
> > > >  * fdt_ensure_aligned() - Obtain an abuf with a devicetree, aligning if needed
> > > >  *
> > > >  * @buf: Returns abuf containing FDT on success; caller must abuf_uninit(buf)
> > > >  * @addr: Address of FDT
> > > >  * Return: 0 on success, -EINVAL if not an FDT, -ENOMEM if out of memory
> > > >  */
> > > > int fdt_ensure_aligned(struct abuf *buf, void *fdt)
> > > > {
> > > >    struct fdt_header fdth __aligned(8);
> > > >
> > > >    memcpy(&fdth, fdt, sizeof(struct fdt_header));
> > > >    if (fdt_check_header(&fdth) != 0)
> > > >       return -EINVAL;
> > > >
> > > >    abuf_init_const(buf, fdt, fdt_totalsize(&fdth));
> > > >
> > > >    /* The device tree must be at an 8-byte aligned address */
> > > >    if ((uintptr_t)fdt & 7) {
> > > >       void *fulldt;
> > > >
> > > >       // a function like abuf_realloc_align(struct abuf *buf, int
> > > > align) would help
> > > >       fulldt = memalign(8, buf->size);
> > > >       if (!fulldt)
> > > >          return -ENOMEM;
> > > >
> > > >       memcpy(fulldt, fdt, buf->size);
> > > >       buf->data = fulldt;
> > > >       buf->alloced = true;
> > > >    }
> > > >
> > > >    return 0;
> > > > }
> > > >
> > > > Then you can use this in various places as needed.
> > >
> > > That looks like massively more code than is needed to just ensure that
> > > the thing we deal with today is also correctly aligned? We don't need to
> > > pull abuf into everything.
> >
> > It is 27 lines of code, less than Marek's delta. Plus by the sounds of
> > it, we can call it from more than one place. I think it is better to
> > have a common function than to open-code this stuff in multiple
> > places. I also think separating out the map_sysmem() stuff would make
> > sense, since it is a bit convoluted at present.
> >
> > Re abuf I believe it is a useful abstraction and I wish I had thought
> > of it some years ago. It is particularly good when you may or may not
> > need to allocate something. Should we discuss abuf in one of the
> > U-Boot meetings?
>
> No, I think you've lost the thread on some of the problems
> unfortunately.

That might be a little too cryptic for me to understand. Which problems?

Regards,
Simon

Re: [PATCH 1/2] boot: android: Always use 8-byte aligned DT with libfdt

[Mattijs Korpershoek]

On Thu, Nov 13, 2025 at 22:42, Marek Vasut <marek.vasut@mailbox.org> wrote:

> On 11/13/25 9:36 PM, Tom Rini wrote:
>
> Hello everyone,
>
>>>> Which is different from disagreeing with your specific feedback about
>>>> how we get there, to be clear.
>> 
>> And again, since your feedback to this patch was "Don't?", I'm saying we
>> need to. But the rest of your feedback was structural on moving towards
>> resolving it and so I assume Marek will respond.
>
> The "blast radius" are these patches, that's all that tripped the tests:
>
>   - boot: android: Always use 8-byte aligned DT with libfdt
>   - test/py: android: Point fdt command to aligned addresses
>   - test/py: Use aligned address for overlays in 'extension' test
>   - sandbox: Fix DT compiler address warnings in sandbox DTs
>   - sandbox: Fix DT compiler pin warnings in sandbox DTs
>   - boot: Assure FDT is always at 8-byte aligned address
>   - arm: qemu: Eliminate fdt_high and initrd_high misuse
>   - efi_loader: Assure fitImage from capsule is used from 8-byte aligned 
> address
>   - MIPS: Assure end of U-Boot is at 8-byte aligned offset
>
> Regarding last minute alignment, the problem with this android image 
> seems to be in the android image itself, which packs in badly aligned 
> FDT. We therefore have to copy it out and realign.

Yes, the Android tools do not handle alignment by default. This is
stated in the Android docs [1].

Also see [2] on how a boot.img is usually structured.

In any case, if DT spec mandates alignement, I think it's good to
enforce the spec in the code.

[1] https://source.android.com/docs/core/architecture/dto/partitions#cfg-create
[2] https://source.android.com/docs/core/architecture/dto

>
> -- 
> Best regards,
> Marek Vasut