[RFC PATCH v1 0/2] Fix the v7.3-rc4 DMABUF_DEBUG regression breaking drm/msm hardware video decode

[RFC PATCH v1 0/2] Fix the v7.3-rc4 DMABUF_DEBUG regression breaking drm/msm hardware video decode

[Jianfeng Liu]

Hardware video decode in clapper and chromium (V4L2 decoder output
buffers imported into drm/msm for rendering and scanout) breaks on
v7.3-rc4 with arm-smmu translation faults:

  gpu fault: ttbr0=000000088a889000 iova=000000010741c000 dir=READ
  type=TRANSLATION source=UCHE

v7.3-rc3 works fine. Bisecting between the two points at
143755bdabaa9 ("dma-buf: Make DMABUF_DEBUG default to y on
DEBUG_KERNEL kernels"), which fixed a dangling reference in the
DMABUF_DEBUG default and thereby silently enabled the option - and
with it the page-stripping sg_table wrapper that
dma_buf_map_attachment() hands to importers - on every kernel with
DEBUG_KERNEL=y, i.e. virtually every distro kernel.

drm/msm is affected in two places. It fills the page array of
imported GEM objects through the deprecated
drm_prime_sg_to_page_array(), and it maps the attachment sg_table
into the GPU's own pagetables with iommu_map_sgtable(). Both need
the struct page of the sg_table, which the debug wrapper removes
(and it zeroes sg->length, so the page iterator yields nothing while
the uninitialized page array is kept, with the helper still
returning success).

When such an import is used for rendering, the VM_BIND map job then
fails asynchronously after userspace has already enqueued GPU work
referencing the mapping, which surfaces as the UCHE translation
fault above instead of a clean error.

Patch 1 restores the DMABUF_DEBUG default to n until msm can be
converted to build its GPU mappings from the attachment's DMA
addresses. Patch 2 replaces the deprecated helper in msm with an
explicit loop that rejects page-less sg_tables at import time, so
userspace gets a clean -EINVAL and can fall back instead of
crashing the GPU.

Tested on a Snapdragon laptop with an Adreno GPU and arm-smmu
(v7.3-rc4):

- DMABUF_DEBUG off: hardware video decode works as on v7.3-rc3
- DMABUF_DEBUG on, without patch 2: GPU faults as above
- DMABUF_DEBUG on, with patch 2: imports are rejected cleanly
  ("import of dmabuf from 'videobuf2_dma_contig' rejected: sg_table
  has no/misaligned struct page info"), no GPU faults. clapper falls
  back to a working display path; chromium shows a black window as
  it has no fallback for a failed zero-copy import.

A full fix for DMABUF_DEBUG=y requires msm to map imported buffers
from their DMA addresses rather than struct pages; that conversion
is left as future work.

Comments welcome.


Jianfeng Liu (2):
  dma-buf: keep DMABUF_DEBUG off by default
  drm/msm: reject dma-buf imports without struct page info

 drivers/dma-buf/Kconfig       |  9 ++++++++-
 drivers/gpu/drm/msm/msm_gem.c | 31 ++++++++++++++++++++++++++++---
 2 files changed, 36 insertions(+), 4 deletions(-)

---
base-commit: 93f51579e7df248780214094418f205253383cc5
branch: fix/dmabuf-debug-msm-import

-- 
2.47.3

[RFC PATCH v1 1/2] dma-buf: keep DMABUF_DEBUG off by default

[Jianfeng Liu]

Commit 143755bdabaa9 ("dma-buf: Make DMABUF_DEBUG default to y on
DEBUG_KERNEL kernels") fixed a dangling reference in the DMABUF_DEBUG
default, which had the side effect of enabling the option (and with it
the page-stripping sg_table wrapper handed to importers) on every
kernel with DEBUG_KERNEL=y - i.e. virtually every distro kernel.

drm/msm is broken by this: it maps imported dma-bufs into the GPU's
own pagetables with iommu_map_sgtable(), which needs the struct page
of the attachment sg_table, and it fills the GEM object's page array
through drm_prime_sg_to_page_array().  With the debug wrapper in
place both silently produce garbage (the wrapper zeroes sg->length,
so the page iterator yields nothing and an uninitialized array is
kept).  The VM_BIND map job then fails asynchronously after userspace
has already enqueued GPU work referencing the mapping, which shows up
as an arm-smmu translation fault from UCHE, e.g.:

  gpu fault: ttbr0=000000088a889000 iova=000000010741c000 dir=READ
  type=TRANSLATION source=UCHE

This breaks hardware video decode (clapper, chromium) on Adreno
systems; bisected on a Snapdragon laptop as v7.3-rc3 good,
v7.3-rc4 bad, culprit 143755bdabaa9.

Revert the default until importers that legitimately need to build
phys-based mappings have been converted.

Fixes: 143755bdabaa9 ("dma-buf: Make DMABUF_DEBUG default to y on DEBUG_KERNEL kernels")
Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
---

 drivers/dma-buf/Kconfig | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig
index e4f078a326a41..b3c581ef4c987 100644
--- a/drivers/dma-buf/Kconfig
+++ b/drivers/dma-buf/Kconfig
@@ -43,7 +43,14 @@ config UDMABUF
 config DMABUF_DEBUG
 	bool "DMA-BUF debug checks"
 	depends on DMA_SHARED_BUFFER
-	default y if DEBUG_KERNEL
+	# NOTE: keep this default n.  The page-stripping sg_table wrapper that
+	# this option installs for importers breaks drivers that build a
+	# second-stage IOMMU mapping (phys -> iova) from the attachment sg_table
+	# and therefore still need the struct page, e.g. drm/msm with its
+	# per-process GPU pagetables.  Until those importers are fixed, making
+	# this default y breaks hardware video decode and GPU workloads out of
+	# the box on affected systems.
+	default n
 	help
 	  This option enables additional checks for DMA-BUF importers and
 	  exporters. Specifically it validates that importers do not peek at the
-- 
2.47.3

[RFC PATCH v1 2/2] drm/msm: reject dma-buf imports without struct page info

[Jianfeng Liu]

msm_gem_import() fills the GEM object's page array with the deprecated
drm_prime_sg_to_page_array() and stores the attachment sg_table for
later mapping into the GPU's own pagetables via iommu_map_sgtable().
Both need the struct page of the sg_table:

 - iommu_map_sg() maps sg_phys() of each entry, and
 - drm_prime_sg_to_page_array() iterates with for_each_sgtable_page,
   which walks sg->length.

When CONFIG_DMABUF_DEBUG=y, dma_buf_map_attachment() hands importers
a copy of the sg_table with the page pointers stripped and sg->length
zeroed.  In that case drm_prime_sg_to_page_array() "succeeds" while
filling zero entries, leaving msm_obj->pages uninitialized garbage
(kvmalloc_objs() does not zero).  The buffer is imported anyway, and
the first VM_BIND map of it fails asynchronously in the scheduler job
run - after userspace has already enqueued GPU work referencing the
mapping.  Userspace then observes arm-smmu translation faults from
UCHE, e.g. hardware video decode in clapper/chromium:

  gpu fault: ttbr0=000000088a889000 iova=000000010741c000 dir=READ
  type=TRANSLATION source=UCHE

Replace the deprecated helper with an explicit loop so that a missing
or short page list is detected at import time and rejected with
-EINVAL.  This turns the silent memory corruption into a clean import
error, letting userspace fall back instead of crashing the GPU.

Note that msm fundamentally cannot map a page-less sg_table into its
per-process GPU pagetables (it needs the physical addresses), so
imports of such buffers can never work until msm is converted to
build its GPU mappings from the attachment's DMA addresses.

Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>

---

 drivers/gpu/drm/msm/msm_gem.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index c4cff3d53d81b..0d5a91181d05b 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -1307,7 +1307,8 @@ struct drm_gem_object *msm_gem_import(struct drm_device *dev,
 	struct msm_gem_object *msm_obj;
 	struct drm_gem_object *obj;
 	struct dma_buf *dmabuf = attach->dmabuf;
-	size_t size, npages;
+	struct sg_page_iter piter;
+	size_t size, npages, filled = 0;
 	int ret;
 
 	size = PAGE_ALIGN(dmabuf->size);
@@ -1333,8 +1334,32 @@ struct drm_gem_object *msm_gem_import(struct drm_device *dev,
 		goto fail;
 	}
 
-	ret = drm_prime_sg_to_page_array(sgt, msm_obj->pages, npages);
-	if (ret) {
+	/*
+	 * Fill the page array ourselves instead of using the deprecated
+	 * drm_prime_sg_to_page_array(), so that we can detect sg_tables
+	 * that carry no struct page at all.  Those must be rejected:
+	 * msm maps imported buffers into the GPU's own pagetables with
+	 * iommu_map_sgtable(), which needs the physical pages, so an
+	 * import without page information could never be mapped.  The
+	 * most prominent case is the page-stripping sg_table wrapper that
+	 * dma_buf_map_attachment() hands out when CONFIG_DMABUF_DEBUG=y.
+	 *
+	 * drm_prime_sg_to_page_array() would "succeed" with zero entries
+	 * filled in that case and leave msm_obj->pages uninitialized,
+	 * which later blows up as arm-smmu translation faults from UCHE.
+	 */
+	for_each_sgtable_page(sgt, &piter, 0) {
+		if (WARN_ON(filled >= npages)) {
+			ret = -EINVAL;
+			goto fail;
+		}
+		msm_obj->pages[filled++] = sg_page_iter_page(&piter);
+	}
+	if (filled != npages) {
+		DRM_DEV_ERROR(dev->dev,
+			      "import of dmabuf from '%s' rejected: sg_table has no/misaligned struct page info\n",
+			      dmabuf->exp_name ?: "?");
+		ret = -EINVAL;
 		goto fail;
 	}
 
-- 
2.47.3

Re: [RFC PATCH v1 1/2] dma-buf: keep DMABUF_DEBUG off by default

[Christian König]

On 9/23/26 09:42, Jianfeng Liu wrote:
> [Sie erhalten nicht häufig E-Mails von liujianfeng1994@gmail.com. Weitere Informationen, warum dies wichtig ist, finden Sie unter https://aka.ms/LearnAboutSenderIdentification ]
> 
> Commit 143755bdabaa9 ("dma-buf: Make DMABUF_DEBUG default to y on
> DEBUG_KERNEL kernels") fixed a dangling reference in the DMABUF_DEBUG
> default, which had the side effect of enabling the option (and with it
> the page-stripping sg_table wrapper handed to importers) on every
> kernel with DEBUG_KERNEL=y - i.e. virtually every distro kernel.
> 
> drm/msm is broken by this: it maps imported dma-bufs into the GPU's
> own pagetables with iommu_map_sgtable(), which needs the struct page
> of the attachment sg_table, and it fills the GEM object's page array
> through drm_prime_sg_to_page_array().  With the debug wrapper in
> place both silently produce garbage (the wrapper zeroes sg->length,

Interesting point, we should probably change that.

> so the page iterator yields nothing and an uninitialized array is
> kept).  The VM_BIND map job then fails asynchronously after userspace
> has already enqueued GPU work referencing the mapping, which shows up
> as an arm-smmu translation fault from UCHE, e.g.:
> 
>   gpu fault: ttbr0=000000088a889000 iova=000000010741c000 dir=READ
>   type=TRANSLATION source=UCHE
> 
> This breaks hardware video decode (clapper, chromium) on Adreno
> systems; bisected on a Snapdragon laptop as v7.3-rc3 good,
> v7.3-rc4 bad, culprit 143755bdabaa9.
> 
> Revert the default until importers that legitimately need to build
> phys-based mappings have been converted.

Well that won't work like this, pointing those things out is exactly what DMABUF_DEBUG is made for.

What MSM is doing here is not allowed at all and can break badly. We gave drivers 5 years to get that fixed and I'm now pushing for completely deprecating that hack.

See patch 84335675f2223cbd25d0de7d38ecc7d40b95bd4a:

Author: Simona Vetter <simona.vetter@ffwll.ch>
Date:   Fri Jan 15 17:47:39 2021 +0100

    dma-buf: Add debug option

When MSM needs the struct page then it must import a shmemfd and not a DMA-buf. What we could do is to either fix MSM or mark it as broken.

Regards,
Christian.

> 
> Fixes: 143755bdabaa9 ("dma-buf: Make DMABUF_DEBUG default to y on DEBUG_KERNEL kernels")
> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
> ---
> 
>  drivers/dma-buf/Kconfig | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig
> index e4f078a326a41..b3c581ef4c987 100644
> --- a/drivers/dma-buf/Kconfig
> +++ b/drivers/dma-buf/Kconfig
> @@ -43,7 +43,14 @@ config UDMABUF
>  config DMABUF_DEBUG
>         bool "DMA-BUF debug checks"
>         depends on DMA_SHARED_BUFFER
> -       default y if DEBUG_KERNEL
> +       # NOTE: keep this default n.  The page-stripping sg_table wrapper that
> +       # this option installs for importers breaks drivers that build a
> +       # second-stage IOMMU mapping (phys -> iova) from the attachment sg_table
> +       # and therefore still need the struct page, e.g. drm/msm with its
> +       # per-process GPU pagetables.  Until those importers are fixed, making
> +       # this default y breaks hardware video decode and GPU workloads out of
> +       # the box on affected systems.
> +       default n
>         help
>           This option enables additional checks for DMA-BUF importers and
>           exporters. Specifically it validates that importers do not peek at the
> --
> 2.47.3
> 

Re: [RFC PATCH v1 1/2] dma-buf: keep DMABUF_DEBUG off by default

[Bryan O'Donoghue]

On 23/09/2026 08:42, Jianfeng Liu wrote:
> Commit 143755bdabaa9 ("dma-buf: Make DMABUF_DEBUG default to y on
> DEBUG_KERNEL kernels") fixed a dangling reference in the DMABUF_DEBUG
> default, which had the side effect of enabling the option (and with it
> the page-stripping sg_table wrapper handed to importers) on every
> kernel with DEBUG_KERNEL=y - i.e. virtually every distro kernel.
> 
> drm/msm is broken by this: it maps imported dma-bufs into the GPU's
> own pagetables with iommu_map_sgtable(), which needs the struct page
> of the attachment sg_table, and it fills the GEM object's page array
> through drm_prime_sg_to_page_array().  With the debug wrapper in
> place both silently produce garbage (the wrapper zeroes sg->length,
> so the page iterator yields nothing and an uninitialized array is
> kept).  The VM_BIND map job then fails asynchronously after userspace
> has already enqueued GPU work referencing the mapping, which shows up
> as an arm-smmu translation fault from UCHE, e.g.:
> 
>    gpu fault: ttbr0=000000088a889000 iova=000000010741c000 dir=READ
>    type=TRANSLATION source=UCHE
> 
> This breaks hardware video decode (clapper, chromium) on Adreno
> systems; bisected on a Snapdragon laptop as v7.3-rc3 good,
> v7.3-rc4 bad, culprit 143755bdabaa9.
> 
> Revert the default until importers that legitimately need to build
> phys-based mappings have been converted.
> 
> Fixes: 143755bdabaa9 ("dma-buf: Make DMABUF_DEBUG default to y on DEBUG_KERNEL kernels")
> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>

This is a very obviously LLM generated patch.

Please state so in the commit log.

Also I don't believe reverting the change is correct. Fix the bug where 
it exists in Adreno.

---
bod

Re: [RFC PATCH v1 2/2] drm/msm: reject dma-buf imports without struct page info

[Bryan O'Donoghue]

On 23/09/2026 08:42, Jianfeng Liu wrote:
> msm_gem_import() fills the GEM object's page array with the deprecated
> drm_prime_sg_to_page_array() and stores the attachment sg_table for
> later mapping into the GPU's own pagetables via iommu_map_sgtable().
> Both need the struct page of the sg_table:
> 
>   - iommu_map_sg() maps sg_phys() of each entry, and
>   - drm_prime_sg_to_page_array() iterates with for_each_sgtable_page,
>     which walks sg->length.
> 
> When CONFIG_DMABUF_DEBUG=y, dma_buf_map_attachment() hands importers
> a copy of the sg_table with the page pointers stripped and sg->length
> zeroed.  In that case drm_prime_sg_to_page_array() "succeeds" while
> filling zero entries, leaving msm_obj->pages uninitialized garbage
> (kvmalloc_objs() does not zero).  The buffer is imported anyway, and
> the first VM_BIND map of it fails asynchronously in the scheduler job
> run - after userspace has already enqueued GPU work referencing the
> mapping.  Userspace then observes arm-smmu translation faults from
> UCHE, e.g. hardware video decode in clapper/chromium:
> 
>    gpu fault: ttbr0=000000088a889000 iova=000000010741c000 dir=READ
>    type=TRANSLATION source=UCHE
> 
> Replace the deprecated helper with an explicit loop so that a missing
> or short page list is detected at import time and rejected with
> -EINVAL.  This turns the silent memory corruption into a clean import
> error, letting userspace fall back instead of crashing the GPU.
> 
> Note that msm fundamentally cannot map a page-less sg_table into its
> per-process GPU pagetables (it needs the physical addresses), so
> imports of such buffers can never work until msm is converted to
> build its GPU mappings from the attachment's DMA addresses.
> 
> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
> 
> ---
> 
>   drivers/gpu/drm/msm/msm_gem.c | 31 ++++++++++++++++++++++++++++---
>   1 file changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index c4cff3d53d81b..0d5a91181d05b 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -1307,7 +1307,8 @@ struct drm_gem_object *msm_gem_import(struct drm_device *dev,
>   	struct msm_gem_object *msm_obj;
>   	struct drm_gem_object *obj;
>   	struct dma_buf *dmabuf = attach->dmabuf;
> -	size_t size, npages;
> +	struct sg_page_iter piter;
> +	size_t size, npages, filled = 0;
>   	int ret;
> 
>   	size = PAGE_ALIGN(dmabuf->size);
> @@ -1333,8 +1334,32 @@ struct drm_gem_object *msm_gem_import(struct drm_device *dev,
>   		goto fail;
>   	}
> 
> -	ret = drm_prime_sg_to_page_array(sgt, msm_obj->pages, npages);
> -	if (ret) {
> +	/*
> +	 * Fill the page array ourselves instead of using the deprecated
> +	 * drm_prime_sg_to_page_array(), so that we can detect sg_tables
> +	 * that carry no struct page at all.  Those must be rejected:
> +	 * msm maps imported buffers into the GPU's own pagetables with
> +	 * iommu_map_sgtable(), which needs the physical pages, so an
> +	 * import without page information could never be mapped.  The
> +	 * most prominent case is the page-stripping sg_table wrapper that
> +	 * dma_buf_map_attachment() hands out when CONFIG_DMABUF_DEBUG=y.
> +	 *
> +	 * drm_prime_sg_to_page_array() would "succeed" with zero entries
> +	 * filled in that case and leave msm_obj->pages uninitialized,
> +	 * which later blows up as arm-smmu translation faults from UCHE.
> +	 */
> +	for_each_sgtable_page(sgt, &piter, 0) {
> +		if (WARN_ON(filled >= npages)) {
> +			ret = -EINVAL;
> +			goto fail;
> +		}
> +		msm_obj->pages[filled++] = sg_page_iter_page(&piter);
> +	}
> +	if (filled != npages) {
> +		DRM_DEV_ERROR(dev->dev,
> +			      "import of dmabuf from '%s' rejected: sg_table has no/misaligned struct page info\n",
> +			      dmabuf->exp_name ?: "?");
> +		ret = -EINVAL;
>   		goto fail;
>   	}
> 
> --
> 2.47.3
> 
> 

This very much looks like an LLM generated patch - the commit log, the 
large comment in the code and TBH the solution too.

Why is the fix Adreno specific ?

Shouldn't this function be ammended with

 > +	if (filled != npages)

instead ?

/**
  * drm_prime_sg_to_page_array - convert an sg table into a page array
  * @sgt: scatter-gather table to convert
  * @pages: array of page pointers to store the pages in
  * @max_entries: size of the passed-in array
  *
  * Exports an sg table into an array of pages.
  *
  * This function is deprecated and strongly discouraged to be used.
  * The page array is only useful for page faults and those can corrupt 
fields
  * in the struct page if they are not handled by the exporting driver.
  */
int __deprecated drm_prime_sg_to_page_array(struct sg_table *sgt,
                                             struct page **pages,
                                             int max_entries)
{
         struct sg_page_iter page_iter;
         struct page **p = pages;

         for_each_sgtable_page(sgt, &page_iter, 0) {
                 if (WARN_ON(p - pages >= max_entries))
                         return -1;
                 *p++ = sg_page_iter_page(&page_iter);
         }
         return 0;
}
EXPORT_SYMBOL(drm_prime_sg_to_page_array);

All the LLM seems to have done here is copy the code out of 
drm_prime_sg_to_page_array() and then add a check for filled != npages

But if that is a valid check for Adreno - then it is a valid check for:

  grep drm_prime_sg_to_page_array drivers/* -r
drivers/gpu/drm/vmwgfx/vmwgfx_blit.c:		ret = 
drm_prime_sg_to_page_array(src->ttm->sg, src_pages,
drivers/gpu/drm/vmwgfx/vmwgfx_blit.c:		ret = 
drm_prime_sg_to_page_array(dst->ttm->sg, dst_pages,
drivers/gpu/drm/omapdrm/omap_gem.c:		ret = 
drm_prime_sg_to_page_array(sgt, pages, npages);
drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c:	ret = 
drm_prime_sg_to_page_array(sgt, etnaviv_obj->pages, npages);
drivers/gpu/drm/drm_prime.c: * drm_prime_sg_to_page_array - convert an 
sg table into a page array
drivers/gpu/drm/drm_prime.c:int __deprecated 
drm_prime_sg_to_page_array(struct sg_table *sgt,
drivers/gpu/drm/drm_prime.c:EXPORT_SYMBOL(drm_prime_sg_to_page_array);
drivers/gpu/drm/msm/msm_gem.c:	ret = drm_prime_sg_to_page_array(sgt, 
msm_obj->pages, npages);
drivers/gpu/drm/xen/xen_drm_front_gem.c:	ret = 
drm_prime_sg_to_page_array(sgt, xen_obj->pages,

and should live in the helper function with an appropriate Fixes: tag 
for backporting...

---
bod

Re: [RFC PATCH v1 1/2] dma-buf: keep DMABUF_DEBUG off by default

[Rob Clark]

On Wed, Sep 23, 2026 at 1:03 AM Christian König
<christian.koenig@amd.com> wrote:
>
> On 9/23/26 09:42, Jianfeng Liu wrote:
> > [Sie erhalten nicht häufig E-Mails von liujianfeng1994@gmail.com. Weitere Informationen, warum dies wichtig ist, finden Sie unter https://aka.ms/LearnAboutSenderIdentification ]
> >
> > Commit 143755bdabaa9 ("dma-buf: Make DMABUF_DEBUG default to y on
> > DEBUG_KERNEL kernels") fixed a dangling reference in the DMABUF_DEBUG
> > default, which had the side effect of enabling the option (and with it
> > the page-stripping sg_table wrapper handed to importers) on every
> > kernel with DEBUG_KERNEL=y - i.e. virtually every distro kernel.
> >
> > drm/msm is broken by this: it maps imported dma-bufs into the GPU's
> > own pagetables with iommu_map_sgtable(), which needs the struct page
> > of the attachment sg_table, and it fills the GEM object's page array
> > through drm_prime_sg_to_page_array().  With the debug wrapper in
> > place both silently produce garbage (the wrapper zeroes sg->length,
>
> Interesting point, we should probably change that.

So the assessment of what is going wrong looks pretty wrong..  VM_BIND
should never lead to iommu_map_sgtable() (which is never used for gpu
per-process pgtables), for example.. but is used for mapping for
scanout.  And pages are never used for mapping in either path.

However there are a few places where sg->length is used (in iommu code
and msm).. AFAICT dma_buf_wrap_sg_table() zeroing out sg->length is
what the actual problem here is, rather than any use of struct page.

(And yeah, I should get rid of use of drm_prime_sg_to_page_array()..
but that cleanup that I haven't found time for shouldn't be the
problem here.)

BR,
-R

Re: [RFC PATCH v1 1/2] dma-buf: keep DMABUF_DEBUG off by default

[Jianfeng Liu]

Hi Rob,

On Thu, Sep 24, 2026 at 7:01 AM Rob Clark wrote:
> So the assessment of what is going wrong looks pretty wrong..  VM_BIND
> should never lead to iommu_map_sgtable() (which is never used for gpu
> per-process pgtables), for example.. but is used for mapping for
> scanout.  And pages are never used for mapping in either path.
>
> However there are a few places where sg->length is used (in iommu code
> and msm).. AFAICT dma_buf_wrap_sg_table() zeroing out sg->length is
> what the actual problem here is, rather than any use of struct page.

Thanks for the correction - you're right, I mis-traced the GPU path.
The per-process pgtable mapping goes through
msm_iommu_pagetable_map(), which walks the sg_table with sg->length
and sg_phys(). With the wrapper zeroing sg->length it iterates the
entries, maps nothing at all and still returns 0 - which explains
the UCHE translation faults without any error anywhere, and is a
nastier failure mode than the async-bind-failure story I wrote in
the commit log.

With that corrected picture, DMABUF_DEBUG=y breaks msm in the map
paths themselves: msm_iommu_pagetable_map() for the GPU and
iommu_map_sg() for scanout both consume sg->length, and
dma_buf_wrap_sg_table() zeroes it, so every mapping of a
page-stripped sg_table silently maps nothing. On top of that msm
also uses sg_phys() in those paths and
drm_prime_sg_to_page_array() for the page array, so even with
sg->length preserved, page-less entries would map garbage
physical addresses instead of failing loudly.

So it looks like this needs work on both sides:

 - dma-buf: preserve sg->length in the debug wrapper, so
   sg->length consumers at least fail loudly instead of silently
   mapping nothing. I think that is what both Christian's "we
   should probably change that" and your "zeroing out sg->length
   is what the actual problem is" are pointing at.

 - msm: stop consuming struct page and sg->length of imported
   sg_tables, i.e. build the GPU and scanout mappings from the DMA
   addresses, plus the drm_prime_sg_to_page_array() cleanup.

Is that the right split, and is there a preferred direction for
the msm side?

> (And yeah, I should get rid of use of drm_prime_sg_to_page_array()..
> but that cleanup that I haven't found time for shouldn't be the
> problem here.)

Agreed on it not being what produced the faults - but it is part
of the same contract problem, see below.

Also answering Bryan's review of patch 2, which is in a different
branch of this thread:

On Thu, Sep 24, 2026 at 10:36 AM Bryan O'Donoghue wrote:
> Why is the fix Adreno specific ?
>
> Shouldn't this function be ammended with
>
>  > +    if (filled != npages)
>
> instead ?

It isn't meant to be - msm_gem_import() is the shared GPU/DPU
import path. And putting the fill-count check into
drm_prime_sg_to_page_array() itself would indeed be the better
generic version of that guard; I checked the other callers
(etnaviv, omapdrm, vmwgfx, xen) and none of them expects a
partial fill either. But with the corrected analysis above, the
page array isn't what produced the GPU faults, so neither variant
is a real fix. I'm not asking for either patch to be merged - the
series is a bug report with code attached, sent to get exactly
this discussion going, which is also why it carries the RFC
prefix.

> This very much looks like an LLM generated patch - the commit log, the
> large comment in the code and TBH the solution too.

Sorry about that - the patches were drafted with LLM assistance
and I should have declared that up front. Any later version will
carry a proper declaration.

Thanks all!
Jianfeng

Re: [RFC PATCH v1 1/2] dma-buf: keep DMABUF_DEBUG off by default

[Rob Clark]

On Thu, Sep 24, 2026 at 7:54 AM Jianfeng Liu <liujianfeng1994@gmail.com> wrote:
>
> Hi Rob,
>
> On Thu, Sep 24, 2026 at 7:01 AM Rob Clark wrote:
> > So the assessment of what is going wrong looks pretty wrong..  VM_BIND
> > should never lead to iommu_map_sgtable() (which is never used for gpu
> > per-process pgtables), for example.. but is used for mapping for
> > scanout.  And pages are never used for mapping in either path.
> >
> > However there are a few places where sg->length is used (in iommu code
> > and msm).. AFAICT dma_buf_wrap_sg_table() zeroing out sg->length is
> > what the actual problem here is, rather than any use of struct page.
>
> Thanks for the correction - you're right, I mis-traced the GPU path.
> The per-process pgtable mapping goes through
> msm_iommu_pagetable_map(), which walks the sg_table with sg->length
> and sg_phys(). With the wrapper zeroing sg->length it iterates the
> entries, maps nothing at all and still returns 0 - which explains
> the UCHE translation faults without any error anywhere, and is a
> nastier failure mode than the async-bind-failure story I wrote in
> the commit log.
>
> With that corrected picture, DMABUF_DEBUG=y breaks msm in the map
> paths themselves: msm_iommu_pagetable_map() for the GPU and
> iommu_map_sg() for scanout both consume sg->length, and
> dma_buf_wrap_sg_table() zeroes it, so every mapping of a
> page-stripped sg_table silently maps nothing. On top of that msm
> also uses sg_phys() in those paths and
> drm_prime_sg_to_page_array() for the page array, so even with
> sg->length preserved, page-less entries would map garbage
> physical addresses instead of failing loudly.

oh, ugg.. I overlooked that sg_phys() uses the pages under the hood..
I think using sg_dma_len()/sg_dma_address() instead should be fine in
msm.

Otherwise, (other than the fault path) obj->pages is mostly a proxy
for "is backing storage allocated".. we could just use obj->sgt for
that instead.

> So it looks like this needs work on both sides:
>
>  - dma-buf: preserve sg->length in the debug wrapper, so
>    sg->length consumers at least fail loudly instead of silently
>    mapping nothing. I think that is what both Christian's "we
>    should probably change that" and your "zeroing out sg->length
>    is what the actual problem is" are pointing at.
>
>  - msm: stop consuming struct page and sg->length of imported
>    sg_tables, i.e. build the GPU and scanout mappings from the DMA
>    addresses, plus the drm_prime_sg_to_page_array() cleanup.
>
> Is that the right split, and is there a preferred direction for
> the msm side?

On the msm side, I think we should be using
sg_dma_len()/sg_dma_address() for the immediate issue.

Maybe we can just do the same on the iommu side, but not sure what
various sharp edges might exist with other iommu users.  We could also
just stop using iommu_map_sgtable() and inline our own iommu_map_sg()?
 It looks like there are a few other drm drivers that use
iommu_map_sgtable() so it might be worth at least trying to fix this
in iommu.  Presumably if there are issues with that approach an iommu
maintainer would speak up.

For the longer term cleanup, I think we should use msm_obj->sgt
instead of msm_obj->pages for "is there backing storage", and allow
msm_obj->pages to be NULL for imported dma_buf's

Given the late stage for v7.3, the immediate thing we should do is
revert the CONFIG_DMABUF_DEBUG change IMHO, and try again once we have
sorted out what to do about the iommu_map_sgtable() path.

BR,
-R

> > (And yeah, I should get rid of use of drm_prime_sg_to_page_array()..
> > but that cleanup that I haven't found time for shouldn't be the
> > problem here.)
>
> Agreed on it not being what produced the faults - but it is part
> of the same contract problem, see below.
>
> Also answering Bryan's review of patch 2, which is in a different
> branch of this thread:
>
> On Thu, Sep 24, 2026 at 10:36 AM Bryan O'Donoghue wrote:
> > Why is the fix Adreno specific ?
> >
> > Shouldn't this function be ammended with
> >
> >  > +    if (filled != npages)
> >
> > instead ?
>
> It isn't meant to be - msm_gem_import() is the shared GPU/DPU
> import path. And putting the fill-count check into
> drm_prime_sg_to_page_array() itself would indeed be the better
> generic version of that guard; I checked the other callers
> (etnaviv, omapdrm, vmwgfx, xen) and none of them expects a
> partial fill either. But with the corrected analysis above, the
> page array isn't what produced the GPU faults, so neither variant
> is a real fix. I'm not asking for either patch to be merged - the
> series is a bug report with code attached, sent to get exactly
> this discussion going, which is also why it carries the RFC
> prefix.
>
> > This very much looks like an LLM generated patch - the commit log, the
> > large comment in the code and TBH the solution too.
>
> Sorry about that - the patches were drafted with LLM assistance
> and I should have declared that up front. Any later version will
> carry a proper declaration.
>
> Thanks all!
> Jianfeng