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 <[email protected]>
> Cc: Christian Koenig <[email protected]>
> Cc: Sumit Semwal <[email protected]>
> Cc: Thomas Hellström <[email protected]>
> Cc: Simona Vetter <[email protected]>
> Signed-off-by: Vivek Kasireddy <[email protected]>
> ---
> 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
