[RESEND,v16,1/5] dma-buf: Add dma-buf heaps framework

Message ID 20191203172641.66642-2-john.stultz@linaro.org
State New
Headers show
Series
  • DMA-BUF Heaps (destaging ION)
Related show

Commit Message

John Stultz Dec. 3, 2019, 5:26 p.m.
From: "Andrew F. Davis" <afd@ti.com>


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.

This code is an evoluiton of the Android ION implementation,
and a big thanks is due to its authors/maintainers over time
for their effort:
  Rebecca Schultz Zavin, Colin Cross, Benjamin Gaignard,
  Laura Abbott, and many other contributors!

Cc: Laura Abbott <labbott@redhat.com>
Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Liam Mark <lmark@codeaurora.org>
Cc: Pratik Patel <pratikp@codeaurora.org>
Cc: Brian Starkey <Brian.Starkey@arm.com>
Cc: Vincent Donnefort <Vincent.Donnefort@arm.com>
Cc: Sudipto Paul <Sudipto.Paul@arm.com>
Cc: Andrew F. Davis <afd@ti.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Chenbo Feng <fengc@google.com>
Cc: Alistair Strachan <astrachan@google.com>
Cc: Hridya Valsaraju <hridya@google.com>
Cc: Sandeep Patil <sspatil@google.com>
Cc: Hillf Danton <hdanton@sina.com>
Cc: Dave Airlie <airlied@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Brian Starkey <brian.starkey@arm.com>

Acked-by: Sandeep Patil <sspatil@android.com>

Signed-off-by: Andrew F. Davis <afd@ti.com>

Signed-off-by: John Stultz <john.stultz@linaro.org>

---
v2:
* Folded down fixes I had previously shared in implementing
  heaps
* Make flags a u64 (Suggested by Laura)
* Add PAGE_ALIGN() fix to the core alloc funciton
* IOCTL fixups suggested by Brian
* Added fixes suggested by Benjamin
* Removed core stats mgmt, as that should be implemented by
  per-heap code
* Changed alloc to return a dma-buf fd, rather than a buffer
  (as it simplifies error handling)
v3:
* Removed scare-quotes in MAINTAINERS email address
* Get rid of .release function as it didn't do anything (from
  Christoph)
* Renamed filp to file (suggested by Christoph)
* Split out ioctl handling to separate function (suggested by
  Christoph)
* Add comment documenting PAGE_ALIGN usage (suggested by Brian)
* Switch from idr to Xarray (suggested by Brian)
* Fixup cdev creation (suggested by Brian)
* Avoid EXPORT_SYMBOL until we finalize modules (suggested by
  Brian)
* Make struct dma_heap internal only (folded in from Andrew)
* Small cleanups suggested by GregKH
* Provide class->devnode callback to get consistent /dev/
  subdirectory naming (Suggested by Bjorn)
v4:
* Folded down dma-heap.h change that was in a following patch
* Added fd_flags entry to allocation structure and pass it
  through to heap code for use on dma-buf fd creation (suggested
  by Benjamin)
v5:
* Minor cleanups
v6:
* Improved error path handling, minor whitespace fixes, both
  suggested by Brian
v7:
* Longer Kconfig description to quiet checkpatch warnings
* Re-add compat_ioctl bits (Hridya noticed 32bit userland wasn't
  working)
v8:
* Make struct dma_heap_ops consts (Suggested by Christoph)
* Checkpatch whitespace fixups
v9:
* Minor cleanups suggested by Brian Starkey
* Rename dma_heap_get_data->dma_heap_get_drvdata suggested
  by Hilf Danton
v11:
* Kconfig text improvements suggested by Randy Dunlap
v12:
* Add logic to prevent duplicately named heaps being added
* Add symbol exports for heaps as modules
v13:
* Re-remove symbol exports per discussion w/ Brian. Will
  resubmit in a separte patch for review
v14:
* Reworked ioctl handler to zero fill any difference in
  structure size, similar to what the DRM core does, as
  suggested by Dave Airlie
* Removed now unnecessary reserved bits in allocate_data
* Added get_features ioctl as suggested by Dave Airlie
* Removed pr_warn_once messages as requested by Dave
  Airlie
v15:
* Dropped the get_features ioctl as suggested by Brian
  Starkey and Daniel Vetter
* Add listhead comment suggested by Sandeep Patil
* Dropped minor value in struct dma_heap as suggested by
  Sandeep Patil
* Fix grammar typo from Brian Starkey
v16:
* Typo fix suggested by Hridya Valsaraju <hridya@google.com>
---
 MAINTAINERS                   |  18 +++
 drivers/dma-buf/Kconfig       |   9 ++
 drivers/dma-buf/Makefile      |   1 +
 drivers/dma-buf/dma-heap.c    | 297 ++++++++++++++++++++++++++++++++++
 include/linux/dma-heap.h      |  59 +++++++
 include/uapi/linux/dma-heap.h |  53 ++++++
 6 files changed, 437 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

-- 
2.17.1

Comments

Andrew F. Davis Dec. 12, 2019, 4:52 p.m. | #1
On 12/3/19 12:26 PM, John Stultz wrote:
> From: "Andrew F. Davis" <afd@ti.com>

> 

> 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.

> 

> This code is an evoluiton of the Android ION implementation,

> and a big thanks is due to its authors/maintainers over time

> for their effort:

>   Rebecca Schultz Zavin, Colin Cross, Benjamin Gaignard,

