[PATCH v2] platform: generic: Keep some empty space in FDT passed to next stage

[PATCH v2] platform: generic: Keep some empty space in FDT passed to next stage

[Anup Patel]

Leaving no empty space in the FDT passed to the next booting stage
causes the following U-Boot crash on Ventana internal platforms:

Unhandled exception: Load access fault
EPC: 00000000fffa6372 RA: 00000000fffa7418 TVAL: 0001746174730068
EPC: 0000000080245372 RA: 0000000080246418 reloc adjusted

SP:  00000000fef38440 GP:  00000000fef40e60 TP:  0000000000000000
T0:  00000000fef40a70 T1:  000000000000ff00 T2:  0000000000000000
S0:  00000000fffc17a8 S1:  00000000fef38d40 A0:  7375746174730068
A1:  00000000fffc17a8 A2:  0000000000000010 A3:  0000000000000010
A4:  0000000000000000 A5:  00000000fffc17b8 A6:  0000000000ff0000
A7:  000000000000b100 S2:  0000000000000000 S3:  0000000000000001
S4:  00000000fef38d40 S5:  7375746174730068 S6:  0000000000000000
S7:  00000000fef4eef0 S8:  00000000fef4ef90 S9:  0000000000000000
S10: 0000000000000000 S11: 00000000fef4efc0 T3:  00000000fef40ea8
T4:  0000000000ff0000 T5:  00000000fef40a60 T6:  00000000fef40a6c

To address the above issue, keep some minimal empty space in the
FDT instead of no empty space.

Fixes: bbe9a23060e9 ("platform: generic: Pack the FDT after applying fixups")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
Changes since v1:
- Add Kconfig option for amount of empty space in FDT with default
  value as 4KB
---
 platform/generic/Kconfig    | 5 +++++
 platform/generic/platform.c | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/platform/generic/Kconfig b/platform/generic/Kconfig
index 25b8886b..b1808012 100644
--- a/platform/generic/Kconfig
+++ b/platform/generic/Kconfig
@@ -23,6 +23,11 @@ config PLATFORM_GENERIC_MINOR_VER
 	range 0 65535
 	default 1
 
+config PLATFORM_GENERIC_FDT_EMPTY_SPACE
+	int "Platform FDT empty space (KB)"
+	range 0 1024
+	default 4
+
 config PLATFORM_ALLWINNER_D1
 	bool "Allwinner D1 support"
 	depends on FDT_IRQCHIP_PLIC
diff --git a/platform/generic/platform.c b/platform/generic/platform.c
index e66f99fa..ddfc9e80 100644
--- a/platform/generic/platform.c
+++ b/platform/generic/platform.c
@@ -245,7 +245,10 @@ int generic_final_init(bool cold_boot)
 	fdt_fixups(fdt);
 	fdt_domain_fixup(fdt);
 
+	/* Minimize the empty space in FDT to 4KB */
 	fdt_pack(fdt);
+	fdt_open_into(fdt, fdt, fdt_totalsize(fdt) +
+		      CONFIG_PLATFORM_GENERIC_FDT_EMPTY_SPACE * 1024);
 
 	return 0;
 }
-- 
2.43.0


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH v2] platform: generic: Keep some empty space in FDT passed to next stage

[Samuel Holland]

Hi Anup,

