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

[Ard Biesheuvel]

On Fri, 12 Apr 2019 at 10:16, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 4/11/19 10:50 PM, Ard Biesheuvel wrote:
> > On Thu, 11 Apr 2019 at 12:59, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >>
> >> On 4/11/19 9:41 PM, Ilias Apalodimas wrote:
> >>> Hi Heinrich,
> >>>> 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?
> >>>>
> >>>> How about functions efi_smbios_register() and efi_acpi_register()?
> >>>>
> >>>> 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."
> >>>>
> >>>> 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?
> >>>
> >>> What about using EFI_BOOT_SERVICES_DATA instead of EFI_ACPI_RECLAIM_MEMORY?
> >>> This still fixes the issue and doesn't cause any of the potential problems you
> >>> mentioned
> >>
> >> Why services data and not loader data?
> >>
> >
> > Services data is used by the boot services and loader data by the
> > loader. GRUB is a loader so it uses loader data, u-boot is both boot
> > services and a loader so it could arguably use both, but boot services
> > data is more idiomatic.
> >
> >>From the pov of the OS, it makes no difference at all.
> >
> >> As said above we should consider all three supported tables ACPI,
> >> SMBIOS, and FDT when thinking about a fix. The UEFI spec describes that
> >> the addresses inside ACPI and SMBIOS tables are not fixed up when
> >> entering runtime.
> >>
> >> This indicates that at least these tables have to be accessible at
> >> runtime.
> >
> > Accessible at runtime but *not* by the runtime services themselves.
> >
> >> Why do you think that the FDT table should be treated> differently to the ACPI table?
> >>
> >
> > Same thing. Accessible at runtime but not by the firmware services themselves.
> >
> > Only data structures that the runtime services need to access in order
> > to implement the functionality they expose to the OS should be in
> > runtime services data memory. This applies to DT, ACPI and SMBIOS
> > tables alike, but as I mentioned, for historical reasons, SMBIOS
> > tables are usually exposed via runtime services data. Interestingly,
> > the UEFI spec mentions that smbios tables should be located in runtime
> > services data but no virtual mapping should be requested for them, and
> > this is actually impossible in EDK2, so the current practice of using
> > rtservicesdata for SMBIOS with the EFI_MEMORY_RUNTIME attribute set
> > also violates the UEFI spec.
> >
>
> Hello Ilias, hello Ard,
>
> please, have a look at this patch:
>
> efi_loader: update virtual address in efi_mem_carve_out
> https://lists.denx.de/pipermail/u-boot/2019-April/364937.html
>
> Possibly the bug reported here could have contributed the Linux crash
> you experienced.
>

Hello Heinrich,

That patch looks completely unrelated, to be honest.