| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <bbb5af208d08acfb9c7b45283840be8719c4c4e2.camel@ndufresne.ca>
Date: Tue, 12 Sep 2023 10:50:34 -0400
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: Christian König <christian.koenig@....com>,
Yong Wu <yong.wu@...iatek.com>,
Rob Herring <robh+dt@...nel.org>,
Sumit Semwal <sumit.semwal@...aro.org>,
Matthias Brugger <matthias.bgg@...il.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Benjamin Gaignard <benjamin.gaignard@...labora.com>,
Brian Starkey <Brian.Starkey@....com>,
John Stultz <jstultz@...gle.com>, tjmercier@...gle.com,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org, dri-devel@...ts.freedesktop.org,
linaro-mm-sig@...ts.linaro.org,
linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, jianjiao.zeng@...iatek.com,
kuohong.wang@...iatek.com
Subject: Re: [PATCH 3/9] dma-heap: Provide accessors so that in-kernel
drivers can allocate dmabufs from specific heaps
Le lundi 11 septembre 2023 à 12:13 +0200, Christian König a écrit :
> Am 11.09.23 um 04:30 schrieb Yong Wu:
> > From: John Stultz <jstultz@...gle.com>
> >
> > This allows drivers who don't want to create their own
> > DMA-BUF exporter to be able to allocate DMA-BUFs directly
> > from existing DMA-BUF Heaps.
> >
> > There is some concern that the premise of DMA-BUF heaps is
> > that userland knows better about what type of heap memory
> > is needed for a pipeline, so it would likely be best for
> > drivers to import and fill DMA-BUFs allocated by userland
> > instead of allocating one themselves, but this is still
> > up for debate.
>
> The main design goal of having DMA-heaps in the first place is to avoid
> per driver allocation and this is not necessary because userland know
> better what type of memory it wants.
If the memory is user visible, yes. When I look at the MTK VCODEC changes, this
seems to be used for internal codec state and SHM buffers used to communicate
with firmware.
>
> The background is rather that we generally want to decouple allocation
> from having a device driver connection so that we have better chance
> that multiple devices can work with the same memory.
>
> I once create a prototype which gives userspace a hint which DMA-heap to
> user for which device:
> https://patchwork.kernel.org/project/linux-media/patch/20230123123756.401692-2-christian.koenig@amd.com/
>
> Problem is that I don't really have time to look into it and maintain
> that stuff, but I think from the high level design that is rather the
> general direction we should push at.
>
> Regards,
> Christian.
>
> >
> > Signed-off-by: John Stultz <jstultz@...gle.com>
> > Signed-off-by: T.J. Mercier <tjmercier@...gle.com>
> > Signed-off-by: Yong Wu <yong.wu@...iatek.com>
> > [Yong: Fix the checkpatch alignment warning]
> > ---
> > drivers/dma-buf/dma-heap.c | 60 ++++++++++++++++++++++++++++----------
> > include/linux/dma-heap.h | 25 ++++++++++++++++
> > 2 files changed, 69 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> > index dcc0e38c61fa..908bb30dc864 100644
> > --- a/drivers/dma-buf/dma-heap.c
> > +++ b/drivers/dma-buf/dma-heap.c
> > @@ -53,12 +53,15 @@ static dev_t dma_heap_devt;
> > static struct class *dma_heap_class;
> > static DEFINE_XARRAY_ALLOC(dma_heap_minors);
> >
> > -static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
> > - unsigned int fd_flags,
> > - unsigned int heap_flags)
> > +struct dma_buf *dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
> > + unsigned int fd_flags,
> > + unsigned int heap_flags)
> > {
> > - struct dma_buf *dmabuf;
> > - int fd;
> > + if (fd_flags & ~DMA_HEAP_VALID_FD_FLAGS)
> > + return ERR_PTR(-EINVAL);
> > +
> > + if (heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS)
> > + return ERR_PTR(-EINVAL);
> >
> > /*
> > * Allocations from all heaps have to begin
> > @@ -66,9 +69,20 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
> > */
> > len = PAGE_ALIGN(len);
> > if (!len)
> > - return -EINVAL;
> > + return ERR_PTR(-EINVAL);
> >
> > - dmabuf = heap->ops->allocate(heap, len, fd_flags, heap_flags);
> > + return heap->ops->allocate(heap, len, fd_flags, heap_flags);
> > +}
> > +EXPORT_SYMBOL_GPL(dma_heap_buffer_alloc);
> > +
> > +static int dma_heap_bufferfd_alloc(struct dma_heap *heap, size_t len,
> > + unsigned int fd_flags,
> > + unsigned int heap_flags)
> > +{
> > + struct dma_buf *dmabuf;
> > + int fd;
> > +
> > + dmabuf = dma_heap_buffer_alloc(heap, len, fd_flags, heap_flags);
> > if (IS_ERR(dmabuf))
> > return PTR_ERR(dmabuf);
> >
> > @@ -106,15 +120,9 @@ static long dma_heap_ioctl_allocate(struct file *file, void *data)
> > if (heap_allocation->fd)
> > return -EINVAL;
> >
> > - if (heap_allocation->fd_flags & ~DMA_HEAP_VALID_FD_FLAGS)
> > - return -EINVAL;
> > -
> > - if (heap_allocation->heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS)
> > - return -EINVAL;
> > -
> > - fd = dma_heap_buffer_alloc(heap, heap_allocation->len,
> > - heap_allocation->fd_flags,
> > - heap_allocation->heap_flags);
> > + fd = dma_heap_bufferfd_alloc(heap, heap_allocation->len,
> > + heap_allocation->fd_flags,
> > + heap_allocation->heap_flags);
> > if (fd < 0)
> > return fd;
> >
> > @@ -205,6 +213,7 @@ const char *dma_heap_get_name(struct dma_heap *heap)
> > {
> > return heap->name;
> > }
> > +EXPORT_SYMBOL_GPL(dma_heap_get_name);
> >
> > struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
> > {
> > @@ -290,6 +299,24 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
> > kfree(heap);
> > return err_ret;
> > }
> > +EXPORT_SYMBOL_GPL(dma_heap_add);
> > +
> > +struct dma_heap *dma_heap_find(const char *name)
> > +{
> > + struct dma_heap *h;
> > +
> > + mutex_lock(&heap_list_lock);
> > + list_for_each_entry(h, &heap_list, list) {
> > + if (!strcmp(h->name, name)) {
> > + kref_get(&h->refcount);
> > + mutex_unlock(&heap_list_lock);
> > + return h;
> > + }
> > + }
> > + mutex_unlock(&heap_list_lock);
> > + return NULL;
> > +}
> > +EXPORT_SYMBOL_GPL(dma_heap_find);
> >
> > static void dma_heap_release(struct kref *ref)
> > {
> > @@ -315,6 +342,7 @@ void dma_heap_put(struct dma_heap *h)
> > kref_put(&h->refcount, dma_heap_release);
> > mutex_unlock(&heap_list_lock);
> > }
> > +EXPORT_SYMBOL_GPL(dma_heap_put);
> >
> > static char *dma_heap_devnode(const struct device *dev, umode_t *mode)
> > {
> > diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
> > index f3c678892c5c..59e70f6c7a60 100644
> > --- a/include/linux/dma-heap.h
> > +++ b/include/linux/dma-heap.h
> > @@ -64,10 +64,35 @@ const char *dma_heap_get_name(struct dma_heap *heap);
> > */
> > struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info);
> >
> > +/**
> > + * dma_heap_find - get the heap registered with the specified name
> > + * @name: Name of the DMA-Heap to find
> > + *
> > + * Returns:
> > + * The DMA-Heap with the provided name.
> > + *
> > + * NOTE: DMA-Heaps returned from this function MUST be released using
> > + * dma_heap_put() when the user is done to enable the heap to be unloaded.
> > + */
> > +struct dma_heap *dma_heap_find(const char *name);
> > +
> > /**
> > * dma_heap_put - drops a reference to a dmabuf heap, potentially freeing it
> > * @heap: the heap whose reference count to decrement
> > */
> > void dma_heap_put(struct dma_heap *heap);
> >
> > +/**
> > + * dma_heap_buffer_alloc - Allocate dma-buf from a dma_heap
> > + * @heap: DMA-Heap to allocate from
> > + * @len: size to allocate in bytes
> > + * @fd_flags: flags to set on returned dma-buf fd
> > + * @heap_flags: flags to pass to the dma heap
> > + *
> > + * This is for internal dma-buf allocations only. Free returned buffers with dma_buf_put().
> > + */
> > +struct dma_buf *dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
> > + unsigned int fd_flags,
> > + unsigned int heap_flags);
> > +
> > #endif /* _DMA_HEAPS_H */
>
Powered by blists - more mailing lists