diff mbox series

[RFC,v2,16/48] lmb: config: add lmb config symbols for SPL

Message ID 20240704073544.670249-17-sughosh.ganu@linaro.org
State Superseded
Headers show
Series Make U-Boot memory reservations coherent | expand

Commit Message

Sughosh Ganu July 4, 2024, 7:35 a.m. UTC
Add separate config symbols for enabling the LMB module for the SPL
phase. The LMB module implementation now relies on alloced list data
structure which requires heap area to be present. Add specific config
symbol for the SPL phase of U-Boot so that this can be enabled on
platforms which support a heap in SPL.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
Changes since V1: New patch

 lib/Kconfig | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Tom Rini July 5, 2024, 7:48 p.m. UTC | #1
On Thu, Jul 04, 2024 at 01:05:12PM +0530, Sughosh Ganu wrote:

> Add separate config symbols for enabling the LMB module for the SPL
> phase. The LMB module implementation now relies on alloced list data
> structure which requires heap area to be present. Add specific config
> symbol for the SPL phase of U-Boot so that this can be enabled on
> platforms which support a heap in SPL.
> 
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> Changes since V1: New patch
> 
>  lib/Kconfig | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 072ed0ecfa..7eea517b3b 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -1103,7 +1103,17 @@ config LMB
>  	default y if ARC || ARM || M68K || MICROBLAZE || MIPS || \
>  		     NIOS2 || PPC || RISCV || SANDBOX || SH || X86 || XTENSA
>  	help
> -	  Support the library logical memory blocks.
> +	  Support the library logical memory blocks. This will require
> +	  a malloc() implementation for defining the data structures
> +	  needed for maintaining the LMB memory map.

Even today, LMB really should be def_bool y rather than an option, so
this series should correct that. That said...

> +config SPL_LMB
> +	bool "Enable LMB module for SPL"
> +	depends on SPL && SPL_FRAMEWORK && SPL_SYS_MALLOC
> +	help
> +	  Enable support for Logical Memory Block library routines in
> +	  SPL. This will require a malloc() implementation for defining
> +	  the data structures needed for maintaining the LMB memory map.

The question I guess becomes when do we need LMB in SPL, exactly? And I
guess it's another case where it should be def_bool y (but still depends
on what you have here) since we need to make sure we don't overwrite
running SPL.
Sughosh Ganu July 8, 2024, 11:36 a.m. UTC | #2
On Sat, 6 Jul 2024 at 01:18, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Jul 04, 2024 at 01:05:12PM +0530, Sughosh Ganu wrote:
>
> > Add separate config symbols for enabling the LMB module for the SPL
> > phase. The LMB module implementation now relies on alloced list data
> > structure which requires heap area to be present. Add specific config
> > symbol for the SPL phase of U-Boot so that this can be enabled on
> > platforms which support a heap in SPL.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> > Changes since V1: New patch
> >
> >  lib/Kconfig | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/Kconfig b/lib/Kconfig
> > index 072ed0ecfa..7eea517b3b 100644
> > --- a/lib/Kconfig
> > +++ b/lib/Kconfig
> > @@ -1103,7 +1103,17 @@ config LMB
> >       default y if ARC || ARM || M68K || MICROBLAZE || MIPS || \
> >                    NIOS2 || PPC || RISCV || SANDBOX || SH || X86 || XTENSA
> >       help
> > -       Support the library logical memory blocks.
> > +       Support the library logical memory blocks. This will require
> > +       a malloc() implementation for defining the data structures
> > +       needed for maintaining the LMB memory map.
>
> Even today, LMB really should be def_bool y rather than an option, so
> this series should correct that. That said...

Okay

>
> > +config SPL_LMB
> > +     bool "Enable LMB module for SPL"
> > +     depends on SPL && SPL_FRAMEWORK && SPL_SYS_MALLOC
> > +     help
> > +       Enable support for Logical Memory Block library routines in
> > +       SPL. This will require a malloc() implementation for defining
> > +       the data structures needed for maintaining the LMB memory map.
>
> The question I guess becomes when do we need LMB in SPL, exactly? And I
> guess it's another case where it should be def_bool y (but still depends
> on what you have here) since we need to make sure we don't overwrite
> running SPL.

So this is a question even I had. Do we really need to enable LMB in
SPL ? The main reason for introducing the symbol was to have more
granularity to remove the LMB code from SPL, but should this really be
enabled in SPL is something that I am not too sure about.

-sughosh

>
> --
> Tom
Tom Rini July 8, 2024, 2:46 p.m. UTC | #3
On Mon, Jul 08, 2024 at 05:06:45PM +0530, Sughosh Ganu wrote:
> On Sat, 6 Jul 2024 at 01:18, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Jul 04, 2024 at 01:05:12PM +0530, Sughosh Ganu wrote:
> >
> > > Add separate config symbols for enabling the LMB module for the SPL
> > > phase. The LMB module implementation now relies on alloced list data
> > > structure which requires heap area to be present. Add specific config
> > > symbol for the SPL phase of U-Boot so that this can be enabled on
> > > platforms which support a heap in SPL.
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > ---
> > > Changes since V1: New patch
> > >
> > >  lib/Kconfig | 12 +++++++++++-
> > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/Kconfig b/lib/Kconfig
> > > index 072ed0ecfa..7eea517b3b 100644
> > > --- a/lib/Kconfig
> > > +++ b/lib/Kconfig
> > > @@ -1103,7 +1103,17 @@ config LMB
> > >       default y if ARC || ARM || M68K || MICROBLAZE || MIPS || \
> > >                    NIOS2 || PPC || RISCV || SANDBOX || SH || X86 || XTENSA
> > >       help
> > > -       Support the library logical memory blocks.
> > > +       Support the library logical memory blocks. This will require
> > > +       a malloc() implementation for defining the data structures
> > > +       needed for maintaining the LMB memory map.
> >
> > Even today, LMB really should be def_bool y rather than an option, so
> > this series should correct that. That said...
> 
> Okay
> 
> >
> > > +config SPL_LMB
> > > +     bool "Enable LMB module for SPL"
> > > +     depends on SPL && SPL_FRAMEWORK && SPL_SYS_MALLOC
> > > +     help
> > > +       Enable support for Logical Memory Block library routines in
> > > +       SPL. This will require a malloc() implementation for defining
> > > +       the data structures needed for maintaining the LMB memory map.
> >
> > The question I guess becomes when do we need LMB in SPL, exactly? And I
> > guess it's another case where it should be def_bool y (but still depends
> > on what you have here) since we need to make sure we don't overwrite
> > running SPL.
> 
> So this is a question even I had. Do we really need to enable LMB in
> SPL ? The main reason for introducing the symbol was to have more
> granularity to remove the LMB code from SPL, but should this really be
> enabled in SPL is something that I am not too sure about.

Yes, we need to ensure we obey reservations in SPL, both for U-Boot and
for when we boot the OS from SPL.
Simon Glass July 13, 2024, 3:15 p.m. UTC | #4
Hi,

On Mon, 8 Jul 2024 at 15:46, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Jul 08, 2024 at 05:06:45PM +0530, Sughosh Ganu wrote:
> > On Sat, 6 Jul 2024 at 01:18, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Jul 04, 2024 at 01:05:12PM +0530, Sughosh Ganu wrote:
> > >
> > > > Add separate config symbols for enabling the LMB module for the SPL
> > > > phase. The LMB module implementation now relies on alloced list data
> > > > structure which requires heap area to be present. Add specific config
> > > > symbol for the SPL phase of U-Boot so that this can be enabled on
> > > > platforms which support a heap in SPL.
> > > >
> > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > > ---
> > > > Changes since V1: New patch
> > > >
> > > >  lib/Kconfig | 12 +++++++++++-
> > > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/lib/Kconfig b/lib/Kconfig
> > > > index 072ed0ecfa..7eea517b3b 100644
> > > > --- a/lib/Kconfig
> > > > +++ b/lib/Kconfig
> > > > @@ -1103,7 +1103,17 @@ config LMB
> > > >       default y if ARC || ARM || M68K || MICROBLAZE || MIPS || \
> > > >                    NIOS2 || PPC || RISCV || SANDBOX || SH || X86 || XTENSA
> > > >       help
> > > > -       Support the library logical memory blocks.
> > > > +       Support the library logical memory blocks. This will require
> > > > +       a malloc() implementation for defining the data structures
> > > > +       needed for maintaining the LMB memory map.
> > >
> > > Even today, LMB really should be def_bool y rather than an option, so
> > > this series should correct that. That said...
> >
> > Okay
> >
> > >
> > > > +config SPL_LMB
> > > > +     bool "Enable LMB module for SPL"
> > > > +     depends on SPL && SPL_FRAMEWORK && SPL_SYS_MALLOC
> > > > +     help
> > > > +       Enable support for Logical Memory Block library routines in
> > > > +       SPL. This will require a malloc() implementation for defining
> > > > +       the data structures needed for maintaining the LMB memory map.
> > >
> > > The question I guess becomes when do we need LMB in SPL, exactly? And I
> > > guess it's another case where it should be def_bool y (but still depends
> > > on what you have here) since we need to make sure we don't overwrite
> > > running SPL.
> >
> > So this is a question even I had. Do we really need to enable LMB in
> > SPL ? The main reason for introducing the symbol was to have more
> > granularity to remove the LMB code from SPL, but should this really be
> > enabled in SPL is something that I am not too sure about.
>
> Yes, we need to ensure we obey reservations in SPL, both for U-Boot and
> for when we boot the OS from SPL.

I believe we will use it, e.g. in VPL. But we don't have (nor perhaps
even want) a mechanism for passing the lmb from one phase to the next.

Regards,
Simon
diff mbox series

Patch

diff --git a/lib/Kconfig b/lib/Kconfig
index 072ed0ecfa..7eea517b3b 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -1103,7 +1103,17 @@  config LMB
 	default y if ARC || ARM || M68K || MICROBLAZE || MIPS || \
 		     NIOS2 || PPC || RISCV || SANDBOX || SH || X86 || XTENSA
 	help
-	  Support the library logical memory blocks.
+	  Support the library logical memory blocks. This will require
+	  a malloc() implementation for defining the data structures
+	  needed for maintaining the LMB memory map.
+
+config SPL_LMB
+	bool "Enable LMB module for SPL"
+	depends on SPL && SPL_FRAMEWORK && SPL_SYS_MALLOC
+	help
+	  Enable support for Logical Memory Block library routines in
+	  SPL. This will require a malloc() implementation for defining
+	  the data structures needed for maintaining the LMB memory map.
 
 config PHANDLE_CHECK_SEQ
 	bool "Enable phandle check while getting sequence number"