From mboxrd@z Thu Jan  1 00:00:00 1970
From: Grant Likely <grant.likely@arm.com>
Date: Tue, 12 May 2020 15:18:20 +0100
Subject: [PATCH v2 1/1] efi_loader: put device tree into
 EfiACPIReclaimMemory
In-Reply-To: <6ca302a6-3287-097f-1f83-0464743dcd66@gmx.de>
References: <20200507181937.8923-1-xypron.glpk@gmx.de>
 <867623da-8b32-fe0d-4149-261e952d93f9@arm.com>
 <6ca302a6-3287-097f-1f83-0464743dcd66@gmx.de>
Message-ID: <589e3eda-b746-64a9-6a72-fadabd8a380f@arm.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de



On 11/05/2020 12:55, Heinrich Schuchardt wrote:
> 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@kernel.org>
>>> Cc: Grant Likely <grant.likely@arm.com>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@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.

Okay. Works for me.

g.

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