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

On Wed, Jul 29, 2026 at 4:03=E2=80=AFPM Alex Williamson <[email protected]> =
wrote:
...
> On Wed, 15 Jul 2026 13:39:55 -0700
> Zhiping Zhang <[email protected]> wrote:
> >
> > Depends on (submitted separately):
> >   net/mlx5: free mlx5_st_idx_data on final dealloc
> >     https://lore.kernel.org/linux-rdma/20260612170406.3339093-1-zhiping=
[email protected]
> >   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.
>

Thanks for applying it by hand and running this locally.  Both
findings are real and v13 will fix them.

> 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.
>

Agreed, and I've said the same on the PCI thread
[https://lore.kernel.org/linux-pci/[email protected]=
m].

...
> [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 =3D pcie_tph_completer_type(vdev->pdev);
>         if (comp =3D=3D 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 !=3D &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?
>

For this VFIO PROBE issue, yes, I agree this should use the same
dma-buf export capability gate as VFIO_DEVICE_FEATURE_DMA_BUF. TPH
metadata is only useful for a vfio-pci dma-buf, so PROBE should not
report support on a device that cannot create one. I will add:

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

before the completer-type check in vfio_pci_core_feature_dma_buf_tph().

> --- Patch [5]: RDMA/mlx5: get tph for p2p access when registering dma-buf=
 mr ---
> commit e612584ae6697d1ebf77baf0fb1487a41e4c6511
> Author: Zhiping Zhang <[email protected]>
>
...
> [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_creat=
e()),
> the DMAH-with-CPU-id path mlx5_ib_alloc_dmah() -> mlx5_st_alloc_index() n=
ow
> 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 intende=
d?
>
> 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() be=
fore
> 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()?

Agreed. The added _DSM query on the !dev->st path was not intended;
that function should preserve the old early -EOPNOTSUPP behavior
before calling pcie_tph_get_cpu_st(). I will restore the !dev->st
early-out in mlx5_st_alloc_index(), while keeping the guard in
mlx5_st_alloc_index_by_tag() for the dma-buf-provided tag path.

I will include these fixes in the next respin, including the folding
patch as patch 1 in the set.

Thanks,
Zhiping