Re: [PATCH iommufd] iommufd: Make vfio_compat's unmap succeed if the range is already empty

[Yi Liu <yi.l.liu@intel.com>]

On 2025/11/5 02:11, Jason Gunthorpe wrote:
> iommufd returns ENOENT when attempting to unmap a range that is already
> empty, while vfio type1 returns success. Fix vfio_compat to match.
> 
> Fixes: d624d6652a65 ("iommufd: vfio container FD ioctl compatibility")
> Reported-by: Alex Mastro <amastro@fb.com>
> Closes: https://lore.kernel.org/r/aP0S5ZF9l3sWkJ1G@devgpu012.nha5.facebook.com
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>   drivers/iommu/iommufd/io_pagetable.c    | 12 +++---------
>   drivers/iommu/iommufd/ioas.c            |  4 ++++
>   tools/testing/selftests/iommu/iommufd.c |  2 ++
>   3 files changed, 9 insertions(+), 9 deletions(-)

Reviewed-by: Yi Liu <yi.l.liu@intel.com>

> diff --git a/drivers/iommu/iommufd/io_pagetable.c b/drivers/iommu/iommufd/io_pagetable.c
> index c0360c450880b8..75d60f2ad90082 100644
> --- a/drivers/iommu/iommufd/io_pagetable.c
> +++ b/drivers/iommu/iommufd/io_pagetable.c
> @@ -707,7 +707,8 @@ static int iopt_unmap_iova_range(struct io_pagetable *iopt, unsigned long start,
>   	struct iopt_area *area;
>   	unsigned long unmapped_bytes = 0;
>   	unsigned int tries = 0;
> -	int rc = -ENOENT;
> +	/* If there are no mapped entries then success */
> +	int rc = 0;
>   
>   	/*
>   	 * The domains_rwsem must be held in read mode any time any area->pages
> @@ -777,8 +778,6 @@ static int iopt_unmap_iova_range(struct io_pagetable *iopt, unsigned long start,
>   
>   		down_write(&iopt->iova_rwsem);
>   	}
> -	if (unmapped_bytes)
> -		rc = 0;
>   
>   out_unlock_iova:
>   	up_write(&iopt->iova_rwsem);
> @@ -815,13 +814,8 @@ int iopt_unmap_iova(struct io_pagetable *iopt, unsigned long iova,
>   
>   int iopt_unmap_all(struct io_pagetable *iopt, unsigned long *unmapped)
>   {
> -	int rc;
> -
> -	rc = iopt_unmap_iova_range(iopt, 0, ULONG_MAX, unmapped);
>   	/* If the IOVAs are empty then unmap all succeeds */
> -	if (rc == -ENOENT)
> -		return 0;
> -	return rc;
> +	return iopt_unmap_iova_range(iopt, 0, ULONG_MAX, unmapped);
>   }
>   
>   /* The caller must always free all the nodes in the allowed_iova rb_root. */
> diff --git a/drivers/iommu/iommufd/ioas.c b/drivers/iommu/iommufd/ioas.c
> index 1542c5fd10a85c..459a7c5169154b 100644
> --- a/drivers/iommu/iommufd/ioas.c
> +++ b/drivers/iommu/iommufd/ioas.c
> @@ -367,6 +367,10 @@ int iommufd_ioas_unmap(struct iommufd_ucmd *ucmd)
>   				     &unmapped);
>   		if (rc)
>   			goto out_put;
> +		if (!unmapped) {
> +			rc = -ENOENT;
> +			goto out_put;
> +		}
>   	}
>   
>   	cmd->length = unmapped;
> diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c
> index 3eebf5e3b974f4..bb4d33dde3c899 100644
> --- a/tools/testing/selftests/iommu/iommufd.c
> +++ b/tools/testing/selftests/iommu/iommufd.c
> @@ -2638,6 +2638,8 @@ TEST_F(vfio_compat_mock_domain, map)
>   	ASSERT_EQ(0, ioctl(self->fd, VFIO_IOMMU_MAP_DMA, &map_cmd));
>   	ASSERT_EQ(0, ioctl(self->fd, VFIO_IOMMU_UNMAP_DMA, &unmap_cmd));
>   	ASSERT_EQ(BUFFER_SIZE, unmap_cmd.size);
> +	/* Unmap of empty is success */
> +	ASSERT_EQ(0, ioctl(self->fd, VFIO_IOMMU_UNMAP_DMA, &unmap_cmd));
>   
>   	/* UNMAP_FLAG_ALL requires 0 iova/size */
>   	ASSERT_EQ(0, ioctl(self->fd, VFIO_IOMMU_MAP_DMA, &map_cmd));
> 
> base-commit: b09ed52db1e688eb8205b1939ca1345179ecd515