>   Laura Abbott, and many other contributors!

> 

> Cc: Laura Abbott <labbott@redhat.com>

> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>

> Cc: Sumit Semwal <sumit.semwal@linaro.org>

> Cc: Liam Mark <lmark@codeaurora.org>

> Cc: Pratik Patel <pratikp@codeaurora.org>

> Cc: Brian Starkey <Brian.Starkey@arm.com>

> Cc: Vincent Donnefort <Vincent.Donnefort@arm.com>

> Cc: Sudipto Paul <Sudipto.Paul@arm.com>

> Cc: Andrew F. Davis <afd@ti.com>

> Cc: Christoph Hellwig <hch@infradead.org>

> Cc: Chenbo Feng <fengc@google.com>

> Cc: Alistair Strachan <astrachan@google.com>

> Cc: Hridya Valsaraju <hridya@google.com>

> Cc: Sandeep Patil <sspatil@google.com>

> Cc: Hillf Danton <hdanton@sina.com>

> Cc: Dave Airlie <airlied@gmail.com>

> Cc: dri-devel@lists.freedesktop.org

> Reviewed-by: Brian Starkey <brian.starkey@arm.com>

> Acked-by: Sandeep Patil <sspatil@android.com>

> Signed-off-by: Andrew F. Davis <afd@ti.com>

> Signed-off-by: John Stultz <john.stultz@linaro.org>

> ---

> v2:

> * Folded down fixes I had previously shared in implementing

>   heaps

> * Make flags a u64 (Suggested by Laura)

> * Add PAGE_ALIGN() fix to the core alloc funciton

> * IOCTL fixups suggested by Brian

> * Added fixes suggested by Benjamin

> * Removed core stats mgmt, as that should be implemented by

>   per-heap code

> * Changed alloc to return a dma-buf fd, rather than a buffer

>   (as it simplifies error handling)

> v3:

> * Removed scare-quotes in MAINTAINERS email address

> * Get rid of .release function as it didn't do anything (from

>   Christoph)

> * Renamed filp to file (suggested by Christoph)

> * Split out ioctl handling to separate function (suggested by

>   Christoph)

> * Add comment documenting PAGE_ALIGN usage (suggested by Brian)

> * Switch from idr to Xarray (suggested by Brian)

> * Fixup cdev creation (suggested by Brian)

> * Avoid EXPORT_SYMBOL until we finalize modules (suggested by

>   Brian)

> * Make struct dma_heap internal only (folded in from Andrew)

> * Small cleanups suggested by GregKH

> * Provide class->devnode callback to get consistent /dev/

>   subdirectory naming (Suggested by Bjorn)

> v4:

> * Folded down dma-heap.h change that was in a following patch

> * Added fd_flags entry to allocation structure and pass it

>   through to heap code for use on dma-buf fd creation (suggested

>   by Benjamin)

> v5:

> * Minor cleanups

> v6:

> * Improved error path handling, minor whitespace fixes, both

>   suggested by Brian

> v7:

> * Longer Kconfig description to quiet checkpatch warnings

> * Re-add compat_ioctl bits (Hridya noticed 32bit userland wasn't

>   working)

> v8:

> * Make struct dma_heap_ops consts (Suggested by Christoph)

> * Checkpatch whitespace fixups

> v9:

> * Minor cleanups suggested by Brian Starkey

> * Rename dma_heap_get_data->dma_heap_get_drvdata suggested

>   by Hilf Danton

> v11:

> * Kconfig text improvements suggested by Randy Dunlap

> v12:

> * Add logic to prevent duplicately named heaps being added

> * Add symbol exports for heaps as modules

> v13:

> * Re-remove symbol exports per discussion w/ Brian. Will

>   resubmit in a separte patch for review

> v14:

> * Reworked ioctl handler to zero fill any difference in

>   structure size, similar to what the DRM core does, as

>   suggested by Dave Airlie

> * Removed now unnecessary reserved bits in allocate_data

> * Added get_features ioctl as suggested by Dave Airlie

> * Removed pr_warn_once messages as requested by Dave

>   Airlie

> v15:

> * Dropped the get_features ioctl as suggested by Brian

>   Starkey and Daniel Vetter

> * Add listhead comment suggested by Sandeep Patil

> * Dropped minor value in struct dma_heap as suggested by

>   Sandeep Patil

> * Fix grammar typo from Brian Starkey

> v16:

> * Typo fix suggested by Hridya Valsaraju <hridya@google.com>

> ---

>  MAINTAINERS                   |  18 +++

>  drivers/dma-buf/Kconfig       |   9 ++

>  drivers/dma-buf/Makefile      |   1 +

>  drivers/dma-buf/dma-heap.c    | 297 ++++++++++++++++++++++++++++++++++

>  include/linux/dma-heap.h      |  59 +++++++

>  include/uapi/linux/dma-heap.h |  53 ++++++

>  6 files changed, 437 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/MAINTAINERS b/MAINTAINERS

> index 8608724835dd..44b7cbc0ec29 100644

> --- a/MAINTAINERS

> +++ b/MAINTAINERS

