Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Message-ID: <c3f0e4ac-1790-40c1-a09e-209a09e3d230@linux.intel.com>
Date:   Wed, 1 Nov 2023 09:59:24 +0800
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [Patch 1/2] KVM: x86/pmu: Add Intel CPUID-hinted TopDown slots
 event
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,
        Zhenyu Wang <zhenyuw@linux.intel.com>,
        Zhang Xiong <xiong.y.zhang@intel.com>,
        Mingwei Zhang <mizhang@google.com>,
        Like Xu <like.xu.linux@gmail.com>,
        Dapeng Mi <dapeng1.mi@intel.com>, Like Xu <likexu@tencent.com>
References: <20231031090613.2872700-1-dapeng1.mi@linux.intel.com>
 <20231031090613.2872700-2-dapeng1.mi@linux.intel.com>
 <CALMp9eR_BFdNNTXhSpbuH66jXcRLVB8VvD8V+kY245NbusN2+g@mail.gmail.com>
From:   "Mi, Dapeng" <dapeng1.mi@linux.intel.com>
In-Reply-To: <CALMp9eR_BFdNNTXhSpbuH66jXcRLVB8VvD8V+kY245NbusN2+g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 11/1/2023 2:22 AM, Jim Mattson wrote:
> On Tue, Oct 31, 2023 at 1:58 AM Dapeng Mi <dapeng1.mi@linux.intel.com> wrote:
>> This patch adds support for the architectural topdown slots event which
>> is hinted by CPUID.0AH.EBX.
> Can't a guest already program an event selector to count event select
> 0xa4, unit mask 1, unless the event is prohibited by
> KVM_SET_PMU_EVENT_FILTER?

Actually defining this new slots arch event is to do the sanity check 
for supported arch-events which is enumerated by CPUID.0AH.EBX. 
Currently vPMU would check if the arch event from guest is supported by 
KVM. If not, it would be rejected just like intel_hw_event_available() 
shows.

If we don't add the slots event in the intel_arch_events[] array, guest 
may program the slots event and pass the sanity check of KVM on a 
platform which actually doesn't support slots event and program the 
event on a real GP counter and got an invalid count. This is not correct.


>
> AFAICT, this change just enables event filtering based on
> CPUID.0AH:EBX[bit 7] (though it's not clear to me why two independent
> mechanisms are necessary for event filtering).


IMO, these are two different things. this change is just to enable the 
supported arch events check for slot events, the event filtering is 
another thing.