Hi Bin, On 4 June 2015 at 04:28, Bin Meng <[email protected]> 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 <[email protected]>
Tested on Minnowboard MAX: Tested-by: Simon Glass <[email protected]> > > --- > > Changes in v2: > - Use CONFIG_RESET_SEG_START to avoid duplication > > arch/x86/cpu/cpu.c | 20 ++++++++++++++++++++ > arch/x86/cpu/start16.S | 1 + > arch/x86/include/asm/u-boot-x86.h | 3 +++ > arch/x86/lib/fsp/fsp_support.c | 3 +++ > 4 files changed, 27 insertions(+) > > diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c > index bb4a110..f4ebc97 100644 > --- a/arch/x86/cpu/cpu.c > +++ b/arch/x86/cpu/cpu.c > @@ -164,6 +164,26 @@ void setup_gdt(gd_t *id, u64 *gdt_addr) > load_fs(X86_GDT_ENTRY_32BIT_FS); > } > > +#ifdef CONFIG_HAVE_FSP > +/* > + * 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 + CONFIG_RESET_SEG_START), 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); > +} > +#endif > + > 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: Could we rename this to gdt_initial or something like that? To me, gdt is a bit vague for an exported symbol. We need to use a name that indicates it is only used at the start. > /* > * The GDT table ... > diff --git a/arch/x86/include/asm/u-boot-x86.h > b/arch/x86/include/asm/u-boot-x86.h > index d1d21ed..67c0098 100644 > --- a/arch/x86/include/asm/u-boot-x86.h > +++ b/arch/x86/include/asm/u-boot-x86.h > @@ -8,12 +8,15 @@ > #ifndef _U_BOOT_I386_H_ > #define _U_BOOT_I386_H_ 1 > > +extern u32 gdt; Perhaps this should be declared as char gdt[] so you don't need to take its address later? See asm/linkage.h for how they do it. > + > /* 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); Please add function comment. > int init_cache(void); > int cleanup_before_linux(void); > > diff --git a/arch/x86/lib/fsp/fsp_support.c b/arch/x86/lib/fsp/fsp_support.c > index 5809235..4585166 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 > -- > 1.8.2.1 > Regards, Simon _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
