[EARLY RFC][PATCH 2/4] ion: Initial hack to create per heap devices

Laura Abbott labbott at redhat.com
Tue Feb 19 21:17:28 UTC 2019


On 2/15/19 12:24 PM, John Stultz wrote:
> One of the issues w/ the /dev/ion interface is that we have to
> provide the complexity of a heap query interface and we end up
> multiplexing all the heap access through that one interface via
> a bit mask (which currently limits the heaps to 32).
> 
> There has been a long running todo to provide per-heap devices
> which would make the heap discovery/query interface "ls", and
> would allow for different heaps to have different permisisons
> and sepolicy rules.
> 
> TODOs:
> * Android doesn't use udev so "ion_heaps/%s" names don't
>    automatically create a /dev/ subdir. I need to rework
>    from miscdev to creating a proper device class and add
>    a "subsystem" entry for the DeviceHandler to match with
> 
> * Each CMA region is exposed via a separate heap, not sure
>    if this is desired or not, and we may need to improve the
>    naming.
> 

Every CMA region getting exposed was a side effect of doing
the eneumeration without tying it to devicetree or other firmware.
I'm not opposed to limiting the heaps exposed if we can find
a good way to do so that's still compliant with devicetree/whatever.

Thanks,
Laura

> Cc: Laura Abbott <labbott at redhat.com>
> Cc: Sumit Semwal <sumit.semwal at linaro.org>
> Cc: Liam Mark <lmark at codeaurora.org>
> Cc: Brian Starkey <Brian.Starkey at arm.com>
> Cc: Andrew F. Davis <afd at ti.com>
> Cc: Alistair Strachan <astrachan at google.com>
> Cc: dri-devel at lists.freedesktop.org
> Signed-off-by: John Stultz <john.stultz at linaro.org>
> ---
>   drivers/staging/android/ion/ion-ioctl.c | 62 +++++++++++++++++++++++++++++++++
>   drivers/staging/android/ion/ion.c       | 18 ++++++++++
>   drivers/staging/android/ion/ion.h       |  2 ++
>   drivers/staging/android/uapi/ion.h      | 28 +++++++++++++++
>   4 files changed, 110 insertions(+)
> 
> diff --git a/drivers/staging/android/ion/ion-ioctl.c b/drivers/staging/android/ion/ion-ioctl.c
> index 458a9f2..ea8d263 100644
> --- a/drivers/staging/android/ion/ion-ioctl.c
> +++ b/drivers/staging/android/ion/ion-ioctl.c
> @@ -12,6 +12,7 @@
>   
>   union ion_ioctl_arg {
>   	struct ion_allocation_data allocation;
> +	struct ion_heap_allocation_data heap_allocation;
>   	struct ion_heap_query query;
>   	u32 version;
>   };
> @@ -100,3 +101,64 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>   	}
>   	return ret;
>   }
> +
> +long ion_heap_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> +{
> +	int ret = 0;
> +	unsigned int dir;
> +	union ion_ioctl_arg data;
> +
> +	dir = ion_ioctl_dir(cmd);
> +
> +	if (_IOC_SIZE(cmd) > sizeof(data))
> +		return -EINVAL;
> +
> +	/*
> +	 * The copy_from_user is unconditional here for both read and write
> +	 * to do the validate. If there is no write for the ioctl, the
> +	 * buffer is cleared
> +	 */
> +	if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))
> +		return -EFAULT;
> +
> +	ret = validate_ioctl_arg(cmd, &data);
> +	if (ret) {
> +		pr_warn_once("%s: ioctl validate failed\n", __func__);
> +		return ret;
> +	}
> +
> +	if (!(dir & _IOC_WRITE))
> +		memset(&data, 0, sizeof(data));
> +
> +	switch (cmd) {
> +	case ION_IOC_HEAP_ALLOC:
> +	{
> +		struct miscdevice *miscdev = filp->private_data;
> +		struct ion_heap *heap;
> +		int fd;
> +
> +		heap = container_of(miscdev, struct ion_heap, heap_dev);
> +
> +		fd = ion_alloc(data.heap_allocation.len,
> +			       (1 << heap->id),
> +			       data.heap_allocation.flags);
> +		if (fd < 0)
> +			return fd;
> +
> +		data.heap_allocation.fd = fd;
> +
> +		break;
> +	}
> +	case ION_IOC_VERSION:
> +		data.version = ION_VERSION;
> +		break;
> +	default:
> +		return -ENOTTY;
> +	}
> +
> +	if (dir & _IOC_READ) {
> +		if (copy_to_user((void __user *)arg, &data, _IOC_SIZE(cmd)))
> +			return -EFAULT;
> +	}
> +	return ret;
> +}
> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> index 6f5afab..1f7c893 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -492,6 +492,14 @@ int ion_query_heaps(struct ion_heap_query *query)
>   	return ret;
>   }
>   
> +static const struct file_operations ion_heap_fops = {
> +	.owner          = THIS_MODULE,
> +	.unlocked_ioctl = ion_heap_ioctl,
> +#ifdef CONFIG_COMPAT
> +	.compat_ioctl	= ion_heap_ioctl,
> +#endif
> +};
> +
>   static const struct file_operations ion_fops = {
>   	.owner          = THIS_MODULE,
>   	.unlocked_ioctl = ion_ioctl,
> @@ -540,12 +548,22 @@ void ion_device_add_heap(struct ion_heap *heap)
>   	struct ion_device *dev = internal_dev;
>   	int ret;
>   	struct dentry *heap_root;
> +	char *heap_name;
>   	char debug_name[64];
>   
>   	if (!heap->ops->allocate || !heap->ops->free)
>   		pr_err("%s: can not add heap with invalid ops struct.\n",
>   		       __func__);
>   
> +	heap_name = kasprintf(GFP_KERNEL, "ion_heaps/%s", heap->name);
> +	heap->heap_dev.name = heap_name;
> +	heap->heap_dev.minor = MISC_DYNAMIC_MINOR;
> +	heap->heap_dev.fops = &ion_heap_fops;
> +	heap->heap_dev.parent = NULL;
> +	ret = misc_register(&heap->heap_dev);
> +	if (ret)
> +		pr_err("ion: failed to register misc device.\n");
> +
>   	spin_lock_init(&heap->free_lock);
>   	spin_lock_init(&heap->stat_lock);
>   	heap->free_list_size = 0;
> diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h
> index 439e682..7ed4a6a 100644
> --- a/drivers/staging/android/ion/ion.h
> +++ b/drivers/staging/android/ion/ion.h
> @@ -170,6 +170,7 @@ struct ion_heap_ops {
>    */
>   struct ion_heap {
>   	struct plist_node node;
> +	struct miscdevice heap_dev;
>   	struct ion_device *dev;
>   	enum ion_heap_type type;
>   	struct ion_heap_ops *ops;
> @@ -333,6 +334,7 @@ int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask,
>   			 int nr_to_scan);
>   
>   long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
> +long ion_heap_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
>   
>   int ion_query_heaps(struct ion_heap_query *query);
>   
> diff --git a/drivers/staging/android/uapi/ion.h b/drivers/staging/android/uapi/ion.h
> index c480448..20db09f 100644
> --- a/drivers/staging/android/uapi/ion.h
> +++ b/drivers/staging/android/uapi/ion.h
> @@ -57,6 +57,25 @@ enum ion_heap_type {
>    */
>   
>   /**
> + * struct ion_heap_allocation_data - metadata passed from userspace for
> + *                                   allocations
> + * @len:		size of the allocation
> + * @heap_id_mask:	mask of heap ids to allocate from
> + * @flags:		flags passed to heap
> + * @handle:		pointer that will be populated with a cookie to use to
> + *			refer to this allocation
> + *
> + * Provided by userspace as an argument to the ioctl
> + */
> +struct ion_heap_allocation_data {
> +	__u64 len;
> +	__u32 flags;
> +	__u32 fd;
> +	__u32 reserved0;
> +	__u32 reserved1;
> +};
> +
> +/**
>    * struct ion_allocation_data - metadata passed from userspace for allocations
>    * @len:		size of the allocation
>    * @heap_id_mask:	mask of heap ids to allocate from
> @@ -131,4 +150,13 @@ struct ion_heap_query {
>    */
>   #define ION_IOC_VERSION		_IOR(ION_IOC_MAGIC, 9, u32)
>   
> +/**
> + * DOC: ION_IOC_HEAP_ALLOC - allocate memory from heap
> + *
> + * Takes an ion_heap_allocation_data struct and returns it with the handle field
> + * populated with the opaque handle for the allocation.
> + */
> +#define ION_IOC_HEAP_ALLOC	_IOWR(ION_IOC_MAGIC, 10, \
> +				      struct ion_heap_allocation_data)
> +
>   #endif /* _UAPI_LINUX_ION_H */
> 



More information about the dri-devel mailing list