Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9a7d5814-9eb1-d7af-7968-a6e3ebb90248@gmail.com>
Date:   Tue, 11 Apr 2023 14:17:14 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.9.1
Subject: Re: [PATCH V5 05/10] KVM: x86/pmu: Disable vPMU if the minimum num of
 counters isn't met
Content-Language: en-US
To:     Jim Mattson <jmattson@google.com>
Cc:     Sean Christopherson <seanjc@google.com>,
        Paolo Bonzini <pbonzini@redhat.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20230410105056.60973-1-likexu@tencent.com>
 <20230410105056.60973-6-likexu@tencent.com>
 <CALMp9eTLvJ6GW1mfgjO7CL7tW-79asykyz9=Fb=FfT74VRkDVA@mail.gmail.com>
From:   Like Xu <like.xu.linux@gmail.com>
In-Reply-To: <CALMp9eTLvJ6GW1mfgjO7CL7tW-79asykyz9=Fb=FfT74VRkDVA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 11/4/2023 1:36 pm, Jim Mattson wrote:
> On Mon, Apr 10, 2023 at 3:51 AM Like Xu <like.xu.linux@gmail.com> wrote:
>>
>> From: Like Xu <likexu@tencent.com>
>>
>> Disable PMU support when running on AMD and perf reports fewer than four
>> general purpose counters. All AMD PMUs must define at least four counters
>> due to AMD's legacy architecture hardcoding the number of counters
>> without providing a way to enumerate the number of counters to software,
>> e.g. from AMD's APM:
>>
>>   The legacy architecture defines four performance counters (PerfCtrn)
>>   and corresponding event-select registers (PerfEvtSeln).
>>
>> Virtualizing fewer than four counters can lead to guest instability as
>> software expects four counters to be available.
> 
> I'm confused. Isn't zero less than four?

As I understand it, you are saying that virtualization of zero counter is also 
reasonable.
If so, the above statement could be refined as:

	Virtualizing fewer than four counters when vPMU is enabled may lead to guest 
instability
	as software expects at least four counters to be available, thus the vPMU is 
disabled if the
	minimum number of KVM supported counters is not reached during initialization.

Jim, does this help you or could you explain more about your confusion ?

> 
>> Suggested-by: Sean Christopherson <seanjc@google.com>
>> Signed-off-by: Like Xu <likexu@tencent.com>
>> ---
>>   arch/x86/kvm/pmu.h | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/x86/kvm/pmu.h b/arch/x86/kvm/pmu.h
>> index dd7c7d4ffe3b..002b527360f4 100644
>> --- a/arch/x86/kvm/pmu.h
>> +++ b/arch/x86/kvm/pmu.h
>> @@ -182,6 +182,9 @@ static inline void kvm_init_pmu_capability(const struct kvm_pmu_ops *pmu_ops)
>>                          enable_pmu = false;
>>          }
>>
>> +       if (!is_intel && kvm_pmu_cap.num_counters_gp < AMD64_NUM_COUNTERS)
>> +               enable_pmu = false;
>> +
>>          if (!enable_pmu) {
>>                  memset(&kvm_pmu_cap, 0, sizeof(kvm_pmu_cap));
>>                  return;
>> --
>> 2.40.0
>>