board_f: save "malloc_base" from zeroing in case of CONFIG_SYS_MALLOC_F_LEN

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".

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.

Signed-off-by Alexey Brodkin <[email protected]>

Cc Wolfgang Denk <[email protected]>

Cc Simon Glass <[email protected]>

Cc Tom Rini <[email protected]>

Cc Masahiro Yamada <[email protected]>

---
 common/board_f.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

common/board_f.c +16 -1

index 3a4b32c..ebdba0e 100644
@@ -999,6 +999,9 @@ static init_fnc_t init_sequence_f[] = {
 void board_init_f(ulong boot_flags)
 {
 #ifdef CONFIG_SYS_GENERIC_GLOBAL_DATA
+#ifdef CONFIG_SYS_MALLOC_F_LEN
+       int malloc_base;
+#endif
        /*
         * For some archtectures, global data is initialized and used before
         * calling this function. The data should be preserved. For others,

@@ -1009,12 +1012,25 @@ void board_init_f(ulong boot_flags)
 
        gd = &data;
 
+#ifdef CONFIG_SYS_MALLOC_F_LEN
+       /*
+        * "malloc_base" is supposed to be set in the very beginning of start-up
+        * code (start.S or crt0.S), now we need to preserve it from zeroing.
+        */
+       malloc_base = gd->malloc_base;
+#endif
+
        /*
         * Clear global data before it is accessed at debug print
         * in initcall_run_list. Otherwise the debug print probably
         * get the wrong vaule of gd->have_console.
         */
        zero_global_data();
+
+#ifdef CONFIG_SYS_MALLOC_F_LEN
+       /* Restore "malloc_base" value */
+       gd->malloc_base = malloc_base;
+#endif
 #endif
 
        gd->flags = boot_flags;
--

2.1.0

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".

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.

Hello Alexey,

Quoted 2 lines · Alexey Brodkin

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"?

Quoted 7 lines

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).

Quoted 3 lines

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?

Hi Albert,

Quoted 8 lines · Albert ARIBAUD

Hello Alexey,

Quoted 2 lines · Alexey Brodkin

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"

Quoted 11 lines

Quoted 10 lines

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.

Quoted 3 lines

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.

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

-Alexey

Hi Albert,

Quoted 8 lines · Albert ARIBAUD

Hello Alexey,

Quoted 2 lines · Alexey Brodkin

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"

Quoted 7 lines

Quoted 7 lines

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).

Quoted 7 lines

Quoted 3 lines

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.