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

[Simon Glass]

Hi Alexey,

On 20 January 2015 at 06:06, 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.
>
> 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.

This should be done before board_init_f(). See for example this patch:

http://patchwork.ozlabs.org/patch/421210/

There is no need for it to be SOC- or even arch-specific. We can clean
this up fairly soon.

But we should not set up global_data in board_init_f(). This is a
hang-over from previous code. It needs to be removed. Perhaps in
addition to my comments above you could add a comment that the code at
the top of board_init_f() is deprecated and will soon be removed?

Regards,
Simon