From: Alex Williamson <alex.williamson@redhat.com>
To: Liu Yi L <yi.l.liu@intel.com>
Cc: kwankhede@nvidia.com, kevin.tian@intel.com,
baolu.lu@linux.intel.com, yi.y.sun@intel.com, joro@8bytes.org,
linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
yan.y.zhao@intel.com, shaopeng.he@intel.com,
chenbo.xia@intel.com, jun.j.tian@intel.com
Subject: Re: [PATCH v2 11/13] samples/vfio-mdev-pci: call vfio_add_group_dev()
Date: Wed, 25 Sep 2019 20:36:58 -0600 [thread overview]
Message-ID: <20190925203658.18ea50ab@x1.home> (raw)
In-Reply-To: <1567670370-4484-12-git-send-email-yi.l.liu@intel.com>
On Thu, 5 Sep 2019 15:59:28 +0800
Liu Yi L <yi.l.liu@intel.com> wrote:
> This patch adds vfio_add_group_dev() calling in probe() to make
> vfio-mdev-pci work well with non-singleton iommu group. User could
> bind devices from a non-singleton iommu group to either vfio-pci
> driver or this sample driver. Existing passthru policy works well
> for this non-singleton group.
>
> This is actually a policy choice. A device driver can make this call
> if it wants to be vfio viable. And it needs to provide dummy
> vfio_device_ops which is required by vfio framework. To prevent user
> from opening the device from the iommu backed group fd, the open
> callback of the dummy vfio_device_ops should return -ENODEV to fail
> the VFIO_GET_DEVICE_FD request from userspace.
>
> Cc: Kevin Tian <kevin.tian@intel.com>
> Cc: Lu Baolu <baolu.lu@linux.intel.com>
> Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> ---
> drivers/vfio/pci/vfio_mdev_pci.c | 91 ++++++++++++++++++++++++++++++++++++----
> 1 file changed, 82 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_mdev_pci.c b/drivers/vfio/pci/vfio_mdev_pci.c
> index 09143d3..a61c20d 100644
> --- a/drivers/vfio/pci/vfio_mdev_pci.c
> +++ b/drivers/vfio/pci/vfio_mdev_pci.c
> @@ -107,19 +107,27 @@ struct vfio_mdev_pci {
> static int vfio_mdev_pci_create(struct kobject *kobj, struct mdev_device *mdev)
> {
> struct device *pdev;
> + struct vfio_device *device;
> struct vfio_mdev_pci_device *vmdev;
> struct vfio_mdev_pci *pmdev;
> int ret;
>
> pdev = mdev_parent_dev(mdev);
> - vmdev = dev_get_drvdata(pdev);
> + device = vfio_device_get_from_dev(pdev);
> + vmdev = vfio_device_data(device);
>
> - if (atomic_dec_if_positive(&vmdev->avail) < 0)
> - return -ENOSPC;
> + if (atomic_dec_if_positive(&vmdev->avail) < 0) {
> + ret = -ENOSPC;
> + goto out;
> + }
>
> + pr_info("%s, available instance: %d\n",
> + __func__, atomic_read(&vmdev->avail));
> pmdev = kzalloc(sizeof(struct vfio_mdev_pci), GFP_KERNEL);
> - if (!pmdev)
> - return -ENOMEM;
> + if (!pmdev) {
> + ret = -ENOMEM;
> + goto out;
> + }
>
> pmdev->mdev = mdev;
> pmdev->vdev = &vmdev->vdev;
> @@ -130,10 +138,11 @@ static int vfio_mdev_pci_create(struct kobject *kobj, struct mdev_device *mdev)
> __func__, dev_name(mdev_dev(mdev)), dev_name(pdev));
> kfree(pmdev);
> atomic_inc(&vmdev->avail);
> - return ret;
> }
>
> - return 0;
> +out:
> + vfio_device_put(device);
> + return ret;
> }
>
> static int vfio_mdev_pci_remove(struct mdev_device *mdev)
> @@ -145,6 +154,8 @@ static int vfio_mdev_pci_remove(struct mdev_device *mdev)
>
> kfree(pmdev);
> atomic_inc(&vmdev->avail);
> + pr_info("%s, available instance: %d\n",
> + __func__, atomic_read(&vmdev->avail));
> pr_info("%s, succeeded for mdev: %s\n", __func__,
> dev_name(mdev_dev(mdev)));
>
> @@ -236,12 +247,65 @@ static ssize_t vfio_mdev_pci_write(struct mdev_device *mdev,
> return vfio_pci_write(pmdev->vdev, (char __user *)buf, count, ppos);
> }
>
> +static int vfio_pci_dummy_open(void *device_data)
> +{
> + struct vfio_mdev_pci_device *vmdev =
> + (struct vfio_mdev_pci_device *) device_data;
> + pr_warn("Device %s is not viable for vfio-pci passthru, please follow"
> + " vfio-mdev passthru path as it has been wrapped as mdev!!!\n",
> + dev_name(&vmdev->vdev.pdev->dev));
> + return -ENODEV;
> +}
> +
> +static void vfio_pci_dummy_release(void *device_data)
> +{
> +}
Theoretically .release will never be called. If we're paranoid, we
could keep it with a pr_warn.
> +
> +long vfio_pci_dummy_ioctl(void *device_data,
> + unsigned int cmd, unsigned long arg)
> +{
> + return 0;
> +}
> +
> +ssize_t vfio_pci_dummy_read(void *device_data, char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + return 0;
> +}
> +
> +ssize_t vfio_pci_dummy_write(void *device_data, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + return 0;
> +}
> +
> +int vfio_pci_dummy_mmap(void *device_data, struct vm_area_struct *vma)
> +{
> + return 0;
> +}
> +
> +void vfio_pci_dummy_request(void *device_data, unsigned int count)
> +{
> +}
AFAICT, none of .ioctl, .read, .write, .mmap, or .request need to be
provided, only .open and only .release for paranoia.
> +
> +static const struct vfio_device_ops vfio_pci_dummy_ops = {
> + .name = "vfio-pci",
This is impersonating vfio-pci, shouldn't we use something like
"vfio-mdev-pci-dummy". Thanks,
Alex
> + .open = vfio_pci_dummy_open,
> + .release = vfio_pci_dummy_release,
> + .ioctl = vfio_pci_dummy_ioctl,
> + .read = vfio_pci_dummy_read,
> + .write = vfio_pci_dummy_write,
> + .mmap = vfio_pci_dummy_mmap,
> + .request = vfio_pci_dummy_request,
> +};
> +
> static int vfio_mdev_pci_driver_probe(struct pci_dev *pdev,
> const struct pci_device_id *id)
> {
> struct vfio_mdev_pci_device *vmdev;
> struct vfio_pci_device *vdev;
> const struct mdev_parent_ops *ops;
> + struct iommu_group *group;
> int ret;
>
> if (pdev->hdr_type != PCI_HEADER_TYPE_NORMAL)
> @@ -260,6 +324,10 @@ static int vfio_mdev_pci_driver_probe(struct pci_dev *pdev,
> return -EBUSY;
> }
>
> + group = vfio_iommu_group_get(&pdev->dev);
> + if (!group)
> + return -EINVAL;
> +
> vmdev = kzalloc(sizeof(*vmdev), GFP_KERNEL);
> if (!vmdev)
> return -ENOMEM;
> @@ -304,7 +372,12 @@ static int vfio_mdev_pci_driver_probe(struct pci_dev *pdev,
> #endif
> vdev->disable_idle_d3 = disable_idle_d3;
>
> - pci_set_drvdata(pdev, vmdev);
> + ret = vfio_add_group_dev(&pdev->dev, &vfio_pci_dummy_ops, vmdev);
> + if (ret) {
> + vfio_iommu_group_put(group, &pdev->dev);
> + kfree(vmdev);
> + return ret;
> + }
>
> ret = vfio_pci_reflck_attach(vdev);
> if (ret) {
> @@ -352,7 +425,7 @@ static void vfio_mdev_pci_driver_remove(struct pci_dev *pdev)
>
> mdev_unregister_device(&pdev->dev);
>
> - vmdev = pci_get_drvdata(pdev);
> + vmdev = vfio_del_group_dev(&pdev->dev);
> if (!vmdev)
> return;
>
next prev parent reply other threads:[~2019-09-26 2:37 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-05 7:59 [PATCH v2 00/13] vfio_pci: wrap pci device as a mediated device Liu Yi L
2019-09-05 7:59 ` [PATCH v2 01/13] vfio_pci: move vfio_pci_is_vga/vfio_vga_disabled to header Liu Yi L
2019-09-05 7:59 ` [PATCH v2 02/13] vfio_pci: refine user config reference in vfio-pci module Liu Yi L
2019-09-26 2:36 ` Alex Williamson
2019-09-30 12:38 ` Liu, Yi L
2019-09-05 7:59 ` [PATCH v2 03/13] vfio_pci: refine vfio_pci_driver reference in vfio_pci.c Liu Yi L
2019-09-05 7:59 ` [PATCH v2 04/13] vfio_pci: make common functions be extern Liu Yi L
2019-09-05 7:59 ` [PATCH v2 05/13] vfio_pci: duplicate vfio_pci.c Liu Yi L
2019-09-05 7:59 ` [PATCH v2 06/13] vfio_pci: shrink vfio_pci_common.c Liu Yi L
2019-09-05 7:59 ` [PATCH v2 07/13] vfio_pci: shrink vfio_pci.c Liu Yi L
2019-09-05 7:59 ` [PATCH v2 08/13] vfio/pci: protect cap/ecap_perm bits alloc/free with atomic op Liu Yi L
2019-09-26 2:36 ` Alex Williamson
2019-09-30 12:38 ` Liu, Yi L
2019-09-05 7:59 ` [PATCH v2 09/13] samples: add vfio-mdev-pci driver Liu Yi L
2019-09-05 7:59 ` [PATCH v2 10/13] samples: refine " Liu Yi L
2019-09-26 2:36 ` Alex Williamson
2019-09-30 12:39 ` Liu, Yi L
2019-09-05 7:59 ` [PATCH v2 11/13] samples/vfio-mdev-pci: call vfio_add_group_dev() Liu Yi L
2019-09-26 2:36 ` Alex Williamson [this message]
2019-09-30 12:40 ` Liu, Yi L
2019-09-05 7:59 ` [PATCH v2 12/13] vfio/type1: use iommu_attach_group() for wrapping PF/VF as mdev Liu Yi L
2019-09-26 2:37 ` Alex Williamson
2019-09-05 7:59 ` [PATCH v2 13/13] vfio/type1: track iommu backed group attach Liu Yi L
2019-09-25 9:13 ` [PATCH v2 00/13] vfio_pci: wrap pci device as a mediated device Liu, Yi L
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=20190925203658.18ea50ab@x1.home \
--to=alex.williamson@redhat.com \
--cc=baolu.lu@linux.intel.com \
--cc=chenbo.xia@intel.com \
--cc=joro@8bytes.org \
--cc=jun.j.tian@intel.com \
--cc=kevin.tian@intel.com \
--cc=kvm@vger.kernel.org \
--cc=kwankhede@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=shaopeng.he@intel.com \
--cc=yan.y.zhao@intel.com \
--cc=yi.l.liu@intel.com \
--cc=yi.y.sun@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).