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

[Sughosh Ganu]

On Thu, 26 Sept 2024 at 16:38, Simon Glass <sjg at chromium.org> wrote:
>
> 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.

How do we get rid of using efi_allocate_pages() in U-Boot ? This
function is used, among other things, to allocate memory used for
loading images to memory. This can be a significant amount of memory.
Are you proposing using malloc() even for loading EFI images ?

Moreover, the syncing of the memory maps is currently needed primarily
as we support printing the EFI memory map through a command. And I
mentioned in one of my earlier replies that not syncing the changes in
the LMB memory map with the EFI module would mean that the EFI memory
map would have to be generated afresh whenever there is a need for the
EFI memory map -- this would be either when requested from an EFI app,
or when a user asks through a command. Re-generating the EFI memory
map is not a trivial task, and syncing the changes in the LMB map as
and when they happen is much easier.

-sughosh

>
> 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
> [..]