[U-Boot] [PATCH] [U-boot]: Change FDT memory typpe from runtime data to acpi reclaim

[Heinrich Schuchardt]

On 4/11/19 9:50 PM, Ard Biesheuvel wrote:
> (+ Leif)
>
> On Thu, 11 Apr 2019 at 12:34, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>> On 4/11/19 8:39 PM, Ilias Apalodimas wrote:
>>> Following Ard's suggestion:
>>> Runtime data sections are intended for data that is used by the runtime
>>> services implementations.
>>> Let's change they type to EFI_ACPI_RECLAIM_MEMORY
>>>
>>> Suggested-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
>>> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
>>> ---
>>>    cmd/bootefi.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>>> index 3619a20e6433..b54181909aff 100644
>>> --- a/cmd/bootefi.c
>>> +++ b/cmd/bootefi.c
>>> @@ -111,13 +111,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_RUNTIME_SERVICES_DATA, fdt_pages,
>>> +                              EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
>>
>> GRUB uses EfiLoaderCode when installing its modified version of the FDT.
>>
>> The "Embedded Base Boot Requirements (EBBR) Specification, Release v1.0"
>> does not require ACPI support. Can we expect EfiACPIReclaimMemory to be
>> supported if we do not have any ACPI table?
>>
>
> In spite of the name, ACPI reclaim memory is fully specified by the
> UEFI spec, so whether you actually use ACPI or not is irrelevant. It
> is memory that can be treated by the OS as conventional memory if it
> doesn't care about preserving the contents.
>
>> How about functions efi_smbios_register() and efi_acpi_register()?
>>
>
> SMBIOS uses RuntimeServicesData memory for historical reasons, but
> that is basically a mistake.
> ACPI tables should use ACPI reclaim memory, and even the allocations
> they refer to (such as the bitmap referenced by the BGRT table) should
> use ACPI reclaim memory, which is another thing we are trying to get
> fixed in the spec.
>
>> How about systab.tables assigned in efi_initialize_system_table()? Which
>> of the addresses in systab.tables should be updated upon relocation.
>>
>> The EFI Spec is really hazy: "A pointer to the table associated with
>> VendorGuid. Whether this pointer is a physical address or a
>> virtual address during runtime is determined by the VendorGuid."
>>
>
> Indeed. So it is up to the publisher to update the reference, but I am
> not aware of any firmware implementations that do this in practice. It
> is typically assumed that a firmware component that is still active at
> runtime holds its own reference to data exposed via a configuration
> table, and updates the reference during SetVirtualAddressMap.
>
> There is also a known bug in EDK2 where the ESRT table is passed in
> boot services memory, but the capsule update code actually tries to
> access it at runtime, so this isn't as clean as we'd like it to be.
>
>> The FDT_TABLE_GUID or DEVICE_TREE_GUID as Linux calls it is not defined
>> in the UEFI spec. So we can marvel about expected behavior. Is there any
>> other document specifying it?
>>
>
> No, its de facto specification is in the EDK2 source tree.

As all ARM systems use it I guess this GUID should move into the UEFI
spec. Maybe Linaro could raise this issue.

Best regards

Heinrich

>
> Since there is no reason the firmware itself would need to consume the
> device tree at runtime, there is no point in keeping it in
> EfiRuntimeServicesData memory or in updating the configuration table
> entry.
>