Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

2 participants
2906 discussions

Re: [Linaro-mm-sig] [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

by Laura Abbott

On 03/13/2017 06:21 AM, Mark Brown wrote: > On Mon, Mar 13, 2017 at 10:54:33AM +0000, Brian Starkey wrote: >> On Sun, Mar 12, 2017 at 02:34:14PM +0100, Benjamin Gaignard wrote: > >>> Another point is how can we put secure rules (like selinux policy) on >>> heaps since all the allocations >>> go to the same device (/dev/ion) ? For example, until now, in Android >>> we have to give the same >>> access rights to all the process that use ION. >>> It will become problem when we will add secure heaps because we won't >>> be able to distinguish secure >>> processes to standard ones or set specific policy per heaps. >>> Maybe I'm wrong here but I have never see selinux policy checking an >>> ioctl field but if that >>> exist it could be a solution. > >> I might be thinking of a different type of "secure", but... > >> Should the security of secure heaps be enforced by OS-level >> permissions? I don't know about other architectures, but at least on >> arm/arm64 this is enforced in hardware; it doesn't matter who has >> access to the ion heap, because only secure devices (or the CPU >> running a secure process) is physically able to access the memory >> backing the buffer. > 3 >> In fact, in the use-cases I know of, the process asking for the ion >> allocation is not a secure process, and so we wouldn't *want* to >> restrict the secure heap to be allocated from only by secure >> processes. > > I think there's an orthogonal level of OS level security that can be > applied here - it's reasonable for it to want to say things like "only > processes that are supposed to be implementing functionality X should be > able to try to allocate memory set aside for that functionality". This > mitigates against escallation attacks and so on, it's not really > directly related to secure memory as such though. > Ion also makes it pretty trivial to allocate large amounts of kernel memory and possibly DoS the system. I'd like to have as little policy in Ion as possible but more important would be a general security review and people shouting "bad idea ahead". Thanks, Laura

8 years, 3 months

Re: [Linaro-mm-sig] [RFC PATCH 06/12] staging: android: ion: Remove crufty cache support

by Laura Abbott

On 03/06/2017 09:00 AM, Emil Velikov wrote: > On 6 March 2017 at 10:29, Daniel Vetter <daniel(a)ffwll.ch> wrote: >> On Fri, Mar 03, 2017 at 10:46:03AM -0800, Laura Abbott wrote: >>> On 03/03/2017 08:39 AM, Laurent Pinchart wrote: >>>> Hi Daniel, >>>> >>>> On Friday 03 Mar 2017 10:56:54 Daniel Vetter wrote: >>>>> On Thu, Mar 02, 2017 at 01:44:38PM -0800, Laura Abbott wrote: >>>>>> Now that we call dma_map in the dma_buf API callbacks there is no need >>>>>> to use the existing cache APIs. Remove the sync ioctl and the existing >>>>>> bad dma_sync calls. Explicit caching can be handled with the dma_buf >>>>>> sync API. >>>>>> >>>>>> Signed-off-by: Laura Abbott <labbott(a)redhat.com> >>>>>> --- >>>>>> >>>>>> drivers/staging/android/ion/ion-ioctl.c | 5 ---- >>>>>> drivers/staging/android/ion/ion.c | 40 -------------------- >>>>>> drivers/staging/android/ion/ion_carveout_heap.c | 6 ---- >>>>>> drivers/staging/android/ion/ion_chunk_heap.c | 6 ---- >>>>>> drivers/staging/android/ion/ion_page_pool.c | 3 -- >>>>>> drivers/staging/android/ion/ion_system_heap.c | 5 ---- >>>>>> 6 files changed, 65 deletions(-) >>>>>> >>>>>> diff --git a/drivers/staging/android/ion/ion-ioctl.c >>>>>> b/drivers/staging/android/ion/ion-ioctl.c index 5b2e93f..f820d77 100644 >>>>>> --- a/drivers/staging/android/ion/ion-ioctl.c >>>>>> +++ b/drivers/staging/android/ion/ion-ioctl.c >>>>>> @@ -146,11 +146,6 @@ long ion_ioctl(struct file *filp, unsigned int cmd, >>>>>> unsigned long arg)> >>>>>> data.handle.handle = handle->id; >>>>>> >>>>>> break; >>>>>> >>>>>> } >>>>>> >>>>>> - case ION_IOC_SYNC: >>>>>> - { >>>>>> - ret = ion_sync_for_device(client, data.fd.fd); >>>>>> - break; >>>>>> - } >>>>> >>>>> You missed the case ION_IOC_SYNC: in compat_ion.c. >>>>> >>>>> While at it: Should we also remove the entire custom_ioctl infrastructure? >>>>> It's entirely unused afaict, and for a pure buffer allocator I don't see >>>>> any need to have custom ioctl. >>>> >>>> I second that, if you want to make ion a standard API, then we certainly don't >>>> want any custom ioctl. >>>> >>>>> More code to remove potentially: >>>>> - The entire compat ioctl stuff - would be an abi break, but I guess if we >>>>> pick the 32bit abi and clean up the uapi headers we'll be mostly fine. >>>>> would allow us to remove compat_ion.c entirely. >>>>> >>>>> - ION_IOC_IMPORT: With this ion is purely an allocator, so not sure we >>>>> still need to be able to import anything. All the cache flushing/mapping >>>>> is done through dma-buf ops/ioctls. >>>>> >>>>> >>> >>> Good point to all of the above. I was considering keeping the import around >>> for backwards compatibility reasons but given how much other stuff is being >>> potentially broken, everything should just get ripped out. >> >> If you're ok with breaking the world, then I strongly suggest we go >> through the uapi header and replace all types with the standard >> fixed-width ones (__s32, __s64 and __u32, __u64). Allows us to remove all >> the compat ioctl code :-) > > I think the other comments from your "botching-up ioctls" [1] also apply ;-) > Namely - align structs to multiple of 64bit, add "flags" and properly > verity user input returning -EINVAL. > > -Emil > > [1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Docume… I'm more torn on this. There's a difference between dropping an old ioctl/implicit caching vs. changing an actual ioctl ABI. Maybe having obvious breakage is better than subtle though, plus nobody has come begging me not to break the ABI yet. I might leave this for right before we do the actual move out of staging. Thanks, Laura

8 years, 3 months

Re: [Linaro-mm-sig] [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

by Daniel Vetter

On Mon, Mar 06, 2017 at 08:42:59AM +0100, Michal Hocko wrote: > On Fri 03-03-17 09:37:55, Laura Abbott wrote: > > On 03/03/2017 05:29 AM, Michal Hocko wrote: > > > On Thu 02-03-17 13:44:32, Laura Abbott wrote: > > >> Hi, > > >> > > >> There's been some recent discussions[1] about Ion-like frameworks. There's > > >> apparently interest in just keeping Ion since it works reasonablly well. > > >> This series does what should be the final clean ups for it to possibly be > > >> moved out of staging. > > >> > > >> This includes the following: > > >> - Some general clean up and removal of features that never got a lot of use > > >> as far as I can tell. > > >> - Fixing up the caching. This is the series I proposed back in December[2] > > >> but never heard any feedback on. It will certainly break existing > > >> applications that rely on the implicit caching. I'd rather make an effort > > >> to move to a model that isn't going directly against the establishement > > >> though. > > >> - Fixing up the platform support. The devicetree approach was never well > > >> recieved by DT maintainers. The proposal here is to think of Ion less as > > >> specifying requirements and more of a framework for exposing memory to > > >> userspace. > > >> - CMA allocations now happen without the need of a dummy device structure. > > >> This fixes a bunch of the reasons why I attempted to add devicetree > > >> support before. > > >> > > >> I've had problems getting feedback in the past so if I don't hear any major > > >> objections I'm going to send out with the RFC dropped to be picked up. > > >> The only reason there isn't a patch to come out of staging is to discuss any > > >> other changes to the ABI people might want. Once this comes out of staging, > > >> I really don't want to mess with the ABI. > > > > > > Could you recapitulate concerns preventing the code being merged > > > normally rather than through the staging tree and how they were > > > addressed? > > > > > > > Sorry, I'm really not understanding your question here, can you > > clarify? > > There must have been a reason why this code ended up in the staging > tree, right? So my question is what those reasons were and how they were > handled in order to move the code from the staging subtree. No one gave a thing about android in upstream, so Greg KH just dumped it all into staging/android/. We've discussed ION a bunch of times, recorded anything we'd like to fix in staging/android/TODO, and Laura's patch series here addresses a big chunk of that. This is pretty much the same approach we (gpu folks) used to de-stage the syncpt stuff. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch

8 years, 3 months

Re: [Linaro-mm-sig] [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

by Laura Abbott

On 03/03/2017 05:29 AM, Michal Hocko wrote: > On Thu 02-03-17 13:44:32, Laura Abbott wrote: >> Hi, >> >> There's been some recent discussions[1] about Ion-like frameworks. There's >> apparently interest in just keeping Ion since it works reasonablly well. >> This series does what should be the final clean ups for it to possibly be >> moved out of staging. >> >> This includes the following: >> - Some general clean up and removal of features that never got a lot of use >> as far as I can tell. >> - Fixing up the caching. This is the series I proposed back in December[2] >> but never heard any feedback on. It will certainly break existing >> applications that rely on the implicit caching. I'd rather make an effort >> to move to a model that isn't going directly against the establishement >> though. >> - Fixing up the platform support. The devicetree approach was never well >> recieved by DT maintainers. The proposal here is to think of Ion less as >> specifying requirements and more of a framework for exposing memory to >> userspace. >> - CMA allocations now happen without the need of a dummy device structure. >> This fixes a bunch of the reasons why I attempted to add devicetree >> support before. >> >> I've had problems getting feedback in the past so if I don't hear any major >> objections I'm going to send out with the RFC dropped to be picked up. >> The only reason there isn't a patch to come out of staging is to discuss any >> other changes to the ABI people might want. Once this comes out of staging, >> I really don't want to mess with the ABI. > > Could you recapitulate concerns preventing the code being merged > normally rather than through the staging tree and how they were > addressed? > Sorry, I'm really not understanding your question here, can you clarify? Thanks, Laura

8 years, 4 months

Re: [Linaro-mm-sig] [RFCv3][PATCH 3/5] arm64: Implement ARCH_HAS_FORCE_CACHE

by Laura Abbott

On 02/22/2017 05:01 PM, Chen Feng wrote: > > > On 2017/2/22 3:29, Laura Abbott wrote: >> On 02/20/2017 10:05 PM, Chen Feng wrote: >>> Hi Laura, >>> >>> When we enable kernel v4.4 or newer version on our platform, we meet the issue >>> of flushing cache without reference device. It seems that this patch set is >>> a solution. I'm curious the progress of the discussion. Do you have any plan >>> to fix it in v4.4 and newer kernel verison? >>> >> >> No, I've abandoned this approach based on feedback. The APIs had too much >> potential for incorrect usage. I'm ripping out the implicit caching in Ion >> and switching it to a model where there should always be a device available. >> >> What's your use case where you don't have a device structure? >> > Userspace use ioctl to flush cache for device. > > ion_sync_for_device > dma_sync_sg_for_device(NULL, buffer->sg_table->sgl, > buffer->sg_table->nents, DMA_BIDIRECTIONAL); > > And sys-heap when allocate a zero buffer flush zero data to ddr. > alloc_buffer_page > ion_pages_sync_for_device(NULL, page, PAGE_SIZE << order, > DMA_BIDIRECTIONAL); > > Yes, those calls are being removed. This is what I proposed back in December https://marc.info/?l=linux-kernel&m=148176054902921&w=2 I never heard any feedback on it so I assume everyone was okay with the general direction. I plan on pushing a revised and expanded version of that series once the merge window closes. Thanks, Laura > >> Thanks, >> Laura >> >>> On 2016/9/14 2:41, Laura Abbott wrote: >>>> On 09/13/2016 08:14 AM, Will Deacon wrote: >>>>> On Tue, Sep 13, 2016 at 08:02:20AM -0700, Laura Abbott wrote: >>>>>> On 09/13/2016 02:19 AM, Will Deacon wrote: >>>>>>> On Mon, Sep 12, 2016 at 02:32:56PM -0700, Laura Abbott wrote: >>>>>>>> >>>>>>>> arm64 may need to guarantee the caches are synced. Implement versions of >>>>>>>> the kernel_force_cache API to allow this. >>>>>>>> >>>>>>>> Signed-off-by: Laura Abbott <labbott(a)redhat.com> >>>>>>>> --- >>>>>>>> v3: Switch to calling cache operations directly instead of relying on >>>>>>>> DMA mapping. >>>>>>>> --- >>>>>>>> arch/arm64/include/asm/cacheflush.h | 8 ++++++++ >>>>>>>> arch/arm64/mm/cache.S | 24 ++++++++++++++++++++---- >>>>>>>> arch/arm64/mm/flush.c | 11 +++++++++++ >>>>>>>> 3 files changed, 39 insertions(+), 4 deletions(-) >>>>>>> >>>>>>> I'm really hesitant to expose these cache routines as an API solely to >>>>>>> support a driver sitting in staging/. I appreciate that there's a chicken >>>>>>> and egg problem here, but we *really* don't want people using these routines >>>>>>> in preference to the DMA API, and I fear that we'll simply grow a bunch >>>>>>> more users of these things if we promote it as an API like you're proposing. >>>>>>> >>>>>>> Can the code not be contained under staging/, as part of ion? >>>>>>> >>>>>> >>>>>> I proposed that in V1 and it was suggested I make it a proper API >>>>>> >>>>>> http://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg47654… >>>>>> http://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg47672… >>>>> >>>>> :/ then I guess we're in disagreement. If ion really needs this stuff >>>>> (which I don't fully grok), perhaps we should be exposing something at >>>>> a higher level from the architecture, so it really can't be used for >>>>> anything other than ion. >>>> >>>> I talked/complained about this at a past plumbers. The gist is that Ion >>>> ends up acting as a fake DMA layer for clients. It doesn't match nicely >>>> because clients can allocate both coherent and non-coherent memory. >>>> Trying to use dma_map doesn't work because a) a device for coherency isn't >>>> known at allocation time b) it kills performance. Part of the motivation >>>> for taking this approach is to avoid the need to rework the existing >>>> Android userspace and keep the existing behavior, as terrible as it >>>> is. Having Ion out of staging and not actually usable isn't helpful. >>>> >>>> I'll give this all some more thought and hopefully have one or two more >>>> proposals before Connect/Plumbers. >>>> >>>>> >>>>> Will >>>>> >>>> >>>> Thanks, >>>> Laura >>>> _______________________________________________ >>>> Linaro-mm-sig mailing list >>>> Linaro-mm-sig(a)lists.linaro.org >>>> https://lists.linaro.org/mailman/listinfo/linaro-mm-sig >>> >>> >>> _______________________________________________ >>> linux-arm-kernel mailing list >>> linux-arm-kernel(a)lists.infradead.org >>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >>> >> >> >> . >> > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel(a)lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >

8 years, 4 months

Re: [Linaro-mm-sig] [PATCH] dma-buf: add support for compat ioctl

by Daniel Vetter

On Tue, Feb 21, 2017 at 4:08 PM, Christian König <deathsimple(a)vodafone.de> wrote: > Am 21.02.2017 um 15:55 schrieb Marek Szyprowski: >> >> Dear All, >> >> On 2017-02-21 15:37, Marek Szyprowski wrote: >>> >>> Hi Christian, >>> >>> On 2017-02-21 14:59, Christian König wrote: >>>> >>>> Am 21.02.2017 um 14:21 schrieb Marek Szyprowski: >>>>> >>>>> Add compat ioctl support to dma-buf. This lets one to use >>>>> DMA_BUF_IOCTL_SYNC >>>>> ioctl from 32bit application on 64bit kernel. Data structures for both >>>>> 32 >>>>> and 64bit modes are same, so there is no need for additional >>>>> translation >>>>> layer. >>>> >>>> >>>> Well I might be wrong, but IIRC compat_ioctl was just optional and if >>>> not specified unlocked_ioctl was called instead. >>>> >>>> If that is true your patch wouldn't have any effect at all. >>> >>> >>> Well, then why I got -ENOTTY in the 32bit test app for this ioctl on >>> 64bit ARM64 kernel without this patch? >>> >> >> I've checked in fs/compat_ioctl.c, I see no fallback in >> COMPAT_SYSCALL_DEFINE3, >> so one has to provide compat_ioctl callback to have ioctl working with >> 32bit >> apps. > > > Then my memory cheated on me. > > In this case the patch is Reviewed-by: Christian König > <christian.koenig(a)amd.com>. Since you have commit rights for drm-misc, care to push this to drm-misc-next-fixes pls? Also I think this warrants a cc: stable, clearly an obvious screw-up in creating this api on our side :( So feel free to smash my ack on the patch. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch

8 years, 4 months

Re: [Linaro-mm-sig] [RFCv3][PATCH 3/5] arm64: Implement ARCH_HAS_FORCE_CACHE

by Laura Abbott

On 02/20/2017 10:05 PM, Chen Feng wrote: > Hi Laura, > > When we enable kernel v4.4 or newer version on our platform, we meet the issue > of flushing cache without reference device. It seems that this patch set is > a solution. I'm curious the progress of the discussion. Do you have any plan > to fix it in v4.4 and newer kernel verison? > No, I've abandoned this approach based on feedback. The APIs had too much potential for incorrect usage. I'm ripping out the implicit caching in Ion and switching it to a model where there should always be a device available. What's your use case where you don't have a device structure? Thanks, Laura > On 2016/9/14 2:41, Laura Abbott wrote: >> On 09/13/2016 08:14 AM, Will Deacon wrote: >>> On Tue, Sep 13, 2016 at 08:02:20AM -0700, Laura Abbott wrote: >>>> On 09/13/2016 02:19 AM, Will Deacon wrote: >>>>> On Mon, Sep 12, 2016 at 02:32:56PM -0700, Laura Abbott wrote: >>>>>> >>>>>> arm64 may need to guarantee the caches are synced. Implement versions of >>>>>> the kernel_force_cache API to allow this. >>>>>> >>>>>> Signed-off-by: Laura Abbott <labbott(a)redhat.com> >>>>>> --- >>>>>> v3: Switch to calling cache operations directly instead of relying on >>>>>> DMA mapping. >>>>>> --- >>>>>> arch/arm64/include/asm/cacheflush.h | 8 ++++++++ >>>>>> arch/arm64/mm/cache.S | 24 ++++++++++++++++++++---- >>>>>> arch/arm64/mm/flush.c | 11 +++++++++++ >>>>>> 3 files changed, 39 insertions(+), 4 deletions(-) >>>>> >>>>> I'm really hesitant to expose these cache routines as an API solely to >>>>> support a driver sitting in staging/. I appreciate that there's a chicken >>>>> and egg problem here, but we *really* don't want people using these routines >>>>> in preference to the DMA API, and I fear that we'll simply grow a bunch >>>>> more users of these things if we promote it as an API like you're proposing. >>>>> >>>>> Can the code not be contained under staging/, as part of ion? >>>>> >>>> >>>> I proposed that in V1 and it was suggested I make it a proper API >>>> >>>> http://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg47654… >>>> http://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg47672… >>> >>> :/ then I guess we're in disagreement. If ion really needs this stuff >>> (which I don't fully grok), perhaps we should be exposing something at >>> a higher level from the architecture, so it really can't be used for >>> anything other than ion. >> >> I talked/complained about this at a past plumbers. The gist is that Ion >> ends up acting as a fake DMA layer for clients. It doesn't match nicely >> because clients can allocate both coherent and non-coherent memory. >> Trying to use dma_map doesn't work because a) a device for coherency isn't >> known at allocation time b) it kills performance. Part of the motivation >> for taking this approach is to avoid the need to rework the existing >> Android userspace and keep the existing behavior, as terrible as it >> is. Having Ion out of staging and not actually usable isn't helpful. >> >> I'll give this all some more thought and hopefully have one or two more >> proposals before Connect/Plumbers. >> >>> >>> Will >>> >> >> Thanks, >> Laura >> _______________________________________________ >> Linaro-mm-sig mailing list >> Linaro-mm-sig(a)lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/linaro-mm-sig > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel(a)lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >

8 years, 4 months

Re: [Linaro-mm-sig] [PATCH] dma-buf: add support for compat ioctl

by Marek Szyprowski

Hi Christian, On 2017-02-21 14:59, Christian König wrote: > Am 21.02.2017 um 14:21 schrieb Marek Szyprowski: >> Add compat ioctl support to dma-buf. This lets one to use >> DMA_BUF_IOCTL_SYNC >> ioctl from 32bit application on 64bit kernel. Data structures for >> both 32 >> and 64bit modes are same, so there is no need for additional translation >> layer. > > Well I might be wrong, but IIRC compat_ioctl was just optional and if > not specified unlocked_ioctl was called instead. > > If that is true your patch wouldn't have any effect at all. Well, then why I got -ENOTTY in the 32bit test app for this ioctl on 64bit ARM64 kernel without this patch? Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland

8 years, 4 months

[PATCH] dma-buf: add support for compat ioctl

by Marek Szyprowski

Add compat ioctl support to dma-buf. This lets one to use DMA_BUF_IOCTL_SYNC ioctl from 32bit application on 64bit kernel. Data structures for both 32 and 64bit modes are same, so there is no need for additional translation layer. Signed-off-by: Marek Szyprowski <m.szyprowski(a)samsung.com> --- drivers/dma-buf/dma-buf.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 718f832a5c71..0007b792827b 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -325,6 +325,9 @@ static long dma_buf_ioctl(struct file *file, .llseek = dma_buf_llseek, .poll = dma_buf_poll, .unlocked_ioctl = dma_buf_ioctl, +#ifdef CONFIG_COMPAT + .compat_ioctl = dma_buf_ioctl, +#endif }; /* -- 1.9.1

8 years, 4 months

[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

8 years, 4 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig