Re: The vcpu won't be wakened for a long time

From: Sean Christopherson <seanjc@google.com>
To: Chao Gao <chao.gao@intel.com>
Cc: "Longpeng (Mike,
	Cloud Infrastructure Service Product Dept.)" 
	<longpeng2@huawei.com>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"Gonglei (Arei)" <arei.gonglei@huawei.com>,
	Huangzhichao <huangzhichao@huawei.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: The vcpu won't be wakened for a long time
Date: Wed, 22 Dec 2021 15:44:41 +0000	[thread overview]
Message-ID: <YcNH6RDemrxMUi15@google.com> (raw)
In-Reply-To: <20211222060735.GA22521@gao-cwp>

On Wed, Dec 22, 2021, Chao Gao wrote:
> On Tue, Dec 21, 2021 at 03:27:01PM +0000, Sean Christopherson wrote:
> >On Sat, Dec 18, 2021, Longpeng (Mike, Cloud Infrastructure Service Product Dept.) wrote:
> >> > Hmm, that strongly suggests the "vcpu != kvm_get_running_vcpu()" is at fault.
> >> > Can you try running with the below commit?  It's currently sitting in kvm/queue,
> >> > but not marked for stable because I didn't think it was possible for the check
> >> > to a cause a missed wake event in KVM's current code base.
> >> > 
> >> 
> >> The below commit can fix the bug, we have just completed  the tests.
> >> Thanks.
> >
> >Aha!  Somehow I missed this call chain when analyzing the change.
> >
> >  irqfd_wakeup()
> >  |
> >  |->kvm_arch_set_irq_inatomic()
> >     |
> >     |-> kvm_irq_delivery_to_apic_fast()
> >         |
> >	 |-> kvm_apic_set_irq()
> >
> >
> >Paolo, can the changelog be amended to the below, and maybe even pull the commit
> >into 5.16?
> >
> >
> >KVM: VMX: Wake vCPU when delivering posted IRQ even if vCPU == this vCPU
> >
> >Drop a check that guards triggering a posted interrupt on the currently
> >running vCPU,
> 
> Can we move (add) this check to kvm_vcpu_trigger_posted_interrupt()?
>
> 	if (vcpu->mode == IN_GUEST_MODE) {
> [...]
> -		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 true;
> 
> It can achieve the purpose of the original patch without (re-)introducing
> this bug.

Hmm, yes, I think that would be safe and would optimize delivery of TSC deadline
timer interrupts when they're emulated via the VMX preemption timer.  The original
patch confused me because the optimization went in before the code it was optimizing.