[PATCH v4 0/2] RFC: Secure Memory Allocation Framework

[PATCH v4 0/2] RFC: Secure Memory Allocation Framework

[Benjamin Gaignard]

version 4 changes:
 - rebased on kernel 4.3-rc3
 - fix missing EXPORT_SYMBOL for smaf_create_handle()

version 3 changes:
 - Remove ioctl for allocator selection instead provide the name of
   the targeted allocator with allocation request.
   Selecting allocator from userland isn't the prefered way of working
   but is needed when the first user of the buffer is a software component.
 - Fix issues in case of error while creating smaf handle.
 - Fix module license.
 - Update libsmaf and tests to care of the SMAF API evolution
   https://git.linaro.org/people/benjamin.gaignard/libsmaf.git

version 2 changes:
 - Add one ioctl to allow allocator selection from userspace.
   This is required for the uses case where the first user of
   the buffer is a software IP which can't perform dma_buf attachement.
 - Add name and ranking to allocator structure to be able to sort them.
 - Create a tiny library to test SMAF:
   https://git.linaro.org/people/benjamin.gaignard/libsmaf.git
 - Fix one issue when try to secure buffer without secure module registered

The outcome of the previous RFC about how do secure data path was the need
of a secure memory allocator (https://lkml.org/lkml/2015/5/5/551)

SMAF goal is to provide a framework that allow allocating and securing
memory by using dma_buf. Each platform have it own way to perform those two
features so SMAF design allow to register helper modules to perform them.

To be sure to select the best allocation method for devices SMAF implement
deferred allocation mechanism: memory allocation is only done when the first
device effectively required it.
Allocator modules have to implement a match() to let SMAF know if they are
compatibles with devices needs.
This patch set provide an example of allocator module which use
dma_{alloc/free/mmap}_attrs() and check if at least one device have
coherent_dma_mask set to DMA_BIT_MASK(32) in match function. 
I have named smaf-cma.c like it is done for drm_gem_cma_helper.c even if 
a better name could be found for this file.

Secure modules are responsibles of granting and revoking devices access rights
on the memory. Secure module is also called to check if CPU map memory into
kernel and user address spaces.
An example of secure module implementation can be found here:
http://git.linaro.org/people/benjamin.gaignard/optee-sdp.git
This code isn't yet part of the patch set because it depends on generic TEE
which is still under discussion (https://lwn.net/Articles/644646/)

For allocation part of SMAF code I get inspirated by Sumit Semwal work about
constraint aware allocator.

Benjamin Gaignard (2):
  create SMAF module
  SMAF: add CMA allocator

 drivers/Kconfig                |   2 +
 drivers/Makefile               |   1 +
 drivers/smaf/Kconfig           |  11 +
 drivers/smaf/Makefile          |   2 +
 drivers/smaf/smaf-cma.c        | 200 +++++++++++
 drivers/smaf/smaf-core.c       | 736 +++++++++++++++++++++++++++++++++++++++++
 include/linux/smaf-allocator.h |  54 +++
 include/linux/smaf-secure.h    |  72 ++++
 include/uapi/linux/smaf.h      |  52 +++
 9 files changed, 1130 insertions(+)
 create mode 100644 drivers/smaf/Kconfig
 create mode 100644 drivers/smaf/Makefile
 create mode 100644 drivers/smaf/smaf-cma.c
 create mode 100644 drivers/smaf/smaf-core.c
 create mode 100644 include/linux/smaf-allocator.h
 create mode 100644 include/linux/smaf-secure.h
 create mode 100644 include/uapi/linux/smaf.h

-- 
1.9.1

[PATCH v4 1/2] create SMAF module

[Benjamin Gaignard]

Secure Memory Allocation Framework goal is to be able
to allocate memory that can be securing.
There is so much ways to allocate and securing memory that SMAF
doesn't do it by itself but need help of additional modules.
To be sure to use the correct allocation method SMAF implement
deferred allocation (i.e. allocate memory when only really needed)

Allocation modules (smaf-alloctor.h):
SMAF could manage with multiple allocation modules at same time.
To select the good one SMAF call match() to be sure that a module
can allocate memory for a given list of devices. It is to the module
to check if the devices are compatible or not with it allocation
method.

Securing module (smaf-secure.h):
The way of how securing memory it is done is platform specific.
Secure module is responsible of grant/revoke memory access.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 drivers/Kconfig                |   2 +
 drivers/Makefile               |   1 +
 drivers/smaf/Kconfig           |   5 +
 drivers/smaf/Makefile          |   1 +
 drivers/smaf/smaf-core.c       | 736 +++++++++++++++++++++++++++++++++++++++++
 include/linux/smaf-allocator.h |  54 +++
 include/linux/smaf-secure.h    |  72 ++++
 include/uapi/linux/smaf.h      |  52 +++
 8 files changed, 923 insertions(+)
 create mode 100644 drivers/smaf/Kconfig
 create mode 100644 drivers/smaf/Makefile
 create mode 100644 drivers/smaf/smaf-core.c
 create mode 100644 include/linux/smaf-allocator.h
 create mode 100644 include/linux/smaf-secure.h
 create mode 100644 include/uapi/linux/smaf.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 46b4a8e..a488c20 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -188,4 +188,6 @@ source "drivers/nvdimm/Kconfig"
 
 source "drivers/nvmem/Kconfig"
 
+source "drivers/smaf/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index b250b36..693390b 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -167,3 +167,4 @@ obj-$(CONFIG_THUNDERBOLT)	+= thunderbolt/
 obj-$(CONFIG_CORESIGHT)		+= hwtracing/coresight/
 obj-$(CONFIG_ANDROID)		+= android/
 obj-$(CONFIG_NVMEM)		+= nvmem/
+obj-$(CONFIG_SMAF) 		+= smaf/
diff --git a/drivers/smaf/Kconfig b/drivers/smaf/Kconfig
new file mode 100644
index 0000000..d36651a
--- /dev/null
+++ b/drivers/smaf/Kconfig
@@ -0,0 +1,5 @@
+config SMAF
+	tristate "Secure Memory Allocation Framework"
+	depends on DMA_SHARED_BUFFER
+	help
+	  Choose this option to enable Secure Memory Allocation Framework
diff --git a/drivers/smaf/Makefile b/drivers/smaf/Makefile
new file mode 100644
index 0000000..40cd882
--- /dev/null
+++ b/drivers/smaf/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SMAF) += smaf-core.o
diff --git a/drivers/smaf/smaf-core.c b/drivers/smaf/smaf-core.c
new file mode 100644
index 0000000..37914e7
--- /dev/null
+++ b/drivers/smaf/smaf-core.c
@@ -0,0 +1,736 @@
+/*
+ * smaf.c
+ *
+ * Copyright (C) Linaro SA 2015
+ * Author: Benjamin Gaignard <benjamin.gaignard@linaro.org> for Linaro.
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#include <linux/device.h>
+#include <linux/dma-buf.h>
+#include <linux/dma-mapping.h>
+#include <linux/fs.h>
+#include <linux/ioctl.h>
+#include <linux/list_sort.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/smaf.h>
+#include <linux/smaf-allocator.h>
+#include <linux/smaf-secure.h>
+#include <linux/uaccess.h>
+
+struct smaf_handle {
+	struct dma_buf *dmabuf;
+	struct smaf_allocator *allocator;
+	struct dma_buf *db_alloc;
+	size_t length;
+	unsigned int flags;
+	int fd;
+	bool is_secure;
+	void *secure_ctx;
+};
+
+/**
+ * struct smaf_device - smaf device node private data
+ * @misc_dev:	the misc device
+ * @head:	list of allocator
+ * @lock:	list and secure pointer mutex
+ * @secure:	pointer to secure functions helpers
+ */
+struct smaf_device {
+	struct miscdevice misc_dev;
+	struct list_head head;
+	/* list and secure pointer lock*/
+	struct mutex lock;
+	struct smaf_secure *secure;
+};
+
+static struct smaf_device smaf_dev;
+
+/**
+ * smaf_allow_cpu_access return true if CPU can access to memory
+ * if their is no secure module associated to SMAF assume that CPU can get
+ * access to the memory.
+ */
+static bool smaf_allow_cpu_access(struct smaf_handle *handle,
+				  unsigned long flags)
+{
+	if (!handle->is_secure)
+		return true;
+
+	if (!smaf_dev.secure)
+		return true;
+
+	if (!smaf_dev.secure->allow_cpu_access)
+		return true;
+
+	return smaf_dev.secure->allow_cpu_access(handle->secure_ctx, flags);
+}
+
+static int smaf_grant_access(struct smaf_handle *handle, struct device *dev,
+			     dma_addr_t addr, size_t size,
+			     enum dma_data_direction dir)
+{
+	if (!handle->is_secure)
+		return 0;
+
+	if (!smaf_dev.secure)
+		return -EINVAL;
+
+	if (!smaf_dev.secure->grant_access)
+		return -EINVAL;
+
+	return smaf_dev.secure->grant_access(handle->secure_ctx,
+					     dev, addr, size, dir);
+}
+
+static void smaf_revoke_access(struct smaf_handle *handle, struct device *dev,
+			       dma_addr_t addr, size_t size,
+			       enum dma_data_direction dir)
+{
+	if (!handle->is_secure)
+		return;
+
+	if (!smaf_dev.secure)
+		return;
+
+	if (!smaf_dev.secure->revoke_access)
+		return;
+
+	smaf_dev.secure->revoke_access(handle->secure_ctx,
+				       dev, addr, size, dir);
+}
+
+static int smaf_secure_handle(struct smaf_handle *handle)
+{
+	if (handle->is_secure)
+		return 0;
+
+	if (!smaf_dev.secure)
+		return -EINVAL;
+
+	if (!smaf_dev.secure->create_context)
+		return -EINVAL;
+
+	handle->secure_ctx = smaf_dev.secure->create_context();
+
+	if (!handle->secure_ctx)
+		return -EINVAL;
+
+	handle->is_secure = true;
+	return 0;
+}
+
+static int smaf_unsecure_handle(struct smaf_handle *handle)
+{
+	if (!handle->is_secure)
+		return 0;
+
+	if (!smaf_dev.secure)
+		return -EINVAL;
+
+	if (!smaf_dev.secure->destroy_context)
+		return -EINVAL;
+
+	if (smaf_dev.secure->destroy_context(handle->secure_ctx))
+		return -EINVAL;
+
+	handle->secure_ctx = NULL;
+	handle->is_secure = false;
+	return 0;
+}
+
+int smaf_register_secure(struct smaf_secure *s)
+{
+	if (smaf_dev.secure || !s)
+		return -EINVAL;
+
+	mutex_lock(&smaf_dev.lock);
+	smaf_dev.secure = s;
+	mutex_unlock(&smaf_dev.lock);
+
+	return 0;
+}
+EXPORT_SYMBOL(smaf_register_secure);
+
+void smaf_unregister_secure(struct smaf_secure *s)
+{
+	mutex_lock(&smaf_dev.lock);
+	if (smaf_dev.secure == s)
+		smaf_dev.secure = NULL;
+	mutex_unlock(&smaf_dev.lock);
+}
+EXPORT_SYMBOL(smaf_unregister_secure);
+
+static struct smaf_allocator *smaf_find_allocator(struct dma_buf *dmabuf)
+{
+	struct smaf_allocator *alloc;
+
+	list_for_each_entry(alloc, &smaf_dev.head, list_node) {
+		if (alloc->match(dmabuf))
+			return alloc;
+	}
+
+	return NULL;
+}
+
+static struct smaf_allocator *smaf_get_first_allocator(struct dma_buf *dmabuf)
+{
+	/* the first allocator of the list is the preferred allocator */
+	return list_first_entry(&smaf_dev.head, struct smaf_allocator,
+			list_node);
+}
+
+static int smaf_allocator_compare(void *priv,
+				  struct list_head *lh_a,
+				  struct list_head *lh_b)
+{
+	struct smaf_allocator *a = list_entry(lh_a,
+					      struct smaf_allocator, list_node);
+	struct smaf_allocator *b = list_entry(lh_b,
+					      struct smaf_allocator, list_node);
+	int diff;
+
+	diff = b->ranking - a->ranking;
+	if (diff)
+		return diff;
+
+	return strcmp(a->name, b->name);
+}
+
+int smaf_register_allocator(struct smaf_allocator *alloc)
+{
+	BUG_ON(!alloc || !alloc->match || !alloc->allocate || !alloc->name);
+
+	mutex_lock(&smaf_dev.lock);
+	list_add(&alloc->list_node, &smaf_dev.head);
+	list_sort(NULL, &smaf_dev.head, smaf_allocator_compare);
+	mutex_unlock(&smaf_dev.lock);
+
+	return 0;
+}
+EXPORT_SYMBOL(smaf_register_allocator);
+
+void smaf_unregister_allocator(struct smaf_allocator *alloc)
+{
+	mutex_lock(&smaf_dev.lock);
+	list_del(&alloc->list_node);
+	mutex_unlock(&smaf_dev.lock);
+}
+EXPORT_SYMBOL(smaf_unregister_allocator);
+
+static struct dma_buf_attachment *smaf_find_attachment(struct dma_buf *db_alloc,
+						       struct device *dev)
+{
+	struct dma_buf_attachment *attach_obj;
+
+	list_for_each_entry(attach_obj, &db_alloc->attachments, node) {
+		if (attach_obj->dev == dev)
+			return attach_obj;
+	}
+
+	return NULL;
+}
+
+static struct sg_table *smaf_map_dma_buf(struct dma_buf_attachment *attachment,
+					 enum dma_data_direction direction)
+{
+	struct dma_buf_attachment *db_attachment;
+	struct dma_buf *dmabuf = attachment->dmabuf;
+	struct smaf_handle *handle = dmabuf->priv;
+	struct sg_table *sgt;
+	unsigned count_done, count;
+	struct scatterlist *sg;
+
+	if (handle->is_secure && !smaf_dev.secure)
+		return NULL;
+
+	/* try to find an allocator */
+	if (!handle->allocator) {
+		struct smaf_allocator *alloc;
+
+		mutex_lock(&smaf_dev.lock);
+		alloc = smaf_find_allocator(dmabuf);
+		mutex_unlock(&smaf_dev.lock);
+
+		/* still no allocator ? */
+		if (!alloc)
+			return NULL;
+
+		handle->allocator = alloc;
+	}
+
+	if (!handle->db_alloc) {
+		struct dma_buf *db_alloc;
+
+		db_alloc = handle->allocator->allocate(dmabuf,
+						       handle->length,
+						       handle->flags);
+		if (!db_alloc)
+			return NULL;
+
+		handle->db_alloc = db_alloc;
+	}
+
+	db_attachment = smaf_find_attachment(handle->db_alloc, attachment->dev);
+	sgt = dma_buf_map_attachment(db_attachment, direction);
+
+	if (!sgt)
+		return NULL;
+
+	if (!handle->is_secure)
+		return sgt;
+
+	/* now secure the data */
+	for_each_sg(sgt->sgl, sg, sgt->nents, count_done) {
+		if (smaf_grant_access(handle, db_attachment->dev,
+				      sg_phys(sg), sg->length, direction))
+			goto failed;
+	}
+
+	return sgt;
+
+failed:
+	for_each_sg(sgt->sgl, sg, count_done, count) {
+		smaf_revoke_access(handle, db_attachment->dev,
+				   sg_phys(sg), sg->length, direction);
+	}
+
+	sg_free_table(sgt);
+	kfree(sgt);
+	return NULL;
+}
+
+static void smaf_unmap_dma_buf(struct dma_buf_attachment *attachment,
+			       struct sg_table *sgt,
+			       enum dma_data_direction direction)
+{
+	struct dma_buf_attachment *db_attachment;
+	struct dma_buf *dmabuf = attachment->dmabuf;
+	struct smaf_handle *handle = dmabuf->priv;
+	struct scatterlist *sg;
+	unsigned count;
+
+	if (!handle->db_alloc)
+		return;
+
+	db_attachment = smaf_find_attachment(handle->db_alloc, attachment->dev);
+	if (!db_attachment)
+		return;
+
+	if (handle->is_secure) {
+		for_each_sg(sgt->sgl, sg, sgt->nents, count) {
+			smaf_revoke_access(handle, db_attachment->dev,
+					   sg_phys(sg), sg->length, direction);
+		}
+	}
+
+	dma_buf_unmap_attachment(db_attachment, sgt, direction);
+}
+
+static int smaf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
+{
+	struct smaf_handle *handle = dmabuf->priv;
+
+	if (!smaf_allow_cpu_access(handle, vma->vm_flags))
+		return -EINVAL;
+
+	/* if no allocator attached, get the first allocator */
+	if (!handle->allocator) {
+		struct smaf_allocator *alloc;
+
+		mutex_lock(&smaf_dev.lock);
+		alloc = smaf_get_first_allocator(dmabuf);
+		mutex_unlock(&smaf_dev.lock);
+
+		/* still no allocator ? */
+		if (!alloc)
+			return -EINVAL;
+
+		handle->allocator = alloc;
+	}
+
+	if (!handle->db_alloc) {
+		struct dma_buf *db_alloc;
+
+		db_alloc = handle->allocator->allocate(dmabuf,
+						       handle->length,
+						       handle->flags);
+		if (!db_alloc)
+			return -EINVAL;
+
+		handle->db_alloc = db_alloc;
+	}
+
+	return dma_buf_mmap(handle->db_alloc, vma, 0);
+}
+
+static void smaf_dma_buf_release(struct dma_buf *dmabuf)
+{
+	struct smaf_handle *handle = dmabuf->priv;
+
+	if (handle->db_alloc)
+		dma_buf_put(handle->db_alloc);
+
+	smaf_unsecure_handle(handle);
+
+	kfree(handle);
+}
+
+static int smaf_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, size_t start,
+					 size_t len,
+					 enum dma_data_direction direction)
+{
+	struct smaf_handle *handle = dmabuf->priv;
+
+	if (!smaf_allow_cpu_access(handle, direction))
+		return -EINVAL;
+
+	if (!handle->db_alloc)
+		return -EINVAL;
+
+	return dma_buf_begin_cpu_access(handle->db_alloc,
+					start, len, direction);
+}
+
+static void smaf_dma_buf_end_cpu_access(struct dma_buf *dmabuf, size_t start,
+					size_t len,
+					enum dma_data_direction direction)
+{
+	struct smaf_handle *handle = dmabuf->priv;
+
+	if (handle->db_alloc)
+		dma_buf_end_cpu_access(handle->db_alloc, start, len, direction);
+}
+
+static void *smaf_dma_buf_kmap_atomic(struct dma_buf *dmabuf,
+				      unsigned long offset)
+{
+	struct smaf_handle *handle = dmabuf->priv;
+
+	if (!handle->db_alloc)
+		return NULL;
+
+	if (!smaf_allow_cpu_access(handle, DMA_BIDIRECTIONAL))
+		return NULL;
+
+	return dma_buf_kmap_atomic(handle->db_alloc, offset);
+}
+
+static void smaf_dma_buf_kunmap_atomic(struct dma_buf *dmabuf,
+				       unsigned long offset, void *ptr)
+{
+	struct smaf_handle *handle = dmabuf->priv;
+
+	if (!handle->db_alloc)
+		return;
+
+	dma_buf_kunmap_atomic(handle->db_alloc, offset, ptr);
+}
+
+static void *smaf_dma_buf_kmap(struct dma_buf *dmabuf, unsigned long offset)
+{
+	struct smaf_handle *handle = dmabuf->priv;
+
+	if (!handle->db_alloc)
+		return NULL;
+
+	if (!smaf_allow_cpu_access(handle, DMA_BIDIRECTIONAL))
+		return NULL;
+
+	return dma_buf_kmap(handle->db_alloc, offset);
+}
+
+static void smaf_dma_buf_kunmap(struct dma_buf *dmabuf, unsigned long offset,
+				void *ptr)
+{
+	struct smaf_handle *handle = dmabuf->priv;
+
+	if (!handle->db_alloc)
+		return;
+
+	dma_buf_kunmap(handle->db_alloc, offset, ptr);
+}
+
+static void *smaf_dma_buf_vmap(struct dma_buf *dmabuf)
+{
+	struct smaf_handle *handle = dmabuf->priv;
+
+	if (!handle->db_alloc)
+		return NULL;
+
+	if (!smaf_allow_cpu_access(handle, DMA_BIDIRECTIONAL))
+		return NULL;
+
+	return dma_buf_vmap(handle->db_alloc);
+}
+
+static void smaf_dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr)
+{
+	struct smaf_handle *handle = dmabuf->priv;
+
+	if (!handle->db_alloc)
+		return;
+
+	dma_buf_vunmap(handle->db_alloc, vaddr);
+}
+
+static int smaf_attach(struct dma_buf *dmabuf, struct device *dev,
+		       struct dma_buf_attachment *attach)
+{
+	struct smaf_handle *handle = dmabuf->priv;
+	struct dma_buf_attachment *db_attach;
+
+	if (!handle->db_alloc)
+		return 0;
+
+	db_attach = dma_buf_attach(handle->db_alloc, dev);
+
+	return IS_ERR(db_attach);
+}
+
+static void smaf_detach(struct dma_buf *dmabuf,
+			struct dma_buf_attachment *attach)
+{
+	struct smaf_handle *handle = dmabuf->priv;
+	struct dma_buf_attachment *db_attachment;
+
+	if (!handle->db_alloc)
+		return;
+
+	db_attachment = smaf_find_attachment(handle->db_alloc, attach->dev);
+	dma_buf_detach(handle->db_alloc, db_attachment);
+}
+
+static struct dma_buf_ops smaf_dma_buf_ops = {
+	.attach = smaf_attach,
+	.detach = smaf_detach,
+	.map_dma_buf = smaf_map_dma_buf,
+	.unmap_dma_buf = smaf_unmap_dma_buf,
+	.mmap = smaf_mmap,
+	.release = smaf_dma_buf_release,
+	.begin_cpu_access = smaf_dma_buf_begin_cpu_access,
+	.end_cpu_access = smaf_dma_buf_end_cpu_access,
+	.kmap_atomic = smaf_dma_buf_kmap_atomic,
+	.kunmap_atomic = smaf_dma_buf_kunmap_atomic,
+	.kmap = smaf_dma_buf_kmap,
+	.kunmap = smaf_dma_buf_kunmap,
+	.vmap = smaf_dma_buf_vmap,
+	.vunmap = smaf_dma_buf_vunmap,
+};
+
+static bool is_smaf_dmabuf(struct dma_buf *dmabuf)
+{
+	return dmabuf->ops == &smaf_dma_buf_ops;
+}
+
+bool smaf_is_secure(struct dma_buf *dmabuf)
+{
+	struct smaf_handle *handle = dmabuf->priv;
+
+	if (!is_smaf_dmabuf(dmabuf))
+		return false;
+
+	return handle->is_secure;
+}
+EXPORT_SYMBOL(smaf_is_secure);
+
+int smaf_set_secure(struct dma_buf *dmabuf, bool secure)
+{
+	struct smaf_handle *handle = dmabuf->priv;
+	int ret;
+
+	if (!is_smaf_dmabuf(dmabuf))
+		return -EINVAL;
+
+	mutex_lock(&smaf_dev.lock);
+	if (secure)
+		ret = smaf_secure_handle(handle);
+	else
+		ret = smaf_unsecure_handle(handle);
+	mutex_unlock(&smaf_dev.lock);
+
+	return ret;
+}
+EXPORT_SYMBOL(smaf_set_secure);
+
+int smaf_select_allocator_by_name(struct dma_buf *dmabuf, char *name)
+{
+	struct smaf_handle *handle = dmabuf->priv;
+	struct smaf_allocator *alloc;
+
+	if (!is_smaf_dmabuf(dmabuf))
+		return -EINVAL;
+
+	if (handle->allocator)
+		return -EINVAL;
+
+	mutex_lock(&smaf_dev.lock);
+
+	list_for_each_entry(alloc, &smaf_dev.head, list_node) {
+		if (!strncmp(alloc->name, name, ALLOCATOR_NAME_LENGTH)) {
+			handle->allocator = alloc;
+			handle->db_alloc = NULL;
+		}
+	}
+
+	mutex_unlock(&smaf_dev.lock);
+
+	if (!handle->allocator)
+		return -EINVAL;
+
+	return 0;
+}
+EXPORT_SYMBOL(smaf_select_allocator_by_name);
+
+struct smaf_handle *smaf_create_handle(size_t length, unsigned int flags)
+{
+	struct smaf_handle *handle;
+
+	DEFINE_DMA_BUF_EXPORT_INFO(info);
+
+	handle = kzalloc(sizeof(*handle), GFP_KERNEL);
+	if (!handle)
+		return ERR_PTR(-ENOMEM);
+
+	info.ops = &smaf_dma_buf_ops;
+	info.size = length;
+	info.flags = flags;
+	info.priv = handle;
+
+	handle->dmabuf = dma_buf_export(&info);
+	if (IS_ERR(handle->dmabuf)) {
+		kfree(handle);
+		return NULL;
+	}
+
+	handle->length = length;
+	handle->flags = flags;
+
+	return handle;
+}
+EXPORT_SYMBOL(smaf_create_handle);
+
+static long smaf_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	switch (cmd) {
+	case SMAF_IOC_CREATE:
+	{
+		struct smaf_create_data data;
+		struct smaf_handle *handle;
+
+		if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))
+			return -EFAULT;
+
+		handle = smaf_create_handle(data.length, data.flags);
+		if (!handle)
+			return -EINVAL;
+
+		if (data.name[0]) {
+			/* user force allocator selection */
+			if (smaf_select_allocator_by_name(handle->dmabuf,
+							  data.name)) {
+				dma_buf_put(handle->dmabuf);
+				return -EINVAL;
+			}
+		}
+
+		handle->fd = dma_buf_fd(handle->dmabuf, data.flags);
+		if (handle->fd < 0) {
+			dma_buf_put(handle->dmabuf);
+			return -EINVAL;
+		}
+
+		data.fd = handle->fd;
+		if (copy_to_user((void __user *)arg, &data, _IOC_SIZE(cmd))) {
+			dma_buf_put(handle->dmabuf);
+			return -EFAULT;
+		}
+		break;
+	}
+	case SMAF_IOC_GET_SECURE_FLAG:
+	{
+		struct smaf_secure_flag data;
+		struct dma_buf *dmabuf;
+
+		if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))
+			return -EFAULT;
+
+		dmabuf = dma_buf_get(data.fd);
+		if (!dmabuf)
+			return -EINVAL;
+
+		data.secure = smaf_is_secure(dmabuf);
+		dma_buf_put(dmabuf);
+
+		if (copy_to_user((void __user *)arg, &data, _IOC_SIZE(cmd)))
+			return -EFAULT;
+		break;
+	}
+	case SMAF_IOC_SET_SECURE_FLAG:
+	{
+		struct smaf_secure_flag data;
+		struct dma_buf *dmabuf;
+		int ret;
+
+		if (!smaf_dev.secure)
+			return -EINVAL;
+
+		if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))
+			return -EFAULT;
+
+		dmabuf = dma_buf_get(data.fd);
+		if (!dmabuf)
+			return -EINVAL;
+
+		ret = smaf_set_secure(dmabuf, data.secure);
+
+		dma_buf_put(dmabuf);
+
+		if (ret)
+			return -EINVAL;
+
+		break;
+	}
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct file_operations smaf_fops = {
+	.owner = THIS_MODULE,
+	.unlocked_ioctl = smaf_ioctl,
+};
+
+static int __init smaf_init(void)
+{
+	int ret = 0;
+
+	smaf_dev.misc_dev.minor = MISC_DYNAMIC_MINOR;
+	smaf_dev.misc_dev.name  = "smaf";
+	smaf_dev.misc_dev.fops  = &smaf_fops;
+
+	/* register misc device */
+	ret = misc_register(&smaf_dev.misc_dev);
+	if (ret < 0)
+		return ret;
+
+	mutex_init(&smaf_dev.lock);
+	INIT_LIST_HEAD(&smaf_dev.head);
+
+	return ret;
+}
+module_init(smaf_init);
+
+static void __exit smaf_deinit(void)
+{
+	misc_deregister(&smaf_dev.misc_dev);
+}
+module_exit(smaf_deinit);
+
+MODULE_DESCRIPTION("Secure Memory Allocation Framework");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Benjamin Gaignard <benjamin.gaignard@linaro.org>");
diff --git a/include/linux/smaf-allocator.h b/include/linux/smaf-allocator.h
new file mode 100644
index 0000000..f764ef4
--- /dev/null
+++ b/include/linux/smaf-allocator.h
@@ -0,0 +1,54 @@
+/*
+ * smaf-allocator.h
+ *
+ * Copyright (C) Linaro SA 2015
+ * Author: Benjamin Gaignard <benjamin.gaignard@linaro.org> for Linaro.
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#ifndef _SMAF_ALLOCATOR_H_
+#define _SMAF_ALLOCATOR_H_
+
+#include <linux/dma-buf.h>
+#include <linux/list.h>
+
+/**
+ * struct smaf_allocator - implement dma_buf_ops like functions
+ *
+ * @match: match function to check if allocator can accept the devices
+ *	   attached to dmabuf
+ * @allocate: allocate memory with the given length and flags
+ *	      return a dma_buf handle
+ * @name: allocator name
+ * @ranking: allocator ranking (bigger is better)
+ */
+struct smaf_allocator {
+	struct list_head list_node;
+	bool (*match)(struct dma_buf *dmabuf);
+	struct dma_buf *(*allocate)(struct dma_buf *dmabuf,
+				    size_t length, unsigned int flags);
+	const char *name;
+	int ranking;
+};
+
+/**
+ * smaf_register_allocator - register an allocator to be used by SMAF
+ * @alloc: smaf_allocator structure
+ */
+int smaf_register_allocator(struct smaf_allocator *alloc);
+
+/**
+ * smaf_unregister_allocator - unregister alloctor
+ * @alloc: smaf_allocator structure
+ */
+void smaf_unregister_allocator(struct smaf_allocator *alloc);
+
+/**
+ * smaf_select_allocator_by_name - select an allocator by it name
+ * return 0 if the allocator has been found and selected.
+ * @dmabuf: dma_buf buffer handler
+ * @name: name of the allocator to be selected
+ */
+int smaf_select_allocator_by_name(struct dma_buf *dmabuf, char *name);
+
+#endif
diff --git a/include/linux/smaf-secure.h b/include/linux/smaf-secure.h
new file mode 100644
index 0000000..f8f954c
--- /dev/null
+++ b/include/linux/smaf-secure.h
@@ -0,0 +1,72 @@
+/*
+ * smaf-secure.h
+ *
+ * Copyright (C) Linaro SA 2015
+ * Author: Benjamin Gaignard <benjamin.gaignard@linaro.org> for Linaro.
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#ifndef _SMAF_SECURE_H_
+#define _SMAF_SECURE_H_
+
+/**
+ * struct smaf_secure
+ * @create_context:	create a secure context for one dmabuf.
+ * @destroy_context:	destroy secure context.
+ * @grant_access:	check and provide access to memroy area for a specific
+ *			device.
+ * @revoke_access:	remove device access rights.
+ * @allow_cpu_access:	return true if CPU can access to memory
+ */
+struct smaf_secure {
+	void *(*create_context)(void);
+	int (*destroy_context)(void *ctx);
+	bool (*grant_access)(void *ctx,
+			     struct device *dev,
+			     size_t addr, size_t size,
+			     enum dma_data_direction direction);
+	void (*revoke_access)(void *ctx,
+			      struct device *dev,
+			      size_t addr, size_t size,
+			      enum dma_data_direction direction);
+	bool (*allow_cpu_access)(void *ctx, enum dma_data_direction direction);
+};
+
+/**
+ * smaf_register_secure - register secure module helper
+ * Secure module helper should be platform specific so only one can be
+ * registered.
+ *
+ * @sec: secure module to be registered
+ */
+int smaf_register_secure(struct smaf_secure *sec);
+
+/**
+ * smaf_unregister_secure - unregister secure module helper
+ */
+void smaf_unregister_secure(struct smaf_secure *sec);
+
+/**
+ * smaf_is_secure - test is a dma_buf handle has been secured by SMAF
+ * @dmabuf: dma_buf handle to be tested
+ */
+bool smaf_is_secure(struct dma_buf *dmabuf);
+
+/**
+ * smaf_set_secure - change dma_buf handle secure status
+ * @dmabuf: dma_buf handle to be change
+ * @secure: if true secure dma_buf handle
+ */
+int smaf_set_secure(struct dma_buf *dmabuf, bool secure);
+
+/**
+ * smaf_create_handle - create a smaf_handle with the give length and flags
+ * do not allocate memory but provide smaf_handle->dmabuf that can be
+ * shared between devices.
+ *
+ * @length: buffer size
+ * @flags: handle flags
+ */
+struct smaf_handle *smaf_create_handle(size_t length, unsigned int flags);
+
+#endif
diff --git a/include/uapi/linux/smaf.h b/include/uapi/linux/smaf.h
new file mode 100644
index 0000000..428168e
--- /dev/null
+++ b/include/uapi/linux/smaf.h
@@ -0,0 +1,52 @@
+/*
+ * smaf.h
+ *
+ * Copyright (C) Linaro SA 2015
+ * Author: Benjamin Gaignard <benjamin.gaignard@linaro.org> for Linaro.
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#ifndef _UAPI_SMAF_H_
+#define _UAPI_SMAF_H_
+
+#include <linux/ioctl.h>
+#include <linux/types.h>
+
+#define ALLOCATOR_NAME_LENGTH 64
+
+/**
+ * struct smaf_create_data - allocation parameters
+ * @length:	size of the allocation
+ * @flags:	flags passed to allocator
+ * @name:	name of the allocator to be selected, could be NULL
+ * @fd:		returned file descriptor
+ */
+struct smaf_create_data {
+	size_t length;
+	unsigned int flags;
+	char name[ALLOCATOR_NAME_LENGTH];
+	int fd;
+};
+
+/**
+ * struct smaf_secure_flag - set/get secure flag
+ * @fd:		file descriptor
+ * @secure:	secure flag value (set or get)
+ */
+struct smaf_secure_flag {
+	int fd;
+	int secure;
+};
+
+#define SMAF_IOC_MAGIC	'S'
+
+#define SMAF_IOC_CREATE		 _IOWR(SMAF_IOC_MAGIC, 0, \
+				       struct smaf_create_data)
+
+#define SMAF_IOC_GET_SECURE_FLAG _IOWR(SMAF_IOC_MAGIC, 1, \
+				       struct smaf_secure_flag)
+
+#define SMAF_IOC_SET_SECURE_FLAG _IOWR(SMAF_IOC_MAGIC, 2, \
+				       struct smaf_secure_flag)
+
+#endif
-- 
1.9.1

