Messages in this thread | | | Date | Tue, 31 Jan 2023 16:53:51 +0800 | Subject | Re: [PATCH v3 1/3] KVM: x86/pmu: Disable guest PEBS on hybird cpu due to heterogeneity | From | Like Xu <> |
| |
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
| |