[PATCH v4 0/5] Adjust initial EFI memory-allocation to be in the U-Boot region

[Ilias Apalodimas]

On Tue, 15 Oct 2024 at 16:26, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Ilias,
>
> On Mon, 14 Oct 2024 at 22:28, Ilias Apalodimas
> <ilias.apalodimas at linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Sat, 12 Oct 2024 at 00:23, Simon Glass <sjg at chromium.org> wrote:
> > >
> > > U-Boot relocates itself to the top of memory and is supposed to avoid
> > > using any memory below that region. This allows the maximum amount of
> > > memory to be available for loading images.
> > >
> > > The lmb subsystem automatically protects U-Boot's code and data from
> > > being overwritten. It relies on this region being a contiguous area of
> > > memory. This setup has worked well for at least 15 years and has been
> > > documented in the README for as long.
> > >
> > > The EFI_LOADER subsystem currently uses memory outside the U-Boot region
> > > when starting up.
> > >
> > > Some of the downstream problems this causes are:
> > >
> > > 1. It can overwrite images that have been loaded
> >
> > So can the load command
>
> Can you point to where? If lmb is enabled, it should be using that, so
> not overwrite anything.
>

If you load the initrd in memory X and make the kernel ovelap you will
overwrite parts of the initrd.
Let's try once again, the ''''problem''' that was discussed in IRC, is
that the initramfs overwrites memory that was allocated by the EFI
subsystem *because* we haven't merged the LMB followup patches yet.

> >
> > > 2. Requires lmb reservations to avoid (1) even if EFI_LOADER is not
> > >    being used
> > > 3. Uses regions which scripts expect to be available, e.g. the existing
> > >    kernel_addr_r variables
> > > 4. Introduces confusion as to what memory U-Boot can use for itself
> > >
> > > This series sets up a very small region of memory which EFI can used at
> > > start-up, thus avoiding touching memory outside the U-Boot region.
> >
> > EFI *is* U-Boot memory region. It lies outside the U-Boot binary,
> > because some services are instantiated on boot. It also *is* U-Boot
> > executable code. We should start treating it as such
>
> This seems like semantics to me. What do you think of this patch?

I don't see any reason for doing this. I've asked for a reproducer of
the problem you are trying to solve quite a few times and I still
don't have an email with one

Thanks
/Ilias
>
> Regards,
> Simon
>
>
> >
> > Thanks
> > /Ilias
> > >
> > > There are still a few tables which are not in the bloblist, but that
> > > will be resolved separately.
> > >
> > > Other than tables, the amount of API-visible memory allocated by EFI
> > > before booting is actually very small: 3 allocations, of a total of
> > > about 250 bytes. However, there are some areas where EFI allocations are
> > > done unnecessarily. For example efi_disk_add_dev() uses page-allocation
> > > to allocate an efi_device_path, then immediately frees it again. In this
> > > case it would be better to use malloc(). Since each allocation consumes
> > > a page (4KB), 256KB has been chosen for the size here, enough for 64
> > > allocations.
> > >
> > > As soon as efi_init_obj_list() is called, we know that the EFI subsystem
> > > is to be used, so we can relax the restrictions and allow EFI to use any
> > > memory it likes. This is handled using a simple boolean flag in BSS, since
> > > it cannot happen before relocation.
> > >
> > > Changes in v4:
> > > - Expand the commit message
> > > - Reword the commit message since this feature is needed
> > > - Use a different technique to keep the memory-usage in place
> > > - Drop changes to pool allocation
> > > - Reorder the patches
> > > - Rewrite the cover letter
> > > - Make it a debug message for now
> > >
> > > Changes in v3:
> > > - Add new patch to drop the memset() from efi_alloc()
> > > - Drop patch to convert device_path allocation to use malloc()
> > >
> > > Changes in v2:
> > > - Drop patch 'Show more information in efi index'
> > > - Drop patch 'Avoid pool allocation in efi_get_memory_map_alloc()'
> > > - Add the word 'warning', use log_warning() and show the end address
> > >
> > > Simon Glass (5):
> > >   efi: Drop the memset() from efi_alloc()
> > >   efi: Show the location of the bounce buffer when debugging
> > >   event: Add a generic way to reserve memory
> > >   efi: Reserve some memory for initial use
> > >   efi: Keep early allocations to the U-Boot region
> > >
> > >  common/board_f.c                  |  1 +
> > >  common/event.c                    |  1 +
> > >  include/asm-generic/global_data.h |  6 ++++
> > >  include/efi_loader.h              | 24 +++++++++++++-
> > >  include/event.h                   | 11 +++++++
> > >  lib/efi_loader/efi_bootbin.c      |  9 ++++++
> > >  lib/efi_loader/efi_memory.c       | 54 ++++++++++++++++++++++++-------
> > >  lib/efi_loader/efi_setup.c        |  5 +++
> > >  test/py/tests/test_event_dump.py  |  1 +
> > >  9 files changed, 100 insertions(+), 12 deletions(-)
> > >
> > > --
> > > 2.34.1
> > >