Hi Tom, On Mon, 17 Nov 2025 at 07:21, Tom Rini <[email protected]> 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 <[email protected]> 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
