Re: [PATCH v4 2/8] media: virtio: Add virtio-media driver structs and function declarations

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Jul 17, 2026 at 03:46:36PM -0400, Brian Daniels wrote:
> On Mon, Jun 22, 2026 at 5:08 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Mon, Jun 22, 2026 at 04:43:37PM -0400, Brian Daniels wrote:
> > > From: Alexandre Courbot <gnurou@gmail.com>
> > >
> > > Add the structs and function declarations for the new virtio-media drvier.
> > >
> > > Signed-off-by: Alexandre Courbot <gnurou@gmail.com>
> > > Co-developed-by: Brian Daniels <briandaniels@google.com>
> > > Signed-off-by: Brian Daniels <briandaniels@google.com>
> > > ---
> > >  drivers/media/virtio/virtio_media.h | 95 +++++++++++++++++++++++++++++
> > >  1 file changed, 95 insertions(+)
> > >  create mode 100644 drivers/media/virtio/virtio_media.h
> > >
> > > diff --git a/drivers/media/virtio/virtio_media.h b/drivers/media/virtio/virtio_media.h
> > > new file mode 100644
> > > index 000000000..52809d4e9
> > > --- /dev/null
> > > +++ b/drivers/media/virtio/virtio_media.h
> > > @@ -0,0 +1,95 @@
> > > +/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0+ */
> > > +
> > > +/*
> > > + * Virtio-media structures & functions declarations.
> > > + *
> > > + * Copyright (c) 2024-2025 Google LLC.
> > > + */
> > > +
> > > +#ifndef __VIRTIO_MEDIA_H
> > > +#define __VIRTIO_MEDIA_H
> > > +
> > > +#include <linux/virtio_config.h>
> > > +#include <media/v4l2-device.h>
> > > +
> > > +#include "protocol.h"
> > > +
> > > +#define DESC_CHAIN_MAX_LEN SG_MAX_SINGLE_ALLOC
> > > +
> > > +#define VIRTIO_MEDIA_DEFAULT_DRIVER_NAME "virtio-media"
> > > +
> > > +extern char *virtio_media_driver_name;
> > > +extern bool virtio_media_allow_userptr;
> > > +
> > > +/**
> > > + * struct virtio_media - Virtio-media device.
> > > + * @v4l2_dev: v4l2_device for the media device.
> > > + * @video_dev: video_device for the media device.
> > > + * @virtio_dev: virtio device for the media device.
> > > + * @commandq: virtio command queue.
> > > + * @eventq: virtio event queue.
> > > + * @eventq_work: work to run when events are received on @eventq.
> > > + * @mmap_region: region into which MMAP buffers are mapped by the host.
> > > + * @event_buffer: buffer for event descriptors.
> > > + * @sessions: list of active sessions on the device.
> > > + * @sessions_lock: protects @sessions and ``virtio_media_session::list``.
> > > + * @events_lock: prevents concurrent processing of events.
> > > + * @cmd: union of the device commands "open" and "munmap". The other
> > > + *       commands are handled by @struct virtio_media_session
> > > + * @resp: union of responses.to device commands "open" and "munmap". The
> > > + *        other responses are handled by @struct virtio_media_session
> > > + * @vlock: serializes access to the command queue.
> > > + * @wq: waitqueue for host responses on the command queue.
> > > + */
> > > +struct virtio_media {
> > > +     struct v4l2_device v4l2_dev;
> > > +     struct video_device video_dev;
> > > +
> > > +     struct virtio_device *virtio_dev;
> > > +     struct virtqueue *commandq;
> > > +     struct virtqueue *eventq;
> > > +     struct work_struct eventq_work;
> > > +
> > > +     struct virtio_shm_region mmap_region;
> > > +
> > > +     void *event_buffer;
> > > +
> > > +     struct list_head sessions;
> > > +     struct mutex sessions_lock; /* protects sessions list */
> > > +
> > > +     struct mutex events_lock; /* prevents concurrent event processing */
> > > +
> > > +     union {
> > > +             struct virtio_media_cmd_open open;
> > > +             struct virtio_media_cmd_munmap munmap;
> > > +     } cmd;
> > > +
> > > +     union {
> > > +             struct virtio_media_resp_open open;
> > > +             struct virtio_media_resp_munmap munmap;
> > > +     } resp;
> >
> >
> > You need DMA alignment padding for these things.
> 
> Each member of the above unions is already padded to 64-bit alignment,
> is that sufficient?
> 
> If not, could you tell me what else is necessary?

Read up "What memory is DMA'able?" and following chapters in
Documentation/core-api/dma-api-howto.rst


> > Which one can only see when I reads the actual driver 8 patches down.
> > Which is why it's not a sensible way to split patches.
> >
> > A sensible way is to have a driver then add functionality
> > in logical pieces gradually.
> >
> 
> Appreciate the feedback. There was a previous comment requesting to
> split the patch into c/h file pairs, but I can try reformatting it as
> you suggest for v5.
> 
> > > +
> > > +     struct mutex vlock; /* serializes command queue access */
> > > +     wait_queue_head_t wq;
> > > +};
> > > +
> > > +static inline struct virtio_media *
> > > +to_virtio_media(struct video_device *video_dev)
> > > +{
> > > +     return container_of(video_dev, struct virtio_media, video_dev);
> > > +}
> > > +
> > > +/* virtio_media_driver.c */
> > > +
> > > +int virtio_media_send_command(struct virtio_media *vv, struct scatterlist **sgs,
> > > +                           const size_t out_sgs, const size_t in_sgs,
> > > +                           size_t minimum_resp_len, size_t *resp_len);
> > > +void virtio_media_process_events(struct virtio_media *vv);
> > > +
> > > +/* virtio_media_ioctls.c */
> > > +
> > > +long virtio_media_device_ioctl(struct file *file, unsigned int cmd,
> > > +                            unsigned long arg);
> > > +extern const struct v4l2_ioctl_ops virtio_media_ioctl_ops;
> > > +
> > > +#endif // __VIRTIO_MEDIA_H
> > > --
> > > 2.55.0.rc0.799.gd6f94ed593-goog
> >