[PATCH v2 0/2] maintaining of kvm_clock stability on guest vCPU hotplug

[PATCH v2 0/2] maintaining of kvm_clock stability on guest vCPU hotplug

[Denis Plotnikov]

new in v2:
  - fix possible wraparound on the synchronization detection
  - lines exceeding 80 chars are re-formatted
  - excess parentheses are removed

======================

It was found, that after adding a new vCPU to a running VM (CPU hotplug)
kvm_clock lost the stability property in case of using it. This happened
because KVM didn't treated cpu hotplug as a special case of TSC
synchronization.
This patch series adds cpu hotplug to the TSC synchronization cases.

Denis Plotnikov (2):
  KVM: x86: remaster kvm_write_tsc code
  KVM: x86: fix maintaining of kvm_clock stability on guest CPU hotplug

 arch/x86/kvm/x86.c | 61 ++++++++++++++++++++----------------------------------
 1 file changed, 22 insertions(+), 39 deletions(-)

-- 
1.8.3.1

[PATCH v2 1/2] KVM: x86: remaster kvm_write_tsc code

[Denis Plotnikov]

Reuse existing code instead of using inline asm.
Make the code more concise and clear in the TSC
synchronization part.

Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>
---
 arch/x86/kvm/x86.c | 52 +++++++++++++---------------------------------------
 1 file changed, 13 insertions(+), 39 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 1faf620..c59e38f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1444,10 +1444,10 @@ void kvm_write_tsc(struct kvm_vcpu *vcpu, struct msr_data *msr)
 	struct kvm *kvm = vcpu->kvm;
 	u64 offset, ns, elapsed;
 	unsigned long flags;
-	s64 usdiff;
 	bool matched;
 	bool already_matched;
 	u64 data = msr->data;
+	bool synchronizing = false;
 
 	raw_spin_lock_irqsave(&kvm->arch.tsc_write_lock, flags);
 	offset = kvm_compute_tsc_offset(vcpu, data);
@@ -1455,51 +1455,25 @@ void kvm_write_tsc(struct kvm_vcpu *vcpu, struct msr_data *msr)
 	elapsed = ns - kvm->arch.last_tsc_nsec;
 
 	if (vcpu->arch.virtual_tsc_khz) {
-		int faulted = 0;
-
-		/* n.b - signed multiplication and division required */
-		usdiff = data - kvm->arch.last_tsc_write;
-#ifdef CONFIG_X86_64
-		usdiff = (usdiff * 1000) / vcpu->arch.virtual_tsc_khz;
-#else
-		/* do_div() only does unsigned */
-		asm("1: idivl %[divisor]\n"
-		    "2: xor %%edx, %%edx\n"
-		    "   movl $0, %[faulted]\n"
-		    "3:\n"
-		    ".section .fixup,\"ax\"\n"
-		    "4: movl $1, %[faulted]\n"
-		    "   jmp  3b\n"
-		    ".previous\n"
-
-		_ASM_EXTABLE(1b, 4b)
-
-		: "=A"(usdiff), [faulted] "=r" (faulted)
-		: "A"(usdiff * 1000), [divisor] "rm"(vcpu->arch.virtual_tsc_khz));
-
-#endif
-		do_div(elapsed, 1000);
-		usdiff -= elapsed;
-		if (usdiff < 0)
-			usdiff = -usdiff;
-
-		/* idivl overflow => difference is larger than USEC_PER_SEC */
-		if (faulted)
-			usdiff = USEC_PER_SEC;
-	} else
-		usdiff = USEC_PER_SEC; /* disable TSC match window below */
+		u64 tsc_exp = kvm->arch.last_tsc_write +
+					nsec_to_cycles(vcpu, elapsed);
+		u64 tsc_hz = vcpu->arch.virtual_tsc_khz * 1000LL;
+		/*
+		 * Special case: TSC write with a small delta (1 second)
+		 * of virtual cycle time against real time is
+		 * interpreted as an attempt to synchronize the CPU.
+		 */
+		synchronizing = data < tsc_exp + tsc_hz &&
+				data + tsc_hz > tsc_exp;
+	}
 
 	/*
-	 * Special case: TSC write with a small delta (1 second) of virtual
-	 * cycle time against real time is interpreted as an attempt to
-	 * synchronize the CPU.
-         *
 	 * For a reliable TSC, we can match TSC offsets, and for an unstable
 	 * TSC, we add elapsed time in this computation.  We could let the
 	 * compensation code attempt to catch up if we fall behind, but
 	 * it's better to try to match offsets from the beginning.
          */
