Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

1 participants
2905 discussions

Re: [Linaro-mm-sig] [PATCH] ion_system_heap: support X86 archtecture

by Laura Abbott

On 9/29/19 3:28 AM, jun.zhang(a)intel.com wrote: > From: zhang jun <jun.zhang(a)intel.com> > > we see tons of warning like: > [ 45.846872] x86/PAT: NDK MediaCodec_:3753 map pfn RAM range req > write-combining for [mem 0x1e7a80000-0x1e7a87fff], got write-back > [ 45.848827] x86/PAT: .vorbis.decoder:4088 map pfn RAM range req > write-combining for [mem 0x1e7a58000-0x1e7a58fff], got write-back > [ 45.848875] x86/PAT: NDK MediaCodec_:3753 map pfn RAM range req > write-combining for [mem 0x1e7a48000-0x1e7a4ffff], got write-back > [ 45.849403] x86/PAT: .vorbis.decoder:4088 map pfn RAM range > req write-combining for [mem 0x1e7a70000-0x1e7a70fff], got write-back > > check the kernel Documentation/x86/pat.txt, it says: > A. Exporting pages to users with remap_pfn_range, io_remap_pfn_range, > vm_insert_pfn > Drivers wanting to export some pages to userspace do it by using > mmap interface and a combination of > 1) pgprot_noncached() > 2) io_remap_pfn_range() or remap_pfn_range() or vm_insert_pfn() > With PAT support, a new API pgprot_writecombine is being added. > So, drivers can continue to use the above sequence, with either > pgprot_noncached() or pgprot_writecombine() in step 1, followed by step 2. > > In addition, step 2 internally tracks the region as UC or WC in > memtype list in order to ensure no conflicting mapping. > > Note that this set of APIs only works with IO (non RAM) regions. > If driver ants to export a RAM region, it has to do set_memory_uc() or > set_memory_wc() as step 0 above and also track the usage of those pages > and use set_memory_wb() before the page is freed to free pool. > > the fix follow the pat document, do set_memory_wc() as step 0 and > use the set_memory_wb() before the page is freed. > All this work needs to be done on the new dma-buf heap rework and I don't think it makes sense to put it on the staging version https://lore.kernel.org/lkml/20190906184712.91980-1-john.stultz@linaro.org/ (I also continue to question the value of uncached buffers, especially on x86) > Signed-off-by: he, bo <bo.he(a)intel.com> > Signed-off-by: zhang jun <jun.zhang(a)intel.com> > Signed-off-by: Bai, Jie A <jie.a.bai(a)intel.com> > --- > drivers/staging/android/ion/ion_system_heap.c | 28 ++++++++++++++++++- > 1 file changed, 27 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c > index b83a1d16bd89..d298b8194820 100644 > --- a/drivers/staging/android/ion/ion_system_heap.c > +++ b/drivers/staging/android/ion/ion_system_heap.c > @@ -13,6 +13,7 @@ > #include <linux/scatterlist.h> > #include <linux/slab.h> > #include <linux/vmalloc.h> > +#include <asm/set_memory.h> > > #include "ion.h" > > @@ -134,6 +135,13 @@ static int ion_system_heap_allocate(struct ion_heap *heap, > sg = table->sgl; > list_for_each_entry_safe(page, tmp_page, &pages, lru) { > sg_set_page(sg, page, page_size(page), 0); > + > +#ifdef CONFIG_X86 > + if (!(buffer->flags & ION_FLAG_CACHED)) > + set_memory_wc((unsigned long)page_address(sg_page(sg)), > + PAGE_ALIGN(sg->length) >> PAGE_SHIFT); > +#endif > + > sg = sg_next(sg); > list_del(&page->lru); > } > @@ -162,8 +170,15 @@ static void ion_system_heap_free(struct ion_buffer *buffer) > if (!(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE)) > ion_heap_buffer_zero(buffer); > > - for_each_sg(table->sgl, sg, table->nents, i) > + for_each_sg(table->sgl, sg, table->nents, i) { > +#ifdef CONFIG_X86 > + if (!(buffer->flags & ION_FLAG_CACHED)) > + set_memory_wb((unsigned long)page_address(sg_page(sg)), > + PAGE_ALIGN(sg->length) >> PAGE_SHIFT); > +#endif > + > free_buffer_page(sys_heap, buffer, sg_page(sg)); > + } > sg_free_table(table); > kfree(table); > } > @@ -316,6 +331,12 @@ static int ion_system_contig_heap_allocate(struct ion_heap *heap, > > buffer->sg_table = table; > > +#ifdef CONFIG_X86 > + if (!(buffer->flags & ION_FLAG_CACHED)) > + set_memory_wc((unsigned long)page_address(page), > + PAGE_ALIGN(len) >> PAGE_SHIFT); > +#endif > + > return 0; > > free_table: > @@ -334,6 +355,11 @@ static void ion_system_contig_heap_free(struct ion_buffer *buffer) > unsigned long pages = PAGE_ALIGN(buffer->size) >> PAGE_SHIFT; > unsigned long i; > > +#ifdef CONFIG_X86 > + if (!(buffer->flags & ION_FLAG_CACHED)) > + set_memory_wb((unsigned long)page_address(page), pages); > +#endif > + > for (i = 0; i < pages; i++) > __free_page(page + i); > sg_free_table(table); >

