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 10:28 -0700, Sean Christopherson wrote:
> > On Tue, Aug 11, 2026, David Woodhouse wrote:
> > > On Tue, 2026-08-11 at 09:40 -0700, Sean Christopherson wrote:
> > > > 
> > > > > 
> > > > > 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.
> > > 
> > > But KVM_[SG]ET_CLOCK_GUEST is *purely* for migration. 
> > 
> > Huh?  I raised my concern in the context of "Allow KVM master clock mode when
> > TSCs are offset from each other", and AFAICT, nothing ensures that won't cause
> > problems.
> > 
> > Aaah, it clears PVCLOCK_TSC_STABLE_BIT and relies on the guest to clean up the
> > mess.  So the guest won't see time go backwards, but it could see time stop for
> > an extended duration, or jump forward.
> 
> It shouldn't. Each vCPU gets its *own* pvclock structure, tailored to
> *its* offset. They should all see *identical* results.

OMG, I hate this code.  After literally hours of staring at this, and even typing
up a lengthy example of why guest time would go off the rails, I finally spotted
that l1_tsc_offset is accounted for by the call to kvm_read_l1_tsc().  FML.

Thanks for being patient and not flaming me too much :-)

> But yeah, if the guest does that then we can't set the
> PVCLOCK_TSC_STABLE_BIT.
> > 
> > > And your variant just added a dependency on wallclock time back into it
> > 
> > Can you elaborate?  I'm guessing I don't entirely understand what you mean by
> > wallclock time.
> 
> The system_time field? The unspecified might-be-UTC-might-have-leap-seconds one :)

Ok, I think I finally understand the goal.  I got turned around by the combination
of the name SET_CLOCK_GUEST and the full pvclock structure being passed to the
guest.  I was expecting SET_CLOCK_GUEST to literally set the entire clock, e.g.
mul+shift, timestamp, etc.

But all of that metadata is just a means to an end: the one and only goal is to
calculate the per-VM kvmclock_offset for the "new" host's TSC+time snapshot, by
computing the nanoseconds delta for the new snapshot as if it the guest observed
the TSC while running on the old host.

And that is done in the kernel instead of in userspace to minimize the amount of
slop introduced due to delay between taking the snapshot and computing the offset.
And for similar reasons, it's undesirable for userspace to redo the GET on the
target to compute the explicit offset, because there would be a massive TOCTOU
issue, e.g. if KVM took a new masterclock snapshot between GET and SET.

After working through that, I feel quite strongly that we should have asymmetric
names for the GET vs. SET flows, because the actually functionality is also very
asymmetric.  And setting a field, and only that field, that's doesn't have a
direct association in the userspace payload is very surprising when the name of
the ioctl suggests a restoration of the entire payload.

E.g. KVM_GET_REFERENCE_PVCLOCK and then KVM_SET_REFERENCE_PVCLOCK_OFFSET?  Or
maybe UPDATE, CALCULATE, REFRESH, or COMPUTE instead of SET?  I think I'd vote
for SET even though it's convoluted, because pretty much everyone associates SET
with the restore side of save/restore.