Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
* Re: [PATCH v12 0/4] vfio/dma-buf: add TPH support for peer-to-peer access
       [not found] <20260715204008.3911275-1-zhipingz@meta.com>
@ 2026-07-16  1:07 ` fengchengwen
  2026-07-16 14:47   ` Jason Gunthorpe
  2026-07-16 20:08   ` Zhiping Zhang
       [not found] ` <20260715204008.3911275-4-zhipingz@meta.com>
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 9+ messages in thread
From: fengchengwen @ 2026-07-16  1:07 UTC (permalink / raw)
  To: Zhiping Zhang, Jason Gunthorpe, Leon Romanovsky, Michael Guralnik,
	Sumit Semwal, Christian Konig, Alex Williamson, Bjorn Helgaas
  Cc: kvm, linux-rdma, linux-pci, dri-devel

Hi Zhiping,

One concern I'd like to raise is the implicit topology constraint of the
TPH P2P path. TPH Steering Tag only works reliably when both importer and
exporter devices sit under the same Root Port (or same TPH-aware switch).

For cross-Root-Port P2P traffic (e.g. P2P devices located in different
RC or even socket), most server-grade Root Complexes do not forward TPH
prefixes through their internal routing. The ST field gets silently
stripped before reaching the target device, so all TPH setup becomes
ineffective with no error reported to user.

Because cross-Root-Port P2P traffic with ST does not affect the host, but
merely the completer may experience some performance degradation because
it lacks the TPH hint. In this case, I suggest adding restrictions in the
uAPI.

A further step is to check whether the importer and exporter are under
the same root port. However, this verification will cause the P2P TPH
to be unsupported in the case of multi-host or multi-root PCIe switch.

Thanks

On 7/16/2026 4:39 AM, Zhiping Zhang wrote:
> This series adds TLP Processing Hints (TPH) support to the VFIO dma-buf
> export path, allowing importing drivers (e.g. mlx5) to use the
> exporter's steering tag when performing peer-to-peer DMA into a
> VFIO-owned device.
> 
> There is no separate in-tree vendor kernel driver for the target device:
> vfio-pci is the in-tree driver and the targeted device is managed
> from userspace via VFIO passthrough. That is why the ST has to flow
> through a uAPI: userspace owns the device and its ST table, so it is the
> entity that can configure a meaningful value for a given dma-buf. The
> kernel-visible participants are still in-tree: vfio-pci exports the
> dma-buf and mlx5 imports it.
> 
> On the effect: the endpoint's PCIe ingress block uses the ST as
> an in-band instruction for the incoming P2P TLP -- selecting a target
> cache partition and, on writes, an in-flight operation on the data
> before it lands. The dma-buf callback keeps this opaque to the
> framework -- only the producer (userspace owner of the VFIO device)
> and the consumer (endpoint block) need to interpret the value. The
> dma-buf get_pci_tph callback itself is optional, but workloads that
> depend on the endpoint's in-flight operation need it because fallback
> does not produce the same result.
> 
> The dma-buf hook is intentionally generic and discoverable rather than
> a private side channel. The exporter owns the completing address
> space for the dma-buf and decides whether it can provide a meaningful
> ST/PH tuple for that completer; the dma-buf core keeps the tuple opaque,
> and importers merely request the namespace they support and place the
> returned value on generated TLPs. Exporters that cannot derive a
> meaningful tuple simply return -EOPNOTSUPP.
> 
> Patch 1 adds small PCI/TPH type helpers so drivers can query the enabled
> TPH requester mode and the device's TPH Completer Supported field
> without reaching into pci_dev internals (and so callers in
> CONFIG_PCIE_TPH=n builds get a clean fallback). pcie_tph_completer_type()
> reuses the reserved-encoding fold introduced by the separately-submitted
> folding patch rather than duplicating the completer decode.
> Patch 2 adds the optional dma_buf_ops::get_pci_tph callback plus the
> dma_buf_get_pci_tph() importer wrapper so importers can fetch TPH
> metadata from an exporter under dmabuf->resv.
> Patch 3 implements get_pci_tph in vfio-pci and adds the new uAPI
> (VFIO_DEVICE_FEATURE_DMA_BUF_TPH) for userspace to attach the metadata.
> Patch 4 wires up the mlx5 RDMA driver as a consumer. It also enforces the
> dma_buf_get_pci_tph() steering-tag lifetime: the tag is only valid for the
> mapping it was queried against, and the mkey's TPH fields cannot be
> reprogrammed in place. mlx5 therefore records the registration-time tuple
> and re-queries after each dma-buf mapping is established under dmabuf->resv;
> unchanged tuples continue with the existing mkey, while changed or missing
> tuples fail the remap rather than continue with a stale hint. For vfio-pci
> BAR dma-bufs this is expected to be a no-op because invalidation is
> revoke/quiesce, not movement to a new backing placement, and the
> userspace-provided tuple is not changed by the revoke/un-revoke path.
> 
> Build-tested with both CONFIG_PCIE_TPH=y and CONFIG_PCIE_TPH=n.
> Functional validation on the target topology: PCIe analyzer captures
> on the P2P TLPs confirm the ST emitted by mlx5 matches the value
> configured through VFIO_DEVICE_FEATURE_DMA_BUF_TPH, and the end-to-end
> P2P workload only produces results consistent with the endpoint's
> ST-selected in-flight operation. For example, with userspace
> configuring 8-bit ST=0xf0 and PH=2, an analyzer capture of a peer-to-
> peer MWr64 shows "STP MWr64 TC=0 OHC=2 ..." followed by "OHC-B
> ST=F0h PH=2 HV=1":
> (TLP Captures)
> 08000260 -> STP MWr64 TC=0 OHC=2 TS=0 Attr=0 L=8
> F0000004 -> RID=4h:0h.0h EP- Tag=F0h
> E0200000 -> AddrH=000020E0h
> 00080006 -> AddrL=06000800h
> 90F00000 -> OHC-B ST=F0h PH=2 HV=1 AMA=0 AV-
> 
> The dma-buf get_pci_tph interface has also been exercised by a second,
> independent importer: a different vendor's NIC whose driver is not yet
> upstream, locally taught to call dma_buf_get_pci_tph(). A PCIe analyzer
> confirmed the ST it placed on outbound P2P TLPs matches the value
> configured through VFIO_DEVICE_FEATURE_DMA_BUF_TPH, the same result as
> with mlx5. Two unrelated importer drivers exercising the callback
> end-to-end shows the interface is not tied to a single consumer. That
> importer change is out-of-tree and not part of this series. For that
> second importer, with userspace configuring 8-bit ST=0xe0 and PH=0,
> an analyzer capture shows:
> (TLP Captures)
> 08200260 -> STP MWr64 TC=0 OHC=2 TS=1 Attr=0 L=8
> 4E00004C -> RID=4Ch:0h.0h EP- Tag=4Eh
> 00170000 -> AddrH=00001700h
> 00200006 -> AddrL=06002000h
> 10E00000 -> OHC-B ST=E0h PH=0 HV=1 AMA=0 AV-
> 
> Depends on (submitted separately):
>   net/mlx5: free mlx5_st_idx_data on final dealloc
>     https://lore.kernel.org/linux-rdma/20260612170406.3339093-1-zhipingz@meta.com
>   PCI/TPH: fold reserved completer encoding in get_rp_completer_type()
>     https://lore.kernel.org/linux-pci/20260715202409.3767494-1-zhipingz@meta.com
> 
> Changes since v11:
>   Patch 1 (PCI/TPH): add pcie_tph_completer_type() and
>   pcie_tph_enabled_req_type(); pcie_tph_completer_type() reuses the
>   reserved-encoding fold from the separately-submitted folding patch
>   rather than duplicating the completer decode.
> 
>   Patch 2 (dma-buf): fix the kernel-doc to reference
>   &dma_buf_attach_ops.invalidate_mappings instead of the non-existent
>   move_notify callback (reported by Sashiko and Alex Williamson).
> 
>   Patch 3 (vfio/pci): hold memory_lock while validating dma-buf ownership
>   and publishing TPH metadata, so cleanup cannot disassociate the dma-buf
>   from the VFIO device under the SET path. Also use WRITE_ONCE() for
>   revoked-state writers to pair with the lockless attach-path READ_ONCE().
> 
>   Patch 4 (mlx5): enforce the steering-tag lifetime documented in patch 2
>   (addresses the lifecycle gap Sashiko and Alex noted on v11). mlx5 now
>   stores the registration-time TPH tuple and re-queries the exporter after
>   each dma-buf mapping is established under dmabuf->resv. If the tuple is
>   unchanged, the existing mkey continues to be used. If it changed or
>   disappeared, mlx5 unmaps the pages and returns -EFAULT because the
>   existing mkey cannot be updated in place (no UMR update mask for TPH
>   fields, and rebuilding the mkey would change the rkey/lkey). The exporter
>   query runs under dmabuf->resv, but the ST-index allocation is done
>   outside it so no GFP_KERNEL allocation is held under dmabuf->resv. For
>   vfio-pci BAR dma-bufs this validation is expected to be a no-op because
>   the tuple is stable across revoke/un-revoke.
> 
> Previous link:
> v11: https://lore.kernel.org/linux-pci/20260702181025.2694961-1-zhipingz@meta.com/
> v10: https://lore.kernel.org/linux-pci/20260630224328.3218796-1-zhipingz@meta.com/
> v9: https://lore.kernel.org/dri-devel/20260622184211.2229399-1-zhipingz@meta.com/
> v8: https://lore.kernel.org/dri-devel/20260615065912.2177918-1-zhipingz@meta.com/
> v7: https://lore.kernel.org/dri-devel/20260611161546.4075580-1-zhipingz@meta.com/
> v6: https://lore.kernel.org/dri-devel/20260608185646.4085127-1-zhipingz@meta.com/
> v5: https://lore.kernel.org/dri-devel/20260526144401.1485788-1-zhipingz@meta.com/
> v4: https://lore.kernel.org/linux-pci/20260519201401.1558410-1-zhipingz@meta.com/
> v3: https://lore.kernel.org/linux-pci/20260512184755.4137227-1-zhipingz@meta.com/
> v2: https://lore.kernel.org/linux-pci/20260430200704.352228-1-zhipingz@meta.com/
> 
> Zhiping Zhang (4):
>   PCI/TPH: Add requester/completer type helpers
>   dma-buf: add optional get_pci_tph() callback
>   vfio/pci: implement get_pci_tph and DMA_BUF_TPH feature
>   RDMA/mlx5: get tph for p2p access when registering dma-buf mr
> 
>  drivers/dma-buf/dma-buf.c                     |  32 ++++
>  drivers/infiniband/hw/mlx5/main.c             |   1 +
>  drivers/infiniband/hw/mlx5/mlx5_ib.h          |  11 ++
>  drivers/infiniband/hw/mlx5/mr.c               | 151 +++++++++++++++++--
>  drivers/infiniband/hw/mlx5/odp.c              |   7 +
>  .../net/ethernet/mellanox/mlx5/core/lib/st.c  |  49 +++++-
>  drivers/pci/tph.c                             |  39 ++++++
>  drivers/vfio/pci/vfio_pci_core.c              |   3 +
>  drivers/vfio/pci/vfio_pci_dmabuf.c            | 117 ++++++++++++-
>  drivers/vfio/pci/vfio_pci_priv.h              |  13 ++
>  include/linux/dma-buf.h                       |  25 +++
>  include/linux/mlx5/driver.h                   |  15 ++
>  include/linux/pci-tph.h                       |   8 +
>  include/uapi/linux/vfio.h                     |  43 +++++
>  14 files changed, 496 insertions(+), 18 deletions(-)
> 
> --
> 2.53.0-Meta
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v12 0/4] vfio/dma-buf: add TPH support for peer-to-peer access
  2026-07-16  1:07 ` [PATCH v12 0/4] vfio/dma-buf: add TPH support for peer-to-peer access fengchengwen
@ 2026-07-16 14:47   ` Jason Gunthorpe
  2026-07-16 20:10     ` Zhiping Zhang
  2026-07-16 20:08   ` Zhiping Zhang
  1 sibling, 1 reply; 9+ messages in thread
