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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZhkQsqRjy1ba+mRm@chao-email>
Date: Fri, 12 Apr 2024 18:45:06 +0800
From: Chao Gao <chao.gao@...el.com>
To: Alejandro Jimenez <alejandro.j.jimenez@...cle.com>
CC: <kvm@...r.kernel.org>, <seanjc@...gle.com>, <pbonzini@...hat.com>,
	<linux-kernel@...r.kernel.org>, <joao.m.martins@...cle.com>,
	<boris.ostrovsky@...cle.com>, <mark.kanda@...cle.com>,
	<suravee.suthikulpanit@....com>, <mlevitsk@...hat.com>
Subject: Re: [RFC 3/3] x86: KVM: stats: Add a stat counter for GALog events

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.