> From: Tian, Kevin <kevin.tian@xxxxxxxxx> > Sent: Friday, March 10, 2023 6:07 PM > > > From: Liu, Yi L <yi.l.liu@xxxxxxxxx> > > Sent: Friday, March 10, 2023 5:58 PM > > > > > From: Tian, Kevin <kevin.tian@xxxxxxxxx> > > > Sent: Friday, March 10, 2023 5:02 PM > > > > > > > From: Liu, Yi L <yi.l.liu@xxxxxxxxx> > > > > Sent: Wednesday, March 8, 2023 9:29 PM > > > > + > > > > +static int vfio_device_cdev_probe_noiommu(struct vfio_device > *device) > > > > +{ > > > > + struct iommu_group *iommu_group; > > > > + int ret = 0; > > > > + > > > > + if (!IS_ENABLED(CONFIG_VFIO_NOIOMMU) || !vfio_noiommu) > > > > + return -EINVAL; > > > > + > > > > + if (!capable(CAP_SYS_RAWIO)) > > > > + return -EPERM; > > > > + > > > > + iommu_group = iommu_group_get(device->dev); > > > > + if (!iommu_group) > > > > + return 0; > > > > + > > > > + /* > > > > + * We cannot support noiommu mode for devices that are > > > protected > > > > + * by IOMMU. So check the iommu_group, if it is a no-iommu group > > > > + * created by VFIO, we support. If not, we refuse. > > > > + */ > > > > + if > > > (!vfio_group_find_noiommu_group_from_iommu(iommu_group)) > > > > + ret = -EINVAL; > > > > + iommu_group_put(iommu_group); > > > > + return ret; > > > > > > can check whether group->name == "vfio-noiommu"? > > > > But VFIO names it to be "vfio-noiommu" for both > VFIO_EMULATED_IOMMU > > and VFIO_NO_IOMMU. And we don't support no-iommu mode for > emulated > > devices since VFIO_MAP/UNMAP, pin_page(), dam_rw() won't work in > the > > no-iommu mode. > > correct. > > > > > So maybe something like below in drivers/vfio/vfio.h. It can be used > > to replace the code from iommu_group_get() to > > vfio_group_find_noiommu_group_from_iommu() In my patch. > > > > #if IS_ENABLED(CONFIG_VFIO_GROUP) > > static inline bool vfio_device_group_allow_noiommu(struct vfio_device > > *device) > > { > > lockdep_assert_held(&device->dev_set->lock); > > > > return device->group->type == VFIO_NO_IOMMU; > > } > > #else > > static inline bool vfio_device_group_allow_noiommu(struct vfio_device > > *device) > > { > > struct iommu_group *iommu_group; > > > > lockdep_assert_held(&device->dev_set->lock); > > > > iommu_group = iommu_group_get(device->dev); > > if (iommu_group) > > iommu_group_put(iommu_group); > > > > return !iommu_group; > > } > > #endif > > this makes sense. Just have one more think. vfio_device_is_noiommu() is already able to cover above vfio_device_group_allow_noiommu(), just needs to make it work when !VFIO_GROUP. In the group code, group->type == VFIO_NO_IOMMU means vfio_noiommu==true. So no need to check it. While in the case !VFIO_GROUP, needs to check it. So the code is as below. I can use vfio_device_is_noiommu() in cdev path. # if IS_ENABLED(CONFIG_VFIO_GROUP) static inline bool vfio_device_is_noiommu(struct vfio_device *vdev) { return IS_ENABLED(CONFIG_VFIO_NOIOMMU) && vdev->group->type == VFIO_NO_IOMMU; } #else static inline bool vfio_device_is_noiommu(struct vfio_device *vdev) { struct iommu_group *iommu_group; if (!IS_ENABLED(CONFIG_VFIO_NOIOMMU) || !vfio_noiommu) return -EINVAL; iommu_group = iommu_group_get(vdev->dev); if (iommu_group) iommu_group_put(iommu_group); return !iommu_group; } #endif