public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/6] dma-buf: heaps: add coherent reserved-memory heap
@ 2026-03-06 10:36 Albert Esteve
  2026-03-06 10:36 ` [PATCH v3 1/6] dma-buf: dma-heap: Keep track of the heap device struct Albert Esteve
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Albert Esteve @ 2026-03-06 10:36 UTC (permalink / raw)
  To: Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz,
	T.J. Mercier, Christian König, Marek Szyprowski,
	Robin Murphy, Rob Herring, Saravana Kannan
  Cc: linux-kernel, linux-media, dri-devel, linaro-mm-sig, iommu,
	devicetree, Albert Esteve, mripard, echanude, John Stultz,
	Maxime Ripard

This patch series adds a new dma-buf heap driver that exposes coherent,
non‑reusable reserved-memory regions as named heaps, so userspace can
explicitly allocate buffers from those device‑specific pools.

Motivation: we want cgroup accounting for all userspace‑visible buffer
allocations (DRM, v4l2, dma‑buf heaps, etc.). That’s hard to do when
drivers call dma_alloc_attrs() directly because the accounting controller
(memcg vs dmem) is ambiguous. The long‑term plan is to steer those paths
toward dma‑buf heaps, where each heap can unambiguously charge a single
controller. To reach that goal, we need a heap backend for each
dma_alloc_attrs() memory type. CMA and system heaps already exist;
coherent reserved‑memory was the missing piece, since many SoCs define
dedicated, device‑local coherent pools in DT under /reserved-memory using
"shared-dma-pool" with non‑reusable regions (i.e., not CMA) that are
carved out exclusively for coherent DMA and are currently only usable by
in‑kernel drivers.

Because these regions are device‑dependent, each heap instance binds a
heap device to its reserved‑mem region via a newly introduced helper
function -namely, of_reserved_mem_device_init_with_mem()- so coherent
allocations use the correct dev->dma_mem.

Charging to cgroups for these buffers is intentionally left out to keep
review focused on the new heap; I plan to follow up based on Eric’s [1]
and Maxime’s [2] work on dmem charging from userspace.

This series also makes the new heap driver modular, in line with the CMA
heap change in [3].

[1] https://lore.kernel.org/all/20260218-dmabuf-heap-cma-dmem-v2-0-b249886fb7b2@redhat.com/
[2] https://lore.kernel.org/all/20250310-dmem-cgroups-v1-0-2984c1bc9312@kernel.org/
[3] https://lore.kernel.org/all/20260303-dma-buf-heaps-as-modules-v3-0-24344812c707@kernel.org/

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
Changes in v3:
- Reorganized changesets among patches to ensure bisectability
- Removed unused dma_heap_coherent_register() leftover
- Removed fallback when setting mask in coherent heap dev, since
  dma_set_mask() already truncates to supported masks
- Moved struct rmem_assigned_device (rd) logic to
  of_reserved_mem_device_init_with_mem() to allow listing the device
- Link to v2: https://lore.kernel.org/r/20260303-b4-dmabuf-heap-coherent-rmem-v2-0-65a4653b3378@redhat.com

Changes in v2:
- Removed dmem charging parts
- Moved coherent heap registering logic to coherent.c
- Made heap device a member of struct dma_heap
- Split dma_heap_add logic into create/register, to be able to
  access the stored heap device before registered.
- Avoid platform device in favour of heap device
- Added a wrapper to rmem device_init() op
- Switched from late_initcall() to module_init()
- Made the coherent heap driver modular
- Link to v1: https://lore.kernel.org/r/20260224-b4-dmabuf-heap-coherent-rmem-v1-1-dffef43298ac@redhat.com

---
Albert Esteve (5):
      dma-buf: dma-heap: split dma_heap_add
      of_reserved_mem: add a helper for rmem device_init op
      dma: coherent: store reserved memory coherent regions
      dma-buf: heaps: Add Coherent heap to dmabuf heaps
      dma-buf: heaps: coherent: Turn heap into a module

John Stultz (1):
      dma-buf: dma-heap: Keep track of the heap device struct

 drivers/dma-buf/dma-heap.c            | 138 +++++++++--
 drivers/dma-buf/heaps/Kconfig         |   9 +
 drivers/dma-buf/heaps/Makefile        |   1 +
 drivers/dma-buf/heaps/coherent_heap.c | 417 ++++++++++++++++++++++++++++++++++
 drivers/of/of_reserved_mem.c          |  68 ++++--
 include/linux/dma-heap.h              |   5 +
 include/linux/dma-map-ops.h           |   7 +
 include/linux/of_reserved_mem.h       |   8 +
 kernel/dma/coherent.c                 |  34 +++
 9 files changed, 640 insertions(+), 47 deletions(-)
---
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
change-id: 20260223-b4-dmabuf-heap-coherent-rmem-91fd3926afe9

Best regards,
-- 
Albert Esteve <aesteve@redhat.com>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3 1/6] dma-buf: dma-heap: Keep track of the heap device struct
  2026-03-06 10:36 [PATCH v3 0/6] dma-buf: heaps: add coherent reserved-memory heap Albert Esteve
@ 2026-03-06 10:36 ` Albert Esteve
  2026-03-10 14:37   ` Andrew Davis
  2026-03-06 10:36 ` [PATCH v3 2/6] dma-buf: dma-heap: split dma_heap_add Albert Esteve
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Albert Esteve @ 2026-03-06 10:36 UTC (permalink / raw)
  To: Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz,
	T.J. Mercier, Christian König, Marek Szyprowski,
	Robin Murphy, Rob Herring, Saravana Kannan
  Cc: linux-kernel, linux-media, dri-devel, linaro-mm-sig, iommu,
	devicetree, Albert Esteve, mripard, echanude, John Stultz,
	Maxime Ripard

From: John Stultz <john.stultz@linaro.org>

Keep track of the heap device struct.

This will be useful for special DMA allocations
and actions.

Signed-off-by: John Stultz <john.stultz@linaro.org>
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 drivers/dma-buf/dma-heap.c | 34 ++++++++++++++++++++++++++--------
 include/linux/dma-heap.h   |  2 ++
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index ac5f8685a6494..1124d63eb1398 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -31,6 +31,7 @@
  * @heap_devt:		heap device node
  * @list:		list head connecting to list of heaps
  * @heap_cdev:		heap char device
+ * @heap_dev:		heap device
  *
  * Represents a heap of memory from which buffers can be made.
  */
@@ -41,6 +42,7 @@ struct dma_heap {
 	dev_t heap_devt;
 	struct list_head list;
 	struct cdev heap_cdev;
+	struct device *heap_dev;
 };
 
 static LIST_HEAD(heap_list);
@@ -223,6 +225,19 @@ const char *dma_heap_get_name(struct dma_heap *heap)
 }
 EXPORT_SYMBOL_NS_GPL(dma_heap_get_name, "DMA_BUF_HEAP");
 
+/**
+ * dma_heap_get_dev() - get device struct for the heap
+ * @heap: DMA-Heap to retrieve device struct from
+ *
+ * Returns:
+ * The device struct for the heap.
+ */
+struct device *dma_heap_get_dev(struct dma_heap *heap)
+{
+	return heap->heap_dev;
+}
+EXPORT_SYMBOL_NS_GPL(dma_heap_get_dev, "DMA_BUF_HEAP");
+
 /**
  * dma_heap_add - adds a heap to dmabuf heaps
  * @exp_info: information needed to register this heap
@@ -230,7 +245,6 @@ EXPORT_SYMBOL_NS_GPL(dma_heap_get_name, "DMA_BUF_HEAP");
 struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
 {
 	struct dma_heap *heap, *h, *err_ret;
-	struct device *dev_ret;
 	unsigned int minor;
 	int ret;
 
@@ -272,14 +286,14 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
 		goto err1;
 	}
 
-	dev_ret = device_create(dma_heap_class,
-				NULL,
-				heap->heap_devt,
-				NULL,
-				heap->name);
-	if (IS_ERR(dev_ret)) {
+	heap->heap_dev = device_create(dma_heap_class,
+				       NULL,
+				       heap->heap_devt,
+				       NULL,
+				       heap->name);
+	if (IS_ERR(heap->heap_dev)) {
 		pr_err("dma_heap: Unable to create device\n");
-		err_ret = ERR_CAST(dev_ret);
+		err_ret = ERR_CAST(heap->heap_dev);
 		goto err2;
 	}
 
@@ -295,6 +309,10 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
 		}
 	}
 
+	/* Make sure it doesn't disappear on us */
+	heap->heap_dev = get_device(heap->heap_dev);
+
+
 	/* Add heap to the list */
 	list_add(&heap->list, &heap_list);
 	mutex_unlock(&heap_list_lock);
diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
index 648328a64b27e..493085e69b70e 100644
--- a/include/linux/dma-heap.h
+++ b/include/linux/dma-heap.h
@@ -12,6 +12,7 @@
 #include <linux/types.h>
 
 struct dma_heap;
+struct device;
 
 /**
  * struct dma_heap_ops - ops to operate on a given heap
@@ -43,6 +44,7 @@ struct dma_heap_export_info {
 void *dma_heap_get_drvdata(struct dma_heap *heap);
 
 const char *dma_heap_get_name(struct dma_heap *heap);
+struct device *dma_heap_get_dev(struct dma_heap *heap);
 
 struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info);
 

-- 
2.52.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 2/6] dma-buf: dma-heap: split dma_heap_add
  2026-03-06 10:36 [PATCH v3 0/6] dma-buf: heaps: add coherent reserved-memory heap Albert Esteve
  2026-03-06 10:36 ` [PATCH v3 1/6] dma-buf: dma-heap: Keep track of the heap device struct Albert Esteve
@ 2026-03-06 10:36 ` Albert Esteve
  2026-03-06 10:36 ` [PATCH v3 3/6] of_reserved_mem: add a helper for rmem device_init op Albert Esteve
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Albert Esteve @ 2026-03-06 10:36 UTC (permalink / raw)
  To: Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz,
	T.J. Mercier, Christian König, Marek Szyprowski,
	Robin Murphy, Rob Herring, Saravana Kannan
  Cc: linux-kernel, linux-media, dri-devel, linaro-mm-sig, iommu,
	devicetree, Albert Esteve, mripard, echanude

Split dma_heap_add() into creation and registration
phases while preserving the ordering between
cdev_add() and device_add(), and ensuring all
device fields are initialised.

This lets callers build a heap and its device,
bind reserved memory, and cleanly unwind on failure
before the heap is registered. It also avoids a window
where userspace can see a heap that exists but isn’t
fully functional. The coherent heap will need this to
bind rmem to the heap device prior to registration.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 drivers/dma-buf/dma-heap.c | 126 +++++++++++++++++++++++++++++++++++----------
 include/linux/dma-heap.h   |   3 ++
 2 files changed, 103 insertions(+), 26 deletions(-)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 1124d63eb1398..ba87e5ac16ae2 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -238,15 +238,30 @@ struct device *dma_heap_get_dev(struct dma_heap *heap)
 }
 EXPORT_SYMBOL_NS_GPL(dma_heap_get_dev, "DMA_BUF_HEAP");
 
+static void dma_heap_dev_release(struct device *dev)
+{
+	struct dma_heap *heap;
+
+	pr_debug("heap device: '%s': %s\n", dev_name(dev), __func__);
+	heap = dev_get_drvdata(dev);
+	kfree(heap->name);
+	kfree(heap);
+	kfree(dev);
+}
+
 /**
- * dma_heap_add - adds a heap to dmabuf heaps
- * @exp_info: information needed to register this heap
+ * dma_heap_create() - allocate and initialize a heap object
+ * @exp_info: information needed to create a heap
+ *
+ * Creates a heap instance but does not register it or create device nodes.
+ * Use dma_heap_register() to make it visible to userspace, or
+ * dma_heap_destroy() to release it.
+ *
+ * Returns a heap on success or ERR_PTR(-errno) on failure.
  */
-struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
+struct dma_heap *dma_heap_create(const struct dma_heap_export_info *exp_info)
 {
-	struct dma_heap *heap, *h, *err_ret;
-	unsigned int minor;
-	int ret;
+	struct dma_heap *heap;
 
 	if (!exp_info->name || !strcmp(exp_info->name, "")) {
 		pr_err("dma_heap: Cannot add heap without a name\n");
@@ -265,13 +280,41 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
 	heap->name = exp_info->name;
 	heap->ops = exp_info->ops;
 	heap->priv = exp_info->priv;
+	heap->heap_dev = kzalloc_obj(*heap->heap_dev);
+	if (!heap->heap_dev) {
+		kfree(heap);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	device_initialize(heap->heap_dev);
+	dev_set_drvdata(heap->heap_dev, heap);
+
+	dev_set_name(heap->heap_dev, heap->name);
+	heap->heap_dev->class = dma_heap_class;
+	heap->heap_dev->release = dma_heap_dev_release;
+
+	return heap;
+}
+EXPORT_SYMBOL_NS_GPL(dma_heap_create, "DMA_BUF_HEAP");
+
+/**
+ * dma_heap_register() - register a heap with the dma-heap framework
+ * @heap: heap instance created with dma_heap_create()
+ *
+ * Registers the heap, creating its device node and adding it to the heap
+ * list. Returns 0 on success or a negative error code on failure.
+ */
+int dma_heap_register(struct dma_heap *heap)
+{
+	struct dma_heap *h;
+	unsigned int minor;
+	int ret;
 
 	/* Find unused minor number */
 	ret = xa_alloc(&dma_heap_minors, &minor, heap,
 		       XA_LIMIT(0, NUM_HEAP_MINORS - 1), GFP_KERNEL);
 	if (ret < 0) {
 		pr_err("dma_heap: Unable to get minor number for heap\n");
-		err_ret = ERR_PTR(ret);
 		goto err0;
 	}
 
@@ -282,42 +325,34 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
 	ret = cdev_add(&heap->heap_cdev, heap->heap_devt, 1);
 	if (ret < 0) {
 		pr_err("dma_heap: Unable to add char device\n");
-		err_ret = ERR_PTR(ret);
 		goto err1;
 	}
 
-	heap->heap_dev = device_create(dma_heap_class,
-				       NULL,
-				       heap->heap_devt,
-				       NULL,
-				       heap->name);
-	if (IS_ERR(heap->heap_dev)) {
-		pr_err("dma_heap: Unable to create device\n");
-		err_ret = ERR_CAST(heap->heap_dev);
+	heap->heap_dev->devt = heap->heap_devt;
+
+	ret = device_add(heap->heap_dev);
+	if (ret) {
+		pr_err("dma_heap: Unable to add device\n");
 		goto err2;
 	}
 
 	mutex_lock(&heap_list_lock);
 	/* check the name is unique */
 	list_for_each_entry(h, &heap_list, list) {
-		if (!strcmp(h->name, exp_info->name)) {
+		if (!strcmp(h->name, heap->name)) {
 			mutex_unlock(&heap_list_lock);
 			pr_err("dma_heap: Already registered heap named %s\n",
-			       exp_info->name);
-			err_ret = ERR_PTR(-EINVAL);
+			       heap->name);
+			ret = -EINVAL;
 			goto err3;
 		}
 	}
 
-	/* Make sure it doesn't disappear on us */
-	heap->heap_dev = get_device(heap->heap_dev);
-
-
 	/* Add heap to the list */
 	list_add(&heap->list, &heap_list);
 	mutex_unlock(&heap_list_lock);
 
-	return heap;
+	return 0;
 
 err3:
 	device_destroy(dma_heap_class, heap->heap_devt);
@@ -326,8 +361,47 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
 err1:
 	xa_erase(&dma_heap_minors, minor);
 err0:
-	kfree(heap);
-	return err_ret;
+	dma_heap_destroy(heap);
+	return ret;
+}
+EXPORT_SYMBOL_NS_GPL(dma_heap_register, "DMA_BUF_HEAP");
+
+/**
+ * dma_heap_destroy() - release a heap created by dma_heap_create()
+ * @heap: heap instance to release
+ *
+ * Drops the heap device reference; the heap and its device are freed in the
+ * device release path when the last reference is gone.
+ */
+void dma_heap_destroy(struct dma_heap *heap)
+{
+	put_device(heap->heap_dev);
+}
+EXPORT_SYMBOL_NS_GPL(dma_heap_destroy, "DMA_BUF_HEAP");
+
+/**
+ * dma_heap_add - adds a heap to dmabuf heaps
+ * @exp_info: information needed to register this heap
+ */
+struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
+{
+	struct dma_heap *heap;
+	int ret;
+
+	heap = dma_heap_create(exp_info);
+	if (IS_ERR(heap)) {
+		pr_err("dma_heap: failed to create heap (%ld)\n", PTR_ERR(heap));
+		return ERR_CAST(heap);
+	}
+
+	ret = dma_heap_register(heap);
+	if (ret) {
+		pr_err("dma_heap: failed to register heap (%d)\n", ret);
+		dma_heap_destroy(heap);
+		return ERR_PTR(ret);
+	}
+
+	return heap;
 }
 EXPORT_SYMBOL_NS_GPL(dma_heap_add, "DMA_BUF_HEAP");
 
diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
index 493085e69b70e..1b0ea43ba66c3 100644
--- a/include/linux/dma-heap.h
+++ b/include/linux/dma-heap.h
@@ -46,6 +46,9 @@ void *dma_heap_get_drvdata(struct dma_heap *heap);
 const char *dma_heap_get_name(struct dma_heap *heap);
 struct device *dma_heap_get_dev(struct dma_heap *heap);
 
+struct dma_heap *dma_heap_create(const struct dma_heap_export_info *exp_info);
+int dma_heap_register(struct dma_heap *heap);
+void dma_heap_destroy(struct dma_heap *heap);
 struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info);
 
 extern bool mem_accounting;

-- 
2.52.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 3/6] of_reserved_mem: add a helper for rmem device_init op
  2026-03-06 10:36 [PATCH v3 0/6] dma-buf: heaps: add coherent reserved-memory heap Albert Esteve
  2026-03-06 10:36 ` [PATCH v3 1/6] dma-buf: dma-heap: Keep track of the heap device struct Albert Esteve
  2026-03-06 10:36 ` [PATCH v3 2/6] dma-buf: dma-heap: split dma_heap_add Albert Esteve
@ 2026-03-06 10:36 ` Albert Esteve
  2026-03-13 23:06   ` Rob Herring
  2026-03-06 10:36 ` [PATCH v3 4/6] dma: coherent: store reserved memory coherent regions Albert Esteve
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Albert Esteve @ 2026-03-06 10:36 UTC (permalink / raw)
  To: Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz,
	T.J. Mercier, Christian König, Marek Szyprowski,
	Robin Murphy, Rob Herring, Saravana Kannan
  Cc: linux-kernel, linux-media, dri-devel, linaro-mm-sig, iommu,
	devicetree, Albert Esteve, mripard, echanude

Add a helper function wrapping internal reserved memory
device_init call and expose it externally.

Use the new helper function within of_reserved_mem_device_init_by_idx().

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 drivers/of/of_reserved_mem.c    | 68 ++++++++++++++++++++++++++---------------
 include/linux/of_reserved_mem.h |  8 +++++
 2 files changed, 52 insertions(+), 24 deletions(-)

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 1fd28f8056108..26ca871f7f919 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -605,6 +605,49 @@ struct rmem_assigned_device {
 static LIST_HEAD(of_rmem_assigned_device_list);
 static DEFINE_MUTEX(of_rmem_assigned_device_mutex);
 
+/**
+ * of_reserved_mem_device_init_with_mem() - assign reserved memory region to
+ *					    given device
+ * @dev:	Pointer to the device to configure
+ * @rmem:	Reserved memory region to assign
+ *
+ * This function assigns respective DMA-mapping operations based on the
+ * reserved memory region already provided in @rmem to the @dev device,
+ * without walking DT nodes.
+ *
+ * Returns error code or zero on success.
+ */
+int of_reserved_mem_device_init_with_mem(struct device *dev,
+					 struct reserved_mem *rmem)
+{
+	struct rmem_assigned_device *rd;
+	int ret;
+
+	if (!dev || !rmem || !rmem->ops || !rmem->ops->device_init)
+		return -EINVAL;
+
+	rd = kmalloc_obj(struct rmem_assigned_device);
+	if (!rd)
+		return -ENOMEM;
+
+	ret = rmem->ops->device_init(rmem, dev);
+	if (ret == 0) {
+		rd->dev = dev;
+		rd->rmem = rmem;
+
+		mutex_lock(&of_rmem_assigned_device_mutex);
+		list_add(&rd->list, &of_rmem_assigned_device_list);
+		mutex_unlock(&of_rmem_assigned_device_mutex);
+
+		dev_info(dev, "assigned reserved memory node %s\n", rmem->name);
+	} else {
+		kfree(rd);
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(of_reserved_mem_device_init_with_mem);
+
 /**
  * of_reserved_mem_device_init_by_idx() - assign reserved memory region to
  *					  given device
@@ -623,10 +666,8 @@ static DEFINE_MUTEX(of_rmem_assigned_device_mutex);
 int of_reserved_mem_device_init_by_idx(struct device *dev,
 				       struct device_node *np, int idx)
 {
-	struct rmem_assigned_device *rd;
 	struct device_node *target;
 	struct reserved_mem *rmem;
-	int ret;
 
 	if (!np || !dev)
 		return -EINVAL;
@@ -643,28 +684,7 @@ int of_reserved_mem_device_init_by_idx(struct device *dev,
 	rmem = of_reserved_mem_lookup(target);
 	of_node_put(target);
 
-	if (!rmem || !rmem->ops || !rmem->ops->device_init)
-		return -EINVAL;
-
-	rd = kmalloc_obj(struct rmem_assigned_device);
-	if (!rd)
-		return -ENOMEM;
-
-	ret = rmem->ops->device_init(rmem, dev);
-	if (ret == 0) {
-		rd->dev = dev;
-		rd->rmem = rmem;
-
-		mutex_lock(&of_rmem_assigned_device_mutex);
-		list_add(&rd->list, &of_rmem_assigned_device_list);
-		mutex_unlock(&of_rmem_assigned_device_mutex);
-
-		dev_info(dev, "assigned reserved memory node %s\n", rmem->name);
-	} else {
-		kfree(rd);
-	}
-
-	return ret;
+	return of_reserved_mem_device_init_with_mem(dev, rmem);
 }
 EXPORT_SYMBOL_GPL(of_reserved_mem_device_init_by_idx);
 
diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
index f573423359f48..12f7ddb7ee61f 100644
--- a/include/linux/of_reserved_mem.h
+++ b/include/linux/of_reserved_mem.h
@@ -32,6 +32,8 @@ typedef int (*reservedmem_of_init_fn)(struct reserved_mem *rmem);
 #define RESERVEDMEM_OF_DECLARE(name, compat, init)			\
 	_OF_DECLARE(reservedmem, name, compat, init, reservedmem_of_init_fn)
 
+int of_reserved_mem_device_init_with_mem(struct device *dev,
+					 struct reserved_mem *rmem);
 int of_reserved_mem_device_init_by_idx(struct device *dev,
 				       struct device_node *np, int idx);
 int of_reserved_mem_device_init_by_name(struct device *dev,
@@ -51,6 +53,12 @@ int of_reserved_mem_region_count(const struct device_node *np);
 #define RESERVEDMEM_OF_DECLARE(name, compat, init)			\
 	_OF_DECLARE_STUB(reservedmem, name, compat, init, reservedmem_of_init_fn)
 
+static inline int of_reserved_mem_device_init_with_mem(struct device *dev,
+						       struct reserved_mem *rmem)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int of_reserved_mem_device_init_by_idx(struct device *dev,
 					struct device_node *np, int idx)
 {

-- 
2.52.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 4/6] dma: coherent: store reserved memory coherent regions
  2026-03-06 10:36 [PATCH v3 0/6] dma-buf: heaps: add coherent reserved-memory heap Albert Esteve
                   ` (2 preceding siblings ...)
  2026-03-06 10:36 ` [PATCH v3 3/6] of_reserved_mem: add a helper for rmem device_init op Albert Esteve
@ 2026-03-06 10:36 ` Albert Esteve
  2026-03-06 10:36 ` [PATCH v3 5/6] dma-buf: heaps: Add Coherent heap to dmabuf heaps Albert Esteve
  2026-03-06 10:36 ` [PATCH v3 6/6] dma-buf: heaps: coherent: Turn heap into a module Albert Esteve
  5 siblings, 0 replies; 16+ messages in thread
From: Albert Esteve @ 2026-03-06 10:36 UTC (permalink / raw)
  To: Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz,
	T.J. Mercier, Christian König, Marek Szyprowski,
	Robin Murphy, Rob Herring, Saravana Kannan
  Cc: linux-kernel, linux-media, dri-devel, linaro-mm-sig, iommu,
	devicetree, Albert Esteve, mripard, echanude

Create the logic to store coherent reserved memory regions
within the coherent DMA code; and provide an iterator
(i.e., dma_coherent_get_reserved_region()) to allow
consumers of this API retrieving the regions.

Note: since the consumer of this iterator is going
to be the specific coherent memory dmabuf heap module, this
commit introduces a check for CONFIG_DMABUF_HEAPS_COHERENT,
which is defined in the subsequent patch, to maintain a
clean split between the kernel code and the heap
module code.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 include/linux/dma-map-ops.h |  7 +++++++
 kernel/dma/coherent.c       | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
index 60b63756df821..c87e5e44e5383 100644
--- a/include/linux/dma-map-ops.h
+++ b/include/linux/dma-map-ops.h
@@ -12,6 +12,7 @@
 
 struct cma;
 struct iommu_ops;
+struct reserved_mem;
 
 struct dma_map_ops {
 	void *(*alloc)(struct device *dev, size_t size,
@@ -161,6 +162,7 @@ int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
 int dma_release_from_dev_coherent(struct device *dev, int order, void *vaddr);
 int dma_mmap_from_dev_coherent(struct device *dev, struct vm_area_struct *vma,
 		void *cpu_addr, size_t size, int *ret);
+struct reserved_mem *dma_coherent_get_reserved_region(unsigned int idx);
 #else
 static inline int dma_declare_coherent_memory(struct device *dev,
 		phys_addr_t phys_addr, dma_addr_t device_addr, size_t size)
@@ -172,6 +174,11 @@ static inline int dma_declare_coherent_memory(struct device *dev,
 #define dma_release_from_dev_coherent(dev, order, vaddr) (0)
 #define dma_mmap_from_dev_coherent(dev, vma, vaddr, order, ret) (0)
 static inline void dma_release_coherent_memory(struct device *dev) { }
+static inline
+struct reserved_mem *dma_coherent_get_reserved_region(unsigned int idx)
+{
+	return NULL;
+}
 #endif /* CONFIG_DMA_DECLARE_COHERENT */
 
 #ifdef CONFIG_DMA_GLOBAL_POOL
diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
index 1147497bc512c..d0d0979ffb153 100644
--- a/kernel/dma/coherent.c
+++ b/kernel/dma/coherent.c
@@ -9,6 +9,7 @@
 #include <linux/module.h>
 #include <linux/dma-direct.h>
 #include <linux/dma-map-ops.h>
+#include <linux/dma-heap.h>
 
 struct dma_coherent_mem {
 	void		*virt_base;
@@ -334,6 +335,31 @@ static phys_addr_t dma_reserved_default_memory_base __initdata;
 static phys_addr_t dma_reserved_default_memory_size __initdata;
 #endif
 
+#define MAX_COHERENT_REGIONS 64
+
+static struct reserved_mem *rmem_coherent_areas[MAX_COHERENT_REGIONS];
+static unsigned int rmem_coherent_areas_num;
+
+static int rmem_coherent_insert_area(struct reserved_mem *rmem)
+{
+	if (rmem_coherent_areas_num >= MAX_COHERENT_REGIONS) {
+		pr_warn("Deferred heap areas list full, dropping %s\n",
+			rmem->name ? rmem->name : "unknown");
+		return -EINVAL;
+	}
+	rmem_coherent_areas[rmem_coherent_areas_num++] = rmem;
+	return 0;
+}
+
+struct reserved_mem *dma_coherent_get_reserved_region(unsigned int idx)
+{
+	if (idx >= rmem_coherent_areas_num)
+		return NULL;
+
+	return rmem_coherent_areas[idx];
+}
+EXPORT_SYMBOL_GPL(dma_coherent_get_reserved_region);
+
 static int rmem_dma_device_init(struct reserved_mem *rmem, struct device *dev)
 {
 	struct dma_coherent_mem *mem = rmem->priv;
@@ -393,6 +419,14 @@ static int __init rmem_dma_setup(struct reserved_mem *rmem)
 	rmem->ops = &rmem_dma_ops;
 	pr_info("Reserved memory: created DMA memory pool at %pa, size %ld MiB\n",
 		&rmem->base, (unsigned long)rmem->size / SZ_1M);
+
+	if (IS_ENABLED(CONFIG_DMABUF_HEAPS_COHERENT)) {
+		int ret = rmem_coherent_insert_area(rmem);
+
+		if (ret)
+			pr_warn("Reserved memory: failed to store coherent area for %s (%d)\n",
+				rmem->name ? rmem->name : "unknown", ret);
+	}
 	return 0;
 }
 

-- 
2.52.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 5/6] dma-buf: heaps: Add Coherent heap to dmabuf heaps
  2026-03-06 10:36 [PATCH v3 0/6] dma-buf: heaps: add coherent reserved-memory heap Albert Esteve
                   ` (3 preceding siblings ...)
  2026-03-06 10:36 ` [PATCH v3 4/6] dma: coherent: store reserved memory coherent regions Albert Esteve
@ 2026-03-06 10:36 ` Albert Esteve
  2026-03-10 15:34   ` Andrew Davis
  2026-03-06 10:36 ` [PATCH v3 6/6] dma-buf: heaps: coherent: Turn heap into a module Albert Esteve
  5 siblings, 1 reply; 16+ messages in thread
From: Albert Esteve @ 2026-03-06 10:36 UTC (permalink / raw)
  To: Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz,
	T.J. Mercier, Christian König, Marek Szyprowski,
	Robin Murphy, Rob Herring, Saravana Kannan
  Cc: linux-kernel, linux-media, dri-devel, linaro-mm-sig, iommu,
	devicetree, Albert Esteve, mripard, echanude

Expose DT coherent reserved-memory pools ("shared-dma-pool"
without "reusable") as dma-buf heaps, creating one heap per
region so userspace can allocate from the exact device-local
pool intended for coherent DMA.

This is a missing backend in the long-term effort to steer
userspace buffer allocations (DRM, v4l2, dma-buf heaps)
through heaps for clearer cgroup accounting. CMA and system
heaps already exist; non-reusable coherent reserved memory
did not.

The heap binds the heap device to each memory region so
coherent allocations use the correct dev->dma_mem, and
it defers registration until module_init when normal
allocators are available.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 drivers/dma-buf/heaps/Kconfig         |   9 +
 drivers/dma-buf/heaps/Makefile        |   1 +
 drivers/dma-buf/heaps/coherent_heap.c | 414 ++++++++++++++++++++++++++++++++++
 3 files changed, 424 insertions(+)

diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
index a5eef06c42264..aeb475e585048 100644
--- a/drivers/dma-buf/heaps/Kconfig
+++ b/drivers/dma-buf/heaps/Kconfig
@@ -12,3 +12,12 @@ config DMABUF_HEAPS_CMA
 	  Choose this option to enable dma-buf CMA heap. This heap is backed
 	  by the Contiguous Memory Allocator (CMA). If your system has these
 	  regions, you should say Y here.
+
+config DMABUF_HEAPS_COHERENT
+	bool "DMA-BUF Coherent Reserved-Memory Heap"
+	depends on DMABUF_HEAPS && OF_RESERVED_MEM && DMA_DECLARE_COHERENT
+	help
+	  Choose this option to enable coherent reserved-memory dma-buf heaps.
+	  This heap is backed by non-reusable DT "shared-dma-pool" regions.
+	  If your system defines coherent reserved-memory regions, you should
+	  say Y here.
diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
index 974467791032f..96bda7a65f041 100644
--- a/drivers/dma-buf/heaps/Makefile
+++ b/drivers/dma-buf/heaps/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)	+= system_heap.o
 obj-$(CONFIG_DMABUF_HEAPS_CMA)		+= cma_heap.o
+obj-$(CONFIG_DMABUF_HEAPS_COHERENT)	+= coherent_heap.o
diff --git a/drivers/dma-buf/heaps/coherent_heap.c b/drivers/dma-buf/heaps/coherent_heap.c
new file mode 100644
index 0000000000000..55f53f87c4c15
--- /dev/null
+++ b/drivers/dma-buf/heaps/coherent_heap.c
@@ -0,0 +1,414 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DMABUF heap for coherent reserved-memory regions
+ *
+ * Copyright (C) 2026 Red Hat, Inc.
+ * Author: Albert Esteve <aesteve@redhat.com>
+ *
+ */
+
+#include <linux/dma-buf.h>
+#include <linux/dma-heap.h>
+#include <linux/dma-map-ops.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
+#include <linux/highmem.h>
+#include <linux/iosys-map.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/scatterlist.h>
+#include <linux/slab.h>
+#include <linux/vmalloc.h>
+
+struct coherent_heap {
+	struct dma_heap *heap;
+	struct reserved_mem *rmem;
+	char *name;
+};
+
+struct coherent_heap_buffer {
+	struct coherent_heap *heap;
+	struct list_head attachments;
+	struct mutex lock;
+	unsigned long len;
+	dma_addr_t dma_addr;
+	void *alloc_vaddr;
+	struct page **pages;
+	pgoff_t pagecount;
+	int vmap_cnt;
+	void *vaddr;
+};
+
+struct dma_heap_attachment {
+	struct device *dev;
+	struct sg_table table;
+	struct list_head list;
+	bool mapped;
+};
+
+static int coherent_heap_attach(struct dma_buf *dmabuf,
+				struct dma_buf_attachment *attachment)
+{
+	struct coherent_heap_buffer *buffer = dmabuf->priv;
+	struct dma_heap_attachment *a;
+	int ret;
+
+	a = kzalloc_obj(*a);
+	if (!a)
+		return -ENOMEM;
+
+	ret = sg_alloc_table_from_pages(&a->table, buffer->pages,
+					buffer->pagecount, 0,
+					buffer->pagecount << PAGE_SHIFT,
+					GFP_KERNEL);
+	if (ret) {
+		kfree(a);
+		return ret;
+	}
+
+	a->dev = attachment->dev;
+	INIT_LIST_HEAD(&a->list);
+	a->mapped = false;
+
+	attachment->priv = a;
+
+	mutex_lock(&buffer->lock);
+	list_add(&a->list, &buffer->attachments);
+	mutex_unlock(&buffer->lock);
+
+	return 0;
+}
+
+static void coherent_heap_detach(struct dma_buf *dmabuf,
+				 struct dma_buf_attachment *attachment)
+{
+	struct coherent_heap_buffer *buffer = dmabuf->priv;
+	struct dma_heap_attachment *a = attachment->priv;
+
+	mutex_lock(&buffer->lock);
+	list_del(&a->list);
+	mutex_unlock(&buffer->lock);
+
+	sg_free_table(&a->table);
+	kfree(a);
+}
+
+static struct sg_table *coherent_heap_map_dma_buf(struct dma_buf_attachment *attachment,
+						  enum dma_data_direction direction)
+{
+	struct dma_heap_attachment *a = attachment->priv;
+	struct sg_table *table = &a->table;
+	int ret;
+
+	ret = dma_map_sgtable(attachment->dev, table, direction, 0);
+	if (ret)
+		return ERR_PTR(-ENOMEM);
+	a->mapped = true;
+
+	return table;
+}
+
+static void coherent_heap_unmap_dma_buf(struct dma_buf_attachment *attachment,
+					struct sg_table *table,
+					enum dma_data_direction direction)
+{
+	struct dma_heap_attachment *a = attachment->priv;
+
+	a->mapped = false;
+	dma_unmap_sgtable(attachment->dev, table, direction, 0);
+}
+
+static int coherent_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
+						  enum dma_data_direction direction)
+{
+	struct coherent_heap_buffer *buffer = dmabuf->priv;
+	struct dma_heap_attachment *a;
+
+	mutex_lock(&buffer->lock);
+	if (buffer->vmap_cnt)
+		invalidate_kernel_vmap_range(buffer->vaddr, buffer->len);
+
+	list_for_each_entry(a, &buffer->attachments, list) {
+		if (!a->mapped)
+			continue;
+		dma_sync_sgtable_for_cpu(a->dev, &a->table, direction);
+	}
+	mutex_unlock(&buffer->lock);
+
+	return 0;
+}
+
+static int coherent_heap_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
+						enum dma_data_direction direction)
+{
+	struct coherent_heap_buffer *buffer = dmabuf->priv;
+	struct dma_heap_attachment *a;
+
+	mutex_lock(&buffer->lock);
+	if (buffer->vmap_cnt)
+		flush_kernel_vmap_range(buffer->vaddr, buffer->len);
+
+	list_for_each_entry(a, &buffer->attachments, list) {
+		if (!a->mapped)
+			continue;
+		dma_sync_sgtable_for_device(a->dev, &a->table, direction);
+	}
+	mutex_unlock(&buffer->lock);
+
+	return 0;
+}
+
+static int coherent_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
+{
+	struct coherent_heap_buffer *buffer = dmabuf->priv;
+	struct coherent_heap *coh_heap = buffer->heap;
+	struct device *heap_dev = dma_heap_get_dev(coh_heap->heap);
+
+	return dma_mmap_coherent(heap_dev, vma, buffer->alloc_vaddr,
+				 buffer->dma_addr, buffer->len);
+}
+
+static void *coherent_heap_do_vmap(struct coherent_heap_buffer *buffer)
+{
+	void *vaddr;
+
+	vaddr = vmap(buffer->pages, buffer->pagecount, VM_MAP, PAGE_KERNEL);
+	if (!vaddr)
+		return ERR_PTR(-ENOMEM);
+
+	return vaddr;
+}
+
+static int coherent_heap_vmap(struct dma_buf *dmabuf, struct iosys_map *map)
+{
+	struct coherent_heap_buffer *buffer = dmabuf->priv;
+	void *vaddr;
+	int ret = 0;
+
+	mutex_lock(&buffer->lock);
+	if (buffer->vmap_cnt) {
+		buffer->vmap_cnt++;
+		iosys_map_set_vaddr(map, buffer->vaddr);
+		goto out;
+	}
+
+	vaddr = coherent_heap_do_vmap(buffer);
+	if (IS_ERR(vaddr)) {
+		ret = PTR_ERR(vaddr);
+		goto out;
+	}
+
+	buffer->vaddr = vaddr;
+	buffer->vmap_cnt++;
+	iosys_map_set_vaddr(map, buffer->vaddr);
+out:
+	mutex_unlock(&buffer->lock);
+
+	return ret;
+}
+
+static void coherent_heap_vunmap(struct dma_buf *dmabuf, struct iosys_map *map)
+{
+	struct coherent_heap_buffer *buffer = dmabuf->priv;
+
+	mutex_lock(&buffer->lock);
+	if (!--buffer->vmap_cnt) {
+		vunmap(buffer->vaddr);
+		buffer->vaddr = NULL;
+	}
+	mutex_unlock(&buffer->lock);
+	iosys_map_clear(map);
+}
+
+static void coherent_heap_dma_buf_release(struct dma_buf *dmabuf)
+{
+	struct coherent_heap_buffer *buffer = dmabuf->priv;
+	struct coherent_heap *coh_heap = buffer->heap;
+	struct device *heap_dev = dma_heap_get_dev(coh_heap->heap);
+
+	if (buffer->vmap_cnt > 0) {
+		WARN(1, "%s: buffer still mapped in the kernel\n", __func__);
+		vunmap(buffer->vaddr);
+		buffer->vaddr = NULL;
+		buffer->vmap_cnt = 0;
+	}
+
+	if (buffer->alloc_vaddr)
+		dma_free_coherent(heap_dev, buffer->len, buffer->alloc_vaddr,
+				  buffer->dma_addr);
+	kfree(buffer->pages);
+	kfree(buffer);
+}
+
+static const struct dma_buf_ops coherent_heap_buf_ops = {
+	.attach = coherent_heap_attach,
+	.detach = coherent_heap_detach,
+	.map_dma_buf = coherent_heap_map_dma_buf,
+	.unmap_dma_buf = coherent_heap_unmap_dma_buf,
+	.begin_cpu_access = coherent_heap_dma_buf_begin_cpu_access,
+	.end_cpu_access = coherent_heap_dma_buf_end_cpu_access,
+	.mmap = coherent_heap_mmap,
+	.vmap = coherent_heap_vmap,
+	.vunmap = coherent_heap_vunmap,
+	.release = coherent_heap_dma_buf_release,
+};
+
+static struct dma_buf *coherent_heap_allocate(struct dma_heap *heap,
+					      unsigned long len,
+					      u32 fd_flags,
+					      u64 heap_flags)
+{
+	struct coherent_heap *coh_heap;
+	struct coherent_heap_buffer *buffer;
+	struct device *heap_dev;
+	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
+	size_t size = PAGE_ALIGN(len);
+	pgoff_t pagecount = size >> PAGE_SHIFT;
+	struct dma_buf *dmabuf;
+	int ret = -ENOMEM;
+	pgoff_t pg;
+
+	coh_heap = dma_heap_get_drvdata(heap);
+	if (!coh_heap)
+		return ERR_PTR(-EINVAL);
+
+	heap_dev = dma_heap_get_dev(coh_heap->heap);
+	if (!heap_dev)
+		return ERR_PTR(-ENODEV);
+
+	buffer = kzalloc_obj(*buffer);
+	if (!buffer)
+		return ERR_PTR(-ENOMEM);
+
+	INIT_LIST_HEAD(&buffer->attachments);
+	mutex_init(&buffer->lock);
+	buffer->len = size;
+	buffer->heap = coh_heap;
+	buffer->pagecount = pagecount;
+
+	buffer->alloc_vaddr = dma_alloc_coherent(heap_dev, buffer->len,
+						 &buffer->dma_addr, GFP_KERNEL);
+	if (!buffer->alloc_vaddr) {
+		ret = -ENOMEM;
+		goto free_buffer;
+	}
+
+	buffer->pages = kmalloc_array(pagecount, sizeof(*buffer->pages),
+				      GFP_KERNEL);
+	if (!buffer->pages) {
+		ret = -ENOMEM;
+		goto free_dma;
+	}
+
+	for (pg = 0; pg < pagecount; pg++)
+		buffer->pages[pg] = virt_to_page((char *)buffer->alloc_vaddr +
+						 (pg * PAGE_SIZE));
+
+	/* create the dmabuf */
+	exp_info.exp_name = dma_heap_get_name(heap);
+	exp_info.ops = &coherent_heap_buf_ops;
+	exp_info.size = buffer->len;
+	exp_info.flags = fd_flags;
+	exp_info.priv = buffer;
+	dmabuf = dma_buf_export(&exp_info);
+	if (IS_ERR(dmabuf)) {
+		ret = PTR_ERR(dmabuf);
+		goto free_pages;
+	}
+	return dmabuf;
+
+free_pages:
+	kfree(buffer->pages);
+free_dma:
+	dma_free_coherent(heap_dev, buffer->len, buffer->alloc_vaddr,
+			  buffer->dma_addr);
+free_buffer:
+	kfree(buffer);
+	return ERR_PTR(ret);
+}
+
+static const struct dma_heap_ops coherent_heap_ops = {
+	.allocate = coherent_heap_allocate,
+};
+
+static int __coherent_heap_register(struct reserved_mem *rmem)
+{
+	struct dma_heap_export_info exp_info;
+	struct coherent_heap *coh_heap;
+	struct device *heap_dev;
+	int ret;
+
+	if (!rmem || !rmem->name)
+		return -EINVAL;
+
+	coh_heap = kzalloc_obj(*coh_heap);
+	if (!coh_heap)
+		return -ENOMEM;
+
+	coh_heap->rmem = rmem;
+	coh_heap->name = kstrdup(rmem->name, GFP_KERNEL);
+	if (!coh_heap->name) {
+		ret = -ENOMEM;
+		goto free_coherent_heap;
+	}
+
+	exp_info.name = coh_heap->name;
+	exp_info.ops = &coherent_heap_ops;
+	exp_info.priv = coh_heap;
+
+	coh_heap->heap = dma_heap_create(&exp_info);
+	if (IS_ERR(coh_heap->heap)) {
+		ret = PTR_ERR(coh_heap->heap);
+		goto free_name;
+	}
+
+	heap_dev = dma_heap_get_dev(coh_heap->heap);
+	ret = dma_coerce_mask_and_coherent(heap_dev, DMA_BIT_MASK(64));
+	if (ret) {
+		pr_err("coherent_heap: failed to set DMA mask (%d)\n", ret);
+		goto destroy_heap;
+	}
+
+	ret = of_reserved_mem_device_init_with_mem(heap_dev, rmem);
+	if (ret) {
+		pr_err("coherent_heap: failed to initialize memory (%d)\n", ret);
+		goto destroy_heap;
+	}
+
+	ret = dma_heap_register(coh_heap->heap);
+	if (ret) {
+		pr_err("coherent_heap: failed to register heap (%d)\n", ret);
+		goto destroy_heap;
+	}
+
+	return 0;
+
+destroy_heap:
+	dma_heap_destroy(coh_heap->heap);
+	coh_heap->heap = NULL;
+free_name:
+	kfree(coh_heap->name);
+free_coherent_heap:
+	kfree(coh_heap);
+
+	return ret;
+}
+
+static int __init coherent_heap_register(void)
+{
+	struct reserved_mem *rmem;
+	unsigned int i;
+	int ret;
+
+	for (i = 0; (rmem = dma_coherent_get_reserved_region(i)) != NULL; i++) {
+		ret = __coherent_heap_register(rmem);
+		if (ret) {
+			pr_warn("Failed to add coherent heap %s",
+				rmem->name ? rmem->name : "unknown");
+			continue;
+		}
+	}
+
+	return 0;
+}
+module_init(coherent_heap_register);
+MODULE_DESCRIPTION("DMA-BUF heap for coherent reserved-memory regions");

-- 
2.52.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 6/6] dma-buf: heaps: coherent: Turn heap into a module
  2026-03-06 10:36 [PATCH v3 0/6] dma-buf: heaps: add coherent reserved-memory heap Albert Esteve
                   ` (4 preceding siblings ...)
  2026-03-06 10:36 ` [PATCH v3 5/6] dma-buf: heaps: Add Coherent heap to dmabuf heaps Albert Esteve
@ 2026-03-06 10:36 ` Albert Esteve
  5 siblings, 0 replies; 16+ messages in thread
From: Albert Esteve @ 2026-03-06 10:36 UTC (permalink / raw)
  To: Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz,
	T.J. Mercier, Christian König, Marek Szyprowski,
	Robin Murphy, Rob Herring, Saravana Kannan
  Cc: linux-kernel, linux-media, dri-devel, linaro-mm-sig, iommu,
	devicetree, Albert Esteve, mripard, echanude

Following the current efforts to make CMA heap as module,
we can do the same and turn the Coherent heap into
a module as well, by changing the Kconfig into a tristate
and importing the proper dma-buf namespaces.

This heap won't be able to unload (same as happens with
the CMA heap), since we're missing a big part of the
infrastructure that would allow to make it safe.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 drivers/dma-buf/heaps/Kconfig         | 2 +-
 drivers/dma-buf/heaps/coherent_heap.c | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
index aeb475e585048..2f84a1018b900 100644
--- a/drivers/dma-buf/heaps/Kconfig
+++ b/drivers/dma-buf/heaps/Kconfig
@@ -14,7 +14,7 @@ config DMABUF_HEAPS_CMA
 	  regions, you should say Y here.
 
 config DMABUF_HEAPS_COHERENT
-	bool "DMA-BUF Coherent Reserved-Memory Heap"
+	tristate "DMA-BUF Coherent Reserved-Memory Heap"
 	depends on DMABUF_HEAPS && OF_RESERVED_MEM && DMA_DECLARE_COHERENT
 	help
 	  Choose this option to enable coherent reserved-memory dma-buf heaps.
diff --git a/drivers/dma-buf/heaps/coherent_heap.c b/drivers/dma-buf/heaps/coherent_heap.c
index 55f53f87c4c15..fdb3f5d907e88 100644
--- a/drivers/dma-buf/heaps/coherent_heap.c
+++ b/drivers/dma-buf/heaps/coherent_heap.c
@@ -412,3 +412,6 @@ static int __init coherent_heap_register(void)
 }
 module_init(coherent_heap_register);
 MODULE_DESCRIPTION("DMA-BUF heap for coherent reserved-memory regions");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("DMA_BUF");
+MODULE_IMPORT_NS("DMA_BUF_HEAP");

-- 
2.52.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/6] dma-buf: dma-heap: Keep track of the heap device struct
  2026-03-06 10:36 ` [PATCH v3 1/6] dma-buf: dma-heap: Keep track of the heap device struct Albert Esteve
@ 2026-03-10 14:37   ` Andrew Davis
  2026-03-10 15:46     ` Albert Esteve
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Davis @ 2026-03-10 14:37 UTC (permalink / raw)
  To: Albert Esteve, Sumit Semwal, Benjamin Gaignard, Brian Starkey,
	John Stultz, T.J. Mercier, Christian König, Marek Szyprowski,
	Robin Murphy, Rob Herring, Saravana Kannan
  Cc: linux-kernel, linux-media, dri-devel, linaro-mm-sig, iommu,
	devicetree, mripard, echanude, John Stultz, Maxime Ripard

On 3/6/26 4:36 AM, Albert Esteve wrote:
> From: John Stultz <john.stultz@linaro.org>
> 
> Keep track of the heap device struct.
> 
> This will be useful for special DMA allocations
> and actions.
> 
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> Reviewed-by: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
>   drivers/dma-buf/dma-heap.c | 34 ++++++++++++++++++++++++++--------
>   include/linux/dma-heap.h   |  2 ++
>   2 files changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index ac5f8685a6494..1124d63eb1398 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -31,6 +31,7 @@
>    * @heap_devt:		heap device node
>    * @list:		list head connecting to list of heaps
>    * @heap_cdev:		heap char device
> + * @heap_dev:		heap device
>    *
>    * Represents a heap of memory from which buffers can be made.
>    */
> @@ -41,6 +42,7 @@ struct dma_heap {
>   	dev_t heap_devt;
>   	struct list_head list;
>   	struct cdev heap_cdev;
> +	struct device *heap_dev;
>   };
>   
>   static LIST_HEAD(heap_list);
> @@ -223,6 +225,19 @@ const char *dma_heap_get_name(struct dma_heap *heap)
>   }
>   EXPORT_SYMBOL_NS_GPL(dma_heap_get_name, "DMA_BUF_HEAP");
>   
> +/**
> + * dma_heap_get_dev() - get device struct for the heap
> + * @heap: DMA-Heap to retrieve device struct from
> + *
> + * Returns:
> + * The device struct for the heap.
> + */
> +struct device *dma_heap_get_dev(struct dma_heap *heap)
> +{
> +	return heap->heap_dev;
> +}
> +EXPORT_SYMBOL_NS_GPL(dma_heap_get_dev, "DMA_BUF_HEAP");
> +
>   /**
>    * dma_heap_add - adds a heap to dmabuf heaps
>    * @exp_info: information needed to register this heap
> @@ -230,7 +245,6 @@ EXPORT_SYMBOL_NS_GPL(dma_heap_get_name, "DMA_BUF_HEAP");
>   struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
>   {
>   	struct dma_heap *heap, *h, *err_ret;
> -	struct device *dev_ret;
>   	unsigned int minor;
>   	int ret;
>   
> @@ -272,14 +286,14 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
>   		goto err1;
>   	}
>   
> -	dev_ret = device_create(dma_heap_class,
> -				NULL,
> -				heap->heap_devt,
> -				NULL,
> -				heap->name);
> -	if (IS_ERR(dev_ret)) {
> +	heap->heap_dev = device_create(dma_heap_class,
> +				       NULL,
> +				       heap->heap_devt,
> +				       NULL,
> +				       heap->name);
> +	if (IS_ERR(heap->heap_dev)) {
>   		pr_err("dma_heap: Unable to create device\n");
> -		err_ret = ERR_CAST(dev_ret);
> +		err_ret = ERR_CAST(heap->heap_dev);
>   		goto err2;
>   	}
>   
> @@ -295,6 +309,10 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
>   		}
>   	}
>   
> +	/* Make sure it doesn't disappear on us */
> +	heap->heap_dev = get_device(heap->heap_dev);
> +
> +

Is this actually something that matters or could happen? Seems you
just remove it in the next patch anyway.

Andrew

>   	/* Add heap to the list */
>   	list_add(&heap->list, &heap_list);
>   	mutex_unlock(&heap_list_lock);
> diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
> index 648328a64b27e..493085e69b70e 100644
> --- a/include/linux/dma-heap.h
> +++ b/include/linux/dma-heap.h
> @@ -12,6 +12,7 @@
>   #include <linux/types.h>
>   
>   struct dma_heap;
> +struct device;
>   
>   /**
>    * struct dma_heap_ops - ops to operate on a given heap
> @@ -43,6 +44,7 @@ struct dma_heap_export_info {
>   void *dma_heap_get_drvdata(struct dma_heap *heap);
>   
>   const char *dma_heap_get_name(struct dma_heap *heap);
> +struct device *dma_heap_get_dev(struct dma_heap *heap);
>   
>   struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info);
>   
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 5/6] dma-buf: heaps: Add Coherent heap to dmabuf heaps
  2026-03-06 10:36 ` [PATCH v3 5/6] dma-buf: heaps: Add Coherent heap to dmabuf heaps Albert Esteve
@ 2026-03-10 15:34   ` Andrew Davis
  2026-03-11 10:19     ` Albert Esteve
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Davis @ 2026-03-10 15:34 UTC (permalink / raw)
  To: Albert Esteve, Sumit Semwal, Benjamin Gaignard, Brian Starkey,
	John Stultz, T.J. Mercier, Christian König, Marek Szyprowski,
	Robin Murphy, Rob Herring, Saravana Kannan
  Cc: linux-kernel, linux-media, dri-devel, linaro-mm-sig, iommu,
	devicetree, mripard, echanude

On 3/6/26 4:36 AM, Albert Esteve wrote:
> Expose DT coherent reserved-memory pools ("shared-dma-pool"
> without "reusable") as dma-buf heaps, creating one heap per
> region so userspace can allocate from the exact device-local
> pool intended for coherent DMA.
> 
> This is a missing backend in the long-term effort to steer
> userspace buffer allocations (DRM, v4l2, dma-buf heaps)
> through heaps for clearer cgroup accounting. CMA and system
> heaps already exist; non-reusable coherent reserved memory
> did not.
> 
> The heap binds the heap device to each memory region so
> coherent allocations use the correct dev->dma_mem, and
> it defers registration until module_init when normal
> allocators are available.
> 
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
>   drivers/dma-buf/heaps/Kconfig         |   9 +
>   drivers/dma-buf/heaps/Makefile        |   1 +
>   drivers/dma-buf/heaps/coherent_heap.c | 414 ++++++++++++++++++++++++++++++++++
>   3 files changed, 424 insertions(+)
> 
> diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> index a5eef06c42264..aeb475e585048 100644
> --- a/drivers/dma-buf/heaps/Kconfig
> +++ b/drivers/dma-buf/heaps/Kconfig
> @@ -12,3 +12,12 @@ config DMABUF_HEAPS_CMA
>   	  Choose this option to enable dma-buf CMA heap. This heap is backed
>   	  by the Contiguous Memory Allocator (CMA). If your system has these
>   	  regions, you should say Y here.
> +
> +config DMABUF_HEAPS_COHERENT
> +	bool "DMA-BUF Coherent Reserved-Memory Heap"
> +	depends on DMABUF_HEAPS && OF_RESERVED_MEM && DMA_DECLARE_COHERENT
> +	help
> +	  Choose this option to enable coherent reserved-memory dma-buf heaps.
> +	  This heap is backed by non-reusable DT "shared-dma-pool" regions.
> +	  If your system defines coherent reserved-memory regions, you should
> +	  say Y here.
> diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
> index 974467791032f..96bda7a65f041 100644
> --- a/drivers/dma-buf/heaps/Makefile
> +++ b/drivers/dma-buf/heaps/Makefile
> @@ -1,3 +1,4 @@
>   # SPDX-License-Identifier: GPL-2.0
>   obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)	+= system_heap.o
>   obj-$(CONFIG_DMABUF_HEAPS_CMA)		+= cma_heap.o
> +obj-$(CONFIG_DMABUF_HEAPS_COHERENT)	+= coherent_heap.o
> diff --git a/drivers/dma-buf/heaps/coherent_heap.c b/drivers/dma-buf/heaps/coherent_heap.c
> new file mode 100644
> index 0000000000000..55f53f87c4c15
> --- /dev/null
> +++ b/drivers/dma-buf/heaps/coherent_heap.c
> @@ -0,0 +1,414 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DMABUF heap for coherent reserved-memory regions
> + *
> + * Copyright (C) 2026 Red Hat, Inc.
> + * Author: Albert Esteve <aesteve@redhat.com>
> + *
> + */
> +
> +#include <linux/dma-buf.h>
> +#include <linux/dma-heap.h>
> +#include <linux/dma-map-ops.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/err.h>
> +#include <linux/highmem.h>
> +#include <linux/iosys-map.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/scatterlist.h>
> +#include <linux/slab.h>
> +#include <linux/vmalloc.h>
> +
> +struct coherent_heap {
> +	struct dma_heap *heap;
> +	struct reserved_mem *rmem;
> +	char *name;
> +};
> +
> +struct coherent_heap_buffer {
> +	struct coherent_heap *heap;
> +	struct list_head attachments;
> +	struct mutex lock;
> +	unsigned long len;
> +	dma_addr_t dma_addr;
> +	void *alloc_vaddr;
> +	struct page **pages;
> +	pgoff_t pagecount;
> +	int vmap_cnt;
> +	void *vaddr;
> +};
> +
> +struct dma_heap_attachment {
> +	struct device *dev;
> +	struct sg_table table;
> +	struct list_head list;
> +	bool mapped;
> +};
> +
> +static int coherent_heap_attach(struct dma_buf *dmabuf,
> +				struct dma_buf_attachment *attachment)
> +{
> +	struct coherent_heap_buffer *buffer = dmabuf->priv;
> +	struct dma_heap_attachment *a;
> +	int ret;
> +
> +	a = kzalloc_obj(*a);
> +	if (!a)
> +		return -ENOMEM;
> +
> +	ret = sg_alloc_table_from_pages(&a->table, buffer->pages,
> +					buffer->pagecount, 0,
> +					buffer->pagecount << PAGE_SHIFT,
> +					GFP_KERNEL);
> +	if (ret) {
> +		kfree(a);
> +		return ret;
> +	}
> +
> +	a->dev = attachment->dev;
> +	INIT_LIST_HEAD(&a->list);
> +	a->mapped = false;
> +
> +	attachment->priv = a;
> +
> +	mutex_lock(&buffer->lock);
> +	list_add(&a->list, &buffer->attachments);
> +	mutex_unlock(&buffer->lock);
> +
> +	return 0;
> +}
> +
> +static void coherent_heap_detach(struct dma_buf *dmabuf,
> +				 struct dma_buf_attachment *attachment)
> +{
> +	struct coherent_heap_buffer *buffer = dmabuf->priv;
> +	struct dma_heap_attachment *a = attachment->priv;
> +
> +	mutex_lock(&buffer->lock);
> +	list_del(&a->list);
> +	mutex_unlock(&buffer->lock);
> +
> +	sg_free_table(&a->table);
> +	kfree(a);
> +}
> +
> +static struct sg_table *coherent_heap_map_dma_buf(struct dma_buf_attachment *attachment,
> +						  enum dma_data_direction direction)
> +{
> +	struct dma_heap_attachment *a = attachment->priv;
> +	struct sg_table *table = &a->table;
> +	int ret;
> +
> +	ret = dma_map_sgtable(attachment->dev, table, direction, 0);
> +	if (ret)
> +		return ERR_PTR(-ENOMEM);
> +	a->mapped = true;
> +
> +	return table;
> +}
> +
> +static void coherent_heap_unmap_dma_buf(struct dma_buf_attachment *attachment,
> +					struct sg_table *table,
> +					enum dma_data_direction direction)
> +{
> +	struct dma_heap_attachment *a = attachment->priv;
> +
> +	a->mapped = false;
> +	dma_unmap_sgtable(attachment->dev, table, direction, 0);
> +}
> +
> +static int coherent_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> +						  enum dma_data_direction direction)
> +{
> +	struct coherent_heap_buffer *buffer = dmabuf->priv;
> +	struct dma_heap_attachment *a;
> +
> +	mutex_lock(&buffer->lock);
> +	if (buffer->vmap_cnt)
> +		invalidate_kernel_vmap_range(buffer->vaddr, buffer->len);
> +
> +	list_for_each_entry(a, &buffer->attachments, list) {
> +		if (!a->mapped)
> +			continue;
> +		dma_sync_sgtable_for_cpu(a->dev, &a->table, direction);
> +	}
> +	mutex_unlock(&buffer->lock);
> +
> +	return 0;
> +}
> +
> +static int coherent_heap_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
> +						enum dma_data_direction direction)
> +{
> +	struct coherent_heap_buffer *buffer = dmabuf->priv;
> +	struct dma_heap_attachment *a;
> +
> +	mutex_lock(&buffer->lock);
> +	if (buffer->vmap_cnt)
> +		flush_kernel_vmap_range(buffer->vaddr, buffer->len);
> +
> +	list_for_each_entry(a, &buffer->attachments, list) {
> +		if (!a->mapped)
> +			continue;
> +		dma_sync_sgtable_for_device(a->dev, &a->table, direction);
> +	}
> +	mutex_unlock(&buffer->lock);
> +
> +	return 0;
> +}
> +
> +static int coherent_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
> +{
> +	struct coherent_heap_buffer *buffer = dmabuf->priv;
> +	struct coherent_heap *coh_heap = buffer->heap;
> +	struct device *heap_dev = dma_heap_get_dev(coh_heap->heap);
> +
> +	return dma_mmap_coherent(heap_dev, vma, buffer->alloc_vaddr,
> +				 buffer->dma_addr, buffer->len);
> +}
> +
> +static void *coherent_heap_do_vmap(struct coherent_heap_buffer *buffer)
> +{
> +	void *vaddr;
> +
> +	vaddr = vmap(buffer->pages, buffer->pagecount, VM_MAP, PAGE_KERNEL);
> +	if (!vaddr)
> +		return ERR_PTR(-ENOMEM);
> +
> +	return vaddr;
> +}
> +
> +static int coherent_heap_vmap(struct dma_buf *dmabuf, struct iosys_map *map)
> +{
> +	struct coherent_heap_buffer *buffer = dmabuf->priv;
> +	void *vaddr;
> +	int ret = 0;
> +
> +	mutex_lock(&buffer->lock);
> +	if (buffer->vmap_cnt) {
> +		buffer->vmap_cnt++;
> +		iosys_map_set_vaddr(map, buffer->vaddr);
> +		goto out;
> +	}
> +
> +	vaddr = coherent_heap_do_vmap(buffer);
> +	if (IS_ERR(vaddr)) {
> +		ret = PTR_ERR(vaddr);
> +		goto out;
> +	}
> +
> +	buffer->vaddr = vaddr;
> +	buffer->vmap_cnt++;
> +	iosys_map_set_vaddr(map, buffer->vaddr);
> +out:
> +	mutex_unlock(&buffer->lock);
> +
> +	return ret;
> +}
> +
> +static void coherent_heap_vunmap(struct dma_buf *dmabuf, struct iosys_map *map)
> +{
> +	struct coherent_heap_buffer *buffer = dmabuf->priv;
> +
> +	mutex_lock(&buffer->lock);
> +	if (!--buffer->vmap_cnt) {
> +		vunmap(buffer->vaddr);
> +		buffer->vaddr = NULL;
> +	}
> +	mutex_unlock(&buffer->lock);
> +	iosys_map_clear(map);
> +}
> +
> +static void coherent_heap_dma_buf_release(struct dma_buf *dmabuf)
> +{
> +	struct coherent_heap_buffer *buffer = dmabuf->priv;
> +	struct coherent_heap *coh_heap = buffer->heap;
> +	struct device *heap_dev = dma_heap_get_dev(coh_heap->heap);
> +
> +	if (buffer->vmap_cnt > 0) {
> +		WARN(1, "%s: buffer still mapped in the kernel\n", __func__);
> +		vunmap(buffer->vaddr);
> +		buffer->vaddr = NULL;
> +		buffer->vmap_cnt = 0;
> +	}
> +
> +	if (buffer->alloc_vaddr)
> +		dma_free_coherent(heap_dev, buffer->len, buffer->alloc_vaddr,
> +				  buffer->dma_addr);
> +	kfree(buffer->pages);
> +	kfree(buffer);
> +}
> +
> +static const struct dma_buf_ops coherent_heap_buf_ops = {
> +	.attach = coherent_heap_attach,
> +	.detach = coherent_heap_detach,
> +	.map_dma_buf = coherent_heap_map_dma_buf,
> +	.unmap_dma_buf = coherent_heap_unmap_dma_buf,
> +	.begin_cpu_access = coherent_heap_dma_buf_begin_cpu_access,
> +	.end_cpu_access = coherent_heap_dma_buf_end_cpu_access,
> +	.mmap = coherent_heap_mmap,
> +	.vmap = coherent_heap_vmap,
> +	.vunmap = coherent_heap_vunmap,
> +	.release = coherent_heap_dma_buf_release,
> +};
> +
> +static struct dma_buf *coherent_heap_allocate(struct dma_heap *heap,
> +					      unsigned long len,
> +					      u32 fd_flags,
> +					      u64 heap_flags)
> +{
> +	struct coherent_heap *coh_heap;
> +	struct coherent_heap_buffer *buffer;
> +	struct device *heap_dev;
> +	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> +	size_t size = PAGE_ALIGN(len);
> +	pgoff_t pagecount = size >> PAGE_SHIFT;
> +	struct dma_buf *dmabuf;
> +	int ret = -ENOMEM;
> +	pgoff_t pg;
> +
> +	coh_heap = dma_heap_get_drvdata(heap);
> +	if (!coh_heap)
> +		return ERR_PTR(-EINVAL);
> +
> +	heap_dev = dma_heap_get_dev(coh_heap->heap);
> +	if (!heap_dev)
> +		return ERR_PTR(-ENODEV);
> +
> +	buffer = kzalloc_obj(*buffer);
> +	if (!buffer)
> +		return ERR_PTR(-ENOMEM);
> +
> +	INIT_LIST_HEAD(&buffer->attachments);
> +	mutex_init(&buffer->lock);
> +	buffer->len = size;
> +	buffer->heap = coh_heap;
> +	buffer->pagecount = pagecount;
> +
> +	buffer->alloc_vaddr = dma_alloc_coherent(heap_dev, buffer->len,
> +						 &buffer->dma_addr, GFP_KERNEL);

You are doing this DMA allocation using a non-DMA pseudo-device (heap_dev).
This is why you need to do that dma_coerce_mask_and_coherent(64) nonsense, you
are doing a DMA alloc for the CPU itself. This might still work, but only if
dma_map_sgtable() can handle swiotlb/iommu for all attaching devices at map
time.

> +	if (!buffer->alloc_vaddr) {
> +		ret = -ENOMEM;
> +		goto free_buffer;
> +	}
> +
> +	buffer->pages = kmalloc_array(pagecount, sizeof(*buffer->pages),
> +				      GFP_KERNEL);
> +	if (!buffer->pages) {
> +		ret = -ENOMEM;
> +		goto free_dma;
> +	}
> +
> +	for (pg = 0; pg < pagecount; pg++)
> +		buffer->pages[pg] = virt_to_page((char *)buffer->alloc_vaddr +
> +						 (pg * PAGE_SIZE));
> +

Is any of this valid if the coherent pool in DT was marked "no-map;"?
I'm sure the .mmap and .cpu_access function are not valid in that case.
Our (TI) evil vendor tree version of this heap sets a flag in that case and
avoids doing anything invalid when the region doesn't have normal backing
page structs. This region is treated more like a P2PDMA area in that case.

https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/tree/drivers/dma-buf/heaps/carveout-heap.c?h=ti-linux-6.18.y#n372

Andrew

> +	/* create the dmabuf */
> +	exp_info.exp_name = dma_heap_get_name(heap);
> +	exp_info.ops = &coherent_heap_buf_ops;
> +	exp_info.size = buffer->len;
> +	exp_info.flags = fd_flags;
> +	exp_info.priv = buffer;
> +	dmabuf = dma_buf_export(&exp_info);
> +	if (IS_ERR(dmabuf)) {
> +		ret = PTR_ERR(dmabuf);
> +		goto free_pages;
> +	}
> +	return dmabuf;
> +
> +free_pages:
> +	kfree(buffer->pages);
> +free_dma:
> +	dma_free_coherent(heap_dev, buffer->len, buffer->alloc_vaddr,
> +			  buffer->dma_addr);
> +free_buffer:
> +	kfree(buffer);
> +	return ERR_PTR(ret);
> +}
> +
> +static const struct dma_heap_ops coherent_heap_ops = {
> +	.allocate = coherent_heap_allocate,
> +};
> +
> +static int __coherent_heap_register(struct reserved_mem *rmem)
> +{
> +	struct dma_heap_export_info exp_info;
> +	struct coherent_heap *coh_heap;
> +	struct device *heap_dev;
> +	int ret;
> +
> +	if (!rmem || !rmem->name)
> +		return -EINVAL;
> +
> +	coh_heap = kzalloc_obj(*coh_heap);
> +	if (!coh_heap)
> +		return -ENOMEM;
> +
> +	coh_heap->rmem = rmem;
> +	coh_heap->name = kstrdup(rmem->name, GFP_KERNEL);
> +	if (!coh_heap->name) {
> +		ret = -ENOMEM;
> +		goto free_coherent_heap;
> +	}
> +
> +	exp_info.name = coh_heap->name;
> +	exp_info.ops = &coherent_heap_ops;
> +	exp_info.priv = coh_heap;
> +
> +	coh_heap->heap = dma_heap_create(&exp_info);
> +	if (IS_ERR(coh_heap->heap)) {
> +		ret = PTR_ERR(coh_heap->heap);
> +		goto free_name;
> +	}
> +
> +	heap_dev = dma_heap_get_dev(coh_heap->heap);
> +	ret = dma_coerce_mask_and_coherent(heap_dev, DMA_BIT_MASK(64));
> +	if (ret) {
> +		pr_err("coherent_heap: failed to set DMA mask (%d)\n", ret);
> +		goto destroy_heap;
> +	}
> +
> +	ret = of_reserved_mem_device_init_with_mem(heap_dev, rmem);
> +	if (ret) {
> +		pr_err("coherent_heap: failed to initialize memory (%d)\n", ret);
> +		goto destroy_heap;
> +	}
> +
> +	ret = dma_heap_register(coh_heap->heap);
> +	if (ret) {
> +		pr_err("coherent_heap: failed to register heap (%d)\n", ret);
> +		goto destroy_heap;
> +	}
> +
> +	return 0;
> +
> +destroy_heap:
> +	dma_heap_destroy(coh_heap->heap);
> +	coh_heap->heap = NULL;
> +free_name:
> +	kfree(coh_heap->name);
> +free_coherent_heap:
> +	kfree(coh_heap);
> +
> +	return ret;
> +}
> +
> +static int __init coherent_heap_register(void)
> +{
> +	struct reserved_mem *rmem;
> +	unsigned int i;
> +	int ret;
> +
> +	for (i = 0; (rmem = dma_coherent_get_reserved_region(i)) != NULL; i++) {
> +		ret = __coherent_heap_register(rmem);
> +		if (ret) {
> +			pr_warn("Failed to add coherent heap %s",
> +				rmem->name ? rmem->name : "unknown");
> +			continue;
> +		}
> +	}
> +
> +	return 0;
> +}
> +module_init(coherent_heap_register);
> +MODULE_DESCRIPTION("DMA-BUF heap for coherent reserved-memory regions");
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/6] dma-buf: dma-heap: Keep track of the heap device struct
  2026-03-10 14:37   ` Andrew Davis
@ 2026-03-10 15:46     ` Albert Esteve
  0 siblings, 0 replies; 16+ messages in thread
