maintaining of kvm_clock stability on guest vCPU hotplug
v2 cover maintaining of kvm_clock stability on guest vCPU hotplug
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(-)
Signature · 1 line
1.8.3.1
PATCH v2 1/2 KVM: x86: remaster kvm_write_tsc code
Reuse existing code instead of using inline asm.
Make the code more concise and clear in the TSC
synchronization part.
--- arch/x86/kvm/x86.c | 52 +++++++++++++--------------------------------------- 1 file changed, 13 insertions(+), 39 deletions(-)
arch/x86/kvm/x86.c +13 -39
@@ -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;
Signature · 1 line
1.8.3.1
PATCH v2 2/2 KVM: x86: fix maintaining of kvm_clock stability on guest CPU hotplug
In reply to Denis PlotnikovVCPU 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.
--- arch/x86/kvm/x86.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-)
arch/x86/kvm/x86.c +19 -10
@@ -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; + } }/*
Signature · 1 line
1.8.3.1
2017-04-07 12:09+0300, Denis Plotnikov:
Quoted 5 lines
new in v2:
- fix possible wraparound on the synchronization detection
- lines exceeding 80 chars are re-formatted
- excess parentheses are removed
Quoted 7 lines
======================
It was found, that after adding a new vCPU to a running VM (CPU hotplug)
kvm_clocklost 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.