Re: [RFC PATCH kvmtool 08/10] vfio/iommufd: Move the hwpt allocation to helper

[Mostafa Saleh <smostafa@google.com>]

On Sun, May 25, 2025 at 01:19:14PM +0530, Aneesh Kumar K.V (Arm) wrote:
> alloc_hwpt.flags = 0; implies we prefer stage1 translation. Hence name
> the helper iommufd_alloc_s2bypass_hwpt().

This patch moves the recently added code into a new function,
can't this be squashed?
Also, I believe that with “IOMMU_HWPT_DATA_NONE”, we shouldn’t make
any assumptions in userspace about which stage is used.

The only guarantee is that IOMMU_IOAS_MAP/IOMMU_IOAS_UNMAP works.

So, I believe the naming for "s2bypass" is not accurate.

> 
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> ---
>  vfio/iommufd.c | 86 +++++++++++++++++++++++++++++---------------------
>  1 file changed, 50 insertions(+), 36 deletions(-)
> 
> diff --git a/vfio/iommufd.c b/vfio/iommufd.c
> index 3728a06cb318..742550705746 100644
> --- a/vfio/iommufd.c
> +++ b/vfio/iommufd.c
> @@ -60,6 +60,54 @@ err_close_device:
>  	return ret;
>  }
>  
> +static int iommufd_alloc_s2bypass_hwpt(struct vfio_device *vdev)
> +{
> +	int ret;
> +	struct iommu_hwpt_alloc alloc_hwpt;
> +	struct vfio_device_bind_iommufd bind;
> +	struct vfio_device_attach_iommufd_pt attach_data;
> +
> +	bind.argsz = sizeof(bind);
> +	bind.flags = 0;
> +	bind.iommufd = iommu_fd;
> +
> +	/* now bind the iommufd */
> +	if (ioctl(vdev->fd, VFIO_DEVICE_BIND_IOMMUFD, &bind)) {
> +		ret = -errno;
> +		vfio_dev_err(vdev, "failed to get info");
> +		goto err_out;
> +	}
> +
> +	alloc_hwpt.size = sizeof(struct iommu_hwpt_alloc);
> +	/* stage1 translate stage 2 bypass table if stage1 is supported */
> +	alloc_hwpt.flags = 0;
> +	alloc_hwpt.dev_id = bind.out_devid;
> +	alloc_hwpt.pt_id = ioas_id;
> +	alloc_hwpt.data_type = IOMMU_HWPT_DATA_NONE;
> +	alloc_hwpt.data_len = 0;
> +	alloc_hwpt.data_uptr = 0;
> +
> +	if (ioctl(iommu_fd, IOMMU_HWPT_ALLOC, &alloc_hwpt)) {
> +		ret = -errno;
> +		pr_err("Failed to allocate HWPT");
> +		goto err_out;
> +	}
> +
> +	attach_data.argsz = sizeof(attach_data);
> +	attach_data.flags = 0;
> +	attach_data.pt_id = alloc_hwpt.out_hwpt_id;
> +
> +	if (ioctl(vdev->fd, VFIO_DEVICE_ATTACH_IOMMUFD_PT, &attach_data)) {
> +		ret = -errno;
> +		vfio_dev_err(vdev, "failed to attach to IOAS ");
> +		goto err_out;
> +	}
> +	return 0;
> +
> +err_out:
> +	return ret;
> +}
> +
>  static int iommufd_configure_device(struct kvm *kvm, struct vfio_device *vdev)
>  {
>  	int ret;
> @@ -68,9 +116,6 @@ static int iommufd_configure_device(struct kvm *kvm, struct vfio_device *vdev)
>  	bool found_dev = false;
>  	char pci_dev_path[PATH_MAX];
>  	char vfio_dev_path[PATH_MAX];
> -	struct iommu_hwpt_alloc alloc_hwpt;
> -	struct vfio_device_bind_iommufd bind;
> -	struct vfio_device_attach_iommufd_pt attach_data;
>  
>  	ret = snprintf(pci_dev_path, PATH_MAX, "%s/vfio-dev/", vdev->sysfs_path);
>  	if (ret < 0 || ret == PATH_MAX)
> @@ -115,40 +160,9 @@ static int iommufd_configure_device(struct kvm *kvm, struct vfio_device *vdev)
>  		goto err_close_device;
>  	}
>  
> -	bind.argsz = sizeof(bind);
> -	bind.flags = 0;
> -	bind.iommufd = iommu_fd;
> -
> -	/* now bind the iommufd */
> -	if (ioctl(vdev->fd, VFIO_DEVICE_BIND_IOMMUFD, &bind)) {
> -		ret = -errno;
> -		vfio_dev_err(vdev, "failed to get info");
> -		goto err_close_device;
> -	}
> -
> -	alloc_hwpt.size = sizeof(struct iommu_hwpt_alloc);
> -	alloc_hwpt.flags = 0;
> -	alloc_hwpt.dev_id = bind.out_devid;
> -	alloc_hwpt.pt_id = ioas_id;
> -	alloc_hwpt.data_type = IOMMU_HWPT_DATA_NONE;
> -	alloc_hwpt.data_len = 0;
> -	alloc_hwpt.data_uptr = 0;
> -
> -	if (ioctl(iommu_fd, IOMMU_HWPT_ALLOC, &alloc_hwpt)) {
> -		ret = -errno;
> -		pr_err("Failed to allocate HWPT");
> -		goto err_close_device;
> -	}
> -
> -	attach_data.argsz = sizeof(attach_data);
> -	attach_data.flags = 0;
> -	attach_data.pt_id = alloc_hwpt.out_hwpt_id;
> -
> -	if (ioctl(vdev->fd, VFIO_DEVICE_ATTACH_IOMMUFD_PT, &attach_data)) {
> -		ret = -errno;
> -		vfio_dev_err(vdev, "failed to attach to IOAS ");
> +	ret = iommufd_alloc_s2bypass_hwpt(vdev);
> +	if (ret)
>  		goto err_close_device;
> -	}
>  
>  	closedir(dir);
>  	return __iommufd_configure_device(kvm, vdev);
> -- 
> 2.43.0
>