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

[Heinrich Schuchardt]

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.

@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
>>