[PATCH] efi_loader: Don't allocate from Special purpose memory

[Ilias Apalodimas]

On Tue, 17 Sept 2024 at 13:05, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> On Tue, 17 Sept 2024 at 14:32, Ilias Apalodimas
> <ilias.apalodimas at linaro.org> wrote:
> >
> > The EFI spec defines special-purpose memory in §7.2. That memory
> > serves as a hint to the OS to avoid allocating this memory for core
> > OS data or code that can not be relocated. So let's ignore it when
> > allocating from conventional memory.
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > ---
>
> Can we rebase this patch on top of the LMB/EFI series patches that are
> currently under review. I believe if we consider the direction that
> the EFI allocation functionality is going to take, that of relying on
> LMB for allocating conventional memory, we will have to take a
> different route to this problem, whereby 1) the LMB module needs to be
> intimated that the region of memory is reserved so that it does not
> get allocated by LMB and 2) then adding this region of memory as
> special-purpose in the EFI memory map.

So, the problem here is LMB (outside EFI) trying to allocate a piece
of memory right? Because the EFI subsystem should do this check an
exit before it allocates memory.
I don't mind waiting, since I discovered the problem while working on
a patch series for persistent memory. I can send it then.
The EFI memory attributes can change at any point in time. So we will
need to augment the notification to the LMB subsystem that marks that
memory as reserved for LMB whenever memory attributes change.

Thanks
/Ilias


>
> -sughosh
>
>
> >  lib/efi_loader/efi_memory.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> > index c6f1dd09456e..74732e37f8aa 100644
> > --- a/lib/efi_loader/efi_memory.c
> > +++ b/lib/efi_loader/efi_memory.c
> > @@ -422,7 +422,8 @@ static efi_status_t efi_check_allocated(u64 addr, bool must_be_allocated)
> >
> >                 if (addr >= start && addr < end) {
> >                         if (must_be_allocated ^
> > -                           (item->desc.type == EFI_CONVENTIONAL_MEMORY))
> > +                           (item->desc.type == EFI_CONVENTIONAL_MEMORY) &&
> > +                           !(item->desc.attribute & EFI_MEMORY_SP))
> >                                 return EFI_SUCCESS;
> >                         else
> >                                 return EFI_NOT_FOUND;
> > @@ -460,6 +461,9 @@ static uint64_t efi_find_free_memory(uint64_t len, uint64_t max_addr)
> >                 if (desc->type != EFI_CONVENTIONAL_MEMORY)
> >                         continue;
> >
> > +               if (desc->attribute & EFI_MEMORY_SP)
> > +                       continue;
> > +
> >                 /* Out of bounds for max_addr */
> >                 if ((ret + len) > max_addr)
> >                         continue;
> > --
> > 2.45.2
> >