[U-Boot] [PATCH v3] Fix board init code to use a valid C runtime environment

Albert ARIBAUD albert.u.boot at aribaud.net
Fri Nov 13 15:43:23 CET 2015


board_init_f_mem() alters the C runtime environment's
stack it ls actually already using. This is not a valid
C runtime environment.

Split board_init_f_mem into C functions which do not
alter their own stack and therefore run in a valid C
runtime environment.

Signed-off-by: Albert ARIBAUD <albert.u.boot at aribaud.net>
---
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 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            | 12 +++++---
 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 | 14 ++++++---
 arch/x86/cpu/start.S            |  7 +++--
 arch/x86/lib/fsp/fsp_common.c   |  4 +--
 common/init/board_init.c        | 67 +++++++++++++++++++++++++++++++++--------
 include/common.h                | 33 ++++++++------------
 10 files changed, 108 insertions(+), 58 deletions(-)

diff --git a/arch/arc/lib/start.S b/arch/arc/lib/start.S
index 26a5934..2a99193 100644
--- a/arch/arc/lib/start.S
+++ b/arch/arc/lib/start.S
@@ -54,14 +54,16 @@ ENTRY(_start)
 	mov	%sp, CONFIG_SYS_INIT_SP_ADDR
 	mov	%fp, %sp
 
-	/* Allocate and zero GD, update SP */
-	mov	%r0, %sp
-	bl	board_init_f_mem
-
+	/* Get reserved area size, update SP and FP */
+	bl	board_init_f_get_reserve_size
 	/* Update stack- and frame-pointers */
-	mov	%sp, %r0
+	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..d0f8db8 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_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..c95f1f4 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_reserve_size at h)
+	ori	r2, r2, %lo(board_init_f_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..cc6e4ec 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_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,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
 	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..b30383f 100644
--- a/arch/x86/cpu/start.S
+++ b/arch/x86/cpu/start.S
@@ -121,10 +121,11 @@ car_init_ret:
 	 * with esi holding the HOB list address returned by the FSP.
 	 */
 #endif
-	/* Set up global data */
+	/* Reserve 'globals' on the stack */
+	call	board_init_f_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..5249455 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_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 1c6126d..e033253 100644
--- a/common/init/board_init.c
+++ b/common/init/board_init.c
@@ -21,39 +21,80 @@ 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_SYS_THUMB_BUILD)
 __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:
+ *
+ * Reservation cannot be actually done from within a C function
+ * as it implies modifying the C stack.
+ *
+ */
+
+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).
+ *
+ * Do not use 'gd->' until arch_setup_gd() has been called!
+ */
+
+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
+	 */
 
-	top -= sizeof(struct global_data);
-	top = ALIGN(top, 16);
-	gd_ptr = (struct global_data *)top;
+	gd_ptr = (struct global_data *)base;
+	/* go down one GD plus 16-byte alignment */
+	base += roundup(sizeof(struct global_data), 16);
+	/* 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_SYS_THUMB_BUILD)
 	arch_setup_gd(gd_ptr);
+#endif
+
+	/*
+	 * 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;
+	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