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.