From: Jason Gunthorpe @ 2026-07-16 14:47 UTC (permalink / raw)
  To: fengchengwen
  Cc: Zhiping Zhang, Leon Romanovsky, Michael Guralnik, Sumit Semwal,
	Christian Konig, Alex Williamson, Bjorn Helgaas, kvm, linux-rdma,
	linux-pci, dri-devel

On Thu, Jul 16, 2026 at 09:07:35AM +0800, fengchengwen wrote:
> Because cross-Root-Port P2P traffic with ST does not affect the host, but
> merely the completer may experience some performance degradation because
> it lacks the TPH hint. In this case, I suggest adding restrictions in the
> uAPI.

I don't think this is complexity is worth doing for something that has
no functional impact.

There is no difference between supplying the TPH and not supplying it,
so may as well supply it.

Jason

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v12 0/4] vfio/dma-buf: add TPH support for peer-to-peer access
  2026-07-16  1:07 ` [PATCH v12 0/4] vfio/dma-buf: add TPH support for peer-to-peer access fengchengwen
  2026-07-16 14:47   ` Jason Gunthorpe
@ 2026-07-16 20:08   ` Zhiping Zhang
  2026-07-17  0:16     ` fengchengwen
  1 sibling, 1 reply; 9+ messages in thread
From: Zhiping Zhang @ 2026-07-16 20:08 UTC (permalink / raw)
  To: fengchengwen
  Cc: Jason Gunthorpe, Leon Romanovsky, Michael Guralnik, Sumit Semwal,
	Christian Konig, Alex Williamson, Bjorn Helgaas, kvm, linux-rdma,
	linux-pci, dri-devel

