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)
