On Fri, 20 Sept 2024 at 14:51, Ilias Apalodimas
<ilias.apalodi...@linaro.org> wrote:
>
> Hi Sughosh,
>
> On Tue, 17 Sept 2024 at 15:33, Sughosh Ganu <sughosh.g...@linaro.org> wrote:
> >
> > On Sat, 14 Sept 2024 at 20:38, Heinrich Schuchardt <xypron.g...@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.

-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.g...@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;
> > > >   };
> > > >
> > > >   /**
> > >

Reply via email to