[U-Boot,v2,1/6] spl: add Kconfig option to clear bss early
diff mbox series

Message ID 20190315201347.29475-2-simon.k.r.goldschmidt@gmail.com
State Superseded
Delegated to: Tom Rini
Headers show
Series
  • spl: full-featured heap cleanups
Related show

Commit Message

Simon Goldschmidt March 15, 2019, 8:13 p.m. UTC
This introduces a new Kconfig option SPL_CLEAR_BSS_F. If enabled, it clears
the bss before calling board_init_f() instead of clearing it before calling
board_init_r().

This also ensures that variables placed in BSS can be shared between
board_init_f() and board_init_r() in SPL.

Make the new option depend on ARM for now until more implementations follow.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---

Changes in v2:
- make CONFIG_SPL_CLEAR_BSS_F depend on ARM for now

 common/spl/Kconfig | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Simon Glass March 22, 2019, 7:53 a.m. UTC | #1
Hi,

On Sat, 16 Mar 2019 at 04:13, Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> This introduces a new Kconfig option SPL_CLEAR_BSS_F. If enabled, it clears
> the bss before calling board_init_f() instead of clearing it before calling
> board_init_r().
>
> This also ensures that variables placed in BSS can be shared between
> board_init_f() and board_init_r() in SPL.
>
> Make the new option depend on ARM for now until more implementations follow.
>
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
>
> Changes in v2:
> - make CONFIG_SPL_CLEAR_BSS_F depend on ARM for now
>
>  common/spl/Kconfig | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

The current restriction is that you are not allowed to use BSS before
board_init_r().

Can you please add a motivation to change this?

Regards,
SImon
Simon Goldschmidt March 22, 2019, 8:16 a.m. UTC | #2
Hi Simon,

On Fri, Mar 22, 2019 at 8:53 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi,
>
> On Sat, 16 Mar 2019 at 04:13, Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com> wrote:
> >
> > This introduces a new Kconfig option SPL_CLEAR_BSS_F. If enabled, it clears
> > the bss before calling board_init_f() instead of clearing it before calling
> > board_init_r().
> >
> > This also ensures that variables placed in BSS can be shared between
> > board_init_f() and board_init_r() in SPL.
> >
> > Make the new option depend on ARM for now until more implementations follow.
> >
> > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> > ---
> >
> > Changes in v2:
> > - make CONFIG_SPL_CLEAR_BSS_F depend on ARM for now
> >
> >  common/spl/Kconfig | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
>
> The current restriction is that you are not allowed to use BSS before
> board_init_r().

I understood that, but I did not understand why. This is perfectly valid for
U-Boot proper or for SPL if SPL_SEPARATE_BSS is used and bss is in
SDRAM. However, the latter seems to be seldom used and for the "default"
case where bss is in SRAM, it's available right from the start and clearing
it after board_init_f() has been run seems quite unuseful and unexpected.

>
> Can you please add a motivation to change this?

The motivation is to allow SPL to use full malloc in board_init_f()
like required
by socfpga_arria10 (see patch 5/6). Without this, the malloc pointers get
overwritten after board_init_f() and further calls to malloc() fail.

Regards,
Simon
Simon Glass March 30, 2019, 8:03 p.m. UTC | #3
Hi Simon,

On Fri, 22 Mar 2019 at 02:16, Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> Hi Simon,
>
> On Fri, Mar 22, 2019 at 8:53 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi,
> >
> > On Sat, 16 Mar 2019 at 04:13, Simon Goldschmidt
> > <simon.k.r.goldschmidt@gmail.com> wrote:
> > >
> > > This introduces a new Kconfig option SPL_CLEAR_BSS_F. If enabled, it clears
> > > the bss before calling board_init_f() instead of clearing it before calling
> > > board_init_r().
> > >
> > > This also ensures that variables placed in BSS can be shared between
> > > board_init_f() and board_init_r() in SPL.
> > >
> > > Make the new option depend on ARM for now until more implementations follow.
> > >
> > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> > > ---
> > >
> > > Changes in v2:
> > > - make CONFIG_SPL_CLEAR_BSS_F depend on ARM for now
> > >
> > >  common/spl/Kconfig | 12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> >
> > The current restriction is that you are not allowed to use BSS before
> > board_init_r().
>
> I understood that, but I did not understand why. This is perfectly valid for
> U-Boot proper or for SPL if SPL_SEPARATE_BSS is used and bss is in
> SDRAM. However, the latter seems to be seldom used and for the "default"
> case where bss is in SRAM, it's available right from the start and clearing
> it after board_init_f() has been run seems quite unuseful and unexpected.

Well this is the API. We have to have some order and conventions
otherwise things just decend into chaos, with so many options that it
all becomes bewildering.

See 'Board Initialisation Flow:' in the README.

>
> >
> > Can you please add a motivation to change this?
>
> The motivation is to allow SPL to use full malloc in board_init_f()
> like required
> by socfpga_arria10 (see patch 5/6). Without this, the malloc pointers get
> overwritten after board_init_f() and further calls to malloc() fail.

Can you call spl_early_init() instead?

Regards,
Simon

Patch
diff mbox series

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 206c24076d..6a4270516a 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -156,6 +156,18 @@  config SPL_STACK_R_MALLOC_SIMPLE_LEN
 	  to give board_init_r() a larger heap then the initial heap in
 	  SRAM which is limited to SYS_MALLOC_F_LEN bytes.
 
+config SPL_CLEAR_BSS_F
+	bool "Clear BSS section before calling board_init_f"
+	depends on ARM
+	help
+	  The BSS section is initialized to zero. In SPL, this is normally done
+	  before calling board_init_r().
+	  For platforms using BSS in board_init_f() already, enable this to
+	  clear the BSS section before calling board_init_f() instead of
+	  clearing it before calling board_init_r(). This also ensures that
+	  variables placed in BSS can be shared between board_init_f() and
+	  board_init_r().
+
 config SPL_SEPARATE_BSS
 	bool "BSS section is in a different memory region from text"
 	help