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

[Yosry Ahmed]

On Mon, Apr 27, 2026 at 12:54 PM Sean Christopherson <seanjc@xxxxxxxxxx> 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?

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

>
> > 3. In this world we still keep the mediated_reprogram_counter() callback, right?
>
> Weren't we planning on a callback that would take the diff of counters?  I.e. one
> callback per kvm_pmu_handle_event(), not one callback per PMC?

Yeah I initially dismissed this because it cannot be done with the
implementation I proposed above, as we don't always go through
kvm_pmu_handle_event(). But with your proposed implementation we can
have a single callback in kvm_pmu_handle_event().

Thank you!