On 2025-11-30 11:00 PM, Anup Patel wrote:
> Leaving no empty space in the FDT passed to the next booting stage
> causes the following U-Boot crash on Ventana internal platforms:
> 
> Unhandled exception: Load access fault
> EPC: 00000000fffa6372 RA: 00000000fffa7418 TVAL: 0001746174730068
> EPC: 0000000080245372 RA: 0000000080246418 reloc adjusted
> 
> SP:  00000000fef38440 GP:  00000000fef40e60 TP:  0000000000000000
> T0:  00000000fef40a70 T1:  000000000000ff00 T2:  0000000000000000
> S0:  00000000fffc17a8 S1:  00000000fef38d40 A0:  7375746174730068
> A1:  00000000fffc17a8 A2:  0000000000000010 A3:  0000000000000010
> A4:  0000000000000000 A5:  00000000fffc17b8 A6:  0000000000ff0000
> A7:  000000000000b100 S2:  0000000000000000 S3:  0000000000000001
> S4:  00000000fef38d40 S5:  7375746174730068 S6:  0000000000000000
> S7:  00000000fef4eef0 S8:  00000000fef4ef90 S9:  0000000000000000
> S10: 0000000000000000 S11: 00000000fef4efc0 T3:  00000000fef40ea8
> T4:  0000000000ff0000 T5:  00000000fef40a60 T6:  00000000fef40a6c
> 
> To address the above issue, keep some minimal empty space in the
> FDT instead of no empty space.
> 
> Fixes: bbe9a23060e9 ("platform: generic: Pack the FDT after applying fixups")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> Changes since v1:
> - Add Kconfig option for amount of empty space in FDT with default
>   value as 4KB
> ---
>  platform/generic/Kconfig    | 5 +++++
>  platform/generic/platform.c | 3 +++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/platform/generic/Kconfig b/platform/generic/Kconfig
> index 25b8886b..b1808012 100644
> --- a/platform/generic/Kconfig
> +++ b/platform/generic/Kconfig
> @@ -23,6 +23,11 @@ config PLATFORM_GENERIC_MINOR_VER
>  	range 0 65535
>  	default 1
>  
> +config PLATFORM_GENERIC_FDT_EMPTY_SPACE
> +	int "Platform FDT empty space (KB)"
> +	range 0 1024
> +	default 4

Since this is a workaround for a specific platform, and the workaround should
eventually be obsoleted by a U-Boot fix, please default this to zero.

> +
>  config PLATFORM_ALLWINNER_D1
>  	bool "Allwinner D1 support"
>  	depends on FDT_IRQCHIP_PLIC
> diff --git a/platform/generic/platform.c b/platform/generic/platform.c
> index e66f99fa..ddfc9e80 100644
> --- a/platform/generic/platform.c
> +++ b/platform/generic/platform.c
> @@ -245,7 +245,10 @@ int generic_final_init(bool cold_boot)
>  	fdt_fixups(fdt);
>  	fdt_domain_fixup(fdt);
>  
> +	/* Minimize the empty space in FDT to 4KB */

This comment is no longer accurate, since the padding is not a fixed size.

>  	fdt_pack(fdt);
> +	fdt_open_into(fdt, fdt, fdt_totalsize(fdt) +
> +		      CONFIG_PLATFORM_GENERIC_FDT_EMPTY_SPACE * 1024);

This change reintroduces the problem that motivated adding fdt_pack().
fdt_open_into() does not initialize the unused space, so once again the range
[fdt, fdt + fdt_totalsize(fdt)) contains unitialized data.

If having unitialized data in the FDT blob is not a problem on the platform
requiring this workaround, then I suppose the code is fine. Otherwise, this
would need a memset().

Regards,
Samuel

>  
>  	return 0;
>  }


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH v2] platform: generic: Keep some empty space in FDT passed to next stage

[Anup Patel]

On Sun, Dec 7, 2025 at 4:29 PM Samuel Holland <samuel.holland@sifive.com> wrote:
>
> Hi Anup,
>
> On 2025-11-30 11:00 PM, Anup Patel wrote:
> > Leaving no empty space in the FDT passed to the next booting stage
> > causes the following U-Boot crash on Ventana internal platforms:
> >
> > Unhandled exception: Load access fault
> > EPC: 00000000fffa6372 RA: 00000000fffa7418 TVAL: 0001746174730068
> > EPC: 0000000080245372 RA: 0000000080246418 reloc adjusted
> >
> > SP:  00000000fef38440 GP:  00000000fef40e60 TP:  0000000000000000
> > T0:  00000000fef40a70 T1:  000000000000ff00 T2:  0000000000000000
> > S0:  00000000fffc17a8 S1:  00000000fef38d40 A0:  7375746174730068
> > A1:  00000000fffc17a8 A2:  0000000000000010 A3:  0000000000000010
> > A4:  0000000000000000 A5:  00000000fffc17b8 A6:  0000000000ff0000
> > A7:  000000000000b100 S2:  0000000000000000 S3:  0000000000000001
> > S4:  00000000fef38d40 S5:  7375746174730068 S6:  0000000000000000
> > S7:  00000000fef4eef0 S8:  00000000fef4ef90 S9:  0000000000000000
> > S10: 0000000000000000 S11: 00000000fef4efc0 T3:  00000000fef40ea8
> > T4:  0000000000ff0000 T5:  00000000fef40a60 T6:  00000000fef40a6c
> >
> > To address the above issue, keep some minimal empty space in the
> > FDT instead of no empty space.
> >
> > Fixes: bbe9a23060e9 ("platform: generic: Pack the FDT after applying fixups")
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > Changes since v1:
> > - Add Kconfig option for amount of empty space in FDT with default
> >   value as 4KB
> > ---
> >  platform/generic/Kconfig    | 5 +++++
> >  platform/generic/platform.c | 3 +++
> >  2 files changed, 8 insertions(+)
> >
> > diff --git a/platform/generic/Kconfig b/platform/generic/Kconfig
> > index 25b8886b..b1808012 100644
> > --- a/platform/generic/Kconfig
> > +++ b/platform/generic/Kconfig
> > @@ -23,6 +23,11 @@ config PLATFORM_GENERIC_MINOR_VER
> >       range 0 65535
> >       default 1
> >
> > +config PLATFORM_GENERIC_FDT_EMPTY_SPACE
> > +     int "Platform FDT empty space (KB)"
> > +     range 0 1024
> > +     default 4
>
> Since this is a workaround for a specific platform, and the workaround should
> eventually be obsoleted by a U-Boot fix, please default this to zero.

Although the issue manifests on Ventana internal platforms,
the issue is not Ventana specific and can happen on any platform
where U-Boot tries to modify FDT passed by OpenSBI without
adding extra space.

>
> > +
> >  config PLATFORM_ALLWINNER_D1
> >       bool "Allwinner D1 support"
> >       depends on FDT_IRQCHIP_PLIC
> > diff --git a/platform/generic/platform.c b/platform/generic/platform.c
> > index e66f99fa..ddfc9e80 100644
> > --- a/platform/generic/platform.c
> > +++ b/platform/generic/platform.c
> > @@ -245,7 +245,10 @@ int generic_final_init(bool cold_boot)
> >       fdt_fixups(fdt);
> >       fdt_domain_fixup(fdt);
> >
> > +     /* Minimize the empty space in FDT to 4KB */
>
> This comment is no longer accurate, since the padding is not a fixed size.

Sure, I will update.

>
> >       fdt_pack(fdt);
> > +     fdt_open_into(fdt, fdt, fdt_totalsize(fdt) +
> > +                   CONFIG_PLATFORM_GENERIC_FDT_EMPTY_SPACE * 1024);
>
> This change reintroduces the problem that motivated adding fdt_pack().
> fdt_open_into() does not initialize the unused space, so once again the range
> [fdt, fdt + fdt_totalsize(fdt)) contains unitialized data.
>
> If having unitialized data in the FDT blob is not a problem on the platform
> requiring this workaround, then I suppose the code is fine. Otherwise, this
> would need a memset().

I think it is better to do memset() on additional space in fdt_open_into()
itself but that would be a separate change in libfdt.

Regards,
Anup

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH v2] platform: generic: Keep some empty space in FDT passed to next stage

[Samuel Holland]

Hi Anup,

On 2025-12-07 11:53 PM, Anup Patel wrote:
> On Sun, Dec 7, 2025 at 4:29 PM Samuel Holland <samuel.holland@sifive.com> wrote:
>> On 2025-11-30 11:00 PM, Anup Patel wrote:
>>> Leaving no empty space in the FDT passed to the next booting stage
>>> causes the following U-Boot crash on Ventana internal platforms:
>>>
>>> Unhandled exception: Load access fault
>>> EPC: 00000000fffa6372 RA: 00000000fffa7418 TVAL: 0001746174730068
>>> EPC: 0000000080245372 RA: 0000000080246418 reloc adjusted
>>>
>>> SP:  00000000fef38440 GP:  00000000fef40e60 TP:  0000000000000000
>>> T0:  00000000fef40a70 T1:  000000000000ff00 T2:  0000000000000000
>>> S0:  00000000fffc17a8 S1:  00000000fef38d40 A0:  7375746174730068
>>> A1:  00000000fffc17a8 A2:  0000000000000010 A3:  0000000000000010
>>> A4:  0000000000000000 A5:  00000000fffc17b8 A6:  0000000000ff0000
>>> A7:  000000000000b100 S2:  0000000000000000 S3:  0000000000000001
>>> S4:  00000000fef38d40 S5:  7375746174730068 S6:  0000000000000000
>>> S7:  00000000fef4eef0 S8:  00000000fef4ef90 S9:  0000000000000000
>>> S10: 0000000000000000 S11: 00000000fef4efc0 T3:  00000000fef40ea8
>>> T4:  0000000000ff0000 T5:  00000000fef40a60 T6:  00000000fef40a6c
>>>
>>> To address the above issue, keep some minimal empty space in the
>>> FDT instead of no empty space.
>>>
>>> Fixes: bbe9a23060e9 ("platform: generic: Pack the FDT after applying fixups")
>>> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
>>> ---
>>> Changes since v1:
>>> - Add Kconfig option for amount of empty space in FDT with default
>>>   value as 4KB
>>> ---
>>>  platform/generic/Kconfig    | 5 +++++
>>>  platform/generic/platform.c | 3 +++
>>>  2 files changed, 8 insertions(+)
>>>
>>> diff --git a/platform/generic/Kconfig b/platform/generic/Kconfig
>>> index 25b8886b..b1808012 100644
>>> --- a/platform/generic/Kconfig
>>> +++ b/platform/generic/Kconfig
>>> @@ -23,6 +23,11 @@ config PLATFORM_GENERIC_MINOR_VER
>>>       range 0 65535
>>>       default 1
>>>
>>> +config PLATFORM_GENERIC_FDT_EMPTY_SPACE
>>> +     int "Platform FDT empty space (KB)"
>>> +     range 0 1024
>>> +     default 4
>>
>> Since this is a workaround for a specific platform, and the workaround should
>> eventually be obsoleted by a U-Boot fix, please default this to zero.
> 
> Although the issue manifests on Ventana internal platforms,
> the issue is not Ventana specific and can happen on any platform
> where U-Boot tries to modify FDT passed by OpenSBI without
> adding extra space.

