On Mon, Jul 20, 2026 at 05:35:19AM -0700, Alistair Delva wrote:
> On Sat, Jul 18, 2026 at 10:29 AM Michael S. Tsirkin <[email protected]> wrote:
> >
> > On Sun, Jul 12, 2026 at 08:57:26AM +0200, Mauro Carvalho Chehab wrote:
> > > On Thu, 25 Jun 2026 16:18:48 -0400
> > > Brian Daniels <[email protected]> wrote:
> > >
> > > > > > From: Alexandre Courbot <[email protected]>
> > > > > >
> > > > > > virtio_media_driver.c provides the expected driver hooks, and
> > > > > > support
> > > > > > for mmapping and polling.
> > > > > >
> > > > > > Signed-off-by: Alexandre Courbot <[email protected]>
> > > > > > Co-developed-by: Brian Daniels <[email protected]>
> > > > > > Signed-off-by: Brian Daniels <[email protected]>
> > > > > > ---
> > > > > > drivers/media/virtio/virtio_media_driver.c | 959
> > > > > > +++++++++++++++++++++
> > > > > > 1 file changed, 959 insertions(+)
> > > > > > create mode 100644 drivers/media/virtio/virtio_media_driver.c
> > > > > >
> > > > > > diff --git a/drivers/media/virtio/virtio_media_driver.c
> > > > > > b/drivers/media/virtio/virtio_media_driver.c
> > > > > > new file mode 100644
> > > > > > index 000000000..d6363c673
> > > > > > --- /dev/null
> > > > > > +++ b/drivers/media/virtio/virtio_media_driver.c
> > > > > > @@ -0,0 +1,959 @@
> > > > > > +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0+
> > > > > > +
> > > > > > +/*
> > > > > > + * Virtio-media driver.
> > > > > > + *
> > > > > > + * Copyright (c) 2024-2025 Google LLC.
> > > > > > + */
> > > > > > +
> > > > > > +#include <linux/delay.h>
> > > > > > +#include <linux/device.h>
> > > > > > +#include <linux/dev_printk.h>
> > > > > > +#include <linux/mm.h>
> > > > > > +#include <linux/mutex.h>
> > > > > > +#include <linux/scatterlist.h>
> > > > > > +#include <linux/types.h>
> > > > > > +#include <linux/videodev2.h>
> > > > > > +#include <linux/vmalloc.h>
> > > > > > +#include <linux/wait.h>
> > > > > > +#include <linux/workqueue.h>
> > > > > > +#include <linux/module.h>
> > > > > > +#include <linux/moduleparam.h>
> > > > > > +#include <linux/virtio.h>
> > > > > > +#include <linux/virtio_config.h>
> > > > > > +#include <linux/virtio_ids.h>
> > > > > > +
> > > > > > +#include <media/frame_vector.h>
> > > > > > +#include <media/v4l2-dev.h>
> > > > > > +#include <media/v4l2-event.h>
> > > > > > +#include <media/videobuf2-memops.h>
> > > > > > +#include <media/v4l2-device.h>
> > > > > > +#include <media/v4l2-ioctl.h>
> > > > > > +
> > > > > > +#include "protocol.h"
> > > > > > +#include "session.h"
> > > > > > +#include "virtio_media.h"
> > > > > > +
> > > > > > +#define VIRTIO_MEDIA_NUM_EVENT_BUFS 16
> > > > > > +
> > > > > > +/* ID of the SHM region into which MMAP buffer will be mapped. */
> > > > > > +#define VIRTIO_MEDIA_SHM_MMAP 0
> > > > > > +
> > > > > > +/*
> > > > > > + * Name of the driver to expose to user-space.
> > > > > > + *
> > > > > > + * This is configurable because v4l2-compliance has workarounds
> > > > > > specific to
> > > > > > + * some drivers. When proxying these directly from the host, this
> > > > > > allows it to
> > > > > > + * apply them as needed.
> > > > > > + */
> > > > > > +char *virtio_media_driver_name;
> > > > > > +module_param_named(driver_name, virtio_media_driver_name, charp,
> > > > > > 0660);
> > > > >
> > > > >
> > > > > Um. What? Not how it should be handled.
> > > >
> > > > I can remove this module param. I didn't end up using this when
> > > > compliance testing.
> > > > Instead, I patched v4l-utils:
> > > > https://lore.kernel.org/all/[email protected]/
> > > >
> > > > Let me know if you think the v4l-utils patch is a good approach,
> > > > otherwise let
> > > > me know how you'd prefer to address the v4l2-compliance driver-specific
> > > > workounds
> > > > when they're being proxied with virtio-media.
> > >
> > > This kind of discussion should happen on a separate PR for
> > > v4l2-compliance,
> > > c/c to the proper developers and maintainers of it.
> >
> >
> > I wonder how migration can work when guest is tied to host driver model like
> > this.
>
> We haven't given migration much thought yet, but I think it wouldn't
> be so different to GPU, where to do snapshot/migration we have to
> record all initialization / context setup state and replay it against
> the GPU driver on restore (see https://github.com/google/gfxstream).
> Some migrations will be possible, some will not, and the host should
> be able to decide.
>
> Also, Brian mentioned the host device pass-through use case, but we
> also have device implementations on the host that work with camera
> emulators or some other data source like video/webrtc, which will
> support snapshot/migration more easily. This use case will probably
> will see more real-world use and migration will be more relevant.
This is par for the course.
But I'm afraid I wasn't clear enough. This thread mentions
supplying the host driver name to guest userspace
in order to implement "driver specific work arounds".
But given such, for migration to work userspace needs
to be notified about driver change? And what to do about
the things it started after migration but before the
notification?
> > > >
> > > > > > +
> > > > > > +/*
> > > > > > + * Whether USERPTR buffers are allowed.
> > > > > > + *
> > > > > > + * This is disabled by default as USERPTR buffers are dangerous,
> > > > > > but the option
> > > > > > + * is left to enable them if desired.
> > > > > > + */
> > > > > > +bool virtio_media_allow_userptr;
> > > > > > +module_param_named(allow_userptr, virtio_media_allow_userptr,
> > > > > > bool, 0660);
> > > > >
> > > > >
> > > > > is this kind of thing common?
> > >
> > > There is one old media device that has it (saa7134).
> > >
> > > >
> > > > To be honest, I don't really know. I'm also not that familiar with the
> > > > USERPTR
> > > > issues. I see a few references online about their use being discouraged
> > > > due to
> > > > possible race conditions, perhaps that was the original motivation for
> > > > this
> > > > parameter (I'm not the original author of this driver).
> > > >
> > > > I'm open to alternatives, feel free to let me know if you have a
> > > > preference.
> > >
> > > We tend to not implement USERPTR on newer drivers. I suggest you
> > > to place the logic with regards to V4L2_MEMORY_USERPTR on a separate
> > > patch for further discussions.
> > >
> > > >
> > > > > > +
> > > > > > +/**
> > > > > > + * virtio_media_session_alloc - Allocate a new session.
> > > > > > + * @vv: virtio-media device the session belongs to.
> > > > > > + * @id: ID of the session.
> > > > > > + * @nonblocking_dequeue: whether dequeuing of buffers should be
> > > > > > blocking or
> > > > > > + * not.
> > > > > > + *
> > > > > > + * The ``id`` and ``list`` fields must still be set by the caller.
> > > > >
> > > > > still in what sense?
> > > >
> > > > Based on the code below, I'm not so sure that the caller is responsible
> > > > for
> > > > setting these values. They seem to be initialized in the function.
> > > >
> > > > Perhaps Alexandre Courbot (the original author) would know more. Unless
> > > > he
> > > > says otherwise though I'm inclined to remove this comment.
> > > >
> > > > > > + */
> > > > > > +static struct virtio_media_session *
> > > > > > +virtio_media_session_alloc(struct virtio_media *vv, u32 id,
> > > > > > + struct file *file)
> > > > > > +{
> > > > > > + struct virtio_media_session *session;
> > > > > > + int i;
> > > > > > + int ret;
> > > > > > +
> > > > > > + session = kzalloc_obj(*session, GFP_KERNEL);
> > > > > > + if (!session)
> > > > > > + goto err_session;
> > > > > > +
> > > > > > + session->shadow_buf = kzalloc(VIRTIO_SHADOW_BUF_SIZE,
> > > > > > GFP_KERNEL);
> > > > > > + if (!session->shadow_buf)
> > > > > > + goto err_shadow_buf;
> > > > > > +
> > > > > > + ret = sg_alloc_table(&session->command_sgs,
> > > > > > DESC_CHAIN_MAX_LEN,
> > > > > > + GFP_KERNEL);
> > > > > > + if (ret)
> > > > > > + goto err_payload_sgs;
> > > > > > +
> > > > > > + session->id = id;
> > > > > > + session->nonblocking_dequeue = file->f_flags & O_NONBLOCK;
> > > > > > +
> > > > > > + INIT_LIST_HEAD(&session->list);
> > > > > > + v4l2_fh_init(&session->fh, &vv->video_dev);
> > > > > > + virtio_media_session_fh_add(session, file);
> > > > > > +
> > > > > > + for (i = 0; i <= VIRTIO_MEDIA_LAST_QUEUE; i++)
> > > > > > + INIT_LIST_HEAD(&session->queues[i].pending_dqbufs);
> > > > > > + mutex_init(&session->queues_lock);
> > > > > > +
> > > > > > + init_waitqueue_head(&session->dqbuf_wait);
> > > > > > +
> > > > > > + mutex_lock(&vv->sessions_lock);
> > > > > > + list_add_tail(&session->list, &vv->sessions);
> > > > > > + mutex_unlock(&vv->sessions_lock);
> > > > > > +
> > > > > > + return session;
> > > > > > +
> > > > > > +err_payload_sgs:
> > > > > > + kfree(session->shadow_buf);
> > > > > > +err_shadow_buf:
> > > > > > + kfree(session);
> > > > > > +err_session:
> > > > > > + return ERR_PTR(-ENOMEM);
> > > > > > +}
> > > > > > +
> > > > > > +/**
> > > > > > + * virtio_media_session_free - Free all resources of a session.
> > > > > > + * @vv: virtio-media device the session belongs to.
> > > > > > + * @session: session to destroy.
> > > > > > + *
> > > > > > + * All the resources of @sesssion, as well as the backing memory
> > > > > > of @session
> > > > > > + * itself, are freed.
> > > > >
> > > > > why @ here and `` above? And typo in the name.
> > > >
> > > > The `@` here was an attempt to follow the guide here for referencing
> > > > function
> > > > parameters:
> > > > https://docs.kernel.org/doc-guide/kernel-doc.html#highlights-and-cross-references
> > >
> > > Yes. This is part of Linux Kernel kernel-doc markup: when referring to
> > > struct fields, you should use @field (or ``field`` if one wants to place
> > > an
> > > asterisk on it, like ``*field``).
> > >
> > > >
> > > > That being said, I don't believe this file is 100% consistent with
> > > > that. I will
> > > > spend some time cleaning up the comments throughout this patch set to
> > > > get them
> > > > consistent for v5. Thanks!
> > >
> > > Please use it on a consistent way along the driver.
> > >
> > >
> > > Thanks,
> > > Mauro
> >
