Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1356180 > unrolled thread

[PATCH 0/1] KVM: x86: using the fpu in interrupt context with a guest's xcr0

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/1] KVM: x86: using the fpu in interrupt context with a guest's xcr0 David Matlack <dmatlack@google.com> - 2016-03-11 21:50 +0100
    [PATCH 1/1] KVM: don't allow irq_fpu_usable when the VCPU's XCR0 is loaded David Matlack <dmatlack@google.com> - 2016-03-11 21:50 +0100
      Re: [PATCH 1/1] KVM: don't allow irq_fpu_usable when the VCPU's XCR0  is loaded Andy Lutomirski <luto@amacapital.net> - 2016-03-11 22:20 +0100
        Re: [PATCH 1/1] KVM: don't allow irq_fpu_usable when the VCPU's XCR0  is loaded David Matlack <dmatlack@google.com> - 2016-03-11 22:40 +0100
          Re: [PATCH 1/1] KVM: don't allow irq_fpu_usable when the VCPU's XCR0  is loaded Paolo Bonzini <pbonzini@redhat.com> - 2016-03-14 14:20 +0100
            Re: [PATCH 1/1] KVM: don't allow irq_fpu_usable when the VCPU's XCR0  is loaded Andy Lutomirski <luto@amacapital.net> - 2016-03-15 19:30 +0100
              Re: [PATCH 1/1] KVM: don't allow irq_fpu_usable when the VCPU's XCR0  is loaded Paolo Bonzini <pbonzini@redhat.com> - 2016-03-15 20:40 +0100
                Re: [PATCH 1/1] KVM: don't allow irq_fpu_usable when the VCPU's XCR0  is loaded Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-03-16 05:00 +0100
                  Re: [PATCH 1/1] KVM: don't allow irq_fpu_usable when the VCPU's XCR0  is loaded Paolo Bonzini <pbonzini@redhat.com> - 2016-03-16 13:10 +0100
    Re: [PATCH 0/1] KVM: x86: using the fpu in interrupt context with a  guest's xcr0 Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-03-14 08:50 +0100
      Re: [PATCH 0/1] KVM: x86: using the fpu in interrupt context with a  guest's xcr0 David Matlack <dmatlack@google.com> - 2016-03-15 20:10 +0100
        Re: [PATCH 0/1] KVM: x86: using the fpu in interrupt context with a  guest's xcr0 Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-03-16 04:50 +0100
          Re: [PATCH 0/1] KVM: x86: using the fpu in interrupt context with a  guest's xcr0 Andy Lutomirski <luto@amacapital.net> - 2016-03-16 04:50 +0100
            Re: [PATCH 0/1] KVM: x86: using the fpu in interrupt context with a  guest's xcr0 David Matlack <dmatlack@google.com> - 2016-03-16 18:20 +0100
          Re: [PATCH 0/1] KVM: x86: using the fpu in interrupt context with a  guest's xcr0 David Matlack <dmatlack@google.com> - 2016-03-16 18:10 +0100

#1356180 — [PATCH 0/1] KVM: x86: using the fpu in interrupt context with a guest's xcr0

We've found that an interrupt handler that uses the fpu can kill a KVM
VM, if it runs under the following conditions:
 - the guest's xcr0 register is loaded on the cpu
 - the guest's fpu context is not loaded
 - the host is using eagerfpu

Note that the guest's xcr0 register and fpu context are not loaded as
part of the atomic world switch into "guest mode". They are loaded by
KVM while the cpu is still in "host mode".

Usage of the fpu in interrupt context is gated by irq_fpu_usable(). The
interrupt handler will look something like this:

if (irq_fpu_usable()) {
	kernel_fpu_begin();

	[... code that uses the fpu ...]

	kernel_fpu_end();
}

As long as the guest's fpu is not loaded and the host is using eager
fpu, irq_fpu_usable() returns true (interrupted_kernel_fpu_idle()
returns true). The interrupt handler proceeds to use the fpu with
the guest's xcr0 live.

kernel_fpu_begin() saves the current fpu context. If this uses
XSAVE[OPT], it may leave the xsave area in an undesirable state.
According to the SDM, during XSAVE bit i of XSTATE_BV is not modified
if bit i is 0 in xcr0. So it's possible that XSTATE_BV[i] == 1 and
xcr0[i] == 0 following an XSAVE.

