Hi, On Tue, Mar 03, 2026 at 01:33:47PM +0100, Albert Esteve wrote: > Add a dma-buf heap for DT coherent reserved-memory > (i.e., 'shared-dma-pool' without 'reusable' property), > exposing one heap per region for userspace buffers. > > 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 <[email protected]> > --- > drivers/dma-buf/dma-heap.c | 4 +- > drivers/dma-buf/heaps/Kconfig | 9 + > drivers/dma-buf/heaps/Makefile | 1 + > drivers/dma-buf/heaps/coherent_heap.c | 426 > ++++++++++++++++++++++++++++++++++ > include/linux/dma-heap.h | 11 + > include/linux/dma-map-ops.h | 7 + > 6 files changed, 456 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c > index 88189d4e48561..ba87e5ac16ae2 100644 > --- a/drivers/dma-buf/dma-heap.c > +++ b/drivers/dma-buf/dma-heap.c > @@ -390,8 +390,8 @@ struct dma_heap *dma_heap_add(const struct > dma_heap_export_info *exp_info) > > heap = dma_heap_create(exp_info); > if (IS_ERR(heap)) { > - pr_err("dma_heap: failed to create heap (%d)\n", PTR_ERR(heap)); > - return PTR_ERR(heap); > + pr_err("dma_heap: failed to create heap (%ld)\n", > PTR_ERR(heap)); > + return ERR_CAST(heap);
This looks unrelated and should possibly be squashed into the previous
patch that introduces dma_heap_create()?
> +static int coherent_heap_init_dma_mask(struct device *dev)
> +{
> + int ret;
> +
> + ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
> + if (!ret)
> + return 0;
> +
> + /* Fallback to 32-bit DMA mask */
> + return dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
> +}
Why do you need to mess with the DMA mask? I'd expect that device to be
able to access everything.
> +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 = coherent_heap_init_dma_mask(heap_dev);
> + 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;
> + }
I guess it's more of a comment about your previous patch, but it's not
clear to me why you needed to split dma_heap_add into dma_heap_create /
_register. Can you expand a bit?
> diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
> index 1b0ea43ba66c3..77e6cb66ffce1 100644
> --- a/include/linux/dma-heap.h
> +++ b/include/linux/dma-heap.h
> @@ -9,10 +9,12 @@
> #ifndef _DMA_HEAPS_H
> #define _DMA_HEAPS_H
>
> +#include <linux/errno.h>
> #include <linux/types.h>
>
> struct dma_heap;
> struct device;
> +struct reserved_mem;
>
> /**
> * struct dma_heap_ops - ops to operate on a given heap
> @@ -53,4 +55,13 @@ struct dma_heap *dma_heap_add(const struct
> dma_heap_export_info *exp_info);
>
> extern bool mem_accounting;
>
> +#if IS_ENABLED(CONFIG_DMABUF_HEAPS_COHERENT)
> +int dma_heap_coherent_register(struct reserved_mem *rmem);
> +#else
> +static inline int dma_heap_coherent_register(struct reserved_mem *rmem)
> +{
> + return -EOPNOTSUPP;
> +}
> +#endif
> +
> #endif /* _DMA_HEAPS_H */
Do you still need that now that you switched to an iterator-like
function?
> 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
To preserve bisectability, you shouldn't do it that way. Introduce this
function into a preliminary patch, and then use it in this one.
Maxime
signature.asc
Description: PGP signature
