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

[Sean Christopherson]

On Mon, Apr 27, 2026, Yosry Ahmed wrote:
> > We can have our cake and eat it too.  Add svm_pmu_handle_nested_transition(),
> > but then also rename and rework reprogram_counters() to support both deferred and
> > synchronous operation, e.g. something like so:
> >
> > ---
> > static inline void __kvm_pmu_reprogram_counters(struct kvm_pmu *pmu, u64 diff,
> 
> I don't like 'diff', I think just 'unsigned long *bitmap' and pass a

Hard no.  I agree @diff is a weird name (I was literally just copy+pasting the
existing code), but I _really_ don't like passing a pointer, especially not to
an unsigned long.  The bitmap usage throughout the PMU code is mostly internal
implementation details.  But for what is reprogram_counters(), and what will be
__kvm_pmu_reprogram_counters(), the "counters to reprogram" is very tightly
coupled to the architectural layout of PERF_GLOBAL_CTRL and PEBS_ENABLED.  And
more broadly in the PMU, for the layout of GLOBAL_STATUS_BUFFER_OVF_BIT,
MSR_CORE_PERF_GLOBAL_STATUS, and probably at least one other MSRs.  That all
should be captured in the APIs.

The other reason I don't want to pass a pointer is so that even when the source
_is_ a PMU-internal bitmap, it's super duper obvious that the source bitmap isn't
modified, and that it's operating on a snapshot in time.

> bitmap in here like most PMU code?

FWIW, AFAICT, passing a bitmap as a function argument isn't common at all.  I
only see kvm_for_each_pmc() and kvm_pmu_trigger_event() taking a bitmap.  There
is a lot of bitmap _usage_, but rarely does KVM pass around a bitmap as a function
argument.

> >                                                 bool defer)
> > {
> >         struct kvm_vcpu *vcpu = pmu_to_vcpu(pmu);
> >
> >         lockdep_assert_once(defer || kvm_get_running_vcpu() == vcpu);
> 
> Hmm why do we need this? Why not just pass in a vcpu? All callers have
> the vcpu and it should always be the running vcpu whether we are
> deferring or not.

No preference on my end (I was again largely just copy+pasting).

> >
> >         if (!diff)
> 
> Then this becomes bitmap_empty(bitmap)
> 
> >                 return;
> >
> >         atomic64_or(diff, &pmu->__reprogram_pmi);
> 
> and this unfortunately becomes aatomic64_or(*(s64 *)bitmap,
> &pmu->__reprogram_pmi);
> 
> >
> >         if (defer)
> >                 kvm_make_request(KVM_REQ_PMU, vcpu);
> >         else
> >                 kvm_pmu_handle_event(pmu_to_vcpu(pmu));
> > }
> >
> > static inline void kvm_pmu_reprogram_counters(struct kvm_pmu *pmu, u64 diff)
> > {
> >         __kvm_pmu_reprogram_counters(pmu, diff, true);
> > }
> > ---
> >
> > and then have SVM code pass in the reprogram_on_nested_transition or whatever.