[RFT 0/5] drm/msm: DMABUF_DEBUG fixes

[RFT 0/5] drm/msm: DMABUF_DEBUG fixes

[Rob Clark]

With DMABUF_DEBUG=y, the page information is stripped from the sgt that
we get for an externally allocated buffer that is dma-buf imported (as
opposed to an exported GEM buffer that is re-imported).  This is a bit
unfortunate, since we need that information for mapping the buffer in
both paths:

1. Userspace GPU pgtables (TTBR0), where we use io_pgtable directly.
   Fortunately this we can fix ourselves easily.

2. Iommu managed pgtables (scanout, kernel GPU buffers (TTBR1)).  In
   normal usage that iommu subsystem envisions the dma-mapping and
   iommu-mapping are combined.  We can't really work around this but
   we can stop trying to pretend to be a normal iommu user and just
   go directly to the iommu_domain_ops.

This all depends on being dma_map_direct() for the dma_map_sgtable()
to work.  This should be the case on modern things that use arm-smmu.
It's less clear when it comes to older pre-arm-smmu hw.  It's possible
that this series causes those things to catch fire.  If so, look to
commit 0e764a01015d ("iommu/arm-smmu: Allow client devices to select
direct mapping") for inspiration.

Rob Clark (5):
  drm/msm: Cleanup if pages_to_sg() fails
  drm/msm/gem: dma_map/unmap_sgtable()
  drm/msm: Extract out map/unmap helpers
  drm/msm: Convert iommu map/unmap to helpers
  drm/msm: Convert map helper to use dma-address

 drivers/gpu/drm/msm/msm_gem.c   |  57 ++++++----
 drivers/gpu/drm/msm/msm_iommu.c | 182 ++++++++++++++++++++------------
 2 files changed, 153 insertions(+), 86 deletions(-)

-- 
2.55.0

[RFT 1/5] drm/msm: Cleanup if pages_to_sg() fails

[Rob Clark]

We never really handled this.  But in practice mapping to smmu would
have failed without an sgt, which would have been pretty obvious.

Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/msm_gem.c | 41 ++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 130ae27ee00b..181b9ad32672 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -190,6 +190,7 @@ static void update_lru(struct drm_gem_object *obj)
 static struct page **get_pages(struct drm_gem_object *obj)
 {
 	struct msm_gem_object *msm_obj = to_msm_bo(obj);
+	int ret = 0;
 
 	msm_gem_assert_locked(obj);
 
@@ -209,19 +210,17 @@ static struct page **get_pages(struct drm_gem_object *obj)
 			return p;
 		}
 
-		update_device_mem(dev->dev_private, obj->size);
-
 		msm_obj->pages = p;
 
 		msm_obj->sgt = drm_prime_pages_to_sg(obj->dev, p, npages);
 		if (IS_ERR(msm_obj->sgt)) {
-			void *ptr = ERR_CAST(msm_obj->sgt);
-
 			DRM_DEV_ERROR(dev->dev, "failed to allocate sgt\n");
-			msm_obj->sgt = NULL;
-			return ptr;
+			ret = PTR_ERR(msm_obj->sgt);
+			goto error;
 		}
 
+		update_device_mem(dev->dev_private, obj->size);
+
 		/* For non-cached buffers, ensure the new pages are clean
 		 * because display controller, GPU, etc. are not coherent:
 		 */
@@ -232,6 +231,14 @@ static struct page **get_pages(struct drm_gem_object *obj)
 	}
 
 	return msm_obj->pages;
+
+error:
+	msm_obj->sgt = NULL;
+
+	drm_gem_put_pages(obj, msm_obj->pages, false, false);
+	msm_obj->pages = NULL;
+
+	return ERR_PTR(ret);
 }
 
 static void put_pages(struct drm_gem_object *obj)
@@ -246,18 +253,16 @@ static void put_pages(struct drm_gem_object *obj)
 		drm_gpuvm_bo_gem_evict(obj, true);
 
 	if (msm_obj->pages) {
-		if (msm_obj->sgt) {
-			/* For non-cached buffers, ensure the new
-			 * pages are clean because display controller,
-			 * GPU, etc. are not coherent:
-			 */
-			if (msm_obj->flags & MSM_BO_WC)
-				sync_for_cpu(msm_obj);
-
-			sg_free_table(msm_obj->sgt);
-			kfree(msm_obj->sgt);
-			msm_obj->sgt = NULL;
-		}
+		/* For non-cached buffers, ensure the new
+		 * pages are clean because display controller,
+		 * GPU, etc. are not coherent:
+		 */
+		if (msm_obj->flags & MSM_BO_WC)
+			sync_for_cpu(msm_obj);
+
+		sg_free_table(msm_obj->sgt);
+		kfree(msm_obj->sgt);
+		msm_obj->sgt = NULL;
 
 		update_device_mem(obj->dev->dev_private, -obj->size);
 
-- 
2.55.0

[RFT 2/5] drm/msm/gem: dma_map/unmap_sgtable()

[Rob Clark]

This should functionally be a no-op for us, since we are managing our
SMMU directly (dma_map_ops is NULL), but it populates the dma_len/
dma_address's so that we can use them for setting up pgtable mappings.

Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/msm_gem.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 181b9ad32672..72e3cd8bb1e7 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -219,6 +219,15 @@ static struct page **get_pages(struct drm_gem_object *obj)
 			goto error;
 		}
 
+		ret = dma_map_sgtable(drm_dev_dma_dev(obj->dev),
+				      msm_obj->sgt,
+				      DMA_BIDIRECTIONAL,
+				      DMA_ATTR_SKIP_CPU_SYNC);
+		if (ret) {
+			DRM_DEV_ERROR(dev->dev, "could not map sgtable: %d\n", ret);
+			goto error;
+		}
+
 		update_device_mem(dev->dev_private, obj->size);
 
 		/* For non-cached buffers, ensure the new pages are clean
@@ -233,6 +242,8 @@ static struct page **get_pages(struct drm_gem_object *obj)
 	return msm_obj->pages;
 
 error:
+	if (!IS_ERR(msm_obj->sgt))
+		drm_prime_gem_destroy(obj, msm_obj->sgt);
 	msm_obj->sgt = NULL;
 
 	drm_gem_put_pages(obj, msm_obj->pages, false, false);
@@ -260,6 +271,11 @@ static void put_pages(struct drm_gem_object *obj)
 		if (msm_obj->flags & MSM_BO_WC)
 			sync_for_cpu(msm_obj);
 
+		dma_unmap_sgtable(drm_dev_dma_dev(obj->dev),
+				  msm_obj->sgt,
+				  DMA_BIDIRECTIONAL,
+				  DMA_ATTR_SKIP_CPU_SYNC);
+
 		sg_free_table(msm_obj->sgt);
 		kfree(msm_obj->sgt);
 		msm_obj->sgt = NULL;
-- 
2.55.0

[RFT 3/5] drm/msm: Extract out map/unmap helpers

[Rob Clark]

We're going to re-use these in the next commit.  No functional changes,
just some ugly callbacks.

Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/msm_iommu.c | 150 ++++++++++++++++++++------------
 1 file changed, 93 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index da6782fca6bd..a658493f8007 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -52,9 +52,8 @@ static struct msm_iommu_pagetable *to_pagetable(struct msm_mmu *mmu)
 }
 
 /* based on iommu_pgsize() in iommu.c: */
