All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Chen, Zide" <zide.chen@intel.com>
To: Yosry Ahmed <yosry@kernel.org>, 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, Andi Kleen <ak@linux.intel.com>,
	Stephane Eranian <eranian@google.com>,
	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>,
	Xudong Hao <xudong.hao@intel.com>
Subject: Re: [PATCH v9 05/12] KVM: x86/pmu: Support PERF_METRICS MSR in mediated vPMU
Date: Fri, 30 Oct 2026 14:25:14 -0500	[thread overview]
Message-ID: <897ffd39-a64b-4f77-a688-b8490d07980a@intel.com> (raw)
In-Reply-To: <CAO9r8zNO6X2-LDx_3n44Uc+9ODtXoH7oLvn8i5yzAa+EUy_qxg@mail.gmail.com>



On 10/29/2026 4:28 PM, Yosry Ahmed wrote:
> On Tue, Oct 27, 2026 at 2:31 PM Chen, Zide <zide.chen@intel.com> wrote:
>>
>>
>>
>> On 10/23/2026 1:27 PM, Jim Mattson wrote:
>>> On Fri, Sep 18, 2026 at 12:39 PM Zide Chen <zide.chen@intel.com> wrote:
>>>>
>>>> +static inline bool kvm_vcpu_has_perf_metrics(struct kvm_vcpu *vcpu)
>>>> +{
>>>> +    return kvm_vcpu_get_perf_caps(vcpu) & PERF_CAP_PERF_METRICS;
>>>> +}
>>>
>>> The SDM, volume 3, section 22.3.9.3, says:
>>>
>>>> IA32_PERF_GLOBAL_CTRL. EN_PERF_METRICS[48]: If this bit is set *and*
>>>> fixed-function performance-monitoring counter 3 is enabled, built-in
>>>> performance metrics are enabled. [emphasis mine]
>>>
>>> and
>>>
>>>> The values in MSR_PERF_METRICS are derived from fixed-function
>>>> performance-monitoring counter 3.
>>>
>>> Hence, a dependency on fixed counter 3 belongs in this predicate.
>>> Perhaps something like:
>>>
>>>       static inline bool kvm_pmu_has_fixed_counter(struct kvm_pmu *pmu,
>>>                                                    unsigned int idx)
>>>       {
>>>               return idx < pmu->nr_arch_fixed_counters;
>>>       }
>>>
>>>       static inline bool kvm_vcpu_has_perf_metrics(struct kvm_vcpu *vcpu)
>>>       {
>>>               return (kvm_vcpu_get_perf_caps(vcpu) & PERF_CAP_PERF_METRICS) &&
>>>                      kvm_pmu_has_fixed_counter(vcpu_to_pmu(vcpu),
>>>                                                INTEL_PMC_IDX_FIXED_SLOTS -
>>>                                                INTEL_PMC_IDX_FIXED);
>>>       }
>>>
>>> with intel_pmu_refresh() calling kvm_vcpu_has_perf_metrics() instead of
>>> testing perf_capabilities directly. (nr_arch_fixed_counters has already
>>> been computed at that point; all_valid_pmc_idx has not.)
>>
>> Thanks for the detailed analysis. That makes total sense.
>>
>> Initially I didn't gate PERF_METRICS on fixed counter 3 out of concern
>> about policing userspace configurations. Additionally, as in the quoted
>> SDM text above, it means PERF_METRICS can't function without fixed
>> counter 3, but strictly speaking, the SDM doesn't prevent other
>> operations, for example, writing the METRICS_CLEAR_EN bit.
>>
>> But having IA32_PERF_CAPABILITIES[15] set in the absence of FC3 is not
>> going to happen natively, so I think it doesn't break any rules if KVM
>> chooses to gate PERF_METRICS on FC3. And as you pointed out, it's a
>> simple and effective way to protect KVM invariants and migration integrity.
>>
>> I will add the FC3 check to kvm_vcpu_has_perf_metrics().
> 
> If userspace sets IA32_PERF_CAPABILITIES[15] but not FC3 (for some
> reason), I think architecturally it should still be valid to
> read/write PERF_METRICS. KVM disallowing access to PERF_METRICS while
> IA32_PERF_CAPABILITIES[15] is set would not be correct
> architecturally, right?

Right. Per SDM 22.8, MSR_PERF_METRICS is available as long as
IA32_PERF_CAPABILITIES[15] is set, so disallowing access to it when bit
15 is exposed seems architecturally incorrect. However, TMA metrics
cannot be enabled or computed without FC3 (SDM 22.3.9.3).

