[U-Boot] [PATCH] board_f: save "malloc_base" from zeroing in case of CONFIG_SYS_MALLOC_F_LEN

[Albert ARIBAUD]

Hello Alexey,

On Tue, 20 Jan 2015 13:06:57 +0000, Alexey Brodkin
<Alexey.Brodkin at synopsys.com> wrote:
> Hi Albert,
> 
> On Tue, 2015-01-20 at 08:07 +0100, Albert ARIBAUD wrote:
> > Hello Alexey,
> > 
> > On Mon, 19 Jan 2015 20:55:03 +0300, Alexey Brodkin
> > <Alexey.Brodkin at synopsys.com> wrote:
> > > In case of CONFIG_SYS_MALLOC_F_LEN "malloc_base" is used for early
> > > start-up code and is set very early, typically in "start.S" or "crt1.S".
> > 
> > There is no "crt1.S" in U-Boot. Did you mean "crt0.S"?
> 
> Indeed I meant "crt0.S"
> 
> > > In current implementation in case of CONFIG_SYS_GENERIC_GLOBAL_DATA all
> > > global data gets zeroed on "board_init_f" entry. But by that time
> > > "malloc_base" could have been set already, which means it will be zeroed
> > > and subsequent C-code will be executed improperly (if executed at all -
> > > if there's no memory mapped to 0 or it is read-only then on some arches
> > > there will be an exception and others will quetly die).
> > > 
> > > To work-around described situation we just need to make sure
> > > "malloc_base" is saved prior zeroing global data and recovered
> > > afterwards.
> > 
> > Keeping data from being zeroed etc is usually done through GD. Could
> > malloc_base be placed there instead of creating a specific exemption
> > for it?
> 
> Unfortunately I didn't understand your suggestion here.
> "malloc_base" is already in global data structure.

My bad; as you had not mentioned GD (or I'd missed it), I thought you
were referring to a standalone variable. I should have grepped, which I
have done now.

> But the point is global data structure also requires zeroing sometime on
> early start-up. This is required to make sure we don't have any garbage
> in GD (for example left-overs from lower-level bootloader or previously
> executed kernel etc).
> 
> So other option is to zero GD earlier in start-up code. This is
> essentially doable but it will be done on per-architecture or even
> per-CPU basis in their "start.S" - which means we'll have duplication of
> the same functionality and maintenance will be difficult then.

Right now, yes, we'd have to duplicate this a bit, but we can minimize
that by doing the GD init in a single routine called from each start.S;
this would only add one "bl" line per start.S (and hopefully, it can be
added the same way in each start.S and we can still eventually merge
these start.S files together).

> Probably I just didn't get you point so then could you please clarify
> what did you mean.

No, I just hadn't completely done my homework.

> -Alexey

Amicalement,
-- 
Albert.