| Message ID | 20221109082802.27543-2-likexu@tencent.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86/pmu: Enable guest PEBS for SPR and later models | expand |
Hi Like, There is a question I would like to ask. As far as I know, Alder Lake uses a hybrid architecture and the kernel presents two types of PMUs.Can the events created on the VCPU still count normally if the VCPU thread gets migrate across different CPUs? As far as I know, ARM64 big.LITTLE is not working properly, according to this set of patches. [PATCH v4 0/6] KVM: arm64: Improve PMU support on heterogeneous systems https://lore.kernel.org/all/20220127161759.53553-1-alexandru.elisei@arm.com/ Thanks， Kunkun Jiang On 2022/11/9 16:28, Like Xu wrote: > From: Like Xu <likexu@tencent.com> > > >From vPMU enabling perspective, KVM does not have proper support for > hybird x86 core. The reported perf_capabilities value (e.g. the format > of pebs record) depends on the type of cpu the kvm-intel module is init. > When a vcpu of one pebs format migrates to a vcpu of another pebs format, > the incorrect parsing of pebs records by guest can make profiling data > analysis extremely problematic. > > The safe way to fix this is to disable this part of the support until the > guest recognizes that it is running on the hybird cpu, which is appropriate > at the moment given that x86 hybrid architectures are not heavily touted > in the data center market. > > Signed-off-by: Like Xu <likexu@tencent.com> > --- > arch/x86/kvm/vmx/capabilities.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h > index cd2ac9536c99..ea0498684048 100644 > --- a/arch/x86/kvm/vmx/capabilities.h > +++ b/arch/x86/kvm/vmx/capabilities.h > @@ -392,7 +392,9 @@ static inline bool vmx_pt_mode_is_host_guest(void) > > static inline bool vmx_pebs_supported(void) > { > - return boot_cpu_has(X86_FEATURE_PEBS) && kvm_pmu_cap.pebs_ept; > + return boot_cpu_has(X86_FEATURE_PEBS) && > + !boot_cpu_has(X86_FEATURE_HYBRID_CPU) && > + kvm_pmu_cap.pebs_ept; > } > > static inline bool cpu_has_notify_vmexit(void)
On 25/11/2022 6:18 pm, Kunkun Jiang wrote: > Hi Like, > > There is a question I would like to ask. As far as I know, Alder Lake uses > a hybrid architecture and the kernel presents two types of PMUs.Can the > events created on the VCPU still count normally if the VCPU thread gets > migrate across different CPUs? The best answer is the test results as no one sponsored me a hybrid x86 box. According to my understanding, when a performance event (e.g. instructions) is supported on both types of pmu (even with different event codes), perf_event will remain enabled after the cpu migration (just changing the per-cpu context based on migrated pmu, and allocating another available hardware counter). Otherwise, the kernel will or should create and enable the perf_event based on current pmu type and disable the event of the previous cpu type. For the guest, KVM will or should recognize the migrated pmu type and enable the currently available perf_event for guest vPMC. But on hybrid x86, pmu capabilities are heterogeneous (even though the ISA is the same), and incompatible migrations can result in previous pmu capabilities (such as PEBS in this case) not being implemented on the new pmu, which breaks the expectation of the guest pmu driver. Making the guest aware of the differences in pmu types requires more fundamental KVM changes (for example, presenting multiple types of cpu model for the guest), and perhaps the simple and safe approach is to provide the guest with only the capabilities that are available to both pmu types. If things don't happen the way you expect them to, work it out w/ or w/o my help. > > As far as I know, ARM64 big.LITTLE is not working properly, according to > this set of patches. > [PATCH v4 0/6] KVM: arm64: Improve PMU support on heterogeneous systems > https://lore.kernel.org/all/20220127161759.53553-1-alexandru.elisei@arm.com/ The arm64 will have more cpu types (especially in terms of power management), but the difference in pmu capabilities will also depend on the design of IP vendors. > > Thanks， > Kunkun Jiang > > On 2022/11/9 16:28, Like Xu wrote: >> From: Like Xu <likexu@tencent.com> >> >> >From vPMU enabling perspective, KVM does not have proper support for >> hybird x86 core. The reported perf_capabilities value (e.g. the format >> of pebs record) depends on the type of cpu the kvm-intel module is init. >> When a vcpu of one pebs format migrates to a vcpu of another pebs format, >> the incorrect parsing of pebs records by guest can make profiling data >> analysis extremely problematic. >> >> The safe way to fix this is to disable this part of the support until the >> guest recognizes that it is running on the hybird cpu, which is appropriate >> at the moment given that x86 hybrid architectures are not heavily touted >> in the data center market. >> >> Signed-off-by: Like Xu <likexu@tencent.com> >> --- >> arch/x86/kvm/vmx/capabilities.h | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h >> index cd2ac9536c99..ea0498684048 100644 >> --- a/arch/x86/kvm/vmx/capabilities.h >> +++ b/arch/x86/kvm/vmx/capabilities.h >> @@ -392,7 +392,9 @@ static inline bool vmx_pt_mode_is_host_guest(void) >> static inline bool vmx_pebs_supported(void) >> { >> - return boot_cpu_has(X86_FEATURE_PEBS) && kvm_pmu_cap.pebs_ept; >> + return boot_cpu_has(X86_FEATURE_PEBS) && >> + !boot_cpu_has(X86_FEATURE_HYBRID_CPU) && >> + kvm_pmu_cap.pebs_ept; >> } >> static inline bool cpu_has_notify_vmexit(void)
On Wed, Nov 09, 2022, Like Xu wrote: > From: Like Xu <likexu@tencent.com> > > From vPMU enabling perspective, KVM does not have proper support for > hybird x86 core. The reported perf_capabilities value (e.g. the format > of pebs record) depends on the type of cpu the kvm-intel module is init. > When a vcpu of one pebs format migrates to a vcpu of another pebs format, > the incorrect parsing of pebs records by guest can make profiling data > analysis extremely problematic. > > The safe way to fix this is to disable this part of the support until the > guest recognizes that it is running on the hybird cpu, which is appropriate > at the moment given that x86 hybrid architectures are not heavily touted > in the data center market. > > Signed-off-by: Like Xu <likexu@tencent.com> > --- > arch/x86/kvm/vmx/capabilities.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h > index cd2ac9536c99..ea0498684048 100644 > --- a/arch/x86/kvm/vmx/capabilities.h > +++ b/arch/x86/kvm/vmx/capabilities.h > @@ -392,7 +392,9 @@ static inline bool vmx_pt_mode_is_host_guest(void) > > static inline bool vmx_pebs_supported(void) > { > - return boot_cpu_has(X86_FEATURE_PEBS) && kvm_pmu_cap.pebs_ept; > + return boot_cpu_has(X86_FEATURE_PEBS) && > + !boot_cpu_has(X86_FEATURE_HYBRID_CPU) && > + kvm_pmu_cap.pebs_ept; I assume the patch I just posted[*] to disable the vPMU entirely is sufficient, or do we need this as well in order to hide X86_FEATURE_DS and X86_FEATURE_DTES64? [*] https://lore.kernel.org/all/20230120004051.2043777-1-seanjc@google.com
On 20/1/2023 8:47 am, Sean Christopherson wrote: > On Wed, Nov 09, 2022, Like Xu wrote: >> From: Like Xu <likexu@tencent.com> >> >> From vPMU enabling perspective, KVM does not have proper support for >> hybird x86 core. The reported perf_capabilities value (e.g. the format >> of pebs record) depends on the type of cpu the kvm-intel module is init. >> When a vcpu of one pebs format migrates to a vcpu of another pebs format, >> the incorrect parsing of pebs records by guest can make profiling data >> analysis extremely problematic. >> >> The safe way to fix this is to disable this part of the support until the >> guest recognizes that it is running on the hybird cpu, which is appropriate >> at the moment given that x86 hybrid architectures are not heavily touted >> in the data center market. >> >> Signed-off-by: Like Xu <likexu@tencent.com> >> --- >> arch/x86/kvm/vmx/capabilities.h | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h >> index cd2ac9536c99..ea0498684048 100644 >> --- a/arch/x86/kvm/vmx/capabilities.h >> +++ b/arch/x86/kvm/vmx/capabilities.h >> @@ -392,7 +392,9 @@ static inline bool vmx_pt_mode_is_host_guest(void) >> >> static inline bool vmx_pebs_supported(void) >> { >> - return boot_cpu_has(X86_FEATURE_PEBS) && kvm_pmu_cap.pebs_ept; >> + return boot_cpu_has(X86_FEATURE_PEBS) && >> + !boot_cpu_has(X86_FEATURE_HYBRID_CPU) && >> + kvm_pmu_cap.pebs_ept; > > I assume the patch I just posted[*] to disable the vPMU entirely is sufficient, or AFAI, some developers doing client-side virtualization on a hybrid cpu will specifically want vPMU, in which case it makes perfect sense for KVM to expose common pmu capabilities (not PEBS at the current) of big and little cores, such as the most basic performance counter. > do we need this as well in order to hide X86_FEATURE_DS and X86_FEATURE_DTES64? I think we still need this diff. Better to prioritize this minor feature a little bit for hungry users. > > [*] https://lore.kernel.org/all/20230120004051.2043777-1-seanjc@google.com
On Mon, Jan 30, 2023, Like Xu wrote: > On 20/1/2023 8:47 am, Sean Christopherson wrote: > > On Wed, Nov 09, 2022, Like Xu wrote: > > > From: Like Xu <likexu@tencent.com> > > > > > > From vPMU enabling perspective, KVM does not have proper support for > > > hybird x86 core. The reported perf_capabilities value (e.g. the format > > > of pebs record) depends on the type of cpu the kvm-intel module is init. > > > When a vcpu of one pebs format migrates to a vcpu of another pebs format, > > > the incorrect parsing of pebs records by guest can make profiling data > > > analysis extremely problematic. > > > > > > The safe way to fix this is to disable this part of the support until the > > > guest recognizes that it is running on the hybird cpu, which is appropriate > > > at the moment given that x86 hybrid architectures are not heavily touted > > > in the data center market. > > > > > > Signed-off-by: Like Xu <likexu@tencent.com> > > > --- > > > arch/x86/kvm/vmx/capabilities.h | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h > > > index cd2ac9536c99..ea0498684048 100644 > > > --- a/arch/x86/kvm/vmx/capabilities.h > > > +++ b/arch/x86/kvm/vmx/capabilities.h > > > @@ -392,7 +392,9 @@ static inline bool vmx_pt_mode_is_host_guest(void) > > > static inline bool vmx_pebs_supported(void) > > > { > > > - return boot_cpu_has(X86_FEATURE_PEBS) && kvm_pmu_cap.pebs_ept; > > > + return boot_cpu_has(X86_FEATURE_PEBS) && > > > + !boot_cpu_has(X86_FEATURE_HYBRID_CPU) && > > > + kvm_pmu_cap.pebs_ept; > > > > I assume the patch I just posted[*] to disable the vPMU entirely is sufficient, or > > AFAI, some developers doing client-side virtualization on a hybrid cpu will > specifically want vPMU, > in which case it makes perfect sense for KVM to expose common pmu > capabilities (not PEBS at the current) of big and little cores, such as the > most basic performance counter. > > > do we need this as well in order to hide X86_FEATURE_DS and X86_FEATURE_DTES64? > > I think we still need this diff. Better to prioritize this minor feature a > little bit for hungry users. That wasn't my question. My question was whether or not wholesale disabling vPMU is sufficient to prevent issues with PEBS. Unless we need this patch on top of disabling the vPMU, my strong preference is to disable vPMU, or at the very least make it off-by-default and require a explicit override. I agree that there are users that want to enable vPMU for hybrid CPUs, but as stated in the link below, that needs to be a dedicated enabling effort. I don't see any reason to exempt PEBS from that. E.g. isn't PEBS usable if userspace pins vCPUs to pCPUs and enumerates an accurate topology to the guest? > > [*] https://lore.kernel.org/all/20230120004051.2043777-1-seanjc@google.com
On 31/1/2023 1:40 am, Sean Christopherson wrote: > On Mon, Jan 30, 2023, Like Xu wrote: >> On 20/1/2023 8:47 am, Sean Christopherson wrote: >>> On Wed, Nov 09, 2022, Like Xu wrote: >>>> From: Like Xu <likexu@tencent.com> >>>> >>>> From vPMU enabling perspective, KVM does not have proper support for >>>> hybird x86 core. The reported perf_capabilities value (e.g. the format >>>> of pebs record) depends on the type of cpu the kvm-intel module is init. >>>> When a vcpu of one pebs format migrates to a vcpu of another pebs format, >>>> the incorrect parsing of pebs records by guest can make profiling data >>>> analysis extremely problematic. >>>> >>>> The safe way to fix this is to disable this part of the support until the >>>> guest recognizes that it is running on the hybird cpu, which is appropriate >>>> at the moment given that x86 hybrid architectures are not heavily touted >>>> in the data center market. >>>> >>>> Signed-off-by: Like Xu <likexu@tencent.com> >>>> --- >>>> arch/x86/kvm/vmx/capabilities.h | 4 +++- >>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h >>>> index cd2ac9536c99..ea0498684048 100644 >>>> --- a/arch/x86/kvm/vmx/capabilities.h >>>> +++ b/arch/x86/kvm/vmx/capabilities.h >>>> @@ -392,7 +392,9 @@ static inline bool vmx_pt_mode_is_host_guest(void) >>>> static inline bool vmx_pebs_supported(void) >>>> { >>>> - return boot_cpu_has(X86_FEATURE_PEBS) && kvm_pmu_cap.pebs_ept; >>>> + return boot_cpu_has(X86_FEATURE_PEBS) && >>>> + !boot_cpu_has(X86_FEATURE_HYBRID_CPU) && >>>> + kvm_pmu_cap.pebs_ept; >>> >>> I assume the patch I just posted[*] to disable the vPMU entirely is sufficient, or >> >> AFAI, some developers doing client-side virtualization on a hybrid cpu will >> specifically want vPMU, >> in which case it makes perfect sense for KVM to expose common pmu >> capabilities (not PEBS at the current) of big and little cores, such as the >> most basic performance counter. >> >>> do we need this as well in order to hide X86_FEATURE_DS and X86_FEATURE_DTES64? >> >> I think we still need this diff. Better to prioritize this minor feature a >> little bit for hungry users. > > That wasn't my question. My question was whether or not wholesale disabling vPMU > is sufficient to prevent issues with PEBS. Unless we need this patch on top of > disabling the vPMU, my strong preference is to disable vPMU, or at the very least > make it off-by-default and require a explicit override. OK and if so, just set global module parameter "enable_pmu=false" for HYBRID_CPU. With "disable vPMU" diff, this patch should be dropped since kvm_pmu_cap.pebs_ept = 0. > > I agree that there are users that want to enable vPMU for hybrid CPUs, but as > stated in the link below, that needs to be a dedicated enabling effort. I don't > see any reason to exempt PEBS from that. E.g. isn't PEBS usable if userspace pins > vCPUs to pCPUs and enumerates an accurate topology to the guest? So for HYBRID_CPU, {pebs, lbr, basic PMU} would be disabled globally by KVM until a dedicated effort enables them one by one in the near future. Follow up with a rewritten diff, 20230131085031.88939-1-likexu@tencent.com > >>> [*] https://lore.kernel.org/all/20230120004051.2043777-1-seanjc@google.com
diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h index cd2ac9536c99..ea0498684048 100644 --- a/arch/x86/kvm/vmx/capabilities.h +++ b/arch/x86/kvm/vmx/capabilities.h @@ -392,7 +392,9 @@ static inline bool vmx_pt_mode_is_host_guest(void) static inline bool vmx_pebs_supported(void) { - return boot_cpu_has(X86_FEATURE_PEBS) && kvm_pmu_cap.pebs_ept; + return boot_cpu_has(X86_FEATURE_PEBS) && + !boot_cpu_has(X86_FEATURE_HYBRID_CPU) && + kvm_pmu_cap.pebs_ept; } static inline bool cpu_has_notify_vmexit(void)