[PATCH 0/2 v2] perf-kvm support for SVM

[PATCH 0/2 v2] perf-kvm support for SVM

[Joerg Roedel]

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

[PATCH 1/2] KVM: SVM: Make sure KERNEL_GS_BASE is valid when loading gs_index

[Joerg Roedel]

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

[PATCH 2/2] KVM: SVM: Add support for perf-kvm

[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

Re: [PATCH 0/2 v2] perf-kvm support for SVM

[Avi Kivity]

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

Re: [PATCH 0/2 v2] perf-kvm support for SVM

[Joerg Roedel]

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

Re: [PATCH 0/2 v2] perf-kvm support for SVM

[Avi Kivity]

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

Re: [PATCH 0/2 v2] perf-kvm support for SVM

[Roedel, Joerg]

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

[PATCH 0/2] perf-kvm support for SVM

[Joerg Roedel]

Hi Avi, Marcelo,

these two patches finally implement perf-kvm support for AMD machines.
The meat is in the second patch. The first one is an important fix
which, when missing, causes system crashes when NMI happen while in
guest mode. So the first patch should also make it to the various
stable-queues.

Regards,

	Joerg

[PATCH 2/2] KVM: SVM: Add support for perf-kvm

[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 8b9bc72..2415129 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3646,13 +3646,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