[U-Boot] [PATCH] Fix board init code to use a valid C runtime environment
Simon Glass
sjg at chromium.org
Tue Nov 10 21:04:50 CET 2015
Hi Albert,
On 9 November 2015 at 17:20, Albert ARIBAUD <albert.u.boot at aribaud.net> wrote:
> board_init_f_mem() alters the C runtime environment's
> stack it ls actually already using. This is not a valid
> C runtime environment and may conflict with the C compiler's
> expectations.
>
> Split board_init_f_mem into C functions which do not
> alter their own stack and therefore function in a valid C
> runtime environment.
>
> NOTE: this has not been tested with all architectures.
>
> Signed-off-by: Albert ARIBAUD <albert.u.boot at aribaud.net>
> ---
> arch/arc/lib/start.S | 20 +++++++++++++---
> arch/arm/lib/crt0.S | 10 ++++++--
> arch/arm/lib/crt0_64.S | 12 +++++++---
> arch/microblaze/cpu/start.S | 4 ++--
> arch/nios2/cpu/start.S | 17 ++++++++++++--
> arch/powerpc/cpu/ppc4xx/start.S | 18 ++++++++++----
> arch/x86/cpu/start.S | 10 ++++++--
> arch/x86/lib/fsp/fsp_common.c | 2 +-
> common/init/board_init.c | 33 +++++++++++++++-----------
> include/common.h | 52 +++++++++++++++++++++++++----------------
> 10 files changed, 126 insertions(+), 52 deletions(-)
>
> diff --git a/arch/arc/lib/start.S b/arch/arc/lib/start.S
> index 26a5934..d24d60b 100644
> --- a/arch/arc/lib/start.S
> +++ b/arch/arc/lib/start.S
> @@ -54,14 +54,28 @@ ENTRY(_start)
> mov %sp, CONFIG_SYS_INIT_SP_ADDR
> mov %fp, %sp
>
> - /* Allocate and zero GD, update SP */
> + /* Allocate GD, update SP */
> mov %r0, %sp
> - bl board_init_f_mem
> + bl board_init_f_gd_size
> + /* Update stack- and frame-pointers */
> + sub %sp, %sp, %r0
> + mov %fp, %sp
> +
> + /* Zero GD */
> + mov %r0, %sp
> + bl board_init_f_gd
>
> + /* Allocate malloc, update SP */
> + mov %r0, %sp
> + bl board_init_f_malloc_size
> /* Update stack- and frame-pointers */
> - mov %sp, %r0
> + sub %sp, %sp, %r0
> mov %fp, %sp
>
> + /* update GD with malloc base */
> + mov %r0, %sp
> + bl board_init_f_malloc
> +
I'm hoping we don't have to bring back this assembler. Do we really
need to put everything in separate functions?
Perhaps you could add a bit more detail in the commit message as to
what problem this solves?
> /* Zero the one and only argument of "board_init_f" */
> mov_s %r0, 0
> j board_init_f
> diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
> index 80548eb..26abab7 100644
> --- a/arch/arm/lib/crt0.S
> +++ b/arch/arm/lib/crt0.S
> @@ -82,9 +82,15 @@ ENTRY(_main)
> #else
> bic sp, sp, #7 /* 8-byte alignment for ABI compliance */
> #endif
> + bl board_init_f_gd_size
> + sub sp, r0
> mov r0, sp
> - bl board_init_f_mem
> - mov sp, r0
> + mov r9, r0
> + bl board_init_f_gd
> + bl board_init_f_malloc_size
> + sub sp, r0
> + mov r0, sp
> + bl board_init_f_malloc
>
> mov r0, #0
> bl board_init_f
> diff --git a/arch/arm/lib/crt0_64.S b/arch/arm/lib/crt0_64.S
> index cef1c71..e716889 100644
> --- a/arch/arm/lib/crt0_64.S
> +++ b/arch/arm/lib/crt0_64.S
> @@ -75,9 +75,15 @@ ENTRY(_main)
> ldr x0, =(CONFIG_SYS_INIT_SP_ADDR)
> #endif
> bic sp, x0, #0xf /* 16-byte alignment for ABI compliance */
> - bl board_init_f_mem
> - mov sp, x0
> -
> + bl board_init_f_gd_size
> + sub sp, x0
> + mov x0, sp
> + bl board_init_f_gd
> + bl board_init_f_malloc_size
> + sub sp, x0
> + mov x0, sp
> + bl board_init_f_malloc
> +
> mov x0, #0
> bl board_init_f
>
> diff --git a/arch/microblaze/cpu/start.S b/arch/microblaze/cpu/start.S
> index 14f46a8..206be3e 100644
> --- a/arch/microblaze/cpu/start.S
> +++ b/arch/microblaze/cpu/start.S
> @@ -25,7 +25,7 @@ _start:
>
> addi r8, r0, __end
> mts rslr, r8
> - /* TODO: Redo this code to call board_init_f_mem() */
> + /* TODO: Redo this code to call board_init_f_*() */
> #if defined(CONFIG_SPL_BUILD)
> addi r1, r0, CONFIG_SPL_STACK_ADDR
> mts rshr, r1
> @@ -142,7 +142,7 @@ _start:
> ori r12, r12, 0x1a0
> mts rmsr, r12
>
> - /* TODO: Redo this code to call board_init_f_mem() */
> + /* TODO: Redo this code to call board_init_f_*() */
> clear_bss:
> /* clear BSS segments */
> addi r5, r0, __bss_start
> diff --git a/arch/nios2/cpu/start.S b/arch/nios2/cpu/start.S
> index 54787c5..4d5f0b0 100644
> --- a/arch/nios2/cpu/start.S
> +++ b/arch/nios2/cpu/start.S
> @@ -107,9 +107,22 @@ _reloc:
> mov fp, sp
>
> /* Allocate and zero GD, update SP */
> + movhi r2, %hi(board_init_f_gd_size at h)
> + ori r2, r2, %lo(board_init_gd_size at h)
> + callr r2
> + sub sp, sp, r4
> + mov r4, sp
> + movhi r2, %hi(board_init_f_gd at h)
> + ori r2, r2, %lo(board_init_f_gd at h)
> + callr r2
> + /* Allocate malloc arena, update SP */
> + movhi r2, %hi(board_init_f_malloc_size at h)
> + ori r2, r2, %lo(board_init_malloc_size at h)
> + callr r2
> + sub sp, sp, r4
> mov r4, sp
> - movhi r2, %hi(board_init_f_mem at h)
> - ori r2, r2, %lo(board_init_f_mem at h)
> + movhi r2, %hi(board_init_f_malloc at h)
> + ori r2, r2, %lo(board_init_f_malloc at h)
> callr r2
>
> /* Update stack- and frame-pointers */
> diff --git a/arch/powerpc/cpu/ppc4xx/start.S b/arch/powerpc/cpu/ppc4xx/start.S
> index 77d4040..c827c95 100644
> --- a/arch/powerpc/cpu/ppc4xx/start.S
> +++ b/arch/powerpc/cpu/ppc4xx/start.S
> @@ -761,9 +761,14 @@ _start:
>
> bl cpu_init_f /* run low-level CPU init code (from Flash) */
> #ifdef CONFIG_SYS_GENERIC_BOARD
> + bl board_init_f_gd_size
> + sub r1, r1, r3
> mr r3, r1
> - bl board_init_f_mem
> - mr r1, r3
> + bl board_init_f_gd
> + bl board_init_f_malloc_size
> + sub r1, r1, r3
> + mr r3, r1
> + bl board_init_f_malloc
> li r0,0
> stwu r0, -4(r1)
> stwu r0, -4(r1)
> @@ -1037,9 +1042,14 @@ _start:
>
> bl cpu_init_f /* run low-level CPU init code (from Flash) */
> #ifdef CONFIG_SYS_GENERIC_BOARD
> + bl board_init_f_gd_size
> + sub r1, r1, r3
> + mr r3, r1
> + bl board_init_f_gd
> + bl board_init_f_malloc_size
> + sub r1, r1, r3
> mr r3, r1
> - bl board_init_f_mem
> - mr r1, r3
> + bl board_init_f_malloc
> stwu r0, -4(r1)
> stwu r0, -4(r1)
> #endif
> diff --git a/arch/x86/cpu/start.S b/arch/x86/cpu/start.S
> index 5b4ee79..25ac286 100644
> --- a/arch/x86/cpu/start.S
> +++ b/arch/x86/cpu/start.S
> @@ -122,9 +122,15 @@ car_init_ret:
> */
> #endif
> /* Set up global data */
> + call board_init_f_gd_size
> + subl %esp, %eax
> mov %esp, %eax
> - call board_init_f_mem
> - mov %eax, %esp
> + call board_init_f_gd
> + /* Set up malloc arena */
> + call board_init_f_malloc_size
> + subl %esp, %eax
> + mov %esp, %eax
> + call board_init_f_malloc
>
> #ifdef CONFIG_DEBUG_UART
> call debug_uart_init
> diff --git a/arch/x86/lib/fsp/fsp_common.c b/arch/x86/lib/fsp/fsp_common.c
> index c78df94..cd7fd86 100644
> --- a/arch/x86/lib/fsp/fsp_common.c
> +++ b/arch/x86/lib/fsp/fsp_common.c
> @@ -95,7 +95,7 @@ int x86_fsp_init(void)
> /*
> * The second time we enter here, adjust the size of malloc()
> * pool before relocation. Given gd->malloc_base was adjusted
> - * after the call to board_init_f_mem() in arch/x86/cpu/start.S,
> + * after the call to board_init_f_malloc() in arch/x86/cpu/start.S,
> * we should fix up gd->malloc_limit here.
> */
> gd->malloc_limit += CONFIG_FSP_SYS_MALLOC_F_LEN;
> diff --git a/common/init/board_init.c b/common/init/board_init.c
> index e74b63b..1313138 100644
> --- a/common/init/board_init.c
> +++ b/common/init/board_init.c
> @@ -29,32 +29,39 @@ __weak void arch_setup_gd(struct global_data *gd_ptr)
> }
> #endif /* !CONFIG_X86 */
>
> -ulong board_init_f_mem(ulong top)
> +ulong board_init_f_gd_size(void)
> +{
> + return roundup(sizeof(struct global_data), 16);
> +}
> +
> +void board_init_f_gd(struct global_data *gd_ptr)
> {
> - struct global_data *gd_ptr;
> #ifndef _USE_MEMCPY
> int *ptr;
> #endif
>
> - /* Leave space for the stack we are running with now */
> - top -= 0x40;
> -
> - top -= sizeof(struct global_data);
> - top = ALIGN(top, 16);
> - gd_ptr = (struct global_data *)top;
> #ifdef _USE_MEMCPY
> memset(gd_ptr, '\0', sizeof(*gd));
> #else
> for (ptr = (int *)gd_ptr; ptr < (int *)(gd_ptr + 1); )
> *ptr++ = 0;
> #endif
> - arch_setup_gd(gd_ptr);
> +}
>
> +ulong board_init_f_malloc_size(void)
> +{
> #if defined(CONFIG_SYS_MALLOC_F) && \
> - (!defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SYS_SPL_MALLOC_START))
> - top -= CONFIG_SYS_MALLOC_F_LEN;
> - gd->malloc_base = top;
> + (!defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SYS_SPL_MALLOC_START))
> + return CONFIG_SYS_MALLOC_F_LEN;
> +#else
> + return 0;
> #endif
> +}
>
> - return top;
> +void board_init_f_malloc(ulong malloc_base)
> +{
> +#if defined(CONFIG_SYS_MALLOC_F) && \
> + (!defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SYS_SPL_MALLOC_START))
> + gd->malloc_base = malloc_base;
> +#endif
> }
> diff --git a/include/common.h b/include/common.h
> index 09a131d..dbc2808 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -225,32 +225,44 @@ void board_init_f(ulong);
> void board_init_r(gd_t *, ulong) __attribute__ ((noreturn));
>
> /**
> - * board_init_f_mem() - Allocate global data and set stack position
> + * board_init_f_gd_size() - Return the size of the global data
> *
> * This function is called by each architecture very early in the start-up
> - * code to set up the environment for board_init_f(). It allocates space for
> - * global_data (see include/asm-generic/global_data.h) and places the stack
> - * below this.
> + * code to allow the C runtime to reserve space for the GD on the stack.
> + *
> + * @return: GD size
> + */
> +ulong board_init_f_gd_size(void);
> +
> +/**
> + * board_init_f_gd() - Initialize (zero) the global data
> *
> - * This function requires a stack[1] Normally this is at @top. The function
> - * starts allocating space from 64 bytes below @top. First it creates space
> - * for global_data. Then it calls arch_setup_gd() which sets gd to point to
> - * the global_data space and can reserve additional bytes of space if
> - * required). Finally it allocates early malloc() memory
> - * (CONFIG_SYS_MALLOC_F_LEN). The new top of the stack is just below this,
> - * and it returned by this function.
> + * This function is called once the C runtime has allocated the GD on
> + * the stack. It must initialize the GD.
> + *
> + * @gd_ptr: the pointer to the GD to initialize
> + */
> +void board_init_f_gd(struct global_data *gd_ptr);
> +
> +/**
> + * board_init_f_malloc_size() - Return the size of the malloc arena
> *
> - * [1] Strictly speaking it would be possible to implement this function
> - * in C on many archs such that it does not require a stack. However this
> - * does not seem hugely important as only 64 byte are wasted. The 64 bytes
> - * are used to handle the calling standard which generally requires pushing
> - * addresses or registers onto the stack. We should be able to get away with
> - * less if this becomes important.
> + * This function is called once the GD is initialized, to allow the C
> + * runtime to allocate the malloc arena on the stack.
> + *
> + * @return: Malloc arena size
> + */
> +ulong board_init_f_malloc_size(void);
> +
> +/**
> + * board_init_f_malloc() - Mark the malloc arena base in the global data
> *
> - * @top: Top of available memory, also normally the top of the stack
> - * @return: New stack location
> + * This function is called once the malloc arena is allocated on the
> + * stack. It marks the malloc arena base in the global data.
> + *
> + * @malloc_base: the base of the malloc arena
> */
> -ulong board_init_f_mem(ulong top);
> +ulong board_init_f_malloc(ulong malloc_base);
>
> /**
> * arch_setup_gd() - Set up the global_data pointer
> --
> 2.5.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Regards,
Simon
More information about the U-Boot
mailing list