Re: [PATCH v6 18/18] RDMA/mlx5: Ask P2PDMA whether ATS takes a direct peer-to-peer route
From: Leon Romanovsky
Date: Fri Sep 18 2026 - 08:19:12 EST
On Thu, Sep 17, 2026 at 03:56:12PM +0200, Thomas Hellström wrote:
> Hi
>
> On Mon, 2026-09-14 at 14:22 +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@xxxxxxxxxx>
> >
> > mlx5_umem_needs_ats() enables ATS for any dma-buf whose caller asked
> > for
> > Relaxed Ordering, on the assumption that a switch in the path has CR,
> > RR
> > and DT all set. It also enables it for a buffer already mapped with
> > the
> > peer's bus addresses, which are not translatable at all.
> >
> > P2PDMA has read the ACS controls, so ask it through
> > dma_buf_p2pdma_map_type(): enable ATS only where the path is not
> > routed
> > directly as it stands, but would be for a Translated Request whose
> > Completions carry Relaxed Ordering. Exporters that name no provider
> > keep
> > the old assumption, since their ACS settings remain hidden.
> >
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxx>
> > ---
> > drivers/infiniband/hw/mlx5/mlx5_ib.h | 36 ++------------------------
> > ------
> > drivers/infiniband/hw/mlx5/mr.c | 40
> > ++++++++++++++++++++++++++++++++++++
> > 2 files changed, 42 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h
> > b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> > index e9ddf2e97a76..ab32742b2180 100644
> > --- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
> > +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> > @@ -1646,40 +1646,8 @@ static inline bool rt_supported(int ts_cap)
> > ts_cap ==
> > MLX5_TIMESTAMP_FORMAT_CAP_FREE_RUNNING_AND_REAL_TIME;
> > }
> >
> > -/*
> > - * PCI Peer to Peer is a trainwreck. If no switch is present then
> > things
> > - * sometimes work, depending on the pci_distance_p2p logic for
> > excluding broken
> > - * root complexes. However if a switch is present in the path, then
> > things get
> > - * really ugly depending on how the switch is setup. This table
> > assumes that the
> > - * root complex is strict and is validating that all req/reps are
> > matches
> > - * perfectly - so any scenario where it sees only half the
> > transaction is a
> > - * failure.
> > - *
> > - * CR/RR/DT ATS RO P2P
> > - * 00X X X OK
> > - * 010 X X fails (request is routed to root but root never
> > sees comp)
> > - * 011 0 X fails (request is routed to root but root never
> > sees comp)
> > - * 011 1 X OK
> > - * 10X X 1 OK
> > - * 101 X 0 fails (completion is routed to root but root
> > didn't see req)
> > - * 110 X 0 SLOW
> > - * 111 0 0 SLOW
> > - * 111 1 0 fails (completion is routed to root but root
> > didn't see req)
> > - * 111 1 1 OK
> > - *
> > - * Unfortunately we cannot reliably know if a switch is present or
> > what the
> > - * CR/RR/DT ACS settings are, as in a VM that is all hidden. Assume
> > that
> > - * CR/RR/DT is 111 if the ATS cap is enabled and follow the last
> > three rows.
> > - *
> > - * For now assume if the umem is a dma_buf then it is P2P.
> > - */
> > -static inline bool mlx5_umem_needs_ats(struct mlx5_ib_dev *dev,
> > - struct ib_umem *umem, int
> > access_flags)
> > -{
> > - if (!MLX5_CAP_GEN(dev->mdev, ats) || !umem->is_dmabuf)
> > - return false;
> > - return access_flags & IB_ACCESS_RELAXED_ORDERING;
> > -}
> > +bool mlx5_umem_needs_ats(struct mlx5_ib_dev *dev, struct ib_umem
> > *umem,
> > + int access_flags);
> >
> > int set_roce_addr(struct mlx5_ib_dev *dev, u32 port_num,
> > unsigned int index, const union ib_gid *gid,
> > diff --git a/drivers/infiniband/hw/mlx5/mr.c
> > b/drivers/infiniband/hw/mlx5/mr.c
> > index 00e13028762a..286f372e5b0c 100644
> > --- a/drivers/infiniband/hw/mlx5/mr.c
> > +++ b/drivers/infiniband/hw/mlx5/mr.c
> > @@ -38,6 +38,7 @@
> > #include <linux/export.h>
> > #include <linux/delay.h>
> > #include <linux/dma-buf.h>
> > +#include <linux/dma-buf-mapping.h>
> > #include <linux/dma-resv.h>
> > #include <rdma/frmr_pools.h>
> > #include <rdma/ib_umem_odp.h>
> > @@ -47,6 +48,45 @@
> > #include "data_direct.h"
> > #include "dmah.h"
> >
> > +MODULE_IMPORT_NS("DMA_BUF");
> > +
> > +bool mlx5_umem_needs_ats(struct mlx5_ib_dev *dev, struct ib_umem
> > *umem,
> > + int access_flags)
> > +{
> > + struct dma_buf_attachment *attach;
> > +
> > + if (!MLX5_CAP_GEN(dev->mdev, ats) || !umem->is_dmabuf)
> > + return false;
> > +
> > + /*
> > + * The Completer decides whether its Completions carry
> > Relaxed
> > + * Ordering, and only a Request that asked for it can expect
> > them to.
> > + */
> > + if (!(access_flags & IB_ACCESS_RELAXED_ORDERING))
> > + return false;
> > +
> > + attach = to_ib_umem_dmabuf(umem)->attach;
> > + switch (dma_buf_p2pdma_map_type(attach, 0)) {
> > + case PCI_P2PDMA_MAP_NONE:
> > + /* Nothing is known about the route, so fall back to
> > the bet. */
> > + return true;
> > + case PCI_P2PDMA_MAP_BUS_ADDR:
> > + /*
> > + * The path is routed directly already and is
> > programmed with
> > + * the peer's bus addresses. Those are not
> > translatable, so
> > + * ATS would be wrong as well as pointless.
> > + */
> > + return false;
> > + default:
> > + break;
> > + }
> > +
> > + return dma_buf_p2pdma_map_type(attach,
> > + PCI_P2PDMA_TLP_TRANSLATED |
> > +
> > PCI_P2PDMA_TLP_RELAXED_CPL) ==
> > + PCI_P2PDMA_MAP_BUS_ADDR;
> > +}
> > +
>
> It looks like this works well when the device can choose whether to
> enable ATS per transaction.
>
> However, at least for the Intel GPUs, ATS enablement is based on the
> PCIe-side enable bit.
>
> This means that if p2pdma tells the dma-mapping layer to give an Xe
> device a bus address rather than an IOVA, things break, while
> pci_p2pdma_distance says everything is OK.
>
> It looks like the infrastructure and solution added in this series is
> targeted at fixing this on the device side by conditionally enabling
> ATS. However I think we need to look also at having the computed
> routing assume untranslated transactions using IOVA rather than bus
> address.
>
> That is, a flag to tell the topology check that some transactions
> *will* take the host-bridge path due to IOVA being used, and that the
> computations including pci_p2pdma_distance() need to check whether that
> is possible (checking whitelist etc.) and return the corresponding
> THRU_HOST_BRIDGE mapping type. Translated transactions taking a short-
> cut using the bus-address would then be hidden from the driver.
The word *will* puzzles me. As I understand it, if your device has ATS
enabled all the time, it should always get THRU_HOST_BRIDGE.
In the mlx5 case, we can enable or disable ATS on the fly, but what does a
device with ATS enabled globally do for different mapping types? Do you
simply not use dma_buf_p2pdma_map_type() at all and continue to operate
as is?
I asked some questions my AI tool about XE:
------------------------------------------------------------------------------------------------
Xe needs THRU_HOST_BRIDGE even with ATS completely disabled. xe_ttm_vram_mgr_alloc_sgt()
maps VRAM with dma_map_resource() unconditionally (xe_ttm_vram_mgr.c:450) — an IOVA under
an IOMMU, never a bus address, with no ATS involvement at all. ATS only adds the possibility
that some of that traffic short-cuts invisibly. The actual requirement is “this caller cannot
program bus addresses”, which is true of Xe regardless of the ATS Enable bit.
-------------------------------------------------------------------------------------------------
Could you please clarify what the expected behavior is here? My series
does not change the existing behavior; it only reports the routing more
clearly for each TLP class.
Thanks
>
> Whether that is best done as a parameter to these functions or perhaps
> as a flag in the PCI device, I'm not sure.
>
> Thanks,
> Thomas
>
>
>
>
> > static int mkey_max_umr_order(struct mlx5_ib_dev *dev)
> > {
> > if (MLX5_CAP_GEN(dev->mdev,
> > umr_extended_translation_offset))