Re: [PATCH 09/15] perf/x86: Remove num_counters_{gp,fixed} from x86_pmu_capability

[Chen, Zide]

On 7/23/2026 4:49 PM, Jim Mattson wrote:
> On Tue, Jul 7, 2026 at 11:44 AM Zide Chen <zide.chen@xxxxxxxxx> wrote:
>>
>> From: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
>>
>> Now that KVM has switched to bitmap-based PMU capabilities,
>> num_counters_{gp,fixed} can be removed from x86_pmu_capability.
>>
>> Signed-off-by: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
>> Signed-off-by: Zide Chen <zide.chen@xxxxxxxxx>
>> ---
>>  arch/x86/events/core.c            | 2 --
>>  arch/x86/include/asm/perf_event.h | 2 --
>>  2 files changed, 4 deletions(-)
>>
>> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
>> index 65349819ba43..5bcd2a48d2b3 100644
>> --- a/arch/x86/events/core.c
>> +++ b/arch/x86/events/core.c
>> @@ -3132,8 +3132,6 @@ void perf_get_x86_pmu_capability(struct x86_pmu_capability *cap)
>>          * base PMU holds the correct number of counters for P-cores.
>>          */
>>         cap->version            = x86_pmu.version;
>> -       cap->num_counters_gp    = x86_pmu_num_counters(NULL);
>> -       cap->num_counters_fixed = x86_pmu_num_counters_fixed(NULL);
>>         cap->cntr_mask64        = x86_pmu.cntr_mask64;
>>         cap->fixed_cntr_mask64  = x86_pmu.fixed_cntr_mask64;
>>         cap->bit_width_gp       = cap->cntr_mask64 ? x86_pmu.cntval_bits : 0;
> Our internal Sashiko says:
> 
> This isn't a bug introduced by this patch, but since KVM is switching
> to bitmap-based PMU capabilities here, I noticed an issue with how legacy
> AMD PMU intercepts are calculated in
> arch/x86/kvm/svm/svm.c: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);
> Does this fail to re-enable intercepts for counters in ~gp_mask up to
> AMD64_NUM_COUNTERS?
> If a guest's CPUID dynamically disables previously enabled legacy counters
> while mediated PMU is active, the intercepts for these counters are skipped.
> Could this leave the guest with unauthorized, direct read/write passthrough
> access to host performance counters?
> The code just below this handles the ~gp_mask case correctly for
> MSR_F15H_PERF_CTR by iterating over host_only_gp_mask:
> host_only_gp_mask = kvm_pmu_cap.cntr_mask64 & ~gp_mask;
> kvm_for_each_set_pmc_idx(i, host_only_gp_mask, AMD_GP)
> svm_enable_intercept_for_msr(vcpu, MSR_F15H_PERF_CTR + 2 * i,
>      MSR_TYPE_RW);
> Would the legacy MSR_K7_PERFCTR0 handling allow a guest-to-host privilege
> escalation and information leak without a similar fix?


Seems this is the same issue raised for patch 8/15:

https://lore.kernel.org/kvm/CALMp9eTYWFvB-ji31Mu=vzJo2HFPL0jZFAiXVGpJx-H8S2mHOw@xxxxxxxxxxxxxx/

It should be addressed by the previously proposed fix:

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)