On Tue, 2026-08-11 at 11:41 -0700, Sean Christopherson wrote: > > > > 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.
Apologies, I don't think I was paying enough attention; mostly my mind
was in the GPC/RCU thing. The system_time field I picked out was
actually the *guest* nanoseconds-since-boot, and there's nothing wrong
with that per se. I was wrong to pick on that specifically.
I think I still stand by my gut reaction, but it deserves a more
coherent analysis...
You had:
struct kvm_pvclock {
__u64 tsc_timestamp; // GUEST — guest TSC cycles at snapshot
__u64 tsc_scaling_ratio; // HOST — guest cycles per *host* cycle
__u64 tsc_offset; // HOST — guest TSC minus scaled *host* TSC
__u64 system_time; // GUEST — guest kvmclock ns at snapshot
__u32 tsc_to_system_mul; // GUEST — guest ns per guest cycle (with
shift)
__s8 tsc_shift; // GUEST — ditto
__u8 pad0;
__u16 pad1;
__u32 pad2;
};
There are two kinds of fields in that — the ones I've annotated as
GUEST vs. HOST. This approach is conflating two things: Setting the
guest TSC, and setting the guest's kvmclock.
The guest part gives the y=m(x−x′)+c relationship of the guest TSC to
its kvmclock. It means exactly the same thing on any host, and it's
precisely what Jack's KVM_[GS]ET_CLOCK_GUEST already passes.
The other two fields are host-relative: guest-cycles-per-host-cycle,
and guest-TSC-minus-scaled-host-TSC. You could use them to set the
guest TSC (its offset *and* its rate)... but you don't. You just kind
of assume this redundant information is true — which it can't possibly
be if this is a migration to a new host — and trust what userspace
provides in precisely the two places where Jack's code was using actual
true data from a vCPU.
It's better without the redundant information.
We *know* how to set the guest TSC. We either restore the *offset* for
a live update, or it's the *one* time we're ever allowed to use
wallclock time, in the case of a live migration¹. That part lives
nowhere near here.
All we need is to express the relationship between guest TSC and the
kvmclock. And that's precisely what the existing pvclock_vcpu_time_info
ABI structure does.
You are right that with the current implementation, the *precise*
bitwise values of those fields might change (you can change the x' and
the c in the equation and have the same line). That's because there's
redundancy in the pvclock_vcpu_time_info structure already. If I
understand correctly, that's where your concern about asymmetry between
GET and SET comes from? I suppose we *could* make a new structure which
doesn't have the redundancy, but it seemed better to use the existing
ABI. (And has the added bonus that userspace can fish it out of the
guest memory even when migrated from a kernel that didn't have this
support. Yes, we've done that).
And again, I don't *like* it when KVM refreshes masterclock and changes
both the x' and the c in the equation... I *want* kvmclock to remain
bit-constant for the whole *lifetime* of a guest, even the redundant
parts. So I'd actually *prefer* our long-term userspace ABI to
accommodate that and not eliminate the "redundancy".
¹ (And with the stuff I've done on exposing the raw TSC values to
userspace through PTP, even *migrations* can be done better by
letting userspace calculate then set an offset).
smime.p7s
Description: S/MIME cryptographic signature
