x86, kvm: fix kvm's usage of kernel_fpu_begin/end()

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@intel.com>
Link: http://lkml.kernel.org/r/1348164109.26695.338.camel@sbsiddha-desk.sc.intel.com
Cc: Avi Kivity <avi@redhat.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>

diff --git a/arch/x86/include/asm/i387.h b/arch/x86/include/asm/i387.h
index 6c3bd3782818e0c765571f965e2c11f17c89ad15..ed8089d69094066985794cf4db39c04ecd545379 100644 (file)
--- a/arch/x86/include/asm/i387.h
+++ b/arch/x86/include/asm/i387.h
@@ -24,8 +24,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 6782e3983865c4daf1be142705e23072bdaae4d5..675a05012449d32ccaaa4abf0a0fb4abd095ed84 100644 (file)
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -73,32 +73,29 @@ 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 if (!use_eager_fpu()) {
                this_cpu_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)
 {
        if (use_eager_fpu())
                math_state_restore();
        else
                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 c00f03de1b794af8fe65387747813d57ed2885a2..70dfcec3c46321229addc7021b9255f1a8c8edaa 100644 (file)
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1493,8 +1493,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));
 }
 
@@ -3730,7 +3734,7 @@ static void vmx_set_constant_host_state(void)
        unsigned long tmpl;
        struct desc_ptr dt;
 
-       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_CR4, read_cr4());  /* 22.2.3, 22.2.5 */
        vmcs_writel(HOST_CR3, read_cr3());  /* 22.2.3  FIXME: shadow tables */
 

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index cf637f572bdd5ed4170274942b3828da93cce380..02b2cd520693563e62659e1983618785433a47a2 100644 (file)
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5972,7 +5972,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);
 }
@@ -5986,7 +5986,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);