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

[Lei Chen]

Hi Sean,

Thanks for your review.

On Wed, Apr 8, 2026 at 2:02 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Tue, Apr 07, 2026, Lei Chen wrote:
> > commit 446fcce2a52b ("Revert "x86: kvm: rate-limit global clock updates"")
> > dropped the rate limiting for KVM_REQ_GLOBAL_CLOCK_UPDATE.
> >
> > As a result, kvm_arch_vcpu_load() can queue global clock update requests
> > every time a vCPU is scheduled when the master clock is disabled or when
> > the vCPU is loaded for the first time.
> >
> > Restore the throttling with a per-VM ratelimit state and gate
> > KVM_REQ_GLOBAL_CLOCK_UPDATE through __ratelimit(), so frequent vCPU
> > scheduling does not generate a steady stream of redundant clock update
> > requests.
> >
> > Fixes: 446fcce2a52b ("Revert "x86: kvm: rate-limit global clock updates"")
> > Signed-off-by: Lei Chen <lei.chen@xxxxxxxxxx>
> > Reported-by: Jaroslav Pulchart <jaroslav.pulchart@xxxxxxxxxxxx>
> > Closes: https://lore.kernel.org/all/CAK8fFZ5gY8_Mw2A=iZVFNVKQNrXQzVsn-HTd+Me9K6ZfmdgA+Q@xxxxxxxxxxxxxx/
> > ---
> >  arch/x86/include/asm/kvm_host.h | 2 ++
> >  arch/x86/kvm/x86.c              | 5 ++++-
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 5a3bfa293e8b..6d3d3f19af01 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1453,6 +1453,8 @@ struct kvm_arch {
> >       bool use_master_clock;
> >       u64 master_kernel_ns;
> >       u64 master_cycle_now;
> > +     /* how often to make KVM_REQ_GLOBAL_CLOCK_UPDATE on vcpu sched*/
>
> Eh, I would just omit this comment.  If we want to document the ratelimit,
> the function comment above kvm_gen_kvmclock_update() is the best place for it.
>
OK, I'll remove this comment in the next patch.

> > +     struct ratelimit_state kvmclock_update_rs;
> >
> >  #ifdef CONFIG_KVM_HYPERV
> >       struct kvm_hv hyperv;
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 63afdb6bb078..4a37027cc0b8 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -5211,7 +5211,9 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> >                * 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 (__ratelimit(&vcpu->kvm->arch.kvmclock_update_rs))
> > +                             kvm_make_request(KVM_REQ_GLOBAL_CLOCK_UPDATE, vcpu);
>
> To maintain pre-revert compatibility, where KVM did this:
>
>         kvm_make_request(KVM_REQ_CLOCK_UPDATE, v);
>         schedule_delayed_work(&kvm->arch.kvmclock_update_work,
>                               KVMCLOCK_UPDATE_DELAY);
>
>
> the ratelimit should be on blasting KVM_REQ_CLOCK_UPDATE to *all* vCPUs, but KVM
> should still trigger KVM_REQ_CLOCK_UPDATE on the initiating vCPU so that the
> immediate update goes through.
>
Yes, this is a mistake, I'll fix it in the next patch.

> That will also apply the ratelimiting to kvm_write_system_time(), though if a
> guest is changing system time that fast, it probably has other issues :-)
>

This patch does not impact kvm_write_system_time, which works as follows:

kvm_write_system_time
    kvm_make_request(KVM_REQ_GLOBAL_CLOCK_UPDATE, vcpu);


vcpu_enter_guest
    if (kvm_check_request(KVM_REQ_GLOBAL_CLOCK_UPDATE, vcpu))
         kvm_gen_kvmclock_update(vcpu);
               kvm_for_each_vcpu
                   kvm_vcpu_kick(vcpu);

This patch limits the rate of GLOBAL_CLOCK_UPDATE only in kvm_arch_vcpu_load.

Maybe I missed something?

Best Regards,
Lei Chen