| Message ID | 20240704073544.670249-39-sughosh.ganu@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Make U-Boot memory reservations coherent | expand |
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@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.
On Fri, 5 Jul 2024 at 22:51, Tom Rini <trini@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@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 ? Cheers /Ilias > > -- > Tom
On Mon, 22 Jul 2024 at 18:00, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > On Fri, 5 Jul 2024 at 22:51, Tom Rini <trini@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@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 ? -sughosh > > Cheers > /Ilias > > > > -- > > Tom
Hi Sughosh, On Mon, 22 Jul 2024 at 15:59, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > On Mon, 22 Jul 2024 at 18:00, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: > > > > On Fri, 5 Jul 2024 at 22:51, Tom Rini <trini@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@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 ? In that case, it shouldn't be a user config option. It should be a Kconfig symbol that is automatically set if EFI_LOADER is enabled. Regards /Ilias > > -sughosh > > > > > Cheers > > /Ilias > > > > > > -- > > > Tom
Hi Sughosh, On Mon, 22 Jul 2024 at 13:59, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > On Mon, 22 Jul 2024 at 18:00, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: > > > > On Fri, 5 Jul 2024 at 22:51, Tom Rini <trini@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@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. Regards, Simon
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@linaro.org> wrote: > > > > On Mon, 22 Jul 2024 at 18:00, Ilias Apalodimas > > <ilias.apalodimas@linaro.org> wrote: > > > > > > On Fri, 5 Jul 2024 at 22:51, Tom Rini <trini@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@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?
Hi Tom, On Tue, 23 Jul 2024 at 08:20, Tom Rini <trini@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@linaro.org> wrote: > > > > > > On Mon, 22 Jul 2024 at 18:00, Ilias Apalodimas > > > <ilias.apalodimas@linaro.org> wrote: > > > > > > > > On Fri, 5 Jul 2024 at 22:51, Tom Rini <trini@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@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. Regards, Simon
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@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@linaro.org> wrote: > > > > > > > > On Mon, 22 Jul 2024 at 18:00, Ilias Apalodimas > > > > <ilias.apalodimas@linaro.org> wrote: > > > > > > > > > > On Fri, 5 Jul 2024 at 22:51, Tom Rini <trini@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@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.
Hi Tom, On Wed, 24 Jul 2024 at 08:52, Tom Rini <trini@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@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@linaro.org> wrote: > > > > > > > > > > On Mon, 22 Jul 2024 at 18:00, Ilias Apalodimas > > > > > <ilias.apalodimas@linaro.org> wrote: > > > > > > > > > > > > On Fri, 5 Jul 2024 at 22:51, Tom Rini <trini@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@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'? 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. Regards, Simon
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@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@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@linaro.org> wrote: > > > > > > > > > > > > On Mon, 22 Jul 2024 at 18:00, Ilias Apalodimas > > > > > > <ilias.apalodimas@linaro.org> wrote: > > > > > > > > > > > > > > On Fri, 5 Jul 2024 at 22:51, Tom Rini <trini@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@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".
Hi Tom, On Wed, 24 Jul 2024 at 16:47, Tom Rini <trini@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@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@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@linaro.org> wrote: > > > > > > > > > > > > > > On Mon, 22 Jul 2024 at 18:00, Ilias Apalodimas > > > > > > > <ilias.apalodimas@linaro.org> wrote: > > > > > > > > > > > > > > > > On Fri, 5 Jul 2024 at 22:51, Tom Rini <trini@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@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
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. + config RANDOM_UUID bool "GPT Random UUID generation" select LIB_UUID
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@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(+)