On Wed, Jul 15, 2026 at 6:07 PM fengchengwen <fengchengwen@huawei.com> wrote:
>
> >
> Hi Zhiping,
>
> One concern I'd like to raise is the implicit topology constraint of the
> TPH P2P path. TPH Steering Tag only works reliably when both importer and
> exporter devices sit under the same Root Port (or same TPH-aware switch).
>
> For cross-Root-Port P2P traffic (e.g. P2P devices located in different
> RC or even socket), most server-grade Root Complexes do not forward TPH
> prefixes through their internal routing. The ST field gets silently
> stripped before reaching the target device, so all TPH setup becomes
> ineffective with no error reported to user.
>
> Because cross-Root-Port P2P traffic with ST does not affect the host, but
> merely the completer may experience some performance degradation because
> it lacks the TPH hint. In this case, I suggest adding restrictions in the
> uAPI.
>
> A further step is to check whether the importer and exporter are under
> the same root port. However, this verification will cause the P2P TPH
> to be unsupported in the case of multi-host or multi-root PCIe switch.
>
> Thanks

Hi Fengcheng,

Agreed the RC may not preserve TPH across Root Ports (PCIe Base Spec
r6.4 §6.17.4 requires all intermediate routing elements to support
TPH, and cross-RP forwarding is optional/implementation-dependent, cf.
§2.2.10.4.2). But the uAPI can't gate it: there's no discoverable "TPH
routing" capability -- DevCap2[13:12] (r6.4 §7.5.3.15) describes the
completer, not P2P forwarding -- and a same-Root-Port check would
wrongly reject switch and multi-root topologies where ST does forward.

And it's harmless anyway: per r6.4 §2.2.7.1 an element without TPH
support ignores the TH bit and completes as if unhinted, so a stripped
ST just degrades to the non-TPH baseline.

Thanks,
Zhiping

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v12 0/4] vfio/dma-buf: add TPH support for peer-to-peer access
  2026-07-16 14:47   ` Jason Gunthorpe
@ 2026-07-16 20:10     ` Zhiping Zhang
  0 siblings, 0 replies; 9+ messages in thread
From: Zhiping Zhang @ 2026-07-16 20:10 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: fengchengwen, Leon Romanovsky, Michael Guralnik, Sumit Semwal,
	Christian Konig, Alex Williamson, Bjorn Helgaas, kvm, linux-rdma,
	linux-pci, dri-devel

On Thu, Jul 16, 2026 at 7:47 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> >
> On Thu, Jul 16, 2026 at 09:07:35AM +0800, fengchengwen wrote:
> > Because cross-Root-Port P2P traffic with ST does not affect the host, but
> > merely the completer may experience some performance degradation because
> > it lacks the TPH hint. In this case, I suggest adding restrictions in the
> > uAPI.
>
> I don't think this is complexity is worth doing for something that has
> no functional impact.
>
> There is no difference between supplying the TPH and not supplying it,
> so may as well supply it.
>
> Jason

Agreed, thanks -- that's the intended model. The ST is an advisory
hint; when it can't reach the completer the traffic just behaves as
the non-TPH baseline, so there's nothing to gate in the uAPI.

Thanks,
Zhiping

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v12 0/4] vfio/dma-buf: add TPH support for peer-to-peer access
  2026-07-16 20:08   ` Zhiping Zhang
@ 2026-07-17  0:16     ` fengchengwen
  0 siblings, 0 replies; 9+ messages in thread
From: fengchengwen @ 2026-07-17  0:16 UTC (permalink / raw)
  To: Zhiping Zhang
  Cc: Jason Gunthorpe, Leon Romanovsky, Michael Guralnik, Sumit Semwal,
	Christian Konig, Alex Williamson, Bjorn Helgaas, kvm, linux-rdma,
	linux-pci, dri-devel

On 7/17/2026 4:08 AM, Zhiping Zhang wrote:
> On Wed, Jul 15, 2026 at 6:07 PM fengchengwen <fengchengwen@huawei.com> wrote:
>>
>>>
>> Hi Zhiping,
>>
>> One concern I'd like to raise is the implicit topology constraint of the
>> TPH P2P path. TPH Steering Tag only works reliably when both importer and
>> exporter devices sit under the same Root Port (or same TPH-aware switch).
>>
>> For cross-Root-Port P2P traffic (e.g. P2P devices located in different
>> RC or even socket), most server-grade Root Complexes do not forward TPH
>> prefixes through their internal routing. The ST field gets silently
>> stripped before reaching the target device, so all TPH setup becomes
>> ineffective with no error reported to user.
>>
>> Because cross-Root-Port P2P traffic with ST does not affect the host, but
>> merely the completer may experience some performance degradation because
>> it lacks the TPH hint. In this case, I suggest adding restrictions in the
>> uAPI.
>>
>> A further step is to check whether the importer and exporter are under
>> the same root port. However, this verification will cause the P2P TPH
>> to be unsupported in the case of multi-host or multi-root PCIe switch.
>>
>> Thanks
> 
> Hi Fengcheng,
> 
> Agreed the RC may not preserve TPH across Root Ports (PCIe Base Spec
> r6.4 §6.17.4 requires all intermediate routing elements to support
> TPH, and cross-RP forwarding is optional/implementation-dependent, cf.
> §2.2.10.4.2). But the uAPI can't gate it: there's no discoverable "TPH
> routing" capability -- DevCap2[13:12] (r6.4 §7.5.3.15) describes the
> completer, not P2P forwarding -- and a same-Root-Port check would
> wrongly reject switch and multi-root topologies where ST does forward.
> 
> And it's harmless anyway: per r6.4 §2.2.7.1 an element without TPH
> support ignores the TH bit and completes as if unhinted, so a stripped
> ST just degrades to the non-TPH baseline.

Thank you for your detailed explanation. I agree to maintain the current
implementation.

Thanks

> 
> Thanks,
> Zhiping
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v12 3/4] vfio/pci: implement get_pci_tph and DMA_BUF_TPH feature
       [not found] ` <20260715204008.3911275-4-zhipingz@meta.com>
@ 2026-07-17  0:36   ` fengchengwen
  0 siblings, 0 replies; 9+ messages in thread
From: fengchengwen @ 2026-07-17  0:36 UTC (permalink / raw)
  To: Zhiping Zhang, Jason Gunthorpe, Leon Romanovsky, Michael Guralnik,
	Sumit Semwal, Christian Konig, Alex Williamson, Bjorn Helgaas
  Cc: kvm, linux-rdma, linux-pci, dri-devel

Acked-by: Chengwen Feng <fengchengwen@huawei.com>

On 7/16/2026 4:39 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.
> 
> Hold memory_lock in the SET path while checking that the dma-buf still
> belongs to this vfio-pci device and while publishing the descriptor.
> Cleanup clears the dma-buf/vdev association under memory_lock; taking the
> same lock prevents publishing into a dma-buf that has already been
> detached from @vdev, and preserves the memory_lock -> dmabuf->resv order
> used by the move path.
> 
> The attach path reads @revoked without holding memory_lock. Use
> READ_ONCE() for that lockless attach-path test and WRITE_ONCE() for the
> revoke/unrevoke writers 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.
> 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>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v12 2/4] dma-buf: add optional get_pci_tph() callback
       [not found] ` <20260715204008.3911275-3-zhipingz@meta.com>
@ 2026-07-17  0:59   ` fengchengwen
  0 siblings, 0 replies; 9+ messages in thread
From: fengchengwen @ 2026-07-17  0:59 UTC (permalink / raw)
  To: Zhiping Zhang, Jason Gunthorpe, Leon Romanovsky, Michael Guralnik,
	Sumit Semwal, Christian Konig, Alex Williamson, Bjorn Helgaas
  Cc: kvm, linux-rdma, linux-pci, dri-devel

Acked-by: Chengwen Feng <fengchengwen@huawei.com>

On 7/16/2026 4:39 AM, Zhiping Zhang wrote:
> Add an optional dma_buf_ops.get_pci_tph callback and a
> DMA-buf importer wrapper, dma_buf_get_pci_tph().
> 
> TPH is PCIe TLP Processing Hint. 8-bit ST and 16-bit Extended ST are
> distinct PCIe TPH namespaces, so the importer requests the namespace it
> can emit and the exporter returns the matching ST/PH tuple or
> -EOPNOTSUPP.
> 
> dma_buf_get_pci_tph() is the importer entry point. It requires
> &dmabuf->resv to be held while the callback runs and returns
> -EOPNOTSUPP when the exporter does not provide PCI TPH metadata.
> 
> The first user is VFIO_DEVICE_FEATURE_DMA_BUF_TPH in vfio-pci, with
> mlx5 as the first importer.
> 
> Signed-off-by: Zhiping Zhang <zhipingz@meta.com>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v12 0/4] vfio/dma-buf: add TPH support for peer-to-peer access
       [not found] <20260715204008.3911275-1-zhipingz@meta.com>
                   ` (2 preceding siblings ...)
       [not found] ` <20260715204008.3911275-3-zhipingz@meta.com>
@ 2026-07-28 17:03 ` Keith Busch
  2026-07-29 23:03 ` Alex Williamson
  4 siblings, 0 replies; 9+ messages in thread
From: Keith Busch @ 2026-07-28 17:03 UTC (permalink / raw)
  To: Zhiping Zhang
  Cc: Jason Gunthorpe, Leon Romanovsky, Michael Guralnik, Sumit Semwal,
	Christian Konig, Alex Williamson, Bjorn Helgaas, kvm, linux-rdma,
	linux-pci, dri-devel

On Wed, Jul 15, 2026 at 01:39:55PM -0700, Zhiping Zhang wrote:
> This series adds TLP Processing Hints (TPH) support to the VFIO dma-buf
> export path, allowing importing drivers (e.g. mlx5) to use the
> exporter's steering tag when performing peer-to-peer DMA into a
> VFIO-owned device.

Thanks, Zhiping. This looks good to me.

Acked-by: Keith Busch <kbusch@kernel.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v12 0/4] vfio/dma-buf: add TPH support for peer-to-peer access
       [not found] <20260715204008.3911275-1-zhipingz@meta.com>
                   ` (3 preceding siblings ...)
  2026-07-28 17:03 ` [PATCH v12 0/4] vfio/dma-buf: add TPH support for peer-to-peer access Keith Busch
@ 2026-07-29 23:03 ` Alex Williamson
  4 siblings, 0 replies; 9+ messages in thread
From: Alex Williamson @ 2026-07-29 23:03 UTC (permalink / raw)
  To: Zhiping Zhang
  Cc: Jason Gunthorpe, Leon Romanovsky, Michael Guralnik, Sumit Semwal,
	Christian Konig, Bjorn Helgaas, kvm, linux-rdma, linux-pci,
	dri-devel, alex

On Wed, 15 Jul 2026 13:39:55 -0700
Zhiping Zhang <zhipingz@meta.com> wrote:
>
> Depends on (submitted separately):
>   net/mlx5: free mlx5_st_idx_data on final dealloc
>     https://lore.kernel.org/linux-rdma/20260612170406.3339093-1-zhipingz@meta.com
>   PCI/TPH: fold reserved completer encoding in get_rp_completer_type()
>     https://lore.kernel.org/linux-pci/20260715202409.3767494-1-zhipingz@meta.com

Sashiko failed to apply[1], it can't handle such dependencies.  The
first dependency already exists in linux-next, I applied the latter
manually, therefore the patch indexes in the below local sashiko run
results are off by one.  This used Claude Opus 4.8 rather than the
standard Gemini 3.1 Pro Preview used in the on-list reviews, other LLMs
may find more or less.

We should probably reevaluate the best merge path for these, it doesn't
make a lot of sense to me to have one patch routed through Bjorn's tree
for a self-contained fix where no known hardware makes use of the
reserved encoding, then depend on that change in the first patch of
this series.  We need Bjorn's ack here anyway, the Fixes: tag is
academic in current code.  Suggest adding that patch into this series
on the re-spin and prodding required subsystem maintainers for review.

I'd upgrade the severity of the first issue below since it affects the
uAPI.  Thanks,

Alex

[1]https://sashiko.dev/#/patchset/20260715204008.3911275-1-zhipingz@meta.com

Inline Review:
--- Patch [4]: vfio/pci: implement get_pci_tph and DMA_BUF_TPH feature ---
commit 3a3a7e88fb6acb9853e8f8bea216613aeea43b4f
Author: Zhiping Zhang <zhipingz@meta.com>

vfio/pci: implement get_pci_tph and DMA_BUF_TPH feature

This patch implements the dma-buf get_pci_tph callback for vfio-pci
exported dma-bufs and adds VFIO_DEVICE_FEATURE_DMA_BUF_TPH so userspace
can publish TPH metadata for a VFIO-owned device. It gates the feature on
the device reporting TPH Completer support in Device Capabilities 2.

Link: https://lore.kernel.org/r/20260715204008.3911275-4-zhipingz@meta.com

> diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c
> index c16f460c01d6..7db84d27dfec 100644
> --- a/drivers/vfio/pci/vfio_pci_dmabuf.c
> +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c

[ ... ]

> +int vfio_pci_core_feature_dma_buf_tph(struct vfio_pci_core_device *vdev,
> +				      u32 flags,
> +				      struct vfio_device_feature_dma_buf_tph __user *arg,
> +				      size_t argsz)
> +{
> +	struct vfio_device_feature_dma_buf_tph set_tph;
> +	struct vfio_pci_dma_buf *priv;
> +	struct dma_buf *dmabuf;
> +	u8 comp;
> +	int ret;
> +
> +	comp = pcie_tph_completer_type(vdev->pdev);
> +	if (comp == PCI_EXP_DEVCAP2_TPH_COMP_NONE)
> +		return -EOPNOTSUPP;
> +
> +	ret = vfio_check_feature(flags, argsz, VFIO_DEVICE_FEATURE_SET,
> +				 sizeof(set_tph));
> +	if (ret != 1)
> +		return ret;

[Severity: Low]
Should the PROBE path here also require that the device can actually
export a vfio dma-buf?

The only gate before vfio_check_feature() is the completer-type check:

	comp = pcie_tph_completer_type(vdev->pdev);
	if (comp == PCI_EXP_DEVCAP2_TPH_COMP_NONE)
		return -EOPNOTSUPP;

vfio_check_feature() returns 0 for a PROBE request, so on a device that
reports TPH Completer support in Device Capabilities 2 but whose pci_ops
has no get_dmabuf_phys, a VFIO_DEVICE_FEATURE_PROBE for
VFIO_DEVICE_FEATURE_DMA_BUF_TPH reports success even though no vfio
dma-buf can ever be created to carry the TPH metadata. Every real SET
would then fail later, either at dma_buf_get() or at the
dmabuf->ops != &vfio_pci_dmabuf_ops ownership check.

The sibling base feature vfio_pci_core_feature_dma_buf() opens with:

	if (!vdev->pci_ops || !vdev->pci_ops->get_dmabuf_phys)
		return -EOPNOTSUPP;

so its PROBE reports unsupported when no dma-buf can be created.

The commit message says PROBE was made to "follow the same hardware gate
so the feature only probes as supported when the device can really
consume it." Would adding the same get_dmabuf_phys check make PROBE
consistent with vfio_pci_core_feature_dma_buf() and match that stated
intent?

--- Patch [5]: RDMA/mlx5: get tph for p2p access when registering dma-buf mr ---
commit e612584ae6697d1ebf77baf0fb1487a41e4c6511
Author: Zhiping Zhang <zhipingz@meta.com>

RDMA/mlx5: get tph for p2p access when registering dma-buf mr

This patch wires up mlx5_ib to query dma_buf_get_pci_tph() metadata at MR
registration time for p2p access and program requester-side TPH on the
outbound mkey.  As part of it, mlx5_st_alloc_index() is split so that a new
mlx5_st_alloc_index_by_tag() helper does the table lookup while the old
entry point still handles the ACPI steering-tag query.

Link: https://lore.kernel.org/r/20260715204008.3911275-5-zhipingz@meta.com

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/st.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/st.c
> index 7cedc348790d..877b37b4e639 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/st.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/st.c
> @@ -92,23 +92,18 @@ void mlx5_st_destroy(struct mlx5_core_dev *dev)
>  	kfree(st);
>  }
>  
> -int mlx5_st_alloc_index(struct mlx5_core_dev *dev, enum tph_mem_type mem_type,
> -			unsigned int cpu_uid, u16 *st_index)
> +int mlx5_st_alloc_index_by_tag(struct mlx5_core_dev *dev, u16 tag,
> +			       u16 *st_index)
>  {
>  	struct mlx5_st_idx_data *idx_data;
>  	struct mlx5_st *st = dev->st;
>  	unsigned long index;
>  	u32 xa_id;
> -	u16 tag;
> -	int ret;
> +	int ret = 0;
>  
>  	if (!st)
>  		return -EOPNOTSUPP;
>  
> -	ret = pcie_tph_get_cpu_st(dev->pdev, mem_type, cpu_uid, &tag);
> -	if (ret)
> -		return ret;
> -
>  	if (st->direct_mode) {
>  		*st_index = tag;
>  		return 0;

[ ... ]

> @@ -152,8 +147,46 @@ int mlx5_st_alloc_index(struct mlx5_core_dev *dev, enum tph_mem_type mem_type,
>  	mutex_unlock(&st->lock);
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(mlx5_st_alloc_index_by_tag);
> +
> +int mlx5_st_alloc_index(struct mlx5_core_dev *dev, enum tph_mem_type mem_type,
> +			unsigned int cpu_uid, u16 *st_index)
> +{
> +	u16 tag;
> +	int ret;
> +
> +	ret = pcie_tph_get_cpu_st(dev->pdev, mem_type, cpu_uid, &tag);
> +	if (ret)
> +		return ret;
> +
> +	return mlx5_st_alloc_index_by_tag(dev, tag, st_index);
> +}
>  EXPORT_SYMBOL_GPL(mlx5_st_alloc_index);

[Severity: Low]
This refactor moves the if (!st) return -EOPNOTSUPP; short-circuit into
mlx5_st_alloc_index_by_tag(), so the remaining mlx5_st_alloc_index() now
calls pcie_tph_get_cpu_st() before that guard is reached.

Before this patch mlx5_st_alloc_index() checked !st first and returned
-EOPNOTSUPP without ever calling pcie_tph_get_cpu_st().  Now the order is
reversed:

mlx5_st_alloc_index()
    pcie_tph_get_cpu_st(dev->pdev, ...)   /* runs first */
    mlx5_st_alloc_index_by_tag()
        if (!st) return -EOPNOTSUPP;      /* guard now lives here */

For a device where dev->st is NULL (TPH not supported, e.g.
!MLX5_CAP_GEN(mkey_pcie_tph), no pdev->tph_cap, or a failed mlx5_st_create()),
the DMAH-with-CPU-id path mlx5_ib_alloc_dmah() -> mlx5_st_alloc_index() now
reaches pcie_tph_get_cpu_st(), which performs an ACPI _DSM invocation
(acpi_get_cpu_uid() + pcie_find_root_port() + tph_invoke_dsm()) that was
previously skipped entirely.

Is this added firmware query on a device that does not support ST intended?

There is also an errno change: when the _DSM lookup itself fails,
mlx5_st_alloc_index() now returns pcie_tph_get_cpu_st()'s error (e.g.
-ENODEV or -EINVAL) to userspace instead of the prior -EOPNOTSUPP.  When the
lookup succeeds the by_tag re-check still returns -EOPNOTSUPP, so the two
differ only on the sub-path where the _DSM fails.

Would it be worth restoring the !st early-out in mlx5_st_alloc_index() before
the pcie_tph_get_cpu_st() call, or noting this behavioral change in the
commit message, which currently presents the change only as extracting
mlx5_st_alloc_index_by_tag()?

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-07-29 23:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260715204008.3911275-1-zhipingz@meta.com>
2026-07-16  1:07 ` [PATCH v12 0/4] vfio/dma-buf: add TPH support for peer-to-peer access fengchengwen
2026-07-16 14:47   ` Jason Gunthorpe
2026-07-16 20:10     ` Zhiping Zhang
2026-07-16 20:08   ` Zhiping Zhang
2026-07-17  0:16     ` fengchengwen
     [not found] ` <20260715204008.3911275-4-zhipingz@meta.com>
2026-07-17  0:36   ` [PATCH v12 3/4] vfio/pci: implement get_pci_tph and DMA_BUF_TPH feature fengchengwen
     [not found] ` <20260715204008.3911275-3-zhipingz@meta.com>
2026-07-17  0:59   ` [PATCH v12 2/4] dma-buf: add optional get_pci_tph() callback fengchengwen
2026-07-28 17:03 ` [PATCH v12 0/4] vfio/dma-buf: add TPH support for peer-to-peer access Keith Busch
2026-07-29 23:03 ` Alex Williamson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox