[RFC PATCH v2 38/48] lib: Kconfig: add a config symbol for getting lmb memory map updates

[Simon Glass]

Hi Tom,

On Wed, 24 Jul 2024 at 16:47, Tom Rini <trini at konsulko.com> wrote:
>
> On Wed, Jul 24, 2024 at 09:40:35AM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Wed, 24 Jul 2024 at 08:52, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Wed, Jul 24, 2024 at 08:37:14AM -0600, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Tue, 23 Jul 2024 at 08:20, Tom Rini <trini at konsulko.com> wrote:
> > > > >
> > > > > On Tue, Jul 23, 2024 at 01:42:59PM +0100, Simon Glass wrote:
> > > > > > Hi Sughosh,
> > > > > >
> > > > > > On Mon, 22 Jul 2024 at 13:59, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > > > > > >
> > > > > > > On Mon, 22 Jul 2024 at 18:00, Ilias Apalodimas
> > > > > > > <ilias.apalodimas at linaro.org> wrote:
> > > > > > > >
> > > > > > > > On Fri, 5 Jul 2024 at 22:51, Tom Rini <trini at konsulko.com> wrote:
> > > > > > > > >
> > > > > > > > > On Thu, Jul 04, 2024 at 01:05:34PM +0530, Sughosh Ganu wrote:
> > > > > > > > >
> > > > > > > > > > Add a Kconfig symbol to enable getting updates on any memory map
> > > > > > > > > > changes that might be done by the LMB module. This notification
> > > > > > > > > > mechanism can then be used to have a synchronous view of allocated and
> > > > > > > > > > free memory.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > > > > > > > > > ---
> > > > > > > > > > Changes since V1:
> > > > > > > > > > * Change the description to highlight only LMB notifications.
> > > > > > > > > > * Add a separate line for dependencies.
> > > > > > > > > >
> > > > > > > > > >  lib/Kconfig | 10 ++++++++++
> > > > > > > > > >  1 file changed, 10 insertions(+)
> > > > > > > > > >
> > > > > > > > > > diff --git a/lib/Kconfig b/lib/Kconfig
> > > > > > > > > > index 7eea517b3b..b422183a0f 100644
> > > > > > > > > > --- a/lib/Kconfig
> > > > > > > > > > +++ b/lib/Kconfig
> > > > > > > > > > @@ -74,6 +74,16 @@ config HAVE_PRIVATE_LIBGCC
> > > > > > > > > >  config LIB_UUID
> > > > > > > > > >       bool
> > > > > > > > > >
> > > > > > > > > > +config MEM_MAP_UPDATE_NOTIFY
> > > > > > > > > > +     bool "Get notified of any changes to the LMB memory map"
> > > > > > > > > > +     depends on EVENT && LMB && EFI_LOADER
> > > > > > > > > > +     default y
> > > > > > > > > > +     help
> > > > > > > > > > +       Enable this option to get notification on any changes to the
> > > > > > > > > > +       memory that is allocated or freed by the LMB module. This will
> > > > > > > > > > +       allow different modules that allocate memory or maintain a memory
> > > > > > > > > > +       map to have a synchronous view of available and allocated memory.
> > > > > > > > >
> > > > > > > > > This needs to be select'd when it's going to be used, opting out of
> > > > > > > > > making sure memory reservations are obeyed isn't a good idea.
> > > > > > > >
> > > > > > > > +1 which begs the question, do we need the config option at all ?
> > > > > > >
> > > > > > > The config symbol can be used for removing the code for platforms
> > > > > > > which do not support EFI ?
> > > > > >
> > > > > > I am still of the so-far firm opinion that this can be done once,
> > > > > > before booting, rather than maintaining two separate tables as we go.
> > > > >
> > > > > Did you see the part in the thread where he explained the multiple entry
> > > > > points that would need to be kept in sync?
> > > >
> > > > Yes, but I'm not sure what they are, nor why a shared function cannot
> > > > be called twice from two different places.
> > >
> > > Because we don't want to miss the third or fourth entry point down the
> > > road. That's why going the other direction makes more sense I believe,
> > > we won't have a future problem here because we designed with that in
> > > mind.
> >
> > You might be right, but I don't even know what we are referring to
> > here...what are the two 'entry points'?
>
> I would have to refer back to Sughosh's explanation to repeat it, sorry.
>
> > My current belief is that we can set up the EFI memory table before
> > booting, with a single pass through the table. Maintaining two
> > independent tables as we go doesn't seem very useful. It is harder to
> > test too.
>
> You seem to be fixated on "booting" when at least part of the issue is
> re-entering the EFI_LOADER. And since EFI_LOADER needs X/Y/Z as well, we
> had much earlier rejected "make EFI_LOADER's memory model what
> everything else uses".

I wouldn't say I am fixated. EFI_LOADER's purpose is to boot an OS.

What do you mean by re-entering the EFI_LOADER? Do you mean running
one app, then coming back to U-Boot, then running another? I am not
suggesting we reinit EFI in that case, if that is what you are worried
about.

I sent a separate series to show some of the issues I've been
concerned about for a few years now. Perhaps we should schedule a
discussion to talk all this through one day?

Regards,
Simon