ping... On 2015/7/29 19:03, Rui Xiang wrote: > From: Suresh Siddha <suresh.b.siddha@xxxxxxxxx> > > commit b1a74bf8212367be2b1d6685c11a84e056eaaaf1 upstream. > > Preemption is disabled between kernel_fpu_begin/end() and as such > it is not a good idea to use these routines in kvm_load/put_guest_fpu() > which can be very far apart. > > kvm_load/put_guest_fpu() routines are already called with > preemption disabled and KVM already uses the preempt notifier to save > the guest fpu state using kvm_put_guest_fpu(). > > So introduce __kernel_fpu_begin/end() routines which don't touch > preemption and use them instead of kernel_fpu_begin/end() > for KVM's use model of saving/restoring guest FPU state. > > Also with this change (and with eagerFPU model), fix the host cr0.TS vm-exit > state in the case of VMX. For eagerFPU case, host cr0.TS is always clear. > So no need to worry about it. For the traditional lazyFPU restore case, > change the cr0.TS bit for the host state during vm-exit to be always clear > and cr0.TS bit is set in the __vmx_load_host_state() when the FPU > (guest FPU or the host task's FPU) state is not active. This ensures > that the host/guest FPU state is properly saved, restored > during context-switch and with interrupts (using irq_fpu_usable()) not > stomping on the active FPU state. > > Signed-off-by: Suresh Siddha <suresh.b.siddha@xxxxxxxxx> > Link: http://lkml.kernel.org/r/1348164109.26695.338.camel@xxxxxxxxxxxxxxxxxxxxxxxxxx > Cc: Avi Kivity <avi@xxxxxxxxxx> > Signed-off-by: H. Peter Anvin <hpa@xxxxxxxxxxxxxxx> > [xr: Backported to 3.4: Adjust context] > Signed-off-by: Rui Xiang <rui.xiang@xxxxxxxxxx> > --- > arch/x86/include/asm/i387.h | 28 ++++++++++++++++++++++++++-- > arch/x86/kernel/i387.c | 13 +++++-------- > arch/x86/kvm/vmx.c | 10 +++++++--- > arch/x86/kvm/x86.c | 4 ++-- > 4 files changed, 40 insertions(+), 15 deletions(-) > > diff --git a/arch/x86/include/asm/i387.h b/arch/x86/include/asm/i387.h > index 257d9cc..1262fb6 100644 > --- a/arch/x86/include/asm/i387.h > +++ b/arch/x86/include/asm/i387.h > @@ -23,8 +23,32 @@ extern int dump_fpu(struct pt_regs *, struct user_i387_struct *); > extern void math_state_restore(void); > > extern bool irq_fpu_usable(void); > -extern void kernel_fpu_begin(void); > -extern void kernel_fpu_end(void); > + > +/* > + * Careful: __kernel_fpu_begin/end() must be called with preempt disabled > + * and they don't touch the preempt state on their own. > + * If you enable preemption after __kernel_fpu_begin(), preempt notifier > + * should call the __kernel_fpu_end() to prevent the kernel/user FPU > + * state from getting corrupted. KVM for example uses this model. > + * > + * All other cases use kernel_fpu_begin/end() which disable preemption > + * during kernel FPU usage. > + */ > +extern void __kernel_fpu_begin(void); > +extern void __kernel_fpu_end(void); > + > +static inline void kernel_fpu_begin(void) > +{ > + WARN_ON_ONCE(!irq_fpu_usable()); > + preempt_disable(); > + __kernel_fpu_begin(); > +} > + > +static inline void kernel_fpu_end(void) > +{ > + __kernel_fpu_end(); > + preempt_enable(); > +} > > /* > * Some instructions like VIA's padlock instructions generate a spurious > diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c > index 6610e81..7aa728d 100644 > --- a/arch/x86/kernel/i387.c > +++ b/arch/x86/kernel/i387.c > @@ -77,29 +77,26 @@ bool irq_fpu_usable(void) > } > EXPORT_SYMBOL(irq_fpu_usable); > > -void kernel_fpu_begin(void) > +void __kernel_fpu_begin(void) > { > struct task_struct *me = current; > > - WARN_ON_ONCE(!irq_fpu_usable()); > - preempt_disable(); > if (__thread_has_fpu(me)) { > __save_init_fpu(me); > __thread_clear_has_fpu(me); > - /* We do 'stts()' in kernel_fpu_end() */ > + /* We do 'stts()' in __kernel_fpu_end() */ > } else { > percpu_write(fpu_owner_task, NULL); > clts(); > } > } > -EXPORT_SYMBOL(kernel_fpu_begin); > +EXPORT_SYMBOL(__kernel_fpu_begin); > > -void kernel_fpu_end(void) > +void __kernel_fpu_end(void) > { > stts(); > - preempt_enable(); > } > -EXPORT_SYMBOL(kernel_fpu_end); > +EXPORT_SYMBOL(__kernel_fpu_end); > > void unlazy_fpu(struct task_struct *tsk) > { > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 2eb4e5a..4ad0d71 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -1455,8 +1455,12 @@ static void __vmx_load_host_state(struct vcpu_vmx *vmx) > #ifdef CONFIG_X86_64 > wrmsrl(MSR_KERNEL_GS_BASE, vmx->msr_host_kernel_gs_base); > #endif > - if (user_has_fpu()) > - clts(); > + /* > + * If the FPU is not active (through the host task or > + * the guest vcpu), then restore the cr0.TS bit. > + */ > + if (!user_has_fpu() && !vmx->vcpu.guest_fpu_loaded) > + stts(); > load_gdt(&__get_cpu_var(host_gdt)); > } > > @@ -3633,7 +3637,7 @@ static void vmx_set_constant_host_state(struct vcpu_vmx *vmx) > struct desc_ptr dt; > unsigned long cr4; > > - vmcs_writel(HOST_CR0, read_cr0() | X86_CR0_TS); /* 22.2.3 */ > + vmcs_writel(HOST_CR0, read_cr0() & ~X86_CR0_TS); /* 22.2.3 */ > vmcs_writel(HOST_CR3, read_cr3()); /* 22.2.3 FIXME: shadow tables */ > > /* Save the most likely value for this task's CR4 in the VMCS. */ > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 55ee4ca..4ad2b7b 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -5907,7 +5907,7 @@ void kvm_load_guest_fpu(struct kvm_vcpu *vcpu) > */ > kvm_put_guest_xcr0(vcpu); > vcpu->guest_fpu_loaded = 1; > - kernel_fpu_begin(); > + __kernel_fpu_begin(); > fpu_restore_checking(&vcpu->arch.guest_fpu); > trace_kvm_fpu(1); > } > @@ -5921,7 +5921,7 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu) > > vcpu->guest_fpu_loaded = 0; > fpu_save_init(&vcpu->arch.guest_fpu); > - kernel_fpu_end(); > + __kernel_fpu_end(); > ++vcpu->stat.fpu_reload; > kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); > trace_kvm_fpu(0); > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
