> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Wednesday, October 26, 2022 2:51 AM
>
> menuconfig VFIO
> tristate "VFIO Non-Privileged userspace driver framework"
> select IOMMU_API
> + depends on IOMMUFD || !IOMMUFD
Out of curiosity. What is the meaning of this dependency claim?
> @@ -717,12 +735,23 @@ static int vfio_group_ioctl_set_container(struct
> vfio_group *group,
> }
>
> container = vfio_container_from_file(f.file);
> - ret = -EINVAL;
this changes the errno from -EINVAL to -EBADF for the original container
path. Is it desired?
> if (container) {
> ret = vfio_container_attach_group(container, group);
> goto out_unlock;
> }
>
> + iommufd = iommufd_ctx_from_file(f.file);
> + if (!IS_ERR(iommufd)) {
The only errno which iommufd_ctx_from_file() may return is -EBADFD
which duplicates with -EBADF assignment in following line.
What about having it return NULL pointer similar as the container
helper does?
> + u32 ioas_id;
> +
> + group->iommufd = iommufd;
> + ret = iommufd_vfio_compat_ioas_id(iommufd, &ioas_id);
exchange the order of above two lines and only assign group->iommufd
when the compat call succeeds.
> @@ -900,7 +940,7 @@ static int vfio_group_ioctl_get_status(struct
> vfio_group *group,
> return -ENODEV;
> }
>
> - if (group->container)
> + if (group->container || group->iommufd)
> status.flags |= VFIO_GROUP_FLAGS_CONTAINER_SET |
> VFIO_GROUP_FLAGS_VIABLE;
Copy some explanation from commit msg to explain the subtle difference
between container and iommufd.
