On 5/9/2024 8:13 AM, Mingwei Zhang wrote: > On Wed, May 8, 2024 at 7:13 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: >> On Wed, May 08, 2024, Dapeng Mi wrote: >>> On 5/8/2024 12:36 PM, Mingwei Zhang wrote: >>>> if (pmu->passthrough && pmu->nr_arch_gp_counters) >>>> >>>> Since mediated passthrough PMU requires PerfMon v4 in Intel (PerfMon >>>> v2 in AMD), once it is enabled (pmu->passthrough = true), then global >>>> ctrl _must_ exist phyiscally. Regardless of whether we expose it to >>>> the guest VM, at reset time, we need to ensure enabling bits for GP >>>> counters are set (behind the screen). This is critical for AMD, since >>>> most of the guests are usually in (AMD) PerfMon v1 in which global >>>> ctrl MSR is inaccessible, but does exist and is operating in HW. >>>> >>>> Yes, if we eliminate that requirement (pmu->passthrough -> Perfmon v4 >>>> Intel / Perfmon v2 AMD), then this code will have to change. However, >>> Yeah, that's what I'm worrying about. We ever discussed to support mediated >>> vPMU on HW below perfmon v4. When someone implements this, he may not >>> notice this place needs to be changed as well, this introduces a potential >>> bug and we should avoid this. > I think you might have worried too much about future problems, but > yes, things are under the radar. For Intel, this version constraint > might be ok as Perfmon v4 is skylake, which is already pretty early. No, I don't think this is redundant worry since we did discuss this requirement before and it could need to be supported in the future. We need to consider the code's extensibility and not introduce potential issue. > > For AMD, things are slightly different, PerfMon v2 in AMD requires > Genoa, which is pretty new. So, this problem probably could be > something for AMD if they want to extend the new vPMU design to Milan, > but we will see how people think. So one potential (easy) extension > for AMD is host PerfMon v1 + guest PerfMon v1 support for mediated > passthrough vPMU. > >> Just add a WARN on the PMU version. I haven't thought much about whether or not >> KVM should support mediated PMU for earlier hardware, but having a sanity check >> on the assumptions of this code is reasonable even if we don't _plan_ on supporting >> earlier hardware. > Sure. That sounds pretty reasonable. Good for me. > > Thanks. > -Mingwei >
