Re: [PATCH v6 17/18] dma-buf: Let importers ask how peer-to-peer traffic is routed

[Christian König]

On 9/14/26 13:22, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@xxxxxxxxxx>
> 
> Exporters keep the &struct p2pdma_provider backing a buffer in their own
> private data. An importer cannot reach it, so it has no way to learn how
> its own peer-to-peer traffic would be routed before it programs its
> hardware.
> 
> Add an optional @p2pdma_provider callback for an exporter to hand that
> provider out, and dma_buf_p2pdma_map_type() for an importer to ask by TLP
> class. Exporters keep the provider where it already lives, so this adds an
> operation rather than changing any existing signature or structure.
> 
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxx>
> ---
>  drivers/dma-buf/dma-buf-mapping.c                 | 32 +++++++++++++++++++++++
>  drivers/infiniband/core/uverbs_std_types_dmabuf.c | 12 +++++++++
>  drivers/vfio/pci/vfio_pci_dmabuf.c                | 12 +++++++++
>  include/linux/dma-buf-mapping.h                   |  3 +++
>  include/linux/dma-buf.h                           | 17 ++++++++++++
>  5 files changed, 76 insertions(+)
> 
> diff --git a/drivers/dma-buf/dma-buf-mapping.c b/drivers/dma-buf/dma-buf-mapping.c
> index 794acff2546a..fb03a698b381 100644
> --- a/drivers/dma-buf/dma-buf-mapping.c
> +++ b/drivers/dma-buf/dma-buf-mapping.c
> @@ -6,6 +6,38 @@
>  #include <linux/dma-buf-mapping.h>
>  #include <linux/dma-resv.h>
>  
> +/**
> + * dma_buf_p2pdma_map_type - How peer-to-peer traffic to a buffer is routed
> + * @attach:	attachment of the importer that will issue the traffic
> + * @tlp_flags:	&enum pci_p2pdma_tlp_flags describing the TLPs it will issue
> + *
> + * Reports how the PCIe fabric routes @tlp_flags traffic between the buffer
> + * behind @attach and the importer attached to it, so that an importer can
> + * choose the TLP attributes that earn it a direct route before it programs
> + * its hardware.
> + *
> + * Return: the mapping type for @tlp_flags traffic, or PCI_P2PDMA_MAP_NONE
> + * when the exporter names no &struct p2pdma_provider and nothing is known
> + * about the route.
> + */
> +enum pci_p2pdma_map_type
> +dma_buf_p2pdma_map_type(struct dma_buf_attachment *attach,
> +			unsigned int tlp_flags)
> +{
> +	struct dma_buf *dmabuf = attach->dmabuf;
> +	struct p2pdma_provider *provider;
> +
> +	if (!dmabuf->ops->p2pdma_provider)
> +		return PCI_P2PDMA_MAP_NONE;
> +
> +	provider = dmabuf->ops->p2pdma_provider(dmabuf);
> +	if (!provider)
> +		return PCI_P2PDMA_MAP_NONE;
> +
> +	return pci_p2pdma_map_type_tlp(provider, attach->dev, tlp_flags);

Calling PCI subsystem functions from dma-buf is a hard NO-GO.

Then wrappers for DMA-buf backend functions should be in dma-buf.c and not here.

I shouldn't have allowed the mapping functions to be added to DMA-buf in the first place, all of this belongs either into the DMA layer or the exporter.

We should probably have a single callback the exporter provides to fill in a structure with PCI specific information for a mapping.


> +}
> +EXPORT_SYMBOL_NS_GPL(dma_buf_p2pdma_map_type, "DMA_BUF");
> +
>  static struct scatterlist *fill_sg_entry(struct scatterlist *sgl, size_t length,
>  					 dma_addr_t addr)
>  {
> diff --git a/drivers/infiniband/core/uverbs_std_types_dmabuf.c b/drivers/infiniband/core/uverbs_std_types_dmabuf.c
> index 2411ebee69e2..5a6a1ee430a7 100644
> --- a/drivers/infiniband/core/uverbs_std_types_dmabuf.c
> +++ b/drivers/infiniband/core/uverbs_std_types_dmabuf.c
> @@ -76,6 +76,17 @@ static void uverbs_dmabuf_release(struct dma_buf *dmabuf)
>  	uverbs_uobject_release(&priv->uobj);
>  }
>  
> +static struct p2pdma_provider *
> +uverbs_dmabuf_provider(struct dma_buf *dmabuf)
> +{
> +	struct ib_uverbs_dmabuf_file *priv = dmabuf->priv;
> +
> +	if (priv->revoked)
> +		return NULL;
> +
> +	return priv->provider;
> +}
> +
>  static const struct dma_buf_ops uverbs_dmabuf_ops = {
>  	.attach = uverbs_dmabuf_attach,
>  	.map_dma_buf = uverbs_dmabuf_map,
> @@ -83,6 +94,7 @@ static const struct dma_buf_ops uverbs_dmabuf_ops = {
>  	.pin = uverbs_dmabuf_pin,
>  	.unpin = uverbs_dmabuf_unpin,
>  	.release = uverbs_dmabuf_release,
> +	.p2pdma_provider = uverbs_dmabuf_provider,
>  };
>  
>  static int UVERBS_HANDLER(UVERBS_METHOD_DMABUF_ALLOC)(
> diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c
> index c16f460c01d6..fcf392cfc3c5 100644
> --- a/drivers/vfio/pci/vfio_pci_dmabuf.c
> +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
> @@ -99,11 +99,23 @@ static void vfio_pci_dma_buf_release(struct dma_buf *dmabuf)
>  	kfree(priv);
>  }
>  
> +static struct p2pdma_provider *
> +vfio_pci_dma_buf_provider(struct dma_buf *dmabuf)
> +{
> +	struct vfio_pci_dma_buf *priv = dmabuf->priv;
> +
> +	if (priv->revoked)
> +		return NULL;
> +
> +	return priv->provider;
> +}
> +
>  static const struct dma_buf_ops vfio_pci_dmabuf_ops = {
>  	.attach = vfio_pci_dma_buf_attach,
>  	.map_dma_buf = vfio_pci_dma_buf_map,
>  	.unmap_dma_buf = vfio_pci_dma_buf_unmap,
>  	.release = vfio_pci_dma_buf_release,
> +	.p2pdma_provider = vfio_pci_dma_buf_provider,

Please split up the patch into the actual DMA-buf changes, exporter changes and importer changes.

Regards,
Christian.

>  };
>  
>  /*
> diff --git a/include/linux/dma-buf-mapping.h b/include/linux/dma-buf-mapping.h
> index 09bde3f748e4..37b3da10b17a 100644
> --- a/include/linux/dma-buf-mapping.h
> +++ b/include/linux/dma-buf-mapping.h
> @@ -7,6 +7,9 @@
>  #define __DMA_BUF_MAPPING_H__
>  #include <linux/dma-buf.h>
>  
> +enum pci_p2pdma_map_type
> +dma_buf_p2pdma_map_type(struct dma_buf_attachment *attach,
> +			unsigned int tlp_flags);
>  struct sg_table *dma_buf_phys_vec_to_sgt(struct dma_buf_attachment *attach,
>  					 struct p2pdma_provider *provider,
>  					 struct phys_vec *phys_vec,
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index d1203da56fc5..cff1ee0838f0 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -186,6 +186,23 @@ struct dma_buf_ops {
>  	 * if the call would block.
>  	 */
>  
> +	/**
> +	 * @p2pdma_provider:
> +	 *
> +	 * Returns the &struct p2pdma_provider backing this buffer, so that an
> +	 * importer can ask how its peer-to-peer traffic would be routed before
> +	 * it programs its hardware. Importers reach this through
> +	 * dma_buf_p2pdma_map_type() rather than calling it directly.
> +	 *
> +	 * Exporters of MMIO memory that is reachable peer-to-peer should
> +	 * implement this. This callback is optional.
> +	 *
> +	 * Returns:
> +	 *
> +	 * The provider backing the buffer.
> +	 */
> +	struct p2pdma_provider *(*p2pdma_provider)(struct dma_buf *dmabuf);
> +
>  	/**
>  	 * @release:
>  	 *
>