Re: [PATCH v2 3/5] KVM: arm64: vgic-v3: Unregister redistributors before freeing regions
From: Marc Zyngier <maz@kernel.org>
Date: 2026-08-20 08:56:28
Also in:
kvm, kvmarm, linux-kselftest, lkml
On Wed, 19 Aug 2026 23:42:27 +0100, Karl Mehltretter [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 <maz@kernel.org> Assisted-by: Codex:gpt-5.6-sol Signed-off-by: Karl Mehltretter <redacted> --- 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.