Re: [PATCH v7 17/36] KVM: x86: Allow KVM master clock mode when TSCs are offset from each other

[Sean Christopherson <seanjc@google.com>]

On Tue, Aug 11, 2026, David Woodhouse wrote:
> On Tue, 2026-08-11 at 07:33 -0700, Sean Christopherson wrote:
> > 
> > Actually, why are KVM_{G,S}ET_CLOCK_GUEST vCPU-scoped?  Per the documentation,
> > the API "Sets the KVM clock (for the whole VM) in terms of the vCPU TSC".  If
> > the APIs are VM-scoped instead of vCPU-scoped, then KVM can simply save/restore
> > what's in the per-VM masterclock state, no?
> 
> They're vCPU-scoped because they need to be tied to a guest TSC (on
> live migration, neither ka->master_cycle_now nor ka->master_kernel_ns
> are useful — those are the "per-VM masterclock state").

But master clock is also tied to guest TSC.

> Theoretically, guest TSCs can be different on each vCPU (different
> offset, different *rate* even. Not that we allow KVM_[GS]ET_CLOCK_GUEST
> at different rates, I concede).

Sure, but not masterclock, and if we're saying that KVM_[GS]ET_CLOCK_GUEST is
for migrating masterclock state, then as you concede, vCPUs with TSCs at different
frequencies is completely out of scope.

> So they operate in the context of a given vCPU, and *its* TSC.

Yes, but KVM_[GS]ET_CLOCK_GUEST aren't saving/restoring vCPU state, they're
saving/restoring masterclock state, which is VM-scoped.  What I don't like about
the proposed uAPI is that it implicitly consumes state, from an arbitrary vCPU,
that KVM very explicitly tracks in masterclock.  And AFAICT, there's zero reason
to do so.

E.g. as a strawman, I would expect something like this to migrate masterclock
state (deliberately avoiding "master" in the uAPI, because checkpatch is already
screaming too much).  I didn't try too hard to get the math right, I just wanted
to highlight that all the state needed to restore the masterclock is available
in the masterclock (which seems comically obvious when I type it out).

struct kvm_pvclock {
	__u64 tsc_timestamp;
	__u64 tsc_scaling_ratio;
	__u64 tsc_offset;
	__u64 system_time;
	__u32 tsc_to_system_mul;
	__s8  tsc_shift;
	__u8  pad0;
	__u16 pad1;
	__u32 pad2;
};

#define KVM_SET_PVCLOCK		_IOW(KVMIO, 0xd6, struct kvm_pvclock)
#define KVM_GET_PVCLOCK		_IOR(KVMIO, 0xd7, struct kvm_pvclock)

static int kvm_vcpu_ioctl_set_pvclock(struct kvm *kvm, void __user *argp)
{
	struct kvm_pvclock user_hv_clock;
	struct kvm_arch *ka = &kvm->arch;
	u64 curr_tsc_hz, user_tsc_hz;
	u64 user_clk_ns;
	u64 guest_tsc;
	int rc = 0;

	if (copy_from_user(&user_hv_clock, argp, sizeof(user_hv_clock)))
		return -EFAULT;

	if (user_hv_clock.pad0 || user_hv_clock.pad1 || user_hv_clock.pad2)
		return -EINVAL;

	if (!user_hv_clock.tsc_scaling_ratio || !user_hv_clock.tsc_to_system_mul)
		return -EINVAL;

	if (user_hv_clock.tsc_shift < -31 || user_hv_clock.tsc_shift > 31)
		return -EINVAL;

	user_tsc_hz = hvclock_to_hz(user_hv_clock.tsc_to_system_mul,
				    user_hv_clock.tsc_shift);

	kvm_hv_request_tsc_page_update(kvm);

	/*
	 * kvm_start_pvclock_update() takes tsc_write_lock and opens
	 * the pvclock seqcount; kvm_end_pvclock_update() closes both.
	 * All clock state modifications between them are atomic with
	 * respect to readers in kvm_guest_time_update().
	 */
	kvm_start_pvclock_update(kvm);
	pvclock_update_vm_gtod_copy(kvm);

	if (!ka->use_master_clock) {
		rc = -ENODATA;
		goto out;
	}

	curr_tsc_hz = (u64)get_cpu_tsc_khz() * HZ_PER_KHZ;
	if (unlikely(curr_tsc_hz == 0)) {
		rc = -EBUSY;
		goto out;
	}

	if (kvm_caps.has_tsc_control)
		curr_tsc_hz = kvm_scale_tsc(curr_tsc_hz,
					    user_hv_clock.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 the existing pvclock information, and by extension
	 * all vCPUs' effective TSC frequenies.  Allow a discrepancy of 1 kHz
	 * either way since independently calibrated hosts will not measure
	 * precisely the same value even for the same nominal frequency.
	 */
	if (user_tsc_hz < curr_tsc_hz - 1000 ||
	    user_tsc_hz > curr_tsc_hz + 1000) {
		rc = -ERANGE;
		goto out;
	}

	/*
	 * Calculate the guest TSC at the new reference point, and the
	 * corresponding KVM clock value according to user_hv_clock.
	 * Adjust kvmclock_offset so both definitions agree.
	 */
	guest_tsc = user_hv_clock.tsc_offset +
		    kvm_scale_tsc(user_hv_clock.system_time,
				  user_hv_clock.tsc_scaling_ratio);

	if (guest_tsc != user_hv_clock.tsc_timestamp +- ???) {
		rc = -EINVAL;
		goto out;
	}

	<fill in masterclock>

out:
	kvm_end_pvclock_update(kvm);
	return rc;
}

> And I think I'm going to defend that 'theoretical they can be
> different', because I *would* like to eliminate the ways that a *guest*
> can force non-masterclock mode, and that does mean allowing the offset-
> TSC case.
> 
> FWIW in my local tree I've just extended the pvclock_migration_test to
> test precisely the thing you were concerned about: three vCPUs with
> divergent TSC offsets, migrated by setting each vCPU's TSC and then
> invoking KVM_SET_CLOCK_GUEST once, through vCPU0. 

I wasn't actually concerned about migration, I was concerned about time going
backwards from the guest's perspective.