Re: [PATCH v5 12/34] KVM: x86: Fix KVM clock precision in get_kvmclock() with TSC scaling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Precision loss discrepancy between master clock and vCPU pvclock causes monotonic time jumps on live migration.
--

commit 47c298cb974d0bc994c9dfd80ba585010b2ad459
Author: David Woodhouse <dwmw@amazon.co.uk>

KVM: x86: Fix KVM clock precision in get_kvmclock() with TSC scaling

This patch fixes a systemic drift in the KVM clock when using TSC scaling.
It precomputes the guest-TSC-based multiplier and shift in
pvclock_update_vm_gtod_copy() and applies them in get_kvmclock() to correctly
scale the host TSC delta to the guest TSC before converting to nanoseconds.

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 6983a7494fcdd..7ae6a77053530 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c

[ ... ]

> @@ -3132,9 +3135,30 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm)
>  				&& !ka->backwards_tsc_observed
>  				&& !ka->boot_vcpu_runs_old_kvmclock;
>  
> -	if (ka->use_master_clock)
> +	if (ka->use_master_clock) {
> +		u64 tsc_hz;
> +
>  		atomic_set(&kvm_guest_has_master_clock, 1);
>  
> +		/*
> +		 * Copy the scaling ratio and precompute the mul/shift for
> +		 * converting guest TSC to nanoseconds. These are used by
> +		 * get_kvmclock() to compute kvmclock from the host TSC
> +		 * without needing a vCPU reference.
> +		 */
> +		ka->master_tsc_scaling_ratio = ka->last_tsc_scaling_ratio;
> +		tsc_hz = (u64)get_cpu_tsc_khz() * 1000;
> +		if (tsc_hz && kvm_caps.has_tsc_control)
> +			tsc_hz = kvm_scale_tsc(tsc_hz,
> +					       ka->master_tsc_scaling_ratio);

[Severity: High]
Does calculating tsc_hz by multiplying by 1000 before scaling create a
precision discrepancy with the guest's pvclock?

In kvm_guest_time_update(), the guest's pvclock frequency is calculated
by first scaling the host frequency in kHz and then multiplying by 1000 to
get Hz:

    tgt_tsc_khz = kvm_scale_tsc(tgt_tsc_khz, v->arch.l1_tsc_scaling_ratio);
    tgt_tsc_khz = tgt_tsc_khz ? : 1;
    ...
    kvm_get_time_scale(NSEC_PER_SEC, tgt_tsc_khz * 1000LL, ...);

This truncates up to 999 Hz of precision. The new calculation here scales
the Hz directly, resulting in a slightly larger tsc_hz and correspondingly
smaller multiplier than the vCPU's pvclock_tsc_mul.

Because the host uses this smaller multiplier in get_kvmclock(), it will
calculate a slightly smaller elapsed time than the guest has internally
observed. Upon live migration, reading this lagging clock via KVM_GET_CLOCK
could cause the guest's monotonic time to jump backwards when it resumes.

Would it be better to match the guest's exact computation, or update the
guest's calculation to match this higher precision?

> +		if (tsc_hz)
> +			kvm_get_time_scale(NSEC_PER_SEC, tsc_hz,
> +					   &ka->master_tsc_shift,
> +					   &ka->master_tsc_mul);
> +		else
> +			ka->use_master_clock = false;
> +	}
> +
>  	vclock_mode = pvclock_gtod_data.clock.vclock_mode;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608145455.89187-1-dwmw2@infradead.org?part=12