Re: [PATCH v11 3/4] vfio/pci: implement get_pci_tph and DMA_BUF_TPH feature

Re: [PATCH v11 3/4] vfio/pci: implement get_pci_tph and DMA_BUF_TPH feature

[fengchengwen]

On 7/3/2026 2:10 AM, Zhiping Zhang wrote:
> Implement dma-buf get_pci_tph for vfio-pci exported dma-bufs and add
> VFIO_DEVICE_FEATURE_DMA_BUF_TPH so userspace can publish TPH metadata
> for a VFIO-owned device.
> 
> 8-bit ST and 16-bit Extended ST are distinct PCIe TPH namespaces; the
> uAPI carries both with explicit validity flags, and get_pci_tph()
> returns the value matching the importer's requested namespace or
> -EOPNOTSUPP.
> 
> Publish and read the TPH descriptor under dmabuf->resv, matching the
> locking used for other importer-visible dma-buf state. The SET ioctl
> takes dma_resv_lock_interruptible(), while the callback runs under
> DMA-buf's asserted resv lock.
> 
> The attach path reads @revoked without holding memory_lock. Annotate it
> with READ_ONCE() to document this intentional lockless access: the read
> is a benign early-out, and a racing revocation is re-checked under
> dmabuf->resv in vfio_pci_dma_buf_map() before any mapping is handed out.

I believe this modification (@revoked) might be related to resetting the
bit field segment and subsequently identifying a concurrency issue. It is
recommended to submit this as an independent commit.

> The annotation only needs to keep the access well-formed against the
> memory_lock-protected writers.
> 
> Reject requests the device cannot consume as a completer:
> pcie_tph_completer_type() must report at least
> PCI_EXP_DEVCAP2_TPH_COMP_TPH_ONLY, and Extended ST requires
> PCI_EXP_DEVCAP2_TPH_COMP_EXT_TPH. Make PROBE follow the same hardware
> gate so the feature only probes as supported when the device can really
> consume it.
> 
> Signed-off-by: Zhiping Zhang <zhipingz@meta.com>

...

>  
>  #include "vfio_pci_priv.h"
> @@ -19,7 +20,17 @@ struct vfio_pci_dma_buf {
>  	u32 nr_ranges;
>  	struct kref kref;
>  	struct completion comp;
> -	u8 revoked : 1;
> +
> +	/*
> +	 * Updates protected by dmabuf->resv, @revoked additionally
> +	 * protected by memory_lock.
> +	 */
> +	u16 tph_st_ext;
> +	u8 tph_st;

how about: u16 tph_xst;
	   u16 tph_st;

> +	bool revoked;

why not move revoked before or after tph fields if it don't take one bit field?

> +	u8 tph_st_valid:1;
> +	u8 tph_st_ext_valid:1;

how about: u8 tph_xst_valid

> +	u8 tph_ph:2;
>  };

...

> +#define VFIO_DEVICE_FEATURE_DMA_BUF_TPH 13
> +
> +#define VFIO_DMA_BUF_TPH_ST		(1 << 0)
> +#define VFIO_DMA_BUF_TPH_ST_EXT		(1 << 1)
> +
> +struct vfio_device_feature_dma_buf_tph {
> +	__s32	dmabuf_fd;
> +	__u32	flags;
> +	__u16	steering_tag_ext;
> +	__u8	steering_tag;

how about:
	__u16 xst;
	__u8 st;
and it corresponding to VFIO_DMA_BUF_TPH_ST/ST_EXT

Thanks

> +	__u8	ph;
> +};
> +
>  /* -------- API for Type1 VFIO IOMMU -------- */
>  
>  /**

Re: [PATCH v11 3/4] vfio/pci: implement get_pci_tph and DMA_BUF_TPH feature

[Zhiping Zhang]

...
> > The attach path reads @revoked without holding memory_lock. Annotate it
> > with READ_ONCE() to document this intentional lockless access: the read
> > is a benign early-out, and a racing revocation is re-checked under
> > dmabuf->resv in vfio_pci_dma_buf_map() before any mapping is handed out.
>
> I believe this modification (@revoked) might be related to resetting the
> bit field segment and subsequently identifying a concurrency issue. It is
> recommended to submit this as an independent commit.
>

Thanks, I think the @revoked part is still closely related here. The lockless
attach-path read existed before this patch; this only annotates it
with READ_ONCE().
The bitfield-to-bool change is needed because READ_ONCE() cannot be used on a
bitfield. The real check remains the dmabuf->resv recheck in
vfio_pci_dma_buf_map(),
so I'd prefer to keep it in this patch.

...
> > +
> > +     /*
> > +      * Updates protected by dmabuf->resv, @revoked additionally
> > +      * protected by memory_lock.
> > +      */
> > +     u16 tph_st_ext;
> > +     u8 tph_st;
>
> how about: u16 tph_xst;
>            u16 tph_st;
>

I'd prefer to keep tph_st/tph_st_ext if that works for you. They map directly to
ST and Extended ST, and non-Extended ST is 8-bit, so u8 tph_st reflects the
actual width.

> > +     bool revoked;
>
> why not move revoked before or after tph fields if it don't take one bit field?
>

I kept @revoked there because the comment documents the locking for these
dma-buf state fields: TPH under dmabuf->resv, and @revoked under dmabuf->resv
plus memory_lock.

> > +     u8 tph_st_valid:1;
> > +     u8 tph_st_ext_valid:1;
>
> how about: u8 tph_xst_valid
>

I'd prefer tph_st_ext_valid since it matches VFIO_DMA_BUF_TPH_ST_EXT and the
"Extended ST" wording used in the documentation.

> > +     u8 tph_ph:2;
> >  };
>
> ...
>
> > +#define VFIO_DEVICE_FEATURE_DMA_BUF_TPH 13
> > +
> > +#define VFIO_DMA_BUF_TPH_ST          (1 << 0)
> > +#define VFIO_DMA_BUF_TPH_ST_EXT              (1 << 1)
> > +
> > +struct vfio_device_feature_dma_buf_tph {
> > +     __s32   dmabuf_fd;
> > +     __u32   flags;
> > +     __u16   steering_tag_ext;
> > +     __u8    steering_tag;
>
> how about:
>         __u16 xst;
>         __u8 st;
> and it corresponding to VFIO_DMA_BUF_TPH_ST/ST_EXT
>
> Thanks
>

If that's OK, I'd prefer steering_tag/steering_tag_ext in the uAPI. They
match the PCIe "Steering Tag" term and seem clearer for userspace than
st/xst.

Thanks a lot!
Zhiping

Re: [PATCH v11 4/4] RDMA/mlx5: get tph for p2p access when registering dma-buf mr

[Michael Gur]

On 7/2/2026 9:10 PM, Zhiping Zhang wrote:
> External email: Use caution opening links or attachments
>
>
> Peer-to-peer DMA between a mlx5 NIC and a foreign PCIe endpoint
> (typically a GPU or a vfio-pci passthrough device) traverses the host
> PCIe fabric. The endpoint exporting the dma-buf knows which PCIe TLP
> Processing Hint (TPH) Steering Tag yields the best placement for the
> traffic it will sink: per-endpoint hint selection lets the root complex
> or switch direct DMA to a specific cache slice / NUMA node, cutting
> cross-socket snoop traffic and DRAM pressure under sustained p2p
> workloads.
>
> Until now the mlx5 importer had no way to learn the exporter's chosen
> ST tag, so dma-buf MRs were registered without TPH and ran with the
> default (no-hint) routing. With dma_buf_get_pci_tph() in place this
> patch wires up mlx5_ib to query that metadata at MR registration time
> for p2p access and use it to program requester-side TPH on the outbound
> mkey. If the exporter has no metadata, fall back to the existing
> no-TPH path so behavior for non-TPH-aware exporters is unchanged.
>
> Use mlx5_st_alloc_index_by_tag() to translate exporter-provided
> steering tags into local ST entries when table mode is active, and add
> mlx5_st_get_index() for DMAH-backed flows that already carry an ST
> index.
>
> For TPH-backed FRMRs, keep the extra ST-table reference tied to MR
> lifetime rather than pooled mkey lifetime. Acquire the ref before MR
> creation and release it again when the MR is returned to the pool or
> the backing mkey is destroyed, while leaving the generic FRMR pool
> core unchanged.
>
> Import the DMA_BUF namespace for the new dma_buf_get_pci_tph() call so
> modular mlx5_ib builds link cleanly.
>
> Signed-off-by: Zhiping Zhang <zhipingz@meta.com>
> ---
>   drivers/infiniband/hw/mlx5/main.c             |   1 +
>   drivers/infiniband/hw/mlx5/mr.c               | 116 +++++++++++++++++-
>   .../net/ethernet/mellanox/mlx5/core/lib/st.c  |  49 ++++++--
>   include/linux/mlx5/driver.h                   |  15 +++
>   4 files changed, 167 insertions(+), 14 deletions(-)
Reviewed-by: Michael Gur <michaelgur@nvidia.com>

Thanks.

Re: [PATCH v11 0/4] vfio/dma-buf: add TPH support for peer-to-peer access

[Alex Williamson]

On Thu, 2 Jul 2026 11:10:13 -0700
Zhiping Zhang <zhipingz@meta.com> wrote:
 
> Changes since v10:
>   Patch 2 (dma-buf): Per Christian König, document that the ST/PH
>   returned by dma_buf_get_pci_tph() is only valid until the exporter
>   invalidates the current mapping and must be re-queried afterwards;
>   note added to the wrapper kernel-doc and referenced from the callback
>   kernel-doc. Also add dma_buf_get_pci_tph() and dma_buf_ops.get_pci_tph()
>   to the central dma-buf locking convention.
> 
>   Patch 3 (vfio/pci): Per Alex Williamson, update the vfio_pci_dma_buf
>   comment to note that @revoked is additionally protected by memory_lock,
>   and describe the READ_ONCE() rationale in the commit log. No behavior
>   change.

Sashiko has valid comments[1] across most of the series.

 - Passing through 0b10 seems mis-categorized as High in patch 1, but
   is valid hardening if tph_req_type can ever hold an invalid value.

 - The documentation error in patch 2 is real.

 - Patch 4 ironically fails to re-validate according to the lifecycle
   requirements that patch 2 specifies.  This is a significant gap in
   the implementation proof for a real requester.

 - The broadened scope of the existing memory leak in patch 4 is
   already addressed in [2], ok.  Maybe should be folded into this
   series if mlx5 isn't going to pick it up separately.

Thanks,
Alex

[1]https://sashiko.dev/#/patchset/20260702181025.2694961-1-zhipingz@meta.com
[2]https://lore.kernel.org/linux-rdma/20260612170406.3339093-1-zhipingz@meta.com

Re: [PATCH v11 0/4] vfio/dma-buf: add TPH support for peer-to-peer access

[Zhiping Zhang]

On Thu, Jul 9, 2026 at 12:26 PM Alex Williamson <alex@shazbot.org> wrote:
>
> >
> On Thu, 2 Jul 2026 11:10:13 -0700
> Zhiping Zhang <zhipingz@meta.com> wrote:
>
> > Changes since v10:
> >   Patch 2 (dma-buf): Per Christian König, document that the ST/PH
> >   returned by dma_buf_get_pci_tph() is only valid until the exporter
> >   invalidates the current mapping and must be re-queried afterwards;
> >   note added to the wrapper kernel-doc and referenced from the callback
> >   kernel-doc. Also add dma_buf_get_pci_tph() and dma_buf_ops.get_pci_tph()
> >   to the central dma-buf locking convention.
> >
> >   Patch 3 (vfio/pci): Per Alex Williamson, update the vfio_pci_dma_buf
> >   comment to note that @revoked is additionally protected by memory_lock,
> >   and describe the READ_ONCE() rationale in the commit log. No behavior
> >   change.
>
> Sashiko has valid comments[1] across most of the series.
>
>  - Passing through 0b10 seems mis-categorized as High in patch 1, but
>    is valid hardening if tph_req_type can ever hold an invalid value.
>

agreed, not High and pre-existing: get_rp_completer_type() is from the
original TPH
support and untouched here. I can send the hardening change in a separate patch.

>  - The documentation error in patch 2 is real.
>

will fix!

>  - Patch 4 ironically fails to re-validate according to the lifecycle
>    requirements that patch 2 specifies.  This is a significant gap in
>    the implementation proof for a real requester.
>

Got it, I'll re-query ma_buf_get_pci_tph() there and reprogram the
mkey's steering tag,
so the lifecycle is honored.

>  - The broadened scope of the existing memory leak in patch 4 is
>    already addressed in [2], ok.  Maybe should be folded into this
>    series if mlx5 isn't going to pick it up separately.
>

 [2] is already accepted and landing through the mlx5/RDMA tree.

I plan to keep it as a standalone dependency.

> Thanks,
> Alex
>
> [1]https://urldefense.com/v3/__https://sashiko.dev/*/patchset/20260702181025.2694961-1-zhipingz@meta.com__;Iw!!Bt8RZUm9aw!5x8TPHfbpTXQJ872nmZaHsPIXH7HsL9ICbZR3G37yKkHgB-RukCDOy3XiLIOhfhP-yTczTyWQmud$
> [2]https://lore.kernel.org/linux-rdma/20260612170406.3339093-1-zhipingz@meta.com

Thanks,
Zhiping