[U-Boot] [RFC PATCH 1/2] x86: fsp: Load GDT before calling FspInitEntry
Currently the FSP execution environment GDT is setup by U-Boot in arch/x86/cpu/start16.S, which works pretty well. But if we try to move the FspInitEntry call a little bit later to better fit into U-Boot's initialization sequence, FSP will fail to bring up the AP due to #GP fault as AP's GDT is duplicated from BSP whose GDT is now moved into CAR, and unfortunately FSP calls AP initialization after it disables the CAR. So basically the BSP's GDT still refers to the one in the CAR, whose content is no longer available, so when AP starts up and loads its segment register, it blows up.
To resolve this, we load GDT before calling into FspInitEntry. The GDT is the same one used in arch/x86/cpu/start16.S, which is in the ROM and exists forever.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
arch/x86/cpu/cpu.c | 18 ++++++++++++++++++ arch/x86/cpu/start16.S | 1 + arch/x86/include/asm/u-boot-x86.h | 4 ++++ arch/x86/lib/fsp/fsp_support.c | 3 +++ 4 files changed, 26 insertions(+)
diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c index bb4a110..a0b101c 100644 --- a/arch/x86/cpu/cpu.c +++ b/arch/x86/cpu/cpu.c @@ -164,6 +164,24 @@ void setup_gdt(gd_t *id, u64 *gdt_addr) load_fs(X86_GDT_ENTRY_32BIT_FS); }
+/* + * Setup FSP execution environment GDT + * + * Per Intel FSP external architecture specification, before calling any FSP + * APIs, we need make sure the system is in flat 32-bit mode and both the code + * and data selectors should have full 4GB access range. Here we reuse the one + * we used in arch/x86/cpu/start16.S, and reload the segement registers. + */ +void setup_fsp_gdt(void) +{ + load_gdt((const u64 *)((u32)&gdt + BOOT_SEG), 4); + load_ds(X86_GDT_ENTRY_32BIT_DS); + load_ss(X86_GDT_ENTRY_32BIT_DS); + load_es(X86_GDT_ENTRY_32BIT_DS); + load_fs(X86_GDT_ENTRY_32BIT_DS); + load_gs(X86_GDT_ENTRY_32BIT_DS); +} + int __weak x86_cleanup_before_linux(void) { #ifdef CONFIG_BOOTSTAGE_STASH diff --git a/arch/x86/cpu/start16.S b/arch/x86/cpu/start16.S index 826e2b4..a3e7ab4 100644 --- a/arch/x86/cpu/start16.S +++ b/arch/x86/cpu/start16.S @@ -75,6 +75,7 @@ gdt_ptr:
/* Some CPUs are picky about GDT alignment... */ .align 16 +.globl gdt gdt: /* * The GDT table ... diff --git a/arch/x86/include/asm/u-boot-x86.h b/arch/x86/include/asm/u-boot-x86.h index be103c0..faa5182 100644 --- a/arch/x86/include/asm/u-boot-x86.h +++ b/arch/x86/include/asm/u-boot-x86.h @@ -8,12 +8,16 @@ #ifndef _U_BOOT_I386_H_ #define _U_BOOT_I386_H_ 1
+#define BOOT_SEG 0xffff0000 +extern u32 gdt; + /* cpu/.../cpu.c */ int arch_cpu_init(void); int x86_cpu_init_f(void); int cpu_init_f(void); void init_gd(gd_t *id, u64 *gdt_addr); void setup_gdt(gd_t *id, u64 *gdt_addr); +void setup_fsp_gdt(void); int init_cache(void); int cleanup_before_linux(void); void panic_puts(const char *str); diff --git a/arch/x86/lib/fsp/fsp_support.c b/arch/x86/lib/fsp/fsp_support.c index 5f96da1..cf6ed04 100644 --- a/arch/x86/lib/fsp/fsp_support.c +++ b/arch/x86/lib/fsp/fsp_support.c @@ -173,6 +173,9 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
post_code(POST_PRE_MRC);
+ /* Load GDT for FSP */ + setup_fsp_gdt(); + /* * Use ASM code to ensure the register value in EAX & ECX * will be passed into BlContinuationFunc
Hi Bin,
On 1 June 2015 at 06:31, Bin Meng bmeng.cn@gmail.com wrote:
Currently the FSP execution environment GDT is setup by U-Boot in arch/x86/cpu/start16.S, which works pretty well. But if we try to move the FspInitEntry call a little bit later to better fit into U-Boot's initialization sequence, FSP will fail to bring up the AP due to #GP fault as AP's GDT is duplicated from BSP whose GDT is now moved into CAR, and unfortunately FSP calls AP initialization after it disables the CAR. So basically the BSP's GDT still refers to the one in the CAR, whose content is no longer available, so when AP starts up and loads its segment register, it blows up.
To resolve this, we load GDT before calling into FspInitEntry. The GDT is the same one used in arch/x86/cpu/start16.S, which is in the ROM and exists forever.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/cpu.c | 18 ++++++++++++++++++ arch/x86/cpu/start16.S | 1 + arch/x86/include/asm/u-boot-x86.h | 4 ++++ arch/x86/lib/fsp/fsp_support.c | 3 +++ 4 files changed, 26 insertions(+)
diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c index bb4a110..a0b101c 100644 --- a/arch/x86/cpu/cpu.c +++ b/arch/x86/cpu/cpu.c @@ -164,6 +164,24 @@ void setup_gdt(gd_t *id, u64 *gdt_addr) load_fs(X86_GDT_ENTRY_32BIT_FS); }
+/*
- Setup FSP execution environment GDT
- Per Intel FSP external architecture specification, before calling any FSP
- APIs, we need make sure the system is in flat 32-bit mode and both the code
- and data selectors should have full 4GB access range. Here we reuse the one
- we used in arch/x86/cpu/start16.S, and reload the segement registers.
- */
+void setup_fsp_gdt(void) +{
load_gdt((const u64 *)((u32)&gdt + BOOT_SEG), 4);load_ds(X86_GDT_ENTRY_32BIT_DS);load_ss(X86_GDT_ENTRY_32BIT_DS);load_es(X86_GDT_ENTRY_32BIT_DS);load_fs(X86_GDT_ENTRY_32BIT_DS);load_gs(X86_GDT_ENTRY_32BIT_DS);+}
int __weak x86_cleanup_before_linux(void) { #ifdef CONFIG_BOOTSTAGE_STASH diff --git a/arch/x86/cpu/start16.S b/arch/x86/cpu/start16.S index 826e2b4..a3e7ab4 100644 --- a/arch/x86/cpu/start16.S +++ b/arch/x86/cpu/start16.S @@ -75,6 +75,7 @@ gdt_ptr:
/* Some CPUs are picky about GDT alignment... */ .align 16+.globl gdt gdt: /* * The GDT table ... diff --git a/arch/x86/include/asm/u-boot-x86.h b/arch/x86/include/asm/u-boot-x86.h index be103c0..faa5182 100644 --- a/arch/x86/include/asm/u-boot-x86.h +++ b/arch/x86/include/asm/u-boot-x86.h @@ -8,12 +8,16 @@ #ifndef _U_BOOT_I386_H_ #define _U_BOOT_I386_H_ 1
+#define BOOT_SEG 0xffff0000
We have RESET_SEG_START in arch/x86/cpu/config.mk. We could move this to Kconfig if you like? But it would be good to avoid the duplication.
+extern u32 gdt;
/* cpu/.../cpu.c */ int arch_cpu_init(void); int x86_cpu_init_f(void); int cpu_init_f(void); void init_gd(gd_t *id, u64 *gdt_addr); void setup_gdt(gd_t *id, u64 *gdt_addr); +void setup_fsp_gdt(void); int init_cache(void); int cleanup_before_linux(void); void panic_puts(const char *str); diff --git a/arch/x86/lib/fsp/fsp_support.c b/arch/x86/lib/fsp/fsp_support.c index 5f96da1..cf6ed04 100644 --- a/arch/x86/lib/fsp/fsp_support.c +++ b/arch/x86/lib/fsp/fsp_support.c @@ -173,6 +173,9 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
post_code(POST_PRE_MRC);
/* Load GDT for FSP */setup_fsp_gdt();
Where does the GDT get set back to the normal U-Boot one?
/* * Use ASM code to ensure the register value in EAX & ECX * will be passed into BlContinuationFunc-- 1.8.2.1
Regards, Simon
Hi Simon,
On Thu, Jun 4, 2015 at 4:59 PM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 1 June 2015 at 06:31, Bin Meng bmeng.cn@gmail.com wrote:
Currently the FSP execution environment GDT is setup by U-Boot in arch/x86/cpu/start16.S, which works pretty well. But if we try to move the FspInitEntry call a little bit later to better fit into U-Boot's initialization sequence, FSP will fail to bring up the AP due to #GP fault as AP's GDT is duplicated from BSP whose GDT is now moved into CAR, and unfortunately FSP calls AP initialization after it disables the CAR. So basically the BSP's GDT still refers to the one in the CAR, whose content is no longer available, so when AP starts up and loads its segment register, it blows up.
To resolve this, we load GDT before calling into FspInitEntry. The GDT is the same one used in arch/x86/cpu/start16.S, which is in the ROM and exists forever.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/cpu.c | 18 ++++++++++++++++++ arch/x86/cpu/start16.S | 1 + arch/x86/include/asm/u-boot-x86.h | 4 ++++ arch/x86/lib/fsp/fsp_support.c | 3 +++ 4 files changed, 26 insertions(+)
diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c index bb4a110..a0b101c 100644 --- a/arch/x86/cpu/cpu.c +++ b/arch/x86/cpu/cpu.c @@ -164,6 +164,24 @@ void setup_gdt(gd_t *id, u64 *gdt_addr) load_fs(X86_GDT_ENTRY_32BIT_FS); }
+/*
- Setup FSP execution environment GDT
- Per Intel FSP external architecture specification, before calling any FSP
- APIs, we need make sure the system is in flat 32-bit mode and both the code
- and data selectors should have full 4GB access range. Here we reuse the one
- we used in arch/x86/cpu/start16.S, and reload the segement registers.
- */
+void setup_fsp_gdt(void) +{
load_gdt((const u64 *)((u32)&gdt + BOOT_SEG), 4);load_ds(X86_GDT_ENTRY_32BIT_DS);load_ss(X86_GDT_ENTRY_32BIT_DS);load_es(X86_GDT_ENTRY_32BIT_DS);load_fs(X86_GDT_ENTRY_32BIT_DS);load_gs(X86_GDT_ENTRY_32BIT_DS);+}
int __weak x86_cleanup_before_linux(void) { #ifdef CONFIG_BOOTSTAGE_STASH diff --git a/arch/x86/cpu/start16.S b/arch/x86/cpu/start16.S index 826e2b4..a3e7ab4 100644 --- a/arch/x86/cpu/start16.S +++ b/arch/x86/cpu/start16.S @@ -75,6 +75,7 @@ gdt_ptr:
/* Some CPUs are picky about GDT alignment... */ .align 16+.globl gdt gdt: /* * The GDT table ... diff --git a/arch/x86/include/asm/u-boot-x86.h b/arch/x86/include/asm/u-boot-x86.h index be103c0..faa5182 100644 --- a/arch/x86/include/asm/u-boot-x86.h +++ b/arch/x86/include/asm/u-boot-x86.h @@ -8,12 +8,16 @@ #ifndef _U_BOOT_I386_H_ #define _U_BOOT_I386_H_ 1
+#define BOOT_SEG 0xffff0000
We have RESET_SEG_START in arch/x86/cpu/config.mk. We could move this to Kconfig if you like? But it would be good to avoid the duplication.
Yes, we could. Will do in v2.
+extern u32 gdt;
/* cpu/.../cpu.c */ int arch_cpu_init(void); int x86_cpu_init_f(void); int cpu_init_f(void); void init_gd(gd_t *id, u64 *gdt_addr); void setup_gdt(gd_t *id, u64 *gdt_addr); +void setup_fsp_gdt(void); int init_cache(void); int cleanup_before_linux(void); void panic_puts(const char *str); diff --git a/arch/x86/lib/fsp/fsp_support.c b/arch/x86/lib/fsp/fsp_support.c index 5f96da1..cf6ed04 100644 --- a/arch/x86/lib/fsp/fsp_support.c +++ b/arch/x86/lib/fsp/fsp_support.c @@ -173,6 +173,9 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
post_code(POST_PRE_MRC);
/* Load GDT for FSP */setup_fsp_gdt();Where does the GDT get set back to the normal U-Boot one?
In arch/x86/cpu/start.S where U-Boot setup its new GDT with a new FS segment descriptor.
/* * Use ASM code to ensure the register value in EAX & ECX * will be passed into BlContinuationFunc--
Regards, Bin
participants (2)
-
Bin Meng -
Simon Glass