Re: [PATCH v13 16/26] KVM: x86: Add KVM_[GS]ET_CLOCK_GUEST for accurate KVM clock migration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] KVM: x86: stale masterclock state in
  kvm_vcpu_ioctl_get_clock_guest()
--

commit 06987440f414497c2402ac529d8fa56884dbca59
Author: Jack Allister <jalliste@amazon.com>

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 per-vCPU
ioctls to save and restore the PV clock parameters in terms of the guest
TSC across migration.

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 743da71cd84d7..16383c71131d0 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
[ ... ]
> @@ -3941,6 +3944,218 @@ 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;
[ ... ]
> +	if (kvm_test_request(KVM_REQ_MASTERCLOCK_UPDATE, v))
> +		kvm_update_masterclock(v->kvm, v);

[Severity: Medium]
Can kvm_vcpu_ioctl_get_clock_guest() read stale VM-wide masterclock state
when KVM_REQ_MASTERCLOCK_UPDATE is queued on a different vCPU?

While host resume and gtod updates broadcast KVM_REQ_MASTERCLOCK_UPDATE
to all vCPUs, per-vCPU TSC and frequency synchronization paths queue it
only on the vCPU being modified:

arch/x86/kvm/x86.c:kvm_track_tsc_matching() {
    ...
	if ((ka->use_master_clock && update_mclock) ||
	    (ka->use_master_clock != use_master_clock))
		kvm_make_request(KVM_REQ_MASTERCLOCK_UPDATE, vcpu);
}

If userspace changes vCPU 1's TSC frequency via KVM_SET_TSC_KHZ so that
ka->all_vcpus_matched_freq becomes false, KVM_REQ_MASTERCLOCK_UPDATE is
set only on vCPU 1 while ka->use_master_clock remains true.  Calling
KVM_GET_CLOCK_GUEST on vCPU 0 before vCPU 1 enters the guest sees no
pending request on vCPU 0, skips kvm_update_masterclock(), and returns
the stale masterclock parameters instead of -ENODATA (or conversely
returns -ENODATA after vCPU 1 restores frequency matching).

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