From: Lu Baolu <baolu.lu@linux.intel.com> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Joerg Roedel <joro@8bytes.org>, Alex Williamson <alex.williamson@redhat.com>, Bjorn Helgaas <bhelgaas@google.com>, Jason Gunthorpe <jgg@nvidia.com>, Kevin Tian <kevin.tian@intel.com>, Ashok Raj <ashok.raj@intel.com> Cc: Chaitanya Kulkarni <kch@nvidia.com>, kvm@vger.kernel.org, rafael@kernel.org, linux-pci@vger.kernel.org, Cornelia Huck <cohuck@redhat.com>, linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, Jacob jun Pan <jacob.jun.pan@intel.com>, Diana Craciun <diana.craciun@oss.nxp.com>, Will Deacon <will@kernel.org> Subject: [PATCH 09/11] vfio: Delete the unbound_list Date: Mon, 15 Nov 2021 10:05:50 +0800 [thread overview] Message-ID: <20211115020552.2378167-10-baolu.lu@linux.intel.com> (raw) In-Reply-To: <20211115020552.2378167-1-baolu.lu@linux.intel.com> From: Jason Gunthorpe <jgg@nvidia.com> commit 60720a0fc646 ("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 5d6dee80a1e9 ("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 <kch@nvidia.com> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> --- drivers/vfio/vfio.c | 74 ++------------------------------------------- 1 file changed, 2 insertions(+), 72 deletions(-) diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 6847117fac6a..8c317d1a0f3c 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -62,11 +62,6 @@ struct vfio_container { bool noiommu; }; -struct vfio_unbound_dev { - struct device *dev; - struct list_head unbound_next; -}; - struct vfio_group { struct device dev; struct cdev cdev; @@ -79,8 +74,6 @@ struct vfio_group { struct notifier_block nb; struct list_head vfio_next; struct list_head container_next; - struct list_head unbound_list; - struct mutex unbound_lock; atomic_t opened; wait_queue_head_t container_q; enum vfio_group_type type; @@ -340,16 +333,8 @@ vfio_group_get_from_iommu(struct iommu_group *iommu_group) static void vfio_group_release(struct device *dev) { struct vfio_group *group = container_of(dev, struct vfio_group, dev); - struct vfio_unbound_dev *unbound, *tmp; - - list_for_each_entry_safe(unbound, tmp, - &group->unbound_list, unbound_next) { - list_del(&unbound->unbound_next); - kfree(unbound); - } mutex_destroy(&group->device_lock); - mutex_destroy(&group->unbound_lock); iommu_group_put(group->iommu_group); ida_free(&vfio.group_ida, MINOR(group->dev.devt)); kfree(group); @@ -381,8 +366,6 @@ static struct vfio_group *vfio_group_alloc(struct iommu_group *iommu_group, refcount_set(&group->users, 1); INIT_LIST_HEAD(&group->device_list); mutex_init(&group->device_lock); - INIT_LIST_HEAD(&group->unbound_list); - mutex_init(&group->unbound_lock); init_waitqueue_head(&group->container_q); group->iommu_group = iommu_group; /* put in vfio_group_release() */ @@ -571,19 +554,8 @@ static int vfio_dev_viable(struct device *dev, void *data) struct vfio_group *group = data; struct vfio_device *device; struct device_driver *drv = READ_ONCE(dev->driver); - struct vfio_unbound_dev *unbound; - int ret = -EINVAL; - mutex_lock(&group->unbound_lock); - list_for_each_entry(unbound, &group->unbound_list, unbound_next) { - if (dev == unbound->dev) { - ret = 0; - break; - } - } - mutex_unlock(&group->unbound_lock); - - if (!ret || !drv || vfio_dev_driver_allowed(dev, drv)) + if (!drv || vfio_dev_driver_allowed(dev, drv)) return 0; device = vfio_group_get_device(group, dev); @@ -592,7 +564,7 @@ static int vfio_dev_viable(struct device *dev, void *data) return 0; } - return ret; + return -EINVAL; } /** @@ -634,7 +606,6 @@ static int vfio_iommu_group_notifier(struct notifier_block *nb, { struct vfio_group *group = container_of(nb, struct vfio_group, nb); struct device *dev = data; - struct vfio_unbound_dev *unbound; switch (action) { case IOMMU_GROUP_NOTIFY_ADD_DEVICE: @@ -663,28 +634,6 @@ static int vfio_iommu_group_notifier(struct notifier_block *nb, __func__, iommu_group_id(group->iommu_group), dev->driver->name); break; - case IOMMU_GROUP_NOTIFY_UNBOUND_DRIVER: - dev_dbg(dev, "%s: group %d unbound from driver\n", __func__, - iommu_group_id(group->iommu_group)); - /* - * XXX An unbound device in a live group is ok, but we'd - * really like to avoid the above BUG_ON by preventing other - * drivers from binding to it. Once that occurs, we have to - * stop the system to maintain isolation. At a minimum, we'd - * want a toggle to disable driver auto probe for this device. - */ - - mutex_lock(&group->unbound_lock); - list_for_each_entry(unbound, - &group->unbound_list, unbound_next) { - if (dev == unbound->dev) { - list_del(&unbound->unbound_next); - kfree(unbound); - break; - } - } - mutex_unlock(&group->unbound_lock); - break; } return NOTIFY_OK; } @@ -889,29 +838,10 @@ static struct vfio_device *vfio_device_get_from_name(struct vfio_group *group, void vfio_unregister_group_dev(struct vfio_device *device) { struct vfio_group *group = device->group; - struct vfio_unbound_dev *unbound; unsigned int i = 0; bool interrupted = false; long rc; - /* - * When the device is removed from the group, the group suddenly - * becomes non-viable; the device has a driver (until the unbind - * completes), but it's not present in the group. This is bad news - * for any external users that need to re-acquire a group reference - * in order to match and release their existing reference. To - * solve this, we track such devices on the unbound_list to bridge - * the gap until they're fully unbound. - */ - unbound = kzalloc(sizeof(*unbound), GFP_KERNEL); - if (unbound) { - unbound->dev = device->dev; - mutex_lock(&group->unbound_lock); - list_add(&unbound->unbound_next, &group->unbound_list); - mutex_unlock(&group->unbound_lock); - } - WARN_ON(!unbound); - vfio_device_put(device); rc = try_wait_for_completion(&device->comp); while (rc <= 0) { -- 2.25.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
WARNING: multiple messages have this Message-ID (diff)
From: Lu Baolu <baolu.lu@linux.intel.com> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Joerg Roedel <joro@8bytes.org>, Alex Williamson <alex.williamson@redhat.com>, Bjorn Helgaas <bhelgaas@google.com>, Jason Gunthorpe <jgg@nvidia.com>, Kevin Tian <kevin.tian@intel.com>, Ashok Raj <ashok.raj@intel.com> Cc: Will Deacon <will@kernel.org>, rafael@kernel.org, Diana Craciun <diana.craciun@oss.nxp.com>, Cornelia Huck <cohuck@redhat.com>, Eric Auger <eric.auger@redhat.com>, Liu Yi L <yi.l.liu@intel.com>, Jacob jun Pan <jacob.jun.pan@intel.com>, Chaitanya Kulkarni <kch@nvidia.com>, iommu@lists.linux-foundation.org, linux-pci@vger.kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Lu Baolu <baolu.lu@linux.intel.com> Subject: [PATCH 09/11] vfio: Delete the unbound_list Date: Mon, 15 Nov 2021 10:05:50 +0800 [thread overview] Message-ID: <20211115020552.2378167-10-baolu.lu@linux.intel.com> (raw) In-Reply-To: <20211115020552.2378167-1-baolu.lu@linux.intel.com> From: Jason Gunthorpe <jgg@nvidia.com> commit 60720a0fc646 ("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 5d6dee80a1e9 ("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 <kch@nvidia.com> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> --- drivers/vfio/vfio.c | 74 ++------------------------------------------- 1 file changed, 2 insertions(+), 72 deletions(-) diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 6847117fac6a..8c317d1a0f3c 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -62,11 +62,6 @@ struct vfio_container { bool noiommu; }; -struct vfio_unbound_dev { - struct device *dev; - struct list_head unbound_next; -}; - struct vfio_group { struct device dev; struct cdev cdev; @@ -79,8 +74,6 @@ struct vfio_group { struct notifier_block nb; struct list_head vfio_next; struct list_head container_next; - struct list_head unbound_list; - struct mutex unbound_lock; atomic_t opened; wait_queue_head_t container_q; enum vfio_group_type type; @@ -340,16 +333,8 @@ vfio_group_get_from_iommu(struct iommu_group *iommu_group) static void vfio_group_release(struct device *dev) { struct vfio_group *group = container_of(dev, struct vfio_group, dev); - struct vfio_unbound_dev *unbound, *tmp; - - list_for_each_entry_safe(unbound, tmp, - &group->unbound_list, unbound_next) { - list_del(&unbound->unbound_next); - kfree(unbound); - } mutex_destroy(&group->device_lock); - mutex_destroy(&group->unbound_lock); iommu_group_put(group->iommu_group); ida_free(&vfio.group_ida, MINOR(group->dev.devt)); kfree(group); @@ -381,8 +366,6 @@ static struct vfio_group *vfio_group_alloc(struct iommu_group *iommu_group, refcount_set(&group->users, 1); INIT_LIST_HEAD(&group->device_list); mutex_init(&group->device_lock); - INIT_LIST_HEAD(&group->unbound_list); - mutex_init(&group->unbound_lock); init_waitqueue_head(&group->container_q); group->iommu_group = iommu_group; /* put in vfio_group_release() */ @@ -571,19 +554,8 @@ static int vfio_dev_viable(struct device *dev, void *data) struct vfio_group *group = data; struct vfio_device *device; struct device_driver *drv = READ_ONCE(dev->driver); - struct vfio_unbound_dev *unbound; - int ret = -EINVAL; - mutex_lock(&group->unbound_lock); - list_for_each_entry(unbound, &group->unbound_list, unbound_next) { - if (dev == unbound->dev) { - ret = 0; - break; - } - } - mutex_unlock(&group->unbound_lock); - - if (!ret || !drv || vfio_dev_driver_allowed(dev, drv)) + if (!drv || vfio_dev_driver_allowed(dev, drv)) return 0; device = vfio_group_get_device(group, dev); @@ -592,7 +564,7 @@ static int vfio_dev_viable(struct device *dev, void *data) return 0; } - return ret; + return -EINVAL; } /** @@ -634,7 +606,6 @@ static int vfio_iommu_group_notifier(struct notifier_block *nb, { struct vfio_group *group = container_of(nb, struct vfio_group, nb); struct device *dev = data; - struct vfio_unbound_dev *unbound; switch (action) { case IOMMU_GROUP_NOTIFY_ADD_DEVICE: @@ -663,28 +634,6 @@ static int vfio_iommu_group_notifier(struct notifier_block *nb, __func__, iommu_group_id(group->iommu_group), dev->driver->name); break; - case IOMMU_GROUP_NOTIFY_UNBOUND_DRIVER: - dev_dbg(dev, "%s: group %d unbound from driver\n", __func__, - iommu_group_id(group->iommu_group)); - /* - * XXX An unbound device in a live group is ok, but we'd - * really like to avoid the above BUG_ON by preventing other - * drivers from binding to it. Once that occurs, we have to - * stop the system to maintain isolation. At a minimum, we'd - * want a toggle to disable driver auto probe for this device. - */ - - mutex_lock(&group->unbound_lock); - list_for_each_entry(unbound, - &group->unbound_list, unbound_next) { - if (dev == unbound->dev) { - list_del(&unbound->unbound_next); - kfree(unbound); - break; - } - } - mutex_unlock(&group->unbound_lock); - break; } return NOTIFY_OK; } @@ -889,29 +838,10 @@ static struct vfio_device *vfio_device_get_from_name(struct vfio_group *group, void vfio_unregister_group_dev(struct vfio_device *device) { struct vfio_group *group = device->group; - struct vfio_unbound_dev *unbound; unsigned int i = 0; bool interrupted = false; long rc; - /* - * When the device is removed from the group, the group suddenly - * becomes non-viable; the device has a driver (until the unbind - * completes), but it's not present in the group. This is bad news - * for any external users that need to re-acquire a group reference - * in order to match and release their existing reference. To - * solve this, we track such devices on the unbound_list to bridge - * the gap until they're fully unbound. - */ - unbound = kzalloc(sizeof(*unbound), GFP_KERNEL); - if (unbound) { - unbound->dev = device->dev; - mutex_lock(&group->unbound_lock); - list_add(&unbound->unbound_next, &group->unbound_list); - mutex_unlock(&group->unbound_lock); - } - WARN_ON(!unbound); - vfio_device_put(device); rc = try_wait_for_completion(&device->comp); while (rc <= 0) { -- 2.25.1
next prev parent reply other threads:[~2021-11-15 2:11 UTC|newest] Thread overview: 120+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-11-15 2:05 [PATCH 00/11] Fix BUG_ON in vfio_iommu_group_notifier() Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 2:05 ` [PATCH 01/11] iommu: Add device dma ownership set/release interfaces Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 13:14 ` Christoph Hellwig 2021-11-15 13:14 ` Christoph Hellwig 2021-11-16 1:57 ` Lu Baolu 2021-11-16 1:57 ` Lu Baolu 2021-11-16 13:46 ` Jason Gunthorpe 2021-11-16 13:46 ` Jason Gunthorpe via iommu 2021-11-17 5:22 ` Lu Baolu 2021-11-17 5:22 ` Lu Baolu 2021-11-17 13:35 ` Jason Gunthorpe 2021-11-17 13:35 ` Jason Gunthorpe via iommu 2021-11-18 1:12 ` Lu Baolu 2021-11-18 1:12 ` Lu Baolu 2021-11-18 14:10 ` Jason Gunthorpe 2021-11-18 14:10 ` Jason Gunthorpe via iommu 2021-11-18 2:39 ` Tian, Kevin 2021-11-18 2:39 ` Tian, Kevin 2021-11-18 13:33 ` Jason Gunthorpe 2021-11-18 13:33 ` Jason Gunthorpe via iommu 2021-11-19 5:44 ` Tian, Kevin 2021-11-19 5:44 ` Tian, Kevin 2021-11-19 11:14 ` Lu Baolu 2021-11-19 11:14 ` Lu Baolu 2021-11-19 15:06 ` Jörg Rödel 2021-11-19 15:06 ` Jörg Rödel 2021-11-19 15:43 ` Jason Gunthorpe 2021-11-19 15:43 ` Jason Gunthorpe via iommu 2021-11-20 11:16 ` Lu Baolu 2021-11-20 11:16 ` Lu Baolu 2021-11-19 12:56 ` Jason Gunthorpe 2021-11-19 12:56 ` Jason Gunthorpe via iommu 2021-11-15 20:38 ` Bjorn Helgaas 2021-11-15 20:38 ` Bjorn Helgaas 2021-11-16 1:52 ` Lu Baolu 2021-11-16 1:52 ` Lu Baolu 2021-11-15 2:05 ` [PATCH 02/11] driver core: Set DMA ownership during driver bind/unbind Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 6:59 ` Greg Kroah-Hartman 2021-11-15 6:59 ` Greg Kroah-Hartman 2021-11-15 13:20 ` Christoph Hellwig 2021-11-15 13:20 ` Christoph Hellwig 2021-11-15 13:38 ` Jason Gunthorpe via iommu 2021-11-15 13:38 ` Jason Gunthorpe 2021-11-15 13:19 ` Christoph Hellwig 2021-11-15 13:19 ` Christoph Hellwig 2021-11-15 13:24 ` Jason Gunthorpe 2021-11-15 13:24 ` Jason Gunthorpe via iommu 2021-11-15 15:37 ` Robin Murphy 2021-11-15 15:37 ` Robin Murphy 2021-11-15 15:56 ` Jason Gunthorpe 2021-11-15 15:56 ` Jason Gunthorpe via iommu 2021-11-15 18:15 ` Christoph Hellwig 2021-11-15 18:15 ` Christoph Hellwig 2021-11-15 18:35 ` Robin Murphy 2021-11-15 18:35 ` Robin Murphy 2021-11-15 19:39 ` Jason Gunthorpe via iommu 2021-11-15 19:39 ` Jason Gunthorpe 2021-11-15 2:05 ` [PATCH 03/11] PCI: pci_stub: Suppress kernel DMA ownership auto-claiming Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 13:21 ` Christoph Hellwig 2021-11-15 13:21 ` Christoph Hellwig 2021-11-15 13:31 ` Jason Gunthorpe via iommu 2021-11-15 13:31 ` Jason Gunthorpe 2021-11-15 15:14 ` Robin Murphy 2021-11-15 15:14 ` Robin Murphy 2021-11-15 16:17 ` Jason Gunthorpe 2021-11-15 16:17 ` Jason Gunthorpe via iommu 2021-11-15 17:54 ` Robin Murphy 2021-11-15 17:54 ` Robin Murphy 2021-11-15 18:19 ` Christoph Hellwig 2021-11-15 18:19 ` Christoph Hellwig 2021-11-15 18:44 ` Robin Murphy 2021-11-15 18:44 ` Robin Murphy 2021-11-15 19:22 ` Jason Gunthorpe via iommu 2021-11-15 19:22 ` Jason Gunthorpe 2021-11-15 20:58 ` Robin Murphy 2021-11-15 20:58 ` Robin Murphy 2021-11-15 21:19 ` Jason Gunthorpe via iommu 2021-11-15 21:19 ` Jason Gunthorpe 2021-11-15 20:48 ` Bjorn Helgaas 2021-11-15 20:48 ` Bjorn Helgaas 2021-11-15 22:17 ` Bjorn Helgaas 2021-11-15 22:17 ` Bjorn Helgaas 2021-11-16 6:05 ` Lu Baolu 2021-11-16 6:05 ` Lu Baolu 2021-11-15 2:05 ` [PATCH 04/11] PCI: portdrv: " Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 20:44 ` Bjorn Helgaas 2021-11-15 20:44 ` Bjorn Helgaas 2021-11-16 7:24 ` Lu Baolu 2021-11-16 7:24 ` Lu Baolu 2021-11-16 20:22 ` Bjorn Helgaas 2021-11-16 20:22 ` Bjorn Helgaas 2021-11-16 20:48 ` Jason Gunthorpe 2021-11-16 20:48 ` Jason Gunthorpe via iommu 2021-11-15 2:05 ` [PATCH 05/11] iommu: Add security context management for assigned devices Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 13:22 ` Christoph Hellwig 2021-11-15 13:22 ` Christoph Hellwig 2021-11-16 7:25 ` Lu Baolu 2021-11-16 7:25 ` Lu Baolu 2021-11-15 2:05 ` [PATCH 06/11] iommu: Expose group variants of dma ownership interfaces Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 13:27 ` Christoph Hellwig 2021-11-15 13:27 ` Christoph Hellwig 2021-11-16 9:42 ` Lu Baolu 2021-11-16 9:42 ` Lu Baolu 2021-11-15 2:05 ` [PATCH 07/11] vfio: Use DMA_OWNER_USER to declaim passthrough devices Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 2:05 ` [PATCH 08/11] vfio: Remove use of vfio_group_viable() Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 2:05 ` Lu Baolu [this message] 2021-11-15 2:05 ` [PATCH 09/11] vfio: Delete the unbound_list Lu Baolu 2021-11-15 2:05 ` [PATCH 10/11] vfio: Remove iommu group notifier Lu Baolu 2021-11-15 2:05 ` Lu Baolu 2021-11-15 2:05 ` [PATCH 11/11] iommu: Remove iommu group changes notifier Lu Baolu 2021-11-15 2:05 ` Lu Baolu
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20211115020552.2378167-10-baolu.lu@linux.intel.com \ --to=baolu.lu@linux.intel.com \ --cc=alex.williamson@redhat.com \ --cc=ashok.raj@intel.com \ --cc=bhelgaas@google.com \ --cc=cohuck@redhat.com \ --cc=diana.craciun@oss.nxp.com \ --cc=gregkh@linuxfoundation.org \ --cc=iommu@lists.linux-foundation.org \ --cc=jacob.jun.pan@intel.com \ --cc=jgg@nvidia.com \ --cc=joro@8bytes.org \ --cc=kch@nvidia.com \ --cc=kevin.tian@intel.com \ --cc=kvm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=rafael@kernel.org \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.