Subject: Re: [PATCH 17/43] KVM: arm64: gic-v5: Enable VPE DBs on VPE reset and disable on teardown

From: Marc Zyngier <[email-masked]>
Date: Wed, 06 May 2026 16:03:53 +0100

On Mon, 27 Apr 2026 17:11:50 +0100,
Sascha Bischoff <[email-masked]> wrote:
> 
> Ensure that each VPE doorbell is enabled on the host prior to running
> the guest by enabling it as part of vgic_v5_reset(). Add a function to
> disable it again as part of VM teardown, vgic_v5_disable_vcpu(), which
> also frees the IRQ again.
> 
> Signed-off-by: Sascha Bischoff <[email-masked]>
> ---
>  arch/arm64/kvm/vgic/vgic-v5.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> index 9347bc6895223..48ffcb2494db3 100644
> --- a/arch/arm64/kvm/vgic/vgic-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5.c
> @@ -736,6 +736,31 @@ void vgic_v5_reset(struct kvm_vcpu *vcpu)
>  		kvm_vm_dead(vcpu->kvm);
>  		return;
>  	}
> +
> +	enable_irq(vgic_v5_vpe_db(vcpu));

Odd. Why do you need to manage the enabling of the doorbell? I see
from the previous patch that you set it as IRQ_NOAUTOEN, but this
seems to be cargo-culted from GICv4 (which really needs it).

My expectations that since the doorbell is controlled from KVM and is
only one-shot, we should be OK. This is the flow that v4.1 adopts,
without a need to baby-sit the interrupt state.

> +}
> +
> +static void vgic_v5_disable_vcpu(struct kvm_vcpu *vcpu)
> +{
> +	int virq = vgic_v5_vpe_db(vcpu);
> +
> +	if (!vcpu->kvm->arch.vgic.gicv5_vm.domain)
> +		return;
> +
> +	if (!virq)
> +		return;
> +
> +	/*
> +	 * We are called in the vgic_v5_teardown path. We no longer need the
> +	 * doorbell virqs.
> +	 */
> +	disable_irq(virq);

And then this can go as well.

> +
> +	/* Free the doorbell irq (counter-part to request_irq)*/
> +	free_irq(virq, vcpu);
> +
> +	/* Remove the irq from the domain too */
> +	irq_domain_free_irqs(virq, 1);

Surely you remove the domain at some point. That should cover the
freeing of all the doorbells.

>  }
>  
>  int vgic_v5_map_resources(struct kvm *kvm)
> @@ -875,6 +900,10 @@ void vgic_v5_teardown(struct kvm *kvm)
>  			kvm_err("Failed to release VM 0x%x\n", dist->gicv5_vm.vm_id);
>  	}
>  
> +	/* Goodbye doorbells */
> +	kvm_for_each_vcpu(i, vcpu, kvm)
> +		vgic_v5_disable_vcpu(vcpu);
> +
>  	vgic_v5_teardown_per_vm_domain(&kvm->arch.vgic.gicv5_vm);
>  
>  	vgic_v5_release_vm_id(kvm);

I reckon this patch would be better folded into the previous one, so
that we can see both the requesting and freeing of the doorbell
interrupt in the same patch.

Thanks,

	M.

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