[PATCH] efi_loader: consider no-map property of reserved memory
Heinrich Schuchardt
xypron.glpk at gmx.de
Thu May 13 21:41:40 CEST 2021
On 5/11/21 1:47 AM, Atish Patra wrote:
> On Wed, Sep 2, 2020 at 12:10 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>> On 31.08.20 20:08, Atish Patra wrote:
>>> On Thu, Aug 27, 2020 at 9:16 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>>>
>>>> If a reserved memory node in the device tree has the property no-map,
>>>> remove it from the UEFI memory map provided by GetMemoryMap().
>>>>
>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>
>> In the mail-thread starting a
>>
>> [PATCH 1/1] EBBR: GetMemoryMap(), handling of no-map DT property
>> https://lists.linaro.org/pipermail/boot-architecture/2020-September/001389.html
>>
>> the issue has been discussed. The conclusion was that we should not
>> change the current behavior.
>>
>
> Digging some old threads :)
>
> The merged version of the patch marks any reserved memory as
> EFI_BOOT_SERVICES_DATA.
> AFAIK, these memory regions will be available after ExitBootservice.
> If the operating system chooses to
> map these region and access them, it violates the purpose of the
> reserved memory region specified by the firmware.
>
> Did I miss something ?
The no-map property is neither described in the EBBR nor in the
Devicetree specification v0.3 but only in Linux' reserved-memory.txt.
In
https://lists.linaro.org/pipermail/boot-architecture/2020-September/001418.html
Ard requested that no-map memory shall be marked EfiReservedMemory
because Linux will not map EfiReservedMemory, see Linux function
is_usable_memory().
All reserved memory that is not marked as no-map shall be mapped by
Linux. It may for instance serve as DMA pool or as video memory. Or it
may even be reusable. See reserved-memory.txt.
Only drivers will access their own reserved memory if the region is not
marked as reusable.
Best regards
Heinrich
>
>> Best regards
>>
>> Heinrich
>>
>>>> ---
>>>> cmd/bootefi.c | 34 ++++++++++++++++++++++++++++------
>>>> include/efi.h | 3 +++
>>>> lib/efi_loader/efi_memory.c | 7 +++++--
>>>> 3 files changed, 36 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>>>> index 40d5ef2b3a..f173105251 100644
>>>> --- a/cmd/bootefi.c
>>>> +++ b/cmd/bootefi.c
>>>> @@ -135,12 +135,29 @@ done:
>>>> return ret;
>>>> }
>>>>
>>>> -static void efi_reserve_memory(u64 addr, u64 size)
>>>> +/**
>>>> + * efi_reserve_memory() - add reserved memory to memory map
>>>> + *
>>>> + * @addr: start address of the reserved memory range
>>>> + * @size: size of the reserved memory range
>>>> + * @nomap: indicates that the memory range shall be hidden from the memory
>>>> + * map
>>>> + */
>>>> +static void efi_reserve_memory(u64 addr, u64 size, bool nomap)
>>>> {
>>>> + int type;
>>>> + efi_uintn_t ret;
>>>> +
>>>> /* Convert from sandbox address space. */
>>>> addr = (uintptr_t)map_sysmem(addr, 0);
>>>> - if (efi_add_memory_map(addr, size,
>>>> - EFI_RESERVED_MEMORY_TYPE) != EFI_SUCCESS)
>>>> +
>>>> + if (nomap)
>>>> + type = EFI_NO_MAP_MEMORY;
>>>> + else
>>>> + type = EFI_RESERVED_MEMORY_TYPE;
>>>> +
>>>> + ret = efi_add_memory_map(addr, size, type);
>>>> + if (ret != EFI_SUCCESS)
>>>> log_err("Reserved memory mapping failed addr %llx size %llx\n",
>>>> addr, size);
>>>> }
>>>> @@ -166,7 +183,7 @@ static void efi_carve_out_dt_rsv(void *fdt)
>>>> for (i = 0; i < nr_rsv; i++) {
>>>> if (fdt_get_mem_rsv(fdt, i, &addr, &size) != 0)
>>>> continue;
>>>> - efi_reserve_memory(addr, size);
>>>> + efi_reserve_memory(addr, size, false);
>>>> }
>>>>
>>>> /* process reserved-memory */
>>>> @@ -186,8 +203,13 @@ static void efi_carve_out_dt_rsv(void *fdt)
>>>> * a size instead of a reg property.
>>>> */
>>>> if (fdt_addr != FDT_ADDR_T_NONE &&
>>>> - fdtdec_get_is_enabled(fdt, subnode))
>>>> - efi_reserve_memory(fdt_addr, fdt_size);
>>>> + fdtdec_get_is_enabled(fdt, subnode)) {
>>>> + bool nomap;
>>>> +
>>>> + nomap = !!fdt_getprop(fdt, subnode, "no-map",
>>>> + NULL);
>>>> + efi_reserve_memory(fdt_addr, fdt_size, nomap);
>>>> + }
>>>> subnode = fdt_next_subnode(fdt, subnode);
>>>> }
>>>> }
>>>> diff --git a/include/efi.h b/include/efi.h
>>>> index f986aad877..50190021ef 100644
>>>> --- a/include/efi.h
>>>> +++ b/include/efi.h
>>>> @@ -181,6 +181,9 @@ enum efi_mem_type {
>>>>
>>>> EFI_MAX_MEMORY_TYPE,
>>>> EFI_TABLE_END, /* For efi_build_mem_table() */
>>>> +
>>>> + /* Memory that shall not be mapped */
>>>> + EFI_NO_MAP_MEMORY,
>>>
>>> Should it be named as EFI_NO_MAP_RSVD_MEMORY to avoid ambiguity ?
>>>
>>>> };
>>>>
>>>> /* Attribute values */
>>>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>>>> index 7be756e370..d156b9533c 100644
>>>> --- a/lib/efi_loader/efi_memory.c
>>>> +++ b/lib/efi_loader/efi_memory.c
>>>> @@ -251,7 +251,7 @@ static efi_status_t efi_add_memory_map_pg(u64 start, u64 pages,
>>>> EFI_PRINT("%s: 0x%llx 0x%llx %d %s\n", __func__,
>>>> start, pages, memory_type, overlap_only_ram ? "yes" : "no");
>>>>
>>>> - if (memory_type >= EFI_MAX_MEMORY_TYPE)
>>>> + if (memory_type >= EFI_MAX_MEMORY_TYPE && memory_type != EFI_NO_MAP_MEMORY)
>>>> return EFI_INVALID_PARAMETER;
>>>>
>>>> if (!pages)
>>>> @@ -327,7 +327,10 @@ static efi_status_t efi_add_memory_map_pg(u64 start, u64 pages,
>>>> }
>>>>
>>>> /* Add our new map */
>>>> - list_add_tail(&newlist->link, &efi_mem);
>>>> + if (memory_type == EFI_NO_MAP_MEMORY)
>>>> + free(newlist);
>>>> + else
>>>> + list_add_tail(&newlist->link, &efi_mem);
>>>>
>>>> /* And make sure memory is listed in descending order */
>>>> efi_mem_sort();
>>>> --
>>>> 2.28.0
>>>>
>>>
>>>
>>
>
>
More information about the U-Boot
mailing list