[PATCH 0/5] Another set of Nested SVM fixes and cleanups

[PATCH 0/5] Another set of Nested SVM fixes and cleanups

[Joerg Roedel]

Hi,

this series of patches contain another set of cleanups and an important fix to
the Nested SVM code. These patches make the TSC handling code for SVM aware of
a nested guest. This fixes the TSC running backwards on guest and nested guest.
The backwards running TSC resulted in stalled guests when kvm-clock was
enabled. Please consider to apply.

Thanks,

	Joerg

Shortlog:

Joerg Roedel (5):
      KVM: SVM: reorganize svm_interrupt_allowed
      KVM: SVM: don't copy exit_int_info on nested vmrun
      KVM: SVM: Fix tsc offset adjustment when running nested
      KVM: SVM: Handle tsc in svm_get_msr/svm_set_msr correctly
      KVM: SVM: Remove remaining occurences of rdtscll


Diffstat:

 arch/x86/kvm/svm.c |   50 ++++++++++++++++++++++++++++++++++----------------
 1 files changed, 34 insertions(+), 16 deletions(-)

[PATCH 1/5] KVM: SVM: reorganize svm_interrupt_allowed

[Joerg Roedel]

This patch reorganizes the logic in svm_interrupt_allowed to
make it better to read. This is important because the logic
is a lot more complicated with Nested SVM.

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index a2f2d43..668460c 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2454,10 +2454,18 @@ static int svm_interrupt_allowed(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 	struct vmcb *vmcb = svm->vmcb;
-	return (vmcb->save.rflags & X86_EFLAGS_IF) &&
-		!(vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) &&
-		gif_set(svm) &&
-		!(is_nested(svm) && (svm->vcpu.arch.hflags & HF_VINTR_MASK));
+	int ret;
+
+	if (!gif_set(svm) ||
+	     (vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK))
+		return 0;
+
+	ret = !!(vmcb->save.rflags & X86_EFLAGS_IF);
+
+	if (is_nested(svm))
+		return ret && !(svm->vcpu.arch.hflags & HF_VINTR_MASK);
+
+	return ret;
 }
 
 static void enable_irq_window(struct kvm_vcpu *vcpu)
-- 
1.6.3.3

[PATCH 2/5] KVM: SVM: don't copy exit_int_info on nested vmrun

[Joerg Roedel]

The exit_int_info field is only written by the hardware and
never read. So it does not need to be copied on a vmrun
emulation.

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 668460c..80f5309 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1790,8 +1790,6 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)
 	svm->nested.intercept            = nested_vmcb->control.intercept;
 
 	force_new_asid(&svm->vcpu);
