From: "Chen, Zide" <zide.chen@intel.com>
To: Jim Mattson <jmattson@google.com>
Cc: Sean Christopherson <seanjc@google.com>,
Paolo Bonzini <pbonzini@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
Mingwei Zhang <mizhang@google.com>,
Das Sandipan <Sandipan.Das@amd.com>,
Shukla Manali <Manali.Shukla@amd.com>,
Dapeng Mi <dapeng1.mi@linux.intel.com>,
Falcon Thomas <thomas.falcon@intel.com>,
Xudong Hao <xudong.hao@intel.com>
Subject: Re: [PATCH 08/15] KVM: x86/pmu: Switch to bitmask-based KVM PMU capabilities
Date: Fri, 24 Jul 2026 10:15:15 -0500 [thread overview]
Message-ID: <65026581-daf9-45b5-8104-a4845544ebe7@intel.com> (raw)
In-Reply-To: <CALMp9eTYWFvB-ji31Mu=vzJo2HFPL0jZFAiXVGpJx-H8S2mHOw@mail.gmail.com>
On 7/23/2026 4:47 PM, Jim Mattson wrote:
> On Tue, Jul 7, 2026 at 11:44 AM Zide Chen <zide.chen@intel.com> wrote:
>>
>> From: Dapeng Mi <dapeng1.mi@linux.intel.com>
>>
>> Intel platforms support non-contiguous fixed counters via CPUID.0AH:ECX
>> starting with PerfMon v5, and support non-contiguous GP counters
>> through the Architectural PerfMon Extension (CPUID leaf 23H).
>>
>> struct x86_pmu_capability now exposes {,fixed_}cntr_mask64 bitmaps,
>> which may contain sparse bits representing non-contiguous counters.
>> Switch KVM's kvm_host_pmu and kvm_pmu_cap consumers over to the new
>> bitmask fields.
>>
>> CPUID.0AH:EAX[15:8] and CPUID.0AH:EDX[4:0] enumerate only contiguous
>> counters. Derive these values from kvm_pmu_cap.{,fixed_}cntr_mask64 as
>> the number of consecutive counters starting at index 0.
>>
>> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
>> Co-developed-by: Zide Chen <zide.chen@intel.com>
>> Signed-off-by: Zide Chen <zide.chen@intel.com>
>> ---
>> arch/x86/kvm/cpuid.c | 14 +++++++++++---
>> arch/x86/kvm/msrs.c | 12 ++++++------
>> arch/x86/kvm/pmu.c | 34 +++++++++++-----------------------
>> arch/x86/kvm/svm/pmu.c | 2 +-
>> arch/x86/kvm/svm/svm.c | 9 +++++----
>> arch/x86/kvm/vmx/pmu_intel.c | 7 ++++---
>> arch/x86/kvm/vmx/vmx.c | 7 +++++--
>> 7 files changed, 43 insertions(+), 42 deletions(-)
>>
>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>> index 2698fa42cd97..151a4794f834 100644
>> --- a/arch/x86/kvm/cpuid.c
>> +++ b/arch/x86/kvm/cpuid.c
>> @@ -1514,10 +1514,18 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
>> }
>>
>> eax.split.version_id = kvm_pmu_cap.version;
>> - eax.split.num_counters = kvm_pmu_cap.num_counters_gp;
>> +
>> + /* Contiguous GP counters only. */
>> + eax.split.num_counters =
>> + find_first_zero_bit(kvm_pmu_cap.cntr_mask,
>> + KVM_MAX_NR_GP_COUNTERS);
>> eax.split.bit_width = kvm_pmu_cap.bit_width_gp;
>> eax.split.mask_length = kvm_pmu_cap.events_mask_len;
>> - edx.split.num_counters_fixed = kvm_pmu_cap.num_counters_fixed;
>> +
>> + /* Contiguous fixed counters only. */
>> + edx.split.num_counters_fixed =
>> + find_first_zero_bit(kvm_pmu_cap.fixed_cntr_mask,
>> + KVM_MAX_NR_FIXED_COUNTERS);
>> edx.split.bit_width_fixed = kvm_pmu_cap.bit_width_fixed;
>>
>> if (kvm_pmu_cap.version)
>> @@ -1882,7 +1890,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
>>
>> cpuid_entry_override(entry, CPUID_8000_0022_EAX);
>>
>> - ebx.split.num_core_pmc = kvm_pmu_cap.num_counters_gp;
>> + ebx.split.num_core_pmc = hweight64(kvm_pmu_cap.cntr_mask64);
>> entry->ebx = ebx.full;
>> break;
>> }
>> diff --git a/arch/x86/kvm/msrs.c b/arch/x86/kvm/msrs.c
>> index c751a8dbd45d..7524d019f1be 100644
>> --- a/arch/x86/kvm/msrs.c
>> +++ b/arch/x86/kvm/msrs.c
>> @@ -2631,20 +2631,20 @@ static void kvm_probe_msr_to_save(u32 msr_index)
>> break;
>> case MSR_ARCH_PERFMON_PERFCTR0 ...
>> MSR_ARCH_PERFMON_PERFCTR0 + KVM_MAX_NR_GP_COUNTERS - 1:
>> - if (msr_index - MSR_ARCH_PERFMON_PERFCTR0 >=
>> - kvm_pmu_cap.num_counters_gp)
>> + if (!(BIT_ULL(msr_index - MSR_ARCH_PERFMON_PERFCTR0) &
>> + kvm_pmu_cap.cntr_mask64))
>> return;
>> break;
>> case MSR_ARCH_PERFMON_EVENTSEL0 ...
>> MSR_ARCH_PERFMON_EVENTSEL0 + KVM_MAX_NR_GP_COUNTERS - 1:
>> - if (msr_index - MSR_ARCH_PERFMON_EVENTSEL0 >=
>> - kvm_pmu_cap.num_counters_gp)
>> + if (!(BIT_ULL(msr_index - MSR_ARCH_PERFMON_EVENTSEL0) &
>> + kvm_pmu_cap.cntr_mask64))
>> return;
>> break;
>> case MSR_ARCH_PERFMON_FIXED_CTR0 ...
>> MSR_ARCH_PERFMON_FIXED_CTR0 + KVM_MAX_NR_FIXED_COUNTERS - 1:
>> - if (msr_index - MSR_ARCH_PERFMON_FIXED_CTR0 >=
>> - kvm_pmu_cap.num_counters_fixed)
>> + if (!(BIT_ULL(msr_index - MSR_ARCH_PERFMON_FIXED_CTR0) &
>> + kvm_pmu_cap.fixed_cntr_mask64))
>> return;
>> break;
>> case MSR_AMD64_PERF_CNTR_GLOBAL_CTL:
>> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
>> index 3647ce3f0e3f..bc2ca60114e9 100644
>> --- a/arch/x86/kvm/pmu.c
>> +++ b/arch/x86/kvm/pmu.c
>> @@ -20,7 +20,6 @@
>> #include <asm/perf_event.h>
>> #include <asm/cpu_device_id.h>
>> #include "x86.h"
>> -#include "cpuid.h"
>> #include "lapic.h"
>> #include "pmu.h"
>>
>> @@ -136,8 +135,6 @@ void kvm_init_pmu_capability(struct kvm_pmu_ops *pmu_ops)
>> {
>> bool is_intel = boot_cpu_data.x86_vendor == X86_VENDOR_INTEL;
>> int min_nr_gp_ctrs = pmu_ops->MIN_NR_GP_COUNTERS;
>> - union cpuid10_edx edx;
>> - u32 eax, ebx, ecx;
>>
>> /*
>> * Hybrid PMUs don't play nice with virtualization without careful
>> @@ -159,8 +156,8 @@ void kvm_init_pmu_capability(struct kvm_pmu_ops *pmu_ops)
>> * there are a non-zero number of counters, but fewer than what
>> * is architecturally required.
>> */
>> - if (!kvm_host_pmu.num_counters_gp ||
>> - WARN_ON_ONCE(kvm_host_pmu.num_counters_gp < min_nr_gp_ctrs))
>> + if (!kvm_host_pmu.cntr_mask64 ||
>> + WARN_ON_ONCE(hweight64(kvm_host_pmu.cntr_mask64) < min_nr_gp_ctrs))
>> enable_pmu = false;
>> else if (is_intel && !kvm_host_pmu.version)
>> enable_pmu = false;
>> @@ -180,23 +177,14 @@ void kvm_init_pmu_capability(struct kvm_pmu_ops *pmu_ops)
>>
>> memcpy(&kvm_pmu_cap, &kvm_host_pmu, sizeof(kvm_host_pmu));
>> kvm_pmu_cap.version = min(kvm_pmu_cap.version, 2);
>> - kvm_pmu_cap.num_counters_gp = min(kvm_pmu_cap.num_counters_gp,
>> - pmu_ops->MAX_NR_GP_COUNTERS);
>> - kvm_pmu_cap.num_counters_fixed = min(kvm_pmu_cap.num_counters_fixed,
>> - KVM_MAX_NR_FIXED_COUNTERS);
>> + kvm_pmu_cap.cntr_mask64 &=
>> + GENMASK_ULL(pmu_ops->MAX_NR_GP_COUNTERS - 1, 0);
>> + kvm_pmu_cap.fixed_cntr_mask64 &=
>> + GENMASK_ULL(KVM_MAX_NR_FIXED_COUNTERS - 1, 0);
>>
>> - /*
>> - * Currently, KVM doesn't support non-contiguous fixed counters; make
>> - * sure only contiguous ones are retained in kvm_pmu_cap.
>> - */
>> - if (kvm_host_pmu.version >= 5) {
>> - cpuid(0xa, &eax, &ebx, &ecx, &edx.full);
>> - if (kvm_pmu_cap.num_counters_fixed > edx.split.num_counters_fixed)
>> - kvm_pmu_cap.num_counters_fixed = edx.split.num_counters_fixed;
>> - }
>> -
>> - if (!enable_mediated_pmu && kvm_pmu_cap.num_counters_fixed > 3)
>> - kvm_pmu_cap.num_counters_fixed = 3;
>> + /* Legacy vPMU exposes at most 3 fixed counters. */
>> + if (!enable_mediated_pmu)
>> + kvm_pmu_cap.fixed_cntr_mask64 &= GENMASK_ULL(2, 0);
>>
>> kvm_pmu_eventsel.INSTRUCTIONS_RETIRED =
>> perf_get_hw_event_config(PERF_COUNT_HW_INSTRUCTIONS);
>> @@ -796,8 +784,8 @@ static bool kvm_need_any_pmc_intercept(struct kvm_vcpu *vcpu)
>> * KVM's capabilities are constrained based on KVM support, i.e. KVM's
>> * capabilities themselves may be a subset of hardware capabilities.
>> */
>> - return kvm_gp_pmc_mask(pmu) != BIT_ULL(kvm_host_pmu.num_counters_gp) - 1 ||
>> - kvm_fixed_pmc_mask(pmu) != BIT_ULL(kvm_host_pmu.num_counters_fixed) - 1;
>> + return kvm_gp_pmc_mask(pmu) != kvm_host_pmu.cntr_mask64 ||
>> + kvm_fixed_pmc_mask(pmu) != kvm_host_pmu.fixed_cntr_mask64;
>> }
>>
>> bool kvm_need_perf_global_ctrl_intercept(struct kvm_vcpu *vcpu)
>> diff --git a/arch/x86/kvm/svm/pmu.c b/arch/x86/kvm/svm/pmu.c
>> index 02cf960a215a..d519eba518bf 100644
>> --- a/arch/x86/kvm/svm/pmu.c
>> +++ b/arch/x86/kvm/svm/pmu.c
>> @@ -208,7 +208,7 @@ static void amd_pmu_refresh(struct kvm_vcpu *vcpu)
>> }
>>
>> pmu->all_valid_pmc_mask64 = (BIT_ULL(nr_gp_counters) - 1) &
>> - (BIT_ULL(kvm_pmu_cap.num_counters_gp) - 1);
>> + kvm_pmu_cap.cntr_mask64;
>>
>> if (pmu->version > 1) {
>> pmu->global_ctrl_rsvd = ~pmu->all_valid_pmc_mask64;
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> index 3b3e98b6abb6..002cdd074fd7 100644
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
>> @@ -754,6 +754,7 @@ static void svm_recalc_pmu_msr_intercepts(struct kvm_vcpu *vcpu)
>> bool intercept = !kvm_vcpu_has_mediated_pmu(vcpu);
>> struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
>> unsigned long gp_mask = kvm_gp_pmc_mask(pmu);
>> + unsigned long host_only_gp_mask;
>> int i;
>>
>> if (!enable_mediated_pmu)
>> @@ -769,7 +770,8 @@ static void svm_recalc_pmu_msr_intercepts(struct kvm_vcpu *vcpu)
>> svm_set_intercept_for_msr(vcpu, MSR_F15H_PERF_CTR + 2 * i,
>> MSR_TYPE_RW, intercept);
>
> Our internal Sashiko says:
>
> This is a pre-existing issue, but does the legacy MSR intercept need to be
> reverted here as well when gp_mask shrinks?
> When guest PMU capabilities are updated and gp_mask shrinks, the new
> host_only_gp_mask loop correctly reinstates the intercepts for
> MSR_F15H_PERF_CTR. However, looking at the code just above this hunk in
> svm_recalc_pmu_msr_intercepts():
> /* Legacy counters are always available for AMD CPUs with a PMU. */
> for_each_set_bit(i, &gp_mask, AMD64_NUM_COUNTERS)
> svm_set_intercept_for_msr(vcpu, MSR_K7_PERFCTR0 + i,
> MSR_TYPE_RW, intercept);
> There does not seem to be a corresponding loop to reinstate the intercept
> for MSR_K7_PERFCTR0 + i.
> If userspace initially exposes 4 counters, disabling intercepts for
> MSR_K7_PERFCTR0 through MSR_K7_PERFCTR3, and later updates CPUID to expose
> only 2 counters, could the removed legacy counters remain unintercepted?
> This sequence might allow an unprivileged userspace to grant the guest
> direct read/write access to host MSR_K7_PERFCTR MSRs without KVM
> intercepting them.
If gp_mask shrinks after intercepts have been disabled, counters that
are no longer guest-visible will not be revisited by the
for_each_set_bit() loop. Since the AMD counters are contiguous,
restoring intercepts for the cleared bits should address the userspace
reconfiguration case:
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index e9f39c884765..2360865651bc 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -763,6 +763,9 @@ static void svm_recalc_pmu_msr_intercepts(struct
kvm_vcpu *vcpu)
for_each_set_bit(i, &gp_mask, AMD64_NUM_COUNTERS)
svm_set_intercept_for_msr(vcpu, MSR_K7_PERFCTR0 + i,
MSR_TYPE_RW, intercept);
+ for_each_clear_bit_from(i, &gp_mask, AMD64_NUM_COUNTERS)
+ svm_enable_intercept_for_msr(vcpu, MSR_K7_PERFCTR0 + i,
+ MSR_TYPE_RW);
intercept |= !guest_cpu_cap_has(vcpu, X86_FEATURE_PERFCTR_CORE);
kvm_for_each_set_pmc_idx(i, gp_mask, AMD_GP)
next prev parent reply other threads:[~2026-07-24 15:15 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-07 18:33 [PATCH 00/15] KVM: x86/pmu: Add mediated vPMU PerfMon v5 support Zide Chen
2026-07-07 18:33 ` [PATCH 01/15] KVM: x86/pmu: Remove redundant Perf Global Status MSR bit definitions Zide Chen
2026-07-07 18:33 ` [PATCH 02/15] KVM: x86/pmu: Rename all_valid_pmc_idx to all_valid_pmc_mask Zide Chen
2026-07-28 19:11 ` Sean Christopherson
2026-07-28 19:51 ` Chen, Zide
2026-07-07 18:33 ` [PATCH 03/15] KVM: x86/pmu: Rename reserved_bits to eventsel_rsvd in kvm_pmu Zide Chen
2026-07-07 18:33 ` [PATCH 04/15] KVM: x86/pmu: Add PMC bitmap accessor helpers Zide Chen
2026-07-28 19:28 ` Sean Christopherson
2026-07-28 20:42 ` Chen, Zide
2026-07-28 23:35 ` Sean Christopherson
2026-07-07 18:33 ` [PATCH 05/15] KVM: x86/pmu: Drop nr_arch_{gp,fixed}_counters from kvm_pmu Zide Chen
2026-07-07 18:33 ` [PATCH 06/15] KVM: x86/pmu: Expose kvm_host_pmu to vendor modules Zide Chen
2026-07-07 18:33 ` [PATCH 07/15] perf/x86: Plumb counter bitmap from x86_pmu to x86_pmu_cap Zide Chen
2026-07-07 18:33 ` [PATCH 08/15] KVM: x86/pmu: Switch to bitmask-based KVM PMU capabilities Zide Chen
2026-07-23 21:47 ` Jim Mattson
2026-07-24 15:15 ` Chen, Zide [this message]
2026-07-07 18:33 ` [PATCH 09/15] perf/x86: Remove num_counters_{gp,fixed} from x86_pmu_capability Zide Chen
2026-07-23 21:49 ` Jim Mattson
2026-07-24 15:52 ` Chen, Zide
2026-07-07 18:34 ` [PATCH 10/15] KVM: x86/pmu: Emulate the GLOBAL_STATUS_SET and GLOBAL_INUSE MSRs Zide Chen
2026-07-07 18:34 ` [PATCH 11/15] KVM: x86/pmu: Emulate streamlined Freeze-LBR-on-PMI Zide Chen
2026-07-28 19:33 ` Sean Christopherson
2026-07-28 19:48 ` Chen, Zide
2026-07-07 18:34 ` [PATCH 12/15] KVM: x86/pmu: Populate CPUID.0AH:ECX fixed-counter bitmap Zide Chen
2026-07-07 18:34 ` [PATCH 13/15] KVM: x86/pmu: Ignore AnyThread bit if CPUID.0AH:EDX[15] is not set Zide Chen
2026-07-07 18:34 ` [PATCH 14/15] KVM: x86/pmu: Advertise PerfMon version 5 on Intel hosts Zide Chen
2026-07-23 21:53 ` Jim Mattson
2026-07-23 23:25 ` Chen, Zide
2026-07-24 3:20 ` Jim Mattson
2026-07-24 14:19 ` Chen, Zide
2026-07-07 18:34 ` [PATCH 15/15] KVM: selftests: Support fixed counters bitmap in pmu_counters_test Zide Chen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=65026581-daf9-45b5-8104-a4845544ebe7@intel.com \
--to=zide.chen@intel.com \
--cc=Manali.Shukla@amd.com \
--cc=Sandipan.Das@amd.com \
--cc=dapeng1.mi@linux.intel.com \
--cc=jmattson@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mizhang@google.com \
--cc=pbonzini@redhat.com \
--cc=peterz@infradead.org \
--cc=seanjc@google.com \
--cc=thomas.falcon@intel.com \
--cc=xudong.hao@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox