Re: [PATCH 5/6] efi_memory: backfill EFI_CONVENTIONAL_MEMORY

[Randolph Sapp <rs@ti.com>]

On Thu Apr 2, 2026 at 12:32 PM CDT, Randolph Sapp wrote:
> On Thu Apr 2, 2026 at 3:53 AM CDT, Ilias Apalodimas wrote:
>> Hi Randolph
>>
>> On Thu, 2 Apr 2026 at 03:14, <rs@ti.com> wrote:
>>>
>>> From: Randolph Sapp <rs@ti.com>
>>>
>>> Backfill any fragmented EFI_CONVENTIONAL_MEMORY when LMB allocations
>>> start to fail. This may be a low memory environment, or maybe LMB is
>>> running up against a tricky reserved region.
>>>
>>> Signed-off-by: Randolph Sapp <rs@ti.com>
>>> ---
>>>  lib/efi_loader/efi_memory.c | 35 +++++++++++++++++++++++++++++++++--
>>>  1 file changed, 33 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>>> index 882366a9f8a..f07cc39b157 100644
>>> --- a/lib/efi_loader/efi_memory.c
>>> +++ b/lib/efi_loader/efi_memory.c
>>> @@ -441,6 +441,27 @@ static efi_status_t efi_check_allocated(u64 addr, bool must_be_allocated)
>>>         return EFI_NOT_FOUND;
>>>  }
>>>
>>> +/**
>>> + * efi_get_conventional_start - find the highest conventional region start
>>> + *                              address with at least the specified number of
>>> + *                              pages
>>> + *
>>> + * @pages:                      number of pages required to be in that carveout
>>> + * Return:                      starting address of the give carveout
>>> + */
>>> +static u64 efi_get_conventional_start(u64 pages)
>>> +{
>>> +       struct efi_mem_list *item;
>>> +
>>> +       list_for_each_entry(item, &efi_mem, link) {
>>> +               if (item->desc.type != EFI_CONVENTIONAL_MEMORY)
>>> +                       continue;
>>> +               if (item->desc.num_pages >= pages)
>>> +                       return item->desc.physical_start;
>>> +       }
>>> +       return 0;
>>
>> We've been bitten by this in the past and although unlikely 0 is a
>> valid address for some hardware. Can we return something different if
>> we don't find a match? UINT64_MAX perhaps?
>>
>>> +}
>>> +
>>>  /**
>>>   * efi_allocate_pages - allocate memory pages
>>>   *
>>> @@ -507,10 +528,20 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type,
>>>         /* Reserve that map in our memory maps */
>>>         ret = efi_update_memory_map(efi_addr, pages, memory_type, true, false);
>>
>> In theory the lmb and the EFI maps are in sync. I haven't checked
>> close enough yet, but do you have cases where lmb_alloc worked and
>> updating the efi memory map failed?
>
> Fair point. Now that I've fixed the FDT warning I should probably check if I can
> actually reproduce the issue that actually required this.

Yeah, this patch isn't necessary anymore. I suppose it should be dropped as it
would potentially mask any discrepancies between the two maps.

Out of curiosity, is there any plan to merge the EFI and LMB allocators more in
the future, or is the current layering scheme the furthest we want to go?

>>>         if (ret != EFI_SUCCESS) {
>>> -               /* Map would overlap, bail out */
>>> +               /* Map would overlap, try something else */
>>>                 lmb_free(addr, (u64)pages << EFI_PAGE_SHIFT, flags);
>>>                 unmap_sysmem((void *)(uintptr_t)efi_addr);
>>> -               return  EFI_OUT_OF_RESOURCES;
>>> +
>>> +               /* See if there is any EFI_CONVENTIONAL_MEMORY allocations */
>>> +               if (type != EFI_ALLOCATE_ADDRESS) {
>>
>> Can you please inverse this. It's going to reduce the identation.
>> if (type == EFI_ALLOCATE_ADDRESS)
>>> +                       *memory = efi_get_conventional_start(pages);
>>> +                       if (*memory != 0)
>>
>> Same here
>>
>>> +                               return efi_allocate_pages(EFI_ALLOCATE_ADDRESS,
>>> +                                                         memory_type, pages,
>>> +                                                         memory);
>>> +               }
>>> +
>>> +               return EFI_OUT_OF_RESOURCES;
>>>         }
>>>
>>>         *memory = efi_addr;
>>> --
>>> 2.53.0
>>>
>>
>> Thanks
>> /Ilias