Linaro-mm-sig September 2018

linaro-mm-sig@lists.linaro.org

7 participants
33 discussions

[PATCH v4 0/4] locking, drm: Fix ww mutex naming / algorithm inconsistency

by Thomas Hellstrom

This is a small fallout from a work to allow batching WW mutex locks and unlocks. Our Wound-Wait mutexes actually don't use the Wound-Wait algorithm but the Wait-Die algorithm. One could perhaps rename those mutexes tree-wide to "Wait-Die mutexes" or "Deadlock Avoidance mutexes". Another approach suggested here is to implement also the "Wound-Wait" algorithm as a per-WW-class choice, as it has advantages in some cases. See for example http://www.mathcs.emory.edu/~cheung/Courses/554/Syllabus/8-recv+serial/dead… Now Wound-Wait is a preemptive algorithm, and the preemption is implemented using a lazy scheme: If a wounded transaction is about to go to sleep on a contended WW mutex, we return -EDEADLK. That is sufficient for deadlock prevention. Since with WW mutexes we also require the aborted transaction to sleep waiting to lock the WW mutex it was aborted on, this choice also provides a suitable WW mutex to sleep on. If we were to return -EDEADLK on the first WW mutex lock after the transaction was wounded whether the WW mutex was contended or not, the transaction might frequently be restarted without a wait, which is far from optimal. Note also that with the lazy preemption scheme, contrary to Wait-Die there will be no rollbacks on lock contention of locks held by a transaction that has completed its locking sequence. The modeset locks are then changed from Wait-Die to Wound-Wait since the typical locking pattern of those locks very well matches the criterion for a substantial reduction in the number of rollbacks. For reservation objects, the benefit is more unclear at this point and they remain using Wait-Die. Cc: Ingo Molnar <mingo(a)redhat.com> Cc: Jonathan Corbet <corbet(a)lwn.net> Cc: Gustavo Padovan <gustavo(a)padovan.org> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Sean Paul <seanpaul(a)chromium.org> Cc: David Airlie <airlied(a)linux.ie> Cc: Davidlohr Bueso <dave(a)stgolabs.net> Cc: "Paul E. McKenney" <paulmck(a)linux.vnet.ibm.com> Cc: Josh Triplett <josh(a)joshtriplett.org> Cc: Thomas Gleixner <tglx(a)linutronix.de> Cc: Kate Stewart <kstewart(a)linuxfoundation.org> Cc: Philippe Ombredanne <pombredanne(a)nexb.com> Cc: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> Cc: linux-doc(a)vger.kernel.org Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org v2: Updated DEFINE_WW_CLASS() API, minor code- and comment fixes as detailed in each patch. v3: Included cleanup patch from Peter Zijlstra. Documentation fixes and a small correctness fix. v4: Reworked the correctness fix.

5 years, 3 months

[RFC] android: ion: How to properly clean caches for uncached allocations

by Liam Mark

The issue: Currently in ION if you allocate uncached memory it is possible that there are still dirty lines in the cache. And often these dirty lines in the cache are the zeros which were meant to clear out any sensitive kernel data. What this means is that if you allocate uncached memory from ION, and then subsequently write to that buffer (using the uncached mapping you are provided by ION) then the data you have written could be corrupted at some point in the future if a dirty line is evicted from the cache. Also this means there is a potential security issue. If an un-privileged userspace user allocated uncached memory (for example from the system heap) and then if they were to read from that buffer (through the un-cached mapping they are provided by ION), and if some of the zeros which were written to that memory are still in the cache then this un-privileged userspace user could read potentially sensitive kernel data. An unacceptable fix: I have included some sample code which should resolve this issue for the system heap and the cma heap on some architectures, however this code would not be acceptable for upstreaming since it uses hacks to clean the cache. Similar changes would need to be made to carveout heap and chunk heap. I would appreciate some feedback on the proper way for ION to clean the caches for memory it has allocated that is intended for uncached access. I realize that it may be tempting, as a solution to this problem, to simply strip uncached support from ION. I hope that we can try to find a solution which preserves uncached memory support as ION uncached memory is often used (though perhaps not in upstreamed code) in cases such as multimedia use cases where there is no CPU access required, in secure heap allocations, and in some cases where there is minimal CPU access and therefore uncached memory performs better. Signed-off-by: Liam Mark <lmark(a)codeaurora.org> --- drivers/staging/android/ion/ion.c | 16 ++++++++++++++++ drivers/staging/android/ion/ion.h | 5 ++++- drivers/staging/android/ion/ion_cma_heap.c | 3 +++ drivers/staging/android/ion/ion_page_pool.c | 8 ++++++-- drivers/staging/android/ion/ion_system_heap.c | 7 ++++++- 5 files changed, 35 insertions(+), 4 deletions(-) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 57e0d8035b2e..10e967b0a0f4 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -38,6 +38,22 @@ bool ion_buffer_cached(struct ion_buffer *buffer) return !!(buffer->flags & ION_FLAG_CACHED); } +void ion_pages_sync_for_device(struct page *page, size_t size, + enum dma_data_direction dir) +{ + struct scatterlist sg; + struct device dev = {0}; + + /* hack, use dummy device */ + arch_setup_dma_ops(&dev, 0, 0, NULL, false); + + sg_init_table(&sg, 1); + sg_set_page(&sg, page, size, 0); + /* hack, use phys address for dma address */ + sg_dma_address(&sg) = page_to_phys(page); + dma_sync_sg_for_device(&dev, &sg, 1, dir); +} + /* this function should only be called while dev->lock is held */ static void ion_buffer_add(struct ion_device *dev, struct ion_buffer *buffer) diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h index a238f23c9116..227b9928d185 100644 --- a/drivers/staging/android/ion/ion.h +++ b/drivers/staging/android/ion/ion.h @@ -192,6 +192,9 @@ struct ion_heap { */ bool ion_buffer_cached(struct ion_buffer *buffer); +void ion_pages_sync_for_device(struct page *page, size_t size, + enum dma_data_direction dir); + /** * ion_buffer_fault_user_mappings - fault in user mappings of this buffer * @buffer: buffer @@ -333,7 +336,7 @@ struct ion_page_pool { struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order, bool cached); void ion_page_pool_destroy(struct ion_page_pool *pool); -struct page *ion_page_pool_alloc(struct ion_page_pool *pool); +struct page *ion_page_pool_alloc(struct ion_page_pool *pool, bool *from_pool); void ion_page_pool_free(struct ion_page_pool *pool, struct page *page); /** ion_page_pool_shrink - shrinks the size of the memory cached in the pool diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c index 49718c96bf9e..82e80621d114 100644 --- a/drivers/staging/android/ion/ion_cma_heap.c +++ b/drivers/staging/android/ion/ion_cma_heap.c @@ -59,6 +59,9 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer, memset(page_address(pages), 0, size); } + if (!ion_buffer_cached(buffer)) + ion_pages_sync_for_device(pages, size, DMA_BIDIRECTIONAL); + table = kmalloc(sizeof(*table), GFP_KERNEL); if (!table) goto err; diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c index b3017f12835f..169a321778ed 100644 --- a/drivers/staging/android/ion/ion_page_pool.c +++ b/drivers/staging/android/ion/ion_page_pool.c @@ -63,7 +63,7 @@ static struct page *ion_page_pool_remove(struct ion_page_pool *pool, bool high) return page; } -struct page *ion_page_pool_alloc(struct ion_page_pool *pool) +struct page *ion_page_pool_alloc(struct ion_page_pool *pool, bool *from_pool) { struct page *page = NULL; @@ -76,8 +76,12 @@ struct page *ion_page_pool_alloc(struct ion_page_pool *pool) page = ion_page_pool_remove(pool, false); mutex_unlock(&pool->mutex); - if (!page) + if (!page) { page = ion_page_pool_alloc_pages(pool); + *from_pool = false; + } else { + *from_pool = true; + } return page; } diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c index bc19cdd30637..3bb4604e032b 100644 --- a/drivers/staging/android/ion/ion_system_heap.c +++ b/drivers/staging/android/ion/ion_system_heap.c @@ -57,13 +57,18 @@ static struct page *alloc_buffer_page(struct ion_system_heap *heap, bool cached = ion_buffer_cached(buffer); struct ion_page_pool *pool; struct page *page; + bool from_pool; if (!cached) pool = heap->uncached_pools[order_to_index(order)]; else pool = heap->cached_pools[order_to_index(order)]; - page = ion_page_pool_alloc(pool); + page = ion_page_pool_alloc(pool, &from_pool); + + if (!from_pool && !ion_buffer_cached(buffer)) + ion_pages_sync_for_device(page, PAGE_SIZE << order, + DMA_BIDIRECTIONAL); return page; } -- 1.8.5.2 Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

5 years, 4 months

[PATCH] udmabuf: fix error code in map_udmabuf()

by Dan Carpenter

We accidentally forgot to set "ret" on this error path so it means we return NULL instead of an error pointer. The caller checks for NULL and changes it to an error pointer so it doesn't cause an issue at run time. Signed-off-by: Dan Carpenter <dan.carpenter(a)oracle.com> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 9edabce0b8ab..5b44ef226904 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -61,8 +61,10 @@ static struct sg_table *map_udmabuf(struct dma_buf_attachment *at, GFP_KERNEL); if (ret < 0) goto err; - if (!dma_map_sg(at->dev, sg->sgl, sg->nents, direction)) + if (!dma_map_sg(at->dev, sg->sgl, sg->nents, direction)) { + ret = -EINVAL; goto err; + } return sg; err:

5 years, 7 months

Re: [Linaro-mm-sig] [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg

by Greg Kroah-Hartman

On Wed, Sep 12, 2018 at 05:08:52PM +0200, Arnd Bergmann wrote: > The .ioctl and .compat_ioctl file operations have the same prototype so > they can both point to the same function, which works great almost all > the time when all the commands are compatible. > > One exception is the s390 architecture, where a compat pointer is only > 31 bit wide, and converting it into a 64-bit pointer requires calling > compat_ptr(). Most drivers here will ever run in s390, but since we now > have a generic helper for it, it's easy enough to use it consistently. > > I double-checked all these drivers to ensure that all ioctl arguments > are used as pointers or are ignored, but are not interpreted as integer > values. > > Signed-off-by: Arnd Bergmann <arnd(a)arndb.de> Acked-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>

5 years, 7 months

[PATCH v3 1/3] udmabuf: add documentation

by Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> --- include/uapi/linux/udmabuf.h | 51 +++++++++++++++++++++++++++++++++--- Documentation/driver-api/dma-buf.rst | 8 ++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/include/uapi/linux/udmabuf.h b/include/uapi/linux/udmabuf.h index 46b6532ed8..281e2c52f1 100644 --- a/include/uapi/linux/udmabuf.h +++ b/include/uapi/linux/udmabuf.h @@ -5,8 +5,39 @@ #include <linux/types.h> #include <linux/ioctl.h> +/** + * DOC: udmabuf + * + * udmabuf is a device driver which allows userspace to create + * dmabufs. The memory used for these dmabufs must be backed by + * memfd. The memfd must have F_SEAL_SHRINK and it must not have + * F_SEAL_WRITE. + * + * The driver has two ioctls, one to create a dmabuf from a single + * memory block and one to create a dmabuf from a list of memory + * blocks. + * + * UDMABUF_CREATE - _IOW('u', 0x42, udmabuf_create) + * + * UDMABUF_CREATE_LIST - _IOW('u', 0x43, udmabuf_create_list) + */ + +#define UDMABUF_CREATE _IOW('u', 0x42, struct udmabuf_create) +#define UDMABUF_CREATE_LIST _IOW('u', 0x43, struct udmabuf_create_list) + #define UDMABUF_FLAGS_CLOEXEC 0x01 +/** + * struct udmabuf_create - create a dmabuf from a single memory block. + * + * @memfd: The file handle. + * @offset: Start of the buffer (from memfd start). + * Must be page aligned. + * @size: Size of the buffer. Must be rounded to page size. + * + * @flags: + * UDMABUF_FLAGS_CLOEXEC: set CLOEXEC flag for the dmabuf. + */ struct udmabuf_create { __u32 memfd; __u32 flags; @@ -14,6 +45,14 @@ struct udmabuf_create { __u64 size; }; +/** + * struct udmabuf_create_item - one memory block list item. + * + * @memfd: The file handle. + * @offset: Start of the buffer (from memfd start). + * Must be page aligned. + * @size: Size of the buffer. Must be rounded to page size. + */ struct udmabuf_create_item { __u32 memfd; __u32 __pad; @@ -21,13 +60,19 @@ struct udmabuf_create_item { __u64 size; }; +/** + * struct udmabuf_create_list - create a dmabuf from a memory block list. + * + * @count: The number of list elements. + * @list: The memory block list + * + * flags: + * UDMABUF_FLAGS_CLOEXEC: set CLOEXEC flag for the dmabuf. + */ struct udmabuf_create_list { __u32 flags; __u32 count; struct udmabuf_create_item list[]; }; -#define UDMABUF_CREATE _IOW('u', 0x42, struct udmabuf_create) -#define UDMABUF_CREATE_LIST _IOW('u', 0x43, struct udmabuf_create_list) - #endif /* _UAPI_LINUX_UDMABUF_H */ diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst index b541e97c7a..1f62c30a14 100644 --- a/Documentation/driver-api/dma-buf.rst +++ b/Documentation/driver-api/dma-buf.rst @@ -166,3 +166,11 @@ DMA Fence uABI/Sync File .. kernel-doc:: include/linux/sync_file.h :internal: +Userspace DMA Buffer driver +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. kernel-doc:: include/uapi/linux/udmabuf.h + :doc: udmabuf + +.. kernel-doc:: include/uapi/linux/udmabuf.h + :internal: -- 2.9.3

5 years, 7 months

[PATCH v3 3/3] udmabuf: check that flags has no unsupported bits set

by Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> Reported-by: Yann Droneaud <ydroneaud(a)opteya.com> --- drivers/dma-buf/udmabuf.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 964beadd11..acd97670c5 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -128,6 +128,9 @@ static long udmabuf_create(const struct udmabuf_create_list *head, int seals, ret = -EINVAL; u32 i, flags; + if (head->flags & ~UDMABUF_FLAGS_CLOEXEC) + return -EINVAL; + ubuf = kzalloc(sizeof(*ubuf), GFP_KERNEL); if (!ubuf) return -ENOMEM; -- 2.9.3

5 years, 7 months

[PATCH v3 2/3] udmabuf: check that __pad is zero

by Gerd Hoffmann

Reported-by: Yann Droneaud <ydroneaud(a)opteya.com> Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> --- drivers/dma-buf/udmabuf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 9edabce0b8..964beadd11 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -134,6 +134,8 @@ static long udmabuf_create(const struct udmabuf_create_list *head, pglimit = (size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; for (i = 0; i < head->count; i++) { + if (list[i].__pad) + goto err; if (!IS_ALIGNED(list[i].offset, PAGE_SIZE)) goto err; if (!IS_ALIGNED(list[i].size, PAGE_SIZE)) -- 2.9.3

5 years, 7 months

[PATCH v2 11/13] udmabuf: drop WARN_ON() check.

by Gerd Hoffmann

Reported-by: Laurent Pinchart <laurent.pinchart(a)ideasonboard.com> Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> --- drivers/dma-buf/udmabuf.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index b637a8be6d..7a4fd2194d 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -25,9 +25,6 @@ static int udmabuf_vm_fault(struct vm_fault *vmf) struct vm_area_struct *vma = vmf->vma; struct udmabuf *ubuf = vma->vm_private_data; - if (WARN_ON(vmf->pgoff >= ubuf->pagecount)) - return VM_FAULT_SIGBUS; - vmf->page = ubuf->pages[vmf->pgoff]; get_page(vmf->page); return 0; -- 2.9.3

5 years, 7 months

[PATCH v2 13/13] udmabuf: add documentation

by Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> --- include/uapi/linux/udmabuf.h | 50 +++++++++++++++++++++++++++++++++--- Documentation/driver-api/dma-buf.rst | 8 ++++++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/include/uapi/linux/udmabuf.h b/include/uapi/linux/udmabuf.h index 46b6532ed8..f30b37cb5c 100644 --- a/include/uapi/linux/udmabuf.h +++ b/include/uapi/linux/udmabuf.h @@ -5,8 +5,38 @@ #include <linux/types.h> #include <linux/ioctl.h> +/** + * DOC: udmabuf + * + * udmabuf is a device driver which allows userspace create dmabufs. + * The memory used for these dmabufs must be backed by memfd. The + * memfd must have F_SEAL_SHRINK and it must not have F_SEAL_WRITE. + * + * The driver has two ioctls, one to create a dmabuf from a single + * memory block and one to create a dmabuf from a list of memory + * blocks. + * + * UDMABUF_CREATE - _IOW('u', 0x42, udmabuf_create) + * + * UDMABUF_CREATE_LIST - _IOW('u', 0x43, udmabuf_create_list) + */ + +#define UDMABUF_CREATE _IOW('u', 0x42, struct udmabuf_create) +#define UDMABUF_CREATE_LIST _IOW('u', 0x43, struct udmabuf_create_list) + #define UDMABUF_FLAGS_CLOEXEC 0x01 +/** + * struct udmabuf_create - create a dmabuf from a single memory block. + * + * @memfd: The file handle. + * @offset: Start of the buffer (from memfd start). + * Must be page aligned. + * @size: Size of the buffer. Must be rounded to page size. + * + * flags: + * UDMABUF_FLAGS_CLOEXEC: set CLOEXEC flag for the dmabuf. + */ struct udmabuf_create { __u32 memfd; __u32 flags; @@ -14,6 +44,14 @@ struct udmabuf_create { __u64 size; }; +/** + * struct udmabuf_create_item - one memory block list item. + * + * @memfd: The file handle. + * @offset: Start of the buffer (from memfd start). + * Must be page aligned. + * @size: Size of the buffer. Must be rounded to page size. + */ struct udmabuf_create_item { __u32 memfd; __u32 __pad; @@ -21,13 +59,19 @@ struct udmabuf_create_item { __u64 size; }; +/** + * struct udmabuf_create_list - create a dmabuf from a memory block list. + * + * @count: The number of list elements. + * @list: The memory block list + * + * flags: + * UDMABUF_FLAGS_CLOEXEC: set CLOEXEC flag for the dmabuf. + */ struct udmabuf_create_list { __u32 flags; __u32 count; struct udmabuf_create_item list[]; }; -#define UDMABUF_CREATE _IOW('u', 0x42, struct udmabuf_create) -#define UDMABUF_CREATE_LIST _IOW('u', 0x43, struct udmabuf_create_list) - #endif /* _UAPI_LINUX_UDMABUF_H */ diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst index b541e97c7a..1f62c30a14 100644 --- a/Documentation/driver-api/dma-buf.rst +++ b/Documentation/driver-api/dma-buf.rst @@ -166,3 +166,11 @@ DMA Fence uABI/Sync File .. kernel-doc:: include/linux/sync_file.h :internal: +Userspace DMA Buffer driver +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. kernel-doc:: include/uapi/linux/udmabuf.h + :doc: udmabuf + +.. kernel-doc:: include/uapi/linux/udmabuf.h + :internal: -- 2.9.3

5 years, 7 months

[PATCH v2 12/13] udmabuf: use sizeof(variable) instead of sizeof(type)

by Gerd Hoffmann

Reported-by: Laurent Pinchart <laurent.pinchart(a)ideasonboard.com> Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> --- drivers/dma-buf/udmabuf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 7a4fd2194d..92af9b5300 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -128,7 +128,7 @@ static long udmabuf_create(const struct udmabuf_create_list *head, int seals, ret = -EINVAL; u32 i, flags; - ubuf = kzalloc(sizeof(struct udmabuf), GFP_KERNEL); + ubuf = kzalloc(sizeof(*ubuf), GFP_KERNEL); if (!ubuf) return -ENOMEM; @@ -142,7 +142,7 @@ static long udmabuf_create(const struct udmabuf_create_list *head, if (ubuf->pagecount > pglimit) goto err; } - ubuf->pages = kmalloc_array(ubuf->pagecount, sizeof(struct page *), + ubuf->pages = kmalloc_array(ubuf->pagecount, sizeof(*ubuf->pages), GFP_KERNEL); if (!ubuf->pages) { ret = -ENOMEM; @@ -211,7 +211,7 @@ static long udmabuf_ioctl_create(struct file *filp, unsigned long arg) struct udmabuf_create_item list; if (copy_from_user(&create, (void __user *)arg, - sizeof(struct udmabuf_create))) + sizeof(create))) return -EFAULT; head.flags = create.flags; -- 2.9.3

5 years, 7 months

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig September 2018