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

[Simon Goldschmidt]

On Mon, Apr 1, 2019 at 8:07 AM Heiko Schocher <hs at denx.de> wrote:
>
> Hello Simon,
>
> Am 30.03.2019 um 23:37 schrieb Simon Glass:
> > Hi Simon,
> >
> > On Sat, 30 Mar 2019 at 15:40, Simon Goldschmidt
> > <simon.k.r.goldschmidt at gmail.com> wrote:
> >>
> >>
> >>
> >> Simon Glass <sjg at chromium.org> schrieb am Sa., 30. März 2019, 22:18:
> >>>
> >>> Hi Simon,
> >>>
> >>> On Sat, 30 Mar 2019 at 14:50, Simon Goldschmidt
> >>> <simon.k.r.goldschmidt at gmail.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com> schrieb am Sa., 30. März 2019, 21:18:
> >>>>>
> >>>>>
> >>>>>
> >>>>> 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.
> >>>>>
> >>>>> One way out could be to move the full mall of state variables into 'gd'...
> >>>>
> >>>>
> >>>> Maybe I should point out again that the whole purpose of this series is to have an SPL that uses full malloc right from the start. This is currently not possible as full malloc needs BSS.
> >>>
> >>> Right, and our assumption/design is that full malloc() requires SRAM.
> >>>
> >>> Here we have an architecture that requires FAT just to init its SDRAM.
> >>> FAT requires malloc() and free() and there is not enough SRAM to skip
> >>> the free() calls. So we have to use full malloc() and that uses BSS.
> >>> But BSS is not available before board_init_r(). But we need to init
> >>> SDRAM before board_init_r().
> >>>
> >>> Firstly I'd point out that someone should speak to the chip designers.
> >>> Did anyone try to boot U-Boot on the SoC model?
> >>
> >>
> >> Well, it's a U-Boot thing to load it from FAT. It could probably be loaded from RAW mmc without problems, so I don't know if it's a chip designers issue. I think it's an issue that we need to fix in U-Boot: we have a good full malloc implementation but it's not usable in all cases were it should be.
> >
> > OK then why use FAT? I assumed it was a boot-ROM requirement. How does
> > the boot ROM load SPL?
> >
> >>
> >>>
> >>> I think it is possible to change dlmalloc to put its variables in a
> >>> struct. Then I suppose the struct pointer could be kept in gd. That
> >>> would be one solution. Then full malloc() could be inited in
> >>> spl_common_init() perhaps.
> >>
> >>
> >> That might be worth a try.
> >
> > Yes shouldn't be too painful. I suspect it would be neutral on code size, too.
>
> Sorry, for digging in so late, here. I just stumbeled over the same
> problem ... but we cannot use BSS before relocation on all platforms,
> so I think, this patch is no option.

Right, by now I know that folks want to keep it that way.

>
> Hmm... I did not take a deep look at the dlmalloc implementation, but
> may my "fix" for the problem with Stefans patch, see here:
>
> http://patchwork.ozlabs.org/patch/1065508/#2130443
>
> is may an option for dlmalloc?

Moving variables from 'bss' into 'data' unoverridable by adding the
'section' attribute?
No, I think that makes it even worse. Why would 'data' always be available when
'bss' is not?

>
> If not, I think, the option suggested from Simon here is the way to
> go ...

I'm not too convinced of that either. I'll take the time to re-think
this specific
problem (using full-malloc in SPL without explicitly defining an address range)
and see what other solutions I can think of.

Regards,
Simon