* [PATCH v2 1/1] efi_loader: put device tree into EfiACPIReclaimMemory
@ 2020-05-07 18:19 Heinrich Schuchardt
2020-05-11 8:48 ` Grant Likely
0 siblings, 1 reply; 4+ messages in thread
From: Heinrich Schuchardt @ 2020-05-07 18:19 UTC (permalink / raw)
To: u-boot
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
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v2 1/1] efi_loader: put device tree into EfiACPIReclaimMemory
2020-05-07 18:19 [PATCH v2 1/1] efi_loader: put device tree into EfiACPIReclaimMemory Heinrich Schuchardt
@ 2020-05-11 8:48 ` Grant Likely
2020-05-11 11:55 ` Heinrich Schuchardt
0 siblings, 1 reply; 4+ messages in thread
From: Grant Likely @ 2020-05-11 8:48 UTC (permalink / raw)
To: u-boot
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?
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v2 1/1] efi_loader: put device tree into EfiACPIReclaimMemory
2020-05-11 8:48 ` Grant Likely
@ 2020-05-11 11:55 ` Heinrich Schuchardt
2020-05-12 14:18 ` Grant Likely
0 siblings, 1 reply; 4+ messages in thread
From: Heinrich Schuchardt @ 2020-05-11 11:55 UTC (permalink / raw)
To: u-boot
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.
@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
>>
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v2 1/1] efi_loader: put device tree into EfiACPIReclaimMemory
2020-05-11 11:55 ` Heinrich Schuchardt
@ 2020-05-12 14:18 ` Grant Likely
0 siblings, 0 replies; 4+ messages in thread
From: Grant Likely @ 2020-05-12 14:18 UTC (permalink / raw)
To: u-boot
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
>>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-12 14:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-07 18:19 [PATCH v2 1/1] efi_loader: put device tree into EfiACPIReclaimMemory Heinrich Schuchardt
2020-05-11 8:48 ` Grant Likely
2020-05-11 11:55 ` Heinrich Schuchardt
2020-05-12 14:18 ` Grant Likely
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.