> From: Christoph Hellwig <hch@xxxxxx>
> Sent: Monday, September 13, 2021 3:16 PM
>
> 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@xxxxxx>
> Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>
> ---
> drivers/vfio/vfio.c | 104 ++++++++++++++++++++++----------------------
> 1 file changed, 52 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 2b2679c7126fdf..b1ed156adccd04 100644
> --- 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;
> --
> 2.30.2
