Hi Heinrich, On Thu, 20 Feb 2025 at 06:47, Heinrich Schuchardt <xypron.g...@gmx.de> wrote: > > On 20.02.25 14:31, Simon Glass wrote: > > Hi, > > > > On Thu, 20 Feb 2025 at 01:21, Heinrich Schuchardt <xypron.g...@gmx.de> > > wrote: > >> > >> On 2/20/25 07:58, Ilias Apalodimas wrote: > >>> On Mon, 10 Feb 2025 at 18:07, Pawel Kochanowski <pkochanow...@sii.pl> > >>> wrote: > >>>> > >>>> From: Gabriel Nesteruk <gneste...@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 <gneste...@sii.pl> > >>>> Signed-off-by: Pawel Kochanowski <pkochanow...@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.apalodi...@linaro.org> > >> > > > > Let's do this instead, as it is generic: > > > > https://lore.kernel.org/u-boot/20241206131113.2865416-3-...@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. > > Hello Simon, > > UEFI specification 2.11: > > 4.6.1.3 Devicetree Tables > The DTB must be contained in memory of type EfiACPIReclaimMemory. > > EBBR: > > The DTB must be contained in memory of type `EfiACPIReclaimMemory`. > https://github.com/ARM-software/ebbr/blob/75112a58a5eacdfee758ce1b93ef3692b225fc36/source/chapter2-uefi.rst?plain=1#L396
Ah, I see, thanks. Confusing. > > We need the copy as the original device-tree which may be at > $fdtcontroladdr will not have the required space for expansion. That's the subject of the patch I linked. It would be nice if EFI could use common code to update the FDT *e.g. image_setup_linux()), then copy it into the new 'ACPI' region after that. Regards, Simon
