Re: [PATCH v2 3/5] KVM: arm64: vgic-v3: Unregister redistributors before freeing regions

On Wed, 19 Aug 2026 23:42:27 +0100,
Karl Mehltretter <[email protected]> wrote:
> 
> vgic_unregister_redist_iodev() uses rdreg to return its region slot. During
> VM teardown, kvm_vgic_destroy() frees the regions before unregistering
> their iodevs, so vgic_v3_free_redist_region() must first clear every rdreg
> pointer.
> 
> Unregister the iodevs before kvm_vgic_dist_destroy(), while the regions are
> still alive. Temporarily drop config_lock because
> vgic_unregister_redist_iodev() takes it; slots_lock remains held for the
> MMIO bus updates. The region destructor no longer needs the vCPU scan.
> 
> Suggested-by: Marc Zyngier <[email protected]>
> Assisted-by: Codex:gpt-5.6-sol
> Signed-off-by: Karl Mehltretter <[email protected]>
> ---
>  arch/arm64/kvm/vgic/vgic-init.c    | 11 +++++++----
>  arch/arm64/kvm/vgic/vgic-mmio-v3.c |  9 ---------
>  2 files changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> index 0a3df6d3a691..df6d0bdda4a9 100644
> --- a/arch/arm64/kvm/vgic/vgic-init.c
> +++ b/arch/arm64/kvm/vgic/vgic-init.c
> @@ -570,14 +570,17 @@ void kvm_vgic_destroy(struct kvm *kvm)
>  	kvm_for_each_vcpu(i, vcpu, kvm)
>  		__kvm_vgic_vcpu_destroy(vcpu);
>  
> +	if (kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
> +		mutex_unlock(&kvm->arch.config_lock);
> +		kvm_for_each_vcpu(i, vcpu, kvm)
> +			vgic_unregister_redist_iodev(vcpu);
> +		mutex_lock(&kvm->arch.config_lock);
> +	}

This really is awful. It'd be easy enough to have an "already locked"
primitive (__vgic_unassign_redist_iodev()) that doesn't take the
config_lock, and simply write this as:

	kvm_for_each_vcpu(i, vcpu, kvm) {
	 	struct vgic_io_device *rd_dev = &vcpu->arch.vgic_cpu.rd_iodev;
 		kvm_io_bus_unregister_dev(vcpu->kvm, KVM_MMIO_BUS, &rd_dev->dev);
		__vgic_unassign_redist_iodev(vcpu);
	}

or another variation on the same theme.

The other bit of interest is in __kvm_vgic_vcpu_destroy(), and its
conditional removal of the RD iodevs. Can't we just get rid of this
whole block altogether now that we have a saner failing path?

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.