[EARLY RFC][PATCH] dma-buf: Add dma-buf heaps framework

[John Stultz]

On Mon, Feb 25, 2019 at 6:36 AM Andrew F. Davis <afd at ti.com> wrote:
>
> This framework allows a unified userspace interface for dma-buf
> exporters, allowing userland to allocate specific types of memory
> for use in dma-buf sharing.
>
> Each heap is given its own device node, which a user can allocate
> a dma-buf fd from using the DMA_HEAP_IOC_ALLOC.
>
> Signed-off-by: Andrew F. Davis <afd at ti.com>
> ---
>
> Hello all,
>
> I had a little less time over the weekend than I thought I would to
> clean this up more and finish the first set of user heaps, but wanted
> to get this out anyway.
>
> ION in its current form assumes a lot about the memory it exports and
> these assumptions lead to restrictions on the full range of operations
> dma-buf's can produce. Due to this if we are to add this as an extension
> of the core dma-buf then it should only be the user-space advertisement
> and allocation front-end. All dma-buf exporting and operation need to be
> placed in the control of the exporting heap. The core becomes rather
> small, just a few hundred lines you see here. This is not to say we
> should not provide helpers to make the heap exporters more simple, but
> they should only be helpers and not enforced by the core framework.
>
> Basic use model here is an exporter (dedicated heap memory driver, CMA,
> System, etc.) registers with the framework by providing a struct
> dma_heap_info which gives us the needed info to export this heap to
> userspace as a device node. Next a user will request an allocation,
> the IOCTL is parsed and the request made to a heap provided alloc() op.
> The heap should return the filled out struct dma_heap_buffer, including
> exporting the buffer as a dma-buf. This dma-buf we then return to the
> user as a fd. Buffer free is still a bit open as we need to update some
> stats and free some memory, but the release operation is controlled by
> the heap exporter, so some hook will have to be created.
>
> It all needs a bit more work, and I'm sure I'll find missing parts when
> I add some more heaps, but hopefully this framework is simple enough that
> it does not impede the implementation of all functionality once provided
> by ION (shrinker, delayed free), nor any new functionality needed for
> future heap exporting devices.
>
> Thanks,
> Andrew
>
>  drivers/dma-buf/Kconfig       |  12 ++
>  drivers/dma-buf/Makefile      |   1 +
>  drivers/dma-buf/dma-heap.c    | 268 ++++++++++++++++++++++++++++++++++
>  include/linux/dma-heap.h      |  57 ++++++++
>  include/uapi/linux/dma-heap.h |  54 +++++++
>  5 files changed, 392 insertions(+)
>  create mode 100644 drivers/dma-buf/dma-heap.c
>  create mode 100644 include/linux/dma-heap.h
>  create mode 100644 include/uapi/linux/dma-heap.h
>
> diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig
> index 2e5a0faa2cb1..30b0d7c83945 100644
> --- a/drivers/dma-buf/Kconfig
> +++ b/drivers/dma-buf/Kconfig
> @@ -39,4 +39,16 @@ config UDMABUF
>           A driver to let userspace turn memfd regions into dma-bufs.
>           Qemu can use this to create host dmabufs for guest framebuffers.
>
> +menuconfig DMABUF_HEAPS
> +       bool "DMA-BUF Userland Memory Heaps"
> +       depends on HAS_DMA && MMU
> +       select GENERIC_ALLOCATOR
> +       select DMA_SHARED_BUFFER
> +       help
> +         Choose this option to enable the DMA-BUF userland memory heaps,
> +         this allows userspace to allocate dma-bufs that can be shared between
> +         drivers.
> +
> +source "drivers/dma-buf/heaps/Kconfig"
> +
>  endmenu
> diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
> index 0913a6ccab5a..b0332f143413 100644
> --- a/drivers/dma-buf/Makefile
> +++ b/drivers/dma-buf/Makefile
> @@ -1,4 +1,5 @@
>  obj-y := dma-buf.o dma-fence.o dma-fence-array.o reservation.o seqno-fence.o
> +obj-$(CONFIG_DMABUF_HEAPS)     += dma-heap.o
>  obj-$(CONFIG_SYNC_FILE)                += sync_file.o
>  obj-$(CONFIG_SW_SYNC)          += sw_sync.o sync_debug.o
>  obj-$(CONFIG_UDMABUF)          += udmabuf.o
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> new file mode 100644
> index 000000000000..72ed225fa892
> --- /dev/null
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -0,0 +1,268 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Framework for userspace DMA-BUF allocations
> + *
> + * Copyright (C) 2011 Google, Inc.
> + * Copyright (C) 2019 Linaro Ltd.
> + */
> +
> +#include <linux/cdev.h>
> +#include <linux/debugfs.h>
> +#include <linux/device.h>
> +#include <linux/dma-buf.h>
> +#include <linux/err.h>
> +#include <linux/idr.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
> +
> +#include <linux/dma-heap.h>
> +#include <uapi/linux/dma-heap.h>
> +
> +#define DEVNAME "dma_heap"
> +
> +#define NUM_HEAP_MINORS 128
> +static DEFINE_IDR(dma_heap_idr);
> +static DEFINE_MUTEX(minor_lock); /* Protect idr accesses */
> +
> +dev_t dma_heap_devt;
> +struct class *dma_heap_class;
> +struct list_head dma_heap_list;
> +struct dentry *dma_heap_debug_root;
> +
> +/**
> + * struct dma_heap - represents a dmabuf heap in the system
> + * @name:              used for debugging/device-node name
> + * @ops:               ops struct for this heap
> + * @minor              minor number of this heap device
> + * @heap_devt          heap device node
> + * @heap_cdev          heap char device
> + * @num_of_buffers     the number of currently allocated buffers
> + * @num_of_alloc_bytes the number of allocated bytes
> + * @alloc_bytes_wm     the number of allocated bytes watermark
> + * @stat_lock          lock for heap statistics
> + *
> + * Represents a heap of memory from which buffers can be made.
> + */
> +struct dma_heap {
> +       const char *name;
> +       struct dma_heap_ops *ops;
> +       unsigned int minor;
> +       dev_t heap_devt;
> +       struct cdev heap_cdev;
> +
> +       /* heap statistics */
> +       u64 num_of_buffers;
> +       u64 num_of_alloc_bytes;
> +       u64 alloc_bytes_wm;
> +       spinlock_t stat_lock;
> +};

One issue I've run into here implementing the heaps on top of this:

So.. you've defined the dma_heap here in the dma-heap.c, but this
structure needs to be visible to the heap implementations, as their
allocate functions look like:
       int (*allocate)(struct dma_heap *heap,
                       struct dma_heap_buffer *buffer,
                       unsigned long len,
                       unsigned long flags);

Plus the dma_heap duplicates the the dma_heap_info fields.

It seems like the dma_heap is what the heap implementation should
allocate register, so it can traverse via container_of up to its own
data structure, rather then doing it in dma_heap_add().

thanks
-john