[PATCH 5/6] efi: Use malloc() for the EFI pool

[Simon Glass]

Hi Ilias,

On Wed, 31 Jul 2024 at 01:40, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> [...]
>
> > > > > > > >
> > > > > > > > Please also see below.
> > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > You can switch some callsite of efi_allocate_pool to
> > > > > > > > > > > efi_allocate_pages() internally. Will that fix the behavior?
> > > > > > > > > >
> > > > > > > > > > It still allocates memory 'in space' and uses 4KB for each allocation.
> > > > > > > > > >
> > > > > > > >
> > > > > > > > This is the key point that doesn't seem to be coming across. The
> > > > > > > > current allocator is allocating memory wherever it likes, potentially
> > > > > > > > interfering with the kernel_addr_r addresses, etc. as on qemu_arm.
> > > > > > >
> > > > > > > It is, but I don't think using malloc is solving it. It's papering
> > > > > > > over the problem, because if someone in the future launches an EFI app
> > > > > > > or allocates EFI memory with a different type you are back on the same
> > > > > > > problem.
> > > > > >
> > > > > > It is certainly solving this problem.
> > > > > >
> > > > > > Once the app is launched it is OK to overwrite memory...
>
> Why is it ok to overwrite memory? Do you assume that all EFI apps do
> is load something and boot it ?

I mean that by that point U-Boot has finished loading images etc., so
it doesn't matter if EFI starts putting things where it likes in the
address space.

>
> > after all it
> > > > > > has been loaded and is running. The issue is that these little
> > > > > > allocations can end up anywhere in memory. Did you see the qemu_arm
> > > > > > note?
> > > > >
> > > > > Isn't this another part of why we need the LMB rework? So that
> > > > > kernel_addr_r, et al, can be marked as reserved.
> > > >
> > > > If we mark them as reserved, we won't be able to load a file into that
> > > > region, so boot scripts will fail.
> > >
> > > No? We mark it as being overwriteable but allocated. This is part of the
> > > LMB rework series.
> >
> > Yes, I see, that makes sense. I don't know any way to guess the
> > expected size of the kernel or ramdisk. I see that Apple uses 128MB
> > for the kernel (plenty) and 1GB for the ramdisk.
>
> FWIW we dont preload the initrd in EFI. The kernel during the efi stub
> hadoff allocates pages, and loads it on the fly.

OK thanks.

>
> /Ilias
> >
> > >
> > > > Please take a look at the whole series and let me know if there is
> > > > anything missing from the descriptions I have given. I have had this
> > > > problem in the back of my mind for some time...but just a few hours of
> > > > investigation was enough to determine that it really is broken.
> > >
> > > I'm missing something, sorry. Yes, it is known that EFI can make some
> > > incorrect assumptions about what memory is/isn't available (as other
> > > implementations give EFI the world to work with), hence the LMB rework
> > > series to address some of these problems.
> >
> > My goal here is to tidy up EFI memory allocation so that it doesn't
> > result in allocating memory in strange places. Avoiding overlaps is
> > one thing, but the way this is heading, we will get overlap errors
> > randomly on platforms when someone tries to load something into RAM,
> > or we won't protect things that need to be protected. It is all a bit
> > mushy without a proper design.
> >
> > Does that make sense?
> >
> > Regards,
> > Simon
> >
> > [1] https://lore.kernel.org/all/CAFLszThqH01GOqnTxucVE7s+wJU-dz7uqUtkb4iKxs6GgX6tqw@mail.gmail.com/T/

Regards,
Simon