Instead of gating kvm_vcpu_has_perf_metrics() on FC3, seems it's better
off to do some changes to fix the invariants where they belong:
1. Untie FC3 from kvm_vcpu_has_perf_metrics() so the MSR remains accessible.
2. In intel_pmu_refresh(), compute perf_metrics_rsvd before the PMU v1
early return so save/restore works properly.
3. Explicitly check for FC3 when unmasking GLOBAL_CTRL[48] (TMA enable)
and FIXED_CTR_CTRL[14] (METRICS_CLEAR_EN), and guard pmc in RDPMC emulation.

This seems satisfy the SDM while keeping migration and invariants intact.

  reply	other threads:[~2026-10-30 19:25 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-18 19:39 [PATCH v9 00/12] KVM: x86/pmu: Add hardware Topdown metrics support Zide Chen
2026-09-18 19:39 ` [PATCH v9 01/12] KVM: x86/pmu: Do not map fixed counters >= 3 to generic perf events Zide Chen
2026-09-18 19:39 ` [PATCH v9 02/12] KVM: x86/pmu: Support Intel fixed counter 3 on mediated vPMU Zide Chen
2026-09-18 19:39 ` [PATCH v9 03/12] KVM: x86/pmu: Rename and move vcpu_get_perf_capabilities() to pmu.h Zide Chen
2026-09-18 19:39 ` [PATCH v9 04/12] KVM: x86/pmu: Snapshot host IA32_PERF_CAPABILITIES in kvm_host Zide Chen
2026-09-18 19:39 ` [PATCH v9 05/12] KVM: x86/pmu: Support PERF_METRICS MSR in mediated vPMU Zide Chen
2026-10-23 18:27   ` Jim Mattson
2026-10-27 21:30     ` Chen, Zide
2026-10-29 21:28       ` Yosry Ahmed
2026-10-30 19:25         ` Chen, Zide [this message]
2026-10-29 22:06   ` Yosry Ahmed
2026-10-29 23:17     ` Chen, Zide
2026-10-29 23:19       ` Yosry Ahmed
2026-10-29 23:26         ` Jim Mattson
2026-10-29 23:32           ` Yosry Ahmed
2026-10-30 15:47             ` Chen, Zide
2026-09-18 19:39 ` [PATCH v9 06/12] KVM: x86/pmu: Move RDPMC emulation into per-vendor callbacks Zide Chen
2026-09-18 19:39 ` [PATCH v9 07/12] KVM: x86/pmu: Emulate RDPMC on performance metrics Zide Chen
2026-10-23 18:33   ` Jim Mattson
2026-09-18 19:39 ` [PATCH v9 08/12] KVM: selftests: Add PERF_METRICS and fixed counter 3 tests Zide Chen
2026-09-18 20:08   ` sashiko-bot
2026-09-18 20:45     ` Chen, Zide
2026-09-18 19:39 ` [PATCH v9 09/12] perf/x86: Add INTEL_TD_METRIC_FIELD_{BITS,MASK} constants Zide Chen
2026-09-18 19:59   ` sashiko-bot
2026-09-18 20:45     ` Chen, Zide
2026-09-21  7:01   ` Mi, Dapeng
2026-10-27 21:39     ` Chen, Zide
2026-09-18 19:39 ` [PATCH v9 10/12] perf/x86: Expose number of Topdown metric events to KVM Zide Chen
2026-09-21  7:06   ` Mi, Dapeng
2026-09-18 19:39 ` [PATCH v9 11/12] KVM: x86/pmu: Reject writes to reserved MSR_PERF_METRICS bits Zide Chen
2026-09-21  7:11   ` Mi, Dapeng
2026-10-23 18:45   ` Jim Mattson
2026-10-27 22:09     ` Chen, Zide
2026-09-18 19:39 ` [PATCH v9 12/12] KVM: x86/pmu: Support RDPMC Metrics Clear Mode Zide Chen
2026-09-18 20:22   ` sashiko-bot
2026-09-18 21:08     ` Chen, Zide
2026-09-21  9:39     ` Mi, Dapeng
2026-10-27 22:56       ` Chen, Zide
2026-10-23 18:55   ` Jim Mattson
2026-10-27 22:17     ` Chen, Zide

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=897ffd39-a64b-4f77-a688-b8490d07980a@intel.com \
    --to=zide.chen@intel.com \
    --cc=Manali.Shukla@amd.com \
    --cc=Sandipan.Das@amd.com \
    --cc=ak@linux.intel.com \
    --cc=dapeng1.mi@linux.intel.com \
    --cc=eranian@google.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=xudong.hao@intel.com \
    --cc=yosry@kernel.org \
    /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.