-	svm->vmcb->control.exit_int_info = nested_vmcb->control.exit_int_info;
-	svm->vmcb->control.exit_int_info_err = nested_vmcb->control.exit_int_info_err;
 	svm->vmcb->control.int_ctl = nested_vmcb->control.int_ctl | V_INTR_MASKING_MASK;
 	if (nested_vmcb->control.int_ctl & V_IRQ_MASK) {
 		nsvm_printk("nSVM Injecting Interrupt: 0x%x\n",
-- 
1.6.3.3

[PATCH 3/5] KVM: SVM: Fix tsc offset adjustment when running nested

[Joerg Roedel]

When svm_vcpu_load is called while the vcpu is running in
guest mode the tsc adjustment made there is lost on the next
emulated #vmexit. This causes the tsc running backwards in
the guest. This patch fixes the issue by also adjusting the
tsc_offset in the emulated hsave area so that it will not
get lost.

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 80f5309..84c2c78 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -767,6 +767,8 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 		rdtscll(tsc_this);
 		delta = vcpu->arch.host_tsc - tsc_this;
 		svm->vmcb->control.tsc_offset += delta;
+		if (is_nested(svm))
+			svm->nested.hsave->control.tsc_offset += delta;
 		vcpu->cpu = cpu;
 		kvm_migrate_timers(vcpu);
 		svm->asid_generation = 0;
-- 
1.6.3.3

[PATCH 4/5] KVM: SVM: Handle tsc in svm_get_msr/svm_set_msr correctly

[Joerg Roedel]

When running nested we need to touch the l1 guests
tsc_offset. Otherwise changes will be lost or a wrong value
be read.

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 84c2c78..e193cf9 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2059,10 +2059,14 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 *data)
 
 	switch (ecx) {
 	case MSR_IA32_TSC: {
-		u64 tsc;
+		u64 tsc_offset;
 
-		rdtscll(tsc);
-		*data = svm->vmcb->control.tsc_offset + tsc;
+		if (is_nested(svm))
+			tsc_offset = svm->nested.hsave->control.tsc_offset;
+		else
+			tsc_offset = svm->vmcb->control.tsc_offset;
+
+		*data = tsc_offset + native_read_tsc();
 		break;
 	}
 	case MSR_K6_STAR:
@@ -2148,10 +2152,17 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 data)
 
 	switch (ecx) {
 	case MSR_IA32_TSC: {
-		u64 tsc;
+		u64 tsc_offset = data - native_read_tsc();
+		u64 g_tsc_offset = 0;
+
+		if (is_nested(svm)) {
+			g_tsc_offset = svm->vmcb->control.tsc_offset -
+				       svm->nested.hsave->control.tsc_offset;
+			svm->nested.hsave->control.tsc_offset = tsc_offset;
+		}
+
+		svm->vmcb->control.tsc_offset = tsc_offset + g_tsc_offset;
 
-		rdtscll(tsc);
-		svm->vmcb->control.tsc_offset = data - tsc;
 		break;
 	}
 	case MSR_K6_STAR:
-- 
1.6.3.3

[PATCH 5/5] KVM: SVM: Remove remaining occurences of rdtscll

[Joerg Roedel]

This patch replaces them with native_read_tsc() which can
also be used in expressions and saves a variable on the
stack in this case.

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index e193cf9..12cef2c 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -758,14 +758,13 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 	int i;
 
 	if (unlikely(cpu != vcpu->cpu)) {
-		u64 tsc_this, delta;
+		u64 delta;
 
 		/*
 		 * Make sure that the guest sees a monotonically
 		 * increasing TSC.
 		 */
-		rdtscll(tsc_this);
-		delta = vcpu->arch.host_tsc - tsc_this;
+		delta = vcpu->arch.host_tsc - native_read_tsc();
 		svm->vmcb->control.tsc_offset += delta;
 		if (is_nested(svm))
 			svm->nested.hsave->control.tsc_offset += delta;
@@ -787,7 +786,7 @@ static void svm_vcpu_put(struct kvm_vcpu *vcpu)
 	for (i = 0; i < NR_HOST_SAVE_USER_MSRS; i++)
 		wrmsrl(host_save_user_msrs[i], svm->host_user_msrs[i]);
 
-	rdtscll(vcpu->arch.host_tsc);
+	vcpu->arch.host_tsc = native_read_tsc();
 }
 
 static unsigned long svm_get_rflags(struct kvm_vcpu *vcpu)
-- 
1.6.3.3

Re: [PATCH 0/5] Another set of Nested SVM fixes and cleanups

[Avi Kivity]

On 09/16/2009 04:24 PM, Joerg Roedel wrote:
> Hi,
>
> this series of patches contain another set of cleanups and an important fix to
> the Nested SVM code. These patches make the TSC handling code for SVM aware of
> a nested guest. This fixes the TSC running backwards on guest and nested guest.
> The backwards running TSC resulted in stalled guests when kvm-clock was
> enabled. Please consider to apply.
>
>    

Looks good.  3, 4 needed for 2.6.32-rc as well, yes?

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

Re: [PATCH 0/5] Another set of Nested SVM fixes and cleanups

[Joerg Roedel]

On Wed, Sep 16, 2009 at 05:02:59PM +0300, Avi Kivity wrote:
> On 09/16/2009 04:24 PM, Joerg Roedel wrote:
> >Hi,
> >
> >this series of patches contain another set of cleanups and an important fix to
> >the Nested SVM code. These patches make the TSC handling code for SVM aware of
> >a nested guest. This fixes the TSC running backwards on guest and nested guest.
> >The backwards running TSC resulted in stalled guests when kvm-clock was
> >enabled. Please consider to apply.
> >
> 
> Looks good.  3, 4 needed for 2.6.32-rc as well, yes?

Yes, would be good since these two are real bugfixes. Probably also
relevant for -stable.

	Joerg

Re: [PATCH 0/5] Another set of Nested SVM fixes and cleanups

[Marcelo Tosatti]

On Wed, Sep 16, 2009 at 03:24:14PM +0200, Joerg Roedel wrote:
> Hi,
> 
> this series of patches contain another set of cleanups and an important fix to
> the Nested SVM code. These patches make the TSC handling code for SVM aware of
> a nested guest. This fixes the TSC running backwards on guest and nested guest.
> The backwards running TSC resulted in stalled guests when kvm-clock was
> enabled. Please consider to apply.

Applied, thanks (added Cc: stable to 3 and 4).