From: Albert Esteve @ 2026-03-10 15:46 UTC (permalink / raw)
  To: Andrew Davis
  Cc: Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz,
	T.J. Mercier, Christian König, Marek Szyprowski,
	Robin Murphy, Rob Herring, Saravana Kannan, linux-kernel,
	linux-media, dri-devel, linaro-mm-sig, iommu, devicetree, mripard,
	echanude, John Stultz, Maxime Ripard

On Tue, Mar 10, 2026 at 3:38 PM Andrew Davis <afd@ti.com> wrote:
>
> On 3/6/26 4:36 AM, Albert Esteve wrote:
> > From: John Stultz <john.stultz@linaro.org>
> >
> > Keep track of the heap device struct.
> >
> > This will be useful for special DMA allocations
> > and actions.
> >
> > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > Reviewed-by: Maxime Ripard <mripard@kernel.org>
> > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > ---
> >   drivers/dma-buf/dma-heap.c | 34 ++++++++++++++++++++++++++--------
> >   include/linux/dma-heap.h   |  2 ++
> >   2 files changed, 28 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> > index ac5f8685a6494..1124d63eb1398 100644
> > --- a/drivers/dma-buf/dma-heap.c
> > +++ b/drivers/dma-buf/dma-heap.c
> > @@ -31,6 +31,7 @@
> >    * @heap_devt:              heap device node
> >    * @list:           list head connecting to list of heaps
> >    * @heap_cdev:              heap char device
> > + * @heap_dev:                heap device
> >    *
> >    * Represents a heap of memory from which buffers can be made.
> >    */
> > @@ -41,6 +42,7 @@ struct dma_heap {
> >       dev_t heap_devt;
> >       struct list_head list;
> >       struct cdev heap_cdev;
> > +     struct device *heap_dev;
> >   };
> >
> >   static LIST_HEAD(heap_list);
> > @@ -223,6 +225,19 @@ const char *dma_heap_get_name(struct dma_heap *heap)
> >   }
> >   EXPORT_SYMBOL_NS_GPL(dma_heap_get_name, "DMA_BUF_HEAP");
> >
> > +/**
> > + * dma_heap_get_dev() - get device struct for the heap
> > + * @heap: DMA-Heap to retrieve device struct from
> > + *
> > + * Returns:
> > + * The device struct for the heap.
> > + */
> > +struct device *dma_heap_get_dev(struct dma_heap *heap)
> > +{
> > +     return heap->heap_dev;
> > +}
> > +EXPORT_SYMBOL_NS_GPL(dma_heap_get_dev, "DMA_BUF_HEAP");
> > +
> >   /**
> >    * dma_heap_add - adds a heap to dmabuf heaps
> >    * @exp_info: information needed to register this heap
> > @@ -230,7 +245,6 @@ EXPORT_SYMBOL_NS_GPL(dma_heap_get_name, "DMA_BUF_HEAP");
> >   struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
> >   {
> >       struct dma_heap *heap, *h, *err_ret;
> > -     struct device *dev_ret;
> >       unsigned int minor;
> >       int ret;
> >
> > @@ -272,14 +286,14 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
> >               goto err1;
> >       }
> >
> > -     dev_ret = device_create(dma_heap_class,
> > -                             NULL,
> > -                             heap->heap_devt,
> > -                             NULL,
> > -                             heap->name);
> > -     if (IS_ERR(dev_ret)) {
> > +     heap->heap_dev = device_create(dma_heap_class,
> > +                                    NULL,
> > +                                    heap->heap_devt,
> > +                                    NULL,
> > +                                    heap->name);
> > +     if (IS_ERR(heap->heap_dev)) {
> >               pr_err("dma_heap: Unable to create device\n");
> > -             err_ret = ERR_CAST(dev_ret);
> > +             err_ret = ERR_CAST(heap->heap_dev);
> >               goto err2;
> >       }
> >
> > @@ -295,6 +309,10 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
> >               }
> >       }
> >
> > +     /* Make sure it doesn't disappear on us */
> > +     heap->heap_dev = get_device(heap->heap_dev);
> > +
> > +
>
> Is this actually something that matters or could happen? Seems you
> just remove it in the next patch anyway.

