[RFC PATCH 0/2] KVM: arm64: fix VGICv3 redistributor rollback

[RFC PATCH 0/2] KVM: arm64: fix VGICv3 redistributor rollback

[Karl Mehltretter]

A failed REDIST_REGION update can unregister redistributor iodevs
belonging to earlier regions without clearing their cached base
addresses. A corrected retry then succeeds without restoring the missing
iodevs.

Limit rollback to vCPUs assigned to the newly added region. Extend
vgic_init to access an earlier redistributor after a failed update and
retry.

Commits 8542a8f95a67, 59112e9c390b, and 0d92e4a7ffd5 progressively fixed
cleanup after a failed region update. Preserving earlier regions follows
that model.

Built with GCC 15.2.0 and tested under QEMU 10.2.1 TCG with
-machine virt,virtualization=on,gic-version=3 and -cpu max. Both runs
reached the VGICv3 tests. The patched kernel passed; the same selftest
against the unpatched base failed with:

  Unexpected MMIO exit at 0x8050008

Karl Mehltretter (2):
  KVM: arm64: vgic-v3: Roll back failed redistributor region setup
  KVM: arm64: selftests: Test VGICv3 redistributor region retry

 arch/arm64/kvm/vgic/vgic-mmio-v3.c            |  42 ++++---
 tools/testing/selftests/kvm/arm64/vgic_init.c | 115 +++++++++++++-----
 2 files changed, 112 insertions(+), 45 deletions(-)


base-commit: b9b3e33b70b71e516930117e21de3ad2a7723747
-- 
2.53.0

[RFC PATCH 1/2] KVM: arm64: vgic-v3: Roll back failed redistributor region setup

[Karl Mehltretter]

A later REDIST_REGION attribute can be inserted successfully and then
fail while registering redistributor iodevs. For example, a region that
overlaps the distributor is caught by vgic_v3_check_base() when the first
unassigned vCPU is processed.

The existing rollback unregisters all previously registered
redistributor iodevs. It leaves their rd_iodev.base_addr values set, so a
corrected retry skips re-registering those stale vCPUs and succeeds with
holes in the MMIO bus.

A failure from kvm_io_bus_register_dev() can leave another hole. The
failing vCPU already has its base address and region assigned, but the
old i < c rollback does not include it.

Preserve devices assigned by earlier successful setters. On failure,
unregister only vCPUs associated with the newly inserted region, clear
their cached base addresses, and free that region. This also includes the
current vCPU when iodev registration itself fails.

Fixes: c011f4ea106b ("KVM: arm/arm64: Check vcpu redist base before registering an iodev")
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 arch/arm64/kvm/vgic/vgic-mmio-v3.c | 42 ++++++++++++++++++------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
index 5913a20d83019..832a7fe65984c 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -841,6 +841,31 @@ 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_v3_rollback_redist_region(struct kvm *kvm, u32 index)
+{
+	struct vgic_redist_region *rdreg;
+	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) {
+		if (vcpu->arch.vgic_cpu.rdreg == rdreg)
+			vgic_unregister_redist_iodev(vcpu);
+	}
+
+	guard(mutex)(&kvm->arch.config_lock);
+
+	kvm_for_each_vcpu(c, vcpu, kvm) {
+		if (vcpu->arch.vgic_cpu.rdreg == rdreg)
+			vcpu->arch.vgic_cpu.rd_iodev.base_addr = VGIC_ADDR_UNDEF;
+	}
+
+	vgic_v3_free_redist_region(kvm, rdreg);
+}
+
 static int vgic_register_all_redist_iodevs(struct kvm *kvm)
 {
 	struct kvm_vcpu *vcpu;
@@ -855,16 +880,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;
 }
 
@@ -984,12 +999,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.53.0

[RFC PATCH 2/2] KVM: arm64: selftests: Test VGICv3 redistributor region retry

[Karl Mehltretter]

