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

Simon Glass
2015-01-19T19:21:23Z

Hi Alexey,

On 19 January 2015 at 10:55, Alexey Brodkin <Alexey.Brodkin@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".
>
> 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 <abrodkin@synopsys.com>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tom Rini <trini@ti.com>
> Cc: Masahiro Yamada <yamada.m@jp.panasonic.com>
> ---
> common/board_f.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/common/board_f.c b/common/board_f.c
> index 3a4b32c..ebdba0e 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -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
>

CONFIG_SYS_GENERIC_GLOBAL_DATA seems to be mis-named, but in any case
it must not be used with CONFIG_SYS_MALLOC_F_LEN. We are trying to get
rid of all these cases of multiple global_data structures. It should
be set up in start.S and not anywhere else.

Better would be:

#ifdef CONFIG_SYS_GENERIC_GLOBAL_DATA
+#ifdef CONFIG_SYS_MALLOC_F_LEN
+#error "CONFIG_SYS_GENERIC_GLOBAL_DATA is deprecated - please remove
use of this if you want to use early malloc()
+#endif

Regards,
Simon