From: David Woodhouse <dwmw2@infradead.org>
To: kvm@vger.kernel.org
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Jonathan Corbet <corbet@lwn.net>,
Sean Christopherson <seanjc@google.com>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
Dave Hansen <dave.hansen@linux.intel.com>,
x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
Paul Durrant <paul@xen.org>, Shuah Khan <shuah@kernel.org>,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest@vger.kernel.org,
Oliver Upton <oliver.upton@linux.dev>,
Marcelo Tosatti <mtosatti@redhat.com>,
jalliste@amazon.co.uk, sveith@amazon.de, zide.chen@intel.com,
Dongli Zhang <dongli.zhang@oracle.com>
Subject: [PATCH v2 14/15] KVM: x86: Allow KVM master clock mode when TSCs are offset from each other
Date: Sat, 27 Apr 2024 12:05:11 +0100 [thread overview]
Message-ID: <20240427111929.9600-15-dwmw2@infradead.org> (raw)
In-Reply-To: <20240427111929.9600-1-dwmw2@infradead.org>
From: David Woodhouse <dwmw@amazon.co.uk>
There is no reason why the KVM clock cannot be in masterclock mode when
the TSCs are not in sync, as long as they are at the same *frequency*.
Running at a different frequency would lead to a systemic skew between
the clock(s) as observed by different vCPUs due to arithmetic precision
in the scaling. So that should indeed force the clock to be based on the
host's CLOCK_MONOTONIC_RAW instead of being in masterclock mode where it
is defined by the (or 'a') guest TSC.
But when the vCPUs merely have a different TSC *offset*, that's not a
problem. The offset is applied to that vCPU's kvmclock->tsc_timestamp
field, and it all comes out in the wash.
So, remove ka->nr_vcpus_matched_tsc and replace it with a new field
ka->all_vcpus_matched_tsc which is not only changed to a boolean, but
also now tracks that the *frequency* matches, not the precise offset.
Using a *count* was always racy because a new vCPU could be being
created *while* kvm_track_tsc_matching() was running and comparing with
kvm->online_vcpus. That variable is only atomic with respect to itself.
In particular, kvm_arch_vcpu_create() runs before kvm->online_vcpus is
incremented for the new vCPU, and kvm_arch_vcpu_postcreate() runs later.
Repurpose kvm_track_tsc_matching() to be called from kvm_set_tsc_khz(),
and kill the cur_tsc_generation/last_tsc_generation fields which tracked
the precise TSC matching.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
arch/x86/include/asm/kvm_host.h | 6 +-
arch/x86/kvm/x86.c | 133 ++++++++++++++++----------------
2 files changed, 70 insertions(+), 69 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 7d06f389a607..52dbb2d7690b 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -906,7 +906,6 @@ struct kvm_vcpu_arch {
u64 tsc_offset_adjustment;
u64 this_tsc_nsec;
u64 this_tsc_write;
- u64 this_tsc_generation;
bool tsc_catchup;
bool tsc_always_catchup;
s8 virtual_tsc_shift;
@@ -1354,11 +1353,10 @@ struct kvm_arch {
u32 last_tsc_khz;
u64 last_tsc_offset;
u64 last_tsc_scaling_ratio;
- u64 cur_tsc_generation;
- int nr_vcpus_matched_tsc;
+ bool all_vcpus_matched_tsc;
- u32 default_tsc_khz;
bool user_set_tsc;
+ u32 default_tsc_khz;
seqcount_raw_spinlock_t pvclock_sc;
bool use_master_clock;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 92e81bfca25a..d6e4469f531a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2493,40 +2493,6 @@ static int set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale)
return 0;
}
-static int kvm_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz)
-{
- u32 thresh_lo, thresh_hi;
- int use_scaling = 0;
-
- /* tsc_khz can be zero if TSC calibration fails */
- if (user_tsc_khz == 0) {
- /* set tsc_scaling_ratio to a safe value */
- kvm_vcpu_write_tsc_multiplier(vcpu, kvm_caps.default_tsc_scaling_ratio);
- return -1;
- }
-
- /* Compute a scale to convert nanoseconds in TSC cycles */
- kvm_get_time_scale(user_tsc_khz * 1000LL, NSEC_PER_SEC,
- &vcpu->arch.virtual_tsc_shift,
- &vcpu->arch.virtual_tsc_mult);
- vcpu->arch.virtual_tsc_khz = user_tsc_khz;
-
- /*
- * 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("requested TSC rate %u falls outside tolerance [%u,%u]\n",
- user_tsc_khz, thresh_lo, thresh_hi);
- use_scaling = 1;
- }
- return set_tsc_khz(vcpu, user_tsc_khz, use_scaling);
-}
-
static u64 compute_guest_tsc(struct kvm_vcpu *vcpu, s64 kernel_ns)
{
s64 delta = kernel_ns - vcpu->arch.this_tsc_nsec;
@@ -2557,14 +2523,34 @@ static void kvm_track_tsc_matching(struct kvm_vcpu *vcpu)
#ifdef CONFIG_X86_64
struct kvm_arch *ka = &vcpu->kvm->arch;
struct pvclock_gtod_data *gtod = &pvclock_gtod_data;
+ struct kvm_vcpu *v;
+ unsigned long i;
+ bool matched = true;
+
+ lockdep_assert_held(&vcpu->kvm->arch.tsc_write_lock);
+
+ /*
+ * When called via kvm_arch_vcpu_create() for a new vCPU, the count
+ * in kvm->online_vcpus will not yet have been incremented and the
+ * kvm_for_each_vcpu() loop will not iterate over 'vcpu'.
+ *
+ * The right thing still happens in that case, because a match will
+ * be found only if the new vCPU's TSC is at the same rate as *all*
+ * the existing vCPUs' TSCs.
+ */
+ kvm_for_each_vcpu(i, v, vcpu->kvm) {
+ if (v->arch.virtual_tsc_khz != vcpu->arch.virtual_tsc_khz) {
+ matched = false;
+ break;
+ }
+ }
+ ka->all_vcpus_matched_tsc = matched;
/*
* To use the masterclock, the host clocksource must be based on TSC
- * and all vCPUs must have matching TSCs. Note, the count for matching
- * vCPUs doesn't include the reference vCPU, hence "+1".
+ * and all vCPUs must have matching TSC frequencies.
*/
- bool use_master_clock = (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);
/*
@@ -2574,12 +2560,51 @@ static void kvm_track_tsc_matching(struct kvm_vcpu *vcpu)
if ((ka->use_master_clock != use_master_clock))
kvm_make_request(KVM_REQ_MASTERCLOCK_UPDATE, vcpu);
- trace_kvm_track_tsc(vcpu->vcpu_id, ka->nr_vcpus_matched_tsc,
+ trace_kvm_track_tsc(vcpu->vcpu_id, ka->all_vcpus_matched_tsc,
atomic_read(&vcpu->kvm->online_vcpus),
ka->use_master_clock, gtod->clock.vclock_mode);
#endif
}
+static int kvm_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz)
+{
+ u32 thresh_lo, thresh_hi;
+ int use_scaling = 0;
+
+ /* tsc_khz can be zero if TSC calibration fails */
+ if (user_tsc_khz == 0) {
+ /* set tsc_scaling_ratio to a safe value */
+ kvm_vcpu_write_tsc_multiplier(vcpu, kvm_caps.default_tsc_scaling_ratio);
+ return -1;
+ }
+
+ /* Compute a scale to convert nanoseconds in TSC cycles */
+ kvm_get_time_scale(user_tsc_khz * 1000LL, NSEC_PER_SEC,
+ &vcpu->arch.virtual_tsc_shift,
+ &vcpu->arch.virtual_tsc_mult);
+
+ raw_spin_lock_irq(&vcpu->kvm->arch.tsc_write_lock);
+ vcpu->arch.virtual_tsc_khz = user_tsc_khz;
+ kvm_track_tsc_matching(vcpu);
+ raw_spin_unlock_irq(&vcpu->kvm->arch.tsc_write_lock);
+
+ /*
+ * 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("requested TSC rate %u falls outside tolerance [%u,%u]\n",
+ user_tsc_khz, thresh_lo, thresh_hi);
+ use_scaling = 1;
+ }
+ return set_tsc_khz(vcpu, user_tsc_khz, use_scaling);
+}
+
+
/*
* Multiply tsc by a fixed point number represented by ratio.
*
@@ -2725,27 +2750,6 @@ static void __kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 offset, u64 tsc,
vcpu->arch.last_guest_tsc = tsc;
kvm_vcpu_write_tsc_offset(vcpu, offset);
-
- if (!matched) {
- /*
- * We split periods of matched TSC writes into generations.
- * For each generation, we track the original measured
- * nanosecond time, offset, and write, so if TSCs are in
- * sync, we can match exact offset, and if not, we can match
- * exact software computation in compute_guest_tsc()
- *
- * These values are tracked in kvm->arch.cur_xxx variables.
- */
- kvm->arch.cur_tsc_generation++;
- kvm->arch.nr_vcpus_matched_tsc = 0;
- } else if (vcpu->arch.this_tsc_generation != kvm->arch.cur_tsc_generation) {
- kvm->arch.nr_vcpus_matched_tsc++;
- }
-
- /* Keep track of which generation this VCPU has synchronized to */
- vcpu->arch.this_tsc_generation = kvm->arch.cur_tsc_generation;
-
- kvm_track_tsc_matching(vcpu);
}
static void kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 *user_value)
@@ -3072,11 +3076,9 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm)
struct kvm_arch *ka = &kvm->arch;
uint64_t last_tsc_hz;
int vclock_mode;
- bool host_tsc_clocksource, vcpus_matched;
+ bool host_tsc_clocksource;
lockdep_assert_held(&kvm->arch.tsc_write_lock);
- vcpus_matched = (ka->nr_vcpus_matched_tsc + 1 ==
- atomic_read(&kvm->online_vcpus));
/*
* If the host uses TSC clock, then passthrough TSC as stable
@@ -3086,7 +3088,8 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm)
&ka->master_kernel_ns,
&ka->master_cycle_now);
- ka->use_master_clock = host_tsc_clocksource && vcpus_matched
+ ka->use_master_clock = host_tsc_clocksource
+ && ka->all_vcpus_matched_tsc
&& !ka->backwards_tsc_observed
&& !ka->boot_vcpu_runs_old_kvmclock;
@@ -3124,7 +3127,7 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm)
vclock_mode = pvclock_gtod_data.clock.vclock_mode;
trace_kvm_update_master_clock(ka->use_master_clock, vclock_mode,
- vcpus_matched);
+ ka->all_vcpus_matched_tsc);
#endif
}
--
2.44.0
next prev parent reply other threads:[~2024-04-27 11:19 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-27 11:04 [RFC PATCH v2] Cleaning up the KVM clock mess David Woodhouse
2024-04-27 11:04 ` [PATCH v2 01/15] KVM: x86/xen: Do not corrupt KVM clock in kvm_xen_shared_info_init() David Woodhouse
2024-05-04 7:42 ` Marcelo Tosatti
2024-05-07 19:08 ` David Woodhouse
2024-04-27 11:04 ` [PATCH v2 02/15] KVM: x86: Improve accuracy of KVM clock when TSC scaling is in force David Woodhouse
2024-04-27 11:05 ` [PATCH v2 03/15] KVM: x86: Add KVM_[GS]ET_CLOCK_GUEST for accurate KVM clock migration David Woodhouse
2024-04-27 11:05 ` [PATCH v2 04/15] KVM: selftests: Add KVM/PV clock selftest to prove timer correction David Woodhouse
2024-04-27 11:05 ` [PATCH v2 05/15] KVM: x86: Explicitly disable TSC scaling without CONSTANT_TSC David Woodhouse
2024-04-27 11:05 ` [PATCH v2 06/15] KVM: x86: Add KVM_VCPU_TSC_SCALE and fix the documentation on TSC migration David Woodhouse
2024-04-27 11:05 ` [PATCH v2 07/15] KVM: x86: Avoid NTP frequency skew for KVM clock on 32-bit host David Woodhouse
2024-04-27 11:05 ` [PATCH v2 08/15] KVM: x86: Fix KVM clock precision in __get_kvmclock() David Woodhouse
2024-04-27 11:05 ` [PATCH v2 09/15] KVM: x86: Fix software TSC upscaling in kvm_update_guest_time() David Woodhouse
2024-04-27 11:05 ` [PATCH v2 10/15] KVM: x86: Simplify and comment kvm_get_time_scale() David Woodhouse
2024-04-27 11:05 ` [PATCH v2 11/15] KVM: x86: Remove implicit rdtsc() from kvm_compute_l1_tsc_offset() David Woodhouse
2024-04-27 11:05 ` [PATCH v2 12/15] KVM: x86: Improve synchronization in kvm_synchronize_tsc() David Woodhouse
2024-04-27 11:05 ` [PATCH v2 13/15] KVM: x86: Kill cur_tsc_{nsec,offset,write} fields David Woodhouse
2024-05-10 9:03 ` Chenyi Qiang
2024-05-14 13:17 ` David Woodhouse
2024-04-27 11:05 ` David Woodhouse [this message]
2024-04-27 11:05 ` [PATCH v2 15/15] KVM: x86: Factor out kvm_use_master_clock() David Woodhouse
2024-05-01 17:55 ` Chen, Zide
2024-05-01 20:45 ` David Woodhouse
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240427111929.9600-15-dwmw2@infradead.org \
--to=dwmw2@infradead.org \
--cc=bp@alien8.de \
--cc=corbet@lwn.net \
--cc=dave.hansen@linux.intel.com \
--cc=dongli.zhang@oracle.com \
--cc=hpa@zytor.com \
--cc=jalliste@amazon.co.uk \
--cc=kvm@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=mtosatti@redhat.com \
--cc=oliver.upton@linux.dev \
--cc=paul@xen.org \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
--cc=shuah@kernel.org \
--cc=sveith@amazon.de \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
--cc=zide.chen@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).