Attention is currently required from: Jérémy Compostella, Shuo Liu.
Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/87255?usp=email )
Change subject: arch/x86: Unify GDT entries ......................................................................
arch/x86: Unify GDT entries
Currently there are 3 GDTs used on x86: - preRAM (gdt_init.S) - SMM (smm_stub.S) - RAM (c_start.S)
They have different layouts and thus different offset for the segments being using in assembly code. Stop using different GDT segments and make sure that for ROM (preRAM + SMM) and RAM (ramstage) the segments match. RAM will have additional entries, not found in pre RAM GDT, but the segments for protected mode and 64-bit mode now match in all stages.
This allows to use the same defines in all stages. It also drops the need to know in which stage the code is compiled and it's no longer necessary to switch the code segment between stages.
While on it fix the comments in the ramstage GDT and drop unused declarations from header files.
Change-Id: I208496e6e4cc82833636f4f42503b44b0d702b9e Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/arch/x86/c_start.S D src/arch/x86/include/arch/ram_segs.h D src/arch/x86/include/arch/rom_segs.h M src/arch/x86/wakeup.S M src/cpu/x86/64bit/entry64.inc M src/cpu/x86/64bit/exit32.inc M src/cpu/x86/entry16.S M src/cpu/x86/entry32.S M src/cpu/x86/sipi_vector.S M src/cpu/x86/smm/smm_stub.S M src/device/oprom/realmode/x86_asm.S M src/include/cpu/x86/gdt.h M src/security/intel/stm/StmPlatformSmm.c M src/security/intel/txt/getsec_enteraccs.S 14 files changed, 80 insertions(+), 125 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/87255/1
diff --git a/src/arch/x86/c_start.S b/src/arch/x86/c_start.S index 94b9bd9..3bdef70 100644 --- a/src/arch/x86/c_start.S +++ b/src/arch/x86/c_start.S @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <cpu/x86/gdt.h> #include <cpu/x86/post_code.h> -#include <arch/ram_segs.h>
/* Place the stack in the bss section. It's not necessary to define it in * the linker script. */ @@ -30,9 +30,9 @@ lgdt (%rax) #else lgdt %cs:gdtaddr - ljmp $RAM_CODE_SEG, $1f + ljmp $GDT_CODE_SEG, $1f #endif -1: movl $RAM_DATA_SEG, %eax +1: movl $GDT_DATA_SEG, %eax movl %eax, %ds movl %eax, %es movl %eax, %ss @@ -40,7 +40,7 @@ movl %eax, %fs movl %eax, %gs /* Will be used for cpu_info */ #if ENV_X86_64 - mov $RAM_CODE_SEG64, %ecx + mov $GDT_CODE_SEG64, %ecx call SetCodeSelector #endif
@@ -159,46 +159,42 @@
.data
- /* This is the gdt for GCC part of coreboot. + /* + * This is the gdt for coreboot's ramstage. * It is different from the gdt in ASM part of coreboot * which is defined in gdt_init.S * * When the machine is initially started, we use a very simple * gdt from ROM (that in gdt_init.S) which only contains those - * entries we need for protected mode. + * entries we need for protected mode and long mode. * * When we're executing code from RAM, we want to do more complex * stuff, like initializing PCI option ROMs in real mode, or doing - * a resume from a suspend to RAM. + * a resume from a suspend to RAM, which happens in real mode. */ gdt: /* selgdt 0, unused */ .word 0x0000, 0x0000 /* dummy */ .byte 0x00, 0x00, 0x00, 0x00
- /* selgdt 8, unused */ - .word 0x0000, 0x0000 /* dummy */ - .byte 0x00, 0x00, 0x00, 0x00 - - /* selgdt 0x10, flat code segment */ + /* selgdt 0x08, flat code segment */ .word 0xffff, 0x0000 - .byte 0x00, 0x9b, 0xcf, 0x00 /* G=1 and 0x0f, So we get 4Gbytes for - * limit - */ + .byte 0x00, 0x9b, 0xcf, 0x00 /* G=1 and 0x0f, So we get 4Gbytes + for limit */
- /* selgdt 0x18, flat data segment */ + /* selgdt 0x10,flat data segment */ .word 0xffff, 0x0000 -#if ENV_X86_64 - .byte 0x00, 0x92, 0xcf, 0x00 -#else .byte 0x00, 0x93, 0xcf, 0x00 -#endif
- /* selgdt 0x20, unused */ - .word 0x0000, 0x0000 /* dummy */ - .byte 0x00, 0x00, 0x00, 0x00 + /* selgdt 0x18, flat code segment (64-bit) */ + .word 0xffff, 0x0000 + .byte 0x00, 0x9b, 0xaf, 0x00
- /* The next two entries are used for executing VGA option ROMs */ + /* gdt selector 0x20 tss segment, used by STM */ + .word 0xffff, 0x0000 + .byte 0x00, 0x8b, 0x80, 0x00 + + /* The next two entries are used for executing ACPI S3 RESUME */
/* selgdt 0x28 16 bit 64k code at 0x00000000 */ .word 0xffff, 0x0000 @@ -208,25 +204,20 @@ .word 0xffff, 0x0000 .byte 0, 0x92, 0, 0
- /* The next two entries are used for ACPI S3 RESUME */ + /* The next two entries are used for VGA option ROMs */
- /* selgdt 0x38, flat data segment 16 bit */ + /* selgdt 0x38, unused */ .word 0x0000, 0x0000 /* dummy */ .byte 0x00, 0x93, 0x8f, 0x00 /* G=1 and 0x0f, So we get 4Gbytes for * limit */
- /* selgdt 0x40, flat code segment 16 bit */ + /* selgdt 0x40, flat data segment 16 bit */ .word 0xffff, 0x0000 .byte 0x00, 0x9b, 0x8f, 0x00 /* G=1 and 0x0f, So we get 4Gbytes for * limit */
-#if ENV_X86_64 - /* selgdt 0x48, flat x64 code segment */ - .word 0xffff, 0x0000 - .byte 0x00, 0x9b, 0xaf, 0x00 -#endif per_cpu_segment_descriptors: .rept CONFIG_MAX_CPUS /* flat data segment */ diff --git a/src/arch/x86/include/arch/ram_segs.h b/src/arch/x86/include/arch/ram_segs.h deleted file mode 100644 index 3f92a1f..0000000 --- a/src/arch/x86/include/arch/ram_segs.h +++ /dev/null @@ -1,14 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#ifndef RAM_SEGS_H -#define RAM_SEGS_H - -#define RAM_CODE_SEG 0x10 -#define RAM_DATA_SEG 0x18 -#define RAM_CODE16_SEG 0x28 -#define RAM_DATA16_SEG 0x30 -#define RAM_CODE_ACPI_SEG 0x38 -#define RAM_DATA_ACPI_SEG 0x40 -#define RAM_CODE_SEG64 0x48 - -#endif /* RAM_SEGS_H */ diff --git a/src/arch/x86/include/arch/rom_segs.h b/src/arch/x86/include/arch/rom_segs.h deleted file mode 100644 index a7e31d2..0000000 --- a/src/arch/x86/include/arch/rom_segs.h +++ /dev/null @@ -1,17 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#ifndef ROM_SEGS_H -#define ROM_SEGS_H - -#define ROM_CODE_SEG 0x08 -#define ROM_DATA_SEG 0x10 -#define ROM_CODE_SEG64 0x18 - -/* - * This define is placed here to make sure future romstage programmers - * know about it. - * It is used for STM setup code. - */ -#define SMM_TASK_STATE_SEG 0x20 - -#endif /* ROM_SEGS_H */ diff --git a/src/arch/x86/wakeup.S b/src/arch/x86/wakeup.S index 7bff006..1afc311 100644 --- a/src/arch/x86/wakeup.S +++ b/src/arch/x86/wakeup.S @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
-#include <arch/ram_segs.h> +#include <cpu/x86/gdt.h>
#define WAKEUP_BASE 0x600 #define RELOCATED(x) (x - __wakeup + WAKEUP_BASE) @@ -31,7 +31,7 @@ add $8, %rax push %rax pushfq - push $RAM_CODE_SEG + push $GDT_CODE_SEG lea 3(%rip), %rax push %rax iretq @@ -60,7 +60,7 @@ movw %ax, (__wakeup_segment)
/* Activate the right segment descriptor real mode. */ - ljmp $RAM_CODE16_SEG, $RELOCATED(1f) + ljmp $GDT_CODE16_SEG, $RELOCATED(1f) 1: .code16 /* 16 bit code from here on... */ @@ -70,7 +70,7 @@ * configurations (limits, writability, etc.) once * protected mode is turned off. */ - mov $RAM_DATA16_SEG, %ax + mov $GDT_DATA16_SEG, %ax mov %ax, %ds mov %ax, %es mov %ax, %fs diff --git a/src/cpu/x86/64bit/entry64.inc b/src/cpu/x86/64bit/entry64.inc index 52da603..a9f7ba4 100644 --- a/src/cpu/x86/64bit/entry64.inc +++ b/src/cpu/x86/64bit/entry64.inc @@ -17,12 +17,8 @@ #endif #endif
+#include <cpu/x86/gdt.h> #include <cpu/x86/msr.h> -#if defined(__RAMSTAGE__) -#include <arch/ram_segs.h> -#else -#include <arch/rom_segs.h> -#endif
.macro setup_longmode page_table /* Get page table address */ @@ -48,12 +44,8 @@ movl %eax, %cr0
/* use long jump to switch to 64-bit code segment */ -#if defined(__RAMSTAGE__) - ljmp $RAM_CODE_SEG64, $jmp_addr@ -#else - ljmp $ROM_CODE_SEG64, $jmp_addr@ + ljmp $GDT_CODE_SEG64, $jmp_addr@
-#endif .code64 jmp_addr@: .endm diff --git a/src/cpu/x86/64bit/exit32.inc b/src/cpu/x86/64bit/exit32.inc index 3ac86a9..a3d215e 100644 --- a/src/cpu/x86/64bit/exit32.inc +++ b/src/cpu/x86/64bit/exit32.inc @@ -10,17 +10,9 @@ */ .code64
+#include <cpu/x86/gdt.h> #include <cpu/x86/msr.h> #include <cpu/x86/cr.h> -#if defined(__RAMSTAGE__) -#include <arch/ram_segs.h> -#define CODE_SEG RAM_CODE_SEG -#define DATA_SEG RAM_DATA_SEG -#else -#include <arch/rom_segs.h> -#define CODE_SEG ROM_CODE_SEG -#define DATA_SEG ROM_DATA_SEG -#endif
drop_longmode: #if !ENV_CACHE_AS_RAM @@ -28,7 +20,7 @@ wbinvd #endif /* Set 32-bit code segment and ss */ - mov $CODE_SEG, %rcx + mov $GDT_CODE_SEG, %rcx /* SetCodeSelector32 will drop us to protected mode on return */ call SetCodeSelector32
@@ -63,7 +55,7 @@ /* Running in 32-bit compatibility mode */
/* Use flat data segment */ - movl $DATA_SEG, %eax + movl $GDT_DATA_SEG, %eax movl %eax, %ds movl %eax, %es movl %eax, %ss diff --git a/src/cpu/x86/entry16.S b/src/cpu/x86/entry16.S index ff4f1a2..b19ffa5 100644 --- a/src/cpu/x86/entry16.S +++ b/src/cpu/x86/entry16.S @@ -27,7 +27,7 @@ /* Start code to put an i386 or later processor into 32-bit protected mode. */
-#include <arch/rom_segs.h> +#include <cpu/x86/gdt.h> #include <cpu/x86/post_code.h>
.section .init._start, "ax", @progbits @@ -136,7 +136,7 @@ movl %ebp, %eax
/* Now that we are in protected mode jump to a 32 bit code segment. */ - ljmpl $ROM_CODE_SEG, $bootblock_protected_mode_entry + ljmpl $GDT_CODE_SEG, $bootblock_protected_mode_entry
/** * The gdt is defined in gdt_init.S, it has a 4 Gb code segment diff --git a/src/cpu/x86/entry32.S b/src/cpu/x86/entry32.S index 5c29581..d013cba 100644 --- a/src/cpu/x86/entry32.S +++ b/src/cpu/x86/entry32.S @@ -11,7 +11,7 @@ * */
-#include <arch/rom_segs.h> +#include <cpu/x86/gdt.h> #include <cpu/x86/cr.h> #include <cpu/x86/post_code.h>
@@ -33,7 +33,7 @@
post_code(POSTCODE_ENTER_PROTECTED_MODE)
- movw $ROM_DATA_SEG, %ax + movw $GDT_DATA_SEG, %ax movw %ax, %ds movw %ax, %es movw %ax, %ss diff --git a/src/cpu/x86/sipi_vector.S b/src/cpu/x86/sipi_vector.S index b7d700f..6057282 100644 --- a/src/cpu/x86/sipi_vector.S +++ b/src/cpu/x86/sipi_vector.S @@ -1,9 +1,9 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <cpu/x86/cr.h> +#include <cpu/x86/gdt.h> #include <cpu/amd/mtrr.h> #include <cpu/x86/msr.h> -#include <arch/ram_segs.h>
#define __RAMSTAGE__ #include <cpu/x86/64bit/entry64.inc> @@ -77,10 +77,10 @@ orl $CR0_SET_FLAGS, %eax movl %eax, %cr0
- ljmpl $RAM_CODE_SEG, $1f + ljmpl $GDT_CODE_SEG, $1f 1: .code32 - movw $RAM_DATA_SEG, %ax + movw $GDT_DATA_SEG, %ax movw %ax, %ds movw %ax, %es movw %ax, %ss diff --git a/src/cpu/x86/smm/smm_stub.S b/src/cpu/x86/smm/smm_stub.S index e42d232..c7a55f6 100644 --- a/src/cpu/x86/smm/smm_stub.S +++ b/src/cpu/x86/smm/smm_stub.S @@ -9,8 +9,8 @@ * found in smm.h. */
-#include <arch/rom_segs.h> #include <cpu/x86/cr.h> +#include <cpu/x86/gdt.h> #include <cpu/x86/msr.h> #include <cpu/x86/lapic_def.h> #include <cpu/x86/64bit/entry64.inc> @@ -94,9 +94,8 @@ movl %eax, %cr0
/* Enable protected mode */ - ljmpl $ROM_CODE_SEG, $smm_trampoline32 + ljmpl $GDT_CODE_SEG, $smm_trampoline32
-/* .align 4 smm_relocate_gdt: /* The first GDT entry is used for the lgdt instruction. */ @@ -126,7 +125,7 @@ .global smm_trampoline32 smm_trampoline32: /* Use flat data segment */ - movw $ROM_DATA_SEG, %ax + movw $GDT_DATA_SEG, %ax movw %ax, %ds movw %ax, %es movw %ax, %ss diff --git a/src/device/oprom/realmode/x86_asm.S b/src/device/oprom/realmode/x86_asm.S index ff33c0f..5ecd82a 100644 --- a/src/device/oprom/realmode/x86_asm.S +++ b/src/device/oprom/realmode/x86_asm.S @@ -3,7 +3,7 @@ #define REALMODE_BASE 0x600 #define RELOCATED(x) (x - __realmode_code + REALMODE_BASE)
-#include <arch/ram_segs.h> +#include <cpu/x86/gdt.h>
/* CR0 bits */ #define PE (1 << 0) @@ -107,7 +107,7 @@ * configurations (limits, writability, etc.) once * protected mode is turned off. */ - mov $RAM_DATA16_SEG, %ax + mov $GDT_DATA16_SEG, %ax mov %ax, %ds mov %ax, %es mov %ax, %fs @@ -150,7 +150,7 @@ mov %ax, %es mov %ax, %fs mov %ax, %gs - mov $RAM_DATA_ACPI_SEG, %ax + mov $GDT_DATA_ACPI_SEG, %ax mov %ax, %ds pop %ax
@@ -177,10 +177,10 @@ /* Now that we are in protected mode * jump to a 32 bit code segment. */ - ljmpl $RAM_CODE_SEG, $RELOCATED(1f) + ljmpl $GDT_CODE_SEG, $RELOCATED(1f) 1: .code32 - mov $RAM_DATA_SEG, %ax + mov $GDT_DATA_SEG, %ax mov %ax, %ds mov %ax, %es mov %ax, %fs @@ -233,7 +233,7 @@ movl %eax, __registers + 20 /* edi */
/* This configures CS properly for real mode. */ - ljmp $RAM_CODE16_SEG, $RELOCATED(1f) + ljmp $GDT_CODE16_SEG, $RELOCATED(1f) 1: .code16 /* 16 bit code from here on... */
@@ -241,7 +241,7 @@ * descriptors. They will retain these configurations (limits, * writability, etc.) once protected mode is turned off. */ - mov $RAM_DATA16_SEG, %ax + mov $GDT_DATA16_SEG, %ax mov %ax, %ds mov %ax, %es mov %ax, %fs @@ -305,10 +305,10 @@ movl %eax, %cr0
/* Now that we are in protected mode jump to a 32-bit code segment. */ - ljmpl $RAM_CODE_SEG, $RELOCATED(1f) + ljmpl $GDT_CODE_SEG, $RELOCATED(1f) 1: .code32 - mov $RAM_DATA_SEG, %ax + mov $GDT_DATA_SEG, %ax mov %ax, %ds mov %ax, %es mov %ax, %fs @@ -354,10 +354,10 @@ movl %eax, %cr0
/* ... and jump to a 32 bit code segment. */ - ljmpl $RAM_CODE_SEG, $RELOCATED(1f) + ljmpl $GDT_CODE_SEG, $RELOCATED(1f) 1: .code32 - mov $RAM_DATA_SEG, %ax + mov $GDT_DATA_SEG, %ax mov %ax, %ds mov %ax, %es mov %ax, %fs @@ -371,14 +371,14 @@ call *%eax
/* Now return to real mode ... */ - ljmp $RAM_CODE16_SEG, $RELOCATED(1f) + ljmp $GDT_CODE16_SEG, $RELOCATED(1f) 1: .code16 /* Load the segment registers with properly configured segment * descriptors. They will retain these configurations (limits, * writability, etc.) once protected mode is turned off. */ - mov $RAM_DATA16_SEG, %ax + mov $GDT_DATA16_SEG, %ax mov %ax, %ds mov %ax, %es mov %ax, %fs diff --git a/src/include/cpu/x86/gdt.h b/src/include/cpu/x86/gdt.h index 27a863e..6902b88 100644 --- a/src/include/cpu/x86/gdt.h +++ b/src/include/cpu/x86/gdt.h @@ -3,16 +3,28 @@ #ifndef CPU_X86_GDT #define CPU_X86_GDT
+#ifndef __ASSEMBLER__ /* These symbols are defined in c_start.S. */ extern char gdt[]; extern char per_cpu_segment_descriptors[]; extern uint32_t per_cpu_segment_selector; extern char gdt_end[]; extern char idtarg[]; +#endif
-/* These symbols are defined in secondary.S. */ -extern char _secondary_gdt_addr[]; -extern char _secondary_start[]; -extern char _secondary_start_end[]; +#define GDT_CODE_SEG 0x08 +#define GDT_DATA_SEG 0x10 +#define GDT_CODE_SEG64 0x18 +/* + * This define is placed here to make sure future romstage programmers + * know about it. + * It is used only in SMM for STM setup code. + */ +#define GDT_TASK_STATE_SEG 0x20 + +#define GDT_CODE16_SEG 0x28 +#define GDT_DATA16_SEG 0x30 +#define GDT_CODE_ACPI_SEG 0x38 +#define GDT_DATA_ACPI_SEG 0x40
#endif /* CPU_X86_GDT */ diff --git a/src/security/intel/stm/StmPlatformSmm.c b/src/security/intel/stm/StmPlatformSmm.c index 9c5ae52..99e62c7 100644 --- a/src/security/intel/stm/StmPlatformSmm.c +++ b/src/security/intel/stm/StmPlatformSmm.c @@ -4,6 +4,7 @@ #include <security/intel/stm/SmmStm.h> #include <security/intel/stm/StmPlatformResource.h> #include <security/tpm/tspi.h> +#include <cpu/x86/gdt.h> #include <cpu/x86/smm.h> #include <cpu/x86/msr.h>
@@ -11,7 +12,6 @@ #include <console/console.h> #include <stdbool.h> #include <stdint.h> -#include <arch/rom_segs.h>
/* * Load STM image to MSEG @@ -109,11 +109,11 @@ psd->acpi_rsdp = 0; psd->bios_hw_resource_requirements_ptr = (uint64_t)((uintptr_t)get_stm_resource()); - psd->smm_cs = ROM_CODE_SEG; - psd->smm_ds = ROM_DATA_SEG; - psd->smm_ss = ROM_DATA_SEG; - psd->smm_other_segment = ROM_DATA_SEG; - psd->smm_tr = SMM_TASK_STATE_SEG; + psd->smm_cs = GDT_CODE_SEG; + psd->smm_ds = GDT_DATA_SEG; + psd->smm_ss = GDT_DATA_SEG; + psd->smm_other_segment = GDT_DATA_SEG; + psd->smm_tr = GDT_TASK_STATE_SEG;
// At this point the coreboot smm_stub is relative to the default // smbase and not the one for the smi handler in tseg. So we have diff --git a/src/security/intel/txt/getsec_enteraccs.S b/src/security/intel/txt/getsec_enteraccs.S index ff9db05..9c0f531 100644 --- a/src/security/intel/txt/getsec_enteraccs.S +++ b/src/security/intel/txt/getsec_enteraccs.S @@ -3,7 +3,7 @@ #include <cpu/x86/mtrr.h> #include <cpu/x86/cr.h> #include <cpu/x86/msr.h> -#include <arch/ram_segs.h> +#include <cpu/x86/gdt.h>
#include "getsec_mtrr_setup.inc"
@@ -257,10 +257,10 @@ lgdt -48(%ebp)
/* Set cs */ - ljmp $RAM_CODE_SEG, $1f + ljmp $GDT_CODE_SEG, $1f 1: /* Fix segment registers */ - movl $RAM_DATA_SEG, %eax + movl $GDT_DATA_SEG, %eax movl %eax, %ds movl %eax, %es movl %eax, %ss
-
Patrick Rudolph (Code Review)