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

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

[Marek Vasut]

In case the 'fdt_high' environment variable is set to ~0 and DT is
at non-8-byte aligned offset, warn users about the dangers of the
fdt_high usage. This will hopefully lead to removal of most of the
fdt_high ~0 usage over time.

Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: "João Paulo Gonçalves" <joao.goncalves@toradex.com>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Sam Protsenko <semen.protsenko@linaro.org>
Cc: Sughosh Ganu <sughosh.ganu@linaro.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: u-boot@lists.denx.de
---
 boot/image-fdt.c | 4 ++++
 1 file changed, 4 insertions(+)

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) {
-- 
2.51.0

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2115 bytes --]

On Thu, Nov 13, 2025 at 03:29:51PM +0100, Marek Vasut wrote:

> In case the 'fdt_high' environment variable is set to ~0 and DT is
> at non-8-byte aligned offset, warn users about the dangers of the
> fdt_high usage. This will hopefully lead to removal of most of the
> fdt_high ~0 usage over time.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: "João Paulo Gonçalves" <joao.goncalves@toradex.com>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Sam Protsenko <semen.protsenko@linaro.org>
> Cc: Sughosh Ganu <sughosh.ganu@linaro.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: u-boot@lists.denx.de
> ---
>  boot/image-fdt.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> 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. 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 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". Historically it was either used to work-around
problems that don't exist anymore, or as a misguided boot time
optimization (this is different from disabling initrd relocation, which
can be noticeable).

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Marek Vasut]

On 11/13/25 4:49 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.

- 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 ?

> 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.

> 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.

[...]

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3451 bytes --]

On Sat, Nov 15, 2025 at 06:34:24PM +0100, Marek Vasut wrote:
> On 11/13/25 4:49 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.

> - 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 */


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)
I think really. I am loath to break ABI like this but I'm not entirely
sure we have a choice. 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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Marek Vasut]

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 ?

> 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 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.

> 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.

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 6635 bytes --]

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@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@st.com>
    Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
    Link: https://www.spinics.net/lists/devicetree-compiler/msg02972.html
    Signed-off-by: Tom Rini <trini@konsulko.com>
    Tested-by: Patrice Chotard <patrice.chotard@st.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Marek Vasut]

On 11/17/25 3:32 PM, Tom Rini wrote:

Hello Tom,

>>> 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.

I see, and in that case, I don't particularly care either way, whether 
we warn always, or only in case of actual problem.

>>> 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.

This is fine by me.

>>> 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.

I think my knowledge of this corner case was from around this time, so 
old kernel versions might work, new ones will likely break. That is fine 
too.