Date:   Mon, 1 Jul 2019 17:45:19 -0400
From:   Laura Abbott <labbott@...hat.com>
To:     John Stultz <john.stultz@...aro.org>,
        lkml <linux-kernel@...r.kernel.org>
Cc:     Benjamin Gaignard <benjamin.gaignard@...aro.org>,
        Sumit Semwal <sumit.semwal@...aro.org>,
        Liam Mark <lmark@...eaurora.org>,
        Pratik Patel <pratikp@...eaurora.org>,
        Brian Starkey <Brian.Starkey@....com>,
        Vincent Donnefort <Vincent.Donnefort@....com>,
        Sudipto Paul <Sudipto.Paul@....com>,
        "Andrew F . Davis" <afd@...com>,
        Christoph Hellwig <hch@...radead.org>,
        Chenbo Feng <fengc@...gle.com>,
        Alistair Strachan <astrachan@...gle.com>,
        dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v6 0/5] DMA-BUF Heaps (destaging ION)

On 6/24/19 3:49 PM, John Stultz wrote:
> Here is another pass at the dma-buf heaps patchset Andrew and I
> have been working on which tries to destage a fair chunk of ION
> functionality.
> 

I've gotten bogged down with both work and personal tasks
so I haven't had a chance to look too closely but, once again,
I'm happy to see this continue to move forward.

> The patchset implements per-heap devices which can be opened
> directly and then an ioctl is used to allocate a dmabuf from the
> heap.
> 
> The interface is similar, but much simpler then IONs, only
> providing an ALLOC ioctl.
> 
> Also, I've provided relatively simple system and cma heaps.
> 
> I've booted and tested these patches with AOSP on the HiKey960
> using the kernel tree here:
>    https://git.linaro.org/people/john.stultz/android-dev.git/log/?h=dev/dma-buf-heap
> 
> And the userspace changes here:
>    https://android-review.googlesource.com/c/device/linaro/hikey/+/909436
> 
> Compared to ION, this patchset is missing the system-contig,
> carveout and chunk heaps, as I don't have a device that uses
> those, so I'm unable to do much useful validation there.
> Additionally we have no upstream users of chunk or carveout,
> and the system-contig has been deprecated in the common/andoid-*
> kernels, so this should be ok.
> 
> I've also removed the stats accounting for now, since any such
> accounting should be implemented by dma-buf core or the heaps
> themselves.
> 
> 
> New in v6:
> * Number of cleanups and error path fixes suggested by Brian Starkey,
>    many thanks for his close review and suggestions!
> 
> 
> Outstanding concerns:
> * Need to better understand various secure heap implementations.
>    Some concern that heap private flags will be needed, but its
>    also possible that dma-buf heaps can't solve everyone's needs,
>    in which case, a vendor's secure buffer driver can implement
>    their own dma-buf exporter. So I'm not too worried here.
> 

syzbot found a DoS with Ion which I ACKed a fix for.
https://lore.kernel.org/lkml/03763360-a7de-de87-eb90-ba7838143930@I-love.SAKURA.ne.jp/
This series doesn't have the page pooling so that particular bug may
not be applicable but given this is not the first time I've
seen Ion used as a DoS mechanism, it would be good to think about
putting in some basic checks.

Thanks,
Laura

> Thoughts and feedback would be greatly appreciated!
> 
> thanks
> -john
> 
> Cc: Laura Abbott <labbott@...hat.com>
> Cc: Benjamin Gaignard <benjamin.gaignard@...aro.org>
> Cc: Sumit Semwal <sumit.semwal@...aro.org>
> Cc: Liam Mark <lmark@...eaurora.org>
> Cc: Pratik Patel <pratikp@...eaurora.org>
> Cc: Brian Starkey <Brian.Starkey@....com>
> Cc: Vincent Donnefort <Vincent.Donnefort@....com>
> Cc: Sudipto Paul <Sudipto.Paul@....com>
> Cc: Andrew F. Davis <afd@...com>
> Cc: Christoph Hellwig <hch@...radead.org>
> Cc: Chenbo Feng <fengc@...gle.com>
> Cc: Alistair Strachan <astrachan@...gle.com>
> Cc: dri-devel@...ts.freedesktop.org
> 
> Andrew F. Davis (1):
>    dma-buf: Add dma-buf heaps framework
> 
> John Stultz (4):
>    dma-buf: heaps: Add heap helpers
>    dma-buf: heaps: Add system heap to dmabuf heaps
>    dma-buf: heaps: Add CMA heap to dmabuf heaps
>    kselftests: Add dma-heap test
> 
>   MAINTAINERS                                   |  18 ++
>   drivers/dma-buf/Kconfig                       |  10 +
>   drivers/dma-buf/Makefile                      |   2 +
>   drivers/dma-buf/dma-heap.c                    | 249 +++++++++++++++++
>   drivers/dma-buf/heaps/Kconfig                 |  14 +
>   drivers/dma-buf/heaps/Makefile                |   4 +
>   drivers/dma-buf/heaps/cma_heap.c              | 169 +++++++++++
>   drivers/dma-buf/heaps/heap-helpers.c          | 262 ++++++++++++++++++
>   drivers/dma-buf/heaps/heap-helpers.h          |  54 ++++
>   drivers/dma-buf/heaps/system_heap.c           | 121 ++++++++
>   include/linux/dma-heap.h                      |  59 ++++
>   include/uapi/linux/dma-heap.h                 |  55 ++++
>   tools/testing/selftests/dmabuf-heaps/Makefile |   9 +
>   .../selftests/dmabuf-heaps/dmabuf-heap.c      | 234 ++++++++++++++++
>   14 files changed, 1260 insertions(+)
>   create mode 100644 drivers/dma-buf/dma-heap.c
>   create mode 100644 drivers/dma-buf/heaps/Kconfig
>   create mode 100644 drivers/dma-buf/heaps/Makefile
>   create mode 100644 drivers/dma-buf/heaps/cma_heap.c
>   create mode 100644 drivers/dma-buf/heaps/heap-helpers.c
>   create mode 100644 drivers/dma-buf/heaps/heap-helpers.h
>   create mode 100644 drivers/dma-buf/heaps/system_heap.c
>   create mode 100644 include/linux/dma-heap.h
>   create mode 100644 include/uapi/linux/dma-heap.h
>   create mode 100644 tools/testing/selftests/dmabuf-heaps/Makefile
>   create mode 100644 tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
> 

Powered by blists - more mailing lists