5 years, 9 months

Re: [Linaro-mm-sig] [PATCH] ion_system_heap: support X86 archtecture

by Greg KH

On Sun, Sep 29, 2019 at 03:28:41PM +0800, jun.zhang(a)intel.com wrote: > From: zhang jun <jun.zhang(a)intel.com> > > we see tons of warning like: > [ 45.846872] x86/PAT: NDK MediaCodec_:3753 map pfn RAM range req > write-combining for [mem 0x1e7a80000-0x1e7a87fff], got write-back > [ 45.848827] x86/PAT: .vorbis.decoder:4088 map pfn RAM range req > write-combining for [mem 0x1e7a58000-0x1e7a58fff], got write-back > [ 45.848875] x86/PAT: NDK MediaCodec_:3753 map pfn RAM range req > write-combining for [mem 0x1e7a48000-0x1e7a4ffff], got write-back > [ 45.849403] x86/PAT: .vorbis.decoder:4088 map pfn RAM range > req write-combining for [mem 0x1e7a70000-0x1e7a70fff], got write-back > > check the kernel Documentation/x86/pat.txt, it says: > A. Exporting pages to users with remap_pfn_range, io_remap_pfn_range, > vm_insert_pfn > Drivers wanting to export some pages to userspace do it by using > mmap interface and a combination of > 1) pgprot_noncached() > 2) io_remap_pfn_range() or remap_pfn_range() or vm_insert_pfn() > With PAT support, a new API pgprot_writecombine is being added. > So, drivers can continue to use the above sequence, with either > pgprot_noncached() or pgprot_writecombine() in step 1, followed by step 2. > > In addition, step 2 internally tracks the region as UC or WC in > memtype list in order to ensure no conflicting mapping. > > Note that this set of APIs only works with IO (non RAM) regions. > If driver ants to export a RAM region, it has to do set_memory_uc() or > set_memory_wc() as step 0 above and also track the usage of those pages > and use set_memory_wb() before the page is freed to free pool. > > the fix follow the pat document, do set_memory_wc() as step 0 and > use the set_memory_wb() before the page is freed. > > Signed-off-by: he, bo <bo.he(a)intel.com> > Signed-off-by: zhang jun <jun.zhang(a)intel.com> > Signed-off-by: Bai, Jie A <jie.a.bai(a)intel.com> > --- > drivers/staging/android/ion/ion_system_heap.c | 28 ++++++++++++++++++- > 1 file changed, 27 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c > index b83a1d16bd89..d298b8194820 100644 > --- a/drivers/staging/android/ion/ion_system_heap.c > +++ b/drivers/staging/android/ion/ion_system_heap.c > @@ -13,6 +13,7 @@ > #include <linux/scatterlist.h> > #include <linux/slab.h> > #include <linux/vmalloc.h> > +#include <asm/set_memory.h> > > #include "ion.h" > > @@ -134,6 +135,13 @@ static int ion_system_heap_allocate(struct ion_heap *heap, > sg = table->sgl; > list_for_each_entry_safe(page, tmp_page, &pages, lru) { > sg_set_page(sg, page, page_size(page), 0); > + > +#ifdef CONFIG_X86 > + if (!(buffer->flags & ION_FLAG_CACHED)) > + set_memory_wc((unsigned long)page_address(sg_page(sg)), > + PAGE_ALIGN(sg->length) >> PAGE_SHIFT); > +#endif There is no way to do this without these #ifdefs? That feels odd, why can't you just always test for this? thanks, greg k-h

5 years, 9 months

Re: [Linaro-mm-sig] [PATCH] dma-buf/resv: fix exclusive fence get

by Chris Wilson

Quoting Qiang Yu (2019-09-22 08:49:00) > This causes kernel crash when testing lima driver. > > Cc: Christian König <christian.koenig(a)amd.com> > Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") > Signed-off-by: Qiang Yu <yuq825(a)gmail.com> > --- > drivers/dma-buf/dma-resv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > index 42a8f3f11681..709002515550 100644 > --- a/drivers/dma-buf/dma-resv.c > +++ b/drivers/dma-buf/dma-resv.c > @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, > if (pfence_excl) > *pfence_excl = fence_excl; > else if (fence_excl) > - shared[++shared_count] = fence_excl; > + shared[shared_count++] = fence_excl; Oops. Reviewed-by: Chris Wilson <chris(a)chris-wilson.co.uk> -Chris

5 years, 9 months

Re: [Linaro-mm-sig] [PATCH] staging: android: ion: Replace strncpy() for stracpy()

by Dan Carpenter

On Sun, Sep 08, 2019 at 02:34:50PM +1000, Adam Zerella wrote: > Using strncpy() does not always terminate the destination string. > stracpy() is a alternative function that does, by using this new > function we will no longer need to insert a null separator. > > Signed-off-by: Adam Zerella <adam.zerella(a)gmail.com> > --- > drivers/staging/android/ion/ion.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c > index e6b1ca141b93..17901bd626be 100644 > --- a/drivers/staging/android/ion/ion.c > +++ b/drivers/staging/android/ion/ion.c > @@ -433,8 +433,7 @@ static int ion_query_heaps(struct ion_heap_query *query) > max_cnt = query->cnt; > > plist_for_each_entry(heap, &dev->heaps, node) { > - strncpy(hdata.name, heap->name, MAX_HEAP_NAME); > - hdata.name[sizeof(hdata.name) - 1] = '\0'; > + stracpy(hdata.name, heap->name, MAX_HEAP_NAME); stracpy() only takes two arguments. This doesn't compile. regards, dan carpenter

5 years, 9 months

Use dma_fence_array for implementing shared dma_resv fences

by Christian König

This is the new dma_fence_array based container for shared fences in the dma_resv object. Advantage of this approach is that you can grab a reference to the current set of shared fences at any time, which allows us to drop the sequence number increment and makes the whole RCU handling much more easier. Disadvantage is that RCU users now have to grab a reference instead of using the sequence counter. As far as I can see i915 was actually the only driver doing this. So we optimize for adding more fences instead of reading them now. Another behavior change worth noting is that the shared fences are now only visible after unlocking the dma_resv object or calling dma_resv_fences_commit() manually. Please review and/or comment, Christian.

5 years, 10 months

Re: [Linaro-mm-sig] [PATCH 08/10] dma-buf/resv: replace shared fence with new fences container

by Chris Wilson

Quoting Christian König (2019-08-21 13:31:45) > @@ -528,20 +352,9 @@ void dma_resv_prune_fences(struct dma_resv *obj) > dma_fence_put(fence); > } > > - list = dma_resv_get_list(obj); > - if (!list) > - return; > - > - for (i = 0; i < list->shared_count; ++i) { > - fence = rcu_dereference_protected(list->shared[i], > - dma_resv_held(obj)); > - > - if (!dma_fence_is_signaled(fence)) > - continue; > - > - RCU_INIT_POINTER(list->shared[i], dma_fence_get_stub()); > - dma_fence_put(fence); > - } > + fence = dma_resv_fences_deref(obj, &obj->readers); > + if (dma_fence_is_signaled(fence)) > + dma_resv_fences_set(obj, &obj->readers, NULL); Something to note is that a dma-fence-array is not automatically signaled and dma_fence_is_signaled() does not check the array. -Chris

5 years, 10 months

[RFC] replacing dma_resv API

by Christian König

Hi everyone, In previous discussion it surfaced that different drivers use the shared and explicit fences in the dma_resv object with different meanings. This is problematic when we share buffers between those drivers and requirements for implicit and explicit synchronization leaded to quite a number of workarounds related to this. So I started an effort to get all drivers back to a common understanding of what the fences in the dma_resv object mean and be able to use the object for different kind of workloads independent of the classic DRM command submission interface. The result is this patch set which modifies the dma_resv API to get away from a single explicit fence and multiple shared fences, towards a notation where we have explicit categories for writers, readers and others. To do this I came up with a new container called dma_resv_fences which can store both a single fence as well as multiple fences in a dma_fence_array. This turned out to actually be even be quite a bit simpler, since we don't need any complicated dance between RCU and sequence count protected updates any more. Instead we can just grab a reference to the dma_fence_array under RCU and so keep the current state of synchronization alive until we are done with it. This results in both a small performance improvement since we don't need so many barriers any more, as well as fewer lines of code in the actual implementation. Please review and/or comment, Christian.

5 years, 10 months

[PATCH 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 6c3ec8fcef01..ca1364102b18 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -131,6 +131,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.18.1

5 years, 10 months

[PATCH 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 9635897458a0..6c3ec8fcef01 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -137,6 +137,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.18.1

5 years, 10 months

[PATCH 1/3] udmabuf: add documentation

by Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> --- include/uapi/linux/udmabuf.h | 52 ++++++++++++++++++++++++++-- Documentation/driver-api/dma-buf.rst | 8 +++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/include/uapi/linux/udmabuf.h b/include/uapi/linux/udmabuf.h index 46b6532ed855..9fe440abf2f9 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,15 @@ struct udmabuf_create { __u64 size; }; +/** + * struct udmabuf_create_item - one memory block list item. + * + * @memfd: The file handle. + * @__pad: Padding field (unused). + * @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 +61,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 b541e97c7ab1..1f62c30a14b0 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.18.1

5 years, 10 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig