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

[Tom Rini]

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 at 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 at 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 at 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 at mailbox.org>
> > > > > > ---
> > > > > > Cc: Aaron Kling <webgeek1234 at gmail.com>
> > > > > > Cc: Eddie Kovsky <ekovsky at redhat.com>
> > > > > > Cc: George Chan <gchan9527 at gmail.com>
> > > > > > Cc: Julien Masson <jmasson at baylibre.com>
> > > > > > Cc: Mattijs Korpershoek <mkorpershoek at kernel.org>
> > > > > > Cc: Nicolas Belin <nbelin at baylibre.com>
> > > > > > Cc: Sam Day <me at samcday.com>
> > > > > > Cc: Simon Glass <sjg at chromium.org>
> > > > > > Cc: Tom Rini <trini at konsulko.com>
> > > > > > Cc: u-boot at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20251113/6537f347/attachment.sig>