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@xxxxxxxxx. 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@xxxxxxxx>
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@xxxxxxxxx>
> ---
> 
>  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
>