Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

15 participants
3030 discussions

[PATCH] dma-buf, drm, ion: Propagate error code from dma_buf_start_cpu_access()

by Chris Wilson

Drivers, especially i915.ko, can fail during the initial migration of a dma-buf for CPU access. However, the error code from the driver was not being propagated back to ioctl and so userspace was blissfully ignorant of the failure. Rendering corruption ensues. Whilst fixing the ioctl to return the error code from dma_buf_start_cpu_access(), also do the same for dma_buf_end_cpu_access(). For most drivers, dma_buf_end_cpu_access() cannot fail. i915.ko however, as most drivers would, wants to avoid being uninterruptible (as would be required to guarrantee no failure when flushing the buffer to the device). As userspace already has to handle errors from the SYNC_IOCTL, take advantage of this to be able to restart the syscall across signals. This fixes a coherency issue for i915.ko as well as reducing the uninterruptible hold upon its BKL, the struct_mutex. Fixes commit c11e391da2a8fe973c3c2398452000bed505851e Author: Daniel Vetter <daniel.vetter(a)ffwll.ch> Date: Thu Feb 11 20:04:51 2016 -0200 dma-buf: Add ioctls to allow userspace to flush Testcase: igt/gem_concurrent_blit/*dmabuf*interruptible Testcase: igt/prime_mmap_coherency/ioctl-errors Signed-off-by: Chris Wilson <chris(a)chris-wilson.co.uk> Cc: Tiago Vignatti <tiago.vignatti(a)intel.com> Cc: Stéphane Marchesin <marcheu(a)chromium.org> Cc: David Herrmann <dh.herrmann(a)gmail.com> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: Daniel Vetter <daniel.vetter(a)intel.com> CC: linux-media(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: intel-gfx(a)lists.freedesktop.org Cc: devel(a)driverdev.osuosl.org --- drivers/dma-buf/dma-buf.c | 17 +++++++++++------ drivers/gpu/drm/i915/i915_gem_dmabuf.c | 15 +++++---------- drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 5 +++-- drivers/gpu/drm/udl/udl_fb.c | 4 ++-- drivers/staging/android/ion/ion.c | 6 ++++-- include/linux/dma-buf.h | 6 +++--- 6 files changed, 28 insertions(+), 25 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 9810d1df0691..774a60f4309a 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -259,6 +259,7 @@ static long dma_buf_ioctl(struct file *file, struct dma_buf *dmabuf; struct dma_buf_sync sync; enum dma_data_direction direction; + int ret; dmabuf = file->private_data; @@ -285,11 +286,11 @@ static long dma_buf_ioctl(struct file *file, } if (sync.flags & DMA_BUF_SYNC_END) - dma_buf_end_cpu_access(dmabuf, direction); + ret = dma_buf_end_cpu_access(dmabuf, direction); else - dma_buf_begin_cpu_access(dmabuf, direction); + ret = dma_buf_begin_cpu_access(dmabuf, direction); - return 0; + return ret; default: return -ENOTTY; } @@ -613,13 +614,17 @@ EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access); * * This call must always succeed. */ -void dma_buf_end_cpu_access(struct dma_buf *dmabuf, - enum dma_data_direction direction) +int dma_buf_end_cpu_access(struct dma_buf *dmabuf, + enum dma_data_direction direction) { + int ret = 0; + WARN_ON(!dmabuf); if (dmabuf->ops->end_cpu_access) - dmabuf->ops->end_cpu_access(dmabuf, direction); + ret = dmabuf->ops->end_cpu_access(dmabuf, direction); + + return ret; } EXPORT_SYMBOL_GPL(dma_buf_end_cpu_access); diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c index 1f3eef6fb345..0506016e18e0 100644 --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c @@ -228,25 +228,20 @@ static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, enum dma_data_dire return ret; } -static void i915_gem_end_cpu_access(struct dma_buf *dma_buf, enum dma_data_direction direction) +static int i915_gem_end_cpu_access(struct dma_buf *dma_buf, enum dma_data_direction direction) { struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf); struct drm_device *dev = obj->base.dev; - struct drm_i915_private *dev_priv = to_i915(dev); - bool was_interruptible; int ret; - mutex_lock(&dev->struct_mutex); - was_interruptible = dev_priv->mm.interruptible; - dev_priv->mm.interruptible = false; + ret = i915_mutex_lock_interruptible(dev); + if (ret) + return ret; ret = i915_gem_object_set_to_gtt_domain(obj, false); - - dev_priv->mm.interruptible = was_interruptible; mutex_unlock(&dev->struct_mutex); - if (unlikely(ret)) - DRM_ERROR("unable to flush buffer following CPU access; rendering may be corrupt\n"); + return ret; } static const struct dma_buf_ops i915_dmabuf_ops = { diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c index 3cf8aab23a39..af267c35d813 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c +++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c @@ -97,11 +97,12 @@ static int omap_gem_dmabuf_begin_cpu_access(struct dma_buf *buffer, return omap_gem_get_pages(obj, &pages, true); } -static void omap_gem_dmabuf_end_cpu_access(struct dma_buf *buffer, - enum dma_data_direction dir) +static int omap_gem_dmabuf_end_cpu_access(struct dma_buf *buffer, + enum dma_data_direction dir) { struct drm_gem_object *obj = buffer->priv; omap_gem_put_pages(obj); + return 0; } diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c index c427499133d6..33239a2b264a 100644 --- a/drivers/gpu/drm/udl/udl_fb.c +++ b/drivers/gpu/drm/udl/udl_fb.c @@ -423,8 +423,8 @@ static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb, } if (ufb->obj->base.import_attach) { - dma_buf_end_cpu_access(ufb->obj->base.import_attach->dmabuf, - DMA_FROM_DEVICE); + ret = dma_buf_end_cpu_access(ufb->obj->base.import_attach->dmabuf, + DMA_FROM_DEVICE); } unlock: diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 0754a37c9674..49436b4510f4 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -1075,14 +1075,16 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, return PTR_ERR_OR_ZERO(vaddr); } -static void ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf, - enum dma_data_direction direction) +static int ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf, + enum dma_data_direction direction) { struct ion_buffer *buffer = dmabuf->priv; mutex_lock(&buffer->lock); ion_buffer_kmap_put(buffer); mutex_unlock(&buffer->lock); + + return 0; } static struct dma_buf_ops dma_buf_ops = { diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 532108ea0c1c..3fe90d494edb 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -94,7 +94,7 @@ struct dma_buf_ops { void (*release)(struct dma_buf *); int (*begin_cpu_access)(struct dma_buf *, enum dma_data_direction); - void (*end_cpu_access)(struct dma_buf *, enum dma_data_direction); + int (*end_cpu_access)(struct dma_buf *, enum dma_data_direction); void *(*kmap_atomic)(struct dma_buf *, unsigned long); void (*kunmap_atomic)(struct dma_buf *, unsigned long, void *); void *(*kmap)(struct dma_buf *, unsigned long); @@ -224,8 +224,8 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct sg_table *, enum dma_data_direction); int dma_buf_begin_cpu_access(struct dma_buf *dma_buf, enum dma_data_direction dir); -void dma_buf_end_cpu_access(struct dma_buf *dma_buf, - enum dma_data_direction dir); +int dma_buf_end_cpu_access(struct dma_buf *dma_buf, + enum dma_data_direction dir); void *dma_buf_kmap_atomic(struct dma_buf *, unsigned long); void dma_buf_kunmap_atomic(struct dma_buf *, unsigned long, void *); void *dma_buf_kmap(struct dma_buf *, unsigned long); -- 2.8.0.rc3

9 years, 5 months

Re: [Linaro-mm-sig] [PATCH] fence: add missing descriptions for fence

by Javier Martinez Canillas

Hello Luis, On Sat, Mar 19, 2016 at 4:50 PM, Luis de Bethencourt <luisbg(a)osg.samsung.com> wrote: > Commit b55b54b5db33 ("staging/android: remove struct sync_pt") > added the members child_list and active_list to the fence struct, but > didn't add descriptions for these. Adding the descriptions. > Patches whose commit message mentions a specific commit that introduced and issue, usually also have a "Fixes:" tag before the S-o-B. For example this patch should have: Fixes: b55b54b5db33 ("staging/android: remove struct sync_pt") > Signed-off-by: Luis de Bethencourt <luisbg(a)osg.samsung.com> > --- > Hi, > > Noticed this missing descriptions when running make htmldocs. > > Got the following warnings: > .//include/linux/fence.h:84: warning: No description found for parameter 'child_list' > .//include/linux/fence.h:84: warning: No description found for parameter 'active_list' > > Thanks :) > Luis > Patch looks good to me. Reviewed-by: Javier Martinez Canillas <javier(a)osg.samsung.com> Best regards, Javier

9 years, 6 months

[PATCH] dma-buf/fence: fix fence_is_later v2

by Alex Deucher

From: Christian König <christian.koenig(a)amd.com> A fence is never later than itself. This caused a bunch of overhead for AMDGPU. v2: simplify check as suggested by Michel. Signed-off-by: Christian König <christian.koenig(a)amd.com> Reviewed-by: Michel Dänzer <michel.daenzer(a)amd.com> Reviewed-by: Alex Deucher <alexander.deucher(a)amd.com> Signed-off-by: Alex Deucher <alexander.deucher(a)amd.com> --- include/linux/fence.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/fence.h b/include/linux/fence.h index bb52201..5aa95eb 100644 --- a/include/linux/fence.h +++ b/include/linux/fence.h @@ -292,7 +292,7 @@ static inline bool fence_is_later(struct fence *f1, struct fence *f2) if (WARN_ON(f1->context != f2->context)) return false; - return f1->seqno - f2->seqno < INT_MAX; + return (int)(f1->seqno - f2->seqno) > 0; } /** -- 2.5.0

9 years, 6 months

[PATCH 0/2] iommu: fix handling of attach/detach for default domains

by Marek Szyprowski

Hello, This patchset fixes attaching and detaching devices to a group, which has default domain configured. With both patches applied all calls to iommu's attach/detach functions will be balanced, whenever caller attaches device to the default domain or the custom one. Best regards Marek Szyprowski Samsung R&D Institute Poland Patch summary: Marek Szyprowski (2): iommu: call detach also for default_domain before attaching to new one iommu: fix default domain handling in __iommu_detach_group() drivers/iommu/iommu.c | 48 +++++++++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 21 deletions(-) -- 1.9.2

9 years, 6 months

[PATCH] dma-buf/fence: fix fence_is_later v2

by Alex Deucher

9 years, 6 months

Re: [Linaro-mm-sig] [PATCH v5 0/3] RFC: Secure Memory Allocation Framework

by Benjamin Gaignard

2015-10-21 16:34 GMT+02:00 James Morris <jmorris(a)namei.org>: > On Wed, 21 Oct 2015, Benjamin Gaignard wrote: > >> >> 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) >> > > Have you addressed all the questions raised by Alan here: > > https://lkml.org/lkml/2015/5/8/629 SMAF create /dev/smaf where all allocations could be done and is the owner of the dmabuf. Secure module is called to check permissions before that the CPU could access to the memory. I hope this cover what Alan expected but I can't speak form him. > > Also, is there any application of this beyond DRM? > If you don't use the secure part you can consider that SMAF is a central allocator with helpers to select the best allocator for your hardware devices. While SMAF doesn't rely on DRM concepts (crypto, CENC, keys etc...) we can use it outside this context but obviously it that been first designed for DRM uses cases. > > - James > -- > James Morris > <jmorris(a)namei.org> > -- Benjamin Gaignard Graphic Working Group Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog

9 years, 11 months

Re: [Linaro-mm-sig] [PATCH v5 1/3] create SMAF module

by Benjamin Gaignard

2015-10-21 16:32 GMT+02:00 James Morris <jmorris(a)namei.org>: > On Wed, 21 Oct 2015, Benjamin Gaignard wrote: > >> 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. >> > > This documentation is highly inadequate. If you give hints I will try to complete it and make it acceptable. > > What does "allocate memory that can be securing" mean? Maybe I could re-phrase it like that: "Secure Memory Allocation Framework goal is to be able to allocate memory that the platform can secure." I'm trying to be generic here because each could have it own method to secure buffers. > > > -- > James Morris > <jmorris(a)namei.org> > -- Benjamin Gaignard Graphic Working Group Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog

9 years, 11 months

[PATCH v5 0/3] RFC: Secure Memory Allocation Framework

by Benjamin Gaignard

version 5 changes: - rebased on kernel 4.3-rc6 - rework locking schema and make handle status use an atomic_t - add a fake secure module to allow performing tests without trusted environment 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 (3): create SMAF module SMAF: add CMA allocator SMAF: add fake secure module drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/smaf/Kconfig | 17 + drivers/smaf/Makefile | 3 + drivers/smaf/smaf-cma.c | 200 +++++++++++ drivers/smaf/smaf-core.c | 753 +++++++++++++++++++++++++++++++++++++++++ drivers/smaf/smaf-fakesecure.c | 92 +++++ include/linux/smaf-allocator.h | 54 +++ include/linux/smaf-secure.h | 75 ++++ include/uapi/linux/smaf.h | 52 +++ 10 files changed, 1249 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 drivers/smaf/smaf-fakesecure.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

9 years, 11 months

Re: [Linaro-mm-sig] [PATCH v4 1/2] create SMAF module

by 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(a)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(a)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

9 years, 11 months

Re: [Linaro-mm-sig] [PATCH v4 0/2] RFC: Secure Memory Allocation Framework

by 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(a)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

9 years, 11 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig