[U-Boot] [PATCH v5] Fix board init code to respect the C runtime environment
Albert ARIBAUD
albert.u.boot at aribaud.net
Mon Nov 16 17:22:07 CET 2015
board_init_f_mem() alters the C runtime environment's
stack it is actually already using. This is not a valid
behaviour within a C runtime environment.
Split board_init_f_mem into C functions which do not alter
their own stack and always behave properly with respect to
their C runtime environment.
Signed-off-by: Albert ARIBAUD <albert.u.boot at aribaud.net>
---
Cc:ing custodians for all architectures which this
patch affects.
This patch has been build-tested for all there
architectures, and run-tested on ARM OpenRD Client.
For NIOS2, this patch contains all manual v1 and v2
fixes by Thomas.
For x86, this patch contains all manual v2 fixes by Bin.
Changes in v5:
- Reword commit message (including summary/subject)
- Reword comments in ARC code
Changes in v4:
- Add comments on reserving GD at the lowest location
- Add comments on post-incrementing base for each "chunk"
Changes in v3:
- Rebase after commit 9ac4fc82
- Simplify malloc conditional as per commit 9ac4fc82
- Fix NIOS2 return value register (r2, not r4)
- Fix x86 subl argument inversion
- Group allocations in a single function
- Group initializations in a single function
Changes in v2:
- Fix all checkpatch issues
- Fix board_init_f_malloc prototype mismatch
- Fix board_init_[f_]xxx typo in NIOS2
- Fix aarch64 asm 'sub' syntax error
arch/arc/lib/start.S | 16 +++---
arch/arm/lib/crt0.S | 6 ++-
arch/arm/lib/crt0_64.S | 6 ++-
arch/microblaze/cpu/start.S | 4 +-
arch/nios2/cpu/start.S | 13 +++--
arch/powerpc/cpu/ppc4xx/start.S | 10 ++--
arch/x86/cpu/start.S | 5 +-
arch/x86/lib/fsp/fsp_common.c | 4 +-
common/init/board_init.c | 108 +++++++++++++++++++++++++++++++++++-----
include/common.h | 33 +++++-------
10 files changed, 146 insertions(+), 59 deletions(-)
diff --git a/arch/arc/lib/start.S b/arch/arc/lib/start.S
index 26a5934..fddd536 100644
--- a/arch/arc/lib/start.S
+++ b/arch/arc/lib/start.S
@@ -50,18 +50,20 @@ ENTRY(_start)
1:
#endif
- /* Setup stack- and frame-pointers */
+ /* Establish C runtime stack and frame */
mov %sp, CONFIG_SYS_INIT_SP_ADDR
mov %fp, %sp
- /* Allocate and zero GD, update SP */
- mov %r0, %sp
- bl board_init_f_mem
-
- /* Update stack- and frame-pointers */
- mov %sp, %r0
+ /* Get reserved area size */
+ bl board_init_f_get_reserve_size
+ /* Allocate reserved size on stack, adjust frame pointer accordingly */
+ sub %sp, %sp, %r0
mov %fp, %sp
+ /* Initialize reserved area */
+ mov %r0, %sp
+ bl board_init_f_init_reserve
+
/* 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..4ec89a4 100644
--- a/arch/arm/lib/crt0.S
+++ b/arch/arm/lib/crt0.S
@@ -82,9 +82,11 @@ ENTRY(_main)
#else
bic sp, sp, #7 /* 8-byte alignment for ABI compliance */
#endif
+ bl board_init_f_get_reserve_size
+ sub sp, sp, r0
+ mov r9, sp
mov r0, sp
- bl board_init_f_mem
- mov sp, r0
+ bl board_init_f_init_reserve
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..2891071 100644
--- a/arch/arm/lib/crt0_64.S
+++ b/arch/arm/lib/crt0_64.S
@@ -75,8 +75,10 @@ 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_get_reserve_size
+ sub sp, sp, x0
+ mov x0, sp
+ bl board_init_f_init_reserve
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..45bf0fd 100644
--- a/arch/nios2/cpu/start.S
+++ b/arch/nios2/cpu/start.S
@@ -106,14 +106,17 @@ _reloc:
stw r0, 4(sp)
mov fp, sp
- /* Allocate and zero GD, update SP */
+ /* Allocate and initialize reserved area, update SP */
+ movhi r2, %hi(board_init_f_get_reserve_size at h)
+ ori r2, r2, %lo(board_init_f_get_reserve_size at h)
+ callr r2
+ sub sp, sp, r2
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_init_reserve at h)
+ ori r2, r2, %lo(board_init_f_init_reserve at h)
callr r2
- /* Update stack- and frame-pointers */
- mov sp, r2
+ /* Update frame-pointer */
mov fp, sp
/* Call board_init_f -- never returns */
diff --git a/arch/powerpc/cpu/ppc4xx/start.S b/arch/powerpc/cpu/ppc4xx/start.S
index 77d4040..44222d2 100644
--- a/arch/powerpc/cpu/ppc4xx/start.S
+++ b/arch/powerpc/cpu/ppc4xx/start.S
@@ -761,9 +761,10 @@ _start:
bl cpu_init_f /* run low-level CPU init code (from Flash) */
#ifdef CONFIG_SYS_GENERIC_BOARD
+ bl board_init_f_get_reserve_size
+ sub r1, r1, r3
mr r3, r1
- bl board_init_f_mem
- mr r1, r3
+ bl board_init_f_init_reserve
li r0,0
stwu r0, -4(r1)
stwu r0, -4(r1)
@@ -1037,9 +1038,10 @@ _start:
bl cpu_init_f /* run low-level CPU init code (from Flash) */
#ifdef CONFIG_SYS_GENERIC_BOARD
+ bl board_init_f_get_reserve_size
+ sub r1, r1, r3
mr r3, r1
- bl board_init_f_mem
- mr r1, r3
+ bl board_init_f_init_reserve
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..932672c 100644
--- a/arch/x86/cpu/start.S
+++ b/arch/x86/cpu/start.S
@@ -122,9 +122,10 @@ car_init_ret:
*/
#endif
/* Set up global data */
+ call board_init_f_get_reserve_size
+ subl %eax, %esp
mov %esp, %eax
- call board_init_f_mem
- mov %eax, %esp
+ call board_init_f_init_reserve
#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..72c458f 100644
--- a/arch/x86/lib/fsp/fsp_common.c
+++ b/arch/x86/lib/fsp/fsp_common.c
@@ -95,8 +95,8 @@ 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,
- * we should fix up gd->malloc_limit here.
+ * after the call to board_init_f_init_reserve() 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 1c6126d..db4d9a0 100644
--- a/common/init/board_init.c
+++ b/common/init/board_init.c
@@ -21,39 +21,121 @@ DECLARE_GLOBAL_DATA_PTR;
#define _USE_MEMCPY
#endif
-/* Unfortunately x86 can't compile this code as gd cannot be assigned */
-#ifndef CONFIG_X86
+/* Unfortunately x86 or ARM can't compile this code as gd cannot be assigned */
+#if !defined(CONFIG_X86) && !defined(CONFIG_ARM)
__weak void arch_setup_gd(struct global_data *gd_ptr)
{
gd = gd_ptr;
}
-#endif /* !CONFIG_X86 */
+#endif /* !CONFIG_X86 && !CONFIG_SYS_THUMB_BUILD */
-ulong board_init_f_mem(ulong top)
+/*
+ * Compute size of space to reserve on stack for use as 'globals'
+ * and return size request for reserved area.
+ *
+ * Notes:
+ *
+ * Actual reservation cannot be done from within this function as
+ * it requires altering the C stack pointer, so this will be done by
+ * the caller upon return from this function.
+ *
+ * IMPORTANT:
+ *
+ * The return value of this function has two uses:
+ * - it determines the amount of stack reserved for global data and
+ * the malloc arena;
+ * - it determines where the global data will be located, as on some
+ * architectures the caller will set gd to the base of the reserved
+ * location.
+ *
+ */
+
+ulong board_init_f_get_reserve_size(void)
+{
+ ulong size;
+
+ /* Reserve GD (rounded up to a multiple of 16 bytes) */
+ size = roundup(sizeof(struct global_data), 16);
+
+ /* Reserve malloc arena */
+#if defined(CONFIG_SYS_MALLOC_F)
+ size += CONFIG_SYS_MALLOC_F_LEN;
+#endif
+
+ return size;
+}
+
+/*
+ * Initialize reserved space (which has been safely allocated on the C
+ * stack from the C runtime environment handling code).
+ *
+ * Notes:
+ *
+ * Actual reservation was done by the caller; the locations from base
+ * to base+size-1 (where 'size' is the value returned by the allocation
+ * function above) can be accessed freely without risk of corrupting the
+ * C runtime environment.
+ *
+ * IMPORTANT:
+ *
+ * Upon return from the allocation function above, on some architectures
+ * the caller will set gd to the lowest reserved location. Therefore, in
+ * this initialization function, the global data MUST be placed at base.
+ *
+ * ALSO IMPORTANT:
+ *
+ * On some architectures, gd will only be set once arch_setup_gd() is
+ * called. Therefore:
+ *
+ * Do not use 'gd->' until arch_setup_gd() has been called!
+ *
+ * IMPORTANT TOO:
+ *
+ * Initialization for each "chunk" (GD, malloc arena...) ends with an
+ * incrementation line of the form 'base += <some size>'. The last of
+ * these incrementations seems useless, as base will not be used any
+ * more after this incrementation; but if/when a new "chunk" is appended,
+ * this increment will be essential as it will give base right value for
+ * this new chunk (which will have to end with its own incrementation
+ * statement). Besides, the compiler's optimizer will silently detect
+ * and remove the last base incrementation, therefore leaving that last
+ * (seemingly useless) incrementation causes no code increase.
+ */
+
+void board_init_f_init_reserve(ulong base)
{
struct global_data *gd_ptr;
#ifndef _USE_MEMCPY
int *ptr;
#endif
- /* Leave space for the stack we are running with now */
- top -= 0x40;
+ /*
+ * clear GD entirely and set it up
+ */
- top -= sizeof(struct global_data);
- top = ALIGN(top, 16);
- gd_ptr = (struct global_data *)top;
+ gd_ptr = (struct global_data *)base;
+ /* zero the area */
#ifdef _USE_MEMCPY
memset(gd_ptr, '\0', sizeof(*gd));
#else
for (ptr = (int *)gd_ptr; ptr < (int *)(gd_ptr + 1); )
*ptr++ = 0;
#endif
+ /* set GD unless architecture did it already */
+#if !defined(CONFIG_X86) && !defined(CONFIG_ARM)
arch_setup_gd(gd_ptr);
+#endif
+ /* next alloc will be higher by one GD plus 16-byte alignment */
+ base += roundup(sizeof(struct global_data), 16);
+
+ /*
+ * record malloc arena start
+ */
#if defined(CONFIG_SYS_MALLOC_F)
- top -= CONFIG_SYS_MALLOC_F_LEN;
- gd->malloc_base = top;
+ /* go down one malloc arena */
+ gd->malloc_base = base;
+ /* next alloc will be higher by one malloc arena size */
+ base += CONFIG_SYS_MALLOC_F_LEN;
#endif
-
- return top;
}
diff --git a/include/common.h b/include/common.h
index 09a131d..988d67a 100644
--- a/include/common.h
+++ b/include/common.h
@@ -225,32 +225,25 @@ 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
+ * ulong board_init_f_get_reserve_size - return size of reserved area
*
* 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 on the stack for writable
+ * 'globals' such as GD and the malloc arena.
*
- * 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.
+ * @return: size of reserved area
+ */
+ulong board_init_f_get_reserve_size(void);
+
+/**
+ * board_init_f_init_reserve - initialize the reserved area(s)
*
- * [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 C runtime has allocated the reserved
+ * area on the stack. It must initialize the GD at the base of that area.
*
- * @top: Top of available memory, also normally the top of the stack
- * @return: New stack location
+ * @base: top from which reservation was done
*/
-ulong board_init_f_mem(ulong top);
+void board_init_f_init_reserve(ulong base);
/**
* arch_setup_gd() - Set up the global_data pointer
--
2.5.0
More information about the U-Boot
mailing list