KVM: destruct kvm_io_device while unregistering it from kvm_io_bus
authorWei Wang <wei.w.wang@intel.com>
Tue, 7 Feb 2023 12:37:12 +0000 (20:37 +0800)
committerSean Christopherson <seanjc@google.com>
Tue, 13 Jun 2023 21:18:09 +0000 (14:18 -0700)
Current usage of kvm_io_device requires users to destruct it with an extra
call of kvm_iodevice_destructor after the device gets unregistered from
kvm_io_bus. This is not necessary and can cause errors if a user forgot
to make the extra call.

Simplify the usage by combining kvm_iodevice_destructor into
kvm_io_bus_unregister_dev. This reduces LOCs a bit for users and can
avoid the leakage of destructing the device explicitly.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Reviewed-by: Sean Christopherson <seanjc@google.com>
Link: https://lore.kernel.org/r/20230207123713.3905-2-wei.w.wang@intel.com
Signed-off-by: Sean Christopherson <seanjc@google.com>
include/kvm/iodev.h
virt/kvm/coalesced_mmio.c
virt/kvm/eventfd.c
virt/kvm/kvm_main.c

index d75fc436574649a078ee83409e71dca0bfaee4f8..56619e33251e2aae75f3de9ac47195193bc3f153 100644 (file)
@@ -55,10 +55,4 @@ static inline int kvm_iodevice_write(struct kvm_vcpu *vcpu,
                                 : -EOPNOTSUPP;
 }
 
-static inline void kvm_iodevice_destructor(struct kvm_io_device *dev)
-{
-       if (dev->ops->destructor)
-               dev->ops->destructor(dev);
-}
-
 #endif /* __KVM_IODEV_H__ */
index 5ef88f5a08640f57a0fbcd10546393039e56e302..1b90acb6e3fef0353c6f66a5cd13beb4daf1070e 100644 (file)
@@ -186,15 +186,10 @@ int kvm_vm_ioctl_unregister_coalesced_mmio(struct kvm *kvm,
                    coalesced_mmio_in_range(dev, zone->addr, zone->size)) {
                        r = kvm_io_bus_unregister_dev(kvm,
                                zone->pio ? KVM_PIO_BUS : KVM_MMIO_BUS, &dev->dev);
-
-                       kvm_iodevice_destructor(&dev->dev);
-
                        /*
                         * On failure, unregister destroys all devices on the
-                        * bus _except_ the target device, i.e. coalesced_zones
-                        * has been modified.  Bail after destroying the target
-                        * device, there's no need to restart the walk as there
-                        * aren't any zones left.
+                        * bus, including the target device. There's no need
+                        * to restart the walk as there aren't any zones left.
                         */
                        if (r)
                                break;
index 7c42441425cf9c47c71d2a794874af4a033539d8..4d47fffe03d9df5cc33c713b64e556b443c9fb11 100644 (file)
@@ -931,7 +931,6 @@ kvm_deassign_ioeventfd_idx(struct kvm *kvm, enum kvm_bus bus_idx,
                bus = kvm_get_bus(kvm, bus_idx);
                if (bus)
                        bus->ioeventfd_count--;
-               ioeventfd_release(p);
                ret = 0;
                break;
        }
index 64dd940c549e17b70f16ec3a27a4d2435db9763c..b8242607392adb4c5d66fcf333fc6466fc55ca65 100644 (file)
@@ -5297,6 +5297,12 @@ static void hardware_disable_all(void)
 }
 #endif /* CONFIG_KVM_GENERIC_HARDWARE_ENABLING */
 
+static void kvm_iodevice_destructor(struct kvm_io_device *dev)
+{
+       if (dev->ops->destructor)
+               dev->ops->destructor(dev);
+}
+
 static void kvm_io_bus_destroy(struct kvm_io_bus *bus)
 {
        int i;
@@ -5520,7 +5526,7 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
                              struct kvm_io_device *dev)
 {
-       int i, j;
+       int i;
        struct kvm_io_bus *new_bus, *bus;
 
        lockdep_assert_held(&kvm->slots_lock);
@@ -5550,18 +5556,19 @@ int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
        rcu_assign_pointer(kvm->buses[bus_idx], new_bus);
        synchronize_srcu_expedited(&kvm->srcu);
 
-       /* Destroy the old bus _after_ installing the (null) bus. */
+       /*
+        * If NULL bus is installed, destroy the old bus, including all the
+        * attached devices. Otherwise, destroy the caller's device only.
+        */
        if (!new_bus) {
                pr_err("kvm: failed to shrink bus, removing it completely\n");
-               for (j = 0; j < bus->dev_count; j++) {
-                       if (j == i)
-                               continue;
-                       kvm_iodevice_destructor(bus->range[j].dev);
-               }
+               kvm_io_bus_destroy(bus);
+               return -ENOMEM;
        }
 
+       kvm_iodevice_destructor(dev);
        kfree(bus);
-       return new_bus ? 0 : -ENOMEM;
+       return 0;
 }
 
 struct kvm_io_device *kvm_io_bus_get_dev(struct kvm *kvm, enum kvm_bus bus_idx,