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

[Heinrich Schuchardt]

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.

Best regards

Heinrich