On 29/06/2026 11:12 am, Krzysztof Karas wrote:
Hi Robin,
thanks for looking at this.
On 2026-06-25 at 15:29:10 +0100, Robin Murphy wrote:
On 25/06/2026 2:43 pm, Krzysztof Karas wrote:
Currently, if iommu maps fewer bytes than requested (iova_len),
it proceeds to free the iova, but never tries to unmap already
touched bytes. This behavior may cause memory hogging down the
line.
Huh? iommu_map_sg() has always unmapped internally upon any error - can you
clarify how you've seen it returning a short mapping in a non-error case?
Yes. I applied some debug logging:
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 381b60d9e7ce..f166cd31d68f 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -1515,8 +1515,10 @@ int iommu_dma_map_sg(struct device *dev, struct
scatterlist *sg, int nents,
* implementation - it knows better than we do.
*/
ret = iommu_map_sg(domain, iova, sg, nents, prot, GFP_ATOMIC);
- if (ret < 0 || ret < iova_len)
+ if (ret < 0 || ret < iova_len) {
+ printk("%s: ret = %zd, iova_len = %lu\n", __func__, ret,
iova_len);
goto out_free_iova;
+ }
return __finalise_sg(dev, sg, nents, iova);
@@ -1525,8 +1527,10 @@ int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
out_restore_sg:
__invalidate_sg(sg, nents);
out:
- if (ret != -ENOMEM && ret != -EREMOTEIO)
+ if (ret != -ENOMEM && ret != -EREMOTEIO) {
+ printk("%s: returning -EINVAL\n", __func__);
return -EINVAL;
+ }
return ret;
}
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d1a9e713d3a0..59163ad0bce3 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2900,6 +2900,7 @@ ssize_t iommu_map_sg(struct iommu_domain *domain,
unsigned long iova,
return mapped;
out_err:
+ printk("%s: Calling iommu_unmap()\n", __func__);
/* undo mappings already done */
iommu_unmap(domain, iova, mapped);
and ran "gem_exec_big" with subtest "single" from igt-gpu-tools
(./igt-gpu-tools/build/tests/gem_exec_big --run-subtest single)
on a Tiger Lake platform. I observed:
[ 77.494513] [IGT] gem_exec_big: executing
[ 77.513162] [IGT] gem_exec_big: starting subtest single
[ 77.513739] gem_exec_big (2863): drop_caches: 4
[ 79.912199] i915 0000:00:02.0: Using 39-bit DMA addresses
[ 79.915571] iommu_dma_map_sg: ret = 7138717696, iova_len = 20023619584
<<===
OK, yeah, something weird is happening there... Given that you're
apparently trying to map over 20GB in a single scatterlist, I suspect we
might be running into some 32-bit integer overflow somewhere - we've
certainly hit issues with gigantic individual segments in the past -
which is then leading to the IOVA calculation and/or field-swizzling in
iommu_dma_map_sg() itself going wrong, or the merging logic in
iommu_map_sg(), or perhaps both.
I'd agree there definitely appears to be a bug here, but ultimately it's
that iommu_map_sg() is somehow returning a short mapping when it should
not, so papering over that in iommu-dma is not the solution.
[ 79.915581] iommu_dma_map_sg: returning -EINVAL
[ 82.680323] [IGT] gem_exec_big: finished subtest single, SUCCESS
[ 82.682692] [IGT] gem_exec_big: exiting, ret=0
on a first test execution and then subsequent runs would result
DMA remap failures:
Indeed once the pagetables _have_ got out of sync with the IOVA
allocator then this is expected behaviour - -EADDRINUSE is generic_pt
refusing to map over an unexpectedly-present PTE, unlike the old
intel-iommu code which would have just silently replaced it and
succeeded, so if you're only seeing these subsequent failures since
6.19, that's probably a factor too.
Thanks,
Robin.
[ 1053.077620] [IGT] gem_exec_big: executing
[ 1053.086272] [IGT] gem_exec_big: starting subtest single
[ 1055.220807] iommu_dma_map_sg: ret = 10581184512, iova_len = 19171119104
[ 1055.220821] iommu_dma_map_sg: returning -EINVAL
[ 1055.343891] iommu_map_sg: Calling iommu_unmap()
[ 1055.343895] iommu_dma_map_sg: ret = -98, iova_len = 15817768960
[ 1055.356710] iommu_dma_map_sg: returning -EINVAL
[ 1055.356807] i915 0000:00:02.0: Failed to DMA remap 3861760 pages
[ 1055.509975] iommu_map_sg: Calling iommu_unmap()
[ 1055.509981] iommu_dma_map_sg: ret = -98, iova_len = 19171119104
[ 1055.509990] iommu_dma_map_sg: returning -EINVAL
[ 1055.636194] iommu_map_sg: Calling iommu_unmap()
[ 1055.636198] iommu_dma_map_sg: ret = -98, iova_len = 15817768960
[ 1055.648844] iommu_dma_map_sg: returning -EINVAL
[ 1055.648955] i915 0000:00:02.0: Failed to DMA remap 3861760 pages
[ 1055.800664] iommu_map_sg: Calling iommu_unmap()
[ 1055.800670] iommu_dma_map_sg: ret = -98, iova_len = 19171119104
[ 1055.800679] iommu_dma_map_sg: returning -EINVAL
[ 1055.924869] iommu_map_sg: Calling iommu_unmap()
[ 1055.924873] iommu_dma_map_sg: ret = -98, iova_len = 15817768960
[ 1055.937634] iommu_dma_map_sg: returning -EINVAL
[ 1055.937729] i915 0000:00:02.0: Failed to DMA remap 3861760 pages
[ 1056.042152] [IGT] gem_exec_big: finished subtest single, FAIL
[ 1056.042736] [IGT] gem_exec_big: exiting, ret=98
which would cause the test to fail. Rebooting the machine is the
only way I could get the test to pass again.
Thanks,
Robin.
Correct that by unmapping before exiting.
Signed-off-by: Krzysztof Karas <[email protected]>
---
drivers/iommu/dma-iommu.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 381b60d9e7ce..c4c058ba07ef 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -1515,8 +1515,14 @@ int iommu_dma_map_sg(struct device *dev, struct
scatterlist *sg, int nents,
* implementation - it knows better than we do.
*/
ret = iommu_map_sg(domain, iova, sg, nents, prot, GFP_ATOMIC);
- if (ret < 0 || ret < iova_len)
+ if (ret < 0 || ret < iova_len) {
+ if (ret > 0) {
+ /* Unmap partially mapped bytes before freeing IOVA */
+ if (iommu_unmap(domain, iova, ret) != ret)
+ ret = -EIO;
+ }
goto out_free_iova;
+ }
return __finalise_sg(dev, sg, nents, iova);
@@ -1525,7 +1531,7 @@ int iommu_dma_map_sg(struct device *dev, struct
scatterlist *sg, int nents,
out_restore_sg:
__invalidate_sg(sg, nents);
out:
- if (ret != -ENOMEM && ret != -EREMOTEIO)
+ if (ret != -ENOMEM && ret != -EREMOTEIO && ret != -EIO)
return -EINVAL;
return ret;
}
