Hi all, before sending this RFC to a larger audience I would like to get our opinion about it. Please feel very free to comment it, thanks !
The goal of this RFC is to understand if a common ioctl for specific memory regions allocations is needed/welcome.
Obviously it will not replace allocation done in linux kernel frameworks like v4l2 or dmr/kms, but offer an alternative when you don't want/need to use them for buffer allocation.
"Unix Device Memory Allocator" project [1] wants to create a userland library which may allow to select, depending of the devices constraint, the best back-end for allocation. With this RFC I would to propose to have common ioctl for a maximum of allocator to avoid to duplicated back-end for this library.
Since the beginning it is a problem to allocate contiguous memory (CMA) without using v4l2 or drm/kms so the first allocator available in this RFC use CMA memory.
An other question is: do we have others memory regions that could be interested by this new framework ? I have in mind that some title memory regions could use it or replace ION heaps (system, carveout etc...). Maybe it only solve CMA allocation issue, in this case no need to create a common ioctl, a custom should be enough.
Maybe the first thing to do is to change the name and the location of this module, suggestions are welcome.
[1] https://github.com/cubanismo/allocator
Benjamin Gaignard (2): Create Simple Allocator module add CMA simple allocator module
drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/simpleallocator/Kconfig | 17 +++ drivers/simpleallocator/Makefile | 2 + drivers/simpleallocator/simple-allocator-cma.c | 163 +++++++++++++++++++++++ drivers/simpleallocator/simple-allocator-priv.h | 27 ++++ drivers/simpleallocator/simple-allocator.c | 167 ++++++++++++++++++++++++ include/uapi/linux/simple-allocator.h | 35 +++++ 8 files changed, 414 insertions(+) create mode 100644 drivers/simpleallocator/Kconfig create mode 100644 drivers/simpleallocator/Makefile create mode 100644 drivers/simpleallocator/simple-allocator-cma.c create mode 100644 drivers/simpleallocator/simple-allocator-priv.h create mode 100644 drivers/simpleallocator/simple-allocator.c create mode 100644 include/uapi/linux/simple-allocator.h
This is the core of simple allocator module. It aim to offert one common ioctl to allocate specific memory.
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org --- drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/simpleallocator/Kconfig | 10 ++ drivers/simpleallocator/Makefile | 1 + drivers/simpleallocator/simple-allocator-priv.h | 27 ++++ drivers/simpleallocator/simple-allocator.c | 167 ++++++++++++++++++++++++ include/uapi/linux/simple-allocator.h | 35 +++++ 7 files changed, 243 insertions(+) create mode 100644 drivers/simpleallocator/Kconfig create mode 100644 drivers/simpleallocator/Makefile create mode 100644 drivers/simpleallocator/simple-allocator-priv.h create mode 100644 drivers/simpleallocator/simple-allocator.c create mode 100644 include/uapi/linux/simple-allocator.h
diff --git a/drivers/Kconfig b/drivers/Kconfig index e1e2066..a6d8828 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -202,4 +202,6 @@ source "drivers/hwtracing/intel_th/Kconfig"
source "drivers/fpga/Kconfig"
+source "drivers/simpleallocator/Kconfig" + endmenu diff --git a/drivers/Makefile b/drivers/Makefile index 060026a..5081eb8 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -173,3 +173,4 @@ obj-$(CONFIG_STM) += hwtracing/stm/ obj-$(CONFIG_ANDROID) += android/ obj-$(CONFIG_NVMEM) += nvmem/ obj-$(CONFIG_FPGA) += fpga/ +obj-$(CONFIG_SIMPLE_ALLOCATOR) += simpleallocator/ diff --git a/drivers/simpleallocator/Kconfig b/drivers/simpleallocator/Kconfig new file mode 100644 index 0000000..c6fc2e3 --- /dev/null +++ b/drivers/simpleallocator/Kconfig @@ -0,0 +1,10 @@ +menu "Simple Allocator" + +config SIMPLE_ALLOCATOR + tristate "Simple Alllocator Framework" + select DMA_SHARED_BUFFER + ---help--- + The Simple Allocator Framework adds an API to allocate and share + memory in userland. + +endmenu diff --git a/drivers/simpleallocator/Makefile b/drivers/simpleallocator/Makefile new file mode 100644 index 0000000..e27c6ad --- /dev/null +++ b/drivers/simpleallocator/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_SIMPLE_ALLOCATOR) += simple-allocator.o diff --git a/drivers/simpleallocator/simple-allocator-priv.h b/drivers/simpleallocator/simple-allocator-priv.h new file mode 100644 index 0000000..b360bb1 --- /dev/null +++ b/drivers/simpleallocator/simple-allocator-priv.h @@ -0,0 +1,27 @@ +/* + * Copyright (C) Linaro 2016 + * + * Author: Benjamin Gaignard benjamin.gaignard@linaro.org + * + * License terms: GNU General Public License (GPL) + */ + +#ifndef _SIMPLE_ALLOCATOR_PRIV_H_ +#define _SIMPLE_ALLOCATOR_PRIV_H_ + +#include <linux/cdev.h> +#include <linux/device.h> +#include <linux/dma-buf.h> + +struct sa_device { + struct device dev; + struct cdev chrdev; + struct module *owner; + const char *name; + struct dma_buf *(*allocate)(struct sa_device *, u64 length, u32 flags); +}; + +int simple_allocator_register(struct sa_device *sadev); +void simple_allocator_unregister(struct sa_device *sadev); + +#endif diff --git a/drivers/simpleallocator/simple-allocator.c b/drivers/simpleallocator/simple-allocator.c new file mode 100644 index 0000000..a76b593 --- /dev/null +++ b/drivers/simpleallocator/simple-allocator.c @@ -0,0 +1,167 @@ +/* + * Copyright (C) Linaro 2016 + * + * Author: Benjamin Gaignard benjamin.gaignard@linaro.org + * + * License terms: GNU General Public License (GPL) + */ + +#include <linux/module.h> +#include <linux/simple-allocator.h> +#include <linux/uaccess.h> + +#include "simple-allocator-priv.h" + +#define SA_MAJOR 222 +#define SA_NUM_DEVICES 256 +#define SA_NAME "simple_allocator" + +static int sa_minor; + +static struct class sa_class = { + .name = SA_NAME, +}; + +static long sa_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) +{ + struct sa_device *sadev = filp->private_data; + int ret = -ENODEV; + + switch (cmd) { + case SA_IOC_ALLOC: + { + struct simple_allocate_data data; + struct dma_buf *dmabuf; + + if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd))) + return -EFAULT; + + if (data.version != 0) + return -EINVAL; + + dmabuf = sadev->allocate(sadev, data.length, data.flags); + if (!dmabuf) + return -EINVAL; + + data.fd = dma_buf_fd(dmabuf, data.flags); + if (data.fd < 0) { + dma_buf_put(dmabuf); + return -EINVAL; + } + + data.length = dmabuf->size; + + if (copy_to_user((void __user *)arg, &data, _IOC_SIZE(cmd))) { + dma_buf_put(dmabuf); + return -EFAULT; + } + + return 0; + } + } + return ret; +} + +static int sa_open(struct inode *inode, struct file *filp) +{ + struct sa_device *sadev = container_of(inode->i_cdev, + struct sa_device, chrdev); + + if (!sadev) + return -ENODEV; + + get_device(&sadev->dev); + filp->private_data = sadev; + return 0; +} + +static int sa_release(struct inode *inode, struct file *filp) +{ + struct sa_device *sadev = container_of(inode->i_cdev, + struct sa_device, chrdev); + + if (!sadev) + return -ENODEV; + + put_device(&sadev->dev); + return 0; +} + +static const struct file_operations sa_fops = { + .owner = THIS_MODULE, + .open = sa_open, + .release = sa_release, + .unlocked_ioctl = sa_ioctl, +}; + +int simple_allocator_register(struct sa_device *sadev) +{ + int ret; + + cdev_init(&sadev->chrdev, &sa_fops); + sadev->chrdev.owner = sadev->owner; + + ret = cdev_add(&sadev->chrdev, MKDEV(SA_MAJOR, sa_minor), 1); + if (ret < 0) + return ret; + + sadev->dev.class = &sa_class; + sadev->dev.devt = MKDEV(SA_MAJOR, sa_minor); + dev_set_name(&sadev->dev, "%s%d", sadev->name, sa_minor); + ret = device_register(&sadev->dev); + if (ret < 0) + goto cleanup; + + sa_minor++; + return 0; + +cleanup: + cdev_del(&sadev->chrdev); + return ret; +} +EXPORT_SYMBOL_GPL(simple_allocator_register); + +void simple_allocator_unregister(struct sa_device *sadev) +{ + if (!sadev) + return; + + cdev_del(&sadev->chrdev); + device_del(&sadev->dev); + put_device(&sadev->dev); +} +EXPORT_SYMBOL_GPL(simple_allocator_unregister); + +static int __init sa_init(void) +{ + dev_t dev = MKDEV(SA_MAJOR, 0); + int ret; + + ret = register_chrdev_region(dev, SA_NUM_DEVICES, SA_NAME); + if (ret < 0) + return ret; + + ret = class_register(&sa_class); + if (ret < 0) { + unregister_chrdev_region(dev, SA_NUM_DEVICES); + return -EIO; + } + + return 0; +} + +static void __exit sa_exit(void) +{ + dev_t dev = MKDEV(SA_MAJOR, 0); + + class_unregister(&sa_class); + unregister_chrdev_region(dev, SA_NUM_DEVICES); +} + +subsys_initcall(sa_init); +module_exit(sa_exit); + +MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@linaro.org"); +MODULE_DESCRIPTION("Simple allocator"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS_CHARDEV_MAJOR(SA_MAJOR); diff --git a/include/uapi/linux/simple-allocator.h b/include/uapi/linux/simple-allocator.h new file mode 100644 index 0000000..5520a85 --- /dev/null +++ b/include/uapi/linux/simple-allocator.h @@ -0,0 +1,35 @@ +/* + * Copyright (C) Linaro 2016 + * + * Author: Benjamin Gaignard benjamin.gaignard@linaro.org + * + * License terms: GNU General Public License (GPL), version 2 + */ + +#ifndef _SIMPLE_ALLOCATOR_H_ +#define _SIMPLE_ALLOCATOR_H_ + +#include <linux/ioctl.h> +#include <linux/types.h> + +/** + * struct simple_allocate_data - allocation parameters + * @version: structure version (must be set to 0) + * @length: size of the requested buffer + * @flags: mode flags for the file like O_RDWR or O_CLOEXEC + * @fd: returned file descriptor + */ +struct simple_allocate_data { + __u64 version; + __u64 length; + __u32 flags; + __u32 reserved1; + __s32 fd; + __u32 reserved2; +}; + +#define SA_IOC_MAGIC 'S' + +#define SA_IOC_ALLOC _IOWR(SA_IOC_MAGIC, 0, struct simple_allocate_data) + +#endif
On 01/12/2017 06:01 AM, Benjamin Gaignard wrote:
This is the core of simple allocator module. It aim to offert one common ioctl to allocate specific memory.
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org
drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/simpleallocator/Kconfig | 10 ++ drivers/simpleallocator/Makefile | 1 + drivers/simpleallocator/simple-allocator-priv.h | 27 ++++ drivers/simpleallocator/simple-allocator.c | 167 ++++++++++++++++++++++++ include/uapi/linux/simple-allocator.h | 35 +++++ 7 files changed, 243 insertions(+) create mode 100644 drivers/simpleallocator/Kconfig create mode 100644 drivers/simpleallocator/Makefile create mode 100644 drivers/simpleallocator/simple-allocator-priv.h create mode 100644 drivers/simpleallocator/simple-allocator.c create mode 100644 include/uapi/linux/simple-allocator.h
diff --git a/drivers/Kconfig b/drivers/Kconfig index e1e2066..a6d8828 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -202,4 +202,6 @@ source "drivers/hwtracing/intel_th/Kconfig" source "drivers/fpga/Kconfig" +source "drivers/simpleallocator/Kconfig"
endmenu diff --git a/drivers/Makefile b/drivers/Makefile index 060026a..5081eb8 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -173,3 +173,4 @@ obj-$(CONFIG_STM) += hwtracing/stm/ obj-$(CONFIG_ANDROID) += android/ obj-$(CONFIG_NVMEM) += nvmem/ obj-$(CONFIG_FPGA) += fpga/ +obj-$(CONFIG_SIMPLE_ALLOCATOR) += simpleallocator/ diff --git a/drivers/simpleallocator/Kconfig b/drivers/simpleallocator/Kconfig new file mode 100644 index 0000000..c6fc2e3 --- /dev/null +++ b/drivers/simpleallocator/Kconfig @@ -0,0 +1,10 @@ +menu "Simple Allocator"
+config SIMPLE_ALLOCATOR
- tristate "Simple Alllocator Framework"
- select DMA_SHARED_BUFFER
- ---help---
The Simple Allocator Framework adds an API to allocate and share
memory in userland.
+endmenu diff --git a/drivers/simpleallocator/Makefile b/drivers/simpleallocator/Makefile new file mode 100644 index 0000000..e27c6ad --- /dev/null +++ b/drivers/simpleallocator/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_SIMPLE_ALLOCATOR) += simple-allocator.o diff --git a/drivers/simpleallocator/simple-allocator-priv.h b/drivers/simpleallocator/simple-allocator-priv.h new file mode 100644 index 0000000..b360bb1 --- /dev/null +++ b/drivers/simpleallocator/simple-allocator-priv.h @@ -0,0 +1,27 @@ +/*
- Copyright (C) Linaro 2016
- Author: Benjamin Gaignard benjamin.gaignard@linaro.org
- License terms: GNU General Public License (GPL)
- */
+#ifndef _SIMPLE_ALLOCATOR_PRIV_H_ +#define _SIMPLE_ALLOCATOR_PRIV_H_
+#include <linux/cdev.h> +#include <linux/device.h> +#include <linux/dma-buf.h>
+struct sa_device {
- struct device dev;
- struct cdev chrdev;
- struct module *owner;
- const char *name;
- struct dma_buf *(*allocate)(struct sa_device *, u64 length, u32 flags);
+};
+int simple_allocator_register(struct sa_device *sadev); +void simple_allocator_unregister(struct sa_device *sadev);
+#endif diff --git a/drivers/simpleallocator/simple-allocator.c b/drivers/simpleallocator/simple-allocator.c new file mode 100644 index 0000000..a76b593 --- /dev/null +++ b/drivers/simpleallocator/simple-allocator.c @@ -0,0 +1,167 @@ +/*
- Copyright (C) Linaro 2016
- Author: Benjamin Gaignard benjamin.gaignard@linaro.org
- License terms: GNU General Public License (GPL)
- */
+#include <linux/module.h> +#include <linux/simple-allocator.h> +#include <linux/uaccess.h>
+#include "simple-allocator-priv.h"
+#define SA_MAJOR 222 +#define SA_NUM_DEVICES 256 +#define SA_NAME "simple_allocator"
+static int sa_minor;
+static struct class sa_class = {
- .name = SA_NAME,
+};
+static long sa_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) +{
- struct sa_device *sadev = filp->private_data;
- int ret = -ENODEV;
- switch (cmd) {
- case SA_IOC_ALLOC:
- {
struct simple_allocate_data data;
struct dma_buf *dmabuf;
if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))
return -EFAULT;
if (data.version != 0)
return -EINVAL;
dmabuf = sadev->allocate(sadev, data.length, data.flags);
if (!dmabuf)
return -EINVAL;
data.fd = dma_buf_fd(dmabuf, data.flags);
if (data.fd < 0) {
dma_buf_put(dmabuf);
return -EINVAL;
}
data.length = dmabuf->size;
if (copy_to_user((void __user *)arg, &data, _IOC_SIZE(cmd))) {
dma_buf_put(dmabuf);
return -EFAULT;
}
return 0;
- }
- }
- return ret;
+}
I've given this model a thought as a possible Ion replacement.
This makes the idea of an 'ion client' implicit in each process instead of explicitly calling open(/dev/ion). This also drops a few layers of Ion abstraction with just the dma_bufs and no handles/buffers which is nice. Based on what I've seen, I think this would work as a model for what tends to happen in userspace.
+static int sa_open(struct inode *inode, struct file *filp) +{
- struct sa_device *sadev = container_of(inode->i_cdev,
struct sa_device, chrdev);
- if (!sadev)
return -ENODEV;
- get_device(&sadev->dev);
- filp->private_data = sadev;
- return 0;
+}
+static int sa_release(struct inode *inode, struct file *filp) +{
- struct sa_device *sadev = container_of(inode->i_cdev,
struct sa_device, chrdev);
- if (!sadev)
return -ENODEV;
- put_device(&sadev->dev);
- return 0;
+}
+static const struct file_operations sa_fops = {
- .owner = THIS_MODULE,
- .open = sa_open,
- .release = sa_release,
- .unlocked_ioctl = sa_ioctl,
+};
+int simple_allocator_register(struct sa_device *sadev) +{
- int ret;
- cdev_init(&sadev->chrdev, &sa_fops);
- sadev->chrdev.owner = sadev->owner;
- ret = cdev_add(&sadev->chrdev, MKDEV(SA_MAJOR, sa_minor), 1);
- if (ret < 0)
return ret;
- sadev->dev.class = &sa_class;
- sadev->dev.devt = MKDEV(SA_MAJOR, sa_minor);
- dev_set_name(&sadev->dev, "%s%d", sadev->name, sa_minor);
- ret = device_register(&sadev->dev);
- if (ret < 0)
goto cleanup;
- sa_minor++;
- return 0;
+cleanup:
- cdev_del(&sadev->chrdev);
- return ret;
+} +EXPORT_SYMBOL_GPL(simple_allocator_register);
+void simple_allocator_unregister(struct sa_device *sadev) +{
- if (!sadev)
return;
- cdev_del(&sadev->chrdev);
- device_del(&sadev->dev);
- put_device(&sadev->dev);
+} +EXPORT_SYMBOL_GPL(simple_allocator_unregister);
+static int __init sa_init(void) +{
- dev_t dev = MKDEV(SA_MAJOR, 0);
- int ret;
- ret = register_chrdev_region(dev, SA_NUM_DEVICES, SA_NAME);
- if (ret < 0)
return ret;
- ret = class_register(&sa_class);
- if (ret < 0) {
unregister_chrdev_region(dev, SA_NUM_DEVICES);
return -EIO;
- }
- return 0;
+}
+static void __exit sa_exit(void) +{
- dev_t dev = MKDEV(SA_MAJOR, 0);
- class_unregister(&sa_class);
- unregister_chrdev_region(dev, SA_NUM_DEVICES);
+}
+subsys_initcall(sa_init); +module_exit(sa_exit);
+MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@linaro.org"); +MODULE_DESCRIPTION("Simple allocator"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS_CHARDEV_MAJOR(SA_MAJOR); diff --git a/include/uapi/linux/simple-allocator.h b/include/uapi/linux/simple-allocator.h new file mode 100644 index 0000000..5520a85 --- /dev/null +++ b/include/uapi/linux/simple-allocator.h @@ -0,0 +1,35 @@ +/*
- Copyright (C) Linaro 2016
- Author: Benjamin Gaignard benjamin.gaignard@linaro.org
- License terms: GNU General Public License (GPL), version 2
- */
+#ifndef _SIMPLE_ALLOCATOR_H_ +#define _SIMPLE_ALLOCATOR_H_
+#include <linux/ioctl.h> +#include <linux/types.h>
+/**
- struct simple_allocate_data - allocation parameters
- @version: structure version (must be set to 0)
- @length: size of the requested buffer
- @flags: mode flags for the file like O_RDWR or O_CLOEXEC
- @fd: returned file descriptor
- */
+struct simple_allocate_data {
- __u64 version;
Last time I proposed versioning for Ion I was told don't bother since we should assume it's not going to be broken.
- __u64 length;
- __u32 flags;
Do you have a use case in mind for specifying descriptor flags? What about allocation option flags?
- __u32 reserved1;
- __s32 fd;
- __u32 reserved2;
+};
+#define SA_IOC_MAGIC 'S'
+#define SA_IOC_ALLOC _IOWR(SA_IOC_MAGIC, 0, struct simple_allocate_data)
+#endif
Thanks, Laura
2017-01-13 20:25 GMT+01:00 Laura Abbott labbott@redhat.com:
On 01/12/2017 06:01 AM, Benjamin Gaignard wrote:
This is the core of simple allocator module. It aim to offert one common ioctl to allocate specific memory.
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org
drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/simpleallocator/Kconfig | 10 ++ drivers/simpleallocator/Makefile | 1 + drivers/simpleallocator/simple-allocator-priv.h | 27 ++++ drivers/simpleallocator/simple-allocator.c | 167 ++++++++++++++++++++++++ include/uapi/linux/simple-allocator.h | 35 +++++ 7 files changed, 243 insertions(+) create mode 100644 drivers/simpleallocator/Kconfig create mode 100644 drivers/simpleallocator/Makefile create mode 100644 drivers/simpleallocator/simple-allocator-priv.h create mode 100644 drivers/simpleallocator/simple-allocator.c create mode 100644 include/uapi/linux/simple-allocator.h
diff --git a/drivers/Kconfig b/drivers/Kconfig index e1e2066..a6d8828 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -202,4 +202,6 @@ source "drivers/hwtracing/intel_th/Kconfig"
source "drivers/fpga/Kconfig"
+source "drivers/simpleallocator/Kconfig"
endmenu diff --git a/drivers/Makefile b/drivers/Makefile index 060026a..5081eb8 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -173,3 +173,4 @@ obj-$(CONFIG_STM) += hwtracing/stm/ obj-$(CONFIG_ANDROID) += android/ obj-$(CONFIG_NVMEM) += nvmem/ obj-$(CONFIG_FPGA) += fpga/ +obj-$(CONFIG_SIMPLE_ALLOCATOR) += simpleallocator/ diff --git a/drivers/simpleallocator/Kconfig b/drivers/simpleallocator/Kconfig new file mode 100644 index 0000000..c6fc2e3 --- /dev/null +++ b/drivers/simpleallocator/Kconfig @@ -0,0 +1,10 @@ +menu "Simple Allocator"
+config SIMPLE_ALLOCATOR
tristate "Simple Alllocator Framework"
select DMA_SHARED_BUFFER
---help---
The Simple Allocator Framework adds an API to allocate and share
memory in userland.
+endmenu diff --git a/drivers/simpleallocator/Makefile b/drivers/simpleallocator/Makefile new file mode 100644 index 0000000..e27c6ad --- /dev/null +++ b/drivers/simpleallocator/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_SIMPLE_ALLOCATOR) += simple-allocator.o diff --git a/drivers/simpleallocator/simple-allocator-priv.h b/drivers/simpleallocator/simple-allocator-priv.h new file mode 100644 index 0000000..b360bb1 --- /dev/null +++ b/drivers/simpleallocator/simple-allocator-priv.h @@ -0,0 +1,27 @@ +/*
- Copyright (C) Linaro 2016
- Author: Benjamin Gaignard benjamin.gaignard@linaro.org
- License terms: GNU General Public License (GPL)
- */
+#ifndef _SIMPLE_ALLOCATOR_PRIV_H_ +#define _SIMPLE_ALLOCATOR_PRIV_H_
+#include <linux/cdev.h> +#include <linux/device.h> +#include <linux/dma-buf.h>
+struct sa_device {
struct device dev;
struct cdev chrdev;
struct module *owner;
const char *name;
struct dma_buf *(*allocate)(struct sa_device *, u64 length, u32 flags);
+};
+int simple_allocator_register(struct sa_device *sadev); +void simple_allocator_unregister(struct sa_device *sadev);
+#endif diff --git a/drivers/simpleallocator/simple-allocator.c b/drivers/simpleallocator/simple-allocator.c new file mode 100644 index 0000000..a76b593 --- /dev/null +++ b/drivers/simpleallocator/simple-allocator.c @@ -0,0 +1,167 @@ +/*
- Copyright (C) Linaro 2016
- Author: Benjamin Gaignard benjamin.gaignard@linaro.org
- License terms: GNU General Public License (GPL)
- */
+#include <linux/module.h> +#include <linux/simple-allocator.h> +#include <linux/uaccess.h>
+#include "simple-allocator-priv.h"
+#define SA_MAJOR 222 +#define SA_NUM_DEVICES 256 +#define SA_NAME "simple_allocator"
+static int sa_minor;
+static struct class sa_class = {
.name = SA_NAME,
+};
+static long sa_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) +{
struct sa_device *sadev = filp->private_data;
int ret = -ENODEV;
switch (cmd) {
case SA_IOC_ALLOC:
{
struct simple_allocate_data data;
struct dma_buf *dmabuf;
if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))
return -EFAULT;
if (data.version != 0)
return -EINVAL;
dmabuf = sadev->allocate(sadev, data.length, data.flags);
if (!dmabuf)
return -EINVAL;
data.fd = dma_buf_fd(dmabuf, data.flags);
if (data.fd < 0) {
dma_buf_put(dmabuf);
return -EINVAL;
}
data.length = dmabuf->size;
if (copy_to_user((void __user *)arg, &data, _IOC_SIZE(cmd))) {
dma_buf_put(dmabuf);
return -EFAULT;
}
return 0;
}
}
return ret;
+}
I've given this model a thought as a possible Ion replacement.
This makes the idea of an 'ion client' implicit in each process instead of explicitly calling open(/dev/ion). This also drops a few layers of Ion abstraction with just the dma_bufs and no handles/buffers which is nice. Based on what I've seen, I think this would work as a model for what tends to happen in userspace.
Great I will continue in this direction
+static int sa_open(struct inode *inode, struct file *filp) +{
struct sa_device *sadev = container_of(inode->i_cdev,
struct sa_device, chrdev);
if (!sadev)
return -ENODEV;
get_device(&sadev->dev);
filp->private_data = sadev;
return 0;
+}
+static int sa_release(struct inode *inode, struct file *filp) +{
struct sa_device *sadev = container_of(inode->i_cdev,
struct sa_device, chrdev);
if (!sadev)
return -ENODEV;
put_device(&sadev->dev);
return 0;
+}
+static const struct file_operations sa_fops = {
.owner = THIS_MODULE,
.open = sa_open,
.release = sa_release,
.unlocked_ioctl = sa_ioctl,
+};
+int simple_allocator_register(struct sa_device *sadev) +{
int ret;
cdev_init(&sadev->chrdev, &sa_fops);
sadev->chrdev.owner = sadev->owner;
ret = cdev_add(&sadev->chrdev, MKDEV(SA_MAJOR, sa_minor), 1);
if (ret < 0)
return ret;
sadev->dev.class = &sa_class;
sadev->dev.devt = MKDEV(SA_MAJOR, sa_minor);
dev_set_name(&sadev->dev, "%s%d", sadev->name, sa_minor);
ret = device_register(&sadev->dev);
if (ret < 0)
goto cleanup;
sa_minor++;
return 0;
+cleanup:
cdev_del(&sadev->chrdev);
return ret;
+} +EXPORT_SYMBOL_GPL(simple_allocator_register);
+void simple_allocator_unregister(struct sa_device *sadev) +{
if (!sadev)
return;
cdev_del(&sadev->chrdev);
device_del(&sadev->dev);
put_device(&sadev->dev);
+} +EXPORT_SYMBOL_GPL(simple_allocator_unregister);
+static int __init sa_init(void) +{
dev_t dev = MKDEV(SA_MAJOR, 0);
int ret;
ret = register_chrdev_region(dev, SA_NUM_DEVICES, SA_NAME);
if (ret < 0)
return ret;
ret = class_register(&sa_class);
if (ret < 0) {
unregister_chrdev_region(dev, SA_NUM_DEVICES);
return -EIO;
}
return 0;
+}
+static void __exit sa_exit(void) +{
dev_t dev = MKDEV(SA_MAJOR, 0);
class_unregister(&sa_class);
unregister_chrdev_region(dev, SA_NUM_DEVICES);
+}
+subsys_initcall(sa_init); +module_exit(sa_exit);
+MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@linaro.org"); +MODULE_DESCRIPTION("Simple allocator"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS_CHARDEV_MAJOR(SA_MAJOR); diff --git a/include/uapi/linux/simple-allocator.h b/include/uapi/linux/simple-allocator.h new file mode 100644 index 0000000..5520a85 --- /dev/null +++ b/include/uapi/linux/simple-allocator.h @@ -0,0 +1,35 @@ +/*
- Copyright (C) Linaro 2016
- Author: Benjamin Gaignard benjamin.gaignard@linaro.org
- License terms: GNU General Public License (GPL), version 2
- */
+#ifndef _SIMPLE_ALLOCATOR_H_ +#define _SIMPLE_ALLOCATOR_H_
+#include <linux/ioctl.h> +#include <linux/types.h>
+/**
- struct simple_allocate_data - allocation parameters
- @version: structure version (must be set to 0)
- @length: size of the requested buffer
- @flags: mode flags for the file like O_RDWR or O_CLOEXEC
- @fd: returned file descriptor
- */
+struct simple_allocate_data {
__u64 version;
Last time I proposed versioning for Ion I was told don't bother since we should assume it's not going to be broken.
version field was a request of drm maintainers when I had try to push smaf few months ago.
__u64 length;
__u32 flags;
Do you have a use case in mind for specifying descriptor flags? What about allocation option flags?
No I was just thinking to O_RDWR or O_CLOEXEC flags but I keep it open to custom flags
__u32 reserved1;
__s32 fd;
__u32 reserved2;
+};
+#define SA_IOC_MAGIC 'S'
+#define SA_IOC_ALLOC _IOWR(SA_IOC_MAGIC, 0, struct simple_allocate_data)
+#endif
Thanks, Laura
This patch add allocator for CMA default region
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org --- drivers/simpleallocator/Kconfig | 7 ++ drivers/simpleallocator/Makefile | 1 + drivers/simpleallocator/simple-allocator-cma.c | 163 +++++++++++++++++++++++++ 3 files changed, 171 insertions(+) create mode 100644 drivers/simpleallocator/simple-allocator-cma.c
diff --git a/drivers/simpleallocator/Kconfig b/drivers/simpleallocator/Kconfig index c6fc2e3..788fb0b 100644 --- a/drivers/simpleallocator/Kconfig +++ b/drivers/simpleallocator/Kconfig @@ -7,4 +7,11 @@ config SIMPLE_ALLOCATOR The Simple Allocator Framework adds an API to allocate and share memory in userland.
+config SIMPLE_ALLOCATOR_CMA + tristate "Simple Allocator CMA" + select SIMPLE_ALLOCATOR + depends on DMA_CMA + ---help--- + Select this option to enable Simple Allocator on CMA area. + endmenu diff --git a/drivers/simpleallocator/Makefile b/drivers/simpleallocator/Makefile index e27c6ad..4e11611 100644 --- a/drivers/simpleallocator/Makefile +++ b/drivers/simpleallocator/Makefile @@ -1 +1,2 @@ obj-$(CONFIG_SIMPLE_ALLOCATOR) += simple-allocator.o +obj-$(CONFIG_SIMPLE_ALLOCATOR_CMA) += simple-allocator-cma.o diff --git a/drivers/simpleallocator/simple-allocator-cma.c b/drivers/simpleallocator/simple-allocator-cma.c new file mode 100644 index 0000000..c4f5767 --- /dev/null +++ b/drivers/simpleallocator/simple-allocator-cma.c @@ -0,0 +1,163 @@ +/* + * Copyright (C) Linaro 2016 + * + * Author: Benjamin Gaignard benjamin.gaignard@linaro.org + * + * License terms: GNU General Public License (GPL) + */ + +#include <linux/module.h> +#include <linux/slab.h> + +#include "simple-allocator-priv.h" + +static struct sa_device *default_cma; + +struct sa_cma_buffer_info { + void *vaddr; + dma_addr_t handle; + size_t size; + struct device *dev; +}; + +static struct sg_table *sa_cma_map_dma_buf(struct dma_buf_attachment *attach, + enum dma_data_direction direction) +{ + struct dma_buf *dmabuf = attach->dmabuf; + struct sa_cma_buffer_info *info = dmabuf->priv; + struct sg_table *sgt; + int ret; + + sgt = kzalloc(sizeof(*sgt), GFP_KERNEL); + if (!sgt) + return NULL; + + ret = dma_get_sgtable(info->dev, sgt, info->vaddr, info->handle, + info->size); + if (ret < 0) + goto out; + + return sgt; + +out: + kfree(sgt); + return NULL; +} + +static void sa_cma_unmap_dma_buf(struct dma_buf_attachment *attach, + struct sg_table *sgt, + enum dma_data_direction dir) +{ + kfree(sgt); +} + +static int sa_cma_mmap_dma_buf(struct dma_buf *dmabuf, + struct vm_area_struct *vma) +{ + struct sa_cma_buffer_info *info = dmabuf->priv; + int ret; + + ret = dma_mmap_wc(info->dev, vma, info->vaddr, info->handle, + vma->vm_end - vma->vm_start); + return ret; +} + +static void sa_cma_release_dma_buf(struct dma_buf *dmabuf) +{ + struct sa_cma_buffer_info *info = dmabuf->priv; + + dma_free_wc(info->dev, info->size, info->vaddr, info->handle); + + kfree(info); +} + +static void *sa_cma_kmap_dma_buf(struct dma_buf *dmabuf, unsigned long offset) +{ + struct sa_cma_buffer_info *info = dmabuf->priv; + + return info->vaddr + offset * PAGE_SIZE; +} + +static struct dma_buf_ops sa_dma_buf_ops = { + .map_dma_buf = sa_cma_map_dma_buf, + .unmap_dma_buf = sa_cma_unmap_dma_buf, + .mmap = sa_cma_mmap_dma_buf, + .release = sa_cma_release_dma_buf, + .kmap_atomic = sa_cma_kmap_dma_buf, + .kmap = sa_cma_kmap_dma_buf, +}; + +static struct dma_buf *sa_cma_allocate(struct sa_device *sadev, + u64 length, u32 flags) +{ + DEFINE_DMA_BUF_EXPORT_INFO(exp_info); + struct sa_cma_buffer_info *info; + struct dma_buf *dmabuf; + + info = kzalloc(sizeof(*info), GFP_KERNEL); + if (!info) + return NULL; + + info->dev = sadev->dev.parent; + info->size = round_up(length, PAGE_SIZE); + + info->vaddr = dma_alloc_wc(info->dev, info->size, &info->handle, + GFP_KERNEL | __GFP_NOWARN); + if (!info->vaddr) + goto cleanup; + + exp_info.ops = &sa_dma_buf_ops; + exp_info.size = info->size; + exp_info.flags = flags; + exp_info.priv = info; + + dmabuf = dma_buf_export(&exp_info); + if (IS_ERR(dmabuf)) + goto export_failed; + + return dmabuf; + +export_failed: + dma_free_wc(info->dev, info->size, info->vaddr, info->handle); +cleanup: + kfree(info); + return NULL; +} + +struct sa_device *simple_allocator_register_cma(struct device *dev) +{ + struct sa_device *sadev; + int ret; + + sadev = kzalloc(sizeof(*sadev), GFP_KERNEL); + if (!sadev) + return NULL; + + sadev->dev.parent = dev; + sadev->owner = THIS_MODULE; + sadev->name = "cma"; + sadev->allocate = sa_cma_allocate; + + ret = simple_allocator_register(sadev); + if (ret) { + kfree(sadev); + return NULL; + } + + return sadev; +} + +static int __init sa_cma_init(void) +{ + default_cma = simple_allocator_register_cma(NULL); + + return 0; +} + +static void __exit sa_cma_exit(void) +{ + simple_allocator_unregister(default_cma); +} + +module_init(sa_cma_init); +module_exit(sa_cma_exit);
On 01/12/2017 06:01 AM, Benjamin Gaignard wrote:
This patch add allocator for CMA default region
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org
drivers/simpleallocator/Kconfig | 7 ++ drivers/simpleallocator/Makefile | 1 + drivers/simpleallocator/simple-allocator-cma.c | 163 +++++++++++++++++++++++++ 3 files changed, 171 insertions(+) create mode 100644 drivers/simpleallocator/simple-allocator-cma.c
diff --git a/drivers/simpleallocator/Kconfig b/drivers/simpleallocator/Kconfig index c6fc2e3..788fb0b 100644 --- a/drivers/simpleallocator/Kconfig +++ b/drivers/simpleallocator/Kconfig @@ -7,4 +7,11 @@ config SIMPLE_ALLOCATOR The Simple Allocator Framework adds an API to allocate and share memory in userland. +config SIMPLE_ALLOCATOR_CMA
- tristate "Simple Allocator CMA"
- select SIMPLE_ALLOCATOR
- depends on DMA_CMA
- ---help---
Select this option to enable Simple Allocator on CMA area.
endmenu diff --git a/drivers/simpleallocator/Makefile b/drivers/simpleallocator/Makefile index e27c6ad..4e11611 100644 --- a/drivers/simpleallocator/Makefile +++ b/drivers/simpleallocator/Makefile @@ -1 +1,2 @@ obj-$(CONFIG_SIMPLE_ALLOCATOR) += simple-allocator.o +obj-$(CONFIG_SIMPLE_ALLOCATOR_CMA) += simple-allocator-cma.o diff --git a/drivers/simpleallocator/simple-allocator-cma.c b/drivers/simpleallocator/simple-allocator-cma.c new file mode 100644 index 0000000..c4f5767 --- /dev/null +++ b/drivers/simpleallocator/simple-allocator-cma.c @@ -0,0 +1,163 @@ +/*
- Copyright (C) Linaro 2016
- Author: Benjamin Gaignard benjamin.gaignard@linaro.org
- License terms: GNU General Public License (GPL)
- */
+#include <linux/module.h> +#include <linux/slab.h>
+#include "simple-allocator-priv.h"
+static struct sa_device *default_cma;
+struct sa_cma_buffer_info {
- void *vaddr;
- dma_addr_t handle;
- size_t size;
- struct device *dev;
+};
+static struct sg_table *sa_cma_map_dma_buf(struct dma_buf_attachment *attach,
enum dma_data_direction direction)
+{
- struct dma_buf *dmabuf = attach->dmabuf;
- struct sa_cma_buffer_info *info = dmabuf->priv;
- struct sg_table *sgt;
- int ret;
- sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
- if (!sgt)
return NULL;
- ret = dma_get_sgtable(info->dev, sgt, info->vaddr, info->handle,
info->size);
- if (ret < 0)
goto out;
- return sgt;
+out:
- kfree(sgt);
- return NULL;
+}
+static void sa_cma_unmap_dma_buf(struct dma_buf_attachment *attach,
struct sg_table *sgt,
enum dma_data_direction dir)
+{
- kfree(sgt);
+}
+static int sa_cma_mmap_dma_buf(struct dma_buf *dmabuf,
struct vm_area_struct *vma)
+{
- struct sa_cma_buffer_info *info = dmabuf->priv;
- int ret;
- ret = dma_mmap_wc(info->dev, vma, info->vaddr, info->handle,
vma->vm_end - vma->vm_start);
- return ret;
+}
+static void sa_cma_release_dma_buf(struct dma_buf *dmabuf) +{
- struct sa_cma_buffer_info *info = dmabuf->priv;
- dma_free_wc(info->dev, info->size, info->vaddr, info->handle);
- kfree(info);
+}
+static void *sa_cma_kmap_dma_buf(struct dma_buf *dmabuf, unsigned long offset) +{
- struct sa_cma_buffer_info *info = dmabuf->priv;
- return info->vaddr + offset * PAGE_SIZE;
+}
+static struct dma_buf_ops sa_dma_buf_ops = {
- .map_dma_buf = sa_cma_map_dma_buf,
- .unmap_dma_buf = sa_cma_unmap_dma_buf,
- .mmap = sa_cma_mmap_dma_buf,
- .release = sa_cma_release_dma_buf,
- .kmap_atomic = sa_cma_kmap_dma_buf,
- .kmap = sa_cma_kmap_dma_buf,
+};
+static struct dma_buf *sa_cma_allocate(struct sa_device *sadev,
u64 length, u32 flags)
+{
- DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
- struct sa_cma_buffer_info *info;
- struct dma_buf *dmabuf;
- info = kzalloc(sizeof(*info), GFP_KERNEL);
- if (!info)
return NULL;
- info->dev = sadev->dev.parent;
- info->size = round_up(length, PAGE_SIZE);
- info->vaddr = dma_alloc_wc(info->dev, info->size, &info->handle,
GFP_KERNEL | __GFP_NOWARN);
- if (!info->vaddr)
goto cleanup;
- exp_info.ops = &sa_dma_buf_ops;
- exp_info.size = info->size;
- exp_info.flags = flags;
- exp_info.priv = info;
- dmabuf = dma_buf_export(&exp_info);
- if (IS_ERR(dmabuf))
goto export_failed;
- return dmabuf;
+export_failed:
- dma_free_wc(info->dev, info->size, info->vaddr, info->handle);
+cleanup:
- kfree(info);
- return NULL;
+}
+struct sa_device *simple_allocator_register_cma(struct device *dev) +{
- struct sa_device *sadev;
- int ret;
- sadev = kzalloc(sizeof(*sadev), GFP_KERNEL);
- if (!sadev)
return NULL;
- sadev->dev.parent = dev;
- sadev->owner = THIS_MODULE;
- sadev->name = "cma";
- sadev->allocate = sa_cma_allocate;
- ret = simple_allocator_register(sadev);
- if (ret) {
kfree(sadev);
return NULL;
- }
- return sadev;
+}
+static int __init sa_cma_init(void) +{
- default_cma = simple_allocator_register_cma(NULL);
- return 0;
+}
+static void __exit sa_cma_exit(void) +{
- simple_allocator_unregister(default_cma);
+}
+module_init(sa_cma_init); +module_exit(sa_cma_exit);
I really don't like having to force the CMA allocation through the device structure. This ends up resulting in the need to create a dummy device just to allocate CMA memory. I'd rather see this code reworked to not require calling dma_alloc and just call cma_alloc directly. This memory can latter be mapped with dma_map.
Thanks, Laura
2017-01-13 20:30 GMT+01:00 Laura Abbott labbott@redhat.com:
On 01/12/2017 06:01 AM, Benjamin Gaignard wrote:
This patch add allocator for CMA default region
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org
drivers/simpleallocator/Kconfig | 7 ++ drivers/simpleallocator/Makefile | 1 + drivers/simpleallocator/simple-allocator-cma.c | 163 +++++++++++++++++++++++++ 3 files changed, 171 insertions(+) create mode 100644 drivers/simpleallocator/simple-allocator-cma.c
diff --git a/drivers/simpleallocator/Kconfig b/drivers/simpleallocator/Kconfig index c6fc2e3..788fb0b 100644 --- a/drivers/simpleallocator/Kconfig +++ b/drivers/simpleallocator/Kconfig @@ -7,4 +7,11 @@ config SIMPLE_ALLOCATOR The Simple Allocator Framework adds an API to allocate and share memory in userland.
+config SIMPLE_ALLOCATOR_CMA
tristate "Simple Allocator CMA"
select SIMPLE_ALLOCATOR
depends on DMA_CMA
---help---
Select this option to enable Simple Allocator on CMA area.
endmenu diff --git a/drivers/simpleallocator/Makefile b/drivers/simpleallocator/Makefile index e27c6ad..4e11611 100644 --- a/drivers/simpleallocator/Makefile +++ b/drivers/simpleallocator/Makefile @@ -1 +1,2 @@ obj-$(CONFIG_SIMPLE_ALLOCATOR) += simple-allocator.o +obj-$(CONFIG_SIMPLE_ALLOCATOR_CMA) += simple-allocator-cma.o diff --git a/drivers/simpleallocator/simple-allocator-cma.c b/drivers/simpleallocator/simple-allocator-cma.c new file mode 100644 index 0000000..c4f5767 --- /dev/null +++ b/drivers/simpleallocator/simple-allocator-cma.c @@ -0,0 +1,163 @@ +/*
- Copyright (C) Linaro 2016
- Author: Benjamin Gaignard benjamin.gaignard@linaro.org
- License terms: GNU General Public License (GPL)
- */
+#include <linux/module.h> +#include <linux/slab.h>
+#include "simple-allocator-priv.h"
+static struct sa_device *default_cma;
+struct sa_cma_buffer_info {
void *vaddr;
dma_addr_t handle;
size_t size;
struct device *dev;
+};
+static struct sg_table *sa_cma_map_dma_buf(struct dma_buf_attachment *attach,
enum dma_data_direction direction)
+{
struct dma_buf *dmabuf = attach->dmabuf;
struct sa_cma_buffer_info *info = dmabuf->priv;
struct sg_table *sgt;
int ret;
sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
if (!sgt)
return NULL;
ret = dma_get_sgtable(info->dev, sgt, info->vaddr, info->handle,
info->size);
if (ret < 0)
goto out;
return sgt;
+out:
kfree(sgt);
return NULL;
+}
+static void sa_cma_unmap_dma_buf(struct dma_buf_attachment *attach,
struct sg_table *sgt,
enum dma_data_direction dir)
+{
kfree(sgt);
+}
+static int sa_cma_mmap_dma_buf(struct dma_buf *dmabuf,
struct vm_area_struct *vma)
+{
struct sa_cma_buffer_info *info = dmabuf->priv;
int ret;
ret = dma_mmap_wc(info->dev, vma, info->vaddr, info->handle,
vma->vm_end - vma->vm_start);
return ret;
+}
+static void sa_cma_release_dma_buf(struct dma_buf *dmabuf) +{
struct sa_cma_buffer_info *info = dmabuf->priv;
dma_free_wc(info->dev, info->size, info->vaddr, info->handle);
kfree(info);
+}
+static void *sa_cma_kmap_dma_buf(struct dma_buf *dmabuf, unsigned long offset) +{
struct sa_cma_buffer_info *info = dmabuf->priv;
return info->vaddr + offset * PAGE_SIZE;
+}
+static struct dma_buf_ops sa_dma_buf_ops = {
.map_dma_buf = sa_cma_map_dma_buf,
.unmap_dma_buf = sa_cma_unmap_dma_buf,
.mmap = sa_cma_mmap_dma_buf,
.release = sa_cma_release_dma_buf,
.kmap_atomic = sa_cma_kmap_dma_buf,
.kmap = sa_cma_kmap_dma_buf,
+};
+static struct dma_buf *sa_cma_allocate(struct sa_device *sadev,
u64 length, u32 flags)
+{
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
struct sa_cma_buffer_info *info;
struct dma_buf *dmabuf;
info = kzalloc(sizeof(*info), GFP_KERNEL);
if (!info)
return NULL;
info->dev = sadev->dev.parent;
info->size = round_up(length, PAGE_SIZE);
info->vaddr = dma_alloc_wc(info->dev, info->size, &info->handle,
GFP_KERNEL | __GFP_NOWARN);
if (!info->vaddr)
goto cleanup;
exp_info.ops = &sa_dma_buf_ops;
exp_info.size = info->size;
exp_info.flags = flags;
exp_info.priv = info;
dmabuf = dma_buf_export(&exp_info);
if (IS_ERR(dmabuf))
goto export_failed;
return dmabuf;
+export_failed:
dma_free_wc(info->dev, info->size, info->vaddr, info->handle);
+cleanup:
kfree(info);
return NULL;
+}
+struct sa_device *simple_allocator_register_cma(struct device *dev) +{
struct sa_device *sadev;
int ret;
sadev = kzalloc(sizeof(*sadev), GFP_KERNEL);
if (!sadev)
return NULL;
sadev->dev.parent = dev;
sadev->owner = THIS_MODULE;
sadev->name = "cma";
sadev->allocate = sa_cma_allocate;
ret = simple_allocator_register(sadev);
if (ret) {
kfree(sadev);
return NULL;
}
return sadev;
+}
+static int __init sa_cma_init(void) +{
default_cma = simple_allocator_register_cma(NULL);
return 0;
+}
+static void __exit sa_cma_exit(void) +{
simple_allocator_unregister(default_cma);
+}
+module_init(sa_cma_init); +module_exit(sa_cma_exit);
I really don't like having to force the CMA allocation through the device structure. This ends up resulting in the need to create a dummy device just to allocate CMA memory. I'd rather see this code reworked to not require calling dma_alloc and just call cma_alloc directly. This memory can latter be mapped with dma_map.
cma_areas array is static in cma.c and I need it content to call cma_alloc. The problem is the same with cma_area_count.
I will have a look into cma.c to see if I can remove those constraints
Benjamin
Thanks, Laura
On 01/16/2017 01:47 AM, Benjamin Gaignard wrote:
2017-01-13 20:30 GMT+01:00 Laura Abbott labbott@redhat.com:
On 01/12/2017 06:01 AM, Benjamin Gaignard wrote:
This patch add allocator for CMA default region
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org
drivers/simpleallocator/Kconfig | 7 ++ drivers/simpleallocator/Makefile | 1 + drivers/simpleallocator/simple-allocator-cma.c | 163 +++++++++++++++++++++++++ 3 files changed, 171 insertions(+) create mode 100644 drivers/simpleallocator/simple-allocator-cma.c
diff --git a/drivers/simpleallocator/Kconfig b/drivers/simpleallocator/Kconfig index c6fc2e3..788fb0b 100644 --- a/drivers/simpleallocator/Kconfig +++ b/drivers/simpleallocator/Kconfig @@ -7,4 +7,11 @@ config SIMPLE_ALLOCATOR The Simple Allocator Framework adds an API to allocate and share memory in userland.
+config SIMPLE_ALLOCATOR_CMA
tristate "Simple Allocator CMA"
select SIMPLE_ALLOCATOR
depends on DMA_CMA
---help---
Select this option to enable Simple Allocator on CMA area.
endmenu diff --git a/drivers/simpleallocator/Makefile b/drivers/simpleallocator/Makefile index e27c6ad..4e11611 100644 --- a/drivers/simpleallocator/Makefile +++ b/drivers/simpleallocator/Makefile @@ -1 +1,2 @@ obj-$(CONFIG_SIMPLE_ALLOCATOR) += simple-allocator.o +obj-$(CONFIG_SIMPLE_ALLOCATOR_CMA) += simple-allocator-cma.o diff --git a/drivers/simpleallocator/simple-allocator-cma.c b/drivers/simpleallocator/simple-allocator-cma.c new file mode 100644 index 0000000..c4f5767 --- /dev/null +++ b/drivers/simpleallocator/simple-allocator-cma.c @@ -0,0 +1,163 @@ +/*
- Copyright (C) Linaro 2016
- Author: Benjamin Gaignard benjamin.gaignard@linaro.org
- License terms: GNU General Public License (GPL)
- */
+#include <linux/module.h> +#include <linux/slab.h>
+#include "simple-allocator-priv.h"
+static struct sa_device *default_cma;
+struct sa_cma_buffer_info {
void *vaddr;
dma_addr_t handle;
size_t size;
struct device *dev;
+};
+static struct sg_table *sa_cma_map_dma_buf(struct dma_buf_attachment *attach,
enum dma_data_direction direction)
+{
struct dma_buf *dmabuf = attach->dmabuf;
struct sa_cma_buffer_info *info = dmabuf->priv;
struct sg_table *sgt;
int ret;
sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
if (!sgt)
return NULL;
ret = dma_get_sgtable(info->dev, sgt, info->vaddr, info->handle,
info->size);
if (ret < 0)
goto out;
return sgt;
+out:
kfree(sgt);
return NULL;
+}
+static void sa_cma_unmap_dma_buf(struct dma_buf_attachment *attach,
struct sg_table *sgt,
enum dma_data_direction dir)
+{
kfree(sgt);
+}
+static int sa_cma_mmap_dma_buf(struct dma_buf *dmabuf,
struct vm_area_struct *vma)
+{
struct sa_cma_buffer_info *info = dmabuf->priv;
int ret;
ret = dma_mmap_wc(info->dev, vma, info->vaddr, info->handle,
vma->vm_end - vma->vm_start);
return ret;
+}
+static void sa_cma_release_dma_buf(struct dma_buf *dmabuf) +{
struct sa_cma_buffer_info *info = dmabuf->priv;
dma_free_wc(info->dev, info->size, info->vaddr, info->handle);
kfree(info);
+}
+static void *sa_cma_kmap_dma_buf(struct dma_buf *dmabuf, unsigned long offset) +{
struct sa_cma_buffer_info *info = dmabuf->priv;
return info->vaddr + offset * PAGE_SIZE;
+}
+static struct dma_buf_ops sa_dma_buf_ops = {
.map_dma_buf = sa_cma_map_dma_buf,
.unmap_dma_buf = sa_cma_unmap_dma_buf,
.mmap = sa_cma_mmap_dma_buf,
.release = sa_cma_release_dma_buf,
.kmap_atomic = sa_cma_kmap_dma_buf,
.kmap = sa_cma_kmap_dma_buf,
+};
+static struct dma_buf *sa_cma_allocate(struct sa_device *sadev,
u64 length, u32 flags)
+{
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
struct sa_cma_buffer_info *info;
struct dma_buf *dmabuf;
info = kzalloc(sizeof(*info), GFP_KERNEL);
if (!info)
return NULL;
info->dev = sadev->dev.parent;
info->size = round_up(length, PAGE_SIZE);
info->vaddr = dma_alloc_wc(info->dev, info->size, &info->handle,
GFP_KERNEL | __GFP_NOWARN);
if (!info->vaddr)
goto cleanup;
exp_info.ops = &sa_dma_buf_ops;
exp_info.size = info->size;
exp_info.flags = flags;
exp_info.priv = info;
dmabuf = dma_buf_export(&exp_info);
if (IS_ERR(dmabuf))
goto export_failed;
return dmabuf;
+export_failed:
dma_free_wc(info->dev, info->size, info->vaddr, info->handle);
+cleanup:
kfree(info);
return NULL;
+}
+struct sa_device *simple_allocator_register_cma(struct device *dev) +{
struct sa_device *sadev;
int ret;
sadev = kzalloc(sizeof(*sadev), GFP_KERNEL);
if (!sadev)
return NULL;
sadev->dev.parent = dev;
sadev->owner = THIS_MODULE;
sadev->name = "cma";
sadev->allocate = sa_cma_allocate;
ret = simple_allocator_register(sadev);
if (ret) {
kfree(sadev);
return NULL;
}
return sadev;
+}
+static int __init sa_cma_init(void) +{
default_cma = simple_allocator_register_cma(NULL);
return 0;
+}
+static void __exit sa_cma_exit(void) +{
simple_allocator_unregister(default_cma);
+}
+module_init(sa_cma_init); +module_exit(sa_cma_exit);
I really don't like having to force the CMA allocation through the device structure. This ends up resulting in the need to create a dummy device just to allocate CMA memory. I'd rather see this code reworked to not require calling dma_alloc and just call cma_alloc directly. This memory can latter be mapped with dma_map.
cma_areas array is static in cma.c and I need it content to call cma_alloc. The problem is the same with cma_area_count.
I will have a look into cma.c to see if I can remove those constraints
This has happened in the powerpc kvm stuff, see arch/powerpc/kvm/book3s_hv_builtin.c The most difficult part is going to be figuring out how to hook up to do the early reservations.
Benjamin
Thanks, Laura
2017-01-17 19:38 GMT+01:00 Laura Abbott labbott@redhat.com:
On 01/16/2017 01:47 AM, Benjamin Gaignard wrote:
2017-01-13 20:30 GMT+01:00 Laura Abbott labbott@redhat.com:
On 01/12/2017 06:01 AM, Benjamin Gaignard wrote:
This patch add allocator for CMA default region
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org
drivers/simpleallocator/Kconfig | 7 ++ drivers/simpleallocator/Makefile | 1 + drivers/simpleallocator/simple-allocator-cma.c | 163 +++++++++++++++++++++++++ 3 files changed, 171 insertions(+) create mode 100644 drivers/simpleallocator/simple-allocator-cma.c
diff --git a/drivers/simpleallocator/Kconfig b/drivers/simpleallocator/Kconfig index c6fc2e3..788fb0b 100644 --- a/drivers/simpleallocator/Kconfig +++ b/drivers/simpleallocator/Kconfig @@ -7,4 +7,11 @@ config SIMPLE_ALLOCATOR The Simple Allocator Framework adds an API to allocate and share memory in userland.
+config SIMPLE_ALLOCATOR_CMA
tristate "Simple Allocator CMA"
select SIMPLE_ALLOCATOR
depends on DMA_CMA
---help---
Select this option to enable Simple Allocator on CMA area.
endmenu diff --git a/drivers/simpleallocator/Makefile b/drivers/simpleallocator/Makefile index e27c6ad..4e11611 100644 --- a/drivers/simpleallocator/Makefile +++ b/drivers/simpleallocator/Makefile @@ -1 +1,2 @@ obj-$(CONFIG_SIMPLE_ALLOCATOR) += simple-allocator.o +obj-$(CONFIG_SIMPLE_ALLOCATOR_CMA) += simple-allocator-cma.o diff --git a/drivers/simpleallocator/simple-allocator-cma.c b/drivers/simpleallocator/simple-allocator-cma.c new file mode 100644 index 0000000..c4f5767 --- /dev/null +++ b/drivers/simpleallocator/simple-allocator-cma.c @@ -0,0 +1,163 @@ +/*
- Copyright (C) Linaro 2016
- Author: Benjamin Gaignard benjamin.gaignard@linaro.org
- License terms: GNU General Public License (GPL)
- */
+#include <linux/module.h> +#include <linux/slab.h>
+#include "simple-allocator-priv.h"
+static struct sa_device *default_cma;
+struct sa_cma_buffer_info {
void *vaddr;
dma_addr_t handle;
size_t size;
struct device *dev;
+};
+static struct sg_table *sa_cma_map_dma_buf(struct dma_buf_attachment *attach,
enum dma_data_direction direction)
+{
struct dma_buf *dmabuf = attach->dmabuf;
struct sa_cma_buffer_info *info = dmabuf->priv;
struct sg_table *sgt;
int ret;
sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
if (!sgt)
return NULL;
ret = dma_get_sgtable(info->dev, sgt, info->vaddr, info->handle,
info->size);
if (ret < 0)
goto out;
return sgt;
+out:
kfree(sgt);
return NULL;
+}
+static void sa_cma_unmap_dma_buf(struct dma_buf_attachment *attach,
struct sg_table *sgt,
enum dma_data_direction dir)
+{
kfree(sgt);
+}
+static int sa_cma_mmap_dma_buf(struct dma_buf *dmabuf,
struct vm_area_struct *vma)
+{
struct sa_cma_buffer_info *info = dmabuf->priv;
int ret;
ret = dma_mmap_wc(info->dev, vma, info->vaddr, info->handle,
vma->vm_end - vma->vm_start);
return ret;
+}
+static void sa_cma_release_dma_buf(struct dma_buf *dmabuf) +{
struct sa_cma_buffer_info *info = dmabuf->priv;
dma_free_wc(info->dev, info->size, info->vaddr, info->handle);
kfree(info);
+}
+static void *sa_cma_kmap_dma_buf(struct dma_buf *dmabuf, unsigned long offset) +{
struct sa_cma_buffer_info *info = dmabuf->priv;
return info->vaddr + offset * PAGE_SIZE;
+}
+static struct dma_buf_ops sa_dma_buf_ops = {
.map_dma_buf = sa_cma_map_dma_buf,
.unmap_dma_buf = sa_cma_unmap_dma_buf,
.mmap = sa_cma_mmap_dma_buf,
.release = sa_cma_release_dma_buf,
.kmap_atomic = sa_cma_kmap_dma_buf,
.kmap = sa_cma_kmap_dma_buf,
+};
+static struct dma_buf *sa_cma_allocate(struct sa_device *sadev,
u64 length, u32 flags)
+{
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
struct sa_cma_buffer_info *info;
struct dma_buf *dmabuf;
info = kzalloc(sizeof(*info), GFP_KERNEL);
if (!info)
return NULL;
info->dev = sadev->dev.parent;
info->size = round_up(length, PAGE_SIZE);
info->vaddr = dma_alloc_wc(info->dev, info->size, &info->handle,
GFP_KERNEL | __GFP_NOWARN);
if (!info->vaddr)
goto cleanup;
exp_info.ops = &sa_dma_buf_ops;
exp_info.size = info->size;
exp_info.flags = flags;
exp_info.priv = info;
dmabuf = dma_buf_export(&exp_info);
if (IS_ERR(dmabuf))
goto export_failed;
return dmabuf;
+export_failed:
dma_free_wc(info->dev, info->size, info->vaddr, info->handle);
+cleanup:
kfree(info);
return NULL;
+}
+struct sa_device *simple_allocator_register_cma(struct device *dev) +{
struct sa_device *sadev;
int ret;
sadev = kzalloc(sizeof(*sadev), GFP_KERNEL);
if (!sadev)
return NULL;
sadev->dev.parent = dev;
sadev->owner = THIS_MODULE;
sadev->name = "cma";
sadev->allocate = sa_cma_allocate;
ret = simple_allocator_register(sadev);
if (ret) {
kfree(sadev);
return NULL;
}
return sadev;
+}
+static int __init sa_cma_init(void) +{
default_cma = simple_allocator_register_cma(NULL);
return 0;
+}
+static void __exit sa_cma_exit(void) +{
simple_allocator_unregister(default_cma);
+}
+module_init(sa_cma_init); +module_exit(sa_cma_exit);
I really don't like having to force the CMA allocation through the device structure. This ends up resulting in the need to create a dummy device just to allocate CMA memory. I'd rather see this code reworked to not require calling dma_alloc and just call cma_alloc directly. This memory can latter be mapped with dma_map.
cma_areas array is static in cma.c and I need it content to call cma_alloc. The problem is the same with cma_area_count.
I will have a look into cma.c to see if I can remove those constraints
This has happened in the powerpc kvm stuff, see arch/powerpc/kvm/book3s_hv_builtin.c The most difficult part is going to be figuring out how to hook up to do the early reservations.
I do not plan to reserve new cma areas with this driver but only provide an userland API to allocate buffer on already created ones. Since my module will be called long time after early reservation I think I will not have any issue.
You were right, cma_areas and cma_area_count are exported so I can use them in my driver. I have changed my code to use cma_alloc instead of dma_alloc. I will send a RFC with this content.
Benjamin
Thanks, Laura
linaro-kernel@lists.linaro.org