The existing redistributor-region test already adds valid partial
regions, rejects a later region that collides with the distributor, and
retries with a valid region. Its guest code does not access a
redistributor, so it misses redistributor MMIO bus entries lost by the
failed rollback.

Run a guest that reads GICR_TYPER from the second redistributor in the
original region and verifies its processor number. Put the test layout
outside the default guest memory slot so an absent iodev results in a
KVM_EXIT_MMIO instead of an ordinary RAM access.

Allow the common VGIC setup helper to select the guest entry point so the
retry case can reuse the normal test setup.

Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 tools/testing/selftests/kvm/arm64/vgic_init.c | 115 +++++++++++++-----
 1 file changed, 86 insertions(+), 29 deletions(-)

diff --git a/tools/testing/selftests/kvm/arm64/vgic_init.c b/tools/testing/selftests/kvm/arm64/vgic_init.c
index 47e34b43afb29..8b321a1409b34 100644
--- a/tools/testing/selftests/kvm/arm64/vgic_init.c
+++ b/tools/testing/selftests/kvm/arm64/vgic_init.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2020, Red Hat, Inc.
  */
 #include <linux/kernel.h>
+#include <linux/sizes.h>
 #include <sys/syscall.h>
 #include <asm/kvm.h>
 #include <asm/kvm_para.h>
@@ -13,12 +14,23 @@
 
 #include "test_util.h"
 #include "kvm_util.h"
+#include "gic.h"
 #include "processor.h"
+#include "ucall_common.h"
 #include "vgic.h"
 #include "gic_v3.h"
 
 #define NR_VCPUS		4
 
+/* Keep the redistributor range outside the default guest memory slot. */
+#define REDIST_TEST_REGION0_BASE	GICR_BASE_GPA
+#define REDIST_TEST_REGION1_BASE	\
+	(REDIST_TEST_REGION0_BASE + 2 * KVM_VGIC_V3_REDIST_SIZE)
+#define REDIST_TEST_DIST_BASE	\
+	(REDIST_TEST_REGION1_BASE + KVM_VGIC_V3_REDIST_SIZE)
+#define REDIST_TEST_REGION2_BASE	\
+	(REDIST_TEST_DIST_BASE + KVM_VGIC_V3_DIST_SIZE)
+
 #define REG_OFFSET(vcpu, offset) (((u64)vcpu << 32) | offset)
 
 #define VGIC_DEV_IS_V2(_d) ((_d) == KVM_DEV_TYPE_ARM_VGIC_V2)
@@ -65,6 +77,16 @@ static void guest_code(void)
 	GUEST_DONE();
 }
 
