From 2fbb6773e3a6d5d18e84b0ac79b645147fbf0d66 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ky=C3=B6sti=20M=C3=A4lkki?= <kyosti.malkki@gmail.com>
Date: Tue, 15 May 2018 19:50:20 +0300
Subject: arch/x86: Enforce CPU stack alignment
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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>
---
src/arch/x86/c_start.S | 7 +++++--
src/cpu/x86/lapic/lapic_cpu_init.c | 22 +++++++++++-----------
src/cpu/x86/mp_init.c | 9 +++++----
3 files changed, 21 insertions(+), 17 deletions(-)
(limited to 'src')
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);
}
--
cgit v1.2.3