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

[Ard Biesheuvel]

On Thu, 11 Apr 2019 at 13:08, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> 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.
>

The whole purpose of using GUIDs is that they are guaranteed to be
unique, and so they don't all have to be recorded in the same spec in
order to be usable safely in combination. So we could add it to the
UEFI spec, or to some other spec, or not at all, but it is not really
relevant to the discussion.