[coreboot-gerrit] New patch to review for coreboot: 19dd02b Set armv7 up for cpu_info to work as on x86 (so threads can work)

Isaac Christensen (isaac.christensen@se-eng.com) gerrit at coreboot.org
Tue Aug 5 18:59:31 CEST 2014


Isaac Christensen (isaac.christensen at se-eng.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/6501

-gerrit

commit 19dd02bbdd176f0a9029a563fb369fde9d18ebaf
Author: Ronald G. Minnich <rminnich at gmail.com>
Date:   Fri Aug 16 15:57:56 2013 -0700

    Set armv7 up for cpu_info to work as on x86 (so threads can work)
    
    On x86, cpu_info lives at the top of stack. Make the arm do that as
    well, as the threading model needs that and so will multicore support.
    
    As part of this change, make the stack size a power of 2.
    Also make it much smaller -- 2048 bytes is PLENTY for ram stage.
    
    Note that the small stack size is counterintuitive for rom stage.  How
    can this work in rom stage, which needs a HUGE stack for lzma? The
    main use of STACK_SIZE has always been in ram stage; since 2002 or so
    it was to size per-core stacks (see, e.g.,
    
    src/arch/x86/lib/c_start.S:.space CONFIG_MAX_CPUS*CONFIG_STACK_SIZE
    
    and, more recently, thread stacks. So, we define the STACK_TOP for rom
    and ram stage, but the STACK_SIZE has no real effect on the ROM stage
    (no hardware red zones on the stack) and hence we're ok with actually
    defining the "wrong" stack size. In fact, the coreboot_ram ldscript
    for armv7 sizes the stack by subtracting CONFIG_STACK_BOTTOM from
    CONFIG_STACK_TOP, so we replicate that arithmetic in bootblock.inc
    
    Observed stack usage in ramstage:
    BS: BS_PAYLOAD_LOAD times (us): entry 1 run 153887 exit 1
    Jumping to boot code at 23104044
    CPU0: stack: 02072800 - 02073000, lowest used address 020728d4, stack used: 1836 bytes
    entry    = 23104044
    
    Which means we do need 2K, not 1K.
    
    Change-Id: I1a21db87081597efe463095bfd33c89eba1d569f
    Signed-off-by: Ronald G. Minnich <rminnich at gmail.com>
    Reviewed-on: https://gerrit.chromium.org/gerrit/66135
    Reviewed-by: Ronald G. Minnich <rminnich at chromium.org>
    Tested-by: Ronald G. Minnich <rminnich at chromium.org>
    Commit-Queue: Ronald G. Minnich <rminnich at chromium.org>
    (cherry picked from commit f011097e9f2bfb2f4c1109d465be89a79a65ba3e)
    Signed-off-by: Isaac Christensen <isaac.christensen at se-eng.com>
---
 src/arch/armv7/Makefile.inc        |  1 +
 src/arch/armv7/bootblock.inc       | 12 +++++++++--
 src/arch/armv7/cpu.c               | 42 ++++++++++++++++++++++++++++++++++++++
 src/arch/armv7/include/arch/cpu.h  |  1 +
 src/cpu/samsung/exynos5420/Kconfig | 22 ++++++++++++++++++--
 5 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/src/arch/armv7/Makefile.inc b/src/arch/armv7/Makefile.inc
index a611b09..2188706 100644
--- a/src/arch/armv7/Makefile.inc
+++ b/src/arch/armv7/Makefile.inc
@@ -170,6 +170,7 @@ ramstage-y += exception_asm.S
 ramstage-y += div0.c
 #ramstage-y += interrupts.c
 ramstage-y += cache.c
+ramstage-y += cpu.c
 ramstage-y += mmu.c
 ramstage-y += eabi_compat.c
 ramstage-y += boot.c
diff --git a/src/arch/armv7/bootblock.inc b/src/arch/armv7/bootblock.inc
index b2c993a..c45259d 100644
--- a/src/arch/armv7/bootblock.inc
+++ b/src/arch/armv7/bootblock.inc
@@ -82,8 +82,15 @@ init_stack_loop:
 /* Set stackpointer in internal RAM to call board_init_f */
 call_bootblock:
 	ldr	sp, .Stack /* Set up stack pointer */
-	bic	sp, sp, #7 /* 8-byte alignment for ABI compliance */
 	ldr	r0,=0x00000000
+	 /*
+	  * The current design of cpu_info places the
+	  * struct at the top of the stack. The number of
+	  * words pushed must be at least as large as that
+	  * struct.
+	  */
+	push	{r0-r2}
+	bic	sp, sp, #7 /* 8-byte alignment for ABI compliance */
 	/*
 	 * Use "bl" instead of "b" even though we do not intend to return.
 	 * "bl" gets compiled to "blx" if we're transitioning from ARM to
@@ -104,5 +111,6 @@ wait_for_interrupt:
 .Stack:
 	.word CONFIG_STACK_TOP
 .align 2
+/* create this size the same way we do in coreboot_ram.ld: top-bottom */
 .Stack_size:
-	.word CONFIG_STACK_SIZE
+	.word CONFIG_STACK_TOP - CONFIG_STACK_BOTTOM
diff --git a/src/arch/armv7/cpu.c b/src/arch/armv7/cpu.c
new file mode 100644
index 0000000..f90c759
--- /dev/null
+++ b/src/arch/armv7/cpu.c
@@ -0,0 +1,42 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright 2013 Google Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. The name of the author may not be used to endorse or promote products
+ *    derived from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ */
+#include <stdlib.h>
+#include <arch/cpu.h>
+
+/* Return the cpu struct which is at the high memory address of the stack.
+ */
+struct cpu_info *cpu_info(void)
+{
+	uintptr_t addr = ALIGN((uintptr_t)__builtin_frame_address(0),
+		CONFIG_STACK_SIZE);
+	addr -= sizeof(struct cpu_info);
+	return (void *)addr;
+}
+
diff --git a/src/arch/armv7/include/arch/cpu.h b/src/arch/armv7/include/arch/cpu.h
index ec37a96..df44a92 100644
--- a/src/arch/armv7/include/arch/cpu.h
+++ b/src/arch/armv7/include/arch/cpu.h
@@ -104,4 +104,5 @@ inline static void set_svc32_mode(void)
 	asm volatile("msr cpsr_c, %0" :: "r"(0x13 | 0xc0));
 }
 
+struct cpu_info *cpu_info(void);
 #endif /* __ARCH_CPU_H__ */
diff --git a/src/cpu/samsung/exynos5420/Kconfig b/src/cpu/samsung/exynos5420/Kconfig
index d3eafcb..66679a0 100644
--- a/src/cpu/samsung/exynos5420/Kconfig
+++ b/src/cpu/samsung/exynos5420/Kconfig
@@ -69,7 +69,17 @@ config ROMSTAGE_SIZE
 # at the top of IRAM for now.
 #
 # Stack grows downward, push operation stores register contents in
-# consecutive memory locations ending just below SP
+# consecutive memory locations ending just below SP.
+# The setup in the exynos 5420 is a new one for coreboot. We have got
+# the bootblock, romstage, and ramstage sharing the same stack space.
+# The SRAM is always there and having a known-good stack memory
+# makes for a more reliable setup.
+# Thus, in this case:
+# STACK_TOP: highest stack address in SRAM
+# STACK_BOTTOM: lowest stack address in SRAM
+# STACK_SIZE: as in standard coreboot usage, size of thread stacks in ramstage
+# ROMSTAGE_STACK_SIZE: size of the single stack in romstage
+
 config STACK_TOP
 	hex
 	default 0x02073000
@@ -78,10 +88,18 @@ config STACK_BOTTOM
 	hex
 	default 0x0206f000
 
-config STACK_SIZE
+# The romstage stack must be large enough to contain the lzma buffer
+config ROMSTAGE_STACK_SIZE
 	hex
 	default 0x4000
 
+# STACK_SIZE is for the ramstage core and thread stacks.
+# It must be a power of 2, to make the cpu_info computation work,
+# and cpu_info needs to work to make SMP startup and threads work.
+config STACK_SIZE
+	hex
+	default 0x0800
+
 # TODO We may probably move this to board-specific implementation files instead
 # of KConfig values.
 config CBFS_CACHE_ADDRESS



More information about the coreboot-gerrit mailing list