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

[Simon Glass]

Hi Alexey,

On 19 January 2015 at 10:55, 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".
>
> 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 at synopsys.com>
> Cc: Wolfgang Denk <wd at denx.de>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Tom Rini <trini at ti.com>
> Cc: Masahiro Yamada <yamada.m at 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