Skip to contentSkip to conversation

Another set of Nested SVM fixes and cleanups

PATCH Applied Joerg Roedel 3 participants 9 messages Sep 16, 2009 – Sep 18, 2009

Subsystems (3)
x86 KVM KVM x86
lore
Share & tools
lore

Joerg Roedel

author
#0 lore raw

cover Another set of Nested SVM fixes and cleanups

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(-)
author
#1 lore raw

PATCH 1/5 KVM: SVM: reorganize svm_interrupt_allowed

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 <[email protected]>
---
 arch/x86/kvm/svm.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)
arch/x86/kvm/svm.c +12 -4
index a2f2d43..668460c 100644
@@ -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)
Signature · 1 line
1.6.3.3
author
#2 lore raw

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

In reply to 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 <[email protected]>
---
 arch/x86/kvm/svm.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)
arch/x86/kvm/svm.c +0 -2
index 668460c..80f5309 100644
@@ -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",
Signature · 1 line
1.6.3.3
author
#3 lore raw

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

In reply to 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 <[email protected]>
---
 arch/x86/kvm/svm.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
arch/x86/kvm/svm.c +2 -0
index 80f5309..84c2c78 100644
@@ -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;
Signature · 1 line
1.6.3.3
author
#4 lore raw

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

In reply to 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 <[email protected]>
---
 arch/x86/kvm/svm.c |   23 +++++++++++++++++------
 1 files changed, 17 insertions(+), 6 deletions(-)
arch/x86/kvm/svm.c +17 -6
index 84c2c78..e193cf9 100644
@@ -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:
Signature · 1 line
1.6.3.3
author
#5 lore raw

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

In reply to 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 <[email protected]>
---
 arch/x86/kvm/svm.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)
arch/x86/kvm/svm.c +3 -4
index e193cf9..12cef2c 100644
@@ -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)
Signature · 1 line
1.6.3.3
#6 lore raw
In reply to Joerg Roedel
Quoted 9 lines · 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.

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

Signature · 1 line
error compiling committee.c: too many arguments to function
author
#7 lore raw
Quoted 11 lines · Avi Kivity
Quoted 8 lines · 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.

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

Applied
#8 lore raw
In reply to Joerg Roedel
Quoted 7 lines · 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.

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