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.
