Linux virtualization list
 help / color / mirror / Atom feed
* Re: [PATCH v4 1/8] media: virtio: Add protocol
From: Brian Daniels @ 2026-06-25 20:24 UTC (permalink / raw)
  To: Bryan O'Donoghue, Mauro Carvalho Chehab
  Cc: Brian Daniels, acourbot, adelva, aesteve, changyeon,
	daniel.almeida, eperezma, gnurou, gurchetansingh, hverkuil,
	jasowang, linux-kernel, linux-media, mst, nicolas.dufresne,
	virtualization, xuanzhuo
In-Reply-To: <4b2078a0-9422-467b-b167-9a0b5444f18d@kernel.org>

> > +	u32 __reserved;
> 
> Knowing ~ nothing about virtio with ~ no experience of it I'll offer up 
> a question/statement anyway.
> 
> Aren't your reservations here a bit conservative ? Would you not be well 
> advised to reserve a bit more space per other virtio protocols ?
> 
> ➜   grep reserved ./include/uapi/linux/*.h | grep virtio
> ./include/uapi/linux/virtio_balloon.h: *         __u8 reserved[6];
> ./include/uapi/linux/virtio_balloon.h: * In other words, add explicit 
> reserved fields to align field and
> ./include/uapi/linux/virtio_blk.h:      __u8 reserved[38];
> ./include/uapi/linux/virtio_blk.h:      __u8 reserved[56];
> ./include/uapi/linux/virtio_config.h: * VIRTIO_TRANSPORT_F_END are 
> reserved for the transport
> ./include/uapi/linux/virtio_crypto.h:   __le32 reserved;
> ./include/uapi/linux/virtio_i2c.h: * Copyright (c) 2021 Intel 
> Corporation. All rights reserved.
> ./include/uapi/linux/virtio_input.h:    __u8    reserved[5];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved[3];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved[3];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved[3];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved[4];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved[8];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved[4];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved[3];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved[64];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved[3];
> ./include/uapi/linux/virtio_iommu.h:    __u8 
>         reserved2[4];
> ./include/uapi/linux/virtio_net.h:      __le16 reserved[4];
> ./include/uapi/linux/virtio_net.h:      __le16 reserved;
> ./include/uapi/linux/virtio_net.h:              __le16 reserved[3];
> ./include/uapi/linux/virtio_net.h:      __u8 reserved;
> ./include/uapi/linux/virtio_net.h:      __le16 reserved1;
> ./include/uapi/linux/virtio_pcidev.h: * @VIRTIO_PCIDEV_OP_RESERVED: 
> reserved to catch errors
> ./include/uapi/linux/virtio_pcidev.h: * @reserved: reserved
> ./include/uapi/linux/virtio_pcidev.h:   __u16 reserved;
> ./include/uapi/linux/virtio_pci.h:      * 2-65535 - reserved
> ./include/uapi/linux/virtio_pci.h:      /* Unused, reserved for future 
> extensions. */
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved1[12];
> ./include/uapi/linux/virtio_pci.h:      /* Unused, reserved for future 
> extensions. */
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved2[4];
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved[7];
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved[6];
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved[6];
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved[2];
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved[7];
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved[7];
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved;
> ./include/uapi/linux/virtio_pci.h:                      __le32 reserved;
> ./include/uapi/linux/virtio_pci.h:                      __u8 reserved[6];
> ./include/uapi/linux/virtio_pci.h:                      __le32 reserved;
> ./include/uapi/linux/virtio_pci.h:                      __le32 reserved;
> ./include/uapi/linux/virtio_pci.h:                      __le32 reserved;
> ./include/uapi/linux/virtio_pci.h:      __u8 reserved[7];
> ./include/uapi/linux/virtio_rtc.h: * Copyright (c) 2024 Qualcomm 
> Innovation Center, Inc. All rights reserved.
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[6];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[7];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[6];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[6];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[5];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[6];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[6];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[5];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[5];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[7];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[6];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[7];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[5];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[5];
> ./include/uapi/linux/virtio_rtc.h:      __u8 reserved[6];
> ./include/uapi/linux/virtio_spi.h: * Copyright (C) 2025 Qualcomm 
> Innovation Center, Inc. All rights reserved.
> ./include/uapi/linux/virtio_spi.h: *   other bits are reserved as 0, 
> 1-bit transfer is always supported.
> ./include/uapi/linux/virtio_spi.h: *   other bits are reserved as 0, 
> 1-bit transfer is always supported.
> ./include/uapi/linux/virtio_spi.h: * @reserved: for future use.
> ./include/uapi/linux/virtio_spi.h:      __u8 reserved[3];+      u32 
> __reserved;

I'm not an expert here, but taking a look at those files, the vast majority of
those reserved fields appear to be padding to ensure the struct has 64-bit
alignment, which matches the use here in virtio-media as well.

virtio_pci appears to be the only device that explicitly states the
reserved bytes are for future extensions. Unless there's a good a reason to
expect a future use case where more space is needed, I would prefer to not add
more at this time.

^ permalink raw reply

* Re: [PATCH v4 0/8] media: add virtio-media driver
From: Brian Daniels @ 2026-06-25 20:21 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Brian Daniels, Mauro Carvalho Chehab, acourbot, adelva, aesteve,
	changyeon, daniel.almeida, eperezma, gnurou, gurchetansingh,
	hverkuil, jasowang, linux-kernel, linux-media, nicolas.dufresne,
	virtualization, xuanzhuo
In-Reply-To: <20260622170857-mutt-send-email-mst@kernel.org>

> > From: Alexandre Courbot <gnurou@gmail.com>
> > 
> > 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].
> > 
> > This version supports MMAP buffers, while USERPTR buffers can also be
> > enabled through a driver option. DMABUF support is still pending.
> > 
> > NOTE: This depends on the VIRTIO ID being added in this patch:
> > https://lore.kernel.org/all/20260310-virtio-media-id-v1-1-be211bcf682b@redhat.com

I saw some CI build failures come back, but that should be resolved if the
patch above is merged. If you'd rather me add it to this patch series let me
know.

> > Signed-off-by: Alexandre Courbot <gnurou@gmail.com>
> > Co-developed-by: Brian Daniels <briandaniels@google.com>
> > Signed-off-by: Brian Daniels <briandaniels@google.com>
> 
> Thanks for the patches!
> Sent some comments on individual patches.

Thanks for the feedback!

I've started to prepare v5 based on the comments so far, I'll hold off on
sending it until we've resolved the other open threads.

> > ---
> > Guest Setup
> > 
> > Tests were ran on a Debian 12 guest running with crosvm. The guest image
> > was created with:
> > 
> > $ virt-builder debian-12 --root-password password:""
> > 
> > Build crosvm and launch the guest starting at the "Crosvm" section on
> > this page: https://github.com/chromeos/virtio-media/blob/main/TRY_IT_OUT.md#crosvm
> > 
> > NOTE: Before running v4l2-compliance in the guest, you need to install
> > v4l-utils and ffmpeg:
> > 
> > $ apt update && apt install v4l-utils ffmpeg
> > 
> > ---
> > Compliance Testing
> > 
> > This was tested using v4l2-compliance. Since virtio-media serves as
> > a proxy to host devices for the guest VMs, we expect the guest
> > compliance test to essentially match the host compliance test for the
> > same device.
> > 
> > NOTE: v4l2-compliance changes its test behavior depending on the driver
> > name. In the guest, the driver name for virtio-media proxied-devices is
> > always "virtio-media", even if the actual host device has a driver name
> > of e.g. "uvcvideo". To ensure the test is consistent between the host
> > and the guest, I created a patch for the v4l2-compliance tool that
> > allows you to override the driver name. All test results that follow use
> > this patch:
> > https://lore.kernel.org/r/20260528163448.4031965-1-briandaniels@google.com/
> > 
> > All tests used a Logitech USB Webcam C925e.
> > 
> > As tested on the host:
> > 
> > $ v4l2-compliance -d1 -s
> > 
> > v4l2-compliance 1.33.0-5471, 64 bits, 64-bit time_t
> > v4l2-compliance SHA: 9f2d3ea879ff 2026-05-28 14:45:11
> > 
> > Compliance test for uvcvideo device /dev/video1:
> > 
> > Driver Info:
> > 	Driver name      : uvcvideo
> > 	Card type        : Logitech Webcam C925e
> > 	Bus info         : usb-0000:04:00.1-3
> > 	Driver version   : 6.18.14
> > 	Capabilities     : 0x84a00001
> > 		Video Capture
> > 		Metadata Capture
> > 		Streaming
> > 		Extended Pix Format
> > 		Device Capabilities
> > 	Device Caps      : 0x04200001
> > 		Video Capture
> > 		Streaming
> > 		Extended Pix Format
> > Media Driver Info:
> > 	Driver name      : uvcvideo
> > 	Model            : Logitech Webcam C925e
> > 	Serial           : 686F371F
> > 	Bus info         : usb-0000:04:00.1-3
> > 	Media version    : 6.18.14
> > 	Hardware revision: 0x00000016 (22)
> > 	Driver version   : 6.18.14
> > Interface Info:
> > 	ID               : 0x03000002
> > 	Type             : V4L Video
> > Entity Info:
> > 	ID               : 0x00000001 (1)
> > 	Name             : Logitech Webcam C925e
> > 	Function         : V4L2 I/O
> > 	Flags            : default
> > 	Pad 0x01000007   : 0: Sink
> > 	  Link 0x0200001f: from remote pad 0x100000a of entity 'Processing 3' (Video Pixel Formatter): Data, Enabled, Immutable
> > 
> > Required ioctls:
> > 	test MC information (see 'Media Driver Info' above): OK
> > 	test VIDIOC_QUERYCAP: OK
> > 	test invalid ioctls: OK
> > 
> > Allow for multiple opens:
> > 	test second /dev/video1 open: OK
> > 	test VIDIOC_QUERYCAP: OK
> > 	test VIDIOC_G/S_PRIORITY: OK
> > 	test for unlimited opens: OK
> > 
> > Debug ioctls:
> > 	test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
> > 	test VIDIOC_LOG_STATUS: OK (Not Supported)
> > 
> > Input ioctls:
> > 	test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
> > 	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> > 	test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
> > 	test VIDIOC_ENUMAUDIO: OK (Not Supported)
> > 	test VIDIOC_G/S/ENUMINPUT: OK
> > 	test VIDIOC_G/S_AUDIO: OK (Not Supported)
> > 	Inputs: 1 Audio Inputs: 0 Tuners: 0
> > 
> > Output ioctls:
> > 	test VIDIOC_G/S_MODULATOR: OK (Not Supported)
> > 	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> > 	test VIDIOC_ENUMAUDOUT: OK (Not Supported)
> > 	test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
> > 	test VIDIOC_G/S_AUDOUT: OK (Not Supported)
> > 	Outputs: 0 Audio Outputs: 0 Modulators: 0
> > 
> > Input/Output configuration ioctls:
> > 	test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
> > 	test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
> > 	test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
> > 	test VIDIOC_G/S_EDID: OK (Not Supported)
> > 
> > Control ioctls (Input 0):
> > 	test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
> > 	test VIDIOC_QUERYCTRL: OK
> > 	test VIDIOC_G/S_CTRL: OK
> > 		fail: v4l2-test-controls.cpp(983): ret != EINVAL (got 13)
> > 	test VIDIOC_G/S/TRY_EXT_CTRLS: FAIL
> > 	test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
> > 	test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
> > 	Standard Controls: 19 Private Controls: 0
> > 
> > Format ioctls (Input 0):
> > 	test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
> > 	test VIDIOC_G/S_PARM: OK
> > 	test VIDIOC_G_FBUF: OK (Not Supported)
> > 	test VIDIOC_G_FMT: OK
> > 	test VIDIOC_TRY_FMT: OK
> > 	test VIDIOC_S_FMT: OK
> > 	test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
> > 	test Cropping: OK (Not Supported)
> > 	test Composing: OK (Not Supported)
> > 	test Scaling: OK (Not Supported)
> > 
> > Codec ioctls (Input 0):
> > 	test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
> > 	test VIDIOC_G_ENC_INDEX: OK (Not Supported)
> > 	test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
> > 
> > Buffer ioctls (Input 0):
> > 	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
> > 	test CREATE_BUFS maximum buffers: OK
> > 	test VIDIOC_REMOVE_BUFS: OK
> > 	test VIDIOC_EXPBUF: OK
> > 	test Requests: OK (Not Supported)
> > 	test blocking wait: OK
> > 
> > Test input 0:
> > 
> > Streaming ioctls:
> > 	test read/write: OK (Not Supported)
> > 
> > 	Video Capture: Frame #000
> > 	Video Capture: Frame #001
> > 	Video Capture: Frame #002
> > 	Video Capture: Frame #003
> > 	Video Capture: Frame #004
> > 	Video Capture: Frame #005
> > 	Video Capture: Frame #006
> > 	Video Capture: Frame #007
> > 	Video Capture: Frame #008
> > 	Video Capture: Frame #009
> > 	Video Capture: Frame #010
> > 	Video Capture: Frame #011
> > 	Video Capture: Frame #012
> > 	Video Capture: Frame #013
> > 	Video Capture: Frame #014
> > 	Video Capture: Frame #015
> > 	Video Capture: Frame #016
> > 	Video Capture: Frame #017
> > 	Video Capture: Frame #018
> > 	Video Capture: Frame #019
> > 	Video Capture: Frame #020
> > 	Video Capture: Frame #021
> > 	Video Capture: Frame #022
> > 	Video Capture: Frame #023
> > 	Video Capture: Frame #024
> > 	Video Capture: Frame #025
> > 	Video Capture: Frame #026
> > 	Video Capture: Frame #027
> > 	Video Capture: Frame #028
> > 	Video Capture: Frame #029
> > 	Video Capture: Frame #030
> > 	Video Capture: Frame #031
> > 	Video Capture: Frame #032
> > 	Video Capture: Frame #033
> > 	Video Capture: Frame #034
> > 	Video Capture: Frame #035
> > 	Video Capture: Frame #036
> > 	Video Capture: Frame #037
> > 	Video Capture: Frame #038
> > 	Video Capture: Frame #039
> > 	Video Capture: Frame #040
> > 	Video Capture: Frame #041
> > 	Video Capture: Frame #042
> > 	Video Capture: Frame #043
> > 	Video Capture: Frame #044
> > 	Video Capture: Frame #045
> > 	Video Capture: Frame #046
> > 	Video Capture: Frame #047
> > 	Video Capture: Frame #048
> > 	Video Capture: Frame #049
> > 	Video Capture: Frame #050
> > 	Video Capture: Frame #051
> > 	Video Capture: Frame #052
> > 	Video Capture: Frame #053
> > 	Video Capture: Frame #054
> > 	Video Capture: Frame #055
> > 	Video Capture: Frame #056
> > 	Video Capture: Frame #057
> > 	Video Capture: Frame #058
> > 	Video Capture: Frame #059
> > 
> > 	test MMAP (no poll, REQBUFS): OK
> > 
> > 	Video Capture: Frame #000 (select)
> > 	Video Capture: Frame #001 (select)
> > 	Video Capture: Frame #002 (select)
> > 	Video Capture: Frame #003 (select)
> > 	Video Capture: Frame #004 (select)
> > 	Video Capture: Frame #005 (select)
> > 	Video Capture: Frame #006 (select)
> > 	Video Capture: Frame #007 (select)
> > 	Video Capture: Frame #008 (select)
> > 	Video Capture: Frame #009 (select)
> > 	Video Capture: Frame #010 (select)
> > 	Video Capture: Frame #011 (select)
> > 	Video Capture: Frame #012 (select)
> > 	Video Capture: Frame #013 (select)
> > 	Video Capture: Frame #014 (select)
> > 	Video Capture: Frame #015 (select)
> > 	Video Capture: Frame #016 (select)
> > 	Video Capture: Frame #017 (select)
> > 	Video Capture: Frame #018 (select)
> > 	Video Capture: Frame #019 (select)
> > 	Video Capture: Frame #020 (select)
> > 	Video Capture: Frame #021 (select)
> > 	Video Capture: Frame #022 (select)
> > 	Video Capture: Frame #023 (select)
> > 	Video Capture: Frame #024 (select)
> > 	Video Capture: Frame #025 (select)
> > 	Video Capture: Frame #026 (select)
> > 	Video Capture: Frame #027 (select)
> > 	Video Capture: Frame #028 (select)
> > 	Video Capture: Frame #029 (select)
> > 	Video Capture: Frame #030 (select)
> > 	Video Capture: Frame #031 (select)
> > 	Video Capture: Frame #032 (select)
> > 	Video Capture: Frame #033 (select)
> > 	Video Capture: Frame #034 (select)
> > 	Video Capture: Frame #035 (select)
> > 	Video Capture: Frame #036 (select)
> > 	Video Capture: Frame #037 (select)
> > 	Video Capture: Frame #038 (select)
> > 	Video Capture: Frame #039 (select)
> > 	Video Capture: Frame #040 (select)
> > 	Video Capture: Frame #041 (select)
> > 	Video Capture: Frame #042 (select)
> > 	Video Capture: Frame #043 (select)
> > 	Video Capture: Frame #044 (select)
> > 	Video Capture: Frame #045 (select)
> > 	Video Capture: Frame #046 (select)
> > 	Video Capture: Frame #047 (select)
> > 	Video Capture: Frame #048 (select)
> > 	Video Capture: Frame #049 (select)
> > 	Video Capture: Frame #050 (select)
> > 	Video Capture: Frame #051 (select)
> > 	Video Capture: Frame #052 (select)
> > 	Video Capture: Frame #053 (select)
> > 	Video Capture: Frame #054 (select)
> > 	Video Capture: Frame #055 (select)
> > 	Video Capture: Frame #056 (select)
> > 	Video Capture: Frame #057 (select)
> > 	Video Capture: Frame #058 (select)
> > 	Video Capture: Frame #059 (select)
> > 
> > 	test MMAP (select, REQBUFS): OK
> > 
> > 	Video Capture: Frame #000 (epoll)
> > 	Video Capture: Frame #001 (epoll)
> > 	Video Capture: Frame #002 (epoll)
> > 	Video Capture: Frame #003 (epoll)
> > 	Video Capture: Frame #004 (epoll)
> > 	Video Capture: Frame #005 (epoll)
> > 	Video Capture: Frame #006 (epoll)
> > 	Video Capture: Frame #007 (epoll)
> > 	Video Capture: Frame #008 (epoll)
> > 	Video Capture: Frame #009 (epoll)
> > 	Video Capture: Frame #010 (epoll)
> > 	Video Capture: Frame #011 (epoll)
> > 	Video Capture: Frame #012 (epoll)
> > 	Video Capture: Frame #013 (epoll)
> > 	Video Capture: Frame #014 (epoll)
> > 	Video Capture: Frame #015 (epoll)
> > 	Video Capture: Frame #016 (epoll)
> > 	Video Capture: Frame #017 (epoll)
> > 	Video Capture: Frame #018 (epoll)
> > 	Video Capture: Frame #019 (epoll)
> > 	Video Capture: Frame #020 (epoll)
> > 	Video Capture: Frame #021 (epoll)
> > 	Video Capture: Frame #022 (epoll)
> > 	Video Capture: Frame #023 (epoll)
> > 	Video Capture: Frame #024 (epoll)
> > 	Video Capture: Frame #025 (epoll)
> > 	Video Capture: Frame #026 (epoll)
> > 	Video Capture: Frame #027 (epoll)
> > 	Video Capture: Frame #028 (epoll)
> > 	Video Capture: Frame #029 (epoll)
> > 	Video Capture: Frame #030 (epoll)
> > 	Video Capture: Frame #031 (epoll)
> > 	Video Capture: Frame #032 (epoll)
> > 	Video Capture: Frame #033 (epoll)
> > 	Video Capture: Frame #034 (epoll)
> > 	Video Capture: Frame #035 (epoll)
> > 	Video Capture: Frame #036 (epoll)
> > 	Video Capture: Frame #037 (epoll)
> > 	Video Capture: Frame #038 (epoll)
> > 	Video Capture: Frame #039 (epoll)
> > 	Video Capture: Frame #040 (epoll)
> > 	Video Capture: Frame #041 (epoll)
> > 	Video Capture: Frame #042 (epoll)
> > 	Video Capture: Frame #043 (epoll)
> > 	Video Capture: Frame #044 (epoll)
> > 	Video Capture: Frame #045 (epoll)
> > 	Video Capture: Frame #046 (epoll)
> > 	Video Capture: Frame #047 (epoll)
> > 	Video Capture: Frame #048 (epoll)
> > 	Video Capture: Frame #049 (epoll)
> > 	Video Capture: Frame #050 (epoll)
> > 	Video Capture: Frame #051 (epoll)
> > 	Video Capture: Frame #052 (epoll)
> > 	Video Capture: Frame #053 (epoll)
> > 	Video Capture: Frame #054 (epoll)
> > 	Video Capture: Frame #055 (epoll)
> > 	Video Capture: Frame #056 (epoll)
> > 	Video Capture: Frame #057 (epoll)
> > 	Video Capture: Frame #058 (epoll)
> > 	Video Capture: Frame #059 (epoll)
> > 
> > 	test MMAP (epoll, REQBUFS): OK
> > 
> > 	Video Capture: Frame #000
> > 	Video Capture: Frame #001
> > 	Video Capture: Frame #002
> > 	Video Capture: Frame #003
> > 	Video Capture: Frame #004
> > 	Video Capture: Frame #005
> > 	Video Capture: Frame #006
> > 	Video Capture: Frame #007
> > 	Video Capture: Frame #008
> > 	Video Capture: Frame #009
> > 	Video Capture: Frame #010
> > 	Video Capture: Frame #011
> > 	Video Capture: Frame #012
> > 	Video Capture: Frame #013
> > 	Video Capture: Frame #014
> > 	Video Capture: Frame #015
> > 	Video Capture: Frame #016
> > 	Video Capture: Frame #017
> > 	Video Capture: Frame #018
> > 	Video Capture: Frame #019
> > 	Video Capture: Frame #020
> > 	Video Capture: Frame #021
> > 	Video Capture: Frame #022
> > 	Video Capture: Frame #023
> > 	Video Capture: Frame #024
> > 	Video Capture: Frame #025
> > 	Video Capture: Frame #026
> > 	Video Capture: Frame #027
> > 	Video Capture: Frame #028
> > 	Video Capture: Frame #029
> > 	Video Capture: Frame #030
> > 	Video Capture: Frame #031
> > 	Video Capture: Frame #032
> > 	Video Capture: Frame #033
> > 	Video Capture: Frame #034
> > 	Video Capture: Frame #035
> > 	Video Capture: Frame #036
> > 	Video Capture: Frame #037
> > 	Video Capture: Frame #038
> > 	Video Capture: Frame #039
> > 	Video Capture: Frame #040
> > 	Video Capture: Frame #041
> > 	Video Capture: Frame #042
> > 	Video Capture: Frame #043
> > 	Video Capture: Frame #044
> > 	Video Capture: Frame #045
> > 	Video Capture: Frame #046
> > 	Video Capture: Frame #047
> > 	Video Capture: Frame #048
> > 	Video Capture: Frame #049
> > 	Video Capture: Frame #050
> > 	Video Capture: Frame #051
> > 	Video Capture: Frame #052
> > 	Video Capture: Frame #053
> > 	Video Capture: Frame #054
> > 	Video Capture: Frame #055
> > 	Video Capture: Frame #056
> > 	Video Capture: Frame #057
> > 	Video Capture: Frame #058
> > 	Video Capture: Frame #059
> > 
> > 	test MMAP (no poll, CREATE_BUFS): OK
> > 
> > 	Video Capture: Frame #000 (select)
> > 	Video Capture: Frame #001 (select)
> > 	Video Capture: Frame #002 (select)
> > 	Video Capture: Frame #003 (select)
> > 	Video Capture: Frame #004 (select)
> > 	Video Capture: Frame #005 (select)
> > 	Video Capture: Frame #006 (select)
> > 	Video Capture: Frame #007 (select)
> > 	Video Capture: Frame #008 (select)
> > 	Video Capture: Frame #009 (select)
> > 	Video Capture: Frame #010 (select)
> > 	Video Capture: Frame #011 (select)
> > 	Video Capture: Frame #012 (select)
> > 	Video Capture: Frame #013 (select)
> > 	Video Capture: Frame #014 (select)
> > 	Video Capture: Frame #015 (select)
> > 	Video Capture: Frame #016 (select)
> > 	Video Capture: Frame #017 (select)
> > 	Video Capture: Frame #018 (select)
> > 	Video Capture: Frame #019 (select)
> > 	Video Capture: Frame #020 (select)
> > 	Video Capture: Frame #021 (select)
> > 	Video Capture: Frame #022 (select)
> > 	Video Capture: Frame #023 (select)
> > 	Video Capture: Frame #024 (select)
> > 	Video Capture: Frame #025 (select)
> > 	Video Capture: Frame #026 (select)
> > 	Video Capture: Frame #027 (select)
> > 	Video Capture: Frame #028 (select)
> > 	Video Capture: Frame #029 (select)
> > 	Video Capture: Frame #030 (select)
> > 	Video Capture: Frame #031 (select)
> > 	Video Capture: Frame #032 (select)
> > 	Video Capture: Frame #033 (select)
> > 	Video Capture: Frame #034 (select)
> > 	Video Capture: Frame #035 (select)
> > 	Video Capture: Frame #036 (select)
> > 	Video Capture: Frame #037 (select)
> > 	Video Capture: Frame #038 (select)
> > 	Video Capture: Frame #039 (select)
> > 	Video Capture: Frame #040 (select)
> > 	Video Capture: Frame #041 (select)
> > 	Video Capture: Frame #042 (select)
> > 	Video Capture: Frame #043 (select)
> > 	Video Capture: Frame #044 (select)
> > 	Video Capture: Frame #045 (select)
> > 	Video Capture: Frame #046 (select)
> > 	Video Capture: Frame #047 (select)
> > 	Video Capture: Frame #048 (select)
> > 	Video Capture: Frame #049 (select)
> > 	Video Capture: Frame #050 (select)
> > 	Video Capture: Frame #051 (select)
> > 	Video Capture: Frame #052 (select)
> > 	Video Capture: Frame #053 (select)
> > 	Video Capture: Frame #054 (select)
> > 	Video Capture: Frame #055 (select)
> > 	Video Capture: Frame #056 (select)
> > 	Video Capture: Frame #057 (select)
> > 	Video Capture: Frame #058 (select)
> > 	Video Capture: Frame #059 (select)
> > 
> > 	test MMAP (select, CREATE_BUFS): OK
> > 
> > 	Video Capture: Frame #000 (epoll)
> > 	Video Capture: Frame #001 (epoll)
> > 	Video Capture: Frame #002 (epoll)
> > 	Video Capture: Frame #003 (epoll)
> > 	Video Capture: Frame #004 (epoll)
> > 	Video Capture: Frame #005 (epoll)
> > 	Video Capture: Frame #006 (epoll)
> > 	Video Capture: Frame #007 (epoll)
> > 	Video Capture: Frame #008 (epoll)
> > 	Video Capture: Frame #009 (epoll)
> > 	Video Capture: Frame #010 (epoll)
> > 	Video Capture: Frame #011 (epoll)
> > 	Video Capture: Frame #012 (epoll)
> > 	Video Capture: Frame #013 (epoll)
> > 	Video Capture: Frame #014 (epoll)
> > 	Video Capture: Frame #015 (epoll)
> > 	Video Capture: Frame #016 (epoll)
> > 	Video Capture: Frame #017 (epoll)
> > 	Video Capture: Frame #018 (epoll)
> > 	Video Capture: Frame #019 (epoll)
> > 	Video Capture: Frame #020 (epoll)
> > 	Video Capture: Frame #021 (epoll)
> > 	Video Capture: Frame #022 (epoll)
> > 	Video Capture: Frame #023 (epoll)
> > 	Video Capture: Frame #024 (epoll)
> > 	Video Capture: Frame #025 (epoll)
> > 	Video Capture: Frame #026 (epoll)
> > 	Video Capture: Frame #027 (epoll)
> > 	Video Capture: Frame #028 (epoll)
> > 	Video Capture: Frame #029 (epoll)
> > 	Video Capture: Frame #030 (epoll)
> > 	Video Capture: Frame #031 (epoll)
> > 	Video Capture: Frame #032 (epoll)
> > 	Video Capture: Frame #033 (epoll)
> > 	Video Capture: Frame #034 (epoll)
> > 	Video Capture: Frame #035 (epoll)
> > 	Video Capture: Frame #036 (epoll)
> > 	Video Capture: Frame #037 (epoll)
> > 	Video Capture: Frame #038 (epoll)
> > 	Video Capture: Frame #039 (epoll)
> > 	Video Capture: Frame #040 (epoll)
> > 	Video Capture: Frame #041 (epoll)
> > 	Video Capture: Frame #042 (epoll)
> > 	Video Capture: Frame #043 (epoll)
> > 	Video Capture: Frame #044 (epoll)
> > 	Video Capture: Frame #045 (epoll)
> > 	Video Capture: Frame #046 (epoll)
> > 	Video Capture: Frame #047 (epoll)
> > 	Video Capture: Frame #048 (epoll)
> > 	Video Capture: Frame #049 (epoll)
> > 	Video Capture: Frame #050 (epoll)
> > 	Video Capture: Frame #051 (epoll)
> > 	Video Capture: Frame #052 (epoll)
> > 	Video Capture: Frame #053 (epoll)
> > 	Video Capture: Frame #054 (epoll)
> > 	Video Capture: Frame #055 (epoll)
> > 	Video Capture: Frame #056 (epoll)
> > 	Video Capture: Frame #057 (epoll)
> > 	Video Capture: Frame #058 (epoll)
> > 	Video Capture: Frame #059 (epoll)
> > 
> > 	test MMAP (epoll, CREATE_BUFS): OK
> > 
> > 	Video Capture: Frame #000
> > 	Video Capture: Frame #001
> > 	Video Capture: Frame #002
> > 	Video Capture: Frame #003
> > 	Video Capture: Frame #004
> > 	Video Capture: Frame #005
> > 	Video Capture: Frame #006
> > 	Video Capture: Frame #007
> > 	Video Capture: Frame #008
> > 	Video Capture: Frame #009
> > 	Video Capture: Frame #010
> > 	Video Capture: Frame #011
> > 	Video Capture: Frame #012
> > 	Video Capture: Frame #013
> > 	Video Capture: Frame #014
> > 	Video Capture: Frame #015
> > 	Video Capture: Frame #016
> > 	Video Capture: Frame #017
> > 	Video Capture: Frame #018
> > 	Video Capture: Frame #019
> > 	Video Capture: Frame #020
> > 	Video Capture: Frame #021
> > 	Video Capture: Frame #022
> > 	Video Capture: Frame #023
> > 	Video Capture: Frame #024
> > 	Video Capture: Frame #025
> > 	Video Capture: Frame #026
> > 	Video Capture: Frame #027
> > 	Video Capture: Frame #028
> > 	Video Capture: Frame #029
> > 	Video Capture: Frame #030
> > 	Video Capture: Frame #031
> > 	Video Capture: Frame #032
> > 	Video Capture: Frame #033
> > 	Video Capture: Frame #034
> > 	Video Capture: Frame #035
> > 	Video Capture: Frame #036
> > 	Video Capture: Frame #037
> > 	Video Capture: Frame #038
> > 	Video Capture: Frame #039
> > 	Video Capture: Frame #040
> > 	Video Capture: Frame #041
> > 	Video Capture: Frame #042
> > 	Video Capture: Frame #043
> > 	Video Capture: Frame #044
> > 	Video Capture: Frame #045
> > 	Video Capture: Frame #046
> > 	Video Capture: Frame #047
> > 	Video Capture: Frame #048
> > 	Video Capture: Frame #049
> > 	Video Capture: Frame #050
> > 	Video Capture: Frame #051
> > 	Video Capture: Frame #052
> > 	Video Capture: Frame #053
> > 	Video Capture: Frame #054
> > 	Video Capture: Frame #055
> > 	Video Capture: Frame #056
> > 	Video Capture: Frame #057
> > 	Video Capture: Frame #058
> > 	Video Capture: Frame #059
> > 
> > 	test USERPTR (no poll): OK
> > 
> > 	Video Capture: Frame #000 (select)
> > 	Video Capture: Frame #001 (select)
> > 	Video Capture: Frame #002 (select)
> > 	Video Capture: Frame #003 (select)
> > 	Video Capture: Frame #004 (select)
> > 	Video Capture: Frame #005 (select)
> > 	Video Capture: Frame #006 (select)
> > 	Video Capture: Frame #007 (select)
> > 	Video Capture: Frame #008 (select)
> > 	Video Capture: Frame #009 (select)
> > 	Video Capture: Frame #010 (select)
> > 	Video Capture: Frame #011 (select)
> > 	Video Capture: Frame #012 (select)
> > 	Video Capture: Frame #013 (select)
> > 	Video Capture: Frame #014 (select)
> > 	Video Capture: Frame #015 (select)
> > 	Video Capture: Frame #016 (select)
> > 	Video Capture: Frame #017 (select)
> > 	Video Capture: Frame #018 (select)
> > 	Video Capture: Frame #019 (select)
> > 	Video Capture: Frame #020 (select)
> > 	Video Capture: Frame #021 (select)
> > 	Video Capture: Frame #022 (select)
> > 	Video Capture: Frame #023 (select)
> > 	Video Capture: Frame #024 (select)
> > 	Video Capture: Frame #025 (select)
> > 	Video Capture: Frame #026 (select)
> > 	Video Capture: Frame #027 (select)
> > 	Video Capture: Frame #028 (select)
> > 	Video Capture: Frame #029 (select)
> > 	Video Capture: Frame #030 (select)
> > 	Video Capture: Frame #031 (select)
> > 	Video Capture: Frame #032 (select)
> > 	Video Capture: Frame #033 (select)
> > 	Video Capture: Frame #034 (select)
> > 	Video Capture: Frame #035 (select)
> > 	Video Capture: Frame #036 (select)
> > 	Video Capture: Frame #037 (select)
> > 	Video Capture: Frame #038 (select)
> > 	Video Capture: Frame #039 (select)
> > 	Video Capture: Frame #040 (select)
> > 	Video Capture: Frame #041 (select)
> > 	Video Capture: Frame #042 (select)
> > 	Video Capture: Frame #043 (select)
> > 	Video Capture: Frame #044 (select)
> > 	Video Capture: Frame #045 (select)
> > 	Video Capture: Frame #046 (select)
> > 	Video Capture: Frame #047 (select)
> > 	Video Capture: Frame #048 (select)
> > 	Video Capture: Frame #049 (select)
> > 	Video Capture: Frame #050 (select)
> > 	Video Capture: Frame #051 (select)
> > 	Video Capture: Frame #052 (select)
> > 	Video Capture: Frame #053 (select)
> > 	Video Capture: Frame #054 (select)
> > 	Video Capture: Frame #055 (select)
> > 	Video Capture: Frame #056 (select)
> > 	Video Capture: Frame #057 (select)
> > 	Video Capture: Frame #058 (select)
> > 	Video Capture: Frame #059 (select)
> > 
> > 	test USERPTR (select): OK
> > 	test DMABUF: Cannot test, specify --expbuf-device
> > 
> > Total for uvcvideo device /dev/video1: 58, Succeeded: 57, Failed: 1, Warnings: 0
> > 
> > As tested on the guest:
> > 
> > $ v4l2-compliance -d0 -s --driver-name uvcvideo
> > 
> > v4l2-compliance 1.33.0-5457, 64 bits, 64-bit time_t
> > v4l2-compliance SHA: e7e240f546f3 2026-05-28 17:06:12
> > 
> > Compliance test for uvcvideo device (overridden from virtio-media) /dev/video0:
> > 
> > Driver Info:
> > 	Driver name      : uvcvideo
> > 	Card type        : Logitech Webcam C925e
> > 	Bus info         : platform:virtio-media
> > 	Driver version   : 7.1.0
> > 	Capabilities     : 0x84200001
> > 		Video Capture
> > 		Streaming
> > 		Extended Pix Format
> > 		Device Capabilities
> > 	Device Caps      : 0x04200001
> > 		Video Capture
> > 		Streaming
> > 		Extended Pix Format
> > 
> > Required ioctls:
> > 	test VIDIOC_QUERYCAP: OK
> > 	test invalid ioctls: OK
> > 
> > Allow for multiple opens:
> > 	test second /dev/video0 open: OK
> > 	test VIDIOC_QUERYCAP: OK
> > 	test VIDIOC_G/S_PRIORITY: OK
> > 	test for unlimited opens: OK
> > 
> > Debug ioctls:
> > 	test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
> > 	test VIDIOC_LOG_STATUS: OK (Not Supported)
> > 
> > Input ioctls:
> > 	test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
> > 	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> > 	test VIDIOC_S_HW_FREQ_SEEK: OK
> > 	test VIDIOC_ENUMAUDIO: OK (Not Supported)
> > 	test VIDIOC_G/S/ENUMINPUT: OK
> > 	test VIDIOC_G/S_AUDIO: OK (Not Supported)
> > 	Inputs: 1 Audio Inputs: 0 Tuners: 0
> > 
> > Output ioctls:
> > 	test VIDIOC_G/S_MODULATOR: OK (Not Supported)
> > 	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> > 	test VIDIOC_ENUMAUDOUT: OK (Not Supported)
> > 	test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
> > 	test VIDIOC_G/S_AUDOUT: OK (Not Supported)
> > 	Outputs: 0 Audio Outputs: 0 Modulators: 0
> > 
> > Input/Output configuration ioctls:
> > 	test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
> > 	test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
> > 	test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
> > 	test VIDIOC_G/S_EDID: OK (Not Supported)
> > 
> > Control ioctls (Input 0):
> > 	test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
> > 	test VIDIOC_QUERYCTRL: OK
> > 	test VIDIOC_G/S_CTRL: OK
> > 		fail: v4l2-test-controls.cpp(981): ret (got 22)
> > 	test VIDIOC_G/S/TRY_EXT_CTRLS: FAIL
> > 	test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
> > 	test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
> > 	Standard Controls: 19 Private Controls: 0
> > 
> > Format ioctls (Input 0):
> > 	test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
> > 	test VIDIOC_G/S_PARM: OK
> > 	test VIDIOC_G_FBUF: OK (Not Supported)
> > 	test VIDIOC_G_FMT: OK
> > 	test VIDIOC_TRY_FMT: OK
> > 	test VIDIOC_S_FMT: OK
> > 	test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
> > 	test Cropping: OK (Not Supported)
> > 	test Composing: OK (Not Supported)
> > 	test Scaling: OK (Not Supported)
> > 
> > Codec ioctls (Input 0):
> > 	test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
> > 	test VIDIOC_G_ENC_INDEX: OK (Not Supported)
> > 	test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
> > 
> > Buffer ioctls (Input 0):
> > 	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
> > 	test CREATE_BUFS maximum buffers: OK
> > 	test VIDIOC_REMOVE_BUFS: OK
> > 	test VIDIOC_EXPBUF: OK (Not Supported)
> > 	test Requests: OK (Not Supported)
> > 	test blocking wait: OK
> > 
> > Test input 0:
> > 
> > Streaming ioctls:
> > 	test read/write: OK (Not Supported)
> > 
> > 	Video Capture: Frame #000
> > 	Video Capture: Frame #001
> > 	Video Capture: Frame #002
> > 	Video Capture: Frame #003
> > 	Video Capture: Frame #004
> > 	Video Capture: Frame #005
> > 	Video Capture: Frame #006
> > 	Video Capture: Frame #007
> > 	Video Capture: Frame #008
> > 	Video Capture: Frame #009
> > 	Video Capture: Frame #010
> > 	Video Capture: Frame #011
> > 	Video Capture: Frame #012
> > 	Video Capture: Frame #013
> > 	Video Capture: Frame #014
> > 	Video Capture: Frame #015
> > 	Video Capture: Frame #016
> > 	Video Capture: Frame #017
> > 	Video Capture: Frame #018
> > 	Video Capture: Frame #019
> > 	Video Capture: Frame #020
> > 	Video Capture: Frame #021
> > 	Video Capture: Frame #022
> > 	Video Capture: Frame #023
> > 	Video Capture: Frame #024
> > 	Video Capture: Frame #025
> > 	Video Capture: Frame #026
> > 	Video Capture: Frame #027
> > 	Video Capture: Frame #028
> > 	Video Capture: Frame #029
> > 	Video Capture: Frame #030
> > 	Video Capture: Frame #031
> > 	Video Capture: Frame #032
> > 	Video Capture: Frame #033
> > 	Video Capture: Frame #034
> > 	Video Capture: Frame #035
> > 	Video Capture: Frame #036
> > 	Video Capture: Frame #037
> > 	Video Capture: Frame #038
> > 	Video Capture: Frame #039
> > 	Video Capture: Frame #040
> > 	Video Capture: Frame #041
> > 	Video Capture: Frame #042
> > 	Video Capture: Frame #043
> > 	Video Capture: Frame #044
> > 	Video Capture: Frame #045
> > 	Video Capture: Frame #046
> > 	Video Capture: Frame #047
> > 	Video Capture: Frame #048
> > 	Video Capture: Frame #049
> > 	Video Capture: Frame #050
> > 	Video Capture: Frame #051
> > 	Video Capture: Frame #052
> > 	Video Capture: Frame #053
> > 	Video Capture: Frame #054
> > 	Video Capture: Frame #055
> > 	Video Capture: Frame #056
> > 	Video Capture: Frame #057
> > 	Video Capture: Frame #058
> > 	Video Capture: Frame #059
> > 
> > 	test MMAP (no poll, REQBUFS): OK
> > 
> > 	Video Capture: Frame #000 (select)
> > 	Video Capture: Frame #001 (select)
> > 	Video Capture: Frame #002 (select)
> > 	Video Capture: Frame #003 (select)
> > 	Video Capture: Frame #004 (select)
> > 	Video Capture: Frame #005 (select)
> > 	Video Capture: Frame #006 (select)
> > 	Video Capture: Frame #007 (select)
> > 	Video Capture: Frame #008 (select)
> > 	Video Capture: Frame #009 (select)
> > 	Video Capture: Frame #010 (select)
> > 	Video Capture: Frame #011 (select)
> > 	Video Capture: Frame #012 (select)
> > 	Video Capture: Frame #013 (select)
> > 	Video Capture: Frame #014 (select)
> > 	Video Capture: Frame #015 (select)
> > 	Video Capture: Frame #016 (select)
> > 	Video Capture: Frame #017 (select)
> > 	Video Capture: Frame #018 (select)
> > 	Video Capture: Frame #019 (select)
> > 	Video Capture: Frame #020 (select)
> > 	Video Capture: Frame #021 (select)
> > 	Video Capture: Frame #022 (select)
> > 	Video Capture: Frame #023 (select)
> > 	Video Capture: Frame #024 (select)
> > 	Video Capture: Frame #025 (select)
> > 	Video Capture: Frame #026 (select)
> > 	Video Capture: Frame #027 (select)
> > 	Video Capture: Frame #028 (select)
> > 	Video Capture: Frame #029 (select)
> > 	Video Capture: Frame #030 (select)
> > 	Video Capture: Frame #031 (select)
> > 	Video Capture: Frame #032 (select)
> > 	Video Capture: Frame #033 (select)
> > 	Video Capture: Frame #034 (select)
> > 	Video Capture: Frame #035 (select)
> > 	Video Capture: Frame #036 (select)
> > 	Video Capture: Frame #037 (select)
> > 	Video Capture: Frame #038 (select)
> > 	Video Capture: Frame #039 (select)
> > 	Video Capture: Frame #040 (select)
> > 	Video Capture: Frame #041 (select)
> > 	Video Capture: Frame #042 (select)
> > 	Video Capture: Frame #043 (select)
> > 	Video Capture: Frame #044 (select)
> > 	Video Capture: Frame #045 (select)
> > 	Video Capture: Frame #046 (select)
> > 	Video Capture: Frame #047 (select)
> > 	Video Capture: Frame #048 (select)
> > 	Video Capture: Frame #049 (select)
> > 	Video Capture: Frame #050 (select)
> > 	Video Capture: Frame #051 (select)
> > 	Video Capture: Frame #052 (select)
> > 	Video Capture: Frame #053 (select)
> > 	Video Capture: Frame #054 (select)
> > 	Video Capture: Frame #055 (select)
> > 	Video Capture: Frame #056 (select)
> > 	Video Capture: Frame #057 (select)
> > 	Video Capture: Frame #058 (select)
> > 	Video Capture: Frame #059 (select)
> > 
> > 	test MMAP (select, REQBUFS): OK
> > 
> > 	Video Capture: Frame #000 (epoll)
> > 	Video Capture: Frame #001 (epoll)
> > 	Video Capture: Frame #002 (epoll)
> > 	Video Capture: Frame #003 (epoll)
> > 	Video Capture: Frame #004 (epoll)
> > 	Video Capture: Frame #005 (epoll)
> > 	Video Capture: Frame #006 (epoll)
> > 	Video Capture: Frame #007 (epoll)
> > 	Video Capture: Frame #008 (epoll)
> > 	Video Capture: Frame #009 (epoll)
> > 	Video Capture: Frame #010 (epoll)
> > 	Video Capture: Frame #011 (epoll)
> > 	Video Capture: Frame #012 (epoll)
> > 	Video Capture: Frame #013 (epoll)
> > 	Video Capture: Frame #014 (epoll)
> > 	Video Capture: Frame #015 (epoll)
> > 	Video Capture: Frame #016 (epoll)
> > 	Video Capture: Frame #017 (epoll)
> > 	Video Capture: Frame #018 (epoll)
> > 	Video Capture: Frame #019 (epoll)
> > 	Video Capture: Frame #020 (epoll)
> > 	Video Capture: Frame #021 (epoll)
> > 	Video Capture: Frame #022 (epoll)
> > 	Video Capture: Frame #023 (epoll)
> > 	Video Capture: Frame #024 (epoll)
> > 	Video Capture: Frame #025 (epoll)
> > 	Video Capture: Frame #026 (epoll)
> > 	Video Capture: Frame #027 (epoll)
> > 	Video Capture: Frame #028 (epoll)
> > 	Video Capture: Frame #029 (epoll)
> > 	Video Capture: Frame #030 (epoll)
> > 	Video Capture: Frame #031 (epoll)
> > 	Video Capture: Frame #032 (epoll)
> > 	Video Capture: Frame #033 (epoll)
> > 	Video Capture: Frame #034 (epoll)
> > 	Video Capture: Frame #035 (epoll)
> > 	Video Capture: Frame #036 (epoll)
> > 	Video Capture: Frame #037 (epoll)
> > 	Video Capture: Frame #038 (epoll)
> > 	Video Capture: Frame #039 (epoll)
> > 	Video Capture: Frame #040 (epoll)
> > 	Video Capture: Frame #041 (epoll)
> > 	Video Capture: Frame #042 (epoll)
> > 	Video Capture: Frame #043 (epoll)
> > 	Video Capture: Frame #044 (epoll)
> > 	Video Capture: Frame #045 (epoll)
> > 	Video Capture: Frame #046 (epoll)
> > 	Video Capture: Frame #047 (epoll)
> > 	Video Capture: Frame #048 (epoll)
> > 	Video Capture: Frame #049 (epoll)
> > 	Video Capture: Frame #050 (epoll)
> > 	Video Capture: Frame #051 (epoll)
> > 	Video Capture: Frame #052 (epoll)
> > 	Video Capture: Frame #053 (epoll)
> > 	Video Capture: Frame #054 (epoll)
> > 	Video Capture: Frame #055 (epoll)
> > 	Video Capture: Frame #056 (epoll)
> > 	Video Capture: Frame #057 (epoll)
> > 	Video Capture: Frame #058 (epoll)
> > 	Video Capture: Frame #059 (epoll)
> > 
> > 	test MMAP (epoll, REQBUFS): OK
> > 
> > 	Video Capture: Frame #000
> > 	Video Capture: Frame #001
> > 	Video Capture: Frame #002
> > 	Video Capture: Frame #003
> > 	Video Capture: Frame #004
> > 	Video Capture: Frame #005
> > 	Video Capture: Frame #006
> > 	Video Capture: Frame #007
> > 	Video Capture: Frame #008
> > 	Video Capture: Frame #009
> > 	Video Capture: Frame #010
> > 	Video Capture: Frame #011
> > 	Video Capture: Frame #012
> > 	Video Capture: Frame #013
> > 	Video Capture: Frame #014
> > 	Video Capture: Frame #015
> > 	Video Capture: Frame #016
> > 	Video Capture: Frame #017
> > 	Video Capture: Frame #018
> > 	Video Capture: Frame #019
> > 	Video Capture: Frame #020
> > 	Video Capture: Frame #021
> > 	Video Capture: Frame #022
> > 	Video Capture: Frame #023
> > 	Video Capture: Frame #024
> > 	Video Capture: Frame #025
> > 	Video Capture: Frame #026
> > 	Video Capture: Frame #027
> > 	Video Capture: Frame #028
> > 	Video Capture: Frame #029
> > 	Video Capture: Frame #030
> > 	Video Capture: Frame #031
> > 	Video Capture: Frame #032
> > 	Video Capture: Frame #033
> > 	Video Capture: Frame #034
> > 	Video Capture: Frame #035
> > 	Video Capture: Frame #036
> > 	Video Capture: Frame #037
> > 	Video Capture: Frame #038
> > 	Video Capture: Frame #039
> > 	Video Capture: Frame #040
> > 	Video Capture: Frame #041
> > 	Video Capture: Frame #042
> > 	Video Capture: Frame #043
> > 	Video Capture: Frame #044
> > 	Video Capture: Frame #045
> > 	Video Capture: Frame #046
> > 	Video Capture: Frame #047
> > 	Video Capture: Frame #048
> > 	Video Capture: Frame #049
> > 	Video Capture: Frame #050
> > 	Video Capture: Frame #051
> > 	Video Capture: Frame #052
> > 	Video Capture: Frame #053
> > 	Video Capture: Frame #054
> > 	Video Capture: Frame #055
> > 	Video Capture: Frame #056
> > 	Video Capture: Frame #057
> > 	Video Capture: Frame #058
> > 	Video Capture: Frame #059
> > 
> > 	test MMAP (no poll, CREATE_BUFS): OK
> > 
> > 	Video Capture: Frame #000 (select)
> > 	Video Capture: Frame #001 (select)
> > 	Video Capture: Frame #002 (select)
> > 	Video Capture: Frame #003 (select)
> > 	Video Capture: Frame #004 (select)
> > 	Video Capture: Frame #005 (select)
> > 	Video Capture: Frame #006 (select)
> > 	Video Capture: Frame #007 (select)
> > 	Video Capture: Frame #008 (select)
> > 	Video Capture: Frame #009 (select)
> > 	Video Capture: Frame #010 (select)
> > 	Video Capture: Frame #011 (select)
> > 	Video Capture: Frame #012 (select)
> > 	Video Capture: Frame #013 (select)
> > 	Video Capture: Frame #014 (select)
> > 	Video Capture: Frame #015 (select)
> > 	Video Capture: Frame #016 (select)
> > 	Video Capture: Frame #017 (select)
> > 	Video Capture: Frame #018 (select)
> > 	Video Capture: Frame #019 (select)
> > 	Video Capture: Frame #020 (select)
> > 	Video Capture: Frame #021 (select)
> > 	Video Capture: Frame #022 (select)
> > 	Video Capture: Frame #023 (select)
> > 	Video Capture: Frame #024 (select)
> > 	Video Capture: Frame #025 (select)
> > 	Video Capture: Frame #026 (select)
> > 	Video Capture: Frame #027 (select)
> > 	Video Capture: Frame #028 (select)
> > 	Video Capture: Frame #029 (select)
> > 	Video Capture: Frame #030 (select)
> > 	Video Capture: Frame #031 (select)
> > 	Video Capture: Frame #032 (select)
> > 	Video Capture: Frame #033 (select)
> > 	Video Capture: Frame #034 (select)
> > 	Video Capture: Frame #035 (select)
> > 	Video Capture: Frame #036 (select)
> > 	Video Capture: Frame #037 (select)
> > 	Video Capture: Frame #038 (select)
> > 	Video Capture: Frame #039 (select)
> > 	Video Capture: Frame #040 (select)
> > 	Video Capture: Frame #041 (select)
> > 	Video Capture: Frame #042 (select)
> > 	Video Capture: Frame #043 (select)
> > 	Video Capture: Frame #044 (select)
> > 	Video Capture: Frame #045 (select)
> > 	Video Capture: Frame #046 (select)
> > 	Video Capture: Frame #047 (select)
> > 	Video Capture: Frame #048 (select)
> > 	Video Capture: Frame #049 (select)
> > 	Video Capture: Frame #050 (select)
> > 	Video Capture: Frame #051 (select)
> > 	Video Capture: Frame #052 (select)
> > 	Video Capture: Frame #053 (select)
> > 	Video Capture: Frame #054 (select)
> > 	Video Capture: Frame #055 (select)
> > 	Video Capture: Frame #056 (select)
> > 	Video Capture: Frame #057 (select)
> > 	Video Capture: Frame #058 (select)
> > 	Video Capture: Frame #059 (select)
> > 
> > 	test MMAP (select, CREATE_BUFS): OK
> > 
> > 	Video Capture: Frame #000 (epoll)
> > 	Video Capture: Frame #001 (epoll)
> > 	Video Capture: Frame #002 (epoll)
> > 	Video Capture: Frame #003 (epoll)
> > 	Video Capture: Frame #004 (epoll)
> > 	Video Capture: Frame #005 (epoll)
> > 	Video Capture: Frame #006 (epoll)
> > 	Video Capture: Frame #007 (epoll)
> > 	Video Capture: Frame #008 (epoll)
> > 	Video Capture: Frame #009 (epoll)
> > 	Video Capture: Frame #010 (epoll)
> > 	Video Capture: Frame #011 (epoll)
> > 	Video Capture: Frame #012 (epoll)
> > 	Video Capture: Frame #013 (epoll)
> > 	Video Capture: Frame #014 (epoll)
> > 	Video Capture: Frame #015 (epoll)
> > 	Video Capture: Frame #016 (epoll)
> > 	Video Capture: Frame #017 (epoll)
> > 	Video Capture: Frame #018 (epoll)
> > 	Video Capture: Frame #019 (epoll)
> > 	Video Capture: Frame #020 (epoll)
> > 	Video Capture: Frame #021 (epoll)
> > 	Video Capture: Frame #022 (epoll)
> > 	Video Capture: Frame #023 (epoll)
> > 	Video Capture: Frame #024 (epoll)
> > 	Video Capture: Frame #025 (epoll)
> > 	Video Capture: Frame #026 (epoll)
> > 	Video Capture: Frame #027 (epoll)
> > 	Video Capture: Frame #028 (epoll)
> > 	Video Capture: Frame #029 (epoll)
> > 	Video Capture: Frame #030 (epoll)
> > 	Video Capture: Frame #031 (epoll)
> > 	Video Capture: Frame #032 (epoll)
> > 	Video Capture: Frame #033 (epoll)
> > 	Video Capture: Frame #034 (epoll)
> > 	Video Capture: Frame #035 (epoll)
> > 	Video Capture: Frame #036 (epoll)
> > 	Video Capture: Frame #037 (epoll)
> > 	Video Capture: Frame #038 (epoll)
> > 	Video Capture: Frame #039 (epoll)
> > 	Video Capture: Frame #040 (epoll)
> > 	Video Capture: Frame #041 (epoll)
> > 	Video Capture: Frame #042 (epoll)
> > 	Video Capture: Frame #043 (epoll)
> > 	Video Capture: Frame #044 (epoll)
> > 	Video Capture: Frame #045 (epoll)
> > 	Video Capture: Frame #046 (epoll)
> > 	Video Capture: Frame #047 (epoll)
> > 	Video Capture: Frame #048 (epoll)
> > 	Video Capture: Frame #049 (epoll)
> > 	Video Capture: Frame #050 (epoll)
> > 	Video Capture: Frame #051 (epoll)
> > 	Video Capture: Frame #052 (epoll)
> > 	Video Capture: Frame #053 (epoll)
> > 	Video Capture: Frame #054 (epoll)
> > 	Video Capture: Frame #055 (epoll)
> > 	Video Capture: Frame #056 (epoll)
> > 	Video Capture: Frame #057 (epoll)
> > 	Video Capture: Frame #058 (epoll)
> > 	Video Capture: Frame #059 (epoll)
> > 
> > 	test MMAP (epoll, CREATE_BUFS): 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)
> > 
> > Total for uvcvideo device /dev/video0: 59, Succeeded: 58, Failed: 1, Warnings: 0
> > 
> > ---
> > Changes in v4:
> > - Rebased on top of v7.1-rc1
> > - Replace usages of filep->private_data with file_to_v4l2_fh()
> >   throughout the driver
> > - Link to v3: https://lore.kernel.org/r/20250412-virtio-media-v3-1-97dc94c18398@gmail.com
> > 
> > Changes in v3:
> > - Rebased on top of v6.15-rc1 and removes obsolete control callbacks.
> > - Link to v2: https://lore.kernel.org/r/20250201-virtio-media-v2-1-ac840681452d@gmail.com
> > 
> > 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/20250123-virtio-media-v1-1-81e2549b86b9@gmail.com
> > 
> > Brian Daniels (8):
> >   media: virtio: Add protocol
> >   media: virtio: Add virtio-media driver structs and function
> >     declarations
> >   media: virtio: Add virtio-media session related structures
> >   media: virtio: Add scatterlist_builder
> >   media: virtio: Add virtio_media_ioctls
> >   media: virtio: Add virtio_media_driver
> >   media: virtio: Add virtio-media to the build system
> >   media: virtio: Add MAINTAINERS entry
> > 
> >  MAINTAINERS                                |    6 +
> >  drivers/media/Kconfig                      |   13 +
> >  drivers/media/Makefile                     |    2 +
> >  drivers/media/virtio/Makefile              |    8 +
> >  drivers/media/virtio/protocol.h            |  287 +++++
> >  drivers/media/virtio/scatterlist_builder.c |  574 +++++++++
> >  drivers/media/virtio/scatterlist_builder.h |  112 ++
> >  drivers/media/virtio/session.h             |  130 ++
> >  drivers/media/virtio/virtio_media.h        |   95 ++
> >  drivers/media/virtio/virtio_media_driver.c |  959 ++++++++++++++
> >  drivers/media/virtio/virtio_media_ioctls.c | 1338 ++++++++++++++++++++
> >  11 files changed, 3524 insertions(+)
> >  create mode 100644 drivers/media/virtio/Makefile
> >  create mode 100644 drivers/media/virtio/protocol.h
> >  create mode 100644 drivers/media/virtio/scatterlist_builder.c
> >  create mode 100644 drivers/media/virtio/scatterlist_builder.h
> >  create mode 100644 drivers/media/virtio/session.h
> >  create mode 100644 drivers/media/virtio/virtio_media.h
> >  create mode 100644 drivers/media/virtio/virtio_media_driver.c
> >  create mode 100644 drivers/media/virtio/virtio_media_ioctls.c
> > 
> > 
> > base-commit: 06cb687a5132fcffe624c0070576ab852ac6b568



^ permalink raw reply

* Re: [PATCH v4 8/8] media: virtio: Add MAINTAINERS entry
From: Brian Daniels @ 2026-06-25 20:20 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Brian Daniels, Mauro Carvalho Chehab, acourbot, adelva, aesteve,
	changyeon, daniel.almeida, eperezma, gnurou, gurchetansingh,
	hverkuil, jasowang, linux-kernel, linux-media, nicolas.dufresne,
	virtualization, xuanzhuo
In-Reply-To: <20260622172208-mutt-send-email-mst@kernel.org>

> > From: Alexandre Courbot <gnurou@gmail.com>
> > 
> > Add an entry for the new virtio-media driver.
> > 
> > Signed-off-by: Alexandre Courbot <gnurou@gmail.com>
> > Co-developed-by: Brian Daniels <briandaniels@google.com>
> > Signed-off-by: Brian Daniels <briandaniels@google.com>
> > ---
> >  MAINTAINERS | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index efbf80806..af370b787 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -28327,6 +28327,12 @@ S:	Maintained
> >  F:	drivers/iommu/virtio-iommu.c
> >  F:	include/uapi/linux/virtio_iommu.h
> >  
> > +VIRTIO MEDIA DRIVER
> > +M:	Brian Daniels <briandaniels@google.com>
> > +L:	linux-media@vger.kernel.org
> > +S:	Maintained
> > +F:	drivers/media/virtio/
> > +
> 
> add to virtio core too pls.

Fix staged in v5

> 
> >  VIRTIO MEM DRIVER
> >  M:	David Hildenbrand <david@kernel.org>
> >  L:	virtualization@lists.linux.dev

^ permalink raw reply

* Re: [PATCH v4 6/8] media: virtio: Add virtio_media_driver
From: Brian Daniels @ 2026-06-25 20:18 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Brian Daniels, Mauro Carvalho Chehab, acourbot, adelva, aesteve,
	changyeon, daniel.almeida, eperezma, gnurou, gurchetansingh,
	hverkuil, jasowang, linux-kernel, linux-media, nicolas.dufresne,
	virtualization, xuanzhuo
In-Reply-To: <20260622171017-mutt-send-email-mst@kernel.org>

> > From: Alexandre Courbot <gnurou@gmail.com>
> > 
> > virtio_media_driver.c provides the expected driver hooks, and support
> > for mmapping and polling.
> > 
> > Signed-off-by: Alexandre Courbot <gnurou@gmail.com>
> > Co-developed-by: Brian Daniels <briandaniels@google.com>
> > Signed-off-by: Brian Daniels <briandaniels@google.com>
> > ---
> >  drivers/media/virtio/virtio_media_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/20260528163448.4031965-1-briandaniels@google.com/

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.

> > +
> > +/*
> > + * 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?

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.

> > +
> > +/**
> > + * 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

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!

> > + */
> > +static void virtio_media_session_free(struct virtio_media *vv,
> > +				      struct virtio_media_session *session)
> > +{
> > +	int i;
> > +
> > +	mutex_lock(&vv->sessions_lock);
> > +	list_del(&session->list);
> > +	mutex_unlock(&vv->sessions_lock);
> > +
> > +	virtio_media_session_fh_del(session);
> > +	v4l2_fh_exit(&session->fh);
> > +
> > +	sg_free_table(&session->command_sgs);
> > +
> > +	for (i = 0; i <= VIRTIO_MEDIA_LAST_QUEUE; i++)
> > +		vfree(session->queues[i].buffers);
> > +
> > +	kfree(session->shadow_buf);
> > +	kfree(session);
> > +}
> > +
> > +/**
> > + * virtio_media_session_close - Close and free a session.
> > + * @vv: virtio-media device the session belongs to.
> > + * @session: session to close and destroy.
> > + *
> > + * This send the ``VIRTIO_MEDIA_CMD_CLOSE`` command to the device, and frees
> 
> sends

Fix staged in v5

> > + * all resources used by @session.
> > + */
> > +static int virtio_media_session_close(struct virtio_media *vv,
> > +				      struct virtio_media_session *session)
> > +{
> > +	struct virtio_media_cmd_close *cmd_close = &session->cmd.close;
> > +	struct scatterlist cmd_sg = {};
> > +	struct scatterlist *sgs[1] = { &cmd_sg };
> > +	int ret;
> > +
> > +	mutex_lock(&vv->vlock);
> > +
> > +	cmd_close->hdr.cmd = VIRTIO_MEDIA_CMD_CLOSE;
> > +	cmd_close->session_id = session->id;
> > +
> > +	sg_set_buf(&cmd_sg, cmd_close, sizeof(*cmd_close));
> > +	sg_mark_end(&cmd_sg);
> > +
> > +	ret = virtio_media_send_command(vv, sgs, 1, 0, 0, NULL);
> > +	mutex_unlock(&vv->vlock);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	virtio_media_session_free(vv, session);
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * virtio_media_find_session - Lookup for the session with a given ID.
> 
> a session

Fix staged in v5

> > + * @vv: virtio-media device to lookup the session from.
> > + * @id: ID of the session to lookup.
> > + */
> > +static struct virtio_media_session *
> > +virtio_media_find_session(struct virtio_media *vv, u32 id)
> > +{
> > +	struct list_head *p;
> > +	struct virtio_media_session *session = NULL;
> > +
> > +	mutex_lock(&vv->sessions_lock);
> > +	list_for_each(p, &vv->sessions) {
> > +		struct virtio_media_session *s =
> > +			list_entry(p, struct virtio_media_session, list);
> > +		if (s->id == id) {
> > +			session = s;
> > +			break;
> > +		}
> > +	}
> > +	mutex_unlock(&vv->sessions_lock);
> > +
> > +	return session;
> > +}
> > +
> > +/**
> > + * struct virtio_media_cmd_callback_param - Callback parameters to the virtio
> > + *                                          command queue.
> > + * @vv: virtio-media device in use.
> > + * @done: flag to be switched once the command is completed.
> > + * @resp_len: length of the received response from the command. Only valid
> > + * after @done_flag has switched to ``true``.
> 
> confusing indent

Fix staged in v5

> > + */
> > +struct virtio_media_cmd_callback_param {
> > +	struct virtio_media *vv;
> > +	bool done;
> > +	size_t resp_len;
> > +};
> > +
> > +/**
> > + * commandq_callback: Callback for the command queue.
> > + * @queue: command virtqueue.
> > + *
> > + * This just wakes up the thread that was waiting on the command to complete.
> > + */
> > +static void commandq_callback(struct virtqueue *queue)
> > +{
> > +	unsigned int len;
> > +	struct virtio_media_cmd_callback_param *param;
> > +
> > +process_bufs:
> > +	while ((param = virtqueue_get_buf(queue, &len))) {
> > +		param->done = true;
> > +		param->resp_len = len;
> > +		wake_up(&param->vv->wq);
> > +	}
> > +
> > +	if (!virtqueue_enable_cb(queue)) {
> > +		virtqueue_disable_cb(queue);
> > +		goto process_bufs;
> > +	}
> > +}
> > +
> > +/**
> > + * virtio_media_kick_command - send a command to the commandq.
> > + * @vv: virtio-media device in use.
> > + * @sgs: descriptor chain to send.
> > + * @out_sgs: number of device-readable descriptors in @sgs.
> > + * @in_sgs: number of device-writable descriptors in @sgs.
> > + * @resp_len: output parameter. Upon success, contains the size of the response
> > + * in bytes.
> 
> confusing indent
> 
> > + *
> 
> why an empty line?

Indent fixed and empty line removed in v5

> > + */
> > +static int virtio_media_kick_command(struct virtio_media *vv,
> > +				     struct scatterlist **sgs,
> > +				     const size_t out_sgs, const size_t in_sgs,
> > +				     size_t *resp_len)
> > +{
> > +	struct virtio_media_cmd_callback_param cb_param = {
> > +		.vv = vv,
> > +		.done = false,
> > +		.resp_len = 0,
> > +	};
> > +	struct virtio_media_resp_header *resp_header;
> > +	int ret;
> > +
> > +	ret = virtqueue_add_sgs(vv->commandq, sgs, out_sgs, in_sgs, &cb_param,
> > +				GFP_ATOMIC);
> 
> 
> can init with declaration.

Thanks, added to v5

> > +	if (ret) {
> > +		v4l2_err(&vv->v4l2_dev,
> > +			 "failed to add sgs to command virtqueue\n");
> > +		return ret;
> > +	}
> > +
> > +	if (!virtqueue_kick(vv->commandq)) {
> > +		v4l2_err(&vv->v4l2_dev, "failed to kick command virtqueue\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* Wait for the response. */
> > +	ret = wait_event_timeout(vv->wq, cb_param.done, 5 * HZ);
> > +	if (ret == 0) {
> > +		v4l2_err(&vv->v4l2_dev,
> > +			 "timed out waiting for response to command\n");
> > +		return -ETIMEDOUT;
> > +	}
> > +
> > +	if (resp_len)
> > +		*resp_len = cb_param.resp_len;
> > +
> > +	if (in_sgs > 0) {
> > +		/*
> > +		 * If we expect a response, make sure we have at least a
> > +		 * response header - anything shorter is invalid.
> > +		 */
> > +		if (cb_param.resp_len < sizeof(*resp_header)) {
> > +			v4l2_err(&vv->v4l2_dev,
> > +				 "received response header is too short\n");
> > +			return -EINVAL;
> > +		}
> > +
> > +		resp_header = sg_virt(sgs[out_sgs]);
> > +		if (resp_header->status)
> > +			/* Host returns a positive error code. */
> > +			return -resp_header->status;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * virtio_media_send_command - Send a command to the device and wait for its
> > + * response.
> > + * @vv: virtio-media device in use.
> > + * @sgs: descriptor chain to send.
> > + * @out_sgs: number of device-readable descriptors in @sgs.
> > + * @in_sgs: number of device-writable descriptors in @sgs.
> > + * @minimum_resp_len: minimum length of the response expected by the caller
> > + * when the command is successful. Anything shorter than that will result in
> > + * ``-EINVAL`` being returned.
> > + * @resp_len: output parameter. Upon success, contains the size of the response
> > + * in bytes.
> > + */
> > +int virtio_media_send_command(struct virtio_media *vv, struct scatterlist **sgs,
> > +			      const size_t out_sgs, const size_t in_sgs,
> > +			      size_t minimum_resp_len, size_t *resp_len)
> > +{
> > +	size_t local_resp_len = resp_len ? *resp_len : 0;
> > +	int ret = virtio_media_kick_command(vv, sgs, out_sgs, in_sgs,
> > +					    &local_resp_len);
> > +	if (resp_len)
> > +		*resp_len = local_resp_len;
> > +
> > +	/*
> > +	 * If the host could not process the command, there is no valid
> > +	 * response.
> > +	 */
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/* Make sure the host wrote a complete reply. */
> > +	if (local_resp_len < minimum_resp_len) {
> > +		v4l2_err(&vv->v4l2_dev,
> > +			 "received response is too short: received %zu, expected at least %zu\n",
> > +			 local_resp_len, minimum_resp_len);
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * virtio_media_send_event_buffer() - Sends an event buffer to the host so it
> > + * can return it with an event.
> > + * @vv: virtio-media device in use.
> > + * @event_buffer: pointer to the event buffer to send to the device.
> > + */
> > +static int virtio_media_send_event_buffer(struct virtio_media *vv,
> > +					  void *event_buffer)
> > +{
> > +	struct scatterlist *sgs[1], vresp;
> > +	int ret;
> > +
> > +	sg_init_one(&vresp, event_buffer, VIRTIO_MEDIA_EVENT_MAX_SIZE);
> > +	sgs[0] = &vresp;
> 
> what is this convoluted thing? why not pass &vresp directly?

`virtqueue_add_sgs` expects a `struct scatterlist **`. Passing `&vresp` is
only a single pointer and it needs a double pointer.

It could be written as:

  struct scatterlist *sgs, vresp;

  sg_init_one(&vresp, event_buffer, VIRTIO_MEDIA_EVENT_MAX_SIZE);
  sgs = &vresp;
  ret = virtqueue_add_sgs(vv->eventq, &sgs, 0, 1, event_buffer,

But I suspect it's written as is to match `virtqueue_add_sgs`'s function
declaration:

  int virtqueue_add_sgs(struct virtqueue *vq,
                        struct scatterlist *sgs[],
                        unsigned int out_sgs,
                        unsigned int in_sgs,
                        void *data,
                        gfp_t gfp);

> > +
> > +	ret = virtqueue_add_sgs(vv->eventq, sgs, 0, 1, event_buffer,
> > +				GFP_ATOMIC);
> 
> 
> This does not work uness event_buffer is aligned for dma.
> But it does not seem to be:
> 
>      for (i = 0; i < VIRTIO_MEDIA_NUM_EVENT_BUFS; i++) {
>             void *ebuf = vv->event_buffer + VIRTIO_MEDIA_EVENT_MAX_SIZE * i;
> 
>              ret = virtio_media_send_event_buffer(vv, ebuf);
>              if (ret)
>                      goto err_send_event_buffer;

Apologies if the following questions are bit basic, I'm  still pretty new to
this.

Is the dma alignment requirement a general requirement for calling the
`virtqueue_add_sgs` function? Or is the dma alignment requirement to allow the
driver to support DMABUF transfers? If its the later, I should note this driver
does not support DMABUF transfers yet when streaming frames.

If it's the former, then how would I go about ensuring its dma aligned?

From the code block you quoted above, `vv->event_buffer` is allocated like so:

  vv->event_buffer = devm_kzalloc(dev,
          VIRTIO_MEDIA_EVENT_MAX_SIZE *
          VIRTIO_MEDIA_NUM_EVENT_BUFS,
          GFP_KERNEL);
  if (!vv->event_buffer)
    return -ENOMEM;

Do I need to call something else besides `devm_kzalloc()` to ensure every
element within the vv->event_buffer array is dma aligned?

> 
> 
> > +	if (ret) {
> > +		v4l2_err(&vv->v4l2_dev,
> > +			 "failed to add sgs to event virtqueue\n");
> > +		return ret;
> > +	}
> > +
> > +	if (!virtqueue_kick(vv->eventq)) {
> > +		v4l2_err(&vv->v4l2_dev, "failed to kick event virtqueue\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * eventq_callback() - Callback for the event queue.
> > + * @queue: event virtqueue.
> > + *
> > + * This just schedules for event work to be run.
> > + */
> > +static void eventq_callback(struct virtqueue *queue)
> > +{
> > +	struct virtio_media *vv = queue->vdev->priv;
> > +
> > +	schedule_work(&vv->eventq_work);
> > +}
> > +
> > +/**
> > + * virtio_media_process_dqbuf_event() - Process a dequeued event for a session.
> > + * @vv: virtio-media device in use.
> > + * @session: session the event is addressed to.
> > + * @dqbuf_evt: the dequeued event to process.
> > + *
> > + * Invalid events are ignored with an error log.
> > + */
> > +static void
> > +virtio_media_process_dqbuf_event(struct virtio_media *vv,
> > +				 struct virtio_media_session *session,
> > +				 struct virtio_media_event_dqbuf *dqbuf_evt)
> > +{
> > +	struct virtio_media_buffer *dqbuf;
> > +	const enum v4l2_buf_type queue_type = dqbuf_evt->buffer.type;
> > +	struct virtio_media_queue_state *queue;
> > +	typeof(dqbuf->buffer.m) buffer_m;
> > +	typeof(dqbuf->buffer.m.planes[0].m) plane_m;
> > +	int i;
> > +
> > +	if (queue_type >= ARRAY_SIZE(session->queues)) {
> > +		v4l2_err(&vv->v4l2_dev,
> > +			 "unmanaged queue %d passed to dqbuf event",
> > +			 dqbuf_evt->buffer.type);
> > +		return;
> > +	}
> > +	queue = &session->queues[queue_type];
> > +
> > +	if (dqbuf_evt->buffer.index >= queue->allocated_bufs) {
> > +		v4l2_err(&vv->v4l2_dev,
> > +			 "invalid buffer ID %d for queue %d in dqbuf event",
> > +			 dqbuf_evt->buffer.index, dqbuf_evt->buffer.type);
> > +		return;
> > +	}
> > +
> > +	dqbuf = &queue->buffers[dqbuf_evt->buffer.index];
> > +
> > +	/*
> > +	 * Preserve the 'm' union that was passed to us during QBUF so userspace
> > +	 * gets back the information it submitted.
> > +	 */
> > +	buffer_m = dqbuf->buffer.m;
> > +	memcpy(&dqbuf->buffer, &dqbuf_evt->buffer, sizeof(dqbuf->buffer));
> > +	dqbuf->buffer.m = buffer_m;
> > +	if (V4L2_TYPE_IS_MULTIPLANAR(dqbuf->buffer.type)) {
> > +		if (dqbuf->buffer.length > VIDEO_MAX_PLANES) {
> > +			v4l2_err(&vv->v4l2_dev,
> > +				 "invalid number of planes received from host for a multiplanar buffer\n");
> > +			return;
> > +		}
> > +		for (i = 0; i < dqbuf->buffer.length; i++) {
> > +			plane_m = dqbuf->planes[i].m;
> > +			memcpy(&dqbuf->planes[i], &dqbuf_evt->planes[i],
> > +			       sizeof(struct v4l2_plane));
> > +			dqbuf->planes[i].m = plane_m;
> > +		}
> > +	}
> > +
> > +	/* Set the DONE flag as the buffer is waiting for being dequeued. */
> > +	dqbuf->buffer.flags |= V4L2_BUF_FLAG_DONE;
> > +
> > +	mutex_lock(&session->queues_lock);
> > +	list_add_tail(&dqbuf->list, &queue->pending_dqbufs);
> > +	queue->queued_bufs -= 1;
> > +	mutex_unlock(&session->queues_lock);
> > +
> > +	wake_up(&session->dqbuf_wait);
> > +}
> > +
> > +/**
> > + * virtio_media_process_events() - Process all pending events on a device.
> > + * @vv: device which pending events we want to process.
> > + *
> > + * Retrieves all pending events on @vv's event queue and dispatch them to their
> > + * corresponding session.
> > + *
> > + * Invalid events are ignored with an error log.
> > + */
> > +void virtio_media_process_events(struct virtio_media *vv)
> > +{
> > +	struct virtio_media_event_error *error_evt;
> > +	struct virtio_media_event_dqbuf *dqbuf_evt;
> > +	struct virtio_media_event_event *event_evt;
> > +	struct virtio_media_session *session;
> > +	struct virtio_media_event_header *evt;
> > +	unsigned int len;
> > +
> > +	mutex_lock(&vv->events_lock);
> > +
> > +process_bufs:
> > +	while ((evt = virtqueue_get_buf(vv->eventq, &len))) {
> > +		/* Make sure we received enough data */
> > +		if (len < sizeof(*evt)) {
> > +			v4l2_err(&vv->v4l2_dev,
> > +				 "event is too short: got %u, expected at least %zu\n",
> > +				 len, sizeof(*evt));
> > +			goto end_of_event;
> > +		}
> > +
> > +		session = virtio_media_find_session(vv, evt->session_id);
> > +		if (!session) {
> > +			v4l2_err(&vv->v4l2_dev, "cannot find session %d\n",
> > +				 evt->session_id);
> > +			goto end_of_event;
> > +		}
> > +
> > +		switch (evt->event) {
> > +		case VIRTIO_MEDIA_EVT_ERROR:
> > +			if (len < sizeof(*error_evt)) {
> > +				v4l2_err(&vv->v4l2_dev,
> > +					 "error event is too short: got %u, expected %zu\n",
> > +					 len, sizeof(*error_evt));
> > +				break;
> > +			}
> > +			error_evt = (struct virtio_media_event_error *)evt;
> > +			v4l2_err(&vv->v4l2_dev,
> > +				 "received error %d for session %d",
> > +				 error_evt->errno, error_evt->hdr.session_id);
> > +			virtio_media_session_close(vv, session);
> > +			break;
> > +
> > +		/*
> > +		 * Dequeued buffer: put it into the right queue so user-space
> > +		 * can dequeue it.
> > +		 */
> > +		case VIRTIO_MEDIA_EVT_DQBUF:
> > +			if (len < sizeof(*dqbuf_evt)) {
> > +				v4l2_err(&vv->v4l2_dev,
> > +					 "dqbuf event is too short: got %u, expected %zu\n",
> > +					 len, sizeof(*dqbuf_evt));
> > +				break;
> > +			}
> > +			dqbuf_evt = (struct virtio_media_event_dqbuf *)evt;
> > +			virtio_media_process_dqbuf_event(vv, session,
> > +							 dqbuf_evt);
> > +			break;
> > +
> > +		case VIRTIO_MEDIA_EVT_EVENT:
> > +			if (len < sizeof(*event_evt)) {
> > +				v4l2_err(&vv->v4l2_dev,
> > +					 "session event is too short: got %u expected %zu\n",
> > +					 len, sizeof(*event_evt));
> > +				break;
> > +			}
> > +
> > +			event_evt = (struct virtio_media_event_event *)evt;
> > +			v4l2_event_queue_fh(&session->fh, &event_evt->event);
> > +			break;
> > +
> > +		default:
> > +			v4l2_err(&vv->v4l2_dev, "unknown event type %d\n",
> > +				 evt->event);
> > +			break;
> > +		}
> > +
> > +end_of_event:
> > +		virtio_media_send_event_buffer(vv, evt);
> > +	}
> > +
> > +	if (!virtqueue_enable_cb(vv->eventq)) {
> > +		virtqueue_disable_cb(vv->eventq);
> > +		goto process_bufs;
> > +	}
> > +
> > +	mutex_unlock(&vv->events_lock);
> > +}
> > +
> > +static void virtio_media_event_work(struct work_struct *work)
> > +{
> > +	struct virtio_media *vv =
> > +		container_of(work, struct virtio_media, eventq_work);
> > +
> > +	virtio_media_process_events(vv);
> > +}
> > +
> > +/**
> > + * virtio_media_device_open() - Create a new session from an opened file.
> > + * @file: opened file for the session.
> > + */
> > +static int virtio_media_device_open(struct file *file)
> > +{
> > +	struct video_device *video_dev = video_devdata(file);
> > +	struct virtio_media *vv = to_virtio_media(video_dev);
> > +	struct virtio_media_cmd_open *cmd_open = &vv->cmd.open;
> > +	struct virtio_media_resp_open *resp_open = &vv->resp.open;
> > +	struct scatterlist cmd_sg = {}, resp_sg = {};
> > +	struct scatterlist *sgs[2] = { &cmd_sg, &resp_sg };
> > +	struct virtio_media_session *session;
> > +	u32 session_id;
> > +	int ret;
> > +
> > +	mutex_lock(&vv->vlock);
> > +
> > +	sg_set_buf(&cmd_sg, cmd_open, sizeof(*cmd_open));
> > +	sg_mark_end(&cmd_sg);
> > +
> > +	sg_set_buf(&resp_sg, resp_open, sizeof(*resp_open));
> > +	sg_mark_end(&resp_sg);
> > +
> > +	cmd_open->hdr.cmd = VIRTIO_MEDIA_CMD_OPEN;
> > +	ret = virtio_media_send_command(vv, sgs, 1, 1, sizeof(*resp_open),
> > +					NULL);
> > +	session_id = resp_open->session_id;
> > +	mutex_unlock(&vv->vlock);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	session = virtio_media_session_alloc(vv, session_id, file);
> > +	if (IS_ERR(session))
> > +		return PTR_ERR(session);
> > +
> > +	file->private_data = &session->fh;
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * virtio_media_device_close() - Close a previously opened session.
> > + * @file: file of the session to close.
> > + *
> > + * This sends to ``VIRTIO_MEDIA_CMD_CLOSE`` command to the device, and close
> > + * the session on the driver side.
> > + */
> > +static int virtio_media_device_close(struct file *file)
> > +{
> > +	struct video_device *video_dev = video_devdata(file);
> > +	struct virtio_media *vv = to_virtio_media(video_dev);
> > +	struct virtio_media_session *session =
> > +		fh_to_session(file->private_data);
> > +
> > +	return virtio_media_session_close(vv, session);
> > +}
> > +
> > +/**
> > + * virtio_media_device_poll() - Poll logic for a virtio-media device.
> > + * @file: file of the session to poll.
> > + * @wait: poll table to wait on.
> > + */
> > +static __poll_t virtio_media_device_poll(struct file *file, poll_table *wait)
> > +{
> > +	struct virtio_media_session *session =
> > +		fh_to_session(file->private_data);
> > +	enum v4l2_buf_type capture_type =
> > +		session->uses_mplane ? V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE :
> > +				       V4L2_BUF_TYPE_VIDEO_CAPTURE;
> > +	enum v4l2_buf_type output_type =
> > +		session->uses_mplane ? V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE :
> > +				       V4L2_BUF_TYPE_VIDEO_OUTPUT;
> > +	struct virtio_media_queue_state *capture_queue =
> > +		&session->queues[capture_type];
> > +	struct virtio_media_queue_state *output_queue =
> > +		&session->queues[output_type];
> > +	__poll_t req_events = poll_requested_events(wait);
> > +	__poll_t rc = 0;
> > +
> > +	poll_wait(file, &session->dqbuf_wait, wait);
> > +	poll_wait(file, &session->fh.wait, wait);
> > +
> > +	mutex_lock(&session->queues_lock);
> > +	if (req_events & (EPOLLIN | EPOLLRDNORM)) {
> > +		if (!capture_queue->streaming ||
> > +		    (capture_queue->queued_bufs == 0 &&
> > +		     list_empty(&capture_queue->pending_dqbufs)))
> > +			rc |= EPOLLERR;
> > +		else if (!list_empty(&capture_queue->pending_dqbufs))
> > +			rc |= EPOLLIN | EPOLLRDNORM;
> > +	}
> > +	if (req_events & (EPOLLOUT | EPOLLWRNORM)) {
> > +		if (!output_queue->streaming)
> > +			rc |= EPOLLERR;
> > +		else if (output_queue->queued_bufs <
> > +			 output_queue->allocated_bufs)
> > +			rc |= EPOLLOUT | EPOLLWRNORM;
> > +	}
> > +	mutex_unlock(&session->queues_lock);
> > +
> > +	if (v4l2_event_pending(&session->fh))
> > +		rc |= EPOLLPRI;
> > +
> > +	return rc;
> > +}
> > +
> > +static void virtio_media_vma_close_locked(struct vm_area_struct *vma)
> > +{
> > +	struct virtio_media *vv = vma->vm_private_data;
> > +	struct virtio_media_cmd_munmap *cmd_munmap = &vv->cmd.munmap;
> > +	struct virtio_media_resp_munmap *resp_munmap = &vv->resp.munmap;
> > +	struct scatterlist cmd_sg = {}, resp_sg = {};
> > +	struct scatterlist *sgs[2] = { &cmd_sg, &resp_sg };
> > +	int ret;
> > +
> > +	sg_set_buf(&cmd_sg, cmd_munmap, sizeof(*cmd_munmap));
> > +	sg_mark_end(&cmd_sg);
> > +
> > +	sg_set_buf(&resp_sg, resp_munmap, sizeof(*resp_munmap));
> > +	sg_mark_end(&resp_sg);
> > +
> > +	cmd_munmap->hdr.cmd = VIRTIO_MEDIA_CMD_MUNMAP;
> > +	cmd_munmap->driver_addr =
> > +		(vma->vm_pgoff << PAGE_SHIFT) - vv->mmap_region.addr;
> > +	ret = virtio_media_send_command(vv, sgs, 1, 1, sizeof(*resp_munmap),
> > +					NULL);
> > +	if (ret < 0) {
> > +		v4l2_err(&vv->v4l2_dev, "host failed to unmap buffer: %d\n",
> > +			 ret);
> > +	}
> > +}
> > +
> > +/**
> > + * virtio_media_vma_close() - Close a MMAP buffer mapping.
> > + * @vma: VMA of the mapping to close.
> > + *
> > + * Inform the host that a previously created MMAP mapping is no longer needed
> > + * and can be removed.
> > + */
> > +static void virtio_media_vma_close(struct vm_area_struct *vma)
> > +{
> > +	struct virtio_media *vv = vma->vm_private_data;
> > +
> > +	mutex_lock(&vv->vlock);
> > +	virtio_media_vma_close_locked(vma);
> > +	mutex_unlock(&vv->vlock);
> > +}
> > +
> > +static const struct vm_operations_struct virtio_media_vm_ops = {
> > +	.close = virtio_media_vma_close,
> > +};
> > +
> > +/**
> > + * virtio_media_device_mmap - Perform a mmap request from userspace.
> > + * @file: opened file of the session to map for.
> > + * @vma: VM area struct describing the desired mapping.
> > + *
> > + * This requests the host to map a MMAP buffer for us, so we can then make that
> > + * mapping visible into user-space address space.
> > + */
> > +static int virtio_media_device_mmap(struct file *file,
> > +				    struct vm_area_struct *vma)
> > +{
> > +	struct video_device *video_dev = video_devdata(file);
> > +	struct virtio_media *vv = to_virtio_media(video_dev);
> > +	struct virtio_media_session *session =
> > +		fh_to_session(file->private_data);
> > +	struct virtio_media_cmd_mmap *cmd_mmap = &session->cmd.mmap;
> > +	struct virtio_media_resp_mmap *resp_mmap = &session->resp.mmap;
> > +	struct scatterlist cmd_sg = {}, resp_sg = {};
> > +	struct scatterlist *sgs[2] = { &cmd_sg, &resp_sg };
> > +	int ret;
> > +
> > +	if (!(vma->vm_flags & VM_SHARED))
> > +		return -EINVAL;
> > +	if (!(vma->vm_flags & (VM_READ | VM_WRITE)))
> > +		return -EINVAL;
> > +
> > +	mutex_lock(&vv->vlock);
> > +
> > +	cmd_mmap->hdr.cmd = VIRTIO_MEDIA_CMD_MMAP;
> > +	cmd_mmap->session_id = session->id;
> > +	cmd_mmap->flags =
> > +		(vma->vm_flags & VM_WRITE) ? VIRTIO_MEDIA_MMAP_FLAG_RW : 0;
> > +	cmd_mmap->offset = vma->vm_pgoff << PAGE_SHIFT;
> > +
> > +	sg_set_buf(&cmd_sg, cmd_mmap, sizeof(*cmd_mmap));
> > +	sg_mark_end(&cmd_sg);
> > +
> > +	sg_set_buf(&resp_sg, resp_mmap, sizeof(*resp_mmap));
> > +	sg_mark_end(&resp_sg);
> > +
> > +	/*
> > +	 * The host performs reference counting and is smart enough to return
> > +	 * the same guest physical address if this is called several times on
> > +	 * the same
> > +	 * buffer.
> > +	 */
> > +	ret = virtio_media_send_command(vv, sgs, 1, 1, sizeof(*resp_mmap),
> > +					NULL);
> > +	if (ret < 0)
> > +		goto end;
> > +
> > +	vma->vm_private_data = vv;
> > +	/*
> > +	 * Keep the guest address at which the buffer is mapped since we will
> > +	 * use that to unmap.
> > +	 */
> > +	vma->vm_pgoff = (resp_mmap->driver_addr + vv->mmap_region.addr) >>
> > +			PAGE_SHIFT;
> > +
> > +	/*
> > +	 * We cannot let the mapping be larger than the buffer.
> > +	 */
> > +	if (vma->vm_end - vma->vm_start > PAGE_ALIGN(resp_mmap->len)) {
> > +		dev_dbg(&video_dev->dev,
> > +			"invalid MMAP, as it would overflow buffer length\n");
> > +		virtio_media_vma_close_locked(vma);
> > +		ret = -EINVAL;
> > +		goto end;
> > +	}
> > +
> > +	ret = io_remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
> > +				 vma->vm_end - vma->vm_start,
> > +				 vma->vm_page_prot);
> > +	if (ret)
> > +		goto end;
> > +
> > +	vma->vm_ops = &virtio_media_vm_ops;
> > +
> > +end:
> > +	mutex_unlock(&vv->vlock);
> > +	return ret;
> > +}
> > +
> > +static const struct v4l2_file_operations virtio_media_fops = {
> > +	.owner = THIS_MODULE,
> > +	.open = virtio_media_device_open,
> > +	.release = virtio_media_device_close,
> > +	.poll = virtio_media_device_poll,
> > +	.unlocked_ioctl = virtio_media_device_ioctl,
> > +	.mmap = virtio_media_device_mmap,
> > +};
> > +
> > +static int virtio_media_probe(struct virtio_device *virtio_dev)
> > +{
> > +	struct device *dev = &virtio_dev->dev;
> > +	struct virtqueue *vqs[2];
> > +	static struct virtqueue_info vq_info[2] = {
> > +		{
> > +			.name = "command",
> > +			.callback = commandq_callback,
> > +		},
> > +		{
> > +			.name = "event",
> > +			.callback = eventq_callback,
> > +		},
> > +	};
> > +	struct virtio_media *vv;
> > +	struct video_device *vd;
> > +	int i;
> > +	int ret;
> > +
> > +	vv = devm_kzalloc(dev, sizeof(*vv), GFP_KERNEL);
> > +	if (!vv)
> > +		return -ENOMEM;
> > +
> > +	vv->event_buffer = devm_kzalloc(dev,
> > +					VIRTIO_MEDIA_EVENT_MAX_SIZE *
> > +					VIRTIO_MEDIA_NUM_EVENT_BUFS,
> > +					GFP_KERNEL);
> > +	if (!vv->event_buffer)
> > +		return -ENOMEM;
> > +
> > +	INIT_LIST_HEAD(&vv->sessions);
> > +	mutex_init(&vv->sessions_lock);
> > +	mutex_init(&vv->events_lock);
> > +	mutex_init(&vv->vlock);
> > +
> > +	vv->virtio_dev = virtio_dev;
> > +	virtio_dev->priv = vv;
> > +
> > +	init_waitqueue_head(&vv->wq);
> > +
> > +	ret = v4l2_device_register(dev, &vv->v4l2_dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = virtio_find_vqs(virtio_dev, 2, vqs, vq_info, NULL);
> > +	if (ret)
> > +		goto err_find_vqs;
> > +
> > +	vv->commandq = vqs[0];
> > +	vv->eventq = vqs[1];
> > +	INIT_WORK(&vv->eventq_work, virtio_media_event_work);
> > +
> > +	/* Get MMAP buffer mapping SHM region */
> > +	virtio_get_shm_region(virtio_dev, &vv->mmap_region,
> > +			      VIRTIO_MEDIA_SHM_MMAP);
> > +
> > +	vd = &vv->video_dev;
> > +
> > +	vd->v4l2_dev = &vv->v4l2_dev;
> > +	vd->vfl_type = VFL_TYPE_VIDEO;
> > +	vd->ioctl_ops = &virtio_media_ioctl_ops;
> > +	vd->fops = &virtio_media_fops;
> > +	vd->device_caps = virtio_cread32(virtio_dev, 0);
> > +	if (vd->device_caps & (V4L2_CAP_VIDEO_M2M | V4L2_CAP_VIDEO_M2M_MPLANE))
> > +		vd->vfl_dir = VFL_DIR_M2M;
> > +	else if (vd->device_caps &
> > +		 (V4L2_CAP_VIDEO_OUTPUT | V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE))
> > +		vd->vfl_dir = VFL_DIR_TX;
> > +	else
> > +		vd->vfl_dir = VFL_DIR_RX;
> > +	vd->release = video_device_release_empty;
> > +	strscpy(vd->name, "virtio-media", sizeof(vd->name));
> > +
> > +	video_set_drvdata(vd, vv);
> > +
> > +	ret = video_register_device(vd, virtio_cread32(virtio_dev, 4), 0);
> > +	if (ret)
> > +		goto err_register_device;
> > +
> > +	for (i = 0; i < VIRTIO_MEDIA_NUM_EVENT_BUFS; i++) {
> > +		void *ebuf = vv->event_buffer + VIRTIO_MEDIA_EVENT_MAX_SIZE * i;
> > +
> > +		ret = virtio_media_send_event_buffer(vv, ebuf);
> > +		if (ret)
> > +			goto err_send_event_buffer;
> > +	}
> > +
> > +	virtio_device_ready(virtio_dev);
> > +
> > +	return 0;
> > +
> > +err_send_event_buffer:
> > +	video_unregister_device(&vv->video_dev);
> > +err_register_device:
> > +	virtio_dev->config->del_vqs(virtio_dev);
> > +err_find_vqs:
> > +	v4l2_device_unregister(&vv->v4l2_dev);
> > +
> > +	return ret;
> > +}
> > +
> > +static void virtio_media_remove(struct virtio_device *virtio_dev)
> > +{
> > +	struct virtio_media *vv = virtio_dev->priv;
> > +	struct list_head *p, *n;
> > +
> > +	cancel_work_sync(&vv->eventq_work);
> > +	virtio_reset_device(virtio_dev);
> > +
> > +	v4l2_device_unregister(&vv->v4l2_dev);
> > +	virtio_dev->config->del_vqs(virtio_dev);
> > +	video_unregister_device(&vv->video_dev);
> > +
> > +	list_for_each_safe(p, n, &vv->sessions) {
> > +		struct virtio_media_session *s =
> > +			list_entry(p, struct virtio_media_session, list);
> > +
> > +		virtio_media_session_free(vv, s);
> > +	}
> > +}
> > +
> > +static struct virtio_device_id id_table[] = {
> > +	{ VIRTIO_ID_MEDIA, VIRTIO_DEV_ANY_ID },
> > +	{ 0 },
> > +};
> > +
> > +static unsigned int features[] = {};
> > +
> > +static struct virtio_driver virtio_media_driver = {
> > +	.feature_table = features,
> > +	.feature_table_size = ARRAY_SIZE(features),
> > +	.driver.name = VIRTIO_MEDIA_DEFAULT_DRIVER_NAME,
> > +	.driver.owner = THIS_MODULE,
> > +	.id_table = id_table,
> > +	.probe = virtio_media_probe,
> > +	.remove = virtio_media_remove,
> > +};
> > +
> > +module_virtio_driver(virtio_media_driver);
> > +
> > +MODULE_DEVICE_TABLE(virtio, id_table);
> > +MODULE_DESCRIPTION("virtio media driver");
> > +MODULE_AUTHOR("Alexandre Courbot <gnurou@gmail.com>");
> > +MODULE_LICENSE("Dual BSD/GPL");

^ permalink raw reply

* Re: [PATCH v4 1/8] media: virtio: Add protocol
From: Brian Daniels @ 2026-06-25 19:50 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Brian Daniels, Mauro Carvalho Chehab, acourbot, adelva, aesteve,
	changyeon, daniel.almeida, eperezma, gnurou, gurchetansingh,
	hverkuil, jasowang, linux-kernel, linux-media, nicolas.dufresne,
	virtualization, xuanzhuo
In-Reply-To: <20260622170054-mutt-send-email-mst@kernel.org>

> > From: Alexandre Courbot <gnurou@gmail.com>
> > 
> > Add the identifiers and structs used to implement the virtio interface
> > as described in section 5.22 of version 1.4 of the virtio spec:
> > 
> > https://docs.oasis-open.org/virtio/virtio/v1.4/csprd01/virtio-v1.4-csprd01-diff-from-v1.2-cs01.html#x1-82200022
> > 
> > Signed-off-by: Alexandre Courbot <gnurou@gmail.com>
> > Co-developed-by: Brian Daniels <briandaniels@google.com>
> > Signed-off-by: Brian Daniels <briandaniels@google.com>
> 
> 
> Please put this under include/uapi/linux/virtio_media.h
> 
> we treat guest/host ABI similar to kernel/user ABI,
> and tools know how to find it there.

Staged in v5

^ permalink raw reply

* [PATCH] drm/virtio: Don't detach GEM from a non-created context
From: Jason Macnak @ 2026-06-25 17:08 UTC (permalink / raw)
  To: David Airlie, Gerd Hoffmann, Dmitry Osipenko, Gurchetan Singh
  Cc: dri-devel, virtualization, linux-kernel, Jason Macnak, stable

Applies the same treatment as commit 7cf6dd467e87 ("drm/virtio:
Don't attach GEM to a non-created context in gem_object_open()")
to virtio_gpu_gem_object_close() to avoid trying to detach
a resource that was never attached due to a context
never being created when context_init is supported.

Fixes: 086b9f27f0ab ("drm/virtio: Don't create a context with default param if context_init is supported")
Cc: <stable@vger.kernel.org> # v6.14+
Signed-off-by: Jason Macnak <natsu@google.com>
---
 drivers/gpu/drm/virtio/virtgpu_gem.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
index 435d37d36034..66c3f6f74e9c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -139,13 +139,15 @@ void virtio_gpu_gem_object_close(struct drm_gem_object *obj,
 	if (!vgdev->has_virgl_3d)
 		return;
 
-	objs = virtio_gpu_array_alloc(1);
-	if (!objs)
-		return;
-	virtio_gpu_array_add_obj(objs, obj);
+	if (vfpriv->context_created) {
+		objs = virtio_gpu_array_alloc(1);
+		if (!objs)
+			return;
+		virtio_gpu_array_add_obj(objs, obj);
 
-	virtio_gpu_cmd_context_detach_resource(vgdev, vfpriv->ctx_id,
-					       objs);
+		virtio_gpu_cmd_context_detach_resource(vgdev, vfpriv->ctx_id,
+						       objs);
+	}
 	virtio_gpu_notify(vgdev);
 }
 
-- 
2.55.0.rc0.799.gd6f94ed593-goog


^ permalink raw reply related

* Re: [PATCH v3 4/4] vhost/vsock: add VHOST_RESET_OWNER ioctl
From: Pavel Tikhomirov @ 2026-06-25 16:13 UTC (permalink / raw)
  To: Andrey Drobyshev, linux-kernel
  Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
	dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, den
In-Reply-To: <20260625155416.480669-5-andrey.drobyshev@virtuozzo.com>

Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

On 6/25/26 17:54, Andrey Drobyshev wrote:
> From: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> 
> This ioctl is needed for QEMU's CPR (checkpoint-restore) migration of
> the guest with vhost-vsock device.  For this to work, we need to reset
> the device ownership on the source side by calling RESET_OWNER, and then
> claim it on the dest side by calling SET_OWNER.  We expect not to lose any
> AF_VSOCK connection while this happens.
> 
> RESET_OWNER keeps the guest CID hashed, so that connections survive. That
> leaves the device reachable by a lockless send/cancel path while the worker
> is being torn down: a concurrent vhost_transport_send_pkt() or
> vhost_transport_cancel_pkt() can call vhost_vq_work_queue() as
> vhost_workers_free() frees the worker.  That might cause a use-after-free
> of vq->worker.  In addition, any work queued onto the dying worker leaves
> VHOST_WORK_QUEUED stuck, stalling send_pkt_queue after resume.
> 
> Fence the send/cancel paths around the teardown: send_pkt()/cancel_pkt()
> only kick the worker while the backend is alive.  And reset_owner() calls
> synchronize_rcu() after drop_backends() so in-flight send/cancel finish
> before the worker is freed.
> 
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
>  drivers/vhost/vsock.c | 51 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index 81d4f7209719..f0a0aa7d3200 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -318,7 +318,14 @@ vhost_transport_send_pkt(struct sk_buff *skb, struct net *net)
>  		atomic_inc(&vsock->queued_replies);
>  
>  	virtio_vsock_skb_queue_tail(&vsock->send_pkt_queue, skb);
> -	vhost_vq_work_queue(&vsock->vqs[VSOCK_VQ_RX], &vsock->send_pkt_work);
> +
> +	/* Skip the kick once the backend is gone (stop/RESET_OWNER); the skb
> +	 * stays queued and vhost_vsock_start() drains it. Pairs with the
> +	 * synchronize_rcu() in vhost_vsock_reset_owner().
> +	 */
> +	if (data_race(vhost_vq_get_backend(&vsock->vqs[VSOCK_VQ_RX])))
> +		vhost_vq_work_queue(&vsock->vqs[VSOCK_VQ_RX],
> +				    &vsock->send_pkt_work);
>  
>  	rcu_read_unlock();
>  	return len;
> @@ -346,7 +353,15 @@ vhost_transport_cancel_pkt(struct vsock_sock *vsk)
>  		int new_cnt;
>  
>  		new_cnt = atomic_sub_return(cnt, &vsock->queued_replies);
> -		if (new_cnt + cnt >= tx_vq->num && new_cnt < tx_vq->num)
> +
> +		/* Skip the kick once the backend is gone (stop/RESET_OWNER):
> +		 * vhost_poll_queue() would touch the worker which is being freed
> +		 * by teardown, e.g. on RESET_OWNER.  Pairs with the
> +		 * synchronize_rcu() in vhost_vsock_reset_owner().  The TX VQ is
> +		 * re-kicked by vhost_vsock_start().
> +		 */
> +		if (data_race(vhost_vq_get_backend(tx_vq)) &&
> +		    new_cnt + cnt >= tx_vq->num && new_cnt < tx_vq->num)
>  			vhost_poll_queue(&tx_vq->poll);
>  	}
>  
> @@ -903,6 +918,36 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
>  	return -EFAULT;
>  }
>  
> +static int vhost_vsock_reset_owner(struct vhost_vsock *vsock)
> +{
> +	struct vhost_iotlb *umem;
> +	long err;
> +
> +	mutex_lock(&vsock->dev.mutex);
> +	err = vhost_dev_check_owner(&vsock->dev);
> +	if (err)
> +		goto done;
> +	umem = vhost_dev_reset_owner_prepare();
> +	if (!umem) {
> +		err = -ENOMEM;
> +		goto done;
> +	}
> +	vhost_vsock_drop_backends(vsock);
> +
> +	/* Let in-flight send_pkt() callers stop touching the worker before the
> +	 * flush + free below. Pairs with the backend check in
> +	 * vhost_transport_send_pkt().
> +	 */
> +	synchronize_rcu();
> +
> +	vhost_vsock_flush(vsock);
> +	vhost_dev_stop(&vsock->dev);
> +	vhost_dev_reset_owner(&vsock->dev, umem);
> +done:
> +	mutex_unlock(&vsock->dev.mutex);
> +	return err;
> +}
> +
>  static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl,
>  				  unsigned long arg)
>  {
> @@ -946,6 +991,8 @@ static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl,
>  			return -EOPNOTSUPP;
>  		vhost_set_backend_features(&vsock->dev, features);
>  		return 0;
> +	case VHOST_RESET_OWNER:
> +		return vhost_vsock_reset_owner(vsock);
>  	default:
>  		mutex_lock(&vsock->dev.mutex);
>  		r = vhost_dev_ioctl(&vsock->dev, ioctl, argp);

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.


^ permalink raw reply

* [PATCH v3 4/4] vhost/vsock: add VHOST_RESET_OWNER ioctl
From: Andrey Drobyshev @ 2026-06-25 15:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
	dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
	den, andrey.drobyshev
In-Reply-To: <20260625155416.480669-1-andrey.drobyshev@virtuozzo.com>

From: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

This ioctl is needed for QEMU's CPR (checkpoint-restore) migration of
the guest with vhost-vsock device.  For this to work, we need to reset
the device ownership on the source side by calling RESET_OWNER, and then
claim it on the dest side by calling SET_OWNER.  We expect not to lose any
AF_VSOCK connection while this happens.

RESET_OWNER keeps the guest CID hashed, so that connections survive. That
leaves the device reachable by a lockless send/cancel path while the worker
is being torn down: a concurrent vhost_transport_send_pkt() or
vhost_transport_cancel_pkt() can call vhost_vq_work_queue() as
vhost_workers_free() frees the worker.  That might cause a use-after-free
of vq->worker.  In addition, any work queued onto the dying worker leaves
VHOST_WORK_QUEUED stuck, stalling send_pkt_queue after resume.

Fence the send/cancel paths around the teardown: send_pkt()/cancel_pkt()
only kick the worker while the backend is alive.  And reset_owner() calls
synchronize_rcu() after drop_backends() so in-flight send/cancel finish
before the worker is freed.

Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 drivers/vhost/vsock.c | 51 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 49 insertions(+), 2 deletions(-)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 81d4f7209719..f0a0aa7d3200 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -318,7 +318,14 @@ vhost_transport_send_pkt(struct sk_buff *skb, struct net *net)
 		atomic_inc(&vsock->queued_replies);
 
 	virtio_vsock_skb_queue_tail(&vsock->send_pkt_queue, skb);
-	vhost_vq_work_queue(&vsock->vqs[VSOCK_VQ_RX], &vsock->send_pkt_work);
+
+	/* Skip the kick once the backend is gone (stop/RESET_OWNER); the skb
+	 * stays queued and vhost_vsock_start() drains it. Pairs with the
+	 * synchronize_rcu() in vhost_vsock_reset_owner().
+	 */
+	if (data_race(vhost_vq_get_backend(&vsock->vqs[VSOCK_VQ_RX])))
+		vhost_vq_work_queue(&vsock->vqs[VSOCK_VQ_RX],
+				    &vsock->send_pkt_work);
 
 	rcu_read_unlock();
 	return len;
@@ -346,7 +353,15 @@ vhost_transport_cancel_pkt(struct vsock_sock *vsk)
 		int new_cnt;
 
 		new_cnt = atomic_sub_return(cnt, &vsock->queued_replies);
-		if (new_cnt + cnt >= tx_vq->num && new_cnt < tx_vq->num)
+
+		/* Skip the kick once the backend is gone (stop/RESET_OWNER):
+		 * vhost_poll_queue() would touch the worker which is being freed
+		 * by teardown, e.g. on RESET_OWNER.  Pairs with the
+		 * synchronize_rcu() in vhost_vsock_reset_owner().  The TX VQ is
+		 * re-kicked by vhost_vsock_start().
+		 */
+		if (data_race(vhost_vq_get_backend(tx_vq)) &&
+		    new_cnt + cnt >= tx_vq->num && new_cnt < tx_vq->num)
 			vhost_poll_queue(&tx_vq->poll);
 	}
 
@@ -903,6 +918,36 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
 	return -EFAULT;
 }
 
+static int vhost_vsock_reset_owner(struct vhost_vsock *vsock)
+{
+	struct vhost_iotlb *umem;
+	long err;
+
+	mutex_lock(&vsock->dev.mutex);
+	err = vhost_dev_check_owner(&vsock->dev);
+	if (err)
+		goto done;
+	umem = vhost_dev_reset_owner_prepare();
+	if (!umem) {
+		err = -ENOMEM;
+		goto done;
+	}
+	vhost_vsock_drop_backends(vsock);
+
+	/* Let in-flight send_pkt() callers stop touching the worker before the
+	 * flush + free below. Pairs with the backend check in
+	 * vhost_transport_send_pkt().
+	 */
+	synchronize_rcu();
+
+	vhost_vsock_flush(vsock);
+	vhost_dev_stop(&vsock->dev);
+	vhost_dev_reset_owner(&vsock->dev, umem);
+done:
+	mutex_unlock(&vsock->dev.mutex);
+	return err;
+}
+
 static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl,
 				  unsigned long arg)
 {
@@ -946,6 +991,8 @@ static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl,
 			return -EOPNOTSUPP;
 		vhost_set_backend_features(&vsock->dev, features);
 		return 0;
+	case VHOST_RESET_OWNER:
+		return vhost_vsock_reset_owner(vsock);
 	default:
 		mutex_lock(&vsock->dev.mutex);
 		r = vhost_dev_ioctl(&vsock->dev, ioctl, argp);
-- 
2.47.1


^ permalink raw reply related

* [PATCH v3 3/4] vhost/vsock: re-scan TX virtqueue on device start
From: Andrey Drobyshev @ 2026-06-25 15:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
	dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
	den, andrey.drobyshev
In-Reply-To: <20260625155416.480669-1-andrey.drobyshev@virtuozzo.com>

During QEMU CPR live-update (and VHOST_RESET_OWNER in general) the guest
keeps running while the host drops and later re-attaches vhost backends.
If the guest adds a buffer to the TX virtqueue (guest->host) and kicks
while the backend is temporarily NULL (between vhost_vsock_drop_backends()
and the next vhost_vsock_start()), then the kick is delivered to the
vhost worker, handle_tx_kick() sees a NULL backend and returns, and the
kick signal is consumed.  The buffer is then left in the ring.

Then upon device start vhost_vsock_start() only re-kicks the RX send
worker, never the TX VQ, so the buffer is processed only if the guest
happens to kick again.  But if the guest itself is now waiting for data
from the host, it will never kick TX VQ again, and we end up in a
deadlock.

The issue itself is pre-existing, but it only manifests during a brief
pause caused by VHOST_RESET_OWNER.  Namely, the deadlock is reproduced
during active host->guest socat data transfer under multiple consecutive
CPR live-update's.

To fix this, in vhost_vsock_start(), after kicking the RX send worker, also
queue the TX vq poll so any buffers the guest enqueued while we were paused
get scanned.

Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 drivers/vhost/vsock.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index bec6bcfd885f..81d4f7209719 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -646,6 +646,13 @@ static int vhost_vsock_start(struct vhost_vsock *vsock)
 	 */
 	vhost_vq_work_queue(&vsock->vqs[VSOCK_VQ_RX], &vsock->send_pkt_work);
 
+	/*
+	 * Some packets might've also been queued in TX VQ.  That is the case
+	 * during the brief device pause caused by VHOST_RESET_OWNER.  Re-scan
+	 * the TX VQ here, mirroring the RX send-worker kick above.
+	 */
+	vhost_poll_queue(&vsock->vqs[VSOCK_VQ_TX].poll);
+
 	mutex_unlock(&vsock->dev.mutex);
 	return 0;
 
-- 
2.47.1


^ permalink raw reply related

* [PATCH v3 2/4] vhost/vsock: suppress EHOSTUNREACH fast-fail during CPR pause
From: Andrey Drobyshev @ 2026-06-25 15:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
	dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
	den, andrey.drobyshev
In-Reply-To: <20260625155416.480669-1-andrey.drobyshev@virtuozzo.com>

Earlier commit bb26ed5f3a8b ("vhost/vsock: Refuse the connection
immediately when guest isn't ready") added a fast-fail in
vhost_transport_send_pkt().  It rejects every host send with -EHOSTUNREACH
until the destination calls SET_RUNNING(1).  The fast-fail condition checks
whether device's backends are dropped, and if they're, the guest is
considered to be not ready.

However, there might be other reasons for backends to be nulled.  In
particular, when QEMU is performing CPR (checkpoint-restore) migration,
device ownership is being RESET and SET again, which leads to backends
drop and reattach.  If we end up connecting during this window, an
AF_VSOCK client gets -EHOSTUNREACH, which is wrong.

Add a 'started' flag which is set once in vhost_vsock_start() and is
never cleared.  The behaviour changes to:

  * When device was never started -> flag is unset -> no listener can
    exist yet -> fast-fail;
  * Once the device starts -> flag is set -> we don't fast-fail ->
    we queue and preserve during any later stop / CPR pause.

Important caveat: after the first start, a connect during any stopped
window is queued instead of fast-failed.  That was the behaviour before
the patch bb26ed5f3a8b, and we're restoring it now.  However we still
keep the behaviour originally intended by that commit (i.e. fast-fail if
there's no real listener yet) while fixing the CPR path.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 drivers/vhost/vsock.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index b12221ce6faf..bec6bcfd885f 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -61,6 +61,7 @@ struct vhost_vsock {
 
 	u32 guest_cid;
 	bool seqpacket_allow;
+	bool started;		/* set on first SET_RUNNING(1); never cleared */
 };
 
 static u32 vhost_transport_get_local_cid(void)
@@ -302,17 +303,12 @@ vhost_transport_send_pkt(struct sk_buff *skb, struct net *net)
 		return -ENODEV;
 	}
 
-	/* Fast-fail if the guest hasn't enabled the RX vq yet. Queuing the packet
-	 * and making the caller wait is pointless: even if the guest manages to init
-	 * within the timeout, it'll immediately reply with RST, because there's no
-	 * listener on the port yet.
-	 *
-	 * vhost_vq_get_backend() without vq->mutex is acceptable here: locking
-	 * the mutex would be too expensive in this hot path, and we already have
-	 * all the outcomes covered: if the backend becomes NULL right after the check,
-	 * vhost_transport_do_send_pkt() will check it under the mutex anyway.
+	/* Fast-fail until the guest first enables the device (SET_RUNNING(1)).
+	 * Before that there is no listener, so queuing is pointless. 'started'
+	 * is never cleared, so once we're up we keep queuing across later
+	 * stop / CPR-pause windows.
 	 */
-	if (unlikely(!data_race(vhost_vq_get_backend(&vsock->vqs[VSOCK_VQ_RX])))) {
+	if (unlikely(!READ_ONCE(vsock->started))) {
 		rcu_read_unlock();
 		kfree_skb(skb);
 		return -EHOSTUNREACH;
@@ -640,6 +636,11 @@ static int vhost_vsock_start(struct vhost_vsock *vsock)
 		mutex_unlock(&vq->mutex);
 	}
 
+	/* Set 'started' flag on the first start; never cleared, so send_pkt
+	 * keeps queuing (instead of fast-failing) on later stop / CPR pauses.
+	 */
+	WRITE_ONCE(vsock->started, true);
+
 	/* Some packets may have been queued before the device was started,
 	 * let's kick the send worker to send them.
 	 */
@@ -728,6 +729,7 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
 
 	vsock->guest_cid = 0; /* no CID assigned yet */
 	vsock->seqpacket_allow = false;
+	vsock->started = false;
 
 	atomic_set(&vsock->queued_replies, 0);
 
-- 
2.47.1


^ permalink raw reply related

* [PATCH v3 1/4] vhost/vsock: split out vhost_vsock_drop_backends helper
From: Andrey Drobyshev @ 2026-06-25 15:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
	dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
	den, andrey.drobyshev
In-Reply-To: <20260625155416.480669-1-andrey.drobyshev@virtuozzo.com>

From: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

Split the actual backend dropping part from vhost_vsock_stop.  We're
going to need it for the VHOST_RESET_OWNER implementation in the
following patch, when vsock->dev.mutex is already taken and owner is
checked.

Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vhost/vsock.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 9aaab6bb8061..b12221ce6faf 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -664,9 +664,24 @@ static int vhost_vsock_start(struct vhost_vsock *vsock)
 	return ret;
 }
 
-static int vhost_vsock_stop(struct vhost_vsock *vsock, bool check_owner)
+static void vhost_vsock_drop_backends(struct vhost_vsock *vsock)
 {
+	struct vhost_virtqueue *vq;
 	size_t i;
+
+	lockdep_assert_held(&vsock->dev.mutex);
+
+	for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) {
+		vq = &vsock->vqs[i];
+
+		mutex_lock(&vq->mutex);
+		vhost_vq_set_backend(vq, NULL);
+		mutex_unlock(&vq->mutex);
+	}
+}
+
+static int vhost_vsock_stop(struct vhost_vsock *vsock, bool check_owner)
+{
 	int ret = 0;
 
 	mutex_lock(&vsock->dev.mutex);
@@ -677,14 +692,7 @@ static int vhost_vsock_stop(struct vhost_vsock *vsock, bool check_owner)
 			goto err;
 	}
 
-	for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) {
-		struct vhost_virtqueue *vq = &vsock->vqs[i];
-
-		mutex_lock(&vq->mutex);
-		vhost_vq_set_backend(vq, NULL);
-		mutex_unlock(&vq->mutex);
-	}
-
+	vhost_vsock_drop_backends(vsock);
 err:
 	mutex_unlock(&vsock->dev.mutex);
 	return ret;
-- 
2.47.1


^ permalink raw reply related

* [PATCH v3 0/4] vhost/vsock: add support for VHOST_RESET_OWNER and CPR migration
From: Andrey Drobyshev @ 2026-06-25 15:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
	dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
	den, andrey.drobyshev

v2 -> v3:

  * Patch 4: skip the kick of TX VQ worker once backend is gone - add
    to cancel_pkt() the same guard as in send_pkt().
    (Reported by Sashiko AI)

v2: https://lore.kernel.org/virtualization/20260622175808.508084-1-andrey.drobyshev@virtuozzo.com

Andrey Drobyshev (2):
  vhost/vsock: suppress EHOSTUNREACH fast-fail during CPR pause
  vhost/vsock: re-scan TX virtqueue on device start

Pavel Tikhomirov (2):
  vhost/vsock: split out vhost_vsock_drop_backends helper
  vhost/vsock: add VHOST_RESET_OWNER ioctl

 drivers/vhost/vsock.c | 106 +++++++++++++++++++++++++++++++++---------
 1 file changed, 85 insertions(+), 21 deletions(-)

-- 
2.47.1


^ permalink raw reply

* Re: [PATCH v2 0/4] vhost/vsock: add support for VHOST_RESET_OWNER and CPR migration
From: Pavel Tikhomirov @ 2026-06-25 15:32 UTC (permalink / raw)
  To: Andrey Drobyshev, linux-kernel
  Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
	dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, den
In-Reply-To: <20260622175808.508084-1-andrey.drobyshev@virtuozzo.com>

Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

On 6/22/26 19:58, Andrey Drobyshev wrote:
> v1 -> v2:
> 
>   * Patch 2 (suppress EHOSTUNREACH): replace 'cpr_paused' + backend check
>     with a single 'started' latch;
>   * Patch 3 (re-scan TX virtqueue): reword commit message;
>   * Patch 4 (VHOST_RESET_OWNER):
>       - fix a vhost_worker use-after-free / stuck VHOST_WORK_QUEUED stall
>         against the lockless send path;
>       - drop the no-op vsock_for_each_connected_socket() iteration;
>   * Shuffle the patches, keep RESET_OWNER implementation last to preserve
>     bisectability;
>   * Reword the cover letter.
> 
> v1: https://lore.kernel.org/virtualization/20260612165718.433546-1-andrey.drobyshev@virtuozzo.com
> 
> Host<-->guest connections via AF_VSOCK sockets aren't supposed to
> outlive VM migration, since VM is moving to another host.  However
> there's a special case, which is QEMU live-update, or CPR
> (checkpoint-restore) migration.  In this case, VM remains on the same
> host, and we'd like such connections to persist.
> 
> For this to work, we need to be able to transfer device ownership from
> source QEMU to dest QEMU.  Namely, source needs to reset ownership by
> issuing VHOST_RESET_OWNER ioctl, and then target has to claim it by
> calling VHOST_SET_OWNER.
> 
> Since VHOST_RESET_OWNER isn't yet implemented for vhost-vsock, let's add
> such implementation.  Patch 1 is a preliminary helper.  Patches 2 and 3
> fix the pre-existing issues which do manifest during CPR / RESET_OWNER.
> Patch 4 is the ioctl's implementation itself - we keep it last to
> preserve bisectability.
> 
> There's a complementary series for QEMU [0] adding support of vhost-vsock
> devices during CPR migration.
> 
> I've tested this (patched QEMU + patched kernel) approximately as follows:
> 
>   * Run listener in the guest:
>   socat -u VSOCK-LISTEN:9999 - >/tmp/recv.bin
> 
>   * Run data transfer from host to guest:
>   socat -u FILE:/root/bigfile.bin VSOCK-CONNECT:CID:9999
> 
>   * Perform CPR migration during transfer (either cpr-exec or cpr-transfer)
>   * Check that file hash sum matches
> 
> [0] https://lore.kernel.org/qemu-devel/20260619105514.128812-1-andrey.drobyshev@virtuozzo.com
> 
> Andrey Drobyshev (2):
>   vhost/vsock: suppress EHOSTUNREACH fast-fail during CPR pause
>   vhost/vsock: re-scan TX virtqueue on device start
> 
> Pavel Tikhomirov (2):
>   vhost/vsock: split out vhost_vsock_drop_backends helper
>   vhost/vsock: add VHOST_RESET_OWNER ioctl
> 
>  drivers/vhost/vsock.c | 96 ++++++++++++++++++++++++++++++++++---------
>  1 file changed, 76 insertions(+), 20 deletions(-)
> 

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.


^ permalink raw reply

* Re: [PATCH 1/2] tools/virtio: Add missing compat definitions for vhost_net_test
From: Eugenio Perez Martin @ 2026-06-25 14:05 UTC (permalink / raw)
  To: Yichong Chen
  Cc: mst, jasowang, xuanzhuo, rppt, ljs, akpm, virtualization,
	linux-kernel
In-Reply-To: <84157955DFE4842D+20260618095642.510565-2-chenyichong@uniontech.com>

On Thu, Jun 18, 2026 at 11:59 AM Yichong Chen <chenyichong@uniontech.com> wrote:
>
> vhost_net_test builds virtio_ring.c in userspace.
>
> Recent virtio headers pull in new helper headers.
>
> They also use new allocation helpers and a DMA attribute.
>
> Add the missing compat definitions.
>

I'm all in with this patch but it still misses some headers, could you
check again with latest master?

$ make vhost_net_test
cc -g -O2 -Werror -Wno-maybe-uninitialized -Wall -I. -I../include/ -I
../../usr/include/ -Wno-pointer-sign -fno-strict-overflow
-fno-strict-aliasing -fno-common -MMD -U_FORTIFY_SOURCE -include
../../include/linux/kconfig.h -mfunction-return=thunk
-fcf-protection=none -mindirect-branch-register -pthread   -c -o
virtio_ring.o ../../drivers/virtio/virtio_ring.c
In file included from ./linux/compiler.h:7,
                 from ./linux/kernel.h:12,
                 from ./linux/scatterlist.h:4,
                 from ./../../include/linux/virtio.h:7,
                 from ./linux/virtio.h:1,
                 from ../../drivers/virtio/virtio_ring.c:6:
./linux/../../../include/linux/compiler_types.h:638:10: fatal error:
asm/percpu_types.h: No such file or directory
  638 | #include <asm/percpu_types.h>
      |          ^~~~~~~~~~~~~~~~~~~~
compilation terminated.
make: *** [<builtin>: virtio_ring.o] Error 1

> Signed-off-by: Yichong Chen <chenyichong@uniontech.com>
> ---
>  tools/virtio/linux/completion.h      |  9 +++++
>  tools/virtio/linux/device.h          |  1 +
>  tools/virtio/linux/dma-mapping.h     |  1 +
>  tools/virtio/linux/mod_devicetable.h | 14 +++++++
>  tools/virtio/linux/slab.h            |  4 ++
>  tools/virtio/linux/virtio_features.h | 56 ++++++++++++++++++++++++++++
>  6 files changed, 85 insertions(+)
>  create mode 100644 tools/virtio/linux/completion.h
>  create mode 100644 tools/virtio/linux/mod_devicetable.h
>  create mode 100644 tools/virtio/linux/virtio_features.h
>
> diff --git a/tools/virtio/linux/completion.h b/tools/virtio/linux/completion.h
> new file mode 100644
> index 000000000000..5e54b679721b
> --- /dev/null
> +++ b/tools/virtio/linux/completion.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_COMPLETION_H
> +#define _LINUX_COMPLETION_H
> +
> +struct completion {
> +       unsigned int done;
> +};
> +
> +#endif /* _LINUX_COMPLETION_H */
> diff --git a/tools/virtio/linux/device.h b/tools/virtio/linux/device.h
> index 075c2140d975..abf100cb0023 100644
> --- a/tools/virtio/linux/device.h
> +++ b/tools/virtio/linux/device.h
> @@ -1,4 +1,5 @@
>  #ifndef LINUX_DEVICE_H
> +#define LINUX_DEVICE_H
>
>  struct device {
>         void *parent;
> diff --git a/tools/virtio/linux/dma-mapping.h b/tools/virtio/linux/dma-mapping.h
> index fddfa2fbb276..65e2974b3908 100644
> --- a/tools/virtio/linux/dma-mapping.h
> +++ b/tools/virtio/linux/dma-mapping.h
> @@ -59,5 +59,6 @@ enum dma_data_direction {
>   * instead.
>   */
>  #define DMA_MAPPING_ERROR              (~(dma_addr_t)0)
> +#define DMA_ATTR_DEBUGGING_IGNORE_CACHELINES   0
>
>  #endif
> diff --git a/tools/virtio/linux/mod_devicetable.h b/tools/virtio/linux/mod_devicetable.h
> new file mode 100644
> index 000000000000..3ba594b8229d
> --- /dev/null
> +++ b/tools/virtio/linux/mod_devicetable.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_MOD_DEVICETABLE_H
> +#define _LINUX_MOD_DEVICETABLE_H
> +
> +#include <linux/types.h>
> +
> +struct virtio_device_id {
> +       __u32 device;
> +       __u32 vendor;
> +};
> +
> +#define VIRTIO_DEV_ANY_ID      0xffffffff
> +
> +#endif /* _LINUX_MOD_DEVICETABLE_H */
> diff --git a/tools/virtio/linux/slab.h b/tools/virtio/linux/slab.h
> index 319dcaa07755..13d94c6f663c 100644
> --- a/tools/virtio/linux/slab.h
> +++ b/tools/virtio/linux/slab.h
> @@ -4,4 +4,8 @@
>  #define GFP_ATOMIC 0
>  #define __GFP_NOWARN 0
>  #define __GFP_ZERO 0
> +#define kmalloc_obj(VAR_OR_TYPE, ...) \
> +       kmalloc(sizeof(VAR_OR_TYPE), GFP_KERNEL)
> +#define kmalloc_objs(VAR_OR_TYPE, COUNT, ...) \
> +       kmalloc_array((COUNT), sizeof(VAR_OR_TYPE), GFP_KERNEL)
>  #endif
> diff --git a/tools/virtio/linux/virtio_features.h b/tools/virtio/linux/virtio_features.h
> new file mode 100644
> index 000000000000..18c56610e209
> --- /dev/null
> +++ b/tools/virtio/linux/virtio_features.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_VIRTIO_FEATURES_H
> +#define _LINUX_VIRTIO_FEATURES_H
> +
> +#include <linux/bug.h>
> +#include <linux/string.h>
> +#include <linux/types.h>
> +
> +#define VIRTIO_FEATURES_U64S   2
> +#define VIRTIO_FEATURES_BITS   (VIRTIO_FEATURES_U64S * 64)
> +
> +#define VIRTIO_BIT(b)          (1ULL << ((b) & 0x3f))
> +#define VIRTIO_U64(b)          ((b) >> 6)
> +
> +#define VIRTIO_DECLARE_FEATURES(name)                  \
> +       union {                                         \
> +               u64 name;                                       \
> +               u64 name##_array[VIRTIO_FEATURES_U64S]; \
> +       }
> +
> +static inline bool virtio_features_chk_bit(unsigned int bit)
> +{
> +       return bit < VIRTIO_FEATURES_BITS;
> +}
> +
> +static inline bool virtio_features_test_bit(const u64 *features,
> +                                           unsigned int bit)
> +{
> +       return virtio_features_chk_bit(bit) &&
> +              !!(features[VIRTIO_U64(bit)] & VIRTIO_BIT(bit));
> +}
> +
> +static inline void virtio_features_set_bit(u64 *features, unsigned int bit)
> +{
> +       if (virtio_features_chk_bit(bit))
> +               features[VIRTIO_U64(bit)] |= VIRTIO_BIT(bit);
> +}
> +
> +static inline void virtio_features_clear_bit(u64 *features, unsigned int bit)
> +{
> +       if (virtio_features_chk_bit(bit))
> +               features[VIRTIO_U64(bit)] &= ~VIRTIO_BIT(bit);
> +}
> +
> +static inline void virtio_features_zero(u64 *features)
> +{
> +       memset(features, 0, sizeof(features[0]) * VIRTIO_FEATURES_U64S);
> +}
> +
> +static inline void virtio_features_from_u64(u64 *features, u64 from)
> +{
> +       virtio_features_zero(features);
> +       features[0] = from;
> +}
> +
> +#endif /* _LINUX_VIRTIO_FEATURES_H */
> --
> 2.51.0
>


^ permalink raw reply

* Re: [PATCH] vhost/net: fix clear_user start address in VHOST_GET_FEATURES_ARRAY
From: Eugenio Perez Martin @ 2026-06-25 13:56 UTC (permalink / raw)
  To: rom.wang
  Cc: Michael S . Tsirkin, Jason Wang, Paolo Abeni, kvm, virtualization,
	netdev, linux-kernel, Yufeng Wang
In-Reply-To: <CAJaqyWcFm0A5ucL5TLP8+T8JNOiZyaL-_mb747_fKhH9Qm83ig@mail.gmail.com>

On Thu, Jun 25, 2026 at 3:48 PM Eugenio Perez Martin
<eperezma@redhat.com> wrote:
>
> On Tue, May 26, 2026 at 10:04 AM rom.wang <r4o5m6e8o@163.com> wrote:
> >
> > From: Yufeng Wang <wangyufeng@kylinos.cn>
> >
> > The clear_user() call in VHOST_GET_FEATURES_ARRAY incorrectly starts
> > at argp, which is the beginning of the features array, overwriting the
> > data just written by copy_to_user(). It should start after the copied
> > elements at argp + copied * sizeof(u64) to only zero the trailing
> > unused space.
> >
> > Fixes: 333c515d1896 ("vhost-net: allow configuring extended features")
> > Signed-off-by: Yufeng Wang <wangyufeng@kylinos.cn>
> > ---
> >  drivers/vhost/net.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > index db341c922673..70c578acf840 100644
> > --- a/drivers/vhost/net.c
> > +++ b/drivers/vhost/net.c
> > @@ -1777,7 +1777,8 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
> >                         return -EFAULT;
> >
> >                 /* Zero the trailing space provided by user-space, if any */
> > -               if (clear_user(argp, size_mul(count - copied, sizeof(u64))))
> > +               if (clear_user(argp + copied * sizeof(u64),
> > +                              size_mul(count - copied, sizeof(u64))))
>
> The fix looks good to me, but why not use size_mul() macro for copied
> * sizeof(u64) multiplication?
>

Also, could you add a new switch to tools/virtio/vhost_net_test.c to
use the VHOST_GET_FEATURES_ARRAY and VHOST_SET_FEATURES_ARRAY instead
of VHOST_GET_FEATURES and VHOST_SET_FEATURES?

> >                         return -EFAULT;
> >                 return 0;
> >         case VHOST_SET_FEATURES_ARRAY:
> > --
> > 2.34.1
> >
> >


^ permalink raw reply

* Re: [PATCH] tools/virtio: Remove unsupported --batch option from vhost_net_test
From: Eugenio Perez Martin @ 2026-06-25 13:55 UTC (permalink / raw)
  To: Yichong Chen; +Cc: mst, jasowang, xuanzhuo, virtualization, linux-kernel
In-Reply-To: <E091F15D8FBE8F16+20260618100254.513806-1-chenyichong@uniontech.com>

On Thu, Jun 18, 2026 at 12:03 PM Yichong Chen <chenyichong@uniontech.com> wrote:
>
> vhost_net_test has --batch in longopts, but not in help.
>
> The parser never handles 'b', so --batch hits assert(0).
>
> Remove the unsupported option.
>
> Signed-off-by: Yichong Chen <chenyichong@uniontech.com>

Acked-by: Eugenio Pérez <eperezma@redhat.com>

Thanks!

> ---
>  tools/virtio/vhost_net_test.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/tools/virtio/vhost_net_test.c b/tools/virtio/vhost_net_test.c
> index 389d99a6d7c7..566e15420bb6 100644
> --- a/tools/virtio/vhost_net_test.c
> +++ b/tools/virtio/vhost_net_test.c
> @@ -450,11 +450,6 @@ static const struct option longopts[] = {
>                 .val = 'n',
>                 .has_arg = required_argument,
>         },
> -       {
> -               .name = "batch",
> -               .val = 'b',
> -               .has_arg = required_argument,
> -       },
>         {
>         }
>  };
> --
> 2.51.0
>


^ permalink raw reply

* Re: [PATCH] vhost/net: fix clear_user start address in VHOST_GET_FEATURES_ARRAY
From: Eugenio Perez Martin @ 2026-06-25 13:48 UTC (permalink / raw)
  To: rom.wang
  Cc: Michael S . Tsirkin, Jason Wang, Paolo Abeni, kvm, virtualization,
	netdev, linux-kernel, Yufeng Wang
In-Reply-To: <20260526080336.61296-1-r4o5m6e8o@163.com>

On Tue, May 26, 2026 at 10:04 AM rom.wang <r4o5m6e8o@163.com> wrote:
>
> From: Yufeng Wang <wangyufeng@kylinos.cn>
>
> The clear_user() call in VHOST_GET_FEATURES_ARRAY incorrectly starts
> at argp, which is the beginning of the features array, overwriting the
> data just written by copy_to_user(). It should start after the copied
> elements at argp + copied * sizeof(u64) to only zero the trailing
> unused space.
>
> Fixes: 333c515d1896 ("vhost-net: allow configuring extended features")
> Signed-off-by: Yufeng Wang <wangyufeng@kylinos.cn>
> ---
>  drivers/vhost/net.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index db341c922673..70c578acf840 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -1777,7 +1777,8 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
>                         return -EFAULT;
>
>                 /* Zero the trailing space provided by user-space, if any */
> -               if (clear_user(argp, size_mul(count - copied, sizeof(u64))))
> +               if (clear_user(argp + copied * sizeof(u64),
> +                              size_mul(count - copied, sizeof(u64))))

The fix looks good to me, but why not use size_mul() macro for copied
* sizeof(u64) multiplication?

>                         return -EFAULT;
>                 return 0;
>         case VHOST_SET_FEATURES_ARRAY:
> --
> 2.34.1
>
>


^ permalink raw reply

* Re: [PATCH v3 1/7] list: Add mutable iterator variants
From: Jani Nikula @ 2026-06-25 11:00 UTC (permalink / raw)
  To: Kaitao Cheng, David Laight, Christian König,
	David Hildenbrand (Arm), Alexei Starovoitov
  Cc: Andrew Morton, David Hildenbrand, Jens Axboe, Tejun Heo,
	Alexander Viro, Christian Brauner, Daniel Borkmann,
	Andrii Nakryiko, Johannes Weiner, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Namhyung Kim, Thomas Gleixner,
	Juri Lelli, Vincent Guittot, Paul Moore, Andy Shevchenko,
	Paul E. McKenney, Shakeel Butt, David Howells, Simona Vetter,
	Randy Dunlap, Luca Ceresoli, Philipp Stanner, linux-block,
	linux-kernel, cgroups, linux-ntfs-dev, linux-fsdevel, io-uring,
	audit, bpf, netdev, dri-devel, linux-perf-users,
	linux-trace-kernel, kexec, live-patching, linux-modules,
	linux-crypto, linux-pm, rcu, sched-ext, linux-mm, virtualization,
	damon, llvm, Kaitao Cheng, Muchun Song
In-Reply-To: <0ed6b5c3-e955-46e2-9fc6-075a0dfd1c4f@linux.dev>

On Thu, 25 Jun 2026, Kaitao Cheng <kaitao.cheng@linux.dev> wrote:
> 在 2026/6/24 22:23, David Laight 写道:
>> On Wed, 24 Jun 2026 15:23:47 +0200
>> Christian König <christian.koenig@amd.com> wrote:
>>> On 6/24/26 15:14, Kaitao Cheng wrote:
>>>> 在 2026/6/22 16:42, David Laight 写道:  
>>>>> On Mon, 22 Jun 2026 12:05:31 +0800
>>>>> Kaitao Cheng <kaitao.cheng@linux.dev> wrote:
>>>>>  
>>>>>> From: Kaitao Cheng <chengkaitao@kylinos.cn>
>>>>>>
>>>>>> The list_for_each*_safe() helpers are used when the loop body may
>>>>>> remove the current entry.  Their API exposes the temporary cursor at
>>>>>> every call site, even though most users only need it for the iterator
>>>>>> implementation and never reference it in the loop body.
>>>>>>
>>>>>> Add *_mutable() variants for list and hlist iteration.  The new helpers
>>>>>> support both forms: callers may keep passing an explicit temporary cursor
>>>>>> when they need to inspect or reset it, or omit it and let the helper use
>>>>>> a unique internal cursor.  
>>>>>
>>>>> I'm not really sure 'mutable' means anything either.
>>>>> It is possible to make it valid for the loop body (or even other threads)
>>>>> to delete arbitrary list items - but that needs significant extra overheads.
>>>>>
>>>>> It might be worth doing something that doesn't need the extra variable,
>>>>> but there is little point doing all the churn just to rename things.
>>>>>  
>>>>>>
>>>>>> This makes call sites that only mutate the list through the current entry
>>>>>> less noisy, while keeping the existing *_safe() helpers available for
>>>>>> compatibility.
>>>>>>
>>>>>> Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
>>>>>> ---
>>>>>>  include/linux/list.h | 269 +++++++++++++++++++++++++++++++++++++------
>>>>>>  1 file changed, 231 insertions(+), 38 deletions(-)
>>>>>>
>>>>>> diff --git a/include/linux/list.h b/include/linux/list.h
>>>>>> index 09d979976b3b..1081def7cea9 100644
>>>>>> --- a/include/linux/list.h
>>>>>> +++ b/include/linux/list.h
>>>>>> @@ -7,6 +7,7 @@
>>>>>>  #include <linux/stddef.h>
>>>>>>  #include <linux/poison.h>
>>>>>>  #include <linux/const.h>
>>>>>> +#include <linux/args.h>
>>>>>>  
>>>>>>  #include <asm/barrier.h>
>>>>>>  
>>>>>> @@ -763,28 +764,72 @@ static inline void list_splice_tail_init(struct list_head *list,
>>>>>>  #define list_for_each_prev(pos, head) \
>>>>>>  	for (pos = (head)->prev; !list_is_head(pos, (head)); pos = pos->prev)
>>>>>>  
>>>>>> -/**
>>>>>> - * list_for_each_safe - iterate over a list safe against removal of list entry
>>>>>> - * @pos:	the &struct list_head to use as a loop cursor.
>>>>>> - * @n:		another &struct list_head to use as temporary storage
>>>>>> - * @head:	the head for your list.
>>>>>> +/*
>>>>>> + * list_for_each_safe is an old interface, use list_for_each_mutable instead.
>>>>>>   */
>>>>>>  #define list_for_each_safe(pos, n, head) \
>>>>>>  	for (pos = (head)->next, n = pos->next; \
>>>>>>  	     !list_is_head(pos, (head)); \
>>>>>>  	     pos = n, n = pos->next)
>>>>>>  
>>>>>> +#define __list_for_each_mutable_internal(pos, tmp, head)		\
>>>>>> +	for (typeof(pos) tmp = (pos = (head)->next)->next;		\  
>>>>>
>>>>> Use auto
>>>>>  
>>>>>> +	     !list_is_head(pos, (head));				\
>>>>>> +	     pos = tmp, tmp = pos->next)
>>>>>> +
>>>>>> +#define __list_for_each_mutable1(pos, head)				\
>>>>>> +	__list_for_each_mutable_internal(pos, __UNIQUE_ID(next), head)
>>>>>> +
>>>>>> +#define __list_for_each_mutable2(pos, next, head)			\
>>>>>> +	list_for_each_safe(pos, next, head)
>>>>>> +
>>>>>>  /**
>>>>>> - * list_for_each_prev_safe - iterate over a list backwards safe against removal of list entry
>>>>>> + * list_for_each_mutable - iterate over a list safe against entry removal
>>>>>>   * @pos:	the &struct list_head to use as a loop cursor.
>>>>>> - * @n:		another &struct list_head to use as temporary storage
>>>>>> - * @head:	the head for your list.
>>>>>> + * @...:	either (head) or (next, head)
>>>>>> + *
>>>>>> + * next:	another &struct list_head to use as optional temporary storage.
>>>>>> + *		The temporary cursor is internal unless explicitly supplied by
>>>>>> + *		the caller.
>>>>>> + * head:	the head for your list.
>>>>>> + */
>>>>>> +#define list_for_each_mutable(pos, ...)					\
>>>>>> +	CONCATENATE(__list_for_each_mutable, COUNT_ARGS(__VA_ARGS__))	\
>>>>>> +		(pos, __VA_ARGS__)  
>>>>>
>>>>> The variable argument count logic really just slows down compilation.
>>>>> Maybe there aren't enough copies of this code to make that significant.
>>>>> But just because you can do it doesn't mean it is a gooD idea.
>>>>> I'm also not sure it really adds anything to the readability.
>>>>>
>>>>> And, it you are going to make the middle argument optional there is
>>>>> no need to change the macro name.  
>>>>
>>>> Christian König and Jani Nikula also disagree with the variadic-argument
>>>> implementation approach. If we abandon that method, it means we will
>>>> inevitably need to add some new macros. If mutable is not a good name,
>>>> suggestions for better alternatives would be welcome; coming up with a
>>>> suitable name is indeed rather tricky.  
>>>
>>> I don't think you need to add a new macro for the specific use case that people want to modify the next element of the iteration.
>>>
>>> If I remember your numbers correctly that is a really corner case and keeping using the existing *_safe() macros for that sounds perfectly fine to me.
>> 
>> IIRC currently you have a choice of either:
>> 	define               Item that can't be deleted
>> 	list_for_each()	     The current item.
>> 	list_for_each_safe() The next item.
>> There is also likely to be code that updates the variables to allow
>> for other scenarios.
>> 
>> Note that if increase a reference count and release a lock then list_for_each()
>> is likely safer than list_for_each_safe() :-)
>> 
>> list.h has 9 variants of the 'safe' loop.
>> The bloat of another 9 is getting excessive.
>> 
>> It has to be said that this is one of my least favourite type of list...
>
> Hi Christian König, David Laight, Jani Nikula, David Hildenbrand,
> Andy Shevchenko, Alexei Starovoitov
>
> For ease of discussion, I need to summarize the currently possible
> approaches and briefly describe their respective pros and cons,
> using the list_for_each_entry* interfaces as examples.
>
> 1. Add list_for_each_entry_mutable, while keeping list_for_each_entry
> and list_for_each_entry_safe unchanged. list_for_each_entry_mutable
> would be used specifically for safe deletion scenarios that do not
> need to expose the temporary cursor externally. The code can refer to
> the v1 version.
>
> Pros: Does not depend on immediate per-subsystem adaptation and can be
>       merged directly.
> Cons: Requires adding a whole set of mutable interfaces, which makes the
>       code somewhat redundant.

Seems fine, and the original _safe naming is ambiguous anyway.

> 2. Directly optimize away the temporary cursor in list_for_each_entry_safe
> and define it inside the loop instead, changing the interface from four
> arguments to three.
>
> Pros: Does not add redundant interfaces.
> Cons: (1) Users need to manually update special cases that use the
>       traversal variable of list_for_each_entry_safe, the new
>       list_for_each_entry_safe would no longer apply there and would
>       need to be open-coded.
>       (2) Because the macro arguments changes, all list_for_each_entry_safe
>       callers would need to be modified and merged together, making it
>       difficult to merge such a large amount of code at once.

This won't fly because there are literally thousands of
list_for_each_entry_safe() users.

> 3. Use a variadic macro approach to optimize list_for_each_entry_safe,
> so that it supports both three and four arguments.
>
> Pros: (1) Does not add redundant interfaces.
>       (2) Does not depend on immediate per-subsystem adaptation and can
>       be merged directly.
> Cons: (1) Increases compile time.
>       (2) Makes the interface harder for users to use.

Basically I'm against any variadic macro tricks where the optional
argument is not the last argument. That's just way too surprising, and
goes against common practice in just about all other languages.

> 4. Optimize list_for_each_entry by defining the temporary cursor internally,
> making it compatible with the functionality of list_for_each_entry_safe.
> The code can refer to the v2 version.
>
> Pros: (1) Does not add redundant interfaces.
>       (2) The number of externally visible arguments of list_for_each_entry
>       remains unchanged, still three.
> Cons: (1) list_for_each_entry and list_for_each_entry_safe would be merged
>       into one, and list_for_each_entry_safe would gradually be deprecated.
>       (2) Users need to manually update special cases that use the traversal
>       variable of list_for_each_entry, the new list_for_each_entry would no
>       longer apply there and would need to be open-coded. There are 15 such
>       cases in total.

This sounds good to me, though I take it there's some code size increase
and/or performance penalty?

Maybe the 15 cases are questionable anyway?

> 5. Use a variadic macro approach to optimize list_for_each_entry, so that
> it supports both three and four arguments.
>
> Pros: (1) Does not add redundant interfaces.
>       (2) Does not depend on immediate per-subsystem adaptation and can be
>       merged directly.
> Cons: (1) Increases compile time.
>       (2) list_for_each_entry and list_for_each_entry_safe would be merged
>       into one, and list_for_each_entry_safe would gradually be deprecated.

Please don't do the macro tricks.

> 6. Make no changes, keep the current logic unchanged, and close the current
> email discussion.

I like hiding the temporary stuff when possible.


BR,
Jani.

-- 
Jani Nikula, Intel

^ permalink raw reply

* Re: [PATCH net v3 1/2] iov_iter: export iov_iter_restore
From: Christian Brauner @ 2026-06-25 10:43 UTC (permalink / raw)
  To: Octavian Purdila
  Cc: netdev, Alexander Viro, Andrew Morton, Arseniy Krasnov,
	David S. Miller, Eric Dumazet, Eugenio Pérez, Jakub Kicinski,
	Jason Wang, kvm, linux-block, linux-fsdevel, linux-kernel,
	Michael S. Tsirkin, Paolo Abeni, Simon Horman, Stefan Hajnoczi,
	Stefano Garzarella, virtualization, Xuan Zhuo, Jens Axboe
In-Reply-To: <20260622222757.2130402-2-tavip@google.com>

> Export iov_iter_restore so that it can be used by modules.
> 
> This is needed by the virtio vsock transport (which can be built as a
> module) to restore the msg_iter state when transmission fails.
> 
> Acked-by: Stefano Garzarella <sgarzare@redhat.com>
> Signed-off-by: Octavian Purdila <tavip@google.com>
>
> diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> index 273919b16161..f5df63961fb2 100644
> --- a/lib/iov_iter.c
> +++ b/lib/iov_iter.c
> @@ -1491,6 +1491,7 @@ void iov_iter_restore(struct iov_iter *i, struct iov_iter_state *state)
>  		i->__iov -= state->nr_segs - i->nr_segs;
>  	i->nr_segs = state->nr_segs;
>  }
> +EXPORT_SYMBOL_GPL(iov_iter_restore);

At least only export it for the module that really needs it. For
example, see:

EXPORT_SYMBOL_FOR_MODULES(__kernel_write, "autofs4");

-- 
Christian Brauner <brauner@kernel.org>

^ permalink raw reply

* Re: [PATCH] vdpa_sim: fix cleanup after worker creation failure
From: Eugenio Perez Martin @ 2026-06-25  8:54 UTC (permalink / raw)
  To: Xiong Weimin; +Cc: virtualization, jasowang, mst, xuanzhuo
In-Reply-To: <20260625083017.497842-1-15927021679@163.com>

On Thu, Jun 25, 2026 at 10:37 AM Xiong Weimin <15927021679@163.com> wrote:
>
> From: Xiong Weimin <xiongweimin@kylinos.cn>
>
> vdpasim_create() leaves vdpasim->worker as an ERR_PTR when
> kthread_run_worker() fails. The error path then drops the device
> reference, which releases the partially initialized simulator.
>
> vdpasim_free() unconditionally passes the worker pointer to
> kthread_destroy_worker(), so the ERR_PTR is dereferenced and can
> trigger a general protection fault.
>
> Store the worker error, clear the pointer, and make the release path
> only clean up resources that were successfully initialized before
> the failure.
>
> Tested on openEuler VM with kernel 6.16.8: module build, reload,
> and vdpa dev add via vdpasim_net.
>
> Signed-off-by: Xiong Weimin <xiongweimin@kylinos.cn>
> ---
>  drivers/vdpa/vdpa_sim/vdpa_sim.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> index 1111111..2222222 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -231,8 +231,11 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr,
>         kthread_init_work(&vdpasim->work, vdpasim_work_fn);
>         vdpasim->worker = kthread_run_worker(0, "vDPA sim worker: %s",
>                                                 dev_attr->name);
> -       if (IS_ERR(vdpasim->worker))
> +       if (IS_ERR(vdpasim->worker)) {
> +               ret = PTR_ERR(vdpasim->worker);
> +               vdpasim->worker = NULL;
>                 goto err_iommu;
> +       }
>
>         mutex_init(&vdpasim->mutex);
>         spin_lock_init(&vdpasim->iommu_lock);
> @@ -750,18 +753,24 @@ static void vdpasim_free(struct vdpa_device *vdpa)
>         struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
>         int i;
>
> -       kthread_cancel_work_sync(&vdpasim->work);
> -       kthread_destroy_worker(vdpasim->worker);
> +       if (vdpasim->worker) {
> +               kthread_cancel_work_sync(&vdpasim->work);
> +               kthread_destroy_worker(vdpasim->worker);
> +       }
>
> -       for (i = 0; i < vdpasim->dev_attr.nvqs; i++) {
> -               vringh_kiov_cleanup(&vdpasim->vqs[i].out_iov);
> -               vringh_kiov_cleanup(&vdpasim->vqs[i].in_iov);
> +       if (vdpasim->vqs) {
> +               for (i = 0; i < vdpasim->dev_attr.nvqs; i++) {
> +                       vringh_kiov_cleanup(&vdpasim->vqs[i].out_iov);
> +                       vringh_kiov_cleanup(&vdpasim->vqs[i].in_iov);
> +               }
>         }
>
>         vdpasim->dev_attr.free(vdpasim);
>
> -       for (i = 0; i < vdpasim->dev_attr.nas; i++)
> -               vhost_iotlb_reset(&vdpasim->iommu[i]);
> +       if (vdpasim->iommu && vdpasim->iommu_pt) {
> +               for (i = 0; i < vdpasim->dev_attr.nas; i++)
> +                       vhost_iotlb_reset(&vdpasim->iommu[i]);
> +       }
>         kfree(vdpasim->iommu);
>         kfree(vdpasim->iommu_pt);
>         kfree(vdpasim->vqs);

Isn't this the exact same fix as v1 of
https://lore.kernel.org/lkml/20260620100959.2070316-1-slf@hdu.edu.cn/
?


^ permalink raw reply

* [PATCH] vdpa_sim: fix cleanup after worker creation failure
From: Xiong Weimin @ 2026-06-25  8:30 UTC (permalink / raw)
  To: virtualization; +Cc: jasowang, mst, xuanzhuo, eperezma

From: Xiong Weimin <xiongweimin@kylinos.cn>

vdpasim_create() leaves vdpasim->worker as an ERR_PTR when
kthread_run_worker() fails. The error path then drops the device
reference, which releases the partially initialized simulator.

vdpasim_free() unconditionally passes the worker pointer to
kthread_destroy_worker(), so the ERR_PTR is dereferenced and can
trigger a general protection fault.

Store the worker error, clear the pointer, and make the release path
only clean up resources that were successfully initialized before
the failure.

Tested on openEuler VM with kernel 6.16.8: module build, reload,
and vdpa dev add via vdpasim_net.

Signed-off-by: Xiong Weimin <xiongweimin@kylinos.cn>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 1111111..2222222 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -231,8 +231,11 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr,
 	kthread_init_work(&vdpasim->work, vdpasim_work_fn);
 	vdpasim->worker = kthread_run_worker(0, "vDPA sim worker: %s",
 						dev_attr->name);
-	if (IS_ERR(vdpasim->worker))
+	if (IS_ERR(vdpasim->worker)) {
+		ret = PTR_ERR(vdpasim->worker);
+		vdpasim->worker = NULL;
 		goto err_iommu;
+	}
 
 	mutex_init(&vdpasim->mutex);
 	spin_lock_init(&vdpasim->iommu_lock);
@@ -750,18 +753,24 @@ static void vdpasim_free(struct vdpa_device *vdpa)
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
 	int i;
 
-	kthread_cancel_work_sync(&vdpasim->work);
-	kthread_destroy_worker(vdpasim->worker);
+	if (vdpasim->worker) {
+		kthread_cancel_work_sync(&vdpasim->work);
+		kthread_destroy_worker(vdpasim->worker);
+	}
 
-	for (i = 0; i < vdpasim->dev_attr.nvqs; i++) {
-		vringh_kiov_cleanup(&vdpasim->vqs[i].out_iov);
-		vringh_kiov_cleanup(&vdpasim->vqs[i].in_iov);
+	if (vdpasim->vqs) {
+		for (i = 0; i < vdpasim->dev_attr.nvqs; i++) {
+			vringh_kiov_cleanup(&vdpasim->vqs[i].out_iov);
+			vringh_kiov_cleanup(&vdpasim->vqs[i].in_iov);
+		}
 	}
 
 	vdpasim->dev_attr.free(vdpasim);
 
-	for (i = 0; i < vdpasim->dev_attr.nas; i++)
-		vhost_iotlb_reset(&vdpasim->iommu[i]);
+	if (vdpasim->iommu && vdpasim->iommu_pt) {
+		for (i = 0; i < vdpasim->dev_attr.nas; i++)
+			vhost_iotlb_reset(&vdpasim->iommu[i]);
+	}
 	kfree(vdpasim->iommu);
 	kfree(vdpasim->iommu_pt);
 	kfree(vdpasim->vqs);
-- 
2.39.0


^ permalink raw reply related

* RE: [RFCv2 PATCH 2/6] efi/unaccepted: Set unaccepted bits for all hotplug memory
From: Duan, Zhenzhong @ 2026-06-25  6:38 UTC (permalink / raw)
  To: Kiryl Shutsemau
  Cc: marcandre.lureau@redhat.com, david@kernel.org, Edgecombe, Rick P,
	prsampat@amd.com, pbonzini@redhat.com, mst@redhat.com,
	peterx@redhat.com, Qiang, Chenyi, Reshetova, Elena,
	michael.roth@amd.com, ackerleytng@google.com,
	linux-kernel@vger.kernel.org, linux-coco@lists.linux.dev,
	virtualization@lists.linux.dev, x86@kernel.org, Xu, Yilun,
	Li, Xiaoyao, Peng, Chao P
In-Reply-To: <ajvNXyYwb7FXAJhP@thinkstation>



>-----Original Message-----
>From: Kiryl Shutsemau <kas@kernel.org>
>Subject: Re: [RFCv2 PATCH 2/6] efi/unaccepted: Set unaccepted bits for all hotplug
>memory
>
>On Tue, Jun 23, 2026 at 06:17:33AM -0400, Zhenzhong Duan wrote:
>> In coco guests, hotpluggable memory ranges are initially unaccepted.
>> While a previous change expanded the unaccepted memory bitmap boundaries
>> to include these hotplug spaces, the actual bits inside the bitmap are
>> not yet marked as unaccepted.
>>
>> Walks SRAT a second time after the bitmap is allocated and sets the bits
>> corresponding to hotpluggable ranges.
>>
>> This ensures the bitmap state accurately reflects all static and hotplug
>> memory ranges before booting kernel.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>  .../firmware/efi/libstub/unaccepted_memory.c   | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/firmware/efi/libstub/unaccepted_memory.c
>b/drivers/firmware/efi/libstub/unaccepted_memory.c
>> index bfbb78bd7b8a..01bed8e751ca 100644
>> --- a/drivers/firmware/efi/libstub/unaccepted_memory.c
>> +++ b/drivers/firmware/efi/libstub/unaccepted_memory.c
>> @@ -92,6 +92,23 @@ static void update_mem_boundaries(struct
>acpi_srat_mem_affinity *mem, struct sra
>>  		*(ctx->mem_end) = range_end;
>>  }
>>
>> +static void mark_hotplug_memory_unaccepted(struct acpi_srat_mem_affinity
>*mem,
>> +					   struct srat_parse_ctx *ctx)
>> +{
>> +	u64 unit_size = unaccepted_table->unit_size;
>> +	u64 start, end;
>> +
>> +	start = round_up(mem->base_address, unit_size);
>> +	end = round_down(mem->base_address + mem->length, unit_size);
>
>We can get here with start > end if srat range is less then unit_size.

Will add a check to ignore small range less than unit_size:

+       if (start >= end)
+               return;
+

Thanks
Zhenzhong

^ permalink raw reply

* RE: [RFCv2 PATCH 5/6] mm/memory_hotplug: Support ACPI hotplug/unplug for coco guest
From: Duan, Zhenzhong @ 2026-06-25  5:56 UTC (permalink / raw)
  To: Kiryl Shutsemau
  Cc: marcandre.lureau@redhat.com, david@kernel.org, Edgecombe, Rick P,
	prsampat@amd.com, pbonzini@redhat.com, mst@redhat.com,
	peterx@redhat.com, Qiang, Chenyi, Reshetova, Elena,
	michael.roth@amd.com, ackerleytng@google.com,
	linux-kernel@vger.kernel.org, linux-coco@lists.linux.dev,
	virtualization@lists.linux.dev, x86@kernel.org, Xu, Yilun,
	Li, Xiaoyao, Peng, Chao P
In-Reply-To: <ajvOP9NwqNstgNvl@thinkstation>



>-----Original Message-----
>From: Kiryl Shutsemau <kas@kernel.org>
>Subject: Re: [RFCv2 PATCH 5/6] mm/memory_hotplug: Support ACPI
>hotplug/unplug for coco guest
>
>On Tue, Jun 23, 2026 at 06:17:36AM -0400, Zhenzhong Duan wrote:
>> +	spin_lock_irqsave(&unaccepted_memory_lock, flags);
>> +	for (; range_start < bitmap_size; range_start = range_end) {
>> +		unsigned long phys_start, phys_end;
>> +		unsigned long unaccepted_one, plugged_zero;
>> +
>> +		range_start = find_next_andnot_bit(plugged_bitmap,
>unaccepted->bitmap,
>> +						   bitmap_size, range_start);
>> +
>> +		if (range_start >= bitmap_size)
>> +			break;
>> +
>> +		unaccepted_one = find_next_bit(unaccepted->bitmap,
>bitmap_size, range_start);
>> +		plugged_zero = find_next_zero_bit(plugged_bitmap, bitmap_size,
>range_start);
>> +		range_end = min(unaccepted_one, plugged_zero);
>> +
>> +		phys_start = range_start * unit_size + unaccepted->phys_base;
>> +		phys_end = range_end * unit_size + unaccepted->phys_base;
>> +
>> +		arch_unaccept_memory(phys_start, phys_end);
>> +		bitmap_set(unaccepted->bitmap, range_start, range_end -
>range_start);
>> +	}
>> +	spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
>
>Accept TDCALL under the spin lock will kill scalability.

OK, I can drop the lock during arch_unaccept_memory() and avoid race
by checking the accepting_list just like in accept_memory().

I initially wrapped this in the spinlock because TDG.MEM.PAGE.RELEASE
is a quick local TDX module call to transition pages back to PENDING state,
without the heavy VMM trapping/faulting overhead associated with
memory acceptance paths.

Thanks
Zhenzhong

^ permalink raw reply

* Re: [PATCH v3 1/7] list: Add mutable iterator variants
From: Kaitao Cheng @ 2026-06-25  3:01 UTC (permalink / raw)
  To: David Laight, Christian König, Jani Nikula,
	David Hildenbrand (Arm), Alexei Starovoitov
  Cc: Andrew Morton, David Hildenbrand, Jens Axboe, Tejun Heo,
	Alexander Viro, Christian Brauner, Daniel Borkmann,
	Andrii Nakryiko, Johannes Weiner, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Namhyung Kim, Thomas Gleixner,
	Juri Lelli, Vincent Guittot, Paul Moore, Andy Shevchenko,
	Paul E. McKenney, Shakeel Butt, David Howells, Simona Vetter,
	Randy Dunlap, Luca Ceresoli, Philipp Stanner, linux-block,
	linux-kernel, cgroups, linux-ntfs-dev, linux-fsdevel, io-uring,
	audit, bpf, netdev, dri-devel, linux-perf-users,
	linux-trace-kernel, kexec, live-patching, linux-modules,
	linux-crypto, linux-pm, rcu, sched-ext, linux-mm, virtualization,
	damon, llvm, Kaitao Cheng, Muchun Song
In-Reply-To: <20260624152324.3def88ce@pumpkin>

在 2026/6/24 22:23, David Laight 写道:
> On Wed, 24 Jun 2026 15:23:47 +0200
> Christian König <christian.koenig@amd.com> wrote:
>> On 6/24/26 15:14, Kaitao Cheng wrote:
>>> 在 2026/6/22 16:42, David Laight 写道:  
>>>> On Mon, 22 Jun 2026 12:05:31 +0800
>>>> Kaitao Cheng <kaitao.cheng@linux.dev> wrote:
>>>>  
>>>>> From: Kaitao Cheng <chengkaitao@kylinos.cn>
>>>>>
>>>>> The list_for_each*_safe() helpers are used when the loop body may
>>>>> remove the current entry.  Their API exposes the temporary cursor at
>>>>> every call site, even though most users only need it for the iterator
>>>>> implementation and never reference it in the loop body.
>>>>>
>>>>> Add *_mutable() variants for list and hlist iteration.  The new helpers
>>>>> support both forms: callers may keep passing an explicit temporary cursor
>>>>> when they need to inspect or reset it, or omit it and let the helper use
>>>>> a unique internal cursor.  
>>>>
>>>> I'm not really sure 'mutable' means anything either.
>>>> It is possible to make it valid for the loop body (or even other threads)
>>>> to delete arbitrary list items - but that needs significant extra overheads.
>>>>
>>>> It might be worth doing something that doesn't need the extra variable,
>>>> but there is little point doing all the churn just to rename things.
>>>>  
>>>>>
>>>>> This makes call sites that only mutate the list through the current entry
>>>>> less noisy, while keeping the existing *_safe() helpers available for
>>>>> compatibility.
>>>>>
>>>>> Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
>>>>> ---
>>>>>  include/linux/list.h | 269 +++++++++++++++++++++++++++++++++++++------
>>>>>  1 file changed, 231 insertions(+), 38 deletions(-)
>>>>>
>>>>> diff --git a/include/linux/list.h b/include/linux/list.h
>>>>> index 09d979976b3b..1081def7cea9 100644
>>>>> --- a/include/linux/list.h
>>>>> +++ b/include/linux/list.h
>>>>> @@ -7,6 +7,7 @@
>>>>>  #include <linux/stddef.h>
>>>>>  #include <linux/poison.h>
>>>>>  #include <linux/const.h>
>>>>> +#include <linux/args.h>
>>>>>  
>>>>>  #include <asm/barrier.h>
>>>>>  
>>>>> @@ -763,28 +764,72 @@ static inline void list_splice_tail_init(struct list_head *list,
>>>>>  #define list_for_each_prev(pos, head) \
>>>>>  	for (pos = (head)->prev; !list_is_head(pos, (head)); pos = pos->prev)
>>>>>  
>>>>> -/**
>>>>> - * list_for_each_safe - iterate over a list safe against removal of list entry
>>>>> - * @pos:	the &struct list_head to use as a loop cursor.
>>>>> - * @n:		another &struct list_head to use as temporary storage
>>>>> - * @head:	the head for your list.
>>>>> +/*
>>>>> + * list_for_each_safe is an old interface, use list_for_each_mutable instead.
>>>>>   */
>>>>>  #define list_for_each_safe(pos, n, head) \
>>>>>  	for (pos = (head)->next, n = pos->next; \
>>>>>  	     !list_is_head(pos, (head)); \
>>>>>  	     pos = n, n = pos->next)
>>>>>  
>>>>> +#define __list_for_each_mutable_internal(pos, tmp, head)		\
>>>>> +	for (typeof(pos) tmp = (pos = (head)->next)->next;		\  
>>>>
>>>> Use auto
>>>>  
>>>>> +	     !list_is_head(pos, (head));				\
>>>>> +	     pos = tmp, tmp = pos->next)
>>>>> +
>>>>> +#define __list_for_each_mutable1(pos, head)				\
>>>>> +	__list_for_each_mutable_internal(pos, __UNIQUE_ID(next), head)
>>>>> +
>>>>> +#define __list_for_each_mutable2(pos, next, head)			\
>>>>> +	list_for_each_safe(pos, next, head)
>>>>> +
>>>>>  /**
>>>>> - * list_for_each_prev_safe - iterate over a list backwards safe against removal of list entry
>>>>> + * list_for_each_mutable - iterate over a list safe against entry removal
>>>>>   * @pos:	the &struct list_head to use as a loop cursor.
>>>>> - * @n:		another &struct list_head to use as temporary storage
>>>>> - * @head:	the head for your list.
>>>>> + * @...:	either (head) or (next, head)
>>>>> + *
>>>>> + * next:	another &struct list_head to use as optional temporary storage.
>>>>> + *		The temporary cursor is internal unless explicitly supplied by
>>>>> + *		the caller.
>>>>> + * head:	the head for your list.
>>>>> + */
>>>>> +#define list_for_each_mutable(pos, ...)					\
>>>>> +	CONCATENATE(__list_for_each_mutable, COUNT_ARGS(__VA_ARGS__))	\
>>>>> +		(pos, __VA_ARGS__)  
>>>>
>>>> The variable argument count logic really just slows down compilation.
>>>> Maybe there aren't enough copies of this code to make that significant.
>>>> But just because you can do it doesn't mean it is a gooD idea.
>>>> I'm also not sure it really adds anything to the readability.
>>>>
>>>> And, it you are going to make the middle argument optional there is
>>>> no need to change the macro name.  
>>>
>>> Christian König and Jani Nikula also disagree with the variadic-argument
>>> implementation approach. If we abandon that method, it means we will
>>> inevitably need to add some new macros. If mutable is not a good name,
>>> suggestions for better alternatives would be welcome; coming up with a
>>> suitable name is indeed rather tricky.  
>>
>> I don't think you need to add a new macro for the specific use case that people want to modify the next element of the iteration.
>>
>> If I remember your numbers correctly that is a really corner case and keeping using the existing *_safe() macros for that sounds perfectly fine to me.
> 
> IIRC currently you have a choice of either:
> 	define               Item that can't be deleted
> 	list_for_each()	     The current item.
> 	list_for_each_safe() The next item.
> There is also likely to be code that updates the variables to allow
> for other scenarios.
> 
> Note that if increase a reference count and release a lock then list_for_each()
> is likely safer than list_for_each_safe() :-)
> 
> list.h has 9 variants of the 'safe' loop.
> The bloat of another 9 is getting excessive.
> 
> It has to be said that this is one of my least favourite type of list...

Hi Christian König, David Laight, Jani Nikula, David Hildenbrand,
Andy Shevchenko, Alexei Starovoitov

For ease of discussion, I need to summarize the currently possible
approaches and briefly describe their respective pros and cons,
using the list_for_each_entry* interfaces as examples.

1. Add list_for_each_entry_mutable, while keeping list_for_each_entry
and list_for_each_entry_safe unchanged. list_for_each_entry_mutable
would be used specifically for safe deletion scenarios that do not
need to expose the temporary cursor externally. The code can refer to
the v1 version.

Pros: Does not depend on immediate per-subsystem adaptation and can be
      merged directly.
Cons: Requires adding a whole set of mutable interfaces, which makes the
      code somewhat redundant.

2. Directly optimize away the temporary cursor in list_for_each_entry_safe
and define it inside the loop instead, changing the interface from four
arguments to three.

Pros: Does not add redundant interfaces.
Cons: (1) Users need to manually update special cases that use the
      traversal variable of list_for_each_entry_safe, the new
      list_for_each_entry_safe would no longer apply there and would
      need to be open-coded.
      (2) Because the macro arguments changes, all list_for_each_entry_safe
      callers would need to be modified and merged together, making it
      difficult to merge such a large amount of code at once.

3. Use a variadic macro approach to optimize list_for_each_entry_safe,
so that it supports both three and four arguments.

Pros: (1) Does not add redundant interfaces.
      (2) Does not depend on immediate per-subsystem adaptation and can
      be merged directly.
Cons: (1) Increases compile time.
      (2) Makes the interface harder for users to use.

4. Optimize list_for_each_entry by defining the temporary cursor internally,
making it compatible with the functionality of list_for_each_entry_safe.
The code can refer to the v2 version.

Pros: (1) Does not add redundant interfaces.
      (2) The number of externally visible arguments of list_for_each_entry
      remains unchanged, still three.
Cons: (1) list_for_each_entry and list_for_each_entry_safe would be merged
      into one, and list_for_each_entry_safe would gradually be deprecated.
      (2) Users need to manually update special cases that use the traversal
      variable of list_for_each_entry, the new list_for_each_entry would no
      longer apply there and would need to be open-coded. There are 15 such
      cases in total.

5. Use a variadic macro approach to optimize list_for_each_entry, so that
it supports both three and four arguments.

Pros: (1) Does not add redundant interfaces.
      (2) Does not depend on immediate per-subsystem adaptation and can be
      merged directly.
Cons: (1) Increases compile time.
      (2) list_for_each_entry and list_for_each_entry_safe would be merged
      into one, and list_for_each_entry_safe would gradually be deprecated.

6. Make no changes, keep the current logic unchanged, and close the current
email discussion.


Which of the six solutions above do people prefer?

-- 
Thanks
Kaitao Cheng


^ permalink raw reply

* [PATCH] drm/virtio: fail init on display-info timeout
From: Pengpeng Hou @ 2026-06-25  3:02 UTC (permalink / raw)
  To: David Airlie, Gerd Hoffmann, Dmitry Osipenko
  Cc: Gurchetan Singh, Chia-I Wu, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Simona Vetter, dri-devel, virtualization,
	linux-kernel, pengpeng

virtio_gpu_init() sends GET_DISPLAY_INFO when scanouts are present and
waits for display_info_pending to clear. If the response never arrives,
the wait result is ignored and probe still succeeds.

Return -ETIMEDOUT on display-info timeout. Because this happens after
virtio_device_ready(), reset the device and tear down modesetting before
using the existing vbuf and virtqueue cleanup path.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/gpu/drm/virtio/virtgpu_kms.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index cfde9f573df6..31209bea97ae 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -262,11 +262,19 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
 			virtio_gpu_cmd_get_edids(vgdev);
 		virtio_gpu_cmd_get_display_info(vgdev);
 		virtio_gpu_notify(vgdev);
-		wait_event_timeout(vgdev->resp_wq, !vgdev->display_info_pending,
-				   5 * HZ);
+		if (!wait_event_timeout(vgdev->resp_wq,
+					!vgdev->display_info_pending,
+					5 * HZ)) {
+			DRM_ERROR("timed out waiting for display info\n");
+			ret = -ETIMEDOUT;
+			goto err_ready;
+		}
 	}
 	return 0;
 
+err_ready:
+	virtio_reset_device(vgdev->vdev);
+	virtio_gpu_modeset_fini(vgdev);
 err_scanouts:
 	virtio_gpu_free_vbufs(vgdev);
 err_vbufs:
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox