Fix board init code to use a valid C runtime environment

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.

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 <[email protected]>

---
For NIOS2, this patch hopefully contains all manual fixes by Thomas.

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            | 20 +++++++++++++---
  arch/arm/lib/crt0.S             | 10 ++++++--
  arch/arm/lib/crt0_64.S          | 10 ++++++--
  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   |  4 ++--
  common/init/board_init.c        | 31 ++++++++++++++----------
  include/common.h                | 52 +++++++++++++++++++++++++----------------
  10 files changed, 125 insertions(+), 51 deletions(-)

Hi Albert,

Quoted 19 lines · Albert ARIBAUD

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 <[email protected]>

---
For NIOS2, this patch hopefully contains all manual fixes by Thomas.

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

Quoted 12 lines

  arch/arc/lib/start.S            | 20 +++++++++++++---
  arch/arm/lib/crt0.S             | 10 ++++++--
  arch/arm/lib/crt0_64.S          | 10 ++++++--
  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   |  4 ++--
  common/init/board_init.c        | 31 ++++++++++++++----------
  include/common.h                | 52 +++++++++++++++++++++++++----------------
  10 files changed, 125 insertions(+), 51 deletions(-)

Additional fixes,
------------------------------------------------------------------------

common/init/board_init.c +0 -0

index 8839a4a..703e6d8 100644

common/init/board_init.c +0 -1

  }
 
  ulong board_init_f_malloc_size(void)
--------------------------------------------------------------------------

arch/nios2/cpu/start.S +0 -0

index c163ce1..0adff46 100644

arch/nios2/cpu/start.S +0 -1

  	/* Call board_init_f -- never returns */
  	mov	r4, r0
  	movhi	r2, %hi(board_init_f at h)
----------------------------------------------------------------------------

Otherwise,

Tested-by Thomas Chou <[email protected]>

Acked-by Thomas Chou <[email protected]>

Thanks.

Best regards,
Thomas

Quoted 6 lines

------------------------------------------------------------------------

common/init/board_init.c +0 -0

index 8839a4a..703e6d8 100644
@@ -46,6 +46,7 @@ void board_init_f_gd(struct global_data *gd_ptr)

common/init/board_init.c void board_init_f_gd(struct global_data *gd_ptr) · line 49

Quoted 4 lines

common/init/board_init.c +1 -0

   	for (ptr = (int *)gd_ptr; ptr < (int *)(gd_ptr + 1); )
   		*ptr++ = 0;
   #endif
+	arch_setup_gd(gd_ptr);

Correct -- in ARM (Thumb-1 at least) we cannot use arch_setup_gd() so we set GD (in r9) from within arch/arm/lib/crt0.S, but for NIOS2 it might (and apparently does) work. Where is GD stored in NIOS2?

Hi Albert,

Quoted 6 lines · Albert ARIBAUD

Quoted 6 lines

------------------------------------------------------------------------

common/init/board_init.c +0 -0

index 8839a4a..703e6d8 100644
@@ -46,6 +46,7 @@ void board_init_f_gd(struct global_data *gd_ptr)

Quoted 9 lines

common/init/board_init.c void board_init_f_gd(struct global_data *gd_ptr) · line 49

Quoted 4 lines

common/init/board_init.c +1 -0

   	for (ptr = (int *)gd_ptr; ptr < (int *)(gd_ptr + 1); )
   		*ptr++ = 0;
   #endif
+	arch_setup_gd(gd_ptr);

Correct -- in ARM (Thumb-1 at least) we cannot use arch_setup_gd() so we set GD (in r9) from within arch/arm/lib/crt0.S, but for NIOS2 it might (and apparently does) work. Where is GD stored in NIOS2?

It is a register, r26 "gp".

I have another question. Will it be simpler to have two calls instead of four?

1. get size of gd plus malloc.

2. init gd and malloc.

Best regards,
Thomas

Best regards,
Thomas

Hello Thomas,

arch/nios2/cpu/start.S _reloc: · line 130

Quoted 5 lines · Thomas Chou

Hi Albert,

arch/nios2/cpu/start.S +0 -3

-	/* Update stack- and frame-pointers */
-	mov	sp, r2
-	mov	fp, sp

Just a sec here on second thought.

I understand the 'mov sp, r2' must be removed as it is replaced by a 'sub sp, sp, r2' a few lines earlier.

But doesn't the 'mov fp, sp' need to remain? Without it, fp is not set any more, is it?

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 <[email protected]>

---

For NIOS2, this patch hopefully contains all manual fixes by Thomas.

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            | 20 +++++++++++++---
 arch/arm/lib/crt0.S             | 10 ++++++--
 arch/arm/lib/crt0_64.S          | 10 ++++++--
 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   |  4 ++--
 common/init/board_init.c        | 31 ++++++++++++++----------
 include/common.h                | 52 +++++++++++++++++++++++++----------------
 10 files changed, 125 insertions(+), 51 deletions(-)

arch/x86/cpu/start.S +2 -0

+       mov     %esp, %eax
+       call    board_init_f_malloc
 
 #ifdef CONFIG_DEBUG_UART
        call    debug_uart_init

arch/x86/lib/fsp/fsp_common.c +2 -2

index c78df94..5249455 100644
@@ -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;
        }

common/init/board_init.c +6 -5

index e74b63b..8839a4a 100644
@@ -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;
-

common/init/board_init.c +13 -3

+}
 
+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;
+       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
 }

include/common.h +30 -17

index 09a131d..a2d81d9 100644
@@ -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.
  *
- * 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:    GD size
+ */
+ulong board_init_f_gd_size(void);
+
+/**
+ * board_init_f_gd() - Initialize (zero) the global data
+ *
+ * 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
+ *
+ * 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
  *
- * [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 malloc arena is allocated on the
+ * stack. It marks the malloc arena base in the global data.
  *

Hi Albert,

Quoted 1 line

+ subl %esp, %eax

I guess you are confused by the AT&T assembly syntax. This should be:
subl %eax, %esp.

Quoted 1 line

+ subl %esp, %eax

Ditto.

Quoted 1 line

- arch_setup_gd(gd_ptr);

This arch_setup_gd(gd_ptr) should not be deleted.

Regards,
Bin