diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index 8ddb12ccc6..20c0bd02d1 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -431,6 +431,16 @@ unsigned long pv_make_cr4(const struct vcpu *v) if ( d->arch.vtsc || (v->arch.pv_vcpu.ctrlreg[4] & X86_CR4_TSD) ) cr4 |= X86_CR4_TSD; + /* + * The {RD,WR}{FS,GS}BASE are only useable in 64bit code segments. While + * we must not have CR4.FSGSBASE set behind the back of a 64bit PV kernel, + * we do leave it set in 32bit PV context to speed up Xen's context switch + * path. + */ + if ( !is_pv_32bit_domain(d) && + !(v->arch.pv_vcpu.ctrlreg[4] & X86_CR4_FSGSBASE) ) + cr4 &= ~X86_CR4_FSGSBASE; + return cr4; } @@ -2012,7 +2022,8 @@ static void save_segments(struct vcpu *v) regs->fs = read_sreg(fs); regs->gs = read_sreg(gs); - if ( cpu_has_fsgsbase && !is_pv_32bit_vcpu(v) ) + /* %fs/%gs bases can only be stale if WR{FS,GS}BASE are usable. */ + if ( (read_cr4() & X86_CR4_FSGSBASE) && !is_pv_32bit_vcpu(v) ) { v->arch.pv_vcpu.fs_base = __rdfsbase(); if ( v->arch.flags & TF_kernel_mode ) diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index 0072de75f9..9eca57acf7 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1442,7 +1442,7 @@ void __init noreturn __start_xen(unsigned long mbi_p) cr4_pv32_mask = mmu_cr4_features & XEN_CR4_PV32_BITS; - if ( cpu_has_fsgsbase ) + if ( boot_cpu_has(X86_FEATURE_FSGSBASE) ) set_in_cr4(X86_CR4_FSGSBASE); if ( opt_invpcid && cpu_has_invpcid ) diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index 6cbbf3ff2e..583936e9cb 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -2722,6 +2722,17 @@ static int emulate_privileged_op(struct cpu_user_regs *regs) } case 4: /* Write CR4 */ + /* + * If this write will disable FSGSBASE, refresh Xen's idea of the + * guest bases now that they can no longer change. + */ + if ( (v->arch.pv_vcpu.ctrlreg[4] & X86_CR4_FSGSBASE) && + !(*reg & X86_CR4_FSGSBASE) ) + { + v->arch.pv_vcpu.fs_base = __rdfsbase(); + v->arch.pv_vcpu.gs_base_kernel = __rdgsbase(); + } + v->arch.pv_vcpu.ctrlreg[4] = pv_fixup_guest_cr4(v, *reg); write_cr4(pv_make_cr4(v)); ctxt_switch_levelling(v); @@ -2993,13 +3004,14 @@ static int emulate_privileged_op(struct cpu_user_regs *regs) case MSR_FS_BASE: if ( is_pv_32bit_domain(currd) ) goto fail; - val = cpu_has_fsgsbase ? __rdfsbase() : v->arch.pv_vcpu.fs_base; + val = (read_cr4() & X86_CR4_FSGSBASE) ? __rdfsbase() + : v->arch.pv_vcpu.fs_base; goto rdmsr_writeback; case MSR_GS_BASE: if ( is_pv_32bit_domain(currd) ) goto fail; - val = cpu_has_fsgsbase ? __rdgsbase() - : v->arch.pv_vcpu.gs_base_kernel; + val = (read_cr4() & X86_CR4_FSGSBASE) ? __rdgsbase() + : v->arch.pv_vcpu.gs_base_kernel; goto rdmsr_writeback; case MSR_SHADOW_GS_BASE: if ( is_pv_32bit_domain(currd) ) diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c index 56ed156753..04cc60dd1f 100644 --- a/xen/arch/x86/x86_64/traps.c +++ b/xen/arch/x86/x86_64/traps.c @@ -265,7 +265,9 @@ void toggle_guest_mode(struct vcpu *v) { if ( is_pv_32bit_vcpu(v) ) return; - if ( cpu_has_fsgsbase ) + + /* %fs/%gs bases can only be stale if WR{FS,GS}BASE are usable. */ + if ( read_cr4() & X86_CR4_FSGSBASE ) { if ( v->arch.flags & TF_kernel_mode ) v->arch.pv_vcpu.gs_base_kernel = __rdgsbase(); diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h index 04d9e28e9b..f9c83352d4 100644 --- a/xen/include/asm-x86/cpufeature.h +++ b/xen/include/asm-x86/cpufeature.h @@ -72,7 +72,6 @@ #define cpu_has_nx boot_cpu_has(X86_FEATURE_NX) #define cpu_has_clflush boot_cpu_has(X86_FEATURE_CLFLUSH) #define cpu_has_page1gb boot_cpu_has(X86_FEATURE_PAGE1GB) -#define cpu_has_fsgsbase boot_cpu_has(X86_FEATURE_FSGSBASE) #define cpu_has_aperfmperf boot_cpu_has(X86_FEATURE_APERFMPERF) #define cpu_has_smep boot_cpu_has(X86_FEATURE_SMEP) #define cpu_has_invpcid boot_cpu_has(X86_FEATURE_INVPCID) diff --git a/xen/include/asm-x86/msr.h b/xen/include/asm-x86/msr.h index 4b4c15668c..252cbdcce6 100644 --- a/xen/include/asm-x86/msr.h +++ b/xen/include/asm-x86/msr.h @@ -93,6 +93,14 @@ static inline uint64_t rdtsc(void) : "=a" (low), "=d" (high) \ : "c" (counter)) +/* + * On hardware supporting FSGSBASE, the value loaded into hardware is the + * guest kernel's choice for 64bit PV guests (Xen's choice for Idle, HVM and + * 32bit PV). + * + * Therefore, the {RD,WR}{FS,GS}BASE instructions are only safe to use if + * %cr4.fsgsbase is set. + */ static inline unsigned long __rdfsbase(void) { unsigned long base; @@ -123,7 +131,7 @@ static inline unsigned long rdfsbase(void) { unsigned long base; - if ( cpu_has_fsgsbase ) + if ( read_cr4() & X86_CR4_FSGSBASE ) return __rdfsbase(); rdmsrl(MSR_FS_BASE, base); @@ -135,7 +143,7 @@ static inline unsigned long rdgsbase(void) { unsigned long base; - if ( cpu_has_fsgsbase ) + if ( read_cr4() & X86_CR4_FSGSBASE ) return __rdgsbase(); rdmsrl(MSR_GS_BASE, base); @@ -145,7 +153,7 @@ static inline unsigned long rdgsbase(void) static inline void wrfsbase(unsigned long base) { - if ( cpu_has_fsgsbase ) + if ( read_cr4() & X86_CR4_FSGSBASE ) #ifdef HAVE_GAS_FSGSBASE asm volatile ( "wrfsbase %0" :: "r" (base) ); #else @@ -157,7 +165,7 @@ static inline void wrfsbase(unsigned long base) static inline void wrgsbase(unsigned long base) { - if ( cpu_has_fsgsbase ) + if ( read_cr4() & X86_CR4_FSGSBASE ) #ifdef HAVE_GAS_FSGSBASE asm volatile ( "wrgsbase %0" :: "r" (base) ); #else diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h index 6a35b89138..610b6f3c26 100644 --- a/xen/include/asm-x86/processor.h +++ b/xen/include/asm-x86/processor.h @@ -363,11 +363,31 @@ static inline unsigned long read_cr4(void) static inline void write_cr4(unsigned long val) { + struct cpu_info *info = get_cpu_info(); + /* No global pages in case of PCIDs enabled! */ ASSERT(!(val & X86_CR4_PGE) || !(val & X86_CR4_PCIDE)); - get_cpu_info()->cr4 = val; - asm volatile ( "mov %0,%%cr4" : : "r" (val) ); + /* + * On hardware supporting FSGSBASE, the value in %cr4 is the kernel's + * choice for 64bit PV guests, which impacts whether Xen can use the + * instructions. + * + * The {rd,wr}{fs,gs}base() helpers use info->cr4 to work out whether it + * is safe to execute the {RD,WR}{FS,GS}BASE instruction, falling back to + * the MSR path if not. Some users require interrupt safety. + * + * If FSGSBASE is currently or about to become clear, reflect this in + * info->cr4 before updating %cr4, so an interrupt which hits in the + * middle won't observe FSGSBASE set in info->cr4 but clear in %cr4. + */ + info->cr4 = val & (info->cr4 | ~X86_CR4_FSGSBASE); + + asm volatile ( "mov %[val], %%cr4" + : "+m" (info->cr4) /* Force ordering without a barrier. */ + : [val] "r" (val) ); + + info->cr4 = val; } /* Clear and set 'TS' bit respectively */