[PATCH] efi_loader: consider no-map property of reserved memory

[Heinrich Schuchardt]

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.

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