Re: [RFC 1/8] dma-buf: Add support for map/unmap APIs for interconnects

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

Hi, Vivek,

On Tue, 2025-10-14 at 00:08 -0700, Vivek Kasireddy wrote:
> For the map operation, the dma-buf core will create an xarray but
> the exporter is expected to populate it with the interconnect
> specific addresses. And, similarly for unmap, the exporter is
> expected to cleanup the individual entries of the xarray.
> 
> Cc: Jason Gunthorpe <jgg@nvidia.com>
> Cc: Christian Koenig <christian.koenig@amd.com>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Simona Vetter <simona.vetter@ffwll.ch>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>  drivers/dma-buf/dma-buf.c            | 68
> ++++++++++++++++++++++++++++
>  include/linux/dma-buf-interconnect.h | 29 ++++++++++++
>  include/linux/dma-buf.h              | 11 +++++
>  3 files changed, 108 insertions(+)
>  create mode 100644 include/linux/dma-buf-interconnect.h
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 2bcf9ceca997..162642bd53e8 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -1612,6 +1612,74 @@ void dma_buf_vunmap_unlocked(struct dma_buf
> *dmabuf, struct iosys_map *map)
>  }
>  EXPORT_SYMBOL_NS_GPL(dma_buf_vunmap_unlocked, "DMA_BUF");
>  
> +struct dma_buf_ranges *
> +dma_buf_map_interconnect(struct dma_buf_attachment *attach)

Even if this is an RFC, please add kerneldoc so that the way the
interface is intended to be used becomes completely clear. Both for
functions and structs.


> +{
> +	const struct dma_buf_interconnect_ops *ic_ops;
> +	struct dma_buf *dmabuf = attach->dmabuf;
> +	struct dma_buf_ranges *ranges;
> +	int ret;
> +
> +	might_sleep();
> +
> +	if (WARN_ON(!attach || !attach->dmabuf))
> +		return ERR_PTR(-EINVAL);
> +
> +	if (!dma_buf_attachment_is_dynamic(attach))
> +		return ERR_PTR(-EINVAL);
> +
> +	if (!attach->allow_ic)
> +		return ERR_PTR(-EOPNOTSUPP);
> +
> +	dma_resv_assert_held(attach->dmabuf->resv);
> +
> +	ic_ops = dmabuf->ops->interconnect_ops;
> +	if (!ic_ops || !ic_ops->map_interconnect)
> +		return ERR_PTR(-EINVAL);
> +
> +	ranges = kzalloc(sizeof(*ranges), GFP_KERNEL);
> +	if (!ranges)
> +		return ERR_PTR(-ENOMEM);
> +
> +	xa_init(&ranges->ranges);
> +	ret = ic_ops->map_interconnect(attach, ranges);
> +	if (ret)
> +		goto err_free_ranges;
> +
> +	return ranges;
> +
> +err_free_ranges:
> +	xa_destroy(&ranges->ranges);
> +	kfree(ranges);
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_NS_GPL(dma_buf_map_interconnect, "DMA_BUF");
> +
> +void dma_buf_unmap_interconnect(struct dma_buf_attachment *attach,
> +				struct dma_buf_ranges *ranges)
> +{
> +	const struct dma_buf_interconnect_ops *ic_ops;
> +	struct dma_buf *dmabuf = attach->dmabuf;
> +
> +	if (WARN_ON(!attach || !attach->dmabuf || !ranges))
> +		return;
> +
> +	if (!attach->allow_ic)
> +		return;
> +
> +	ic_ops = dmabuf->ops->interconnect_ops;
> +	if (!ic_ops || !ic_ops->unmap_interconnect)
> +		return;
> +
> +	dma_resv_assert_held(attach->dmabuf->resv);
> +
> +	ic_ops->unmap_interconnect(attach, ranges);
> +
> +	xa_destroy(&ranges->ranges);
> +	kfree(ranges);
> +}
> +EXPORT_SYMBOL_NS_GPL(dma_buf_unmap_interconnect, "DMA_BUF");
> +
>  #ifdef CONFIG_DEBUG_FS
>  static int dma_buf_debug_show(struct seq_file *s, void *unused)
>  {
> diff --git a/include/linux/dma-buf-interconnect.h
> b/include/linux/dma-buf-interconnect.h
> new file mode 100644
> index 000000000000..17504dea9691
> --- /dev/null
> +++ b/include/linux/dma-buf-interconnect.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: MIT */
> +
> +#ifndef __DMA_BUF_INTERCONNECT_H__
> +#define __DMA_BUF_INTERCONNECT_H__
> +
> +#include <linux/xarray.h>
> +
> +struct dma_buf_attachment;
> +
> +struct dma_buf_ranges {
> +	struct xarray ranges;
> +	unsigned int nranges;

IIUC this would replace the sg-table right? I guess Jason or Christian
would need to comment on whether this is generic enough or whether it
needs to be interconnect-dependent.

> +};
> +
> +enum dma_buf_interconnect_type {
> +	DMA_BUF_INTERCONNECT_NONE = 0,
> +};

This calls for registering all known interconnects with the dma-buf
layer even if the interconnects are completely driver-private. I'd
suggest using a pointer to identify interconnect and whatever entity
defines the interconnect provides a unique pointer. For globally
visible interconnects this could be done in dma-buf.c or a dma-buf-
interconnect.c

> +
> +struct dma_buf_interconnect {
> +	enum dma_buf_interconnect_type type;
> +};
> +
> +struct dma_buf_interconnect_ops {
> +	int (*map_interconnect)(struct dma_buf_attachment *attach,
> +				struct dma_buf_ranges *ranges);
> +	void (*unmap_interconnect)(struct dma_buf_attachment
> *attach,
> +				   struct dma_buf_ranges *ranges);
> +};
> +#endif
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index d58e329ac0e7..db91c67c00d6 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -23,6 +23,8 @@
>  #include <linux/dma-fence.h>
>  #include <linux/wait.h>
>  
> +#include <linux/dma-buf-interconnect.h>
> +
>  struct device;
>  struct dma_buf;
>  struct dma_buf_attachment;
> @@ -276,6 +278,8 @@ struct dma_buf_ops {
>  
>  	int (*vmap)(struct dma_buf *dmabuf, struct iosys_map *map);
>  	void (*vunmap)(struct dma_buf *dmabuf, struct iosys_map
> *map);
> +
> +	const struct dma_buf_interconnect_ops *interconnect_ops;
>  };
>  
>  /**
> @@ -502,7 +506,9 @@ struct dma_buf_attachment {
>  	struct device *dev;
>  	struct list_head node;
>  	bool peer2peer;
> +	bool allow_ic;
>  	const struct dma_buf_attach_ops *importer_ops;
> +	struct dma_buf_interconnect interconnect;

Hmm. Could we have a pointer to the interconnect here? Let's say the
interconnect implementation would want to subclass with additional
information?


>  	void *importer_priv;
>  	void *priv;
>  };
> @@ -589,6 +595,11 @@ struct sg_table *dma_buf_map_attachment(struct
> dma_buf_attachment *,
>  					enum dma_data_direction);
>  void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct
> sg_table *,
>  				enum dma_data_direction);
> +
> +struct dma_buf_ranges *dma_buf_map_interconnect(struct
> dma_buf_attachment *);
> +void dma_buf_unmap_interconnect(struct dma_buf_attachment *,
> +				struct dma_buf_ranges *);
> +
>  void dma_buf_move_notify(struct dma_buf *dma_buf);
>  int dma_buf_begin_cpu_access(struct dma_buf *dma_buf,
>  			     enum dma_data_direction dir);

Thanks,
Thomas