[coreboot-gerrit] Change in coreboot[master]: AGESA,binaryPI: Fix stack location on entry to romstage

[Kyösti Mälkki (Code Review)]

Hello Michał Żygowski,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/37351

to review the following change.

Change subject: AGESA,binaryPI: Fix stack location on entry to romstage
......................................................................

AGESA,binaryPI: Fix stack location on entry to romstage

For BSP CPU, setup stack location to match the symbol
from car.ld. For AP CPUs the stack is located outside
_car_region and is currently not accounted for in the
linker scripts.

Change-Id: I0ec84ae4e73ecca5034f799cdc2a5c1056ad8b74
Signed-off-by: Michał Żygowski 
Signed-off-by: Kyösti Mälkki 
---
M src/drivers/amd/agesa/cache_as_ram.S
M src/drivers/amd/agesa/romstage.c
M src/include/cpu/amd/car.h
3 files changed, 27 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/37351/1

diff --git a/src/drivers/amd/agesa/cache_as_ram.S b/src/drivers/amd/agesa/cache_as_ram.S
index 4f1cbea..8d5db98 100644
--- a/src/drivers/amd/agesa/cache_as_ram.S
+++ b/src/drivers/amd/agesa/cache_as_ram.S
@@ -22,6 +22,7 @@
  */

#include "gcccar.inc"
+#include <cpu/x86/lapic_def.h>
 #include <cpu/x86/post_code.h>

.code32
@@ -36,6 +37,19 @@

AMD_ENABLE_STACK

+	/*
+	 * Setup bootblock stack on BSP.
+	 * AMD_ENABLE_STACK macro sets up a stack for BSP at BSP_STACK_BASE_ADDR
+	 * which is 0x30000 (_car_region_end), but for C bootblock the stack
+	 * begins at _ecar_stack (see arch/x86/car.ld)
+	 */
+	mov	$LAPIC_BASE_MSR, %ecx
+	rdmsr
+	test	$LAPIC_BASE_MSR_BOOTSTRAP_PROCESSOR, %eax
+	jz	ap_entry
+
+	mov	$_ecar_stack, %esp
+
    /* Align the stack. */
    and	$0xFFFFFFF0, %esp

@@ -47,11 +61,19 @@
    pushl	%eax
    call	romstage_main

-	/* Should never see this postcode */
-	post_code(0xae)
+	/* Never reached. */

stop:
+	post_code(POST_DEAD_CODE)
    hlt
    jmp	stop

+ap_entry:
+	/* Align the stack for call to ap_romstage_main() */
+	and	$0xfffffff0, %esp
+	call	ap_romstage_main
+
+	/* Never reached. */
+	jmp	stop
+
 _cache_as_ram_setup_end:
diff --git a/src/drivers/amd/agesa/romstage.c b/src/drivers/amd/agesa/romstage.c
index 8460035..571397f 100644
--- a/src/drivers/amd/agesa/romstage.c
+++ b/src/drivers/amd/agesa/romstage.c
@@ -39,7 +39,7 @@
    agesa_set_interface(cb);
 }

-static void bsp_romstage_main(void)
+asmlinkage void romstage_main(unsigned long bist)
 {
    struct postcar_frame pcf;
    struct sysinfo romstage_state;
@@ -99,7 +99,7 @@
    /* We do not return. */
 }

-static void __noreturn ap_romstage_main(void)
+asmlinkage void ap_romstage_main(void)
 {
    struct sysinfo romstage_state;
    struct sysinfo *cb = &romstage_state;
@@ -116,11 +116,3 @@
    /* Not reached. */
    halt();
 }
-
-asmlinkage void romstage_main(unsigned long bist)
-{
-	if (boot_cpu())
-		bsp_romstage_main();
-	else
-		ap_romstage_main();
-}
diff --git a/src/include/cpu/amd/car.h b/src/include/cpu/amd/car.h
index 7e2fccd..f57ea82 100644
--- a/src/include/cpu/amd/car.h
+++ b/src/include/cpu/amd/car.h
@@ -4,5 +4,6 @@
 #include <arch/cpu.h>

asmlinkage void romstage_main(unsigned long bist);
+asmlinkage void ap_romstage_main(void);

#endif