All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Chen, Zide" <zide.chen@intel.com>
To: sashiko-reviews@lists.linux.dev
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH v7 2/9] KVM: x86/pmu: Support Intel fixed counter 3 on mediated vPMU
Date: Mon, 27 Jul 2026 17:44:01 -0500	[thread overview]
Message-ID: <f87ab430-61a6-43cf-87ee-b88ab58ec9ef@intel.com> (raw)
In-Reply-To: <20260727195130.007701F00AC4@smtp.kernel.org>



On 7/27/2026 2:51 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The logic to filter non-contiguous fixed counters incorrectly uses CPUID.0A:EDX[4:0] instead of counting the contiguous 1-bits in the ECX bitmask. This causes KVM to expose non-existent fixed counters to the guest while hiding supported ones.
> --
> 
> --- Patch [2]: [PATCH v7 2/9] KVM: x86/pmu: Support Intel fixed counter 3 on mediated vPMU ---
> commit 7c8d5061e2216416ace2a9414a66d8b5fc0bf8bd
> Author: Dapeng Mi <dapeng1.mi@linux.intel.com>
> 
> KVM: x86/pmu: Support Intel fixed counter 3 on mediated vPMU
> 
> This commit exposes Intel's fixed counter 3 to guests when mediated vPMU is
> enabled. It updates the maximum number of fixed counters and adds logic to
> filter non-contiguous fixed counters since KVM currently does not support
> them.
> 
>> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
>> --- a/arch/x86/kvm/pmu.c
>> +++ b/arch/x86/kvm/pmu.c
> [ ... ]
>> @@ -181,6 +185,19 @@ void kvm_init_pmu_capability(struct kvm_pmu_ops *pmu_ops)
>>  	kvm_pmu_cap.num_counters_fixed = min(kvm_pmu_cap.num_counters_fixed,
>>  					     KVM_MAX_NR_FIXED_COUNTERS);
>>  
>> +	/*
>> +	 * 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;
> 
> [Severity: High]
> Does this logic correctly determine the number of contiguous fixed counters?
> 
> If a host supports PMU version 5 with fixed counters 0, 1, and 3, the ecx
> bitmask would be 0xB, and kvm_pmu_cap.num_counters_fixed would initially be
> populated with the weight of that mask, which is 3.
> 
> The value in edx.split.num_counters_fixed reports the maximum fixed counter
> index plus one (4 in this scenario). The condition 3 > 4 evaluates to
> false, so kvm_pmu_cap.num_counters_fixed remains 3.

I think it's a false positive.

edx.split.num_counters_fixed represents the number of contiguous fixed
counters, which is 2 in this case. The code correctly sets
kvm_pmu_cap.num_counters_fixed to 2.

> This leaves KVM exposing the first three contiguous counters (0, 1, and 2)
> to the guest. Since the host hardware does not actually support counter 2,
> KVMs attempt to back it with a host perf event will fail.
> 
> Should this instead inspect the contiguous 1-bits starting from bit 0 in the
> ecx bitmask, for example by using ffz(~ecx)?
> 
>> +	}
>> +
>> +	if (!enable_mediated_pmu && kvm_pmu_cap.num_counters_fixed > 3)
>> +		kvm_pmu_cap.num_counters_fixed = 3;
>> +
> 


  reply	other threads:[~2026-07-27 22:44 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-27 19:21 [PATCH v7 0/9] KVM: x86/pmu: Add hardware Topdown metrics support Zide Chen
2026-07-27 19:21 ` [PATCH v7 1/9] KVM: x86/pmu: Do not map fixed counters >= 3 to generic perf events Zide Chen
2026-07-27 19:21 ` [PATCH v7 2/9] KVM: x86/pmu: Support Intel fixed counter 3 on mediated vPMU Zide Chen
2026-07-27 19:51   ` sashiko-bot
2026-07-27 22:44     ` Chen, Zide [this message]
2026-07-27 19:21 ` [PATCH v7 3/9] KVM: x86/pmu: Rename and move vcpu_get_perf_capabilities() to pmu.h Zide Chen
2026-07-27 19:42   ` sashiko-bot
2026-07-27 22:19     ` Chen, Zide
2026-07-27 19:21 ` [PATCH v7 4/9] KVM: x86/pmu: Snapshot host IA32_PERF_CAPABILITIES in kvm_host Zide Chen
2026-07-27 19:44   ` sashiko-bot
2026-07-27 23:16     ` Chen, Zide
2026-07-27 19:21 ` [PATCH v7 5/9] KVM: x86/pmu: Support PERF_METRICS MSR in mediated vPMU Zide Chen
2026-07-27 19:50   ` sashiko-bot
2026-07-27 23:55     ` Chen, Zide
2026-07-27 19:21 ` [PATCH v7 6/9] KVM: x86/pmu: Move RDPMC emulation into per-vendor callbacks Zide Chen
2026-07-27 19:21 ` [PATCH v7 7/9] KVM: x86/pmu: Restrict RDPMC passthrough to known CPUs Zide Chen
2026-07-27 19:41   ` sashiko-bot
2026-07-27 21:41     ` Chen, Zide
2026-07-27 19:21 ` [PATCH v7 8/9] KVM: x86/pmu: Emulate RDPMC on performance metrics Zide Chen
2026-07-27 19:21 ` [PATCH v7 9/9] KVM: selftests: Add PERF_METRICS and fixed counter 3 tests 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=f87ab430-61a6-43cf-87ee-b88ab58ec9ef@intel.com \
    --to=zide.chen@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.