Re: [PATCH v4 3/6] KVM: x86/pmu: Disable counters based on Host-Only/Guest-Only bits in SVM

[Sean Christopherson <seanjc@google.com>]

On Mon, Apr 27, 2026, Yosry Ahmed wrote:
> On Mon, Apr 27, 2026 at 12:54 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Mon, Apr 27, 2026, Yosry Ahmed wrote:
> > > > static inline void __kvm_pmu_reprogram_counters(struct kvm_pmu *pmu, u64 diff,
> > > >                                                 bool defer)
> > > > {
> > > >         struct kvm_vcpu *vcpu = pmu_to_vcpu(pmu);
> > > >
> > > >         lockdep_assert_once(defer || kvm_get_running_vcpu() == vcpu);
> > > >
> > > >         if (!diff)
> > > >                 return;
> > > >
> > > >         atomic64_or(diff, &pmu->__reprogram_pmi);
> > > >
> > > >         if (defer)
> > > >                 kvm_make_request(KVM_REQ_PMU, vcpu);
> > > >         else
> > > >                 kvm_pmu_handle_event(pmu_to_vcpu(pmu));
> > > > }
> > >
> > > I like that the KVM PMU code is now presenting a generic API to
> > > reprogram counters rather than handling nested transitions, even
> > > though reprogram_on_nested_transition fits better semantically in
> > > kvm_pmu (than svm_nested_state).
> > >
> > > I do have a few questions:
> > >
> > > 1. Do we want to do all of the work in kvm_pmu_handle_event() on every
> > > nested transition (rather than just reprogram counters)? Genuinely
> > > asking as I am not sure if the rest of it is significant.
> >
> > Yes, we have to for correctness.  And somewhat sneakily, it's not that as much
> > work as it might seem at first glance because the Host/Guest stuff is limited to
> > the mediated PMU.  Specifically, pmu->need_cleanup will never be true and so the
> > heavy-ish kvm_pmu_cleanup() will never be invoked.
> >
> > As for correctness, we either need to run through this code:
> >
> >         kvm_for_each_pmc(pmu, pmc, bit, bitmap)
> >                 kvm_pmu_recalc_pmc_emulation(pmu, pmc);
> >
> > or pend a KVM_REQ_PMU so that it's done before re-entering the guest, so that
> > KVM does the right thing when skipping/emulating guest instructions.  That flow
> > is relatively cheap, so I don't see any reason to defer it.
> 
> As a micro-optimization, should kvm_pmu_handle_event() clear KVM_REQ_PMU?

I vote no.  The odds of introducing a race, now or in the future, far outweigh
the benefits.

> > > 2. This approach will reprogram all counters that need it on nested
> > > transitions. In my proposed approach above, I only iterate over
> > > counters in reprogram_on_nested_transition and reprogram them. Do you
> > > think it matters? I guess if other counters need reprogramming we'll
> > > probably do it in kvm_pmu_handle_event() before running the vCPU
> > > anyway,
> >
> > Correct.  KVM has to do the work before the next VMRUN, all we're doing is
> > completing the work earlier than is strictly necessary.
> >
> > > but then we're repeating the work here?
> >
> > No, it's not repeated.  That's why I want to callkvm_pmu_handle_event(): it
> > updates pmu->reprogram_pmi to clear bits for PMCs that are successfully reprogrammed.
> 
> Yeah kvm_pmu_cleanup() is the only thing that could be done, I didn't
> know that doesn't apply to the mediated PMU.

It's a less-than-awesome name.  It's a flag that says "go ahead and release
perf_events that haven't been used for an entire time slice".  I.e. it's garbage
collection for the legacy PMU.