Sync TSC for guest when vcpu has been migrated to another cpu

When a vcpu is migrated to another cpu, an unexpected difference in current and previous cpu TSC values can cause a guest to wait indefinitely. A known workaround has been to run kvm with taskset, locking it to a single cpu. This patch modifies the guest viewable TSC so that the TSC is guaranteed to be monotonically increasing.

Thanks to Avi Kivity for help in pinpointing this issue and advice for fixing it.

Signed-off-by Leonard Norrgård <[email protected]>

------------------------------------------------------------------------

Index: linux-2.6/drivers/kvm/svm.c
===================================================================

drivers/kvm/svm.c +37 -1

@@ -598,9 +598,45 @@
 	kfree(vcpu->svm);
 }
 
+#ifdef CONFIG_SMP
+static void ipi_rdtscll(void *arg)
+{
+	u64 *tsc = arg;
+	rdtscll(*tsc);
+}
+#endif
+
+/*
+ * Switches to specified vcpu, until a matching vcpu_put(), but assumes
+ * vcpu mutex is already taken.
+ */
 static struct kvm_vcpu *svm_vcpu_load(struct kvm_vcpu *vcpu)
 {
-	get_cpu();
+	int cpu;
+
+	cpu = get_cpu();
+
+#ifdef CONFIG_SMP
+	if (vcpu->cpu != cpu) {
+		if (vcpu->cpu != -1) {
+			u64 tsc_this, tsc_previous;
+
+			/* Get TSC value for this and the previous cpu. */
+			rdtscll(tsc_this);
+			smp_call_function_single(vcpu->cpu, ipi_rdtscll,
+						 &tsc_previous, 0, 1);
+
+			/*
+			 * Make sure that the guest sees a monotonically
+			 * increasing TSC.
+			 */
+			vcpu->svm->vmcb->control.tsc_offset +=
+						tsc_previous - tsc_this;
+		}
+	}
+#endif
+
+	vcpu->cpu = cpu;
 	return vcpu;
 }
 

Index: linux-2.6/drivers/kvm/vmx.c
===================================================================

drivers/kvm/vmx.c +27 -2

@@ -200,6 +200,14 @@
 #endif
 }
 
+#ifdef CONFIG_SMP
+static void ipi_rdtscll(void *arg)
+{
+	u64 *tsc = arg;
+	rdtscll(*tsc);
+}
+#endif
+
 /*
  * Switches to specified vcpu, until a matching vcpu_put(), but assumes
  * vcpu mutex is already taken.

@@ -230,6 +238,25 @@
 		struct descriptor_table dt;
 		unsigned long sysenter_esp;
 
+#ifdef CONFIG_SMP
+		if (vcpu->cpu != -1) {
+			u64 tsc_this, tsc_previous, guest_tsc_offset;
+
+			/* Get TSC value for this and the previous cpu. */
+			rdtscll(tsc_this);
+			smp_call_function_single(vcpu->cpu, ipi_rdtscll,
+						 &tsc_previous, 0, 1);
+
+			/*
+			 * Make sure that the guest sees a monotonically
+			 * increasing TSC.
+			 */
+			guest_tsc_offset = vmcs_read64(TSC_OFFSET);
+			vmcs_write64(TSC_OFFSET, guest_tsc_offset +
+				     		tsc_previous - tsc_this);
+		}
+#endif
+
 		vcpu->cpu = cpu;
 		/*
 		 * Linux uses per-cpu TSS and GDT, so set these when switching
 
------------------------------------------------------------------------
 
-------------------------------------------------------------------------

Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
------------------------------------------------------------------------

_______________________________________________
kvm-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Hi Leonard,

Why not factor the common code into kvm_main.c and add a set_tsc_offset member to the kvm ops function table?

Regards,

Anthony Liguori

Index: linux-2.6/drivers/kvm/svm.c
===================================================================

drivers/kvm/svm.c +11 -0

@@ -598,9 +598,45 @@
 	kfree(vcpu->svm);
 }
 
+#ifdef CONFIG_SMP
+static void ipi_rdtscll(void *arg)
+{
+	u64 *tsc = arg;
+	rdtscll(*tsc);
+}
+#endif
+
+/*
+ * Switches to specified vcpu, until a matching vcpu_put(), but assumes
+ * vcpu mutex is already taken.

drivers/kvm/svm.c +15 -0

+	if (vcpu->cpu != cpu) {
+		if (vcpu->cpu != -1) {
+			u64 tsc_this, tsc_previous;
+
+			/* Get TSC value for this and the previous cpu. */
+			rdtscll(tsc_this);
+			smp_call_function_single(vcpu->cpu, ipi_rdtscll,
+						 &tsc_previous, 0, 1);
+
+			/*
+			 * Make sure that the guest sees a monotonically
+			 * increasing TSC.
+			 */
+			vcpu->svm->vmcb->control.tsc_offset +=
+						tsc_previous - tsc_this;
 
 

Index: linux-2.6/drivers/kvm/vmx.c
===================================================================

drivers/kvm/vmx.c +18 -0

@@ -200,6 +200,14 @@
 #endif
 }
 
+#ifdef CONFIG_SMP
+static void ipi_rdtscll(void *arg)
+{
+	u64 *tsc = arg;
+	rdtscll(*tsc);
+}
+#endif
+
 /*
  * Switches to specified vcpu, until a matching vcpu_put(), but assumes
  * vcpu mutex is already taken.

@@ -230,6 +238,25 @@
 		struct descriptor_table dt;
 		unsigned long sysenter_esp;
 
+#ifdef CONFIG_SMP
+		if (vcpu->cpu != -1) {
+			u64 tsc_this, tsc_previous, guest_tsc_offset;
+
+			/* Get TSC value for this and the previous cpu. */
+			rdtscll(tsc_this);
+			smp_call_function_single(vcpu->cpu, ipi_rdtscll,
+						 &tsc_previous, 0, 1);
+
+			/*

The previous patch introduces an IPI on every vcpu migration on SMP machines to find out the TSC value on the previous cpu. We might be able to avoid this IPI on systems that have a synchronized TSC by checking for it in svm/vmx_vcpu_load:

(quoted diff) +5 -0

+               /* If this isn't the first vcpu_load and if the TSC
+                  doesn't run at a constant rate, we must handle TSC
+                  offsets. */
+               if (vcpu->cpu != -1
+                   && !boot_cpu_has(X86_FEATURE_CONSTANT_TSC)) {

Wether this is actually reliable and working in the intended way is not know yet.

In the meantime, introduce a statistic for vcpu migration, so we get some decision support for the needs for such an optimization.