kernel_fpu_end() restores the fpu context. Now if any bit i in
XSTATE_BV is 1 while xcr0[i] is 0, XRSTOR generates a #GP fault.
(This #GP gets trapped and turned into a SIGSEGV, which kills the
VM.)

In guests that have access to the same CPU features as the host, this
bug is more likely to reproduce during VM boot, while the guest xcr0
is 1. Once the guest's xcr0 is indistinguishable from the host's,
there is no issue.

I have not been able to trigger this bug on Linux 4.3, and suspect
it is due to this commit from Linux 4.2:

653f52c kvm,x86: load guest FPU context more eagerly

With this commit, as long as the host is using eagerfpu, the guest's
fpu is always loaded just before the guest's xcr0 (vcpu->fpu_active
is always 1 in the following snippet):

6569         if (vcpu->fpu_active)
6570                 kvm_load_guest_fpu(vcpu);
6571         kvm_load_guest_xcr0(vcpu);

When the guest's fpu is loaded, irq_fpu_usable() returns false.

We've included our workaround for this bug, which applies to Linux 3.11.
It does not apply cleanly to HEAD since the fpu subsystem was refactored
in Linux 4.2. While the latest kernel does not look vulnerable, we may
want to apply a fix to the vulnerable stable kernels.

An equally effective solution may be to just backport 653f52c to stable.

Attached here is a stress module we used to reproduce the bug. It
fires IPIs at all online CPUs and uses the fpu in the IPI handler. We
found that running this module while booting a VM was an extremely
effective way to kill said VM :). For the kernel developers who are
working to make eagerfpu the global default, this module might be a
useful stress test, especially when run in the background during
other tests.

--- 8< ---
 irq_fpu_stress.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)
 create mode 100644 irq_fpu_stress.c

diff --git a/irq_fpu_stress.c b/irq_fpu_stress.c
new file mode 100644
index 0000000..faa6ba3
--- /dev/null
+++ b/irq_fpu_stress.c
@@ -0,0 +1,95 @@
+/*
+ * For the duration of time this module is loaded, this module fires
+ * IPIs at all CPUs and tries to use the FPU on that CPU in irq
+ * context.
+ */
+#include <linux/futex.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/kprobes.h>
+#include <linux/signal.h>
+#include <linux/debugfs.h>
+#include <linux/fs.h>
+#include <linux/hardirq.h>
+#include <linux/workqueue.h>
+
+#include <asm/uaccess.h>
+#include <asm/bug.h>
+#include <asm/fpu/api.h>
+
+MODULE_LICENSE("GPL");
+
+#define MODNAME "irq_fpu_stress"
+#undef pr_fmt
+#define pr_fmt(fmt) MODNAME": "fmt
+
+struct workqueue_struct *work_queue;
+struct work_struct work;
+
+struct {
+	atomic_t irq_fpu_usable;
+	atomic_t irq_fpu_unusable;
+	unsigned long num_tests;
+} stats;
+
+bool done;
+
+static void test_irq_fpu(void *info)
+{
+	BUG_ON(!in_interrupt());
+
+	if (irq_fpu_usable()) {
+		atomic_inc(&stats.irq_fpu_usable);
+
+		kernel_fpu_begin();
+		kernel_fpu_end();
+	} else {
+		atomic_inc(&stats.irq_fpu_unusable);
+	}
+}
+
+static void do_work(struct work_struct *w)
+{
+	pr_info("starting test\n");
+
+	stats.num_tests = 0;
+	atomic_set(&stats.irq_fpu_usable, 0);
+	atomic_set(&stats.irq_fpu_unusable, 0);
+
+	while (!ACCESS_ONCE(done)) {
+		preempt_disable();
+		smp_call_function_many(
+			cpu_online_mask, test_irq_fpu, NULL, 1 /* wait */);
+		preempt_enable();
+
+		stats.num_tests++;
+
+		if (need_resched())
+			schedule();
+	}
+
+	pr_info("finished test\n");
+}
+
+int init_module(void)
+{
+	work_queue = create_singlethread_workqueue(MODNAME);
+
+	INIT_WORK(&work, do_work);
+	queue_work(work_queue, &work);
+
+	return 0;
+}
+
+void cleanup_module(void)
+{
+	ACCESS_ONCE(done) = true;
+
+	flush_workqueue(work_queue);
+	destroy_workqueue(work_queue);
+
+	pr_info("num_tests %lu, irq_fpu_usable %d, irq_fpu_unusable %d\n",
+		stats.num_tests,
+		atomic_read(&stats.irq_fpu_usable),
+		atomic_read(&stats.irq_fpu_unusable));
+}
--- 8< ---

Eric Northup (1):
  KVM: don't allow irq_fpu_usable when the VCPU's XCR0 is loaded

 arch/x86/include/asm/i387.h |  3 +++
 arch/x86/kernel/i387.c      | 10 ++++++++--
 arch/x86/kvm/x86.c          |  4 ++++
 3 files changed, 15 insertions(+), 2 deletions(-)

-- 
2.7.0.rc3.207.g0ac5344

[toc] | [next] | [standalone]

#1356181 — [PATCH 1/1] KVM: don't allow irq_fpu_usable when the VCPU's XCR0 is loaded

From: Eric Northup <digitaleric@google.com>

Add a percpu boolean, tracking whether a KVM vCPU is running on the
host CPU.  KVM will set and clear it as it loads/unloads guest XCR0.
(Note that the rest of the guest FPU load/restore is safe, because
kvm_load_guest_fpu and kvm_put_guest_fpu call __kernel_fpu_begin()
and __kernel_fpu_end(), respectively.)  irq_fpu_usable() will then
also check for this percpu boolean.
---
 arch/x86/include/asm/i387.h |  3 +++
 arch/x86/kernel/i387.c      | 10 ++++++++--
 arch/x86/kvm/x86.c          |  4 ++++
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/i387.h b/arch/x86/include/asm/i387.h
index ed8089d..ca2c173 100644
--- a/arch/x86/include/asm/i387.h
+++ b/arch/x86/include/asm/i387.h
@@ -14,6 +14,7 @@
 
 #include <linux/sched.h>
 #include <linux/hardirq.h>
+#include <linux/percpu.h>
 
 struct pt_regs;
 struct user_i387_struct;
@@ -25,6 +26,8 @@ extern void math_state_restore(void);
 
 extern bool irq_fpu_usable(void);
 
+DECLARE_PER_CPU(bool, kvm_xcr0_loaded);
+
 /*
  * Careful: __kernel_fpu_begin/end() must be called with preempt disabled
  * and they don't touch the preempt state on their own.
diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index b627746..9015828 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -19,6 +19,9 @@
 #include <asm/fpu-internal.h>
 #include <asm/user.h>
 
+DEFINE_PER_CPU(bool, kvm_xcr0_loaded);
+EXPORT_PER_CPU_SYMBOL(kvm_xcr0_loaded);
+
 /*
  * Were we in an interrupt that interrupted kernel mode?
  *
@@ -33,8 +36,11 @@
  */
 static inline bool interrupted_kernel_fpu_idle(void)
 {
-	if (use_eager_fpu())
-		return __thread_has_fpu(current);
+	if (use_eager_fpu()) {
+		/* Preempt already disabled, safe to read percpu. */
+		return __thread_has_fpu(current) &&
+			!__this_cpu_read(kvm_xcr0_loaded);
+	}
 
 	return !__thread_has_fpu(current) &&
 		(read_cr0() & X86_CR0_TS);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d21bce5..f0ba7a1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -557,8 +557,10 @@ EXPORT_SYMBOL_GPL(kvm_lmsw);
 
 static void kvm_load_guest_xcr0(struct kvm_vcpu *vcpu)
 {
+	BUG_ON(this_cpu_read(kvm_xcr0_loaded) != vcpu->guest_xcr0_loaded);
 	if (kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) &&
 			!vcpu->guest_xcr0_loaded) {
+		this_cpu_write(kvm_xcr0_loaded, 1);
 		/* kvm_set_xcr() also depends on this */
 		xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
 		vcpu->guest_xcr0_loaded = 1;
@@ -571,7 +573,9 @@ static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
 		if (vcpu->arch.xcr0 != host_xcr0)
 			xsetbv(XCR_XFEATURE_ENABLED_MASK, host_xcr0);
 		vcpu->guest_xcr0_loaded = 0;
+		this_cpu_write(kvm_xcr0_loaded, 0);
 	}
+	BUG_ON(this_cpu_read(kvm_xcr0_loaded) != vcpu->guest_xcr0_loaded);
 }
 
 int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
-- 
2.7.0.rc3.207.g0ac5344

[toc] | [prev] | [next] | [standalone]

#1356190 — Re: [PATCH 1/1] KVM: don't allow irq_fpu_usable when the VCPU's XCR0 is loaded

On Fri, Mar 11, 2016 at 12:47 PM, David Matlack <dmatlack@google.com> wrote:
> From: Eric Northup <digitaleric@google.com>
>
> Add a percpu boolean, tracking whether a KVM vCPU is running on the
> host CPU.  KVM will set and clear it as it loads/unloads guest XCR0.
> (Note that the rest of the guest FPU load/restore is safe, because
> kvm_load_guest_fpu and kvm_put_guest_fpu call __kernel_fpu_begin()
> and __kernel_fpu_end(), respectively.)  irq_fpu_usable() will then
> also check for this percpu boolean.

Is this better than just always keeping the host's XCR0 loaded outside
if the KVM interrupts-disabled region?

--Andy

[toc] | [prev] | [next] | [standalone]

#1356198 — Re: [PATCH 1/1] KVM: don't allow irq_fpu_usable when the VCPU's XCR0 is loaded

On Fri, Mar 11, 2016 at 1:14 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> On Fri, Mar 11, 2016 at 12:47 PM, David Matlack <dmatlack@google.com> wrote:
> > From: Eric Northup <digitaleric@google.com>
> >
> > Add a percpu boolean, tracking whether a KVM vCPU is running on the
> > host CPU.  KVM will set and clear it as it loads/unloads guest XCR0.
> > (Note that the rest of the guest FPU load/restore is safe, because
> > kvm_load_guest_fpu and kvm_put_guest_fpu call __kernel_fpu_begin()
> > and __kernel_fpu_end(), respectively.)  irq_fpu_usable() will then
> > also check for this percpu boolean.
>
> Is this better than just always keeping the host's XCR0 loaded outside
> if the KVM interrupts-disabled region?

Probably not. AFAICT KVM does not rely on it being loaded outside that
region. xsetbv isn't insanely expensive, is it? Maybe to minimize the
time spent with interrupts disabled it was put outside.

I do like that your solution would be contained to KVM.

>
> --Andy

[toc] | [prev] | [next] | [standalone]

#1357229 — Re: [PATCH 1/1] KVM: don't allow irq_fpu_usable when the VCPU's XCR0 is loaded

On 11/03/2016 22:33, David Matlack wrote:
> > Is this better than just always keeping the host's XCR0 loaded outside
> > if the KVM interrupts-disabled region?
> 
> Probably not. AFAICT KVM does not rely on it being loaded outside that
> region. xsetbv isn't insanely expensive, is it? Maybe to minimize the
> time spent with interrupts disabled it was put outside.
> 
> I do like that your solution would be contained to KVM.

I agree with Andy.  We do want a fix for recent kernels because of the
!eager_fpu case that Guangrong mentioned.

Paolo

ps: while Andy is planning to kill lazy FPU, I want to benchmark it with
KVM...  Remember that with a single pre-xsave host in your cluster, your
virt management might happily default your VMs to a Westmere or Nehalem
CPU model.  GCC might be a pretty good testbench for this (e.g. a kernel
compile with very high make -j), because outside of the lexer (which
plays SIMD games) it never uses the FPU.

[toc] | [prev] | [next] | [standalone]

#1358184 — Re: [PATCH 1/1] KVM: don't allow irq_fpu_usable when the VCPU's XCR0 is loaded

On Mon, Mar 14, 2016 at 6:17 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 11/03/2016 22:33, David Matlack wrote:
>> > Is this better than just always keeping the host's XCR0 loaded outside
>> > if the KVM interrupts-disabled region?
>>
>> Probably not. AFAICT KVM does not rely on it being loaded outside that
>> region. xsetbv isn't insanely expensive, is it? Maybe to minimize the
>> time spent with interrupts disabled it was put outside.
>>
>> I do like that your solution would be contained to KVM.
>
> I agree with Andy.  We do want a fix for recent kernels because of the
> !eager_fpu case that Guangrong mentioned.
>
> Paolo
>
> ps: while Andy is planning to kill lazy FPU, I want to benchmark it with
> KVM...  Remember that with a single pre-xsave host in your cluster, your
> virt management might happily default your VMs to a Westmere or Nehalem
> CPU model.  GCC might be a pretty good testbench for this (e.g. a kernel
> compile with very high make -j), because outside of the lexer (which
> plays SIMD games) it never uses the FPU.

Aren't pre-xsave CPUs really, really old?  A brief search suggests
that Intel Core added it somewhere in the middle of the cycle.

For pre-xsave, it could indeed hurt performance a tiny bit under
workloads that use the FPU and then stop completely because the
xsaveopt and init optimizations aren't available.  But even that is
probably a very small effect, especially because pre-xsave CPUs have
smaller FPU state sizes.


-- 
Andy Lutomirski
AMA Capital Management, LLC

[toc] | [prev] | [next] | [standalone]

#1358204 — Re: [PATCH 1/1] KVM: don't allow irq_fpu_usable when the VCPU's XCR0 is loaded

On 15/03/2016 19:27, Andy Lutomirski wrote:
> On Mon, Mar 14, 2016 at 6:17 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>
>> On 11/03/2016 22:33, David Matlack wrote:
>>>> Is this better than just always keeping the host's XCR0 loaded outside
>>>> if the KVM interrupts-disabled region?
>>>
>>> Probably not. AFAICT KVM does not rely on it being loaded outside that
>>> region. xsetbv isn't insanely expensive, is it? Maybe to minimize the
>>> time spent with interrupts disabled it was put outside.
>>>
>>> I do like that your solution would be contained to KVM.
>>
>> I agree with Andy.  We do want a fix for recent kernels because of the
>> !eager_fpu case that Guangrong mentioned.
>>
>> Paolo
>>
>> ps: while Andy is planning to kill lazy FPU, I want to benchmark it with
>> KVM...  Remember that with a single pre-xsave host in your cluster, your
>> virt management might happily default your VMs to a Westmere or Nehalem
>> CPU model.  GCC might be a pretty good testbench for this (e.g. a kernel
>> compile with very high make -j), because outside of the lexer (which
>> plays SIMD games) it never uses the FPU.
> 
> Aren't pre-xsave CPUs really, really old?  A brief search suggests
> that Intel Core added it somewhere in the middle of the cycle.

I am fairly sure it was added in Sandy Bridge, together with AVX. But
what really matters for eager FPU is not xsave, it's xsaveopt, and I
think AMD has never even produced a microprocessor that supports it.

> For pre-xsave, it could indeed hurt performance a tiny bit under
> workloads that use the FPU and then stop completely because the
> xsaveopt and init optimizations aren't available.  But even that is
> probably a very small effect, especially because pre-xsave CPUs have
> smaller FPU state sizes.

It's still a few cache lines.  Benchmarks will tell.

Paolo

[toc] | [prev] | [next] | [standalone]

#1358516 — Re: [PATCH 1/1] KVM: don't allow irq_fpu_usable when the VCPU's XCR0 is loaded

On 03/16/2016 03:32 AM, Paolo Bonzini wrote:
>
>
> On 15/03/2016 19:27, Andy Lutomirski wrote:
>> On Mon, Mar 14, 2016 at 6:17 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>
>>>
>>> On 11/03/2016 22:33, David Matlack wrote:
>>>>> Is this better than just always keeping the host's XCR0 loaded outside
>>>>> if the KVM interrupts-disabled region?
>>>>
>>>> Probably not. AFAICT KVM does not rely on it being loaded outside that
>>>> region. xsetbv isn't insanely expensive, is it? Maybe to minimize the
>>>> time spent with interrupts disabled it was put outside.
>>>>
>>>> I do like that your solution would be contained to KVM.
>>>
>>> I agree with Andy.  We do want a fix for recent kernels because of the
>>> !eager_fpu case that Guangrong mentioned.

Relying on interrupt is not easy as XCR0 can not be automatically saved/loaded
by VMCS... Once interrupt happens, it will use guest's XCR0 anyway.

[toc] | [prev] | [next] | [standalone]

#1358930 — Re: [PATCH 1/1] KVM: don't allow irq_fpu_usable when the VCPU's XCR0 is loaded

On 16/03/2016 04:55, Xiao Guangrong wrote:
>>>>>
>>>>> Probably not. AFAICT KVM does not rely on it being loaded outside that
>>>>> region. xsetbv isn't insanely expensive, is it? Maybe to minimize the
>>>>> time spent with interrupts disabled it was put outside.
>>>>>
>>>>> I do like that your solution would be contained to KVM.
>>>>
>>>> I agree with Andy.  We do want a fix for recent kernels because of the
>>>> !eager_fpu case that Guangrong mentioned.
> 
> Relying on interrupt is not easy as XCR0 can not be automatically
> saved/loaded by VMCS... Once interrupt happens, it will use guest's XCR0 anyway.

Right, that's why an xsetbv while interrupts are disabled is appealing.

Paolo

[toc] | [prev] | [next] | [standalone]

#1357018 — Re: [PATCH 0/1] KVM: x86: using the fpu in interrupt context with a guest's xcr0

On 03/12/2016 04:47 AM, David Matlack wrote:

> I have not been able to trigger this bug on Linux 4.3, and suspect
> it is due to this commit from Linux 4.2:
>
> 653f52c kvm,x86: load guest FPU context more eagerly
>
> With this commit, as long as the host is using eagerfpu, the guest's
> fpu is always loaded just before the guest's xcr0 (vcpu->fpu_active
> is always 1 in the following snippet):
>
> 6569         if (vcpu->fpu_active)
> 6570                 kvm_load_guest_fpu(vcpu);
> 6571         kvm_load_guest_xcr0(vcpu);
>
> When the guest's fpu is loaded, irq_fpu_usable() returns false.

Er, i did not see that commit introduced this change.

>
> We've included our workaround for this bug, which applies to Linux 3.11.
> It does not apply cleanly to HEAD since the fpu subsystem was refactored
> in Linux 4.2. While the latest kernel does not look vulnerable, we may
> want to apply a fix to the vulnerable stable kernels.

Is the latest kvm safe if we use !eager fpu? Under this case, kvm_load_guest_fpu()
is not called for every single VM-enter, that means kernel will use guest's xcr0 to
save/restore XSAVE area.

Maybe a simpler fix is just calling __kernel_fpu_begin() when the CPU switches
to vCPU and reverts it when the vCPU is scheduled out or returns to userspace.

[toc] | [prev] | [next] | [standalone]

#1358196 — Re: [PATCH 0/1] KVM: x86: using the fpu in interrupt context with a guest's xcr0

On Mon, Mar 14, 2016 at 12:46 AM, Xiao Guangrong
<guangrong.xiao@linux.intel.com> wrote:
>
>
> On 03/12/2016 04:47 AM, David Matlack wrote:
>
>> I have not been able to trigger this bug on Linux 4.3, and suspect
>> it is due to this commit from Linux 4.2:
>>
>> 653f52c kvm,x86: load guest FPU context more eagerly
>>
>> With this commit, as long as the host is using eagerfpu, the guest's
>> fpu is always loaded just before the guest's xcr0 (vcpu->fpu_active
>> is always 1 in the following snippet):
>>
>> 6569         if (vcpu->fpu_active)
>> 6570                 kvm_load_guest_fpu(vcpu);
>> 6571         kvm_load_guest_xcr0(vcpu);
>>
>> When the guest's fpu is loaded, irq_fpu_usable() returns false.
>
>
> Er, i did not see that commit introduced this change.
>
>>
>> We've included our workaround for this bug, which applies to Linux 3.11.
>> It does not apply cleanly to HEAD since the fpu subsystem was refactored
>> in Linux 4.2. While the latest kernel does not look vulnerable, we may
>> want to apply a fix to the vulnerable stable kernels.
>
>
> Is the latest kvm safe if we use !eager fpu?

Yes I believe so. When !eagerfpu, interrupted_kernel_fpu_idle()
returns "!current->thread.fpu.fpregs_active && (read_cr0() &
X86_CR0_TS)". This should ensure the interrupt handler never does
XSAVE/XRSTOR with the guest's xcr0.

> Under this case,
> kvm_load_guest_fpu()
> is not called for every single VM-enter, that means kernel will use guest's
> xcr0 to
> save/restore XSAVE area.
>
> Maybe a simpler fix is just calling __kernel_fpu_begin() when the CPU
> switches
> to vCPU and reverts it when the vCPU is scheduled out or returns to
> userspace.
>

[toc] | [prev] | [next] | [standalone]

#1358512 — Re: [PATCH 0/1] KVM: x86: using the fpu in interrupt context with a guest's xcr0

On 03/16/2016 03:01 AM, David Matlack wrote:
> On Mon, Mar 14, 2016 at 12:46 AM, Xiao Guangrong
> <guangrong.xiao@linux.intel.com> wrote:
>>
>>
>> On 03/12/2016 04:47 AM, David Matlack wrote:
>>
>>> I have not been able to trigger this bug on Linux 4.3, and suspect
>>> it is due to this commit from Linux 4.2:
>>>
>>> 653f52c kvm,x86: load guest FPU context more eagerly
>>>
>>> With this commit, as long as the host is using eagerfpu, the guest's
>>> fpu is always loaded just before the guest's xcr0 (vcpu->fpu_active
>>> is always 1 in the following snippet):
>>>
>>> 6569         if (vcpu->fpu_active)
>>> 6570                 kvm_load_guest_fpu(vcpu);
>>> 6571         kvm_load_guest_xcr0(vcpu);
>>>
>>> When the guest's fpu is loaded, irq_fpu_usable() returns false.
>>
>>
>> Er, i did not see that commit introduced this change.
>>
>>>
>>> We've included our workaround for this bug, which applies to Linux 3.11.
>>> It does not apply cleanly to HEAD since the fpu subsystem was refactored
>>> in Linux 4.2. While the latest kernel does not look vulnerable, we may
>>> want to apply a fix to the vulnerable stable kernels.
>>
>>
>> Is the latest kvm safe if we use !eager fpu?
>
> Yes I believe so. When !eagerfpu, interrupted_kernel_fpu_idle()
> returns "!current->thread.fpu.fpregs_active && (read_cr0() &
> X86_CR0_TS)". This should ensure the interrupt handler never does
> XSAVE/XRSTOR with the guest's xcr0.


interrupted_kernel_fpu_idle() returns true if KVM-based hypervisor (e.g. QEMU)
is not using fpu.　That can not stop handler using fpu.

[toc] | [prev] | [next] | [standalone]

#1358513 — Re: [PATCH 0/1] KVM: x86: using the fpu in interrupt context with a guest's xcr0

On Tue, Mar 15, 2016 at 8:43 PM, Xiao Guangrong
<guangrong.xiao@linux.intel.com> wrote:
>
>
> On 03/16/2016 03:01 AM, David Matlack wrote:
>>
>> On Mon, Mar 14, 2016 at 12:46 AM, Xiao Guangrong
>> <guangrong.xiao@linux.intel.com> wrote:
>>>
>>>
>>>
>>> On 03/12/2016 04:47 AM, David Matlack wrote:
>>>
>>>> I have not been able to trigger this bug on Linux 4.3, and suspect
>>>> it is due to this commit from Linux 4.2:
>>>>
>>>> 653f52c kvm,x86: load guest FPU context more eagerly
>>>>
>>>> With this commit, as long as the host is using eagerfpu, the guest's
>>>> fpu is always loaded just before the guest's xcr0 (vcpu->fpu_active
>>>> is always 1 in the following snippet):
>>>>
>>>> 6569         if (vcpu->fpu_active)
>>>> 6570                 kvm_load_guest_fpu(vcpu);
>>>> 6571         kvm_load_guest_xcr0(vcpu);
>>>>
>>>> When the guest's fpu is loaded, irq_fpu_usable() returns false.
>>>
>>>
>>>
>>> Er, i did not see that commit introduced this change.
>>>
>>>>
>>>> We've included our workaround for this bug, which applies to Linux 3.11.
>>>> It does not apply cleanly to HEAD since the fpu subsystem was refactored
>>>> in Linux 4.2. While the latest kernel does not look vulnerable, we may
>>>> want to apply a fix to the vulnerable stable kernels.
>>>
>>>
>>>
>>> Is the latest kvm safe if we use !eager fpu?
>>
>>
>> Yes I believe so. When !eagerfpu, interrupted_kernel_fpu_idle()
>> returns "!current->thread.fpu.fpregs_active && (read_cr0() &
>> X86_CR0_TS)". This should ensure the interrupt handler never does
>> XSAVE/XRSTOR with the guest's xcr0.
>
>
>
> interrupted_kernel_fpu_idle() returns true if KVM-based hypervisor (e.g.
> QEMU)
> is not using fpu.　That can not stop handler using fpu.

Why is it safe to rely on interrupted_kernel_fpu_idle?  That function
is for interrupts, but is there any reason that KVM can't be preempted
(or explicitly schedule) with XCR0 having some funny value?

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC

[toc] | [prev] | [next] | [standalone]

#1359183 — Re: [PATCH 0/1] KVM: x86: using the fpu in interrupt context with a guest's xcr0

On Tue, Mar 15, 2016 at 8:48 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> Why is it safe to rely on interrupted_kernel_fpu_idle?  That function
> is for interrupts, but is there any reason that KVM can't be preempted
> (or explicitly schedule) with XCR0 having some funny value?

KVM restores the host's xcr0 in the sched-out preempt notifier and
prior to returning to userspace.

[toc] | [prev] | [next] | [standalone]

#1359176 — Re: [PATCH 0/1] KVM: x86: using the fpu in interrupt context with a guest's xcr0

On Tue, Mar 15, 2016 at 8:43 PM, Xiao Guangrong
<guangrong.xiao@linux.intel.com> wrote:
>
>
> On 03/16/2016 03:01 AM, David Matlack wrote:
>>
>> On Mon, Mar 14, 2016 at 12:46 AM, Xiao Guangrong
>> <guangrong.xiao@linux.intel.com> wrote:
>>>
>>> On 03/12/2016 04:47 AM, David Matlack wrote:
>>>
>>>> I have not been able to trigger this bug on Linux 4.3, and suspect
>>>> it is due to this commit from Linux 4.2:
>>>>
>>>> 653f52c kvm,x86: load guest FPU context more eagerly
>>>>
>>>> With this commit, as long as the host is using eagerfpu, the guest's
>>>> fpu is always loaded just before the guest's xcr0 (vcpu->fpu_active
>>>> is always 1 in the following snippet):
>>>>
>>>> 6569         if (vcpu->fpu_active)
>>>> 6570                 kvm_load_guest_fpu(vcpu);
>>>> 6571         kvm_load_guest_xcr0(vcpu);
>>>>
>>>> When the guest's fpu is loaded, irq_fpu_usable() returns false.
>>>
>>> Er, i did not see that commit introduced this change.
>>>
>>>>
>>>> We've included our workaround for this bug, which applies to Linux 3.11.
>>>> It does not apply cleanly to HEAD since the fpu subsystem was refactored
>>>> in Linux 4.2. While the latest kernel does not look vulnerable, we may
>>>> want to apply a fix to the vulnerable stable kernels.
>>>
>>> Is the latest kvm safe if we use !eager fpu?
>>
>> Yes I believe so. When !eagerfpu, interrupted_kernel_fpu_idle()
>> returns "!current->thread.fpu.fpregs_active && (read_cr0() &
>> X86_CR0_TS)". This should ensure the interrupt handler never does
>> XSAVE/XRSTOR with the guest's xcr0.
>
> interrupted_kernel_fpu_idle() returns true if KVM-based hypervisor (e.g.
> QEMU)
> is not using fpu.　That can not stop handler using fpu.

You are correct, the interrupt handler can still use the fpu. But
kernel_fpu_{begin,end} will not execute XSAVE / XRSTOR.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web