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@xxxxxxxxxx>
> ---
> 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
>
