PCI P2PDMA applies Request and Completion Redirect throughout both paths.
This misclassifies asymmetric and nested switches, and reports one answer
for every kind of TLP.
Three ACS controls act on TLP attributes the client chooses rather than
on the topology: Translation Blocking and Direct Translated P2P act on
a Request's Address Type, and Completion Redirect skips Completions carrying
Relaxed Ordering.
Evaluate each direction at the path divergence, decide every class from the
one walk, expose the provider to dma-buf importers, and let mlx5 ask rather
than assume.
Signed-off-by: Leon Romanovsky <leonro(a)nvidia.com>
---
Changes in v8:
- Added extra Reviewed-by from Logan.
- Added patches to take care of ATS per-device vs. per-mapping option
- Link to v7: https://patch.msgid.link/20260920-fix-p2p-acs-v4-0-v7-0-ca0828ab697c@nvidia…
Changes in v7:
- Removed "Document pdev->p2pdma lifetime rules" patch, it gives nothing
after p2pmem fix.
- Split dmabuf patch.
- Tushar retested the series, so added his Tested-by.
- Added Logan's ROB tags and fixed minor documentation issues pointed
by him.
- Link to v6: https://lore.kernel.org/all/20260914-fix-p2p-acs-v4-0-v6-0-5ef07ec9ef06@nvi…
Changes in v6:
- Changed DMABUF to use callbacks and not directly stored pointer.
- Link to v5: https://patch.msgid.link/20260910-fix-p2p-acs-v4-0-v5-0-856087f63c0d@nvidia…
Changes in v5:
- Rebase on the posted fixes.
- Dropped tags from changed patches.
- Remove Egress Control Vector interpretation and coverage.
- Keep enabled Egress Control conservative as a Request redirect.
- Use pci_dbg()/dev_dbg() for diagnostics and drop the "debug" prefix.
- Removed code comments from "Document the pdev->p2pdma lifetime and RCU
rules" patch and reduced description to actual lifetime explanation.
- Added note that Linux assumes that TLPs are in strict-ordering and
untranslated.
- Added code to calculate p2p paths per-TLP type.
- Converted mlx5 to use that new proposed API.
- Link to https://patch.msgid.link/20260821-fix-p2p-acs-v4-0-v4-0-94426b96de73@nvidia…
Changes in v4:
- Reject ACS Violations and unreadable routing state instead of treating
them as host-bridge redirects
- Added Tested-by tags from Tushar Dave
- Added support to asymmetric ACS routing
- Limited redirect checks to the two ports at the path divergence
- Added standalone ACS routing diagnostics for hardware retesting
- Dropped " PCI: Account for Direct Translated P2P in ACS isolation checks" patch
- Link to v3: https://patch.msgid.link/20260811-fix-p2p-acs-v3-0-efc488ee7c03@nvidia.com
Changes in v3:
- Fixed pci_p2pdma_add_resource() error unwinding
- Made pdev->p2pdma teardown wait unconditionally for RCU readers
- Restricted pci_p2pmem_find_many() to pool-backed providers
- Documented the pdev->p2pdma lifetime and RCU rules
- Fixed calc_map_type_and_dist() handling of the verbose argument
- Required the ACS port and target to share a bus before indexing the
Egress Control Vector
- Gave pci_acs_enabled() and pci_acs_path_enabled() a scope, so the ACS
Direct Translated P2P rule no longer stops pci_enable_pasid() from
enabling PASID
- Dropped "Report ACS ports when the paths share no upstream bridge":
the mapping type cannot change without a shared upstream bridge, so
the pci=disable_acs_redir= hint was not actionable there and the ACS
walk only cost config space reads
- Folded the Request Redirect rule into pci_acs_rr_ineffective(), so
pci_acs_flags_enabled() and the Intel SPT PCH quirk share one copy
- Renamed pci_acs_egress_ctrl_set() to pci_acs_egress_ctrl_is_set(), it
reads the bit rather than setting it
- Reworded the blocked-path warning: ACS may also leave the direct route
indeterminate rather than blocked
- Added KUnit coverage for the shared-bus guard, a device with no ACS
capability and an unreadable ACS Control register
- Added the missing Fixes: tags, a second one on the
pci_p2pdma_add_resource() unwinding fix (the dangling devres action
dates to f58ef9d1d135) and one on the Egress Control isolation change
- Link to v2: https://patch.msgid.link/20260806-fix-p2p-acs-v2-0-0cec14812965@nvidia.com
Changes in v2:
- Added Logan's ROB tags
- Added commas in Documentation patch
- Link to v1: https://patch.msgid.link/20260802-fix-p2p-acs-v1-0-a7c5eb64fff6@nvidia.com
---
Leon Romanovsky (23):
PCI/P2PDMA: Document the TLP attribute assumptions
PCI/P2PDMA: Derive routing from directional ACS controls
PCI: Reject unreadable ACS controls in isolation checks
PCI/P2PDMA: Evaluate ACS controls at the path divergence
PCI/P2PDMA: Document directional ACS routing
PCI/P2PDMA: Collect the path's ACS controls before deciding
PCI/P2PDMA: Answer routing per TLP class
PCI/P2PDMA: Route Relaxed Ordering Completions directly
PCI/P2PDMA: Reject Translated Requests blocked by Translation Blocking
PCI/P2PDMA: Route Translated Requests under Direct Translated P2P
PCI/P2PDMA: Log detailed ACS routing diagnostics
PCI/P2PDMA: Add KUnit tests for the ACS routing decisions
PCI/P2PDMA: Test the ACS P2P routing walk
PCI: Add KUnit coverage for ACS isolation checks
PCI/P2PDMA: Document TLP-class routing
dma-buf: Let importers ask how peer-to-peer traffic is routed
vfio/pci: Hand out the P2PDMA provider behind a dma-buf
RDMA/uverbs: Hand out the P2PDMA provider behind a dma-buf
RDMA/mlx5: Ask P2PDMA whether ATS takes a direct peer-to-peer route
PCI/P2PDMA: Let a client declare that it selects ATS per mapping
RDMA/mlx5: Declare to P2PDMA that ATS is selected per memory key
PCI/P2PDMA: Evaluate the ATS path for clients with ATS enabled
PCI/P2PDMA: Test the routing of clients with ATS enabled
Documentation/admin-guide/kernel-parameters.txt | 15 +-
Documentation/driver-api/pci/p2pdma.rst | 80 ++
drivers/dma-buf/dma-buf-mapping.c | 39 +
drivers/infiniband/core/uverbs_std_types_dmabuf.c | 12 +
drivers/infiniband/hw/mlx5/main.c | 2 +
drivers/infiniband/hw/mlx5/mlx5_ib.h | 36 +-
drivers/infiniband/hw/mlx5/mr.c | 47 ++
drivers/pci/Kconfig | 15 +
drivers/pci/Makefile | 1 +
drivers/pci/p2pdma.c | 704 ++++++++++++++++--
drivers/pci/pci.c | 7 +-
drivers/pci/pci.h | 31 +
drivers/pci/pci_acs_test.c | 860 ++++++++++++++++++++++
drivers/pci/quirks.c | 6 +-
drivers/vfio/pci/vfio_pci_dmabuf.c | 12 +
include/linux/dma-buf-mapping.h | 3 +
include/linux/dma-buf.h | 19 +
include/linux/pci-p2pdma.h | 62 +-
18 files changed, 1828 insertions(+), 123 deletions(-)
---
base-commit: 08dbfad3f5040f5bdb6c529da20d6d4e81fefd72
change-id: 20260821-fix-p2p-acs-v4-0-e72455e3a261
prerequisite-message-id: <20260830-batch-p2p-fixes-v1-0-5044e8dfbe2e(a)nvidia.com>
prerequisite-patch-id: 6b25c7fcf164cdfc14e9fac5b908d97fcf6509d7
prerequisite-patch-id: 0d083c281001365aae4b35544cf28891a6ab9a96
prerequisite-patch-id: bfd9dabf271f3cc9a3a61f46387d20c20311363d
prerequisite-patch-id: fad0275efc722830fc591509506c0a5e4f581073
prerequisite-patch-id: 0c83bee688fec1f6d1564654df7c630fa6a4a978
Best regards,
--
Leon Romanovsky <leonro(a)nvidia.com>
On Wed, Sep 30, 2026 at 01:48:49PM +0100, Pavel Begunkov wrote:
> On 9/30/26 11:08, Pavel Begunkov wrote:
> > On 9/30/26 04:00, Matthew Brost wrote:
> > > On Mon, Sep 21, 2026 at 02:38:45PM +0100, Pavel Begunkov wrote:
> > ...>> +struct dma_buf_io_map *dma_buf_io_create_map(struct dma_buf_io_ctx *ctx)
> > > > +{
> > > > + struct dma_buf *dmabuf = ctx->dmabuf;
> > > > + struct dma_buf_io_map *map;
> > > > + long ret;
> > > > +
> > > > + guard(mutex)(&ctx->map_create_mutex);
> > > > +
> > > > + scoped_guard(mutex, &ctx->map_mutex) {
> > > > + if (ctx->maps_killed)
> > > > + return ERR_PTR(-ENOENT);
> > > > + /* recheck under the lock in case it has already been re-created */
> > > > + map = __dma_buf_io_get_map(ctx);
> > > > + if (map)
> > > > + return map;
> > > > + }
> > > > +
> > > > + dma_buf_io_wait_active_maps(ctx);
> > > > +
> > > > + ret = dma_resv_lock_interruptible(dmabuf->resv, NULL);
> > > > + if (ret)
> > > > + return ERR_PTR(ret);
> > > > +
> > > > + ret = dma_resv_wait_timeout(dmabuf->resv, DMA_RESV_USAGE_KERNEL,
> > > > + true, MAX_SCHEDULE_TIMEOUT);
> > > > + if (ret <= 0) {
> > > > + if (!ret)
> > > > + ret = -EAGAIN;
> > > > + dma_resv_unlock(dmabuf->resv);
> > > > + return ERR_PTR(ret);
> > > > + }
> > > > +
> > > > + map = ctx->dev_ops->map(ctx);
> > >
> > > I'm playing around this code now.
> > >
> > > I think you need the dma_resv_wait_timeout after the 'map'?
> > >
> > > If a device doesn't support p2p ->map() will typically trigger an async
> > > migrate to system memory and data will be moving but the map is valid -
> > > Xe 100% does this, I checked AMDGPU and fairly confident it has the same
> > > async behavior.
> >
> > There is a wait right before because I read somewhere in dma-buf
> > comments that I need to do that, sounds a bit odd if I need to
> > wait on fences before and after. I can add it, just curious how
> > come that other dma_buf_map_attachment() callers don't need to do
> > that. Or maybe they wait somewhere else?
In GPU drivers, when we receive a foreign object and map it in Xe or
AMDGPU, this logic sits deep in the stack. However, the top-level call
is typically ttm_bo_validate(), which in both drivers eventually
resolves to the TTM ->move() vfunc. That path calls
dma_buf_map_attachment(), which in turn invokes the dma-buf's ->map()
callback.
That ->map() callback can trigger a asynchronous move on a different
device, where the top-level call is again ttm_bo_validate().
We then end up back in the TTM ->move() vfunc, where kernel fences are
installed. I realize that's a lot of layers, but the call chain can end
up looking like this.
Now we have a mapping and an object with kernel fences attached. Before
the object can be used by either the exec IOCTL (batch buffer
submission), the VM bind IOCTL (mapping into the GPU address space), or
a CPU page fault (not relevant for dma-bufs since they cannot be
CPU-mapped, but a useful example of a normal BO move), we wait on those
kernel fences.
In the case of the exec IOCTL or VM bind IOCTL, the kernel fences are
added as dependencies to a drm_sched job, delaying its execution until
the fences signal. That is where the wait occurs. In the case of a CPU
page fault, the kernel waits directly on the fences before installing
the CPU page mappings.
So the TL;DR is if you want to immediately hand back a valid mapping,
the kernel fences must be waited on before returning after ->map() call.
Matt
>
> I can't find it, so maybe it was the comment below and I mixed
> sth up back then. I'll move it after ->map().
>
>
> * Note that for non-dynamic exporters the driver must guarantee that
> * that the memory is available for use and cleared of any old data by
> * the time this function returns. Drivers which pipeline their buffer
> * moves internally must wait for all moves and clears to complete.
> * Dynamic exporters do not need to follow this rule: For non-dynamic
> * importers the buffer is already pinned through @pin, which has the
> * same requirements. Dynamic importers otoh are required to obey the
> * dma_resv fences.
> *
>
> --
> Pavel Begunkov
>
Le 28/09/2026 à 15:32, Pavel Begunkov a écrit :
> The goal is to be able to natively use dma-buf in the read-write / IO
> path. This patch adds basic building blocks serving as a glue and API
> between drivers and upper layer subsystems providing the uAPI. Later
> patches implement it for NVMe raw block devices and expose it to the
> user space via io_uring.
>
> There are two main objects. struct dma_buf_io_ctx and struct
> dma_buf_io_map. The ctx is used during initial registration and serves
> as an interface between the upper layer user like io_uring and to the
> importer subsystem / driver. The map represents the actual dma map
> established for the target device[s] with dma_buf_map_attachment() and
> stored in a device specific format. The context is created via a new
> file operation ->init_dma_buf_io_ctx.
>
> The ctx-map separation exists to support map invalidation (see
> dma_buf_io_invalidate_mappings()). A ctx can create
> multiple maps during its lifetime, but there can only be no more than
> one (active) map attached to it. Invalidation drops the active map
> if present, and the next map will only be attempted to be created
> once there is a new request that wants to use the dma-buf IO ctx.
>
> The primary task of the dma_buf_io_map object is to count requests
> using it and to wait for their completion when we want to destroy the
> DMA map.
>
> [un]mapping and any work with dma addresses is delegated to the
> importer driver via an ops table stored in the ctx, see struct
> dma_buf_io_ops. Only the target driver / subsystem knows about devices
> it wants to use the dma-buf with, especially in case of multi-device
> filesystems or stacking in the future.
>
> Signed-off-by: Pavel Begunkov <asml.silence(a)gmail.com>
> ---
Hi,
a few nitpicks below, should it help
> +static void dma_buf_io_kill_maps(struct dma_buf_io_ctx *ctx, bool final)
> +{
> + struct dma_buf_io_map *map;
> +
> + scoped_guard(mutex, &ctx->map_mutex) {
guard() is enough. This saves indentation.
> + if (final)
> + ctx->maps_killed = true;
> +
> + map = rcu_dereference_protected(ctx->map,
> + lockdep_is_held(&ctx->map_mutex));
> + if (!map)
> + return;
> + rcu_assign_pointer(ctx->map, NULL);
> + percpu_ref_kill(&map->refs);
> + }
> +}
...
> +int dma_buf_io_ctx_create(struct file *file,
> + struct dma_buf *dmabuf,
> + enum dma_data_direction dir,
> + struct dma_buf_io_ctx **out_ctx)
> +{
> + struct dma_buf_io_ctx *ctx;
> + int ret;
> +
> + if (!file->f_op->init_dma_buf_io_ctx)
> + return -EOPNOTSUPP;
> +
> + ctx = kmalloc_obj(*ctx);
kzalloc_obj() and save the memset() below
> + if (!ctx)
> + return -ENOMEM;
> +
> + memset(ctx, 0, sizeof(*ctx));
> + ctx->dir = dir;
> + ctx->dmabuf = dmabuf;
> + get_dma_buf(dmabuf);
> + mutex_init(&ctx->map_mutex);
> + mutex_init(&ctx->map_create_mutex);
> + atomic_set(&ctx->active_maps, 0);
> + atomic_set(&ctx->all_maps, 0);
> + refcount_set(&ctx->refs, 1);
> + init_waitqueue_head(&ctx->maps_wq);
> +
> + ret = file->f_op->init_dma_buf_io_ctx(file, ctx);
> + if (ret) {
> + kfree(ctx);
> + dma_buf_put(dmabuf);
> + return ret;
> + }
> +
> + if (WARN_ON_ONCE(!ctx->dev_ops ||
> + !ctx->dev_ops->map ||
> + !ctx->dev_ops->unmap ||
> + !ctx->dev_ops->release))
> + return -EINVAL;
> +
> + *out_ctx = ctx;
> + return 0;
> +}
...
CJ
On 9/26/26 09:12, Karl Mehltretter wrote:
> get_sg_table() merges physically contiguous pages without accounting
> for the mapping device's maximum segment size. This affects both
> importer mappings and the udmabuf misc device mapping used for CPU
> access.
>
> With DMA_API_DEBUG enabled, DMA_BUF_IOCTL_SYNC on a 64 MiB udmabuf
> reports:
>
> DMA-API: misc udmabuf: mapping sg segment longer than device claims to support [len=65884160] [max=65536]
>
> Use sg_alloc_table_from_pages_segment() with the mapping device's
> maximum segment size. Keep a PAGE_SIZE minimum because the allocator
> warns and returns -EINVAL for smaller limits.
>
> Before commit 5bf888673e0d ("udmabuf: Do not create malformed
> scatterlists"), each entry covered one page.
>
> Fixes: 5bf888673e0d ("udmabuf: Do not create malformed scatterlists")
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter(a)gmail.com>
> ---
>
> Notes:
> Tested on v7.3-rc4-70-gfe2ec83746e5 in QEMU (x86_64, TCG) with
> DMA_API_DEBUG (all_errors=1) and DMABUF_DEBUG, A/B against the same
> base:
>
> before after
> DMA_BUF_IOCTL_SYNC, 64 MiB udmabuf 1 report 0
> vivid import, 4 MiB udmabuf 2 reports 0
> vivid import, 2 MiB hugetlb udmabuf 2 reports 0
> frames captured 5/5 5/5
>
> vb2-dma-contig rejected the non-contiguous import in both runs.
>
> drivers/dma-buf/udmabuf.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> index df6dd00462423..09f1eb8432f19 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -139,9 +139,13 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf,
> if (!sg)
> return ERR_PTR(-ENOMEM);
>
> - ret = sg_alloc_table_from_pages(sg, ubuf->pages, ubuf->pagecount, 0,
> - ubuf->pagecount << PAGE_SHIFT,
> - GFP_KERNEL);
> + /* The SG allocator requires a segment limit of at least PAGE_SIZE. */
> + ret = sg_alloc_table_from_pages_segment(sg, ubuf->pages, ubuf->pagecount,
> + 0, ubuf->pagecount << PAGE_SHIFT,
> + max_t(unsigned int,
> + dma_get_max_seg_size(dev),
> + PAGE_SIZE),
Please return -EINVAL instead when dma_get_max_seg_size() returns that the segment size is smaller than a page.
In general I think that the sg_alloc_table_from_pages_segment() approach is because of the broken design of the old DMA API. Stuff like that should be handled by the iterator going over the DMA segments instead. But yeah that is not something you can fix in one patch.
So apart from the error handling the patch looks good to me.
Regards,
Christian.
> + GFP_KERNEL);
> if (ret < 0)
> goto err_alloc;
>
On Tue, Sep 29, 2026 at 08:56:01AM +0200, Karl Mehltretter wrote:
> On Tue, Sep 29, 2026 at 06:00:27AM +0000, sashiko-bot(a)kernel.org wrote:
> > On architectures with a 256KB PAGE_SIZE (such as Hexagon or PowerPC), this
> > new validation evaluates to 65536 < 262144, unconditionally returning
> > -EINVAL and breaking all CPU mappings.
>
> Right, the misc device has no dma_parms, so it gets the 64K default and
> sync fails with 256K pages
I would ignore this, it seems fictional to me, and if someone really
needs to support this rediculous combination it should be done inside
sg_alloc_table_from_pages_segment(:)
> The misc device has no real segment limit, so for v3 I'll give it
> dma_parms with an unlimited max segment size and keep the -EINVAL for
> importers that report a limit below PAGE_SIZE.
The params have to come from the *importing* device, not the udmabuf
misc device. You don't get to control what they are, and there is no
reason to put a dma_parms on a misc deivce.
Jason
On 9/28/26 12:36, Rob Clark wrote:
> On Mon, Sep 28, 2026 at 1:19 AM Christian König
> <christian.koenig(a)amd.com> wrote:
>>
>> On 9/26/26 04:20, Jianfeng Liu wrote:
>>> That commit fixed a dangling reference in the DMABUF_DEBUG default
>>> and thereby 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 broken by the wrapper. Both of msm's map paths consume
>>> sg->length and sg_phys() of the attachment sg_table:
>>> msm_iommu_pagetable_map() for the per-process GPU pagetables, and
>>> iommu_map_sg() (via iommu_map_sgtable()) for scanout. The wrapper
>>> zeroes sg->length and strips the page pointers, so mappings of
>>> imported dma-bufs silently map nothing, and userspace observes
>>> arm-smmu translation faults from UCHE, e.g. during hardware video
>>> decode (clapper, chromium) on Adreno systems:
>>>
>>> gpu fault: ttbr0=000000088a889000 iova=000000010741c000 dir=READ
>>> type=TRANSLATION source=UCHE
>>>
>>> Bisected on a Snapdragon X1E78100 laptop as v7.3-rc3 good,
>>> v7.3-rc4 bad, culprit 143755bdabaa9.
>>>
>>> Switching msm to sg_dma_address()/sg_dma_len() is not a trivial fix
>>> either: those fields are only valid for sg_tables that msm has
>>> dma-mapped itself, which native non-MSM_BO_WC objects' sg_tables
>>> are not, so the conversion needs more work. The msm maintainer has
>>> therefore requested restoring the previous default for v7.3, to be
>>> revisited once msm no longer consumes struct page and sg->length of
>>> imported sg_tables.
>>
>> Yeah as I said before the problematic part is MSM here. We have enforced correct driver behavior for over 5 years now when that option is enabled.
>
> The problem is bigger than MSM here
Well, so far I have only heard about MSM.
But yes I mean the config option is doing exactly what it is supposed to do, pointing out when driver need some work to get this fixed.
I also agree that we shouldn't have allowed driver to touch that stuff in the first place and better document how to do things but yeah I can't change the past I can only try to fix it now.
>> What we can do is to mark MSM as broken and/or give a warning in MSM when DMABUF_DEBUG is enabled and you try to import a DMA-buf.
>
> sorry, no, we can't mark MSM as broken.. we can mark DMABUF_DEBUG as BROKEN
As I wrote the debug functionality to enforce not using struct pages has been around for over 5 years now, it was just not enabled by default.
What I can offer is to set it to default N for another few month to give you more time to fix things.
Regards,
Christian.
>
> BR,
> -R
>
>> But making the check not default to enable on debug kernels is not an option. This check here is exactly to point out broken drivers and you can manually disable it.
>>
>> Regards,
>> Christian.
>>
>>>
>>> Link: https://lore.kernel.org/linux-arm-msm/20260923074256.9357-1-liujianfeng1994…
>>> Suggested-by: Rob Clark <robin.clark(a)oss.qualcomm.com>
>>> Cc: Christian König <christian.koenig(a)amd.com>
>>> Cc: Sumit Semwal <sumit.semwal(a)linaro.org>
>>> Cc: Karl Mehltretter <kmehltretter(a)gmail.com>
>>>
>>> Signed-off-by: Jianfeng Liu <liujianfeng1994(a)gmail.com>
>>> ---
>>>
>>> drivers/dma-buf/Kconfig | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig
>>> index e4f078a326a41..7efc0f0d07126 100644
>>> --- a/drivers/dma-buf/Kconfig
>>> +++ b/drivers/dma-buf/Kconfig
>>> @@ -43,7 +43,7 @@ config UDMABUF
>>> config DMABUF_DEBUG
>>> bool "DMA-BUF debug checks"
>>> depends on DMA_SHARED_BUFFER
>>> - default y if DEBUG_KERNEL
>>> + default y if DEBUG
>>> help
>>> This option enables additional checks for DMA-BUF importers and
>>> exporters. Specifically it validates that importers do not peek at the
>>> ---
>>> base-commit: 93f51579e7df248780214094418f205253383cc5
>>> branch: revert-dmabuf-debug-for-7.3
>>>
>>
On 9/26/26 20:30, Rob Clark wrote:
> This was always the way it was supposed to work, and when we drop the
> page array for imported dma-bufs our fault handling path will no longer
> work.
>
> Signed-off-by: Rob Clark <robin.clark(a)oss.qualcomm.com>
Nice to see that finally happening.
Reviewed-by: Christian König <christian.koenig(a)amd.com> for this patch here, Acked-by: Christian König <christian.koenig(a)amd.com> for the rest of the series.
Regards,
Christian.
> ---
> drivers/gpu/drm/msm/msm_gem.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index e8390ebd5dd5..c90336b3b231 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -23,6 +23,8 @@
> #include "msm_gpu.h"
> #include "msm_kms.h"
>
> +MODULE_IMPORT_NS("DMA_BUF");
> +
> static void update_device_mem(struct msm_drm_private *priv, ssize_t size)
> {
> uint64_t total_mem = atomic64_add_return(size, &priv->total_mem);
> @@ -338,6 +340,9 @@ static vm_fault_t msm_gem_fault(struct vm_fault *vmf)
> int err;
> vm_fault_t ret;
>
> + if (drm_WARN_ON_ONCE(obj->dev, drm_gem_is_imported(obj)))
> + return VM_FAULT_SIGBUS;
> +
> /*
> * vm_ops.open/drm_gem_mmap_obj and close get and put
> * a reference on obj. So, we dont need to hold one here.
> @@ -1126,6 +1131,25 @@ static int msm_gem_object_mmap(struct drm_gem_object *obj, struct vm_area_struct
> {
> struct msm_gem_object *msm_obj = to_msm_bo(obj);
>
> + if (drm_gem_is_imported(obj)) {
> + int ret;
> +
> + /* Reset both vm_ops and vm_private_data, so we don't end up with
> + * vm_ops pointing to our implementation if the dma-buf backend
> + * doesn't set those fields.
> + */
> + vma->vm_private_data = NULL;
> + vma->vm_ops = NULL;
> +
> + ret = dma_buf_mmap(obj->dma_buf, vma, 0);
> +
> + /* Drop the reference drm_gem_mmap_obj() acquired.*/
> + if (!ret)
> + drm_gem_object_put(obj);
> +
> + return ret;
> + }
> +
> vm_flags_set(vma, VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP);
> vma->vm_page_prot = msm_gem_pgprot(msm_obj, vma_get_page_prot(vma));
>
On 9/26/26 04:20, Jianfeng Liu wrote:
> That commit fixed a dangling reference in the DMABUF_DEBUG default
> and thereby 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 broken by the wrapper. Both of msm's map paths consume
> sg->length and sg_phys() of the attachment sg_table:
> msm_iommu_pagetable_map() for the per-process GPU pagetables, and
> iommu_map_sg() (via iommu_map_sgtable()) for scanout. The wrapper
> zeroes sg->length and strips the page pointers, so mappings of
> imported dma-bufs silently map nothing, and userspace observes
> arm-smmu translation faults from UCHE, e.g. during hardware video
> decode (clapper, chromium) on Adreno systems:
>
> gpu fault: ttbr0=000000088a889000 iova=000000010741c000 dir=READ
> type=TRANSLATION source=UCHE
>
> Bisected on a Snapdragon X1E78100 laptop as v7.3-rc3 good,
> v7.3-rc4 bad, culprit 143755bdabaa9.
>
> Switching msm to sg_dma_address()/sg_dma_len() is not a trivial fix
> either: those fields are only valid for sg_tables that msm has
> dma-mapped itself, which native non-MSM_BO_WC objects' sg_tables
> are not, so the conversion needs more work. The msm maintainer has
> therefore requested restoring the previous default for v7.3, to be
> revisited once msm no longer consumes struct page and sg->length of
> imported sg_tables.
Yeah as I said before the problematic part is MSM here. We have enforced correct driver behavior for over 5 years now when that option is enabled.
What we can do is to mark MSM as broken and/or give a warning in MSM when DMABUF_DEBUG is enabled and you try to import a DMA-buf.
But making the check not default to enable on debug kernels is not an option. This check here is exactly to point out broken drivers and you can manually disable it.
Regards,
Christian.
>
> Link: https://lore.kernel.org/linux-arm-msm/20260923074256.9357-1-liujianfeng1994…
> Suggested-by: Rob Clark <robin.clark(a)oss.qualcomm.com>
> Cc: Christian König <christian.koenig(a)amd.com>
> Cc: Sumit Semwal <sumit.semwal(a)linaro.org>
> Cc: Karl Mehltretter <kmehltretter(a)gmail.com>
>
> Signed-off-by: Jianfeng Liu <liujianfeng1994(a)gmail.com>
> ---
>
> drivers/dma-buf/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig
> index e4f078a326a41..7efc0f0d07126 100644
> --- a/drivers/dma-buf/Kconfig
> +++ b/drivers/dma-buf/Kconfig
> @@ -43,7 +43,7 @@ config UDMABUF
> config DMABUF_DEBUG
> bool "DMA-BUF debug checks"
> depends on DMA_SHARED_BUFFER
> - default y if DEBUG_KERNEL
> + default y if DEBUG
> help
> This option enables additional checks for DMA-BUF importers and
> exporters. Specifically it validates that importers do not peek at the
> ---
> base-commit: 93f51579e7df248780214094418f205253383cc5
> branch: revert-dmabuf-debug-for-7.3
>