> @@ -4971,6 +4971,24 @@ F:	include/linux/*fence.h

>  F:	Documentation/driver-api/dma-buf.rst

>  T:	git git://anongit.freedesktop.org/drm/drm-misc

>  

> +DMA-BUF HEAPS FRAMEWORK

> +M:	Sumit Semwal <sumit.semwal@linaro.org>

> +R:	Andrew F. Davis <afd@ti.com>

> +R:	Benjamin Gaignard <benjamin.gaignard@linaro.org>

> +R:	Liam Mark <lmark@codeaurora.org>

> +R:	Laura Abbott <labbott@redhat.com>

> +R:	Brian Starkey <Brian.Starkey@arm.com>

> +R:	John Stultz <john.stultz@linaro.org>

> +S:	Maintained

> +L:	linux-media@vger.kernel.org

> +L:	dri-devel@lists.freedesktop.org

> +L:	linaro-mm-sig@lists.linaro.org (moderated for non-subscribers)

> +F:	include/uapi/linux/dma-heap.h

> +F:	include/linux/dma-heap.h

> +F:	drivers/dma-buf/dma-heap.c

> +F:	drivers/dma-buf/heaps/*

> +T:	git git://anongit.freedesktop.org/drm/drm-misc

> +

>  DMA GENERIC OFFLOAD ENGINE SUBSYSTEM

>  M:	Vinod Koul <vkoul@kernel.org>

>  L:	dmaengine@vger.kernel.org

> diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig

> index a23b6752d11a..bffa58fc3e6e 100644

> --- a/drivers/dma-buf/Kconfig

> +++ b/drivers/dma-buf/Kconfig

> @@ -44,4 +44,13 @@ config DMABUF_SELFTESTS

>  	default n

>  	depends on DMA_SHARED_BUFFER

>  

> +menuconfig DMABUF_HEAPS

> +	bool "DMA-BUF Userland Memory Heaps"

> +	select DMA_SHARED_BUFFER

> +	help

> +	  Choose this option to enable the DMA-BUF userland memory heaps.

> +	  This options creates per heap chardevs in /dev/dma_heap/ which

> +	  allows userspace to allocate dma-bufs that can be shared

> +	  between drivers.

> +

>  endmenu

> diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile

> index 03479da06422..caee5eb3d351 100644

> --- a/drivers/dma-buf/Makefile

> +++ b/drivers/dma-buf/Makefile

> @@ -1,6 +1,7 @@

>  # SPDX-License-Identifier: GPL-2.0-only

>  obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \

>  	 dma-resv.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..4f04d104ae61

> --- /dev/null

> +++ b/drivers/dma-buf/dma-heap.c

> @@ -0,0 +1,297 @@

> +// 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/xarray.h>

> +#include <linux/list.h>

> +#include <linux/slab.h>

> +#include <linux/uaccess.h>

> +#include <linux/syscalls.h>

> +#include <linux/dma-heap.h>

> +#include <uapi/linux/dma-heap.h>

> +

> +#define DEVNAME "dma_heap"

> +

> +#define NUM_HEAP_MINORS 128

> +

> +/**

> + * struct dma_heap - represents a dmabuf heap in the system

> + * @name:		used for debugging/device-node name

> + * @ops:		ops struct for this heap

> + * @heap_devt		heap device node

> + * @list		list head connecting to list of heaps

> + * @heap_cdev		heap char device

> + *

> + * Represents a heap of memory from which buffers can be made.

> + */

> +struct dma_heap {

> +	const char *name;

> +	const struct dma_heap_ops *ops;

> +	void *priv;

> +	dev_t heap_devt;

> +	struct list_head list;

> +	struct cdev heap_cdev;

> +};

> +

> +static LIST_HEAD(heap_list);

> +static DEFINE_MUTEX(heap_list_lock);

> +static dev_t dma_heap_devt;

> +static struct class *dma_heap_class;

> +static DEFINE_XARRAY_ALLOC(dma_heap_minors);

> +

> +static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,

> +				 unsigned int fd_flags,

> +				 unsigned int heap_flags)

> +{

> +	/*

> +	 * Allocations from all heaps have to begin

> +	 * and end on page boundaries.

> +	 */

> +	len = PAGE_ALIGN(len);

> +	if (!len)

> +		return -EINVAL;

> +

> +	return heap->ops->allocate(heap, len, fd_flags, heap_flags);

> +}

> +

> +static int dma_heap_open(struct inode *inode, struct file *file)

> +{

> +	struct dma_heap *heap;

> +

> +	heap = xa_load(&dma_heap_minors, iminor(inode));

> +	if (!heap) {

> +		pr_err("dma_heap: minor %d unknown.\n", iminor(inode));

> +		return -ENODEV;

> +	}

> +

> +	/* instance data as context */

> +	file->private_data = heap;

> +	nonseekable_open(inode, file);

> +

> +	return 0;

> +}

> +

> +static long dma_heap_ioctl_allocate(struct file *file, void *data)

> +{

> +	struct dma_heap_allocation_data *heap_allocation = data;

> +	struct dma_heap *heap = file->private_data;

> +	int fd;

> +

> +	if (heap_allocation->fd)

> +		return -EINVAL;

> +

> +	if (heap_allocation->fd_flags & ~DMA_HEAP_VALID_FD_FLAGS)

> +		return -EINVAL;

> +

> +	if (heap_allocation->heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS)

> +		return -EINVAL;

> +

> +	fd = dma_heap_buffer_alloc(heap, heap_allocation->len,

> +				   heap_allocation->fd_flags,

> +				   heap_allocation->heap_flags);

> +	if (fd < 0)

> +		return fd;

> +

> +	heap_allocation->fd = fd;

> +

> +	return 0;

> +}

> +

> +unsigned int dma_heap_ioctl_cmds[] = {

> +	DMA_HEAP_IOC_ALLOC,

> +};

> +

> +static long dma_heap_ioctl(struct file *file, unsigned int ucmd,

> +			   unsigned long arg)

> +{

> +	char stack_kdata[128];

> +	char *kdata = stack_kdata;

> +	unsigned int kcmd;

> +	unsigned int in_size, out_size, drv_size, ksize;

> +	int nr = _IOC_NR(ucmd);

> +	int ret = 0;

> +

> +	if (nr >= ARRAY_SIZE(dma_heap_ioctl_cmds))

> +		return -EINVAL;

> +

> +	/* Get the kernel ioctl cmd that matches */

> +	kcmd = dma_heap_ioctl_cmds[nr];

> +

> +	/* Figure out the delta between user cmd size and kernel cmd size */

> +	drv_size = _IOC_SIZE(kcmd);

> +	out_size = _IOC_SIZE(ucmd);

> +	in_size = out_size;

> +	if ((ucmd & kcmd & IOC_IN) == 0)

> +		in_size = 0;

> +	if ((ucmd & kcmd & IOC_OUT) == 0)

> +		out_size = 0;

> +	ksize = max(max(in_size, out_size), drv_size);

> +

> +	/* If necessary, allocate buffer for ioctl argument */

> +	if (ksize > sizeof(stack_kdata)) {

> +		kdata = kmalloc(ksize, GFP_KERNEL);

> +		if (!kdata)

> +			return -ENOMEM;

> +	}

> +

> +	if (copy_from_user(kdata, (void __user *)arg, in_size) != 0) {

> +		ret = -EFAULT;

> +		goto err;

> +	}

> +

> +	/* zero out any difference between the kernel/user structure size */

> +	if (ksize > in_size)

> +		memset(kdata + in_size, 0, ksize - in_size);

> +

> +	switch (kcmd) {

> +	case DMA_HEAP_IOC_ALLOC:

> +		ret = dma_heap_ioctl_allocate(file, kdata);

> +		break;

> +	default:

> +		return -ENOTTY;

> +	}

> +

> +	if (copy_to_user((void __user *)arg, kdata, out_size) != 0)

> +		ret = -EFAULT;

> +err:

> +	if (kdata != stack_kdata)

> +		kfree(kdata);

> +	return ret;

> +}

> +

> +static const struct file_operations dma_heap_fops = {

> +	.owner          = THIS_MODULE,

> +	.open		= dma_heap_open,

> +	.unlocked_ioctl = dma_heap_ioctl,

> +#ifdef CONFIG_COMPAT

> +	.compat_ioctl	= dma_heap_ioctl,

> +#endif

> +};

> +

> +/**

> + * dma_heap_get_drvdata() - get per-subdriver data for the heap

> + * @heap: DMA-Heap to retrieve private data for

> + *

> + * Returns:

> + * The per-subdriver data for the heap.

> + */

> +void *dma_heap_get_drvdata(struct dma_heap *heap)

> +{

> +	return heap->priv;

> +}

> +

> +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;

> +

> +	if (!exp_info->name || !strcmp(exp_info->name, "")) {

> +		pr_err("dma_heap: Cannot add heap without a name\n");

> +		return ERR_PTR(-EINVAL);

> +	}

> +

> +	if (!exp_info->ops || !exp_info->ops->allocate) {

> +		pr_err("dma_heap: Cannot add heap with invalid ops struct\n");

> +		return ERR_PTR(-EINVAL);

> +	}

> +

> +	/* check the name is unique */

> +	mutex_lock(&heap_list_lock);

> +	list_for_each_entry(h, &heap_list, list) {

> +		if (!strcmp(h->name, exp_info->name)) {

> +			mutex_unlock(&heap_list_lock);

> +			pr_err("dma_heap: Already registered heap named %s\n",

> +			       exp_info->name);

> +			return ERR_PTR(-EINVAL);

> +		}

> +	}

> +	mutex_unlock(&heap_list_lock);

> +

> +	heap = kzalloc(sizeof(*heap), GFP_KERNEL);

> +	if (!heap)

> +		return ERR_PTR(-ENOMEM);

> +

> +	heap->name = exp_info->name;

> +	heap->ops = exp_info->ops;

> +	heap->priv = exp_info->priv;

> +

> +	/* 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;

> +	}

> +

> +	/* Create device */

> +	heap->heap_devt = MKDEV(MAJOR(dma_heap_devt), minor);

> +

> +	cdev_init(&heap->heap_cdev, &dma_heap_fops);

> +	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;

> +	}

> +

> +	dev_ret = device_create(dma_heap_class,

> +				NULL,

> +				heap->heap_devt,

> +				NULL,

> +				heap->name);

> +	if (IS_ERR(dev_ret)) {

> +		pr_err("dma_heap: Unable to create device\n");

> +		err_ret = ERR_CAST(dev_ret);

> +		goto err2;

> +	}

> +	/* Add heap to the list */

> +	mutex_lock(&heap_list_lock);

> +	list_add(&heap->list, &heap_list);

> +	mutex_unlock(&heap_list_lock);

> +

> +	return heap;

> +

> +err2:

> +	cdev_del(&heap->heap_cdev);

> +err1:

> +	xa_erase(&dma_heap_minors, minor);

> +err0:

> +	kfree(heap);

> +	return err_ret;

> +}

> +

> +static char *dma_heap_devnode(struct device *dev, umode_t *mode)

