[PATCH RFC 07/14] efi: stub: add known memory to U-Boot's EFI memory map

[Caleb Connolly]

Hi Sughosh,

Thanks for the review.

On 28/11/2024 07:38, Sughosh Ganu wrote:
> On Mon, 25 Nov 2024 at 01:58, Caleb Connolly <caleb.connolly at linaro.org> wrote:
>>
>> When running U-Boot as an EFI payload with CONFIG_EFI_STUB, the reserved
>> regions from the previous stage EFI bootloader should be carried over
>> since they may not fully align with the reserved-memory regions in
>> devicetree.
>>
>> Implement a helper to map these pages when the EFI subsystem starts up.
>>
>> Signed-off-by: Caleb Connolly <caleb.connolly at linaro.org>
>> ---
>>  include/efi_stub.h          |  7 +++++++
>>  lib/efi/efi_info.c          | 31 +++++++++++++++++++++++++++++++
>>  lib/efi_loader/efi_memory.c |  5 +++++
>>  3 files changed, 43 insertions(+)
>>
>> diff --git a/include/efi_stub.h b/include/efi_stub.h
>> index ff3befd4830b..343c84435970 100644
>> --- a/include/efi_stub.h
>> +++ b/include/efi_stub.h
>> @@ -56,5 +56,12 @@ int of_populate_from_efi(struct device_node *root);
>>   * EFI payload with CONFIG_EFI_STUB enabled.
>>   */
>>  int dram_init_banksize_from_efi(void);
>>
>> +/**
>> + * efi_add_known_memory_from_efi() - Add known memory pages from the memory map
>> + * of the EFI bootloader that booted U-Boot. This is only applicable when running
>> + * U-Boot as an EFI payload with CONFIG_EFI_STUB enabled.
>> + */
>> +void efi_add_known_memory_from_efi(void);
>> +
>>  #endif /* _EFI_STUB_H */
>> diff --git a/lib/efi/efi_info.c b/lib/efi/efi_info.c
>> index f9743a3e7fad..ef4e646b082b 100644
>> --- a/lib/efi/efi_info.c
>> +++ b/lib/efi/efi_info.c
>> @@ -5,8 +5,9 @@
>>   * Access to the EFI information table
>>   */
>>
>>  #include <efi.h>
>> +#include <efi_loader.h>
>>  #include <efi_stub.h>
>>  #include <errno.h>
>>  #include <mapmem.h>
>>  #include <asm/global_data.h>
>> @@ -177,4 +178,34 @@ int dram_init_banksize_from_efi(void)
>>         }
>>
>>         return 0;
>>  }
>> +
>> +/* Called by U-Boot's EFI subsystem to add known memory. In our case
>> + * we need to add some specific memory types from the original bootloaders
>> + * EFI memory map
>> + */
>> +void efi_add_known_memory_from_efi(void)
>> +{
>> +       struct efi_mem_desc *desc, *end;
>> +       struct efi_entry_memmap *map;
>> +       int ret, size;
>> +
>> +       EFI_PRINT("Adding known memory from previous stage EFI bootloader\n");
>> +
>> +       ret = efi_info_get(EFIET_MEMORY_MAP, (void **)&map, &size);
>> +       if (ret) {
>> +               EFI_PRINT("%s: Missing memory map\n", __func__);
>> +               return;
>> +       }
>> +       end = (struct efi_mem_desc *)((ulong)map + size);
>> +
>> +       for (desc = map->desc; desc < end; desc = efi_get_next_mem_desc(desc, map->desc_size)) {
>> +               switch (desc->type) {
>> +               case EFI_RESERVED_MEMORY_TYPE:
>> +                       efi_add_memory_map_pg(desc->physical_start, desc->num_pages, desc->type, false);
>> +                       break;
>> +               default:
>> +                       continue;
>> +               }
>> +       }
>> +}
> 
> If this is DRAM memory that is being added, this now needs to happen
> through LMB to ensure that these reserved memory regions don't get
> used by the LMB allocator. You can check if something can be added to
> the lmb_reserve_common() function. These regions will get marked as
> boot services data. If these are to be kept as reserved memory even in
> the OS, the memory type of these regions can be updated by keeping
> this function. But the point to consider is that this needs to be
> updated in the LMB memory map also.

Right I see. Thanks.

I'm not sure if this function is really necessary, I added it while
trying to debug some EFI crash but the cause was something else.

In theory the DT reserved-memory regions should match the reserved
regions from the EFI (I'm fairly sure this is the case for the X1E
laptops anyways), and I suppose there's no risk of mapping these as
normal memory anyway since we populate the dram banks from the same EFI
memory map.

So probably I'll remove this function from the next revision.

Kind regards,
> 
> -sughosh
> 
>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>> index d2f5d563f2a0..50b0010608e7 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -7,8 +7,9 @@
>>
>>  #define LOG_CATEGORY LOGC_EFI
>>
>>  #include <efi_loader.h>da
>> +#include <efi_stub.h>
>>  #include <init.h>
>>  #include <lmb.h>
>>  #include <log.h>
>>  #include <malloc.h>
>> @@ -834,8 +835,12 @@ static void add_u_boot_and_runtime(void)
>>  int efi_memory_init(void)
>>  {
>>         efi_add_known_memory();
>>
>> +#ifdef CONFIG_EFI_STUB
>> +       efi_add_known_memory_from_efi();
>> +#endif
>> +
>>         add_u_boot_and_runtime();
>>
>>  #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
>>         /* Request a 32bit 64MB bounce buffer region */
>>
>> --
>> 2.47.0
>>

-- 
// Caleb (they/them)