| Message ID | 20250324123712.34096-3-dapeng1.mi@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enable x86 mediated vPMU | expand |
Hi Dapeng, PATCH 1-4 from the below patchset are already reviewed. (PATCH 5-10 are for PMU registers reset). https://lore.kernel.org/all/20250302220112.17653-1-dongli.zhang@oracle.com/ They require only trivial modification. i.e.: https://github.com/finallyjustice/patchset/tree/master/qemu-amd-pmu-mid/v03 Therefore, since PATCH 5-10 are for another topic, any chance if I re-send 1-4 as a prerequisite for the patch to explicitly call KVM_CAP_PMU_CAPABILITY? In addition, I have a silly question. Can mediated vPMU coexist with legacy perf-based vPMU, that is, something like tdp and tdp_mmu? Or the legacy perf-based vPMU is going to be purged from the most recent kernel? If they can coexist, how about add property to QEMU control between legacy/modern? i.e. by default use legacy and change to modern as default in the future once the feature is stable. Thank you very much! Dongli Zhang On 3/24/25 5:37 AM, Dapeng Mi wrote: > After introducing mediated vPMU, mediated vPMU must be enabled by > explicitly calling KVM_CAP_PMU_CAPABILITY to enable. Thus call > KVM_CAP_PMU_CAPABILITY to enable/disable PMU base on user configuration. > > Suggested-by: Zhao Liu <zhao1.liu@intel.com> > Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com> > --- > target/i386/kvm/kvm.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c > index f41e190fb8..d3e6984844 100644 > --- a/target/i386/kvm/kvm.c > +++ b/target/i386/kvm/kvm.c > @@ -2051,8 +2051,25 @@ full: > abort(); > } > > +static bool pmu_cap_set = false; > int kvm_arch_pre_create_vcpu(CPUState *cpu, Error **errp) > { > + KVMState *s = kvm_state; > + X86CPU *x86_cpu = X86_CPU(cpu); > + > + if (!pmu_cap_set && kvm_check_extension(s, KVM_CAP_PMU_CAPABILITY)) { > + int r = kvm_vm_enable_cap(s, KVM_CAP_PMU_CAPABILITY, 0, > + KVM_PMU_CAP_DISABLE & !x86_cpu->enable_pmu); > + if (r < 0) { > + error_report("kvm: Failed to %s pmu cap: %s", > + x86_cpu->enable_pmu ? "enable" : "disable", > + strerror(-r)); > + return r; > + } > + > + pmu_cap_set = true; > + } > + > return 0; > } >
On 3/26/2025 2:46 PM, Dongli Zhang wrote: > Hi Dapeng, > > PATCH 1-4 from the below patchset are already reviewed. (PATCH 5-10 are for PMU > registers reset). > > https://lore.kernel.org/all/20250302220112.17653-1-dongli.zhang@oracle.com/ > > They require only trivial modification. i.e.: > > https://github.com/finallyjustice/patchset/tree/master/qemu-amd-pmu-mid/v03 > > Therefore, since PATCH 5-10 are for another topic, any chance if I re-send 1-4 > as a prerequisite for the patch to explicitly call KVM_CAP_PMU_CAPABILITY? any option is fine for me, spiting it into two separated ones or still keep in a whole patch series. I would rebase this this patchset on top of your v3 patches. > > In addition, I have a silly question. Can mediated vPMU coexist with legacy > perf-based vPMU, that is, something like tdp and tdp_mmu? Or the legacy > perf-based vPMU is going to be purged from the most recent kernel? No, they can't. As long as mediated vPMU is enabled, it would totally replace the legacy perf-based vPMU. The legacy perf-based vPMU code would still be kept in the kernel in near future, but the long-term target is to totally remove the perf-based vPMU once mediated vPMU is mature. > > If they can coexist, how about add property to QEMU control between > legacy/modern? i.e. by default use legacy and change to modern as default in the > future once the feature is stable. > > Thank you very much! > > Dongli Zhang > > On 3/24/25 5:37 AM, Dapeng Mi wrote: >> After introducing mediated vPMU, mediated vPMU must be enabled by >> explicitly calling KVM_CAP_PMU_CAPABILITY to enable. Thus call >> KVM_CAP_PMU_CAPABILITY to enable/disable PMU base on user configuration. >> >> Suggested-by: Zhao Liu <zhao1.liu@intel.com> >> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com> >> --- >> target/i386/kvm/kvm.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c >> index f41e190fb8..d3e6984844 100644 >> --- a/target/i386/kvm/kvm.c >> +++ b/target/i386/kvm/kvm.c >> @@ -2051,8 +2051,25 @@ full: >> abort(); >> } >> >> +static bool pmu_cap_set = false; >> int kvm_arch_pre_create_vcpu(CPUState *cpu, Error **errp) >> { >> + KVMState *s = kvm_state; >> + X86CPU *x86_cpu = X86_CPU(cpu); >> + >> + if (!pmu_cap_set && kvm_check_extension(s, KVM_CAP_PMU_CAPABILITY)) { >> + int r = kvm_vm_enable_cap(s, KVM_CAP_PMU_CAPABILITY, 0, >> + KVM_PMU_CAP_DISABLE & !x86_cpu->enable_pmu); >> + if (r < 0) { >> + error_report("kvm: Failed to %s pmu cap: %s", >> + x86_cpu->enable_pmu ? "enable" : "disable", >> + strerror(-r)); >> + return r; >> + } >> + >> + pmu_cap_set = true; >> + } >> + >> return 0; >> } >>
On Wed, Mar 26, 2025 at 5:44 PM Mi, Dapeng <dapeng1.mi@linux.intel.com> wrote: > > > On 3/26/2025 2:46 PM, Dongli Zhang wrote: > > Hi Dapeng, > > > > PATCH 1-4 from the below patchset are already reviewed. (PATCH 5-10 are for PMU > > registers reset). > > > > https://lore.kernel.org/all/20250302220112.17653-1-dongli.zhang@oracle.com/ > > > > They require only trivial modification. i.e.: > > > > https://github.com/finallyjustice/patchset/tree/master/qemu-amd-pmu-mid/v03 > > > > Therefore, since PATCH 5-10 are for another topic, any chance if I re-send 1-4 > > as a prerequisite for the patch to explicitly call KVM_CAP_PMU_CAPABILITY? > > any option is fine for me, spiting it into two separated ones or still keep > in a whole patch series. I would rebase this this patchset on top of your > v3 patches. > > > > > > In addition, I have a silly question. Can mediated vPMU coexist with legacy > > perf-based vPMU, that is, something like tdp and tdp_mmu? Or the legacy > > perf-based vPMU is going to be purged from the most recent kernel? > > No, they can't. As long as mediated vPMU is enabled, it would totally > replace the legacy perf-based vPMU. The legacy perf-based vPMU code would > still be kept in the kernel in near future, but the long-term target is to > totally remove the perf-based vPMU once mediated vPMU is mature. mediated vPMU will co-exist with legacy vPMU right? Mediated vPMU currently was constrained to SPR+ on Intel and Genoa+ on AMD. So legacy CPUs will have no choice but legacy vPMU. In the future, to fully replace legacy vPMU we need to solve the performance issue due to PMU context switching being located at VM enter/exit boundary. Once those limitations are resolved, and older x86 CPUs fade away, mediated vPMU can fully take over. Thanks. -Mingwei > > > > > > If they can coexist, how about add property to QEMU control between > > legacy/modern? i.e. by default use legacy and change to modern as default in the > > future once the feature is stable. > > > > Thank you very much! > > > > Dongli Zhang > > > > On 3/24/25 5:37 AM, Dapeng Mi wrote: > >> After introducing mediated vPMU, mediated vPMU must be enabled by > >> explicitly calling KVM_CAP_PMU_CAPABILITY to enable. Thus call > >> KVM_CAP_PMU_CAPABILITY to enable/disable PMU base on user configuration. > >> > >> Suggested-by: Zhao Liu <zhao1.liu@intel.com> > >> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com> > >> --- > >> target/i386/kvm/kvm.c | 17 +++++++++++++++++ > >> 1 file changed, 17 insertions(+) > >> > >> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c > >> index f41e190fb8..d3e6984844 100644 > >> --- a/target/i386/kvm/kvm.c > >> +++ b/target/i386/kvm/kvm.c > >> @@ -2051,8 +2051,25 @@ full: > >> abort(); > >> } > >> > >> +static bool pmu_cap_set = false; > >> int kvm_arch_pre_create_vcpu(CPUState *cpu, Error **errp) > >> { > >> + KVMState *s = kvm_state; > >> + X86CPU *x86_cpu = X86_CPU(cpu); > >> + > >> + if (!pmu_cap_set && kvm_check_extension(s, KVM_CAP_PMU_CAPABILITY)) { > >> + int r = kvm_vm_enable_cap(s, KVM_CAP_PMU_CAPABILITY, 0, > >> + KVM_PMU_CAP_DISABLE & !x86_cpu->enable_pmu); > >> + if (r < 0) { > >> + error_report("kvm: Failed to %s pmu cap: %s", > >> + x86_cpu->enable_pmu ? "enable" : "disable", > >> + strerror(-r)); > >> + return r; > >> + } > >> + > >> + pmu_cap_set = true; > >> + } > >> + > >> return 0; > >> } > >>
On 3/27/2025 10:15 AM, Mingwei Zhang wrote: > On Wed, Mar 26, 2025 at 5:44 PM Mi, Dapeng <dapeng1.mi@linux.intel.com> wrote: >> >> On 3/26/2025 2:46 PM, Dongli Zhang wrote: >>> Hi Dapeng, >>> >>> PATCH 1-4 from the below patchset are already reviewed. (PATCH 5-10 are for PMU >>> registers reset). >>> >>> https://lore.kernel.org/all/20250302220112.17653-1-dongli.zhang@oracle.com/ >>> >>> They require only trivial modification. i.e.: >>> >>> https://github.com/finallyjustice/patchset/tree/master/qemu-amd-pmu-mid/v03 >>> >>> Therefore, since PATCH 5-10 are for another topic, any chance if I re-send 1-4 >>> as a prerequisite for the patch to explicitly call KVM_CAP_PMU_CAPABILITY? >> any option is fine for me, spiting it into two separated ones or still keep >> in a whole patch series. I would rebase this this patchset on top of your >> v3 patches. >> >> >>> In addition, I have a silly question. Can mediated vPMU coexist with legacy >>> perf-based vPMU, that is, something like tdp and tdp_mmu? Or the legacy >>> perf-based vPMU is going to be purged from the most recent kernel? >> No, they can't. As long as mediated vPMU is enabled, it would totally >> replace the legacy perf-based vPMU. The legacy perf-based vPMU code would >> still be kept in the kernel in near future, but the long-term target is to >> totally remove the perf-based vPMU once mediated vPMU is mature. > mediated vPMU will co-exist with legacy vPMU right? Mediated vPMU > currently was constrained to SPR+ on Intel and Genoa+ on AMD. So > legacy CPUs will have no choice but legacy vPMU. > > In the future, to fully replace legacy vPMU we need to solve the > performance issue due to PMU context switching being located at VM > enter/exit boundary. Once those limitations are resolved, and older > x86 CPUs fade away, mediated vPMU can fully take over. yeah, the code would co-exist in near feature or maybe longer, but mediated vPMU and the legacy vPMU won't work concurrently, once mediated vPMU is enabled, it would preempt the legacy vPMU. > > Thanks. > -Mingwei >> >>> If they can coexist, how about add property to QEMU control between >>> legacy/modern? i.e. by default use legacy and change to modern as default in the >>> future once the feature is stable. I don't prefer to add such property in Qemu. Whether KVM selects to enable mediated vPMU or the legacy perf-based vPMU, it should be transparent for Qemu. Qemu is unnecessary to know it. Currently Qemu already has too much PMU related options, such as pmu, lbr/arch_lbr. It's too complicated, it introduces too much dependency on the code and user needs to take time to learn how to configure these options correctly as well. We don't need more options, on the contrary we need to simplify the PMU options in Qemu. The ideal situation is to keep only one "pmu" option which is used to manage all these PMU features, like basic perfmon, LBR and PEBS etc,. But it may cause back-compatible issues if remove these additional options... >>> >>> Thank you very much! >>> >>> Dongli Zhang >>> >>> On 3/24/25 5:37 AM, Dapeng Mi wrote: >>>> After introducing mediated vPMU, mediated vPMU must be enabled by >>>> explicitly calling KVM_CAP_PMU_CAPABILITY to enable. Thus call >>>> KVM_CAP_PMU_CAPABILITY to enable/disable PMU base on user configuration. >>>> >>>> Suggested-by: Zhao Liu <zhao1.liu@intel.com> >>>> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com> >>>> --- >>>> target/i386/kvm/kvm.c | 17 +++++++++++++++++ >>>> 1 file changed, 17 insertions(+) >>>> >>>> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c >>>> index f41e190fb8..d3e6984844 100644 >>>> --- a/target/i386/kvm/kvm.c >>>> +++ b/target/i386/kvm/kvm.c >>>> @@ -2051,8 +2051,25 @@ full: >>>> abort(); >>>> } >>>> >>>> +static bool pmu_cap_set = false; >>>> int kvm_arch_pre_create_vcpu(CPUState *cpu, Error **errp) >>>> { >>>> + KVMState *s = kvm_state; >>>> + X86CPU *x86_cpu = X86_CPU(cpu); >>>> + >>>> + if (!pmu_cap_set && kvm_check_extension(s, KVM_CAP_PMU_CAPABILITY)) { >>>> + int r = kvm_vm_enable_cap(s, KVM_CAP_PMU_CAPABILITY, 0, >>>> + KVM_PMU_CAP_DISABLE & !x86_cpu->enable_pmu); >>>> + if (r < 0) { >>>> + error_report("kvm: Failed to %s pmu cap: %s", >>>> + x86_cpu->enable_pmu ? "enable" : "disable", >>>> + strerror(-r)); >>>> + return r; >>>> + } >>>> + >>>> + pmu_cap_set = true; >>>> + } >>>> + >>>> return 0; >>>> } >>>>
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index f41e190fb8..d3e6984844 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -2051,8 +2051,25 @@ full: abort(); } +static bool pmu_cap_set = false; int kvm_arch_pre_create_vcpu(CPUState *cpu, Error **errp) { + KVMState *s = kvm_state; + X86CPU *x86_cpu = X86_CPU(cpu); + + if (!pmu_cap_set && kvm_check_extension(s, KVM_CAP_PMU_CAPABILITY)) { + int r = kvm_vm_enable_cap(s, KVM_CAP_PMU_CAPABILITY, 0, + KVM_PMU_CAP_DISABLE & !x86_cpu->enable_pmu); + if (r < 0) { + error_report("kvm: Failed to %s pmu cap: %s", + x86_cpu->enable_pmu ? "enable" : "disable", + strerror(-r)); + return r; + } + + pmu_cap_set = true; + } + return 0; }
After introducing mediated vPMU, mediated vPMU must be enabled by explicitly calling KVM_CAP_PMU_CAPABILITY to enable. Thus call KVM_CAP_PMU_CAPABILITY to enable/disable PMU base on user configuration. Suggested-by: Zhao Liu <zhao1.liu@intel.com> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com> --- target/i386/kvm/kvm.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)