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
linaro-mm-sig@lists.linaro.org