[PATCH] efi: Make FDT extra space configurable

[Heinrich Schuchardt]

On 20.02.25 14:31, Simon Glass 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.

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

We need the copy as the original device-tree which may be at
$fdtcontroladdr will not have the required space for expansion.

Best regards

Heinrich