Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Karl Mehltretter <kmehltretter@gmail.com>
To: Marc Zyngier <maz@kernel.org>, Oliver Upton <oupton@kernel.org>,
	kvmarm@lists.linux.dev
Cc: Karl Mehltretter <kmehltretter@gmail.com>,
	Fuad Tabba <fuad.tabba@linux.dev>,
	Joey Gouly <joey.gouly@arm.com>,
	Steffen Eiden <seiden@linux.ibm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Zenghui Yu <yuzenghui@huawei.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Shuah Khan <shuah@kernel.org>, Eric Auger <eric.auger@redhat.com>,
	Christoffer Dall <christoffer.dall@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	linux-kselftest@vger.kernel.org
Subject: [PATCH v3 3/5] KVM: arm64: vgic-v3: Separate redistributor teardown from unassignment
Date: Sat, 22 Aug 2026 11:53:44 +0200	[thread overview]
Message-ID: <20260822095346.53882-4-kmehltretter@gmail.com> (raw)
In-Reply-To: <20260822095346.53882-1-kmehltretter@gmail.com>

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 <maz@kernel.org>
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 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)



  parent reply	other threads:[~2026-08-22  9:54 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-22  9:53 [PATCH v3 0/5] KVM: arm64: fix VGICv3 redistributor rollback Karl Mehltretter
2026-08-22  9:53 ` [PATCH v3 1/5] KVM: arm64: vgic-v3: Undo assignment on iodev registration failure Karl Mehltretter
2026-08-22  9:53 ` [PATCH v3 2/5] KVM: arm64: vgic-v3: Reset redistributors after failed region setup Karl Mehltretter
2026-08-22  9:53 ` Karl Mehltretter [this message]
2026-08-22  9:53 ` [PATCH v3 4/5] KVM: arm64: selftests: Pass guest code to vm_gic_create_with_vcpus() Karl Mehltretter
2026-08-22  9:53 ` [PATCH v3 5/5] KVM: arm64: selftests: Test VGICv3 redistributor region retry Karl Mehltretter
2026-08-23 15:39 ` [PATCH v3 0/5] KVM: arm64: fix VGICv3 redistributor rollback Fuad Tabba

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260822095346.53882-4-kmehltretter@gmail.com \
    --to=kmehltretter@gmail.com \
    --cc=catalin.marinas@arm.com \
    --cc=christoffer.dall@arm.com \
    --cc=eric.auger@redhat.com \
    --cc=fuad.tabba@linux.dev \
    --cc=joey.gouly@arm.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=oupton@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=seiden@linux.ibm.com \
    --cc=shuah@kernel.org \
    --cc=suzuki.poulose@arm.com \
    --cc=will@kernel.org \
    --cc=yuzenghui@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox