Skip to contentSkip to conversation

KVM: x86: allow TSC to differ by NTP correction bounds without TSC scaling

PATCH In review Marcelo Tosatti 2 participants 4 messages Jun 15, 2020 – Jun 23, 2020

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 KVM: x86: allow TSC to differ by NTP correction bounds without TSC scaling

The Linux TSC calibration procedure is subject to small variations (its common to see +-1 kHz difference between reboots on a given CPU, for example).

So migrating a guest between two hosts with identical processor can fail, in case of a small variation in calibrated TSC between them.

Allow a conservative 250ppm error between host TSC and VM TSC frequencies, rather than requiring an exact match. NTP daemon in the guest can correct this difference.

arch/x86/kvm/x86.c +4 -1
index 3156e25..39a6664 100644
@@ -1772,6 +1772,8 @@ static int set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale)
 
 	/* TSC scaling supported? */
 	if (!kvm_has_tsc_control) {
+		if (!scale)
+			return 0;
 		if (user_tsc_khz > tsc_khz) {
 			vcpu->arch.tsc_catchup = 1;
 			vcpu->arch.tsc_always_catchup = 1;
@@ -4473,7 +4475,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		r = -EINVAL;
 		user_tsc_khz = (u32)arg;
 
-		if (user_tsc_khz >= kvm_max_guest_tsc_khz)
+		if (kvm_has_tsc_control &&
+		    user_tsc_khz >= kvm_max_guest_tsc_khz)
 			goto out;
 
 		if (user_tsc_khz == 0)
maintainer
#1 lore raw
Quoted 11 lines · Marcelo Tosatti

The Linux TSC calibration procedure is subject to small variations (its common to see +-1 kHz difference between reboots on a given CPU, for example).

So migrating a guest between two hosts with identical processor can fail, in case of a small variation in calibrated TSC between them.

Allow a conservative 250ppm error between host TSC and VM TSC frequencies, rather than requiring an exact match. NTP daemon in the guest can correct this difference.

This is the userspace commit message. Can you resend with a better commit message that actually matches what the patch does and explains why the userspace patch is not enough?

Also you should explain what happens with new userspace and old kernel.

Thanks,

Paolo

Quoted 24 lines
arch/x86/kvm/x86.c +4 -1
index 3156e25..39a6664 100644
@@ -1772,6 +1772,8 @@ static int set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale)
 
 	/* TSC scaling supported? */
 	if (!kvm_has_tsc_control) {
+		if (!scale)
+			return 0;
 		if (user_tsc_khz > tsc_khz) {
 			vcpu->arch.tsc_catchup = 1;
 			vcpu->arch.tsc_always_catchup = 1;
@@ -4473,7 +4475,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		r = -EINVAL;
 		user_tsc_khz = (u32)arg;
 
-		if (user_tsc_khz >= kvm_max_guest_tsc_khz)
+		if (kvm_has_tsc_control &&
+		    user_tsc_khz >= kvm_max_guest_tsc_khz)
 			goto out;
 
 		if (user_tsc_khz == 0)
#2 lore raw

PATCH v2 1/1 KVM: x86: allow TSC to differ by NTP correction bounds without TSC scaling

The Linux TSC calibration procedure is subject to small variations (its common to see +-1 kHz difference between reboots on a given CPU, for example).

So migrating a guest between two hosts with identical processor can fail, in case of a small variation in calibrated TSC between them.

Without TSC scaling, the current kernel interface will either return an error (if user_tsc_khz <= tsc_khz) or enable TSC catchup mode.

This change enables the following TSC tolerance check to
accept KVM_SET_TSC_KHZ within tsc_tolerance_ppm (which is 250ppm by default).

        /*
         * Compute the variation in TSC rate which is acceptable
         * within the range of tolerance and decide if the
         * rate being applied is within that bounds of the hardware
         * rate.  If so, no scaling or compensation need be done.
         */
        thresh_lo = adjust_tsc_khz(tsc_khz, -tsc_tolerance_ppm);
        thresh_hi = adjust_tsc_khz(tsc_khz, tsc_tolerance_ppm);
        if (user_tsc_khz < thresh_lo || user_tsc_khz > thresh_hi) {
                pr_debug("kvm: requested TSC rate %u falls outside tolerance [%u,%u]\n", user_tsc_khz, thresh_lo, thresh_hi);
                use_scaling = 1;
        }

NTP daemon in the guest can correct this difference (NTP can correct upto 500ppm).

---

v2: improve changelog (Paolo Bonzini)

arch/x86/kvm/x86.c +4 -1
index 3156e25..39a6664 100644
@@ -1772,6 +1772,8 @@ static int set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale)
 
 	/* TSC scaling supported? */
 	if (!kvm_has_tsc_control) {
+		if (!scale)
+			return 0;
 		if (user_tsc_khz > tsc_khz) {
 			vcpu->arch.tsc_catchup = 1;
 			vcpu->arch.tsc_always_catchup = 1;
@@ -4473,7 +4475,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		r = -EINVAL;
 		user_tsc_khz = (u32)arg;
 
-		if (user_tsc_khz >= kvm_max_guest_tsc_khz)
+		if (kvm_has_tsc_control &&
+		    user_tsc_khz >= kvm_max_guest_tsc_khz)
 			goto out;
 
 		if (user_tsc_khz == 0)
maintainer
#3 lore raw

arch/x86/kvm/x86.c static int set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale) · line 1776

Quoted 12 lines · Marcelo Tosatti

v2: improve changelog (Paolo Bonzini)

arch/x86/kvm/x86.c +2 -0
index 3156e25..39a6664 100644
@@ -1772,6 +1772,8 @@ static int set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale)
 
 	/* TSC scaling supported? */
 	if (!kvm_has_tsc_control) {
+		if (!scale)
+			return 0;

This cannot happen, there is an "if (!scale) right above.

Quoted 5 lines
arch/x86/kvm/x86.c +0 -0
 		if (user_tsc_khz > tsc_khz) {
 			vcpu->arch.tsc_catchup = 1;
 			vcpu->arch.tsc_always_catchup = 1;

arch/x86/kvm/x86.c long kvm_arch_vcpu_ioctl(struct file *filp, · line 4482

Quoted 9 lines
arch/x86/kvm/x86.c +2 -1
@@ -4473,7 +4475,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		r = -EINVAL;
 		user_tsc_khz = (u32)arg;
 
-		if (user_tsc_khz >= kvm_max_guest_tsc_khz)
+		if (kvm_has_tsc_control &&
+		    user_tsc_khz >= kvm_max_guest_tsc_khz)
 			goto out;
 
 		if (user_tsc_khz == 0)

Queued this second hunk.

Paolo