> From: Liu, Yi L <yi.l.liu@xxxxxxxxx> > Sent: Wednesday, March 15, 2023 12:40 PM > > > 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 works for me.