[PATCH v2 0/5] KVM: arm64: fix VGICv3 redistributor rollback

[PATCH v2 0/5] KVM: arm64: fix VGICv3 redistributor rollback

[Karl Mehltretter]

A failed REDIST_REGION write can unregister redistributor iodevs while
leaving their cached vCPU assignments and free_index values intact. A retry
then skips those assignments and succeeds with redistributors missing from
the MMIO bus.

Patches 1 and 2 fix VGICv3 redistributor failure handling. Patch 3 cleans
up redistributor teardown. Patches 4 and 5 extend the KVM selftest
infrastructure and add regression coverage.

Patch 1 reserves the current vCPU's region slot before registering its
iodev and releases the assignment if registration fails. This keeps
free_index consistent with the cached assignments.

Patch 2 uses the same cleanup during redistributor unregistration, allowing
the existing rollback to reset the vCPUs processed before the failure.

Patch 3 unregisters redistributors before freeing their regions and removes
the region destructor's now-redundant vCPU scan. It is not required by the
first two patches.

Patch 4 lets the common VGIC setup helper select the guest entry point.

Patch 5 adds a regression test that rejects an overlapping region, retries
at a valid address, and then accesses all four redistributors. It exercises
the failed region update fixed by patch 2. The MMIO bus registration
failure handled by patch 1 is not fault-injected.

Testing: built the patched kernel and selftest in an arm64 Linux Docker
container with GCC 13.3.0. The selftest passed under QEMU 11.0.2 TCG with
-machine virt,virtualization=on,gic-version=3 and -cpu max. The same
selftest against the exact unpatched base failed with:

  Unexpected MMIO exit at 0x8030008

---
Changes since RFC:
- Reset all redistributor assignments after a failed region update.
- Split MMIO bus registration failure, region rollback, and teardown
  cleanup into separate patches.
- Split the selftest infrastructure change from the regression test and
  check all four redistributors after retry.
- Rebase onto the current kvmarm/next.

Previous version:
RFC: https://lore.kernel.org/r/cover.1786344511.git.kmehltretter@gmail.com

Karl Mehltretter (5):
  KVM: arm64: vgic-v3: Undo assignment on iodev registration failure
  KVM: arm64: vgic-v3: Reset assignments after failed region setup
  KVM: arm64: vgic-v3: Unregister redistributors before freeing regions
  KVM: arm64: selftests: Pass guest code to vm_gic_create_with_vcpus()
  KVM: arm64: selftests: Test VGICv3 redistributor region retry

 arch/arm64/kvm/vgic/vgic-init.c               |  11 +-
 arch/arm64/kvm/vgic/vgic-mmio-v3.c            |  33 ++---
 tools/testing/selftests/kvm/arm64/vgic_init.c | 117 ++++++++++++++++--
 3 files changed, 134 insertions(+), 27 deletions(-)


base-commit: 57e7cf13ac26bf1a3dba6cfa601f7b2481811575
-- 
2.39.5 (Apple Git-154)

[PATCH v2 1/5] KVM: arm64: vgic-v3: Undo assignment on iodev registration failure

[Karl Mehltretter]

vgic_register_redist_iodev() assigns a redistributor region and base
address to the vCPU before adding its iodev to the MMIO bus. However, the
region's free_index is advanced only after registration succeeds.

If kvm_io_bus_register_dev() fails, the vCPU retains the assignment while
free_index still identifies the same slot as free. A later registration can
therefore reuse a slot that remains assigned to the vCPU.

Advance free_index when assigning the slot. On registration failure, return
the slot and clear rdreg and base_addr.

Fixes: dbd9733ab674 ("KVM: arm/arm64: Replace the single rdist region by a list")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 arch/arm64/kvm/vgic/vgic-mmio-v3.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
index 5913a20d8301..c75caadbd1d3 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -766,6 +766,18 @@ unsigned int vgic_v3_init_dist_iodev(struct vgic_io_device *dev)
 	return SZ_64K;
 }
 
+static void vgic_unassign_redist_iodev(struct kvm_vcpu *vcpu)
+{
+	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+
+	guard(mutex)(&vcpu->kvm->arch.config_lock);
+	if (vgic_cpu->rdreg) {
+		vgic_cpu->rdreg->free_index--;
+		vgic_cpu->rdreg = NULL;
+	}
+	vgic_cpu->rd_iodev.base_addr = VGIC_ADDR_UNDEF;
+}
+
 /**
  * vgic_register_redist_iodev - register a single redist iodev
  * @vcpu:    The VCPU to which the redistributor belongs
@@ -818,16 +830,17 @@ int vgic_register_redist_iodev(struct kvm_vcpu *vcpu)
 	rd_dev->nr_regions = ARRAY_SIZE(vgic_v3_rd_registers);
 	rd_dev->redist_vcpu = vcpu;
 
+	/* Protected by slots_lock */
+	rdreg->free_index++;
+
 	mutex_unlock(&kvm->arch.config_lock);
 
 	ret = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS, rd_base,
 				      2 * SZ_64K, &rd_dev->dev);
 	if (ret)
-		return ret;
+		vgic_unassign_redist_iodev(vcpu);
 
-	/* Protected by slots_lock */
-	rdreg->free_index++;
-	return 0;
+	return ret;
 
 out_unlock:
 	mutex_unlock(&kvm->arch.config_lock);
-- 
2.39.5 (Apple Git-154)

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

[Karl Mehltretter]

A REDIST_REGION write can fail after redistributors from earlier regions
have been registered. The rollback removes their iodevs from the MMIO bus
but leaves rdreg and base_addr set. It also leaves free_index unchanged. A
retry sees base_addr set and skips registration, leaving the redistributors
absent from the MMIO bus.

Make redistributor unregistration clear the cached assignment and return
the region slot. The failing vCPU already unassigns itself, so the existing
rollback resets the vCPUs processed before the failure. A retry can then
assign and register all redistributors again.

Fixes: c011f4ea106b ("KVM: arm/arm64: Check vcpu redist base before registering an iodev")
Suggested-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 arch/arm64/kvm/vgic/vgic-mmio-v3.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
index c75caadbd1d3..331d203acb43 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -852,6 +852,7 @@ void vgic_unregister_redist_iodev(struct kvm_vcpu *vcpu)
 	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);
 }
 
 static int vgic_register_all_redist_iodevs(struct kvm *kvm)
@@ -869,7 +870,7 @@ static int vgic_register_all_redist_iodevs(struct kvm *kvm)
 	}
 
 	if (ret) {
-		/* The current c failed, so iterate over the previous ones. */
+		/* The failing vCPU has no assignment to undo. */
 		int i;
 
 		for (i = 0; i < c; i++) {
-- 
2.39.5 (Apple Git-154)

[PATCH v2 3/5] KVM: arm64: vgic-v3: Unregister redistributors before freeing regions

[Karl Mehltretter]

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 <kmehltretter@gmail.com>
---
 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);
+	}
+
 	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 331d203acb43..77a611564f8e 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -967,17 +967,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);
 }
-- 
2.39.5 (Apple Git-154)

[PATCH v2 4/5] KVM: arm64: selftests: Pass guest code to vm_gic_create_with_vcpus()

[Karl Mehltretter]

Pass the guest entry point to vm_gic_create_with_vcpus(), and use
guest_code at the existing call sites. This lets tests with different guest
code reuse the common VM and VGIC setup.

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

diff --git a/tools/testing/selftests/kvm/arm64/vgic_init.c b/tools/testing/selftests/kvm/arm64/vgic_init.c
index 47e34b43afb2..91484fd4f786 100644
--- a/tools/testing/selftests/kvm/arm64/vgic_init.c
+++ b/tools/testing/selftests/kvm/arm64/vgic_init.c
@@ -73,12 +73,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;
@@ -338,7 +339,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 +360,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);
 
@@ -411,7 +413,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);
@@ -422,7 +425,8 @@ static void test_v3_new_redist_regions(void)
 
 	/* step2 */
 
-	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);
@@ -436,7 +440,8 @@ static void test_v3_new_redist_regions(void)
 
 	/* step 3 */
 
-	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);
 
 	ret = __kvm_device_attr_set(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
@@ -608,7 +613,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 +647,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 +691,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.39.5 (Apple Git-154)

[PATCH v2 5/5] KVM: arm64: selftests: Test VGICv3 redistributor region retry

[Karl Mehltretter]

A failed REDIST_REGION write can remove redistributor iodevs without
clearing their cached vCPU assignments. A retry then skips those
assignments and leaves the redistributors absent from the MMIO bus.

Create two partial regions, reject a third region that overlaps the
distributor, and retry at a valid address. After initializing the VGIC,
have the guest read GICR_TYPER from all four redistributors and check their
processor numbers. This verifies the final configuration without assuming
that assignments survive the failed write.

Keep the redistributor IPAs outside the guest RAM memslot and install only
the stage-1 mappings needed for the MMIO accesses. If an iodev is missing,
KVM_RUN therefore exits with KVM_EXIT_MMIO. Without the fix, the first
redistributor access exits at 0x8030008.

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

diff --git a/tools/testing/selftests/kvm/arm64/vgic_init.c b/tools/testing/selftests/kvm/arm64/vgic_init.c
index 91484fd4f786..6d9998c764bc 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,21 @@
 
 #include "test_util.h"
 #include "kvm_util.h"
+#include "gic.h"
 #include "processor.h"
 #include "vgic.h"
 #include "gic_v3.h"
 
 #define NR_VCPUS		4
 
+#define REDIST_RETRY_REGION0_BASE	GICR_BASE_GPA
+#define REDIST_RETRY_REGION1_BASE	\
+	(REDIST_RETRY_REGION0_BASE + 2 * KVM_VGIC_V3_REDIST_SIZE)
+#define REDIST_RETRY_DIST_BASE		\
+	(REDIST_RETRY_REGION1_BASE + KVM_VGIC_V3_REDIST_SIZE)
+#define REDIST_RETRY_REGION2_BASE	\
+	(REDIST_RETRY_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 +75,22 @@ static void guest_code(void)
 	GUEST_DONE();
 }
 
+static void guest_check_redist_retry(void)
+{
+	unsigned int i;
+
+	for (i = 0; i < NR_VCPUS; i++) {
+		u64 base = i < 3 ? REDIST_RETRY_REGION0_BASE +
+				       i * KVM_VGIC_V3_REDIST_SIZE :
+				       REDIST_RETRY_REGION2_BASE;
+		u64 typer = readq((void *)(unsigned long)(base + GICR_TYPER));
+
+		GUEST_ASSERT_EQ(GICR_TYPER_CPU_NUMBER(typer), i);
+	}
+
+	GUEST_DONE();
+}
+
 /* we don't want to assert on run execution, hence that helper */
 static int run_vcpu(struct kvm_vcpu *vcpu)
 {
@@ -462,6 +488,70 @@ static void test_v3_new_redist_regions(void)
 	vm_gic_destroy(&v);
 }
 
+static void test_v3_redist_region_retry(void)
+{
+	struct kvm_vcpu *vcpus[NR_VCPUS];
+	struct vm_gic v;
+	struct ucall uc;
+	u64 addr;
+	int ret;
+
+	v = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS,
+				     guest_check_redist_retry, vcpus);
+
+	addr = REDIST_REGION_ATTR_ADDR(2, REDIST_RETRY_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(1, REDIST_RETRY_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);
+
+	addr = REDIST_RETRY_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, REDIST_RETRY_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");
+
+	addr = REDIST_REGION_ATTR_ADDR(1, REDIST_RETRY_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_RETRY_REGION0_BASE, REDIST_RETRY_REGION0_BASE,
+		 vm_calc_num_guest_pages(v.vm->mode,
+					 3 * KVM_VGIC_V3_REDIST_SIZE));
+	virt_map(v.vm, REDIST_RETRY_REGION2_BASE, REDIST_RETRY_REGION2_BASE,
+		 vm_calc_num_guest_pages(v.vm->mode,
+					 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);
+
+	vcpu_run(vcpus[0]);
+	switch (get_ucall(vcpus[0], &uc)) {
+	case UCALL_DONE:
+		break;
+	case UCALL_ABORT:
+		REPORT_GUEST_ASSERT(uc);
+		break;
+	case UCALL_NONE:
+		if (vcpus[0]->run->exit_reason == KVM_EXIT_MMIO)
+			TEST_FAIL("Unexpected MMIO exit at 0x%llx",
+				  vcpus[0]->run->mmio.phys_addr);
+		fallthrough;
+	default:
+		TEST_FAIL("Unexpected ucall %lu, exit_reason %u",
+			  uc.cmd, vcpus[0]->run->exit_reason);
+	}
+
+	vm_gic_destroy(&v);
+}
+
 static void test_v3_typer_accesses(void)
 {
 	struct vm_gic v;
@@ -986,6 +1076,7 @@ void run_tests(u32 gic_dev_type)
 
 	if (VGIC_DEV_IS_V3(gic_dev_type)) {
 		test_v3_new_redist_regions();
+		test_v3_redist_region_retry();
 		test_v3_typer_accesses();
 		test_v3_last_bit_redist_regions();
 		test_v3_last_bit_single_rdist();
-- 
2.39.5 (Apple Git-154)

Re: [PATCH v2 4/5] KVM: arm64: selftests: Pass guest code to vm_gic_create_with_vcpus()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The `guest_fn` parameter restricts the guest entry point type to `void (*)(void)`, deviating from the selftests standard `void *`.
--

--- Patch [4]: [PATCH v2 4/5] KVM: arm64: selftests: Pass guest code to vm_gic_create_with_vcpus() ---
commit 6c09a6b348b8fbe5cd4e0cef1198db5d49812401
Author: Karl Mehltretter <kmehltretter@gmail.com>

KVM: arm64: selftests: Pass guest code to vm_gic_create_with_vcpus()

This commit adds a guest entry point parameter to vm_gic_create_with_vcpus()
and updates the existing call sites to pass guest_code. This allows tests
with different guest code to reuse the common VM and VGIC initialization
sequence.

> diff --git a/tools/testing/selftests/kvm/arm64/vgic_init.c b/tools/testing/selftests/kvm/arm64/vgic_init.c
> index 47e34b43afb29..91484fd4f7869 100644
> --- a/tools/testing/selftests/kvm/arm64/vgic_init.c
> +++ b/tools/testing/selftests/kvm/arm64/vgic_init.c
> @@ -73,12 +73,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),

[Severity: Low]
Does this unnecessarily restrict the guest entry point type?

The underlying kvm selftests API, such as vm_create_with_vcpus(), typically
uses a generic void pointer for the guest code. If a future test tries to
reuse this setup function and pass a parameterized guest code function, the
compiler might emit an incompatible pointer type warning.

Would it be better to change the type of guest_fn to void * to maintain API
consistency with the rest of the selftests framework?

>  					      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);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819224229.82948-1-kmehltretter@gmail.com?part=4

Re: [PATCH v2 1/5] KVM: arm64: vgic-v3: Undo assignment on iodev registration failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete rollback in `vgic_register_all_redist_iodevs` fails to clear `base_addr`, permanently breaking rolled-back vCPUs on retry.
--

--- Patch [1]: [PATCH v2 1/5] KVM: arm64: vgic-v3: Undo assignment on iodev registration failure ---
commit 1ddcb102c04536e66942920259e184122195f9ff
Author: Karl Mehltretter <kmehltretter@gmail.com>

KVM: arm64: vgic-v3: Undo assignment on iodev registration failure

This commit introduces vgic_unassign_redist_iodev() to clear rdreg and base_addr
when kvm_io_bus_register_dev() fails during redistributor iodev registration.
This prevents reusing a slot that remains assigned to a vCPU and fixes the
state leak for the current failing vCPU.

> diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> index 5913a20d83019..c75caadbd1d3c 100644
> --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> @@ -766,6 +766,18 @@ unsigned int vgic_v3_init_dist_iodev(struct vgic_io_device *dev)
[ ... ]
> @@ -818,16 +830,17 @@ int vgic_register_redist_iodev(struct kvm_vcpu *vcpu)
[ ... ]
>  	ret = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS, rd_base,
>  				      2 * SZ_64K, &rd_dev->dev);
>  	if (ret)
> -		return ret;
> +		vgic_unassign_redist_iodev(vcpu);

[Severity: High]
This is a pre-existing issue, but does the rollback loop in
vgic_register_all_redist_iodevs() also need to use this new unassign helper?

If vgic_register_redist_iodev() fails, the rollback loop in
vgic_register_all_redist_iodevs() handles previously processed vCPUs by calling
vgic_unregister_redist_iodev():

arch/arm64/kvm/vgic/vgic-mmio-v3.c:vgic_register_all_redist_iodevs() {
    ...
	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);
		}
	}
    ...
}

Because vgic_unregister_redist_iodev() only unregisters the bus device, it
does not clear base_addr or reset rdreg. These rolled-back vCPUs remain
assigned but are removed from the MMIO bus.

If userspace retries the ioctl, vgic_register_redist_iodev() will see that
base_addr is no longer VGIC_ADDR_UNDEF and skip registration.

Will this leave the guest permanently missing MMIO mappings for those
earlier vCPUs, breaking guest execution?

>  
> -	/* Protected by slots_lock */
> -	rdreg->free_index++;
> -	return 0;
> +	return ret;
>  
>  out_unlock:
>  	mutex_unlock(&kvm->arch.config_lock);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819224229.82948-1-kmehltretter@gmail.com?part=1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Naive decrement of `rdreg->free_index` during concurrent vCPU destruction corrupts the redistributor allocation state, causing overlapping MMIO base addresses.
--

--- Patch [2]: [PATCH v2 2/5] KVM: arm64: vgic-v3: Reset assignments after failed region setup ---
commit 00ec61ba92a90418f1516339ae4c96442b750b29
Author: Karl Mehltretter <kmehltretter@gmail.com>

KVM: arm64: vgic-v3: Reset assignments after failed region setup

This commit makes redistributor unregistration clear the cached
assignment and return the region slot to address an issue where a
failed REDIST_REGION write leaves redistributors partially registered.
It also updates the rollback path in vgic_register_all_redist_iodevs().

> diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> index c75caadbd1d3c..331d203acb430 100644
> --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> @@ -852,6 +852,7 @@ void vgic_unregister_redist_iodev(struct kvm_vcpu *vcpu)
>  	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);

[Severity: High]
Does calling vgic_unassign_redist_iodev() here corrupt the redistributor
allocation state during concurrent vCPU destruction?

Because vgic_unregister_redist_iodev() is also called from the vCPU error
path in __kvm_vgic_vcpu_destroy(), this could be triggered if a vCPU
creation fails.

If multiple vCPUs are created concurrently, and VCPU A and VCPU B acquire
their redistributor slots sequentially (e.g., slots X and X+1), and then
VCPU A's creation fails, the error path for VCPU A will call:

__kvm_vgic_vcpu_destroy()
  vgic_unregister_redist_iodev()
    vgic_unassign_redist_iodev()

This blindly decrements the free_index from X+2 to X+1, since
vgic_unassign_redist_iodev() assumes it is unassigning the most recently
allocated slot:

arch/arm64/kvm/vgic/vgic-mmio-v3.c:vgic_unassign_redist_iodev() {
	...
	if (vgic_cpu->rdreg) {
		vgic_cpu->rdreg->free_index--;
		vgic_cpu->rdreg = NULL;
	}
	...
}

When VCPU C is later created, will it be assigned slot X+1, leading to an
overlap with VCPU B's MMIO base address?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819224229.82948-1-kmehltretter@gmail.com?part=2