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

["Christian König" <christian.koenig@amd.com>]

On 9/10/26 13:32, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@nvidia.com>
> 
> Exporters keep the &struct p2pdma_provider in their own private data and
> hand it to dma_buf_phys_vec_to_sgt() on every map. 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.

Why should that be interesting to importers?

In general importers should just consome the data provided by a DMA-buf and don't participate in any way in the decision how that data is made available.
 
> Publish the provider through &struct dma_buf_export_info instead,

That is a very bad idea and not even remotely acceptable.

This needs to be a callback instead if that should really fly.

Regards,
Christian.

> and add
> dma_buf_p2pdma_map_type() for importers to query it by TLP class. It is
> fixed at export time, so dma_buf_phys_vec_to_sgt() no longer needs it as an
> argument and the two exporters no longer need a copy of it.
> 
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> ---
>  drivers/dma-buf/dma-buf-mapping.c                 | 41 +++++++++++++++++++----
>  drivers/dma-buf/dma-buf.c                         |  1 +
>  drivers/infiniband/core/uverbs.h                  |  1 -
>  drivers/infiniband/core/uverbs_std_types_dmabuf.c |  7 ++--
>  drivers/vfio/pci/vfio_pci_dmabuf.c                |  8 ++---
>  include/linux/dma-buf-mapping.h                   |  4 ++-
>  include/linux/dma-buf.h                           |  5 +++
>  7 files changed, 49 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-buf-mapping.c b/drivers/dma-buf/dma-buf-mapping.c
> index 794acff2546a..8b431000e906 100644
> --- a/drivers/dma-buf/dma-buf-mapping.c
> +++ b/drivers/dma-buf/dma-buf-mapping.c
> @@ -6,6 +6,32 @@
>  #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 named 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)
> +{
> +	if (!attach->dmabuf->provider)
> +		return PCI_P2PDMA_MAP_NONE;
> +
> +	return pci_p2pdma_map_type_tlp(attach->dmabuf->provider, attach->dev,
> +				       tlp_flags);
> +}
> +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)
>  {
> @@ -71,7 +97,6 @@ struct dma_buf_dma {
>   * from arrays of physical vectors. This funciton is intended for MMIO memory
>   * only.
>   * @attach:	[in]	attachment whose scatterlist is to be returned
> - * @provider:	[in]	p2pdma provider
>   * @phys_vec:	[in]	array of physical vectors
>   * @nr_ranges:	[in]	number of entries in phys_vec array
>   * @size:	[in]	total size of phys_vec
> @@ -85,16 +110,17 @@ struct dma_buf_dma {
>   *
>   * A mapping must be unmapped by using dma_buf_free_sgt().
>   *
> - * NOTE: This function is intended for exporters. If direct traffic routing is
> - * mandatory exporter should call routing pci_p2pdma_map_type() before calling
> - * this function.
> + * NOTE: This function is intended for exporters, and works on MMIO memory
> + * only, so &dma_buf.provider must have been set at export time. If direct
> + * traffic routing is mandatory the exporter should call
> + * pci_p2pdma_map_type() before calling this function.
>   */
>  struct sg_table *dma_buf_phys_vec_to_sgt(struct dma_buf_attachment *attach,
> -					 struct p2pdma_provider *provider,
>  					 struct phys_vec *phys_vec,
>  					 size_t nr_ranges, size_t size,
>  					 enum dma_data_direction dir)
>  {
> +	struct p2pdma_provider *provider;
>  	unsigned int nents, mapped_len = 0;
>  	struct dma_buf_dma *dma;
>  	struct scatterlist *sgl;
> @@ -104,10 +130,11 @@ struct sg_table *dma_buf_phys_vec_to_sgt(struct dma_buf_attachment *attach,
>  
>  	dma_resv_assert_held(attach->dmabuf->resv);
>  
> -	if (WARN_ON(!attach || !attach->dmabuf || !provider))
> -		/* This function is supposed to work on MMIO memory only */
> +	if (WARN_ON(!attach || !attach->dmabuf || !attach->dmabuf->provider))
>  		return ERR_PTR(-EINVAL);
>  
> +	provider = attach->dmabuf->provider;
> +
>  	dma = kzalloc_obj(*dma);
>  	if (!dma)
>  		return ERR_PTR(-ENOMEM);
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index d504c636dc29..c2b9944e9659 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -747,6 +747,7 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
>  	dmabuf->size = exp_info->size;
>  	dmabuf->exp_name = exp_info->exp_name;
>  	dmabuf->owner = exp_info->owner;
> +	dmabuf->provider = exp_info->provider;
>  	spin_lock_init(&dmabuf->name_lock);
>  	init_waitqueue_head(&dmabuf->poll);
>  	dmabuf->cb_in.poll = dmabuf->cb_out.poll = &dmabuf->poll;
> diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
> index c64dd6b94e10..fbdac18b69ca 100644
> --- a/drivers/infiniband/core/uverbs.h
> +++ b/drivers/infiniband/core/uverbs.h
> @@ -139,7 +139,6 @@ struct ib_uverbs_dmabuf_file {
>  	struct list_head dmabufs_elm;
>  	struct rdma_user_mmap_entry *mmap_entry;
>  	struct phys_vec phys_vec;
> -	struct p2pdma_provider *provider;
>  	struct kref kref;
>  	struct completion comp;
>  	u8 revoked :1;
> diff --git a/drivers/infiniband/core/uverbs_std_types_dmabuf.c b/drivers/infiniband/core/uverbs_std_types_dmabuf.c
> index 2411ebee69e2..94d3719a34da 100644
> --- a/drivers/infiniband/core/uverbs_std_types_dmabuf.c
> +++ b/drivers/infiniband/core/uverbs_std_types_dmabuf.c
> @@ -33,9 +33,8 @@ uverbs_dmabuf_map(struct dma_buf_attachment *attachment,
>  	if (priv->revoked)
>  		return ERR_PTR(-ENODEV);
>  
> -	ret = dma_buf_phys_vec_to_sgt(attachment, priv->provider,
> -				      &priv->phys_vec, 1, priv->phys_vec.len,
> -				      dir);
> +	ret = dma_buf_phys_vec_to_sgt(attachment, &priv->phys_vec, 1,
> +				      priv->phys_vec.len, dir);
>  	if (IS_ERR(ret))
>  		return ret;
>  
> @@ -108,7 +107,7 @@ static int UVERBS_HANDLER(UVERBS_METHOD_DMABUF_ALLOC)(
>  		return -EINVAL;
>  
>  	ret = ib_dev->ops.mmap_get_pfns(mmap_entry, &uverbs_dmabuf->phys_vec,
> -					&uverbs_dmabuf->provider);
> +					&exp_info.provider);
>  	if (ret)
>  		goto err;
>  
> diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c
> index c16f460c01d6..10f47992e7f5 100644
> --- a/drivers/vfio/pci/vfio_pci_dmabuf.c
> +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
> @@ -15,7 +15,6 @@ struct vfio_pci_dma_buf {
>  	struct list_head dmabufs_elm;
>  	size_t size;
>  	struct phys_vec *phys_vec;
> -	struct p2pdma_provider *provider;
>  	u32 nr_ranges;
>  	struct kref kref;
>  	struct completion comp;
> @@ -59,9 +58,8 @@ vfio_pci_dma_buf_map(struct dma_buf_attachment *attachment,
>  	if (priv->revoked)
>  		return ERR_PTR(-ENODEV);
>  
> -	ret = dma_buf_phys_vec_to_sgt(attachment, priv->provider,
> -				      priv->phys_vec, priv->nr_ranges,
> -				      priv->size, dir);
> +	ret = dma_buf_phys_vec_to_sgt(attachment, priv->phys_vec,
> +				      priv->nr_ranges, priv->size, dir);
>  	if (IS_ERR(ret))
>  		return ret;
>  
> @@ -274,7 +272,7 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
>  	priv->vdev = vdev;
>  	priv->nr_ranges = get_dma_buf.nr_ranges;
>  	priv->size = length;
> -	ret = vdev->pci_ops->get_dmabuf_phys(vdev, &priv->provider,
> +	ret = vdev->pci_ops->get_dmabuf_phys(vdev, &exp_info.provider,
>  					     get_dma_buf.region_index,
>  					     priv->phys_vec, dma_ranges,
>  					     priv->nr_ranges);
> diff --git a/include/linux/dma-buf-mapping.h b/include/linux/dma-buf-mapping.h
> index 09bde3f748e4..d63ac4d52aa5 100644
> --- a/include/linux/dma-buf-mapping.h
> +++ b/include/linux/dma-buf-mapping.h
> @@ -7,8 +7,10 @@
>  #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,
>  					 size_t nr_ranges, size_t size,
>  					 enum dma_data_direction dir);
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index d1203da56fc5..81c136fcee6e 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -368,6 +368,9 @@ struct dma_buf {
>  	/** @priv: exporter specific private data for this buffer object. */
>  	void *priv;
>  
> +	/** @provider: The peer-to-peer provider backing this buffer. */
> +	struct p2pdma_provider *provider;
> +
>  	/**
>  	 * @resv:
>  	 *
> @@ -501,6 +504,7 @@ struct dma_buf_attachment {
>   * @flags:	mode flags for the file
>   * @resv:	reservation-object, NULL to allocate default one
>   * @priv:	Attach private data of allocator to this buffer
> + * @provider:	Peer-to-peer provider backing the buffer
>   *
>   * This structure holds the information required to export the buffer. Used
>   * with dma_buf_export() only.
> @@ -512,6 +516,7 @@ struct dma_buf_export_info {
>  	size_t size;
>  	int flags;
>  	struct dma_resv *resv;
> +	struct p2pdma_provider *provider;
>  	void *priv;
>  };
>  
>