Re: [PATCH v2] KVM: x86: Rate-limit global clock updates on vCPU load

[Sean Christopherson <seanjc@google.com>]

On Fri, Jun 26, 2026, David Woodhouse wrote:
> On Thu, 2026-04-09 at 22:22 +0800, Lei Chen wrote:
> > 
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -5210,8 +5210,13 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> >  		 * On a host with synchronized TSC, there is no need to update
> >  		 * kvmclock on vcpu->cpu migration
> >  		 */
> > -		if (!vcpu->kvm->arch.use_master_clock || vcpu->cpu == -1)
> > -			kvm_make_request(KVM_REQ_GLOBAL_CLOCK_UPDATE, vcpu);
> > +		if (!vcpu->kvm->arch.use_master_clock || vcpu->cpu == -1) {
> > +			if (__ratelimit(&vcpu->kvm->arch.kvmclock_update_rs))
> > +				kvm_make_request(KVM_REQ_GLOBAL_CLOCK_UPDATE, vcpu);
> > +			else
> > +				kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
> > +		}
> > +
> >  		if (vcpu->cpu != cpu)
> >  			kvm_make_request(KVM_REQ_MIGRATE_TIMER, vcpu);
> >  		vcpu->cpu = cpu;
> 
> I don't like this.

I don't think anyone likes this :-)

> We do the GLOBAL_CLOCK_UPDATE in non-masterclock mode because
> theoretically, a *failure* to do so could cause the guest to observe
> non-monotonicity across vCPUs.
> 
> With this rate-limiting, we *sometimes* protect against that non-
> monotonicity. Why even bother? Why not just rip it out completely, in
> that case? AIUI the guest should cope with anyway if its
> PVCLOCK_TSC_STABLE_BIT isn't set, and enforce monotonicity for itself
> side in __pvclock_clocksource_read() ? 

Because we needed a fix somewhat urgently, and I didn't want to risk more breakage
by going with an aggressive change.

> But also, I would *love* to kill non-masterclock mode completely.

That's not feasible until KVM drops support for hardware that, per Linux standards,
isn't _that_ old, is it?

> In this nested case, the L1 TSCs *are* actually in sync; it just don't
> have the +invtsc CPUID to *promise* that they will be.
> 
> Could we find a way for L1 KVM to use masterclock mode for its nested
> L2 VMs even in this case? Could we make it use PVCLOCK_TSC_STABLE_BIT
> to enable masterclock mode, instead?
> 
> At at the very least, could we find a way for L1 to trigger the
> GLOBAL_CLOCK_UPDATE only if an *actual* discrepancy in its TSCs is
> found? (Or, if vdso mode is not VDSO_CLOCKMODE_PVCLOCK?)

Or just advertise invtsc?  I appreciate the desire to eliminate mostly-useless
overhead like this, but at some point doesn't this boil down to "don't mess up
your setup"?