-	if (usdiff < USEC_PER_SEC &&
+	if (synchronizing &&
 	    vcpu->arch.virtual_tsc_khz == kvm->arch.last_tsc_khz) {
 		if (!check_tsc_unstable()) {
 			offset = kvm->arch.cur_tsc_offset;
-- 
1.8.3.1

[PATCH v2 2/2] KVM: x86: fix maintaining of kvm_clock stability on guest CPU hotplug

[Denis Plotnikov]

VCPU TSC synchronization is perfromed in kvm_write_tsc() when the TSC
value being set is within 1 second from the expected, as obtained by
extrapolating of the TSC in already synchronized VCPUs.

This is naturally achieved on all VCPUs at VM start and resume;
however on VCPU hotplug it is not: the newly added VCPU is created
with TSC == 0 while others are well ahead.

To compensate for that, consider host-initiated kvm_write_tsc() with
TSC == 0 a special case requiring synchronization regardless of the
current TSC on other VCPUs.

Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>
---
 arch/x86/kvm/x86.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c59e38f..cf67091 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1455,16 +1455,25 @@ void kvm_write_tsc(struct kvm_vcpu *vcpu, struct msr_data *msr)
 	elapsed = ns - kvm->arch.last_tsc_nsec;
 
 	if (vcpu->arch.virtual_tsc_khz) {
-		u64 tsc_exp = kvm->arch.last_tsc_write +
-					nsec_to_cycles(vcpu, elapsed);
-		u64 tsc_hz = vcpu->arch.virtual_tsc_khz * 1000LL;
-		/*
-		 * Special case: TSC write with a small delta (1 second)
-		 * of virtual cycle time against real time is
-		 * interpreted as an attempt to synchronize the CPU.
-		 */
-		synchronizing = data < tsc_exp + tsc_hz &&
-				data + tsc_hz > tsc_exp;
+		if (data == 0 && msr->host_initiated) {
+			/*
+			 * detection of vcpu initialization -- need to sync
+			 * with other vCPUs. This particularly helps to keep
+			 * kvm_clock stable after CPU hotplug
+			 */
+			synchronizing = true;
+		} else {
+			u64 tsc_exp = kvm->arch.last_tsc_write +
+						nsec_to_cycles(vcpu, elapsed);
+			u64 tsc_hz = vcpu->arch.virtual_tsc_khz * 1000LL;
+			/*
+			 * Special case: TSC write with a small delta (1 second)
+			 * of virtual cycle time against real time is
+			 * interpreted as an attempt to synchronize the CPU.
+			 */
+			synchronizing = data < tsc_exp + tsc_hz &&
+					data + tsc_hz > tsc_exp;
+		}
 	}
 
 	/*
-- 
1.8.3.1

Re: [PATCH v2 0/2] maintaining of kvm_clock stability on guest vCPU hotplug

[Radim Krčmář]

2017-04-07 12:09+0300, Denis Plotnikov:
> new in v2:
>   - fix possible wraparound on the synchronization detection
>   - lines exceeding 80 chars are re-formatted
>   - excess parentheses are removed
> 
> ======================
> 
> It was found, that after adding a new vCPU to a running VM (CPU hotplug)
> kvm_clock lost the stability property in case of using it. This happened
> because KVM didn't treated cpu hotplug as a special case of TSC
> synchronization.
> This patch series adds cpu hotplug to the TSC synchronization cases.

Great cleanup as well, applied to kvm/queue, thanks.

[PATCH v2 0/2] maintaining of kvm_clock stability on guest vCPU hotplug

[Denis Plotnikov]

new in v2:
  - fix possible wraparound on the synchronization detection
  - lines exceeding 80 chars are re-formatted
  - excess parentheses are removed

======================

It was found, that after adding a new vCPU to a running VM (CPU hotplug)
kvm_clock lost the stability property in case of using it. This happened
because KVM didn't treated cpu hotplug as a special case of TSC
synchronization.
This patch series adds cpu hotplug to the TSC synchronization cases.

Denis Plotnikov (2):
  KVM: x86: remaster kvm_write_tsc code
  KVM: x86: fix maintaining of kvm_clock stability on guest CPU hotplug

 arch/x86/kvm/x86.c | 61 ++++++++++++++++++++----------------------------------
 1 file changed, 22 insertions(+), 39 deletions(-)

-- 
1.8.3.1