arch/x86: Enforce CPU stack alignment

When rmodule is loaded CPU stack alignment is only guaranteed
to 4kiB. Implementation of cpu_info() requires that each
CPU sees its stack aligned to CONFIG_STACK_SIZE.

Add one spare CPU for the stack reserve, such that alignment can
be enforced runtime.

Change-Id: Ie04956c64df0dc7bb156002d3d4f2629f92b340e
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/26302
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>

3 files changed, 21 insertions, 17 deletions

diff --git a/src/arch/x86/c_start.S b/src/arch/x86/c_start.S
index 3fbdac11b3..86147ecdc7 100644
--- a/src/arch/x86/c_start.S
+++ b/src/arch/x86/c_start.S
@@ -19,9 +19,11 @@
 .global _stack
 .global _estack
 
+/* Stack alignment is not enforced with rmodule loader, reserve one
+ * extra CPU such that alignment can be enforced on entry. */
 .align CONFIG_STACK_SIZE
 _stack:
-.space CONFIG_MAX_CPUS*CONFIG_STACK_SIZE
+.space (CONFIG_MAX_CPUS+1)*CONFIG_STACK_SIZE
 _estack:
 #if IS_ENABLED(CONFIG_COOP_MULTITASKING)
 .global thread_stacks
@@ -70,8 +72,9 @@ _start:
 	rep
 	stosl
 
-	/* set new stack */
+	/* Set new stack with enforced alignment. */
 	movl	$_estack, %esp
+	andl	$(~(CONFIG_STACK_SIZE-1)), %esp
 
 #if IS_ENABLED(CONFIG_COOP_MULTITASKING)
 	/* Push the thread pointer. */
diff --git a/src/cpu/x86/lapic/lapic_cpu_init.c b/src/cpu/x86/lapic/lapic_cpu_init.c
index 4498b97b1c..7daca0ac67 100644
--- a/src/cpu/x86/lapic/lapic_cpu_init.c
+++ b/src/cpu/x86/lapic/lapic_cpu_init.c
@@ -263,8 +263,8 @@ volatile unsigned int secondary_cpu_index;
 int start_cpu(struct device *cpu)
 {
 	struct cpu_info *info;
-	unsigned long stack_end;
-	unsigned long stack_base;
+	uintptr_t stack_top;
+	uintptr_t stack_base;
 	unsigned long apicid;
 	unsigned int index;
 	unsigned long count;
@@ -278,23 +278,23 @@ int start_cpu(struct device *cpu)
 	/* Get an index for the new processor */
 	index = ++last_cpu_index;
 
-	/* Find end of the new processor's stack */
-	stack_end = ((unsigned long)_estack) - (CONFIG_STACK_SIZE*index) -
-			sizeof(struct cpu_info);
-
-	stack_base = ((unsigned long)_estack) - (CONFIG_STACK_SIZE*(index+1));
-	printk(BIOS_SPEW, "CPU%d: stack_base %p, stack_end %p\n", index,
-		(void *)stack_base, (void *)stack_end);
+	/* Find boundaries of the new processor's stack */
+	stack_top = ALIGN_DOWN((uintptr_t)_estack, CONFIG_STACK_SIZE);
+	stack_top -= (CONFIG_STACK_SIZE*index);
+	stack_base = stack_top - CONFIG_STACK_SIZE;
+	stack_top -= sizeof(struct cpu_info);
+	printk(BIOS_SPEW, "CPU%d: stack_base %p, stack_top %p\n", index,
+		(void *)stack_base, (void *)stack_top);
 	stacks[index] = (void *)stack_base;
 
 	/* Record the index and which CPU structure we are using */
-	info = (struct cpu_info *)stack_end;
+	info = (struct cpu_info *)stack_top;
 	info->index = index;
 	info->cpu   = cpu;
 	thread_init_cpu_info_non_bsp(info);
 
 	/* Advertise the new stack and index to start_cpu */
-	secondary_stack = stack_end;
+	secondary_stack = stack_top;
 	secondary_cpu_index = index;
 
 	/* Until the CPU starts up report the CPU is not enabled */
diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c
index 9526824976..3889c7d28e 100644
--- a/src/cpu/x86/mp_init.c
+++ b/src/cpu/x86/mp_init.c
@@ -18,6 +18,7 @@
 #include <stdint.h>
 #include <rmodule.h>
 #include <arch/cpu.h>
+#include <commonlib/helpers.h>
 #include <cpu/cpu.h>
 #include <cpu/intel/microcode.h>
 #include <cpu/x86/cache.h>
@@ -229,11 +230,11 @@ static void asmlinkage ap_init(unsigned int cpu)
 
 static void setup_default_sipi_vector_params(struct sipi_params *sp)
 {
-	sp->gdt = (uint32_t)&gdt;
-	sp->gdtlimit = (uint32_t)&gdt_end - (u32)&gdt - 1;
-	sp->idt_ptr = (uint32_t)&idtarg;
+	sp->gdt = (uintptr_t)&gdt;
+	sp->gdtlimit = (uintptr_t)&gdt_end - (uintptr_t)&gdt - 1;
+	sp->idt_ptr = (uintptr_t)&idtarg;
 	sp->stack_size = CONFIG_STACK_SIZE;
-	sp->stack_top = (uint32_t)&_estack;
+	sp->stack_top = ALIGN_DOWN((uintptr_t)&_estack, CONFIG_STACK_SIZE);
 	/* Adjust the stack top to take into account cpu_info. */
 	sp->stack_top -= sizeof(struct cpu_info);
 }