Re: [RFC 3/3] x86: KVM: stats: Add a stat counter for GALog events

[Chao Gao <chao.gao@intel.com>]

On Tue, Apr 09, 2024 at 09:31:45PM -0400, Alejandro Jimenez wrote:
>
>On 4/9/24 02:45, Chao Gao wrote:
>> > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
>> > index 4b74ea91f4e6..853cafe4a9af 100644
>> > --- a/arch/x86/kvm/svm/avic.c
>> > +++ b/arch/x86/kvm/svm/avic.c
>> > @@ -165,8 +165,10 @@ int avic_ga_log_notifier(u32 ga_tag)
>> > 	 * bit in the vAPIC backing page. So, we just need to schedule
>> > 	 * in the vcpu.
>> > 	 */
>> > -	if (vcpu)
>> > +	if (vcpu) {
>> > 		kvm_vcpu_wake_up(vcpu);
>> > +		++vcpu->stat.ga_log_event;
>> > +	}
>> > 
>> 
>> I am not sure why this is added for SVM only.
>
>I am mostly familiar with AVIC, and much less so with VMX's PI, so this is
>why I am likely missing potential stats that could be useful to expose from
>the VMX  side. I'll be glad to implement any other suggestions you have.
>
>
>it looks to me GALog events are
>> similar to Intel IOMMU's wakeup events. Can we have a general name? maybe
>> iommu_wakeup_event
>
>I believe that after:
>d588bb9be1da ("KVM: VMX: enable IPI virtualization")
>
>both the VT-d PI and the virtualized IPIs code paths will use POSTED_INTR_WAKEUP_VECTOR
>for interrupts targeting a blocked vCPU. So on Intel hosts enabling IPI virtualization,
>a counter incremented in pi_wakeup_handler() would record interrupts from both virtualized
>IPIs and VT-d sources.
>
>I don't think it is correct to generalize this counter since AMD's implementation is
>different; when a blocked vCPU is targeted:
>
>- by device interrupts, it uses the GA Log mechanism
>- by an IPI, it generates an AVIC_INCOMPLETE_IPI #VMEXIT
>
>If the reasoning above is correct, we can add a VMX specific counter (vmx_pi_wakeup_event?)
>that is increased in pi_wakeup_handler() as you suggest, and document the difference
>in behavior so that is not confused as equivalent with the ga_log_event counter.

Correct. If we cannot generalize the counter, I think it is ok to
add the counter for SVM only. Thank you for the clarification.