On Thu, Mar 02, 2023 at 09:55:46AM +0000, Tian, Kevin wrote: > > From: Liu, Yi L <yi.l.liu@xxxxxxxxx> > > Sent: Thursday, March 2, 2023 2:07 PM > > > > > - if (!vfio_dev_in_groups(cur_vma, groups)) { > > > + if (cur_vma->vdev.open_count && > > > + !vfio_dev_in_groups(cur_vma, groups) && > > > + !vfio_dev_in_iommufd_ctx(cur_vma, iommufd_ctx)) { > > > > Hi Alex, Jason, > > > > There is one concern on this approach which is related to the > > cdev noiommu mode. As patch 16 of this series, cdev path > > supports noiommu mode by passing a negative iommufd to > > kernel. In such case, the vfio_device is not bound to a valid > > iommufd. Then the check in vfio_dev_in_iommufd_ctx() is > > to be broken. > > > > An idea is to add a cdev_noiommu flag in vfio_device, when > > checking the iommufd_ictx, also check this flag. If all the opened > > devices in the dev_set have vfio_device->cdev_noiommu==true, > > then the reset is considered to be doable. But there is a special > > case. If devices in this dev_set are opened by two applications > > that operates in cdev noiommu mode, then this logic is not able > > to differentiate them. In that case, should we allow the reset? > > It seems to ok to allow reset since noiommu mode itself means > > no security between the applications that use it. thoughts? > > > > Probably we need still pass in a valid iommufd (instead of using > a negative value) in noiommu case to mark the ownership so the > check in the reset path can correctly catch whether an opened > device belongs to this user. There should be no iommufd at all in no-iommu mode Adding one just to deal with noiommu reset seems pretty sad :\ no-iommu is only really used by dpdk, and it doesn't invoke VFIO_DEVICE_PCI_HOT_RESET at all. I'd say as long as VFIO_DEVICE_PCI_HOT_RESET works if only one vfio device is open using a empty list (eg we should ensure that the invoking cdev itself is allowed) then I think it is OK. Jason