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

[Marc Zyngier <maz@kernel.org>]

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.

> 
> 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>

	M.

-- 
Without deviation from the norm, progress is not possible.