[RFC PATCH v2 41/48] efi_memory: add an event handler to update memory map

[Sughosh Ganu]

hi Simon,

On Sat, 13 Jul 2024 at 20:46, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Sughosh,
>
> On Thu, 4 Jul 2024 at 08:38, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> >
> > There are events that would be used to notify other interested modules
> > of any changes in available and occupied memory. This would happen
> > when a module allocates or reserves memory, or frees up memory. These
> > changes in memory map should be notified to other interested modules
> > so that the allocated memory does not get overwritten. Add an event
> > handler in the EFI memory module to update the EFI memory map
> > accordingly when such changes happen. As a consequence, any subsequent
> > memory request would honour the updated memory map and only available
> > memory would be allocated from.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > ---
> > Changes since V1:
> > * Handle the addition of memory to the LMB memory map.
> > * Pass the overlap_only_ram parameter to the efi_add_memory_map_pg()
> >   based on the type of operation.
> >
> >  lib/efi_loader/Kconfig      |  1 +
> >  lib/efi_loader/efi_memory.c | 34 ++++++++++++++++++++++++++++++++++
> >  2 files changed, 35 insertions(+)
> >
>
> This is getting complicated and I don't believe it is needed.
>
> EFI should not be allocating memory 'in free space' until it starts
> up. For the very few (if any) cases where it does, it can do an lmb
> allocation.

EFI memory module is not allocating memory at all now. This patch is
adding an event handler for updating the EFI memory map, whenever the
LMB memory map changes. All the EFI allocations are now being routed
through the LMB API's.

>
> As to the lmb allocations themselves, EFI can simply call look through
> the lmb list and call efi_add_memory_map_pg() for each entry, when it
> is ready to boot. There is no need to do it earlier.

So in this case, I believe that rather than adding code in multiple
places where the EFI memory module would have to get the LMB map and
then update it's own, I think it is easier to update the EFI memory
map as and when the LMB map gets updated. Else, we have a scenario
where the EFI memory map would have to be updated as part of the EFI
memory map dump function, as well as before the EFI boot. Any new code
that would be subsequently introduced that might have a similar
requirement would then be needed to keep this point in mind(to get the
memory map from LMB).

This is not an OS file-system kind of an operation where performance
is critical, nor is this event(LMB memory map update) going to happen
very frequently. So I believe that it would be better to keep the EFI
memory map updated along with the LMB one.

-sughosh

>
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index bdf5732974..2d90bcef2f 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -16,6 +16,7 @@ config EFI_LOADER
> >         select CHARSET
> >         # We need to send DM events, dynamically, in the EFI block driver
> >         select DM_EVENT
> > +       select EVENT
> >         select EVENT_DYNAMIC
> >         select LIB_UUID
> >         select LMB
> > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> > index 5691b5da03..bd12504f72 100644
> > --- a/lib/efi_loader/efi_memory.c
> > +++ b/lib/efi_loader/efi_memory.c
> > @@ -45,6 +45,10 @@ static LIST_HEAD(efi_mem);
> >  void *efi_bounce_buffer;
> >  #endif
> >
> > +#define MAP_OP_RESERVE         (u8)0x1
> > +#define MAP_OP_FREE            (u8)0x2
> > +#define MAP_OP_ADD             (u8)0x3
> > +
> >  /**
> >   * struct efi_pool_allocation - memory block allocated from pool
> >   *
> > @@ -928,3 +932,33 @@ int efi_memory_init(void)
> >
> >         return 0;
> >  }
> > +
> > +#if CONFIG_IS_ENABLED(MEM_MAP_UPDATE_NOTIFY)
> > +static int lmb_mem_map_update_sync(void *ctx, struct event *event)
> > +{
> > +       u8 op;
> > +       u64 addr;
> > +       u64 pages;
> > +       efi_status_t status;
> > +       struct event_lmb_map_update *lmb_map = &event->data.lmb_map;
> > +
> > +       addr = (uintptr_t)map_sysmem(lmb_map->base, 0);
> > +       pages = efi_size_in_pages(lmb_map->size + (addr & EFI_PAGE_MASK));
> > +       op = lmb_map->op;
> > +       addr &= ~EFI_PAGE_MASK;
> > +
> > +       if (op != MAP_OP_RESERVE && op != MAP_OP_FREE && op != MAP_OP_ADD) {
> > +               log_debug("Invalid map update op received (%d)\n", op);
> > +               return -1;
> > +       }
> > +
> > +       status = efi_add_memory_map_pg(addr, pages,
> > +                                      op == MAP_OP_RESERVE ?
> > +                                      EFI_BOOT_SERVICES_DATA :
> > +                                      EFI_CONVENTIONAL_MEMORY,
> > +                                      op == MAP_OP_RESERVE ? true : false);
> > +
> > +       return status == EFI_SUCCESS ? 0 : -1;
> > +}
> > +EVENT_SPY_FULL(EVT_LMB_MAP_UPDATE, lmb_mem_map_update_sync);
> > +#endif /* MEM_MAP_UPDATE_NOTIFY */
> > --
> > 2.34.1
> >
>
> Regards,
> Simon