Hi Alex, > From: Alex Williamson [mailto:alex.williamson@xxxxxxxxxx] > Sent: Thursday, September 26, 2019 10:37 AM > To: Liu, Yi L <yi.l.liu@xxxxxxxxx> > Subject: Re: [PATCH v2 11/13] samples/vfio-mdev-pci: call vfio_add_group_dev() > > On Thu, 5 Sep 2019 15:59:28 +0800 > Liu Yi L <yi.l.liu@xxxxxxxxx> 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@xxxxxxxxx> > > Cc: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx> > > Signed-off-by: Liu Yi L <yi.l.liu@xxxxxxxxx> > > --- > > drivers/vfio/pci/vfio_mdev_pci.c | 91 > > ++++++++++++++++++++++++++++++++++++---- > > 1 file changed, 82 insertions(+), 9 deletions(-) > > [...] > > +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. yes, it is. > > + > > +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. sure. let me fix it. > > + > > +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, Yep. will modify it. > Alex Thanks, Yi Liu
