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

[Brian Daniels]

On Mon, Jun 22, 2026 at 5:08 PM Michael S. Tsirkin <mst@xxxxxxxxxx> wrote:
>
> On Mon, Jun 22, 2026 at 04:43:37PM -0400, Brian Daniels wrote:
> > From: Alexandre Courbot <gnurou@xxxxxxxxx>
> >
> > Add the structs and function declarations for the new virtio-media drvier.
> >
> > Signed-off-by: Alexandre Courbot <gnurou@xxxxxxxxx>
> > Co-developed-by: Brian Daniels <briandaniels@xxxxxxxxxx>
> > Signed-off-by: Brian Daniels <briandaniels@xxxxxxxxxx>
> > ---
> >  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?

> 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
>