KVM: x86: Cleaning up the KVM clock mess, part 2
Subsystems (8)
David Woodhouse
maintainerv12 cover KVM: x86: Cleaning up the KVM clock mess, part 2
From: David Woodhouse <[email protected]>
I love it when sashiko-bot finds things that my own local sashiko run didn't.
This is the second half of the KVM clock series which has been churning since about 2024. The first half¹ is upstream as of 7.4-rc1 (yay!). This is the remainder — the masterclock overhaul, and new API for fixing migration.
In v12 I've finally removed the last reason why a *guest* can force the KVM clock out of masterclock mode (by using the old SYSTEM_TIME MSR). Even if we do still care about that SUSE kernel from 2015, we can satisfy it just by updating the master clock when it rewrites the MSR.
Tree: https://git.infradead.org/?p=users/dwmw2/linux.git;a=shortlog;h=refs/heads/kvmclock12
Since v11², fixing what the on-list review found, and more that arose out of that:
KVM_SET_TSC_KHZ:
• Synchronizing during vCPU creation, before the vCPU is counted in online_vcpus, corrupted the VM-wide matching state.
• Because the user_set_tsc flag was VM-wide, restoring one vCPU's TSC made a sibling's *unrestored* value be preserved across a frequency change.
• Time spent in the ioctl between sampling the guest TSC and reinstalling it was silently subtracted from the vCPU's clock.
• Setting the unchanged frequency perturbed the generation tracking, and a failed call committed the rejected frequency anyway.
• An above-host-rate frequency on scaling-less hardware left the vCPU in catchup mode forever.
The master clock:
• Making the first vCPU match the TSC generation established at VM creation was a silent guest-visible change (its TSC became the cycles elapsed since KVM_CREATE_VM), and handed it a stale offset if the host suspended while the VM had no vCPUs.
• KVM_GET_CLOCK trusted a master snapshot which nothing refreshes for a VM with no vCPUs across a host suspend, returning a discontinuity flagged KVM_CLOCK_TSC_STABLE.
• A TSC synchronization which changed the vCPU's offset left the guest's pvclock unrefreshed, now that offset changes no longer turn the master clock off.
• When the last vCPU reached a new common frequency, nothing refreshed the masterclock scaling factors.
• A pending masterclock update on a newly created vCPU wasn't serviced until it first ran, and a vCPU whose creation was aborted part-way stayed counted as matched forever.
• Guests using the legacy MSR_KVM_SYSTEM_TIME lost the master clock entirely — the 2015 workaround for SUSE's 2.6.16 freshness heuristic, which is better satisfied by sliding the reference point forward when the legacy MSR is rewritten (new patch 24).
KVM_[GS]ET_CLOCK_GUEST:
• The guest TSC comparison mishandled wraparound.
• The frequency sanity check could underflow, rejected two hosts calibrated exactly 1kHz apart, and hvclock_to_hz() could wrap on absurd user-supplied mul/shift.
• The -EBUSY case was unreachable, and GET returned from inside the seqcount read section.
Elsewhere:
• kvm_vm_ioctl_set_clock() wrapped if the host clock had legally been set to within moments of the 1970 epoch.
• clang couldn't see that guest_now was always initialized in kvm_xen_start_timer() on 32-bit builds (kernel test robot³).
• Comments and commit messages overclaimed in places, the kvm_update_master_clock tracepoint still said "offsetmatched", the tools/ uapi header mirror had drifted, and api.rst didn't document the new KVM_SET_TSC_KHZ behaviour.
• Selftests skipped on any errno rather than just -ENODATA, could pass on a buggy kernel under adverse scheduling, lost a whole frequency's coverage when hardware scaling wasn't engaged, and carried a third private copy of the pvclock arithmetic.
¹ https://lore.kernel.org/all/[email protected]
² https://lore.kernel.org/all/[email protected]
³ https://lore.kernel.org/all/[email protected]
David Woodhouse (21):
KVM: x86: Improve synchronization in kvm_synchronize_tsc()
KVM: x86: Kill last_tsc_{nsec,write,offset} fields
KVM: x86: Replace nr_vcpus_matched_tsc count with all_vcpus_matched_tsc bool
KVM: x86: Allow KVM master clock mode when TSCs are offset from each other
KVM: x86: Factor out kvm_use_master_clock()
KVM: x86: Avoid gratuitous global clock updates
KVM: x86: Avoid redundant masterclock updates from multiple vCPUs
KVM: x86: Re-synchronize TSC after KVM_SET_TSC_KHZ
KVM: x86: Cache host vclock_mode for masterclock eligibility checks
KVM: x86: Remove pvclock_gtod_data and private timekeeping code
KVM: x86: Activate master clock from kvm_arch_init_vm()
KVM: selftests: Use UAPI pvclock-abi.h in xen_shinfo_test
KVM: selftests: Add library helpers for clock and TSC-offset ioctls
KVM: x86: Add KVM_VCPU_TSC_SCALE and fix the documentation on TSC migration
KVM: selftests: Add master clock offset test
KVM: selftests: Add Xen/generic CPUID timing leaf test
KVM: selftests: Add Xen runstate migration test
KVM: selftests: Test that the first user TSC write is honoured exactly
KVM: x86/xen: Drop open-coded fallback clock read in kvm_xen_start_timer()
KVM: x86: Use a single time snapshot in kvm_vm_ioctl_set_clock()
KVM: x86: Keep master clock for guests using legacy MSR_KVM_SYSTEM_TIME
Jack Allister (3):
UAPI: x86: Move pvclock-abi to UAPI for x86 platforms
KVM: x86: Add KVM_[GS]ET_CLOCK_GUEST for accurate KVM clock migration
KVM: selftests: Add KVM/PV clock selftest to prove timer correction
Documentation/virt/kvm/api.rst | 61 ++
Documentation/virt/kvm/devices/vcpu.rst | 137 ++-
Documentation/virt/kvm/locking.rst | 2 +-
MAINTAINERS | 4 +-
arch/x86/include/asm/kvm_host.h | 14 +-
arch/x86/include/uapi/asm/kvm.h | 7 +
arch/x86/include/{ => uapi}/asm/pvclock-abi.h | 27 +-
arch/x86/kvm/msrs.c | 28 +-
arch/x86/kvm/trace.h | 12 +-
arch/x86/kvm/x86.c | 957 ++++++++++++++----
arch/x86/kvm/x86.h | 2 +-
arch/x86/kvm/xen.c | 48 +-
include/uapi/linux/kvm.h | 4 +
scripts/xen-hypercalls.sh | 2 +-
tools/arch/x86/include/asm/pvclock-abi.h | 27 +-
tools/arch/x86/include/uapi/asm/kvm.h | 6 +
tools/testing/selftests/kvm/Makefile.kvm | 6 +
.../selftests/kvm/include/x86/processor.h | 50 +
.../kvm/x86/masterclock_offset_test.c | 203 ++++
.../kvm/x86/pvclock_migration_test.c | 521 ++++++++++
.../testing/selftests/kvm/x86/pvclock_test.c | 518 ++++++++++
.../selftests/kvm/x86/tsc_user_set_test.c | 153 +++
.../selftests/kvm/x86/xen_cpuid_timing_test.c | 271 +++++
.../selftests/kvm/x86/xen_migration_test.c | 222 ++++
.../selftests/kvm/x86/xen_shinfo_test.c | 31 +-
25 files changed, 3010 insertions(+), 303 deletions(-)
rename arch/x86/include/{ => uapi}/asm/pvclock-abi.h (82%)
create mode 100644 tools/testing/selftests/kvm/x86/masterclock_offset_test.c
create mode 100644 tools/testing/selftests/kvm/x86/pvclock_migration_test.c
create mode 100644 tools/testing/selftests/kvm/x86/pvclock_test.c
create mode 100644 tools/testing/selftests/kvm/x86/tsc_user_set_test.c
create mode 100644 tools/testing/selftests/kvm/x86/xen_cpuid_timing_test.c
create mode 100644 tools/testing/selftests/kvm/x86/xen_migration_test.cbase-commit: dea4f6380f4ec96e8f3b0aaf2343764bd0d25ef4
Signature · 1 line
2.43.0
David Woodhouse
maintainerPATCH v12 1/24 KVM: x86: Improve synchronization in kvm_synchronize_tsc()
From: David Woodhouse <[email protected]>
When synchronizing to an existing TSC (either by explicitly writing zero, or the legacy hack where the TSC is written within one second's worth of the previously written TSC), the last_tsc_write and last_tsc_nsec values were being misrecorded by __kvm_synchronize_tsc(). The *unsynchronized* value of the TSC (perhaps even zero) was being recorded, along with the current time at which kvm_synchronize_tsc() was called. This could cause *subsequent* writes to fail to synchronize correctly.
Fix that by extrapolating the generation's TSC value to the current time (data = cur_tsc_write + elapsed cycles) before passing it to __kvm_synchronize_tsc(), so the recorded {data, ns} pair always lies on the generation's line and the one-second matching window slides forward with each synchronized write. When the TSC is stable, reuse the generation's offset exactly; when it is unstable, compute a new offset for the extrapolated value.
Furthermore, there were *three* different TSC reads used for calculating the "current" time, all slightly different from each other. Fix that by using kvm_get_time_and_clockread() where possible and using the same host_tsc value in all cases.
Extrapolating the matching window from the generation's origin at the nominal guest frequency means that, for a guest whose requested frequency is within tsc_tolerance_ppm of the host's (so hardware scaling is not engaged), the window drifts from the guest's actual TSC line by up to 250ppm — crossing the one-second window after about 67 minutes. A write which then fails to match still installs the exact value with a freshly computed offset; the cost is only a fresh generation. The previous code re-anchored on every matched write, which bounded that drift per-interval but recorded values the generation's line did not pass through, which is the bug this patch exists to fix.
--- arch/x86/kvm/x86.c | 39 ++++++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 9 deletions(-)
arch/x86/kvm/x86.c +30 -9
@@ -210,6 +210,10 @@ module_param(pi_inject_timer, bint, 0644);static bool __read_mostly mitigate_smt_rsb;module_param (mitigate_smt_rsb, bool,0444 ); +#ifdef CONFIG_X86_64 +static bool kvm_get_time_and_clockread (s64 *kernel_ns,u64 *tsc_timestamp); +#endif +#define KVM_SUPPORTED_XCR0 (XFEATURE_MASK_FP | XFEATURE_MASK_SSE \ | XFEATURE_MASK_YMM | XFEATURE_MASK_BNDREGS \ | XFEATURE_MASK_BNDCSR | XFEATURE_MASK_AVX512 \
@@ -1374,14 +1378,23 @@ void kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 *user_value) {u64 data = user_value ? *user_value :0 ;struct kvm *kvm = vcpu->kvm; -u64 offset, ns, elapsed; +u64 offset, host_tsc, elapsed; +s64 ns;unsigned long flags;bool matched = false;bool synchronizing = false;raw_spin_lock_irqsave (&kvm->arch.tsc_write_lock, flags); - offset =kvm_compute_l1_tsc_offset (vcpu,rdtsc (), data); - ns =get_kvmclock_base_ns (); + +#ifdef CONFIG_X86_64 + if (!kvm_get_time_and_clockread(&ns, &host_tsc)) +#endif + { + host_tsc =rdtsc (); + ns =get_kvmclock_base_ns (); + } + + offset =kvm_compute_l1_tsc_offset (vcpu, host_tsc, data); elapsed = ns - kvm->arch.last_tsc_nsec;if (vcpu->arch.virtual_tsc_khz) {
@@ -1424,13 +1437,21 @@ void kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 *user_value)*/ if (synchronizing && vcpu->arch.virtual_tsc_khz == kvm->arch.last_tsc_khz) { -if (!kvm_check_tsc_unstable ()) { +/* +* If synchronizing, advance the reference point to "now" +* so the matching window slides forward with each vCPU. +* Extrapolate from the generation's own origin: 'elapsed' +* is measured from the most recent write, which lies on +* the generation's line only for the first matched write. +*/ +u64 delta =nsec_to_cycles (vcpu, ns - kvm->arch.cur_tsc_nsec); + + data = kvm->arch.cur_tsc_write + delta; + +if (!kvm_check_tsc_unstable ()) offset = kvm->arch.cur_tsc_offset; - }else { -u64 delta =nsec_to_cycles (vcpu, elapsed); - data += delta; - offset =kvm_compute_l1_tsc_offset (vcpu,rdtsc (), data); - } +else + offset =kvm_compute_l1_tsc_offset (vcpu, host_tsc, data); matched = true; }
Signature · 1 line
2.43.0
David Woodhouse
maintainerPATCH v12 2/24 KVM: x86: Kill last_tsc_{nsec,write,offset} fields
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
These mostly duplicate the cur_tsc_{nsec,write,offset} values. In the matching paths for a stable TSC, the values used were definitely identical. On the unstable-TSC paths the two could diverge (a matched synchronization recorded its freshly computed offset in last_tsc_offset while cur_tsc_offset kept the generation's reference), so matching in kvm_arch_tsc_set_attr() now compares against the generation's reference offset instead. With an unstable TSC the master clock is off and the match only affects generation accounting, and matching against the generation's own reference is the more principled behaviour anyway.
Rename last_tsc_khz and last_tsc_scaling_ratio to cur_tsc_khz and cur_tsc_scaling_ratio respectively, since they are properties of the current TSC generation.
Since cur_tsc_offset now serves as the single source of truth for the TSC generation's offset, the backwards-TSC adjustment (used during S4 resume when the host TSC resets) must also update cur_tsc_offset by the same delta applied to all vCPUs (scaled to the guest TSC rate, as adjust_tsc_offset_host() does). Without this, subsequent vCPUs that synchronize after the adjustment would get the stale pre-S4 offset.
Since a synchronization now establishes an absolute offset which (via a matched reuse of cur_tsc_offset) already includes any backwards-TSC delta, clear the vCPU's pending tsc_offset_adjustment when writing it:
the deferred per-vCPU compensation would otherwise be applied on top at the next vcpu_load, doubling the delta. Previously a nonzero write could not hit this, because zeroing the last_tsc values broke the 1-second matching window after a backwards TSC; a forced synchronization (vCPU creation, or a zero write) could always match, and takes the compensated offset as intended.
The lockless vm_list walk in kvm_arch_enable_virtualization_cpu() is unsound against a concurrent kvm_destroy_vm(), but that predates this patch and is not made materially worse by it; it cannot be fixed by taking kvm_lock here, which would recreate the lock inversion against cpus_read_lock() that commit 44d174596260 ("KVM: Use dedicated mutex to protect kvm_usage_count to avoid deadlock") untangled.
--- Documentation/virt/kvm/locking.rst | 2 +- arch/x86/include/asm/kvm_host.h | 7 +-- arch/x86/kvm/x86.c | 98 +++++++++++++++++++++--------- 3 files changed, 71 insertions(+), 36 deletions(-)
Documentation/virt/kvm/locking.rst +1 -1
@@ -263,7 +263,7 @@ time it will be set using the Dirty tracking mechanism described above. :Type: raw_spinlock_t :Arch: x86 -:Protects: - kvm_arch::{last_tsc_write,last_tsc_nsec,last_tsc_offset} +:Protects: - kvm_arch::{cur_tsc_write,cur_tsc_nsec,cur_tsc_offset} - tsc offset in vmcb :Comment: 'raw' because updating the tsc offsets must not be preempted.
arch/x86/include/asm/kvm_host.h +2 -5
@@ -1236,11 +1236,8 @@ struct kvm_arch {* preemption-disabled region, so it must be a raw spinlock. */ raw_spinlock_t tsc_write_lock; -u64 last_tsc_nsec; -u64 last_tsc_write; -u32 last_tsc_khz; -u64 last_tsc_offset; -u64 last_tsc_scaling_ratio; +u32 cur_tsc_khz; +u64 cur_tsc_scaling_ratio;u64 cur_tsc_nsec;u64 cur_tsc_write;u64 cur_tsc_offset;
arch/x86/kvm/x86.c +68 -30
164 lines not on this page · raw message
Signature · 1 line
2.43.0
David Woodhouse
maintainerPATCH v12 3/24 KVM: x86: Replace nr_vcpus_matched_tsc count with all_vcpus_matched_tsc bool
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
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().
--- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/x86.c | 41 +++++++++++++++++++++++++++++---- 2 files changed, 37 insertions(+), 5 deletions(-)
arch/x86/include/asm/kvm_host.h +1 -0
@@ -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;
arch/x86/kvm/x86.c +36 -5
72 lines not on this page · raw message
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH v12 03/24] KVM: x86: Replace nr_vcpus_matched_tsc count with all_vcpus_matched_tsc bool
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [Medium] KVM: x86: incomplete TSC match invalidation in kvm_arch_vcpu_postcreate() - [Medium] KVM: x86…
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [Medium] KVM: x86: incomplete TSC match invalidation in kvm_arch_vcpu_postcreate()
- [Medium] KVM: x86: stale cur_tsc_scaling_ratio for scaled protected-TSC guests
--
commit 496cc2fbe510de0a04aff77a0441a50fcb8f4d44
Author: David Woodhouse <[email protected]>
KVM: x86: Replace nr_vcpus_matched_tsc count with all_vcpus_matched_tsc bool
This commit replaces the read-time comparison of nr_vcpus_matched_tsc against kvm->online_vcpus with a cached all_vcpus_matched_tsc boolean updated under tsc_write_lock and initialized to true at VM creation.
Quoted 4 lines
arch/x86/kvm/x86.c +0
-0
[ ... ]
Quoted 12 lines
arch/x86/kvm/x86.c +7
-1@@ -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
arch/x86/kvm/x86.c int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) · line 9573
Quoted 10 lines
arch/x86/kvm/x86.c +9
-0+* 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 ; + }
[Severity: Medium]
Does clearing ka->all_vcpus_matched_tsc here without calling kvm_update_masterclock() leave ka->use_master_clock stale?
ka->all_vcpus_matched_tsc is consumed by pvclock_update_vm_gtod_copy(), so returning without triggering a masterclock update leaves ka->use_master_clock enabled (initially set in kvm_arch_init_vm()) and does not request KVM_REQ_CLOCK_UPDATE on existing vCPUs. Even after commit "KVM:
x86: Allow KVM master clock mode when TSCs are offset from each other" checks ka->all_vcpus_matched_tsc in kvm_guest_time_update() and clears ka->all_vcpus_matched_freq here, ka->use_master_clock remains true and KVM_GET_CLOCK still reports KVM_CLOCK_TSC_STABLE.
Also, __kvm_synchronize_tsc() returns early when vcpu->arch.guest_tsc_protected is true, and KVM_RUN does not call kvm_synchronize_tsc(), so clearing the flag on this path will not heal for protected-TSC guests or on normal vCPU entry; conversely, if the thread holding vcpu->mutex already synchronized the vCPU via a host-initiated TSC write, this error path overwrites the matched state back to false.
Quoted 14 lines
arch/x86/kvm/x86.c +7
-0vcpu_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
arch/x86/kvm/x86.c int kvm_arch_init_vm(struct kvm *kvm, unsigned long type) · line 10065
Quoted 10 lines
arch/x86/kvm/x86.c +7
-0+* 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);
[Severity: Medium]
Does initializing kvm->arch.cur_tsc_scaling_ratio to kvm_caps.default_tsc_scaling_ratio here leave the masterclock unscaled for protected-TSC guests configured with a scaled TSC frequency?
Protected-TSC guests configure their TSC frequency before vCPU creation via the VM-level KVM_SET_TSC_KHZ ioctl, which only updates kvm->arch.default_tsc_khz. When kvm_arch_vcpu_create() later calls kvm_set_tsc_khz(), the scaled ratio is stored in vcpu->arch.l1_tsc_scaling_ratio, but __kvm_synchronize_tsc() returns early on vcpu->arch.guest_tsc_protected before copying l1_tsc_scaling_ratio into kvm->arch.cur_tsc_scaling_ratio or requesting KVM_REQ_MASTERCLOCK_UPDATE:
arch/x86/kvm/x86.c:__kvm_synchronize_tsc() {
if (vcpu->arch.guest_tsc_protected)
return;
...
kvm->arch.cur_tsc_khz = vcpu->arch.virtual_tsc_khz;
kvm->arch.cur_tsc_scaling_ratio = vcpu->arch.l1_tsc_scaling_ratio;
}As a result, kvm->arch.master_tsc_scaling_ratio, master_tsc_shift, and master_tsc_mul remain computed for the 1:1 host TSC frequency, so __get_kvmclock_master_clock() (and KVM_GET_CLOCK_GUEST in commit "KVM: x86:
Add KVM_[GS]ET_CLOCK_GUEST for accurate KVM clock migration") computes kvmclock with unscaled host parameters that drift from the scaled pvclock time info exposed to the guest.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
David Woodhouse
maintainerPATCH v12 4/24 KVM: x86: Allow KVM master clock mode when TSCs are offset from each other
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
Previously, a guest writing different TSC values on different vCPUs could force KVM out of master clock mode. With this change, only a frequency mismatch disables master clock. The only ways for non-master-clock mode to happen now are archaic hardware without a TSC-based clocksource, a VMM that sets different TSC frequencies across vCPUs, or a guest using the legacy MSR_KVM_SYSTEM_TIME (which could be addressed in future by simply updating tsc_timestamp more frequently rather than falling out of master clock mode entirely).
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 guest TSC.
But vCPUs having different TSC *offsets* is not a problem, because the KVM clock is a single VM-wide nanosecond timeline; only its expression in TSC cycles is per-vCPU. Each vCPU's pvclock structure is generated by translating the shared master clock snapshot into that vCPU's own TSC domain via kvm_read_l1_tsc(), which applies the vCPU's scaling ratio and offset to the snapshot's tsc_timestamp. When the guest then computes system_time + (rdtsc - tsc_timestamp) * rate, its own offset is present in both rdtsc and tsc_timestamp and cancels out: every vCPU reads the same VM-wide clock from its own pvclock. VM-scoped consumers of the master clock (KVM_GET_CLOCK, the wall clock epoch, the Xen runstate times) operate purely in the nanosecond domain, where the per-vCPU offset never appears at all.
Only two consumers genuinely depend on the TSC offsets matching, and both are already guarded by PVCLOCK_TSC_STABLE_BIT:
1. A guest vCPU reading a *sibling* vCPU's pvclock structure with its own TSC would compute garbage. This is precisely why commit b48aa97e3820 ("KVM: x86: require matched TSC offsets for master clock") required matched offsets. But the guest-side guard for it already
exists: when PVCLOCK_TSC_STABLE_BIT is clear, guests fall back to the global last_value monotonicity clamp for cross-vCPU reads.
2. The Hyper-V TSC page is a single VM-wide {scale, offset} pair which every guest vCPU applies to its own rdtsc, so it cannot represent divergent offsets. It is self-guarding: compute_tsc_page_parameters() refuses to populate the page unless PVCLOCK_TSC_STABLE_BIT is set, which forces such guests onto the (nanosecond domain, and thus safe) TIME_REF_COUNT MSR instead.
So: use frequency match for master clock eligibility, and full TSC match (including offset) only for PVCLOCK_TSC_STABLE_BIT, which tells the guest it is safe to skip cross-vCPU monotonicity enforcement.
Track frequency matching separately from offset matching using a dedicated freq generation counter (cur_tsc_freq_generation) that only bumps on actual frequency changes. Each vCPU is counted exactly once per freq generation via a per-vCPU this_tsc_freq_generation field, preventing repeated syncs of the same vCPU from falsely re-enabling master clock.
Note that the generation-based counting has a known limitation: if all vCPUs are in sync and one changes away and then back again, the other vCPUs are still at the old generation and won't be counted until they sync again (which may never happen). This was always the case for the offset tracking and isn't expected VMM behaviour — although it is the scenario that the VM-wide KVM_SET_TSC_KHZ ioctl was introduced to handle cleanly.
While at it, restructure the existing TSC offset generation tracking to use the same pattern: reset counter to zero on new generation, then unconditionally count vCPUs that haven't been seen in this generation. Both counters now use a consistent >= online_vcpus threshold (1-based counting where the reference vCPU is included in the count).
The divergent-offset behaviour is exercised by pvclock_migration_test, which runs vCPUs with TSCs offset by ±0.5-1s from each other and verifies that the master clock survives, that PVCLOCK_TSC_STABLE_BIT is cleared, and that all vCPUs nevertheless agree on the KVM clock to within a nanosecond, before and after a simulated live migration.
KVM_SET_TSC_KHZ on a vCPU which has already run does not pass through __kvm_synchronize_tsc(), so at this point in the series it leaves the frequency-match state stale, exactly as it previously left the offset match state stale. The later patch "KVM: x86: Re-synchronize TSC after KVM_SET_TSC_KHZ" funnels it through the invalidation above.
--- arch/x86/include/asm/kvm_host.h | 4 ++ arch/x86/kvm/trace.h | 12 ++-- arch/x86/kvm/x86.c | 114 ++++++++++++++++++++++++++------ 3 files changed, 102 insertions(+), 28 deletions(-)
arch/x86/include/asm/kvm_host.h +4 -0
@@ -887,6 +887,7 @@ struct kvm_vcpu_arch {u64 this_tsc_nsec;u64 this_tsc_write;u64 this_tsc_generation; +u64 this_tsc_freq_generation;bool tsc_catchup;bool tsc_always_catchup;s8 virtual_tsc_shift;
@@ -1243,6 +1244,9 @@ struct kvm_arch {u64 cur_tsc_offset;u64 cur_tsc_generation;bool all_vcpus_matched_tsc; +bool all_vcpus_matched_freq; +int nr_vcpus_matched_freq; +u64 cur_tsc_freq_generation;int nr_vcpus_matched_tsc;u32 default_tsc_khz;
arch/x86/kvm/trace.h +6 -6
@@ -1003,25 +1003,25 @@ TRACE_EVENT(kvm_write_tsc_offset, {VDSO_CLOCKMODE_TSC, "tsc"} \TRACE_EVENT (kvm_update_master_clock, - TP_PROTO(bool use_master_clock, unsigned int host_clock, bool offset_matched), -TP_ARGS (use_master_clock, host_clock, offset_matched), + TP_PROTO(bool use_master_clock, unsigned int host_clock, bool freq_matched), +TP_ARGS (use_master_clock, host_clock, freq_matched),TP_STRUCT__entry ( __field( bool, use_master_clock ) __field( unsigned int, host_clock ) -__field ( bool, offset_matched ) +__field ( bool, freq_matched ) ),TP_fast_assign ( __entry->use_master_clock = use_master_clock; __entry->host_clock = host_clock; - __entry->offset_matched = offset_matched; + __entry->freq_matched = freq_matched; ), -TP_printk ("masterclock %d hostclock %s offsetmatched %u" , +TP_printk ("masterclock %d hostclock %s freqmatched %u" , __entry->use_master_clock,__print_symbolic (__entry->host_clock, host_clocks), - __entry->offset_matched) + __entry->freq_matched) ); TRACE_EVENT(kvm_track_tsc,
arch/x86/kvm/x86.c +92 -22
195 lines not on this page · raw message
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH v12 04/24] KVM: x86: Allow KVM master clock mode when TSCs are offset from each other
Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [Low] KVM: x86: missing masterclock update on error in kvm_arch_vcpu_postcreate() --
Thank you for your contribution! Sashiko AI review found 1 potential issue to consider:
- [Low] KVM: x86: missing masterclock update on error in kvm_arch_vcpu_postcreate()
--
commit 63e87344888586250d53904cdb6246fdd7dee791
Author: David Woodhouse <[email protected]>
KVM: x86: Allow KVM master clock mode when TSCs are offset from each other
This commit tracks vCPU TSC frequency matching separately from TSC offset matching so that master clock mode remains enabled when vCPUs have matching frequencies but different TSC offsets, while requiring matched offsets for PVCLOCK_TSC_STABLE_BIT.
Quoted 4 lines
arch/x86/kvm/x86.c +0
-0
[ ... ]
arch/x86/kvm/x86.c void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu) · line 9642
Quoted 8 lines
arch/x86/kvm/x86.c +1
-0@@ -9568,6 +9636,7 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)raw_spin_lock_irqsave (&ka->tsc_write_lock, flags);WRITE_ONCE (ka->all_vcpus_matched_tsc, false); + ka->all_vcpus_matched_freq = false;raw_spin_unlock_irqrestore (&ka->tsc_write_lock, flags);return ; }
[Severity: Low]
Does this error path also need to call kvm_update_masterclock(vcpu->kvm) when mutex_lock_killable(&vcpu->mutex) is interrupted?
Without kvm_update_masterclock(), pvclock_update_vm_gtod_copy() does not run, so ka->use_master_clock stays true while the new unsynchronized vCPU is runnable, and already-running vCPUs do not receive a KVM_REQ_CLOCK_UPDATE broadcast to clear PVCLOCK_TSC_STABLE_BIT in their guest pvclock structures.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
David Woodhouse
maintainerPATCH v12 5/24 KVM: x86: Factor out kvm_use_master_clock()
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
Both kvm_track_tsc_matching() and pvclock_update_vm_gtod_copy() make a decision about whether the KVM clock should be in master clock mode. They used *different* criteria for the decision though. This isn't really a problem; it only has the potential to cause unnecessary invocations of KVM_REQ_MASTERCLOCK_UPDATE if the masterclock was disabled due to TSC going backwards, or the guest using the old MSR. But it isn't pretty.
Factor the decision out to a single function. And document the historical reason why it's disabled for guests that use the old MSR_KVM_SYSTEM_TIME.
--- arch/x86/kvm/x86.c | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-)
arch/x86/kvm/x86.c +30 -10
@@ -1112,11 +1112,30 @@ static inline bool gtod_is_based_on_tsc(int mode) {return mode ==VDSO_CLOCKMODE_TSC || mode ==VDSO_CLOCKMODE_HVCLOCK ; } -#endif + +static bool kvm_use_master_clock (struct kvm *kvm) +{ +struct kvm_arch *ka = &kvm->arch; + +/* +* The 'old kvmclock' check is a workaround (from 2015) for a +* SUSE 2.6.16 kernel that didn't boot if the system_time in +* its kvmclock was too far behind the current time. So the +* mode of just setting the reference point and allowing time +* to proceed linearly from there makes it fail to boot. +* Despite that being kind of the *point* of the way the clock +* is exposed to the guest. By coincidence, the offending +* kernels used the old MSR_KVM_SYSTEM_TIME, which was moved +* only because it resided in the wrong number range. So the +* workaround is activated for *all* guests using the old MSR. +*/ +return ka->all_vcpus_matched_freq && + !ka->backwards_tsc_observed && + !ka->boot_vcpu_runs_old_kvmclock; +}static void kvm_track_tsc_matching (struct kvm_vcpu *vcpu,bool update_mclock) { -#ifdef CONFIG_X86_64 struct kvm_arch *ka = &vcpu->kvm->arch;struct pvclock_gtod_data *gtod = &pvclock_gtod_data;bool prev_matched_tsc = ka->all_vcpus_matched_tsc;
@@ -1171,7 +1190,7 @@ static void kvm_track_tsc_matching(struct kvm_vcpu *vcpu, bool update_mclock)* are fine — each vCPU's pvclock has its own tsc_timestamp that * accounts for its offset. */ -bool use_master_clock = ka->all_vcpus_matched_freq && +bool use_master_clock =kvm_use_master_clock (vcpu->kvm) &>od_is_based_on_tsc (gtod->clock.vclock_mode);/*
@@ -1186,8 +1205,11 @@ static void kvm_track_tsc_matching(struct kvm_vcpu *vcpu, bool update_mclock)trace_kvm_track_tsc (vcpu->vcpu_id, ka->nr_vcpus_matched_tsc,atomic_read (&vcpu->kvm->online_vcpus), ka->use_master_clock, gtod->clock.vclock_mode); -#endif } +#else +static inline void kvm_track_tsc_matching (struct kvm_vcpu *vcpu, +bool update_mclock) {} +#endif /* * Multiply tsc by a fixed point number represented by ratio.
@@ -1661,10 +1683,9 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm)#ifdef CONFIG_X86_64 struct kvm_arch *ka = &kvm->arch;int vclock_mode; -bool host_tsc_clocksource, freqs_matched; +bool host_tsc_clocksource;lockdep_assert_held (&kvm->arch.tsc_write_lock); - freqs_matched = ka->all_vcpus_matched_freq;/* * If the host uses TSC clock, then passthrough TSC as stable
@@ -1674,9 +1695,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 && freqs_matched - && !ka->backwards_tsc_observed - && !ka->boot_vcpu_runs_old_kvmclock; + ka->use_master_clock = host_tsc_clocksource && +kvm_use_master_clock (kvm);if (ka->use_master_clock) {u64 tsc_hz;
@@ -1704,7 +1724,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, - freqs_matched); + ka->all_vcpus_matched_freq);#endif }
Signature · 1 line
2.43.0
David Woodhouse
maintainerPATCH v12 6/24 KVM: x86: Avoid gratuitous global clock updates
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
Eliminate two sources of unnecessary KVM_REQ_GLOBAL_CLOCK_UPDATE:
1. kvm_write_system_time(): The global clock update was a workaround for ever-drifting clocks based on the host's CLOCK_MONOTONIC subject to NTP skew. Now that the KVM clock uses CLOCK_MONOTONIC_RAW, the clock does not drift with NTP corrections and there is no need to synchronize all vCPUs on boot or resume. Use KVM_REQ_CLOCK_UPDATE on the vCPU itself, and only when the clock is being enabled, not disabled.
2. kvm_arch_vcpu_load(): In master clock mode, migration between pCPUs does not require any clock update since the master clock reference is shared. Only request a local KVM_REQ_CLOCK_UPDATE for the vCPU's first-ever load (vcpu->cpu == -1) to generate initial pvclock params. In non-master-clock mode, keep the global update to synchronize all vCPUs.
--- arch/x86/kvm/msrs.c | 7 ++++--- arch/x86/kvm/x86.c | 4 +++- 2 files changed, 7 insertions(+), 4 deletions(-)
arch/x86/kvm/msrs.c +4 -3
@@ -1285,14 +1285,15 @@ static void kvm_write_system_time(struct kvm_vcpu *vcpu, gpa_t system_time, } vcpu->arch.time = system_time; -kvm_make_request (KVM_REQ_GLOBAL_CLOCK_UPDATE , vcpu);/* we verify if the enable bit is set... */ -if (system_time &1 ) +if (system_time &1 ) {kvm_gpc_activate (&vcpu->arch.pv_time, system_time & ~1ULL ,sizeof (struct pvclock_vcpu_time_info )); -else +kvm_make_request (KVM_REQ_CLOCK_UPDATE , vcpu); + }else {kvm_gpc_deactivate (&vcpu->arch.pv_time); + }return ; }
arch/x86/kvm/x86.c +3 -1
@@ -2723,11 +2723,13 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)* On a host with synchronized TSC, there is no need to update * kvmclock on vcpu->cpu migration */ -if (!vcpu->kvm->arch.use_master_clock || vcpu->cpu ==-1 ) { +if (!vcpu->kvm->arch.use_master_clock) {if (__ratelimit (&vcpu->kvm->arch.kvmclock_update_rs))kvm_make_request (KVM_REQ_GLOBAL_CLOCK_UPDATE , vcpu);else kvm_make_request (KVM_REQ_CLOCK_UPDATE , vcpu); + }else if (vcpu->cpu ==-1 ) { +kvm_make_request (KVM_REQ_CLOCK_UPDATE , vcpu); }if (vcpu->cpu != cpu)
Signature · 1 line
2.43.0
David Woodhouse
maintainerPATCH v12 7/24 KVM: x86: Avoid redundant masterclock updates from multiple vCPUs
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
When a masterclock update is triggered (e.g. by the clocksource change notifier), KVM_REQ_MASTERCLOCK_UPDATE is set on all vCPUs. Without this fix, each vCPU independently processes the request and redundantly re-executes the entire pvclock_update_vm_gtod_copy() sequence, serialized only by tsc_write_lock. Each redundant re-snapshot of the master clock reference point introduces potential clock drift.
Fix this by having __kvm_start_pvclock_update() check, after acquiring the lock, whether the requesting vCPU's KVM_REQ_MASTERCLOCK_UPDATE is still set. If another vCPU already did the update and cleared it, bail out. Otherwise, clear the request on all other vCPUs before proceeding.
The caller in vcpu_enter_guest() now uses kvm_test_request() (non-clearing) since the clearing is done inside __kvm_start_pvclock_update() under the lock.
kvm_end_pvclock_update() now broadcasts KVM_REQ_CLOCK_UPDATE and clears KVM_REQ_MCLOCK_INPROGRESS while still inside the interrupts-off lock section with the seqcount held open. That ordering is load-bearing for the bail-out path: a vCPU whose request was consumed by another updater must be guaranteed to observe either the open seqcount or its pending KVM_REQ_CLOCK_UPDATE before it can enter the guest.
Since the update now consumes pending KVM_REQ_MASTERCLOCK_UPDATE requests, the Hyper-V reenlightenment notifier must invalidate the Hyper-V TSC page itself; previously a pending request survived the notifier and refreshed the page via kvm_update_masterclock() later.
--- arch/x86/kvm/msrs.c | 11 ++++-- arch/x86/kvm/x86.c | 89 ++++++++++++++++++++++++++++++++++++++------- 2 files changed, 83 insertions(+), 17 deletions(-)
arch/x86/kvm/msrs.c +8 -3
@@ -1278,10 +1278,15 @@ static void kvm_write_system_time(struct kvm_vcpu *vcpu, gpa_t system_time,struct kvm_arch *ka = &vcpu->kvm->arch;if (vcpu->vcpu_id ==0 && !host_initiated) { -if (ka->boot_vcpu_runs_old_kvmclock != old_msr) +/* +* Store the flag before requesting the masterclock update: +* the update may now be performed by another vCPU (which +* consumes this request), and it must observe the new value. +*/ +if (ka->boot_vcpu_runs_old_kvmclock != old_msr) { + ka->boot_vcpu_runs_old_kvmclock = old_msr;kvm_make_request (KVM_REQ_MASTERCLOCK_UPDATE , vcpu); - - ka->boot_vcpu_runs_old_kvmclock = old_msr; + } } vcpu->arch.time = system_time;
arch/x86/kvm/x86.c +75 -14
136 lines not on this page · raw message
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH v12 07/24] KVM: x86: Avoid redundant masterclock updates from multiple vCPUs
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [Medium] KVM: x86: premature kvm_guest_time_update() in __kvm_start_pvclock_update() - [Medium] KVM: x86…
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [Medium] KVM: x86: premature kvm_guest_time_update() in __kvm_start_pvclock_update()
- [Medium] KVM: x86: race in __kvm_start_pvclock_update() lets vCPUs skip clock update
--
commit 562c60eff9324c1010b59724e91ff05f45041197
Author: David Woodhouse <[email protected]>
KVM: x86: Avoid redundant masterclock updates from multiple vCPUs
This commit deduplicates VM-wide masterclock updates by clearing pending KVM_REQ_MASTERCLOCK_UPDATE bits across all vCPUs inside __kvm_start_pvclock_update() under tsc_write_lock and bailing out if another vCPU already serviced the request.
Quoted 37 lines
arch/x86/kvm/x86.c +26
-1@@ -1733,10 +1733,51 @@ static void kvm_make_mclock_inprogress_request(struct kvm *kvm)kvm_make_all_cpus_request (kvm,KVM_REQ_MCLOCK_INPROGRESS ); } -static void __kvm_start_pvclock_update (struct kvm *kvm) +static void kvm_clear_mclock_inprogress_request (struct kvm *kvm) { +struct kvm_vcpu *vcpu; +unsigned long i; + +kvm_for_each_vcpu (i, vcpu, kvm) +kvm_clear_request (KVM_REQ_MCLOCK_INPROGRESS , vcpu); +} + +static bool __kvm_start_pvclock_update (struct kvm *kvm,struct kvm_vcpu *requesting_vcpu) +{ +struct kvm_vcpu *vcpu; +unsigned long i; +raw_spin_lock_irq (&kvm->arch.tsc_write_lock); + +/* +* If another vCPU already did the update while we were waiting +* for the lock, our request will have been cleared. Bail out, +* clearing the KVM_REQ_MCLOCK_INPROGRESS kicks made on entry to +* kvm_update_masterclock() so vCPUs can enter the guest again. +* +* Clearing them VM-wide can also clear kicks just made by yet +* another updater still waiting for tsc_write_lock, letting +* vCPUs run while that update is in flight. That is not new: +* kvm_end_pvclock_update() has always cleared the request on +* every vCPU regardless of which updater made it. The window is +* harmless to readers (the pvclock seqcount covers the master
arch/x86/kvm/x86.c static void kvm_make_mclock_inprogress_request(struct kvm *kvm) · line 1773
Quoted 10 lines
arch/x86/kvm/x86.c +10
-0+* state); a vCPU which slips in runs on the old snapshot until +* its KVM_REQ_CLOCK_UPDATE is serviced, exactly as if it had +* entered just before the update began. +*/ +if (requesting_vcpu && + !kvm_test_request (KVM_REQ_MASTERCLOCK_UPDATE , requesting_vcpu)) { +kvm_clear_mclock_inprogress_request (kvm); +raw_spin_unlock_irq (&kvm->arch.tsc_write_lock); +return false; + }
[Severity: Medium]
Can clearing KVM_REQ_MCLOCK_INPROGRESS while another updater is waiting on tsc_write_lock allow a third vCPU whose KVM_REQ_MASTERCLOCK_UPDATE was consumed to enter the guest before the update finishes, and stay in guest mode without servicing KVM_REQ_CLOCK_UPDATE?
Because kvm_make_mclock_inprogress_request() runs before acquiring tsc_write_lock, an updater bailing out (or finishing in kvm_end_pvclock_update()) can clear the KVM_REQ_MCLOCK_INPROGRESS bits just set by a second updater waiting on the lock. When that second updater acquires tsc_write_lock, it clears KVM_REQ_MASTERCLOCK_UPDATE on all vCPUs before setting KVM_REQ_CLOCK_UPDATE in kvm_end_pvclock_update():
CPU0 (vcpu0) CPU1 (vcpu1) CPU2 (vcpu2) ------------ ------------ ------------
kvm_update_masterclock()
/* cleared by prior
updater */
kvm_update_masterclock()
kvm_make_mclock_inprogress_request()
__kvm_start_pvclock_update()
kvm_clear_mclock_inprogress_request()
unlock(tsc_write_lock)
__kvm_start_pvclock_update()
lock(tsc_write_lock)
kvm_clear_request(MASTERCLOCK, vcpu2)
vcpu_enter_guest()
test(MASTERCLOCK) -> 0
check(CLOCK_UPDATE) -> 0
exit_request() -> 0
/* enters guest */
write_seqcount_begin(&pvclock_sc)
pvclock_update_vm_gtod_copy()
kvm_end_pvclock_update()
kvm_make_request(CLOCK_UPDATE, vcpu2)
/* no kick */vcpu_enter_guest() only reads pvclock_sc inside kvm_guest_time_update() when KVM_REQ_CLOCK_UPDATE is set. If vcpu2 only had KVM_REQ_MASTERCLOCK_UPDATE pending (for example, after enabling HV_X64_MSR_REFERENCE_TSC in kvm_hv_set_msr_pw()), it skips both kvm_update_masterclock() and kvm_guest_time_update(), sees KVM_REQ_MCLOCK_INPROGRESS cleared, and enters the guest before kvm_hv_setup_tsc_page() or pvclock_update_vm_gtod_copy() has run. Unlike a vCPU that entered before the update began and was kicked by kvm_make_mclock_inprogress_request(), kvm_end_pvclock_update() calls kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu) without kicking, so vcpu2 remains in guest mode with stale clock state until an unrelated VM-exit.
arch/x86/kvm/x86.c static void kvm_make_mclock_inprogress_request(struct kvm *kvm) · line 1781
Quoted 8 lines
arch/x86/kvm/x86.c +6
-0+ +/* The update is VM-wide; prevent other vCPUs from redoing it. */ +kvm_for_each_vcpu (i, vcpu, kvm) +kvm_clear_request (KVM_REQ_MASTERCLOCK_UPDATE , vcpu); +write_seqcount_begin (&kvm->arch.pvclock_sc); +return true; }
[Severity: Medium]
Does clearing KVM_REQ_MASTERCLOCK_UPDATE on all vCPUs before write_seqcount_begin(&kvm->arch.pvclock_sc) allow another vCPU to run kvm_guest_time_update() against the stale masterclock snapshot?
When both KVM_REQ_MASTERCLOCK_UPDATE and KVM_REQ_CLOCK_UPDATE are pending on vcpu1 (such as after kvm_write_system_time() or __kvm_synchronize_tsc()), clearing KVM_REQ_MASTERCLOCK_UPDATE before opening pvclock_sc lets vcpu1 skip kvm_update_masterclock() without waiting on tsc_write_lock and read pvclock_sc before the write section begins:
CPU0 (vcpu0) CPU1 (vcpu1)
------------ ------------__kvm_start_pvclock_update()
kvm_clear_request(MASTERCLOCK, vcpu1)
vcpu_enter_guest()
test(MASTERCLOCK) -> false
check(CLOCK_UPDATE) -> true
kvm_guest_time_update()
read_seqcount_begin(&pvclock_sc)
/* reads stale masterclock */
kvm_hv_setup_tsc_page() write_seqcount_begin(&pvclock_sc)pvclock_update_vm_gtod_copy()kvm_end_pvclock_update()
In addition to writing stale pvclock data to guest memory, the premature kvm_hv_setup_tsc_page() call consumes HV_TSC_PAGE_HOST_CHANGED or HV_TSC_PAGE_GUEST_CHANGED using the pre-update state and transitions hv_tsc_page_status to HV_TSC_PAGE_SET or HV_TSC_PAGE_BROKEN:
arch/x86/kvm/hyperv.c:kvm_hv_setup_tsc_page() {
...
if (hv->hv_tsc_page_status == HV_TSC_PAGE_BROKEN ||
hv->hv_tsc_page_status == HV_TSC_PAGE_SET ||
hv->hv_tsc_page_status == HV_TSC_PAGE_UNSET)
return;
...
}When kvm_end_pvclock_update() subsequently requests KVM_REQ_CLOCK_UPDATE, kvm_hv_setup_tsc_page() returns early and leaves the Hyper-V reference TSC page stuck with stale parameters or permanently disabled. Opening pvclock_sc with write_seqcount_begin() before the kvm_for_each_vcpu() loop that clears KVM_REQ_MASTERCLOCK_UPDATE would ensure any vCPU that sees its request cleared already observes the open seqcount.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
David Woodhouse
maintainerPATCH v12 8/24 KVM: x86: Re-synchronize TSC after KVM_SET_TSC_KHZ
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
KVM_SET_TSC_KHZ changes the vCPU's TSC scaling ratio but does not update the VM-wide cur_tsc_scaling_ratio used by get_kvmclock(). This causes get_kvmclock() to use a stale (default 1:1) ratio when computing the KVM clock, leading to drift between the host-side kvmclock and what the guest observes.
Fix this by calling kvm_synchronize_tsc() after changing the TSC frequency. This:
- Updates cur_tsc_scaling_ratio (consumed by pvclock_update_vm_gtod_copy)
- Ensures the TSC value is continuous across the frequency change
- Triggers kvm_track_tsc_matching() for proper masterclock handling
- Establishes the new TSC generation for subsequent vCPUs to match
Since kvm_synchronize_tsc() previously inferred "userspace wrote the TSC" from being given a non-NULL value, and this new call is a purely kernel-internal resynchronization, make that an explicit 'user_initiated' parameter instead. Otherwise the kernel's own call would set kvm->arch.user_set_tsc and incorrectly enable the 1-second slop heuristic before userspace has ever written a TSC value, breaking the restore of a guest with less than one second of runtime.
The resynchronization is skipped in two cases. At vCPU creation (which comes through here with the default frequency), the vCPU is not yet counted in online_vcpus, so synchronizing would corrupt the VM-wide matching state — and leave it corrupted if creation subsequently fails; the initial synchronization in kvm_arch_vcpu_postcreate() covers it. And when the frequency is unchanged, there is nothing to
re-synchronize: opening a new generation for a no-op ioctl would needlessly orphan the matched state of the sibling vCPUs.
The preserved value is sampled as a {guest TSC, host clock} tuple and handed to the synchronization together with its reference point, so the new offset is computed against the host TSC the sample was taken at — not against "now". The guest TSC keeps counting through the frequency change, and any delay between sample and installation (preemption included) costs nothing. The new frequency itself is only committed once set_tsc_khz() can no longer fail; a failed ioctl must leave no trace, or a retry with the still-active frequency would wrongly look like a change and open a gratuitous generation.
Because the internal sample is exact, it is not given the 1-second slop window intended for the racy legacy userspace API: a vCPU on the generation's timeline still matches within a 1ms window, but one which genuinely diverged (a guest TSC_ADJUST write, or a deliberate sub-second offset) keeps its exact value rather than being snapped back onto the generation by up to a second. Setting a frequency which no longer needs catchup also clears the always-catchup state which an earlier above-host-rate setting on scaling-less hardware left behind, and which otherwise persisted for the life of the vCPU.
Only preserve the TSC value across a frequency change for a vCPU which has run, or whose TSC userspace has explicitly written (tracked with a new per-vCPU user_set_tsc flag — the VM-wide flag would make one vCPU's restored TSC cause a sibling's unrestored, creation-time value to be preserved, clobbering the generation it could otherwise match). A never-run vCPU holds nothing worth preserving, so synchronize it as for a zero write, placing it on the generation's timeline at the new frequency.
A KVM_GET_CLOCK issued before the resulting masterclock update is serviced still returns the correct time: the masterclock parameters are consumed as one self-consistent snapshot, and a host TSC delta scaled by the snapshot's ratio and converted by its mul/shift yields nanoseconds independent of the guest frequency they encode.
--- Documentation/virt/kvm/api.rst | 8 ++ arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/msrs.c | 2 +- arch/x86/kvm/x86.c | 156 +++++++++++++++++++++++++++----- arch/x86/kvm/x86.h | 2 +- 5 files changed, 142 insertions(+), 27 deletions(-)
Documentation/virt/kvm/api.rst +8 -0
@@ -2023,6 +2023,14 @@ If the KVM_CAP_VM_TSC_CONTROL capability is advertised, this can also be used as a vm ioctl to set the initial tsc frequency of subsequently created vCPUs. Note, the vm ioctl is only allowed prior to creating vCPUs. +Changing the frequency of a vCPU with the vcpu ioctl re-synchronizes +its TSC: for a vCPU which has run, or whose TSC userspace has written, +the current guest TSC value is preserved across the change (the TSC +offset, as visible through the KVM_VCPU_TSC_OFFSET attribute, is +rewritten accordingly); otherwise the vCPU is synchronized to the +VM's existing TSC timeline at the new frequency. Setting the current +frequency again is a no-op. + For TSC protected Confidential Computing (CoCo) VMs where TSC frequency is configured once at VM scope and remains unchanged during VM's lifetime, the vm ioctl should be used to configure the TSC frequency
arch/x86/include/asm/kvm_host.h +1 -0
@@ -888,6 +888,7 @@ struct kvm_vcpu_arch {u64 this_tsc_write;u64 this_tsc_generation;u64 this_tsc_freq_generation; +bool user_set_tsc;bool tsc_catchup;bool tsc_always_catchup;s8 virtual_tsc_shift;
arch/x86/kvm/msrs.c +1 -1
@@ -1645,7 +1645,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)break ;case MSR_IA32_TSC :if (msr_info->host_initiated) { -kvm_synchronize_tsc (vcpu, &data); +kvm_synchronize_tsc (vcpu, &data, true); }else if (!vcpu->arch.guest_tsc_protected) {u64 adj =kvm_compute_l1_tsc_offset (vcpu,rdtsc (), data) - vcpu->arch.l1_tsc_offset;
arch/x86/kvm/x86.c +131 -25
247 lines not on this page · raw message
arch/x86/kvm/x86.h +1 -1
@@ -330,7 +330,7 @@ uint64_t kvm_get_wall_clock_epoch(struct kvm *kvm);bool kvm_get_monotonic_and_clockread (s64 *kernel_ns,u64 *tsc_timestamp);int kvm_guest_time_update (struct kvm_vcpu *v); -void kvm_synchronize_tsc (struct kvm_vcpu *vcpu,u64 *user_value); +void kvm_synchronize_tsc (struct kvm_vcpu *vcpu,u64 *value,bool user_initiated);u64 kvm_scale_tsc (u64 tsc,u64 ratio);u64 kvm_read_l1_tsc (struct kvm_vcpu *vcpu,u64 host_tsc);u64 kvm_calc_nested_tsc_offset (u64 l1_offset,u64 l2_offset,u64 l2_multiplier);
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH v12 08/24] KVM: x86: Re-synchronize TSC after KVM_SET_TSC_KHZ
Thank you for your contribution! Sashiko AI review found 4 potential issues to consider: - [Medium] KVM: x86: unreachable 1ms sync window in kvm_synchronize_tsc_at() - [Medium] KVM: x86: unsigned…
Thank you for your contribution! Sashiko AI review found 4 potential issues to consider:
- [Medium] KVM: x86: unreachable 1ms sync window in kvm_synchronize_tsc_at()
- [Medium] KVM: x86: unsigned elapsed underflow in kvm_synchronize_tsc_at()
- [Medium] KVM: x86: kvm_set_tsc_khz() sync breaks per-vCPU init and restore
- [Medium] KVM: x86: TSC sync in kvm_set_tsc_khz() skews across vCPUs by (F1 - F2) * dt
--
commit 7a8c0445d433604607064df4c153782c818355a7
Author: David Woodhouse <[email protected]>
KVM: x86: Re-synchronize TSC after KVM_SET_TSC_KHZ
This commit re-synchronizes the vCPU's TSC in kvm_set_tsc_khz() across KVM_SET_TSC_KHZ frequency changes, preserving the current guest TSC value when the vCPU has already run or had its TSC written by userspace.
Quoted 4 lines
arch/x86/kvm/x86.c +0
-0
[ ... ]
Quoted 1 line
arch/x86/kvm/x86.c +0
-0@@ -1091,7 +1110,67 @@ static int kvm_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz)
[ ... ]
Quoted 13 lines
arch/x86/kvm/x86.c +13
-0+if (init || !freq_changed) +return 0 ; + +/* +* Changing the frequency changes the vCPU's position on the +* generation's cycle timeline, so re-synchronize. For a vCPU +* which has run, preserve the guest TSC value read above across +* the change; likewise if userspace has written a TSC on this +* vCPU — a zero write would discard it. Otherwise the vCPU holds +* only its creation-time synchronization, so synchronize as for +* a zero write. When the generation is already at the new +* frequency this places the vCPU on its timeline rather than +* opening a gratuitous new generation and orphaning the matched
arch/x86/kvm/x86.c static int kvm_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz) · line 1174
Quoted 10 lines
arch/x86/kvm/x86.c +9
-0+* state of its siblings; the first vCPU to adopt a new frequency +* necessarily opens a new generation, starting at zero — mixed +* frequencies within a VM forfeit the master clock anyway. +*/ +if (vcpu->arch.last_vmentry_cpu !=-1 || vcpu->arch.user_set_tsc) +kvm_synchronize_tsc_at (vcpu, &tsc, host_tsc, ns, false); +else +kvm_synchronize_tsc_at (vcpu,NULL , host_tsc, ns, false); +return 0 ; }
[Severity: Medium]
Does re-synchronizing in kvm_set_tsc_khz() break frequency and TSC matching when userspace creates and configures vCPUs one at a time?
When a VMM runs KVM_CREATE_VCPU followed by KVM_SET_TSC_KHZ(F_new) for vCPU 0, kvm->arch.cur_tsc_khz becomes F_new. When vCPU 1 is then created, kvm_arch_vcpu_postcreate() synchronizes it at default_tsc_khz, which sees virtual_tsc_khz != cur_tsc_khz, bumps cur_tsc_freq_generation and cur_tsc_generation, resets nr_vcpus_matched_freq and nr_vcpus_matched_tsc, and sets cur_tsc_khz back to default_tsc_khz. When KVM_SET_TSC_KHZ(F_new) then runs on vCPU 1, cur_tsc_khz is no longer F_new, so vCPU 1 opens another new generation and orphans vCPU 0; the same ping-pong happens when restoring vCPUs one by one in MSR_IA32_TSC-then-KVM_SET_TSC_KHZ order. After all vCPUs are configured to F_new, all_vcpus_matched_freq and all_vcpus_matched_tsc remain false (disabling masterclock and PVCLOCK_TSC_STABLE_BIT), and repeating KVM_SET_TSC_KHZ on earlier vCPUs cannot recover because !freq_changed returns early.
[ ... ]
Quoted 18 lines
arch/x86/kvm/x86.c +7
-0@@ -1506,6 +1586,7 @@ void kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 *user_value)u64 tsc_exp = kvm->arch.cur_tsc_write +nsec_to_cycles (vcpu, elapsed);u64 tsc_hz = vcpu->arch.virtual_tsc_khz *1000LL ; +u64 window;/* * Here lies UAPI baggage: when a user-initiated TSC write has * a small delta (1 second) of virtual cycle time against the @@ -1520,9 +1601,18 @@ void kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 *user_value)* to this 'correction' to make it sync up with values that only * come from the kernel's default vCPU creation. Make the 1-second * slop hack only trigger if the user_set_tsc flag is already set. +* +* A kernel-internal write (a frequency change preserving the +* guest TSC) carries an exact value sampled with its own +* reference point, not a racy userspace one; a vCPU on the +* generation's line still matches it to within arithmetic +* rounding, but one which genuinely diverged (e.g. by a guest
arch/x86/kvm/x86.c void kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 *user_value) · line 1617
Quoted 10 lines
arch/x86/kvm/x86.c +5
-2+* TSC_ADJUST write) must not be snapped back by up to a +* second. Use a 1ms window for those. */ - synchronizing = data < tsc_exp + tsc_hz && - data + tsc_hz > tsc_exp; + window = user_initiated ? tsc_hz : tsc_hz /1000 ; + synchronizing = data < tsc_exp + window && + data + window > tsc_exp; } }
[Severity: Medium]
Is the 1ms window for !user_initiated unreachable when userspace has never written the TSC?
The window check sits inside:
arch/x86/kvm/x86.c:kvm_synchronize_tsc_at() {
if (vcpu->arch.virtual_tsc_khz) {
if (data == 0) {
synchronizing = true;
} else if (kvm->arch.user_set_tsc) {
...
window = user_initiated ? tsc_hz : tsc_hz / 1000;When KVM_SET_TSC_KHZ is called across vCPUs that have already run (last_vmentry_cpu != -1) without userspace ever writing MSR_IA32_TSC or KVM_VCPU_TSC_OFFSET, kvm_set_tsc_khz() passes a non-zero tsc and user_initiated = false. Because __kvm_synchronize_tsc() only sets kvm->arch.user_set_tsc when user_initiated is true, kvm->arch.user_set_tsc stays false and every subsequent vCPU skips the else-if branch, fails to match the generation, and clears all_vcpus_matched_tsc. Should the condition be "else if (kvm->arch.user_set_tsc || !user_initiated)"?
[Severity: Medium]
Even when that branch is reached, doesn't comparing data against tsc_exp within a 1ms window fail whenever the delay dt between vCPU 0's and a sibling vCPU's KVM_SET_TSC_KHZ ioctls exceeds 1ms * F2 / |F1 - F2|?
When changing multiple vCPUs from frequency F1 to F2 while preserving their TSCs, vCPU 0 anchors cur_tsc_nsec and cur_tsc_write at t0. Over the elapsed time dt before a sibling vCPU's KVM_SET_TSC_KHZ ioctl runs, that sibling is still scaled at F1, so its sampled data = kvm_read_l1_tsc() has advanced by F1 * dt cycles. However, kvm_set_tsc_khz() updates virtual_tsc_mult and virtual_tsc_shift to F2 before calling kvm_synchronize_tsc_at(), so nsec_to_cycles(vcpu, elapsed) advances tsc_exp by F2 * dt cycles. The difference |data - tsc_exp| is therefore
|F1 - F2| * dt rather than arithmetic rounding, which exceeds the 1ms window (tsc_hz / 1000) once dt >= 1ms * F2 / |F1 - F2| (for example 1ms
when halving the frequency), causing sibling vCPUs to miss the generation and lose all_vcpus_matched_tsc.
Quoted 14 lines
arch/x86/kvm/x86.c +6
-1@@ -1552,10 +1642,26 @@ void kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 *user_value) matched = true; } -__kvm_synchronize_tsc (vcpu, offset, data, ns, matched, !!user_value); +__kvm_synchronize_tsc (vcpu, offset, data, ns, matched, user_initiated);raw_spin_unlock_irqrestore (&kvm->arch.tsc_write_lock, flags); } +void kvm_synchronize_tsc (struct kvm_vcpu *vcpu,u64 *value,bool user_initiated) +{ +u64 host_tsc; +s64 ns; +
arch/x86/kvm/x86.c void kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 *user_value) · line 1663
Quoted 10 lines
arch/x86/kvm/x86.c +10
-0+#ifdef CONFIG_X86_64 + if (!kvm_get_time_and_clockread(&ns, &host_tsc)) +#endif + { + host_tsc =rdtsc (); + ns =get_kvmclock_base_ns (); + } + +kvm_synchronize_tsc_at (vcpu, value, host_tsc, ns, user_initiated); +}
[Severity: Medium]
Can sampling {ns, host_tsc} outside kvm->arch.tsc_write_lock in kvm_set_tsc_khz() and kvm_synchronize_tsc() underflow elapsed in kvm_synchronize_tsc_at()?
CPU0 CPU1kvm_set_tsc_khz(vcpu0)
sample ns = 100
<preempted>
kvm_set_tsc_khz(vcpu1)
sample ns = 200
kvm_synchronize_tsc_at()
lock(&kvm->arch.tsc_write_lock)
kvm->arch.cur_tsc_nsec = 200
unlock(&kvm->arch.tsc_write_lock) kvm_synchronize_tsc_at()
lock(&kvm->arch.tsc_write_lock)
elapsed = ns - cur_tsc_nsec <-- wraps u64
nsec_to_cycles(vcpu, elapsed)
In kvm_synchronize_tsc_at(), elapsed is a u64:
arch/x86/kvm/x86.c:kvm_synchronize_tsc_at() {
offset = kvm_compute_l1_tsc_offset(vcpu, host_tsc, data);
elapsed = ns - kvm->arch.cur_tsc_nsec;When CPU0 acquires tsc_write_lock after CPU1 has already recorded a newer cur_tsc_nsec, elapsed wraps around near U64_MAX and is passed to unsigned nsec_to_cycles() (pvclock_scale_delta()). For non-zero writes this corrupts tsc_exp so CPU0 fails to match and overwrites cur_tsc_nsec with the older timestamp; for zero-value syncs (synchronizing == true) it corrupts delta, data, vcpu->arch.last_guest_tsc, and (when kvm_check_tsc_unstable() is true) offset. In addition, the fallback rdtsc() + get_kvmclock_base_ns() path now runs with preemption and interrupts enabled, so preemption between the two reads can skew the {host_tsc, ns} pair.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8
kernel test robot
build botRe: [PATCH v12 08/24] KVM: x86: Re-synchronize TSC after KVM_SET_TSC_KHZ
In reply to David Woodhousekernel test robot noticed the following build errors:
Hi David,
kernel test robot noticed the following build errors:
[auto build test ERROR on dea4f6380f4ec96e8f3b0aaf2343764bd0d25ef4]
url: https://github.com/intel-lab-lkp/linux/commits/David-Woodhouse/KVM-x86-Improve-synchronization-in-kvm_synchronize_tsc/20261108-172047 base: dea4f6380f4ec96e8f3b0aaf2343764bd0d25ef4 patch link: https://lore.kernel.org/r/20261109012112.3415328-9-dwmw2%40infradead.org patch subject: [PATCH v12 08/24] KVM: x86: Re-synchronize TSC after KVM_SET_TSC_KHZ config: i386-buildonly-randconfig-1005-20261109 (https://download.01.org/0day-ci/archive/20261109/[email protected]/config) compiler: gcc-14 (Debian 14.2.0-19) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261109/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
ld: arch/x86/kvm/x86.o: in function `kvm_synchronize_tsc_at':
>> x86.c:(.text+0xfaca): undefined reference to `__udivdi3'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
kernel test robot
build botRe: [PATCH v12 08/24] KVM: x86: Re-synchronize TSC after KVM_SET_TSC_KHZ
In reply to David Woodhousekernel test robot noticed the following build errors:
Hi David,
kernel test robot noticed the following build errors:
[auto build test ERROR on dea4f6380f4ec96e8f3b0aaf2343764bd0d25ef4]
url: https://github.com/intel-lab-lkp/linux/commits/David-Woodhouse/KVM-x86-Improve-synchronization-in-kvm_synchronize_tsc/20261108-172047 base: dea4f6380f4ec96e8f3b0aaf2343764bd0d25ef4 patch link: https://lore.kernel.org/r/20261109012112.3415328-9-dwmw2%40infradead.org patch subject: [PATCH v12 08/24] KVM: x86: Re-synchronize TSC after KVM_SET_TSC_KHZ config: i386-allmodconfig (https://download.01.org/0day-ci/archive/20261110/[email protected]/config) compiler: gcc-14 (Debian 14.2.0-19) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261110/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: arch/x86/kvm/kvm.ko: symbol '__udivdi3' undefined!
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
David Woodhouse
maintainerPATCH v12 9/24 KVM: x86: Cache host vclock_mode for masterclock eligibility checks
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
The remaining users of pvclock_gtod_data only need to know whether the host clocksource is TSC-based. Cache the vclock_mode value in a simple kvm_host_vclock_mode variable, updated by the pvclock_gtod_notify callback, and use gtod_is_based_on_tsc() for eligibility checks.
This is inherently racy (as it always was — kvm_track_tsc_matching never held the gtod seqcount), relying on eventual consistency: the notifier fires on every timekeeping update and will correct any transient inconsistency within one tick.
--- arch/x86/kvm/x86.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-)
arch/x86/kvm/x86.c +17 -10
@@ -1187,6 +1187,8 @@ static u64 compute_guest_tsc(struct kvm_vcpu *vcpu, s64 kernel_ns) }#ifdef CONFIG_X86_64 +static int kvm_host_vclock_mode; +static inline bool gtod_is_based_on_tsc (int mode) {return mode ==VDSO_CLOCKMODE_TSC || mode ==VDSO_CLOCKMODE_HVCLOCK ;
@@ -1216,7 +1218,6 @@ static bool kvm_use_master_clock(struct kvm *kvm)static void kvm_track_tsc_matching (struct kvm_vcpu *vcpu,bool update_mclock) {struct kvm_arch *ka = &vcpu->kvm->arch; -struct pvclock_gtod_data *gtod = &pvclock_gtod_data;bool prev_matched_tsc = ka->all_vcpus_matched_tsc;bool prev_matched_freq = ka->all_vcpus_matched_freq;
@@ -1269,8 +1270,9 @@ static void kvm_track_tsc_matching(struct kvm_vcpu *vcpu, bool update_mclock)* are fine — each vCPU's pvclock has its own tsc_timestamp that * accounts for its offset. */ +int host_vclock_mode =READ_ONCE (kvm_host_vclock_mode);bool use_master_clock =kvm_use_master_clock (vcpu->kvm) && -gtod_is_based_on_tsc (gtod->clock.vclock_mode); +gtod_is_based_on_tsc (host_vclock_mode);/* * Request a masterclock update if needed: toggling master clock,
@@ -1283,7 +1285,7 @@ static void kvm_track_tsc_matching(struct kvm_vcpu *vcpu, bool update_mclock)trace_kvm_track_tsc (vcpu->vcpu_id, ka->nr_vcpus_matched_tsc,atomic_read (&vcpu->kvm->online_vcpus), - ka->use_master_clock, gtod->clock.vclock_mode); + ka->use_master_clock, host_vclock_mode); }#else static inline void kvm_track_tsc_matching(struct kvm_vcpu *vcpu,
@@ -1454,7 +1456,7 @@ static inline bool kvm_check_tsc_unstable(void)* TSC is marked unstable when we're running on Hyper-V, * 'TSC page' clocksource is good. */ -if (pvclock_gtod_data.clock.vclock_mode ==VDSO_CLOCKMODE_HVCLOCK ) +if (READ_ONCE (kvm_host_vclock_mode) ==VDSO_CLOCKMODE_HVCLOCK )return false;#endif return check_tsc_unstable ();
@@ -1788,7 +1790,6 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm) {#ifdef CONFIG_X86_64 struct kvm_arch *ka = &kvm->arch; -int vclock_mode;bool host_tsc_clocksource;lockdep_assert_held (&kvm->arch.tsc_write_lock);
@@ -1828,9 +1829,9 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm) ka->use_master_clock = false; } - vclock_mode = pvclock_gtod_data.clock.vclock_mode; -trace_kvm_update_master_clock (ka->use_master_clock, vclock_mode, - ka->all_vcpus_matched_freq); +trace_kvm_update_master_clock (ka->use_master_clock, +READ_ONCE (kvm_host_vclock_mode), + ka->all_vcpus_matched_freq);#endif }
@@ -7210,17 +7211,23 @@ static DEFINE_IRQ_WORK(pvclock_irq_work, pvclock_irq_work_fn);static int pvclock_gtod_notify (struct notifier_block *nb,unsigned long unused,void *priv) { -struct pvclock_gtod_data *gtod = &pvclock_gtod_data;struct timekeeper *tk = priv;update_pvclock_gtod (tk); +/* +* Pairs with the READ_ONCE() in the lockless readers; the race is +* benign by design, relying on eventual consistency (the notifier +* fires on every timekeeping update). +*/ +WRITE_ONCE (kvm_host_vclock_mode, tk->tkr_mono.clock->vdso_clock_mode); +/* * Disable master clock if host does not trust, or does not use, * TSC based clocksource. Delegate queue_work() to irq_work as * this is invoked with tk_core.seq write held. */ -if (!gtod_is_based_on_tsc (gtod->clock.vclock_mode) && +if (!gtod_is_based_on_tsc (kvm_host_vclock_mode) &&atomic_read (&kvm_guest_has_master_clock) !=0 )irq_work_queue (&pvclock_irq_work);return 0 ;
Signature · 1 line
2.43.0
David Woodhouse
maintainerPATCH v12 10/24 KVM: x86: Remove pvclock_gtod_data and private timekeeping code
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
Remove the now-unused KVM-private timekeeping infrastructure:
- struct pvclock_clock and struct pvclock_gtod_data
- update_pvclock_gtod() and its seqcount-protected state copy (the private readers were already removed when their last callers went away)
--- arch/x86/kvm/x86.c | 56 ---------------------------------------------- 1 file changed, 56 deletions(-)
arch/x86/kvm/x86.c +0 -56
@@ -889,60 +889,6 @@ static inline bool kvm_vcpu_exit_request(struct kvm_vcpu *vcpu)kvm_request_pending (vcpu) ||xfer_to_guest_mode_work_pending (); } -#ifdef CONFIG_X86_64 -struct pvclock_clock { -int vclock_mode; -u64 cycle_last; -u64 mask; -u32 mult; -u32 shift; -u64 base_cycles; -u64 offset; -}; - -struct pvclock_gtod_data { -seqcount_t seq; - -struct pvclock_clock clock;/* extract of a clocksource struct */ -struct pvclock_clock raw_clock;/* extract of a clocksource struct */ - -ktime_t offs_boot; -u64 wall_time_sec; -}; - -static struct pvclock_gtod_data pvclock_gtod_data; - -static void update_pvclock_gtod (struct timekeeper *tk) -{ -struct pvclock_gtod_data *vdata = &pvclock_gtod_data; - -write_seqcount_begin (&vdata->seq); - -/* copy pvclock gtod data */ - vdata->clock.vclock_mode = tk->tkr_mono.clock->vdso_clock_mode; - vdata->clock.cycle_last = tk->tkr_mono.cycle_last; - vdata->clock.mask = tk->tkr_mono.mask; - vdata->clock.mult = tk->tkr_mono.mult; - vdata->clock.shift = tk->tkr_mono.shift; - vdata->clock.base_cycles = tk->tkr_mono.xtime_nsec; - vdata->clock.offset = tk->tkr_mono.base; - - vdata->raw_clock.vclock_mode = tk->tkr_raw.clock->vdso_clock_mode; - vdata->raw_clock.cycle_last = tk->tkr_raw.cycle_last; - vdata->raw_clock.mask = tk->tkr_raw.mask; - vdata->raw_clock.mult = tk->tkr_raw.mult; - vdata->raw_clock.shift = tk->tkr_raw.shift; - vdata->raw_clock.base_cycles = tk->tkr_raw.xtime_nsec; - vdata->raw_clock.offset = tk->tkr_raw.base; - - vdata->wall_time_sec = tk->xtime_sec; - - vdata->offs_boot = tk->offs_boot; - -write_seqcount_end (&vdata->seq); -} -#endif -static s64 get_kvmclock_base_ns (void ) {/* Count up from boot time, but with the frequency of the raw clock. */
@@ -7213,8 +7159,6 @@ static int pvclock_gtod_notify(struct notifier_block *nb, unsigned long unused, {struct timekeeper *tk = priv; -update_pvclock_gtod (tk); -/* * Pairs with the READ_ONCE() in the lockless readers; the race is * benign by design, relying on eventual consistency (the notifier
Signature · 1 line
2.43.0
David Woodhouse
maintainerPATCH v12 11/24 KVM: x86: Activate master clock from kvm_arch_init_vm()
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
Initialize the TSC generation and master clock state at VM creation time, rather than relying on kvm_update_masterclock() from kvm_arch_vcpu_postcreate().
Set cur_tsc_generation=1, cur_tsc_offset, cur_tsc_nsec and kvmclock_offset from an atomic ktime/TSC snapshot. This establishes a coherent epoch so that:
- pvclock_update_vm_gtod_copy() succeeds immediately.
- KVM_GET_CLOCK returns valid {host_tsc, realtime} fields as soon as the VM is created.
- KVM_[GS]ET_CLOCK_GUEST works without a prior KVM_RUN.
- No O(N) iteration of all vCPUs on each vCPU creation.
The initial generation is deliberately not matchable: cur_tsc_khz is left at zero, which no vCPU frequency ever equals, so the first vCPU opens its own generation with its TSC starting at zero as it always has. Letting it match the VM-creation generation would silently change guest-visible behaviour (a first-vCPU TSC equal to the host cycles elapsed since KVM_CREATE_VM, over an unbounded window), and would hand the first vCPU a stale offset when a host suspend resets the TSC before any vCPU exists, since the backwards-TSC compensation in kvm_arch_enable_virtualization_cpu() detects the reset only through existing vCPUs.
The same no-vCPU suspend window can leave the master snapshot itself
stale: nothing refreshes it, as pvclock_gtod_update_fn() also iterates vCPUs, and the post-reset host TSC may read below master_cycle_now (underflowing the delta), or — given enough uptime before the suspend, or a firmware re-init to an arbitrary value — above it, silently dropping the sleep period. Make __get_kvmclock_master_clock() fall back to the raw clock whenever the elapsed time computed from the snapshot disagrees with the raw clock by more than a second, instead of returning a discontinuity flagged as KVM_CLOCK_TSC_STABLE.
A masterclock refresh is still performed in postcreate (synchronously, so that it is valid even for userspace which queries the clock before ever running a vCPU) when the first vCPU is created, since the VM-creation snapshot can go stale while the VM has no vCPUs: a clocksource change notifies only VMs with vCPUs, and a change back to TSC leaves nothing to disagree with the stale snapshot. The same call services a KVM_REQ_MASTERCLOCK_UPDATE that the postcreate synchronization raised on any later vCPU, which would otherwise wait until that vCPU first entered the guest while KVM_GET_CLOCK observed the stale state.
--- arch/x86/kvm/x86.c | 109 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 92 insertions(+), 17 deletions(-)
arch/x86/kvm/x86.c +92 -17
@@ -1583,6 +1583,14 @@ static void kvm_synchronize_tsc_at(struct kvm_vcpu *vcpu, u64 *value, data = kvm->arch.cur_tsc_write + delta; +/* +* The generation's offset can be reused directly: it is in +* this vCPU's scaling domain, because every generation is +* opened by a vCPU (the initial VM-creation generation is +* not matchable) and set_tsc_khz() derives the scaling +* ratio deterministically from the kHz value, so two vCPUs +* at the same frequency carry the same ratio. +*/ if (!kvm_check_tsc_unstable ()) offset = kvm->arch.cur_tsc_offset;else
@@ -1902,6 +1910,7 @@ static bool __get_kvmclock_master_clock(struct kvm *kvm,struct kvm_arch *ka = &kvm->arch;struct pvclock_vcpu_time_info hv_clock;struct timespec64 ts; +u64 clock;if (!ka->use_master_clock)return false;
@@ -1909,10 +1918,6 @@ static bool __get_kvmclock_master_clock(struct kvm *kvm,if (!kvm_get_walltime_and_clockread (&ts, &data->host_tsc))return false; - data->realtime = ts.tv_nsec +NSEC_PER_SEC * ts.tv_sec; - data->flags |=KVM_CLOCK_REALTIME |KVM_CLOCK_HOST_TSC | -KVM_CLOCK_TSC_STABLE ; - hv_clock.tsc_timestamp = ka->master_cycle_now; hv_clock.system_time = ka->master_kernel_ns + ka->kvmclock_offset;
@@ -1927,13 +1932,34 @@ static bool __get_kvmclock_master_clock(struct kvm *kvm,u64 tsc_delta = data->host_tsc - ka->master_cycle_now; tsc_delta =kvm_scale_tsc (tsc_delta, ka->master_tsc_scaling_ratio); - data->clock = hv_clock.system_time + -pvclock_scale_delta (tsc_delta, - hv_clock.tsc_to_system_mul, - hv_clock.tsc_shift); + clock = hv_clock.system_time + +pvclock_scale_delta (tsc_delta, + hv_clock.tsc_to_system_mul, + hv_clock.tsc_shift); }else { - data->clock =__pvclock_read_cycles (&hv_clock, data->host_tsc); + clock =__pvclock_read_cycles (&hv_clock, data->host_tsc); } + +/* +* Sanity-check the snapshot: if the elapsed time it computes from +* the host TSC disagrees with the raw clock by more than a second, +* the TSC was reset (suspend/resume — possibly re-inited by +* firmware to an arbitrary value, including an underflowing one +* below master_cycle_now) while nothing refreshed the snapshot. +* That can happen while the VM has no vCPUs, since both the +* resume compensation and the clocksource notifier work through +* vCPUs. Use the fallback clock rather than returning a +* discontinuity flagged KVM_CLOCK_TSC_STABLE. +*/ +if (unlikely (abs ((s64)(clock - hv_clock.system_time) - + (get_kvmclock_base_ns () - ka->master_kernel_ns)) > +NSEC_PER_SEC )) +return false; + + data->clock = clock; + data->realtime = ts.tv_nsec +NSEC_PER_SEC * ts.tv_sec; + data->flags |=KVM_CLOCK_REALTIME |KVM_CLOCK_HOST_TSC | +KVM_CLOCK_TSC_STABLE ;return true;#else return false;
@@ -9782,8 +9808,19 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu) }vcpu_load (vcpu);kvm_synchronize_tsc (vcpu,NULL , false); -if (kvm_check_request (KVM_REQ_MASTERCLOCK_UPDATE , vcpu)) + +/* +* Refresh the VM-creation masterclock snapshot (see +* kvm_arch_init_vm), synchronously so that it is valid even for +* userspace which queries the clock before ever running a vCPU. +* Likewise service any masterclock update the synchronization +* above requested on this vCPU, which would otherwise languish +* until it first enters the guest. +*/ +if (kvm_check_request (KVM_REQ_MASTERCLOCK_UPDATE , vcpu) || + vcpu->vcpu_idx ==0 )kvm_update_masterclock (vcpu->kvm,NULL ); +vcpu_put (vcpu);/* poll control enabled by default */
@@ -10222,6 +10259,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type) {int ret;unsigned long flags; +u64 kvmclock_host_tsc; +s64 kvmclock_ns;if (!kvm_is_vm_type_supported (type))return -EINVAL ;
@@ -10253,9 +10292,13 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)seqcount_raw_spinlock_init (&kvm->arch.pvclock_sc, &kvm->arch.tsc_write_lock);ratelimit_state_init (&kvm->arch.kvmclock_update_rs,HZ ,10 );ratelimit_set_flags (&kvm->arch.kvmclock_update_rs,RATELIMIT_MSG_ON_RELEASE ); - kvm->arch.kvmclock_offset = -get_kvmclock_base_ns (); - kvm->arch.all_vcpus_matched_freq = true; + kvm->arch.default_tsc_khz = max_tsc_khz ? : tsc_khz; + kvm->arch.apic_bus_cycle_ns =APIC_BUS_CYCLE_NS_DEFAULT ; + kvm->arch.guest_can_read_msr_platform_info = true; + kvm->arch.enable_pmu = enable_pmu && !kvm->arch.has_protected_pmu; + + kvm->arch.all_vcpus_matched_freq = true;/* * Start with all (zero) vCPUs considered matched, so that a VM whose * vCPUs never reach kvm_track_tsc_matching() — notably when the guest
@@ -10271,14 +10314,46 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type) kvm->arch.cur_tsc_scaling_ratio = kvm_caps.default_tsc_scaling_ratio;raw_spin_lock_irqsave (&kvm->arch.tsc_write_lock, flags); + +/* +* Establish an initial TSC generation so that the master clock +* snapshot taken below is immediately valid and KVM_GET_CLOCK +* works before any vCPU exists. The snapshot is refreshed again +* when the first vCPU is created, in case the host clocksource +* changed (and changed back) in between: pvclock_gtod_update_fn() +* does not notify a VM with no vCPUs. +* +* This generation is deliberately not matchable: cur_tsc_khz is +* left at zero, which no vCPU frequency ever equals, so the first +* vCPU opens its own generation with its TSC starting at zero, +* exactly as it always has. Letting it match would make the first +* vCPU's TSC start at the cycles elapsed since KVM_CREATE_VM — a +* silent guest-visible change over an unbounded window — and +* would hand it this offset verbatim even when a host suspend +* reset the TSC in between, since the backwards-TSC compensation +* in kvm_arch_enable_virtualization_cpu() detects the jump only +* through existing vCPUs. +*/ +#ifdef CONFIG_X86_64 + if (!kvm_get_time_and_clockread(&kvmclock_ns, &kvmclock_host_tsc)) +#endif + { +/* +* If not simultaneous, we want the TSC to be zero slightly +* *after* the kvmclock is zero. +*/ + kvmclock_ns =get_kvmclock_base_ns (); + kvmclock_host_tsc =rdtsc (); + } + kvm->arch.cur_tsc_write =0 ; + kvm->arch.cur_tsc_offset = -(s64 )kvmclock_host_tsc; + kvm->arch.cur_tsc_nsec = kvmclock_ns; + kvm->arch.cur_tsc_generation =1 ; + kvm->arch.kvmclock_offset = -kvmclock_ns; +pvclock_update_vm_gtod_copy (kvm);raw_spin_unlock_irqrestore (&kvm->arch.tsc_write_lock, flags); - kvm->arch.default_tsc_khz = max_tsc_khz ? : tsc_khz; - kvm->arch.apic_bus_cycle_ns =APIC_BUS_CYCLE_NS_DEFAULT ; - kvm->arch.guest_can_read_msr_platform_info = true; - kvm->arch.enable_pmu = enable_pmu && !kvm->arch.has_protected_pmu; -#if IS_ENABLED (CONFIG_HYPERV )spin_lock_init (&kvm->arch.hv_root_tdp_lock); kvm->arch.hv_root_tdp =INVALID_PAGE ;
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH v12 11/24] KVM: x86: Activate master clock from kvm_arch_init_vm()
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [Medium] KVM: x86: host-guest clock divergence in __get_kvmclock_master_clock() - [Low] KVM: x86: dead…
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [Medium] KVM: x86: host-guest clock divergence in __get_kvmclock_master_clock()
- [Low] KVM: x86: dead initial TSC generation stores in kvm_arch_init_vm()
--
commit 862c03c2a3fcfa08f8c8dedc05fb896a0f4d58c9
Author: David Woodhouse <[email protected]>
KVM: x86: Activate master clock from kvm_arch_init_vm()
This commit initializes the master clock snapshot in kvm_arch_init_vm(), adds a sanity check against raw clock divergence in __get_kvmclock_master_clock(), and refreshes the master clock when the first vCPU is created.
Quoted 4 lines
arch/x86/kvm/x86.c +0
-0
[ ... ]
Quoted 22 lines
arch/x86/kvm/x86.c +11
-5@@ -1927,13 +1932,34 @@ static bool __get_kvmclock_master_clock(struct kvm *kvm,u64 tsc_delta = data->host_tsc - ka->master_cycle_now; tsc_delta =kvm_scale_tsc (tsc_delta, ka->master_tsc_scaling_ratio); - data->clock = hv_clock.system_time + -pvclock_scale_delta (tsc_delta, - hv_clock.tsc_to_system_mul, - hv_clock.tsc_shift); + clock = hv_clock.system_time + +pvclock_scale_delta (tsc_delta, + hv_clock.tsc_to_system_mul, + hv_clock.tsc_shift); }else { - data->clock =__pvclock_read_cycles (&hv_clock, data->host_tsc); + clock =__pvclock_read_cycles (&hv_clock, data->host_tsc); } + +/* +* Sanity-check the snapshot: if the elapsed time it computes from +* the host TSC disagrees with the raw clock by more than a second, +* the TSC was reset (suspend/resume — possibly re-inited by +* firmware to an arbitrary value, including an underflowing one
arch/x86/kvm/x86.c static bool __get_kvmclock_master_clock(struct kvm *kvm, · line 1957
Quoted 10 lines
arch/x86/kvm/x86.c +10
-0+* below master_cycle_now) while nothing refreshed the snapshot. +* That can happen while the VM has no vCPUs, since both the +* resume compensation and the clocksource notifier work through +* vCPUs. Use the fallback clock rather than returning a +* discontinuity flagged KVM_CLOCK_TSC_STABLE. +*/ +if (unlikely (abs ((s64)(clock - hv_clock.system_time) - + (get_kvmclock_base_ns () - ka->master_kernel_ns)) > +NSEC_PER_SEC )) +return false;
[Severity: Medium]
Can this sanity check trigger while vCPUs are active and cause a 1-second divergence between host-side get_kvmclock() and guest pvclock?
Although the comment describes the no-vCPU window, this check runs even when vCPUs exist and ka->use_master_clock remains true. While the master clock is active, ka->master_cycle_now and ka->master_kernel_ns are not re-anchored. Because tk_core.timekeeper.tkr_raw uses a 22- or 23-bit multiplier from clocks_calc_mult_shift() whereas ka->master_tsc_mul uses a 32-bit multiplier from kvm_get_time_scale() (plus intermediate rounding in kvm_scale_tsc() when TSC scaling is active), the two elapsed times drift apart over long uninterrupted uptime until their difference exceeds NSEC_PER_SEC.
Once that threshold is crossed, __get_kvmclock_master_clock() returns false, causing get_kvmclock() (and KVM_GET_CLOCK) to jump by at least a second to get_kvmclock_base_ns() + ka->kvmclock_offset and drop KVM_CLOCK_TSC_STABLE, KVM_CLOCK_REALTIME, and KVM_CLOCK_HOST_TSC, while leaving data->host_tsc modified from the earlier kvm_get_walltime_and_clockread() call. Meanwhile, kvm_guest_time_update() continues populating the guest's pvclock from the master clock snapshot with PVCLOCK_TSC_STABLE_BIT set.
Should this check be gated on having no online/created vCPUs, or should ka->master_cycle_now and ka->master_kernel_ns be refreshed when the first vCPU is created so no fallback check is needed in __get_kvmclock_master_clock()?
[ ... ]
Quoted 33 lines
arch/x86/kvm/x86.c +29
-0@@ -10271,14 +10314,46 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type) kvm->arch.cur_tsc_scaling_ratio = kvm_caps.default_tsc_scaling_ratio;raw_spin_lock_irqsave (&kvm->arch.tsc_write_lock, flags); + +/* +* Establish an initial TSC generation so that the master clock +* snapshot taken below is immediately valid and KVM_GET_CLOCK +* works before any vCPU exists. The snapshot is refreshed again +* when the first vCPU is created, in case the host clocksource +* changed (and changed back) in between: pvclock_gtod_update_fn() +* does not notify a VM with no vCPUs. +* +* This generation is deliberately not matchable: cur_tsc_khz is +* left at zero, which no vCPU frequency ever equals, so the first +* vCPU opens its own generation with its TSC starting at zero, +* exactly as it always has. Letting it match would make the first +* vCPU's TSC start at the cycles elapsed since KVM_CREATE_VM — a +* silent guest-visible change over an unbounded window — and +* would hand it this offset verbatim even when a host suspend +* reset the TSC in between, since the backwards-TSC compensation +* in kvm_arch_enable_virtualization_cpu() detects the jump only +* through existing vCPUs. +*/ +#ifdef CONFIG_X86_64 + if (!kvm_get_time_and_clockread(&kvmclock_ns, &kvmclock_host_tsc)) +#endif + { +/* +* If not simultaneous, we want the TSC to be zero slightly +* *after* the kvmclock is zero. +*/ + kvmclock_ns =get_kvmclock_base_ns ();
arch/x86/kvm/x86.c int kvm_arch_init_vm(struct kvm *kvm, unsigned long type) · line 10355
Quoted 10 lines
arch/x86/kvm/x86.c +8
-0+ kvmclock_host_tsc =rdtsc (); + } + kvm->arch.cur_tsc_write =0 ; + kvm->arch.cur_tsc_offset = -(s64 )kvmclock_host_tsc; + kvm->arch.cur_tsc_nsec = kvmclock_ns; + kvm->arch.cur_tsc_generation =1 ; + kvm->arch.kvmclock_offset = -kvmclock_ns; +pvclock_update_vm_gtod_copy (kvm);raw_spin_unlock_irqrestore (&kvm->arch.tsc_write_lock, flags);
[Severity: Low]
Are the stores to cur_tsc_write, cur_tsc_offset, cur_tsc_nsec, and cur_tsc_generation (along with the kvmclock_host_tsc read) dead code?
pvclock_update_vm_gtod_copy() takes its own snapshot into master_kernel_ns and master_cycle_now and does not read any of the cur_tsc_* fields. Because cur_tsc_khz is left at 0 and user_set_tsc is false, no caller can match or consume this initial generation before the first vCPU's __kvm_synchronize_tsc() call runs with matched == false and overwrites cur_tsc_generation, cur_tsc_nsec, cur_tsc_write, and cur_tsc_offset.
Also, the commit message states that this patch makes pvclock_update_vm_gtod_copy() succeed in kvm_arch_init_vm(), but earlier commits in the series (commit "KVM: x86: Replace nr_vcpus_matched_tsc count with all_vcpus_matched_tsc bool" and commit "KVM: x86: Allow KVM master clock mode when TSCs are offset from each other") already initialized all_vcpus_matched_tsc, cur_tsc_scaling_ratio, and all_vcpus_matched_freq in kvm_arch_init_vm().
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11
David Woodhouse
maintainerPATCH v12 12/24 UAPI: x86: Move pvclock-abi to UAPI for x86 platforms
In reply to David WoodhouseFrom: Jack Allister <[email protected]>
A subsequent commit will provide a new KVM interface for performing a fixup/correction of the KVM clock against the reference TSC. The KVM_[GS]ET_CLOCK_GUEST API requires a pvclock_vcpu_time_info, as such the caller must know about this definition.
Move the definition to the UAPI folder so that it is exported to usermode, change the type definitions to use the standard for UAPI exports, and add the Linux-syscall-note exception to the SPDX licence identifier as is conventional for exported headers.
Define __KERNEL__ in scripts/xen-hypercalls.sh: the UAPI header now pulls in linux/types.h, and the script's raw $CPP invocation with $LINUXINCLUDE does not define it, which would trigger the "attempt to use kernel headers from user space" warning from the UAPI type headers.
---
MAINTAINERS | 4 +--
arch/x86/include/{ => uapi}/asm/pvclock-abi.h | 27 ++++++++++---------
scripts/xen-hypercalls.sh | 2 +-
3 files changed, 18 insertions(+), 15 deletions(-)
rename arch/x86/include/{ => uapi}/asm/pvclock-abi.h (82%)MAINTAINERS +2 -2
@@ -14663,8 +14663,8 @@ S: Supported T: git git://git.kernel.org/pub/scm/virt/kvm/kvm.git F: arch/um/include/asm/kvm_para.h F: arch/x86/include/asm/kvm_para.h -F: arch/x86/include/asm/pvclock-abi.h F: arch/x86/include/uapi/asm/kvm_para.h +F: arch/x86/include/uapi/asm/pvclock-abi.h F: arch/x86/kernel/kvm.c F: arch/x86/kernel/kvmclock.c F: include/asm-generic/kvm_para.h
@@ -29780,8 +29780,8 @@ R: Boris Ostrovsky <[email protected]> L: [email protected] (moderated for non-subscribers) S: Supported F: arch/x86/configs/xen.config -F: arch/x86/include/asm/pvclock-abi.h F: arch/x86/include/asm/xen/ +F: arch/x86/include/uapi/asm/pvclock-abi.h F: arch/x86/platform/pvh/ F: arch/x86/xen/
arch/x86/include/uapi/asm/pvclock-abi.h renamed from arch/x86/include/asm/pvclock-abi.h +15 -12
@@ -1,6 +1,9 @@ -/* SPDX-License-Identifier: GPL-2.0 */ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ #ifndef _ASM_X86_PVCLOCK_ABI_H#define _ASM_X86_PVCLOCK_ABI_H + +#include <linux/types.h> +#ifndef __ASSEMBLER__/*
@@ -24,20 +27,20 @@*/ struct pvclock_vcpu_time_info { -u32 version; -u32 pad0; -u64 tsc_timestamp; -u64 system_time; -u32 tsc_to_system_mul; -s8 tsc_shift; -u8 flags; -u8 pad[2 ]; +__u32 version; +__u32 pad0; +__u64 tsc_timestamp; +__u64 system_time; +__u32 tsc_to_system_mul; +__s8 tsc_shift; +__u8 flags; +__u8 pad[2 ]; } __attribute__((__packed__ ));/* 32 bytes */ struct pvclock_wall_clock { -u32 version; -u32 sec; -u32 nsec; +__u32 version; +__u32 sec; +__u32 nsec; } __attribute__((__packed__ ));#define PVCLOCK_TSC_STABLE_BIT (1 << 0)
scripts/xen-hypercalls.sh +1 -1
@@ -5,7 +5,7 @@ shift in="$@" for i in $in; do - eval $CPP $LINUXINCLUDE -dD -imacros "$i" -x c /dev/null + eval $CPP -D__KERNEL__ $LINUXINCLUDE -dD -imacros "$i" -x c /dev/null done | \ awk '$1 == "#define" && $2 ~ /__HYPERVISOR_[a-z][a-z_0-9]*/ { v[$3] = $2 } END { print "/* auto-generated by scripts/xen-hypercall.sh */"
Signature · 1 line
2.43.0
David Woodhouse
maintainerPATCH v12 13/24 KVM: selftests: Use UAPI pvclock-abi.h in xen_shinfo_test
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
Now that pvclock-abi.h is a UAPI header, use it in xen_shinfo_test.c instead of locally duplicating the struct definitions.
Sync the tools/ mirror of the header to the UAPI content: the KVM selftests include path puts tools/arch/x86/include ahead of the installed kernel headers, so the mirror is what actually gets used. That makes the struct fields __u64 (unsigned long long), so switch the verbose printfs away from PRIu64, which is "lu" on 64-bit.
--- tools/arch/x86/include/asm/pvclock-abi.h | 27 +++++++++------- .../selftests/kvm/x86/xen_shinfo_test.c | 31 ++++++------------- 2 files changed, 24 insertions(+), 34 deletions(-)
tools/arch/x86/include/asm/pvclock-abi.h +15 -12
@@ -1,6 +1,9 @@ -/* SPDX-License-Identifier: GPL-2.0 */ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ #ifndef _ASM_X86_PVCLOCK_ABI_H#define _ASM_X86_PVCLOCK_ABI_H + +#include <linux/types.h> +#ifndef __ASSEMBLER__/*
@@ -24,20 +27,20 @@*/ struct pvclock_vcpu_time_info { -u32 version; -u32 pad0; -u64 tsc_timestamp; -u64 system_time; -u32 tsc_to_system_mul; -s8 tsc_shift; -u8 flags; -u8 pad[2 ]; +__u32 version; +__u32 pad0; +__u64 tsc_timestamp; +__u64 system_time; +__u32 tsc_to_system_mul; +__s8 tsc_shift; +__u8 flags; +__u8 pad[2 ]; } __attribute__((__packed__ ));/* 32 bytes */ struct pvclock_wall_clock { -u32 version; -u32 sec; -u32 nsec; +__u32 version; +__u32 sec; +__u32 nsec; } __attribute__((__packed__ ));#define PVCLOCK_TSC_STABLE_BIT (1 << 0)
tools/testing/selftests/kvm/x86/xen_shinfo_test.c +9 -22
@@ -98,22 +98,7 @@ struct sched_poll {u64 timeout; }; -struct pvclock_vcpu_time_info { -u32 version; -u32 pad0; -u64 tsc_timestamp; -u64 system_time; -u32 tsc_to_system_mul; -s8 tsc_shift; -u8 flags; -u8 pad[2 ]; -} __attribute__((__packed__ ));/* 32 bytes */ - -struct pvclock_wall_clock { -u32 version; -u32 sec; -u32 nsec; -} __attribute__((__packed__ )); +#include <asm/pvclock-abi.h> struct vcpu_runstate_info {u32 state;
@@ -987,12 +972,14 @@ int main(int argc, char *argv[])if (verbose) {printf ("Wall clock (v %d) %d.%09d\n" , wc->version, wc->sec, wc->nsec); -printf ("Time info 1: v %u tsc %" PRIu64" time %" PRIu64" mul %u shift %u flags %x\n" , - ti->version, ti->tsc_timestamp, ti->system_time, ti->tsc_to_system_mul, - ti->tsc_shift, ti->flags); -printf ("Time info 2: v %u tsc %" PRIu64" time %" PRIu64" mul %u shift %u flags %x\n" , - ti2->version, ti2->tsc_timestamp, ti2->system_time, ti2->tsc_to_system_mul, - ti2->tsc_shift, ti2->flags); +printf ("Time info 1: v %u tsc %llu time %llu mul %u shift %d flags %x\n" , + ti->version, (unsigned long long )ti->tsc_timestamp, + (unsigned long long )ti->system_time, + ti->tsc_to_system_mul, ti->tsc_shift, ti->flags); +printf ("Time info 2: v %u tsc %llu time %llu mul %u shift %d flags %x\n" , + ti2->version, (unsigned long long )ti2->tsc_timestamp, + (unsigned long long )ti2->system_time, + ti2->tsc_to_system_mul, ti2->tsc_shift, ti2->flags); } TEST_ASSERT(wc->version && !(wc->version & 1),
Signature · 1 line
2.43.0
David Woodhouse
maintainerPATCH v12 14/24 KVM: x86: Add KVM_[GS]ET_CLOCK_GUEST for accurate KVM clock migration
In reply to David WoodhouseFrom: Jack Allister <[email protected]>
In the common case (where kvm->arch.use_master_clock is true), the KVM clock is defined as a simple arithmetic function of the guest TSC, based on a reference point stored in kvm->arch.master_kernel_ns and kvm->arch.master_cycle_now.
The existing KVM_[GS]ET_CLOCK functionality does not allow for this relationship to be precisely saved and restored by userspace. All it can currently do is set the KVM clock at a given UTC reference time, which is necessarily imprecise.
So on live update, the guest TSC can remain cycle accurate at precisely the same offset from the host TSC, but there is no way for userspace to restore the KVM clock accurately.
Even on live migration to a new host, where the accuracy of the guest time-keeping is fundamentally limited by the accuracy of wallclock synchronization between the source and destination hosts, the clock jump experienced by the guest's TSC and its KVM clock should at least be *consistent*. Even when the guest TSC suffers a discontinuity, its KVM clock should still remain the *same* arithmetic function of the guest TSC, and not suffer an *additional* discontinuity.
To allow for accurate migration of the KVM clock, add per-vCPU ioctls which save and restore the actual PV clock info in pvclock_vcpu_time_info.
The restoration in KVM_SET_CLOCK_GUEST works by creating a new reference point in time just as kvm_update_masterclock() does, and calculating the corresponding guest TSC value. This guest TSC value is then passed through the user-provided pvclock structure to generate the *intended* KVM clock value at that point in time, and through the *actual* KVM clock calculation. Then kvm->arch.kvmclock_offset is adjusted to eliminate the difference.
Where kvm->arch.use_master_clock is false (because the host TSC is unreliable, or the guest TSCs are configured strangely), the KVM clock is *not* defined as a function of the guest TSC so KVM_GET_CLOCK_GUEST returns an error. In this case, as documented, userspace shall use the legacy KVM_GET_CLOCK ioctl. The loss of precision is acceptable in this case since the clocks are imprecise in this mode anyway.
On *restoration*, if kvm->arch.use_master_clock is false, an error is returned for similar reasons and userspace shall fall back to using KVM_SET_CLOCK. This does mean that, as documented, userspace needs to use *both* KVM_GET_CLOCK_GUEST and KVM_GET_CLOCK and send both results with the migration data (unless the intent is to refuse to resume on a host with bad TSC).
Both ioctls return -ENODATA when the KVM clock is not in master clock mode. That covers the case of an unknown TSC frequency on the current CPU (possible only on hosts without CONSTANT_TSC): the masterclock update performed under the same IRQs-off critical section disables master clock mode when the frequency reads as zero. Advertise the ioctls with KVM_CAP_CLOCK_GUEST so that userspace need not probe for them.
--- Documentation/virt/kvm/api.rst | 53 +++++++++ arch/x86/include/uapi/asm/kvm.h | 1 + arch/x86/kvm/x86.c | 202 ++++++++++++++++++++++++++++++++ include/uapi/linux/kvm.h | 4 + 4 files changed, 260 insertions(+)
Documentation/virt/kvm/api.rst +53 -0
@@ -6767,6 +6767,59 @@ if the guest_memfd memory was pinned in IOMMU page tables. See also: :ref:`KVM_SET_MEMORY_ATTRIBUTES`. +4.147 KVM_GET_CLOCK_GUEST +---------------------------- + +:Capability: KVM_CAP_CLOCK_GUEST +:Architectures: x86_64 +:Type: vcpu ioctl +:Parameters: struct pvclock_vcpu_time_info (out) +:Returns: 0 on success, <0 on error + +Retrieves the current time information structure used for KVM/PV clocks, +in precisely the form advertised to the guest vCPU, which gives parameters +for a direct conversion from a guest TSC value to nanoseconds. + +The structure is in terms of the given vCPU's TSC. It must be passed back +to KVM_SET_CLOCK_GUEST on the *same* vCPU (on the destination VM, for +migration); vCPUs may legitimately have different TSC offsets, which would +otherwise be silently folded into the restored KVM clock. + +When the KVM clock is not in "master clock" mode, for example because the +host TSC is unreliable or the guest TSCs are oddly configured, the KVM clock +is actually defined by the host CLOCK_MONOTONIC_RAW instead of the guest TSC. +In this case, the KVM_GET_CLOCK_GUEST ioctl returns -ENODATA. + +4.148 KVM_SET_CLOCK_GUEST +---------------------------- + +:Capability: KVM_CAP_CLOCK_GUEST +:Architectures: x86_64 +:Type: vcpu ioctl +:Parameters: struct pvclock_vcpu_time_info (in) +:Returns: 0 on success, <0 on error + +Sets the KVM clock (for the whole VM) in terms of the vCPU TSC, using the +pvclock structure as returned by KVM_GET_CLOCK_GUEST. This allows the precise +arithmetic relationship between guest TSC and KVM clock to be preserved by +userspace across migration. + +When the KVM clock is not in "master clock" mode, and the KVM clock is actually +defined by the host CLOCK_MONOTONIC_RAW, this ioctl returns -ENODATA. Userspace +may choose to set the clock using the less precise KVM_SET_CLOCK ioctl, or may +choose to fail, denying migration to a host whose TSC is misbehaving. A host +whose CPU TSC frequency is unknown (only possible without +X86_FEATURE_CONSTANT_TSC) is never in "master clock" mode, so it also returns +-ENODATA. + +If the mul/shift fields of the provided pvclock structure encode a TSC +frequency inconsistent with the vCPU's actual TSC frequency, this ioctl +returns -ERANGE. + +If the vCPU's current TSC is behind the tsc_timestamp in the provided +structure, this ioctl returns -EINVAL. Restore the guest TSC (which the +documented migration procedure keeps monotonic) before invoking it. + .. _kvm_run: 5. The kvm_run structure
arch/x86/include/uapi/asm/kvm.h +1 -0
@@ -12,6 +12,7 @@#include <linux/types.h> #include <linux/ioctl.h> #include <linux/stddef.h> +#include <asm/pvclock-abi.h> #define KVM_PIO_PAGE_OFFSET 1#define KVM_COALESCED_MMIO_PAGE_OFFSET 2
arch/x86/kvm/x86.c +202 -0
220 lines not on this page · raw message
include/uapi/linux/kvm.h +4 -0
@@ -1000,6 +1000,7 @@ struct kvm_enable_cap {#define KVM_CAP_PPC_COMPAT_CAPS 250#define KVM_CAP_ARM_PMU_V3_STRICT 251#define KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES 252 +#define KVM_CAP_CLOCK_GUEST 253struct kvm_irq_routing_irqchip {__u32 irqchip;
@@ -1690,4 +1691,7 @@ struct kvm_pre_fault_memory { __u64 padding[5]; }; +#define KVM_SET_CLOCK_GUEST _IOW(KVMIO, 0xd6, struct pvclock_vcpu_time_info) +#define KVM_GET_CLOCK_GUEST _IOR(KVMIO, 0xd7, struct pvclock_vcpu_time_info) +#endif /* __LINUX_KVM_H */
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH v12 14/24] KVM: x86: Add KVM_[GS]ET_CLOCK_GUEST for accurate KVM clock migration
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [High] KVM: x86: stale masterclock state read in kvm_vcpu_ioctl_get_clock_guest() - [Medium] KVM: x86…
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [High] KVM: x86: stale masterclock state read in kvm_vcpu_ioctl_get_clock_guest()
- [Medium] KVM: x86: mismatched TSC scaling in KVM_[GS]ET_CLOCK_GUEST for protected TSC
--
commit c242a430a092692493307282dff57c9d0a945477
Author: Jack Allister <[email protected]>
KVM: x86: Add KVM_[GS]ET_CLOCK_GUEST for accurate KVM clock migration
This commit adds the KVM_GET_CLOCK_GUEST and KVM_SET_CLOCK_GUEST vCPU ioctls and KVM_CAP_CLOCK_GUEST capability to save and restore the guest pvclock relationship to the guest TSC across migration.
Quoted 4 lines
arch/x86/kvm/x86.c +0
-0
[ ... ]
Quoted 40 lines
arch/x86/kvm/x86.c +36
-0@@ -3674,6 +3677,197 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu, } } +#ifdef CONFIG_X86_64 +static int kvm_vcpu_ioctl_get_clock_guest (struct kvm_vcpu *v,void __user *argp) +{ +struct pvclock_vcpu_time_info hv_clock = {}; +struct kvm_arch *ka = &v->kvm->arch; +bool use_master_clock; +unsigned int seq; + +/* +* Construct the pvclock purely from the master clock state. The +* master mul/shift are computed for the guest TSC frequency, which +* in master clock mode is the frequency of every vCPU; only the +* tsc_timestamp is per-vCPU, translating the master snapshot into +* this vCPU's TSC domain via its scaling ratio and offset. +* +* Note, this deliberately does NOT invoke kvm_guest_time_update(), +* which would write the guest's pvclock pages: dirtying guest +* memory after the final KVM_RUN would break post-copy migration +* flows. The pvclock returned here describes the same clock as the +* guest-visible copy (the same linear function of the guest TSC), +* anchored at the current master clock snapshot. +* +* Translating the snapshot through the vCPU's *current* scaling +* ratio and offset is correct even if they changed after the +* snapshot was taken: the snapshot anchors a host TSC value, not +* a guest one. A frequency change whose masterclock update is +* still pending does leave stale mul/shift here, but that is the +* paused-VM migration path reading a frequency userspace itself +* has just changed — and the result fails safe, as the same-vCPU +* KVM_SET_CLOCK_GUEST on the destination rejects a mul/shift +* inconsistent with the vCPU's frequency with -ERANGE. +*/ + do { + seq = read_seqcount_begin(&ka->pvclock_sc); + + use_master_clock = ka->use_master_clock;
arch/x86/kvm/x86.c static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu, · line 3725
Quoted 10 lines
arch/x86/kvm/x86.c +10
-0+if (!use_master_clock) +continue ; + + hv_clock.tsc_timestamp =kvm_read_l1_tsc (v, ka->master_cycle_now); + hv_clock.system_time = ka->master_kernel_ns + ka->kvmclock_offset; + hv_clock.tsc_shift = ka->master_tsc_shift; + hv_clock.tsc_to_system_mul = ka->master_tsc_mul; + hv_clock.flags =READ_ONCE (ka->all_vcpus_matched_tsc) ? +PVCLOCK_TSC_STABLE_BIT :0 ; + }while (read_seqcount_retry (&ka->pvclock_sc, seq));
[Severity: High]
Can kvm_vcpu_ioctl_get_clock_guest() return stale or corrupted pvclock data when a KVM_REQ_MASTERCLOCK_UPDATE is pending?
Events that invalidate or change the master clock (such as a host suspend and resume with a backwards TSC, a host clocksource change away from TSC, or a per-vCPU KVM_SET_TSC_KHZ) only queue KVM_REQ_MASTERCLOCK_UPDATE on vCPUs without updating ka->use_master_clock, ka->master_cycle_now, or ka->master_tsc_mul and ka->master_tsc_shift until vcpu_enter_guest() runs.
If the host suspends and resumes while the VM is paused, kvm_arch_enable_virtualization_cpu() sets ka->backwards_tsc_observed, adds vcpu->arch.tsc_offset_adjustment, and requests KVM_REQ_MASTERCLOCK_UPDATE. When userspace then calls KVM_GET_CLOCK_GUEST, vcpu_load() -> kvm_arch_vcpu_load() applies tsc_offset_adjustment to v->arch.l1_tsc_offset before kvm_vcpu_ioctl_get_clock_guest() runs:
arch/x86/kvm/x86.c:kvm_arch_vcpu_load() {
...
if (unlikely(vcpu->arch.tsc_offset_adjustment)) {
adjust_tsc_offset_host(vcpu, vcpu->arch.tsc_offset_adjustment);
vcpu->arch.tsc_offset_adjustment = 0;
kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
}
...
}kvm_vcpu_ioctl_get_clock_guest() still sees ka->use_master_clock == true, combines the post-resume l1_tsc_offset with the pre-suspend ka->master_cycle_now, and succeeds instead of returning -ENODATA. Because ka->master_tsc_mul and ka->master_tsc_shift still match the vCPU's TSC frequency, KVM_SET_CLOCK_GUEST does not reject the state with -ERANGE and instead corrupts ka->kvmclock_offset. Similarly, if KVM_SET_TSC_KHZ changes one vCPU's frequency in a multi-vCPU VM, ka->all_vcpus_matched_freq becomes false, but calling KVM_GET_CLOCK_GUEST on an unmodified vCPU sees stale ka->use_master_clock == true with a mul/shift that still matches that vCPU.
Calling kvm_update_masterclock() when needed would refresh the host-side master clock state in struct kvm_arch without calling kvm_guest_time_update() or dirtying guest pvclock pages.
[ ... ]
Quoted 19 lines
arch/x86/kvm/x86.c +19
-0+/* +* Nonzero, or pvclock_update_vm_gtod_copy() — which read it on +* this CPU, with interrupts still disabled — would have cleared +* use_master_clock above. +*/ + curr_tsc_hz = (u64 )get_cpu_tsc_khz () *HZ_PER_KHZ ; + +if (kvm_caps.has_tsc_control) + curr_tsc_hz =kvm_scale_tsc (curr_tsc_hz, + v->arch.l1_tsc_scaling_ratio); + +/* +* The mul/shift in the provided pvclock structure encode the guest +* TSC frequency at which it was generated. Sanity-check that it is +* consistent with this vCPU's effective TSC frequency, allowing a +* discrepancy of 1 kHz either way since independently calibrated +* hosts will not measure precisely the same value even for the +* same nominal frequency. Compare in kHz: the reconstruction of +* each side's frequency in Hz carries a few Hz of rounding, which
arch/x86/kvm/x86.c static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu, · line 3850
Quoted 10 lines
arch/x86/kvm/x86.c +10
-0+* must not push two hosts calibrated exactly 1 kHz apart over the +* line. +*/ + user_tsc_khz =DIV_ROUND_CLOSEST_ULL (user_tsc_hz,HZ_PER_KHZ ); + curr_tsc_khz =DIV_ROUND_CLOSEST_ULL (curr_tsc_hz,HZ_PER_KHZ ); +if (user_tsc_khz +1 < curr_tsc_khz || + user_tsc_khz > curr_tsc_khz +1 ) { + rc = -ERANGE ; + goto out; + }
[Severity: Medium]
Does KVM_GET_CLOCK_GUEST return an unscaled mul/shift that KVM_SET_CLOCK_GUEST rejects with -ERANGE when a protected-TSC guest uses a non-default TSC frequency?
For guests with vcpu->arch.guest_tsc_protected set (such as TDX or SEV-SNP with Secure TSC), userspace configures the guest TSC frequency via the VM KVM_SET_TSC_KHZ ioctl before creating vCPUs, which sets kvm->arch.default_tsc_khz without updating kvm->arch.cur_tsc_scaling_ratio. During KVM_CREATE_VCPU, kvm_arch_vcpu_create() sets vcpu->arch.l1_tsc_scaling_ratio from default_tsc_khz, but kvm_arch_vcpu_postcreate() -> kvm_synchronize_tsc() -> __kvm_synchronize_tsc() returns early before updating kvm->arch.cur_tsc_scaling_ratio:
arch/x86/kvm/x86.c:__kvm_synchronize_tsc() {
...
if (vcpu->arch.guest_tsc_protected)
return;
...
kvm->arch.cur_tsc_scaling_ratio = vcpu->arch.l1_tsc_scaling_ratio;
...
}pvclock_update_vm_gtod_copy() then computes ka->master_tsc_shift and ka->master_tsc_mul from ka->cur_tsc_scaling_ratio, which is still kvm_caps.default_tsc_scaling_ratio. As a result, kvm_vcpu_ioctl_get_clock_guest() scales hv_clock.tsc_timestamp with v->arch.l1_tsc_scaling_ratio via kvm_read_l1_tsc(), but copies the unscaled ka->master_tsc_shift and ka->master_tsc_mul, and kvm_vcpu_ioctl_set_clock_guest() rejects that structure with -ERANGE when comparing against curr_tsc_hz scaled by v->arch.l1_tsc_scaling_ratio.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=14
David Woodhouse
maintainerPATCH v12 15/24 KVM: selftests: Add library helpers for clock and TSC-offset ioctls
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
Add wrappers for KVM_[GS]ET_CLOCK_GUEST, the KVM_VCPU_TSC_OFFSET device attribute and KVM_[GS]ET_CLOCK to the x86 library header, so that the KVM clock selftests don't each open-code the same ioctl invocations.
--- .../selftests/kvm/include/x86/processor.h | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+)
tools/testing/selftests/kvm/include/x86/processor.h +50 -0
@@ -12,6 +12,7 @@#include <asm/msr-index.h> #include <asm/prctl.h> +#include <asm/pvclock-abi.h> #include <linux/kvm_para.h> #include <linux/stringify.h>
@@ -1029,6 +1030,55 @@ static inline void vcpu_xcrs_set(struct kvm_vcpu *vcpu, struct kvm_xcrs *xcrs)vcpu_ioctl (vcpu,KVM_SET_XCRS , xcrs); } +static inline int __vcpu_get_clock_guest (struct kvm_vcpu *vcpu, +struct pvclock_vcpu_time_info *pvti) +{ +return __vcpu_ioctl (vcpu,KVM_GET_CLOCK_GUEST , pvti); +} + +static inline void vcpu_get_clock_guest (struct kvm_vcpu *vcpu, +struct pvclock_vcpu_time_info *pvti) +{ +vcpu_ioctl (vcpu,KVM_GET_CLOCK_GUEST , pvti); +} + +static inline int __vcpu_set_clock_guest (struct kvm_vcpu *vcpu, +struct pvclock_vcpu_time_info *pvti) +{ +return __vcpu_ioctl (vcpu,KVM_SET_CLOCK_GUEST , pvti); +} + +static inline void vcpu_set_clock_guest (struct kvm_vcpu *vcpu, +struct pvclock_vcpu_time_info *pvti) +{ +vcpu_ioctl (vcpu,KVM_SET_CLOCK_GUEST , pvti); +} + +static inline u64 vcpu_get_tsc_offset (struct kvm_vcpu *vcpu) +{ +u64 offset; + +vcpu_device_attr_get (vcpu,KVM_VCPU_TSC_CTRL ,KVM_VCPU_TSC_OFFSET , + &offset); +return offset; +} + +static inline void vcpu_set_tsc_offset (struct kvm_vcpu *vcpu,u64 offset) +{ +vcpu_device_attr_set (vcpu,KVM_VCPU_TSC_CTRL ,KVM_VCPU_TSC_OFFSET , + &offset); +} + +static inline void vm_get_clock (struct kvm_vm *vm,struct kvm_clock_data *data) +{ +vm_ioctl (vm,KVM_GET_CLOCK , data); +} + +static inline void vm_set_clock (struct kvm_vm *vm,struct kvm_clock_data *data) +{ +vm_ioctl (vm,KVM_SET_CLOCK , data); +} +const struct kvm_cpuid_entry2 *get_cpuid_entry (const struct kvm_cpuid2 *cpuid,u32 function,u32 index);const struct kvm_cpuid2 *kvm_get_supported_cpuid (void );
Signature · 1 line
2.43.0
David Woodhouse
maintainerPATCH v12 16/24 KVM: x86: Add KVM_VCPU_TSC_SCALE and fix the documentation on TSC migration
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
The documentation on TSC migration using KVM_VCPU_TSC_OFFSET is woefully inadequate. It ignores TSC scaling, and ignores the fact that the host TSC may differ from one host to the next (and in fact because of the way the kernel calibrates it, it generally differs from one boot to the next even on the same hardware).
Add KVM_VCPU_TSC_SCALE to extract the actual scale ratio and frac_bits, and attempt to document the process that userspace needs to follow to preserve the TSC across migration. Add a self test to function as an exemplar.
Only enumerate KVM_VCPU_TSC_SCALE when kvm_caps.has_tsc_control is true, since the scaling ratio is only meaningful when hardware TSC scaling is supported.
--- Documentation/virt/kvm/devices/vcpu.rst | 137 ++++- arch/x86/include/uapi/asm/kvm.h | 6 + arch/x86/kvm/x86.c | 21 + tools/arch/x86/include/uapi/asm/kvm.h | 6 + tools/testing/selftests/kvm/Makefile.kvm | 1 + .../kvm/x86/pvclock_migration_test.c | 521 ++++++++++++++++++ 6 files changed, 662 insertions(+), 30 deletions(-) create mode 100644 tools/testing/selftests/kvm/x86/pvclock_migration_test.c
Documentation/virt/kvm/devices/vcpu.rst +107 -30
@@ -239,6 +239,7 @@ including the layout of the stolen time structure. :Architectures: x86 4.1 ATTRIBUTE: KVM_VCPU_TSC_OFFSET +---------------------------------- :Parameters: 64-bit unsigned TSC offset
@@ -253,7 +254,10 @@ Returns: Specifies the guest's TSC offset relative to the host's TSC. The guest's TSC is then derived by the following equation: - guest_tsc = host_tsc + KVM_VCPU_TSC_OFFSET + guest_tsc = ((host_tsc * tsc_ratio) >> tsc_frac_bits) + KVM_VCPU_TSC_OFFSET + +The values of tsc_ratio and tsc_frac_bits can be obtained using +the KVM_VCPU_TSC_SCALE attribute. This attribute is useful to adjust the guest's TSC on live migration, so that the TSC counts the time during which the VM was paused. The
@@ -261,44 +265,117 @@ following describes a possible algorithm to use for this purpose. From the source VMM process: -1. Invoke the KVM_GET_CLOCK ioctl to record the host TSC (tsc_src), +1. Invoke the KVM_GET_CLOCK ioctl to record the host TSC (host_tsc_src), kvmclock nanoseconds (guest_src), and host CLOCK_REALTIME nanoseconds - (host_src). + (time_src) at a given moment (Tsrc). The host_tsc field is only valid + when the returned flags include KVM_CLOCK_HOST_TSC. + +2. For each vCPU[i]: + + a. Read the KVM_VCPU_TSC_OFFSET attribute to record the guest TSC offset + (ofs_src[i]). + + b. Read the KVM_VCPU_TSC_SCALE attribute to record the guest TSC scaling + ratio (ratio_src[i], frac_bits_src[i]). If the attribute is not + supported (-ENXIO), there is no hardware TSC scaling and identity + scaling applies: use ratio 1, frac_bits 0. + + c. Invoke the KVM_GET_TSC_KHZ ioctl to record the frequency of the + guest's TSC (freq[i]). + + d. Use host_tsc_src and the scaling/offset factors to calculate this + vCPU's TSC at time Tsrc: + + tsc_src[i] = ((host_tsc_src * ratio_src[i]) >> frac_bits_src[i]) + ofs_src[i] -2. Read the KVM_VCPU_TSC_OFFSET attribute for every vCPU to record the - guest TSC offset (ofs_src[i]). + Note that frac_bits may be as high as 48, so this multiplication + needs a 128-bit intermediate to avoid overflow. -3. Invoke the KVM_GET_TSC_KHZ ioctl to record the frequency of the - guest's TSC (freq). +3. Invoke the KVM_GET_CLOCK_GUEST ioctl on the boot vCPU to return the KVM + clock as a function of the guest TSC (pvti_src). (This ioctl may not + succeed if the host and guest TSCs are not consistent and well-behaved.) From the destination VMM process: -4. Invoke the KVM_SET_CLOCK ioctl, providing the source nanoseconds from - kvmclock (guest_src) and CLOCK_REALTIME (host_src) in their respective - fields. Ensure that the KVM_CLOCK_REALTIME flag is set in the provided - structure. +4. Invoke the KVM_SET_TSC_KHZ ioctl to set the scaled frequency of the + guest's TSC (freq[i]) — either on the VM before creating the vCPUs, or + on each vCPU after creating it (as the exemplar test does). When using + the per-vCPU form, set the frequency before performing step 7 on that + vCPU: KVM_VCPU_TSC_SCALE reports the ratio for the currently + configured frequency. + +5. Invoke the KVM_GET_CLOCK ioctl to record the host TSC (host_tsc_dst) and + host CLOCK_REALTIME nanoseconds (time_dst) at a given moment (Tdst), + again checking KVM_CLOCK_HOST_TSC in the returned flags. + +6. Calculate the number of nanoseconds elapsed between Tsrc and Tdst: + + ΔT = time_dst - time_src + +7. As each vCPU[i] is created: + + a. Read the KVM_VCPU_TSC_SCALE attribute to record the guest TSC scaling + ratio (ratio_dst[i], frac_bits_dst[i]). As in step 2b, -ENXIO means + identity scaling: use ratio 1, frac_bits 0. + + b. Calculate the intended guest TSC value at time Tdst, taking care + with the units (ΔT is in nanoseconds; freq, as set by + KVM_SET_TSC_KHZ, is in kHz): - KVM will advance the VM's kvmclock to account for elapsed time since - recording the clock values. Note that this will cause problems in - the guest (e.g., timeouts) unless CLOCK_REALTIME is synchronized - between the source and destination, and a reasonably short time passes - between the source pausing the VMs and the destination executing - steps 4-7. + tsc_dst[i] = tsc_src[i] + (ΔT * freq[i] / 10^6) + + c. Use host_tsc_dst and the scaling factors to calculate this vCPU's + raw scaled TSC at time Tdst without offsetting: + + raw_dst[i] = ((host_tsc_dst * ratio_dst[i]) >> frac_bits_dst[i]) + + d. Calculate ofs_dst[i] = tsc_dst[i] - raw_dst[i] and set the resulting + offset using the KVM_VCPU_TSC_OFFSET attribute. + +8. If pvti_src was provided, invoke the KVM_SET_CLOCK_GUEST ioctl on the boot + vCPU to restore the KVM clock as a precise function of the guest TSC. + +9. If KVM_SET_CLOCK_GUEST was not available or failed (e.g. because the + master clock is not active), fall back to the KVM_SET_CLOCK ioctl, + providing the source nanoseconds from kvmclock (guest_src) and + CLOCK_REALTIME (time_src) in their respective fields. Ensure that the + KVM_CLOCK_REALTIME flag is set in the provided structure. + + KVM will restore the VM's kvmclock, accounting for elapsed time since + the clock values were recorded. Note that this will cause problems in + the guest (e.g., timeouts) unless CLOCK_REALTIME is synchronized between + the source and destination, and a reasonably short time passes between + the source pausing the VMs and the destination resuming them. + Due to the KVM_[SG]ET_CLOCK API using CLOCK_REALTIME instead of + CLOCK_TAI, leap seconds during the migration may also introduce errors. + +4.2 ATTRIBUTE: KVM_VCPU_TSC_SCALE +--------------------------------- + +:Parameters: struct kvm_vcpu_tsc_scale + +Returns: + + ======= ====================================== + -EFAULT Error writing the result to the + provided address. + -ENXIO Attribute not supported (no TSC scaling) + -EINVAL Invalid request to write the attribute + ======= ====================================== -5. Invoke the KVM_GET_CLOCK ioctl to record the host TSC (tsc_dest) and - kvmclock nanoseconds (guest_dest). +This read-only attribute reports the guest's TSC scaling factor, in the form +of a fixed-point number represented by the following structure:: -6. Adjust the guest TSC offsets for every vCPU to account for (1) time - elapsed since recording state and (2) difference in TSCs between the - source and destination machine: + struct kvm_vcpu_tsc_scale { + __u64 tsc_ratio; + __u64 tsc_frac_bits; + }; - ofs_dst[i] = ofs_src[i] - - (guest_src - guest_dest) * freq + - (tsc_src - tsc_dest) +The tsc_frac_bits field indicates the location of the fixed point, such that +host TSC values are converted to guest TSC using the formula: - ("ofs[i] + tsc - guest * freq" is the guest TSC value corresponding to - a time of 0 in kvmclock. The above formula ensures that it is the - same on the destination as it was on the source). + guest_tsc = ((host_tsc * tsc_ratio) >> tsc_frac_bits) + offset -7. Write the KVM_VCPU_TSC_OFFSET attribute for every vCPU with the - respective value derived in the previous step. +Userspace can use this to precisely calculate the guest TSC from the host +TSC at any given moment. This is needed for accurate migration of guests, +as described in the documentation for the KVM_VCPU_TSC_OFFSET attribute.
arch/x86/include/uapi/asm/kvm.h +6 -0
@@ -964,6 +964,12 @@ struct kvm_hyperv_eventfd {/* for KVM_{GET,SET,HAS}_DEVICE_ATTR */ #define KVM_VCPU_TSC_CTRL 0/* control group for the timestamp counter (TSC) */ #define KVM_VCPU_TSC_OFFSET 0/* attribute for the TSC offset */ +#define KVM_VCPU_TSC_SCALE 1/* attribute for TSC scaling factor */ + +struct kvm_vcpu_tsc_scale { +__u64 tsc_ratio; +__u64 tsc_frac_bits; +};/* x86-specific KVM_EXIT_HYPERCALL flags. */ #define KVM_EXIT_HYPERCALL_LONG_MODE _BITULL(0)
arch/x86/kvm/x86.c +21 -0
@@ -3529,6 +3529,9 @@ static int kvm_arch_tsc_has_attr(struct kvm_vcpu *vcpu,case KVM_VCPU_TSC_OFFSET : r =0 ;break ; +case KVM_VCPU_TSC_SCALE : + r = kvm_caps.has_tsc_control ?0 : -ENXIO ; +break ;default : r = -ENXIO ; }
@@ -3549,6 +3552,21 @@ static int kvm_arch_tsc_get_attr(struct kvm_vcpu *vcpu,break ; r =0 ;break ; +case KVM_VCPU_TSC_SCALE : { +struct kvm_vcpu_tsc_scale scale; + + r = -ENXIO ; +if (!kvm_caps.has_tsc_control) +break ; + + scale.tsc_ratio = vcpu->arch.l1_tsc_scaling_ratio; + scale.tsc_frac_bits = kvm_caps.tsc_scaling_ratio_frac_bits; + r = -EFAULT ; +if (copy_to_user (uaddr, &scale,sizeof (scale))) +break ; + r =0 ; +break ; + }default : r = -ENXIO ; }
@@ -3588,6 +3606,9 @@ static int kvm_arch_tsc_set_attr(struct kvm_vcpu *vcpu, r =0 ;break ; } +case KVM_VCPU_TSC_SCALE : + r = kvm_caps.has_tsc_control ? -EINVAL : -ENXIO ; +break ;default : r = -ENXIO ; }
tools/arch/x86/include/uapi/asm/kvm.h +6 -0
@@ -963,6 +963,12 @@ struct kvm_hyperv_eventfd {/* for KVM_{GET,SET,HAS}_DEVICE_ATTR */ #define KVM_VCPU_TSC_CTRL 0/* control group for the timestamp counter (TSC) */ #define KVM_VCPU_TSC_OFFSET 0/* attribute for the TSC offset */ +#define KVM_VCPU_TSC_SCALE 1/* attribute for TSC scaling factor */ + +struct kvm_vcpu_tsc_scale { +__u64 tsc_ratio; +__u64 tsc_frac_bits; +};/* x86-specific KVM_EXIT_HYPERCALL flags. */ #define KVM_EXIT_HYPERCALL_LONG_MODE _BITULL(0)
tools/testing/selftests/kvm/Makefile.kvm +1 -0
@@ -113,6 +113,7 @@ TEST_GEN_PROGS_x86 += x86/pmu_counters_test TEST_GEN_PROGS_x86 += x86/pmu_event_filter_test TEST_GEN_PROGS_x86 += x86/private_mem_conversions_test TEST_GEN_PROGS_x86 += x86/private_mem_kvm_exits_test +TEST_GEN_PROGS_x86 += x86/pvclock_migration_test TEST_GEN_PROGS_x86 += x86/save_restore_pf_stress_test TEST_GEN_PROGS_x86 += x86/set_boot_cpu_id TEST_GEN_PROGS_x86 += x86/set_sregs_test
tools/testing/selftests/kvm/x86/pvclock_migration_test.c new file +521 -0
521 lines not on this page · raw message
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH v12 16/24] KVM: x86: Add KVM_VCPU_TSC_SCALE and fix the documentation on TSC migration
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [Low] KVM: x86: doc per-vCPU KVM_SET_TSC_KHZ order breaks KVM_SET_CLOCK_GUEST - [Low] KVM: selftests…
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [Low] KVM: x86: doc per-vCPU KVM_SET_TSC_KHZ order breaks KVM_SET_CLOCK_GUEST
- [Low] KVM: selftests: duplicated pvclock and TSC scaling helpers in test
--
commit 83ddc67114d3c0896cacdd30d2c4eb5ecc38fbfe
Author: David Woodhouse <[email protected]>
KVM: x86: Add KVM_VCPU_TSC_SCALE and fix the documentation on TSC migration
This commit adds the KVM_VCPU_TSC_SCALE vCPU device attribute to expose the guest TSC scaling ratio and fractional bits to userspace, updates the TSC migration documentation, and adds a selftest for pvclock and TSC migration.
Quoted 4 lines
Documentation/virt/kvm/devices/vcpu.rst +0
-0
[ ... ]
Quoted 1 line
Documentation/virt/kvm/devices/vcpu.rst +0
-0@@ -261,44 +265,117 @@ following describes a possible algorithm to use for this purpose.
[ ... ]
Quoted 8 lines
Documentation/virt/kvm/devices/vcpu.rst +8
-0+From the destination VMM process: + +4. Invoke the KVM_SET_TSC_KHZ ioctl to set the scaled frequency of the + guest's TSC (freq[i]) — either on the VM before creating the vCPUs, or + on each vCPU after creating it (as the exemplar test does). When using + the per-vCPU form, set the frequency before performing step 7 on that + vCPU: KVM_VCPU_TSC_SCALE reports the ratio for the currently + configured frequency.
Documentation/virt/kvm/devices/vcpu.rst following describes a possible algorithm to use for this purpose. · line 315
Quoted 10 lines
Documentation/virt/kvm/devices/vcpu.rst +10
-0+ +5. Invoke the KVM_GET_CLOCK ioctl to record the host TSC (host_tsc_dst) and + host CLOCK_REALTIME nanoseconds (time_dst) at a given moment (Tdst), + again checking KVM_CLOCK_HOST_TSC in the returned flags. + +6. Calculate the number of nanoseconds elapsed between Tsrc and Tdst: + + ΔT = time_dst - time_src + +7. As each vCPU[i] is created:
[Severity: Low]
When using the per-vCPU KVM_SET_TSC_KHZ ioctl, does creating and configuring vCPUs one at a time disable the master clock?
Without the VM-scoped KVM_SET_TSC_KHZ ioctl, each KVM_CREATE_VCPU runs kvm_arch_vcpu_postcreate() -> kvm_synchronize_tsc() at default_tsc_khz. If a VMM sets freq[0] on vCPU[0] and then creates vCPU[1], vCPU[1]'s creation bumps cur_tsc_freq_generation back to default_tsc_khz and resets nr_vcpus_matched_freq, and setting freq[1] on vCPU[1] bumps cur_tsc_freq_generation again, leaving vCPU[0] in an older generation:
arch/x86/kvm/x86.c:__kvm_synchronize_tsc() {
...
if (vcpu->arch.virtual_tsc_khz != kvm->arch.cur_tsc_khz) {
kvm->arch.cur_tsc_freq_generation++;
kvm->arch.all_vcpus_matched_freq = false;
kvm->arch.nr_vcpus_matched_freq = 0;
}
/* Count each vCPU once per freq generation */
if (vcpu->arch.this_tsc_freq_generation != kvm->arch.cur_tsc_freq_generation) {
vcpu->arch.this_tsc_freq_generation = kvm->arch.cur_tsc_freq_generation;
kvm->arch.nr_vcpus_matched_freq++;
}
...
}Because all_vcpus_matched_freq stays false, KVM_GET_CLOCK in step 5 will not set KVM_CLOCK_HOST_TSC (or KVM_SET_CLOCK_GUEST in step 8 will fail with -ENODATA if vCPUs are created during step 7). Should step 4 note that when using the per-vCPU KVM_SET_TSC_KHZ ioctl, all vCPUs must be created before setting the frequency on any of them (as create_vm() in pvclock_migration_test.c does)?
Quoted 6 lines
tools/testing/selftests/kvm/x86/pvclock_migration_test.c new file +0
-0@@ -0,0 +1,521 @@
[ ... ]
Quoted 14 lines
tools/testing/selftests/kvm/x86/pvclock_migration_test.c new file +14
-0+static u64 pvclock_read_cycles (struct pvclock_vcpu_time_info *src, +u64 tsc) +{ +u64 delta = tsc - src->tsc_timestamp; +u64 ns; + +if (src->tsc_shift >=0 ) + delta <<= src->tsc_shift; +else + delta >>= -(s32 )src->tsc_shift; + + ns = (unsigned __int128 )delta * src->tsc_to_system_mul >>32 ; +return src->system_time + ns; +}
[ ... ]
tools/testing/selftests/kvm/x86/pvclock_migration_test.c line 340
Quoted 5 lines
tools/testing/selftests/kvm/x86/pvclock_migration_test.c new file +5
-0+if (scale_dst.tsc_frac_bits) + raw_dst = (unsigned __int128 )host_tsc_dst * scale_dst.tsc_ratio + >> scale_dst.tsc_frac_bits; +else + raw_dst = host_tsc_dst;
[Severity: Low]
This isn't a bug, but pvclock_read_cycles() duplicates __pvclock_read_cycles() from tools/arch/x86/include/asm/pvclock.h, and raw_dst open-codes host_to_guest_tsc(&scale_dst, 0, host_tsc_dst) defined earlier in this file.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=16
David Woodhouse
maintainerPATCH v12 17/24 KVM: selftests: Add KVM/PV clock selftest to prove timer correction
In reply to David WoodhouseFrom: Jack Allister <[email protected]>
A VM's KVM/PV clock has an inherent relationship to its TSC. When either the host system live-updates or the VM is live-migrated this pairing of the two clock sources should stay the same. In reality this is not the case without some correction taking place.
The KVM_GET_CLOCK_GUEST/KVM_SET_CLOCK_GUEST ioctls can be used to perform a correction on the PVTI (PV time information) structure held by KVM to effectively fix up the kvmclock_offset prior to the guest VM resuming in either a live-update/migration scenario.
This test proves that without the necessary fixup there is a perceived change in the guest TSC and KVM/PV clock relationship before and after a simulated LU/LM takes place, and that the correction eliminates it.
The test:
1. Snapshots the PVTI at boot (PVTI0).
2. Steps the KVM clock forward 10 seconds with KVM_SET_CLOCK and induces a change in PVTI data, so the divergence the correction must eliminate is deterministic.
3. Snapshots the PVTI after the change (PVTI1).
4. Requests correction via KVM_SET_CLOCK_GUEST using PVTI0.
5. Snapshots the PVTI after correction (PVTI2).
Then samples the TSC at a single point in time and calculates the KVM clock using each PVTI snapshot, verifying that the uncorrected clock diverged and that the corrected clock matches the boot clock to within ±2ns.
Also test PVCLOCK_TSC_STABLE_BIT across matched, offset-mismatched and frequency-mismatched vCPUs, and KVM_[GS]ET_CLOCK_GUEST on vCPUs with divergent TSC offsets.
The test enumerates multiple TSC frequencies from 1GHz to 5GHz at 500MHz steps, crossing the 32-bit boundary, to exercise the scaling path at various ratios. The sleep duration between snapshots is configurable via the -s/--sleep command line option.
--- tools/testing/selftests/kvm/Makefile.kvm | 1 + .../testing/selftests/kvm/x86/pvclock_test.c | 455 ++++++++++++++++++ 2 files changed, 456 insertions(+) create mode 100644 tools/testing/selftests/kvm/x86/pvclock_test.c
tools/testing/selftests/kvm/Makefile.kvm +1 -0
@@ -114,6 +114,7 @@ TEST_GEN_PROGS_x86 += x86/pmu_event_filter_test TEST_GEN_PROGS_x86 += x86/private_mem_conversions_test TEST_GEN_PROGS_x86 += x86/private_mem_kvm_exits_test TEST_GEN_PROGS_x86 += x86/pvclock_migration_test +TEST_GEN_PROGS_x86 += x86/pvclock_test TEST_GEN_PROGS_x86 += x86/save_restore_pf_stress_test TEST_GEN_PROGS_x86 += x86/set_boot_cpu_id TEST_GEN_PROGS_x86 += x86/set_sregs_test
tools/testing/selftests/kvm/x86/pvclock_test.c new file +455 -0
455 lines not on this page · raw message
Signature · 1 line
2.43.0
David Woodhouse
maintainerPATCH v12 18/24 KVM: selftests: Add master clock offset test
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
Verify that KVM master clock mode remains active when vCPUs have different TSC offsets but the same frequency. Creates three vCPUs, sets one to a different TSC value, and confirms:
- KVM_CLOCK_HOST_TSC is set (master clock active)
- KVM_CLOCK_TSC_STABLE IS set (master clock ⇒ stable flag)
- PVCLOCK_TSC_STABLE_BIT is NOT set (offsets differ)
Then set the KVM clock forward one hour with KVM_SET_CLOCK and verify that all vCPUs still agree on the KVM clock computed from their own pvclock parameters.
--- tools/testing/selftests/kvm/Makefile.kvm | 1 + .../kvm/x86/masterclock_offset_test.c | 203 ++++++++++++++++++ 2 files changed, 204 insertions(+) create mode 100644 tools/testing/selftests/kvm/x86/masterclock_offset_test.c
tools/testing/selftests/kvm/Makefile.kvm +1 -0
@@ -95,6 +95,7 @@ TEST_GEN_PROGS_x86 += x86/hyperv_tlb_flush TEST_GEN_PROGS_x86 += x86/kvm_clock_test TEST_GEN_PROGS_x86 += x86/kvm_pv_test TEST_GEN_PROGS_x86 += x86/kvm_buslock_test +TEST_GEN_PROGS_x86 += x86/masterclock_offset_test TEST_GEN_PROGS_x86 += x86/monitor_mwait_test TEST_GEN_PROGS_x86 += x86/msrs_test TEST_GEN_PROGS_x86 += x86/nested_close_kvm_test
tools/testing/selftests/kvm/x86/masterclock_offset_test.c new file +203 -0
203 lines not on this page · raw message
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH v12 18/24] KVM: selftests: Add master clock offset test
Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [Medium] KVM: selftests: get_pvtis() never reads guest pvclock pages at KVMCLOCK_GPA --
Thank you for your contribution! Sashiko AI review found 1 potential issue to consider:
- [Medium] KVM: selftests: get_pvtis() never reads guest pvclock pages at KVMCLOCK_GPA
--
commit afe5627be945a0319bdf870a16137575eb9a6ca5
Author: David Woodhouse <[email protected]>
KVM: selftests: Add master clock offset test
This commit adds an x86 KVM selftest to verify that master clock mode remains active and pvclock readings stay consistent across vCPUs with different TSC offsets, both initially and after advancing the clock via KVM_SET_CLOCK.
Quoted 6 lines
tools/testing/selftests/kvm/x86/masterclock_offset_test.c new file +0
-0@@ -0,0 +1,203 @@
[ ... ]
Quoted 16 lines
tools/testing/selftests/kvm/x86/masterclock_offset_test.c new file +16
-0+static void guest_code (u64 pvti_gpa) +{ +wrmsr (MSR_KVM_SYSTEM_TIME_NEW , pvti_gpa |KVM_MSR_ENABLED ); +for (;;) +GUEST_SYNC (0 ); +} + +static void get_pvtis (struct kvm_vcpu **vcpus, +struct pvclock_vcpu_time_info *pvti) +{ +int i; + +for (i =0 ; i <NR_VCPUS ; i++) { +int idx = vcpu_order[i]; + +vcpu_get_clock_guest (vcpus[idx], &pvti[idx]);
[ ... ]
Quoted 17 lines
tools/testing/selftests/kvm/x86/masterclock_offset_test.c new file +17
-0+static void test_set_clock_consistency (struct kvm_vm *vm, +struct kvm_vcpu **vcpus, +struct pvclock_vcpu_time_info *pvti, +struct kvm_clock_data *clock, +u64 offset0) +{ +struct kvm_clock_data setclk = { .clock = clock->clock +ONE_HOUR_NS }; +u64 clk0; +int i; + +/* +* Add an hour to the VM kvmclock via KVM_SET_CLOCK, run each +* vCPU to pick up the update, and check they're still in sync. +*/ +vm_set_clock (vm, &setclk); + +/* Guest code does GUEST_SYNC then exits — run each to see update */
tools/testing/selftests/kvm/x86/masterclock_offset_test.c line 146
Quoted 10 lines
tools/testing/selftests/kvm/x86/masterclock_offset_test.c new file +10
-0+for (i =0 ; i <NR_VCPUS ; i++) { +vcpu_run (vcpus[vcpu_order[i]]); +TEST_ASSERT_KVM_EXIT_REASON (vcpus[vcpu_order[i]],KVM_EXIT_IO ); + } + +get_pvtis (vcpus, pvti); + +TEST_ASSERT (pvti[0 ].system_time == pvti[1 ].system_time && + pvti[0 ].system_time == pvti[2 ].system_time, +"system_time should still match after KVM_SET_CLOCK" );
[Severity: Medium]
Should get_pvtis() read the per-vCPU pvclock_vcpu_time_info structures from guest memory at KVMCLOCK_GPA (or check both guest memory and vcpu_get_clock_guest())?
The test sets up guest memory at KVMCLOCK_GPA, enables MSR_KVM_SYSTEM_TIME_NEW in guest_code(), and runs each vCPU after vm_set_clock() so kvm_guest_time_update() refreshes the guest pvclock structures. However, get_pvtis() only calls vcpu_get_clock_guest() (KVM_GET_CLOCK_GUEST), which synthesizes hv_clock directly from the VM-wide master clock fields in struct kvm_arch without reading the guest pvclock pages:
arch/x86/kvm/x86.c:kvm_vcpu_ioctl_get_clock_guest() {
...
hv_clock.tsc_timestamp = kvm_read_l1_tsc(v, ka->master_cycle_now);
hv_clock.system_time = ka->master_kernel_ns + ka->kvmclock_offset;
hv_clock.tsc_shift = ka->master_tsc_shift;
hv_clock.tsc_to_system_mul = ka->master_tsc_mul;
hv_clock.flags = READ_ONCE(ka->all_vcpus_matched_tsc) ?
PVCLOCK_TSC_STABLE_BIT : 0;
...
}As a result, the guest memory at KVMCLOCK_GPA is never inspected, and checking that tsc_to_system_mul, tsc_shift, and system_time match across vCPUs compares the same VM-wide ka->master_* fields against themselves regardless of whether vcpu_run() updated the guest pvclock structures. In addition, because vcpu_set_tsc_offset() on vcpus[2] in main() queues KVM_REQ_MASTERCLOCK_UPDATE only on vcpus[2], running vcpus[0..2] once in order populates the guest pvclock structures for vcpus[0] and vcpus[1] before vcpus[2] runs kvm_update_masterclock() and broadcasts KVM_REQ_CLOCK_UPDATE, leaving vcpus[0] and vcpus[1] with stale guest memory state until the next vcpu_run().
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=18
David Woodhouse
maintainerPATCH v12 19/24 KVM: selftests: Add Xen/generic CPUID timing leaf test
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
Verify that userspace can correctly populate Xen and generic CPUID timing leaves using the KVM_GET_TSC_KHZ ioctl and KVM_VCPU_TSC_SCALE attribute.
This validates that commit 7d3bd21e457b ("KVM: x86: Remove runtime Xen
TSC frequency CPUID update") doesn't break guests: userspace queries the effective TSC and bus frequencies, computes the pvclock mul/shift, populates the CPUID leaves,
and the guest verifies the values match.
The test exercises:
- KVM_GET_TSC_KHZ at native and scaled frequencies
- KVM_VCPU_TSC_SCALE ratio verification against effective frequency
- Generic timing leaf 0x40000010 (EAX=tsc_khz, EBX=bus_khz)
- Xen leaf 3 sub-leaf 0 (ECX=guest TSC kHz)
- Xen leaf 3 sub-leaf 1 (ECX=mul, EDX=shift)
Where KVM_CAP_XEN_HVM is available, the guest is given the Xen signature leaf and the Xen hypercall MSR is enabled — the exact configuration in which the removed runtime update used to fire — and a sentinel is planted in leaf 3 sub-leaf 2 EAX, where it used to write, which the guest verifies survives untouched.
Gracefully skips TSC scaling tests on hardware without support, and skips requested frequencies within tsc_tolerance_ppm of the host's, where hardware scaling does not engage and the effective frequency is the requested one with an identity ratio.
--- tools/testing/selftests/kvm/Makefile.kvm | 1 + .../selftests/kvm/x86/xen_cpuid_timing_test.c | 271 ++++++++++++++++++ 2 files changed, 272 insertions(+) create mode 100644 tools/testing/selftests/kvm/x86/xen_cpuid_timing_test.c
tools/testing/selftests/kvm/Makefile.kvm +1 -0
@@ -152,6 +152,7 @@ TEST_GEN_PROGS_x86 += x86/xss_msr_test TEST_GEN_PROGS_x86 += x86/debug_regs TEST_GEN_PROGS_x86 += x86/tsc_msrs_test TEST_GEN_PROGS_x86 += x86/vmx_pmu_caps_test +TEST_GEN_PROGS_x86 += x86/xen_cpuid_timing_test TEST_GEN_PROGS_x86 += x86/xen_shinfo_test TEST_GEN_PROGS_x86 += x86/xen_vmcall_test TEST_GEN_PROGS_x86 += x86/sev_dbg_test
tools/testing/selftests/kvm/x86/xen_cpuid_timing_test.c new file +271 -0
271 lines not on this page · raw message
Signature · 1 line
2.43.0
David Woodhouse
maintainerPATCH v12 20/24 KVM: selftests: Add Xen runstate migration test
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
Test that Xen runstate (steal time) is correctly accounted across a simulated live migration using KVM_XEN_VCPU_ATTR and KVM_[GS]ET_CLOCK_GUEST.
The test simulates what a real VMM does during migration:
1. Creates a VM with Xen HVM config and runstate tracking
2. Runs the guest to accumulate some kvmclock time
3. Saves clock (KVM_GET_CLOCK_GUEST), TSC offset, and runstate
4. Marks the saved state as RUNSTATE_runnable (vCPU not running)
5. Destroys the source VM
6. Sleeps 10ms (simulating migration network transfer time)
7. Creates a new VM and restores all state precisely as saved
8. Runs the guest and verifies the migration gap appears as steal
The VM is placed in long mode explicitly (KVM_XEN_ATTR_TYPE_LONG_MODE) so the kernel writes the 64-bit runstate layout; nothing in this test writes the hypercall-page MSR, which is what would otherwise latch the mode from the guest's EFER.
The guest TSC is transferred via the KVM_VCPU_TSC_OFFSET attribute. As the "migration" is to a new VM on the same host, restoring the same offset makes the guest TSC continue uninterrupted across the gap, so the restored pvclock yields a kvmclock which has kept running too. That is what makes the migration gap visible: the destination kvmclock is ahead of the saved state_entry_time by (at least) the gap duration, rather than restarting from the destination VM's creation time.
The kernel accounts the gap because: on vcpu_load, it transitions from RUNSTATE_runnable to RUNSTATE_running, computing delta = kvmclock_now - state_entry_time. Since kvmclock has advanced past the saved entry time (real time elapsed during migration), the delta is added to time_runnable.
--- tools/testing/selftests/kvm/Makefile.kvm | 1 + .../selftests/kvm/x86/xen_migration_test.c | 222 ++++++++++++++++++ 2 files changed, 223 insertions(+) create mode 100644 tools/testing/selftests/kvm/x86/xen_migration_test.c
tools/testing/selftests/kvm/Makefile.kvm +1 -0
@@ -153,6 +153,7 @@ TEST_GEN_PROGS_x86 += x86/debug_regs TEST_GEN_PROGS_x86 += x86/tsc_msrs_test TEST_GEN_PROGS_x86 += x86/vmx_pmu_caps_test TEST_GEN_PROGS_x86 += x86/xen_cpuid_timing_test +TEST_GEN_PROGS_x86 += x86/xen_migration_test TEST_GEN_PROGS_x86 += x86/xen_shinfo_test TEST_GEN_PROGS_x86 += x86/xen_vmcall_test TEST_GEN_PROGS_x86 += x86/sev_dbg_test
tools/testing/selftests/kvm/x86/xen_migration_test.c new file +222 -0
222 lines not on this page · raw message
Signature · 1 line
2.43.0
David Woodhouse
maintainerPATCH v12 21/24 KVM: selftests: Test that the first user TSC write is honoured exactly
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
The 1-second "slop" heuristic in kvm_synchronize_tsc() is only supposed to apply between successive userspace TSC writes, i.e. once kvm->arch.user_set_tsc has been set by a previous write. The kernel's own internal TSC synchronization on vCPU creation (and frequency changes) must not arm it, otherwise the first genuine userspace write falling within 1 second of virtual cycle time of the kernel's default generation is "corrected" back to that generation instead of being honoured — breaking the restore of a guest with less than one second of runtime.
Add a test which creates a vCPU and immediately performs a first host-initiated write of a TSC value corresponding to ~500ms of guest runtime, then verifies the value reads back exactly (modulo elapsed time) rather than being snapped back to the kernel's default generation near zero.
--- tools/testing/selftests/kvm/Makefile.kvm | 1 + .../selftests/kvm/x86/tsc_user_set_test.c | 153 ++++++++++++++++++ 2 files changed, 154 insertions(+) create mode 100644 tools/testing/selftests/kvm/x86/tsc_user_set_test.c
tools/testing/selftests/kvm/Makefile.kvm +1 -0
@@ -151,6 +151,7 @@ TEST_GEN_PROGS_x86 += x86/xcr0_cpuid_test TEST_GEN_PROGS_x86 += x86/xss_msr_test TEST_GEN_PROGS_x86 += x86/debug_regs TEST_GEN_PROGS_x86 += x86/tsc_msrs_test +TEST_GEN_PROGS_x86 += x86/tsc_user_set_test TEST_GEN_PROGS_x86 += x86/vmx_pmu_caps_test TEST_GEN_PROGS_x86 += x86/xen_cpuid_timing_test TEST_GEN_PROGS_x86 += x86/xen_migration_test
tools/testing/selftests/kvm/x86/tsc_user_set_test.c new file +153 -0
153 lines not on this page · raw message
Signature · 1 line
2.43.0
David Woodhouse
maintainerPATCH v12 22/24 KVM: x86/xen: Drop open-coded fallback clock read in kvm_xen_start_timer()
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
When no coherent (CLOCK_MONOTONIC, TSC) pair was available, kvm_xen_start_timer() computed one from separate ktime_get() and rdtsc() calls rather than using get_kvmclock_ns(), with a comment explaining that the latter had a systemic error from scaling host TSC directly to nanoseconds instead of scaling to the guest TSC first.
Commit abdef42276d8 ("KVM: x86: Fix KVM clock precision in get_kvmclock() with TSC scaling") removed that systemic error: in master clock mode, get_kvmclock_ns() now scales the host TSC delta to the guest TSC and converts it with the guest's own mul/shift, computing precisely what the guest would.
The fallback is also nearly unreachable. It sits behind the use_master_clock check, so on 32-bit (where the master clock is never active) it is dead code, and on 64-bit kvm_get_monotonic_and_clockread() can only fail there if the host clocksource changed after the last master clock update, in the window before the resulting masterclock update is processed. In exactly that window get_kvmclock_ns() takes its raw-clock fallback path rather than the guest TSC computation, which is continuous with the last valid snapshot and tolerable for the one timer that can be started before the pending masterclock update lands.
Drop it and fall back to the existing get_kvmclock_ns() path.
--- arch/x86/kvm/xen.c | 48 ++++++++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 25 deletions(-)
arch/x86/kvm/xen.c +23 -25
@@ -188,7 +188,7 @@ static void kvm_xen_start_timer(struct kvm_vcpu *vcpu, u64 guest_abs,struct kvm_vcpu_xen *xen = &vcpu->arch.xen;int64_t kernel_now, delta;uint64_t guest_now; -int r = -EOPNOTSUPP ; +bool fallback = true;/* * The guest provides the requested timeout in absolute nanoseconds
@@ -210,6 +210,7 @@ static void kvm_xen_start_timer(struct kvm_vcpu *vcpu, u64 guest_abs, do { struct pvclock_vcpu_time_info hv_clock; uint64_t host_tsc, guest_tsc; + int r = -EOPNOTSUPP; if (!cpu_feature_enabled(X86_FEATURE_CONSTANT_TSC) || !vcpu->kvm->arch.use_master_clock)
@@ -230,43 +231,40 @@ static void kvm_xen_start_timer(struct kvm_vcpu *vcpu, u64 guest_abs,if (r)break ; +/* +* With use_master_clock set, the clocksource was TSC-based +* when the master clock was last updated. The clockread can +* only fail here if the clocksource has changed since, in +* which case a masterclock update is already pending and +* get_kvmclock_ns() returns the raw-clock fallback value — +* continuous with the last valid snapshot, which is as good +* as it gets in that transient window. +*/ if (!IS_ENABLED (CONFIG_64BIT ) || - !kvm_get_monotonic_and_clockread (&kernel_now, &host_tsc)) { -/* -* Don't fall back to get_kvmclock_ns() because it's -* broken; it has a systemic error in its results -* because it scales directly from host TSC to -* nanoseconds, and doesn't scale first to guest TSC -* and *then* to nanoseconds as the guest does. -* -* There is a small error introduced here because time -* continues to elapse between the ktime_get() and the -* subsequent rdtsc(). But not the systemic drift due -* to get_kvmclock_ns(). -*/ - kernel_now =ktime_get ();/* This is CLOCK_MONOTONIC */ - host_tsc =rdtsc (); - } + !kvm_get_monotonic_and_clockread (&kernel_now, &host_tsc)) +break ;/* Calculate the guest kvmclock as the guest would do it. */ guest_tsc =kvm_read_l1_tsc (vcpu, host_tsc); guest_now =__pvclock_read_cycles (&hv_clock, guest_tsc); + fallback = false; }while (0 ); -if (r) { +if (fallback) {/* -* Without CONSTANT_TSC, get_kvmclock_ns() is the only option. +* Without CONSTANT_TSC and the master clock, +* get_kvmclock_ns() is the only option. * * Also if the guest PV clock hasn't been set up yet, as is * likely to be the case during migration when the vCPU has * not been run yet. It would be possible to calculate the * scaling factors properly in that case but there's not much -* point in doing so. The get_kvmclock_ns() drift accumulates -* over time, so it's OK to use it at startup. Besides, on -* migration there's going to be a little bit of skew in the -* precise moment at which timers fire anyway. Often they'll -* be in the "past" by the time the VM is running again after -* migration. +* point in doing so. Without the master clock, the +* get_kvmclock_ns() drift accumulates over time, so it's OK +* to use it at startup. Besides, on migration there's going +* to be a little bit of skew in the precise moment at which +* timers fire anyway. Often they'll be in the "past" by the +* time the VM is running again after migration. */ guest_now =get_kvmclock_ns (vcpu->kvm); kernel_now =ktime_get ();
Signature · 1 line
2.43.0
David Woodhouse
maintainerPATCH v12 23/24 KVM: x86: Use a single time snapshot in kvm_vm_ioctl_set_clock()
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
The KVM_CLOCK_REALTIME handling read CLOCK_REALTIME with ktime_get_real_ns() and then anchored the result at a different
instant: the master clock reference point captured a few lines earlier, or a separate get_kvmclock_base_ns() read. The time elapsed between the two reads is added to the restored clock as error -- bounded by the interrupts-off critical section, but gratuitous now that the timekeeping core can snapshot CLOCK_REALTIME and CLOCK_MONOTONIC_RAW at the same instant.
Take one ktime_get_snapshot_id(CLOCK_REALTIME) snapshot and derive both the realtime correction and the raw reference point from it. In master clock mode, keep anchoring at master_kernel_ns -- so that 'system_time' cannot go negative for a very small data.clock, as the existing comment explains -- by projecting the snapshot's realtime value back to that reference point; CLOCK_REALTIME and CLOCK_MONOTONIC_RAW tick at the same rate across the few ns in between, give or take NTP slew.
--- arch/x86/kvm/x86.c | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-)
arch/x86/kvm/x86.c +31 -5
@@ -4855,19 +4855,45 @@ static int kvm_vm_ioctl_set_clock(struct kvm *kvm, void __user *argp)* 'system_time' when 'data.clock' is very small. */ if (data.flags &KVM_CLOCK_REALTIME ) { -u64 now_real_ns =ktime_get_real_ns (); +struct system_time_snapshot snap = {}; +u64 now_real_ns; + +/* +* Snapshot CLOCK_REALTIME and CLOCK_MONOTONIC_RAW at the +* same instant, so that the elapsed time applied to +* data.clock and the reference point the result is anchored +* to are coherent. +*/ +ktime_get_snapshot_id (CLOCK_REALTIME , &snap); + now_real_ns =ktime_to_ns (snap.systime); + now_raw_ns =ktime_to_ns (ktime_mono_to_any (snap.monoraw, +TK_OFFS_BOOT )); + +/* +* When the master clock is in use, anchor at its reference +* point instead, projecting the realtime value back to the +* same instant. The two clocks tick at the same rate across +* the few ns in between, give or take NTP slew. Skip the +* projection in the absurd case that it would take a +* CLOCK_REALTIME set to within moments of the 1970 epoch +* (legal with CAP_SYS_TIME) below zero and wrap. +*/ +if (ka->use_master_clock && + now_real_ns >= now_raw_ns - ka->master_kernel_ns) { + now_real_ns -= now_raw_ns - ka->master_kernel_ns; + now_raw_ns = ka->master_kernel_ns; + }/* * Avoid stepping the kvmclock backwards. */ if (now_real_ns > data.realtime) data.clock += now_real_ns - data.realtime; - } - -if (ka->use_master_clock) + }else if (ka->use_master_clock) { now_raw_ns = ka->master_kernel_ns; -else + }else { now_raw_ns =get_kvmclock_base_ns (); + } ka->kvmclock_offset = data.clock - now_raw_ns;kvm_end_pvclock_update (kvm);return 0 ;
Signature · 1 line
2.43.0
David Woodhouse
maintainerPATCH v12 24/24 KVM: x86: Keep master clock for guests using legacy MSR_KVM_SYSTEM_TIME
In reply to David WoodhouseFrom: David Woodhouse <[email protected]>
Commit 54750f2cf042 ("KVM: x86: workaround SuSE's 2.6.16 pvclock vs masterclock issue") disables the master clock entirely for any guest whose boot vCPU uses the legacy MSR. SUSE's 2.6.16 kernel treats a pvclock tsc_timestamp more than 1/5 jiffie behind the TSC as suspect and re-requests the time by rewriting the MSR; against the master clock's long-lived reference point the delta never shrinks, so it re-requests in a loop and fails to boot.
Satisfy the guest's freshness heuristic instead of capitulating to it:
a guest write to the legacy MSR now requests a masterclock update, sliding the reference point forward before the pvclock is written (the masterclock request is serviced before KVM_REQ_CLOCK_UPDATE on the same guest entry). The re-requested pvclock then carries a recent tsc_timestamp, the delta is back under the threshold, and the 2.6.16 heuristic is content. The reference point moving is invisible to any correct guest — the clock remains the same function of the guest TSC.
A guest hammering the legacy MSR now forces repeated VM-wide masterclock updates, but only its own VM's: a guest has always been able to burn its own cycles. Re-anchoring just the writing vCPU's pvclock along the masterclock line would avoid even that, but isn't worth the extra plumbing for a 2006 guest kernel.
This removes the last way a guest could force the master clock off. The remaining disable, backwards_tsc_observed, is the host's own doing. The pvclock_test selftest grows a legacy-MSR case proving the master clock survives and the rewritten pvclock is fresh.
Not tested against a real SLES10 image; the behaviour it demands is reconstructed from the guest comment quoted in 54750f2cf042.
--- arch/x86/include/asm/kvm_host.h | 1 - arch/x86/kvm/msrs.c | 26 ++++---- arch/x86/kvm/x86.c | 15 +---- .../testing/selftests/kvm/x86/pvclock_test.c | 63 +++++++++++++++++++ 4 files changed, 77 insertions(+), 28 deletions(-)
arch/x86/include/asm/kvm_host.h +0 -1
@@ -1272,7 +1272,6 @@ struct kvm_arch {#endif bool backwards_tsc_observed; -bool boot_vcpu_runs_old_kvmclock;u32 bsp_vcpu_id;u64 disabled_quirks;
arch/x86/kvm/msrs.c +13 -13
@@ -1275,19 +1275,19 @@ static void kvm_write_wall_clock(struct kvm *kvm, gpa_t wall_clock, int sec_hi_ostatic void kvm_write_system_time (struct kvm_vcpu *vcpu,gpa_t system_time,bool old_msr,bool host_initiated) { -struct kvm_arch *ka = &vcpu->kvm->arch; - -if (vcpu->vcpu_id ==0 && !host_initiated) { -/* -* Store the flag before requesting the masterclock update: -* the update may now be performed by another vCPU (which -* consumes this request), and it must observe the new value. -*/ -if (ka->boot_vcpu_runs_old_kvmclock != old_msr) { - ka->boot_vcpu_runs_old_kvmclock = old_msr; -kvm_make_request (KVM_REQ_MASTERCLOCK_UPDATE , vcpu); - } - } +/* +* Slide the master clock reference point forward, so that the +* pvclock written below carries a recent tsc_timestamp. SUSE's +* 2.6.16 kernel (the only known user of the legacy MSR with a +* reason to care) re-requests the time by rewriting this MSR +* whenever tsc_timestamp falls more than 1/5 jiffie behind the +* TSC, and would livelock against a fixed reference point; each +* rewrite now genuinely freshens it. A write with the enable +* bit clear deactivates the pvclock; there is nothing to +* freshen. +*/ +if (old_msr && !host_initiated && (system_time &1 )) +kvm_make_request (KVM_REQ_MASTERCLOCK_UPDATE , vcpu); vcpu->arch.time = system_time;
arch/x86/kvm/x86.c +1 -14
@@ -1144,21 +1144,8 @@ static bool kvm_use_master_clock(struct kvm *kvm) {struct kvm_arch *ka = &kvm->arch; -/* -* The 'old kvmclock' check is a workaround (from 2015) for a -* SUSE 2.6.16 kernel that didn't boot if the system_time in -* its kvmclock was too far behind the current time. So the -* mode of just setting the reference point and allowing time -* to proceed linearly from there makes it fail to boot. -* Despite that being kind of the *point* of the way the clock -* is exposed to the guest. By coincidence, the offending -* kernels used the old MSR_KVM_SYSTEM_TIME, which was moved -* only because it resided in the wrong number range. So the -* workaround is activated for *all* guests using the old MSR. -*/ return ka->all_vcpus_matched_freq && - !ka->backwards_tsc_observed && - !ka->boot_vcpu_runs_old_kvmclock; + !ka->backwards_tsc_observed; }static void kvm_track_tsc_matching (struct kvm_vcpu *vcpu,bool update_mclock)
tools/testing/selftests/kvm/x86/pvclock_test.c +63 -0
79 lines not on this page · raw message
Signature · 1 line
2.43.0