Message-ID: <617d9a3f-63a2-3ce2-b19a-2427ebbb7754@gmail.com>
Date:   Tue, 31 Jan 2023 16:53:51 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.6.1
Subject: Re: [PATCH v3 1/3] KVM: x86/pmu: Disable guest PEBS on hybird cpu due
 to heterogeneity
Content-Language: en-US
To:     Sean Christopherson <seanjc@google.com>
Cc:     Paolo Bonzini <pbonzini@redhat.com>, linux-kernel@vger.kernel.org,
        kvm@vger.kernel.org
References: <20221109082802.27543-1-likexu@tencent.com>
 <20221109082802.27543-2-likexu@tencent.com> <Y8nknyxfKl4p/0GY@google.com>
 <9b422d58-72ab-051f-e317-02b4d8e7211d@gmail.com>
 <Y9gA9aZNxYAZGgPh@google.com>
From:   Like Xu <like.xu.linux@gmail.com>
In-Reply-To: <Y9gA9aZNxYAZGgPh@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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