Re: [PATCH v10 21/21] KVM: x86: Use kernel timekeeping snapshot to get walltime+TSC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removal of KVM's local TSC clamping exposes guests to mismatched (tsc, walltime) pairs when the host TSC goes backwards, causing guest time to jump forward.
--

commit 538536e82164aaa6297ef0ce3bf878f455e79fc7
Author: Sean Christopherson <seanjc@google.com>

KVM: x86: Use kernel timekeeping snapshot to get walltime+TSC

This commit replaces the KVM-private timekeeping reimplementation with calls
to the generic ktime_get_snapshot_id() interface. It provides both the system
time and raw cycles atomically paired using a sequence counter, allowing the
deletion of the unused read_tsc() and vgettsc() functions.

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 0352bd147c400..34eadc75fee46 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
[ ... ]
> @@ -1525,11 +1449,14 @@ bool kvm_get_monotonic_and_clockread(s64 *kernel_ns, u64 *tsc_timestamp)
>  static bool kvm_get_walltime_and_clockread(struct timespec64 *ts,
>  					   u64 *tsc_timestamp)
>  {
> -	/* checked again under seqlock below */
> -	if (!gtod_is_based_on_tsc(pvclock_gtod_data.clock.vclock_mode))
> +	struct system_time_snapshot snap = {};
> +
> +	ktime_get_snapshot_id(CLOCK_REALTIME, &snap);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
Does removing KVM's local TSC clamping expose guests to mismatched time pairs?

When establishing the guest's PV clock, KVM provides a master clock
snapshot pairing a host TSC with a system time. The previous read_tsc()
logic explicitly clamped the TSC to cycle_last to maintain mathematical
consistency.

Looking at ktime_get_snapshot_id() in kernel/time/timekeeping.c, the
function does not clamp the returned raw TSC (cycles):

kernel/time/timekeeping.c:ktime_get_snapshot_id() {
    ...
	systime_snapshot->cycles = now;
    ...
}

However, the system time is clamped by timekeeping_cycles_to_ns(), which
skips the negative delta if now < cycle_last.

If the host TSC read on the current CPU is slightly behind tkr->cycle_last
(for example, due to cross-CPU TSC skew), could this result in KVM
returning a mathematically mismatched pair?

An older unclamped TSC (now) paired with a clamped future walltime
(at cycle_last) would cause the elapsed delta (guest_tsc - master_tsc)
to be artificially inflated when the guest later reads the TSC.

Could this cause the guest's clock to jump forward by that offset?

> +	if (!kvm_snapshot_has_tsc(&snap, tsc_timestamp))
>  		return false;
>  
> -	return gtod_is_based_on_tsc(do_realtime(ts, tsc_timestamp));
> +	*ts = ktime_to_timespec64(snap.systime);
> +	return true;
>  }
>  #endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826213303.914988-1-seanjc@google.com?part=21