Message-ID: <afb9faeb-11f4-47a2-a77b-4f2496182952@linux.intel.com>
Date: Thu, 11 Apr 2024 15:13:33 -0400
From: "Liang, Kan" <kan.liang@...ux.intel.com>
To: Sean Christopherson <seanjc@...gle.com>, Jim Mattson <jmattson@...gle.com>
Cc: Xiong Zhang <xiong.y.zhang@...ux.intel.com>, pbonzini@...hat.com,
 peterz@...radead.org, mizhang@...gle.com, kan.liang@...el.com,
 zhenyuw@...ux.intel.com, dapeng1.mi@...ux.intel.com, kvm@...r.kernel.org,
 linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
 zhiyuan.lv@...el.com, eranian@...gle.com, irogers@...gle.com,
 samantha.alt@...el.com, like.xu.linux@...il.com, chao.gao@...el.com
Subject: Re: [RFC PATCH 01/41] perf: x86/intel: Support
 PERF_PMU_CAP_VPMU_PASSTHROUGH



On 2024-04-11 1:46 p.m., Sean Christopherson wrote:
> On Thu, Apr 11, 2024, Jim Mattson wrote:
>> On Thu, Apr 11, 2024 at 10:21 AM Liang, Kan <kan.liang@...ux.intel.com> wrote:
>>> On 2024-04-11 1:04 p.m., Sean Christopherson wrote:
>>>> On Fri, Jan 26, 2024, Xiong Zhang wrote:
>>>>> From: Kan Liang <kan.liang@...ux.intel.com>
>>>>>
>>>>> Define and apply the PERF_PMU_CAP_VPMU_PASSTHROUGH flag for the version 4
>>>>> and later PMUs
>>>>
>>>> Why?  I get that is an RFC, but it's not at all obvious to me why this needs to
>>>> take a dependency on v4+.
>>>
>>> The IA32_PERF_GLOBAL_STATUS_RESET/SET MSRs are introduced in v4. They
>>> are used in the save/restore of PMU state. Please see PATCH 23/41.
>>> So it's limited to v4+ for now.
>>
>> Prior to version 4, semi-passthrough is possible, but IA32_PERF_GLOBAL_STATUS
>> has to be intercepted and emulated, since it is non-trivial to set bits in
>> this MSR.
> 
> Ah, then this _perf_ capability should be PERF_PMU_CAP_WRITABLE_GLOBAL_STATUS or
> so, especially since it's introduced in advance of the KVM side of things.  Then
> whether or not to support a mediated PMU becomes a KVM decision, e.g. intercepting
> accesses to IA32_PERF_GLOBAL_STATUS doesn't seem like a complete deal breaker
> (or maybe it is, I now see the comment about it being used to do the context switch).

The PERF_PMU_CAP_VPMU_PASSTHROUGH is to indicate whether the PMU has the
capability to support passthrough mode. It's used to distinguish the
other PMUs, e.g., uncore PMU. It's just because the current RFC utilizes
the IA32_PERF_GLOBAL_STATUS_RESET/SET MSRs, I have to limit it to V4+.

I agree that it should be a KVM decision, not perf. The v4 check should
be removed.

Regarding the PERF_PMU_CAP_WRITABLE_GLOBAL_STATUS, I think perf already
passes the x86_pmu.version to KVM. Maybe KVM can add an internal flag to
track it, so a PERF_PMU_CAP_ bit can be saved?

> 
> And peeking ahead, IIUC perf effectively _forces_ a passthrough model when
> has_vpmu_passthrough_cap() is true, which is wrong.  There needs to be a user/admin
> opt-in (or opt-out) to that behavior, at a kernel/perf level, not just at a KVM
> level.  Hmm, or is perf relying on KVM to do that right thing?  I.e. relying on
> KVM to do perf_guest_{enter,exit}() if and only if the PMU can support the
> passthrough model.
>

Yes, perf relies on KVM to tell if a guest is entering the passthrough mode.

> If that's the case, most of the has_vpmu_passthrough_cap() checks are gratiutous
> and confusing, e.g. just WARN if KVM (or some other module) tries to trigger a
> PMU context switch when it's not supported by perf.

If there is only non supported PMUs running in the host, perf wouldn't
do any context switch. The guest can feel free to use the core PMU. We
should not WARN for this case.

Thanks,
Kan

Powered by blists - more mailing lists