-static size_t calc_pgsize(struct msm_iommu_pagetable *pagetable,
-			   unsigned long iova, phys_addr_t paddr,
-			   size_t size, size_t *count)
+static size_t calc_pgsize(unsigned long pgsize_bitmap, unsigned long iova,
+			  phys_addr_t paddr, size_t size, size_t *count)
 {
 	unsigned int pgsize_idx, pgsize_idx_next;
 	unsigned long pgsizes;
@@ -62,7 +61,7 @@ static size_t calc_pgsize(struct msm_iommu_pagetable *pagetable,
 	unsigned long addr_merge = paddr | iova;
 
 	/* Page sizes supported by the hardware and small enough for @size */
-	pgsizes = pagetable->pgsize_bitmap & GENMASK(__fls(size), 0);
+	pgsizes = pgsize_bitmap & GENMASK(__fls(size), 0);
 
 	/* Constrain the page sizes further based on the maximum alignment */
 	if (likely(addr_merge))
@@ -78,7 +77,7 @@ static size_t calc_pgsize(struct msm_iommu_pagetable *pagetable,
 		return pgsize;
 
 	/* Find the next biggest support page size, if it exists */
-	pgsizes = pagetable->pgsize_bitmap & ~GENMASK(pgsize_idx, 0);
+	pgsizes = pgsize_bitmap & ~GENMASK(pgsize_idx, 0);
 	if (!pgsizes)
 		goto out_set_count;
 
@@ -107,24 +106,27 @@ static size_t calc_pgsize(struct msm_iommu_pagetable *pagetable,
 	return pgsize;
 }
 
-static int msm_iommu_pagetable_unmap(struct msm_mmu *mmu, u64 iova,
-		size_t size)
+typedef size_t (*unmap_fn)(void *arg, u64 iova, size_t pgsize, size_t count);
+typedef int (*map_fn)(void *arg, phys_addr_t paddr, u64 iova, size_t pgsize,
+		      size_t pgcount, int prot, size_t *mapped);
+
+static inline int
+__do_unmap(unsigned long pgsize_bitmap, u64 iova, size_t size,
+	   void *arg, unmap_fn unmap)
 {
-	struct msm_iommu_pagetable *pagetable = to_pagetable(mmu);
-	struct io_pgtable_ops *ops = pagetable->pgtbl_ops;
 	int ret = 0;
 
 	while (size) {
 		size_t pgsize, count;
 		ssize_t unmapped;
 
-		pgsize = calc_pgsize(pagetable, iova, iova, size, &count);
+		pgsize = calc_pgsize(pgsize_bitmap, iova, iova, size, &count);
 
-		unmapped = ops->unmap_pages(ops, iova, pgsize, count, NULL);
+		unmapped = unmap(arg, iova, pgsize, count);
 		if (unmapped <= 0) {
 			ret = -EINVAL;
 			/*
-			 * Continue attempting to unamp the remained of the
+			 * Continue attempting to unmap the remained of the
 			 * range, so we don't end up with some dangling
 			 * mapped pages
 			 */
@@ -135,55 +137,18 @@ static int msm_iommu_pagetable_unmap(struct msm_mmu *mmu, u64 iova,
 		size -= unmapped;
 	}
 
-	iommu_flush_iotlb_all(to_msm_iommu(pagetable->parent)->domain);
-
 	return ret;
 }
 
-static int msm_iommu_pagetable_map_prr(struct msm_mmu *mmu, u64 iova, size_t len, int prot)
-{
-	struct msm_iommu_pagetable *pagetable = to_pagetable(mmu);
-	struct io_pgtable_ops *ops = pagetable->pgtbl_ops;
-	struct msm_iommu *iommu = to_msm_iommu(pagetable->parent);
-	phys_addr_t phys = page_to_phys(iommu->prr_page);
-	u64 addr = iova;
-
-	while (len) {
-		size_t mapped = 0;
-		size_t size = PAGE_SIZE;
-		int ret;
-
-		ret = ops->map_pages(ops, addr, phys, size, 1, prot, GFP_KERNEL, &mapped);
-
-		/* map_pages could fail after mapping some of the pages,
-		 * so update the counters before error handling.
-		 */
-		addr += mapped;
-		len  -= mapped;
-
-		if (ret) {
-			msm_iommu_pagetable_unmap(mmu, iova, addr - iova);
-			return -EINVAL;
-		}
-	}
-
-	return 0;
-}
-
-static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64 iova,
-				   struct sg_table *sgt, size_t off, size_t len,
-				   int prot)
+static inline int
+__do_map(unsigned long pgsize_bitmap, u64 iova, struct sg_table *sgt, size_t off,
+	 size_t len, int prot, void *arg, map_fn map, unmap_fn unmap)
 {
-	struct msm_iommu_pagetable *pagetable = to_pagetable(mmu);
-	struct io_pgtable_ops *ops = pagetable->pgtbl_ops;
 	struct scatterlist *sg;
 	u64 addr = iova;
 	unsigned int i;
 
-	if (!sgt)
-		return msm_iommu_pagetable_map_prr(mmu, iova, len, prot);
-
-	for_each_sgtable_sg(sgt, sg, i) {
+	for_each_sgtable_sg (sgt, sg, i) {
 		size_t size = sg->length;
 		phys_addr_t phys = sg_phys(sg);
 
@@ -204,10 +169,9 @@ static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64 iova,
 			size_t pgsize, count, mapped = 0;
 			int ret;
 
-			pgsize = calc_pgsize(pagetable, addr, phys, size, &count);
+			pgsize = calc_pgsize(pgsize_bitmap, addr, phys, size, &count);
 
-			ret = ops->map_pages(ops, addr, phys, pgsize, count,
-					     prot, GFP_KERNEL, &mapped);
+			ret = map(arg, phys, addr, pgsize, count, prot, &mapped);
 
 			/* map_pages could fail after mapping some of the pages,
 			 * so update the counters before error handling.
@@ -218,7 +182,7 @@ static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64 iova,
 			len  -= mapped;
 
 			if (ret) {
-				msm_iommu_pagetable_unmap(mmu, iova, addr - iova);
+				__do_unmap(pgsize_bitmap, iova, addr - iova, arg, unmap);
 				return -EINVAL;
 			}
 		}
@@ -227,6 +191,78 @@ static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64 iova,
 	return 0;
 }
 
+static size_t
+__unmap_pgtable(void *arg, u64 iova, size_t pgsize, size_t pgcount)
+{
+	struct io_pgtable_ops *ops = arg;
+	return ops->unmap_pages(ops, iova, pgsize, pgcount, NULL);
+}
+
+static int
+__map_pgtable(void *arg, phys_addr_t paddr, u64 iova, size_t pgsize,
+	      size_t pgcount, int prot, size_t *mapped)
+{
+	struct io_pgtable_ops *ops = arg;
+	return ops->map_pages(ops, iova, paddr, pgsize, pgcount, prot, GFP_KERNEL, mapped);
+}
+
+static int msm_iommu_pagetable_unmap(struct msm_mmu *mmu, u64 iova, size_t size)
+{
+	struct msm_iommu_pagetable *pagetable = to_pagetable(mmu);
+	struct io_pgtable_ops *ops = pagetable->pgtbl_ops;
+	int ret = 0;
+
+	ret = __do_unmap(pagetable->pgsize_bitmap, iova, size, ops, __unmap_pgtable);
+
+	iommu_flush_iotlb_all(to_msm_iommu(pagetable->parent)->domain);
+
+	return ret;
+}
+
+static int msm_iommu_pagetable_map_prr(struct msm_mmu *mmu, u64 iova, size_t len, int prot)
+{
+	struct msm_iommu_pagetable *pagetable = to_pagetable(mmu);
+	struct io_pgtable_ops *ops = pagetable->pgtbl_ops;
+	struct msm_iommu *iommu = to_msm_iommu(pagetable->parent);
+	phys_addr_t phys = page_to_phys(iommu->prr_page);
+	u64 addr = iova;
+
+	while (len) {
+		size_t mapped = 0;
+		size_t size = PAGE_SIZE;
+		int ret;
+
+		ret = ops->map_pages(ops, addr, phys, size, 1, prot, GFP_KERNEL, &mapped);
+
+		/* map_pages could fail after mapping some of the pages,
+		 * so update the counters before error handling.
+		 */
+		addr += mapped;
+		len  -= mapped;
+
+		if (ret) {
+			msm_iommu_pagetable_unmap(mmu, iova, addr - iova);
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64 iova,
+				   struct sg_table *sgt, size_t off, size_t len,
+				   int prot)
+{
+	struct msm_iommu_pagetable *pagetable = to_pagetable(mmu);
+	struct io_pgtable_ops *ops = pagetable->pgtbl_ops;
+
+	if (!sgt)
+		return msm_iommu_pagetable_map_prr(mmu, iova, len, prot);
+
+	return __do_map(pagetable->pgsize_bitmap, iova, sgt, off, len, prot,
+			ops, __map_pgtable, __unmap_pgtable);
+}
+
 static void msm_iommu_pagetable_destroy(struct msm_mmu *mmu)
 {
 	struct msm_iommu_pagetable *pagetable = to_pagetable(mmu);
-- 
2.55.0

Re: [RFT 3/5] drm/msm: Extract out map/unmap helpers

[Karl Mehltretter]

> Does this error path miss an IOTLB flush?

Yes. msm_iommu_pagetable_unmap() called iommu_flush_iotlb_all() after
rollback. __do_map() now calls __do_unmap() directly and loses that
flush.

An arm64 QEMU/SMMUv3 test:

- mapped one 4 KiB page,
- hit -EEXIST on the second page, and
- verified that rollback removed the first PTE.

No stale translation was observed because the device had not accessed
the mapping. This tested the low-level IOMMU sequence, not the MSM driver
or Qualcomm hardware.

Restoring iommu_flush_iotlb_all() on this error path preserves the old
behavior.

Karl

Re: [RFT 3/5] drm/msm: Extract out map/unmap helpers

[Rob Clark]

On Tue, Oct 6, 2026 at 10:19 PM Karl Mehltretter <kmehltretter@gmail.com> wrote:
>
> > Does this error path miss an IOTLB flush?
>
> Yes. msm_iommu_pagetable_unmap() called iommu_flush_iotlb_all() after
> rollback. __do_map() now calls __do_unmap() directly and loses that
> flush.
>
> An arm64 QEMU/SMMUv3 test:
>
> - mapped one 4 KiB page,
> - hit -EEXIST on the second page, and
> - verified that rollback removed the first PTE.
>
> No stale translation was observed because the device had not accessed
> the mapping. This tested the low-level IOMMU sequence, not the MSM driver
> or Qualcomm hardware.
>
> Restoring iommu_flush_iotlb_all() on this error path preserves the old
> behavior.

I have a fixup, but haven't sent a v2 yet.. current series is still
enough to verify whether there is more work needed on older
pre-arm-smmu hw or not

BR,
-R

[RFT 4/5] drm/msm: Convert iommu map/unmap to helpers

[Rob Clark]

Re-use the previously introduced map/unmap helpers in the iommu path
(TTBR1 for GPU, ie. kernel managed buffers, and display/etc).

The callbacks work in terms of iommu_domain/ops instead of io_pgtable.
It would be kinda nice to just deal with io_pgtable, but the tlb ops
that arm-smmu uses for it's own io_pgtable require doing runpm, which
we wouldn't otherwise be able to do.

Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/msm_iommu.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index a658493f8007..50038d00c3a8 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -720,12 +720,27 @@ static void msm_iommu_detach(struct msm_mmu *mmu)
 	iommu_detach_device(iommu->domain, mmu->dev);
 }
 
+static size_t
+__unmap_iommu(void *arg, u64 iova, size_t pgsize, size_t pgcount)
+{
+	struct iommu_domain *domain = arg;
+	return domain->ops->unmap_pages(domain, iova, pgsize, pgcount, NULL);
+}
+
+static int
+__map_iommu(void *arg, phys_addr_t paddr, u64 iova, size_t pgsize,
+	    size_t pgcount, int prot, size_t *mapped)
+{
+	struct iommu_domain *domain = arg;
+	return domain->ops->map_pages(domain, iova, paddr, pgsize, pgcount,
+				      prot, GFP_KERNEL, mapped);
+}
+
 static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova,
 			 struct sg_table *sgt, size_t off, size_t len,
 			 int prot)
 {
-	struct msm_iommu *iommu = to_msm_iommu(mmu);
-	ssize_t ret;
+	struct iommu_domain *domain = to_msm_iommu(mmu)->domain;
 
 	WARN_ON(off != 0);
 
@@ -733,23 +748,18 @@ static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova,
 	if (iova & BIT_ULL(48))
 		iova |= GENMASK_ULL(63, 49);
 
-	ret = iommu_map_sgtable(iommu->domain, iova, sgt, prot);
-	if (ret < 0)
-		return ret;
-
-	return (ret == len) ? 0 : -EINVAL;
+	return __do_map(domain->pgsize_bitmap, iova, sgt, off, len, prot,
+			domain, __map_iommu, __unmap_iommu);
 }
 
 static int msm_iommu_unmap(struct msm_mmu *mmu, uint64_t iova, size_t len)
 {
-	struct msm_iommu *iommu = to_msm_iommu(mmu);
+	struct iommu_domain *domain = to_msm_iommu(mmu)->domain;
 
 	if (iova & BIT_ULL(48))
 		iova |= GENMASK_ULL(63, 49);
 
-	iommu_unmap(iommu->domain, iova, len);
-
-	return 0;
+	return __do_unmap(domain->pgsize_bitmap, iova, len, domain, __unmap_iommu);
 }
 
 static void msm_iommu_destroy(struct msm_mmu *mmu)
-- 
2.55.0

Re: [RFT 4/5] drm/msm: Convert iommu map/unmap to helpers

[Karl Mehltretter]

> By passing NULL for iotlb_gather, does this bypass the IOMMU driver's TLB
> gathering and flushing mechanism?

Yes. On arm64 QEMU/SMMUv3, unmap_pages(..., NULL) returned 4096 after
device DMA populated the IOTLB, but the next DMA still reached the old
page. It faulted after an explicit IOTLB flush.

Using iommu_unmap() invalidated the translation:

	return iommu_unmap(domain, iova, pgsize * pgcount);

> It appears this replaces iommu_map_sgtable() with a call to __do_map(),
> but there is no declaration or definition of __do_map() available.
>
> [...]
>
> Similar to the mapping path, is __do_unmap() actually defined?

Patch 3 defines both helpers. msm_iommu.o builds with all five patches.

Karl

Re: [RFT 4/5] drm/msm: Convert iommu map/unmap to helpers

[Rob Clark]

On Tue, Oct 6, 2026 at 10:23 PM Karl Mehltretter <kmehltretter@gmail.com> wrote:
>
> > By passing NULL for iotlb_gather, does this bypass the IOMMU driver's TLB
> > gathering and flushing mechanism?
>
> Yes. On arm64 QEMU/SMMUv3, unmap_pages(..., NULL) returned 4096 after
> device DMA populated the IOTLB, but the next DMA still reached the old
> page. It faulted after an explicit IOTLB flush.
>
> Using iommu_unmap() invalidated the translation:
>
>         return iommu_unmap(domain, iova, pgsize * pgcount);
>
> > It appears this replaces iommu_map_sgtable() with a call to __do_map(),
> > but there is no declaration or definition of __do_map() available.
> >
> > [...]
> >
> > Similar to the mapping path, is __do_unmap() actually defined?
>
> Patch 3 defines both helpers. msm_iommu.o builds with all five patches.

Sashiko is kinda funny sometimes :-)

BR,
-R

[RFT 5/5] drm/msm: Convert map helper to use dma-address

[Rob Clark]

Now that we have everything unified in one path, switch over to using
dma addresses for setting up the mapping.  This avoids issues with
CONFIG_DMABUF_DEBUG stripping the pages from the sgt.

Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/msm_iommu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index 50038d00c3a8..27a74514e942 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -148,9 +148,9 @@ __do_map(unsigned long pgsize_bitmap, u64 iova, struct sg_table *sgt, size_t off
 	u64 addr = iova;
 	unsigned int i;
 
-	for_each_sgtable_sg (sgt, sg, i) {
-		size_t size = sg->length;
-		phys_addr_t phys = sg_phys(sg);
+	for_each_sgtable_dma_sg (sgt, sg, i) {
+		size_t size = sg_dma_len(sg);
+		phys_addr_t phys = sg_dma_address(sg);
 
 		if (!len)
 			break;
-- 
2.55.0

Re: [RFT 0/5] drm/msm: DMABUF_DEBUG fixes

[Jianfeng Liu]

Hi Rob,

On Tue, Oct 6, 2026 at 6:09 AM Rob Clark wrote:
> With DMABUF_DEBUG=y, the page information is stripped from the sgt that
> we get for an externally allocated buffer that is dma-buf imported (as
> opposed to an exported GEM buffer that is re-imported). [...]

Tested on the machine that originally reported the breakage - this
exercises exactly the externally-allocated-buffer case from your cover
letter:

Tested-by: Jianfeng Liu <liujianfeng1994@gmail.com> # x1e78100 (Acer
  SFA14-11), v7.3-rc5 + this series, CONFIG_DMABUF_DEBUG=y

Hardware video decode in chromium (V4L2 decoder capture buffers from
videobuf2-dma-contig imported into msm and rendered by the GPU)
displays correctly, with zero arm-smmu faults and zero io-pgtable
WARNs.  For comparison, on plain v7.3-rc5 with DMABUF_DEBUG=y the
same workload logs UCHE translation faults and a __arm_lpae_unmap()
WARN storm (~470 traces per minute of playback).

This is also much cleaner than the translation-based approach in the
follow-up series I withdrew - mapping from the DMA addresses is where
I should have ended up in the first place.

One small suggestion for __do_map(): panthor treats "map_pages()
mapped nothing" as an error (panthor_mmu.c):

        /* If nothing was mapped, consider it an ENOMEM. */
        if (!ret && !mapped)
                ret = -ENOMEM;

With the DMA fields now populated on both paths this should not
trigger in practice, but it would turn any future regression back
into a loud failure instead of a silent empty mapping - which is
the failure mode this whole series fixes.

Happy to run more (heap-exported dmabuf import test, kmssink scanout
of V4L2 frames) on any revision.

BR,
Jianfeng

Re: [RFT 0/5] drm/msm: DMABUF_DEBUG fixes

[Rob Clark]

On Wed, Oct 7, 2026 at 6:16 AM Jianfeng Liu <liujianfeng1994@gmail.com> wrote:
>
> Hi Rob,
>
> On Tue, Oct 6, 2026 at 6:09 AM Rob Clark wrote:
> > With DMABUF_DEBUG=y, the page information is stripped from the sgt that
> > we get for an externally allocated buffer that is dma-buf imported (as
> > opposed to an exported GEM buffer that is re-imported). [...]
>
> Tested on the machine that originally reported the breakage - this
> exercises exactly the externally-allocated-buffer case from your cover
> letter:
>
> Tested-by: Jianfeng Liu <liujianfeng1994@gmail.com> # x1e78100 (Acer
>   SFA14-11), v7.3-rc5 + this series, CONFIG_DMABUF_DEBUG=y
>
> Hardware video decode in chromium (V4L2 decoder capture buffers from
> videobuf2-dma-contig imported into msm and rendered by the GPU)
> displays correctly, with zero arm-smmu faults and zero io-pgtable
> WARNs.  For comparison, on plain v7.3-rc5 with DMABUF_DEBUG=y the
> same workload logs UCHE translation faults and a __arm_lpae_unmap()
> WARN storm (~470 traces per minute of playback).
>
> This is also much cleaner than the translation-based approach in the
> follow-up series I withdrew - mapping from the DMA addresses is where
> I should have ended up in the first place.
>
> One small suggestion for __do_map(): panthor treats "map_pages()
> mapped nothing" as an error (panthor_mmu.c):
>
>         /* If nothing was mapped, consider it an ENOMEM. */
>         if (!ret && !mapped)
>                 ret = -ENOMEM;

Yeah, that is probably a good idea.

Thanks for testing

BR,
-R

>
> With the DMA fields now populated on both paths this should not
> trigger in practice, but it would turn any future regression back
> into a loud failure instead of a silent empty mapping - which is
> the failure mode this whole series fixes.
>
> Happy to run more (heap-exported dmabuf import test, kmssink scanout
> of V4L2 frames) on any revision.
>
> BR,
> Jianfeng