Re: [PATCH] KVM: arm64: vgic-v4: Consistently request doorbell irq for blocking vCPU

[Oliver Upton <oliver.upton@linux.dev>]

On Tue, Jul 11, 2023 at 08:23:25AM +0100, Marc Zyngier wrote:
> On Mon, 10 Jul 2023 18:55:53 +0100,
> Oliver Upton <oliver.upton@linux.dev> wrote:
> > 
> > Xiang reports that VMs occasionally fail to boot on GICv4.1 systems when
> > running a preemptible kernel, as it is possible that a vCPU is blocked
> > without requesting a doorbell interrupt.
> > 
> > The issue is that any preemption that occurs between vgic_v4_put() and
> > schedule() on the block path will mark the vPE as nonresident and *not*
> > request a doorbell irq.
> 
> It'd be worth spelling out. You need to go via *three* schedule()
> calls: one to be preempted (with DB set), one to be made resident
> again, and then the final one in kvm_vcpu_halt(), clearing the DB on
> vcpu_put() due to the bug.

Yeah, a bit lazy in the wording. What I had meant to imply was
preemption happening after the doorbell is set up and before the thread
has an opportunity to explicitly schedule out. Perhaps I should just say
that.

> > 
> > Fix it by consistently requesting a doorbell irq in the vcpu put path if
> > the vCPU is blocking. While this technically means we could drop the
> > early doorbell irq request in kvm_vcpu_wfi(), deliberately leave it
> > intact such that vCPU halt polling can properly detect the wakeup
> > condition before actually scheduling out a vCPU.
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: 8e01d9a396e6 ("KVM: arm64: vgic-v4: Move the GICv4 residency flow to be driven by vcpu_load/put")
> > Reported-by: Xiang Chen <chenxiang66@hisilicon.com>
> > Tested-by: Xiang Chen <chenxiang66@hisilicon.com>
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
> >  arch/arm64/kvm/vgic/vgic-v3.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
> > index c3b8e132d599..8c467e9f4f11 100644
> > --- a/arch/arm64/kvm/vgic/vgic-v3.c
> > +++ b/arch/arm64/kvm/vgic/vgic-v3.c
> > @@ -749,7 +749,7 @@ void vgic_v3_put(struct kvm_vcpu *vcpu)
> >  {
> >  	struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3;
> >  
> > -	WARN_ON(vgic_v4_put(vcpu, false));
> > +	WARN_ON(vgic_v4_put(vcpu, kvm_vcpu_is_blocking(vcpu)));
> >  
> >  	vgic_v3_vmcr_sync(vcpu);
> >  
> 
> Other than the above nitpicking, this looks good. Thanks both for the
> very detailed report and the fix.
> 
> Reviewed-by: Marc Zyngier <maz@kernel.org>

Thanks!

--
Best,
Oliver