Re: [PATCH v5 21/34] KVM: x86: Allow KVM master clock mode when TSCs are offset from each other

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Guest pvclocks are not updated when all vCPUs re-synchronize their TSC offsets, permanently depriving the guest of the PVCLOCK_TSC_STABLE_BIT optimization.
--

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

KVM: x86: Track frequency matching separately from offset matching

Previously, a guest writing different TSC values on different vCPUs could
force KVM out of master clock mode. This patch separates frequency tracking
from offset tracking, allowing master clock to persist despite offset
mismatches, while maintaining PVCLOCK_TSC_STABLE_BIT semantics.

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index ac66f8e7116f..86c30be4c5d2 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2647,14 +2647,37 @@ static void kvm_track_tsc_matching(struct kvm_vcpu *vcpu, bool new_generation)

[ ... ]

> +	/*
> +	 * To use the masterclock, the host clocksource must be based on TSC
> +	 * and all vCPUs must have matching TSC *frequency*. Different offsets
> +	 * 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 &&
>  				gtod_is_based_on_tsc(gtod->clock.vclock_mode);

[Severity: Medium]
Will guest pvclocks be correctly updated when the final vCPU synchronizes its
TSC offset to match the others?

Because all_vcpus_matched_tsc is now decoupled from use_master_clock, the
broadcast that restores PVCLOCK_TSC_STABLE_BIT across all vCPUs appears to be
broken, causing a performance regression for guest vDSO time reads.

When the final vCPU synchronizes its offset, new_generation evaluates to false
and use_master_clock remains true.

Therefore, the existing condition further down in kvm_track_tsc_matching():

if ((ka->use_master_clock && new_generation) ||
    (ka->use_master_clock != use_master_clock))
        kvm_make_request(KVM_REQ_MASTERCLOCK_UPDATE, vcpu);

will fail to trigger the KVM_REQ_MASTERCLOCK_UPDATE request.

Other vCPUs will not get the broadcast to run kvm_guest_time_update() and will
be permanently deprived of the PVCLOCK_TSC_STABLE_BIT optimization until an
unrelated event forces a global clock update.

[ ... ]

> @@ -3527,7 +3566,7 @@ int kvm_guest_time_update(struct kvm_vcpu *v)
>  
>  	/* If the host uses TSC clocksource, then it is stable */
>  	hv_clock.flags = 0;
> -	if (use_master_clock)
> +	if (use_master_clock && ka->all_vcpus_matched_tsc)
>  		hv_clock.flags |= PVCLOCK_TSC_STABLE_BIT;

[Severity: Medium]
Without the KVM_REQ_MASTERCLOCK_UPDATE broadcast mentioned above, will other
vCPUs ever reach this code to restore their PVCLOCK_TSC_STABLE_BIT flag once
ka->all_vcpus_matched_tsc becomes true again?

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