vfio: refactor noiommu group creation

Split the actual noiommu group creation from vfio_iommu_group_get into a
new helper, and open code the rest of vfio_iommu_group_get in its only
caller.  This creates an entirely separate and clear code path for the
noiommu group creation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Link: https://lore.kernel.org/r/20210924155705.4258-6-hch@lst.de
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 2b2679c7126fdfc27c1f1fbfb6e0b05e2af7907f..b1ed156adccd04d22f3de7adf2b3a5b93e29160d 100644 (file)
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -169,50 +169,6 @@ static void vfio_release_device_set(struct vfio_device *device)
        xa_unlock(&vfio_device_set_xa);
 }
 
-static struct iommu_group *vfio_iommu_group_get(struct device *dev)
-{
-       struct iommu_group *group;
-       int __maybe_unused ret;
-
-       group = iommu_group_get(dev);
-
-#ifdef CONFIG_VFIO_NOIOMMU
-       /*
-        * With noiommu enabled, an IOMMU group will be created for a device
-        * that doesn't already have one and doesn't have an iommu_ops on their
-        * bus.  We set iommudata simply to be able to identify these groups
-        * as special use and for reclamation later.
-        */
-       if (group || !noiommu || iommu_present(dev->bus))
-               return group;
-
-       group = iommu_group_alloc();
-       if (IS_ERR(group))
-               return NULL;
-
-       iommu_group_set_name(group, "vfio-noiommu");
-       iommu_group_set_iommudata(group, &noiommu, NULL);
-       ret = iommu_group_add_device(group, dev);
-       if (ret) {
-               iommu_group_put(group);
-               return NULL;
-       }
-
-       /*
-        * Where to taint?  At this point we've added an IOMMU group for a
-        * device that is not backed by iommu_ops, therefore any iommu_
-        * callback using iommu_ops can legitimately Oops.  So, while we may
-        * be about to give a DMA capable device to a user without IOMMU
-        * protection, which is clearly taint-worthy, let's go ahead and do
-        * it here.
-        */
-       add_taint(TAINT_USER, LOCKDEP_STILL_OK);
-       dev_warn(dev, "Adding kernel taint for vfio-noiommu group on device\n");
-#endif
-
-       return group;
-}
-
 #ifdef CONFIG_VFIO_NOIOMMU
 static void *vfio_noiommu_open(unsigned long arg)
 {
@@ -823,12 +779,61 @@ void vfio_uninit_group_dev(struct vfio_device *device)
 }
 EXPORT_SYMBOL_GPL(vfio_uninit_group_dev);
 
-struct vfio_group *vfio_group_find_or_alloc(struct device *dev)
+#ifdef CONFIG_VFIO_NOIOMMU
+static struct vfio_group *vfio_noiommu_group_alloc(struct device *dev)
 {
        struct iommu_group *iommu_group;
        struct vfio_group *group;
+       int ret;
+
+       iommu_group = iommu_group_alloc();
+       if (IS_ERR(iommu_group))
+               return ERR_CAST(iommu_group);
 
-       iommu_group = vfio_iommu_group_get(dev);
+       iommu_group_set_name(iommu_group, "vfio-noiommu");
+       iommu_group_set_iommudata(iommu_group, &noiommu, NULL);
+       ret = iommu_group_add_device(iommu_group, dev);
+       if (ret)
+               goto out_put_group;
+
+       group = vfio_create_group(iommu_group);
+       if (IS_ERR(group)) {
+               ret = PTR_ERR(group);
+               goto out_remove_device;
+       }
+
+       return group;
+
+out_remove_device:
+       iommu_group_remove_device(dev);
+out_put_group:
+       iommu_group_put(iommu_group);
+       return ERR_PTR(ret);
+}
+#endif
+
+static struct vfio_group *vfio_group_find_or_alloc(struct device *dev)
+{
+       struct iommu_group *iommu_group;
+       struct vfio_group *group;
+
+       iommu_group = iommu_group_get(dev);
+#ifdef CONFIG_VFIO_NOIOMMU
+       if (!iommu_group && noiommu && !iommu_present(dev->bus)) {
+               /*
+                * With noiommu enabled, create an IOMMU group for devices that
+                * don't already have one and don't have an iommu_ops on their
+                * bus.  Taint the kernel because we're about to give a DMA
+                * capable device to a user without IOMMU protection.
+                */
+               group = vfio_noiommu_group_alloc(dev);
+               if (!IS_ERR(group)) {
+                       add_taint(TAINT_USER, LOCKDEP_STILL_OK);
+                       dev_warn(dev, "Adding kernel taint for vfio-noiommu group on device\n");
+               }
+               return group;
+       }
+#endif
        if (!iommu_group)
                return ERR_PTR(-EINVAL);
 
@@ -840,14 +845,9 @@ struct vfio_group *vfio_group_find_or_alloc(struct device *dev)
        /* a newly created vfio_group keeps the reference. */
        group = vfio_create_group(iommu_group);
        if (IS_ERR(group))
-               goto out_remove;
+               goto out_put;
        return group;
 
-out_remove:
-#ifdef CONFIG_VFIO_NOIOMMU
-       if (iommu_group_get_iommudata(iommu_group) == &noiommu)
-               iommu_group_remove_device(dev);
-#endif
 out_put:
        iommu_group_put(iommu_group);
        return group;