> +{

> +	return kasprintf(GFP_KERNEL, "dma_heap/%s", dev_name(dev));

> +}

> +

> +static int dma_heap_init(void)

> +{

> +	int ret;

> +

> +	ret = alloc_chrdev_region(&dma_heap_devt, 0, NUM_HEAP_MINORS, DEVNAME);

> +	if (ret)

> +		return ret;

> +

> +	dma_heap_class = class_create(THIS_MODULE, DEVNAME);

> +	if (IS_ERR(dma_heap_class)) {

> +		unregister_chrdev_region(dma_heap_devt, NUM_HEAP_MINORS);

> +		return PTR_ERR(dma_heap_class);

> +	}

> +	dma_heap_class->devnode = dma_heap_devnode;

> +

> +	return 0;

> +}

> +subsys_initcall(dma_heap_init);

> diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h

> new file mode 100644

> index 000000000000..454e354d1ffb

> --- /dev/null

> +++ b/include/linux/dma-heap.h

> @@ -0,0 +1,59 @@

> +/* SPDX-License-Identifier: GPL-2.0 */

> +/*

> + * DMABUF Heaps Allocation Infrastructure

> + *

> + * Copyright (C) 2011 Google, Inc.

> + * Copyright (C) 2019 Linaro Ltd.

> + */

> +

> +#ifndef _DMA_HEAPS_H

> +#define _DMA_HEAPS_H

> +

> +#include <linux/cdev.h>

> +#include <linux/types.h>

> +

> +struct dma_heap;

> +

> +/**

> + * struct dma_heap_ops - ops to operate on a given heap

> + * @allocate:		allocate dmabuf and return fd

> + *

> + * allocate returns dmabuf fd  on success, -errno on error.

> + */

> +struct dma_heap_ops {

> +	int (*allocate)(struct dma_heap *heap,

> +			unsigned long len,

> +			unsigned long fd_flags,

> +			unsigned long heap_flags);

> +};

> +

> +/**

> + * struct dma_heap_export_info - information needed to export a new dmabuf heap

> + * @name:	used for debugging/device-node name

> + * @ops:	ops struct for this heap

> + * @priv:	heap exporter private data

> + *

> + * Information needed to export a new dmabuf heap.

> + */

> +struct dma_heap_export_info {

> +	const char *name;

> +	const struct dma_heap_ops *ops;

> +	void *priv;

> +};

> +

> +/**

> + * dma_heap_get_drvdata() - get per-heap driver data

> + * @heap: DMA-Heap to retrieve private data for

> + *

> + * Returns:

> + * The per-heap data for the heap.

> + */

> +void *dma_heap_get_drvdata(struct dma_heap *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);

> +

> +#endif /* _DMA_HEAPS_H */

> diff --git a/include/uapi/linux/dma-heap.h b/include/uapi/linux/dma-heap.h

> new file mode 100644

> index 000000000000..73e7f66c1cae

> --- /dev/null

> +++ b/include/uapi/linux/dma-heap.h

> @@ -0,0 +1,53 @@

> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */

> +/*

> + * DMABUF Heaps Userspace API

> + *

> + * Copyright (C) 2011 Google, Inc.

> + * Copyright (C) 2019 Linaro Ltd.

> + */

> +#ifndef _UAPI_LINUX_DMABUF_POOL_H

> +#define _UAPI_LINUX_DMABUF_POOL_H

> +

> +#include <linux/ioctl.h>

> +#include <linux/types.h>

> +

> +/**

> + * DOC: DMABUF Heaps Userspace API

> + */

> +

> +/* Valid FD_FLAGS are O_CLOEXEC, O_RDONLY, O_WRONLY, O_RDWR */

> +#define DMA_HEAP_VALID_FD_FLAGS (O_CLOEXEC | O_ACCMODE)

> +

> +/* Currently no heap flags */

> +#define DMA_HEAP_VALID_HEAP_FLAGS (0)

> +

> +/**

> + * struct dma_heap_allocation_data - metadata passed from userspace for

> + *                                      allocations

> + * @len:		size of the allocation

> + * @fd:			will be populated with a fd which provides the

> + *			handle to the allocated dma-buf

> + * @fd_flags:		file descriptor flags used when allocating

> + * @heap_flags:		flags passed to heap

> + *

> + * Provided by userspace as an argument to the ioctl

> + */

> +struct dma_heap_allocation_data {

> +	__u64 len;

> +	__u32 fd;

> +	__u32 fd_flags;

> +	__u64 heap_flags;

> +};

> +

> +#define DMA_HEAP_IOC_MAGIC		'H'

> +

> +/**

> + * DOC: DMA_HEAP_IOC_ALLOC - allocate memory from pool

> + *

> + * Takes a dma_heap_allocation_data struct and returns it with the fd field

> + * populated with the dmabuf handle of the allocation.

> + */

> +#define DMA_HEAP_IOC_ALLOC	_IOWR(DMA_HEAP_IOC_MAGIC, 0x0,\

> +				      struct dma_heap_allocation_data)

> +



<subsytem>_IOC_

Seems more common for the internal numberings and such, what the user
calls is more often (espesially in DRM and DMA_BUF):

<subsytem>_IOCTL_<function>

This is really just another naming nit but becouse this one really *is*
ABI then getting our prefrence right is a must, up to you here.

Andrew


> +#endif /* _UAPI_LINUX_DMABUF_POOL_H */

>
John Stultz Dec. 12, 2019, 6:33 p.m. | #2
On Thu, Dec 12, 2019 at 8:52 AM Andrew F. Davis <afd@ti.com> wrote:
> On 12/3/19 12:26 PM, John Stultz wrote:

> > +#define DMA_HEAP_IOC_MAGIC           'H'

> > +

> > +/**

> > + * DOC: DMA_HEAP_IOC_ALLOC - allocate memory from pool

> > + *

> > + * Takes a dma_heap_allocation_data struct and returns it with the fd field

> > + * populated with the dmabuf handle of the allocation.

> > + */

> > +#define DMA_HEAP_IOC_ALLOC   _IOWR(DMA_HEAP_IOC_MAGIC, 0x0,\

> > +                                   struct dma_heap_allocation_data)

> > +

>

> <subsytem>_IOC_

>

> Seems more common for the internal numberings and such, what the user

> calls is more often (espesially in DRM and DMA_BUF):

>

> <subsytem>_IOCTL_<function>

>

> This is really just another naming nit but becouse this one really *is*

> ABI then getting our prefrence right is a must, up to you here.


So yea, Sumit has already pulled these into drm-misc-next, so this
feedback is just a few days too late.

But it's not a bad suggestion, so if you want to submit a tack-on
patch to drm-misc-next I suspect we can, try to pull it in.

thanks
-john

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 8608724835dd..44b7cbc0ec29 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4971,6 +4971,24 @@  F:	include/linux/*fence.h
 F:	Documentation/driver-api/dma-buf.rst
 T:	git git://anongit.freedesktop.org/drm/drm-misc
 
+DMA-BUF HEAPS FRAMEWORK
+M:	Sumit Semwal <sumit.semwal@linaro.org>
+R:	Andrew F. Davis <afd@ti.com>
+R:	Benjamin Gaignard <benjamin.gaignard@linaro.org>
+R:	Liam Mark <lmark@codeaurora.org>
+R:	Laura Abbott <labbott@redhat.com>
+R:	Brian Starkey <Brian.Starkey@arm.com>
+R:	John Stultz <john.stultz@linaro.org>
+S:	Maintained
+L:	linux-media@vger.kernel.org
+L:	dri-devel@lists.freedesktop.org
+L:	linaro-mm-sig@lists.linaro.org (moderated for non-subscribers)
+F:	include/uapi/linux/dma-heap.h
+F:	include/linux/dma-heap.h
+F:	drivers/dma-buf/dma-heap.c
+F:	drivers/dma-buf/heaps/*
+T:	git git://anongit.freedesktop.org/drm/drm-misc
+
 DMA GENERIC OFFLOAD ENGINE SUBSYSTEM
 M:	Vinod Koul <vkoul@kernel.org>
 L:	dmaengine@vger.kernel.org
diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig
index a23b6752d11a..bffa58fc3e6e 100644
--- a/drivers/dma-buf/Kconfig
+++ b/drivers/dma-buf/Kconfig
@@ -44,4 +44,13 @@  config DMABUF_SELFTESTS
 	default n
 	depends on DMA_SHARED_BUFFER
 
+menuconfig DMABUF_HEAPS
+	bool "DMA-BUF Userland Memory Heaps"
+	select DMA_SHARED_BUFFER
+	help
+	  Choose this option to enable the DMA-BUF userland memory heaps.
+	  This options creates per heap chardevs in /dev/dma_heap/ which
+	  allows userspace to allocate dma-bufs that can be shared
+	  between drivers.
+
 endmenu
diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
index 03479da06422..caee5eb3d351 100644
--- a/drivers/dma-buf/Makefile
+++ b/drivers/dma-buf/Makefile
@@ -1,6 +1,7 @@ 
 # SPDX-License-Identifier: GPL-2.0-only
 obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \
 	 dma-resv.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..4f04d104ae61
--- /dev/null
+++ b/drivers/dma-buf/dma-heap.c
@@ -0,0 +1,297 @@ 
+// 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/xarray.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/syscalls.h>
+#include <linux/dma-heap.h>
+#include <uapi/linux/dma-heap.h>
+
+#define DEVNAME "dma_heap"
+
+#define NUM_HEAP_MINORS 128
+
+/**
+ * struct dma_heap - represents a dmabuf heap in the system
+ * @name:		used for debugging/device-node name
+ * @ops:		ops struct for this heap
+ * @heap_devt		heap device node
+ * @list		list head connecting to list of heaps
+ * @heap_cdev		heap char device
+ *
+ * Represents a heap of memory from which buffers can be made.
+ */
+struct dma_heap {
+	const char *name;
+	const struct dma_heap_ops *ops;
+	void *priv;
+	dev_t heap_devt;
+	struct list_head list;
+	struct cdev heap_cdev;
+};
+
+static LIST_HEAD(heap_list);
+static DEFINE_MUTEX(heap_list_lock);
+static dev_t dma_heap_devt;
+static struct class *dma_heap_class;
+static DEFINE_XARRAY_ALLOC(dma_heap_minors);
+
+static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
+				 unsigned int fd_flags,
+				 unsigned int heap_flags)
+{
+	/*
+	 * Allocations from all heaps have to begin
+	 * and end on page boundaries.
+	 */
+	len = PAGE_ALIGN(len);
+	if (!len)
+		return -EINVAL;
+
+	return heap->ops->allocate(heap, len, fd_flags, heap_flags);
+}
+
+static int dma_heap_open(struct inode *inode, struct file *file)
+{
+	struct dma_heap *heap;
+
+	heap = xa_load(&dma_heap_minors, iminor(inode));
+	if (!heap) {
+		pr_err("dma_heap: minor %d unknown.\n", iminor(inode));
+		return -ENODEV;
+	}
+
+	/* instance data as context */
+	file->private_data = heap;
+	nonseekable_open(inode, file);
+
+	return 0;
+}
+
+static long dma_heap_ioctl_allocate(struct file *file, void *data)
+{
+	struct dma_heap_allocation_data *heap_allocation = data;
+	struct dma_heap *heap = file->private_data;
+	int fd;
+
+	if (heap_allocation->fd)
+		return -EINVAL;
+
+	if (heap_allocation->fd_flags & ~DMA_HEAP_VALID_FD_FLAGS)
+		return -EINVAL;
+
+	if (heap_allocation->heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS)
+		return -EINVAL;
+
+	fd = dma_heap_buffer_alloc(heap, heap_allocation->len,
+				   heap_allocation->fd_flags,
+				   heap_allocation->heap_flags);
+	if (fd < 0)
+		return fd;
+
+	heap_allocation->fd = fd;
+
+	return 0;
+}
+
+unsigned int dma_heap_ioctl_cmds[] = {
+	DMA_HEAP_IOC_ALLOC,
+};
+
+static long dma_heap_ioctl(struct file *file, unsigned int ucmd,
+			   unsigned long arg)
+{
+	char stack_kdata[128];
+	char *kdata = stack_kdata;
+	unsigned int kcmd;
+	unsigned int in_size, out_size, drv_size, ksize;
+	int nr = _IOC_NR(ucmd);
+	int ret = 0;
+
+	if (nr >= ARRAY_SIZE(dma_heap_ioctl_cmds))
+		return -EINVAL;
+
+	/* Get the kernel ioctl cmd that matches */
+	kcmd = dma_heap_ioctl_cmds[nr];
+
+	/* Figure out the delta between user cmd size and kernel cmd size */
+	drv_size = _IOC_SIZE(kcmd);
+	out_size = _IOC_SIZE(ucmd);
+	in_size = out_size;
+	if ((ucmd & kcmd & IOC_IN) == 0)
+		in_size = 0;
+	if ((ucmd & kcmd & IOC_OUT) == 0)
+		out_size = 0;
+	ksize = max(max(in_size, out_size), drv_size);
+
+	/* If necessary, allocate buffer for ioctl argument */
+	if (ksize > sizeof(stack_kdata)) {
+		kdata = kmalloc(ksize, GFP_KERNEL);
+		if (!kdata)
+			return -ENOMEM;
+	}
+
+	if (copy_from_user(kdata, (void __user *)arg, in_size) != 0) {
+		ret = -EFAULT;
+		goto err;
+	}
+
+	/* zero out any difference between the kernel/user structure size */
+	if (ksize > in_size)
+		memset(kdata + in_size, 0, ksize - in_size);
+
+	switch (kcmd) {
+	case DMA_HEAP_IOC_ALLOC:
+		ret = dma_heap_ioctl_allocate(file, kdata);
+		break;
+	default:
+		return -ENOTTY;
+	}
+
+	if (copy_to_user((void __user *)arg, kdata, out_size) != 0)
+		ret = -EFAULT;
+err:
+	if (kdata != stack_kdata)
+		kfree(kdata);
+	return ret;
+}
+
+static const struct file_operations dma_heap_fops = {
+	.owner          = THIS_MODULE,
+	.open		= dma_heap_open,
+	.unlocked_ioctl = dma_heap_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl	= dma_heap_ioctl,
+#endif
+};
+
+/**
+ * dma_heap_get_drvdata() - get per-subdriver data for the heap
+ * @heap: DMA-Heap to retrieve private data for
+ *
+ * Returns:
+ * The per-subdriver data for the heap.
+ */
+void *dma_heap_get_drvdata(struct dma_heap *heap)
+{
+	return heap->priv;
+}
+
+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;
+
+	if (!exp_info->name || !strcmp(exp_info->name, "")) {
+		pr_err("dma_heap: Cannot add heap without a name\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (!exp_info->ops || !exp_info->ops->allocate) {
+		pr_err("dma_heap: Cannot add heap with invalid ops struct\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	/* check the name is unique */
+	mutex_lock(&heap_list_lock);
+	list_for_each_entry(h, &heap_list, list) {
+		if (!strcmp(h->name, exp_info->name)) {
+			mutex_unlock(&heap_list_lock);
+			pr_err("dma_heap: Already registered heap named %s\n",
+			       exp_info->name);
+			return ERR_PTR(-EINVAL);
+		}
+	}
+	mutex_unlock(&heap_list_lock);
+
+	heap = kzalloc(sizeof(*heap), GFP_KERNEL);
+	if (!heap)
+		return ERR_PTR(-ENOMEM);
+
+	heap->name = exp_info->name;
+	heap->ops = exp_info->ops;
+	heap->priv = exp_info->priv;
+
+	/* 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;
+	}
+
+	/* Create device */
+	heap->heap_devt = MKDEV(MAJOR(dma_heap_devt), minor);
+
+	cdev_init(&heap->heap_cdev, &dma_heap_fops);
+	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;
+	}
+
+	dev_ret = device_create(dma_heap_class,
+				NULL,
+				heap->heap_devt,
+				NULL,
+				heap->name);
+	if (IS_ERR(dev_ret)) {
+		pr_err("dma_heap: Unable to create device\n");
+		err_ret = ERR_CAST(dev_ret);
+		goto err2;
+	}
+	/* Add heap to the list */
+	mutex_lock(&heap_list_lock);
+	list_add(&heap->list, &heap_list);
+	mutex_unlock(&heap_list_lock);
+
+	return heap;
+
+err2:
+	cdev_del(&heap->heap_cdev);
+err1:
+	xa_erase(&dma_heap_minors, minor);
+err0:
+	kfree(heap);
+	return err_ret;
+}
+
+static char *dma_heap_devnode(struct device *dev, umode_t *mode)
+{
+	return kasprintf(GFP_KERNEL, "dma_heap/%s", dev_name(dev));
+}
+
+static int dma_heap_init(void)
+{
+	int ret;
+
+	ret = alloc_chrdev_region(&dma_heap_devt, 0, NUM_HEAP_MINORS, DEVNAME);
+	if (ret)
+		return ret;
+
+	dma_heap_class = class_create(THIS_MODULE, DEVNAME);
+	if (IS_ERR(dma_heap_class)) {
+		unregister_chrdev_region(dma_heap_devt, NUM_HEAP_MINORS);
+		return PTR_ERR(dma_heap_class);
+	}
+	dma_heap_class->devnode = dma_heap_devnode;
+
+	return 0;
+}
+subsys_initcall(dma_heap_init);
diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
new file mode 100644
index 000000000000..454e354d1ffb
--- /dev/null
+++ b/include/linux/dma-heap.h
@@ -0,0 +1,59 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * DMABUF Heaps Allocation Infrastructure
+ *
+ * Copyright (C) 2011 Google, Inc.
+ * Copyright (C) 2019 Linaro Ltd.
+ */
+
+#ifndef _DMA_HEAPS_H
+#define _DMA_HEAPS_H
+
+#include <linux/cdev.h>
+#include <linux/types.h>
+
+struct dma_heap;
+
+/**
+ * struct dma_heap_ops - ops to operate on a given heap
+ * @allocate:		allocate dmabuf and return fd
+ *
+ * allocate returns dmabuf fd  on success, -errno on error.
+ */
+struct dma_heap_ops {
+	int (*allocate)(struct dma_heap *heap,
+			unsigned long len,
+			unsigned long fd_flags,
+			unsigned long heap_flags);
+};
+
+/**
+ * struct dma_heap_export_info - information needed to export a new dmabuf heap
+ * @name:	used for debugging/device-node name
+ * @ops:	ops struct for this heap
+ * @priv:	heap exporter private data
+ *
+ * Information needed to export a new dmabuf heap.
+ */
+struct dma_heap_export_info {
+	const char *name;
+	const struct dma_heap_ops *ops;
+	void *priv;
+};
+
+/**
+ * dma_heap_get_drvdata() - get per-heap driver data
+ * @heap: DMA-Heap to retrieve private data for
+ *
+ * Returns:
+ * The per-heap data for the heap.
+ */
+void *dma_heap_get_drvdata(struct dma_heap *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);
+
+#endif /* _DMA_HEAPS_H */
diff --git a/include/uapi/linux/dma-heap.h b/include/uapi/linux/dma-heap.h
new file mode 100644
index 000000000000..73e7f66c1cae
--- /dev/null
+++ b/include/uapi/linux/dma-heap.h
@@ -0,0 +1,53 @@ 
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * DMABUF Heaps Userspace API
+ *
+ * Copyright (C) 2011 Google, Inc.
+ * Copyright (C) 2019 Linaro Ltd.
+ */
+#ifndef _UAPI_LINUX_DMABUF_POOL_H
+#define _UAPI_LINUX_DMABUF_POOL_H
+
+#include <linux/ioctl.h>
+#include <linux/types.h>
+
+/**
+ * DOC: DMABUF Heaps Userspace API
+ */
+
+/* Valid FD_FLAGS are O_CLOEXEC, O_RDONLY, O_WRONLY, O_RDWR */
+#define DMA_HEAP_VALID_FD_FLAGS (O_CLOEXEC | O_ACCMODE)
+
+/* Currently no heap flags */
+#define DMA_HEAP_VALID_HEAP_FLAGS (0)
+
+/**
+ * struct dma_heap_allocation_data - metadata passed from userspace for
+ *                                      allocations
+ * @len:		size of the allocation
+ * @fd:			will be populated with a fd which provides the
+ *			handle to the allocated dma-buf
+ * @fd_flags:		file descriptor flags used when allocating
+ * @heap_flags:		flags passed to heap
+ *
+ * Provided by userspace as an argument to the ioctl
+ */
+struct dma_heap_allocation_data {
+	__u64 len;
+	__u32 fd;
+	__u32 fd_flags;
+	__u64 heap_flags;
+};
+
+#define DMA_HEAP_IOC_MAGIC		'H'
+
+/**
+ * DOC: DMA_HEAP_IOC_ALLOC - allocate memory from pool
+ *
+ * Takes a dma_heap_allocation_data struct and returns it with the fd field
+ * populated with the dmabuf handle of the allocation.
+ */
+#define DMA_HEAP_IOC_ALLOC	_IOWR(DMA_HEAP_IOC_MAGIC, 0x0,\
+				      struct dma_heap_allocation_data)
+
+#endif /* _UAPI_LINUX_DMABUF_POOL_H */