Re: [PATCH v4 1/2] create SMAF module

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 5016 bytes --]

Hi Benjamin,

[auto build test ERROR on v4.3-rc4 -- if it's inappropriate base, please ignore]

config: s390-allmodconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=s390 

All error/warnings (new ones prefixed by >>):

   In file included from arch/s390/include/asm/elf.h:130:0,
                    from include/linux/elf.h:4,
                    from include/linux/module.h:15,
                    from drivers/smaf/smaf-core.c:16:
   drivers/smaf/smaf-core.c: In function 'smaf_unsecure_handle':
>> arch/s390/include/asm/mmu_context.h:33:41: error: expected identifier before 'do'
    #define destroy_context(mm)             do { } while (0)
                                            ^
>> drivers/smaf/smaf-core.c:136:23: note: in expansion of macro 'destroy_context'
     if (smaf_dev.secure->destroy_context(handle->secure_ctx))
                          ^

vim +/destroy_context +136 drivers/smaf/smaf-core.c

    10	#include <linux/dma-buf.h>
    11	#include <linux/dma-mapping.h>
    12	#include <linux/fs.h>
    13	#include <linux/ioctl.h>
    14	#include <linux/list_sort.h>
    15	#include <linux/miscdevice.h>
  > 16	#include <linux/module.h>
    17	#include <linux/slab.h>
    18	#include <linux/smaf.h>
    19	#include <linux/smaf-allocator.h>
    20	#include <linux/smaf-secure.h>
    21	#include <linux/uaccess.h>
    22	
    23	struct smaf_handle {
    24		struct dma_buf *dmabuf;
    25		struct smaf_allocator *allocator;
    26		struct dma_buf *db_alloc;
    27		size_t length;
    28		unsigned int flags;
    29		int fd;
    30		bool is_secure;
    31		void *secure_ctx;
    32	};
    33	
    34	/**
    35	 * struct smaf_device - smaf device node private data
    36	 * @misc_dev:	the misc device
    37	 * @head:	list of allocator
    38	 * @lock:	list and secure pointer mutex
    39	 * @secure:	pointer to secure functions helpers
    40	 */
    41	struct smaf_device {
    42		struct miscdevice misc_dev;
    43		struct list_head head;
    44		/* list and secure pointer lock*/
    45		struct mutex lock;
    46		struct smaf_secure *secure;
    47	};
    48	
    49	static struct smaf_device smaf_dev;
    50	
    51	/**
    52	 * smaf_allow_cpu_access return true if CPU can access to memory
    53	 * if their is no secure module associated to SMAF assume that CPU can get
    54	 * access to the memory.
    55	 */
    56	static bool smaf_allow_cpu_access(struct smaf_handle *handle,
    57					  unsigned long flags)
    58	{
    59		if (!handle->is_secure)
    60			return true;
    61	
    62		if (!smaf_dev.secure)
    63			return true;
    64	
    65		if (!smaf_dev.secure->allow_cpu_access)
    66			return true;
    67	
    68		return smaf_dev.secure->allow_cpu_access(handle->secure_ctx, flags);
    69	}
    70	
    71	static int smaf_grant_access(struct smaf_handle *handle, struct device *dev,
    72				     dma_addr_t addr, size_t size,
    73				     enum dma_data_direction dir)
    74	{
    75		if (!handle->is_secure)
    76			return 0;
    77	
    78		if (!smaf_dev.secure)
    79			return -EINVAL;
    80	
    81		if (!smaf_dev.secure->grant_access)
    82			return -EINVAL;
    83	
    84		return smaf_dev.secure->grant_access(handle->secure_ctx,
    85						     dev, addr, size, dir);
    86	}
    87	
    88	static void smaf_revoke_access(struct smaf_handle *handle, struct device *dev,
    89				       dma_addr_t addr, size_t size,
    90				       enum dma_data_direction dir)
    91	{
    92		if (!handle->is_secure)
    93			return;
    94	
    95		if (!smaf_dev.secure)
    96			return;
    97	
    98		if (!smaf_dev.secure->revoke_access)
    99			return;
   100	
   101		smaf_dev.secure->revoke_access(handle->secure_ctx,
   102					       dev, addr, size, dir);
   103	}
   104	
   105	static int smaf_secure_handle(struct smaf_handle *handle)
   106	{
   107		if (handle->is_secure)
   108			return 0;
   109	
   110		if (!smaf_dev.secure)
   111			return -EINVAL;
   112	
   113		if (!smaf_dev.secure->create_context)
   114			return -EINVAL;
   115	
   116		handle->secure_ctx = smaf_dev.secure->create_context();
   117	
   118		if (!handle->secure_ctx)
   119			return -EINVAL;
   120	
   121		handle->is_secure = true;
   122		return 0;
   123	}
   124	
   125	static int smaf_unsecure_handle(struct smaf_handle *handle)
   126	{
   127		if (!handle->is_secure)
   128			return 0;
   129	
   130		if (!smaf_dev.secure)
   131			return -EINVAL;
   132	
   133		if (!smaf_dev.secure->destroy_context)
   134			return -EINVAL;
   135	
 > 136		if (smaf_dev.secure->destroy_context(handle->secure_ctx))
   137			return -EINVAL;
   138	
   139		handle->secure_ctx = NULL;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 38491 bytes --]

Re: [PATCH v4 1/2] create SMAF module

[Laura Abbott]

On 10/05/2015 03:11 AM, Benjamin Gaignard wrote:
> diff --git a/drivers/smaf/smaf-core.c b/drivers/smaf/smaf-core.c
> new file mode 100644
> index 0000000..37914e7
> --- /dev/null
> +++ b/drivers/smaf/smaf-core.c
> @@ -0,0 +1,736 @@
> +/*
> + * smaf.c
> + *
> + * Copyright (C) Linaro SA 2015
> + * Author: Benjamin Gaignard <benjamin.gaignard@linaro.org> for Linaro.
> + * License terms:  GNU General Public License (GPL), version 2
> + */
> +
> +#include <linux/device.h>
> +#include <linux/dma-buf.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/fs.h>
> +#include <linux/ioctl.h>
> +#include <linux/list_sort.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/smaf.h>
> +#include <linux/smaf-allocator.h>
> +#include <linux/smaf-secure.h>
> +#include <linux/uaccess.h>
> +
> +struct smaf_handle {
> +	struct dma_buf *dmabuf;
> +	struct smaf_allocator *allocator;
> +	struct dma_buf *db_alloc;
> +	size_t length;
> +	unsigned int flags;
> +	int fd;
> +	bool is_secure;
> +	void *secure_ctx;
> +};
> +
> +/**
> + * struct smaf_device - smaf device node private data
> + * @misc_dev:	the misc device
> + * @head:	list of allocator
> + * @lock:	list and secure pointer mutex
> + * @secure:	pointer to secure functions helpers
> + */
> +struct smaf_device {
> +	struct miscdevice misc_dev;
> +	struct list_head head;
> +	/* list and secure pointer lock*/
> +	struct mutex lock;
> +	struct smaf_secure *secure;
> +};
> +
> +static struct smaf_device smaf_dev;
> +
> +/**
> + * smaf_allow_cpu_access return true if CPU can access to memory
> + * if their is no secure module associated to SMAF assume that CPU can get
> + * access to the memory.
> + */
> +static bool smaf_allow_cpu_access(struct smaf_handle *handle,
> +				  unsigned long flags)
> +{
> +	if (!handle->is_secure)
> +		return true;
> +
> +	if (!smaf_dev.secure)
> +		return true;
> +
> +	if (!smaf_dev.secure->allow_cpu_access)
> +		return true;
> +
> +	return smaf_dev.secure->allow_cpu_access(handle->secure_ctx, flags);
> +}
> +
> +static int smaf_grant_access(struct smaf_handle *handle, struct device *dev,
> +			     dma_addr_t addr, size_t size,
> +			     enum dma_data_direction dir)
> +{
> +	if (!handle->is_secure)
> +		return 0;
> +
> +	if (!smaf_dev.secure)
> +		return -EINVAL;
> +
> +	if (!smaf_dev.secure->grant_access)
> +		return -EINVAL;
> +
> +	return smaf_dev.secure->grant_access(handle->secure_ctx,
> +					     dev, addr, size, dir);
> +}
> +
> +static void smaf_revoke_access(struct smaf_handle *handle, struct device *dev,
> +			       dma_addr_t addr, size_t size,
> +			       enum dma_data_direction dir)
> +{
> +	if (!handle->is_secure)
> +		return;
> +
> +	if (!smaf_dev.secure)
> +		return;
> +
> +	if (!smaf_dev.secure->revoke_access)
> +		return;
> +
> +	smaf_dev.secure->revoke_access(handle->secure_ctx,
> +				       dev, addr, size, dir);
> +}
> +
> +static int smaf_secure_handle(struct smaf_handle *handle)
> +{
> +	if (handle->is_secure)
> +		return 0;
> +
> +	if (!smaf_dev.secure)
> +		return -EINVAL;
> +
> +	if (!smaf_dev.secure->create_context)
> +		return -EINVAL;
> +
> +	handle->secure_ctx = smaf_dev.secure->create_context();
> +
> +	if (!handle->secure_ctx)
> +		return -EINVAL;
> +
> +	handle->is_secure = true;
> +	return 0;
> +}
> +
> +static int smaf_unsecure_handle(struct smaf_handle *handle)
> +{
> +	if (!handle->is_secure)
> +		return 0;
> +
> +	if (!smaf_dev.secure)
> +		return -EINVAL;
> +
> +	if (!smaf_dev.secure->destroy_context)
> +		return -EINVAL;
> +
> +	if (smaf_dev.secure->destroy_context(handle->secure_ctx))
> +		return -EINVAL;
> +
> +	handle->secure_ctx = NULL;
> +	handle->is_secure = false;
> +	return 0;
> +}

All these functions need to be protected by a lock, otherwise the
secure state could change. For that matter, I think the smaf_handle
needs a lock to protect its state as well for places like map_dma_buf

>
<snip>
> +static long smaf_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> +	switch (cmd) {
> +	case SMAF_IOC_CREATE:
> +	{
> +		struct smaf_create_data data;
> +		struct smaf_handle *handle;
> +
> +		if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))
> +			return -EFAULT;
> +
> +		handle = smaf_create_handle(data.length, data.flags);
> +		if (!handle)
> +			return -EINVAL;
> +
> +		if (data.name[0]) {
> +			/* user force allocator selection */
> +			if (smaf_select_allocator_by_name(handle->dmabuf,
> +							  data.name)) {
> +				dma_buf_put(handle->dmabuf);
> +				return -EINVAL;
> +			}
> +		}
> +
> +		handle->fd = dma_buf_fd(handle->dmabuf, data.flags);
> +		if (handle->fd < 0) {
> +			dma_buf_put(handle->dmabuf);
> +			return -EINVAL;
> +		}
> +
> +		data.fd = handle->fd;
> +		if (copy_to_user((void __user *)arg, &data, _IOC_SIZE(cmd))) {
> +			dma_buf_put(handle->dmabuf);
> +			return -EFAULT;
> +		}
> +		break;
> +	}
> +	case SMAF_IOC_GET_SECURE_FLAG:
> +	{
> +		struct smaf_secure_flag data;
> +		struct dma_buf *dmabuf;
> +
> +		if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))
> +			return -EFAULT;
> +
> +		dmabuf = dma_buf_get(data.fd);
> +		if (!dmabuf)
> +			return -EINVAL;
> +
> +		data.secure = smaf_is_secure(dmabuf);
> +		dma_buf_put(dmabuf);
> +
> +		if (copy_to_user((void __user *)arg, &data, _IOC_SIZE(cmd)))
> +			return -EFAULT;
> +		break;
> +	}
> +	case SMAF_IOC_SET_SECURE_FLAG:
> +	{
> +		struct smaf_secure_flag data;
> +		struct dma_buf *dmabuf;
> +		int ret;
> +
> +		if (!smaf_dev.secure)
> +			return -EINVAL;
> +
> +		if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))
> +			return -EFAULT;
> +
> +		dmabuf = dma_buf_get(data.fd);
> +		if (!dmabuf)
> +			return -EINVAL;
> +
> +		ret = smaf_set_secure(dmabuf, data.secure);
> +
> +		dma_buf_put(dmabuf);
> +
> +		if (ret)
> +			return -EINVAL;
> +
> +		break;
> +	}
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}

How would you see this tying into something like Ion? It seems like
Ion and SMAF have non-zero over lapping functionality for some things
or that SMAF could be implemented as a heap type. I think my biggest
concern here is that it seems like either Ion or SMAF is going to feel
redundant as an interface.

Thanks,
Laura

Re: [PATCH v4 1/2] create SMAF module

[Benjamin Gaignard]

Thanks for your review I will add a lock in smaf_handle structure.

One of the goal of smaf is to create a standard kernel API to allocate
and secure buffers to avoid forking
while implementing buffer securing feature.

One concern about ION is that the selection of the heap is done by userland
so hardware constraints need to be known by the userland, which is
problematic from my point of view.
Compare to ION I have try to introduce features like securing API,
flexible allocator selection on kernel
and the possibility to add custom allocator and securing modules.

Benjamin



2015-10-06 3:58 GMT+02:00 Laura Abbott <labbott@redhat.com>:
> On 10/05/2015 03:11 AM, Benjamin Gaignard wrote:
>>
>> diff --git a/drivers/smaf/smaf-core.c b/drivers/smaf/smaf-core.c
>> new file mode 100644
>> index 0000000..37914e7
>> --- /dev/null
>> +++ b/drivers/smaf/smaf-core.c
>> @@ -0,0 +1,736 @@
>> +/*
>> + * smaf.c
>> + *
>> + * Copyright (C) Linaro SA 2015
>> + * Author: Benjamin Gaignard <benjamin.gaignard@linaro.org> for Linaro.
>> + * License terms:  GNU General Public License (GPL), version 2
>> + */
>> +
>> +#include <linux/device.h>
>> +#include <linux/dma-buf.h>
>> +#include <linux/dma-mapping.h>
>> +#include <linux/fs.h>
>> +#include <linux/ioctl.h>
>> +#include <linux/list_sort.h>
>> +#include <linux/miscdevice.h>
>> +#include <linux/module.h>
>> +#include <linux/slab.h>
>> +#include <linux/smaf.h>
>> +#include <linux/smaf-allocator.h>
>> +#include <linux/smaf-secure.h>
>> +#include <linux/uaccess.h>
>> +
>> +struct smaf_handle {
>> +       struct dma_buf *dmabuf;
>> +       struct smaf_allocator *allocator;
>> +       struct dma_buf *db_alloc;
>> +       size_t length;
>> +       unsigned int flags;
>> +       int fd;
>> +       bool is_secure;
>> +       void *secure_ctx;
>> +};
>> +
>> +/**
>> + * struct smaf_device - smaf device node private data
>> + * @misc_dev:  the misc device
>> + * @head:      list of allocator
>> + * @lock:      list and secure pointer mutex
>> + * @secure:    pointer to secure functions helpers
>> + */
>> +struct smaf_device {
>> +       struct miscdevice misc_dev;
>> +       struct list_head head;
>> +       /* list and secure pointer lock*/
>> +       struct mutex lock;
>> +       struct smaf_secure *secure;
>> +};
>> +
>> +static struct smaf_device smaf_dev;
>> +
>> +/**
>> + * smaf_allow_cpu_access return true if CPU can access to memory
>> + * if their is no secure module associated to SMAF assume that CPU can
>> get
>> + * access to the memory.
>> + */
>> +static bool smaf_allow_cpu_access(struct smaf_handle *handle,
>> +                                 unsigned long flags)
>> +{
>> +       if (!handle->is_secure)
>> +               return true;
>> +
>> +       if (!smaf_dev.secure)
>> +               return true;
>> +
>> +       if (!smaf_dev.secure->allow_cpu_access)
>> +               return true;
>> +
>> +       return smaf_dev.secure->allow_cpu_access(handle->secure_ctx,
>> flags);
>> +}
>> +
>> +static int smaf_grant_access(struct smaf_handle *handle, struct device
>> *dev,
>> +                            dma_addr_t addr, size_t size,
>> +                            enum dma_data_direction dir)
>> +{
>> +       if (!handle->is_secure)
>> +               return 0;
>> +
>> +       if (!smaf_dev.secure)
>> +               return -EINVAL;
>> +
>> +       if (!smaf_dev.secure->grant_access)
>> +               return -EINVAL;
>> +
>> +       return smaf_dev.secure->grant_access(handle->secure_ctx,
>> +                                            dev, addr, size, dir);
>> +}
>> +
>> +static void smaf_revoke_access(struct smaf_handle *handle, struct device
>> *dev,
>> +                              dma_addr_t addr, size_t size,
>> +                              enum dma_data_direction dir)
>> +{
>> +       if (!handle->is_secure)
>> +               return;
>> +
>> +       if (!smaf_dev.secure)
>> +               return;
>> +
>> +       if (!smaf_dev.secure->revoke_access)
>> +               return;
>> +
>> +       smaf_dev.secure->revoke_access(handle->secure_ctx,
>> +                                      dev, addr, size, dir);
>> +}
>> +
>> +static int smaf_secure_handle(struct smaf_handle *handle)
>> +{
>> +       if (handle->is_secure)
>> +               return 0;
>> +
>> +       if (!smaf_dev.secure)
>> +               return -EINVAL;
>> +
>> +       if (!smaf_dev.secure->create_context)
>> +               return -EINVAL;
>> +
>> +       handle->secure_ctx = smaf_dev.secure->create_context();
>> +
>> +       if (!handle->secure_ctx)
>> +               return -EINVAL;
>> +
>> +       handle->is_secure = true;
>> +       return 0;
>> +}
>> +
>> +static int smaf_unsecure_handle(struct smaf_handle *handle)
>> +{
>> +       if (!handle->is_secure)
>> +               return 0;
>> +
>> +       if (!smaf_dev.secure)
>> +               return -EINVAL;
>> +
>> +       if (!smaf_dev.secure->destroy_context)
>> +               return -EINVAL;
>> +
>> +       if (smaf_dev.secure->destroy_context(handle->secure_ctx))
>> +               return -EINVAL;
>> +
>> +       handle->secure_ctx = NULL;
>> +       handle->is_secure = false;
>> +       return 0;
>> +}
>
>
> All these functions need to be protected by a lock, otherwise the
> secure state could change. For that matter, I think the smaf_handle
> needs a lock to protect its state as well for places like map_dma_buf
>
>>
> <snip>
>
>> +static long smaf_ioctl(struct file *file, unsigned int cmd, unsigned long
>> arg)
>> +{
>> +       switch (cmd) {
>> +       case SMAF_IOC_CREATE:
>> +       {
>> +               struct smaf_create_data data;
>> +               struct smaf_handle *handle;
>> +
>> +               if (copy_from_user(&data, (void __user *)arg,
>> _IOC_SIZE(cmd)))
>> +                       return -EFAULT;
>> +
>> +               handle = smaf_create_handle(data.length, data.flags);
>> +               if (!handle)
>> +                       return -EINVAL;
>> +
>> +               if (data.name[0]) {
>> +                       /* user force allocator selection */
>> +                       if (smaf_select_allocator_by_name(handle->dmabuf,
>> +                                                         data.name)) {
>> +                               dma_buf_put(handle->dmabuf);
>> +                               return -EINVAL;
>> +                       }
>> +               }
>> +
>> +               handle->fd = dma_buf_fd(handle->dmabuf, data.flags);
>> +               if (handle->fd < 0) {
>> +                       dma_buf_put(handle->dmabuf);
>> +                       return -EINVAL;
>> +               }
>> +
>> +               data.fd = handle->fd;
>> +               if (copy_to_user((void __user *)arg, &data,
>> _IOC_SIZE(cmd))) {
>> +                       dma_buf_put(handle->dmabuf);
>> +                       return -EFAULT;
>> +               }
>> +               break;
>> +       }
>> +       case SMAF_IOC_GET_SECURE_FLAG:
>> +       {
>> +               struct smaf_secure_flag data;
>> +               struct dma_buf *dmabuf;
>> +
>> +               if (copy_from_user(&data, (void __user *)arg,
>> _IOC_SIZE(cmd)))
>> +                       return -EFAULT;
>> +
>> +               dmabuf = dma_buf_get(data.fd);
>> +               if (!dmabuf)
>> +                       return -EINVAL;
>> +
>> +               data.secure = smaf_is_secure(dmabuf);
>> +               dma_buf_put(dmabuf);
>> +
>> +               if (copy_to_user((void __user *)arg, &data,
>> _IOC_SIZE(cmd)))
>> +                       return -EFAULT;
>> +               break;
>> +       }
>> +       case SMAF_IOC_SET_SECURE_FLAG:
>> +       {
>> +               struct smaf_secure_flag data;
>> +               struct dma_buf *dmabuf;
>> +               int ret;
>> +
>> +               if (!smaf_dev.secure)
>> +                       return -EINVAL;
>> +
>> +               if (copy_from_user(&data, (void __user *)arg,
>> _IOC_SIZE(cmd)))
>> +                       return -EFAULT;
>> +
>> +               dmabuf = dma_buf_get(data.fd);
>> +               if (!dmabuf)
>> +                       return -EINVAL;
>> +
>> +               ret = smaf_set_secure(dmabuf, data.secure);
>> +
>> +               dma_buf_put(dmabuf);
>> +
>> +               if (ret)
>> +                       return -EINVAL;
>> +
>> +               break;
>> +       }
>> +       default:
>> +               return -EINVAL;
>> +       }
>> +
>> +       return 0;
>> +}
>
>
> How would you see this tying into something like Ion? It seems like
> Ion and SMAF have non-zero over lapping functionality for some things
> or that SMAF could be implemented as a heap type. I think my biggest
> concern here is that it seems like either Ion or SMAF is going to feel
> redundant as an interface.
>
> Thanks,
> Laura



