Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/69435 )
Change subject: cpu/x86: Set thread local storage in C code ......................................................................
cpu/x86: Set thread local storage in C code
Doing this in C code is way easier to understand. Also the thread local storage is now in .bss instead of the AP stack. This makes it more robust against stack overflows, as APs stacks overflow in each other.
TESTED: work on qemu.
Change-Id: I19d3285daf97798a2d28408b5601ad991e29e718 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/c_start.S M src/arch/x86/cpu.c M src/arch/x86/include/arch/cpu.h D src/cpu/x86/cpu_info.S.inc M src/cpu/x86/mp_init.c M src/cpu/x86/sipi_vector.S 6 files changed, 72 insertions(+), 110 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/69435/1
diff --git a/src/arch/x86/c_start.S b/src/arch/x86/c_start.S index 02a9b89..5b7052e 100644 --- a/src/arch/x86/c_start.S +++ b/src/arch/x86/c_start.S @@ -1,6 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-only */
-#include <cpu/x86/cpu_info.S.inc> #include <cpu/x86/post_code.h> #include <arch/ram_segs.h>
@@ -78,21 +77,6 @@ movl $_estack, %esp andl $(~(CONFIG_STACK_SIZE-1)), %esp
- push_cpu_info - - /* Allocate the per_cpu_segment_data on the stack */ - push_per_cpu_segment_data - - /* - * Update the BSP's per_cpu_segment_descriptor to point to the - * per_cpu_segment_data that was allocated on the stack. - */ - set_segment_descriptor_base $per_cpu_segment_descriptors, %esp - - mov $per_cpu_segment_selector, %eax - movl (%eax), %eax - mov %eax, %gs - /* * Now we are finished. Memory is up, data is copied and * bss is cleared. Now we call the main routine and diff --git a/src/arch/x86/cpu.c b/src/arch/x86/cpu.c index f4cb83a..56ee1ec 100644 --- a/src/arch/x86/cpu.c +++ b/src/arch/x86/cpu.c @@ -6,6 +6,7 @@ #include <cpu/cpu.h> #include <post.h> #include <string.h> +#include <cpu/x86/gdt.h> #include <cpu/x86/mp.h> #include <cpu/x86/lapic.h> #include <cpu/x86/tsc.h> @@ -340,3 +341,41 @@ } return -1; } + +/* cpu_info() looks at address 0 at the base of %gs for a pointer to struct cpu_info */ +static struct cpu_info *cpu_infos_ptr[CONFIG_MAX_CPUS]; +static struct cpu_info cpu_infos[CONFIG_MAX_CPUS]; + +enum cb_err set_cpu_info(unsigned int index, struct device *cpu) +{ + if (index >= ARRAY_SIZE(cpu_infos)) + return CB_ERR; + + if (!cpu) + return CB_ERR; + + const struct cpu_info info = { .cpu = cpu, .index = index}; + cpu_infos[index] = info; + cpu_infos_ptr[index] = &cpu_infos[index]; + + struct segment_descriptor { + uint16_t segment_limit_0_15; + uint16_t base_address_0_15; + uint8_t base_address_16_23; + uint8_t attrs[2]; + uint8_t base_address_24_31; + } *segment_descriptor = (void *)&per_cpu_segment_descriptors; + + segment_descriptor[index].base_address_0_15 = (uintptr_t)&cpu_infos_ptr[index]; + segment_descriptor[index].base_address_16_23 = ((uintptr_t)&cpu_infos_ptr[index] >> 16); + segment_descriptor[index].base_address_24_31 = ((uintptr_t)&cpu_infos_ptr[index] >> 24); + + const unsigned int cpu_segment = per_cpu_segment_selector + (index << 3); + + __asm__ __volatile__ ("mov %0, %%gs\n" + : + : "r" (cpu_segment) + : ); + + return CB_SUCCESS; +} diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h index 0087a79..c7c6b4e 100644 --- a/src/arch/x86/include/arch/cpu.h +++ b/src/arch/x86/include/arch/cpu.h @@ -147,11 +147,13 @@ struct cpu_info *cpu_info; };
+enum cb_err set_cpu_info(unsigned int index, struct device *cpu); + static inline struct cpu_info *cpu_info(void) { struct cpu_info *ci = NULL;
- __asm__("mov %%gs:%c[offset], %[ci]" + __asm__ __volatile__("mov %%gs:%c[offset], %[ci]" : [ci] "=r" (ci) : [offset] "i" (offsetof(struct per_cpu_segment_data, cpu_info)) ); diff --git a/src/cpu/x86/cpu_info.S.inc b/src/cpu/x86/cpu_info.S.inc deleted file mode 100644 index 6dca920..0000000 --- a/src/cpu/x86/cpu_info.S.inc +++ /dev/null @@ -1,72 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -/* - * Pushes a 32-bit register onto the stack. - * - * There are two possible code sections where this code can be included: - * .code32 and .code64 - * - * Doing a `push %eax` while in a .code64 section will result in a compiler - * error. This macro manually pushes the 32-bit register onto the stack so we - * can share the code between 32 and 64 bit builds. - */ -.macro pushr reg:req -#if ENV_X86_64 - movl $0, -4(%esp) - movl \reg, -8(%esp) - sub $8, %esp -#else - push \reg -#endif -.endm - -/* Push struct cpu_info */ -.macro push_cpu_info index=$0 - pushr \index /* index (size_t) */ - pushr $0 /* *cpu */ -.endm - -/* Push struct per_cpu_segment_data */ -.macro push_per_cpu_segment_data cpu_info_pointer=%esp - pushr \cpu_info_pointer /* *cpu_info */ -.endm - -/* - * Sets the base address in the segment descriptor array. - * - * A segment descriptor has the following structure: - * struct { - * uint16_t segment_limit_0_15; - * uint16_t base_address_0_15; - * uint8_t base_address_16_23; - * uint8_t attrs[2]; - * uint8_t base_address_24_31; - * }; - * - * @desc_array: Address of the descriptor table - * @base: Address to set in the descriptor - * @desc_index: Index of the descriptor in the table. Defaults to 0. Must be a - * register if specified. - * - * Clobbers %eax, %ebx. - */ -.macro set_segment_descriptor_base desc_array:req, base:req, desc_index - mov \base, %eax - - mov \desc_array, %ebx - -.ifb \desc_index - movw %ax, 2(%ebx) - shr $16, %eax - movb %al, 4(%ebx) - shr $8, %eax - movb %al, 7(%ebx) -.else - movw %ax, 2(%ebx, \desc_index, 8) - shr $16, %eax - movb %al, 4(%ebx, \desc_index, 8) - shr $8, %eax - movb %al, 7(%ebx, \desc_index, 8) -.endif - -.endm diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 0e4a571..67ea6f4 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -176,20 +176,19 @@
/* By the time APs call ap_init() caching has been setup, and microcode has * been loaded. */ -static void asmlinkage ap_init(void) +static void asmlinkage ap_init(unsigned int index) { - struct cpu_info *info = cpu_info(); - /* Ensure the local APIC is enabled */ enable_lapic(); setup_lapic_interrupts();
struct device *dev = g_cpu_bus->children; - for (unsigned int i = info->index; i > 0; i--) + for (unsigned int i = index; i > 0; i--) dev = dev->sibling;
- info->cpu = dev; + set_cpu_info(index, dev);
+ struct cpu_info *info = cpu_info(); cpu_add_map_entry(info->index);
/* Fix up APIC id with reality. */ @@ -199,7 +198,7 @@ printk(BIOS_INFO, "AP: slot %zu apic_id %x, MCU rev: 0x%08x\n", info->index, info->cpu->path.apic.apic_id, get_current_microcode_rev()); else - printk(BIOS_INFO, "AP: slot %zu apic_id %x\n", info->index, + printk(BIOS_INFO, "AP: slot %u apic_id %x\n", index, info->cpu->path.apic.apic_id);
/* Walk the flight plan */ @@ -540,6 +539,7 @@ }
/* Find the device structure for the boot CPU. */ + set_cpu_info(0, bsp); info = cpu_info(); info->cpu = bsp; info->cpu->name = processor_name; diff --git a/src/cpu/x86/sipi_vector.S b/src/cpu/x86/sipi_vector.S index b8cac96..2a19404 100644 --- a/src/cpu/x86/sipi_vector.S +++ b/src/cpu/x86/sipi_vector.S @@ -1,6 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-only */
-#include <cpu/x86/cpu_info.S.inc> #include <cpu/x86/cr.h> #include <cpu/amd/mtrr.h> #include <cpu/x86/msr.h> @@ -104,19 +103,8 @@ subl %eax, %edx mov %edx, %esp
- push_cpu_info index=%ecx - push_per_cpu_segment_data - - /* - * Update the AP's per_cpu_segment_descriptor to point to the - * per_cpu_segment_data that was allocated on the stack. - */ - set_segment_descriptor_base per_cpu_segment_descriptors, %esp, %ecx - - mov %ecx, %eax - shl $3, %eax /* The index is << 3 in the segment selector */ - add per_cpu_segment_selector, %eax - mov %eax, %gs + /* Save CPU number for calling the AP entry */ + push %ecx
/* * The following code only needs to run on Intel platforms and thus the caller @@ -230,15 +218,20 @@ mov %eax, %cr4 #endif
+ pop %ebx /* Retrieve cpu index */ andl $0xfffffff0, %esp /* ensure stack alignment */
#if ENV_X86_64 /* entry64.inc preserves ebx. */ #include <cpu/x86/64bit/entry64.inc> - + mov %rbx, %rdi movabs c_handler, %eax call *%rax #else + push $0 + push $0 + push $0 + push %ebx mov c_handler, %eax call *%eax #endif