+static void guest_read_second_redist(void)
+{
+	u64 typer = readq((void *)(unsigned long)(REDIST_TEST_REGION0_BASE +
+						   KVM_VGIC_V3_REDIST_SIZE +
+						   GICR_TYPER));
+
+	GUEST_ASSERT_EQ(GICR_TYPER_CPU_NUMBER(typer), 1);
+	GUEST_DONE();
+}
+
 /* we don't want to assert on run execution, hence that helper */
 static int run_vcpu(struct kvm_vcpu *vcpu)
 {
@@ -73,12 +95,13 @@ static int run_vcpu(struct kvm_vcpu *vcpu)
 
 static struct vm_gic vm_gic_create_with_vcpus(u32 gic_dev_type,
 					      u32 nr_vcpus,
+					      void (*guest_fn)(void),
 					      struct kvm_vcpu *vcpus[])
 {
 	struct vm_gic v;
 
 	v.gic_dev_type = gic_dev_type;
-	v.vm = vm_create_with_vcpus(nr_vcpus, guest_code, vcpus);
+	v.vm = vm_create_with_vcpus(nr_vcpus, guest_fn, vcpus);
 	v.gic_fd = kvm_create_device(v.vm, gic_dev_type);
 
 	return v;
@@ -240,38 +263,40 @@ static void subtest_v3_redist_regions(struct vm_gic *v)
 				    KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &addr);
 	TEST_ASSERT(ret && errno == EINVAL, "redist region attr value with count== 0");
 
-	addr = REDIST_REGION_ATTR_ADDR(2, 0x200000, 0, 1);
+	addr = REDIST_REGION_ATTR_ADDR(2, REDIST_TEST_REGION0_BASE, 0, 1);
 	ret = __kvm_device_attr_set(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
 				    KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &addr);
 	TEST_ASSERT(ret && errno == EINVAL,
 		    "attempt to register the first rdist region with index != 0");
 
-	addr = REDIST_REGION_ATTR_ADDR(2, 0x201000, 0, 1);
+	addr = REDIST_REGION_ATTR_ADDR(2, REDIST_TEST_REGION0_BASE + 0x1000,
+				       0, 1);
 	ret = __kvm_device_attr_set(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
 				    KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &addr);
 	TEST_ASSERT(ret && errno == EINVAL, "rdist region with misaligned address");
 
-	addr = REDIST_REGION_ATTR_ADDR(2, 0x200000, 0, 0);
+	addr = REDIST_REGION_ATTR_ADDR(2, REDIST_TEST_REGION0_BASE, 0, 0);
 	kvm_device_attr_set(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
 			    KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &addr);
 
-	addr = REDIST_REGION_ATTR_ADDR(2, 0x200000, 0, 1);
+	addr = REDIST_REGION_ATTR_ADDR(2, REDIST_TEST_REGION0_BASE, 0, 1);
 	ret = __kvm_device_attr_set(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
 				    KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &addr);
 	TEST_ASSERT(ret && errno == EINVAL, "register an rdist region with already used index");
 
-	addr = REDIST_REGION_ATTR_ADDR(1, 0x210000, 0, 2);
+	addr = REDIST_REGION_ATTR_ADDR(1, REDIST_TEST_REGION0_BASE + 0x10000,
+				       0, 2);
 	ret = __kvm_device_attr_set(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
 				    KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &addr);
 	TEST_ASSERT(ret && errno == EINVAL,
 		    "register an rdist region overlapping with another one");
 
-	addr = REDIST_REGION_ATTR_ADDR(1, 0x240000, 0, 2);
+	addr = REDIST_REGION_ATTR_ADDR(1, REDIST_TEST_REGION1_BASE, 0, 2);
 	ret = __kvm_device_attr_set(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
 				    KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &addr);
 	TEST_ASSERT(ret && errno == EINVAL, "register redist region with index not +1");
 
-	addr = REDIST_REGION_ATTR_ADDR(1, 0x240000, 0, 1);
+	addr = REDIST_REGION_ATTR_ADDR(1, REDIST_TEST_REGION1_BASE, 0, 1);
 	kvm_device_attr_set(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
 			    KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &addr);
 
@@ -288,7 +313,7 @@ static void subtest_v3_redist_regions(struct vm_gic *v)
 	TEST_ASSERT(ret && errno == E2BIG,
 		    "register redist region with top address beyond IPA range");
 
-	addr = 0x260000;
+	addr = REDIST_TEST_DIST_BASE;
 	ret = __kvm_device_attr_set(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
 				    KVM_VGIC_V3_ADDR_TYPE_REDIST, &addr);
 	TEST_ASSERT(ret && errno == EINVAL,
@@ -296,19 +321,19 @@ static void subtest_v3_redist_regions(struct vm_gic *v)
 
 	/*
 	 * Now there are 2 redist regions:
-	 * region 0 @ 0x200000 2 redists
-	 * region 1 @ 0x240000 1 redist
+	 * region 0 has 2 redistributors
+	 * region 1 has 1 redistributor
 	 * Attempt to read their characteristics
 	 */
 
 	addr = REDIST_REGION_ATTR_ADDR(0, 0, 0, 0);
-	expected_addr = REDIST_REGION_ATTR_ADDR(2, 0x200000, 0, 0);
+	expected_addr = REDIST_REGION_ATTR_ADDR(2, REDIST_TEST_REGION0_BASE, 0, 0);
 	ret = __kvm_device_attr_get(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
 				    KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &addr);
 	TEST_ASSERT(!ret && addr == expected_addr, "read characteristics of region #0");
 
 	addr = REDIST_REGION_ATTR_ADDR(0, 0, 0, 1);
-	expected_addr = REDIST_REGION_ATTR_ADDR(1, 0x240000, 0, 1);
+	expected_addr = REDIST_REGION_ATTR_ADDR(1, REDIST_TEST_REGION1_BASE, 0, 1);
 	ret = __kvm_device_attr_get(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
 				    KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &addr);
 	TEST_ASSERT(!ret && addr == expected_addr, "read characteristics of region #1");
@@ -318,11 +343,11 @@ static void subtest_v3_redist_regions(struct vm_gic *v)
 				    KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &addr);
 	TEST_ASSERT(ret && errno == ENOENT, "read characteristics of non existing region");
 
-	addr = 0x260000;
+	addr = REDIST_TEST_DIST_BASE;
 	kvm_device_attr_set(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
 			    KVM_VGIC_V3_ADDR_TYPE_DIST, &addr);
 
-	addr = REDIST_REGION_ATTR_ADDR(1, 0x260000, 0, 2);
+	addr = REDIST_REGION_ATTR_ADDR(1, REDIST_TEST_DIST_BASE, 0, 2);
 	ret = __kvm_device_attr_set(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
 				    KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &addr);
 	TEST_ASSERT(ret && errno == EINVAL, "register redist region colliding with dist");
@@ -338,7 +363,7 @@ static void test_vgic_then_vcpus(u32 gic_dev_type)
 	struct vm_gic v;
 	int ret, i;
 
-	v = vm_gic_create_with_vcpus(gic_dev_type, 1, vcpus);
+	v = vm_gic_create_with_vcpus(gic_dev_type, 1, guest_code, vcpus);
 
 	subtest_dist_rdist(&v);
 
@@ -359,7 +384,8 @@ static void test_vcpus_then_vgic(u32 gic_dev_type)
 	struct vm_gic v;
 	int ret;
 
-	v = vm_gic_create_with_vcpus(gic_dev_type, NR_VCPUS, vcpus);
+	v = vm_gic_create_with_vcpus(gic_dev_type, NR_VCPUS, guest_code,
+				     vcpus);
 
 	subtest_dist_rdist(&v);
 
@@ -403,6 +429,29 @@ static void test_v2_uaccess_cpuif_no_vcpus(void)
 	vm_gic_destroy(&v);
 }
 
+static void run_vcpu_expect_done(struct kvm_vcpu *vcpu)
+{
+	struct ucall uc;
+
+	vcpu_run(vcpu);
+
+	switch (get_ucall(vcpu, &uc)) {
+	case UCALL_DONE:
+		return;
+	case UCALL_ABORT:
+		REPORT_GUEST_ASSERT(uc);
+		break;
+	case UCALL_NONE:
+		if (vcpu->run->exit_reason == KVM_EXIT_MMIO)
+			TEST_FAIL("Unexpected MMIO exit at 0x%llx",
+				  vcpu->run->mmio.phys_addr);
+		fallthrough;
+	default:
+		TEST_FAIL("Unexpected ucall %lu, exit_reason %u",
+			  uc.cmd, vcpu->run->exit_reason);
+	}
+}
+
 static void test_v3_new_redist_regions(void)
 {
 	struct kvm_vcpu *vcpus[NR_VCPUS];
@@ -411,7 +460,8 @@ static void test_v3_new_redist_regions(void)
 	u64 addr;
 	int ret;
 
-	v = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS, vcpus);
+	v = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS,
+				     guest_code, vcpus);
 	subtest_v3_redist_regions(&v);
 	kvm_device_attr_set(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
 			    KVM_DEV_ARM_VGIC_CTRL_INIT, NULL);
@@ -420,12 +470,13 @@ static void test_v3_new_redist_regions(void)
 	TEST_ASSERT(ret == -ENXIO, "running without sufficient number of rdists");
 	vm_gic_destroy(&v);
 
