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

Sean Christopherson
2026-04-27T23:53:43Z

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.