[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v12 03/24] KVM: x86: Replace nr_vcpus_matched_tsc count with all_vcpus_matched_tsc bool



From: David Woodhouse <dwmw@xxxxxxxxxxxx>

Using a count and comparing with kvm->online_vcpus was always racy
because a new vCPU could be created while kvm_track_tsc_matching() was
running and comparing with kvm->online_vcpus. That variable is only
atomic with respect to itself; kvm_arch_vcpu_create() runs before
kvm->online_vcpus is incremented for the new vCPU.

Replace the count with a boolean that is set in kvm_track_tsc_matching()
after comparing the count, and cleared when a new TSC generation starts.
The boolean is consumed by pvclock_update_vm_gtod_copy() under the
tsc_write_lock, which serializes against __kvm_synchronize_tsc().

Keep the count for now as it's still used in the trace event.

Note that the explicit clear of the flag when a new generation starts
is currently overwritten by the unconditional recompute in
kvm_track_tsc_matching() under the same lock; it is preparation for
subsequent patches which consume the previous value of the flag.

Initialize the flag to true at VM creation: a VM whose vCPUs never
reach kvm_track_tsc_matching() — notably with a protected guest TSC
(TDX, SNP Secure TSC), where __kvm_synchronize_tsc() returns early —
would otherwise never be eligible for PVCLOCK_TSC_STABLE_BIT, where
the old read-time computation gave 0 + 1 == 1 for a single-vCPU VM.

Initializing the flag to true means pvclock_update_vm_gtod_copy() now
reaches its scaling path from kvm_arch_init_vm(), where the scaling
ratio is still zero from allocation; initialize it to the default
ratio at the same point, since a zero ratio would zero tsc_hz and turn
the master clock straight back off — permanently for protected-TSC
guests, whose vCPUs never write the ratio in __kvm_synchronize_tsc().

Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
---
 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/x86.c              | 41 +++++++++++++++++++++++++++++----
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index ea48b1949316..1efca01647d2 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1242,6 +1242,7 @@ struct kvm_arch {
        u64 cur_tsc_write;
        u64 cur_tsc_offset;
        u64 cur_tsc_generation;
+       bool all_vcpus_matched_tsc;
        int nr_vcpus_matched_tsc;
 
        u32 default_tsc_khz;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 20b84b2e4424..d29e5bc75f7a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1125,8 +1125,10 @@ static void kvm_track_tsc_matching(struct kvm_vcpu 
*vcpu, bool new_generation)
         * and all vCPUs must have matching TSCs.  Note, the count for matching
         * vCPUs doesn't include the reference vCPU, hence "+1".
         */
-       bool use_master_clock = (ka->nr_vcpus_matched_tsc + 1 ==
-                                atomic_read(&vcpu->kvm->online_vcpus)) &&
+       ka->all_vcpus_matched_tsc = (ka->nr_vcpus_matched_tsc + 1 ==
+                                    atomic_read(&vcpu->kvm->online_vcpus));
+
+       bool use_master_clock = ka->all_vcpus_matched_tsc &&
                                gtod_is_based_on_tsc(gtod->clock.vclock_mode);
 
        /*
@@ -1367,6 +1369,7 @@ static void __kvm_synchronize_tsc(struct kvm_vcpu *vcpu, 
u64 offset, u64 tsc,
                kvm->arch.cur_tsc_write = tsc;
                kvm->arch.cur_tsc_offset = offset;
                kvm->arch.nr_vcpus_matched_tsc = 0;
+               kvm->arch.all_vcpus_matched_tsc = false;
        } else if (vcpu->arch.this_tsc_generation != 
kvm->arch.cur_tsc_generation) {
                kvm->arch.nr_vcpus_matched_tsc++;
        }
@@ -1593,8 +1596,7 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm)
        bool host_tsc_clocksource, vcpus_matched;
 
        lockdep_assert_held(&kvm->arch.tsc_write_lock);
-       vcpus_matched = (ka->nr_vcpus_matched_tsc + 1 ==
-                       atomic_read(&kvm->online_vcpus));
+       vcpus_matched = ka->all_vcpus_matched_tsc;
 
        /*
         * If the host uses TSC clock, then passthrough TSC as stable
@@ -9552,8 +9554,23 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 
 void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
 {
-       if (mutex_lock_killable(&vcpu->mutex))
+       if (mutex_lock_killable(&vcpu->mutex)) {
+               /*
+                * The new vCPU is already counted in online_vcpus but has
+                * not been synchronized. Nothing recomputes the cached
+                * matched state against the raised count until the next
+                * synchronization, so clear it here rather than leave the
+                * clock advertised as matched while an unmatched vCPU is
+                * runnable. It heals when this vCPU first synchronizes.
+                */
+               struct kvm_arch *ka = &vcpu->kvm->arch;
+               unsigned long flags;
+
+               raw_spin_lock_irqsave(&ka->tsc_write_lock, flags);
+               WRITE_ONCE(ka->all_vcpus_matched_tsc, false);
+               raw_spin_unlock_irqrestore(&ka->tsc_write_lock, flags);
                return;
+       }
        vcpu_load(vcpu);
        kvm_synchronize_tsc(vcpu, NULL);
        if (kvm_check_request(KVM_REQ_MASTERCLOCK_UPDATE, vcpu))
@@ -10029,6 +10046,20 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long 
type)
        ratelimit_set_flags(&kvm->arch.kvmclock_update_rs, 
RATELIMIT_MSG_ON_RELEASE);
        kvm->arch.kvmclock_offset = -get_kvmclock_base_ns();
 
+       /*
+        * Start with all (zero) vCPUs considered matched, so that a VM whose
+        * vCPUs never reach kvm_track_tsc_matching() — notably when the guest
+        * TSC is protected (TDX, SNP Secure TSC) and __kvm_synchronize_tsc()
+        * returns early — is still eligible for PVCLOCK_TSC_STABLE_BIT. The
+        * scaling ratio must then also be initialized: with vcpus_matched
+        * now true here, pvclock_update_vm_gtod_copy() reaches the scaling
+        * path, and a zero ratio would zero tsc_hz and turn the master
+        * clock straight back off (permanently so for protected-TSC guests,
+        * which never write the ratio in __kvm_synchronize_tsc()).
+        */
+       kvm->arch.all_vcpus_matched_tsc = true;
+       kvm->arch.cur_tsc_scaling_ratio = kvm_caps.default_tsc_scaling_ratio;
+
        raw_spin_lock_irqsave(&kvm->arch.tsc_write_lock, flags);
        pvclock_update_vm_gtod_copy(kvm);
        raw_spin_unlock_irqrestore(&kvm->arch.tsc_write_lock, flags);
-- 
2.43.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.