[U-Boot] [PATCH v3 1/6] spl: add Kconfig option to clear bss early

Fri May 3 20:24:56 UTC 2019

Simon,

On Sat, Mar 30, 2019 at 09:18:08PM +0100, Simon Goldschmidt wrote:
> Simon Glass <sjg at chromium.org> schrieb am Sa., 30. März 2019, 21:06:
> 
> > Hi Simon,
> >
> > On Wed, 27 Mar 2019 at 13:40, Simon Goldschmidt
> > <simon.k.r.goldschmidt at 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.
> > >
> > > Such global variables are used, for example, when loading things from FAT
> > > before SDRAM is available: the full heap required for FAT uses global
> > > variables and clearing BSS after board_init_f() would reset the heap
> > state.
> > > An example for such a usage is socfpa_arria10 where an FPGA configuration
> > > is required before SDRAM can be used.
> > >
> > > Make the new option depend on ARM for now until more implementations
> > follow.
> > >
> >
> > I still have objections to this series and I think we should discuss
> > other ways of solving this problem.
> >
> > Does socfgpa have SRAM that could be used before SDRAM is available?
> > If so, can we not use that for the configuration? What various are
> > actually in BSS that are needed before board_init_r() is called? Can
> > they not be in a struct created from malloc()?
> >
> 
> The problem is the board needs to load an FPGA configuration from FAT
> before SDRAM is available. Yes, this is loaded into SRAM of course, but the
> whole code until that is done uses so many malloc/free iterations that The
> simple mall of implementation would require too much memory.
> 
> And it's the full malloc state variables only that use BSS, not the FAT
> code.

I've actually faced very similar issues working on our TI AM654x "System
Firmware Loader" implementation (will post upstream soon), where I need
to load this firmware and other files from media such as MMC/FAT in a very
memory-constrained SPL pre-relocation environment *before* I can bring up
DDR.

Initially, I modified the fat.c driver to re-use memory so it is not as
wasteful during SYS_MALLOC_SIMPLE. While I'm not proud of this solution [1]
this allowed us to get going, allowing to load multiple files without
issues in pre-relocation SPL.

In the quest of creating something more upstream-friendly I had then
switched to using full malloc in pre-relocation SPL so that I didn't
have to hack the FAT driver, encountering similar issues like you
brought up and got this working, but ultimately abandoned this
approach after bundling all files needed to get loaded into a single
image tree blob which no longer required any of those solutions.

What remained till today however is a need to preserve specific BSS
state from pre-relocation SPL over to post-relocation SPL environment,
namely flags set to avoid the (expensive) re-probing of peripheral
drivers by the SPL loader. For that I introduced a Kconfig option that
allows skipping the automatic clearing of BSS during relocation [2].

Seeing this very related discussion here got me thinking about how else
I can carry over this "state" from pre- to post relocation but that's
probably a discussion to be had once I post my "System Firmware Loader
Series", probably next week.

PS: If you want to save a ton of memory during FAT loading you can
try something like CONFIG_FS_FAT_MAX_CLUSTSIZE=16384, I argue the
default is overkill for all practical scenarios.

--
Andreas Dannenberg
Texas Instruments Inc

[1] http://git.ti.com/gitweb/?p=ti-u-boot/ti-u-boot.git;a=commitdiff;h=3de26c27d232425e6a3b337dc402d73fe22ea88c
[2] http://git.ti.com/gitweb/?p=ti-u-boot/ti-u-boot.git;a=commitdiff;h=9ab4a405c98e966a59c7c3005c08cb95ed87eea8

>
> One way out could be to move the full mall of state variables into 'gd'...
> 
> Another way would be to continue into board_init_f without SDRAM enabled
> and in it it later...
> 
> Regards,
> Simon
> 
> 
> > If this is a limitation of FAT, then I think we should fix that, instead.
> >
> > Regards,
> > Simon
> >
> > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> > > ---
> > >
> > > Changes in v3:
> > > - improve commit message to show why CONFIG_CLEAR_BSS_F is needed
> > >
> > > Changes in v2:
> > > - make CONFIG_SPL_CLEAR_BSS_F depend on ARM for now
> > >
> > >  common/spl/Kconfig | 12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> > >
> > > 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
> > > --
> > > 2.17.1
> > >
> >
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot