[PATCH] boot: Warn users about fdt_high=~0 usage

Tom Rini trini at konsulko.com
Mon Nov 17 15:32:45 CET 2025 

On Sun, Nov 16, 2025 at 10:43:47PM +0100, Marek Vasut wrote:
> On 11/16/25 3:09 PM, Tom Rini wrote:
> 
> Hello Tom,
> 
> > > > > diff --git a/boot/image-fdt.c b/boot/image-fdt.c
> > > > > index 3f0ac54f76f..e88525a3846 100644
> > > > > --- a/boot/image-fdt.c
> > > > > +++ b/boot/image-fdt.c
> > > > > @@ -189,6 +189,10 @@ int boot_relocate_fdt(char **of_flat_tree, ulong *of_size)
> > > > >    			/* All ones means use fdt in place */
> > > > >    			of_start = fdt_blob;
> > > > >    			addr = map_to_sysmem(fdt_blob);
> > > > > +			if (addr & 7) {
> > > > > +				printf("WARNING: The 'fdt_high' environment variable is set to ~0 and DT is at non-8-byte aligned address.\nWARNING: This system will likely fail to boot. Unset 'fdt_high' environment variable and submit fix upstream.\n");
> > > > > +			}
> > > > > +
> > > > >    			err = lmb_alloc_mem(LMB_MEM_ALLOC_ADDR, 0, &addr,
> > > > >    					    of_len, LMB_NONE);
> > > > >    			if (err) {
> > > > 
> > > > I think we need to yell about it sooner.
> > > 
> > > I don't think so, for two reasons:
> > > 
> > > - If the user uses e.g. bootz to boot their kernel, which on arm32 is still
> > > sadly happening, they wouldn't trigger this problem at all and they wouldn't
> > > care about whichever way their fdt_high is set.
> > 
> > Which part of the problem? If the dtb is not 8-byte aligned the kernel
> > will fail. All the accessors require 8 byte alignment.
> 
> On 64bit systems, yes. On 32bit systems it is not so clear cut, they might
> boot with 4-byte aligned DTs. I might be wrong about this, but if I recall
> it right, bootz is very simplistic and it doesn't handle much in terms of DT
> relocation, and ignores fdt_high, so the users on those 32bit systems that
> use bootz to boot also don't care about fdt_high much.
> 
> > > - If the user sets 'fdt_high' as part of their boot command, they would not
> > > get any warning print if we warn earlier, but they would get one when they
> > > boot and trigger this affected code path.
> > > 
> > > Maybe we need warnings in two locations ?
> > 
> > Maybe? I was thinking it should be around this in boot/image-fdt.c:
> >          /* If fdt_high is set use it to select the relocation address */
> >          fdt_high = env_get("fdt_high");
> >          if (fdt_high) {
> >                  ulong high_addr = hextoul(fdt_high, NULL);
> > 
> >                  if (high_addr == ~0UL) {
> >                          /* All ones means use fdt in place */
> 
> This is exactly the code that this patch modifies . Where in the above do
> you think this change should be ?

You're right and I did post the last line of context that's the first
line of your patch. I really meant that we shouldn't even be checking if
the current location is misaligned. We should tell people to stop
disabling relocation.

> > As that's where all of the code paths end up calling, I think. But if
> > it's multiple paths, yes, multiple warnings if we can't abstract it out
> > to a common path.
> > 
> > > > Today (and for quite some
> > > > years) if you pass a 4 byte and not 8 byte aligned DT to Linux, it fails
> > > > to boot or breaks in loud
> > > 
> > > Worse, not loud, but silent.
> > 
> > Also true, yes.
> > 
> > > > and odd ways. This has in turn lead to much
> > > > time spent and some of our older threads with the libfdt folks years
> > > > ago. So I think we need something earlier in code where we're seeing
> > > > that fdt_high is set to ~0 and that's where we say "Stop doing this, it
> > > > will be removed soon".
> > > 
> > > We cannot remove this functionality, someone might actually depend on it for
> > > whatever reason. It is part of the command line ABI now.
> > 
> > The valid use case is boot time optimization. It's also a
> > micro-optimization (as actually megabyte sized device trees like when
> > people wanted to pass FPGA bitstreams in 15+ years ago are not allowed)
> 
> ( not allowed where exactly ? fitImage is a DT too )

I wasn't clear enough. I mean having the FPGA bitstream as a device tree
node, for the linux kernel to parse. Because that's what fdt_high is
about, not FIT images :)

> > I think really. I am loath to break ABI like this but I'm not entirely
> > sure we have a choice.
> 
> We do, we simply warn users and remove the usage and fdt_high=~0 assignments
> from the tree. The functionality itself does not have to be removed.

The problem is the functionality has always been for a hack workaround
and bootm_low/bootm_size/etc were the right answer.

But maybe step one is just remove the in-tree usage and a big loud
warning when it's set telling people to not do that.

> > Maybe we detect disabled relocation and
> > misaligned device tree and fall back to prompt? Or if it's too late,
> > panic with an explanation? Or maybe we just move it 4 bytes higher. The
> > device is in a going to fail state anyhow, so trying to recover it might
> > be OK, and since the device tree needs to be modified by us it has to be
> > in writable memory.
> Keep in mind, on arm32 it may not necessarily fail to boot with 4-byte
> aligned DT.

I'm pretty sure it is. It's not a problem about doing misaligned reads
it's that the data structure and it's accessors require 8 byte
alignment. That's what I recall right now from everything around:
commit e8c2d25845c72c7202a628a97d45e31beea40668
Author: Tom Rini <trini at konsulko.com>
Date:   Mon Jan 27 12:10:31 2020 -0500

    libfdt: Revert 6dcb8ba4 from upstream libfdt
    
    In upstream libfdt, 6dcb8ba4 "libfdt: Add helpers for accessing
    unaligned words" introduced changes to support unaligned reads for ARM
    platforms and 11738cf01f15 "libfdt: Don't use memcpy to handle unaligned
    reads on ARM" improved the performance of these helpers.
    
    In practice however, this only occurs when the user has forced the
    device tree to be placed in memory in a non-aligned way, which in turn
    violates both our rules and the Linux Kernel rules for how things must
    reside in memory to function.
    
    This "in practice" part is important as handling these other cases adds
    visible (1 second or more) delay to boot in what would be considered the
    fast path of the code.
    
    Cc: Patrice CHOTARD <patrice.chotard at st.com>
    Cc: Patrick DELAUNAY <patrick.delaunay at st.com>
    Link: https://www.spinics.net/lists/devicetree-compiler/msg02972.html
    Signed-off-by: Tom Rini <trini at konsulko.com>
    Tested-by: Patrice Chotard <patrice.chotard at st.com>

-- 
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/45e832e6/attachment.sig>

More information about the U-Boot mailing list