It's still a U-Boot bug, no? Is your plan to keep the workaround indefinitely?

>>> +
>>>  config PLATFORM_ALLWINNER_D1
>>>       bool "Allwinner D1 support"
>>>       depends on FDT_IRQCHIP_PLIC
>>> diff --git a/platform/generic/platform.c b/platform/generic/platform.c
>>> index e66f99fa..ddfc9e80 100644
>>> --- a/platform/generic/platform.c
>>> +++ b/platform/generic/platform.c
>>> @@ -245,7 +245,10 @@ int generic_final_init(bool cold_boot)
>>>       fdt_fixups(fdt);
>>>       fdt_domain_fixup(fdt);
>>>
>>> +     /* Minimize the empty space in FDT to 4KB */
>>
>> This comment is no longer accurate, since the padding is not a fixed size.
> 
> Sure, I will update.
> 
>>
>>>       fdt_pack(fdt);
>>> +     fdt_open_into(fdt, fdt, fdt_totalsize(fdt) +
>>> +                   CONFIG_PLATFORM_GENERIC_FDT_EMPTY_SPACE * 1024);
>>
>> This change reintroduces the problem that motivated adding fdt_pack().
>> fdt_open_into() does not initialize the unused space, so once again the range
>> [fdt, fdt + fdt_totalsize(fdt)) contains unitialized data.
>>
>> If having unitialized data in the FDT blob is not a problem on the platform
>> requiring this workaround, then I suppose the code is fine. Otherwise, this
>> would need a memset().
> 
> I think it is better to do memset() on additional space in fdt_open_into()
> itself but that would be a separate change in libfdt.

I don't have an opinion on whether it is better to change libfdt, but we need to
add the memset() in at least one place to avoid regressing here.

Regards,
Samuel


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH v2] platform: generic: Keep some empty space in FDT passed to next stage

[Anup Patel]

On Tue, Dec 9, 2025 at 12:03 PM Samuel Holland
<samuel.holland@sifive.com> wrote:
>
> Hi Anup,
>
> On 2025-12-07 11:53 PM, Anup Patel wrote:
> > On Sun, Dec 7, 2025 at 4:29 PM Samuel Holland <samuel.holland@sifive.com> wrote:
> >> On 2025-11-30 11:00 PM, Anup Patel wrote:
> >>> Leaving no empty space in the FDT passed to the next booting stage
> >>> causes the following U-Boot crash on Ventana internal platforms:
> >>>
> >>> Unhandled exception: Load access fault
> >>> EPC: 00000000fffa6372 RA: 00000000fffa7418 TVAL: 0001746174730068
> >>> EPC: 0000000080245372 RA: 0000000080246418 reloc adjusted
> >>>
> >>> SP:  00000000fef38440 GP:  00000000fef40e60 TP:  0000000000000000
> >>> T0:  00000000fef40a70 T1:  000000000000ff00 T2:  0000000000000000
> >>> S0:  00000000fffc17a8 S1:  00000000fef38d40 A0:  7375746174730068
> >>> A1:  00000000fffc17a8 A2:  0000000000000010 A3:  0000000000000010
> >>> A4:  0000000000000000 A5:  00000000fffc17b8 A6:  0000000000ff0000
> >>> A7:  000000000000b100 S2:  0000000000000000 S3:  0000000000000001
> >>> S4:  00000000fef38d40 S5:  7375746174730068 S6:  0000000000000000
> >>> S7:  00000000fef4eef0 S8:  00000000fef4ef90 S9:  0000000000000000
> >>> S10: 0000000000000000 S11: 00000000fef4efc0 T3:  00000000fef40ea8
> >>> T4:  0000000000ff0000 T5:  00000000fef40a60 T6:  00000000fef40a6c
> >>>
> >>> To address the above issue, keep some minimal empty space in the
> >>> FDT instead of no empty space.
> >>>
> >>> Fixes: bbe9a23060e9 ("platform: generic: Pack the FDT after applying fixups")
> >>> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> >>> ---
> >>> Changes since v1:
> >>> - Add Kconfig option for amount of empty space in FDT with default
> >>>   value as 4KB
> >>> ---
> >>>  platform/generic/Kconfig    | 5 +++++
> >>>  platform/generic/platform.c | 3 +++
> >>>  2 files changed, 8 insertions(+)
> >>>
> >>> diff --git a/platform/generic/Kconfig b/platform/generic/Kconfig
> >>> index 25b8886b..b1808012 100644
> >>> --- a/platform/generic/Kconfig
> >>> +++ b/platform/generic/Kconfig
> >>> @@ -23,6 +23,11 @@ config PLATFORM_GENERIC_MINOR_VER
> >>>       range 0 65535
> >>>       default 1
> >>>
> >>> +config PLATFORM_GENERIC_FDT_EMPTY_SPACE
> >>> +     int "Platform FDT empty space (KB)"
> >>> +     range 0 1024
> >>> +     default 4
> >>
> >> Since this is a workaround for a specific platform, and the workaround should
> >> eventually be obsoleted by a U-Boot fix, please default this to zero.
> >
> > Although the issue manifests on Ventana internal platforms,
> > the issue is not Ventana specific and can happen on any platform
> > where U-Boot tries to modify FDT passed by OpenSBI without
> > adding extra space.
>
> It's still a U-Boot bug, no? Is your plan to keep the workaround indefinitely?

It is certainly a U-Boot bug. We will keep the workaround for atleast
6 months (one OpenSBI release) after U-Boot has fixed it.

>
> >>> +
> >>>  config PLATFORM_ALLWINNER_D1
> >>>       bool "Allwinner D1 support"
> >>>       depends on FDT_IRQCHIP_PLIC
> >>> diff --git a/platform/generic/platform.c b/platform/generic/platform.c
> >>> index e66f99fa..ddfc9e80 100644
> >>> --- a/platform/generic/platform.c
> >>> +++ b/platform/generic/platform.c
> >>> @@ -245,7 +245,10 @@ int generic_final_init(bool cold_boot)
> >>>       fdt_fixups(fdt);
> >>>       fdt_domain_fixup(fdt);
> >>>
> >>> +     /* Minimize the empty space in FDT to 4KB */
> >>
> >> This comment is no longer accurate, since the padding is not a fixed size.
> >
> > Sure, I will update.
> >
> >>
> >>>       fdt_pack(fdt);
> >>> +     fdt_open_into(fdt, fdt, fdt_totalsize(fdt) +
> >>> +                   CONFIG_PLATFORM_GENERIC_FDT_EMPTY_SPACE * 1024);
> >>
> >> This change reintroduces the problem that motivated adding fdt_pack().
> >> fdt_open_into() does not initialize the unused space, so once again the range
> >> [fdt, fdt + fdt_totalsize(fdt)) contains unitialized data.
> >>
> >> If having unitialized data in the FDT blob is not a problem on the platform
> >> requiring this workaround, then I suppose the code is fine. Otherwise, this
> >> would need a memset().
> >
> > I think it is better to do memset() on additional space in fdt_open_into()
> > itself but that would be a separate change in libfdt.
>
> I don't have an opinion on whether it is better to change libfdt, but we need to
> add the memset() in at least one place to avoid regressing here.
>

Understood. My suggestion is to do it as a separate patch irrespective
of where we do it.

Regards,
Anup

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi