public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2 v2] perf-kvm support for SVM
@ 2011-01-14 15:45 Joerg Roedel
  2011-01-14 15:45 ` [PATCH 1/2] KVM: SVM: Make sure KERNEL_GS_BASE is valid when loading gs_index Joerg Roedel
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Joerg Roedel @ 2011-01-14 15:45 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, linux-kernel

Hi,

here is the reworked version of the patch-set. Only patch 1/2 has
changed and now contains the real fix for the crashes that were seen and
has an updated log message.

Regards,

	Joerg

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] KVM: SVM: Make sure KERNEL_GS_BASE is valid when loading gs_index
  2011-01-14 15:45 [PATCH 0/2 v2] perf-kvm support for SVM Joerg Roedel
@ 2011-01-14 15:45 ` Joerg Roedel
  2011-01-14 15:45 ` [PATCH 2/2] KVM: SVM: Add support for perf-kvm Joerg Roedel
  2011-01-16 10:49 ` [PATCH 0/2 v2] perf-kvm support for SVM Avi Kivity
  2 siblings, 0 replies; 7+ messages in thread
From: Joerg Roedel @ 2011-01-14 15:45 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: Joerg Roedel, linux-kernel, kvm, stable

The gs_index loading code uses the swapgs instruction to
switch to the user gs_base temporarily. This is unsave in an
lightweight exit-path in KVM on AMD because the
KERNEL_GS_BASE MSR is switches lazily. An NMI happening in
the critical path of load_gs_index may use the wrong GS_BASE
value then leading to unpredictable behavior, e.g. a
triple-fault.
This patch fixes the issue by making sure that load_gs_index
is called only with a valid KERNEL_GS_BASE value loaded in
KVM.

Cc: stable@kernel.org
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 25bd1bc..54ce246 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1150,8 +1150,8 @@ static void svm_vcpu_put(struct kvm_vcpu *vcpu)
 	kvm_load_ldt(svm->host.ldt);
 #ifdef CONFIG_X86_64
 	loadsegment(fs, svm->host.fs);
-	load_gs_index(svm->host.gs);
 	wrmsrl(MSR_KERNEL_GS_BASE, current->thread.gs);
+	load_gs_index(svm->host.gs);
 #else
 	loadsegment(gs, svm->host.gs);
 #endif
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] KVM: SVM: Add support for perf-kvm
  2011-01-14 15:45 [PATCH 0/2 v2] perf-kvm support for SVM Joerg Roedel
  2011-01-14 15:45 ` [PATCH 1/2] KVM: SVM: Make sure KERNEL_GS_BASE is valid when loading gs_index Joerg Roedel
@ 2011-01-14 15:45 ` Joerg Roedel
  2011-01-16 10:49 ` [PATCH 0/2 v2] perf-kvm support for SVM Avi Kivity
  2 siblings, 0 replies; 7+ messages in thread
From: Joerg Roedel @ 2011-01-14 15:45 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, linux-kernel, Joerg Roedel

This patch adds the necessary code to run perf-kvm on AMD
machines.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 54ce246..73a8f1d 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3645,13 +3645,21 @@ static void svm_vcpu_run(struct kvm_vcpu *vcpu)
 
 	local_irq_disable();
 
-	stgi();
-
 	vcpu->arch.cr2 = svm->vmcb->save.cr2;
 	vcpu->arch.regs[VCPU_REGS_RAX] = svm->vmcb->save.rax;
 	vcpu->arch.regs[VCPU_REGS_RSP] = svm->vmcb->save.rsp;
 	vcpu->arch.regs[VCPU_REGS_RIP] = svm->vmcb->save.rip;
 
+	if (unlikely(svm->vmcb->control.exit_code == SVM_EXIT_NMI))
+		kvm_before_handle_nmi(&svm->vcpu);
+
+	stgi();
+
+	/* Any pending NMI will happen here */
+
+	if (unlikely(svm->vmcb->control.exit_code == SVM_EXIT_NMI))
+		kvm_after_handle_nmi(&svm->vcpu);
+
 	sync_cr8_to_lapic(vcpu);
 
 	svm->next_rip = 0;
-- 
1.7.1



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2 v2] perf-kvm support for SVM
  2011-01-14 15:45 [PATCH 0/2 v2] perf-kvm support for SVM Joerg Roedel
  2011-01-14 15:45 ` [PATCH 1/2] KVM: SVM: Make sure KERNEL_GS_BASE is valid when loading gs_index Joerg Roedel
  2011-01-14 15:45 ` [PATCH 2/2] KVM: SVM: Add support for perf-kvm Joerg Roedel
@ 2011-01-16 10:49 ` Avi Kivity
  2011-01-16 15:35   ` Joerg Roedel
  2 siblings, 1 reply; 7+ messages in thread
From: Avi Kivity @ 2011-01-16 10:49 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: Marcelo Tosatti, kvm, linux-kernel

On 01/14/2011 05:45 PM, Joerg Roedel wrote:
> Hi,
>
> here is the reworked version of the patch-set. Only patch 1/2 has
> changed and now contains the real fix for the crashes that were seen and
> has an updated log message.
>

Thanks, applied.  2.6.37 and earlier aren't affected, yes?  So I'm 
queuing it for 2.6.38 only.

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2 v2] perf-kvm support for SVM
  2011-01-16 10:49 ` [PATCH 0/2 v2] perf-kvm support for SVM Avi Kivity
@ 2011-01-16 15:35   ` Joerg Roedel
  2011-01-16 15:38     ` Avi Kivity
  0 siblings, 1 reply; 7+ messages in thread
From: Joerg Roedel @ 2011-01-16 15:35 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Joerg Roedel, Marcelo Tosatti, kvm, linux-kernel

On Sun, Jan 16, 2011 at 12:49:41PM +0200, Avi Kivity wrote:
> On 01/14/2011 05:45 PM, Joerg Roedel wrote:

>> here is the reworked version of the patch-set. Only patch 1/2 has
>> changed and now contains the real fix for the crashes that were seen and
>> has an updated log message.
>>
>
> Thanks, applied.  2.6.37 and earlier aren't affected, yes?  So I'm  
> queuing it for 2.6.38 only.

I think the problem is there since KVM has lazy state switching. So the
fix in patch 1 should make it in all currently maintained stable-trees.

	Joerg


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2 v2] perf-kvm support for SVM
  2011-01-16 15:35   ` Joerg Roedel
@ 2011-01-16 15:38     ` Avi Kivity
  2011-01-17 11:34       ` Roedel, Joerg
  0 siblings, 1 reply; 7+ messages in thread
From: Avi Kivity @ 2011-01-16 15:38 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: Joerg Roedel, Marcelo Tosatti, kvm, linux-kernel

On 01/16/2011 05:35 PM, Joerg Roedel wrote:
> On Sun, Jan 16, 2011 at 12:49:41PM +0200, Avi Kivity wrote:
> >  On 01/14/2011 05:45 PM, Joerg Roedel wrote:
>
> >>  here is the reworked version of the patch-set. Only patch 1/2 has
> >>  changed and now contains the real fix for the crashes that were seen and
> >>  has an updated log message.
> >>
> >
> >  Thanks, applied.  2.6.37 and earlier aren't affected, yes?  So I'm
> >  queuing it for 2.6.38 only.
>
> I think the problem is there since KVM has lazy state switching. So the
> fix in patch 1 should make it in all currently maintained stable-trees.
>

The problem is with load_gs_index(), yes?  In 2.6.37 this is called 
before stgi(), so it's protected from nmi.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2 v2] perf-kvm support for SVM
  2011-01-16 15:38     ` Avi Kivity
@ 2011-01-17 11:34       ` Roedel, Joerg
  0 siblings, 0 replies; 7+ messages in thread
From: Roedel, Joerg @ 2011-01-17 11:34 UTC (permalink / raw)
  To: Avi Kivity
  Cc: Joerg Roedel, Marcelo Tosatti, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Sun, Jan 16, 2011 at 10:38:11AM -0500, Avi Kivity wrote:
> On 01/16/2011 05:35 PM, Joerg Roedel wrote:
> > On Sun, Jan 16, 2011 at 12:49:41PM +0200, Avi Kivity wrote:
> > >  On 01/14/2011 05:45 PM, Joerg Roedel wrote:
> >
> > >>  here is the reworked version of the patch-set. Only patch 1/2 has
> > >>  changed and now contains the real fix for the crashes that were seen and
> > >>  has an updated log message.
> > >>
> > >
> > >  Thanks, applied.  2.6.37 and earlier aren't affected, yes?  So I'm
> > >  queuing it for 2.6.38 only.
> >
> > I think the problem is there since KVM has lazy state switching. So the
> > fix in patch 1 should make it in all currently maintained stable-trees.
> >
> 
> The problem is with load_gs_index(), yes?  In 2.6.37 this is called 
> before stgi(), so it's protected from nmi.

Ok, you are right :) So the fix is only necessary for 2.6.38.

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2011-01-17 11:34 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-14 15:45 [PATCH 0/2 v2] perf-kvm support for SVM Joerg Roedel
2011-01-14 15:45 ` [PATCH 1/2] KVM: SVM: Make sure KERNEL_GS_BASE is valid when loading gs_index Joerg Roedel
2011-01-14 15:45 ` [PATCH 2/2] KVM: SVM: Add support for perf-kvm Joerg Roedel
2011-01-16 10:49 ` [PATCH 0/2 v2] perf-kvm support for SVM Avi Kivity
2011-01-16 15:35   ` Joerg Roedel
2011-01-16 15:38     ` Avi Kivity
2011-01-17 11:34       ` Roedel, Joerg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox