[RFC v1 0/2] vfio/pci: wrap pci device as mdev with vfio-pci driver

[RFC v1 0/2] vfio/pci: wrap pci device as mdev with vfio-pci driver

[Liu, Yi L]

This patchset aims to add a vfio-pci-like meta driver on existing
PCI devices, as a demo user of the vfio changes introduced in
"vfio/mdev: IOMMU aware mediated device" patchset from Baolu Lu.

To build such a meta driver. We have two choices.
a) add a vfio-pci alike sample driver under samples directory
b) add some extensions in vfio-pci driver to make it wrap pci
   device as mdev

For choice a), the new sample driver will have quite a few
duplicated code with vfio-pci driver since the new sample
driver also wants to virtualize the PCI config space. So
this choice may bring in extra maintain effort in kernel
and also looks strange since there will be a bunch of
duplicated code with vfio-pci driver.

For choice b), it may reuse the existing vfio-pci driver
by adding a new working mode. With this mode, user can wrap
a pci device as a mediated device by binding it with the
vfio-pci driver which works in the new mode. Thus can be used
to verify the ""vfio/mdev: IOMMU aware mediated device"
patchset.

This patchset is following choice b). However, we are open on
the direction of the implementation of this vfio-pci-like meta
driver. Pls feel free give your suggestions.

Specific interface tested in this proposal:

*) int mdev_set_iommu_device(struct device *dev,
				struct device *iommu_device)
   introduced in the patch as below:
   "[PATCH v5 6/8] vfio/mdev: Add iommu related member in mdev_device"


Links:
*) Link of "vfio/mdev: IOMMU aware mediated device"
	https://lwn.net/Articles/780522/

Liu, Yi L (2):
  vfio/pci: register vfio-pci driver to mdev framework
  vfio/pci: expose only mdev interface if in pci-mdev mode

 drivers/vfio/pci/vfio_pci.c | 199 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 193 insertions(+), 6 deletions(-)

-- 
2.7.4

[RFC v1 1/2] vfio/pci: register vfio-pci driver to mdev framework

[Liu, Yi L]

This patch adds a new working mode in vfio-pci driver, may mentioned
as pci-mdev mode. User can config vfio-pci driver to work in this mode
by module param "vfio_pci_mdev_mode". When working in this mode, vfio-pci
driver will wrap a pci device as a mediated device. User space access of
this device will go through vfio mdev framework.

This new mode is exclusive to existing pci passthrough, i.e. once
enabled the physical device cannot be assigned directly but through
mdev framework. The next patch of this patchset will give a way to
ensure this exclusiveness.

To use this driver:
a) load vfio-pci.ko module with "vfio_pci_mdev_mode=1"
   > sudo modprobe vfio
   > sudo modprobe vfio-pci vfio_pci_mdev_mode=1
   > sudo modprobe vfio_mdev

b) unbind original device driver
   e.g. for device with its bdf as $dev_bdf, use following command
   to unbind its original driver
   > echo $dev_bdf > /sys/bus/pci/devices/$dev_bdf/driver/unbind

c) bind vfio-pci driver to the physical device
   > echo $vend_id $dev_id > /sys/bus/pci/drivers/vfio-pci/new_id

d) check the supported mdev instances
   > ls /sys/bus/pci/devices/$dev_bdf/mdev_supported_types/
     vfio-pmdev-type1
   > ls /sys/bus/pci/devices/$dev_bdf/mdev_supported_types/\
     vfio-pmdev-type1/
     available_instances  create  device_api  devices  name

e)  create mdev on this physical device
   > echo "83b8f4f2-509f-382f-3c1e-e6bfe0fa1003" > \
     /sys/bus/pci/devices/$dev_bdf/mdev_supported_types/\
     vfio-pmdev-type1/create

f) passthru the mdev to guest
   add the following line in Qemu boot command
   -device vfio-pci,\
    sysfsdev=/sys/bus/mdev/devices/83b8f4f2-509f-382f-3c1e-e6bfe0fa1003

g) destroy mdev
   > echo 1 > /sys/bus/mdev/devices/83b8f4f2-509f-382f-3c1e-e6bfe0fa1003/\
     remove

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_pci.c | 180 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 180 insertions(+)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index ff60bd1..d1c3fe6 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -29,6 +29,7 @@
 #include <linux/vfio.h>
 #include <linux/vgaarb.h>
 #include <linux/nospec.h>
+#include <linux/mdev.h>
 
 #include "vfio_pci_private.h"
 
@@ -56,6 +57,13 @@ module_param(disable_idle_d3, bool, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(disable_idle_d3,
 		 "Disable using the PCI D3 low power state for idle, unused devices");
 
+static bool vfio_pci_mdev_mode;
+module_param(vfio_pci_mdev_mode, bool, 0644);
+MODULE_PARM_DESC(vfio_pci_mdev_mode,
+		 "A mode of vfio-pci driver, which wrapa a PCI device as a mediated device. Further user-space direct access of the PCI device will go thru mediated framework");
+
+static const struct mdev_parent_ops vfio_pci_mdev_ops;
+
 static inline bool vfio_vga_disabled(void)
 {
 #ifdef CONFIG_VFIO_PCI_VGA
@@ -1300,6 +1308,20 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		pci_set_power_state(pdev, PCI_D3hot);
 	}
 
+	if (vfio_pci_mdev_mode) {
+		ret = mdev_register_device(&pdev->dev, &vfio_pci_mdev_ops);
+		if (ret)
+			pr_err("Cannot register mdev for device %s\n",
+				dev_name(&pdev->dev));
+		else
+			pr_info("vfio-pci will wrap %s as a mdev\n",
+				dev_name(&pdev->dev));
+	} else {
+
+		pr_info("vfio-pci work in legacy mode for %s\n",
+						dev_name(&pdev->dev));
+	}
+
 	return ret;
 }
 
@@ -1607,6 +1629,164 @@ static void __init vfio_pci_fill_ids(void)
 	}
 }
 
+static ssize_t
+name_show(struct kobject *kobj, struct device *dev, char *buf)
+{
+	return sprintf(buf, "%s-type1\n", dev_name(dev));
+}
+
+MDEV_TYPE_ATTR_RO(name);
+
+static ssize_t
+available_instances_show(struct kobject *kobj, struct device *dev, char *buf)
+{
+	return sprintf(buf, "%d\n", 1);
+}
+
+MDEV_TYPE_ATTR_RO(available_instances);
+
+static ssize_t device_api_show(struct kobject *kobj, struct device *dev,
+		char *buf)
+{
+	return sprintf(buf, "%s\n", VFIO_DEVICE_API_PCI_STRING);
+}
+
+MDEV_TYPE_ATTR_RO(device_api);
+
+static struct attribute *vfio_pci_mdev_types_attrs[] = {
+	&mdev_type_attr_name.attr,
+	&mdev_type_attr_device_api.attr,
+	&mdev_type_attr_available_instances.attr,
+	NULL,
+};
+
+static struct attribute_group vfio_pci_mdev_type_group1 = {
+	.name  = "type1",
+	.attrs = vfio_pci_mdev_types_attrs,
+};
+
+struct attribute_group *vfio_pci_mdev_type_groups[] = {
+	&vfio_pci_mdev_type_group1,
+	NULL,
+};
+
+struct vfio_pci_mdev {
+	struct vfio_pci_device *vdev;
+	struct mdev_device *mdev;
+	unsigned long handle;
+};
+
+static int vfio_pci_mdev_create(struct kobject *kobj, struct mdev_device *mdev)
+{
+	struct device *pdev;
+	struct vfio_pci_device *vdev;
+	struct vfio_pci_mdev *pmdev;
+	int ret;
+
+	pdev = mdev_parent_dev(mdev);
+	vdev = dev_get_drvdata(pdev);
+	pmdev = kzalloc(sizeof(struct vfio_pci_mdev), GFP_KERNEL);
+	if (pmdev == NULL) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	pmdev->mdev = mdev;
+	pmdev->vdev = vdev;
+	mdev_set_drvdata(mdev, pmdev);
+	ret = mdev_set_iommu_device(mdev_dev(mdev), pdev);
+	if (ret) {
+		pr_info("%s, failed to config iommu isolation for mdev: %s on pf: %s\n",
+			__func__, dev_name(mdev_dev(mdev)), dev_name(pdev));
+		goto out;
+	}
+
+out:
+	return ret;
+}
+
+static int vfio_pci_mdev_remove(struct mdev_device *mdev)
+{
+	struct vfio_pci_mdev *pmdev = mdev_get_drvdata(mdev);
+
+	kfree(pmdev);
+	pr_info("%s, succeeded for mdev: %s\n", __func__,
+		     dev_name(mdev_dev(mdev)));
+
+	return 0;
+}
+
+static int vfio_pci_mdev_open(struct mdev_device *mdev)
+{
+	int ret = 0;
+	struct vfio_pci_mdev *pmdev = mdev_get_drvdata(mdev);
+
+	ret = vfio_pci_open(pmdev->vdev);
+	if (!ret)
+		pr_info("Succeeded to open mdev: %s on pf: %s\n",
+		dev_name(mdev_dev(mdev)), dev_name(&pmdev->vdev->pdev->dev));
+	else
+		pr_info("Failed to open mdev: %s on pf: %s\n",
+		dev_name(mdev_dev(mdev)), dev_name(&pmdev->vdev->pdev->dev));
+	return ret;
+}
+
+static void vfio_pci_mdev_release(struct mdev_device *mdev)
+{
+	struct vfio_pci_mdev *pmdev = mdev_get_drvdata(mdev);
+
+	pr_info("Release mdev: %s on pf: %s\n",
+		dev_name(mdev_dev(mdev)), dev_name(&pmdev->vdev->pdev->dev));
+	vfio_pci_release(pmdev->vdev);
+}
+
+static long vfio_pci_mdev_ioctl(struct mdev_device *mdev, unsigned int cmd,
+			     unsigned long arg)
+{
+	struct vfio_pci_mdev *pmdev = mdev_get_drvdata(mdev);
+
+	return vfio_pci_ioctl(pmdev->vdev, cmd, arg);
+}
+
+static int vfio_pci_mdev_mmap(struct mdev_device *mdev,
+				struct vm_area_struct *vma)
+{
+	struct vfio_pci_mdev *pmdev = mdev_get_drvdata(mdev);
+
+	return vfio_pci_mmap(pmdev->vdev, vma);
+}
+
+static ssize_t vfio_pci_mdev_read(struct mdev_device *mdev, char __user *buf,
+			size_t count, loff_t *ppos)
+{
+	struct vfio_pci_mdev *pmdev = mdev_get_drvdata(mdev);
+
+	return vfio_pci_read(pmdev->vdev, buf, count, ppos);
+}
+
+static ssize_t vfio_pci_mdev_write(struct mdev_device *mdev,
+				const char __user *buf,
+				size_t count, loff_t *ppos)
+{
+	struct vfio_pci_mdev *pmdev = mdev_get_drvdata(mdev);
+
+	return vfio_pci_write(pmdev->vdev, buf, count, ppos);
+}
+
+static const struct mdev_parent_ops vfio_pci_mdev_ops = {
+	.supported_type_groups	= vfio_pci_mdev_type_groups,
+	.create			= vfio_pci_mdev_create,
+	.remove			= vfio_pci_mdev_remove,
+
+	.open			= vfio_pci_mdev_open,
+	.release		= vfio_pci_mdev_release,
+
+	.read			= vfio_pci_mdev_read,
+	.write			= vfio_pci_mdev_write,
+	.mmap			= vfio_pci_mdev_mmap,
+	.ioctl			= vfio_pci_mdev_ioctl,
+};
+
 static int __init vfio_pci_init(void)
 {
 	int ret;
-- 
2.7.4

[RFC v1 2/2] vfio/pci: expose only mdev interface if in pci-mdev mode

[Liu, Yi L]

If a pci device is wrapped as a mediated device. Then user should
only passthru it via vfio mdev framework. If not, there would be
conflict during the device access. This patch prevents user from
using legacy vfio-pci passthru interface. vfio-pci driver will only
expose mdev interface when working in pci-mdev mode. Thus could
prevent user from passthru device in legacy manner.

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_pci.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index d1c3fe6..422a3ff 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -1273,11 +1273,15 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	mutex_init(&vdev->ioeventfds_lock);
 	INIT_LIST_HEAD(&vdev->ioeventfds_list);
 
-	ret = vfio_add_group_dev(&pdev->dev, &vfio_pci_ops, vdev);
-	if (ret) {
-		vfio_iommu_group_put(group, &pdev->dev);
-		kfree(vdev);
-		return ret;
+	if (vfio_pci_mdev_mode) {
+		pci_set_drvdata(pdev, vdev);
+	} else {
+		ret = vfio_add_group_dev(&pdev->dev, &vfio_pci_ops, vdev);
+		if (ret) {
+			vfio_iommu_group_put(group, &pdev->dev);
+			kfree(vdev);
+			return ret;
+		}
 	}
 
 	ret = vfio_pci_reflck_attach(vdev);
@@ -1329,7 +1333,10 @@ static void vfio_pci_remove(struct pci_dev *pdev)
 {
 	struct vfio_pci_device *vdev;
 
-	vdev = vfio_del_group_dev(&pdev->dev);
+	if (vfio_pci_mdev_mode)
+		vdev = pci_get_drvdata(pdev);
+	else
+		vdev = vfio_del_group_dev(&pdev->dev);
 	if (!vdev)
 		return;
 
-- 
2.7.4

Re: [RFC v1 0/2] vfio/pci: wrap pci device as mdev with vfio-pci driver

[Alex Williamson]

On Sun,  3 Mar 2019 20:57:59 +0800
"Liu, Yi L" <yi.l.liu@intel.com> wrote:

> This patchset aims to add a vfio-pci-like meta driver on existing
> PCI devices, as a demo user of the vfio changes introduced in
> "vfio/mdev: IOMMU aware mediated device" patchset from Baolu Lu.
> 
> To build such a meta driver. We have two choices.
> a) add a vfio-pci alike sample driver under samples directory
> b) add some extensions in vfio-pci driver to make it wrap pci
>    device as mdev
> 
> For choice a), the new sample driver will have quite a few
> duplicated code with vfio-pci driver since the new sample
> driver also wants to virtualize the PCI config space. So
> this choice may bring in extra maintain effort in kernel
> and also looks strange since there will be a bunch of
> duplicated code with vfio-pci driver.
> 
> For choice b), it may reuse the existing vfio-pci driver
> by adding a new working mode. With this mode, user can wrap
> a pci device as a mediated device by binding it with the
> vfio-pci driver which works in the new mode. Thus can be used
> to verify the ""vfio/mdev: IOMMU aware mediated device"
> patchset.
> 
> This patchset is following choice b). However, we are open on
> the direction of the implementation of this vfio-pci-like meta
> driver. Pls feel free give your suggestions.

Thanks for doing this Yi!  Rather than a module option for vfio-pci,
what about having this build into a separate module (ex.
vfio-pci-mdev)?  Then we could test "regular" vfio-pci along side mdev
wrapped devices simply by which driver we bind and it'd probably be more
friendly to existing users, like libvirt.  This might also make a good
base driver for experimenting with device specific mdev migration as
well.  Thanks,

Alex

RE: [RFC v1 0/2] vfio/pci: wrap pci device as mdev with vfio-pci driver

[Liu, Yi L]

> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Friday, March 8, 2019 4:40 AM
> To: Liu, Yi L <yi.l.liu@intel.com>
> Cc: kwankhede@nvidia.com; Tian, Kevin <kevin.tian@intel.com>;
> baolu.lu@linux.intel.com; joro@8bytes.org; jean-philippe.brucker@arm.com;
> peterx@redhat.com; linux-kernel@vger.kernel.org; Sun, Yi Y <yi.y.sun@intel.com>
> Subject: Re: [RFC v1 0/2] vfio/pci: wrap pci device as mdev with vfio-pci driver
> 
> On Sun,  3 Mar 2019 20:57:59 +0800
> "Liu, Yi L" <yi.l.liu@intel.com> wrote:
> 
> > This patchset aims to add a vfio-pci-like meta driver on existing
> > PCI devices, as a demo user of the vfio changes introduced in
> > "vfio/mdev: IOMMU aware mediated device" patchset from Baolu Lu.
> >
> > To build such a meta driver. We have two choices.
> > a) add a vfio-pci alike sample driver under samples directory
> > b) add some extensions in vfio-pci driver to make it wrap pci
> >    device as mdev
> >
> > For choice a), the new sample driver will have quite a few
> > duplicated code with vfio-pci driver since the new sample
> > driver also wants to virtualize the PCI config space. So
> > this choice may bring in extra maintain effort in kernel
> > and also looks strange since there will be a bunch of
> > duplicated code with vfio-pci driver.
> >
> > For choice b), it may reuse the existing vfio-pci driver
> > by adding a new working mode. With this mode, user can wrap
> > a pci device as a mediated device by binding it with the
> > vfio-pci driver which works in the new mode. Thus can be used
> > to verify the ""vfio/mdev: IOMMU aware mediated device"
> > patchset.
> >
> > This patchset is following choice b). However, we are open on
> > the direction of the implementation of this vfio-pci-like meta
> > driver. Pls feel free give your suggestions.
> 
> Thanks for doing this Yi!  Rather than a module option for vfio-pci,
> what about having this build into a separate module (ex.
> vfio-pci-mdev)?  Then we could test "regular" vfio-pci along side mdev
> wrapped devices simply by which driver we bind and it'd probably be more
> friendly to existing users, like libvirt.  This might also make a good
> base driver for experimenting with device specific mdev migration as
> well.  Thanks,

Yeah, much better user experience. Let me do that. Thanks for the suggestion. :-)

Regards,
Yi Liu