-	/* step2 */
+	/* Step 2: adding enough redistributors after KVM_RUN is rejected. */
 
-	v = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS, vcpus);
+	v = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS,
+				     guest_code, vcpus);
 	subtest_v3_redist_regions(&v);
 
-	addr = REDIST_REGION_ATTR_ADDR(1, 0x280000, 0, 2);
+	addr = REDIST_REGION_ATTR_ADDR(1, REDIST_TEST_REGION2_BASE, 0, 2);
 	kvm_device_attr_set(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
 			    KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &addr);
 
@@ -434,9 +485,10 @@ static void test_v3_new_redist_regions(void)
 
 	vm_gic_destroy(&v);
 
-	/* step 3 */
+	/* Step 3: retry the failed region setup and exercise an existing rdist. */
 
-	v = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS, vcpus);
+	v = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS,
+				     guest_read_second_redist, vcpus);
 	subtest_v3_redist_regions(&v);
 
 	ret = __kvm_device_attr_set(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
@@ -444,15 +496,17 @@ static void test_v3_new_redist_regions(void)
 	TEST_ASSERT(ret && errno == EFAULT,
 		    "register a third region allowing to cover the 4 vcpus");
 
-	addr = REDIST_REGION_ATTR_ADDR(1, 0x280000, 0, 2);
+	addr = REDIST_REGION_ATTR_ADDR(1, REDIST_TEST_REGION2_BASE, 0, 2);
 	kvm_device_attr_set(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
 			    KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &addr);
 
+	virt_map(v.vm, REDIST_TEST_REGION0_BASE, REDIST_TEST_REGION0_BASE,
+		 vm_calc_num_guest_pages(v.vm->mode,
+					 2 * KVM_VGIC_V3_REDIST_SIZE));
 	kvm_device_attr_set(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
 			    KVM_DEV_ARM_VGIC_CTRL_INIT, NULL);
 
-	ret = run_vcpu(vcpus[3]);
-	TEST_ASSERT(!ret, "vcpu run");
+	run_vcpu_expect_done(vcpus[0]);
 
 	vm_gic_destroy(&v);
 }
@@ -608,7 +662,8 @@ static void test_v3_redist_ipa_range_check_at_vcpu_run(void)
 	int ret, i;
 	u64 addr;
 
-	v = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, 1, vcpus);
+	v = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, 1,
+				     guest_code, vcpus);
 
 	/* Set space for 3 redists, we have 1 vcpu, so this succeeds. */
 	addr = max_phys_size - (3 * 2 * 0x10000);
@@ -641,7 +696,8 @@ static void test_v3_its_region(void)
 	u64 addr;
 	int its_fd, ret;
 
-	v = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS, vcpus);
+	v = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS,
+				     guest_code, vcpus);
 	its_fd = kvm_create_device(v.vm, KVM_DEV_TYPE_ARM_VGIC_ITS);
 
 	addr = 0x401000;
@@ -684,7 +740,8 @@ static void test_v3_nassgicap(void)
 	u32 typer2;
 	int ret;
 
-	vm = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS, vcpus);
+	vm = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS,
+				      guest_code, vcpus);
 	kvm_device_attr_get(vm.gic_fd, KVM_DEV_ARM_VGIC_GRP_DIST_REGS,
 			    GICD_TYPER2, &typer2);
 	has_nassgicap = typer2 & GICD_TYPER2_nASSGIcap;
-- 
2.53.0

Re: [RFC PATCH 1/2] KVM: arm64: vgic-v3: Roll back failed redistributor region setup

[Marc Zyngier]

On Mon, 10 Aug 2026 07:52:16 +0100,
Karl Mehltretter <kmehltretter@gmail.com> wrote:

I'm sorry, but I find it difficult understand what you are trying to
explain here:

> A later REDIST_REGION attribute can be inserted successfully and then

Later than what?

> fail while registering redistributor iodevs. For example, a region that
> overlaps the distributor is caught by vgic_v3_check_base() when the first
> unassigned vCPU is processed.
>
> The existing rollback unregisters all previously registered
> redistributor iodevs. It leaves their rd_iodev.base_addr values set, so a
> corrected retry skips re-registering those stale vCPUs and succeeds with
> holes in the MMIO bus.

Holes in the MMIO space are the norm. The IPA space can multi-TB
large, and there is no reason why it'd cover everything (where would
you place the RAM otherwise?).

Is the problem here that you are left with vcpus that seem to have
been matched to an RD (base_addr being set), but that really are left
unconnected?

> 
> A failure from kvm_io_bus_register_dev() can leave another hole. The

MMIO hole? Coverage hole?

> failing vCPU already has its base address and region assigned, but the
> old i < c rollback does not include it.

What is "it"?

> 
> Preserve devices assigned by earlier successful setters. On failure,
> unregister only vCPUs associated with the newly inserted region, clear
> their cached base addresses, and free that region. This also includes the
> current vCPU when iodev registration itself fails.

What I don't see here is an argument explaining that doing this
doesn't change the guest-visible assignment of RDs, which would be a
regression.

> 
> Fixes: c011f4ea106b ("KVM: arm/arm64: Check vcpu redist base before registering an iodev")
> Assisted-by: Codex:gpt-5.6-sol
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
>  arch/arm64/kvm/vgic/vgic-mmio-v3.c | 42 ++++++++++++++++++------------
>  1 file changed, 26 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> index 5913a20d83019..832a7fe65984c 100644
> --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> @@ -841,6 +841,31 @@ 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_v3_rollback_redist_region(struct kvm *kvm, u32 index)
> +{
> +	struct vgic_redist_region *rdreg;
> +	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) {
> +		if (vcpu->arch.vgic_cpu.rdreg == rdreg)
> +			vgic_unregister_redist_iodev(vcpu);
> +	}
> +
> +	guard(mutex)(&kvm->arch.config_lock);
> +
> +	kvm_for_each_vcpu(c, vcpu, kvm) {
> +		if (vcpu->arch.vgic_cpu.rdreg == rdreg)
> +			vcpu->arch.vgic_cpu.rd_iodev.base_addr = VGIC_ADDR_UNDEF;
> +	}
> +
> +	vgic_v3_free_redist_region(kvm, rdreg);
> +}
> +
>  static int vgic_register_all_redist_iodevs(struct kvm *kvm)
>  {
>  	struct kvm_vcpu *vcpu;
> @@ -855,16 +880,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;
>  }
>  
> @@ -984,12 +999,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;
>  	}
>  

Based on what I understand of your earlier description, why isn't this
as simple as this untested hack:

diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
index 5913a20d83019..804fcc69cbd48 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -859,7 +859,7 @@ static int vgic_register_all_redist_iodevs(struct kvm *kvm)
 		/* The current c failed, so iterate over the previous ones. */
 		int i;
 
-		for (i = 0; i < c; i++) {
+		for (i = 0; i <= c; i++) {
 			vcpu = kvm_get_vcpu(kvm, i);
 			vgic_unregister_redist_iodev(vcpu);
 		}
@@ -960,8 +960,10 @@ void vgic_v3_free_redist_region(struct kvm *kvm, struct vgic_redist_region *rdre
 
 	/* Garbage collect the region */
 	kvm_for_each_vcpu(c, vcpu, kvm) {
-		if (vcpu->arch.vgic_cpu.rdreg == rdreg)
+		if (vcpu->arch.vgic_cpu.rdreg == rdreg) {
 			vcpu->arch.vgic_cpu.rdreg = NULL;
+			vcpu->arch.vgic_cpu.rd_iodev.base_addr = VGIC_ADDR_UNDEF;
+		}
 	}
 
 	list_del(&rdreg->list);

I don't mind the cleaning up, but not as part of fixing the issue,
which has to be as small as possible (think of the backports).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.