* [PATCH iommufd] iommufd: Make vfio_compat's unmap succeed if the range is already empty
@ 2025-11-04 18:11 Jason Gunthorpe
2025-11-04 18:56 ` Alex Mastro
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Jason Gunthorpe @ 2025-11-04 18:11 UTC (permalink / raw)
To: iommu, Joerg Roedel, linux-kselftest, Robin Murphy, Shuah Khan,
Will Deacon
Cc: Alex Mastro, Eric Auger, Kevin Tian, Lixiao Yang, Matthew Rosato,
Nicolin Chen, patches, Yi Liu
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(-)
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
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH iommufd] iommufd: Make vfio_compat's unmap succeed if the range is already empty
2025-11-04 18:11 [PATCH iommufd] iommufd: Make vfio_compat's unmap succeed if the range is already empty Jason Gunthorpe
@ 2025-11-04 18:56 ` Alex Mastro
2025-11-05 4:47 ` Nicolin Chen
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Alex Mastro @ 2025-11-04 18:56 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: iommu, Joerg Roedel, linux-kselftest, Robin Murphy, Shuah Khan,
Will Deacon, Eric Auger, Kevin Tian, Lixiao Yang, Matthew Rosato,
Nicolin Chen, patches, Yi Liu
On Tue, Nov 04, 2025 at 02:11:49PM -0400, 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>
Thanks Jason.
Reviewed-by: Alex Mastro <amastro@fb.com>
Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH iommufd] iommufd: Make vfio_compat's unmap succeed if the range is already empty
2025-11-04 18:11 [PATCH iommufd] iommufd: Make vfio_compat's unmap succeed if the range is already empty Jason Gunthorpe
2025-11-04 18:56 ` Alex Mastro
@ 2025-11-05 4:47 ` Nicolin Chen
2025-11-10 11:22 ` Yi Liu
2025-11-17 6:56 ` Tian, Kevin
3 siblings, 0 replies; 6+ messages in thread
From: Nicolin Chen @ 2025-11-05 4:47 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: iommu, Joerg Roedel, linux-kselftest, Robin Murphy, Shuah Khan,
Will Deacon, Alex Mastro, Eric Auger, Kevin Tian, Lixiao Yang,
Matthew Rosato, patches, Yi Liu
On Tue, Nov 04, 2025 at 02:11:49PM -0400, 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>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH iommufd] iommufd: Make vfio_compat's unmap succeed if the range is already empty
2025-11-04 18:11 [PATCH iommufd] iommufd: Make vfio_compat's unmap succeed if the range is already empty Jason Gunthorpe
2025-11-04 18:56 ` Alex Mastro
2025-11-05 4:47 ` Nicolin Chen
@ 2025-11-10 11:22 ` Yi Liu
2025-11-17 6:56 ` Tian, Kevin
3 siblings, 0 replies; 6+ messages in thread
From: Yi Liu @ 2025-11-10 11:22 UTC (permalink / raw)
To: Jason Gunthorpe, iommu, Joerg Roedel, linux-kselftest,
Robin Murphy, Shuah Khan, Will Deacon
Cc: Alex Mastro, Eric Auger, Kevin Tian, Lixiao Yang, Matthew Rosato,
Nicolin Chen, patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH iommufd] iommufd: Make vfio_compat's unmap succeed if the range is already empty
2025-11-04 18:11 [PATCH iommufd] iommufd: Make vfio_compat's unmap succeed if the range is already empty Jason Gunthorpe
` (2 preceding siblings ...)
2025-11-10 11:22 ` Yi Liu
@ 2025-11-17 6:56 ` Tian, Kevin
2025-11-17 15:34 ` Jason Gunthorpe
3 siblings, 1 reply; 6+ messages in thread
From: Tian, Kevin @ 2025-11-17 6:56 UTC (permalink / raw)
To: Jason Gunthorpe, iommu@lists.linux.dev, Joerg Roedel,
linux-kselftest@vger.kernel.org, Robin Murphy, Shuah Khan,
Will Deacon
Cc: Alex Mastro, Eric Auger, Lixiao Yang, Matthew Rosato,
Nicolin Chen, patches@lists.linux.dev, Liu, Yi L
> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Wednesday, November 5, 2025 2:12 AM
>
sorry coming to this late, but why do we leave inconsistent behavior
between unmapping all vs. unmapping a range:
> 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);
> }
here empty IOVAs succeeds, while...
> @@ -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;
> + }
> }
...here it's a failure.
from uAPI p.o.v. better the two scenarios are consistent?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH iommufd] iommufd: Make vfio_compat's unmap succeed if the range is already empty
2025-11-17 6:56 ` Tian, Kevin
@ 2025-11-17 15:34 ` Jason Gunthorpe
0 siblings, 0 replies; 6+ messages in thread
From: Jason Gunthorpe @ 2025-11-17 15:34 UTC (permalink / raw)
To: Tian, Kevin
Cc: iommu@lists.linux.dev, Joerg Roedel,
linux-kselftest@vger.kernel.org, Robin Murphy, Shuah Khan,
Will Deacon, Alex Mastro, Eric Auger, Lixiao Yang, Matthew Rosato,
Nicolin Chen, patches@lists.linux.dev, Liu, Yi L
On Mon, Nov 17, 2025 at 06:56:56AM +0000, Tian, Kevin wrote:
> > From: Jason Gunthorpe <jgg@nvidia.com>
> > Sent: Wednesday, November 5, 2025 2:12 AM
> >
>
> sorry coming to this late, but why do we leave inconsistent behavior
> between unmapping all vs. unmapping a range:
>
> > 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);
> > }
>
> here empty IOVAs succeeds, while...
>
> > @@ -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;
> > + }
> > }
>
> ...here it's a failure.
>
> from uAPI p.o.v. better the two scenarios are consistent?
Maybe, but this has been like this from the start, so I don't think we
should change it..
This patch was about aligning the type 1 emulation, not changing the
existing iommufd uapi.
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-11-17 15:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-04 18:11 [PATCH iommufd] iommufd: Make vfio_compat's unmap succeed if the range is already empty Jason Gunthorpe
2025-11-04 18:56 ` Alex Mastro
2025-11-05 4:47 ` Nicolin Chen
2025-11-10 11:22 ` Yi Liu
2025-11-17 6:56 ` Tian, Kevin
2025-11-17 15:34 ` Jason Gunthorpe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).