[PATCH 04/16] event: add event to notify lmb memory map changes

[Simon Glass]

Hi Sughosh,

On Fri, 20 Sept 2024 at 13:38, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> On Fri, 20 Sept 2024 at 14:51, Ilias Apalodimas
> <ilias.apalodimas at linaro.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Tue, 17 Sept 2024 at 15:33, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > >
> > > On Sat, 14 Sept 2024 at 20:38, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > > >
> > > > On 9/5/24 10:27, Sughosh Ganu wrote:
> > > > > Add an event which would be used for notifying changes in the
> > > > > LMB modules' memory map. This is to be used for having a
> > > > > synchronous view of the memory that is currently in use, and that is
> > > > > available for allocations.
> > > >
> > > > The synchronous view problem only exists because we are duplicating
> > > > data. Store the EFI memory type in LMB and the problem vanishes.
> > >
> > > The LMB module only concerns itself with RAM memory. If I understand
> > > correctly, you are proposing maintaining the EFI memory map within the
> > > LMB module ? That would mean handling memory types other than
> > > conventional memory in LMB.
> >
> > I am pretty sure I've asked this before, but do these *always* need to
> > be in sync?
> >
> > The efi allocators will call LMB now. So when we allocate something
> > gtom EFI, even if any potential changes from LMB haven't been
> > published to EFI, we won't have any memory corruptions. Can't we just
> > opportunistically update the memory map once someone requests it?
>
> I have given this a thought. Because what you mention, Simon has a
> similar comment. But to achieve this, it would require generating a
> new efi memory map afresh every time such a requirement comes up. This
> would mean, create a new memory map, put in the conventional memory,
> and add other memory types that were part of the existing memory map.
> And then remove the older memory map. This would need to be done every
> time a memory map is needed to be generated. And that would also
> include instances when a user enters a command to get the current
> memory map. I think notifying any changes to the lmb memory map to the
> efi memory module is easier, and less error prone.

We need to get some agreement on my memory-allocation patches first. I
don't believe we are on the same page on those, despite some weeks of
discussion. We need to resolve that issue first. I did try right from
the start to first agree on the problem to be solved. We skipped that,
so now we are having to do it now...

Regards,
Simon


>
> -sughosh
>
> >
> > Thanks
> > /Ilias
> > >
> > > -sughosh
> > >
> > > >
> > > > The event is only needed to notify EFI_EVENT_GROUP_MEMORY_MAP_CHANGE.
> > > >
> > > > Best regards
> > > >
> > > > Heinrich
> > > >
> > > > >
> > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > > > > ---
> > > > >   common/event.c  |  2 ++
> > > > >   include/event.h | 14 ++++++++++++++
> > > > >   2 files changed, 16 insertions(+)
> > > > >
> > > > > diff --git a/common/event.c b/common/event.c
> > > > > index dda569d447..fc8002603c 100644
> > > > > --- a/common/event.c
> > > > > +++ b/common/event.c
> > > > > @@ -48,6 +48,8 @@ const char *const type_name[] = {
> > > > >
> > > > >       /* main loop events */
> > > > >       "main_loop",
> > > > > +
> > > > > +     "lmb_map_update",
> > > > >   };
> > > > >
> > > > >   _Static_assert(ARRAY_SIZE(type_name) == EVT_COUNT, "event type_name size");
> > > > > diff --git a/include/event.h b/include/event.h
> > > > > index fb353ad623..fce7e96170 100644
> > > > > --- a/include/event.h
> > > > > +++ b/include/event.h
> > > > > @@ -153,6 +153,14 @@ enum event_t {
> > > > >        */
> > > > >       EVT_MAIN_LOOP,
> > > > >
> > > > > +     /**
> > > > > +      * @EVT_LMB_MAP_UPDATE:
> > > > > +      * This event is triggered on an update to the LMB reserved memory
> > > > > +      * region. This can be used to notify about any LMB memory allocation
> > > > > +      * or freeing of memory having occurred.
> > > > > +      */
> > > > > +     EVT_LMB_MAP_UPDATE,
> > > > > +
> > > > >       /**
> > > > >        * @EVT_COUNT:
> > > > >        * This constants holds the maximum event number + 1 and is used when
> > > > > @@ -203,6 +211,12 @@ union event_data {
> > > > >               oftree tree;
> > > > >               struct bootm_headers *images;
> > > > >       } ft_fixup;
> > > > > +
> > > > > +     struct event_lmb_map_update {
> > > > > +             u64 base;
> > > > > +             u64 size;
> > > > > +             u8 op;
> > > > > +     } lmb_map;
> > > > >   };
> > > > >
> > > > >   /**
> > > >