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.

E.g. if tsc_timestamp is set to a per-VM value, and vCPU0's offset matches the
effectively offset, but vCPU1's offset does not, then vCPU1 could see time that
is either far in the past or far in the future when doing 
__pvclock_clocksource_read().
If vCPU1 computes time that's in the future, the guest would see a potentially
massive jump forward in time.

And after that, or if vCPU1 computes time that's in the past, if the "future"
vCPU stopped running, the "past" vCPU would see time stop as last_value would be
ahead of the "past" vCPU's current time for a very long while.

        if ((valid_flags & PVCLOCK_TSC_STABLE_BIT) &&
                (flags & PVCLOCK_TSC_STABLE_BIT))
                return ret;

        /*
         * Assumption here is that last_value, a global accumulator, always goes
         * forward. If we are less than that, we should not be much smaller.
         * We assume there is an error margin we're inside, and then the 
correction
         * does not sacrifice accuracy.
         *
         * For reads: global may have changed between test and return,
         * but this means someone else updated poked the clock at a later time.
         * We just need to make sure we are not seeing a backwards event.
         *
         * For updates: last_value = ret is not enough, since two vcpus could be
         * updating at the same time, and one of them could be slightly behind,
         * making the assumption that last_value always go forward fail to hold.
         */
        last = raw_atomic64_read(&last_value);
        do {
                if (ret <= last)
                        return last;
        } while (!raw_atomic64_try_cmpxchg(&last_value, &last, ret));


> 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.

> again, where wallclock should *only* be used for setting the TSC, and even
> then *only* for a live *migration* to a different host, not a live *update*
> via kexec/KHO on the same host, where the TSC should be restored as an offset
> from the host TSC.



Reply via email to