RE: [PATCH 06/10] vfio-iommufd: Allow iommufd to be used in place of a container fd

["Tian, Kevin" <kevin.tian@intel.com>]

> From: Jason Gunthorpe <jgg@nvidia.com>
> 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.