Hi Tom, On Thu, 13 Nov 2025 at 13:05, Tom Rini <[email protected]> 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 <[email protected]> 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 > > > > <[email protected]> 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 <[email protected]> > > > > > --- > > > > > Cc: Aaron Kling <[email protected]> > > > > > Cc: Eddie Kovsky <[email protected]> > > > > > Cc: George Chan <[email protected]> > > > > > Cc: Julien Masson <[email protected]> > > > > > Cc: Mattijs Korpershoek <[email protected]> > > > > > Cc: Nicolas Belin <[email protected]> > > > > > Cc: Sam Day <[email protected]> > > > > > Cc: Simon Glass <[email protected]> > > > > > Cc: Tom Rini <[email protected]> > > > > > Cc: [email protected] > > > > > --- > > > > > 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
