Hi Thomas,
> Subject: Re: [RFC 1/8] dma-buf: Add support for map/unmap APIs for
> interconnects
>
> 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.
Ok, will add documentation in the next version.
>
>
> > +{
> > + 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?
Yes, that is the intended goal.
> I guess Jason or Christian
> would need to comment on whether this is generic enough or whether it
> needs to be interconnect-dependent.
AFAIU, the individual entries of the xarray could be of any type that is
interconnect-specific and shared between exporter and importer.
For example, for IOV interconnect, I have picked struct range as the
type (to represent individual entries of the xarray) to share addresses
between exporter and importer.
>
> > +};
> > +
> > +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
Thank you for your suggestion. I'll explore the idea in more detail.
>
> > +
> > +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?
Sure. I was going to do that in the next version.
Thanks,
Vivek
>
>
> > 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
>