-- 
Benjamin Gaignard

Graphic Working Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

[PATCH v4 2/2] SMAF: add CMA allocator

[Benjamin Gaignard]

SMAF CMA allocator implement helpers functions to allow SMAF
to allocate contiguous memory.

match() each if at least one of the attached devices have coherent_dma_mask
set to DMA_BIT_MASK(32).

For allocation it use dma_alloc_attrs() with DMA_ATTR_WRITE_COMBINE and not
dma_alloc_writecombine to be compatible with ARM 64bits architecture

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 drivers/smaf/Kconfig    |   6 ++
 drivers/smaf/Makefile   |   1 +
 drivers/smaf/smaf-cma.c | 200 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 207 insertions(+)
 create mode 100644 drivers/smaf/smaf-cma.c

diff --git a/drivers/smaf/Kconfig b/drivers/smaf/Kconfig
index d36651a..058ec4c 100644
--- a/drivers/smaf/Kconfig
+++ b/drivers/smaf/Kconfig
@@ -3,3 +3,9 @@ config SMAF
 	depends on DMA_SHARED_BUFFER
 	help
 	  Choose this option to enable Secure Memory Allocation Framework
+
+config SMAF_CMA
+	tristate "SMAF CMA allocator"
+	depends on SMAF && HAVE_DMA_ATTRS
+	help
+	  Choose this option to enable CMA allocation within SMAF
diff --git a/drivers/smaf/Makefile b/drivers/smaf/Makefile
index 40cd882..05bab01 100644
--- a/drivers/smaf/Makefile
+++ b/drivers/smaf/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_SMAF) += smaf-core.o
+obj-$(CONFIG_SMAF_CMA) += smaf-cma.o
diff --git a/drivers/smaf/smaf-cma.c b/drivers/smaf/smaf-cma.c
new file mode 100644
index 0000000..ab38717
--- /dev/null
+++ b/drivers/smaf/smaf-cma.c
@@ -0,0 +1,200 @@
+/*
+ * smaf-cma.c
+ *
+ * Copyright (C) Linaro SA 2015
+ * Author: Benjamin Gaignard <benjamin.gaignard@linaro.org> for Linaro.
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#include <linux/dma-mapping.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/smaf-allocator.h>
+
+struct smaf_cma_buffer_info {
+	struct device *dev;
+	size_t size;
+	void *vaddr;
+	dma_addr_t paddr;
+};
+
+/**
+ * find_matching_device - iterate over the attached devices to find one
+ * with coherent_dma_mask correctly set to DMA_BIT_MASK(32).
+ * Matching device (if any) will be used to aim CMA area.
+ */
+static struct device *find_matching_device(struct dma_buf *dmabuf)
+{
+	struct dma_buf_attachment *attach_obj;
+
+	list_for_each_entry(attach_obj, &dmabuf->attachments, node) {
+		if (attach_obj->dev->coherent_dma_mask == DMA_BIT_MASK(32))
+			return attach_obj->dev;
+	}
+
+	return NULL;
+}
+
+/**
+ * smaf_cma_match - return true if at least one device has been found
+ */
+static bool smaf_cma_match(struct dma_buf *dmabuf)
+{
+	return !!find_matching_device(dmabuf);
+}
+
+static void smaf_cma_release(struct dma_buf *dmabuf)
+{
+	struct smaf_cma_buffer_info *info = dmabuf->priv;
+	DEFINE_DMA_ATTRS(attrs);
+
+	dma_set_attr(DMA_ATTR_WRITE_COMBINE, &attrs);
+
+	dma_free_attrs(info->dev, info->size, info->vaddr, info->paddr, &attrs);
+
+	kfree(info);
+}
+
+static struct sg_table *smaf_cma_map(struct dma_buf_attachment *attachment,
+				     enum dma_data_direction direction)
+{
+	struct smaf_cma_buffer_info *info = attachment->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->paddr, info->size);
+	if (ret < 0)
+		goto out;
+
+	sg_dma_address(sgt->sgl) = info->paddr;
+	return sgt;
+
+out:
+	kfree(sgt);
+	return NULL;
+}
+
+static void smaf_cma_unmap(struct dma_buf_attachment *attachment,
+			   struct sg_table *sgt,
+			   enum dma_data_direction direction)
+{
+	/* do nothing */
+}
+
+static int smaf_cma_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
+{
+	struct smaf_cma_buffer_info *info = dmabuf->priv;
+	int ret;
+	DEFINE_DMA_ATTRS(attrs);
+
+	dma_set_attr(DMA_ATTR_WRITE_COMBINE, &attrs);
+
+	if (info->size < vma->vm_end - vma->vm_start)
+		return -EINVAL;
+
+	vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
+	ret = dma_mmap_attrs(info->dev, vma, info->vaddr, info->paddr,
+			     info->size, &attrs);
+
+	return ret;
+}
+
+static void *smaf_cma_vmap(struct dma_buf *dmabuf)
+{
+	struct smaf_cma_buffer_info *info = dmabuf->priv;
+
+	return info->vaddr;
+}
+
+static void *smaf_kmap_atomic(struct dma_buf *dmabuf, unsigned long offset)
+{
+	struct smaf_cma_buffer_info *info = dmabuf->priv;
+
+	return (void *)info->paddr + offset;
+}
+
+static struct dma_buf_ops smaf_cma_ops = {
+	.map_dma_buf = smaf_cma_map,
+	.unmap_dma_buf = smaf_cma_unmap,
+	.mmap = smaf_cma_mmap,
+	.release = smaf_cma_release,
+	.kmap_atomic = smaf_kmap_atomic,
+	.kmap = smaf_kmap_atomic,
+	.vmap = smaf_cma_vmap,
+};
+
+static struct dma_buf *smaf_cma_allocate(struct dma_buf *dmabuf,
+					 size_t length, unsigned int flags)
+{
+	struct dma_buf_attachment *attach_obj;
+	struct smaf_cma_buffer_info *info;
+	struct dma_buf *cma_dmabuf;
+	int ret;
+
+	DEFINE_DMA_BUF_EXPORT_INFO(export);
+	DEFINE_DMA_ATTRS(attrs);
+
+	dma_set_attr(DMA_ATTR_WRITE_COMBINE, &attrs);
+
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return NULL;
+
+	info->size = round_up(length, PAGE_SIZE);
+	info->dev = find_matching_device(dmabuf);
+
+	info->vaddr = dma_alloc_attrs(info->dev, info->size, &info->paddr,
+				      GFP_KERNEL | __GFP_NOWARN, &attrs);
+	if (!info->vaddr) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
+	export.ops = &smaf_cma_ops;
+	export.size = info->size;
+	export.flags = flags;
+	export.priv = info;
+
+	cma_dmabuf = dma_buf_export(&export);
+	if (IS_ERR(cma_dmabuf))
+		goto error;
+
+	list_for_each_entry(attach_obj, &dmabuf->attachments, node) {
+		dma_buf_attach(cma_dmabuf, attach_obj->dev);
+	}
+
+	return cma_dmabuf;
+
+error:
+	kfree(info);
+	return NULL;
+}
+
+struct smaf_allocator smaf_cma = {
+	.match = smaf_cma_match,
+	.allocate = smaf_cma_allocate,
+	.name = "smaf-cma",
+	.ranking = 0,
+};
+
+static int __init smaf_cma_init(void)
+{
+	INIT_LIST_HEAD(&smaf_cma.list_node);
+	return smaf_register_allocator(&smaf_cma);
+}
+module_init(smaf_cma_init);
+
+static void __exit smaf_cma_deinit(void)
+{
+	smaf_unregister_allocator(&smaf_cma);
+}
+module_exit(smaf_cma_deinit);
+
+MODULE_DESCRIPTION("SMAF CMA module");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Benjamin Gaignard <benjamin.gaignard@linaro.org>");
-- 
1.9.1

Re: [PATCH v4 2/2] SMAF: add CMA allocator

[kbuild test robot]

Hi Benjamin,

[auto build test WARNING on v4.3-rc4 -- if it's inappropriate base, please ignore]

reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/smaf/smaf-cma.c:178:23: sparse: symbol 'smaf_cma' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[RFC PATCH] SMAF: smaf_cma can be static

[kbuild test robot]

Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
 smaf-cma.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/smaf/smaf-cma.c b/drivers/smaf/smaf-cma.c
index ab38717..9fbd9b7 100644
--- a/drivers/smaf/smaf-cma.c
+++ b/drivers/smaf/smaf-cma.c
@@ -175,7 +175,7 @@ error:
 	return NULL;
 }
 
-struct smaf_allocator smaf_cma = {
+static struct smaf_allocator smaf_cma = {
 	.match = smaf_cma_match,
 	.allocate = smaf_cma_allocate,
 	.name = "smaf-cma",

Re: [PATCH v4 2/2] SMAF: add CMA allocator

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1581 bytes --]

Hi Benjamin,

[auto build test WARNING on v4.3-rc4 -- if it's inappropriate base, please ignore]

config: sparc-allmodconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc 

All warnings (new ones prefixed by >>):

   drivers/smaf/smaf-cma.c: In function 'smaf_kmap_atomic':
>> drivers/smaf/smaf-cma.c:118:9: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
     return (void *)info->paddr + offset;
            ^

vim +118 drivers/smaf/smaf-cma.c

   102				     info->size, &attrs);
   103	
   104		return ret;
   105	}
   106	
   107	static void *smaf_cma_vmap(struct dma_buf *dmabuf)
   108	{
   109		struct smaf_cma_buffer_info *info = dmabuf->priv;
   110	
   111		return info->vaddr;
   112	}
   113	
   114	static void *smaf_kmap_atomic(struct dma_buf *dmabuf, unsigned long offset)
   115	{
   116		struct smaf_cma_buffer_info *info = dmabuf->priv;
   117	
 > 118		return (void *)info->paddr + offset;
   119	}
   120	
   121	static struct dma_buf_ops smaf_cma_ops = {
   122		.map_dma_buf = smaf_cma_map,
   123		.unmap_dma_buf = smaf_cma_unmap,
   124		.mmap = smaf_cma_mmap,
   125		.release = smaf_cma_release,
   126		.kmap_atomic = smaf_kmap_atomic,

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 43420 bytes --]

Re: [PATCH v4 0/2] RFC: Secure Memory Allocation Framework

[Laura Abbott]

On 10/05/2015 03:11 AM, Benjamin Gaignard wrote:
> version 4 changes:
>   - rebased on kernel 4.3-rc3
>   - fix missing EXPORT_SYMBOL for smaf_create_handle()
>
> version 3 changes:
>   - Remove ioctl for allocator selection instead provide the name of
>     the targeted allocator with allocation request.
>     Selecting allocator from userland isn't the prefered way of working
>     but is needed when the first user of the buffer is a software component.
>   - Fix issues in case of error while creating smaf handle.
>   - Fix module license.
>   - Update libsmaf and tests to care of the SMAF API evolution
>     https://git.linaro.org/people/benjamin.gaignard/libsmaf.git
>
> version 2 changes:
>   - Add one ioctl to allow allocator selection from userspace.
>     This is required for the uses case where the first user of
>     the buffer is a software IP which can't perform dma_buf attachement.
>   - Add name and ranking to allocator structure to be able to sort them.
>   - Create a tiny library to test SMAF:
>     https://git.linaro.org/people/benjamin.gaignard/libsmaf.git
>   - Fix one issue when try to secure buffer without secure module registered
>
> The outcome of the previous RFC about how do secure data path was the need
> of a secure memory allocator (https://lkml.org/lkml/2015/5/5/551)
>
> SMAF goal is to provide a framework that allow allocating and securing
> memory by using dma_buf. Each platform have it own way to perform those two
> features so SMAF design allow to register helper modules to perform them.
>
> To be sure to select the best allocation method for devices SMAF implement
> deferred allocation mechanism: memory allocation is only done when the first
> device effectively required it.
> Allocator modules have to implement a match() to let SMAF know if they are
> compatibles with devices needs.
> This patch set provide an example of allocator module which use
> dma_{alloc/free/mmap}_attrs() and check if at least one device have
> coherent_dma_mask set to DMA_BIT_MASK(32) in match function.
> I have named smaf-cma.c like it is done for drm_gem_cma_helper.c even if
> a better name could be found for this file.
>
> Secure modules are responsibles of granting and revoking devices access rights
> on the memory. Secure module is also called to check if CPU map memory into
> kernel and user address spaces.
> An example of secure module implementation can be found here:
> http://git.linaro.org/people/benjamin.gaignard/optee-sdp.git
> This code isn't yet part of the patch set because it depends on generic TEE
> which is still under discussion (https://lwn.net/Articles/644646/)
>
> For allocation part of SMAF code I get inspirated by Sumit Semwal work about
> constraint aware allocator.
>

Overall I like the abstraction. Do you have a use case in mind right now for
the best allocation method? Some of the classic examples (mmu vs. no mmu)
are gradually becoming less relevant as the systems have evolved. I was
discussing constraints with Sumit w.r.t. Ion at plumbers so I'm curious about
your uses.

Thanks,
Laura

Re: [PATCH v4 0/2] RFC: Secure Memory Allocation Framework

[Benjamin Gaignard]

I have mind few uses cases:
- the basic one when an HW device need contiguous memory.
- I have a device that could not cross some memory boundaries so I
need a specific allocator for it.
- when allocating memory for security some platform have address
constraints or need to allocate memory in specific areas (most of the
time because of firewall limited capacities in terms of regions)



2015-10-06 4:07 GMT+02:00 Laura Abbott <labbott@redhat.com>:
> On 10/05/2015 03:11 AM, Benjamin Gaignard wrote:
>>
>> version 4 changes:
>>   - rebased on kernel 4.3-rc3
>>   - fix missing EXPORT_SYMBOL for smaf_create_handle()
>>
>> version 3 changes:
>>   - Remove ioctl for allocator selection instead provide the name of
>>     the targeted allocator with allocation request.
>>     Selecting allocator from userland isn't the prefered way of working
>>     but is needed when the first user of the buffer is a software
>> component.
>>   - Fix issues in case of error while creating smaf handle.
>>   - Fix module license.
>>   - Update libsmaf and tests to care of the SMAF API evolution
>>     https://git.linaro.org/people/benjamin.gaignard/libsmaf.git
>>
>> version 2 changes:
>>   - Add one ioctl to allow allocator selection from userspace.
>>     This is required for the uses case where the first user of
>>     the buffer is a software IP which can't perform dma_buf attachement.
>>   - Add name and ranking to allocator structure to be able to sort them.
>>   - Create a tiny library to test SMAF:
>>     https://git.linaro.org/people/benjamin.gaignard/libsmaf.git
>>   - Fix one issue when try to secure buffer without secure module
>> registered
>>
>> The outcome of the previous RFC about how do secure data path was the need
>> of a secure memory allocator (https://lkml.org/lkml/2015/5/5/551)
>>
>> SMAF goal is to provide a framework that allow allocating and securing
>> memory by using dma_buf. Each platform have it own way to perform those
>> two
>> features so SMAF design allow to register helper modules to perform them.
>>
>> To be sure to select the best allocation method for devices SMAF implement
>> deferred allocation mechanism: memory allocation is only done when the
>> first
>> device effectively required it.
>> Allocator modules have to implement a match() to let SMAF know if they are
>> compatibles with devices needs.
>> This patch set provide an example of allocator module which use
>> dma_{alloc/free/mmap}_attrs() and check if at least one device have
>> coherent_dma_mask set to DMA_BIT_MASK(32) in match function.
>> I have named smaf-cma.c like it is done for drm_gem_cma_helper.c even if
>> a better name could be found for this file.
>>
>> Secure modules are responsibles of granting and revoking devices access
>> rights
>> on the memory. Secure module is also called to check if CPU map memory
>> into
>> kernel and user address spaces.
>> An example of secure module implementation can be found here:
>> http://git.linaro.org/people/benjamin.gaignard/optee-sdp.git
>> This code isn't yet part of the patch set because it depends on generic
>> TEE
>> which is still under discussion (https://lwn.net/Articles/644646/)
>>
>> For allocation part of SMAF code I get inspirated by Sumit Semwal work
>> about
>> constraint aware allocator.
>>
>
> Overall I like the abstraction. Do you have a use case in mind right now for
> the best allocation method? Some of the classic examples (mmu vs. no mmu)
> are gradually becoming less relevant as the systems have evolved. I was
> discussing constraints with Sumit w.r.t. Ion at plumbers so I'm curious
> about
> your uses.
>
> Thanks,
> Laura
>
>



-- 
Benjamin Gaignard

Graphic Working Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog