All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Tian, Kevin" <kevin.tian@intel.com>
To: Christoph Hellwig <hch@lst.de>,
	Alex Williamson <alex.williamson@redhat.com>
Cc: Diana Craciun <diana.craciun@oss.nxp.com>,
	Cornelia Huck <cohuck@redhat.com>,
	Kirti Wankhede <kwankhede@nvidia.com>,
	Eric Auger <eric.auger@redhat.com>,
	Jason Gunthorpe <jgg@ziepe.ca>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	Jason Gunthorpe <jgg@nvidia.com>
Subject: RE: [PATCH 01/14] vfio: Move vfio_iommu_group_get() to vfio_register_group_dev()
Date: Fri, 27 Aug 2021 02:02:12 +0000	[thread overview]
Message-ID: <BN9PR11MB5433FD7780C3F482349FBADF8CC89@BN9PR11MB5433.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20210826133424.3362-2-hch@lst.de>

> From: Christoph Hellwig <hch@lst.de>
> Sent: Thursday, August 26, 2021 9:34 PM
> 
> From: Jason Gunthorpe <jgg@nvidia.com>
> 
> We don't need to hold a reference to the group in the driver as well as
> obtain a reference to the same group as the first thing
> vfio_register_group_dev() does.
> 
> Since the drivers never use the group move this all into the core code.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

> ---
>  drivers/vfio/fsl-mc/vfio_fsl_mc.c            | 17 ++-------
>  drivers/vfio/pci/vfio_pci_core.c             | 13 ++-----
>  drivers/vfio/platform/vfio_platform_common.c | 13 +------
>  drivers/vfio/vfio.c                          | 36 ++++++++------------
>  include/linux/vfio.h                         |  3 --
>  5 files changed, 19 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-
> mc/vfio_fsl_mc.c
> index 0ead91bfa83867..9e838fed560339 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> @@ -505,22 +505,13 @@ static void vfio_fsl_uninit_device(struct
> vfio_fsl_mc_device *vdev)
> 
>  static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
>  {
> -	struct iommu_group *group;
>  	struct vfio_fsl_mc_device *vdev;
>  	struct device *dev = &mc_dev->dev;
>  	int ret;
> 
> -	group = vfio_iommu_group_get(dev);
> -	if (!group) {
> -		dev_err(dev, "VFIO_FSL_MC: No IOMMU group\n");
> -		return -EINVAL;
> -	}
> -
>  	vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
> -	if (!vdev) {
> -		ret = -ENOMEM;
> -		goto out_group_put;
> -	}
> +	if (!vdev)
> +		return -ENOMEM;
> 
>  	vfio_init_group_dev(&vdev->vdev, dev, &vfio_fsl_mc_ops);
>  	vdev->mc_dev = mc_dev;
> @@ -556,8 +547,6 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device
> *mc_dev)
>  out_uninit:
>  	vfio_uninit_group_dev(&vdev->vdev);
>  	kfree(vdev);
> -out_group_put:
> -	vfio_iommu_group_put(group, dev);
>  	return ret;
>  }
> 
> @@ -574,8 +563,6 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device
> *mc_dev)
> 
>  	vfio_uninit_group_dev(&vdev->vdev);
>  	kfree(vdev);
> -	vfio_iommu_group_put(mc_dev->dev.iommu_group, dev);
> -
>  	return 0;
>  }
> 
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index c67751948504af..4134dceab3f73b 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -1807,7 +1807,6 @@
> EXPORT_SYMBOL_GPL(vfio_pci_core_uninit_device);
>  int vfio_pci_core_register_device(struct vfio_pci_core_device *vdev)
>  {
>  	struct pci_dev *pdev = vdev->pdev;
> -	struct iommu_group *group;
>  	int ret;
> 
>  	if (pdev->hdr_type != PCI_HEADER_TYPE_NORMAL)
> @@ -1826,10 +1825,6 @@ int vfio_pci_core_register_device(struct
> vfio_pci_core_device *vdev)
>  		return -EBUSY;
>  	}
> 
> -	group = vfio_iommu_group_get(&pdev->dev);
> -	if (!group)
> -		return -EINVAL;
> -
>  	if (pci_is_root_bus(pdev->bus)) {
>  		ret = vfio_assign_device_set(&vdev->vdev, vdev);
>  	} else if (!pci_probe_reset_slot(pdev->slot)) {
> @@ -1843,10 +1838,10 @@ int vfio_pci_core_register_device(struct
> vfio_pci_core_device *vdev)
>  	}
> 
>  	if (ret)
> -		goto out_group_put;
> +		return ret;
>  	ret = vfio_pci_vf_init(vdev);
>  	if (ret)
> -		goto out_group_put;
> +		return ret;
>  	ret = vfio_pci_vga_init(vdev);
>  	if (ret)
>  		goto out_vf;
> @@ -1877,8 +1872,6 @@ int vfio_pci_core_register_device(struct
> vfio_pci_core_device *vdev)
>  		vfio_pci_set_power_state(vdev, PCI_D0);
>  out_vf:
>  	vfio_pci_vf_uninit(vdev);
> -out_group_put:
> -	vfio_iommu_group_put(group, &pdev->dev);
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(vfio_pci_core_register_device);
> @@ -1894,8 +1887,6 @@ void vfio_pci_core_unregister_device(struct
> vfio_pci_core_device *vdev)
>  	vfio_pci_vf_uninit(vdev);
>  	vfio_pci_vga_uninit(vdev);
> 
> -	vfio_iommu_group_put(pdev->dev.iommu_group, &pdev->dev);
> -
>  	if (!disable_idle_d3)
>  		vfio_pci_set_power_state(vdev, PCI_D0);
>  }
> diff --git a/drivers/vfio/platform/vfio_platform_common.c
> b/drivers/vfio/platform/vfio_platform_common.c
> index 6af7ce7d619c25..256f55b84e70a0 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -642,7 +642,6 @@ static int vfio_platform_of_probe(struct
> vfio_platform_device *vdev,
>  int vfio_platform_probe_common(struct vfio_platform_device *vdev,
>  			       struct device *dev)
>  {
> -	struct iommu_group *group;
>  	int ret;
> 
>  	vfio_init_group_dev(&vdev->vdev, dev, &vfio_platform_ops);
> @@ -663,24 +662,15 @@ int vfio_platform_probe_common(struct
> vfio_platform_device *vdev,
>  		goto out_uninit;
>  	}
> 
> -	group = vfio_iommu_group_get(dev);
> -	if (!group) {
> -		dev_err(dev, "No IOMMU group for device %s\n", vdev-
> >name);
> -		ret = -EINVAL;
> -		goto put_reset;
> -	}
> -
>  	ret = vfio_register_group_dev(&vdev->vdev);
>  	if (ret)
> -		goto put_iommu;
> +		goto put_reset;
> 
>  	mutex_init(&vdev->igate);
> 
>  	pm_runtime_enable(dev);
>  	return 0;
> 
> -put_iommu:
> -	vfio_iommu_group_put(group, dev);
>  put_reset:
>  	vfio_platform_put_reset(vdev);
>  out_uninit:
> @@ -696,7 +686,6 @@ void vfio_platform_remove_common(struct
> vfio_platform_device *vdev)
>  	pm_runtime_disable(vdev->device);
>  	vfio_platform_put_reset(vdev);
>  	vfio_uninit_group_dev(&vdev->vdev);
> -	vfio_iommu_group_put(vdev->vdev.dev->iommu_group, vdev-
> >vdev.dev);
>  }
>  EXPORT_SYMBOL_GPL(vfio_platform_remove_common);
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 3c034fe14ccb03..b39da9b90c95bc 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -169,15 +169,7 @@ static void vfio_release_device_set(struct
> vfio_device *device)
>  	xa_unlock(&vfio_device_set_xa);
>  }
> 
> -/*
> - * vfio_iommu_group_{get,put} are only intended for VFIO bus driver probe
> - * and remove functions, any use cases other than acquiring the first
> - * reference for the purpose of calling vfio_register_group_dev() or
> removing
> - * that symmetric reference after vfio_unregister_group_dev() should use
> the raw
> - * iommu_group_{get,put} functions.  In particular, vfio_iommu_group_put()
> - * removes the device from the dummy group and cannot be nested.
> - */
> -struct iommu_group *vfio_iommu_group_get(struct device *dev)
> +static struct iommu_group *vfio_iommu_group_get(struct device *dev)
>  {
>  	struct iommu_group *group;
>  	int __maybe_unused ret;
> @@ -220,18 +212,6 @@ struct iommu_group *vfio_iommu_group_get(struct
> device *dev)
> 
>  	return group;
>  }
> -EXPORT_SYMBOL_GPL(vfio_iommu_group_get);
> -
> -void vfio_iommu_group_put(struct iommu_group *group, struct device
> *dev)
> -{
> -#ifdef CONFIG_VFIO_NOIOMMU
> -	if (iommu_group_get_iommudata(group) == &noiommu)
> -		iommu_group_remove_device(dev);
> -#endif
> -
> -	iommu_group_put(group);
> -}
> -EXPORT_SYMBOL_GPL(vfio_iommu_group_put);
> 
>  #ifdef CONFIG_VFIO_NOIOMMU
>  static void *vfio_noiommu_open(unsigned long arg)
> @@ -841,7 +821,7 @@ int vfio_register_group_dev(struct vfio_device
> *device)
>  	if (!device->dev_set)
>  		vfio_assign_device_set(device, device);
> 
> -	iommu_group = iommu_group_get(device->dev);
> +	iommu_group = vfio_iommu_group_get(device->dev);
>  	if (!iommu_group)
>  		return -EINVAL;
> 
> @@ -849,6 +829,10 @@ int vfio_register_group_dev(struct vfio_device
> *device)
>  	if (!group) {
>  		group = vfio_create_group(iommu_group);
>  		if (IS_ERR(group)) {
> +#ifdef CONFIG_VFIO_NOIOMMU
> +			if (iommu_group_get_iommudata(iommu_group) ==
> &noiommu)
> +				iommu_group_remove_device(device->dev);
> +#endif
>  			iommu_group_put(iommu_group);
>  			return PTR_ERR(group);
>  		}
> @@ -865,6 +849,10 @@ int vfio_register_group_dev(struct vfio_device
> *device)
>  		dev_WARN(device->dev, "Device already exists on
> group %d\n",
>  			 iommu_group_id(iommu_group));
>  		vfio_device_put(existing_device);
> +#ifdef CONFIG_VFIO_NOIOMMU
> +		if (iommu_group_get_iommudata(iommu_group) ==
> &noiommu)
> +			iommu_group_remove_device(device->dev);
> +#endif
>  		vfio_group_put(group);
>  		return -EBUSY;
>  	}
> @@ -1010,6 +998,10 @@ void vfio_unregister_group_dev(struct vfio_device
> *device)
>  	if (list_empty(&group->device_list))
>  		wait_event(group->container_q, !group->container);
> 
> +#ifdef CONFIG_VFIO_NOIOMMU
> +	if (iommu_group_get_iommudata(group) == &noiommu)
> +		iommu_group_remove_device(dev);
> +#endif
>  	/* Matches the get in vfio_register_group_dev() */
>  	vfio_group_put(group);
>  }
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index b53a9557884ada..f7083c2fd0d099 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -71,9 +71,6 @@ struct vfio_device_ops {
>  	int	(*match)(struct vfio_device *vdev, char *buf);
>  };
> 
> -extern struct iommu_group *vfio_iommu_group_get(struct device *dev);
> -extern void vfio_iommu_group_put(struct iommu_group *group, struct
> device *dev);
> -
>  void vfio_init_group_dev(struct vfio_device *device, struct device *dev,
>  			 const struct vfio_device_ops *ops);
>  void vfio_uninit_group_dev(struct vfio_device *device);
> --
> 2.30.2


  reply	other threads:[~2021-08-27  2:02 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-26 13:34 cleanup vfio iommu_group creation v4 Christoph Hellwig
2021-08-26 13:34 ` [PATCH 01/14] vfio: Move vfio_iommu_group_get() to vfio_register_group_dev() Christoph Hellwig
2021-08-27  2:02   ` Tian, Kevin [this message]
2021-08-26 13:34 ` [PATCH 02/14] vfio: factor out a vfio_iommu_driver_allowed helper Christoph Hellwig
2021-08-26 13:34 ` [PATCH 03/14] vfio: remove the iommudata check in vfio_noiommu_attach_group Christoph Hellwig
2021-08-26 13:34 ` [PATCH 04/14] vfio: factor out a vfio_group_find_or_alloc helper Christoph Hellwig
2021-08-26 19:54   ` Alex Williamson
2021-08-26 23:35     ` Jason Gunthorpe
2021-08-27  2:07       ` Tian, Kevin
2021-08-27 14:35       ` Alex Williamson
2021-08-26 13:34 ` [PATCH 05/14] vfio: refactor noiommu group creation Christoph Hellwig
2021-08-26 22:47   ` Alex Williamson
2021-08-27  2:08     ` Tian, Kevin
2021-08-26 13:34 ` [PATCH 06/14] vfio: remove the iommudata hack for noiommu groups Christoph Hellwig
2021-08-26 13:34 ` [PATCH 07/14] vfio: simplify iommu group allocation for mediated devices Christoph Hellwig
2021-08-26 17:24   ` Alex Williamson
2021-08-26 22:59   ` Alex Williamson
2021-08-27  2:17     ` Tian, Kevin
2021-08-27  5:42       ` Christoph Hellwig
2021-08-26 13:34 ` [PATCH 08/14] vfio: remove unused method from vfio_iommu_driver_ops Christoph Hellwig
2021-08-26 13:34 ` [PATCH 09/14] vfio: move the vfio_iommu_driver_ops interface out of <linux/vfio.h> Christoph Hellwig
2021-08-26 13:34 ` [PATCH 10/14] vfio: remove the unused mdev iommu hook Christoph Hellwig
2021-08-26 13:34 ` [PATCH 11/14] vfio: clean up the check for mediated device in vfio_iommu_type1 Christoph Hellwig
2021-08-26 13:34 ` [PATCH 12/14] vfio/spapr_tce: reject mediated devices Christoph Hellwig
2021-08-26 13:34 ` [PATCH 13/14] vfio/iommu_type1: remove the "external" domain Christoph Hellwig
2021-08-26 23:08   ` Alex Williamson
2021-08-26 13:34 ` [PATCH 14/14] vfio/iommu_type1: remove IS_IOMMU_CAP_DOMAIN_IN_CONTAINER Christoph Hellwig
2021-08-31 15:26 ` cleanup vfio iommu_group creation v4 Xu, Terrence
  -- strict thread matches above, loose matches on Subject: below --
2021-09-13  7:15 cleanup vfio iommu_group creation v5 Christoph Hellwig
2021-09-13  7:15 ` [PATCH 01/14] vfio: Move vfio_iommu_group_get() to vfio_register_group_dev() Christoph Hellwig
2021-09-14  1:57   ` Tian, Kevin
2021-09-14  5:46     ` Christoph Hellwig
2021-09-14  6:11       ` Tian, Kevin
2021-08-25 16:19 cleanup vfio iommu_group creation v3 Christoph Hellwig
2021-08-25 16:19 ` [PATCH 01/14] vfio: Move vfio_iommu_group_get() to vfio_register_group_dev() Christoph Hellwig
2021-08-26  1:40   ` Tian, Kevin
2021-08-26  8:21     ` Christoph Hellwig
2021-08-24 14:46 cleanup vfio iommu_group creation v2 Christoph Hellwig
2021-08-24 14:46 ` [PATCH 01/14] vfio: Move vfio_iommu_group_get() to vfio_register_group_dev() Christoph Hellwig
2021-08-24 20:25   ` Alex Williamson
2021-08-24 22:50     ` Jason Gunthorpe
2021-08-25 12:43     ` Christoph Hellwig
2021-08-25 15:48       ` Alex Williamson
2021-08-25 16:08         ` Jason Gunthorpe
2021-08-11 15:14 cleanup vfio iommu_group creation Christoph Hellwig
2021-08-11 15:14 ` [PATCH 01/14] vfio: Move vfio_iommu_group_get() to vfio_register_group_dev() Christoph Hellwig

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=BN9PR11MB5433FD7780C3F482349FBADF8CC89@BN9PR11MB5433.namprd11.prod.outlook.com \
    --to=kevin.tian@intel.com \
    --cc=alex.williamson@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=diana.craciun@oss.nxp.com \
    --cc=eric.auger@redhat.com \
    --cc=hch@lst.de \
    --cc=jgg@nvidia.com \
    --cc=jgg@ziepe.ca \
    --cc=kvm@vger.kernel.org \
    --cc=kwankhede@nvidia.com \
    /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: link
Be 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.