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

[Simon Glass]

Hi Sughosh,

On Thu, 26 Sept 2024 at 09:12, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> On Wed, 25 Sept 2024 at 18:23, Simon Glass <sjg at chromium.org> wrote:
> >
> > 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...
>
> These patches are currently under review. But fwiw, I think you are
> aware that these patches are not related to what your patch series is
> attempting. Your patches are related to the efi_allocate_pool()
> function, whereas this series is trying to use LMB as the backend for
> allocating pages, as requested from efi_allocate_pages(). So these are
> not related. But like I said, you are aware of these details :)

The thing is, if we actually tidy up the EFI allocations then there
will be no use of allocate-pages before the app starts. So we won't
need to 'sync' the two different sets of memory tables. There will
just be one.

If I were confident that we could apply your series and then clean it
up later, I would be fine with it. But given that my two series have
sat here for a considerable period of time and are still not really
making progress, I lack confidence.

Regards,
Simon
[..]