[coreboot-gerrit] Patch set updated for coreboot: 1fbb363 arm: Fix checkstack() to use correct stack size

[Patrick Georgi (pgeorgi@google.com)]

Patrick Georgi (pgeorgi at google.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/9610

-gerrit

commit 1fbb3631ecf18dfe081af05919bb647446fd2385
Author: Julius Werner <jwerner at chromium.org>
Date:   Mon Dec 15 18:19:03 2014 -0800

    arm: Fix checkstack() to use correct stack size
    
    checkstack() runs at the end of ramstage to warn about stack overflows,
    and it assumes that CONFIG_STACK_SIZE is always the size of the stack to
    check. This is only true for systems that bring up multiprocessing in
    ramstage and assign a separate stack for each core, like x86 and ARM64.
    Other architectures like ARM and MIPS (for now) don't touch secondary
    CPUs at all and currently don't look like they'll ever need to, so they
    generally stay on the same (SRAM-based) stack they have been on since
    their bootblock.
    
    This patch tries to model that difference by making these architectures
    explicitly set CONFIG_STACK_SIZE to zero, and using that as a cue to
    assume the whole (_estack - _stack) area in checkstack() instead. Also
    adds a BUG() to the stack overflow check, since that is currently just
    as non-fatal as the BIOS_ERR message (despite the incorrect "SYSTEM
    HALTED" output) but a little more easy to spot. Such a serious failure
    should not drown out in all the normal random pieces of lower case boot
    spam (also, I was intending to eventually have a look at assert() and
    BUG() to hopefully make them a little more useful/noticeable if I ever
    find the time for it).
    
    BRANCH=None
    BUG=None
    TEST=Booted Pinky, noticed it no longer complains about stack overflows.
    Built Falco, Ryu and Urara.
    
    Change-Id: I6826e0ec24201d4d83c5929b281828917bc9abf4
    Signed-off-by: Patrick Georgi <pgeorgi at chromium.org>
    Original-Commit-Id: 54229a725e8907b84a105c04ecea33b8f9b91dd4
    Original-Change-Id: I49f70bb7ad192bd1c48e077802085dc5ecbfd58b
    Original-Signed-off-by: Julius Werner <jwerner at chromium.org>
    Original-Reviewed-on: https://chromium-review.googlesource.com/235894
    Original-Reviewed-by: Aaron Durbin <adurbin at chromium.org>
---
 src/arch/arm/Kconfig  |  5 +++++
 src/arch/mips/Kconfig |  5 +++++
 src/lib/stack.c       | 20 ++++++++++++--------
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/arch/arm/Kconfig b/src/arch/arm/Kconfig
index 2499d2d..6bef91c 100644
--- a/src/arch/arm/Kconfig
+++ b/src/arch/arm/Kconfig
@@ -31,3 +31,8 @@ config ARM_BOOTBLOCK_CUSTOM
 config ARM_LPAE
 	bool
 	default n
+
+# Mark SMP stack size as 0 since we keep using SRAM stack throughout ramstage.
+config STACK_SIZE
+	hex
+	default 0x0
diff --git a/src/arch/mips/Kconfig b/src/arch/mips/Kconfig
index 10349f2..71e5dc8 100644
--- a/src/arch/mips/Kconfig
+++ b/src/arch/mips/Kconfig
@@ -39,3 +39,8 @@ config ARCH_ROMSTAGE_MIPS
 config ARCH_RAMSTAGE_MIPS
 	bool
 	default n
+
+# Mark SMP stack size as 0 since we keep using SRAM stack throughout ramstage.
+config STACK_SIZE
+	hex
+	default 0x0
diff --git a/src/lib/stack.c b/src/lib/stack.c
index 05e7975..52dd723 100644
--- a/src/lib/stack.c
+++ b/src/lib/stack.c
@@ -20,31 +20,35 @@ it with the version available from LANL.
  * rminnich at lanl.gov
  */
 
+#include <assert.h>
 #include <lib.h>
 #include <console/console.h>
+#include <symbols.h>
 
 int checkstack(void *top_of_stack, int core)
 {
+	/* Not all archs use CONFIG_STACK_SIZE, those who don't set it to 0. */
+	size_t stack_size = CONFIG_STACK_SIZE ? CONFIG_STACK_SIZE : _stack_size;
 	int i;
-	u32 *stack = (u32 *) (top_of_stack - CONFIG_STACK_SIZE);
+	u32 *stack = (u32 *) (top_of_stack - stack_size);
 
 	if (stack[0] != 0xDEADBEEF){
 		printk(BIOS_ERR, "Stack overrun on CPU%d. "
-			"Increase stack from current %d bytes\n",
-			core, CONFIG_STACK_SIZE);
+			"Increase stack from current %zu bytes\n",
+			core, stack_size);
+		BUG();
 		return -1;
 	}
 
-	for(i = 1; i < CONFIG_STACK_SIZE/sizeof(stack[0]); i++){
+	for(i = 1; i < stack_size/sizeof(stack[0]); i++){
 		if (stack[i] == 0xDEADBEEF)
 			continue;
 		printk(BIOS_SPEW, "CPU%d: stack: %p - %p, ",
-			core, stack,
-			&stack[CONFIG_STACK_SIZE/sizeof(stack[0])]);
+			core, stack, &stack[stack_size/sizeof(stack[0])]);
 		printk(BIOS_SPEW, "lowest used address %p, ", &stack[i]);
 		printk(BIOS_SPEW, "stack used: %ld bytes\n",
-			(unsigned long)&stack[CONFIG_STACK_SIZE /
-			sizeof(stack[0])] - (unsigned long)&stack[i]);
+			(unsigned long)&stack[stack_size / sizeof(stack[0])]
+			- (unsigned long)&stack[i]);
 		return 0;
 	}