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: <ac5ff45b-ba7d-6548-e133-a3c77ea7c713@redhat.com>
Date:   Tue, 1 Feb 2022 13:11:59 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH RESEND v2] KVM: VMX: Dont' send posted IRQ if vCPU == this
 vCPU and vCPU is IN_GUEST_MODE
Content-Language: en-US
To:     Wanpeng Li <kernellwp@gmail.com>, linux-kernel@vger.kernel.org,
        kvm@vger.kernel.org
Cc:     Sean Christopherson <seanjc@google.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>
References: <1643111979-36447-1-git-send-email-wanpengli@tencent.com>
From:   Paolo Bonzini <pbonzini@redhat.com>
In-Reply-To: <1643111979-36447-1-git-send-email-wanpengli@tencent.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 1/25/22 12:59, Wanpeng Li wrote:
> From: Wanpeng Li <wanpengli@tencent.com>
> 
> When delivering a virtual interrupt, don't actually send a posted interrupt
> if the target vCPU is also the currently running vCPU and is IN_GUEST_MODE,
> in which case the interrupt is being sent from a VM-Exit fastpath and the
> core run loop in vcpu_enter_guest() will manually move the interrupt from
> the PIR to vmcs.GUEST_RVI.  IRQs are disabled while IN_GUEST_MODE, thus
> there's no possibility of the virtual interrupt being sent from anything
> other than KVM, i.e. KVM won't suppress a wake event from an IRQ handler
> (see commit fdba608f15e2, "KVM: VMX: Wake vCPU when delivering posted IRQ
> even if vCPU == this vCPU").
> 
> Eliding the posted interrupt restores the performance provided by the
> combination of commits 379a3c8ee444 ("KVM: VMX: Optimize posted-interrupt
> delivery for timer fastpath") and 26efe2fd92e5 ("KVM: VMX: Handle
> preemption timer fastpath").
> 
> Thanks Sean for better comments.
> 
> Suggested-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> ---
>   arch/x86/kvm/vmx/vmx.c | 40 +++++++++++++++++++++-------------------
>   1 file changed, 21 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index fe06b02994e6..e06377c9a4cf 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -3908,31 +3908,33 @@ static inline void kvm_vcpu_trigger_posted_interrupt(struct kvm_vcpu *vcpu,
>   #ifdef CONFIG_SMP
>   	if (vcpu->mode == IN_GUEST_MODE) {
>   		/*
> -		 * The vector of interrupt to be delivered to vcpu had
> -		 * been set in PIR before this function.
> +		 * The vector of the virtual has already been set in the PIR.
> +		 * Send a notification event to deliver the virtual interrupt
> +		 * unless the vCPU is the currently running vCPU, i.e. the
> +		 * event is being sent from a fastpath VM-Exit handler, in
> +		 * which case the PIR will be synced to the vIRR before
> +		 * re-entering the guest.
>   		 *
> -		 * Following cases will be reached in this block, and
> -		 * we always send a notification event in all cases as
> -		 * explained below.
> +		 * When the target is not the running vCPU, the following
> +		 * possibilities emerge:
>   		 *
> -		 * Case 1: vcpu keeps in non-root mode. Sending a
> -		 * notification event posts the interrupt to vcpu.
> +		 * Case 1: vCPU stays in non-root mode. Sending a notification
> +		 * event posts the interrupt to the vCPU.
>   		 *
> -		 * Case 2: vcpu exits to root mode and is still
> -		 * runnable. PIR will be synced to vIRR before the
> -		 * next vcpu entry. Sending a notification event in
> -		 * this case has no effect, as vcpu is not in root
> -		 * mode.
> +		 * Case 2: vCPU exits to root mode and is still runnable. The
> +		 * PIR will be synced to the vIRR before re-entering the guest.
> +		 * Sending a notification event is ok as the host IRQ handler
> +		 * will ignore the spurious event.
>   		 *
> -		 * Case 3: vcpu exits to root mode and is blocked.
> -		 * vcpu_block() has already synced PIR to vIRR and
> -		 * never blocks vcpu if vIRR is not cleared. Therefore,
> -		 * a blocked vcpu here does not wait for any requested
> -		 * interrupts in PIR, and sending a notification event
> -		 * which has no effect is safe here.
> +		 * Case 3: vCPU exits to root mode and is blocked. vcpu_block()
> +		 * has already synced PIR to vIRR and never blocks the vCPU if
> +		 * the vIRR is not empty. Therefore, a blocked vCPU here does
> +		 * not wait for any requested interrupts in PIR, and sending a
> +		 * notification event also results in a benign, spurious event.
>   		 */
>   
> -		apic->send_IPI_mask(get_cpu_mask(vcpu->cpu), pi_vec);
> +		if (vcpu != kvm_get_running_vcpu())
> +			apic->send_IPI_mask(get_cpu_mask(vcpu->cpu), pi_vec);
>   		return;
>   	}
>   #endif

Queued, thanks.

Paolo