Good question. Technically, device_add() (and therefore, also
device_create()) already increments the refcount by calling
get_device() internally. So I'm not sure if this is necessary, I just
carried it over when I picked the patch. It feels like a safeguard to
have the device owner hold an extra reference so that if other code
decreases the reference count, the heap device won't be destroyed. So,
having the extra reference causes no harm.

I dropped it in the next patch because otherwise, I would have to
account for (and drop) both references when implementing
dma_heap_destroy().

So, I'm not sure what the best pattern is here. But I do agree that I
should either remove it from both patches or keep it for both.

BR,
Albert.


>
> Andrew
>
> >       /* Add heap to the list */
> >       list_add(&heap->list, &heap_list);
> >       mutex_unlock(&heap_list_lock);
> > diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
> > index 648328a64b27e..493085e69b70e 100644
> > --- a/include/linux/dma-heap.h
> > +++ b/include/linux/dma-heap.h
> > @@ -12,6 +12,7 @@
> >   #include <linux/types.h>
> >
> >   struct dma_heap;
> > +struct device;
> >
> >   /**
> >    * struct dma_heap_ops - ops to operate on a given heap
> > @@ -43,6 +44,7 @@ struct dma_heap_export_info {
> >   void *dma_heap_get_drvdata(struct dma_heap *heap);
> >
> >   const char *dma_heap_get_name(struct dma_heap *heap);
> > +struct device *dma_heap_get_dev(struct dma_heap *heap);
> >
> >   struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info);
> >
> >
>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 5/6] dma-buf: heaps: Add Coherent heap to dmabuf heaps
  2026-03-10 15:34   ` Andrew Davis
@ 2026-03-11 10:19     ` Albert Esteve
  2026-03-11 13:18       ` Andrew Davis
  0 siblings, 1 reply; 16+ messages in thread
From: Albert Esteve @ 2026-03-11 10:19 UTC (permalink / raw)
  To: Andrew Davis
  Cc: Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz,
	T.J. Mercier, Christian König, Marek Szyprowski,
	Robin Murphy, Rob Herring, Saravana Kannan, linux-kernel,
	linux-media, dri-devel, linaro-mm-sig, iommu, devicetree, mripard,
	echanude

On Tue, Mar 10, 2026 at 4:34 PM Andrew Davis <afd@ti.com> wrote:
>
> On 3/6/26 4:36 AM, Albert Esteve wrote:
> > Expose DT coherent reserved-memory pools ("shared-dma-pool"
> > without "reusable") as dma-buf heaps, creating one heap per
> > region so userspace can allocate from the exact device-local
> > pool intended for coherent DMA.
> >
> > This is a missing backend in the long-term effort to steer
> > userspace buffer allocations (DRM, v4l2, dma-buf heaps)
> > through heaps for clearer cgroup accounting. CMA and system
> > heaps already exist; non-reusable coherent reserved memory
> > did not.
> >
> > The heap binds the heap device to each memory region so
> > coherent allocations use the correct dev->dma_mem, and
> > it defers registration until module_init when normal
> > allocators are available.
> >
> > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > ---
> >   drivers/dma-buf/heaps/Kconfig         |   9 +
> >   drivers/dma-buf/heaps/Makefile        |   1 +
> >   drivers/dma-buf/heaps/coherent_heap.c | 414 ++++++++++++++++++++++++++++++++++
> >   3 files changed, 424 insertions(+)
> >
> > diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> > index a5eef06c42264..aeb475e585048 100644
> > --- a/drivers/dma-buf/heaps/Kconfig
> > +++ b/drivers/dma-buf/heaps/Kconfig
> > @@ -12,3 +12,12 @@ config DMABUF_HEAPS_CMA
> >         Choose this option to enable dma-buf CMA heap. This heap is backed
> >         by the Contiguous Memory Allocator (CMA). If your system has these
> >         regions, you should say Y here.
> > +
> > +config DMABUF_HEAPS_COHERENT
> > +     bool "DMA-BUF Coherent Reserved-Memory Heap"
> > +     depends on DMABUF_HEAPS && OF_RESERVED_MEM && DMA_DECLARE_COHERENT
> > +     help
> > +       Choose this option to enable coherent reserved-memory dma-buf heaps.
> > +       This heap is backed by non-reusable DT "shared-dma-pool" regions.
> > +       If your system defines coherent reserved-memory regions, you should
> > +       say Y here.
> > diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
> > index 974467791032f..96bda7a65f041 100644
> > --- a/drivers/dma-buf/heaps/Makefile
> > +++ b/drivers/dma-buf/heaps/Makefile
> > @@ -1,3 +1,4 @@
> >   # SPDX-License-Identifier: GPL-2.0
> >   obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)   += system_heap.o
> >   obj-$(CONFIG_DMABUF_HEAPS_CMA)              += cma_heap.o
> > +obj-$(CONFIG_DMABUF_HEAPS_COHERENT)  += coherent_heap.o
> > diff --git a/drivers/dma-buf/heaps/coherent_heap.c b/drivers/dma-buf/heaps/coherent_heap.c
> > new file mode 100644
> > index 0000000000000..55f53f87c4c15
> > --- /dev/null
> > +++ b/drivers/dma-buf/heaps/coherent_heap.c
> > @@ -0,0 +1,414 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * DMABUF heap for coherent reserved-memory regions
> > + *
> > + * Copyright (C) 2026 Red Hat, Inc.
> > + * Author: Albert Esteve <aesteve@redhat.com>
> > + *
> > + */
> > +
> > +#include <linux/dma-buf.h>
> > +#include <linux/dma-heap.h>
> > +#include <linux/dma-map-ops.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/err.h>
> > +#include <linux/highmem.h>
> > +#include <linux/iosys-map.h>
> > +#include <linux/of_reserved_mem.h>
> > +#include <linux/scatterlist.h>
> > +#include <linux/slab.h>
> > +#include <linux/vmalloc.h>
> > +
> > +struct coherent_heap {
> > +     struct dma_heap *heap;
> > +     struct reserved_mem *rmem;
> > +     char *name;
> > +};
> > +
> > +struct coherent_heap_buffer {
> > +     struct coherent_heap *heap;
> > +     struct list_head attachments;
> > +     struct mutex lock;
> > +     unsigned long len;
> > +     dma_addr_t dma_addr;
> > +     void *alloc_vaddr;
> > +     struct page **pages;
> > +     pgoff_t pagecount;
> > +     int vmap_cnt;
> > +     void *vaddr;
> > +};
> > +
> > +struct dma_heap_attachment {
> > +     struct device *dev;
> > +     struct sg_table table;
> > +     struct list_head list;
> > +     bool mapped;
> > +};
> > +
> > +static int coherent_heap_attach(struct dma_buf *dmabuf,
> > +                             struct dma_buf_attachment *attachment)
> > +{
> > +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> > +     struct dma_heap_attachment *a;
> > +     int ret;
> > +
> > +     a = kzalloc_obj(*a);
> > +     if (!a)
> > +             return -ENOMEM;
> > +
> > +     ret = sg_alloc_table_from_pages(&a->table, buffer->pages,
> > +                                     buffer->pagecount, 0,
> > +                                     buffer->pagecount << PAGE_SHIFT,
> > +                                     GFP_KERNEL);
> > +     if (ret) {
> > +             kfree(a);
> > +             return ret;
> > +     }
> > +
> > +     a->dev = attachment->dev;
> > +     INIT_LIST_HEAD(&a->list);
> > +     a->mapped = false;
> > +
> > +     attachment->priv = a;
> > +
> > +     mutex_lock(&buffer->lock);
> > +     list_add(&a->list, &buffer->attachments);
> > +     mutex_unlock(&buffer->lock);
> > +
> > +     return 0;
> > +}
> > +
> > +static void coherent_heap_detach(struct dma_buf *dmabuf,
> > +                              struct dma_buf_attachment *attachment)
> > +{
> > +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> > +     struct dma_heap_attachment *a = attachment->priv;
> > +
> > +     mutex_lock(&buffer->lock);
> > +     list_del(&a->list);
> > +     mutex_unlock(&buffer->lock);
> > +
> > +     sg_free_table(&a->table);
> > +     kfree(a);
> > +}
> > +
> > +static struct sg_table *coherent_heap_map_dma_buf(struct dma_buf_attachment *attachment,
> > +                                               enum dma_data_direction direction)
> > +{
> > +     struct dma_heap_attachment *a = attachment->priv;
> > +     struct sg_table *table = &a->table;
> > +     int ret;
> > +
> > +     ret = dma_map_sgtable(attachment->dev, table, direction, 0);
> > +     if (ret)
> > +             return ERR_PTR(-ENOMEM);
> > +     a->mapped = true;
> > +
> > +     return table;
> > +}
> > +
> > +static void coherent_heap_unmap_dma_buf(struct dma_buf_attachment *attachment,
> > +                                     struct sg_table *table,
> > +                                     enum dma_data_direction direction)
> > +{
> > +     struct dma_heap_attachment *a = attachment->priv;
> > +
> > +     a->mapped = false;
> > +     dma_unmap_sgtable(attachment->dev, table, direction, 0);
> > +}
> > +
> > +static int coherent_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> > +                                               enum dma_data_direction direction)
> > +{
> > +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> > +     struct dma_heap_attachment *a;
> > +
> > +     mutex_lock(&buffer->lock);
> > +     if (buffer->vmap_cnt)
> > +             invalidate_kernel_vmap_range(buffer->vaddr, buffer->len);
> > +
> > +     list_for_each_entry(a, &buffer->attachments, list) {
> > +             if (!a->mapped)
> > +                     continue;
> > +             dma_sync_sgtable_for_cpu(a->dev, &a->table, direction);
> > +     }
> > +     mutex_unlock(&buffer->lock);
> > +
> > +     return 0;
> > +}
> > +
> > +static int coherent_heap_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
> > +                                             enum dma_data_direction direction)
> > +{
> > +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> > +     struct dma_heap_attachment *a;
> > +
> > +     mutex_lock(&buffer->lock);
> > +     if (buffer->vmap_cnt)
> > +             flush_kernel_vmap_range(buffer->vaddr, buffer->len);
> > +
> > +     list_for_each_entry(a, &buffer->attachments, list) {
> > +             if (!a->mapped)
> > +                     continue;
> > +             dma_sync_sgtable_for_device(a->dev, &a->table, direction);
> > +     }
> > +     mutex_unlock(&buffer->lock);
> > +
> > +     return 0;
> > +}
> > +
> > +static int coherent_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
> > +{
> > +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> > +     struct coherent_heap *coh_heap = buffer->heap;
> > +     struct device *heap_dev = dma_heap_get_dev(coh_heap->heap);
> > +
> > +     return dma_mmap_coherent(heap_dev, vma, buffer->alloc_vaddr,
> > +                              buffer->dma_addr, buffer->len);
> > +}
> > +
> > +static void *coherent_heap_do_vmap(struct coherent_heap_buffer *buffer)
> > +{
> > +     void *vaddr;
> > +
> > +     vaddr = vmap(buffer->pages, buffer->pagecount, VM_MAP, PAGE_KERNEL);
> > +     if (!vaddr)
> > +             return ERR_PTR(-ENOMEM);
> > +
> > +     return vaddr;
> > +}
> > +
> > +static int coherent_heap_vmap(struct dma_buf *dmabuf, struct iosys_map *map)
> > +{
> > +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> > +     void *vaddr;
> > +     int ret = 0;
> > +
> > +     mutex_lock(&buffer->lock);
> > +     if (buffer->vmap_cnt) {
> > +             buffer->vmap_cnt++;
> > +             iosys_map_set_vaddr(map, buffer->vaddr);
> > +             goto out;
> > +     }
> > +
> > +     vaddr = coherent_heap_do_vmap(buffer);
> > +     if (IS_ERR(vaddr)) {
> > +             ret = PTR_ERR(vaddr);
> > +             goto out;
> > +     }
> > +
> > +     buffer->vaddr = vaddr;
> > +     buffer->vmap_cnt++;
> > +     iosys_map_set_vaddr(map, buffer->vaddr);
> > +out:
> > +     mutex_unlock(&buffer->lock);
> > +
> > +     return ret;
> > +}
> > +
> > +static void coherent_heap_vunmap(struct dma_buf *dmabuf, struct iosys_map *map)
> > +{
> > +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> > +
> > +     mutex_lock(&buffer->lock);
> > +     if (!--buffer->vmap_cnt) {
> > +             vunmap(buffer->vaddr);
> > +             buffer->vaddr = NULL;
> > +     }
> > +     mutex_unlock(&buffer->lock);
> > +     iosys_map_clear(map);
> > +}
> > +
> > +static void coherent_heap_dma_buf_release(struct dma_buf *dmabuf)
> > +{
> > +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> > +     struct coherent_heap *coh_heap = buffer->heap;
> > +     struct device *heap_dev = dma_heap_get_dev(coh_heap->heap);
> > +
> > +     if (buffer->vmap_cnt > 0) {
> > +             WARN(1, "%s: buffer still mapped in the kernel\n", __func__);
> > +             vunmap(buffer->vaddr);
> > +             buffer->vaddr = NULL;
> > +             buffer->vmap_cnt = 0;
> > +     }
> > +
> > +     if (buffer->alloc_vaddr)
> > +             dma_free_coherent(heap_dev, buffer->len, buffer->alloc_vaddr,
> > +                               buffer->dma_addr);
> > +     kfree(buffer->pages);
> > +     kfree(buffer);
> > +}
> > +
> > +static const struct dma_buf_ops coherent_heap_buf_ops = {
> > +     .attach = coherent_heap_attach,
> > +     .detach = coherent_heap_detach,
> > +     .map_dma_buf = coherent_heap_map_dma_buf,
> > +     .unmap_dma_buf = coherent_heap_unmap_dma_buf,
> > +     .begin_cpu_access = coherent_heap_dma_buf_begin_cpu_access,
> > +     .end_cpu_access = coherent_heap_dma_buf_end_cpu_access,
> > +     .mmap = coherent_heap_mmap,
> > +     .vmap = coherent_heap_vmap,
> > +     .vunmap = coherent_heap_vunmap,
> > +     .release = coherent_heap_dma_buf_release,
> > +};
> > +
> > +static struct dma_buf *coherent_heap_allocate(struct dma_heap *heap,
> > +                                           unsigned long len,
> > +                                           u32 fd_flags,
> > +                                           u64 heap_flags)
> > +{
> > +     struct coherent_heap *coh_heap;
> > +     struct coherent_heap_buffer *buffer;
> > +     struct device *heap_dev;
> > +     DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> > +     size_t size = PAGE_ALIGN(len);
> > +     pgoff_t pagecount = size >> PAGE_SHIFT;
> > +     struct dma_buf *dmabuf;
> > +     int ret = -ENOMEM;
> > +     pgoff_t pg;
> > +
> > +     coh_heap = dma_heap_get_drvdata(heap);
> > +     if (!coh_heap)
> > +             return ERR_PTR(-EINVAL);
> > +
> > +     heap_dev = dma_heap_get_dev(coh_heap->heap);
> > +     if (!heap_dev)
> > +             return ERR_PTR(-ENODEV);
> > +
> > +     buffer = kzalloc_obj(*buffer);
> > +     if (!buffer)
> > +             return ERR_PTR(-ENOMEM);
> > +
> > +     INIT_LIST_HEAD(&buffer->attachments);
> > +     mutex_init(&buffer->lock);
> > +     buffer->len = size;
> > +     buffer->heap = coh_heap;
> > +     buffer->pagecount = pagecount;
> > +
> > +     buffer->alloc_vaddr = dma_alloc_coherent(heap_dev, buffer->len,
> > +                                              &buffer->dma_addr, GFP_KERNEL);
>
> You are doing this DMA allocation using a non-DMA pseudo-device (heap_dev).
> This is why you need to do that dma_coerce_mask_and_coherent(64) nonsense, you
> are doing a DMA alloc for the CPU itself. This might still work, but only if
> dma_map_sgtable() can handle swiotlb/iommu for all attaching devices at map
> time.

The concern is valid. We're allocating via a synthetic device, which
ties the allocation to that device's DMA domain. I looked deeper into
this trying to address the concern.

The approach works because dma_map_sgtable() handles both
dma_map_direct and use_dma_iommu cases in __dma_map_sg_attrs(). For
each physical address in the sg_table (extracted via sg_phys()), it
creates device-specific DMA mappings:
- For direct mapping: it checks if the address is directly accessible
(dma_capable()), and if not, it falls back to swiotlb.
- For IOMMU: it creates mappings that allow the device to access
physical addresses.

This means every attached device gets its own device-specific DMA
mapping, properly handling cases where the physical addresses are
inaccessible or have DMA constraints.

I'm not sure whether other approaches (whatever they may be) would be
better, as here we are leveraging a great part of the existing
infrastructure.

>
> > +     if (!buffer->alloc_vaddr) {
> > +             ret = -ENOMEM;
> > +             goto free_buffer;
> > +     }
> > +
> > +     buffer->pages = kmalloc_array(pagecount, sizeof(*buffer->pages),
> > +                                   GFP_KERNEL);
> > +     if (!buffer->pages) {
> > +             ret = -ENOMEM;
> > +             goto free_dma;
> > +     }
> > +
> > +     for (pg = 0; pg < pagecount; pg++)
> > +             buffer->pages[pg] = virt_to_page((char *)buffer->alloc_vaddr +
> > +                                              (pg * PAGE_SIZE));
> > +
>
> Is any of this valid if the coherent pool in DT was marked "no-map;"?
> I'm sure the .mmap and .cpu_access function are not valid in that case.
> Our (TI) evil vendor tree version of this heap sets a flag in that case and
> avoids doing anything invalid when the region doesn't have normal backing
> page structs. This region is treated more like a P2PDMA area in that case.
>
> https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/tree/drivers/dma-buf/heaps/carveout-heap.c?h=ti-linux-6.18.y#n372

I completely missed the "no-map" case. Thanks for the review and the
link! I will address this in the next version, using a logic similar
to the one from the linked driver.

BR,
Albert.

>
> Andrew
>
> > +     /* create the dmabuf */
> > +     exp_info.exp_name = dma_heap_get_name(heap);
> > +     exp_info.ops = &coherent_heap_buf_ops;
> > +     exp_info.size = buffer->len;
> > +     exp_info.flags = fd_flags;
> > +     exp_info.priv = buffer;
> > +     dmabuf = dma_buf_export(&exp_info);
> > +     if (IS_ERR(dmabuf)) {
> > +             ret = PTR_ERR(dmabuf);
> > +             goto free_pages;
> > +     }
> > +     return dmabuf;
> > +
> > +free_pages:
> > +     kfree(buffer->pages);
> > +free_dma:
> > +     dma_free_coherent(heap_dev, buffer->len, buffer->alloc_vaddr,
> > +                       buffer->dma_addr);
> > +free_buffer:
> > +     kfree(buffer);
> > +     return ERR_PTR(ret);
> > +}
> > +
> > +static const struct dma_heap_ops coherent_heap_ops = {
> > +     .allocate = coherent_heap_allocate,
> > +};
> > +
> > +static int __coherent_heap_register(struct reserved_mem *rmem)
> > +{
> > +     struct dma_heap_export_info exp_info;
> > +     struct coherent_heap *coh_heap;
> > +     struct device *heap_dev;
> > +     int ret;
> > +
> > +     if (!rmem || !rmem->name)
> > +             return -EINVAL;
> > +
> > +     coh_heap = kzalloc_obj(*coh_heap);
> > +     if (!coh_heap)
> > +             return -ENOMEM;
> > +
> > +     coh_heap->rmem = rmem;
> > +     coh_heap->name = kstrdup(rmem->name, GFP_KERNEL);
> > +     if (!coh_heap->name) {
> > +             ret = -ENOMEM;
> > +             goto free_coherent_heap;
> > +     }
> > +
> > +     exp_info.name = coh_heap->name;
> > +     exp_info.ops = &coherent_heap_ops;
> > +     exp_info.priv = coh_heap;
> > +
> > +     coh_heap->heap = dma_heap_create(&exp_info);
> > +     if (IS_ERR(coh_heap->heap)) {
> > +             ret = PTR_ERR(coh_heap->heap);
> > +             goto free_name;
> > +     }
> > +
> > +     heap_dev = dma_heap_get_dev(coh_heap->heap);
> > +     ret = dma_coerce_mask_and_coherent(heap_dev, DMA_BIT_MASK(64));
> > +     if (ret) {
> > +             pr_err("coherent_heap: failed to set DMA mask (%d)\n", ret);
> > +             goto destroy_heap;
> > +     }
> > +
> > +     ret = of_reserved_mem_device_init_with_mem(heap_dev, rmem);
> > +     if (ret) {
> > +             pr_err("coherent_heap: failed to initialize memory (%d)\n", ret);
> > +             goto destroy_heap;
> > +     }
> > +
> > +     ret = dma_heap_register(coh_heap->heap);
> > +     if (ret) {
> > +             pr_err("coherent_heap: failed to register heap (%d)\n", ret);
> > +             goto destroy_heap;
> > +     }
> > +
> > +     return 0;
> > +
> > +destroy_heap:
> > +     dma_heap_destroy(coh_heap->heap);
> > +     coh_heap->heap = NULL;
> > +free_name:
> > +     kfree(coh_heap->name);
> > +free_coherent_heap:
> > +     kfree(coh_heap);
> > +
> > +     return ret;
> > +}
> > +
> > +static int __init coherent_heap_register(void)
> > +{
> > +     struct reserved_mem *rmem;
> > +     unsigned int i;
> > +     int ret;
> > +
> > +     for (i = 0; (rmem = dma_coherent_get_reserved_region(i)) != NULL; i++) {
> > +             ret = __coherent_heap_register(rmem);
> > +             if (ret) {
> > +                     pr_warn("Failed to add coherent heap %s",
> > +                             rmem->name ? rmem->name : "unknown");
> > +                     continue;
> > +             }
> > +     }
> > +
> > +     return 0;
> > +}
> > +module_init(coherent_heap_register);
> > +MODULE_DESCRIPTION("DMA-BUF heap for coherent reserved-memory regions");
> >
>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 5/6] dma-buf: heaps: Add Coherent heap to dmabuf heaps
  2026-03-11 10:19     ` Albert Esteve
@ 2026-03-11 13:18       ` Andrew Davis
  2026-03-11 15:28         ` Albert Esteve
  2026-03-16 12:08         ` Maxime Ripard
  0 siblings, 2 replies; 16+ messages in thread
From: Andrew Davis @ 2026-03-11 13:18 UTC (permalink / raw)
  To: Albert Esteve
  Cc: Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz,
	T.J. Mercier, Christian König, Marek Szyprowski,
	Robin Murphy, Rob Herring, Saravana Kannan, linux-kernel,
	linux-media, dri-devel, linaro-mm-sig, iommu, devicetree, mripard,
	echanude

On 3/11/26 5:19 AM, Albert Esteve wrote:
> On Tue, Mar 10, 2026 at 4:34 PM Andrew Davis <afd@ti.com> wrote:
>>
>> On 3/6/26 4:36 AM, Albert Esteve wrote:
>>> Expose DT coherent reserved-memory pools ("shared-dma-pool"
>>> without "reusable") as dma-buf heaps, creating one heap per
>>> region so userspace can allocate from the exact device-local
>>> pool intended for coherent DMA.
>>>
>>> This is a missing backend in the long-term effort to steer
>>> userspace buffer allocations (DRM, v4l2, dma-buf heaps)
>>> through heaps for clearer cgroup accounting. CMA and system
>>> heaps already exist; non-reusable coherent reserved memory
>>> did not.
>>>
>>> The heap binds the heap device to each memory region so
>>> coherent allocations use the correct dev->dma_mem, and
>>> it defers registration until module_init when normal
>>> allocators are available.
>>>
>>> Signed-off-by: Albert Esteve <aesteve@redhat.com>
>>> ---
>>>    drivers/dma-buf/heaps/Kconfig         |   9 +
>>>    drivers/dma-buf/heaps/Makefile        |   1 +
>>>    drivers/dma-buf/heaps/coherent_heap.c | 414 ++++++++++++++++++++++++++++++++++
>>>    3 files changed, 424 insertions(+)
>>>
>>> diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
>>> index a5eef06c42264..aeb475e585048 100644
>>> --- a/drivers/dma-buf/heaps/Kconfig
>>> +++ b/drivers/dma-buf/heaps/Kconfig
>>> @@ -12,3 +12,12 @@ config DMABUF_HEAPS_CMA
>>>          Choose this option to enable dma-buf CMA heap. This heap is backed
>>>          by the Contiguous Memory Allocator (CMA). If your system has these
>>>          regions, you should say Y here.
>>> +
>>> +config DMABUF_HEAPS_COHERENT
>>> +     bool "DMA-BUF Coherent Reserved-Memory Heap"
>>> +     depends on DMABUF_HEAPS && OF_RESERVED_MEM && DMA_DECLARE_COHERENT
>>> +     help
>>> +       Choose this option to enable coherent reserved-memory dma-buf heaps.
>>> +       This heap is backed by non-reusable DT "shared-dma-pool" regions.
>>> +       If your system defines coherent reserved-memory regions, you should
>>> +       say Y here.
>>> diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
>>> index 974467791032f..96bda7a65f041 100644
>>> --- a/drivers/dma-buf/heaps/Makefile
>>> +++ b/drivers/dma-buf/heaps/Makefile
>>> @@ -1,3 +1,4 @@
>>>    # SPDX-License-Identifier: GPL-2.0
>>>    obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)   += system_heap.o
>>>    obj-$(CONFIG_DMABUF_HEAPS_CMA)              += cma_heap.o
>>> +obj-$(CONFIG_DMABUF_HEAPS_COHERENT)  += coherent_heap.o
>>> diff --git a/drivers/dma-buf/heaps/coherent_heap.c b/drivers/dma-buf/heaps/coherent_heap.c
>>> new file mode 100644
>>> index 0000000000000..55f53f87c4c15
>>> --- /dev/null
>>> +++ b/drivers/dma-buf/heaps/coherent_heap.c
>>> @@ -0,0 +1,414 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * DMABUF heap for coherent reserved-memory regions
>>> + *
>>> + * Copyright (C) 2026 Red Hat, Inc.
>>> + * Author: Albert Esteve <aesteve@redhat.com>
>>> + *
>>> + */
>>> +
>>> +#include <linux/dma-buf.h>
>>> +#include <linux/dma-heap.h>
>>> +#include <linux/dma-map-ops.h>
>>> +#include <linux/dma-mapping.h>
>>> +#include <linux/err.h>
>>> +#include <linux/highmem.h>
>>> +#include <linux/iosys-map.h>
>>> +#include <linux/of_reserved_mem.h>
>>> +#include <linux/scatterlist.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/vmalloc.h>
>>> +
>>> +struct coherent_heap {
>>> +     struct dma_heap *heap;
>>> +     struct reserved_mem *rmem;
>>> +     char *name;
>>> +};
>>> +
>>> +struct coherent_heap_buffer {
>>> +     struct coherent_heap *heap;
>>> +     struct list_head attachments;
>>> +     struct mutex lock;
>>> +     unsigned long len;
>>> +     dma_addr_t dma_addr;
>>> +     void *alloc_vaddr;
>>> +     struct page **pages;
>>> +     pgoff_t pagecount;
>>> +     int vmap_cnt;
>>> +     void *vaddr;
>>> +};
>>> +
>>> +struct dma_heap_attachment {
>>> +     struct device *dev;
>>> +     struct sg_table table;
>>> +     struct list_head list;
>>> +     bool mapped;
>>> +};
>>> +
>>> +static int coherent_heap_attach(struct dma_buf *dmabuf,
>>> +                             struct dma_buf_attachment *attachment)
>>> +{
>>> +     struct coherent_heap_buffer *buffer = dmabuf->priv;
>>> +     struct dma_heap_attachment *a;
>>> +     int ret;
>>> +
>>> +     a = kzalloc_obj(*a);
>>> +     if (!a)
>>> +             return -ENOMEM;
>>> +
>>> +     ret = sg_alloc_table_from_pages(&a->table, buffer->pages,
>>> +                                     buffer->pagecount, 0,
>>> +                                     buffer->pagecount << PAGE_SHIFT,
>>> +                                     GFP_KERNEL);
>>> +     if (ret) {
>>> +             kfree(a);
>>> +             return ret;
>>> +     }
>>> +
>>> +     a->dev = attachment->dev;
>>> +     INIT_LIST_HEAD(&a->list);
>>> +     a->mapped = false;
>>> +
>>> +     attachment->priv = a;
>>> +
>>> +     mutex_lock(&buffer->lock);
>>> +     list_add(&a->list, &buffer->attachments);
>>> +     mutex_unlock(&buffer->lock);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static void coherent_heap_detach(struct dma_buf *dmabuf,
>>> +                              struct dma_buf_attachment *attachment)
>>> +{
>>> +     struct coherent_heap_buffer *buffer = dmabuf->priv;
>>> +     struct dma_heap_attachment *a = attachment->priv;
>>> +
>>> +     mutex_lock(&buffer->lock);
>>> +     list_del(&a->list);
>>> +     mutex_unlock(&buffer->lock);
>>> +
>>> +     sg_free_table(&a->table);
>>> +     kfree(a);
>>> +}
>>> +
>>> +static struct sg_table *coherent_heap_map_dma_buf(struct dma_buf_attachment *attachment,
>>> +                                               enum dma_data_direction direction)
>>> +{
>>> +     struct dma_heap_attachment *a = attachment->priv;
>>> +     struct sg_table *table = &a->table;
>>> +     int ret;
>>> +
>>> +     ret = dma_map_sgtable(attachment->dev, table, direction, 0);
>>> +     if (ret)
>>> +             return ERR_PTR(-ENOMEM);
>>> +     a->mapped = true;
>>> +
>>> +     return table;
>>> +}
>>> +
>>> +static void coherent_heap_unmap_dma_buf(struct dma_buf_attachment *attachment,
>>> +                                     struct sg_table *table,
>>> +                                     enum dma_data_direction direction)
>>> +{
>>> +     struct dma_heap_attachment *a = attachment->priv;
>>> +
>>> +     a->mapped = false;
>>> +     dma_unmap_sgtable(attachment->dev, table, direction, 0);
>>> +}
>>> +
>>> +static int coherent_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>>> +                                               enum dma_data_direction direction)
>>> +{
>>> +     struct coherent_heap_buffer *buffer = dmabuf->priv;
>>> +     struct dma_heap_attachment *a;
>>> +
>>> +     mutex_lock(&buffer->lock);
>>> +     if (buffer->vmap_cnt)
>>> +             invalidate_kernel_vmap_range(buffer->vaddr, buffer->len);
>>> +
>>> +     list_for_each_entry(a, &buffer->attachments, list) {
>>> +             if (!a->mapped)
>>> +                     continue;
>>> +             dma_sync_sgtable_for_cpu(a->dev, &a->table, direction);
>>> +     }
>>> +     mutex_unlock(&buffer->lock);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int coherent_heap_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>>> +                                             enum dma_data_direction direction)
>>> +{
>>> +     struct coherent_heap_buffer *buffer = dmabuf->priv;
>>> +     struct dma_heap_attachment *a;
>>> +
>>> +     mutex_lock(&buffer->lock);
>>> +     if (buffer->vmap_cnt)
>>> +             flush_kernel_vmap_range(buffer->vaddr, buffer->len);
>>> +
>>> +     list_for_each_entry(a, &buffer->attachments, list) {
>>> +             if (!a->mapped)
>>> +                     continue;
>>> +             dma_sync_sgtable_for_device(a->dev, &a->table, direction);
>>> +     }
>>> +     mutex_unlock(&buffer->lock);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int coherent_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
>>> +{
>>> +     struct coherent_heap_buffer *buffer = dmabuf->priv;
>>> +     struct coherent_heap *coh_heap = buffer->heap;
>>> +     struct device *heap_dev = dma_heap_get_dev(coh_heap->heap);
>>> +
>>> +     return dma_mmap_coherent(heap_dev, vma, buffer->alloc_vaddr,
>>> +                              buffer->dma_addr, buffer->len);
>>> +}
>>> +
>>> +static void *coherent_heap_do_vmap(struct coherent_heap_buffer *buffer)
>>> +{
>>> +     void *vaddr;
>>> +
>>> +     vaddr = vmap(buffer->pages, buffer->pagecount, VM_MAP, PAGE_KERNEL);
>>> +     if (!vaddr)
>>> +             return ERR_PTR(-ENOMEM);
>>> +
>>> +     return vaddr;
>>> +}
>>> +
>>> +static int coherent_heap_vmap(struct dma_buf *dmabuf, struct iosys_map *map)
>>> +{
>>> +     struct coherent_heap_buffer *buffer = dmabuf->priv;
>>> +     void *vaddr;
>>> +     int ret = 0;
>>> +
>>> +     mutex_lock(&buffer->lock);
>>> +     if (buffer->vmap_cnt) {
>>> +             buffer->vmap_cnt++;
>>> +             iosys_map_set_vaddr(map, buffer->vaddr);
>>> +             goto out;
>>> +     }
>>> +
>>> +     vaddr = coherent_heap_do_vmap(buffer);
>>> +     if (IS_ERR(vaddr)) {
>>> +             ret = PTR_ERR(vaddr);
>>> +             goto out;
>>> +     }
>>> +
>>> +     buffer->vaddr = vaddr;
>>> +     buffer->vmap_cnt++;
>>> +     iosys_map_set_vaddr(map, buffer->vaddr);
>>> +out:
>>> +     mutex_unlock(&buffer->lock);
>>> +
>>> +     return ret;
>>> +}
>>> +
>>> +static void coherent_heap_vunmap(struct dma_buf *dmabuf, struct iosys_map *map)
>>> +{
>>> +     struct coherent_heap_buffer *buffer = dmabuf->priv;
>>> +
>>> +     mutex_lock(&buffer->lock);
>>> +     if (!--buffer->vmap_cnt) {
>>> +             vunmap(buffer->vaddr);
>>> +             buffer->vaddr = NULL;
>>> +     }
>>> +     mutex_unlock(&buffer->lock);
>>> +     iosys_map_clear(map);
>>> +}
>>> +
>>> +static void coherent_heap_dma_buf_release(struct dma_buf *dmabuf)
>>> +{
>>> +     struct coherent_heap_buffer *buffer = dmabuf->priv;
>>> +     struct coherent_heap *coh_heap = buffer->heap;
>>> +     struct device *heap_dev = dma_heap_get_dev(coh_heap->heap);
>>> +
>>> +     if (buffer->vmap_cnt > 0) {
>>> +             WARN(1, "%s: buffer still mapped in the kernel\n", __func__);
>>> +             vunmap(buffer->vaddr);
>>> +             buffer->vaddr = NULL;
>>> +             buffer->vmap_cnt = 0;
>>> +     }
>>> +
>>> +     if (buffer->alloc_vaddr)
>>> +             dma_free_coherent(heap_dev, buffer->len, buffer->alloc_vaddr,
>>> +                               buffer->dma_addr);
>>> +     kfree(buffer->pages);
>>> +     kfree(buffer);
>>> +}
>>> +
>>> +static const struct dma_buf_ops coherent_heap_buf_ops = {
>>> +     .attach = coherent_heap_attach,
>>> +     .detach = coherent_heap_detach,
>>> +     .map_dma_buf = coherent_heap_map_dma_buf,
>>> +     .unmap_dma_buf = coherent_heap_unmap_dma_buf,
>>> +     .begin_cpu_access = coherent_heap_dma_buf_begin_cpu_access,
>>> +     .end_cpu_access = coherent_heap_dma_buf_end_cpu_access,
>>> +     .mmap = coherent_heap_mmap,
>>> +     .vmap = coherent_heap_vmap,
>>> +     .vunmap = coherent_heap_vunmap,
>>> +     .release = coherent_heap_dma_buf_release,
>>> +};
>>> +
>>> +static struct dma_buf *coherent_heap_allocate(struct dma_heap *heap,
>>> +                                           unsigned long len,
>>> +                                           u32 fd_flags,
>>> +                                           u64 heap_flags)
>>> +{
>>> +     struct coherent_heap *coh_heap;
>>> +     struct coherent_heap_buffer *buffer;
>>> +     struct device *heap_dev;
>>> +     DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
>>> +     size_t size = PAGE_ALIGN(len);
>>> +     pgoff_t pagecount = size >> PAGE_SHIFT;
>>> +     struct dma_buf *dmabuf;
>>> +     int ret = -ENOMEM;
>>> +     pgoff_t pg;
>>> +
>>> +     coh_heap = dma_heap_get_drvdata(heap);
>>> +     if (!coh_heap)
>>> +             return ERR_PTR(-EINVAL);
>>> +
>>> +     heap_dev = dma_heap_get_dev(coh_heap->heap);
>>> +     if (!heap_dev)
>>> +             return ERR_PTR(-ENODEV);
>>> +
>>> +     buffer = kzalloc_obj(*buffer);
>>> +     if (!buffer)
>>> +             return ERR_PTR(-ENOMEM);
>>> +
>>> +     INIT_LIST_HEAD(&buffer->attachments);
>>> +     mutex_init(&buffer->lock);
>>> +     buffer->len = size;
>>> +     buffer->heap = coh_heap;
>>> +     buffer->pagecount = pagecount;
>>> +
>>> +     buffer->alloc_vaddr = dma_alloc_coherent(heap_dev, buffer->len,
>>> +                                              &buffer->dma_addr, GFP_KERNEL);
>>
>> You are doing this DMA allocation using a non-DMA pseudo-device (heap_dev).
>> This is why you need to do that dma_coerce_mask_and_coherent(64) nonsense, you
>> are doing a DMA alloc for the CPU itself. This might still work, but only if
>> dma_map_sgtable() can handle swiotlb/iommu for all attaching devices at map
>> time.
> 
> The concern is valid. We're allocating via a synthetic device, which
> ties the allocation to that device's DMA domain. I looked deeper into
> this trying to address the concern.
> 
> The approach works because dma_map_sgtable() handles both
> dma_map_direct and use_dma_iommu cases in __dma_map_sg_attrs(). For
> each physical address in the sg_table (extracted via sg_phys()), it
> creates device-specific DMA mappings:
> - For direct mapping: it checks if the address is directly accessible
> (dma_capable()), and if not, it falls back to swiotlb.
> - For IOMMU: it creates mappings that allow the device to access
> physical addresses.
> 
> This means every attached device gets its own device-specific DMA
> mapping, properly handling cases where the physical addresses are
> inaccessible or have DMA constraints.
> 

While this means it might still "work" it won't always be ideal. Take
the case where the consuming device(s) have a 32bit address restriction,
if the allocation was done using the real devices then the backing buffer
itself would be allocated in <32bit mem. Whereas here the allocation
could end up in >32bit mem, as the CPU/synthetic device supports that.
Then each mapping device would instead get a bounce buffer.

(this example might not be great as we usually know the address of
carveout/reserved memory regions, but substitute in whatever restriction
makes more sense)

These non-reusable carveouts tend to be made for some specific device, and
they are made specifically because that device has some memory restriction.
So we might run into the situation above more than one would expect.

Not a blocker here, but just something worth thinking on.

> I'm not sure whether other approaches (whatever they may be) would be
> better, as here we are leveraging a great part of the existing
> infrastructure.
> 
>>
>>> +     if (!buffer->alloc_vaddr) {
>>> +             ret = -ENOMEM;
>>> +             goto free_buffer;
>>> +     }
>>> +
>>> +     buffer->pages = kmalloc_array(pagecount, sizeof(*buffer->pages),
>>> +                                   GFP_KERNEL);
>>> +     if (!buffer->pages) {
>>> +             ret = -ENOMEM;
>>> +             goto free_dma;
>>> +     }
>>> +
>>> +     for (pg = 0; pg < pagecount; pg++)
>>> +             buffer->pages[pg] = virt_to_page((char *)buffer->alloc_vaddr +
>>> +                                              (pg * PAGE_SIZE));
>>> +
>>
>> Is any of this valid if the coherent pool in DT was marked "no-map;"?
>> I'm sure the .mmap and .cpu_access function are not valid in that case.
>> Our (TI) evil vendor tree version of this heap sets a flag in that case and
>> avoids doing anything invalid when the region doesn't have normal backing
>> page structs. This region is treated more like a P2PDMA area in that case.
>>
>> https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/tree/drivers/dma-buf/heaps/carveout-heap.c?h=ti-linux-6.18.y#n372
> 
> I completely missed the "no-map" case. Thanks for the review and the
> link! I will address this in the next version, using a logic similar
> to the one from the linked driver.
> 

Do take note that the linked driver is only part of an evil vendor tree,
I do things in that driver that are not correct and would not fly upstream.

For "no-map" I chose to make un-cached mappings for the CPU. This allowed for
kernel/userspace access without changing cacheability (which can't be done
safely on ARM). The issue is that "no-map" really should mean DO NOT MAP.
It might be these carveouts are firewalled or have some other side effect
that prevent *any* mapping from CPU. The safer thing to do would be to
simply not allow CPU mappings (vmap/mmap) if "no-map" is set.

Andrew

> BR,
> Albert.
> 
>>
>> Andrew
>>
>>> +     /* create the dmabuf */
>>> +     exp_info.exp_name = dma_heap_get_name(heap);
>>> +     exp_info.ops = &coherent_heap_buf_ops;
>>> +     exp_info.size = buffer->len;
>>> +     exp_info.flags = fd_flags;
>>> +     exp_info.priv = buffer;
>>> +     dmabuf = dma_buf_export(&exp_info);
>>> +     if (IS_ERR(dmabuf)) {
>>> +             ret = PTR_ERR(dmabuf);
>>> +             goto free_pages;
>>> +     }
>>> +     return dmabuf;
>>> +
>>> +free_pages:
>>> +     kfree(buffer->pages);
>>> +free_dma:
>>> +     dma_free_coherent(heap_dev, buffer->len, buffer->alloc_vaddr,
>>> +                       buffer->dma_addr);
>>> +free_buffer:
>>> +     kfree(buffer);
>>> +     return ERR_PTR(ret);
>>> +}
>>> +
>>> +static const struct dma_heap_ops coherent_heap_ops = {
>>> +     .allocate = coherent_heap_allocate,
>>> +};
>>> +
>>> +static int __coherent_heap_register(struct reserved_mem *rmem)
>>> +{
>>> +     struct dma_heap_export_info exp_info;
>>> +     struct coherent_heap *coh_heap;
>>> +     struct device *heap_dev;
>>> +     int ret;
>>> +
>>> +     if (!rmem || !rmem->name)
>>> +             return -EINVAL;
>>> +
>>> +     coh_heap = kzalloc_obj(*coh_heap);
>>> +     if (!coh_heap)
>>> +             return -ENOMEM;
>>> +
>>> +     coh_heap->rmem = rmem;
>>> +     coh_heap->name = kstrdup(rmem->name, GFP_KERNEL);
>>> +     if (!coh_heap->name) {
>>> +             ret = -ENOMEM;
>>> +             goto free_coherent_heap;
>>> +     }
>>> +
>>> +     exp_info.name = coh_heap->name;
>>> +     exp_info.ops = &coherent_heap_ops;
>>> +     exp_info.priv = coh_heap;
>>> +
>>> +     coh_heap->heap = dma_heap_create(&exp_info);
>>> +     if (IS_ERR(coh_heap->heap)) {
>>> +             ret = PTR_ERR(coh_heap->heap);
>>> +             goto free_name;
>>> +     }
>>> +
>>> +     heap_dev = dma_heap_get_dev(coh_heap->heap);
>>> +     ret = dma_coerce_mask_and_coherent(heap_dev, DMA_BIT_MASK(64));
>>> +     if (ret) {
>>> +             pr_err("coherent_heap: failed to set DMA mask (%d)\n", ret);
>>> +             goto destroy_heap;
>>> +     }
>>> +
>>> +     ret = of_reserved_mem_device_init_with_mem(heap_dev, rmem);
>>> +     if (ret) {
>>> +             pr_err("coherent_heap: failed to initialize memory (%d)\n", ret);
>>> +             goto destroy_heap;
>>> +     }
>>> +
>>> +     ret = dma_heap_register(coh_heap->heap);
>>> +     if (ret) {
>>> +             pr_err("coherent_heap: failed to register heap (%d)\n", ret);
>>> +             goto destroy_heap;
>>> +     }
>>> +
>>> +     return 0;
>>> +
>>> +destroy_heap:
>>> +     dma_heap_destroy(coh_heap->heap);
>>> +     coh_heap->heap = NULL;
>>> +free_name:
>>> +     kfree(coh_heap->name);
>>> +free_coherent_heap:
>>> +     kfree(coh_heap);
>>> +
>>> +     return ret;
>>> +}
>>> +
>>> +static int __init coherent_heap_register(void)
>>> +{
>>> +     struct reserved_mem *rmem;
>>> +     unsigned int i;
>>> +     int ret;
>>> +
>>> +     for (i = 0; (rmem = dma_coherent_get_reserved_region(i)) != NULL; i++) {
>>> +             ret = __coherent_heap_register(rmem);
>>> +             if (ret) {
>>> +                     pr_warn("Failed to add coherent heap %s",
>>> +                             rmem->name ? rmem->name : "unknown");
>>> +                     continue;
>>> +             }
>>> +     }
>>> +
>>> +     return 0;
>>> +}
>>> +module_init(coherent_heap_register);
>>> +MODULE_DESCRIPTION("DMA-BUF heap for coherent reserved-memory regions");
>>>
>>
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 5/6] dma-buf: heaps: Add Coherent heap to dmabuf heaps
  2026-03-11 13:18       ` Andrew Davis
@ 2026-03-11 15:28         ` Albert Esteve
  2026-03-16 12:08         ` Maxime Ripard
  1 sibling, 0 replies; 16+ messages in thread
From: Albert Esteve @ 2026-03-11 15:28 UTC (permalink / raw)
  To: Andrew Davis
  Cc: Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz,
	T.J. Mercier, Christian König, Marek Szyprowski,
	Robin Murphy, Rob Herring, Saravana Kannan, linux-kernel,
	linux-media, dri-devel, linaro-mm-sig, iommu, devicetree, mripard,
	echanude

On Wed, Mar 11, 2026 at 2:18 PM Andrew Davis <afd@ti.com> wrote:
>
> On 3/11/26 5:19 AM, Albert Esteve wrote:
> > On Tue, Mar 10, 2026 at 4:34 PM Andrew Davis <afd@ti.com> wrote:
> >>
> >> On 3/6/26 4:36 AM, Albert Esteve wrote:
> >>> Expose DT coherent reserved-memory pools ("shared-dma-pool"
> >>> without "reusable") as dma-buf heaps, creating one heap per
> >>> region so userspace can allocate from the exact device-local
> >>> pool intended for coherent DMA.
> >>>
> >>> This is a missing backend in the long-term effort to steer
> >>> userspace buffer allocations (DRM, v4l2, dma-buf heaps)
> >>> through heaps for clearer cgroup accounting. CMA and system
> >>> heaps already exist; non-reusable coherent reserved memory
> >>> did not.
> >>>
> >>> The heap binds the heap device to each memory region so
> >>> coherent allocations use the correct dev->dma_mem, and
> >>> it defers registration until module_init when normal
> >>> allocators are available.
> >>>
> >>> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> >>> ---
> >>>    drivers/dma-buf/heaps/Kconfig         |   9 +
> >>>    drivers/dma-buf/heaps/Makefile        |   1 +
> >>>    drivers/dma-buf/heaps/coherent_heap.c | 414 ++++++++++++++++++++++++++++++++++
> >>>    3 files changed, 424 insertions(+)
> >>>
> >>> diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> >>> index a5eef06c42264..aeb475e585048 100644
> >>> --- a/drivers/dma-buf/heaps/Kconfig
> >>> +++ b/drivers/dma-buf/heaps/Kconfig
> >>> @@ -12,3 +12,12 @@ config DMABUF_HEAPS_CMA
> >>>          Choose this option to enable dma-buf CMA heap. This heap is backed
> >>>          by the Contiguous Memory Allocator (CMA). If your system has these
> >>>          regions, you should say Y here.
> >>> +
> >>> +config DMABUF_HEAPS_COHERENT
> >>> +     bool "DMA-BUF Coherent Reserved-Memory Heap"
> >>> +     depends on DMABUF_HEAPS && OF_RESERVED_MEM && DMA_DECLARE_COHERENT
> >>> +     help
> >>> +       Choose this option to enable coherent reserved-memory dma-buf heaps.
> >>> +       This heap is backed by non-reusable DT "shared-dma-pool" regions.
> >>> +       If your system defines coherent reserved-memory regions, you should
> >>> +       say Y here.
> >>> diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
> >>> index 974467791032f..96bda7a65f041 100644
> >>> --- a/drivers/dma-buf/heaps/Makefile
> >>> +++ b/drivers/dma-buf/heaps/Makefile
> >>> @@ -1,3 +1,4 @@
> >>>    # SPDX-License-Identifier: GPL-2.0
> >>>    obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)   += system_heap.o
> >>>    obj-$(CONFIG_DMABUF_HEAPS_CMA)              += cma_heap.o
> >>> +obj-$(CONFIG_DMABUF_HEAPS_COHERENT)  += coherent_heap.o
> >>> diff --git a/drivers/dma-buf/heaps/coherent_heap.c b/drivers/dma-buf/heaps/coherent_heap.c
> >>> new file mode 100644
> >>> index 0000000000000..55f53f87c4c15
> >>> --- /dev/null
> >>> +++ b/drivers/dma-buf/heaps/coherent_heap.c
> >>> @@ -0,0 +1,414 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +/*
> >>> + * DMABUF heap for coherent reserved-memory regions
> >>> + *
> >>> + * Copyright (C) 2026 Red Hat, Inc.
> >>> + * Author: Albert Esteve <aesteve@redhat.com>
> >>> + *
> >>> + */
> >>> +
> >>> +#include <linux/dma-buf.h>
> >>> +#include <linux/dma-heap.h>
> >>> +#include <linux/dma-map-ops.h>
> >>> +#include <linux/dma-mapping.h>
> >>> +#include <linux/err.h>
> >>> +#include <linux/highmem.h>
> >>> +#include <linux/iosys-map.h>
> >>> +#include <linux/of_reserved_mem.h>
> >>> +#include <linux/scatterlist.h>
> >>> +#include <linux/slab.h>
> >>> +#include <linux/vmalloc.h>
> >>> +
> >>> +struct coherent_heap {
> >>> +     struct dma_heap *heap;
> >>> +     struct reserved_mem *rmem;
> >>> +     char *name;
> >>> +};
> >>> +
> >>> +struct coherent_heap_buffer {
> >>> +     struct coherent_heap *heap;
> >>> +     struct list_head attachments;
> >>> +     struct mutex lock;
> >>> +     unsigned long len;
> >>> +     dma_addr_t dma_addr;
> >>> +     void *alloc_vaddr;
> >>> +     struct page **pages;
> >>> +     pgoff_t pagecount;
> >>> +     int vmap_cnt;
> >>> +     void *vaddr;
> >>> +};
> >>> +
> >>> +struct dma_heap_attachment {
> >>> +     struct device *dev;
> >>> +     struct sg_table table;
> >>> +     struct list_head list;
> >>> +     bool mapped;
> >>> +};
> >>> +
> >>> +static int coherent_heap_attach(struct dma_buf *dmabuf,
> >>> +                             struct dma_buf_attachment *attachment)
> >>> +{
> >>> +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> >>> +     struct dma_heap_attachment *a;
> >>> +     int ret;
> >>> +
> >>> +     a = kzalloc_obj(*a);
> >>> +     if (!a)
> >>> +             return -ENOMEM;
> >>> +
> >>> +     ret = sg_alloc_table_from_pages(&a->table, buffer->pages,
> >>> +                                     buffer->pagecount, 0,
> >>> +                                     buffer->pagecount << PAGE_SHIFT,
> >>> +                                     GFP_KERNEL);
> >>> +     if (ret) {
> >>> +             kfree(a);
> >>> +             return ret;
> >>> +     }
> >>> +
> >>> +     a->dev = attachment->dev;
> >>> +     INIT_LIST_HEAD(&a->list);
> >>> +     a->mapped = false;
> >>> +
> >>> +     attachment->priv = a;
> >>> +
> >>> +     mutex_lock(&buffer->lock);
> >>> +     list_add(&a->list, &buffer->attachments);
> >>> +     mutex_unlock(&buffer->lock);
> >>> +
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +static void coherent_heap_detach(struct dma_buf *dmabuf,
> >>> +                              struct dma_buf_attachment *attachment)
> >>> +{
> >>> +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> >>> +     struct dma_heap_attachment *a = attachment->priv;
> >>> +
> >>> +     mutex_lock(&buffer->lock);
> >>> +     list_del(&a->list);
> >>> +     mutex_unlock(&buffer->lock);
> >>> +
> >>> +     sg_free_table(&a->table);
> >>> +     kfree(a);
> >>> +}
> >>> +
> >>> +static struct sg_table *coherent_heap_map_dma_buf(struct dma_buf_attachment *attachment,
> >>> +                                               enum dma_data_direction direction)
> >>> +{
> >>> +     struct dma_heap_attachment *a = attachment->priv;
> >>> +     struct sg_table *table = &a->table;
> >>> +     int ret;
> >>> +
> >>> +     ret = dma_map_sgtable(attachment->dev, table, direction, 0);
> >>> +     if (ret)
> >>> +             return ERR_PTR(-ENOMEM);
> >>> +     a->mapped = true;
> >>> +
> >>> +     return table;
> >>> +}
> >>> +
> >>> +static void coherent_heap_unmap_dma_buf(struct dma_buf_attachment *attachment,
> >>> +                                     struct sg_table *table,
> >>> +                                     enum dma_data_direction direction)
> >>> +{
> >>> +     struct dma_heap_attachment *a = attachment->priv;
> >>> +
> >>> +     a->mapped = false;
> >>> +     dma_unmap_sgtable(attachment->dev, table, direction, 0);
> >>> +}
> >>> +
> >>> +static int coherent_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> >>> +                                               enum dma_data_direction direction)
> >>> +{
> >>> +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> >>> +     struct dma_heap_attachment *a;
> >>> +
> >>> +     mutex_lock(&buffer->lock);
> >>> +     if (buffer->vmap_cnt)
> >>> +             invalidate_kernel_vmap_range(buffer->vaddr, buffer->len);
> >>> +
> >>> +     list_for_each_entry(a, &buffer->attachments, list) {
> >>> +             if (!a->mapped)
> >>> +                     continue;
> >>> +             dma_sync_sgtable_for_cpu(a->dev, &a->table, direction);
> >>> +     }
> >>> +     mutex_unlock(&buffer->lock);
> >>> +
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +static int coherent_heap_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
> >>> +                                             enum dma_data_direction direction)
> >>> +{
> >>> +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> >>> +     struct dma_heap_attachment *a;
> >>> +
> >>> +     mutex_lock(&buffer->lock);
> >>> +     if (buffer->vmap_cnt)
> >>> +             flush_kernel_vmap_range(buffer->vaddr, buffer->len);
> >>> +
> >>> +     list_for_each_entry(a, &buffer->attachments, list) {
> >>> +             if (!a->mapped)
> >>> +                     continue;
> >>> +             dma_sync_sgtable_for_device(a->dev, &a->table, direction);
> >>> +     }
> >>> +     mutex_unlock(&buffer->lock);
> >>> +
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +static int coherent_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
> >>> +{
> >>> +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> >>> +     struct coherent_heap *coh_heap = buffer->heap;
> >>> +     struct device *heap_dev = dma_heap_get_dev(coh_heap->heap);
> >>> +
> >>> +     return dma_mmap_coherent(heap_dev, vma, buffer->alloc_vaddr,
> >>> +                              buffer->dma_addr, buffer->len);
> >>> +}
> >>> +
> >>> +static void *coherent_heap_do_vmap(struct coherent_heap_buffer *buffer)
> >>> +{
> >>> +     void *vaddr;
> >>> +
> >>> +     vaddr = vmap(buffer->pages, buffer->pagecount, VM_MAP, PAGE_KERNEL);
> >>> +     if (!vaddr)
> >>> +             return ERR_PTR(-ENOMEM);
> >>> +
> >>> +     return vaddr;
> >>> +}
> >>> +
> >>> +static int coherent_heap_vmap(struct dma_buf *dmabuf, struct iosys_map *map)
> >>> +{
> >>> +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> >>> +     void *vaddr;
> >>> +     int ret = 0;
> >>> +
> >>> +     mutex_lock(&buffer->lock);
> >>> +     if (buffer->vmap_cnt) {
> >>> +             buffer->vmap_cnt++;
> >>> +             iosys_map_set_vaddr(map, buffer->vaddr);
> >>> +             goto out;
> >>> +     }
> >>> +
> >>> +     vaddr = coherent_heap_do_vmap(buffer);
> >>> +     if (IS_ERR(vaddr)) {
> >>> +             ret = PTR_ERR(vaddr);
> >>> +             goto out;
> >>> +     }
> >>> +
> >>> +     buffer->vaddr = vaddr;
> >>> +     buffer->vmap_cnt++;
> >>> +     iosys_map_set_vaddr(map, buffer->vaddr);
> >>> +out:
> >>> +     mutex_unlock(&buffer->lock);
> >>> +
> >>> +     return ret;
> >>> +}
> >>> +
> >>> +static void coherent_heap_vunmap(struct dma_buf *dmabuf, struct iosys_map *map)
> >>> +{
> >>> +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> >>> +
> >>> +     mutex_lock(&buffer->lock);
> >>> +     if (!--buffer->vmap_cnt) {
> >>> +             vunmap(buffer->vaddr);
> >>> +             buffer->vaddr = NULL;
> >>> +     }
> >>> +     mutex_unlock(&buffer->lock);
> >>> +     iosys_map_clear(map);
> >>> +}
> >>> +
> >>> +static void coherent_heap_dma_buf_release(struct dma_buf *dmabuf)
> >>> +{
> >>> +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> >>> +     struct coherent_heap *coh_heap = buffer->heap;
> >>> +     struct device *heap_dev = dma_heap_get_dev(coh_heap->heap);
> >>> +
> >>> +     if (buffer->vmap_cnt > 0) {
> >>> +             WARN(1, "%s: buffer still mapped in the kernel\n", __func__);
> >>> +             vunmap(buffer->vaddr);
> >>> +             buffer->vaddr = NULL;
> >>> +             buffer->vmap_cnt = 0;
> >>> +     }
> >>> +
> >>> +     if (buffer->alloc_vaddr)
> >>> +             dma_free_coherent(heap_dev, buffer->len, buffer->alloc_vaddr,
> >>> +                               buffer->dma_addr);
> >>> +     kfree(buffer->pages);
> >>> +     kfree(buffer);
> >>> +}
> >>> +
> >>> +static const struct dma_buf_ops coherent_heap_buf_ops = {
> >>> +     .attach = coherent_heap_attach,
> >>> +     .detach = coherent_heap_detach,
> >>> +     .map_dma_buf = coherent_heap_map_dma_buf,
> >>> +     .unmap_dma_buf = coherent_heap_unmap_dma_buf,
> >>> +     .begin_cpu_access = coherent_heap_dma_buf_begin_cpu_access,
> >>> +     .end_cpu_access = coherent_heap_dma_buf_end_cpu_access,
> >>> +     .mmap = coherent_heap_mmap,
> >>> +     .vmap = coherent_heap_vmap,
> >>> +     .vunmap = coherent_heap_vunmap,
> >>> +     .release = coherent_heap_dma_buf_release,
> >>> +};
> >>> +
> >>> +static struct dma_buf *coherent_heap_allocate(struct dma_heap *heap,
> >>> +                                           unsigned long len,
> >>> +                                           u32 fd_flags,
> >>> +                                           u64 heap_flags)
> >>> +{
> >>> +     struct coherent_heap *coh_heap;
> >>> +     struct coherent_heap_buffer *buffer;
> >>> +     struct device *heap_dev;
> >>> +     DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> >>> +     size_t size = PAGE_ALIGN(len);
> >>> +     pgoff_t pagecount = size >> PAGE_SHIFT;
> >>> +     struct dma_buf *dmabuf;
> >>> +     int ret = -ENOMEM;
> >>> +     pgoff_t pg;
> >>> +
> >>> +     coh_heap = dma_heap_get_drvdata(heap);
> >>> +     if (!coh_heap)
> >>> +             return ERR_PTR(-EINVAL);
> >>> +
> >>> +     heap_dev = dma_heap_get_dev(coh_heap->heap);
> >>> +     if (!heap_dev)
> >>> +             return ERR_PTR(-ENODEV);
> >>> +
> >>> +     buffer = kzalloc_obj(*buffer);
> >>> +     if (!buffer)
> >>> +             return ERR_PTR(-ENOMEM);
> >>> +
> >>> +     INIT_LIST_HEAD(&buffer->attachments);
> >>> +     mutex_init(&buffer->lock);
> >>> +     buffer->len = size;
> >>> +     buffer->heap = coh_heap;
> >>> +     buffer->pagecount = pagecount;
> >>> +
> >>> +     buffer->alloc_vaddr = dma_alloc_coherent(heap_dev, buffer->len,
> >>> +                                              &buffer->dma_addr, GFP_KERNEL);
> >>
> >> You are doing this DMA allocation using a non-DMA pseudo-device (heap_dev).
> >> This is why you need to do that dma_coerce_mask_and_coherent(64) nonsense, you
> >> are doing a DMA alloc for the CPU itself. This might still work, but only if
> >> dma_map_sgtable() can handle swiotlb/iommu for all attaching devices at map
> >> time.
> >
> > The concern is valid. We're allocating via a synthetic device, which
> > ties the allocation to that device's DMA domain. I looked deeper into
> > this trying to address the concern.
> >
> > The approach works because dma_map_sgtable() handles both
> > dma_map_direct and use_dma_iommu cases in __dma_map_sg_attrs(). For
> > each physical address in the sg_table (extracted via sg_phys()), it
> > creates device-specific DMA mappings:
> > - For direct mapping: it checks if the address is directly accessible
> > (dma_capable()), and if not, it falls back to swiotlb.
> > - For IOMMU: it creates mappings that allow the device to access
> > physical addresses.
> >
> > This means every attached device gets its own device-specific DMA
> > mapping, properly handling cases where the physical addresses are
> > inaccessible or have DMA constraints.
> >
>
> While this means it might still "work" it won't always be ideal. Take
> the case where the consuming device(s) have a 32bit address restriction,
> if the allocation was done using the real devices then the backing buffer
> itself would be allocated in <32bit mem. Whereas here the allocation
> could end up in >32bit mem, as the CPU/synthetic device supports that.
> Then each mapping device would instead get a bounce buffer.
>
> (this example might not be great as we usually know the address of
> carveout/reserved memory regions, but substitute in whatever restriction
> makes more sense)
>
> These non-reusable carveouts tend to be made for some specific device, and
> they are made specifically because that device has some memory restriction.
> So we might run into the situation above more than one would expect.
>
> Not a blocker here, but just something worth thinking on.

Thanks for the explanation and the example. I understand the issue.

Finding a general solution feels difficult, though. Since we can't
know ahead of time which devices will consume buffers from a heap, we
can't constrain allocations to match all potential consumers'
restrictions. But at least the current approach handles it correctly
via dma_map_sgtable() with bounce buffers when needed.

I'll keep it in mind.

>
> > I'm not sure whether other approaches (whatever they may be) would be
> > better, as here we are leveraging a great part of the existing
> > infrastructure.
> >
> >>
> >>> +     if (!buffer->alloc_vaddr) {
> >>> +             ret = -ENOMEM;
> >>> +             goto free_buffer;
> >>> +     }
> >>> +
> >>> +     buffer->pages = kmalloc_array(pagecount, sizeof(*buffer->pages),
> >>> +                                   GFP_KERNEL);
> >>> +     if (!buffer->pages) {
> >>> +             ret = -ENOMEM;
> >>> +             goto free_dma;
> >>> +     }
> >>> +
> >>> +     for (pg = 0; pg < pagecount; pg++)
> >>> +             buffer->pages[pg] = virt_to_page((char *)buffer->alloc_vaddr +
> >>> +                                              (pg * PAGE_SIZE));
> >>> +
> >>
> >> Is any of this valid if the coherent pool in DT was marked "no-map;"?
> >> I'm sure the .mmap and .cpu_access function are not valid in that case.
> >> Our (TI) evil vendor tree version of this heap sets a flag in that case and
> >> avoids doing anything invalid when the region doesn't have normal backing
> >> page structs. This region is treated more like a P2PDMA area in that case.
> >>
> >> https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/tree/drivers/dma-buf/heaps/carveout-heap.c?h=ti-linux-6.18.y#n372
> >
> > I completely missed the "no-map" case. Thanks for the review and the
> > link! I will address this in the next version, using a logic similar
> > to the one from the linked driver.
> >
>
> Do take note that the linked driver is only part of an evil vendor tree,
> I do things in that driver that are not correct and would not fly upstream.
>
> For "no-map" I chose to make un-cached mappings for the CPU. This allowed for
> kernel/userspace access without changing cacheability (which can't be done
> safely on ARM). The issue is that "no-map" really should mean DO NOT MAP.
> It might be these carveouts are firewalled or have some other side effect
> that prevent *any* mapping from CPU. The safer thing to do would be to
> simply not allow CPU mappings (vmap/mmap) if "no-map" is set.

Noted, thanks!

>
> Andrew
>
> > BR,
> > Albert.
> >
> >>
> >> Andrew
> >>
> >>> +     /* create the dmabuf */
> >>> +     exp_info.exp_name = dma_heap_get_name(heap);
> >>> +     exp_info.ops = &coherent_heap_buf_ops;
> >>> +     exp_info.size = buffer->len;
> >>> +     exp_info.flags = fd_flags;
> >>> +     exp_info.priv = buffer;
> >>> +     dmabuf = dma_buf_export(&exp_info);
> >>> +     if (IS_ERR(dmabuf)) {
> >>> +             ret = PTR_ERR(dmabuf);
> >>> +             goto free_pages;
> >>> +     }
> >>> +     return dmabuf;
> >>> +
> >>> +free_pages:
> >>> +     kfree(buffer->pages);
> >>> +free_dma:
> >>> +     dma_free_coherent(heap_dev, buffer->len, buffer->alloc_vaddr,
> >>> +                       buffer->dma_addr);
> >>> +free_buffer:
> >>> +     kfree(buffer);
> >>> +     return ERR_PTR(ret);
> >>> +}
> >>> +
> >>> +static const struct dma_heap_ops coherent_heap_ops = {
> >>> +     .allocate = coherent_heap_allocate,
> >>> +};
> >>> +
> >>> +static int __coherent_heap_register(struct reserved_mem *rmem)
> >>> +{
> >>> +     struct dma_heap_export_info exp_info;
> >>> +     struct coherent_heap *coh_heap;
> >>> +     struct device *heap_dev;
> >>> +     int ret;
> >>> +
> >>> +     if (!rmem || !rmem->name)
> >>> +             return -EINVAL;
> >>> +
> >>> +     coh_heap = kzalloc_obj(*coh_heap);
> >>> +     if (!coh_heap)
> >>> +             return -ENOMEM;
> >>> +
> >>> +     coh_heap->rmem = rmem;
> >>> +     coh_heap->name = kstrdup(rmem->name, GFP_KERNEL);
> >>> +     if (!coh_heap->name) {
> >>> +             ret = -ENOMEM;
> >>> +             goto free_coherent_heap;
> >>> +     }
> >>> +
> >>> +     exp_info.name = coh_heap->name;
> >>> +     exp_info.ops = &coherent_heap_ops;
> >>> +     exp_info.priv = coh_heap;
> >>> +
> >>> +     coh_heap->heap = dma_heap_create(&exp_info);
> >>> +     if (IS_ERR(coh_heap->heap)) {
> >>> +             ret = PTR_ERR(coh_heap->heap);
> >>> +             goto free_name;
> >>> +     }
> >>> +
> >>> +     heap_dev = dma_heap_get_dev(coh_heap->heap);
> >>> +     ret = dma_coerce_mask_and_coherent(heap_dev, DMA_BIT_MASK(64));
> >>> +     if (ret) {
> >>> +             pr_err("coherent_heap: failed to set DMA mask (%d)\n", ret);
> >>> +             goto destroy_heap;
> >>> +     }
> >>> +
> >>> +     ret = of_reserved_mem_device_init_with_mem(heap_dev, rmem);
> >>> +     if (ret) {
> >>> +             pr_err("coherent_heap: failed to initialize memory (%d)\n", ret);
> >>> +             goto destroy_heap;
> >>> +     }
> >>> +
> >>> +     ret = dma_heap_register(coh_heap->heap);
> >>> +     if (ret) {
> >>> +             pr_err("coherent_heap: failed to register heap (%d)\n", ret);
> >>> +             goto destroy_heap;
> >>> +     }
> >>> +
> >>> +     return 0;
> >>> +
> >>> +destroy_heap:
> >>> +     dma_heap_destroy(coh_heap->heap);
> >>> +     coh_heap->heap = NULL;
> >>> +free_name:
> >>> +     kfree(coh_heap->name);
> >>> +free_coherent_heap:
> >>> +     kfree(coh_heap);
> >>> +
> >>> +     return ret;
> >>> +}
> >>> +
> >>> +static int __init coherent_heap_register(void)
> >>> +{
> >>> +     struct reserved_mem *rmem;
> >>> +     unsigned int i;
> >>> +     int ret;
> >>> +
> >>> +     for (i = 0; (rmem = dma_coherent_get_reserved_region(i)) != NULL; i++) {
> >>> +             ret = __coherent_heap_register(rmem);
> >>> +             if (ret) {
> >>> +                     pr_warn("Failed to add coherent heap %s",
> >>> +                             rmem->name ? rmem->name : "unknown");
> >>> +                     continue;
> >>> +             }
> >>> +     }
> >>> +
> >>> +     return 0;
> >>> +}
> >>> +module_init(coherent_heap_register);
> >>> +MODULE_DESCRIPTION("DMA-BUF heap for coherent reserved-memory regions");
> >>>
> >>
> >
>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/6] of_reserved_mem: add a helper for rmem device_init op
  2026-03-06 10:36 ` [PATCH v3 3/6] of_reserved_mem: add a helper for rmem device_init op Albert Esteve
@ 2026-03-13 23:06   ` Rob Herring
  2026-03-16 10:54     ` Albert Esteve
  0 siblings, 1 reply; 16+ messages in thread
From: Rob Herring @ 2026-03-13 23:06 UTC (permalink / raw)
  To: Albert Esteve
  Cc: Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz,
	T.J. Mercier, Christian König, Marek Szyprowski,
	Robin Murphy, Saravana Kannan, linux-kernel, linux-media,
	dri-devel, linaro-mm-sig, iommu, devicetree, mripard, echanude

On Fri, Mar 06, 2026 at 11:36:34AM +0100, Albert Esteve wrote:
> Add a helper function wrapping internal reserved memory
> device_init call and expose it externally.

Why?

The diff tells us what. The commit msg needs to tell us why. Maybe the 
rest of the series explains it, but this commit needs to stand on its 
own.

> 
> Use the new helper function within of_reserved_mem_device_init_by_idx().
> 
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---

Version history?

>  drivers/of/of_reserved_mem.c    | 68 ++++++++++++++++++++++++++---------------
>  include/linux/of_reserved_mem.h |  8 +++++
>  2 files changed, 52 insertions(+), 24 deletions(-)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/6] of_reserved_mem: add a helper for rmem device_init op
  2026-03-13 23:06   ` Rob Herring
@ 2026-03-16 10:54     ` Albert Esteve
  0 siblings, 0 replies; 16+ messages in thread
From: Albert Esteve @ 2026-03-16 10:54 UTC (permalink / raw)
  To: Rob Herring
  Cc: Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz,
	T.J. Mercier, Christian König, Marek Szyprowski,
	Robin Murphy, Saravana Kannan, linux-kernel, linux-media,
	dri-devel, linaro-mm-sig, iommu, devicetree, mripard, echanude

On Sat, Mar 14, 2026 at 12:06 AM Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Mar 06, 2026 at 11:36:34AM +0100, Albert Esteve wrote:
> > Add a helper function wrapping internal reserved memory
> > device_init call and expose it externally.
>
> Why?
>
> The diff tells us what. The commit msg needs to tell us why. Maybe the
> rest of the series explains it, but this commit needs to stand on its
> own.

ACK.

This patch prepares for the heap implementation in patch #4, which
uses the new helper function. I will add the information to the commit
body.

>
> >
> > Use the new helper function within of_reserved_mem_device_init_by_idx().
> >
> > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > ---
>
> Version history?

You mean track it in the commit body? History is already detailed in
the cover letter.

>
> >  drivers/of/of_reserved_mem.c    | 68 ++++++++++++++++++++++++++---------------
> >  include/linux/of_reserved_mem.h |  8 +++++
> >  2 files changed, 52 insertions(+), 24 deletions(-)
>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 5/6] dma-buf: heaps: Add Coherent heap to dmabuf heaps
  2026-03-11 13:18       ` Andrew Davis
  2026-03-11 15:28         ` Albert Esteve
