Skip to content

Commit 93219ea

Browse files
jgunthorpejoergroedel
authored andcommitted
vfio: Delete the unbound_list
commit 60720a0 ("vfio: Add device tracking during unbind") added the unbound list to plug a problem with KVM where KVM_DEV_VFIO_GROUP_DEL relied on vfio_group_get_external_user() succeeding to return the vfio_group from a group file descriptor. The unbound list allowed vfio_group_get_external_user() to continue to succeed in edge cases. However commit 5d6dee8 ("vfio: New external user group/file match") deleted the call to vfio_group_get_external_user() during KVM_DEV_VFIO_GROUP_DEL. Instead vfio_external_group_match_file() is used to directly match the file descriptor to the group pointer. This in turn avoids the call down to vfio_dev_viable() during KVM_DEV_VFIO_GROUP_DEL and also avoids the trouble the first commit was trying to fix. There are no other users of vfio_dev_viable() that care about the time after vfio_unregister_group_dev() returns, so simply delete the unbound_list entirely. Reviewed-by: Chaitanya Kulkarni <[email protected]> Signed-off-by: Jason Gunthorpe <[email protected]> Signed-off-by: Lu Baolu <[email protected]> Acked-by: Alex Williamson <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Joerg Roedel <[email protected]>
1 parent 31076af commit 93219ea

File tree

1 file changed

+2
-72
lines changed

1 file changed

+2
-72
lines changed

drivers/vfio/vfio.c

Lines changed: 2 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,6 @@ struct vfio_container {
6262
bool noiommu;
6363
};
6464

65-
struct vfio_unbound_dev {
66-
struct device *dev;
67-
struct list_head unbound_next;
68-
};
69-
7065
struct vfio_group {
7166
struct device dev;
7267
struct cdev cdev;
@@ -79,8 +74,6 @@ struct vfio_group {
7974
struct notifier_block nb;
8075
struct list_head vfio_next;
8176
struct list_head container_next;
82-
struct list_head unbound_list;
83-
struct mutex unbound_lock;
8477
atomic_t opened;
8578
wait_queue_head_t container_q;
8679
enum vfio_group_type type;
@@ -340,16 +333,8 @@ vfio_group_get_from_iommu(struct iommu_group *iommu_group)
340333
static void vfio_group_release(struct device *dev)
341334
{
342335
struct vfio_group *group = container_of(dev, struct vfio_group, dev);
343-
struct vfio_unbound_dev *unbound, *tmp;
344-
345-
list_for_each_entry_safe(unbound, tmp,
346-
&group->unbound_list, unbound_next) {
347-
list_del(&unbound->unbound_next);
348-
kfree(unbound);
349-
}
350336

351337
mutex_destroy(&group->device_lock);
352-
mutex_destroy(&group->unbound_lock);
353338
iommu_group_put(group->iommu_group);
354339
ida_free(&vfio.group_ida, MINOR(group->dev.devt));
355340
kfree(group);
@@ -381,8 +366,6 @@ static struct vfio_group *vfio_group_alloc(struct iommu_group *iommu_group,
381366
refcount_set(&group->users, 1);
382367
INIT_LIST_HEAD(&group->device_list);
383368
mutex_init(&group->device_lock);
384-
INIT_LIST_HEAD(&group->unbound_list);
385-
mutex_init(&group->unbound_lock);
386369
init_waitqueue_head(&group->container_q);
387370
group->iommu_group = iommu_group;
388371
/* put in vfio_group_release() */
@@ -571,19 +554,8 @@ static int vfio_dev_viable(struct device *dev, void *data)
571554
struct vfio_group *group = data;
572555
struct vfio_device *device;
573556
struct device_driver *drv = READ_ONCE(dev->driver);
574-
struct vfio_unbound_dev *unbound;
575-
int ret = -EINVAL;
576557

577-
mutex_lock(&group->unbound_lock);
578-
list_for_each_entry(unbound, &group->unbound_list, unbound_next) {
579-
if (dev == unbound->dev) {
580-
ret = 0;
581-
break;
582-
}
583-
}
584-
mutex_unlock(&group->unbound_lock);
585-
586-
if (!ret || !drv || vfio_dev_driver_allowed(dev, drv))
558+
if (!drv || vfio_dev_driver_allowed(dev, drv))
587559
return 0;
588560

589561
device = vfio_group_get_device(group, dev);
@@ -592,7 +564,7 @@ static int vfio_dev_viable(struct device *dev, void *data)
592564
return 0;
593565
}
594566

595-
return ret;
567+
return -EINVAL;
596568
}
597569

598570
/*
@@ -634,7 +606,6 @@ static int vfio_iommu_group_notifier(struct notifier_block *nb,
634606
{
635607
struct vfio_group *group = container_of(nb, struct vfio_group, nb);
636608
struct device *dev = data;
637-
struct vfio_unbound_dev *unbound;
638609

639610
switch (action) {
640611
case IOMMU_GROUP_NOTIFY_ADD_DEVICE:
@@ -663,28 +634,6 @@ static int vfio_iommu_group_notifier(struct notifier_block *nb,
663634
__func__, iommu_group_id(group->iommu_group),
664635
dev->driver->name);
665636
break;
666-
case IOMMU_GROUP_NOTIFY_UNBOUND_DRIVER:
667-
dev_dbg(dev, "%s: group %d unbound from driver\n", __func__,
668-
iommu_group_id(group->iommu_group));
669-
/*
670-
* XXX An unbound device in a live group is ok, but we'd
671-
* really like to avoid the above BUG_ON by preventing other
672-
* drivers from binding to it. Once that occurs, we have to
673-
* stop the system to maintain isolation. At a minimum, we'd
674-
* want a toggle to disable driver auto probe for this device.
675-
*/
676-
677-
mutex_lock(&group->unbound_lock);
678-
list_for_each_entry(unbound,
679-
&group->unbound_list, unbound_next) {
680-
if (dev == unbound->dev) {
681-
list_del(&unbound->unbound_next);
682-
kfree(unbound);
683-
break;
684-
}
685-
}
686-
mutex_unlock(&group->unbound_lock);
687-
break;
688637
}
689638
return NOTIFY_OK;
690639
}
@@ -889,29 +838,10 @@ static struct vfio_device *vfio_device_get_from_name(struct vfio_group *group,
889838
void vfio_unregister_group_dev(struct vfio_device *device)
890839
{
891840
struct vfio_group *group = device->group;
892-
struct vfio_unbound_dev *unbound;
893841
unsigned int i = 0;
894842
bool interrupted = false;
895843
long rc;
896844

897-
/*
898-
* When the device is removed from the group, the group suddenly
899-
* becomes non-viable; the device has a driver (until the unbind
900-
* completes), but it's not present in the group. This is bad news
901-
* for any external users that need to re-acquire a group reference
902-
* in order to match and release their existing reference. To
903-
* solve this, we track such devices on the unbound_list to bridge
904-
* the gap until they're fully unbound.
905-
*/
906-
unbound = kzalloc(sizeof(*unbound), GFP_KERNEL);
907-
if (unbound) {
908-
unbound->dev = device->dev;
909-
mutex_lock(&group->unbound_lock);
910-
list_add(&unbound->unbound_next, &group->unbound_list);
911-
mutex_unlock(&group->unbound_lock);
912-
}
913-
WARN_ON(!unbound);
914-
915845
vfio_device_put(device);
916846
rc = try_wait_for_completion(&device->comp);
917847
while (rc <= 0) {

0 commit comments

Comments
 (0)