RE: [RFC 15/20] vfio/pci: Add VFIO_DEVICE_[DE]ATTACH_IOASID

2021-09-22 Thread Tian, Kevin
> From: Jason Gunthorpe 
> Sent: Wednesday, September 22, 2021 8:59 PM
> 
> On Wed, Sep 22, 2021 at 03:56:18AM +, Tian, Kevin wrote:
> > > From: Jason Gunthorpe 
> > > Sent: Wednesday, September 22, 2021 2:04 AM
> > >
> > > On Sun, Sep 19, 2021 at 02:38:43PM +0800, Liu Yi L wrote:
> > > > This patch adds interface for userspace to attach device to specified
> > > > IOASID.
> > > >
> > > > Note:
> > > > One device can only be attached to one IOASID in this version. This is
> > > > on par with what vfio provides today. In the future this restriction can
> > > > be relaxed when multiple I/O address spaces are supported per device
> > >
> > > ?? In VFIO the container is the IOS and the container can be shared
> > > with multiple devices. This needs to start at about the same
> > > functionality.
> >
> > a device can be only attached to one container. One container can be
> > shared by multiple devices.
> >
> > a device can be only attached to one IOASID. One IOASID can be shared
> > by multiple devices.
> >
> > it does start at the same functionality.
> >
> > >
> > > > +   } else if (cmd == VFIO_DEVICE_ATTACH_IOASID) {
> > >
> > > This should be in the core code, right? There is nothing PCI specific
> > > here.
> > >
> >
> > but if you insist on a pci-wrapper attach function, we still need something
> > here (e.g. with .attach_ioasid() callback)?
> 
> I would like to stop adding ioctls to this switch, the core code
> should decode the ioctl and call an per-ioctl op like every other
> subsystem does..
> 
> If you do that then you could have an op
> 
>  .attach_ioasid = vfio_full_device_attach,
> 
> And that is it for driver changes.
> 
> Every driver that use type1 today should be updated to have the above
> line and will work with iommufd. mdevs will not be updated and won't
> work.
> 

will do. 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [RFC 15/20] vfio/pci: Add VFIO_DEVICE_[DE]ATTACH_IOASID

2021-09-22 Thread Jason Gunthorpe via iommu
On Wed, Sep 22, 2021 at 03:56:18AM +, Tian, Kevin wrote:
> > From: Jason Gunthorpe 
> > Sent: Wednesday, September 22, 2021 2:04 AM
> > 
> > On Sun, Sep 19, 2021 at 02:38:43PM +0800, Liu Yi L wrote:
> > > This patch adds interface for userspace to attach device to specified
> > > IOASID.
> > >
> > > Note:
> > > One device can only be attached to one IOASID in this version. This is
> > > on par with what vfio provides today. In the future this restriction can
> > > be relaxed when multiple I/O address spaces are supported per device
> > 
> > ?? In VFIO the container is the IOS and the container can be shared
> > with multiple devices. This needs to start at about the same
> > functionality.
> 
> a device can be only attached to one container. One container can be
> shared by multiple devices.
> 
> a device can be only attached to one IOASID. One IOASID can be shared
> by multiple devices.
> 
> it does start at the same functionality.
> 
> > 
> > > + } else if (cmd == VFIO_DEVICE_ATTACH_IOASID) {
> > 
> > This should be in the core code, right? There is nothing PCI specific
> > here.
> > 
> 
> but if you insist on a pci-wrapper attach function, we still need something
> here (e.g. with .attach_ioasid() callback)?

I would like to stop adding ioctls to this switch, the core code
should decode the ioctl and call an per-ioctl op like every other
subsystem does..

If you do that then you could have an op

 .attach_ioasid = vfio_full_device_attach,

And that is it for driver changes.

Every driver that use type1 today should be updated to have the above
line and will work with iommufd. mdevs will not be updated and won't
work.

Jason
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [RFC 15/20] vfio/pci: Add VFIO_DEVICE_[DE]ATTACH_IOASID

2021-09-21 Thread Tian, Kevin
> From: Jason Gunthorpe 
> Sent: Wednesday, September 22, 2021 2:04 AM
> 
> On Sun, Sep 19, 2021 at 02:38:43PM +0800, Liu Yi L wrote:
> > This patch adds interface for userspace to attach device to specified
> > IOASID.
> >
> > Note:
> > One device can only be attached to one IOASID in this version. This is
> > on par with what vfio provides today. In the future this restriction can
> > be relaxed when multiple I/O address spaces are supported per device
> 
> ?? In VFIO the container is the IOS and the container can be shared
> with multiple devices. This needs to start at about the same
> functionality.

a device can be only attached to one container. One container can be
shared by multiple devices.

a device can be only attached to one IOASID. One IOASID can be shared
by multiple devices.

it does start at the same functionality.

> 
> > +   } else if (cmd == VFIO_DEVICE_ATTACH_IOASID) {
> 
> This should be in the core code, right? There is nothing PCI specific
> here.
> 

but if you insist on a pci-wrapper attach function, we still need something
here (e.g. with .attach_ioasid() callback)?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [RFC 15/20] vfio/pci: Add VFIO_DEVICE_[DE]ATTACH_IOASID

2021-09-21 Thread Jason Gunthorpe via iommu
On Sun, Sep 19, 2021 at 02:38:43PM +0800, Liu Yi L wrote:
> This patch adds interface for userspace to attach device to specified
> IOASID.
> 
> Note:
> One device can only be attached to one IOASID in this version. This is
> on par with what vfio provides today. In the future this restriction can
> be relaxed when multiple I/O address spaces are supported per device

?? In VFIO the container is the IOS and the container can be shared
with multiple devices. This needs to start at about the same
functionality.

> + } else if (cmd == VFIO_DEVICE_ATTACH_IOASID) {

This should be in the core code, right? There is nothing PCI specific
here.

Jason
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[RFC 15/20] vfio/pci: Add VFIO_DEVICE_[DE]ATTACH_IOASID

2021-09-19 Thread Liu Yi L
This patch adds interface for userspace to attach device to specified
IOASID.

Note:
One device can only be attached to one IOASID in this version. This is
on par with what vfio provides today. In the future this restriction can
be relaxed when multiple I/O address spaces are supported per device

Signed-off-by: Liu Yi L 
---
 drivers/vfio/pci/vfio_pci.c | 82 +
 drivers/vfio/pci/vfio_pci_private.h |  1 +
 include/linux/iommufd.h |  1 +
 include/uapi/linux/vfio.h   | 26 +
 4 files changed, 110 insertions(+)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 20006bb66430..5b1fda333122 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -557,6 +557,11 @@ static void vfio_pci_release(struct vfio_device *core_vdev)
if (vdev->videv) {
struct vfio_iommufd_device *videv = vdev->videv;
 
+   if (videv->ioasid != IOMMUFD_INVALID_IOASID) {
+   iommufd_device_detach_ioasid(videv->idev,
+videv->ioasid);
+   videv->ioasid = IOMMUFD_INVALID_IOASID;
+   }
vdev->videv = NULL;
iommufd_unbind_device(videv->idev);
kfree(videv);
@@ -839,6 +844,7 @@ static long vfio_pci_ioctl(struct vfio_device *core_vdev,
}
videv->idev = idev;
videv->iommu_fd = bind_data.iommu_fd;
+   videv->ioasid = IOMMUFD_INVALID_IOASID;
/*
 * A security context has been established. Unblock
 * user access.
@@ -848,6 +854,82 @@ static long vfio_pci_ioctl(struct vfio_device *core_vdev,
vdev->videv = videv;
mutex_unlock(>videv_lock);
 
+   return 0;
+   } else if (cmd == VFIO_DEVICE_ATTACH_IOASID) {
+   struct vfio_device_attach_ioasid attach;
+   unsigned long minsz;
+   struct vfio_iommufd_device *videv;
+   int ret = 0;
+
+   /* not allowed if the device is opened in legacy interface */
+   if (vfio_device_in_container(core_vdev))
+   return -ENOTTY;
+
+   minsz = offsetofend(struct vfio_device_attach_ioasid, ioasid);
+   if (copy_from_user(, (void __user *)arg, minsz))
+   return -EFAULT;
+
+   if (attach.argsz < minsz || attach.flags ||
+   attach.iommu_fd < 0 || attach.ioasid < 0)
+   return -EINVAL;
+
+   mutex_lock(>videv_lock);
+
+   videv = vdev->videv;
+   if (!videv || videv->iommu_fd != attach.iommu_fd) {
+   mutex_unlock(>videv_lock);
+   return -EINVAL;
+   }
+
+   /* Currently only allows one IOASID attach */
+   if (videv->ioasid != IOMMUFD_INVALID_IOASID) {
+   mutex_unlock(>videv_lock);
+   return -EBUSY;
+   }
+
+   ret = __pci_iommufd_device_attach_ioasid(vdev->pdev,
+videv->idev,
+attach.ioasid);
+   if (!ret)
+   videv->ioasid = attach.ioasid;
+   mutex_unlock(>videv_lock);
+
+   return ret;
+   } else if (cmd == VFIO_DEVICE_DETACH_IOASID) {
+   struct vfio_device_attach_ioasid attach;
+   unsigned long minsz;
+   struct vfio_iommufd_device *videv;
+
+   /* not allowed if the device is opened in legacy interface */
+   if (vfio_device_in_container(core_vdev))
+   return -ENOTTY;
+
+   minsz = offsetofend(struct vfio_device_attach_ioasid, ioasid);
+   if (copy_from_user(, (void __user *)arg, minsz))
+   return -EFAULT;
+
+   if (attach.argsz < minsz || attach.flags ||
+   attach.iommu_fd < 0 || attach.ioasid < 0)
+   return -EINVAL;
+
+   mutex_lock(>videv_lock);
+
+   videv = vdev->videv;
+   if (!videv || videv->iommu_fd != attach.iommu_fd) {
+   mutex_unlock(>videv_lock);
+   return -EINVAL;
+   }
+
+   if (videv->ioasid == IOMMUFD_INVALID_IOASID ||
+   videv->ioasid != attach.ioasid) {
+   mutex_unlock(>videv_lock);
+   return -EINVAL;
+   }
+
+   videv->ioasid = IOMMUFD_INVALID_IOASID;
+   iommufd_device_detach_ioasid(videv->idev, attach.ioasid);
+   mutex_unlock(>videv_lock);
+
return 0;