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

[Tom Rini]

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 at 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 at 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
-------------- 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/20251117/ab12d8f6/attachment.sig>