Linaro-mm-sig October 2015

linaro-mm-sig@lists.linaro.org

1 participants
8 discussions

[PATCH] allow buffers allocation with DMABUF memory type

by Benjamin Gaignard

Until now the only way to make the driver allocate buffers and share them using dma_buf was to use V4L2_MEMORY_MMAP memory type. Use of MMAP memory type is a problem because vb2 never call dma_buf_map_attachment() to attach itself while queuing the buffer so dma_buf importer will not know that another device use this buffer. This patch allow to allocate buffer even for DMABUF memory type and correctly manage dma_buf buffer attachment. vb2_mem_ops attach_dmabuf() prototype has been changed to be able to distinguish if the attachment is done on a already existing buffer or on imported one. Signed-off-by: Benjamin Gaignard <benjamin.gaignard(a)linaro.org> --- drivers/media/v4l2-core/videobuf2-core.c | 76 ++++++++++++++++++++++++-- drivers/media/v4l2-core/videobuf2-dma-contig.c | 10 ++-- drivers/media/v4l2-core/videobuf2-dma-sg.c | 8 ++- drivers/media/v4l2-core/videobuf2-vmalloc.c | 8 ++- include/media/videobuf2-core.h | 5 +- 5 files changed, 91 insertions(+), 16 deletions(-) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 1329dcc..c5968aa 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -337,6 +337,67 @@ static void __setup_offsets(struct vb2_queue *q, unsigned int n) } /** + * __setup_dmabufs() - setup dmabuf fd for every plane in + * every buffer on the queue + */ +static void __setup_dmabufs(struct vb2_queue *q, unsigned int n) +{ + unsigned int buffer, plane; + struct vb2_buffer *vb; + struct vb2_plane *vb_plane; + struct dma_buf *dbuf; + void *mem_priv; + int fd; + int write = !V4L2_TYPE_IS_OUTPUT(q->type); + int flags = write ? O_WRONLY : O_RDONLY; + + for (buffer = q->num_buffers; buffer < q->num_buffers + n; ++buffer) { + vb = q->bufs[buffer]; + if (!vb) + continue; + + for (plane = 0; plane < vb->num_planes; ++plane) { + vb_plane = &vb->planes[plane]; + + dbuf = call_ptr_memop(vb, get_dmabuf, + vb_plane->mem_priv, + flags & O_ACCMODE); + if (IS_ERR_OR_NULL(dbuf)) { + dprintk(1, "Failed to export buffer %d, " \ + "plane %d\n", buffer, plane); + continue; + } + + fd = dma_buf_fd(dbuf, flags & ~O_ACCMODE); + if (fd < 0) { + dprintk(3, "buffer %d, plane %d failed " \ + "to export (%d)\n", buffer, plane, fd); + dma_buf_put(dbuf); + continue; + } + + /* Acquire each plane's memory */ + mem_priv = call_ptr_memop(vb, attach_dmabuf, + q->alloc_ctx[plane], dbuf, + vb->v4l2_planes[plane].length, + write, vb_plane->mem_priv); + if (IS_ERR(mem_priv)) { + dprintk(1, "Buffer %d plane %d failed "\ + "to attach dmabuf\n", buffer, plane); + dma_buf_put(dbuf); + continue; + } + + vb_plane->dbuf = dbuf; + vb->v4l2_planes[plane].m.fd = fd; + + dprintk(3, "Buffer %d, plane %d fd %x\n", + buffer, plane, fd); + } + } +} + +/** * __vb2_queue_alloc() - allocate videobuf buffer structures and (for MMAP type) * video buffer memory for all buffers/planes on the queue and initializes the * queue @@ -369,8 +430,9 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory, vb->v4l2_buf.type = q->type; vb->v4l2_buf.memory = memory; - /* Allocate video buffer memory for the MMAP type */ - if (memory == V4L2_MEMORY_MMAP) { + /* Allocate video buffer memory for the MMAP and DMABUF types */ + if (memory == V4L2_MEMORY_MMAP || + memory == V4L2_MEMORY_DMABUF) { ret = __vb2_buf_mem_alloc(vb); if (ret) { dprintk(1, "failed allocating memory for " @@ -400,6 +462,9 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory, if (memory == V4L2_MEMORY_MMAP) __setup_offsets(q, buffer); + if (memory == V4L2_MEMORY_DMABUF) + __setup_dmabufs(q, buffer); + dprintk(1, "allocated %d buffers, %d plane(s) each\n", buffer, num_planes); @@ -859,7 +924,7 @@ static int __verify_memory_type(struct vb2_queue *q, * to be used during streaming, * 4) allocates internal buffer structures (struct vb2_buffer), according to * the agreed parameters, - * 5) for MMAP memory type, allocates actual video memory, using the + * 5) for MMAP and DMABUF memory types, allocates actual video memory, using the * memory handling/allocation routines provided during queue initialization * * If req->count is 0, all the memory will be freed instead. @@ -885,7 +950,8 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) * are not in use and can be freed. */ mutex_lock(&q->mmap_lock); - if (q->memory == V4L2_MEMORY_MMAP && __buffers_in_use(q)) { + if ((q->memory == V4L2_MEMORY_MMAP || + q->memory == V4L2_MEMORY_DMABUF) && __buffers_in_use(q)) { mutex_unlock(&q->mmap_lock); dprintk(1, "memory in use, cannot free\n"); return -EBUSY; @@ -1540,7 +1606,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b) /* Acquire each plane's memory */ mem_priv = call_ptr_memop(vb, attach_dmabuf, q->alloc_ctx[plane], - dbuf, planes[plane].length, dma_dir); + dbuf, planes[plane].length, dma_dir, NULL); if (IS_ERR(mem_priv)) { dprintk(1, "failed to attach dmabuf\n"); ret = PTR_ERR(mem_priv); diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c index b481d20..8dc945e 100644 --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c @@ -793,7 +793,7 @@ static void vb2_dc_detach_dmabuf(void *mem_priv) struct vb2_dc_buf *buf = mem_priv; /* if vb2 works correctly you should never detach mapped buffer */ - if (WARN_ON(buf->dma_addr)) + if (WARN_ON(buf->dma_sgt)) vb2_dc_unmap_dmabuf(buf); /* detach this attachment */ @@ -802,16 +802,18 @@ static void vb2_dc_detach_dmabuf(void *mem_priv) } static void *vb2_dc_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf, - unsigned long size, enum dma_data_direction dma_dir) + unsigned long size, enum dma_data_direction dma_dir, void *buf_priv) { struct vb2_dc_conf *conf = alloc_ctx; - struct vb2_dc_buf *buf; + struct vb2_dc_buf *buf = buf_priv; struct dma_buf_attachment *dba; if (dbuf->size < size) return ERR_PTR(-EFAULT); - buf = kzalloc(sizeof(*buf), GFP_KERNEL); + if (!buf) + buf = kzalloc(sizeof(*buf), GFP_KERNEL); + if (!buf) return ERR_PTR(-ENOMEM); diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c index b1838ab..b9855fd 100644 --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c @@ -667,16 +667,18 @@ static void vb2_dma_sg_detach_dmabuf(void *mem_priv) } static void *vb2_dma_sg_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf, - unsigned long size, enum dma_data_direction dma_dir) + unsigned long size, enum dma_data_direction dma_dir, void *buf_priv) { struct vb2_dma_sg_conf *conf = alloc_ctx; - struct vb2_dma_sg_buf *buf; + struct vb2_dma_sg_buf *buf = buf_priv; struct dma_buf_attachment *dba; if (dbuf->size < size) return ERR_PTR(-EFAULT); - buf = kzalloc(sizeof(*buf), GFP_KERNEL); + if (!buf) + buf = kzalloc(sizeof(*buf), GFP_KERNEL); + if (!buf) return ERR_PTR(-ENOMEM); diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c b/drivers/media/v4l2-core/videobuf2-vmalloc.c index bcde885..373294c 100644 --- a/drivers/media/v4l2-core/videobuf2-vmalloc.c +++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c @@ -416,14 +416,16 @@ static void vb2_vmalloc_detach_dmabuf(void *mem_priv) } static void *vb2_vmalloc_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf, - unsigned long size, enum dma_data_direction dma_dir) + unsigned long size, enum dma_data_direction dma_dir, void *buf_priv) { - struct vb2_vmalloc_buf *buf; + struct vb2_vmalloc_buf *buf = buf_priv; if (dbuf->size < size) return ERR_PTR(-EFAULT); - buf = kzalloc(sizeof(*buf), GFP_KERNEL); + if (!buf) + buf = kzalloc(sizeof(*buf), GFP_KERNEL); + if (!buf) return ERR_PTR(-ENOMEM); diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index a5790fd..374b26b 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -49,6 +49,8 @@ struct vb2_threadio_data; * used for DMABUF memory types; alloc_ctx is the alloc context * dbuf is the shared dma_buf; returns NULL on failure; * allocator private per-buffer structure on success; + * if private per-buffer structure is provided reuse it + * instead of allocating a new one. * this needs to be used for further accesses to the buffer. * @detach_dmabuf: inform the exporter of the buffer that the current DMABUF * buffer is no longer used; the buf_priv argument is the @@ -98,7 +100,8 @@ struct vb2_mem_ops { void *(*attach_dmabuf)(void *alloc_ctx, struct dma_buf *dbuf, unsigned long size, - enum dma_data_direction dma_dir); + enum dma_data_direction dma_dir, + void *buf_priv); void (*detach_dmabuf)(void *buf_priv); int (*map_dmabuf)(void *buf_priv); void (*unmap_dmabuf)(void *buf_priv); -- 1.9.1

7 years, 8 months

[PATCH v8 00/11] Add DRM for stih4xx platforms

by Benjamin Gaignard

This series of patches add the support of DRM/KMS drivers for STMicroelectronics chipsets stih416 and stih407. version 8: - Allow driver to be build as kernel module. - Use Universal Plane instead of legacy code. patches could be found here: git://git.linaro.org/people/benjamin.gaignard/kernel.git on branch: drm_kms_for_next-v8 version 7: - rebased on top of drm-next (2014-07-24) patches could be found here: git://git.linaro.org/people/benjamin.gaignard/kernel.git on branch: drm_kms_for_next-v7 version 6: - rebased on top of new component framework API. patches could be found here: git://git.linaro.org/people/benjamin.gaignard/kernel.git on branch: drm_kms_for_next-v6 version 5: - Rework sti_drm_drv probes functions to support deferred probe. This allow hdmi to delayed framebuffer creation until I2C is available. - Add ops functions in sti_layer structure to simpify GDP and VID code. - Add NOTES file to describe how the DRM concepts are mapped on hardware. patches could be found here: git://git.linaro.org/people/benjamin.gaignard/kernel.git on branch: drm_kms_for_next-v5 version 4: - Remove depency between TVout it subdevices HDMI and HDA - Rework and simplify VTG and VTAC code - Fix numbers of typo and indentation - Remove debug (will be push in separate patches) - Fix remarks done in previous patcheset patches could be found here: git://git.linaro.org/people/benjamin.gaignard/kernel.git on branch: drm_kms_for_next-v4 version 3: - Correctly split code between probe and bind funtions - Squash some commits - remove HQ-VDP device code to have a smaller patcheset, we will introduce it later. patches could be found here: git://git.linaro.org/people/benjamin.gaignard/kernel.git on branch: drm_kms_for_next-v3 version 2: - Use componentized device instead of register sub-devices in master driver probe function - Fix Makefile and Kconfig to only allow built-in compilation patches could be found here: git://git.linaro.org/people/benjamin.gaignard/kernel.git on branch: drm_kms_for_next-v2 version 1: - First path submission Hardware is split in two main blocks: Compositor and TVout. Each of them includes specific hardware IPs and the display timing are controlled by a specific Video Timing Generator hardware IP (VTG). Compositor is made of the follow hardware IPs: - GDP (Generic Display Pipeline) which is an entry point for graphic (RGB) buffers - VDP (Video Diplay Pipeline) which is an entry point for video (YUV) buffers - HQVDP (High Quality Video Display Processor) that supports scaling, deinterlacing and some miscellaneous image quality improvements. It fetches the Video decoded buffers from memory, processes them and pushes them to the Compositor through a HW dedicated bus. - Mixer is responsible of mixing all the entries depending of their respective z-order and layout TVout is divided in 3 parts: - HDMI to generate HDMI signals, depending of chipset version HDMI phy can change. - HDA to generate signals for HD analog TV - VIP to control/switch data path coming from Compositor On stih416 compositor and Tvout are on different dies so a Video Trafic Advance inter-die Communication mechanism (VTAC) is needed. +---------------------------------------------+ +----------------------------------------+ | +-------------------------------+ +----+ | | +----+ +--------------------------+ | | | | | | | | | | | +---------+ +----+ | | | | +----+ +------+ | | | | | | | | | VIP |---->|HDMI| | | | | |GPD +------------->| | | | | | | | | | | | +----+ | | | | +----+ |Mixer |--|-->| | | | | |---|->| switcher| | | | | | | | | | | | | | | | | +----+ | | | | | | | | | | | | | | | |---->|HDA | | | | | +------+ | |VTAC|========>|VTAC| | +---------+ +----+ | | | | | | | | | | | | | | | | Compositor | | | | | | | | TVout | | | +-------------------------------+ | | | | | | +--------------------------+ | | ^ | | | | | | ^ | | | | | | | | | | | | +--------------+ | | | | | | +-------------+ | | | VTG (master) |----->| | | | | |----->| VTG (slave) | | | +--------------+ +----+ | | +----+ +-------------+ | |Digital die | | Analog Die| +---------------------------------------------+ +----------------------------------------+ On stih407 Compositor and Tvout are on the same die +-----------------------------------------------------------------+ | +-------------------------------+ +--------------------------+ | | | | | +---------+ +----+ | | | | +----+ +------+ | | | VIP |---->|HDMI| | | | | |GPD +------------->| | | | | | +----+ | | | | +----+ |Mixer |--|--|->| switcher| | | | | +----+ +-----+ | | | | | | +----+ | | | | |VDP +-->+HQVDP+--->| | | | | |---->|HDA | | | | | +----+ +-----+ +------+ | | +---------+ +----+ | | | | | | | | | | Compositor | | TVout | | | +-------------------------------+ +--------------------------+ | | ^ ^ | | | | | | +--------------+ | | | VTG | | | +--------------+ | |Digital die | +-----------------------------------------------------------------+ In addition of the drivers for the IPs listed before a thin I2C driver (hdmiddc) is used by HDMI driver to retrieve EDID for monitor. To unify interfaces of GDP and VDP we create a "layer" interface called by compositor to control both GPD and VDP. Hardware have memory contraints (alignment, contiguous) so we use CMA drm helpers functions to allocate frame buffer. File naming convention is: - sti_* for IPs drivers - sti_drm_* for drm functions implementation. Benjamin Gaignard (11): drm: sti: add bindings for DRM driver drm: sti: add VTG driver drm: sti: add VTAC drivers drm: sti: add HDMI driver drm: sti: add HDA driver drm: sti: add TVOut driver drm: sti: add GDP layer drm: sti: add VID layer drm: sti: add Mixer drm: sti: add Compositor drm: sti: Add DRM driver itself .../devicetree/bindings/gpu/st,stih4xx.txt | 189 +++++ drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/sti/Kconfig | 14 + drivers/gpu/drm/sti/Makefile | 21 + drivers/gpu/drm/sti/NOTES | 58 ++ drivers/gpu/drm/sti/sti_compositor.c | 279 +++++++ drivers/gpu/drm/sti/sti_compositor.h | 90 +++ drivers/gpu/drm/sti/sti_drm_crtc.c | 423 +++++++++++ drivers/gpu/drm/sti/sti_drm_crtc.h | 22 + drivers/gpu/drm/sti/sti_drm_drv.c | 241 ++++++ drivers/gpu/drm/sti/sti_drm_drv.h | 29 + drivers/gpu/drm/sti/sti_drm_plane.c | 195 +++++ drivers/gpu/drm/sti/sti_drm_plane.h | 18 + drivers/gpu/drm/sti/sti_gdp.c | 549 ++++++++++++++ drivers/gpu/drm/sti/sti_gdp.h | 16 + drivers/gpu/drm/sti/sti_hda.c | 794 ++++++++++++++++++++ drivers/gpu/drm/sti/sti_hdmi.c | 810 +++++++++++++++++++++ drivers/gpu/drm/sti/sti_hdmi.h | 88 +++ drivers/gpu/drm/sti/sti_hdmi_tx3g0c55phy.c | 336 +++++++++ drivers/gpu/drm/sti/sti_hdmi_tx3g0c55phy.h | 14 + drivers/gpu/drm/sti/sti_hdmi_tx3g4c28phy.c | 211 ++++++ drivers/gpu/drm/sti/sti_hdmi_tx3g4c28phy.h | 14 + drivers/gpu/drm/sti/sti_layer.c | 197 +++++ drivers/gpu/drm/sti/sti_layer.h | 123 ++++ drivers/gpu/drm/sti/sti_mixer.c | 249 +++++++ drivers/gpu/drm/sti/sti_mixer.h | 54 ++ drivers/gpu/drm/sti/sti_tvout.c | 648 +++++++++++++++++ drivers/gpu/drm/sti/sti_vid.c | 138 ++++ drivers/gpu/drm/sti/sti_vid.h | 12 + drivers/gpu/drm/sti/sti_vtac.c | 215 ++++++ drivers/gpu/drm/sti/sti_vtg.c | 366 ++++++++++ drivers/gpu/drm/sti/sti_vtg.h | 28 + 33 files changed, 6444 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpu/st,stih4xx.txt create mode 100644 drivers/gpu/drm/sti/Kconfig create mode 100644 drivers/gpu/drm/sti/Makefile create mode 100644 drivers/gpu/drm/sti/NOTES create mode 100644 drivers/gpu/drm/sti/sti_compositor.c create mode 100644 drivers/gpu/drm/sti/sti_compositor.h create mode 100644 drivers/gpu/drm/sti/sti_drm_crtc.c create mode 100644 drivers/gpu/drm/sti/sti_drm_crtc.h create mode 100644 drivers/gpu/drm/sti/sti_drm_drv.c create mode 100644 drivers/gpu/drm/sti/sti_drm_drv.h create mode 100644 drivers/gpu/drm/sti/sti_drm_plane.c create mode 100644 drivers/gpu/drm/sti/sti_drm_plane.h create mode 100644 drivers/gpu/drm/sti/sti_gdp.c create mode 100644 drivers/gpu/drm/sti/sti_gdp.h create mode 100644 drivers/gpu/drm/sti/sti_hda.c create mode 100644 drivers/gpu/drm/sti/sti_hdmi.c create mode 100644 drivers/gpu/drm/sti/sti_hdmi.h create mode 100644 drivers/gpu/drm/sti/sti_hdmi_tx3g0c55phy.c create mode 100644 drivers/gpu/drm/sti/sti_hdmi_tx3g0c55phy.h create mode 100644 drivers/gpu/drm/sti/sti_hdmi_tx3g4c28phy.c create mode 100644 drivers/gpu/drm/sti/sti_hdmi_tx3g4c28phy.h create mode 100644 drivers/gpu/drm/sti/sti_layer.c create mode 100644 drivers/gpu/drm/sti/sti_layer.h create mode 100644 drivers/gpu/drm/sti/sti_mixer.c create mode 100644 drivers/gpu/drm/sti/sti_mixer.h create mode 100644 drivers/gpu/drm/sti/sti_tvout.c create mode 100644 drivers/gpu/drm/sti/sti_vid.c create mode 100644 drivers/gpu/drm/sti/sti_vid.h create mode 100644 drivers/gpu/drm/sti/sti_vtac.c create mode 100644 drivers/gpu/drm/sti/sti_vtg.c create mode 100644 drivers/gpu/drm/sti/sti_vtg.h -- 1.9.1

8 years, 5 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

8 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

8 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

8 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

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

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

by 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

9 years

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig October 2015