@ 2026-03-16 12:08         ` Maxime Ripard
  1 sibling, 0 replies; 16+ messages in thread
From: Maxime Ripard @ 2026-03-16 12:08 UTC (permalink / raw)
  To: Andrew Davis
  Cc: Albert Esteve, Sumit Semwal, Benjamin Gaignard, Brian Starkey,
	John Stultz, T.J. Mercier, Christian König, Marek Szyprowski,
	Robin Murphy, Rob Herring, Saravana Kannan, linux-kernel,
	linux-media, dri-devel, linaro-mm-sig, iommu, devicetree,
	echanude

[-- Attachment #1: Type: text/plain, Size: 17194 bytes --]

Hi Andrew, Albert,

On Wed, Mar 11, 2026 at 08:18:28AM -0500, Andrew Davis wrote:
> On 3/11/26 5:19 AM, Albert Esteve wrote:
> > On Tue, Mar 10, 2026 at 4:34 PM Andrew Davis <afd@ti.com> wrote:
> > > 
> > > On 3/6/26 4:36 AM, Albert Esteve wrote:
> > > > Expose DT coherent reserved-memory pools ("shared-dma-pool"
> > > > without "reusable") as dma-buf heaps, creating one heap per
> > > > region so userspace can allocate from the exact device-local
> > > > pool intended for coherent DMA.
> > > > 
> > > > This is a missing backend in the long-term effort to steer
> > > > userspace buffer allocations (DRM, v4l2, dma-buf heaps)
> > > > through heaps for clearer cgroup accounting. CMA and system
> > > > heaps already exist; non-reusable coherent reserved memory
> > > > did not.
> > > > 
> > > > The heap binds the heap device to each memory region so
> > > > coherent allocations use the correct dev->dma_mem, and
> > > > it defers registration until module_init when normal
> > > > allocators are available.
> > > > 
> > > > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > > > ---
> > > >    drivers/dma-buf/heaps/Kconfig         |   9 +
> > > >    drivers/dma-buf/heaps/Makefile        |   1 +
> > > >    drivers/dma-buf/heaps/coherent_heap.c | 414 ++++++++++++++++++++++++++++++++++
> > > >    3 files changed, 424 insertions(+)
> > > > 
> > > > diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> > > > index a5eef06c42264..aeb475e585048 100644
> > > > --- a/drivers/dma-buf/heaps/Kconfig
> > > > +++ b/drivers/dma-buf/heaps/Kconfig
> > > > @@ -12,3 +12,12 @@ config DMABUF_HEAPS_CMA
> > > >          Choose this option to enable dma-buf CMA heap. This heap is backed
> > > >          by the Contiguous Memory Allocator (CMA). If your system has these
> > > >          regions, you should say Y here.
> > > > +
> > > > +config DMABUF_HEAPS_COHERENT
> > > > +     bool "DMA-BUF Coherent Reserved-Memory Heap"
> > > > +     depends on DMABUF_HEAPS && OF_RESERVED_MEM && DMA_DECLARE_COHERENT
> > > > +     help
> > > > +       Choose this option to enable coherent reserved-memory dma-buf heaps.
> > > > +       This heap is backed by non-reusable DT "shared-dma-pool" regions.
> > > > +       If your system defines coherent reserved-memory regions, you should
> > > > +       say Y here.
> > > > diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
> > > > index 974467791032f..96bda7a65f041 100644
> > > > --- a/drivers/dma-buf/heaps/Makefile
> > > > +++ b/drivers/dma-buf/heaps/Makefile
> > > > @@ -1,3 +1,4 @@
> > > >    # SPDX-License-Identifier: GPL-2.0
> > > >    obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)   += system_heap.o
> > > >    obj-$(CONFIG_DMABUF_HEAPS_CMA)              += cma_heap.o
> > > > +obj-$(CONFIG_DMABUF_HEAPS_COHERENT)  += coherent_heap.o
> > > > diff --git a/drivers/dma-buf/heaps/coherent_heap.c b/drivers/dma-buf/heaps/coherent_heap.c
> > > > new file mode 100644
> > > > index 0000000000000..55f53f87c4c15
> > > > --- /dev/null
> > > > +++ b/drivers/dma-buf/heaps/coherent_heap.c
> > > > @@ -0,0 +1,414 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * DMABUF heap for coherent reserved-memory regions
> > > > + *
> > > > + * Copyright (C) 2026 Red Hat, Inc.
> > > > + * Author: Albert Esteve <aesteve@redhat.com>
> > > > + *
> > > > + */
> > > > +
> > > > +#include <linux/dma-buf.h>
> > > > +#include <linux/dma-heap.h>
> > > > +#include <linux/dma-map-ops.h>
> > > > +#include <linux/dma-mapping.h>
> > > > +#include <linux/err.h>
> > > > +#include <linux/highmem.h>
> > > > +#include <linux/iosys-map.h>
> > > > +#include <linux/of_reserved_mem.h>
> > > > +#include <linux/scatterlist.h>
> > > > +#include <linux/slab.h>
> > > > +#include <linux/vmalloc.h>
> > > > +
> > > > +struct coherent_heap {
> > > > +     struct dma_heap *heap;
> > > > +     struct reserved_mem *rmem;
> > > > +     char *name;
> > > > +};
> > > > +
> > > > +struct coherent_heap_buffer {
> > > > +     struct coherent_heap *heap;
> > > > +     struct list_head attachments;
> > > > +     struct mutex lock;
> > > > +     unsigned long len;
> > > > +     dma_addr_t dma_addr;
> > > > +     void *alloc_vaddr;
> > > > +     struct page **pages;
> > > > +     pgoff_t pagecount;
> > > > +     int vmap_cnt;
> > > > +     void *vaddr;
> > > > +};
> > > > +
> > > > +struct dma_heap_attachment {
> > > > +     struct device *dev;
> > > > +     struct sg_table table;
> > > > +     struct list_head list;
> > > > +     bool mapped;
> > > > +};
> > > > +
> > > > +static int coherent_heap_attach(struct dma_buf *dmabuf,
> > > > +                             struct dma_buf_attachment *attachment)
> > > > +{
> > > > +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> > > > +     struct dma_heap_attachment *a;
> > > > +     int ret;
> > > > +
> > > > +     a = kzalloc_obj(*a);
> > > > +     if (!a)
> > > > +             return -ENOMEM;
> > > > +
> > > > +     ret = sg_alloc_table_from_pages(&a->table, buffer->pages,
> > > > +                                     buffer->pagecount, 0,
> > > > +                                     buffer->pagecount << PAGE_SHIFT,
> > > > +                                     GFP_KERNEL);
> > > > +     if (ret) {
> > > > +             kfree(a);
> > > > +             return ret;
> > > > +     }
> > > > +
> > > > +     a->dev = attachment->dev;
> > > > +     INIT_LIST_HEAD(&a->list);
> > > > +     a->mapped = false;
> > > > +
> > > > +     attachment->priv = a;
> > > > +
> > > > +     mutex_lock(&buffer->lock);
> > > > +     list_add(&a->list, &buffer->attachments);
> > > > +     mutex_unlock(&buffer->lock);
> > > > +
> > > > +     return 0;
> > > > +}
> > > > +
> > > > +static void coherent_heap_detach(struct dma_buf *dmabuf,
> > > > +                              struct dma_buf_attachment *attachment)
> > > > +{
> > > > +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> > > > +     struct dma_heap_attachment *a = attachment->priv;
> > > > +
> > > > +     mutex_lock(&buffer->lock);
> > > > +     list_del(&a->list);
> > > > +     mutex_unlock(&buffer->lock);
> > > > +
> > > > +     sg_free_table(&a->table);
> > > > +     kfree(a);
> > > > +}
> > > > +
> > > > +static struct sg_table *coherent_heap_map_dma_buf(struct dma_buf_attachment *attachment,
> > > > +                                               enum dma_data_direction direction)
> > > > +{
> > > > +     struct dma_heap_attachment *a = attachment->priv;
> > > > +     struct sg_table *table = &a->table;
> > > > +     int ret;
> > > > +
> > > > +     ret = dma_map_sgtable(attachment->dev, table, direction, 0);
> > > > +     if (ret)
> > > > +             return ERR_PTR(-ENOMEM);
> > > > +     a->mapped = true;
> > > > +
> > > > +     return table;
> > > > +}
> > > > +
> > > > +static void coherent_heap_unmap_dma_buf(struct dma_buf_attachment *attachment,
> > > > +                                     struct sg_table *table,
> > > > +                                     enum dma_data_direction direction)
> > > > +{
> > > > +     struct dma_heap_attachment *a = attachment->priv;
> > > > +
> > > > +     a->mapped = false;
> > > > +     dma_unmap_sgtable(attachment->dev, table, direction, 0);
> > > > +}
> > > > +
> > > > +static int coherent_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> > > > +                                               enum dma_data_direction direction)
> > > > +{
> > > > +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> > > > +     struct dma_heap_attachment *a;
> > > > +
> > > > +     mutex_lock(&buffer->lock);
> > > > +     if (buffer->vmap_cnt)
> > > > +             invalidate_kernel_vmap_range(buffer->vaddr, buffer->len);
> > > > +
> > > > +     list_for_each_entry(a, &buffer->attachments, list) {
> > > > +             if (!a->mapped)
> > > > +                     continue;
> > > > +             dma_sync_sgtable_for_cpu(a->dev, &a->table, direction);
> > > > +     }
> > > > +     mutex_unlock(&buffer->lock);
> > > > +
> > > > +     return 0;
> > > > +}
> > > > +
> > > > +static int coherent_heap_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
> > > > +                                             enum dma_data_direction direction)
> > > > +{
> > > > +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> > > > +     struct dma_heap_attachment *a;
> > > > +
> > > > +     mutex_lock(&buffer->lock);
> > > > +     if (buffer->vmap_cnt)
> > > > +             flush_kernel_vmap_range(buffer->vaddr, buffer->len);
> > > > +
> > > > +     list_for_each_entry(a, &buffer->attachments, list) {
> > > > +             if (!a->mapped)
> > > > +                     continue;
> > > > +             dma_sync_sgtable_for_device(a->dev, &a->table, direction);
> > > > +     }
> > > > +     mutex_unlock(&buffer->lock);
> > > > +
> > > > +     return 0;
> > > > +}
> > > > +
> > > > +static int coherent_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
> > > > +{
> > > > +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> > > > +     struct coherent_heap *coh_heap = buffer->heap;
> > > > +     struct device *heap_dev = dma_heap_get_dev(coh_heap->heap);
> > > > +
> > > > +     return dma_mmap_coherent(heap_dev, vma, buffer->alloc_vaddr,
> > > > +                              buffer->dma_addr, buffer->len);
> > > > +}
> > > > +
> > > > +static void *coherent_heap_do_vmap(struct coherent_heap_buffer *buffer)
> > > > +{
> > > > +     void *vaddr;
> > > > +
> > > > +     vaddr = vmap(buffer->pages, buffer->pagecount, VM_MAP, PAGE_KERNEL);
> > > > +     if (!vaddr)
> > > > +             return ERR_PTR(-ENOMEM);
> > > > +
> > > > +     return vaddr;
> > > > +}
> > > > +
> > > > +static int coherent_heap_vmap(struct dma_buf *dmabuf, struct iosys_map *map)
> > > > +{
> > > > +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> > > > +     void *vaddr;
> > > > +     int ret = 0;
> > > > +
> > > > +     mutex_lock(&buffer->lock);
> > > > +     if (buffer->vmap_cnt) {
> > > > +             buffer->vmap_cnt++;
> > > > +             iosys_map_set_vaddr(map, buffer->vaddr);
> > > > +             goto out;
> > > > +     }
> > > > +
> > > > +     vaddr = coherent_heap_do_vmap(buffer);
> > > > +     if (IS_ERR(vaddr)) {
> > > > +             ret = PTR_ERR(vaddr);
> > > > +             goto out;
> > > > +     }
> > > > +
> > > > +     buffer->vaddr = vaddr;
> > > > +     buffer->vmap_cnt++;
> > > > +     iosys_map_set_vaddr(map, buffer->vaddr);
> > > > +out:
> > > > +     mutex_unlock(&buffer->lock);
> > > > +
> > > > +     return ret;
> > > > +}
> > > > +
> > > > +static void coherent_heap_vunmap(struct dma_buf *dmabuf, struct iosys_map *map)
> > > > +{
> > > > +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> > > > +
> > > > +     mutex_lock(&buffer->lock);
> > > > +     if (!--buffer->vmap_cnt) {
> > > > +             vunmap(buffer->vaddr);
> > > > +             buffer->vaddr = NULL;
> > > > +     }
> > > > +     mutex_unlock(&buffer->lock);
> > > > +     iosys_map_clear(map);
> > > > +}
> > > > +
> > > > +static void coherent_heap_dma_buf_release(struct dma_buf *dmabuf)
> > > > +{
> > > > +     struct coherent_heap_buffer *buffer = dmabuf->priv;
> > > > +     struct coherent_heap *coh_heap = buffer->heap;
> > > > +     struct device *heap_dev = dma_heap_get_dev(coh_heap->heap);
> > > > +
> > > > +     if (buffer->vmap_cnt > 0) {
> > > > +             WARN(1, "%s: buffer still mapped in the kernel\n", __func__);
> > > > +             vunmap(buffer->vaddr);
> > > > +             buffer->vaddr = NULL;
> > > > +             buffer->vmap_cnt = 0;
> > > > +     }
> > > > +
> > > > +     if (buffer->alloc_vaddr)
> > > > +             dma_free_coherent(heap_dev, buffer->len, buffer->alloc_vaddr,
> > > > +                               buffer->dma_addr);
> > > > +     kfree(buffer->pages);
> > > > +     kfree(buffer);
> > > > +}
> > > > +
> > > > +static const struct dma_buf_ops coherent_heap_buf_ops = {
> > > > +     .attach = coherent_heap_attach,
> > > > +     .detach = coherent_heap_detach,
> > > > +     .map_dma_buf = coherent_heap_map_dma_buf,
> > > > +     .unmap_dma_buf = coherent_heap_unmap_dma_buf,
> > > > +     .begin_cpu_access = coherent_heap_dma_buf_begin_cpu_access,
> > > > +     .end_cpu_access = coherent_heap_dma_buf_end_cpu_access,
> > > > +     .mmap = coherent_heap_mmap,
> > > > +     .vmap = coherent_heap_vmap,
> > > > +     .vunmap = coherent_heap_vunmap,
> > > > +     .release = coherent_heap_dma_buf_release,
> > > > +};
> > > > +
> > > > +static struct dma_buf *coherent_heap_allocate(struct dma_heap *heap,
> > > > +                                           unsigned long len,
> > > > +                                           u32 fd_flags,
> > > > +                                           u64 heap_flags)
> > > > +{
> > > > +     struct coherent_heap *coh_heap;
> > > > +     struct coherent_heap_buffer *buffer;
> > > > +     struct device *heap_dev;
> > > > +     DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> > > > +     size_t size = PAGE_ALIGN(len);
> > > > +     pgoff_t pagecount = size >> PAGE_SHIFT;
> > > > +     struct dma_buf *dmabuf;
> > > > +     int ret = -ENOMEM;
> > > > +     pgoff_t pg;
> > > > +
> > > > +     coh_heap = dma_heap_get_drvdata(heap);
> > > > +     if (!coh_heap)
> > > > +             return ERR_PTR(-EINVAL);
> > > > +
> > > > +     heap_dev = dma_heap_get_dev(coh_heap->heap);
> > > > +     if (!heap_dev)
> > > > +             return ERR_PTR(-ENODEV);
> > > > +
> > > > +     buffer = kzalloc_obj(*buffer);
> > > > +     if (!buffer)
> > > > +             return ERR_PTR(-ENOMEM);
> > > > +
> > > > +     INIT_LIST_HEAD(&buffer->attachments);
> > > > +     mutex_init(&buffer->lock);
> > > > +     buffer->len = size;
> > > > +     buffer->heap = coh_heap;
> > > > +     buffer->pagecount = pagecount;
> > > > +
> > > > +     buffer->alloc_vaddr = dma_alloc_coherent(heap_dev, buffer->len,
> > > > +                                              &buffer->dma_addr, GFP_KERNEL);
> > > 
> > > You are doing this DMA allocation using a non-DMA pseudo-device (heap_dev).
> > > This is why you need to do that dma_coerce_mask_and_coherent(64) nonsense, you
> > > are doing a DMA alloc for the CPU itself. This might still work, but only if
> > > dma_map_sgtable() can handle swiotlb/iommu for all attaching devices at map
> > > time.
> > 
> > The concern is valid. We're allocating via a synthetic device, which
> > ties the allocation to that device's DMA domain. I looked deeper into
> > this trying to address the concern.
> > 
> > The approach works because dma_map_sgtable() handles both
> > dma_map_direct and use_dma_iommu cases in __dma_map_sg_attrs(). For
> > each physical address in the sg_table (extracted via sg_phys()), it
> > creates device-specific DMA mappings:
> > - For direct mapping: it checks if the address is directly accessible
> > (dma_capable()), and if not, it falls back to swiotlb.
> > - For IOMMU: it creates mappings that allow the device to access
> > physical addresses.
> > 
> > This means every attached device gets its own device-specific DMA
> > mapping, properly handling cases where the physical addresses are
> > inaccessible or have DMA constraints.
> > 
> 
> While this means it might still "work" it won't always be ideal. Take
> the case where the consuming device(s) have a 32bit address restriction,
> if the allocation was done using the real devices then the backing buffer
> itself would be allocated in <32bit mem. Whereas here the allocation
> could end up in >32bit mem, as the CPU/synthetic device supports that.
> Then each mapping device would instead get a bounce buffer.
> 
> (this example might not be great as we usually know the address of
> carveout/reserved memory regions, but substitute in whatever restriction
> makes more sense)
> 
> These non-reusable carveouts tend to be made for some specific device, and
> they are made specifically because that device has some memory restriction.
> So we might run into the situation above more than one would expect.
> 
> Not a blocker here, but just something worth thinking on.

As I detailed in the previous version [1] the main idea behind that work
is to allow to get rid of dma_alloc_attrs for framework and drivers to
allocate from the heaps instead.

Robin was saying he wasn't comfortable with exposing this heap to
userspace, and we're saying here that maybe this might not always work
anyway (or at least that we couldn't test it fully).

Maybe the best thing is to defer this series until we are at a point
where we can start enabling the "heap allocations" in frameworks then?
Hopefully we will have hardware to test it with by then, and we might
not even need to expose it to userspace at all but only to the kernel.

What do you think?
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2026-03-16 12:09 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-06 10:36 [PATCH v3 0/6] dma-buf: heaps: add coherent reserved-memory heap Albert Esteve
2026-03-06 10:36 ` [PATCH v3 1/6] dma-buf: dma-heap: Keep track of the heap device struct Albert Esteve
2026-03-10 14:37   ` Andrew Davis
2026-03-10 15:46     ` Albert Esteve
2026-03-06 10:36 ` [PATCH v3 2/6] dma-buf: dma-heap: split dma_heap_add Albert Esteve
2026-03-06 10:36 ` [PATCH v3 3/6] of_reserved_mem: add a helper for rmem device_init op Albert Esteve
2026-03-13 23:06   ` Rob Herring
2026-03-16 10:54     ` Albert Esteve
2026-03-06 10:36 ` [PATCH v3 4/6] dma: coherent: store reserved memory coherent regions Albert Esteve
2026-03-06 10:36 ` [PATCH v3 5/6] dma-buf: heaps: Add Coherent heap to dmabuf heaps Albert Esteve
2026-03-10 15:34   ` Andrew Davis
2026-03-11 10:19     ` Albert Esteve
2026-03-11 13:18       ` Andrew Davis
2026-03-11 15:28         ` Albert Esteve
2026-03-16 12:08         ` Maxime Ripard
2026-03-06 10:36 ` [PATCH v3 6/6] dma-buf: heaps: coherent: Turn heap into a module Albert Esteve

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox