Re: [PATCH v3] media: add virtio-media driver
Em Sat, 12 Apr 2025 13:08:01 +0900 Alexandre Courbot escreveu: > Add the first version of the virtio-media driver. > > This driver acts roughly as a V4L2 relay between user-space and the > virtio virtual device on the host, so it is relatively simple, yet > unconventional. It doesn't use VB2 or other frameworks typically used in > a V4L2 driver, and most of its complexity resides in correctly and > efficiently building the virtio descriptor chain to pass to the host, > avoiding copies whenever possible. This is done by > scatterlist_builder.[ch]. > > virtio_media_ioctls.c proxies each supported ioctl to the host, using > code generated through macros for ioctls that can be forwarded directly, > which is most of them. > > virtio_media_driver.c provides the expected driver hooks, and support > for mmapping and polling. > > This version supports MMAP buffers, while USERPTR buffers can also be > enabled through a driver option. DMABUF support is still pending. > > Signed-off-by: Alexandre Courbot > Signed-off-by: Alexandre Courbot > --- > This patch adds the virtio-media kernel driver. Virtio-media [1] > encapsulates the V4L2 structures and protocol to enable the > virtualization of host media devices into a guest. It's specification is > in the final stages [2] of being merged and the virtualization of > cameras and video accelerator devices has already been demonstrated > using crosvm [3] and QEmu. v4l2-compliance also passes on all tested > devices, which includes the "simple" virtual test device, proxied host > UVC and vivid devices, and the FFmpeg virtual decoder devices (refer to > [3] in order to test these if desired). > > Virtio-media is merged in AOSP [4] and ChromeOS. Upstreaming of the > driver is overdue, but I hope we can start the review process and > converge into something that can be merged. > > Limitations: > > - The driver is currently only available to little-endian, 64-bit > kernels. This is because some of the V4L2 structures used for > communication between guest and host have a layout dependent on the > architecture, and the virtio-media protocol is standardized on the > little-endian 64-bit versions. This can be fixed with a conversion > layer similar to the one used to convert 32-bit ioctls to their 64-bit > counterpart. > - DMABUF support is currently missing. It should be implemented using > virtio objects, with possible support for memfds using the > SHARED_PAGES memory type. > - No support for the media API and requests. While the use-case for > these is less important on virtual devices where we want to present an > abstraction as high as possible to limit VM exits, they do exist and > it would be nice to add behind a virtio feature bit. > - Locking in the driver is still very basic. This is something I want to > improve before merging, but I didn't want to delay upstream review any > further. Hi Alex, I didn't see on a first glance anything that would cause locking issues here, but, as I pointed on my last e-mail, testing with qv4l2 at the max res of my C920 camera, it ended keeping 24 CPUs busy without showing any results with qv4l2 (via ssh at the same machine). So, I suspect that there are issues somewhere, but I didn't debug any further. Please do some tests with a high-res camera, using either ssh or GPU emulation to see how this behaves with real apps. - I did a driver review, but reviewing a new driver with ~4K lines in one row is hard. For the next version, please split this patch on multiple ones (one for each *.c/*.h pair) with the final one adding it to the build system, as this makes easier to review. See my comments below. > > [1] https://github.com/chromeos/virtio-media > [2] > https://lore.kernel.org/virtio-comment/[email protected]/ > [3] https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md > [4] https://android.googlesource.com/platform/external/virtio-media/ > --- > Changes in v3: > - Rebased on top of v6.15-rc1 and removes obsolete control callbacks. > - Link to v2: > https://lore.kernel.org/r/[email protected] > > Changes in v2: > - Fixed kernel test robot and media CI warnings (ignored a few false > positives). > - Changed in-driver email address to personal one since my Google one > will soon become invalid. > - Link to v1: > https://lore.kernel.org/r/[email protected] > --- > MAINTAINERS|6 + > drivers/media/Kconfig | 13 + > drivers/media/Makefile |2 + > drivers/media/virtio/Makefile |9 + > drivers/media/virtio/protocol.h| 288 ++ > drivers/media/virtio/scatterlist_builder.c | 563 > drivers/media/virtio/scatterlist_builder.h | 111 +++ > drivers/media/virtio/session.h | 109 +++ > drivers/media/virtio/virtio_media.h| 93 ++ > drivers/media/vir
Re: [PATCH v3] media: add virtio-media driver
Em Fri, 20 Jun 2025 21:03:34 +0900 "Alexandre Courbot" escreveu: > Hi Mauro, > > Really appreciating the time you are spending reviewing and testing > this! m(__)m Thanks also for sharing your script, I've learned a few > things I didn't know about crosvm. :P I'm resuming the tests with virtio-media. I did some tests here, running camorama and qv4l2, setting the maximum resolution allowed by uvcvideo on a C920 camera, e.g. 2304x1296. Such setup works with camorama, on both host and VM (acessed via ssh with X11 forward), both at the same machine. However, when I run qv4l2, top presents a really high load: 1752251 mchehab 20 0 10,4g 1,6g 1,6g S 1182 2,5 9:21.86 crosvm (It can reach up to 2400% on my machine, which has 24 CPU threads). and the image freezes. The same setup works fine when calling it directly from the host. No idea if this could be some bug at crosvm, or if there are dead locks somewhere. Thanks, Mauro
Re: [PATCH v3] media: add virtio-media driver
Hi Mauro,
On Fri, Jul 25, 2025 at 2:24 AM Mauro Carvalho Chehab
wrote:
>
> Em Sun, 1 Jun 2025 12:01:22 +0200
> Ricardo Ribalda escreveu:
>
> > Hi Mauro
> >
> > On Sun, 1 Jun 2025 at 11:34, Mauro Carvalho Chehab
> > wrote:
> > >
> > > Em Wed, 28 May 2025 18:23:02 +0200
> > > Ricardo Ribalda escreveu:
> > >
> > > > > +static int scatterlist_builder_add_userptr(struct
> > > > > scatterlist_builder *builder,
> > > > > + unsigned long userptr,
> > > > > + unsigned long length)
> > > > > +{
> > > > > + int ret;
> > > > > + int nents;
> > > > Could you initialize nents and sg_list?
> > > > old versions of gcc are a bit picky
> > > > https://gitlab.freedesktop.org/linux-media/users/ribalda/-/jobs/77042562#L4381
> > >
> > > Please don't. In this specific case, ret is always initialized:
> > >
> > > > + struct virtio_media_sg_entry *sg_list;
> > > > +
> > > > + ret = __scatterlist_builder_add_userptr(builder, userptr,
> > > > length,
> > > > + &sg_list, &nents);
> > >
> > > nents and sg_list may or may not be initialized at the function,
> > > but initializing it is wrong, as, when they are not initialized, the
> > > ret code shall catch it (and if not, we *do* want gcc to warn).
> > >
> > > So, if our CI is warning about that due to an old version, please upgrade
> > > the version at the CI runner.
> >
> > The main version of gcc works fine. It is the minimal version (8.1)
> > required by
> > https://www.kernel.org/doc/html/next/process/changes.html
> > that complains.
>
> Ricardo,
>
> gcc 8.1 was released in May 2, 2018. I don't think it makes sense to
> address bogus warnings with that old gcc versions. I would just disable
> WERROR for such versions on our CI tests.
>
> ---
>
> Alexandre/Michael,
>
> I need a couple of full days to properly review virtio-media.
> I was planning to do it during this Kernel cycle, but I ended
> allocating too much time just to be able to create a crossvm
> that would allow testing it. Afterwards, I got sidetracked with other
> issues. I won't be able to review it in time for this merge window.
>
> I'm planning to do it at the beginning of the next merge cycle.
Not a worry at all and I appreciate the time you are putting aside to
review this properly!
Thanks,
Alex.
Re: [PATCH v3] media: add virtio-media driver
Em Sun, 1 Jun 2025 12:01:22 +0200
Ricardo Ribalda escreveu:
> Hi Mauro
>
> On Sun, 1 Jun 2025 at 11:34, Mauro Carvalho Chehab
> wrote:
> >
> > Em Wed, 28 May 2025 18:23:02 +0200
> > Ricardo Ribalda escreveu:
> >
> > > > +static int scatterlist_builder_add_userptr(struct scatterlist_builder
> > > > *builder,
> > > > + unsigned long userptr,
> > > > + unsigned long length)
> > > > +{
> > > > + int ret;
> > > > + int nents;
> > > Could you initialize nents and sg_list?
> > > old versions of gcc are a bit picky
> > > https://gitlab.freedesktop.org/linux-media/users/ribalda/-/jobs/77042562#L4381
> > >
> >
> > Please don't. In this specific case, ret is always initialized:
> >
> > > + struct virtio_media_sg_entry *sg_list;
> > > +
> > > + ret = __scatterlist_builder_add_userptr(builder, userptr, length,
> > > + &sg_list, &nents);
> >
> > nents and sg_list may or may not be initialized at the function,
> > but initializing it is wrong, as, when they are not initialized, the
> > ret code shall catch it (and if not, we *do* want gcc to warn).
> >
> > So, if our CI is warning about that due to an old version, please upgrade
> > the version at the CI runner.
>
> The main version of gcc works fine. It is the minimal version (8.1) required
> by
> https://www.kernel.org/doc/html/next/process/changes.html
> that complains.
Ricardo,
gcc 8.1 was released in May 2, 2018. I don't think it makes sense to
address bogus warnings with that old gcc versions. I would just disable
WERROR for such versions on our CI tests.
---
Alexandre/Michael,
I need a couple of full days to properly review virtio-media.
I was planning to do it during this Kernel cycle, but I ended
allocating too much time just to be able to create a crossvm
that would allow testing it. Afterwards, I got sidetracked with other
issues. I won't be able to review it in time for this merge window.
I'm planning to do it at the beginning of the next merge cycle.
Thanks,
Mauro
Re: [PATCH v3] media: add virtio-media driver
Hi Mauro, Really appreciating the time you are spending reviewing and testing this! m(__)m Thanks also for sharing your script, I've learned a few things I didn't know about crosvm. :P On Thu Jun 19, 2025 at 12:05 AM JST, Mauro Carvalho Chehab wrote: >> > Found how to setup cpus and memory, but didn't find a way to setup >> > network without running it as root. The gpu parameter has several >> > options. Not sure what backend works well for media apps like qv4l2, >> > camorama, X11, ... >> >> I'm afraid getting GPU and graphics in general to work is more involved >> and tricky on a regular Linux setup (crosvm was primarily designed for >> ChromeOS). If you really need it I can do some more research; most of my >> tests have been done using v4l2-ctl or ffmpeg and saving the output on >> disk for later inspection. > > It was actually easier than what I expected, but it had to run > as root. Due to that, I had to move it to a test machine that I > use just for such kind of tests. I updated it to the Ubuntu > version 24.10, but crossvm refused to build even. I end needing > to install rust via rustup, as only version 1.81.0 had what it is > required to run with the needed features (network, media and gpu). Yes, rustup is the preferred way (if not traditional from the point of view of Linux distros) to get the latest Rust toolchain. > >> >> > Btw, on a quick test with v4l2-compliance, something looks weird: >> >> > I started a camera application at the host. Still, v4l2-compliance >> >> > said successfully excecuted mmap: >> >> > >> >> > Streaming ioctls: >> >> > test read/write: OK (Not Supported) >> >> > test blocking wait: OK >> >> > test MMAP (no poll): OK >> >> > test MMAP (select): OK >> >> > Vide[2025-06-17T08:44:49.177972817+00:00 ERROR >> >> > virtio_media::ioctl] VIDIOC_REQBUFS: memory type DmaBuf is currently >> >> > unsupported >> >> > [2025-06-17T08:44:49.178164554+00:00 ERROR virtio_media::ioctl] >> >> > VIDIOC_REQBUFS: memory type DmaBuf is currently unsupported >> >> > o Capturtest MMAP (epoll): OK >> >> > test USERPTR (no poll): OK (Not Supported) >> >> > test USERPTR (select): OK (Not Supported) >> >> > test DMABUF (no poll): OK (Not Supported) >> >> > test DMABUF (select): OK (Not Supported) >> >> > >> >> > Which doesn't make any sense, as the host OS should not allow access >> >> > to mmap while streaming. >> >> >> >> Ah, this was with the "simple" device, not with the proxy one. >> >> With the proxy one, I'm getting: >> >> >> >> # v4l2-ctl --all >> >> Driver Info: >> >> Driver name : virtio-media >> >> Card type: usb video: usb video >> >> Bus info : platform:virtio-media >> >> Driver version : 6.15.0 >> >> Capabilities : 0x8421 >> >> Video Capture >> >> Streaming >> >> Extended Pix Format >> >> Device Capabilities >> >> Device Caps : 0x0421 >> >> Video Capture >> >> Streaming >> >> Extended Pix Format >> >> Priority: 2 >> >> Video input : 0 (Camera 1: ok) >> >> Format Video Capture: >> >> Width/Height : 1280/720 >> >> Pixel Format : 'MJPG' (Motion-JPEG) >> >> Field : None >> >> Bytes per Line: 0 >> >> Size Image: 1843200 >> >> Colorspace: sRGB >> >> Transfer Function : Rec. 709 >> >> YCbCr/HSV Encoding: ITU-R 601 >> >> Quantization : Default (maps to Full Range) >> >> Flags : >> >> Crop Capability Video Capture: >> >> Bounds : Left 0, Top 0, Width 1280, Height 720 >> >> Default : Left 0, Top 0, Width 1280, Height 720 >> >> Pixel Aspect: 1/1 >> >> Selection Video Capture: crop_default, Left 0, Top 0, Width 1280, Height >> >> 720, Flags: >> >> Selection Video Capture: crop_bounds, Left 0, Top 0, Width 1280, Height >> >> 720, Flags: >> >> Streaming Parameters Video Capture: >> >> Capabilities : timeperframe >> >> Frames per second: 30.000 (30/1) >> >> Read buffers : 0 >> >> >> >> User Controls >> >> >> >> brightness 0x00980900 (int): min=-128 max=127 >> >> step=1 default=-11 value=-11 >> >>contrast 0x00980901 (int): min=0 max=255 >> >> step=1 default=148 value=148 >> >> saturation 0x00980902 (int): min=0 max=255 >> >> step=1 default=180 value=180 >> >> hue 0x00980903 (int): min=-128 max=127 >> >> step=1 default=0 value=0 >> >> >> >> # v4l2-compliance -d0 -s >> >> >> >> Streaming ioctls: >> >> test read/write: OK (Not Supported) >> >> test blocking wait: OK >> >> fail: v4l
Re: [PATCH v3] media: add virtio-media driver
Em Wed, 18 Jun 2025 23:27:13 +0900 "Alexandre Courbot" escreveu: > On Tue Jun 17, 2025 at 7:20 PM JST, Mauro Carvalho Chehab wrote: > > Em Tue, 17 Jun 2025 11:03:18 +0200 > > Mauro Carvalho Chehab escreveu: > > > >> Em Tue, 17 Jun 2025 10:49:38 +0200 > >> Mauro Carvalho Chehab escreveu: > >> > >> > Hi Alex, > >> > > >> > Em Tue, 27 May 2025 23:03:39 +0900 > >> > Alexandre Courbot escreveu: > >> > > >> > > > > > Btw, I was looking at: > >> > > > > > > >> > > > > > https://github.com/chromeos/virtio-media > >> > > > > > > >> > > > > > (I'm assuming that this is the QEMU counterpart, right?) > >> > > > > > >> > > > > crosvm actually, but QEMU support is also being worked on. > >> > > > > >> > > > Do you have already QEMU patches? The best is to have the Kernel > >> > > > driver > >> > > > submitted altogether with QEMU, as Kernel developers need it to do > >> > > > the > >> > > > tests. In my case, I never use crosvm, and I don't have any > >> > > > Chromebook > >> > > > anymore. > >> > > > >> > > IIRC Albert Esteve was working on this, maybe he can share the current > >> > > status. > >> > > >> > Any news regards to it? > >> > > >> > > Note that crosvm does not require a Chromebook, you can build and run > >> > > it pretty easily on a regular PC. I have put together a document to > >> > > help with that: > >> > > > >> > > https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md > >> > > >> > I started looking on it today. Already installed crossvm (I had to > >> > install libcap-devel to build it). Still, I'm not familiar with > >> > crossvm, which is a little be painful. In particular, how can I > >> > enable network on it and speedup it? With suggested parameters, > >> > it picked only one CPU, and very few memory on it: > >> > > >> > # cat /proc/cpuinfo|grep processor > >> > processor : 0 > >> > > >> > # free > >> >totalusedfree shared buff/cache > >> > available > >> > Mem: 221876 34780 139712 272 56096 > >> > 187096 > >> > Swap: 0 0 0 > >> > > >> > I'd like to be able to compile things on it and use ssh/scp. So, > >> > the VM needs more CPUs, more memory, more network and GPU. > > > > Found how to setup cpus and memory, but didn't find a way to setup > > network without running it as root. The gpu parameter has several > > options. Not sure what backend works well for media apps like qv4l2, > > camorama, X11, ... > > I'm afraid getting GPU and graphics in general to work is more involved > and tricky on a regular Linux setup (crosvm was primarily designed for > ChromeOS). If you really need it I can do some more research; most of my > tests have been done using v4l2-ctl or ffmpeg and saving the output on > disk for later inspection. It was actually easier than what I expected, but it had to run as root. Due to that, I had to move it to a test machine that I use just for such kind of tests. I updated it to the Ubuntu version 24.10, but crossvm refused to build even. I end needing to install rust via rustup, as only version 1.81.0 had what it is required to run with the needed features (network, media and gpu). > >> > Btw, on a quick test with v4l2-compliance, something looks weird: > >> > I started a camera application at the host. Still, v4l2-compliance > >> > said successfully excecuted mmap: > >> > > >> > Streaming ioctls: > >> > test read/write: OK (Not Supported) > >> > test blocking wait: OK > >> > test MMAP (no poll): OK > >> > test MMAP (select): OK > >> > Vide[2025-06-17T08:44:49.177972817+00:00 ERROR > >> > virtio_media::ioctl] VIDIOC_REQBUFS: memory type DmaBuf is currently > >> > unsupported > >> > [2025-06-17T08:44:49.178164554+00:00 ERROR virtio_media::ioctl] > >> > VIDIOC_REQBUFS: memory type DmaBuf is currently unsupported > >> > o Capturtest MMAP (epoll): OK > >> > test USERPTR (no poll): OK (Not Supported) > >> > test USERPTR (select): OK (Not Supported) > >> > test DMABUF (no poll): OK (Not Supported) > >> > test DMABUF (select): OK (Not Supported) > >> > > >> > Which doesn't make any sense, as the host OS should not allow access > >> > to mmap while streaming. > >> > >> Ah, this was with the "simple" device, not with the proxy one. > >> With the proxy one, I'm getting: > >> > >> # v4l2-ctl --all > >> Driver Info: > >> Driver name : virtio-media > >> Card type: usb video: usb video > >> Bus info : platform:virtio-media > >> Driver version : 6.15.0 > >> Capabilities : 0x8421 > >> Video Capture > >> Streaming > >> Extended Pix Format > >> Device Capabilities > >> Device
Re: [PATCH v3] media: add virtio-media driver
Em Wed, 18 Jun 2025 23:16:47 +0900 "Alexandre Courbot" escreveu: > Hi Mauro, > > On Tue Jun 17, 2025 at 5:49 PM JST, Mauro Carvalho Chehab wrote: > > Hi Alex, > > > > Em Tue, 27 May 2025 23:03:39 +0900 > > Alexandre Courbot escreveu: > > > >> > > > Btw, I was looking at: > >> > > > > >> > > > https://github.com/chromeos/virtio-media > >> > > > > >> > > > (I'm assuming that this is the QEMU counterpart, right?) > >> > > > >> > > crosvm actually, but QEMU support is also being worked on. > >> > > >> > Do you have already QEMU patches? The best is to have the Kernel driver > >> > submitted altogether with QEMU, as Kernel developers need it to do the > >> > tests. In my case, I never use crosvm, and I don't have any Chromebook > >> > anymore. > >> > >> IIRC Albert Esteve was working on this, maybe he can share the current > >> status. > > > > Any news regards to it? > > Albert shared the latest status. There is one in-flight patch series > required in qemu [1], and then this branch of vhost-device should > contain the necessary support [2]. Albert is waiting for the virtio spec > to get merged before sending a pull request IIUC. > > [1] https://patchew.org/QEMU/[email protected]/ > [2] https://github.com/aesteve-rh/vhost-device/tree/virtio-media > > > > >> Note that crosvm does not require a Chromebook, you can build and run > >> it pretty easily on a regular PC. I have put together a document to > >> help with that: > >> > >> https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md > > > > I started looking on it today. Already installed crossvm (I had to > > install libcap-devel to build it). Still, I'm not familiar with > > crossvm, which is a little be painful. In particular, how can I > > enable network on it and speedup it? > > There is a "./tools/examples/setup_network" in the crosvm repository that > will setup a TAP device. Once this is done, you can pass the "--net > tap-name=crosvm_tap" argument to crosvm, and the network device should > be visible and usable. > > Let me reply to the rest of your questions in your latest mail, with the > most recent logs. Heh, I just managed to get it work maybe 10 minutes before your e-mail... I'm building crossvm with: cargo build --release --features "gpu,media,virgl_renderer,x" To also have GPU working. Network setup required a rather complex script to set it up without breaking my ssh section with the machine where I'm running crossvm. In case you need, I'm enclosing it. Now, I need to allocate another time slot for tests and review. --- #!/bin/bash PHY=enp0s25 BRIDGE=crossvm_br TAP=crossvm_tap IP_MASK=$(ip -br addr show $PHY | awk '{print $3}' | grep -oE '^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+/[0-9]+') if [ -z "$IP_MASK" ]; then echo "Failed to detect IP address on $PHY. Exiting." exit 1 fi GW=$(ip -br route show default dev $PHY | awk '{print $3}') if [ -z "$GW" ]; then echo "Failed to detect default gateway on $PHY. Exiting." exit 1 fi restore_network() { echo "Restoring original network config on $PHY..." sudo ip link set dev $BRIDGE down || true sudo ip link del $BRIDGE || true sudo ip link set dev $TAP down || true sudo ip tuntap del dev $TAP mode tap || true sudo ip addr flush dev $PHY sudo ip addr add ${IP_MASK} dev $PHY sudo ip link set dev $PHY up sudo ip route add default via $GW } if ! lsmod | grep -q '^bridge'; then sudo modprobe bridge fi trap 'catch $LINENO "$BASH_COMMAND"' ERR catch() { echo "Error on line $1: $2" restore_network exit 1 } if ip link show $TAP &>/dev/null; then echo "Removing existing tap $TAP" sudo ip link set dev $TAP down sudo ip tuntap del dev $TAP mode tap fi if ip link show $BRIDGE &>/dev/null; then echo "Removing existing bridge $BRIDGE" sudo ip link set dev $BRIDGE down sudo ip link del $BRIDGE fi # Create bridge device sudo ip link add name $BRIDGE type bridge sudo ip link set dev $BRIDGE up sudo ip link set dev $BRIDGE type bridge forward_delay 0 # Add physical interface to bridge sudo ip link set dev $PHY master $BRIDGE # Create tap device sudo ip tuntap add dev $TAP mode tap sudo ip link set dev $TAP up # Add tap to bridge sudo ip link set dev $TAP master $BRIDGE # Get an address to the bridge sudo dhclient $BRIDGE # Start crossvm sudo ./crosvm/target/release/crosvm run \ linux/arch/x86/boot/bzImage \ -c 4 -m size=4096 \ --disable-sandbox \ --block debian-12.img \ -p "root=/dev/vda1" \ --v4l2-proxy /dev/video0 \ --gpu backend=virglrenderer \ --net tap-name=crossvm_tap,vhost-net
Re: [PATCH v3] media: add virtio-media driver
On Tue Jun 17, 2025 at 7:20 PM JST, Mauro Carvalho Chehab wrote: > Em Tue, 17 Jun 2025 11:03:18 +0200 > Mauro Carvalho Chehab escreveu: > >> Em Tue, 17 Jun 2025 10:49:38 +0200 >> Mauro Carvalho Chehab escreveu: >> >> > Hi Alex, >> > >> > Em Tue, 27 May 2025 23:03:39 +0900 >> > Alexandre Courbot escreveu: >> > >> > > > > > Btw, I was looking at: >> > > > > > >> > > > > > https://github.com/chromeos/virtio-media >> > > > > > >> > > > > > (I'm assuming that this is the QEMU counterpart, right?) >> > > > > >> > > > > crosvm actually, but QEMU support is also being worked on. >> > > > >> > > > Do you have already QEMU patches? The best is to have the Kernel driver >> > > > submitted altogether with QEMU, as Kernel developers need it to do the >> > > > tests. In my case, I never use crosvm, and I don't have any Chromebook >> > > > anymore. >> > > >> > > IIRC Albert Esteve was working on this, maybe he can share the current >> > > status. >> > >> > Any news regards to it? >> > >> > > Note that crosvm does not require a Chromebook, you can build and run >> > > it pretty easily on a regular PC. I have put together a document to >> > > help with that: >> > > >> > > https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md >> > >> > I started looking on it today. Already installed crossvm (I had to >> > install libcap-devel to build it). Still, I'm not familiar with >> > crossvm, which is a little be painful. In particular, how can I >> > enable network on it and speedup it? With suggested parameters, >> > it picked only one CPU, and very few memory on it: >> > >> ># cat /proc/cpuinfo|grep processor >> >processor : 0 >> > >> ># free >> >totalusedfree shared buff/cache >> > available >> >Mem: 221876 34780 139712 272 56096 >> > 187096 >> >Swap: 0 0 0 >> > >> > I'd like to be able to compile things on it and use ssh/scp. So, >> > the VM needs more CPUs, more memory, more network and GPU. > > Found how to setup cpus and memory, but didn't find a way to setup > network without running it as root. The gpu parameter has several > options. Not sure what backend works well for media apps like qv4l2, > camorama, X11, ... I'm afraid getting GPU and graphics in general to work is more involved and tricky on a regular Linux setup (crosvm was primarily designed for ChromeOS). If you really need it I can do some more research; most of my tests have been done using v4l2-ctl or ffmpeg and saving the output on disk for later inspection. > >> > >> > Btw, on a quick test with v4l2-compliance, something looks weird: >> > I started a camera application at the host. Still, v4l2-compliance >> > said successfully excecuted mmap: >> > >> > Streaming ioctls: >> > test read/write: OK (Not Supported) >> > test blocking wait: OK >> > test MMAP (no poll): OK >> > test MMAP (select): OK >> > Vide[2025-06-17T08:44:49.177972817+00:00 ERROR >> > virtio_media::ioctl] VIDIOC_REQBUFS: memory type DmaBuf is currently >> > unsupported >> > [2025-06-17T08:44:49.178164554+00:00 ERROR virtio_media::ioctl] >> > VIDIOC_REQBUFS: memory type DmaBuf is currently unsupported >> > o Capturtest MMAP (epoll): OK >> > test USERPTR (no poll): OK (Not Supported) >> > test USERPTR (select): OK (Not Supported) >> > test DMABUF (no poll): OK (Not Supported) >> > test DMABUF (select): OK (Not Supported) >> > >> > Which doesn't make any sense, as the host OS should not allow access >> > to mmap while streaming. >> >> Ah, this was with the "simple" device, not with the proxy one. >> With the proxy one, I'm getting: >> >> # v4l2-ctl --all >> Driver Info: >> Driver name : virtio-media >> Card type: usb video: usb video >> Bus info : platform:virtio-media >> Driver version : 6.15.0 >> Capabilities : 0x8421 >> Video Capture >> Streaming >> Extended Pix Format >> Device Capabilities >> Device Caps : 0x0421 >> Video Capture >> Streaming >> Extended Pix Format >> Priority: 2 >> Video input : 0 (Camera 1: ok) >> Format Video Capture: >> Width/Height : 1280/720 >> Pixel Format : 'MJPG' (Motion-JPEG) >> Field : None >> Bytes per Line: 0 >> Size Image: 1843200 >> Colorspace: sRGB >> Transfer Function : Rec. 709 >> YCbCr/HSV Encoding: ITU-R 601 >> Quantization : Default (maps to Full Range) >> Flags : >> Crop Capability Video Capture: >> Bounds : Left 0, Top 0, Width 1280, Height 7
Re: [PATCH v3] media: add virtio-media driver
Hi Mauro, On Tue Jun 17, 2025 at 5:49 PM JST, Mauro Carvalho Chehab wrote: > Hi Alex, > > Em Tue, 27 May 2025 23:03:39 +0900 > Alexandre Courbot escreveu: > >> > > > Btw, I was looking at: >> > > > >> > > > https://github.com/chromeos/virtio-media >> > > > >> > > > (I'm assuming that this is the QEMU counterpart, right?) >> > > >> > > crosvm actually, but QEMU support is also being worked on. >> > >> > Do you have already QEMU patches? The best is to have the Kernel driver >> > submitted altogether with QEMU, as Kernel developers need it to do the >> > tests. In my case, I never use crosvm, and I don't have any Chromebook >> > anymore. >> >> IIRC Albert Esteve was working on this, maybe he can share the current >> status. > > Any news regards to it? Albert shared the latest status. There is one in-flight patch series required in qemu [1], and then this branch of vhost-device should contain the necessary support [2]. Albert is waiting for the virtio spec to get merged before sending a pull request IIUC. [1] https://patchew.org/QEMU/[email protected]/ [2] https://github.com/aesteve-rh/vhost-device/tree/virtio-media > >> Note that crosvm does not require a Chromebook, you can build and run >> it pretty easily on a regular PC. I have put together a document to >> help with that: >> >> https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md > > I started looking on it today. Already installed crossvm (I had to > install libcap-devel to build it). Still, I'm not familiar with > crossvm, which is a little be painful. In particular, how can I > enable network on it and speedup it? There is a "./tools/examples/setup_network" in the crosvm repository that will setup a TAP device. Once this is done, you can pass the "--net tap-name=crosvm_tap" argument to crosvm, and the network device should be visible and usable. Let me reply to the rest of your questions in your latest mail, with the most recent logs. Cheers, Alex.
Re: [PATCH v3] media: add virtio-media driver
Em Tue, 17 Jun 2025 11:03:18 +0200 Mauro Carvalho Chehab escreveu: > Em Tue, 17 Jun 2025 10:49:38 +0200 > Mauro Carvalho Chehab escreveu: > > > Hi Alex, > > > > Em Tue, 27 May 2025 23:03:39 +0900 > > Alexandre Courbot escreveu: > > > > > > > > Btw, I was looking at: > > > > > > > > > > > > https://github.com/chromeos/virtio-media > > > > > > > > > > > > (I'm assuming that this is the QEMU counterpart, right?) > > > > > > > > > > crosvm actually, but QEMU support is also being worked on. > > > > > > > > Do you have already QEMU patches? The best is to have the Kernel driver > > > > submitted altogether with QEMU, as Kernel developers need it to do the > > > > tests. In my case, I never use crosvm, and I don't have any Chromebook > > > > anymore. > > > > > > IIRC Albert Esteve was working on this, maybe he can share the current > > > status. > > > > Any news regards to it? > > > > > Note that crosvm does not require a Chromebook, you can build and run > > > it pretty easily on a regular PC. I have put together a document to > > > help with that: > > > > > > https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md > > > > I started looking on it today. Already installed crossvm (I had to > > install libcap-devel to build it). Still, I'm not familiar with > > crossvm, which is a little be painful. In particular, how can I > > enable network on it and speedup it? With suggested parameters, > > it picked only one CPU, and very few memory on it: > > > > # cat /proc/cpuinfo|grep processor > > processor : 0 > > > > # free > >totalusedfree shared buff/cache > > available > > Mem: 221876 34780 139712 272 56096 > > 187096 > > Swap: 0 0 0 > > > > I'd like to be able to compile things on it and use ssh/scp. So, > > the VM needs more CPUs, more memory, more network and GPU. Found how to setup cpus and memory, but didn't find a way to setup network without running it as root. The gpu parameter has several options. Not sure what backend works well for media apps like qv4l2, camorama, X11, ... > > > > Btw, on a quick test with v4l2-compliance, something looks weird: > > I started a camera application at the host. Still, v4l2-compliance > > said successfully excecuted mmap: > > > > Streaming ioctls: > > test read/write: OK (Not Supported) > > test blocking wait: OK > > test MMAP (no poll): OK > > test MMAP (select): OK > > Vide[2025-06-17T08:44:49.177972817+00:00 ERROR virtio_media::ioctl] > > VIDIOC_REQBUFS: memory type DmaBuf is currently unsupported > > [2025-06-17T08:44:49.178164554+00:00 ERROR virtio_media::ioctl] > > VIDIOC_REQBUFS: memory type DmaBuf is currently unsupported > > o Capturtest MMAP (epoll): OK > > test USERPTR (no poll): OK (Not Supported) > > test USERPTR (select): OK (Not Supported) > > test DMABUF (no poll): OK (Not Supported) > > test DMABUF (select): OK (Not Supported) > > > > Which doesn't make any sense, as the host OS should not allow access > > to mmap while streaming. > > Ah, this was with the "simple" device, not with the proxy one. > With the proxy one, I'm getting: > > # v4l2-ctl --all > Driver Info: > Driver name : virtio-media > Card type: usb video: usb video > Bus info : platform:virtio-media > Driver version : 6.15.0 > Capabilities : 0x8421 > Video Capture > Streaming > Extended Pix Format > Device Capabilities > Device Caps : 0x0421 > Video Capture > Streaming > Extended Pix Format > Priority: 2 > Video input : 0 (Camera 1: ok) > Format Video Capture: > Width/Height : 1280/720 > Pixel Format : 'MJPG' (Motion-JPEG) > Field : None > Bytes per Line: 0 > Size Image: 1843200 > Colorspace: sRGB > Transfer Function : Rec. 709 > YCbCr/HSV Encoding: ITU-R 601 > Quantization : Default (maps to Full Range) > Flags : > Crop Capability Video Capture: > Bounds : Left 0, Top 0, Width 1280, Height 720 > Default : Left 0, Top 0, Width 1280, Height 720 > Pixel Aspect: 1/1 > Selection Video Capture: crop_default, Left 0, Top 0, Width 1280, Height 720, > Flags: > Selection Video Capture: crop_bounds, Left 0, Top 0, Width 1280, Height 720, > Flags: > Streaming Parameters Video Capture: > Capabilities : timeperframe > Frames per second: 30.000 (30/1) > Read buffers : 0 > > User Controls > > brightness 0x00980900 (int)
Re: [PATCH v3] media: add virtio-media driver
Em Tue, 17 Jun 2025 10:49:38 +0200 Mauro Carvalho Chehab escreveu: > Hi Alex, > > Em Tue, 27 May 2025 23:03:39 +0900 > Alexandre Courbot escreveu: > > > > > > Btw, I was looking at: > > > > > > > > > > https://github.com/chromeos/virtio-media > > > > > > > > > > (I'm assuming that this is the QEMU counterpart, right?) > > > > > > > > crosvm actually, but QEMU support is also being worked on. > > > > > > Do you have already QEMU patches? The best is to have the Kernel driver > > > submitted altogether with QEMU, as Kernel developers need it to do the > > > tests. In my case, I never use crosvm, and I don't have any Chromebook > > > anymore. > > > > IIRC Albert Esteve was working on this, maybe he can share the current > > status. > > Any news regards to it? > > > Note that crosvm does not require a Chromebook, you can build and run > > it pretty easily on a regular PC. I have put together a document to > > help with that: > > > > https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md > > I started looking on it today. Already installed crossvm (I had to > install libcap-devel to build it). Still, I'm not familiar with > crossvm, which is a little be painful. In particular, how can I > enable network on it and speedup it? With suggested parameters, > it picked only one CPU, and very few memory on it: > > # cat /proc/cpuinfo|grep processor > processor : 0 > > # free >totalusedfree shared buff/cache > available > Mem: 221876 34780 139712 272 56096 > 187096 > Swap: 0 0 0 > > I'd like to be able to compile things on it and use ssh/scp. So, > the VM needs more CPUs, more memory, more network and GPU. > > Btw, on a quick test with v4l2-compliance, something looks weird: > I started a camera application at the host. Still, v4l2-compliance > said successfully excecuted mmap: > > Streaming ioctls: > test read/write: OK (Not Supported) > test blocking wait: OK > test MMAP (no poll): OK > test MMAP (select): OK > Vide[2025-06-17T08:44:49.177972817+00:00 ERROR virtio_media::ioctl] > VIDIOC_REQBUFS: memory type DmaBuf is currently unsupported > [2025-06-17T08:44:49.178164554+00:00 ERROR virtio_media::ioctl] > VIDIOC_REQBUFS: memory type DmaBuf is currently unsupported > o Capturtest MMAP (epoll): OK > test USERPTR (no poll): OK (Not Supported) > test USERPTR (select): OK (Not Supported) > test DMABUF (no poll): OK (Not Supported) > test DMABUF (select): OK (Not Supported) > > Which doesn't make any sense, as the host OS should not allow access > to mmap while streaming. Ah, this was with the "simple" device, not with the proxy one. With the proxy one, I'm getting: # v4l2-ctl --all Driver Info: Driver name : virtio-media Card type: usb video: usb video Bus info : platform:virtio-media Driver version : 6.15.0 Capabilities : 0x8421 Video Capture Streaming Extended Pix Format Device Capabilities Device Caps : 0x0421 Video Capture Streaming Extended Pix Format Priority: 2 Video input : 0 (Camera 1: ok) Format Video Capture: Width/Height : 1280/720 Pixel Format : 'MJPG' (Motion-JPEG) Field : None Bytes per Line: 0 Size Image: 1843200 Colorspace: sRGB Transfer Function : Rec. 709 YCbCr/HSV Encoding: ITU-R 601 Quantization : Default (maps to Full Range) Flags : Crop Capability Video Capture: Bounds : Left 0, Top 0, Width 1280, Height 720 Default : Left 0, Top 0, Width 1280, Height 720 Pixel Aspect: 1/1 Selection Video Capture: crop_default, Left 0, Top 0, Width 1280, Height 720, Flags: Selection Video Capture: crop_bounds, Left 0, Top 0, Width 1280, Height 720, Flags: Streaming Parameters Video Capture: Capabilities : timeperframe Frames per second: 30.000 (30/1) Read buffers : 0 User Controls brightness 0x00980900 (int): min=-128 max=127 step=1 default=-11 value=-11 contrast 0x00980901 (int): min=0 max=255 step=1 default=148 value=148 saturation 0x00980902 (int): min=0 max=255 step=1 default=180 value=180 hue 0x00980903 (int): min=-128 max=127 step=1 default=0 value=0 # v4l2-compliance -d0 -s Streaming ioctls: test read/write: OK (Not Supported) test blocking wait: OK fail: v4l2-test-buffers.cpp(1345): node->streamon(q.g_type(
Re: [PATCH v3] media: add virtio-media driver
Hi Alex, Em Tue, 27 May 2025 23:03:39 +0900 Alexandre Courbot escreveu: > > > > Btw, I was looking at: > > > > > > > > https://github.com/chromeos/virtio-media > > > > > > > > (I'm assuming that this is the QEMU counterpart, right?) > > > > > > crosvm actually, but QEMU support is also being worked on. > > > > Do you have already QEMU patches? The best is to have the Kernel driver > > submitted altogether with QEMU, as Kernel developers need it to do the > > tests. In my case, I never use crosvm, and I don't have any Chromebook > > anymore. > > IIRC Albert Esteve was working on this, maybe he can share the current status. Any news regards to it? > Note that crosvm does not require a Chromebook, you can build and run > it pretty easily on a regular PC. I have put together a document to > help with that: > > https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md I started looking on it today. Already installed crossvm (I had to install libcap-devel to build it). Still, I'm not familiar with crossvm, which is a little be painful. In particular, how can I enable network on it and speedup it? With suggested parameters, it picked only one CPU, and very few memory on it: # cat /proc/cpuinfo|grep processor processor : 0 # free totalusedfree shared buff/cache available Mem: 221876 34780 139712 272 56096 187096 Swap: 0 0 0 I'd like to be able to compile things on it and use ssh/scp. So, the VM needs more CPUs, more memory, more network and GPU. Btw, on a quick test with v4l2-compliance, something looks weird: I started a camera application at the host. Still, v4l2-compliance said successfully excecuted mmap: Streaming ioctls: test read/write: OK (Not Supported) test blocking wait: OK test MMAP (no poll): OK test MMAP (select): OK Vide[2025-06-17T08:44:49.177972817+00:00 ERROR virtio_media::ioctl] VIDIOC_REQBUFS: memory type DmaBuf is currently unsupported [2025-06-17T08:44:49.178164554+00:00 ERROR virtio_media::ioctl] VIDIOC_REQBUFS: memory type DmaBuf is currently unsupported o Capturtest MMAP (epoll): OK test USERPTR (no poll): OK (Not Supported) test USERPTR (select): OK (Not Supported) test DMABUF (no poll): OK (Not Supported) test DMABUF (select): OK (Not Supported) Which doesn't make any sense, as the host OS should not allow access to mmap while streaming. Thanks, Mauro
Re: [PATCH v3] media: add virtio-media driver
Hi Mauro
On Sun, 1 Jun 2025 at 11:34, Mauro Carvalho Chehab
wrote:
>
> Em Wed, 28 May 2025 18:23:02 +0200
> Ricardo Ribalda escreveu:
>
> > > +static int scatterlist_builder_add_userptr(struct scatterlist_builder
> > > *builder,
> > > + unsigned long userptr,
> > > + unsigned long length)
> > > +{
> > > + int ret;
> > > + int nents;
> > Could you initialize nents and sg_list?
> > old versions of gcc are a bit picky
> > https://gitlab.freedesktop.org/linux-media/users/ribalda/-/jobs/77042562#L4381
>
> Please don't. In this specific case, ret is always initialized:
>
> > + struct virtio_media_sg_entry *sg_list;
> > +
> > + ret = __scatterlist_builder_add_userptr(builder, userptr, length,
> > + &sg_list, &nents);
>
> nents and sg_list may or may not be initialized at the function,
> but initializing it is wrong, as, when they are not initialized, the
> ret code shall catch it (and if not, we *do* want gcc to warn).
>
> So, if our CI is warning about that due to an old version, please upgrade
> the version at the CI runner.
The main version of gcc works fine. It is the minimal version (8.1) required by
https://www.kernel.org/doc/html/next/process/changes.html
that complains.
>
> Regards,
> Mauro
>
>
> Thanks,
> Mauro
--
Ricardo Ribalda
Re: [PATCH v3] media: add virtio-media driver
Em Wed, 28 May 2025 18:23:02 +0200
Ricardo Ribalda escreveu:
> > +static int scatterlist_builder_add_userptr(struct scatterlist_builder
> > *builder,
> > + unsigned long userptr,
> > + unsigned long length)
> > +{
> > + int ret;
> > + int nents;
> Could you initialize nents and sg_list?
> old versions of gcc are a bit picky
> https://gitlab.freedesktop.org/linux-media/users/ribalda/-/jobs/77042562#L4381
Please don't. In this specific case, ret is always initialized:
> + struct virtio_media_sg_entry *sg_list;
> +
> + ret = __scatterlist_builder_add_userptr(builder, userptr, length,
> + &sg_list, &nents);
nents and sg_list may or may not be initialized at the function,
but initializing it is wrong, as, when they are not initialized, the
ret code shall catch it (and if not, we *do* want gcc to warn).
So, if our CI is warning about that due to an old version, please upgrade
the version at the CI runner.
Regards,
Mauro
Thanks,
Mauro
Re: [PATCH v3] media: add virtio-media driver
Hi Alexandre On Sat, 12 Apr 2025 at 06:08, Alexandre Courbot wrote: > > Add the first version of the virtio-media driver. > > This driver acts roughly as a V4L2 relay between user-space and the > virtio virtual device on the host, so it is relatively simple, yet > unconventional. It doesn't use VB2 or other frameworks typically used in > a V4L2 driver, and most of its complexity resides in correctly and > efficiently building the virtio descriptor chain to pass to the host, > avoiding copies whenever possible. This is done by > scatterlist_builder.[ch]. > > virtio_media_ioctls.c proxies each supported ioctl to the host, using > code generated through macros for ioctls that can be forwarded directly, > which is most of them. > > virtio_media_driver.c provides the expected driver hooks, and support > for mmapping and polling. > > This version supports MMAP buffers, while USERPTR buffers can also be > enabled through a driver option. DMABUF support is still pending. > > Signed-off-by: Alexandre Courbot > Signed-off-by: Alexandre Courbot > --- > This patch adds the virtio-media kernel driver. Virtio-media [1] > encapsulates the V4L2 structures and protocol to enable the > virtualization of host media devices into a guest. It's specification is > in the final stages [2] of being merged and the virtualization of > cameras and video accelerator devices has already been demonstrated > using crosvm [3] and QEmu. v4l2-compliance also passes on all tested > devices, which includes the "simple" virtual test device, proxied host > UVC and vivid devices, and the FFmpeg virtual decoder devices (refer to > [3] in order to test these if desired). > > Virtio-media is merged in AOSP [4] and ChromeOS. Upstreaming of the > driver is overdue, but I hope we can start the review process and > converge into something that can be merged. > > Limitations: > > - The driver is currently only available to little-endian, 64-bit > kernels. This is because some of the V4L2 structures used for > communication between guest and host have a layout dependent on the > architecture, and the virtio-media protocol is standardized on the > little-endian 64-bit versions. This can be fixed with a conversion > layer similar to the one used to convert 32-bit ioctls to their 64-bit > counterpart. > - DMABUF support is currently missing. It should be implemented using > virtio objects, with possible support for memfds using the > SHARED_PAGES memory type. > - No support for the media API and requests. While the use-case for > these is less important on virtual devices where we want to present an > abstraction as high as possible to limit VM exits, they do exist and > it would be nice to add behind a virtio feature bit. > - Locking in the driver is still very basic. This is something I want to > improve before merging, but I didn't want to delay upstream review any > further. > > [1] https://github.com/chromeos/virtio-media > [2] > https://lore.kernel.org/virtio-comment/[email protected]/ > [3] https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md > [4] https://android.googlesource.com/platform/external/virtio-media/ > --- > Changes in v3: > - Rebased on top of v6.15-rc1 and removes obsolete control callbacks. > - Link to v2: > https://lore.kernel.org/r/[email protected] > > Changes in v2: > - Fixed kernel test robot and media CI warnings (ignored a few false > positives). > - Changed in-driver email address to personal one since my Google one > will soon become invalid. > - Link to v1: > https://lore.kernel.org/r/[email protected] > --- > MAINTAINERS|6 + > drivers/media/Kconfig | 13 + > drivers/media/Makefile |2 + > drivers/media/virtio/Makefile |9 + > drivers/media/virtio/protocol.h| 288 ++ > drivers/media/virtio/scatterlist_builder.c | 563 > drivers/media/virtio/scatterlist_builder.h | 111 +++ > drivers/media/virtio/session.h | 109 +++ > drivers/media/virtio/virtio_media.h| 93 ++ > drivers/media/virtio/virtio_media_driver.c | 959 > drivers/media/virtio/virtio_media_ioctls.c | 1297 > > include/uapi/linux/virtio_ids.h|1 + > 12 files changed, 3451 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index > 96b82704950184bd71623ff41fc4df31e4c7fe87..f60e17011124fe8c0be0343d4f87e1458f311dcc > 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -25641,6 +25641,12 @@ S: Maintained > F: drivers/iommu/virtio-iommu.c > F: include/uapi/linux/virtio_iommu.h > > +VIRTIO MEDIA DRIVER > +M: Alexandre Courbot > +L: [email protected] > +S: Maintained > +F: drivers/media/virtio/ > + > VIRTIO MEM DRIVER > M: David Hildenbrand > L: virtualiza
Re: [PATCH v3] media: add virtio-media driver
Hi Michael, On Wed, May 28, 2025 at 12:06 AM Michael S. Tsirkin wrote: > > On Tue, May 27, 2025 at 04:39:27PM +0200, Mauro Carvalho Chehab wrote: > > > It's up to you though. > > > I can keep it in next for now, so it gets some coverage by > > > tools scanning that tree. > > > > Sure, feel free to keep it on next if you prefer so. Just > > please don't submit it upstream while we don't review and > > properly test it. > > No prob. I just want to see it get reviewed and merged. > My understanding is, it wasn't because maintainers were > not Cc'd so that should be all ironed out now. > Alexandre, do you want this in next for now or just drop it? I think it doesn't hurt to give some exposure to this driver, if Mauro is ok with it (and it eventually gets merged through the media tree).
Re: [PATCH v3] media: add virtio-media driver
On Tue, May 27, 2025 at 04:39:27PM +0200, Mauro Carvalho Chehab wrote: > > It's up to you though. > > I can keep it in next for now, so it gets some coverage by > > tools scanning that tree. > > Sure, feel free to keep it on next if you prefer so. Just > please don't submit it upstream while we don't review and > properly test it. No prob. I just want to see it get reviewed and merged. My understanding is, it wasn't because maintainers were not Cc'd so that should be all ironed out now. Alexandre, do you want this in next for now or just drop it? -- MST
Re: [PATCH v3] media: add virtio-media driver
Em Tue, 27 May 2025 23:03:39 +0900 Alexandre Courbot escreveu: > On Tue, May 27, 2025 at 10:35 PM Mauro Carvalho Chehab > wrote: > > > > Em Tue, 27 May 2025 22:21:42 +0900 > > Alexandre Courbot escreveu: > > > > > On Tue, May 27, 2025 at 6:13 PM Mauro Carvalho Chehab > > > wrote: > > > > > > > > Em Tue, 27 May 2025 15:14:50 +0900 > > > > "Alexandre Courbot" escreveu: > > > > > > > > > Hi Mauro, > > > > > > > > > > On Mon May 26, 2025 at 9:13 PM JST, Mauro Carvalho Chehab wrote: > > > > > > Hi Michael, > > > > > > > > > > > > Em Sat, 12 Apr 2025 13:08:01 +0900 > > > > > > Alexandre Courbot escreveu: > > > > > > > > > > > >> Add the first version of the virtio-media driver. > > > > > >> > > > > > >> This driver acts roughly as a V4L2 relay between user-space and the > > > > > >> virtio virtual device on the host, so it is relatively simple, yet > > > > > >> unconventional. It doesn't use VB2 or other frameworks typically > > > > > >> used in > > > > > >> a V4L2 driver, and most of its complexity resides in correctly and > > > > > >> efficiently building the virtio descriptor chain to pass to the > > > > > >> host, > > > > > >> avoiding copies whenever possible. This is done by > > > > > >> scatterlist_builder.[ch]. > > > > > >> > > > > > >> virtio_media_ioctls.c proxies each supported ioctl to the host, > > > > > >> using > > > > > >> code generated through macros for ioctls that can be forwarded > > > > > >> directly, > > > > > >> which is most of them. > > > > > >> > > > > > >> virtio_media_driver.c provides the expected driver hooks, and > > > > > >> support > > > > > >> for mmapping and polling. > > > > > >> > > > > > >> This version supports MMAP buffers, while USERPTR buffers can > > > > > >> also be > > > > > >> enabled through a driver option. DMABUF support is still pending. > > > > > >> > > > > > > > > > > > > It sounds that you applied this one at the virtio tree, but it > > > > > > hasn't > > > > > > being reviewed or acked by media maintainers. > > > > > > > > > > > > Please drop it. > > > > > > > > > > > > Alexandre, > > > > > > > > > > > > Please send media patches to media maintainers, c/c other subsystem > > > > > > maintainers, as otherwise they might end being merged without a > > > > > > proper review. > > > > > > > > > > Sorry about that, I put everyone in "To:" without giving it a second > > > > > thought. > > > > > > > > > > > > > > > > > In this particular case, we need to double-check if this won't cause > > > > > > any issues, in special with regards to media locks and mutexes. > > > > > > > > > > Agreed, I am not 100% confident about that part myself. > > > > > > > > > > > > > > > > > I'll try to look on it after this merge window, as it is too late > > > > > > for it to be applied during this one. > > > > > > > > > > Appreciate that - given the high traffic on the list I was worried > > > > > that > > > > > this patch would eventually be overlooked. Not making it for this > > > > > merge > > > > > window should not be a problem, so please take the time you need. > > > > > > > > Provided that your patch was caught by patchwork, it won't be lost: > > > > > > > > https://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/ > > > > > > > > Please notice that our CI got a number of checkpatch issues there. > > > > Please check and fix the non-false-positive ones. > > > > > > Will do. The macro-related ones are false-positives AFAICT. Some of > > > the "lines should not end with a '('" are actually the result of > > > applying clang-format with the kernel-provided style... > > > > I don't know any lint tool that honors kernel style. The best one > > is checkpatch with the auto-correcting parameter in pedantic mode, > > but still one needs to manually review its output. > > > > > > > > > > > > > Btw, I was looking at: > > > > > > > > https://github.com/chromeos/virtio-media > > > > > > > > (I'm assuming that this is the QEMU counterpart, right?) > > > > > > crosvm actually, but QEMU support is also being worked on. > > > > Do you have already QEMU patches? The best is to have the Kernel driver > > submitted altogether with QEMU, as Kernel developers need it to do the > > tests. In my case, I never use crosvm, and I don't have any Chromebook > > anymore. > > IIRC Albert Esteve was working on this, maybe he can share the current status. That would be nice. > > Note that crosvm does not require a Chromebook, you can build and run > it pretty easily on a regular PC. I have put together a document to > help with that: > > https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md > > It also shows how to proxy a host UVC camera into the guest and use > some of the "fake" devices I talked about earlier. Ok, I'll look on it, thanks for the hint! > If following these > steps is too much friction, just reading through the document should > give you a decent idea of
Re: [PATCH v3] media: add virtio-media driver
Em Tue, 27 May 2025 10:23:32 -0400 "Michael S. Tsirkin" escreveu: > On Mon, May 26, 2025 at 02:13:16PM +0200, Mauro Carvalho Chehab wrote: > > Hi Michael, > > > > Em Sat, 12 Apr 2025 13:08:01 +0900 > > Alexandre Courbot escreveu: > > > > > Add the first version of the virtio-media driver. > > > > > > This driver acts roughly as a V4L2 relay between user-space and the > > > virtio virtual device on the host, so it is relatively simple, yet > > > unconventional. It doesn't use VB2 or other frameworks typically used in > > > a V4L2 driver, and most of its complexity resides in correctly and > > > efficiently building the virtio descriptor chain to pass to the host, > > > avoiding copies whenever possible. This is done by > > > scatterlist_builder.[ch]. > > > > > > virtio_media_ioctls.c proxies each supported ioctl to the host, using > > > code generated through macros for ioctls that can be forwarded directly, > > > which is most of them. > > > > > > virtio_media_driver.c provides the expected driver hooks, and support > > > for mmapping and polling. > > > > > > This version supports MMAP buffers, while USERPTR buffers can also be > > > enabled through a driver option. DMABUF support is still pending. > > > > It sounds that you applied this one at the virtio tree, but it hasn't > > being reviewed or acked by media maintainers. > > > > Please drop it. > > > > Alexandre, > > > > Please send media patches to media maintainers, c/c other subsystem > > maintainers, as otherwise they might end being merged without a > > proper review. > > > > In this particular case, we need to double-check if this won't cause > > any issues, in special with regards to media locks and mutexes. > > > > I'll try to look on it after this merge window, as it is too late > > for it to be applied during this one. > > > > Regards, > > Mauro > > New drivers generally can be merged during the merge window, > especially early. Sure, but this one was not reviewed or tested yet by media maintainers, nor its submission came with the tests from the regression tool we use (v4l2-compliance). In particular, we need to double-check if it won't cause any issues with the complex set of mutexes and spinlocks that we have within the core. There is an additional concern related to V4L2: on media, only one process is allowed to have exclusive streaming access to the device: all other opens on the same device get permission denied (by default - there is an optional ioctl that allows a process to "abdicate" its streaming rights). We need to double-check how this is implemented and how this would behavior when multiple VMs have the driver installed and might try to use (or not), and how this would affect the host access to the device. There are also some coding style issues that cause our CI to complain. Those are minor and could be fixed by a separate patch, but better to have them placed altogether as otherwise our CI will keep complaining about until the fix is merged. On other words, this driver is not ready for merge yet. We need some time to test and review it properly. > It's up to you though. > I can keep it in next for now, so it gets some coverage by > tools scanning that tree. Sure, feel free to keep it on next if you prefer so. Just please don't submit it upstream while we don't review and properly test it. Thanks! Mauro
Re: [PATCH v3] media: add virtio-media driver
On Mon, May 26, 2025 at 02:13:16PM +0200, Mauro Carvalho Chehab wrote: > Hi Michael, > > Em Sat, 12 Apr 2025 13:08:01 +0900 > Alexandre Courbot escreveu: > > > Add the first version of the virtio-media driver. > > > > This driver acts roughly as a V4L2 relay between user-space and the > > virtio virtual device on the host, so it is relatively simple, yet > > unconventional. It doesn't use VB2 or other frameworks typically used in > > a V4L2 driver, and most of its complexity resides in correctly and > > efficiently building the virtio descriptor chain to pass to the host, > > avoiding copies whenever possible. This is done by > > scatterlist_builder.[ch]. > > > > virtio_media_ioctls.c proxies each supported ioctl to the host, using > > code generated through macros for ioctls that can be forwarded directly, > > which is most of them. > > > > virtio_media_driver.c provides the expected driver hooks, and support > > for mmapping and polling. > > > > This version supports MMAP buffers, while USERPTR buffers can also be > > enabled through a driver option. DMABUF support is still pending. > > It sounds that you applied this one at the virtio tree, but it hasn't > being reviewed or acked by media maintainers. > > Please drop it. > > Alexandre, > > Please send media patches to media maintainers, c/c other subsystem > maintainers, as otherwise they might end being merged without a > proper review. > > In this particular case, we need to double-check if this won't cause > any issues, in special with regards to media locks and mutexes. > > I'll try to look on it after this merge window, as it is too late > for it to be applied during this one. > > Regards, > Mauro New drivers generally can be merged during the merge window, especially early. It's up to you though. I can keep it in next for now, so it gets some coverage by tools scanning that tree. > > > > Signed-off-by: Alexandre Courbot > > Signed-off-by: Alexandre Courbot > > --- > > This patch adds the virtio-media kernel driver. Virtio-media [1] > > encapsulates the V4L2 structures and protocol to enable the > > virtualization of host media devices into a guest. It's specification is > > in the final stages [2] of being merged and the virtualization of > > cameras and video accelerator devices has already been demonstrated > > using crosvm [3] and QEmu. v4l2-compliance also passes on all tested > > devices, which includes the "simple" virtual test device, proxied host > > UVC and vivid devices, and the FFmpeg virtual decoder devices (refer to > > [3] in order to test these if desired). > > > > Virtio-media is merged in AOSP [4] and ChromeOS. Upstreaming of the > > driver is overdue, but I hope we can start the review process and > > converge into something that can be merged. > > > > Limitations: > > > > - The driver is currently only available to little-endian, 64-bit > > kernels. This is because some of the V4L2 structures used for > > communication between guest and host have a layout dependent on the > > architecture, and the virtio-media protocol is standardized on the > > little-endian 64-bit versions. This can be fixed with a conversion > > layer similar to the one used to convert 32-bit ioctls to their 64-bit > > counterpart. > > - DMABUF support is currently missing. It should be implemented using > > virtio objects, with possible support for memfds using the > > SHARED_PAGES memory type. > > - No support for the media API and requests. While the use-case for > > these is less important on virtual devices where we want to present an > > abstraction as high as possible to limit VM exits, they do exist and > > it would be nice to add behind a virtio feature bit. > > - Locking in the driver is still very basic. This is something I want to > > improve before merging, but I didn't want to delay upstream review any > > further. > > > > [1] https://github.com/chromeos/virtio-media > > [2] > > https://lore.kernel.org/virtio-comment/[email protected]/ > > [3] https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md > > [4] https://android.googlesource.com/platform/external/virtio-media/ > > --- > > Changes in v3: > > - Rebased on top of v6.15-rc1 and removes obsolete control callbacks. > > - Link to v2: > > https://lore.kernel.org/r/[email protected] > > > > Changes in v2: > > - Fixed kernel test robot and media CI warnings (ignored a few false > > positives). > > - Changed in-driver email address to personal one since my Google one > > will soon become invalid. > > - Link to v1: > > https://lore.kernel.org/r/[email protected] > > --- > > MAINTAINERS|6 + > > drivers/media/Kconfig | 13 + > > drivers/media/Makefile |2 + > > drivers/media/virtio/Makefile |9 + > > drivers/media/virtio/protocol.h
Re: [PATCH v3] media: add virtio-media driver
On Tue, May 27, 2025 at 10:35 PM Mauro Carvalho Chehab wrote: > > Em Tue, 27 May 2025 22:21:42 +0900 > Alexandre Courbot escreveu: > > > On Tue, May 27, 2025 at 6:13 PM Mauro Carvalho Chehab > > wrote: > > > > > > Em Tue, 27 May 2025 15:14:50 +0900 > > > "Alexandre Courbot" escreveu: > > > > > > > Hi Mauro, > > > > > > > > On Mon May 26, 2025 at 9:13 PM JST, Mauro Carvalho Chehab wrote: > > > > > Hi Michael, > > > > > > > > > > Em Sat, 12 Apr 2025 13:08:01 +0900 > > > > > Alexandre Courbot escreveu: > > > > > > > > > >> Add the first version of the virtio-media driver. > > > > >> > > > > >> This driver acts roughly as a V4L2 relay between user-space and the > > > > >> virtio virtual device on the host, so it is relatively simple, yet > > > > >> unconventional. It doesn't use VB2 or other frameworks typically > > > > >> used in > > > > >> a V4L2 driver, and most of its complexity resides in correctly and > > > > >> efficiently building the virtio descriptor chain to pass to the host, > > > > >> avoiding copies whenever possible. This is done by > > > > >> scatterlist_builder.[ch]. > > > > >> > > > > >> virtio_media_ioctls.c proxies each supported ioctl to the host, using > > > > >> code generated through macros for ioctls that can be forwarded > > > > >> directly, > > > > >> which is most of them. > > > > >> > > > > >> virtio_media_driver.c provides the expected driver hooks, and support > > > > >> for mmapping and polling. > > > > >> > > > > >> This version supports MMAP buffers, while USERPTR buffers can also > > > > >> be > > > > >> enabled through a driver option. DMABUF support is still pending. > > > > > > > > > > It sounds that you applied this one at the virtio tree, but it hasn't > > > > > being reviewed or acked by media maintainers. > > > > > > > > > > Please drop it. > > > > > > > > > > Alexandre, > > > > > > > > > > Please send media patches to media maintainers, c/c other subsystem > > > > > maintainers, as otherwise they might end being merged without a > > > > > proper review. > > > > > > > > Sorry about that, I put everyone in "To:" without giving it a second > > > > thought. > > > > > > > > > > > > > > In this particular case, we need to double-check if this won't cause > > > > > any issues, in special with regards to media locks and mutexes. > > > > > > > > Agreed, I am not 100% confident about that part myself. > > > > > > > > > > > > > > I'll try to look on it after this merge window, as it is too late > > > > > for it to be applied during this one. > > > > > > > > Appreciate that - given the high traffic on the list I was worried that > > > > this patch would eventually be overlooked. Not making it for this merge > > > > window should not be a problem, so please take the time you need. > > > > > > Provided that your patch was caught by patchwork, it won't be lost: > > > > > > https://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/ > > > > > > Please notice that our CI got a number of checkpatch issues there. > > > Please check and fix the non-false-positive ones. > > > > Will do. The macro-related ones are false-positives AFAICT. Some of > > the "lines should not end with a '('" are actually the result of > > applying clang-format with the kernel-provided style... > > I don't know any lint tool that honors kernel style. The best one > is checkpatch with the auto-correcting parameter in pedantic mode, > but still one needs to manually review its output. > > > > > > > > > Btw, I was looking at: > > > > > > https://github.com/chromeos/virtio-media > > > > > > (I'm assuming that this is the QEMU counterpart, right?) > > > > crosvm actually, but QEMU support is also being worked on. > > Do you have already QEMU patches? The best is to have the Kernel driver > submitted altogether with QEMU, as Kernel developers need it to do the > tests. In my case, I never use crosvm, and I don't have any Chromebook > anymore. IIRC Albert Esteve was working on this, maybe he can share the current status. Note that crosvm does not require a Chromebook, you can build and run it pretty easily on a regular PC. I have put together a document to help with that: https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md It also shows how to proxy a host UVC camera into the guest and use some of the "fake" devices I talked about earlier. If following these steps is too much friction, just reading through the document should give you a decent idea of what virtio-media can do. > > > > And I noticed something weird there: > > > > > > "Unsupported ioctls > > > > > > A few ioctls are replaced by other, more suitable mechanisms. If > > > being requested these ioctls, the device must return the same response as > > > it would for an unknown ioctl, i.e. ENOTTY. > > > > > > VIDIOC_QUERYCAP is replaced by reading the configuration area. > > > VIDIOC_DQBUF is replaced by a dedicated event. > > >
Re: [PATCH v3] media: add virtio-media driver
Em Tue, 27 May 2025 22:21:42 +0900 Alexandre Courbot escreveu: > On Tue, May 27, 2025 at 6:13 PM Mauro Carvalho Chehab > wrote: > > > > Em Tue, 27 May 2025 15:14:50 +0900 > > "Alexandre Courbot" escreveu: > > > > > Hi Mauro, > > > > > > On Mon May 26, 2025 at 9:13 PM JST, Mauro Carvalho Chehab wrote: > > > > Hi Michael, > > > > > > > > Em Sat, 12 Apr 2025 13:08:01 +0900 > > > > Alexandre Courbot escreveu: > > > > > > > >> Add the first version of the virtio-media driver. > > > >> > > > >> This driver acts roughly as a V4L2 relay between user-space and the > > > >> virtio virtual device on the host, so it is relatively simple, yet > > > >> unconventional. It doesn't use VB2 or other frameworks typically used > > > >> in > > > >> a V4L2 driver, and most of its complexity resides in correctly and > > > >> efficiently building the virtio descriptor chain to pass to the host, > > > >> avoiding copies whenever possible. This is done by > > > >> scatterlist_builder.[ch]. > > > >> > > > >> virtio_media_ioctls.c proxies each supported ioctl to the host, using > > > >> code generated through macros for ioctls that can be forwarded > > > >> directly, > > > >> which is most of them. > > > >> > > > >> virtio_media_driver.c provides the expected driver hooks, and support > > > >> for mmapping and polling. > > > >> > > > >> This version supports MMAP buffers, while USERPTR buffers can also be > > > >> enabled through a driver option. DMABUF support is still pending. > > > > > > > > It sounds that you applied this one at the virtio tree, but it hasn't > > > > being reviewed or acked by media maintainers. > > > > > > > > Please drop it. > > > > > > > > Alexandre, > > > > > > > > Please send media patches to media maintainers, c/c other subsystem > > > > maintainers, as otherwise they might end being merged without a > > > > proper review. > > > > > > Sorry about that, I put everyone in "To:" without giving it a second > > > thought. > > > > > > > > > > > In this particular case, we need to double-check if this won't cause > > > > any issues, in special with regards to media locks and mutexes. > > > > > > Agreed, I am not 100% confident about that part myself. > > > > > > > > > > > I'll try to look on it after this merge window, as it is too late > > > > for it to be applied during this one. > > > > > > Appreciate that - given the high traffic on the list I was worried that > > > this patch would eventually be overlooked. Not making it for this merge > > > window should not be a problem, so please take the time you need. > > > > Provided that your patch was caught by patchwork, it won't be lost: > > > > https://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/ > > > > Please notice that our CI got a number of checkpatch issues there. > > Please check and fix the non-false-positive ones. > > Will do. The macro-related ones are false-positives AFAICT. Some of > the "lines should not end with a '('" are actually the result of > applying clang-format with the kernel-provided style... I don't know any lint tool that honors kernel style. The best one is checkpatch with the auto-correcting parameter in pedantic mode, but still one needs to manually review its output. > > > > > Btw, I was looking at: > > > > https://github.com/chromeos/virtio-media > > > > (I'm assuming that this is the QEMU counterpart, right?) > > crosvm actually, but QEMU support is also being worked on. Do you have already QEMU patches? The best is to have the Kernel driver submitted altogether with QEMU, as Kernel developers need it to do the tests. In my case, I never use crosvm, and I don't have any Chromebook anymore. > > And I noticed something weird there: > > > > "Unsupported ioctls > > > > A few ioctls are replaced by other, more suitable mechanisms. If > > being requested these ioctls, the device must return the same response as > > it would for an unknown ioctl, i.e. ENOTTY. > > > > VIDIOC_QUERYCAP is replaced by reading the configuration area. > > VIDIOC_DQBUF is replaced by a dedicated event. > > VIDIOC_DQEVENT is replaced by a dedicated event." > > > > While this could be ok for cromeOS, this will be broken for guests with > > Linux, as all Linux applications rely on VIDIOC_QUERYCAP and VIDIOC_DQBUF > > to work. Please implement support for it, as otherwise we won't even be > > able to test the driver with the v4l2-compliance tool (*). > > The phrasing was a bit confusing. The guest driver does support these > ioctls, and passes v4l2-compliance. So there is no problem here. Please add v4l2-compliance output on the next patch series. > Where these ioctls are not supported is between the guest and the > host, i.e. as ioctls encapsulated into a virtio request. For QUERYCAP, > that's because the virtio configuration area already provides the same > information. For DQBUF and DQEVENT, that'
Re: [PATCH v3] media: add virtio-media driver
On Tue, May 27, 2025 at 6:13 PM Mauro Carvalho Chehab wrote: > > Em Tue, 27 May 2025 15:14:50 +0900 > "Alexandre Courbot" escreveu: > > > Hi Mauro, > > > > On Mon May 26, 2025 at 9:13 PM JST, Mauro Carvalho Chehab wrote: > > > Hi Michael, > > > > > > Em Sat, 12 Apr 2025 13:08:01 +0900 > > > Alexandre Courbot escreveu: > > > > > >> Add the first version of the virtio-media driver. > > >> > > >> This driver acts roughly as a V4L2 relay between user-space and the > > >> virtio virtual device on the host, so it is relatively simple, yet > > >> unconventional. It doesn't use VB2 or other frameworks typically used in > > >> a V4L2 driver, and most of its complexity resides in correctly and > > >> efficiently building the virtio descriptor chain to pass to the host, > > >> avoiding copies whenever possible. This is done by > > >> scatterlist_builder.[ch]. > > >> > > >> virtio_media_ioctls.c proxies each supported ioctl to the host, using > > >> code generated through macros for ioctls that can be forwarded directly, > > >> which is most of them. > > >> > > >> virtio_media_driver.c provides the expected driver hooks, and support > > >> for mmapping and polling. > > >> > > >> This version supports MMAP buffers, while USERPTR buffers can also be > > >> enabled through a driver option. DMABUF support is still pending. > > > > > > It sounds that you applied this one at the virtio tree, but it hasn't > > > being reviewed or acked by media maintainers. > > > > > > Please drop it. > > > > > > Alexandre, > > > > > > Please send media patches to media maintainers, c/c other subsystem > > > maintainers, as otherwise they might end being merged without a > > > proper review. > > > > Sorry about that, I put everyone in "To:" without giving it a second > > thought. > > > > > > > > In this particular case, we need to double-check if this won't cause > > > any issues, in special with regards to media locks and mutexes. > > > > Agreed, I am not 100% confident about that part myself. > > > > > > > > I'll try to look on it after this merge window, as it is too late > > > for it to be applied during this one. > > > > Appreciate that - given the high traffic on the list I was worried that > > this patch would eventually be overlooked. Not making it for this merge > > window should not be a problem, so please take the time you need. > > Provided that your patch was caught by patchwork, it won't be lost: > > https://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/ > > Please notice that our CI got a number of checkpatch issues there. > Please check and fix the non-false-positive ones. Will do. The macro-related ones are false-positives AFAICT. Some of the "lines should not end with a '('" are actually the result of applying clang-format with the kernel-provided style... > > Btw, I was looking at: > > https://github.com/chromeos/virtio-media > > (I'm assuming that this is the QEMU counterpart, right?) crosvm actually, but QEMU support is also being worked on. > > And I noticed something weird there: > > "Unsupported ioctls > > A few ioctls are replaced by other, more suitable mechanisms. If > being requested these ioctls, the device must return the same response as it > would for an unknown ioctl, i.e. ENOTTY. > > VIDIOC_QUERYCAP is replaced by reading the configuration area. > VIDIOC_DQBUF is replaced by a dedicated event. > VIDIOC_DQEVENT is replaced by a dedicated event." > > While this could be ok for cromeOS, this will be broken for guests with > Linux, as all Linux applications rely on VIDIOC_QUERYCAP and VIDIOC_DQBUF > to work. Please implement support for it, as otherwise we won't even be > able to test the driver with the v4l2-compliance tool (*). The phrasing was a bit confusing. The guest driver does support these ioctls, and passes v4l2-compliance. So there is no problem here. Where these ioctls are not supported is between the guest and the host, i.e. as ioctls encapsulated into a virtio request. For QUERYCAP, that's because the virtio configuration area already provides the same information. For DQBUF and DQEVENT, that's because we want to serve these asynchronously for performance reasons (hence the dedicated events). But these differences don't affect guest user applications which will be able to perform these ioctls exactly as they expect. > > (*) Passing at v4l2-compliance is a requirement for any media driver > to be merged. > > With regards to testing, what's the expected testing scenario? > My guess is that, as a virtio device, a possible test scenario would be > to have the UVC camera from the host OS mapped using virtio-camera into > the guest OS, allowing a V4L2 application running at the guest to map the > camera from the host, right? That's one of the scenarios, yes. Another one is to expose an accelerated decoder on the host to the guest. One can also create "fake" devices
Re: [PATCH v3] media: add virtio-media driver
Em Tue, 27 May 2025 15:14:50 +0900 "Alexandre Courbot" escreveu: > Hi Mauro, > > On Mon May 26, 2025 at 9:13 PM JST, Mauro Carvalho Chehab wrote: > > Hi Michael, > > > > Em Sat, 12 Apr 2025 13:08:01 +0900 > > Alexandre Courbot escreveu: > > > >> Add the first version of the virtio-media driver. > >> > >> This driver acts roughly as a V4L2 relay between user-space and the > >> virtio virtual device on the host, so it is relatively simple, yet > >> unconventional. It doesn't use VB2 or other frameworks typically used in > >> a V4L2 driver, and most of its complexity resides in correctly and > >> efficiently building the virtio descriptor chain to pass to the host, > >> avoiding copies whenever possible. This is done by > >> scatterlist_builder.[ch]. > >> > >> virtio_media_ioctls.c proxies each supported ioctl to the host, using > >> code generated through macros for ioctls that can be forwarded directly, > >> which is most of them. > >> > >> virtio_media_driver.c provides the expected driver hooks, and support > >> for mmapping and polling. > >> > >> This version supports MMAP buffers, while USERPTR buffers can also be > >> enabled through a driver option. DMABUF support is still pending. > > > > It sounds that you applied this one at the virtio tree, but it hasn't > > being reviewed or acked by media maintainers. > > > > Please drop it. > > > > Alexandre, > > > > Please send media patches to media maintainers, c/c other subsystem > > maintainers, as otherwise they might end being merged without a > > proper review. > > Sorry about that, I put everyone in "To:" without giving it a second > thought. > > > > > In this particular case, we need to double-check if this won't cause > > any issues, in special with regards to media locks and mutexes. > > Agreed, I am not 100% confident about that part myself. > > > > > I'll try to look on it after this merge window, as it is too late > > for it to be applied during this one. > > Appreciate that - given the high traffic on the list I was worried that > this patch would eventually be overlooked. Not making it for this merge > window should not be a problem, so please take the time you need. Provided that your patch was caught by patchwork, it won't be lost: https://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/ Please notice that our CI got a number of checkpatch issues there. Please check and fix the non-false-positive ones. Btw, I was looking at: https://github.com/chromeos/virtio-media (I'm assuming that this is the QEMU counterpart, right?) And I noticed something weird there: "Unsupported ioctls A few ioctls are replaced by other, more suitable mechanisms. If being requested these ioctls, the device must return the same response as it would for an unknown ioctl, i.e. ENOTTY. VIDIOC_QUERYCAP is replaced by reading the configuration area. VIDIOC_DQBUF is replaced by a dedicated event. VIDIOC_DQEVENT is replaced by a dedicated event." While this could be ok for cromeOS, this will be broken for guests with Linux, as all Linux applications rely on VIDIOC_QUERYCAP and VIDIOC_DQBUF to work. Please implement support for it, as otherwise we won't even be able to test the driver with the v4l2-compliance tool (*). (*) Passing at v4l2-compliance is a requirement for any media driver to be merged. With regards to testing, what's the expected testing scenario? My guess is that, as a virtio device, a possible test scenario would be to have the UVC camera from the host OS mapped using virtio-camera into the guest OS, allowing a V4L2 application running at the guest to map the camera from the host, right? Regards, Mauro
Re: [PATCH v3] media: add virtio-media driver
Hi Mauro, On Mon May 26, 2025 at 9:13 PM JST, Mauro Carvalho Chehab wrote: > Hi Michael, > > Em Sat, 12 Apr 2025 13:08:01 +0900 > Alexandre Courbot escreveu: > >> Add the first version of the virtio-media driver. >> >> This driver acts roughly as a V4L2 relay between user-space and the >> virtio virtual device on the host, so it is relatively simple, yet >> unconventional. It doesn't use VB2 or other frameworks typically used in >> a V4L2 driver, and most of its complexity resides in correctly and >> efficiently building the virtio descriptor chain to pass to the host, >> avoiding copies whenever possible. This is done by >> scatterlist_builder.[ch]. >> >> virtio_media_ioctls.c proxies each supported ioctl to the host, using >> code generated through macros for ioctls that can be forwarded directly, >> which is most of them. >> >> virtio_media_driver.c provides the expected driver hooks, and support >> for mmapping and polling. >> >> This version supports MMAP buffers, while USERPTR buffers can also be >> enabled through a driver option. DMABUF support is still pending. > > It sounds that you applied this one at the virtio tree, but it hasn't > being reviewed or acked by media maintainers. > > Please drop it. > > Alexandre, > > Please send media patches to media maintainers, c/c other subsystem > maintainers, as otherwise they might end being merged without a > proper review. Sorry about that, I put everyone in "To:" without giving it a second thought. > > In this particular case, we need to double-check if this won't cause > any issues, in special with regards to media locks and mutexes. Agreed, I am not 100% confident about that part myself. > > I'll try to look on it after this merge window, as it is too late > for it to be applied during this one. Appreciate that - given the high traffic on the list I was worried that this patch would eventually be overlooked. Not making it for this merge window should not be a problem, so please take the time you need. Cheers, Alex.
Re: [PATCH v3] media: add virtio-media driver
Hi Michael, Em Sat, 12 Apr 2025 13:08:01 +0900 Alexandre Courbot escreveu: > Add the first version of the virtio-media driver. > > This driver acts roughly as a V4L2 relay between user-space and the > virtio virtual device on the host, so it is relatively simple, yet > unconventional. It doesn't use VB2 or other frameworks typically used in > a V4L2 driver, and most of its complexity resides in correctly and > efficiently building the virtio descriptor chain to pass to the host, > avoiding copies whenever possible. This is done by > scatterlist_builder.[ch]. > > virtio_media_ioctls.c proxies each supported ioctl to the host, using > code generated through macros for ioctls that can be forwarded directly, > which is most of them. > > virtio_media_driver.c provides the expected driver hooks, and support > for mmapping and polling. > > This version supports MMAP buffers, while USERPTR buffers can also be > enabled through a driver option. DMABUF support is still pending. It sounds that you applied this one at the virtio tree, but it hasn't being reviewed or acked by media maintainers. Please drop it. Alexandre, Please send media patches to media maintainers, c/c other subsystem maintainers, as otherwise they might end being merged without a proper review. In this particular case, we need to double-check if this won't cause any issues, in special with regards to media locks and mutexes. I'll try to look on it after this merge window, as it is too late for it to be applied during this one. Regards, Mauro > > Signed-off-by: Alexandre Courbot > Signed-off-by: Alexandre Courbot > --- > This patch adds the virtio-media kernel driver. Virtio-media [1] > encapsulates the V4L2 structures and protocol to enable the > virtualization of host media devices into a guest. It's specification is > in the final stages [2] of being merged and the virtualization of > cameras and video accelerator devices has already been demonstrated > using crosvm [3] and QEmu. v4l2-compliance also passes on all tested > devices, which includes the "simple" virtual test device, proxied host > UVC and vivid devices, and the FFmpeg virtual decoder devices (refer to > [3] in order to test these if desired). > > Virtio-media is merged in AOSP [4] and ChromeOS. Upstreaming of the > driver is overdue, but I hope we can start the review process and > converge into something that can be merged. > > Limitations: > > - The driver is currently only available to little-endian, 64-bit > kernels. This is because some of the V4L2 structures used for > communication between guest and host have a layout dependent on the > architecture, and the virtio-media protocol is standardized on the > little-endian 64-bit versions. This can be fixed with a conversion > layer similar to the one used to convert 32-bit ioctls to their 64-bit > counterpart. > - DMABUF support is currently missing. It should be implemented using > virtio objects, with possible support for memfds using the > SHARED_PAGES memory type. > - No support for the media API and requests. While the use-case for > these is less important on virtual devices where we want to present an > abstraction as high as possible to limit VM exits, they do exist and > it would be nice to add behind a virtio feature bit. > - Locking in the driver is still very basic. This is something I want to > improve before merging, but I didn't want to delay upstream review any > further. > > [1] https://github.com/chromeos/virtio-media > [2] > https://lore.kernel.org/virtio-comment/[email protected]/ > [3] https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md > [4] https://android.googlesource.com/platform/external/virtio-media/ > --- > Changes in v3: > - Rebased on top of v6.15-rc1 and removes obsolete control callbacks. > - Link to v2: > https://lore.kernel.org/r/[email protected] > > Changes in v2: > - Fixed kernel test robot and media CI warnings (ignored a few false > positives). > - Changed in-driver email address to personal one since my Google one > will soon become invalid. > - Link to v1: > https://lore.kernel.org/r/[email protected] > --- > MAINTAINERS|6 + > drivers/media/Kconfig | 13 + > drivers/media/Makefile |2 + > drivers/media/virtio/Makefile |9 + > drivers/media/virtio/protocol.h| 288 ++ > drivers/media/virtio/scatterlist_builder.c | 563 > drivers/media/virtio/scatterlist_builder.h | 111 +++ > drivers/media/virtio/session.h | 109 +++ > drivers/media/virtio/virtio_media.h| 93 ++ > drivers/media/virtio/virtio_media_driver.c | 959 > drivers/media/virtio/virtio_media_ioctls.c | 1297 > > include/uapi/linux/virtio_ids.h|1 + > 12 fi
Re: [PATCH v3] media: add virtio-media driver
…
> +++ b/drivers/media/virtio/virtio_media_driver.c
> @@ -0,0 +1,959 @@
…
> +static struct virtio_media_session *
> +virtio_media_session_alloc(struct virtio_media *vv, u32 id,
> +bool nonblocking_dequeue)
> +{
…
> + 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;
…
Under which circumstances would you become interested to apply a statement
like “guard(mutex)(&vv->sessions_lock);”?
https://elixir.bootlin.com/linux/v6.14-rc6/source/include/linux/mutex.h#L201
Regards,
Markus
