> From: Alex Williamson <[email protected]> > Sent: Tuesday, May 23, 2023 4:25 AM > > On Sat, 13 May 2023 06:28:14 -0700 > Yi Liu <[email protected]> wrote: > > > into vfio_device_group_open(). This is more consistent with what will > > be done in vfio device cdev path. > > Same comment regarding flowing commit subject into body on this series.
Yes, I've modified the commit message in my local branch.:-) > > > > Reviewed-by: Kevin Tian <[email protected]> > > Tested-by: Terrence Xu <[email protected]> > > Tested-by: Nicolin Chen <[email protected]> > > Tested-by: Yanting Jiang <[email protected]> > > Tested-by: Shameer Kolothum <[email protected]> > > Signed-off-by: Yi Liu <[email protected]> > > --- > > drivers/vfio/group.c | 6 ++++++ > > drivers/vfio/iommufd.c | 32 +++++++++++++++++++------------- > > drivers/vfio/vfio.h | 9 +++++++++ > > 3 files changed, 34 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c > > index a17584e8be15..cfd0b9254bbc 100644 > > --- a/drivers/vfio/group.c > > +++ b/drivers/vfio/group.c > > @@ -192,6 +192,12 @@ static int vfio_device_group_open(struct > > vfio_device_file *df) > > vfio_device_group_get_kvm_safe(device); > > > > df->iommufd = device->group->iommufd; > > + if (df->iommufd && vfio_device_is_noiommu(device) && device->open_count > == 0) { > > + ret = vfio_iommufd_compat_probe_noiommu(device, > > + df->iommufd); > > + if (ret) > > + goto out_put_kvm; > > + } > > > > ret = vfio_device_open(df); > > if (ret) { > > diff --git a/drivers/vfio/iommufd.c b/drivers/vfio/iommufd.c > > index a18e920be164..7a654a1437f0 100644 > > --- a/drivers/vfio/iommufd.c > > +++ b/drivers/vfio/iommufd.c > > @@ -46,6 +46,24 @@ static void vfio_iommufd_noiommu_unbind(struct > > vfio_device > *vdev) > > } > > } > > > > +int vfio_iommufd_compat_probe_noiommu(struct vfio_device *device, > > + struct iommufd_ctx *ictx) > > +{ > > + u32 ioas_id; > > + > > + if (!capable(CAP_SYS_RAWIO)) > > + return -EPERM; > > + > > + /* > > + * Require no compat ioas to be assigned to proceed. The basic > > + * statement is that the user cannot have done something that > > + * implies they expected translation to exist > > + */ > > + if (!iommufd_vfio_compat_ioas_get_id(ictx, &ioas_id)) > > + return -EPERM; > > + return 0; > > +} > > I think the purpose of this function is to keep the iommufd namespace > out of the group, Yes. > but we're muddying it as a general grab bag of > noiommu validation. What if the caller retained the RAWIO test and > comment, and this function simply became a wrapper around the iommufd > function, ex: > > bool vfio_iommufd_device_has_compat_ioas(struct vfio_device *device, > struct iommufd_ctx *ictx) > { > u32 ioas_id; > > return !iommufd_vfio_compat_ioas_get_id(ictx, &ioas_id)); > } Sure. This looks better. Regards, Yi Liu > Thanks, > Alex > > > + > > int vfio_iommufd_bind(struct vfio_device *vdev, struct iommufd_ctx *ictx) > > { > > u32 ioas_id; > > @@ -54,20 +72,8 @@ int vfio_iommufd_bind(struct vfio_device *vdev, struct > iommufd_ctx *ictx) > > > > lockdep_assert_held(&vdev->dev_set->lock); > > > > - if (vfio_device_is_noiommu(vdev)) { > > - if (!capable(CAP_SYS_RAWIO)) > > - return -EPERM; > > - > > - /* > > - * Require no compat ioas to be assigned to proceed. The basic > > - * statement is that the user cannot have done something that > > - * implies they expected translation to exist > > - */ > > - if (!iommufd_vfio_compat_ioas_get_id(ictx, &ioas_id)) > > - return -EPERM; > > - > > + if (vfio_device_is_noiommu(vdev)) > > return vfio_iommufd_noiommu_bind(vdev, ictx, &device_id); > > - } > > > > ret = vdev->ops->bind_iommufd(vdev, ictx, &device_id); > > if (ret) > > diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h > > index 785afc40ece8..8884b557fb26 100644 > > --- a/drivers/vfio/vfio.h > > +++ b/drivers/vfio/vfio.h > > @@ -234,9 +234,18 @@ static inline void vfio_container_cleanup(void) > > #endif > > > > #if IS_ENABLED(CONFIG_IOMMUFD) > > +int vfio_iommufd_compat_probe_noiommu(struct vfio_device *device, > > + struct iommufd_ctx *ictx); > > int vfio_iommufd_bind(struct vfio_device *device, struct iommufd_ctx > > *ictx); > > void vfio_iommufd_unbind(struct vfio_device *device); > > #else > > +static inline int > > +vfio_iommufd_compat_probe_noiommu(struct vfio_device *device, > > + struct iommufd_ctx *ictx) > > +{ > > + return -EOPNOTSUPP; > > +} > > + > > static inline int vfio_iommufd_bind(struct vfio_device *device, > > struct iommufd_ctx *ictx) > > {
