[PATCH v2 1/1] efi_loader: put device tree into EfiACPIReclaimMemory

[Grant Likely]

On 11/05/2020 12:55, Heinrich Schuchardt wrote:
> On 11.05.20 10:48, Grant Likely wrote:
>> On 07/05/2020 19:19, Heinrich Schuchardt wrote:
>>> According to the UEFI spec ACPI tables should be placed in
>>> EfiACPIReclaimMemory. Let's do the same with the device tree.
>>>
>>> Suggested-by: Ard Biesheuvel <ardb at kernel.org>
>>> Cc: Grant Likely <grant.likely at arm.com>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>> ---
>>> v2:
>>>      adjust the unit test
>>
>> Is there any impact to changing the memory type for current users? Does
>> the kernel currently expect the EFI_BOOT_SERVICES_DATA memory type? What
>> happens if Grub or another EFI application replaces the DTB table? Will
>> it try to use a different memory type, and will that matter?
> 
> If we are using GRUB, linux_boot() allocates EFI_LOADER_DATA memory and
> copies the FDT to it. When the devicetree command is used GRUB also
> allocates EFI_LOADER_DATA.
> 
> So changes in U-Boot have no effect on what Linux sees if booted via GRUB.
> 
> Both EFI_BOOT_SERVICES_DATA and EFI_LOADER_DATA are meant to be gone
> past ExitBootServices(). EfiACPIReclaimMemory is were the UEFI payload
> is adviced to consider if keeping the memory at runtime is needed.
> 
> The current patch therefore makes no difference to Linux before
> ExitBootServices(). A side effect could be that Linux unnecessarily
> keeps the EfiACPIReclaimMemory instead of reusing it. But Linux's
> is_usable_memory() in drivers/firmware/efi/arm-init.c treats
> EFI_ACPI_RECLAIM_MEMORY, EFI_ACPI_RECLAIM_MEMORY,
> EFI_ACPI_RECLAIM_MEMORY just the same.

Okay. Works for me.

g.

> @Daniel, Leif
> A patch for the EBBR specification suggests to use
> EFI_ACPI_RECLAIM_MEMORY for the devicetree passed to Linux. Please,
> consider if you would apply this in GRUB too.
> 
> Best regards
> 
> Heinrich
> 
>>
>> g.
>>
>>> ---
>>>    cmd/bootefi.c                          | 4 ++--
>>>    lib/efi_selftest/efi_selftest_memory.c | 4 ++--
>>>    2 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>>> index 54b4b8f984..06573b14e9 100644
>>> --- a/cmd/bootefi.c
>>> +++ b/cmd/bootefi.c
>>> @@ -127,13 +127,13 @@ static efi_status_t copy_fdt(void **fdtp)
>>>        new_fdt_addr = (uintptr_t)map_sysmem(fdt_ram_start + 0x7f00000 +
>>>                             fdt_size, 0);
>>>        ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
>>> -                 EFI_BOOT_SERVICES_DATA, fdt_pages,
>>> +                 EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
>>>                     &new_fdt_addr);
>>>        if (ret != EFI_SUCCESS) {
>>>            /* If we can't put it there, put it somewhere */
>>>            new_fdt_addr = (ulong)memalign(EFI_PAGE_SIZE, fdt_size);
>>>            ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
>>> -                     EFI_BOOT_SERVICES_DATA, fdt_pages,
>>> +                     EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
>>>                         &new_fdt_addr);
>>>            if (ret != EFI_SUCCESS) {
>>>                printf("ERROR: Failed to reserve space for FDT\n");
>>> diff --git a/lib/efi_selftest/efi_selftest_memory.c
>>> b/lib/efi_selftest/efi_selftest_memory.c
>>> index e71732dc6d..4d32a28006 100644
>>> --- a/lib/efi_selftest/efi_selftest_memory.c
>>> +++ b/lib/efi_selftest/efi_selftest_memory.c
>>> @@ -176,9 +176,9 @@ static int execute(void)
>>>        /* Check memory reservation for the device tree */
>>>        if (fdt_addr &&
>>>            find_in_memory_map(map_size, memory_map, desc_size, fdt_addr,
>>> -                   EFI_BOOT_SERVICES_DATA) != EFI_ST_SUCCESS) {
>>> +                   EFI_ACPI_RECLAIM_MEMORY) != EFI_ST_SUCCESS) {
>>>            efi_st_error
>>> -            ("Device tree not marked as boot services data\n");
>>> +            ("Device tree not marked as ACPI reclaim memory\n");
>>>            return EFI_ST_FAILURE;
>>>        }
>>>        return EFI_ST_SUCCESS;
>>> --
>>> 2.26.2
>>>
>