KVM: arm64: vgic: Simplify kvm_vgic_destroy()
authorMarc Zyngier <maz@kernel.org>
Thu, 7 Dec 2023 15:11:57 +0000 (15:11 +0000)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Mon, 1 Jan 2024 12:42:46 +0000 (12:42 +0000)
commit 01ad29d224ff73bc4e16e0ef9ece17f28598c4a4 upstream.

When destroying a vgic, we have rather cumbersome rules about
when slots_lock and config_lock are held, resulting in fun
buglets.

The first port of call is to simplify kvm_vgic_map_resources()
so that there is only one call to kvm_vgic_destroy() instead of
two, with the second only holding half of the locks.

For that, we kill the non-locking primitive and move the call
outside of the locking altogether. This doesn't change anything
(we re-acquire the locks and teardown the whole vgic), and
simplifies the code significantly.

Cc: stable@vger.kernel.org
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20231207151201.3028710-2-maz@kernel.org
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
arch/arm64/kvm/vgic/vgic-init.c

index c8c3cb8..ad7e868 100644 (file)
@@ -382,26 +382,24 @@ void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu)
        vgic_cpu->rd_iodev.base_addr = VGIC_ADDR_UNDEF;
 }
 
-static void __kvm_vgic_destroy(struct kvm *kvm)
+void kvm_vgic_destroy(struct kvm *kvm)
 {
        struct kvm_vcpu *vcpu;
        unsigned long i;
 
-       lockdep_assert_held(&kvm->arch.config_lock);
+       mutex_lock(&kvm->slots_lock);
 
        vgic_debug_destroy(kvm);
 
        kvm_for_each_vcpu(i, vcpu, kvm)
                kvm_vgic_vcpu_destroy(vcpu);
 
+       mutex_lock(&kvm->arch.config_lock);
+
        kvm_vgic_dist_destroy(kvm);
-}
 
-void kvm_vgic_destroy(struct kvm *kvm)
-{
-       mutex_lock(&kvm->arch.config_lock);
-       __kvm_vgic_destroy(kvm);
        mutex_unlock(&kvm->arch.config_lock);
+       mutex_unlock(&kvm->slots_lock);
 }
 
 /**
@@ -469,25 +467,26 @@ int kvm_vgic_map_resources(struct kvm *kvm)
                type = VGIC_V3;
        }
 
-       if (ret) {
-               __kvm_vgic_destroy(kvm);
+       if (ret)
                goto out;
-       }
+
        dist->ready = true;
        dist_base = dist->vgic_dist_base;
        mutex_unlock(&kvm->arch.config_lock);
 
        ret = vgic_register_dist_iodev(kvm, dist_base, type);
-       if (ret) {
+       if (ret)
                kvm_err("Unable to register VGIC dist MMIO regions\n");
-               kvm_vgic_destroy(kvm);
-       }
-       mutex_unlock(&kvm->slots_lock);
-       return ret;
 
+       goto out_slots;
 out:
        mutex_unlock(&kvm->arch.config_lock);
+out_slots:
        mutex_unlock(&kvm->slots_lock);
+
+       if (ret)
+               kvm_vgic_destroy(kvm);
+
        return ret;
 }