Messages in this thread |  | | Date | Tue, 10 Mar 2026 10:34:33 -0500 | | Subject | Re: [PATCH v3 5/6] dma-buf: heaps: Add Coherent heap to dmabuf heaps | | From | Andrew Davis <> |
| |
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"); >
|  |