[PATCH] efi: Make FDT extra space configurable

[Simon Glass]

Hi Ilias,

On Thu, 20 Feb 2025 at 06:50, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Simon,
>
> On Thu, 20 Feb 2025 at 15:31, Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi,
> >
> > On Thu, 20 Feb 2025 at 01:21, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > >
> > > On 2/20/25 07:58, Ilias Apalodimas wrote:
> > > > On Mon, 10 Feb 2025 at 18:07, Pawel Kochanowski <pkochanowski at sii.pl> wrote:
> > > >>
> > > >> From: Gabriel Nesteruk <gnesteruk at sii.pl>
> > > >>
> > > >> U-Boot currently reserves only 0x3000 bytes when copying the FDT
> > > >> in `copy_fdt()`, which may not be sufficient if additional nodes
> > > >> (such as FMAN firmware) are added later.
> > > >>
> > > >> This patch introduces a new `CONFIG_FDT_EXTRA_SPACE` Kconfig option
> > > >> to allow users to define the amount of extra space reserved for FDT
> > > >> expansion. The default remains 0x3000.
> > > >>
> > > >> This change prevents potential corruption when resizing FDT after
> > > >> EFI boot, especially when firmware like FMAN requires additional
> > > >> space.
> > > >>
> > > >> Signed-off-by: Gabriel Nesteruk <gnesteruk at sii.pl>
> > > >> Signed-off-by: Pawel Kochanowski <pkochanowski at sii.pl>
> > > >> ---
> > > >>   lib/efi_loader/Kconfig      | 9 +++++++++
> > > >>   lib/efi_loader/efi_helper.c | 2 +-
> > > >>   2 files changed, 10 insertions(+), 1 deletion(-)
> > > >>
> > > >> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > > >> index d4f6b56afaa..bd2cbe995c9 100644
> > > >> --- a/lib/efi_loader/Kconfig
> > > >> +++ b/lib/efi_loader/Kconfig
> > > >> @@ -586,6 +586,15 @@ config BOOTEFI_TESTAPP_COMPILE
> > > >>            No additional space will be required in the resulting U-Boot binary
> > > >>            when this option is enabled.
> > > >>
> > > >> +config EFI_FDT_EXTRA_SPACE
> > > >> +    hex "Extra space to allocate for FDT expansion"
> > >
> > > Hello Pawel,
> > >
> > > Is this problem really EFI specific? Wouldn't you have a similar problem
> > > when booting via booti?
> > >
> > > >> +    default 0x3000
> > > >> +    help
> > > >> +      Defines additional space (in bytes) reserved for expanding the
> > > >> +      Flattened Device Tree (FDT) when passed to the EFI system.
> > > >> +      Increase this value if your firmware (e.g., FMAN) needs to add
> > > >> +      more data to the device tree after U-Boot relocation.
> > > >> +
> > > >>   endif
> > > >>
> > > >>   source "lib/efi/Kconfig"
> > > >> diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
> > > >> index 04b2efc4a3b..91b2a5b8653 100644
> > > >> --- a/lib/efi_loader/efi_helper.c
> > > >> +++ b/lib/efi_loader/efi_helper.c
> > > >> @@ -477,7 +477,7 @@ static efi_status_t copy_fdt(void **fdtp)
> > > >>           * needs to be expanded later.
> > > >>           */
> > > >>          fdt = *fdtp;
> > > >> -       fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000);
> > > >> +       fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + CONFIG_EFI_FDT_EXTRA_SPACE);
> > >
> > > This is not the only place where the configuration value needs to be
> > > considered:
> > >
> > > lib/efi_loader/efi_dt_fixup.c:171:
> > >                                0x3000;
> > >
> > > Could you, please, send a new revision of your patch.
> > >
> > > Best regards
> > >
> > > Heinrich
> > >
> > > >>          fdt_size = fdt_pages << EFI_PAGE_SHIFT;
> > > >>
> > > >>          ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
> > > >> --
> > > >> 2.43.0
> > > >>
> > > >
> > > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > >
> >
> > Let's do this instead, as it is generic:
> >
> > https://lore.kernel.org/u-boot/20241206131113.2865416-3-sjg@chromium.org/
> >
> > I believe the copy_fdt() function can just be removed, as the FDT is
> > already in reserved memory. Putting it in the ACPI_RECLAIM region is
> > wrong, too, as it is not an ACPI region.
>
> That's not true. The EFI specification explicitly requests is as such.
> ACPI_RECLAIM is a bit of an unfortunate misnomer, but it's memory the
> OS can reclaim if it wants. IIRC Linux doesn't currently

Got it. Yes it looks like Linux doesn't reclaim it.

Regards,
Simon