On 2026-06-29 at 11:49:56 +0100, Robin Murphy wrote:
> 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.
Hmm, so possible overflow and better to dig deeper, got it.
Thanks for your input!
>
> > [ 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.
I do not know when this issue was first introduced, because I
ran into it while debugging other problems addressed in this
series, but since you have a suspicion about 6.19 I'll stick to
that.
--
Best Regards,
Krzysztof
