[PATCH v3 3/5] KVM: arm64: vgic-v3: Separate redistributor teardown from unassignment

Karl Mehltretter <[email protected]>
Newsgroups org.kernel.vger.linux-kselftest,dev.linux.lists.kvmarm,org.infradead.lists.linux-arm-kernel,org.kernel.vger.kvm,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
MMIO-bus unregistration may synchronize SRCU and must run outside
config_lock. Conversely, clearing the redistributor assignment needs
config_lock, and teardown must do so before freeing the redistributor
regions.

Introduce an already-locked unassignment primitive that only clears the
cached region and base address. It deliberately does not adjust free_index:
failure rollback resets all region counters, while VM teardown frees the
regions. Keep MMIO-bus unregistration separate. Unregister devices before
taking config_lock in VM teardown, then unassign the vCPUs before freeing
their regions.

Move redistributor cleanup out of __kvm_vgic_vcpu_destroy() and into its
outer wrapper. This preserves failed-vCPU cleanup before its memory can be
freed, without the special conditional in the common destructor. The region
destructor no longer needs to scan the vCPUs.

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    | 46 +++++++++++-------------------
 arch/arm64/kvm/vgic/vgic-mmio-v3.c | 16 ++---------
 arch/arm64/kvm/vgic/vgic.h         |  1 +
 3 files changed, 21 insertions(+), 42 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index 0a3df6d3a691..a0d72b540331 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -523,29 +523,6 @@ static void __kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu)
 	INIT_LIST_HEAD(&vgic_cpu->ap_list_head);
 	kfree(vgic_cpu->private_irqs);
 	vgic_cpu->private_irqs = NULL;
-
-	if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
-		/*
-		 * If this vCPU is being destroyed because of a failed creation
-		 * then unregister the redistributor to avoid leaving behind a
-		 * dangling pointer to the vCPU struct.
-		 *
-		 * vCPUs that have been successfully created (i.e. added to
-		 * kvm->vcpu_array) get unregistered in kvm_vgic_destroy(), as
-		 * this function gets called while holding kvm->arch.config_lock
-		 * in the VM teardown path and would otherwise introduce a lock
-		 * inversion w.r.t. kvm->srcu.
-		 *
-		 * vCPUs that failed creation are torn down outside of the
-		 * kvm->arch.config_lock and do not get unregistered in
-		 * kvm_vgic_destroy(), meaning it is both safe and necessary to
-		 * do so here.
-		 */
-		if (kvm_get_vcpu_by_id(vcpu->kvm, vcpu->vcpu_id) != vcpu)
-			vgic_unregister_redist_iodev(vcpu);
-
-		vgic_cpu->rd_iodev.base_addr = VGIC_ADDR_UNDEF;
-	}
 }
 
 void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu)
@@ -553,7 +530,16 @@ void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu)
 	struct kvm *kvm = vcpu->kvm;
 
 	mutex_lock(&kvm->slots_lock);
+	if (kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
+		vgic_unregister_redist_iodev(vcpu);
+
 	__kvm_vgic_vcpu_destroy(vcpu);
+
+	if (kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
+		mutex_lock(&kvm->arch.config_lock);
+		__vgic_unassign_redist_iodev(vcpu);
+		mutex_unlock(&kvm->arch.config_lock);
+	}
 	mutex_unlock(&kvm->slots_lock);
 }
 
@@ -563,21 +549,23 @@ void kvm_vgic_destroy(struct kvm *kvm)
 	unsigned long i;
 
 	mutex_lock(&kvm->slots_lock);
+	if (kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
+		kvm_for_each_vcpu(i, vcpu, kvm)
+			vgic_unregister_redist_iodev(vcpu);
+
 	mutex_lock(&kvm->arch.config_lock);
 
 	vgic_debug_destroy(kvm);
 
-	kvm_for_each_vcpu(i, vcpu, 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)
+			__vgic_unassign_redist_iodev(vcpu);
+	}
 
 	kvm_vgic_dist_destroy(kvm);
 
 	mutex_unlock(&kvm->arch.config_lock);
-
-	if (kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
-		kvm_for_each_vcpu(i, vcpu, kvm)
-			vgic_unregister_redist_iodev(vcpu);
-
 	mutex_unlock(&kvm->slots_lock);
 }
 
diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
index 6c009deb11d4..dc860178105d 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -775,8 +775,7 @@ static void vgic_undo_redist_assignment(struct kvm_vcpu *vcpu)
 	guard(mutex)(&vcpu->kvm->arch.config_lock);
 
 	vgic_cpu->rdreg->free_index--;
-	vgic_cpu->rdreg = NULL;
-	vgic_cpu->rd_iodev.base_addr = VGIC_ADDR_UNDEF;
+	__vgic_unassign_redist_iodev(vcpu);
 }
 
 /**
@@ -855,7 +854,7 @@ void vgic_unregister_redist_iodev(struct kvm_vcpu *vcpu)
 	kvm_io_bus_unregister_dev(vcpu->kvm, KVM_MMIO_BUS, &rd_dev->dev);
 }
 
-static void vgic_reset_redist_iodev(struct kvm_vcpu *vcpu)
+void __vgic_unassign_redist_iodev(struct kvm_vcpu *vcpu)
 {
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 
@@ -881,7 +880,7 @@ static void vgic_v3_rollback_redist_region(struct kvm *kvm, u32 index)
 	guard(mutex)(&kvm->arch.config_lock);
 
 	kvm_for_each_vcpu(c, vcpu, kvm)
-		vgic_reset_redist_iodev(vcpu);
+		__vgic_unassign_redist_iodev(vcpu);
 
 	list_for_each_entry(iter, &kvm->arch.vgic.rd_regions, list)
 		iter->free_index = 0;
@@ -991,17 +990,8 @@ static int vgic_v3_alloc_redist_region(struct kvm *kvm, uint32_t index,
 
 void vgic_v3_free_redist_region(struct kvm *kvm, struct vgic_redist_region *rdreg)
 {
-	struct kvm_vcpu *vcpu;
-	unsigned long c;
-
 	lockdep_assert_held(&kvm->arch.config_lock);
 
-	/* Garbage collect the region */
-	kvm_for_each_vcpu(c, vcpu, kvm) {
-		if (vcpu->arch.vgic_cpu.rdreg == rdreg)
-			vcpu->arch.vgic_cpu.rdreg = NULL;
-	}
-
 	list_del(&rdreg->list);
 	kfree(rdreg);
 }
diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
index b71d486ae514..1a2e40004a47 100644
--- a/arch/arm64/kvm/vgic/vgic.h
+++ b/arch/arm64/kvm/vgic/vgic.h
@@ -350,6 +350,7 @@ int vgic_v3_save_pending_tables(struct kvm *kvm);
 int vgic_v3_set_redist_base(struct kvm *kvm, u32 index, u64 addr, u32 count);
 int vgic_register_redist_iodev(struct kvm_vcpu *vcpu);
 void vgic_unregister_redist_iodev(struct kvm_vcpu *vcpu);
+void __vgic_unassign_redist_iodev(struct kvm_vcpu *vcpu);
 bool vgic_v3_check_base(struct kvm *kvm);
 
 void vgic_v3_load(struct kvm_vcpu *vcpu);
-- 
2.39.5 (Apple Git-154)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.