[PATCH v3 2/5] KVM: arm64: vgic-v3: Reset redistributors after failed region setup

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,org.kernel.vger.stable
Message-ID <[email protected]>
A REDIST_REGION write can fail after redistributor iodevs have been
registered. The existing rollback unregisters only vCPUs processed before
the failure, leaving their cached assignments and free_index values intact.
A retry then skips the unregistered iodevs.

Userspace has no guarantee that redistributor assignments survive a failed
region update. On failure, unregister every redistributor iodev and clear
every cached vCPU assignment. Reset all region free_index values and free
the newly inserted region. The next successful region update rebuilds all
possible assignments in region-index order.

While a vCPU is being created, its redistributor iodev may be registered
before the vCPU is visible to kvm_for_each_vcpu(). Reject REDIST and
REDIST_REGION writes while creation is in flight, so rollback can reset every
assignment.

The registration failure for the current vCPU is already undone by
vgic_register_redist_iodev().

Fixes: c011f4ea106b ("KVM: arm/arm64: Check vcpu redist base before registering an iodev")
Suggested-by: Marc Zyngier <[email protected]>
Cc: [email protected]
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Karl Mehltretter <[email protected]>
---
 arch/arm64/kvm/vgic/vgic-kvm-device.c | 20 ++++++++++
 arch/arm64/kvm/vgic/vgic-mmio-v3.c    | 53 +++++++++++++++++++--------
 2 files changed, 57 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c
index 90be99443df3..48c3b2a48c20 100644
--- a/arch/arm64/kvm/vgic/vgic-kvm-device.c
+++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c
@@ -97,6 +97,9 @@ static int kvm_vgic_addr(struct kvm *kvm, struct kvm_device_attr *attr, bool wri
 	phys_addr_t *addr_ptr, alignment, size;
 	u64 undef_value = VGIC_ADDR_UNDEF;
 	u64 addr;
+	bool redist_write = write &&
+		(attr->attr == KVM_VGIC_V3_ADDR_TYPE_REDIST ||
+		 attr->attr == KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION);
 	int r;
 
 	/* Reading a redistributor region addr implies getting the index */
@@ -104,6 +107,19 @@ static int kvm_vgic_addr(struct kvm *kvm, struct kvm_device_attr *attr, bool wri
 		if (get_user(addr, uaddr))
 			return -EFAULT;
 
+	/*
+	 * A vCPU can have an RD assignment before it is visible to
+	 * kvm_for_each_vcpu(). Reject redistributor updates while vCPU creation
+	 * is in progress, so rollback can reset every assignment.
+	 */
+	if (redist_write) {
+		mutex_lock(&kvm->lock);
+		if (kvm->created_vcpus != atomic_read(&kvm->online_vcpus)) {
+			r = -EBUSY;
+			goto out_unlock_kvm;
+		}
+	}
+
 	/*
 	 * Since we can't hold config_lock while registering the redistributor
 	 * iodevs, take the slots_lock immediately.
@@ -201,6 +217,10 @@ static int kvm_vgic_addr(struct kvm *kvm, struct kvm_device_attr *attr, bool wri
 out:
 	mutex_unlock(&kvm->slots_lock);
 
+out_unlock_kvm:
+	if (redist_write)
+		mutex_unlock(&kvm->lock);
+
 	if (!r && !write)
 		r =  put_user(addr, uaddr);
 
diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
index 22897ce64dbf..6c009deb11d4 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -855,6 +855,40 @@ 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)
+{
+	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+
+	lockdep_assert_held(&vcpu->kvm->arch.config_lock);
+
+	vgic_cpu->rdreg = NULL;
+	vgic_cpu->rd_iodev.base_addr = VGIC_ADDR_UNDEF;
+}
+
+static void vgic_v3_rollback_redist_region(struct kvm *kvm, u32 index)
+{
+	struct vgic_redist_region *rdreg, *iter;
+	struct kvm_vcpu *vcpu;
+	unsigned long c;
+
+	lockdep_assert_held(&kvm->slots_lock);
+
+	rdreg = vgic_v3_rdist_region_from_index(kvm, index);
+
+	kvm_for_each_vcpu(c, vcpu, kvm)
+		vgic_unregister_redist_iodev(vcpu);
+
+	guard(mutex)(&kvm->arch.config_lock);
+
+	kvm_for_each_vcpu(c, vcpu, kvm)
+		vgic_reset_redist_iodev(vcpu);
+
+	list_for_each_entry(iter, &kvm->arch.vgic.rd_regions, list)
+		iter->free_index = 0;
+
+	vgic_v3_free_redist_region(kvm, rdreg);
+}
+
 static int vgic_register_all_redist_iodevs(struct kvm *kvm)
 {
 	struct kvm_vcpu *vcpu;
@@ -869,16 +903,6 @@ static int vgic_register_all_redist_iodevs(struct kvm *kvm)
 			break;
 	}
 
-	if (ret) {
-		/* The current c failed, so iterate over the previous ones. */
-		int i;
-
-		for (i = 0; i < c; i++) {
-			vcpu = kvm_get_vcpu(kvm, i);
-			vgic_unregister_redist_iodev(vcpu);
-		}
-	}
-
 	return ret;
 }
 
@@ -986,6 +1010,8 @@ int vgic_v3_set_redist_base(struct kvm *kvm, u32 index, u64 addr, u32 count)
 {
 	int ret;
 
+	lockdep_assert_held(&kvm->lock);
+
 	mutex_lock(&kvm->arch.config_lock);
 	ret = vgic_v3_alloc_redist_region(kvm, index, addr, count);
 	mutex_unlock(&kvm->arch.config_lock);
@@ -998,12 +1024,7 @@ int vgic_v3_set_redist_base(struct kvm *kvm, u32 index, u64 addr, u32 count)
 	 */
 	ret = vgic_register_all_redist_iodevs(kvm);
 	if (ret) {
-		struct vgic_redist_region *rdreg;
-
-		mutex_lock(&kvm->arch.config_lock);
-		rdreg = vgic_v3_rdist_region_from_index(kvm, index);
-		vgic_v3_free_redist_region(kvm, rdreg);
-		mutex_unlock(&kvm->arch.config_lock);
+		vgic_v3_rollback_redist_region(kvm, index);
 		return ret;
 	}
 
-- 
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.