Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

26 participants
3083 discussions

[PATCH] staging: android: ion: Restrict cache maintenance to dma mapped memory

by Liam Mark

The ION begin_cpu_access and end_cpu_access functions use the dma_sync_sg_for_cpu and dma_sync_sg_for_device APIs to perform cache maintenance. Currently it is possible to apply cache maintenance, via the begin_cpu_access and end_cpu_access APIs, to ION buffers which are not dma mapped. The dma sync sg APIs should not be called on sg lists which have not been dma mapped as this can result in cache maintenance being applied to the wrong address. If an sg list has not been dma mapped then its dma_address field has not been populated, some dma ops such as the swiotlb_dma_ops ops use the dma_address field to calculate the address onto which to apply cache maintenance. Fix the ION begin_cpu_access and end_cpu_access functions to only apply cache maintenance to buffers which have been dma mapped. Fixes: 2a55e7b5e544 ("staging: android: ion: Call dma_map_sg for syncing and mapping") Signed-off-by: Liam Mark <lmark(a)codeaurora.org> --- drivers/staging/android/ion/ion.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index f480885e346b..e5df5272823d 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -214,6 +214,7 @@ struct ion_dma_buf_attachment { struct device *dev; struct sg_table *table; struct list_head list; + bool dma_mapped; }; static int ion_dma_buf_attach(struct dma_buf *dmabuf, struct device *dev, @@ -235,6 +236,7 @@ static int ion_dma_buf_attach(struct dma_buf *dmabuf, struct device *dev, a->table = table; a->dev = dev; + a->dma_mapped = false; INIT_LIST_HEAD(&a->list); attachment->priv = a; @@ -272,6 +274,7 @@ static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment, direction)) return ERR_PTR(-ENOMEM); + a->dma_mapped = true; return table; } @@ -279,7 +282,10 @@ static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment, struct sg_table *table, enum dma_data_direction direction) { + struct ion_dma_buf_attachment *a = attachment->priv; + dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction); + a->dma_mapped = false; } static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma) @@ -345,8 +351,9 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, mutex_lock(&buffer->lock); list_for_each_entry(a, &buffer->attachments, list) { - dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents, - direction); + if (a->dma_mapped) + dma_sync_sg_for_cpu(a->dev, a->table->sgl, + a->table->nents, direction); } mutex_unlock(&buffer->lock); @@ -367,8 +374,9 @@ static int ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf, mutex_lock(&buffer->lock); list_for_each_entry(a, &buffer->attachments, list) { - dma_sync_sg_for_device(a->dev, a->table->sgl, a->table->nents, - direction); + if (a->dma_mapped) + dma_sync_sg_for_device(a->dev, a->table->sgl, + a->table->nents, direction); } mutex_unlock(&buffer->lock); -- 1.8.5.2 Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

7 years, 8 months

[PATCH v3] staging: android: ion: Zero CMA allocated memory

by Liam Mark

Since commit 204f672255c2 ("staging: android: ion: Use CMA APIs directly") the CMA API is now used directly and therefore the allocated memory is no longer automatically zeroed. Explicitly zero CMA allocated memory to ensure that no data is exposed to userspace. Fixes: 204f672255c2 ("staging: android: ion: Use CMA APIs directly") Signed-off-by: Liam Mark <lmark(a)codeaurora.org> --- Changes in v2: - Clean up the commit message. - Add 'Fixes:' Changes in v3: - Add support for highmem pages drivers/staging/android/ion/ion_cma_heap.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c index 86196ffd2faf..fa3e4b7e0c9f 100644 --- a/drivers/staging/android/ion/ion_cma_heap.c +++ b/drivers/staging/android/ion/ion_cma_heap.c @@ -21,6 +21,7 @@ #include <linux/err.h> #include <linux/cma.h> #include <linux/scatterlist.h> +#include <linux/highmem.h> #include "ion.h" @@ -51,6 +52,22 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer, if (!pages) return -ENOMEM; + if (PageHighMem(pages)) { + unsigned long nr_clear_pages = nr_pages; + struct page *page = pages; + + while (nr_clear_pages > 0) { + void *vaddr = kmap_atomic(page); + + memset(vaddr, 0, PAGE_SIZE); + kunmap_atomic(vaddr); + page++; + nr_clear_pages--; + } + } else { + memset(page_address(pages), 0, size); + } + table = kmalloc(sizeof(*table), GFP_KERNEL); if (!table) goto err; -- 1.8.5.2 Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

7 years, 8 months

[PATCH v2] staging: android: ion: Zero CMA allocated memory

by Liam Mark

Since commit 204f672255c2 ("staging: android: ion: Use CMA APIs directly") the CMA API is now used directly and therefore the allocated memory is no longer automatically zeroed. Explicitly zero CMA allocated memory to ensure that no data is exposed to userspace. Fixes: 204f672255c2 ("staging: android: ion: Use CMA APIs directly") Signed-off-by: Liam Mark <lmark(a)codeaurora.org> --- Changes in v2: - Clean up the commit message. - Add 'Fixes:' drivers/staging/android/ion/ion_cma_heap.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c index 86196ffd2faf..91a98785607a 100644 --- a/drivers/staging/android/ion/ion_cma_heap.c +++ b/drivers/staging/android/ion/ion_cma_heap.c @@ -51,6 +51,8 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer, if (!pages) return -ENOMEM; + memset(page_address(pages), 0, size); + table = kmalloc(sizeof(*table), GFP_KERNEL); if (!table) goto err; -- 1.8.5.2 Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

7 years, 9 months

[PATCH] staging: android: ion: Zero CMA allocated memory

by Liam Mark

Since the CMA API is now used directly the allocated memory is no longer automatically zeroed. Explicitly zero CMA allocated memory to ensure that no data is exposed to userspace. Change-Id: I08e143707a0d31610821a7f16826c262bf3c1999 Signed-off-by: Liam Mark <lmark(a)codeaurora.org> --- drivers/staging/android/ion/ion_cma_heap.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c index 86196ff..91a9878 100644 --- a/drivers/staging/android/ion/ion_cma_heap.c +++ b/drivers/staging/android/ion/ion_cma_heap.c @@ -51,6 +51,8 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer, if (!pages) return -ENOMEM; + memset(page_address(pages), 0, size); + table = kmalloc(sizeof(*table), GFP_KERNEL); if (!table) goto err; -- 1.8.5.2 Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

7 years, 9 months

[PATCH 1/3] dma-buf: make returning the exclusive fence optional

by Christian König

Change reservation_object_get_fences_rcu to make the exclusive fence pointer optional. If not specified the exclusive fence is put into the fence array as well. This is helpful for a couple of cases where we need all fences in a single array. Signed-off-by: Christian König <christian.koenig(a)amd.com> --- drivers/dma-buf/reservation.c | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index b759a569b7b8..461afa9febd4 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -374,8 +374,9 @@ EXPORT_SYMBOL(reservation_object_copy_fences); * @pshared: the array of shared fence ptrs returned (array is krealloc'd to * the required size, and must be freed by caller) * - * RETURNS - * Zero or -errno + * Retrieve all fences from the reservation object. If the pointer for the + * exclusive fence is not specified the fence is put into the array of the + * shared fences as well. Returns either zero or -ENOMEM. */ int reservation_object_get_fences_rcu(struct reservation_object *obj, struct dma_fence **pfence_excl, @@ -389,8 +390,8 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj, do { struct reservation_object_list *fobj; - unsigned seq; - unsigned int i; + unsigned int i, seq; + size_t sz = 0; shared_count = i = 0; @@ -402,9 +403,14 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj, goto unlock; fobj = rcu_dereference(obj->fence); - if (fobj) { + if (fobj) + sz += sizeof(*shared) * fobj->shared_max; + + if (!pfence_excl && fence_excl) + sz += sizeof(*shared); + + if (sz) { struct dma_fence **nshared; - size_t sz = sizeof(*shared) * fobj->shared_max; nshared = krealloc(shared, sz, GFP_NOWAIT | __GFP_NOWARN); @@ -420,13 +426,19 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj, break; } shared = nshared; - shared_count = fobj->shared_count; - + shared_count = fobj ? fobj->shared_count : 0; for (i = 0; i < shared_count; ++i) { shared[i] = rcu_dereference(fobj->shared[i]); if (!dma_fence_get_rcu(shared[i])) break; } + + if (!pfence_excl && fence_excl) { + shared[i] = fence_excl; + fence_excl = NULL; + ++i; + ++shared_count; + } } if (i != shared_count || read_seqcount_retry(&obj->seq, seq)) { @@ -448,7 +460,8 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj, *pshared_count = shared_count; *pshared = shared; - *pfence_excl = fence_excl; + if (pfence_excl) + *pfence_excl = fence_excl; return ret; } -- 2.14.1

7 years, 9 months

[Patch v2 1/2] dma-buf: add some lockdep asserts to the reservation object implementation

by Lucas Stach

This adds lockdep asserts to the reservation functions which state in their documentation that obj->lock must be held. Allows builds with PROVE_LOCKING enabled to check that the locking requirements are met. Signed-off-by: Lucas Stach <l.stach(a)pengutronix.de> --- v2: remove erroneous check from reservation_object_get_excl --- drivers/dma-buf/reservation.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index b44d9d7db347..accd398e2ea6 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -71,6 +71,8 @@ int reservation_object_reserve_shared(struct reservation_object *obj) struct reservation_object_list *fobj, *old; u32 max; + reservation_object_assert_held(obj); + old = reservation_object_get_list(obj); if (old && old->shared_max) { @@ -211,6 +213,8 @@ void reservation_object_add_shared_fence(struct reservation_object *obj, { struct reservation_object_list *old, *fobj = obj->staged; + reservation_object_assert_held(obj); + old = reservation_object_get_list(obj); obj->staged = NULL; @@ -236,6 +240,8 @@ void reservation_object_add_excl_fence(struct reservation_object *obj, struct reservation_object_list *old; u32 i = 0; + reservation_object_assert_held(obj); + old = reservation_object_get_list(obj); if (old) i = old->shared_count; @@ -276,6 +282,8 @@ int reservation_object_copy_fences(struct reservation_object *dst, size_t size; unsigned i; + reservation_object_assert_held(dst); + rcu_read_lock(); src_list = rcu_dereference(src->fence); -- 2.11.0

7 years, 9 months

[PATCH] dma-buf: add some lockdep asserts to the reservation object implementation

by Lucas Stach

This adds lockdep asserts to the reservation functions which state in their documentation that obj->lock must be held. Allows builds with PROVE_LOCKING enabled to check that the locking requirements are met. Signed-off-by: Lucas Stach <l.stach(a)pengutronix.de> --- drivers/dma-buf/reservation.c | 8 ++++++++ include/linux/reservation.h | 2 ++ 2 files changed, 10 insertions(+) diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index b44d9d7db347..accd398e2ea6 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -71,6 +71,8 @@ int reservation_object_reserve_shared(struct reservation_object *obj) struct reservation_object_list *fobj, *old; u32 max; + reservation_object_assert_held(obj); + old = reservation_object_get_list(obj); if (old && old->shared_max) { @@ -211,6 +213,8 @@ void reservation_object_add_shared_fence(struct reservation_object *obj, { struct reservation_object_list *old, *fobj = obj->staged; + reservation_object_assert_held(obj); + old = reservation_object_get_list(obj); obj->staged = NULL; @@ -236,6 +240,8 @@ void reservation_object_add_excl_fence(struct reservation_object *obj, struct reservation_object_list *old; u32 i = 0; + reservation_object_assert_held(obj); + old = reservation_object_get_list(obj); if (old) i = old->shared_count; @@ -276,6 +282,8 @@ int reservation_object_copy_fences(struct reservation_object *dst, size_t size; unsigned i; + reservation_object_assert_held(dst); + rcu_read_lock(); src_list = rcu_dereference(src->fence); diff --git a/include/linux/reservation.h b/include/linux/reservation.h index 21fc84d82d41..55e7318800fd 100644 --- a/include/linux/reservation.h +++ b/include/linux/reservation.h @@ -212,6 +212,8 @@ reservation_object_unlock(struct reservation_object *obj) static inline struct dma_fence * reservation_object_get_excl(struct reservation_object *obj) { + reservation_object_assert_held(obj); + return rcu_dereference_protected(obj->fence_excl, reservation_object_held(obj)); } -- 2.11.0

7 years, 9 months

[PATCH] dma-buf: make returning the exclusive fence optional

by Christian König

7 years, 9 months

Re: [Linaro-mm-sig] [PATCH] staging: android: ion: Fix dma direction for dma_sync_sg_for_cpu/device

by Laura Abbott

On 12/15/2017 12:59 PM, Sushmita Susheelendra wrote: > Use the direction argument passed into begin_cpu_access > and end_cpu_access when calling the dma_sync_sg_for_cpu/device. > The actual cache primitive called depends on the direction > passed in. > Acked-by: Laura Abbott <labbott(a)redhat.com> > Signed-off-by: Sushmita Susheelendra <ssusheel(a)codeaurora.org> > --- > drivers/staging/android/ion/ion.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c > index a7d9b0e..f480885 100644 > --- a/drivers/staging/android/ion/ion.c > +++ b/drivers/staging/android/ion/ion.c > @@ -346,7 +346,7 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, > mutex_lock(&buffer->lock); > list_for_each_entry(a, &buffer->attachments, list) { > dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents, > - DMA_BIDIRECTIONAL); > + direction); > } > mutex_unlock(&buffer->lock); > > @@ -368,7 +368,7 @@ static int ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf, > mutex_lock(&buffer->lock); > list_for_each_entry(a, &buffer->attachments, list) { > dma_sync_sg_for_device(a->dev, a->table->sgl, a->table->nents, > - DMA_BIDIRECTIONAL); > + direction); > } > mutex_unlock(&buffer->lock); > >

7 years, 10 months

Re: [Linaro-mm-sig] [PATCH 0/4] Backported amdgpu ttm deadlock fixes for 4.14

by Christian König

Am 01.12.2017 um 01:23 schrieb Lyude Paul: > I haven't gone to see where it started, but as of late a good number of > pretty nasty deadlock issues have appeared with the kernel. Easy > reproduction recipe on a laptop with i915/amdgpu prime with lockdep enabled: > > DRI_PRIME=1 glxinfo Acked-by: Christian König <christian.koenig(a)amd.com> Thanks for taking care of this, Christian. > > Additionally, some more race conditions exist that I've managed to > trigger with piglit and lockdep enabled after applying these patches: > > ============================= > WARNING: suspicious RCU usage > 4.14.3Lyude-Test+ #2 Not tainted > ----------------------------- > ./include/linux/reservation.h:216 suspicious rcu_dereference_protected() usage! > > other info that might help us debug this: > > rcu_scheduler_active = 2, debug_locks = 1 > 1 lock held by ext_image_dma_b/27451: > #0: (reservation_ww_class_mutex){+.+.}, at: [<ffffffffa034f2ff>] ttm_bo_unref+0x9f/0x3c0 [ttm] > > stack backtrace: > CPU: 0 PID: 27451 Comm: ext_image_dma_b Not tainted 4.14.3Lyude-Test+ #2 > Hardware name: HP HP ZBook 15 G4/8275, BIOS P70 Ver. 01.02 06/09/2017 > Call Trace: > dump_stack+0x8e/0xce > lockdep_rcu_suspicious+0xc5/0x100 > reservation_object_copy_fences+0x292/0x2b0 > ? ttm_bo_unref+0x9f/0x3c0 [ttm] > ttm_bo_unref+0xbd/0x3c0 [ttm] > amdgpu_bo_unref+0x2a/0x50 [amdgpu] > amdgpu_gem_object_free+0x4b/0x50 [amdgpu] > drm_gem_object_free+0x1f/0x40 [drm] > drm_gem_object_put_unlocked+0x40/0xb0 [drm] > drm_gem_object_handle_put_unlocked+0x6c/0xb0 [drm] > drm_gem_object_release_handle+0x51/0x90 [drm] > drm_gem_handle_delete+0x5e/0x90 [drm] > ? drm_gem_handle_create+0x40/0x40 [drm] > drm_gem_close_ioctl+0x20/0x30 [drm] > drm_ioctl_kernel+0x5d/0xb0 [drm] > drm_ioctl+0x2f7/0x3b0 [drm] > ? drm_gem_handle_create+0x40/0x40 [drm] > ? trace_hardirqs_on_caller+0xf4/0x190 > ? trace_hardirqs_on+0xd/0x10 > amdgpu_drm_ioctl+0x4f/0x90 [amdgpu] > do_vfs_ioctl+0x93/0x670 > ? __fget+0x108/0x1f0 > SyS_ioctl+0x79/0x90 > entry_SYSCALL_64_fastpath+0x23/0xc2 > > I've also added the relevant fixes for the issue mentioned above. > > Christian König (3): > drm/ttm: fix ttm_bo_cleanup_refs_or_queue once more > dma-buf: make reservation_object_copy_fences rcu save > drm/amdgpu: reserve root PD while releasing it > > Michel Dänzer (1): > drm/ttm: Always and only destroy bo->ttm_resv in ttm_bo_release_list > > drivers/dma-buf/reservation.c | 56 +++++++++++++++++++++++++--------- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 13 ++++++-- > drivers/gpu/drm/ttm/ttm_bo.c | 43 +++++++++++++------------- > 3 files changed, 74 insertions(+), 38 deletions(-) > > -- > 2.14.3 > > _______________________________________________ > amd-gfx mailing list > amd-gfx(a)lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx

7 years, 10 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig