| 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: <0cdcf83a1ab7c199244d4a00157f6fa8979ef819.camel@crapouillou.net>
Date: Sat, 15 Jun 2024 13:07:58 +0200
From: Paul Cercueil <paul@...pouillou.net>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Lars-Peter Clausen <lars@...afoo.de>, Vinod Koul <vkoul@...nel.org>,
Sumit Semwal <sumit.semwal@...aro.org>, Christian König
<christian.koenig@....com>, Jonathan Corbet <corbet@....net>, Nuno Sa
<nuno.sa@...log.com>, linux-iio@...r.kernel.org,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
dmaengine@...r.kernel.org, linux-media@...r.kernel.org,
dri-devel@...ts.freedesktop.org, linaro-mm-sig@...ts.linaro.org
Subject: Re: [PATCH v10 3/6] iio: core: Add new DMABUF interface
infrastructure
Le dimanche 09 juin 2024 à 10:53 +0100, Jonathan Cameron a écrit :
> On Wed, 5 Jun 2024 13:08:42 +0200
> Paul Cercueil <paul@...pouillou.net> wrote:
>
> > Add the necessary infrastructure to the IIO core to support a new
> > optional DMABUF based interface.
> >
> > With this new interface, DMABUF objects (externally created) can be
> > attached to a IIO buffer, and subsequently used for data transfer.
> >
> > A userspace application can then use this interface to share DMABUF
> > objects between several interfaces, allowing it to transfer data in
> > a
> > zero-copy fashion, for instance between IIO and the USB stack.
> >
> > The userspace application can also memory-map the DMABUF objects,
> > and
> > access the sample data directly. The advantage of doing this vs.
> > the
> > read() interface is that it avoids an extra copy of the data
> > between the
> > kernel and userspace. This is particularly userful for high-speed
> > devices which produce several megabytes or even gigabytes of data
> > per
> > second.
> >
> > As part of the interface, 3 new IOCTLs have been added:
> >
> > IIO_BUFFER_DMABUF_ATTACH_IOCTL(int fd):
> > Attach the DMABUF object identified by the given file descriptor
> > to the
> > buffer.
> >
> > IIO_BUFFER_DMABUF_DETACH_IOCTL(int fd):
> > Detach the DMABUF object identified by the given file descriptor
> > from
> > the buffer. Note that closing the IIO buffer's file descriptor
> > will
> > automatically detach all previously attached DMABUF objects.
> >
> > IIO_BUFFER_DMABUF_ENQUEUE_IOCTL(struct iio_dmabuf *):
> > Request a data transfer to/from the given DMABUF object. Its file
> > descriptor, as well as the transfer size and flags are provided in
> > the
> > "iio_dmabuf" structure.
> >
> > These three IOCTLs have to be performed on the IIO buffer's file
> > descriptor, obtained using the IIO_BUFFER_GET_FD_IOCTL() ioctl.
> >
> > Signed-off-by: Paul Cercueil <paul@...pouillou.net>
> > Signed-off-by: Nuno Sa <nuno.sa@...log.com>
>
> Need a brief note on the sign off chain.
> What is Nuno's role in this series as he's not sending the emails and
> not
> marked with Co-developed-by
That's a good question. I think he sent one revision of the patchset
(v7 or something like that) so he added his SoB.
(Nuno: you confirm?)
I'll add his Co-developed-by then.
Cheers,
-Paul
> I gave this a much more thorough look in earlier versions than I have
> today but
> a few really minor things inline (that I might have fixed up whilst
> applying)
> but looks like you'll be done a v11 for Randy's docs comments anyway
> :(
>
> Jonathan
>
>
> > diff --git a/drivers/iio/industrialio-buffer.c
> > b/drivers/iio/industrialio-buffer.c
> > index 0138b21b244f..c98c8ac83785 100644
> > --- a/drivers/iio/industrialio-buffer.c
> > +++ b/drivers/iio/industrialio-buffer.c
>
> > +struct iio_dmabuf_priv {
> > + struct list_head entry;
> > + struct kref ref;
> > +
> > + struct iio_buffer *buffer;
> > + struct iio_dma_buffer_block *block;
> > +
> > + u64 context;
> > + spinlock_t lock;
>
> Given you are going to have a v11, please add a comment to this lock
> to say what data it is protecting.
>
> > +
> > + struct dma_buf_attachment *attach;
> > + struct sg_table *sgt;
> > + enum dma_data_direction dir;
> > + atomic_t seqno;
> > +};
>
>
> > diff --git a/include/linux/iio/buffer_impl.h
> > b/include/linux/iio/buffer_impl.h
> > index 89c3fd7c29ca..1a221c1d7736 100644
> > --- a/include/linux/iio/buffer_impl.h
> > +++ b/include/linux/iio/buffer_impl.h
> > @@ -9,8 +9,12 @@
> > #include <uapi/linux/iio/buffer.h>
> > #include <linux/iio/buffer.h>
> >
> > +struct dma_buf_attachment;
> > +struct dma_fence;
> > struct iio_dev;
> > +struct iio_dma_buffer_block;
> > struct iio_buffer;
> > +struct sg_table;
> >
> > /**
> > * INDIO_BUFFER_FLAG_FIXED_WATERMARK - Watermark level of the
> > buffer can not be
> > @@ -39,6 +43,13 @@ struct iio_buffer;
> > * device stops sampling. Calles are balanced
> > with @enable.
> > * @release: called when the last reference to the
> > buffer is dropped,
> > * should free all resources allocated by the
> > buffer.
> > + * @attach_dmabuf: called from userspace via ioctl to attach
> > one external
> > + * DMABUF.
> > + * @detach_dmabuf: called from userspace via ioctl to detach
> > one previously
> > + * attached DMABUF.
> > + * @enqueue_dmabuf: called from userspace via ioctl to queue
> > this DMABUF
> > + * object to this buffer. Requires a valid
> > DMABUF fd, that
> > + * was previouly attached to this buffer.
>
> Missing docs for lock_queue() and unlock_queue()
>
> Kernel-doc must be complete or bots are going to moan at us :(
>
> > * @modes: Supported operating modes by this buffer
> > type
> > * @flags: A bitmask combination of
> > INDIO_BUFFER_FLAG_*
> > *
> > @@ -68,6 +79,17 @@ struct iio_buffer_access_funcs {
> >
> > void (*release)(struct iio_buffer *buffer);
> >
> > + struct iio_dma_buffer_block * (*attach_dmabuf)(struct
> > iio_buffer *buffer,
> > + struct
> > dma_buf_attachment *attach);
> > + void (*detach_dmabuf)(struct iio_buffer *buffer,
> > + struct iio_dma_buffer_block *block);
> > + int (*enqueue_dmabuf)(struct iio_buffer *buffer,
> > + struct iio_dma_buffer_block *block,
> > + struct dma_fence *fence, struct
> > sg_table *sgt,
> > + size_t size, bool cyclic);
> > + void (*lock_queue)(struct iio_buffer *buffer);
> > + void (*unlock_queue)(struct iio_buffer *buffer);
> > +
>
>
Powered by blists - more mailing lists