* Re: [PATCH v3] media: add virtio-media driver
From: Brian Daniels @ 2026-07-20 20:50 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: acourbot, adelva, aesteve, changyeon, daniel.almeida, eperezma,
gnurou, gurchetansingh, hverkuil, jasowang, linux-kernel,
linux-media, mchehab, mst, nicolas.dufresne, virtualization,
xuanzhuo
In-Reply-To: <20260712082408.49de652a@foz.lan>
On Sun, Jul 12, 2026 at 2:30 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> On Fri, 29 May 2026 12:03:04 -0400
> Brian Daniels <briandaniels@google.com> wrote:
>
> > Hi there! My name is Brian Daniels and I'll be taking over upstreaming this
> > driver from Alexandre Courbot.
> >
> > I've consulted with Alexandre and my plan is to upload a v4 set of patches
> > shortly based on the feedback from this revision.
>
> As you promised a v5 to Michael, I'll wait for it to review the new series.
>
> For now, let me address the replies from you on the top of v3.
Thanks very much! I'll try to note any comments that we can move to
the v4 thread so we can consolidate our discussion.
> > Before doing so, I'd like to address some of your comments:
> >
> > > Hi Alex,
> > >
> > > I didn't see on a first glance anything that would cause locking
> > > issues here, but, as I pointed on my last e-mail, testing with
> > > qv4l2 at the max res of my C920 camera, it ended keeping 24 CPUs
> > > busy without showing any results with qv4l2 (via ssh at the same
> > > machine). So, I suspect that there are issues somewhere, but I didn't
> > > debug any further.
> > >
> > > Please do some tests with a high-res camera, using either ssh or
> > > GPU emulation to see how this behaves with real apps.
> >
> > I did some testing with a 1080p USB webcam over ssh using ffmpeg and I was able
> > to stream video to disk without any issue. Let me know if you'd prefer I test
> > with a specific setup.
>
> You need to test it displaying on a X11 or Wayland window inside the VM, not
> just via an app like ffmeg.
I was able to test this successfully. Please see my comment in the v4 thread.
> >
> > > > +config MEDIA_VIRTIO
> > > > + tristate "Virtio-media Driver"
> > > > + depends on VIRTIO && VIDEO_DEV && 64BIT && (X86 || (ARM && CPU_LITTLE_ENDIAN))
> > >
> > > Why are you limiting it to x86_64 and arm64 little endian?
> >
> > The little endian requirement comes for the section 5.22.6.1.5 of the virtio
> > v1.4 specification [1]. The limitation to x86_64 and arm64 is for two reasons:
> >
> > 1. The specification requires all v4l2 structures to use the 64-bit layout
> > 2. This driver has only been tested on x86_64 and arm64 so far
>
> I don't like non-portable interfaces. If the spec is not good, it needs to
> be fixed; if the problem is at the API interface, you may need compat32 or
> similar to ensure that the interface will work independently on host/guest
> CPU differences.
>
For guests that are not natively 64-bit little endian, the driver
would be required to perform the translation. This is done in other
virtio drivers (e.g. virtio input: in drivers/virtio/virtio_input.c,
you see `cpu_to_le*` and `le*_to_cpu` doing the conversions).
This driver currently doesn't perform this translation, which is why
the Kconfig includes the restrictions. Does this need to be addressed
in the initial version? Or can the translation be added in the future
for expanded platform support?
> >
> >
> > > > +/**
> > > > + * enum virtio_media_memory - Memory types supported by virtio-media.
> > > > + * @VIRTIO_MEDIA_MMAP: memory allocated and managed by device. Can be mapped
> > > > + * into the guest using VIRTIO_MEDIA_CMD_MMAP.
> > > > + * @VIRTIO_MEDIA_SHARED_PAGES: memory allocated by the driver. Passed to the
> > > > + * device using virtio_media_sg_entry.
> > > > + * @VIRTIO_MEDIA_OBJECT: memory backed by a virtio object.
> > > > + */
> > > > +enum virtio_media_memory {
> > > > + VIRTIO_MEDIA_MMAP = V4L2_MEMORY_MMAP,
> > > > + VIRTIO_MEDIA_SHARED_PAGES = V4L2_MEMORY_USERPTR,
> > > > + VIRTIO_MEDIA_OBJECT = V4L2_MEMORY_DMABUF,
> > > > +};
> > >
> > > I'm not a big fan of renaming USERPTR to SHARED_PAGES and
> > > DMABUF to OBJECT, as it makes harder for reviewers and contributors
> > > to remember about this mapping. Also, everybody knows exactly what
> > > DMABUF means, so, it sounds to me that this obfuscates a little bit
> > > the driver.
> > >
> > > Also, why are you encapsulating V4L2 names into VIRTIO_* namespace?
> > > This just adds extra complexity for reviewers without any real
> > > benefit.
> > >
> > > Besides that, doing a grep on the patch, it sounds that this ma
> > > is not used anywhere.
> > >
> > > So, please drop this mapping.
> >
> > Agreed I don't see it being used anywhere, I think this was included originally
> > since it's part of the virtio spec. I will remove it.
> >
> >
> > > > +#define VIRTIO_MEDIA_EVT_ERROR 0
> > > > +#define VIRTIO_MEDIA_EVT_DQBUF 1
> > > > +#define VIRTIO_MEDIA_EVT_EVENT 2
> > >
> > > OK, here, media events are different than virtio events, so having
> > > a virtio-specific events make sense. Yet, better to add a comment
> > > about that.
> > >
> > > Also, V4L events are defined as:
> > >
> > > #define V4L2_EVENT_ALL 0
> > > #define V4L2_EVENT_VSYNC 1
> > > #define V4L2_EVENT_EOS 2
> > > #define V4L2_EVENT_CTRL 3
> > > #define V4L2_EVENT_FRAME_SYNC 4
> > > #define V4L2_EVENT_SOURCE_CHANGE 5
> > > #define V4L2_EVENT_MOTION_DET 6
> > > #define V4L2_EVENT_PRIVATE_START 0x08000000
> > >
> > > As one may end wanting to map them on some future, I would change
> > > the definitions above to:
> > >
> > > #define VIRTIO_MEDIA_EVT_ERROR V4L2_EVENT_PRIVATE_START
> > > #define VIRTIO_MEDIA_EVT_DQBUF (V4L2_EVENT_PRIVATE_START + 1)
> > > #define VIRTIO_MEDIA_EVT_EVENT (V4L2_EVENT_PRIVATE_START + 2)
> >
> > These event values (VIRTIO_MEDIA_EVT_*) are set by section 5.22.6.2.1 of the
> > virtio v1.4 specification [2]. I don't believe we have the ability to change
> > these values as suggested without changing the specification. That would most
> > likely be a lengthy process, so I'd prefer to keep it as written if that's not
> > an issue.
>
> The specs you pointed seem to be specific to V4L2. I can't see any reason
> why not adjusting it to provide an interface that doesn't conflict with
> V4L2 existing events.
I added a response in the v4 thread where you brought it up as well,
let's continue the discussion there. Thanks!
> >
> > > > +#define VIRTIO_MEDIA_MAX_PLANES VIDEO_MAX_PLANES
> > >
> > > Here: why renaming it to VIRTIO_* namespace?
> >
> > This matches the name in section 5.22.6.2.3 of the virtio v1.4 specification
> > [3]. And Alexandre stated the reason why it was renamed from VIDEO_* to
> > VIRTIO_MEDIA_* was from an early piece of feedback to avoid V4L2-specific names.
> > Even though virtio-media reuses the v4l2 structures and API, its possible to use
> > virtio-media with a different media implementation other than v4l2.
>
> In thesis, it would be possible, but it sounds unlikely that this would
> ever happen, as other OSes may have completely different media APIs.
>
> So, in practice, if one wants, let's say, a Windows host to run a media
> virio, it would either need to implement V4L2 internally or will need
> different virtio drivers using a different API.
As far as using a non-Linux OS as your host, then yes you would need
to implement v4l2 internally,
or at least a subset that can interact with v4l2 drivers.
For your original question about why VIDEO_MAX_PLANES was renamed to
VIRTIO_MEDIA_MAX_PLANES, is there any action to take here? Or can it
be left as is?
> > > > +/**
> > > > + * struct virtio_media_session - A session on a virtio_media device.
> > > > + * @fh: file handler for the session.
> > > > + * @id: session ID used to communicate with the device.
> > > > + * @nonblocking_dequeue: whether dequeue should block or not (nonblocking if
> > > > + * file opened with O_NONBLOCK).
> > > > + * @uses_mplane: whether the queues for this session use the MPLANE API or not.
> > > > + * @cmd: union of session-related commands. A session can have one command currently running.
> > > > + * @resp: union of session-related responses. A session can wait on one command only.
> > > > + * @shadow_buf: shadow buffer where data to be added to the descriptor chain can
> > > > + * be staged before being sent to the device.
> > > > + * @command_sgs: SG table gathering descriptors for a given command and its response.
> > > > + * @queues: state of all the queues for this session.
> > > > + * @queues_lock: protects all members fo the queues for this session.
> > > > + * virtio_media_queue_state`.
> > > > + * @dqbuf_wait: waitqueue for dequeued buffers, if ``VIDIOC_DQBUF`` needs to
> > > > + * block or when polling.
> > > > + * @list: link into the list of sessions for the device.
> > > > + */
> > > > +struct virtio_media_session {
> > > > + struct v4l2_fh fh;
> > > > + u32 id;
> > > > + bool nonblocking_dequeue;
> > > > + bool uses_mplane;
> > > > +
> > > > + union {
> > > > + struct virtio_media_cmd_close close;
> > > > + struct virtio_media_cmd_ioctl ioctl;
> > > > + struct virtio_media_cmd_mmap mmap;
> > > > + } cmd;
> > > > +
> > > > + union {
> > > > + struct virtio_media_resp_ioctl ioctl;
> > > > + struct virtio_media_resp_mmap mmap;
> > > > + } resp;
> > > > +
> > >
> > > Heh, the above is tricky, as to parse the struct, one needs first
> > > to check cmd.cmd to identify what values to pick from enums.
> > > Also, the command is stored as: cmd.[close|ioctl|imap].cmd.
> > >
> > > IMO, better to place the headers explicitly there, e.g.
> > >
> > > union {
> > > struct virtio_media_cmd_header hdr;
> > > struct virtio_media_cmd_close close;
> > > struct virtio_media_cmd_ioctl ioctl;
> > > struct virtio_media_cmd_mmap mmap;
> > > } send;
> > > union {
> > > struct virtio_media_resp_header hdr;
> > > struct virtio_media_resp_ioctl ioctl;
> > > struct virtio_media_resp_mmap mmap;
> > > } resp;
> > >
> > > Also, currently, there are 5 defined commands:
> > >
> > > #define VIRTIO_MEDIA_CMD_OPEN 1
> > > #define VIRTIO_MEDIA_CMD_CLOSE 2
> > > #define VIRTIO_MEDIA_CMD_IOCTL 3
> > > #define VIRTIO_MEDIA_CMD_MMAP 4
> > > #define VIRTIO_MEDIA_CMD_MUNMAP 5
> > >
> > > If the data struct is limited only for close/ioctl/mmap, please
> > > document it and point what structure(s) other commands use.
> >
> > Tricky indeed!
> >
> > However, I don't believe its necessary to add an explict header member to the
> > union.
>
> My main concern is that the field used to check what is the content
> of the union is inside the union itself, e.g. virtio_media_cmd_header.cmd.
>
> So, if one does:
>
> struct virtio_media_session session;
>
> it can't check what's the command with session.cmd or session.hdr.cmd.
> Instead, it needs to guess one type, like session.close.cmd to check
> if the command encoded there is close, mmap or ioctl.
>
> > Whenever the driver parses the union, it already has the necessary
> > context to determine which union member to use:
> >
> > - The struct virtio_media_cmd_* instances are always created by the driver and
> > sent to the device, so there are no unknowns there
> > - The struct virtio_media_resp_* instances are always parsed in the same
> > function that sends the corresponding command, so again there's no
> > uncertainty about which union member to access.
>
> If it is always the same function that creates and uses it, you don't
> need an union. You can just use for instance
>
> struct virtio_media_cmd_ioctl
>
> inside the instances that deal with ioctl resp (or ioctl creation).
I will look into this further in v5. It sounds like there are some DMA
alignment and padding requirements that need addressing for these
unions as well.
I can add the "struct virtio_media_*_header" to the unions if it helps
improve readability, but the headers won't be used by the driver
as it's currently written.
> >
> > For these reasons, I would hesistate to add the `struct virtio_media_cmd_header
> > hdr` as previously suggested. Please let me know if you disagree or if I've
> > misunderstood your concern.
>
> >
> > That all being said, I have added comments about which structures use which
> > commands in the upcoming v4 of the patches.
> >
> >
> > > > +/**
> > > > + * struct virtio_media - Virtio-media device.
> > > > + * @v4l2_dev: v4l2_device for the media device.
> > > > + * @video_dev: video_device for the media device.
> > > > + * @virtio_dev: virtio device for the media device.
> > > > + * @commandq: virtio command queue.
> > > > + * @eventq: virtio event queue.
> > > > + * @eventq_work: work to run when events are received on @eventq.
> > > > + * @mmap_region: region into which MMAP buffers are mapped by the host.
> > > > + * @event_buffer: buffer for event descriptors.
> > > > + * @sessions: list of active sessions on the device.
> > > > + * @sessions_lock: protects @sessions and ``virtio_media_session::list``.
> > > > + * @events_lock: prevents concurrent processing of events.
> > > > + * @cmd: union of device-related commands.
> > > > + * @resp: union of device-related responses.
> > > > + * @vlock: serializes access to the command queue.
> > > > + * @wq: waitqueue for host responses on the command queue.
> > > > + */
> > > > +struct virtio_media {
> > > > + struct v4l2_device v4l2_dev;
> > > > + struct video_device video_dev;
> > > > +
> > > > + struct virtio_device *virtio_dev;
> > > > + struct virtqueue *commandq;
> > > > + struct virtqueue *eventq;
> > > > + struct work_struct eventq_work;
> > > > +
> > > > + struct virtio_shm_region mmap_region;
> > > > +
> > > > + void *event_buffer;
> > > > +
> > > > + struct list_head sessions;
> > > > + struct mutex sessions_lock;
> > > > +
> > > > + struct mutex events_lock;
> > > > +
>
> > > > + union {
> > > > + struct virtio_media_cmd_open open;
> > > > + struct virtio_media_cmd_munmap munmap;
> > > > + } cmd;
> > > > +
> > > > + union {
> > > > + struct virtio_media_resp_open open;
> > > > + struct virtio_media_resp_munmap munmap;
> > > > + } resp;
>
> btw the same comment about the past union also applies here:
> if the code needs somehow to identify if the command is open or
> munmap, the field used to determine what command is there should
> be independent of open/munmap.
Yup makes sense. Let's see where our discussion above goes and assume
it applies here as well going forward.
> > >
> > > Based on struct virtio_media_session, I'm assuming here that
> > > this struct is used only for two commands, right? Please document
> > > it at kernel-doc markup and add a point to the other structure used
> > > for the other commands.
> > >
> > > The same comment about headers apply to the union here: place
> > > the header explicitly at the union.
> >
> > The same thing I said above applies here as well. I will add the comments as
> > requested in v4.
> >
> > [1] https://docs.oasis-open.org/virtio/virtio/v1.4/csprd01/virtio-v1.4-csprd01-diff-from-v1.2-cs01.html#x1-8360005
> > [2] https://docs.oasis-open.org/virtio/virtio/v1.4/csprd01/virtio-v1.4-csprd01-diff-from-v1.2-cs01.html#x1-8560001
> > [3] https://docs.oasis-open.org/virtio/virtio/v1.4/csprd01/virtio-v1.4-csprd01-diff-from-v1.2-cs01.html#x1-8600003
>
>
>
> Thanks,
> Mauro
^ permalink raw reply
* Re: [PATCH v2 2/2] virtio_balloon: avoid shrinker execution during PM suspend
From: Link Lin @ 2026-07-20 23:44 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Andrew Morton, Vlastimil Babka, Michael S . Tsirkin,
virtualization, linux-mm, linux-kernel, prasin, rientjes, duenwen,
jasowang, xuanzhuo, Ammar Faizi, jiaqiyan, ahwilkins, Greg Thelen,
Alexander Duyck, jthoughton, stable
In-Reply-To: <29457fe7-0fe6-4bc0-81a8-9e67f85b3a3a@kernel.org>
On Fri, 17 Jul 2026 02:17:24 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > reclaiming free pages under memory pressure can trigger page reporting
> > which might access the deleted reporting virtqueue if it is not yet
> > frozen, or interact with other parts of the driver in a teardown state.
>
> i feel this is the thing to fix then? not the freeing itself.
>
> ...
>
> this is remove, taking locks seems futile - if something will acquire
> the lock here, we will be in trouble.
On Fri, 17 Jul 2026 11:30:44 +0200 David Hildenbrand <david@kernel.org> wrote:
> Why not set the boolean immediately after this, and call it "shrinker_disabled"
> instead of "suspended"?
>
> Then you can keep calling "return_free_pages_to_mm(vb, ULONG_MAX);" here.
Hi Michael, David,
Michael, your intuition here is exactly right: the freeing itself is
perfectly safe, and taking locks in the teardown path is an
anti-pattern we should absolutely avoid.
After re-tracing the execution paths based on your feedback, I
realized that Patch 2/2 is completely unnecessary.
The FPH shrinker doesn't touch virtqueues itself. It simply returns
pages to the buddy allocator. While this can trigger Free Page
Reporting, Patch 1/2 migrates the reporting worker to
system_freezable_wq, meaning the PM core natively guarantees that
worker is frozen before virtballoon_freeze() ever runs.
Furthermore, any asynchronous FPH hinting work happens on
vb->balloon_wq (triggered via config changes). Because vb->balloon_wq
is natively allocated with WQ_FREEZABLE, the PM core successfully
freezes this worker prior to virtballoon_freeze() as well.
Because all downstream paths are safely frozen by the PM core, a
concurrent shrinker call during suspend is entirely harmless. It will
merely return 0 (since remove_common() safely drains the
free_page_list anyway using ULONG_MAX) or safely execute without
interacting with a teardown state.
David, thank you for the cleaner `shrinker_disabled` suggestion, but
since the asynchronous workers are already natively frozen, we can
avoid touching the teardown paths and locking logic altogether.
I am going to drop this patch entirely from the series. I will send
out a single-patch v3 consisting solely of the page reporting fix
(Patch 1/2) shortly. Thank you both for the rigorous review that
steered us away from this unnecessary complexity!
Best,
Link
^ permalink raw reply
* [PATCH v3] mm/page_reporting: use system_freezable_wq to fix UAF during suspend
From: Link Lin @ 2026-07-21 0:55 UTC (permalink / raw)
To: Andrew Morton, Vlastimil Babka, Michael S . Tsirkin,
David Hildenbrand
Cc: virtualization, linux-mm, linux-kernel, prasin, rientjes, duenwen,
jasowang, xuanzhuo, Ammar Faizi, jiaqiyan, ahwilkins, Greg Thelen,
Alexander Duyck, jthoughton, stable, Link Lin
During PM freeze (e.g. S3 suspend or S4 hibernation), device drivers like
virtio_balloon reset their underlying virtio devices and delete their
virtqueues via vdev->config->del_vqs().
However, page reporting work (page_reporting_process) was scheduled on
the global system_wq. Because system_wq lacks the WQ_FREEZABLE flag, the
PM freezer skips it, leaving page_reporting_process active during suspend.
If pages are freed into the buddy allocator while suspending (for example,
when core MM invokes the balloon shrinker during S4 hibernation image
saving), page reporting triggers virtballoon_free_page_report() on deleted
virtqueues, resulting in a Use-After-Free / General Protection Fault:
[ 196.795226] general protection fault, probably for non-canonical address 0xaa1436fe70dae6df: 0000 [#1] SMP NOPTI
[ 196.825967] Workqueue: events page_reporting_process
[ 196.831038] RIP: 0010:virtqueue_add_split+0x233/0x4c0 [virtio_ring]
[ 196.927073] virtballoon_free_page_report+0x3a/0xe0 [virtio_balloon]
[ 196.946943] page_reporting_process+0x370/0x4f0
Fix this by switching page reporting work to system_freezable_wq. This
ensures that the PM freezer pauses page_reporting_process before device
drivers destroy their reporting virtqueues. Because the reporting worker
is frozen, memory reclamation/freeing (e.g. via shrinker execution) can
safely return pages to MM during freeze without triggering unfrozen
reporting work on deleted virtqueues.
This aligns with the driver's existing design. The comment in
virtballoon_freeze() states:
/*
* The workqueue is already frozen by the PM core before this
* function is called.
*/
Testing:
I have verified these fixes using Google’s virtualization infrastructure by
running continuous suspend/resume iterations (40+ cycles) while churning
memory using stress-ng (`stress-ng --vm 4 --vm-bytes 60% --timeout 1`) to
constantly create free pages for the buddy allocator. We also set the
`page_reporting_order` parameter to 0 to make the page reporting worker
highly sensitive, forcing it to pick up any 4K free pages. This confirmed
that the UAF crashes are no longer reproducible.
Fixes: 924a663f75e2 ("virtio-balloon: Reporting free page reservations")
Cc: stable@vger.kernel.org
Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: David Rientjes <rientjes@google.com>
Acked-by: David Hildenbrand (Arm) <david@kernel.org>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Link Lin <linkl@google.com>
---
v3:
- Dropped Patch 2/2 (virtio_balloon shrinker flag). Freeing pages via
the shrinker only returns pages to MM; with page reporting work now
properly serialized on system_freezable_wq, shrinker execution during
freeze is harmless and requires no additional locking/flags in
virtio_balloon.
- Link to v2: https://lore.kernel.org/all/20260717002311.681748-1-linkl@google.com/
v2:
- Split into a 2-patch series including explicit shrinker fencing.
- Link to RFC: https://lore.kernel.org/all/20260709224330.946683-1-linkl@google.com/
mm/page_reporting.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/mm/page_reporting.c b/mm/page_reporting.c
index 7418f2e500..4dc6f4b852 100644
--- a/mm/page_reporting.c
+++ b/mm/page_reporting.c
@@ -80,7 +80,8 @@ __page_reporting_request(struct page_reporting_dev_info *prdev)
* now we are limiting this to running no more than once every
* couple of seconds.
*/
- schedule_delayed_work(&prdev->work, PAGE_REPORTING_DELAY);
+ queue_delayed_work(system_freezable_wq, &prdev->work,
+ PAGE_REPORTING_DELAY);
}
/* notify prdev of free page reporting request */
@@ -343,7 +344,8 @@ static void page_reporting_process(struct work_struct *work)
*/
state = atomic_cmpxchg(&prdev->state, state, PAGE_REPORTING_IDLE);
if (state == PAGE_REPORTING_REQUESTED)
- schedule_delayed_work(&prdev->work, PAGE_REPORTING_DELAY);
+ queue_delayed_work(system_freezable_wq, &prdev->work,
+ PAGE_REPORTING_DELAY);
}
static DEFINE_MUTEX(page_reporting_mutex);
--
2.55.0
^ permalink raw reply related
* Re: [PATCH RFC v3 0/6] Add Rust virtio bindings and sample device
From: Manos Pitsidianakis @ 2026-07-21 6:21 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Manos Pitsidianakis, Peter Hilber, Stefano Garzarella,
Stefan Hajnoczi, Viresh Kumar, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Daniel Almeida, rust-for-linux,
Jason Wang, Xuan Zhuo, Eugenio Pérez, virtualization,
linux-kernel, Manos Pitsidianakis
In-Reply-To: <20260510-rust-virtio-v3-0-1427f14d67e1@pitsidianak.is>
Ping.
I have a different version of the series WIP with a virtio-rng instead
of virtio-rtc, based on
https://lore.kernel.org/lkml/20260529-rust-hw_random-virtio-rng-v1-0-b3153dd90311@pitsidianak.is/
(which needs a v2 also)
But since this series hasn't gotten any feedback on the virtio part, I'd
like to ask if there is any before respinning.
Thanks!
On Sun, 10 May 2026 16:38, Manos Pitsidianakis <manos@pitsidianak.is> wrote:
>Hi all, this RFC series adds Rust bindings for Virtio drivers
>(frontends in virtio parlance).
>
>As a PoC, it also adds a sample virtio-rtc driver which performs
>capability discovery through the virtqueue without registering any clock.
>
>Before I send a cleaned-up non-RFC I would like some initial feedback
>(i.e. is it something the upstream wants?)
>
>This was tested with the rust-vmm vhost-device-rtc device backend that I
>wrote[^0]:
>
>[^0]: https://github.com/rust-vmm/vhost-device/tree/main/vhost-device-rtc
>
>Instructions:
>
> Run the daemon in a separate terminal:
>
> $ cargo run --bin vhost-device-rtc -- -s /tmp/rtc.sock
>
> Then run the VM:
>
> $ qemu-system-aarch64 \
> -machine type=virt,virtualization=off,acpi=on \
> -cpu host \
> -smp 8 \
> -accel kvm \
> -drive if=virtio,format=qcow2,file=./debian-13-nocloud-arm64-daily.qcow2 \
> -device virtio-net-pci,netdev=unet \
> -device virtio-scsi-pci \
> -serial mon:stdio \
> -m 8192 \
> -object memory-backend-memfd,id=mem,size=8G,share=on \
> -numa node,memdev=mem \
> -display none \
> -vga none \
> -kernel /path/to/linux/build/arch/arm64/boot/Image \
> -device vhost-user-test-device,chardev=rtc,id=rtc,virtio-id=17,num_vqs=2,vq_size=1024 \
> -chardev socket,path=/tmp/rtc.sock,id=rtc \
> ...
>
> Example output:
> [ 1.105238] rust_virtio_rtc: Probe Rust virtio driver sample.
> [ 1.105645] rust_virtio_rtc: Found 1 vqs.
> [ 1.136050] rust_virtio_rtc: process_requestq got buf 16 bytes
> [ 1.136125] rust_virtio_rtc: Got response! Ok(RespCfg { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, num_clocks: Le16(3), reserved: [0, 0, 0, 0, 0, 0] })
> [ 1.136701] rust_virtio_rtc: Got response! Ok(RespClockCap { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, clock_type: 3, leap_second_smearing: 0, flags: 0, reserved: [0, 0, 0, 0, 0] })
> [ 1.136724] rust_virtio_rtc virtio0: cannot expose clock 0 (type 3, variant 0, flags 0) to userspace
> [ 1.137259] rust_virtio_rtc: Got response! Ok(RespRead { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, clock_reading: Le64(1777890485031060388) })
> [ 1.137277] rust_virtio_rtc: #0 clock reading = 1777890485031060388
> [ 1.137749] rust_virtio_rtc: Got response! Ok(RespClockCap { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, clock_type: 1, leap_second_smearing: 0, flags: 0, reserved: [0, 0, 0, 0, 0] })
> [ 1.137769] rust_virtio_rtc virtio0: cannot expose clock 1 (type 1, variant 0, flags 0) to userspace
> [ 1.138247] rust_virtio_rtc: Got response! Ok(RespRead { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, clock_reading: Le64(1777890485032086075) })
> [ 1.138264] rust_virtio_rtc: #1 clock reading = 1777890485032086075
> [ 1.138730] rust_virtio_rtc: Got response! Ok(RespClockCap { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, clock_type: 2, leap_second_smearing: 0, flags: 0, reserved: [0, 0, 0, 0, 0] })
> [ 1.138751] rust_virtio_rtc virtio0: cannot expose clock 2 (type 2, variant 0, flags 0) to userspace
> [ 1.139253] rust_virtio_rtc: Got response! Ok(RespRead { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, clock_reading: Le64(338567896865557) })
> [ 1.139270] rust_virtio_rtc: #2 clock reading = 338567896865557
>
>Concerns - Notes - TODOs
>========================
>
>- Virtqueue lifetimes don't neatly apply to Rust as expected, so a lot
> of times we have to go through unsafe pointer dereferences (though
> which are guaranteed by Virtio subsystem to be valid, for example when
> a callback is called with the vq argument). There's a potential for
> misuse and definitely could use better thinking.
>- `struct virtio_device` is not reference-counted like other implemented
> device types in rust/kernel. Maybe we need to change C API first to
> make them reference counted, assuming this doesn't break anything?
>- The sample driver obviously conflicts with the C implementation, so
> this would either need to move out of samples/ or figure out some way
> to handle this in kbuild.
>- kernel::virtio module and its types need a few rustdoc examples that I
> will add in followup series
>- Note that the registration of RTC clocks etc in the sample driver is
> not done, I'm putting it off until I receive some feedback first. The
> sample driver otherwise does send and receive data from the virtqueue
> as a PoC.
>
>PS: No LLMs used so any mistakes and goofs are solely written by me.
>
>Signed-off-by: Manos Pitsidianakis <manos@pitsidianak.is>
>---
>Changes in v3:
>- Removed unused methods from virtio API
>- Clean up how scattergather lists are added to virtqueues by using
> owned SGTables only, and make the API safe(r)
>- Add RAII cleanup for find_vqs return value that calls del_vqs
>- Reset device after remove callback
>- Significantly clean up sample driver as a result of the other cleanups
>- Link to v2: https://lore.kernel.org/r/20260509-rust-virtio-v2-0-c1e30ec2bd21@pitsidianak.is
>
>Changes in v2:
>- Move helper ifdefs to helper file (thanks Alice)
>- Changed CONFIG checks to IS_ENABLED to allow for CONFIG_VIRTIO=m
>- Split all use imports to one item per line according to style guide
>- Fixed wait_for_completion_interruptible*() rustdocs
>- Use Jiffy type alias in wait_for_completion_interruptible_timeout()
>- Pepper and salt #[inline]s wherever appropriate as per style guide
>- Split probe() into probe() and init() to allow cleaning up if init
> fails
>- Remove unnecessary Send and Sync unsafe impls for
> kernel::virtio::Device
>- Remove unnecessary LeSize and BeSize
>- Accept Option<_> for virtqueue callback when creating a VirtqueueInfo
>- Made all vq buffer adding operations unsafe
>- Use AtomicU16 instead of Cell<u16> for sample virtio driver
>- Fix RespHead field types in sample virtio driver
>- Fix response error checking in sample virtio driver
>- Change some device contexts in method signatures
>- Link to v1: https://lore.kernel.org/r/20260505-rust-virtio-v1-0-9563383909e4@pitsidianak.is
>
>---
>Manos Pitsidianakis (6):
> rust/bindings: generate virtio bindings
> rust/helpers: add virtio.c
> rust/kernel/device: return parent at same context
> rust: add virtio module
> rust: impl interruptible waits for Completion
> samples/rust: Add sample virtio-rtc driver [WIP]
>
> MAINTAINERS | 9 +
> rust/bindings/bindings_helper.h | 5 +
> rust/helpers/helpers.c | 1 +
> rust/helpers/virtio.c | 37 ++++
> rust/kernel/device.rs | 2 +-
> rust/kernel/lib.rs | 2 +
> rust/kernel/sync/completion.rs | 42 +++-
> rust/kernel/virtio.rs | 423 ++++++++++++++++++++++++++++++++++++++++
> rust/kernel/virtio/utils.rs | 57 ++++++
> rust/kernel/virtio/virtqueue.rs | 314 +++++++++++++++++++++++++++++
> samples/rust/Kconfig | 15 ++
> samples/rust/Makefile | 1 +
> samples/rust/rust_virtio_rtc.rs | 403 ++++++++++++++++++++++++++++++++++++++
> 13 files changed, 1309 insertions(+), 2 deletions(-)
>---
>base-commit: 028ef9c96e96197026887c0f092424679298aae8
>change-id: 20260504-rust-virtio-8523b01dfdc2
>
>Best regards,
>--
>Manos Pitsidianakis <manos@pitsidianak.is>
>
--
foo
^ permalink raw reply
* Re: [PATCH v2] virtio_net: fix infinite loop in virtnet_poll_cleantx when device is broken
From: Xuan Zhuo @ 2026-07-21 6:52 UTC (permalink / raw)
To: Jinqian Yang
Cc: netdev, virtualization, linux-kernel, liuyonglong, wangzhou1,
linuxarm, Jinqian Yang, mst, jasowang, xuanzhuo, eperezma,
andrew+netdev, davem, edumazet, kuba, pabeni
In-Reply-To: <20260716035201.3736582-1-yangjinqian1@huawei.com>
On Thu, 16 Jul 2026 11:52:01 +0800, Jinqian Yang <yangjinqian1@huawei.com> wrote:
> virtnet_poll_cleantx() contains a do-while loop that cleans up
> transmitted TX buffers and calls virtqueue_enable_cb_delayed() to check
> whether more buffers need processing. When the virtio backend stops
> responding during guest reboot, used->idx is never updated, so
> virtqueue_enable_cb_delayed() always returns false and the loop never
> terminates. Then it will block reboot process, and the guest will hang.
>
> The problem occurs during guest reboot under network traffic:
>
> 1. kernel_restart() -> device_shutdown() traverses the device list
> 2. virtio_dev_shutdown() calls virtio_break_device() which sets
> vq->broken = true
> 3. virtio_dev_shutdown() then calls virtio_synchronize_cbs() to wait
> for in-flight callbacks to complete
> 4. A virtio interrupt fires, softirq is deferred to ksoftirqd which
> calls net_rx_action() -> virtnet_poll() -> virtnet_poll_cleantx()
> 5. virtnet_poll_cleantx() enters the do-while loop and never exits
> because the QEMU backend has stopped updating used->idx, despite
> vq->broken having been set to true in step 2.
>
> Since the loop runs inside ksoftirqd (a SCHED_OTHER kthread), it is
> visible to the scheduler and does not trigger a hard lockup. However,
> the kthread never leaves the loop, so RCU detects it as a CPU stall
> and reports it periodically. Meanwhile, the reboot process remains
> blocked in device_shutdown() because virtio_dev_shutdown() cannot
> complete its synchronization step, and the guest hangs permanently.
>
> This can be reproduced on a guest with a virtio-net device: run iperf3
> traffic in the guest, then trigger reboot. The reboot occasionally hangs
> permanently with RCU stall on ksoftirqd.
>
> Observed on ARM64 KVM guest:
>
> CPU#1 RCU stall (ksoftirqd/1), repeated periodically:
> virtqueue_enable_cb_delayed_split <- virtnet_poll <- __napi_poll <-
> net_rx_action <- handle_softirqs <- run_ksoftirqd <-
> smpboot_thread_fn <- kthread
>
> Fix by adding a virtqueue_is_broken() check to the loop condition, so
> that the loop exits immediately when the device is broken, allowing
> the device shutdown to proceed.
>
> Signed-off-by: Jinqian Yang <yangjinqian1@huawei.com>
Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
> Changes in v2:
> - Moved vq->broken check to virtqueue_enable_cb_delayed().
>
> v1: https://lore.kernel.org/lkml/20260713132025.703147-1-yangjinqian1@huawei.com/
> ---
> drivers/virtio/virtio_ring.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index b438dc2ce1b8..5c169fbb418a 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -3233,6 +3233,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *_vq)
> {
> struct vring_virtqueue *vq = to_vvq(_vq);
>
> + /*
> + * When the device is broken there is no point in polling used->idx,
> + * the backend will never update it. Return true to let callers
> + * exit their cleanup loops instead of spinning forever.
> + */
> + if (unlikely(vq->broken))
> + return true;
> +
> if (vq->event_triggered)
> data_race(vq->event_triggered = false);
>
> --
> 2.33.0
>
^ permalink raw reply
* Re: [PATCH v3] virtio_ring: fix infinite loop in virtnet_poll_cleantx when device is broken
From: Xuan Zhuo @ 2026-07-21 6:53 UTC (permalink / raw)
To: Jinqian Yang
Cc: netdev, virtualization, linux-kernel, liuyonglong, wangzhou1,
linuxarm, Jinqian Yang, mst, jasowang, xuanzhuo, eperezma,
andrew+netdev, davem, edumazet, kuba, pabeni
In-Reply-To: <20260716115940.394832-1-yangjinqian1@huawei.com>
On Thu, 16 Jul 2026 19:59:40 +0800, Jinqian Yang <yangjinqian1@huawei.com> wrote:
> virtnet_poll_cleantx() contains a do-while loop that cleans up
> transmitted TX buffers and calls virtqueue_enable_cb_delayed() to check
> whether more buffers need processing. When the virtio backend stops
> responding during guest reboot, used->idx is never updated, so
> virtqueue_enable_cb_delayed() always returns false and the loop never
> terminates. Then it will block reboot process, and the guest will hang.
>
> The problem occurs during guest reboot under network traffic:
>
> 1. kernel_restart() -> device_shutdown() traverses the device list
> 2. virtio_dev_shutdown() calls virtio_break_device() which sets
> vq->broken = true
> 3. virtio_dev_shutdown() then calls virtio_synchronize_cbs() to wait
> for in-flight callbacks to complete
> 4. A virtio interrupt fires, softirq is deferred to ksoftirqd which
> calls net_rx_action() -> virtnet_poll() -> virtnet_poll_cleantx()
> 5. virtnet_poll_cleantx() enters the do-while loop and never exits
> because the QEMU backend has stopped updating used->idx, despite
> vq->broken having been set to true in step 2.
>
> Since the loop runs inside ksoftirqd (a SCHED_OTHER kthread), it is
> visible to the scheduler and does not trigger a hard lockup. However,
> the kthread never leaves the loop, so RCU detects it as a CPU stall
> and reports it periodically. Meanwhile, the reboot process remains
> blocked in device_shutdown() because virtio_dev_shutdown() cannot
> complete its synchronization step, and the guest hangs permanently.
>
> This can be reproduced on a guest with a virtio-net device: run iperf3
> traffic in the guest, then trigger reboot. The reboot occasionally hangs
> permanently with RCU stall on ksoftirqd.
>
> Observed on ARM64 KVM guest:
>
> CPU#1 RCU stall (ksoftirqd/1), repeated periodically:
> virtqueue_enable_cb_delayed_split <- virtnet_poll <- __napi_poll <-
> net_rx_action <- handle_softirqs <- run_ksoftirqd <-
> smpboot_thread_fn <- kthread
>
> Fix by adding a vq->broken check in virtqueue_enable_cb_delayed(), so
> that the loop exits immediately when the device is broken, allowing
> the device shutdown to proceed.
>
> Signed-off-by: Jinqian Yang <yangjinqian1@huawei.com>
Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
> Changes in v2:
> - Moved vq->broken check to virtqueue_enable_cb_delayed().
> Changes in v3:
> - Updated the patch subject prefix.
>
> v1: https://lore.kernel.org/lkml/20260713132025.703147-1-yangjinqian1@huawei.com/
> v2: https://lore.kernel.org/lkml/20260716035201.3736582-1-yangjinqian1@huawei.com/
> ---
> drivers/virtio/virtio_ring.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index b438dc2ce1b8..5c169fbb418a 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -3233,6 +3233,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *_vq)
> {
> struct vring_virtqueue *vq = to_vvq(_vq);
>
> + /*
> + * When the device is broken there is no point in polling used->idx,
> + * the backend will never update it. Return true to let callers
> + * exit their cleanup loops instead of spinning forever.
> + */
> + if (unlikely(vq->broken))
> + return true;
> +
> if (vq->event_triggered)
> data_race(vq->event_triggered = false);
>
> --
> 2.33.0
>
^ permalink raw reply
* [PATCH] vhost-scsi: flush backend after device ioctls
From: Jia Jia @ 2026-07-21 7:36 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang, Mike Christie
Cc: Paolo Bonzini, Stefan Hajnoci, Eugenio Pérez, virtualization,
kvm, netdev, Jia Jia
vhost-scsi translates guest response descriptors into userspace iovecs
at command submission time and later completes those commands
asynchronously through target-core. Device-wide control operations such
as VHOST_SET_MEM_TABLE replace the memory table under the device and
virtqueue mutexes, but historically returned without waiting for
outstanding SCSI commands that still hold the pre-update response
iovecs.
After such a replacement, completion may write virtio_scsi_cmd_resp
through the old host virtual addresses. If the owner has already
remapped those addresses, the write lands on the wrong userspace object.
The kernel tree has carried a TODO for this since the 2012 split of
vhost_dev_ioctl() and vhost_vring_ioctl():
/* TODO: flush backend after dev ioctl. */
A userspace test kept a READ(10) pending, replaced the memory table so
the response GPA mapped to a new HVA, remapped the old response address
as a victim page, and then let the command complete. The completion
wrote the victim page (victim_changed=yes) and left the replacement
page unchanged; a later TUR updated the new mapping instead. So the
pending command retained the pre-update response address across
VHOST_SET_MEM_TABLE.
That same 2012 change deliberately avoided a second backend flush on the
vring-ioctl path: vring updates already flush where appropriate, and an
extra heavy flush would hurt when kick or call fds are reconfigured on
the data path. This fix does not reintroduce that. The default branch
still routes unknown commands through vhost_dev_ioctl() first; only a
non-ENOIOCTLCMD result flushes. Vring ops such as SET_VRING_KICK/CALL,
num, addr, and base return -ENOIOCTLCMD there and fall through to
vhost_vring_ioctl() without this backend flush.
What vhost_dev_ioctl() actually handles on this path is small:
VHOST_SET_OWNER, VHOST_SET_MEM_TABLE, VHOST_SET_LOG_BASE,
VHOST_SET_LOG_FD, and the optional fork-owner ioctls when enabled.
Flushing after those is fine: they are rare device-wide control ops,
and SET_OWNER normally runs before any inflight SCSI work. Call
vhost_scsi_flush() so pre-update worker work and target-core inflight
commands finish before the ioctl returns. As with the existing net
pattern, any non-ENOIOCTLCMD result flushes, including failures that
may have applied a partial update such as VHOST_SET_LOG_BASE.
I later noticed vhost-net and vhost-vsock already use the same device
versus vring split.
This is a control-plane barrier only. Ordinary submission, completion,
kick, and call paths are unchanged. The owner is expected to keep
pre-update mappings valid until the device ioctl returns. Completion
copies the response and signals from the vhost worker without needing
further userspace progress, so waiting in this ioctl does not leave the
owner process stuck on itself.
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
drivers/vhost/scsi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 9a1253b9d8c5..c3e8f1a0b2d4 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -2424,10 +2424,11 @@ vhost_scsi_ioctl(struct file *f, unsigned int ioctl, unsigned long arg)
default:
mutex_lock(&vs->dev.mutex);
r = vhost_dev_ioctl(&vs->dev, ioctl, argp);
- /* TODO: flush backend after dev ioctl. */
if (r == -ENOIOCTLCMD)
r = vhost_vring_ioctl(&vs->dev, ioctl, argp);
+ else
+ vhost_scsi_flush(vs);
mutex_unlock(&vs->dev.mutex);
return r;
}
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH RFC v3 0/6] Add Rust virtio bindings and sample device
From: Stefano Garzarella @ 2026-07-21 7:46 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: Michael S. Tsirkin, Manos Pitsidianakis, Peter Hilber,
Stefan Hajnoczi, Viresh Kumar, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Daniel Almeida, rust-for-linux,
Jason Wang, Xuan Zhuo, Eugenio Pérez, virtualization,
linux-kernel
In-Reply-To: <tiiht3.22qk83rct1pd@pitsidianak.is>
On Tue, Jul 21, 2026 at 09:21:15AM +0300, Manos Pitsidianakis wrote:
>Ping.
>
>I have a different version of the series WIP with a virtio-rng instead
>of virtio-rtc, based on https://lore.kernel.org/lkml/20260529-rust-hw_random-virtio-rng-v1-0-b3153dd90311@pitsidianak.is/
>(which needs a v2 also)
>
>But since this series hasn't gotten any feedback on the virtio part,
>I'd like to ask if there is any before respinning.
Sorry I haven't replied until now, but I haven't been able to find the
time to take a look. I'm very interested in seeing Rust used in virtio
drivers, and I'd be happy to help maintain them.
I'll send you my feedback within the next few days.
Thanks for your work!
Stefano
^ permalink raw reply
* Re: [RFC PATCH 0/4] virtio: SQ/CQ doorbell polling for vhost-scsi
From: Eugenio Perez Martin @ 2026-07-21 9:09 UTC (permalink / raw)
To: rom.wang
Cc: mst, jasowangio, stefanha, virtualization, linux-kernel,
linux-scsi, kvm, qemu-devel, Yufeng Wang
In-Reply-To: <20260720141040.181946-1-r4o5m6e8o@163.com>
On Mon, Jul 20, 2026 at 4:19 PM rom.wang <r4o5m6e8o@163.com> wrote:
>
> From: Yufeng Wang <wangyufeng@kylinos.cn>
>
> This RFC proposes a new virtio transport feature, VIRTIO_F_SQCQ_POLL,
> that eliminates VM exits on both submission and completion paths for
> vhost-scsi by using shared-memory doorbells and kernel polling
> threads, following the io_uring SQPOLL model.
>
> This is an early RFC to gather design feedback. The implementation is
> functional and has been tested on arm64 and x86_64. We are not
> requesting merge at this time.
>
>
> Problem
> -------
>
> vhost-scsi uses MMIO writes (Guest -> Host) and MSI-X interrupts
> (Host -> Guest) for notification. Each notification involves a VM exit,
> which becomes a bottleneck at high IOPS:
>
> - 4K random read, QD32, 8 jobs: 341K IOPS baseline
> - With ~340K VM exits/second, the exit overhead dominates
>
> Existing mitigations (vhost-net's tx polling, blk-mq iopoll) only
> address one direction or require the submitting task to poll. Neither
> eliminates VM exits on both paths simultaneously.
>
>
> Solution
> --------
>
> Introduce two cache-line-aligned doorbell structures, SQ (Submission
> Queue) and CQ (Completion Queue), placed alongside the standard split
> virtqueue:
>
> - Guest writes sq->idx instead of MMIO kick; Host poll thread
> detects the change and processes submissions.
> - Host writes cq->idx instead of MSI-X interrupt; Guest poll
> thread detects the change and invokes completion callbacks.
>
> A NEED_WAKEUP protocol (mirroring io_uring's SQ_NEED_WAKEUP) allows
> either side to sleep when idle, with the other side responsible for
> waking it via eventfd.
>
> Feature negotiation via VIRTIO_F_SQCQ_POLL (bit 42) ensures zero
> overhead when not negotiated — the driver falls back to traditional
> MMIO kick + MSI-X interrupt.
>
>
> Performance
> -----------
>
> Benchmark: fio, 4K random I/O
>
> Test configuration:
>
> arm64:
> CPU: Kunpeng 920 (2.6GHz), 8 vCPUs
> Disk: NVMe INTEL SSDPED1K375GA (375GB)
>
> x86_64:
> CPU: Intel Xeon E5-2680 v4 @ 2.40GHz, 8 vCPUs
> Disk: NVMe SAMSUNG MZ1LB960HAJQ-000MV (960GB)
>
> Backend: vhost-scsi with TCM loopback to NVMe device
> QEMU: vhost-scsi-pci with VIRTIO_F_SQCQ_POLL negotiated
>
> arm64 results:
>
> Test Baseline SQ/CQ Poll Change
> ----------- ---------- ---------- -------
> randread QD1 22,427 28,289 +26%
> randread QD32 NJ1 89,910 75,665 -16%
> randread QD32 NJ4 186,763 379,549 +103%
> randread QD32 NJ8 199,967 550,633 +175%
> randwrite QD1 21,912 27,261 +24%
> randwrite QD32 NJ1 85,349 81,389 -5%
> randwrite QD32 NJ4 190,443 355,811 +87%
> randwrite QD32 NJ8 196,552 566,640 +188%
>
> x86_64 results:
>
> Test Baseline SQ/CQ Poll Change
> ----------- ---------- ---------- -------
> randread QD1 8,263 9,552 +16%
> randread QD32 NJ1 127,412 162,805 +28%
> randread QD32 NJ4 303,208 375,056 +24%
> randread QD32 NJ8 341,625 371,193 +9%
> randwrite QD1 20,773 30,332 +46%
> randwrite QD32 NJ1 133,316 159,207 +19%
> randwrite QD32 NJ4 233,373 229,224 -2%
> randwrite QD32 NJ8 231,442 231,676 +0%
>
> Multi-queue workloads (NJ4/NJ8) see significant improvement on arm64
> (87-188%) and moderate improvement on x86_64 (9-24%). Single-VQ
> high-queue-depth workloads show a minor regression on arm64 due to
> polling overhead vs. VM-exit savings trade-off, while x86_64 shows
> improvement across most configurations (16-46% for QD1, 19-28%
> for QD32-NJ1) due to lower per-VM-exit cost on x86.
>
>
> Why Not vDPA?
> -------------
>
> vhost-vDPA already provides doorbell mmap and polling. A reasonable
> reviewer would ask: why not extend vhost-vDPA instead?
>
> Three reasons:
>
> 1. No vdpa-scsi device exists. The vDPA framework
> (drivers/vdpa/) currently has hardware devices for net (mlx5,
> ifcvf, etc.) and software devices for net and blk (vdpa_sim).
> There is no virtio-scsi vDPA device, hardware or software.
> Building one means re-implementing vhost-scsi's TCM integration
> (SCSI CDB processing, ALUA, persistent reservations) under the
> vDPA device abstraction — 3-5x the work of extending vhost-scsi.
>
I don't have a lot of experience in vhost-scsi, but maybe using
generic vdpa device and ~passthrough all those calls to something
similar to vhost-scsi helps? Live migration is still a challenge that
way but you already mention that in Known Limitations, so maybe going
to generic saves a significant amount of work.
https://patchew.org/QEMU/20221215134944.2809-1-longpeng2@huawei.com/
Also, I'm failing to see the advantage of the Send / Completion queues
over a more agressive usage of event_idx, VIRTQ_USED_F_NO_NOTIFY /
VIRTQ_AVAIL_F_NO_INTERRUPT or VIRTIO_F_NOTIFICATION_DATA combined. Can
we explore what does these lacks compared with the send and completion
queues?
> 2. vhost-scsi is a deployed interface. libvirt, QEMU, and
> OpenStack have vhost-scsi configuration APIs and operational
> tooling. Switching to vhost-vdpa requires a new backend, user
> migration, and toolchain updates. SQ/CQ poll as a vhost-scsi
> feature is fully backward-compatible — no existing deployments
> break.
>
> 3. The protocol is transport-agnostic. The SQ/CQ doorbell design
> (struct vring_sq, struct vring_cq, NEED_WAKEUP handshake) is
> orthogonal to vhost vs. vDPA. The same UAPI can be consumed by
> vhost-scsi today and a future vdpa-scsi device. Implementing in
> vhost-scsi first does not block future vDPA integration.
>
> We acknowledge that vDPA is the long-term direction for virtio
> backends. If this SQ/CQ poll protocol is accepted, it can be ported
> to the vDPA framework; a vdpa-scsi device is independent work.
>
>
> Patch Structure
> ---------------
>
> Patch 1: UAPI definitions (virtio_config.h, virtio_ring.h,
> virtio_pci.h) — shared interface for all components
> Patch 2: vhost kernel support (vhost.c, vhost.h, scsi.c,
> vhost.h UAPI, vhost_types.h UAPI) — Host poll thread
> Patch 3: virtio guest driver (virtio_ring.c, virtio_sqcq_poll.c,
> virtio_pci_modern.c, virtio.c, virtio_scsi.c) — Guest
> poll thread and submission path
> Patch 4: QEMU support (virtio-pci.c, vhost.c) — PCI config
> forwarding and vhost ioctl bridge
>
> Patches 1-3 apply to the Linux kernel tree. Patch 4 applies to
> the QEMU tree separately.
>
>
> Spec Status
> -----------
>
> A virtio-spec format document has been prepared and will be submitted
> to the OASIS virtio TC as a proposal. This RFC stage seeks design
> feedback before initiating the formal spec process.
>
>
> Known Limitations (Future Work)
> -------------------------------
>
> - CPU hotplug: no notifier registered; poll thread may be
> migrated when its CPU goes offline. Planned: kthread_park +
> dynamic rebind.
> - Live migration: no explicit stop/flush coordination during
> migration. Planned: VHOST_BACKEND_F_SUSPEND/RESUME integration.
> - SMAP overhead: Host uses get_user/put_user for doorbell access.
> Future optimization: GUP + kmap to map pages into kernel space.
> - Backpressure: no "slow down" signal from device to driver.
> Future: CQ throttle flag or avail_event reuse.
> - Packed ring: not supported; explicitly rejected at feature
> negotiation. Future: add packed ring doorbell support.
>
>
> RFC Goals
> ---------
>
> 1. Validate the overall design direction (doorbell + polling
> model, NEED_WAKEUP protocol)
> 2. Get feedback on the feature bit allocation (42) and UAPI
> structure design
> 3. Understand whether vDPA concern is a blocker or can be
> addressed with the transport-agnostic argument
> 4. Collect guidance on prioritizing future work items
> (hotplug, migration, spec process)
>
> We welcome all feedback, especially on:
> - Whether the NEED_WAKEUP protocol design is sound
> - Whether the per-device Guest poll thread model (vs per-VQ)
> is acceptable
> - Whether feature bit 42 is appropriate or if a different
> allocation is needed
> - Any concerns about the SMAP overhead in the Host poll path
>
>
> How to Test
> -----------
>
> Patch usage:
> Guest Kernel: apply patch 1 (UAPI) + patch 3 (guest driver)
> Host Kernel: apply patch 1 (UAPI) + patch 2 (vhost support)
> QEMU: apply patch 4 (vhost-scsi bridge)
>
> 1. Set up vhost-scsi target on the host (see:
> https://wiki.libvirt.org/Vhost-scsi_target.html#Host_Setup)
> using targetcli to create a TCM loopback device, e.g.:
> targetcli /backstores/loopback create dev=/dev/sda
> targetcli /vhost create naa.5001405376e34400
> targetcli /vhost/naa.5001405376e34400/lun create \
> /backstores/loopback/dev,/dev/sda
>
> 2. Boot VM with vhost-scsi using patched QEMU:
> qemu-system-aarch64 ... \
> -device vhost-scsi-pci,wwpn=naa.5001405376e34400
>
> 3. Verify SQ/CQ poll mode is active on the host:
> dmesg | grep "vhost-sqcq"
> # Expected: "vhost-sqcq: vq[N] poll thread bound to cpuN"
> # and 10s stats: "vhost-sqcq: vq[N] cq=... ema_lat=... interval=..."
>
> 4. Verify feature negotiated in guest:
> dmesg | grep "VIRTIO_F_SQCQ_POLL"
> # Expected: "VIRTIO_F_SQCQ_POLL negotiated, starting poll thread"
> # and 10s stats: "io_stats: cq=... avg_lat=... interval=..."
>
> 5. Run fio benchmark (compare with unpatched QEMU/kernel baseline):
> fio --name=randread --rw=randread --bs=4k --iodepth=32 \
> --numjobs=4 --runtime=60 --time_based --direct=1 \
> --filename=/dev/sda
>
> Test scripts (run-sqcq-compare.sh, compare-sqcq-results.sh) are
> available and will be sent as a follow-up to this RFC.
>
>
> Thank you for your time.
>
> ---
> Yufeng Wang (4):
> common: add UAPI for SQ/CQ doorbell polling
> vhost: host kernel support for SQ/CQ polling
> virtio: guest driver support for SQ/CQ polling
> qemu: add SQ/CQ polling mode support for vhost-scsi
>
> Patch 1 (UAPI, 3 files): +41 -1
> Patch 2 (vhost, 5 files): +794 -20
> Patch 3 (virtio guest, 10 files): +840 -3
> Patch 4 (QEMU, 14 files): +179 -3
>
> Total: 32 files changed, 1854 insertions(+), 27 deletions(-)
>
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH net v2 2/2] vsock/test: add test for small packets under pressure
From: Paolo Abeni @ 2026-07-21 10:10 UTC (permalink / raw)
To: Stefano Garzarella, Michael S. Tsirkin
Cc: netdev, Jason Wang, Xuan Zhuo, Eric Dumazet, Eugenio Pérez,
Simon Horman, Stefan Hajnoczi, David S. Miller, linux-kernel, kvm,
virtualization, Jakub Kicinski, Jason Wang
In-Reply-To: <ak9h1kWcLmpP74PI@sgarzare-redhat>
On 7/9/26 11:17 AM, Stefano Garzarella wrote:
> On Wed, Jul 08, 2026 at 06:59:41AM -0400, Michael S. Tsirkin wrote:
>> On Wed, Jul 08, 2026 at 12:29:04PM +0200, Stefano Garzarella wrote:
>>> From: Stefano Garzarella <sgarzare@redhat.com>
>>>
>>> Add a test that sends 2 MB of data using randomly sized small packets
>>> (129-512 bytes) over a SOCK_STREAM connection. Packets above
>>> GOOD_COPY_LEN (128) bypass the in-place coalescing in recv_enqueue(),
>>> forcing each one into its own skb.
>>>
>>> Without receive queue collapsing, the per-skb overhead eventually
>>> exceeds buf_alloc and the connection is reset. The test verifies
>>> that all data arrives and that content integrity is preserved.
>>>
>>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>>
>> maybe cut down SO_VM_SOCKETS_BUFFER_SIZE? will make it easier to
>> trigger?
>
> Currently, with the default value, the trigger is practically immediate
> for packets between 129 and 512 bytes, but yes, a smaller buffer size
> certainly makes this effect even more pronounced.
Given that PW is quite in a sorrow status, I think a (net-next)
follow-up is the better option here.
/P
^ permalink raw reply
* Re: [PATCH net v2 0/2] vsock/virtio: collapse receive queue under memory pressure
From: patchwork-bot+netdevbpf @ 2026-07-21 10:20 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, jasowangio, xuanzhuo, edumazet, eperezma, horms, stefanha,
davem, linux-kernel, mst, kvm, pabeni, virtualization, kuba,
jasowang
In-Reply-To: <20260708102904.50732-1-sgarzare@redhat.com>
Hello:
This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Wed, 8 Jul 2026 12:29:02 +0200 you wrote:
> This series contains a patch (the first one) that is part of work I'm
> doing to improve the tracking of memory used by AF_VSOCK sockets.
> The second patch is a test for our suite that highlights the issue.
>
> Since Brien reported an issue with his environment (based on Linux 6.12.y)
> related to the work I’m doing, I extracted this patch and tried to make it
> as easy as possible to backport. Brien tested it by backporting it to
> 6.12.y, which now contains the backport of the 059b7dbd20a6
> ("vsock/virtio: fix potential unbounded skb queue").
>
> [...]
Here is the summary with links:
- [net,v2,1/2] vsock/virtio: collapse receive queue under memory pressure
https://git.kernel.org/netdev/net/c/2a12c05aef21
- [net,v2,2/2] vsock/test: add test for small packets under pressure
https://git.kernel.org/netdev/net/c/30c82aa0a8b1
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH net] vhost-net: fix TX stall when vhost owns virtio-net header
From: Paolo Abeni @ 2026-07-21 11:19 UTC (permalink / raw)
To: Michael S. Tsirkin, enrico.zanda
Cc: jasowangio, virtualization, netdev, kuba, kvm, linux-kernel,
eperezma, nd
In-Reply-To: <20260708124901-mutt-send-email-mst@kernel.org>
On 7/8/26 6:50 PM, Michael S. Tsirkin wrote:
> On Wed, Jul 08, 2026 at 04:22:42PM +0100, enrico.zanda@arm.com wrote:
>> From: Enrico Zanda <enrico.zanda@arm.com>
>>
>> When vhost owns the virtio-net header, i.e. when
>> VHOST_NET_F_VIRTIO_NET_HDR is negotiated, sock_hlen is 0,
>> meaning that no header will be forwarded to the TAP device.
>>
>> In the current vhost_net_build_xdp() implementation,
>> when sock_hlen == 0, the gso pointer can point at the start of the
>> Ethernet frame instead of a virtio-net header.
>> This results in a wrong interpretation of the destination MAC address
>> bytes as struct virtio_net_hdr fields.
>>
>> This can, for some MAC addresses, trigger -EINVAL and return early
>> before the TX descriptor is completed, which can stall vhost-net TX.
>>
>> Before 97b2409f28e0, the gso pointer was set to the zeroed padding area,
>> using it as a synthetic virtio-net header. Restore that behavior.
>>
>> Fixes: 97b2409f28e0 ("vhost-net: reduce one userspace copy when building XDP buff")
>> Signed-off-by: Enrico Zanda <enrico.zanda@arm.com>
>
>
> The fix looks good:
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> Sashiko thinks there's something something security here, but I think
> it is misguided. It's just guest hurting itself. driver breaks the
> device it gets to keep both pieces.
Out of sheer curiosity: which sashiko instance? AFAICS both gemini and
nipa are clean:
https://sashiko.dev/#/patchset/20260708152242.2268848-1-enrico.zanda%40arm.com
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260708152242.2268848-1-enrico.zanda%40arm.com
/P
^ permalink raw reply
* Re: [PATCH net] vhost-net: fix TX stall when vhost owns virtio-net header
From: patchwork-bot+netdevbpf @ 2026-07-21 13:20 UTC (permalink / raw)
To: enrico.zanda
Cc: jasowangio, virtualization, mst, netdev, kuba, kvm, linux-kernel,
eperezma, nd
In-Reply-To: <20260708152242.2268848-1-enrico.zanda@arm.com>
Hello:
This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Wed, 8 Jul 2026 16:22:42 +0100 you wrote:
> From: Enrico Zanda <enrico.zanda@arm.com>
>
> When vhost owns the virtio-net header, i.e. when
> VHOST_NET_F_VIRTIO_NET_HDR is negotiated, sock_hlen is 0,
> meaning that no header will be forwarded to the TAP device.
>
> In the current vhost_net_build_xdp() implementation,
> when sock_hlen == 0, the gso pointer can point at the start of the
> Ethernet frame instead of a virtio-net header.
> This results in a wrong interpretation of the destination MAC address
> bytes as struct virtio_net_hdr fields.
>
> [...]
Here is the summary with links:
- [net] vhost-net: fix TX stall when vhost owns virtio-net header
https://git.kernel.org/netdev/net/c/3c0d10f233f1
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [PATCH] drm/virtio: Remove usage of drm_simple_encoder_init()
From: liutianyi2211 @ 2026-07-21 16:21 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann, Dmitry Osipenko, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter, open list:VIRTIO GPU DRIVER,
open list:VIRTIO GPU DRIVER, open list
Cc: Tianyi Liu
From: Tianyi Liu <liutianyi2211@gmail.com>
Remove the deprecated trivial helper drm_simple_encoder_init(). Inline
the call to drm_encoder_init and add instance of
drm_encoder_funcs.
Signed-off-by: Tianyi Liu <liutianyi2211@gmail.com>
---
drivers/gpu/drm/virtio/virtgpu_display.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
index 44ffffec550f..048c19543218 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -32,7 +32,7 @@
#include <drm/drm_gem_framebuffer_helper.h>
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
-#include <drm/drm_simple_kms_helper.h>
+#include <drm/drm_encoder.h>
#include <drm/drm_vblank.h>
#include <drm/drm_vblank_helper.h>
@@ -67,6 +67,10 @@ static const struct drm_framebuffer_funcs virtio_gpu_fb_funcs = {
.dirty = drm_atomic_helper_dirtyfb,
};
+static const struct drm_encoder_funcs virtio_gpu_encoder_funcs_cleanup = {
+ .destroy = drm_encoder_cleanup,
+};
+
static int
virtio_gpu_framebuffer_init(struct drm_device *dev,
struct virtio_gpu_framebuffer *vgfb,
@@ -306,7 +310,8 @@ static int vgdev_output_init(struct virtio_gpu_device *vgdev, int index)
if (vgdev->has_edid)
drm_connector_attach_edid_property(connector);
- drm_simple_encoder_init(dev, encoder, DRM_MODE_ENCODER_VIRTUAL);
+ drm_encoder_init(dev, encoder, &virtio_gpu_encoder_funcs_cleanup,
+ DRM_MODE_ENCODER_VIRTUAL, NULL);
drm_encoder_helper_add(encoder, &virtio_gpu_enc_helper_funcs);
encoder->possible_crtcs = 1 << index;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v8 08/11] virt: Introduce steal governor driver
From: Yury Norov @ 2026-07-21 17:09 UTC (permalink / raw)
To: Shrikanth Hegde
Cc: linux-kernel, mingo, peterz, juri.lelli, vincent.guittot,
yury.norov, kprateek.nayak, iii, corbet, tglx, gregkh, pbonzini,
seanjc, vschneid, huschle, rostedt, dietmar.eggemann, maddy,
srikar, hdanton, chleroy, vineeth, frederic, arighi, pauld,
christian.loehle, tj, tommaso.cucinotta, maz, rafael, rdunlap,
kernellwp, linux-doc, jgross, virtualization
In-Reply-To: <20260720172250.2257582-9-sshegde@linux.ibm.com>
On Mon, Jul 20, 2026 at 10:52:47PM +0530, Shrikanth Hegde wrote:
> Introduce a new driver in virt named steal_governor. This driver
> will compute the steal time and drive the policy decisions of preferred
> CPU state.
>
> More on it can be found in the Documentation/driver-api/steal-governor.rst
>
> There is a new kconfig called STEAL_GOVERNOR which is introduced in
> subsequent patches. That driver is going to select PREFERRED_CPU.
> This makes configs driven by user preference.
> When the driver is disabled, preferred CPUs is same as active CPUs.
>
> File layout of the driver is being kept simple.
> - core.c - contains main driver code. This includes the periodic
> work function and take action on steal time which is introduced
> in subsequent patches.
> - core.h - header file which includes data structure.
>
> Main structure of steal governor has,
> - work: deferred periodic work function
> - steal, time: To calculate the delta in periodic work.
> - interval_ms, high_threshold, low_threshold: debug knobs of
> steal_governor.
>
> While there, Add MAINTAINERS entry for this new driver.
>
> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
> ---
> Documentation/driver-api/index.rst | 1 +
> Documentation/driver-api/steal-governor.rst | 117 ++++++++++++++++++++
> MAINTAINERS | 9 ++
> drivers/virt/steal_governor/core.c | 48 ++++++++
> drivers/virt/steal_governor/core.h | 25 +++++
Unless I missed something, you don't use struct steal_governor out of
the driver. If so, you don't need this layout. Just put everything in
drivers/virt/steal_governor.c.
Thanks,
Yury
^ permalink raw reply
* Re: [PATCH v8 10/11] virt/steal_governor: Implement steal_governor policy loop
From: Yury Norov @ 2026-07-21 19:28 UTC (permalink / raw)
To: Shrikanth Hegde
Cc: linux-kernel, mingo, peterz, juri.lelli, vincent.guittot,
yury.norov, kprateek.nayak, iii, corbet, tglx, gregkh, pbonzini,
seanjc, vschneid, huschle, rostedt, dietmar.eggemann, maddy,
srikar, hdanton, chleroy, vineeth, frederic, arighi, pauld,
christian.loehle, tj, tommaso.cucinotta, maz, rafael, rdunlap,
kernellwp, linux-doc, jgross, virtualization
In-Reply-To: <20260720172250.2257582-11-sshegde@linux.ibm.com>
On Mon, Jul 20, 2026 at 10:52:49PM +0530, Shrikanth Hegde wrote:
> schedule work at regular intervals to implement the steal_governor
> policy of monitoring the steal time and take action on the state of
> preferred CPUs. The interval is determined by interval_ms parameter.
> schedule_delayed_work is used since interval_ms
> is in the order of milliseconds. Work need not happen instantly.
>
> Periodic policy loop essentially does:
> - Gets the total/delta steal values and cpus to use steal_ratio.
> (get_system_steal_time, get_num_cpus_steal_ratio)
> - Calculate the steal_ratio as below.
>
> steal_ratio = (delta_steal * 100*100)/(delta_ns * num_cpus())
>
> It is calculated to consider the fractional values of steal time.
> I.e 10 means 0.1% steal time. A few tricks such as divide by 10,000
> are used to avoid possible overflow.
> - If steal value is higher than high threshold, call the method to reduce
> the preferred CPUs. (decrease_preferred_cpus)
> - If steal value is lower or equal to low threshold, call the method to
> increase the preferred CPUs. (increase_preferred_cpus)
> - If the steal value is in between, no action is taken.
> - Save the values for next delta calculations.
> - Ensure design checks are met.
> 1. At least one core/CPU must be there in preferred mask.
> 2. preferred CPUs is subset of active CPUs.
> If not met, then restore preferred CPUs to active and stop
> requeue of the work. Driver is effectively non-functional after that.
>
> In Order to help the above loop, a few helper functions have been added.
>
> 1. get_system_steal_time()
> - steal governor takes global view of steal time instead of individual
> vCPU. Collect the steal values across the vCPUs of interest.
> - Sum up steal time values across possible CPUs. This helps to keep it
> a monotonically increasing number and avoids spikes due to CPU
> hotplug.
>
> 2. decrease_preferred_cpus()
> - Called when there is high steal time. It needs to decide which CPUs to
> mark as non-preferred and set that state.
> - Get first housekeeping CPU and its core mask. Mark it as
> protected core. This helps to keep at least one core as preferred.
> kernel ensures at least one housekeeping CPU stays active.
> - Find the last CPU outside of this protected core mask. (target CPU)
> - Based on that target CPU, get its sibling and mark them as
> non-preferred.
>
> 3. increase_preferred_cpus()
> - Called when there is low steal time. It needs to decide which CPUs to
> mark as preferred and set that state.
> - Get the first active non-preferred CPUs. This likely is the last
> set of CPUs being marked as non-preferred.
> - get the siblings of that CPU and mark them as preferred.
>
> 4. get_num_cpus_steal_ratio()
> - informs how many CPUs needs to be considered for steal_ratio
> calculations.
> - Return number of possible CPUs as get_system_steal_time computes
> steal values across possible CPUs.
>
> Notes:
> 1. Using core instead of individual CPUs performs better as SMT is
> quite common and some hypervisor such as powerVM does core scheduling.
>
> 2. This doesn't do any NUMA splicing to keep the code simpler and
> minimal overhead. Current code expects CPUs spread uniformly
> across NUMA nodes.
>
> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
> ---
> drivers/virt/steal_governor/core.c | 160 +++++++++++++++++++++++++++++
> drivers/virt/steal_governor/core.h | 5 +
> 2 files changed, 165 insertions(+)
>
> diff --git a/drivers/virt/steal_governor/core.c b/drivers/virt/steal_governor/core.c
> index 1cb766a8ce28..97f82d6df60f 100644
> --- a/drivers/virt/steal_governor/core.c
> +++ b/drivers/virt/steal_governor/core.c
> @@ -89,6 +89,158 @@ module_param_named(low_threshold, sg_core_ctx.low_threshold, uint, 0444);
> MODULE_PARM_DESC(low_threshold,
> "Low steal threshold. default: 200 i.e 2%. Must be < high_threshold");
>
> +/*
> + * Returns steal time of the full system.
> + * Compute collective steal time across all possible CPUs.
> + */
> +static u64 get_system_steal_time(void)
> +{
> + int cpu;
> + u64 total_steal = 0;
> +
> + for_each_possible_cpu(cpu)
> + total_steal += kcpustat_cpu(cpu).cpustat[CPUTIME_STEAL];
> +
> + return total_steal;
> +}
There's another implementation of the same logic in
hd_calculate_steal_percentage())
It means it should live in include/linux/kernel_stat.h as:
u64 kcpustat_steal_time(struct cpumask *cpus);
Or possibly even more generic:
u64 kcpustat_field_total(enum cpu_usage_stat usage, struct cpumask *cpus);
Similarly to the existing kcpustat_field().
The other possible candidates are: fs/proc/stat.c:: show_stat()
arch_cpu_idle_time(), but I think it's out of the scope of your
series.
What's the relation between the arch/s390/kernel/hiperdispatch.c and
your steal governor? Is that a similar concept?
> +/*
> + * Returns number of CPUs to consider for steal ratio.
> + * Return possible CPUs.
> + */
Can you rephrase the comment? It has 2 'return' sections with
different meaning. If the 2nd one is the implementation detail,
I'd put it inside the function scope, or drop entirely.
> +static unsigned int get_num_cpus_steal_ratio(void)
> +{
> + return num_possible_cpus();
> +}
> +
> +/*
> + * Take action to decrease preferred CPUs.
> + *
Drop this 'Take action' wording please.
> + * Decrease the preferred CPUs by 1 core.
> + * Take out the last core in the active & preferred.
> + *
> + * Must ensure
> + * - least one housekeeping core is always kept as preferred
s/least/at least/ ?
> + * - preferred is always subset of active.
> + */
> +static void decrease_preferred_cpus(void)
> +{
> + int tmp_cpu, first_hk_cpu, last_cpu;
> + const struct cpumask *first_hk_core;
> + int target_cpu = nr_cpu_ids;
> +
> + guard(cpus_read_lock)();
> + first_hk_cpu = cpumask_first_and(housekeeping_cpumask(HK_TYPE_KERNEL_NOISE),
> + cpu_preferred_mask);
> + if (first_hk_cpu >= nr_cpu_ids)
> + return;
> +
> + last_cpu = cpumask_last(cpu_preferred_mask);
> +
> + if (last_cpu >= nr_cpu_ids)
> + return;
> +
> + /* Always leave first housekeeping core as preferred. */
> + first_hk_core = topology_sibling_cpumask(first_hk_cpu);
> +
> + /* Find the last CPU which doesn't belong to that first hk_core. */
> + if (!cpumask_test_cpu(last_cpu, first_hk_core)) {
> + target_cpu = last_cpu;
> + } else {
> + for_each_cpu_andnot(tmp_cpu, cpu_preferred_mask, first_hk_core)
> + target_cpu = tmp_cpu;
> + }
Too much local variables. You can drop those tmp_cpu, last_cpu and
target_cpu, and just use a single variable 'cpu'. That would also
simplify your logic:
cpu = cpumask_last(cpu_preferred_mask);
core = topology_sibling_cpumask(first_hk_cpu);
if (cpumask_test_cpu(cpu, core)) {
for_each_cpu_andnot(cpu, cpu_preferred_mask, core)
/* nop */ ;
}
if (cpu >= nr_cpu_ids)
return;
And so on.
> +
> + /* Only the first housekeeping core remains */
> + if (target_cpu >= nr_cpu_ids)
> + return;
> +
> + for_each_cpu_and(tmp_cpu, topology_sibling_cpumask(target_cpu),
> + cpu_preferred_mask)
> + set_cpu_preferred(tmp_cpu, false);
> +}
> +
> +/*
> + * Take action to increase preferred CPUs.
> + *
Again, drop this 'take action' thing.
> + * Increase the preferred CPUs by 1 core.
> + * Add the first core in active & !preferred
> + *
> + * Must ensure preferred is subset of active.
> + */
> +static void increase_preferred_cpus(void)
> +{
> + int first_cpu, tmp_cpu;
> +
> + guard(cpus_read_lock)();
> + first_cpu = cpumask_first_andnot(cpu_active_mask, cpu_preferred_mask);
> +
> + /* All CPUs are preferred. Nothing to increase further */
> + if (first_cpu >= nr_cpu_ids)
> + return;
> +
> + for_each_cpu_and(tmp_cpu, topology_sibling_cpumask(first_cpu),
> + cpu_active_mask)
> + set_cpu_preferred(tmp_cpu, true);
> +}
> +
> +static void compute_preferred_cpus_work(struct work_struct *work)
> +{
> + u64 curr_steal, delta_steal, delta_ns, steal_ratio;
> + ktime_t now;
> +
> + now = ktime_get();
> + delta_ns = ktime_to_ns(ktime_sub(now, sg_core_ctx.time));
> +
> + if (unlikely(delta_ns < NSEC_PER_MSEC)) {
> + pr_err_ratelimited("steal_governor: work scheduled too soon delta_ns: %llu\n",
> + delta_ns);
> + goto requeue_work;
> + }
> +
> + curr_steal = get_system_steal_time();
> + delta_steal = curr_steal > sg_core_ctx.steal ?
> + curr_steal - sg_core_ctx.steal : 0;
> +
> + /* Update for next calculation */
> + sg_core_ctx.steal = curr_steal;
> + sg_core_ctx.time = now;
> +
> + /*
> + * steal_ratio = (delta_steal * 100*100)/(delta_ns * num_cpus())
> + * To avoid possible overflow, divide the denominator early.
> + * Note minimum interval is 100ms.
> + */
> + delta_ns = max_t(u64, div_u64(delta_ns * get_num_cpus_steal_ratio(),
> + 100 * 100), 1);
> + steal_ratio = div64_u64(delta_steal, delta_ns);
> +
> + if (steal_ratio > sg_core_ctx.high_threshold)
> + decrease_preferred_cpus();
> + if (steal_ratio <= sg_core_ctx.low_threshold)
> + increase_preferred_cpus();
If you neither increase, nor decrease, you don't need to check the mask
because you know you don't modify it. Also, I'd wrap the below integrity
checks into a helper function.
if (steal_ratio > sg_core_ctx.high_threshold)
decrease_preferred_cpus();
else if (steal_ratio <= sg_core_ctx.low_threshold)
increase_preferred_cpus();
else
goto requeue_work;
if (check_integrity())
return;
> + /* maintain design constructs always */
> + if (cpumask_empty(cpu_preferred_mask)) {
> + pr_err("empty preferred mask. stop steal governor\n");
> + restore_preferred_to_active();
> + return;
> + }
> +
> + if (!cpumask_subset(cpu_preferred_mask, cpu_active_mask)) {
> + pr_err("preferred: %*pbl is not subset of active: %*pbl, stop steal governor\n",
> + cpumask_pr_args(cpu_preferred_mask),
> + cpumask_pr_args(cpu_active_mask));
> + restore_preferred_to_active();
> + return;
> + }
> +
> +requeue_work:
> + /* Trigger for next sampling */
The lablel above is pretty explaining to me. The comment just
duplicates it. Maybe drop the comment?
> + schedule_delayed_work(&sg_core_ctx.work,
> + msecs_to_jiffies(sg_core_ctx.interval_ms));
If you need jiffies, why don't you have them in the structure, instead of
milliseconds?
schedule_delayed_work(&sg_core_ctx.work, sg_core_ctx.delay);
> +}
> +
> static int __init steal_governor_init(void)
> {
> if (sg_core_ctx.low_threshold >= sg_core_ctx.high_threshold) {
> @@ -100,11 +252,19 @@ static int __init steal_governor_init(void)
> pr_info("steal_governor is enabled. interval: %ums, high_threshold: %u, low_threshold: %u\n",
> sg_core_ctx.interval_ms, sg_core_ctx.high_threshold, sg_core_ctx.low_threshold);
>
> + INIT_DELAYED_WORK(&sg_core_ctx.work, compute_preferred_cpus_work);
> + sg_core_ctx.steal = get_system_steal_time();
> + sg_core_ctx.time = ktime_get();
> +
> + schedule_delayed_work(&sg_core_ctx.work,
> + msecs_to_jiffies(sg_core_ctx.interval_ms));
> +
> return 0;
> }
>
> static void __exit steal_governor_exit(void)
> {
> + disable_delayed_work_sync(&sg_core_ctx.work);
> restore_preferred_to_active();
> pr_info("steal_governor is disabled\n");
> }
> diff --git a/drivers/virt/steal_governor/core.h b/drivers/virt/steal_governor/core.h
> index e27305284ac0..59329c1d7109 100644
> --- a/drivers/virt/steal_governor/core.h
> +++ b/drivers/virt/steal_governor/core.h
> @@ -12,6 +12,11 @@
> #include <linux/workqueue.h>
> #include <linux/ktime.h>
> #include <linux/kconfig.h>
> +#include <linux/kernel_stat.h>
> +#include <linux/topology.h>
> +#include <linux/sched/isolation.h>
> +#include <linux/cleanup.h>
> +#include <linux/math64.h>
>
> struct steal_governor {
> struct delayed_work work;
> --
> 2.47.3
^ permalink raw reply
* Re: [PATCH] virtio_net: fix spelling of aggressively in comments
From: Jakub Kicinski @ 2026-07-21 22:22 UTC (permalink / raw)
To: Weimin Xiong
Cc: virtualization, netdev, mst, jasowangio, xuanzhuo, eperezma,
andrew+netdev, davem, edumazet, pabeni, linux-kernel
In-Reply-To: <20260716011201.86688-1-xiongwm2026@163.com>
On Thu, 16 Jul 2026 09:12:01 +0800 Weimin Xiong wrote:
> - /* Pre-fill rq agressively, to make sure we are ready to
> + /* Pre-fill rq aggressively, to make sure we are ready to
> * get packets immediately.
Move the "to" to the next line to avoid going over 80 chars
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH v8 08/11] virt: Introduce steal governor driver
From: Shrikanth Hegde @ 2026-07-22 5:38 UTC (permalink / raw)
To: Yury Norov
Cc: linux-kernel, mingo, peterz, juri.lelli, vincent.guittot,
yury.norov, kprateek.nayak, iii, corbet, tglx, gregkh, pbonzini,
seanjc, vschneid, huschle, rostedt, dietmar.eggemann, maddy,
srikar, hdanton, chleroy, vineeth, frederic, arighi, pauld,
christian.loehle, tj, tommaso.cucinotta, maz, rafael, rdunlap,
kernellwp, linux-doc, jgross, virtualization
In-Reply-To: <al-n0iPGnlTr39P9@yury>
Hi Yury,
Thanks for taking a look at the series. Much appreciated _/\_
On 7/21/26 10:39 PM, Yury Norov wrote:
> On Mon, Jul 20, 2026 at 10:52:47PM +0530, Shrikanth Hegde wrote:
>> Introduce a new driver in virt named steal_governor. This driver
>> will compute the steal time and drive the policy decisions of preferred
>> CPU state.
>>
>> More on it can be found in the Documentation/driver-api/steal-governor.rst
>>
>> There is a new kconfig called STEAL_GOVERNOR which is introduced in
>> subsequent patches. That driver is going to select PREFERRED_CPU.
>> This makes configs driven by user preference.
>> When the driver is disabled, preferred CPUs is same as active CPUs.
>>
>> File layout of the driver is being kept simple.
>> - core.c - contains main driver code. This includes the periodic
>> work function and take action on steal time which is introduced
>> in subsequent patches.
>> - core.h - header file which includes data structure.
>>
>> Main structure of steal governor has,
>> - work: deferred periodic work function
>> - steal, time: To calculate the delta in periodic work.
>> - interval_ms, high_threshold, low_threshold: debug knobs of
>> steal_governor.
>>
>> While there, Add MAINTAINERS entry for this new driver.
>>
>> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
>> ---
>> Documentation/driver-api/index.rst | 1 +
>> Documentation/driver-api/steal-governor.rst | 117 ++++++++++++++++++++
>> MAINTAINERS | 9 ++
>> drivers/virt/steal_governor/core.c | 48 ++++++++
>> drivers/virt/steal_governor/core.h | 25 +++++
>
> Unless I missed something, you don't use struct steal_governor out of
> the driver. If so, you don't need this layout. Just put everything in
> drivers/virt/steal_governor.c.
>
Ok.
By that I assume you mean to move all those headers also into
steal_governor.c, remove core.h and remove folder for steal_governor.
> Thanks,
> Yury
^ permalink raw reply
* [PATCH] vhost-scsi: fix T10-PI lifecycle hard BUG after endpoint
From: Jia Jia @ 2026-07-22 6:36 UTC (permalink / raw)
To: mst
Cc: jasowangio, michael.christie, pbonzini, stefanha, eperezma,
virtualization, linux-kernel, Jia Jia
vhost_scsi_setup_vq_cmds() runs only from VHOST_SCSI_SET_ENDPOINT and
allocates each command's protection scatterlist array (prot_sgl) only
when VIRTIO_SCSI_F_T10_PI is already set at that moment. Command pools
are not rebuilt later. vhost_scsi_set_features() may still flip that
bit after the endpoint is live: it updates acked_features and returns
success without reallocating prot_sgl.
That is a feature-versus-resource lifetime mismatch. The broken order
is:
VHOST_SET_FEATURES(0)
VHOST_SCSI_SET_ENDPOINT
setup_vq_cmds() sees no T10-PI -> prot_sgl stays NULL
VHOST_SET_FEATURES(VIRTIO_SCSI_F_T10_PI)
only acked_features changes; command resources stay as above
submit a T10-PI WRITE with a 129-page protection payload
The I/O path then follows the new feature bit while using the old
command objects. vhost_scsi_mapal() (static, inlined into
vhost_scsi_handle_vq() here) does:
sg_alloc_table_chained(table, 129, first_chunk=NULL,
nents_first_chunk=inline_sg_cnt)
With first_chunk NULL, __sg_alloc_table() sets curr_max_ents from
nents_first_chunk and calls sg_pool_alloc(129). sg_pool_index() then
hits:
BUG_ON(nents > SG_CHUNK_SIZE); /* 129 > 128 */
The kernel reported the following call trace and register state:
Call Trace:
<TASK>
? __sg_alloc_table+0x1d8/0x250
? __pfx_vhost_run_work_list+0x10/0x10 [vhost]
sg_alloc_table_chained+0x59/0xf0
? __pfx_sg_pool_alloc+0x10/0x10
? vhost_scsi_calc_sgls.constprop.0+0x43/0x60 [vhost_scsi]
vhost_scsi_handle_vq+0xf02/0x1700 [vhost_scsi]
? __pfx_vhost_scsi_handle_vq+0x10/0x10 [vhost_scsi]
vhost_scsi_handle_kick+0x37/0x50 [vhost_scsi]
vhost_run_work_list+0x8e/0xd0 [vhost]
vhost_task_fn+0xe1/0x210
ret_from_fork+0x348/0x540
</TASK>
RIP: 0010:0x4
CR2: 0000000000000004
RSP: 0018:ffffc90000dbf940 EFLAGS: 00010202
RAX: ffffffff82396810 RBX: ffff88811dc28b80 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000820 RDI: 0000000000000081
The host worker dies and the machine stops accepting network service
until reset. The missing object is prot_sgl after a post-endpoint
T10-PI enable introduced by commit bf2d650391be ("vhost-scsi: Allocate
T10 PI structs only when enabled").
I also checked the other feature bits handled around endpoint setup.
Only T10-PI allocates this endpoint-time per-command resource and does
not rebuild it on a later SET_FEATURES. LOG_ALL uses lazy tvc_log and
already tears log storage down when cleared; HOTPLUG does not allocate
prot_sgl. So the chosen fix is to refuse T10-PI enable/disable while
an endpoint is active, rather than adding a second rebuild path in
set_features().
Return -EBUSY if vs->vs_tpg is set and the T10-PI bit would change.
Userspace must CLEAR_ENDPOINT, SET_FEATURES, then SET_ENDPOINT again so
setup_vq_cmds() can allocate protection SGLs when needed. That matches
the existing "build command resources at endpoint" model.
Fixes: bf2d650391be ("vhost-scsi: Allocate T10 PI structs only when enabled")
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
drivers/vhost/scsi.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 9a1253b9d8c5..000000000000 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -2219,6 +2219,7 @@ static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features)
{
struct vhost_virtqueue *vq;
bool is_log, was_log;
+ bool is_t10_pi, was_t10_pi;
int i;
if (features & ~VHOST_SCSI_FEATURES)
@@ -2234,6 +2235,13 @@ static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features)
if (!vs->dev.nvqs)
goto out;
+ is_t10_pi = features & (1ULL << VIRTIO_SCSI_F_T10_PI);
+ was_t10_pi = vhost_has_feature(&vs->vqs[0].vq, VIRTIO_SCSI_F_T10_PI);
+ if (vs->vs_tpg && is_t10_pi != was_t10_pi) {
+ mutex_unlock(&vs->dev.mutex);
+ return -EBUSY;
+ }
+
is_log = features & (1 << VHOST_F_LOG_ALL);
/*
* All VQs should have same feature.
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v8 10/11] virt/steal_governor: Implement steal_governor policy loop
From: Shrikanth Hegde @ 2026-07-22 7:14 UTC (permalink / raw)
To: Yury Norov
Cc: linux-kernel, mingo, peterz, juri.lelli, vincent.guittot,
yury.norov, kprateek.nayak, iii, corbet, tglx, gregkh, pbonzini,
seanjc, vschneid, huschle, rostedt, dietmar.eggemann, maddy,
srikar, hdanton, chleroy, vineeth, frederic, arighi, pauld,
christian.loehle, tj, tommaso.cucinotta, maz, rafael, rdunlap,
kernellwp, linux-doc, jgross, virtualization
In-Reply-To: <al_IbeAa3sDrt9A7@yury>
Hi Yury.
On 7/22/26 12:58 AM, Yury Norov wrote:
> On Mon, Jul 20, 2026 at 10:52:49PM +0530, Shrikanth Hegde wrote:
[...]
>> +/*
>> + * Returns steal time of the full system.
>> + * Compute collective steal time across all possible CPUs.
>> + */
>> +static u64 get_system_steal_time(void)
>> +{
>> + int cpu;
>> + u64 total_steal = 0;
>> +
>> + for_each_possible_cpu(cpu)
>> + total_steal += kcpustat_cpu(cpu).cpustat[CPUTIME_STEAL];
>> +
>> + return total_steal;
>> +}
>
> There's another implementation of the same logic in
> hd_calculate_steal_percentage())
>
> It means it should live in include/linux/kernel_stat.h as:
>
> u64 kcpustat_steal_time(struct cpumask *cpus);
>
> Or possibly even more generic:
>
> u64 kcpustat_field_total(enum cpu_usage_stat usage, struct cpumask *cpus);
That's good idea. Below are the callsites i think can be
refactored in addition to this patch.
arch/s390/kernel/hiperdispatch.c: hd_calculate_steal_percentage
fs/proc/uptime.c: uptime_proc_show
I can pick up this refactoring post the series. I think it is better
not to club them together. is that ok?
>
> Similarly to the existing kcpustat_field().
>
> The other possible candidates are: fs/proc/stat.c:: show_stat()
> arch_cpu_idle_time(), but I think it's out of the scope of your
> series.
Yes.
drivers/leds/trigger/ledtrig-activity.c: led_activity_function
fs/proc/stat.c: show_stat
These two sum up multiple CPUTIME_ fields. Doing them in one loop is
likely better than getting each of them in a separate loop. No?
>
> What's the relation between the arch/s390/kernel/hiperdispatch.c and
> your steal governor? Is that a similar concept?
>
It is similar but independent idea. Ilya explained it in brief below.
https://youtu.be/adxUKFPlOp0?t=1846
Tobais, Ilya, Correct me if i am wrong.
- It samples steal time and decides on CPU_CAPACITY of a
CPU (between 1024 and 126) periodically.
- Triggers sched domain rebuild.
But Using CPU_CAPACITY to ensure no task running has drawbacks as we had
explored it very early.
- It fails under high concurrency. Typical real life workloads spawns
threads/tasks based on online CPUs. Scheduler chooses low capacity
CPUs instead of running them on busy CPUs. So under high concurrency or
high utilization it simply doesn't work.
- It takes a long time to get the running task out of it.
- It needs a sched domain rebuild. That is costly
(One can play tricks to avoid it)
- There are PR/SM overheads.
Since S390 has explicit knowledge of which CPUs to take out, it will likely
want a arch specific mechanism in steal_governor to take out Vertical Low CPUs
first. That is being deferred for now, once s390 has that, my take is
hiperdispatch can be removed altogether.
>> +/*
>> + * Returns number of CPUs to consider for steal ratio.
>> + * Return possible CPUs.
>> + */
>
> Can you rephrase the comment? It has 2 'return' sections with
> different meaning. If the 2nd one is the implementation detail,
> I'd put it inside the function scope, or drop entirely.
>
Ok. Let me drop second statement there.
>> +static unsigned int get_num_cpus_steal_ratio(void)
>> +{
>> + return num_possible_cpus();
>> +}
>> +
>> +/*
>> + * Take action to decrease preferred CPUs.
>> + *
>
> Drop this 'Take action' wording please.
Ok.
>
>> + * Decrease the preferred CPUs by 1 core.
>> + * Take out the last core in the active & preferred.
>> + *
>> + * Must ensure
>> + * - least one housekeeping core is always kept as preferred
>
> s/least/at least/ ?
Yep.
>
>> + * - preferred is always subset of active.
>> + */
>> +static void decrease_preferred_cpus(void)
>> +{
>> + int tmp_cpu, first_hk_cpu, last_cpu;
>> + const struct cpumask *first_hk_core;
>> + int target_cpu = nr_cpu_ids;
>> +
>> + guard(cpus_read_lock)();
>> + first_hk_cpu = cpumask_first_and(housekeeping_cpumask(HK_TYPE_KERNEL_NOISE),
>> + cpu_preferred_mask);
>> + if (first_hk_cpu >= nr_cpu_ids)
>> + return;
>> +
>> + last_cpu = cpumask_last(cpu_preferred_mask);
>> +
>> + if (last_cpu >= nr_cpu_ids)
>> + return;
>> +
>> + /* Always leave first housekeeping core as preferred. */
>> + first_hk_core = topology_sibling_cpumask(first_hk_cpu);
>> +
>> + /* Find the last CPU which doesn't belong to that first hk_core. */
>> + if (!cpumask_test_cpu(last_cpu, first_hk_core)) {
>> + target_cpu = last_cpu;
>> + } else {
>> + for_each_cpu_andnot(tmp_cpu, cpu_preferred_mask, first_hk_core)
>> + target_cpu = tmp_cpu;
>> + }
>
> Too much local variables. You can drop those tmp_cpu, last_cpu and
> target_cpu, and just use a single variable 'cpu'. That would also
> simplify your logic:
>
> cpu = cpumask_last(cpu_preferred_mask);
> core = topology_sibling_cpumask(first_hk_cpu);
>
> if (cpumask_test_cpu(cpu, core)) {
> for_each_cpu_andnot(cpu, cpu_preferred_mask, core)
> /* nop */ ;
> }
>
> if (cpu >= nr_cpu_ids)
> return;
>
> And so on.
Ok. Just two would suffice i think. cpu, and target_cpu.
With a bit shuffling first_hk_cpu can also go away.
I need at least two so that this loop works.
for_each_cpu_and(tmp_cpu, topology_sibling_cpumask(target_cpu),
cpu_preferred_mask)
set_cpu_preferred(tmp_cpu, false);
>
>> +
>> + /* Only the first housekeeping core remains */
>> + if (target_cpu >= nr_cpu_ids)
>> + return;
>> +
>> + for_each_cpu_and(tmp_cpu, topology_sibling_cpumask(target_cpu),
>> + cpu_preferred_mask)
>> + set_cpu_preferred(tmp_cpu, false);
>> +}
>> +
>> +/*
>> + * Take action to increase preferred CPUs.
>> + *
>
> Again, drop this 'take action' thing.
Sure.
>
>> + * Increase the preferred CPUs by 1 core.
>> + * Add the first core in active & !preferred
>> + *
>> + * Must ensure preferred is subset of active.
>> + */
>> +static void increase_preferred_cpus(void)
>> +{
>> + int first_cpu, tmp_cpu;
>> +
>> + guard(cpus_read_lock)();
>> + first_cpu = cpumask_first_andnot(cpu_active_mask, cpu_preferred_mask);
>> +
>> + /* All CPUs are preferred. Nothing to increase further */
>> + if (first_cpu >= nr_cpu_ids)
>> + return;
>> +
>> + for_each_cpu_and(tmp_cpu, topology_sibling_cpumask(first_cpu),
>> + cpu_active_mask)
>> + set_cpu_preferred(tmp_cpu, true);
>> +}
>> +
>> +static void compute_preferred_cpus_work(struct work_struct *work)
>> +{
>> + u64 curr_steal, delta_steal, delta_ns, steal_ratio;
>> + ktime_t now;
>> +
>> + now = ktime_get();
>> + delta_ns = ktime_to_ns(ktime_sub(now, sg_core_ctx.time));
>> +
>> + if (unlikely(delta_ns < NSEC_PER_MSEC)) {
>> + pr_err_ratelimited("steal_governor: work scheduled too soon delta_ns: %llu\n",
>> + delta_ns);
>> + goto requeue_work;
>> + }
>> +
>> + curr_steal = get_system_steal_time();
>> + delta_steal = curr_steal > sg_core_ctx.steal ?
>> + curr_steal - sg_core_ctx.steal : 0;
>> +
>> + /* Update for next calculation */
>> + sg_core_ctx.steal = curr_steal;
>> + sg_core_ctx.time = now;
>> +
>> + /*
>> + * steal_ratio = (delta_steal * 100*100)/(delta_ns * num_cpus())
>> + * To avoid possible overflow, divide the denominator early.
>> + * Note minimum interval is 100ms.
>> + */
>> + delta_ns = max_t(u64, div_u64(delta_ns * get_num_cpus_steal_ratio(),
>> + 100 * 100), 1);
>> + steal_ratio = div64_u64(delta_steal, delta_ns);
>> +
>> + if (steal_ratio > sg_core_ctx.high_threshold)
>> + decrease_preferred_cpus();
>> + if (steal_ratio <= sg_core_ctx.low_threshold)
>> + increase_preferred_cpus();
>
> If you neither increase, nor decrease, you don't need to check the mask
> because you know you don't modify it. Also, I'd wrap the below integrity
> checks into a helper function.
>
> if (steal_ratio > sg_core_ctx.high_threshold)
> decrease_preferred_cpus();
> else if (steal_ratio <= sg_core_ctx.low_threshold)
> increase_preferred_cpus();
> else
> goto requeue_work;
>
> if (check_integrity())
> return;
>
Ack. Good catch. Will do.
>> + /* maintain design constructs always */
>> + if (cpumask_empty(cpu_preferred_mask)) {
>> + pr_err("empty preferred mask. stop steal governor\n");
>> + restore_preferred_to_active();
>> + return;
>> + }
>> +
>> + if (!cpumask_subset(cpu_preferred_mask, cpu_active_mask)) {
>> + pr_err("preferred: %*pbl is not subset of active: %*pbl, stop steal governor\n",
>> + cpumask_pr_args(cpu_preferred_mask),
>> + cpumask_pr_args(cpu_active_mask));
>> + restore_preferred_to_active();
>> + return;
>> + }
>> +
>> +requeue_work:
>> + /* Trigger for next sampling */
>
> The lablel above is pretty explaining to me. The comment just
> duplicates it. Maybe drop the comment?
Ok.
>
>> + schedule_delayed_work(&sg_core_ctx.work,
>> + msecs_to_jiffies(sg_core_ctx.interval_ms));
>
> If you need jiffies, why don't you have them in the structure, instead of
> milliseconds?
>
> schedule_delayed_work(&sg_core_ctx.work, sg_core_ctx.delay);
>
I would prefer milliseconds as jiffies is very difficult for users to understand.
It depends on HZ value and one has to query from configs.
HZ can very from 100 to 1000 today. Again I will have to play tricks to schedule
the governor at fixed intervals.
So i think it is better to use milliseconds here.
Correct me if i am not making sense.
>> +}
>> +
>> static int __init steal_governor_init(void)
>> {
>> if (sg_core_ctx.low_threshold >= sg_core_ctx.high_threshold) {
>> @@ -100,11 +252,19 @@ static int __init steal_governor_init(void)
>> pr_info("steal_governor is enabled. interval: %ums, high_threshold: %u, low_threshold: %u\n",
>> sg_core_ctx.interval_ms, sg_core_ctx.high_threshold, sg_core_ctx.low_threshold);
>>
>> + INIT_DELAYED_WORK(&sg_core_ctx.work, compute_preferred_cpus_work);
>> + sg_core_ctx.steal = get_system_steal_time();
>> + sg_core_ctx.time = ktime_get();
>> +
>> + schedule_delayed_work(&sg_core_ctx.work,
>> + msecs_to_jiffies(sg_core_ctx.interval_ms));
>> +
>> return 0;
>> }
>>
>> static void __exit steal_governor_exit(void)
>> {
>> + disable_delayed_work_sync(&sg_core_ctx.work);
>> restore_preferred_to_active();
>> pr_info("steal_governor is disabled\n");
>> }
>> diff --git a/drivers/virt/steal_governor/core.h b/drivers/virt/steal_governor/core.h
>> index e27305284ac0..59329c1d7109 100644
>> --- a/drivers/virt/steal_governor/core.h
>> +++ b/drivers/virt/steal_governor/core.h
>> @@ -12,6 +12,11 @@
>> #include <linux/workqueue.h>
>> #include <linux/ktime.h>
>> #include <linux/kconfig.h>
>> +#include <linux/kernel_stat.h>
>> +#include <linux/topology.h>
>> +#include <linux/sched/isolation.h>
>> +#include <linux/cleanup.h>
>> +#include <linux/math64.h>
>>
>> struct steal_governor {
>> struct delayed_work work;
>> --
>> 2.47.3
^ permalink raw reply
* Re: [PATCH v8 10/11] virt/steal_governor: Implement steal_governor policy loop
From: Yury Norov @ 2026-07-22 7:39 UTC (permalink / raw)
To: Shrikanth Hegde
Cc: linux-kernel, mingo, peterz, juri.lelli, vincent.guittot,
yury.norov, kprateek.nayak, iii, corbet, tglx, gregkh, pbonzini,
seanjc, vschneid, huschle, rostedt, dietmar.eggemann, maddy,
srikar, hdanton, chleroy, vineeth, frederic, arighi, pauld,
christian.loehle, tj, tommaso.cucinotta, maz, rafael, rdunlap,
kernellwp, linux-doc, jgross, virtualization
In-Reply-To: <c64ed18e-7863-44c3-9c7b-2f1efd1a805e@linux.ibm.com>
On Wed, Jul 22, 2026 at 12:44:03PM +0530, Shrikanth Hegde wrote:
> Hi Yury.
...
> > > + schedule_delayed_work(&sg_core_ctx.work,
> > > + msecs_to_jiffies(sg_core_ctx.interval_ms));
> >
> > If you need jiffies, why don't you have them in the structure, instead of
> > milliseconds?
> >
> > schedule_delayed_work(&sg_core_ctx.work, sg_core_ctx.delay);
> >
>
> I would prefer milliseconds as jiffies is very difficult for users to understand.
> It depends on HZ value and one has to query from configs.
>
> HZ can very from 100 to 1000 today. Again I will have to play tricks to schedule
> the governor at fixed intervals.
>
> So i think it is better to use milliseconds here.
> Correct me if i am not making sense.
User provides milliseconds, then in init() you convert them into
jiffies and save in sg.delay. That's it.
^ permalink raw reply
* Re: [PATCH] vsock: use sock_error() to consume sk_err after connect timeout
From: Stefano Garzarella @ 2026-07-22 7:55 UTC (permalink / raw)
To: Phi Nguyen
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, syzbot+1b2c9c4a0f8708082678, virtualization, netdev,
linux-kernel
In-Reply-To: <78225425-1ca7-45fb-85cb-9e04f489e68f@gmail.com>
On Tue, Jul 21, 2026 at 01:34:03AM +0800, Phi Nguyen wrote:
>On 7/20/2026 4:17 PM, Stefano Garzarella wrote:
>>On Mon, Jul 20, 2026 at 05:57:47AM +0800, Nguyen Dinh Phi wrote:
>>>After vsock_connect() exits the wait loop due to sk->sk_err being
>>>set, the error was read but not cleared. This left sk->sk_err set
>>>for subsequent operations.
>>
>>So, is this a fix? If yes, we should put a Fixes tag.
>>
>>Also, can you describe how to trigger the issue?
>>
>>Because I see this in vsock_connect(), so I thought it was in some
>>way already handled:
>>
>> /* sk_err might have been set as a result of an earlier
>> * (failed) connect attempt.
>> */
>> sk->sk_err = 0;
>>
>This only handles the case where the function following the failed
>connect is another connect() call.
So, can we remove that with this patch, or better to leave as defensive
action?
>
>>>Switch to sock_error() which atomically reads and clears sk->sk_err,
>>>so the error is consumed when returned.
>>>
>>>Signed-off-by: Nguyen Dinh Phi <phind.uet@gmail.com>
>>>Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
>>
>>Can you explain how this patch fixes that issue?
>>(this should be the first information to be put in the commit message IMHO)
>>
>>I'd like to understand better if this is a fix of real bug or just
>>an improvement to the code (which is fine by me).
>>
>>Thanks,
>>Stefano
>>
>Here are the steps of the syzkaller reproducer:
>
> r0 = socket(AF_VSOCK, SOCK_STREAM, 0)
>
> bind(r0, {VMADDR_CID_ANY, PORT})
>
> connect(r0, {VMADDR_CID_LOCAL, PORT})
>
> listen(r0, backlog)
>
> r1 = socket(AF_VSOCK, SOCK_STREAM, 0)
>
> connect(r1, {VMADDR_CID_LOCAL, PORT})
>
> connect(r0 -> self) -> -1, EPROTO
>
> listen(r0) -> 0
>
> connect(r1 -> r0) -> 0
>
> accept(r0) -> -1, EPROTO
>
>Basically, it creates a socket (r0) and triggers a self-connect after
>binding it. This self-connect fails with EPROTO because it loops back
>to r0 while the socket is still in the TCP_SYN_SENT state, causing it
>to be incorrectly dispatched to the connecting-client path. The
>unexpected packet type encountered there sets sk_err to EPROTO.
>
>After that, it invokes a listen() call on the same socket. This
>listen() call succeeds because the kernel's listening path never
>inspects or clears sk_err. Then, a new socket (r1) is created as a
>normal client and connects to r0. However, vsock_accept() rejects this
>incoming connection because the listener's sk_err still holds the
>EPROTO error from the earlier failed self-connect.
>
>This rejection causes the child socket created for r1's connection to
>never be freed on virtio or hyperv transports; only the VMCI transport
>implements pending_work to revisit and clean up a rejected socket
>This patch will prevent the rejection branch to occur in this scenario.
Okay, get it now, thanks! Please include a summary of this in the commit
description.
I understand that this resolves syzbot's specific test case, but it
would be best to handle rejected sockets more effectively in af_vsock.c
rather than in the transport layers (if possible). In any case, this can
be done in another patch.
>
>I think we might schedule the cleanup worker to run in the rejection
>path for these transports as well.
Yeah, we need to handle that part better, I think it's a leftover when
we generalized AF_VSOCK to support more transport than vmci.
Indeed this part is a bit confusing:
/* If the listener socket has received an error, then we should
* reject this socket and return. Note that we simply mark the
* socket rejected, drop our reference, and let the cleanup
* function handle the cleanup; the fact that we found it in
* the listener's accept queue guarantees that the cleanup
* function hasn't run yet.
*/
if (err) {
vconnected->rejected = true;
} else {
Would be nice to handle everything in af_vsock.c in some way.
In conclusion, the patch LGTM, but please expand the commit description,
add the Fixes tag, and target the net tree in the v2.
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH RFC v3 0/6] Add Rust virtio bindings and sample device
From: Eugenio Perez Martin @ 2026-07-22 8:49 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: Michael S. Tsirkin, Manos Pitsidianakis, Peter Hilber,
Stefano Garzarella, Stefan Hajnoczi, Viresh Kumar, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Daniel Almeida, rust-for-linux, Jason Wang, Xuan Zhuo,
virtualization, linux-kernel
In-Reply-To: <tiiht3.22qk83rct1pd@pitsidianak.is>
On Tue, Jul 21, 2026 at 8:24 AM Manos Pitsidianakis
<manos@pitsidianak.is> wrote:
>
> Ping.
>
> I have a different version of the series WIP with a virtio-rng instead
> of virtio-rtc, based on
> https://lore.kernel.org/lkml/20260529-rust-hw_random-virtio-rng-v1-0-b3153dd90311@pitsidianak.is/
> (which needs a v2 also)
>
> But since this series hasn't gotten any feedback on the virtio part, I'd
> like to ask if there is any before respinning.
>
> Thanks!
>
I also think this will be a great addition, but I have very little
knowledge of Rust. Is it easy to implement the simplest virtio-net
driver using Rust, without CVQ or extra features, so I can test it and
hopefully build features on top? Would it help?
Thanks!
> On Sun, 10 May 2026 16:38, Manos Pitsidianakis <manos@pitsidianak.is> wrote:
> >Hi all, this RFC series adds Rust bindings for Virtio drivers
> >(frontends in virtio parlance).
> >
> >As a PoC, it also adds a sample virtio-rtc driver which performs
> >capability discovery through the virtqueue without registering any clock.
> >
> >Before I send a cleaned-up non-RFC I would like some initial feedback
> >(i.e. is it something the upstream wants?)
> >
> >This was tested with the rust-vmm vhost-device-rtc device backend that I
> >wrote[^0]:
> >
> >[^0]: https://github.com/rust-vmm/vhost-device/tree/main/vhost-device-rtc
> >
> >Instructions:
> >
> > Run the daemon in a separate terminal:
> >
> > $ cargo run --bin vhost-device-rtc -- -s /tmp/rtc.sock
> >
> > Then run the VM:
> >
> > $ qemu-system-aarch64 \
> > -machine type=virt,virtualization=off,acpi=on \
> > -cpu host \
> > -smp 8 \
> > -accel kvm \
> > -drive if=virtio,format=qcow2,file=./debian-13-nocloud-arm64-daily.qcow2 \
> > -device virtio-net-pci,netdev=unet \
> > -device virtio-scsi-pci \
> > -serial mon:stdio \
> > -m 8192 \
> > -object memory-backend-memfd,id=mem,size=8G,share=on \
> > -numa node,memdev=mem \
> > -display none \
> > -vga none \
> > -kernel /path/to/linux/build/arch/arm64/boot/Image \
> > -device vhost-user-test-device,chardev=rtc,id=rtc,virtio-id=17,num_vqs=2,vq_size=1024 \
> > -chardev socket,path=/tmp/rtc.sock,id=rtc \
> > ...
> >
> > Example output:
> > [ 1.105238] rust_virtio_rtc: Probe Rust virtio driver sample.
> > [ 1.105645] rust_virtio_rtc: Found 1 vqs.
> > [ 1.136050] rust_virtio_rtc: process_requestq got buf 16 bytes
> > [ 1.136125] rust_virtio_rtc: Got response! Ok(RespCfg { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, num_clocks: Le16(3), reserved: [0, 0, 0, 0, 0, 0] })
> > [ 1.136701] rust_virtio_rtc: Got response! Ok(RespClockCap { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, clock_type: 3, leap_second_smearing: 0, flags: 0, reserved: [0, 0, 0, 0, 0] })
> > [ 1.136724] rust_virtio_rtc virtio0: cannot expose clock 0 (type 3, variant 0, flags 0) to userspace
> > [ 1.137259] rust_virtio_rtc: Got response! Ok(RespRead { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, clock_reading: Le64(1777890485031060388) })
> > [ 1.137277] rust_virtio_rtc: #0 clock reading = 1777890485031060388
> > [ 1.137749] rust_virtio_rtc: Got response! Ok(RespClockCap { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, clock_type: 1, leap_second_smearing: 0, flags: 0, reserved: [0, 0, 0, 0, 0] })
> > [ 1.137769] rust_virtio_rtc virtio0: cannot expose clock 1 (type 1, variant 0, flags 0) to userspace
> > [ 1.138247] rust_virtio_rtc: Got response! Ok(RespRead { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, clock_reading: Le64(1777890485032086075) })
> > [ 1.138264] rust_virtio_rtc: #1 clock reading = 1777890485032086075
> > [ 1.138730] rust_virtio_rtc: Got response! Ok(RespClockCap { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, clock_type: 2, leap_second_smearing: 0, flags: 0, reserved: [0, 0, 0, 0, 0] })
> > [ 1.138751] rust_virtio_rtc virtio0: cannot expose clock 2 (type 2, variant 0, flags 0) to userspace
> > [ 1.139253] rust_virtio_rtc: Got response! Ok(RespRead { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, clock_reading: Le64(338567896865557) })
> > [ 1.139270] rust_virtio_rtc: #2 clock reading = 338567896865557
> >
> >Concerns - Notes - TODOs
> >========================
> >
> >- Virtqueue lifetimes don't neatly apply to Rust as expected, so a lot
> > of times we have to go through unsafe pointer dereferences (though
> > which are guaranteed by Virtio subsystem to be valid, for example when
> > a callback is called with the vq argument). There's a potential for
> > misuse and definitely could use better thinking.
> >- `struct virtio_device` is not reference-counted like other implemented
> > device types in rust/kernel. Maybe we need to change C API first to
> > make them reference counted, assuming this doesn't break anything?
> >- The sample driver obviously conflicts with the C implementation, so
> > this would either need to move out of samples/ or figure out some way
> > to handle this in kbuild.
> >- kernel::virtio module and its types need a few rustdoc examples that I
> > will add in followup series
> >- Note that the registration of RTC clocks etc in the sample driver is
> > not done, I'm putting it off until I receive some feedback first. The
> > sample driver otherwise does send and receive data from the virtqueue
> > as a PoC.
> >
> >PS: No LLMs used so any mistakes and goofs are solely written by me.
> >
> >Signed-off-by: Manos Pitsidianakis <manos@pitsidianak.is>
> >---
> >Changes in v3:
> >- Removed unused methods from virtio API
> >- Clean up how scattergather lists are added to virtqueues by using
> > owned SGTables only, and make the API safe(r)
> >- Add RAII cleanup for find_vqs return value that calls del_vqs
> >- Reset device after remove callback
> >- Significantly clean up sample driver as a result of the other cleanups
> >- Link to v2: https://lore.kernel.org/r/20260509-rust-virtio-v2-0-c1e30ec2bd21@pitsidianak.is
> >
> >Changes in v2:
> >- Move helper ifdefs to helper file (thanks Alice)
> >- Changed CONFIG checks to IS_ENABLED to allow for CONFIG_VIRTIO=m
> >- Split all use imports to one item per line according to style guide
> >- Fixed wait_for_completion_interruptible*() rustdocs
> >- Use Jiffy type alias in wait_for_completion_interruptible_timeout()
> >- Pepper and salt #[inline]s wherever appropriate as per style guide
> >- Split probe() into probe() and init() to allow cleaning up if init
> > fails
> >- Remove unnecessary Send and Sync unsafe impls for
> > kernel::virtio::Device
> >- Remove unnecessary LeSize and BeSize
> >- Accept Option<_> for virtqueue callback when creating a VirtqueueInfo
> >- Made all vq buffer adding operations unsafe
> >- Use AtomicU16 instead of Cell<u16> for sample virtio driver
> >- Fix RespHead field types in sample virtio driver
> >- Fix response error checking in sample virtio driver
> >- Change some device contexts in method signatures
> >- Link to v1: https://lore.kernel.org/r/20260505-rust-virtio-v1-0-9563383909e4@pitsidianak.is
> >
> >---
> >Manos Pitsidianakis (6):
> > rust/bindings: generate virtio bindings
> > rust/helpers: add virtio.c
> > rust/kernel/device: return parent at same context
> > rust: add virtio module
> > rust: impl interruptible waits for Completion
> > samples/rust: Add sample virtio-rtc driver [WIP]
> >
> > MAINTAINERS | 9 +
> > rust/bindings/bindings_helper.h | 5 +
> > rust/helpers/helpers.c | 1 +
> > rust/helpers/virtio.c | 37 ++++
> > rust/kernel/device.rs | 2 +-
> > rust/kernel/lib.rs | 2 +
> > rust/kernel/sync/completion.rs | 42 +++-
> > rust/kernel/virtio.rs | 423 ++++++++++++++++++++++++++++++++++++++++
> > rust/kernel/virtio/utils.rs | 57 ++++++
> > rust/kernel/virtio/virtqueue.rs | 314 +++++++++++++++++++++++++++++
> > samples/rust/Kconfig | 15 ++
> > samples/rust/Makefile | 1 +
> > samples/rust/rust_virtio_rtc.rs | 403 ++++++++++++++++++++++++++++++++++++++
> > 13 files changed, 1309 insertions(+), 2 deletions(-)
> >---
> >base-commit: 028ef9c96e96197026887c0f092424679298aae8
> >change-id: 20260504-rust-virtio-8523b01dfdc2
> >
> >Best regards,
> >--
> >Manos Pitsidianakis <manos@pitsidianak.is>
> >
>
>
> --
>
> foo
>
^ permalink raw reply
* Re: [PATCH RFC v3 0/6] Add Rust virtio bindings and sample device
From: Manos Pitsidianakis @ 2026-07-22 8:55 UTC (permalink / raw)
To: Eugenio Perez Martin
Cc: Michael S. Tsirkin, Manos Pitsidianakis, Peter Hilber,
Stefano Garzarella, Stefan Hajnoczi, Viresh Kumar, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Daniel Almeida, rust-for-linux, Jason Wang, Xuan Zhuo,
virtualization, linux-kernel
In-Reply-To: <CAJaqyWcSeXW_vdP03-oTXp5uO8vNBg224LDgyCn3QrXhuJ6tHw@mail.gmail.com>
Hi Eugenio,
On Wed, 22 Jul 2026 11:49, Eugenio Perez Martin <eperezma@redhat.com> wrote:
>On Tue, Jul 21, 2026 at 8:24 AM Manos Pitsidianakis
><manos@pitsidianak.is> wrote:
>>
>> Ping.
>>
>> I have a different version of the series WIP with a virtio-rng instead
>> of virtio-rtc, based on
>> https://lore.kernel.org/lkml/20260529-rust-hw_random-virtio-rng-v1-0-b3153dd90311@pitsidianak.is/
>> (which needs a v2 also)
>>
>> But since this series hasn't gotten any feedback on the virtio part, I'd
>> like to ask if there is any before respinning.
>>
>> Thanks!
>>
>
>I also think this will be a great addition, but I have very little
>knowledge of Rust. Is it easy to implement the simplest virtio-net
>driver using Rust, without CVQ or extra features, so I can test it and
>hopefully build features on top? Would it help?
I think so, however it would not be upstreamable right away since (I
think) there are no netdev Rust bindings and abstractions yet. I will
take a look when I have the time...
(This is something I am also interested in because I'm working on a
different kind of network device)
You can definitely play with the sample driver included in this series
however!
Manos
>
>Thanks!
>
>> On Sun, 10 May 2026 16:38, Manos Pitsidianakis <manos@pitsidianak.is> wrote:
>> >Hi all, this RFC series adds Rust bindings for Virtio drivers
>> >(frontends in virtio parlance).
>> >
>> >As a PoC, it also adds a sample virtio-rtc driver which performs
>> >capability discovery through the virtqueue without registering any clock.
>> >
>> >Before I send a cleaned-up non-RFC I would like some initial feedback
>> >(i.e. is it something the upstream wants?)
>> >
>> >This was tested with the rust-vmm vhost-device-rtc device backend that I
>> >wrote[^0]:
>> >
>> >[^0]: https://github.com/rust-vmm/vhost-device/tree/main/vhost-device-rtc
>> >
>> >Instructions:
>> >
>> > Run the daemon in a separate terminal:
>> >
>> > $ cargo run --bin vhost-device-rtc -- -s /tmp/rtc.sock
>> >
>> > Then run the VM:
>> >
>> > $ qemu-system-aarch64 \
>> > -machine type=virt,virtualization=off,acpi=on \
>> > -cpu host \
>> > -smp 8 \
>> > -accel kvm \
>> > -drive if=virtio,format=qcow2,file=./debian-13-nocloud-arm64-daily.qcow2 \
>> > -device virtio-net-pci,netdev=unet \
>> > -device virtio-scsi-pci \
>> > -serial mon:stdio \
>> > -m 8192 \
>> > -object memory-backend-memfd,id=mem,size=8G,share=on \
>> > -numa node,memdev=mem \
>> > -display none \
>> > -vga none \
>> > -kernel /path/to/linux/build/arch/arm64/boot/Image \
>> > -device vhost-user-test-device,chardev=rtc,id=rtc,virtio-id=17,num_vqs=2,vq_size=1024 \
>> > -chardev socket,path=/tmp/rtc.sock,id=rtc \
>> > ...
>> >
>> > Example output:
>> > [ 1.105238] rust_virtio_rtc: Probe Rust virtio driver sample.
>> > [ 1.105645] rust_virtio_rtc: Found 1 vqs.
>> > [ 1.136050] rust_virtio_rtc: process_requestq got buf 16 bytes
>> > [ 1.136125] rust_virtio_rtc: Got response! Ok(RespCfg { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, num_clocks: Le16(3), reserved: [0, 0, 0, 0, 0, 0] })
>> > [ 1.136701] rust_virtio_rtc: Got response! Ok(RespClockCap { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, clock_type: 3, leap_second_smearing: 0, flags: 0, reserved: [0, 0, 0, 0, 0] })
>> > [ 1.136724] rust_virtio_rtc virtio0: cannot expose clock 0 (type 3, variant 0, flags 0) to userspace
>> > [ 1.137259] rust_virtio_rtc: Got response! Ok(RespRead { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, clock_reading: Le64(1777890485031060388) })
>> > [ 1.137277] rust_virtio_rtc: #0 clock reading = 1777890485031060388
>> > [ 1.137749] rust_virtio_rtc: Got response! Ok(RespClockCap { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, clock_type: 1, leap_second_smearing: 0, flags: 0, reserved: [0, 0, 0, 0, 0] })
>> > [ 1.137769] rust_virtio_rtc virtio0: cannot expose clock 1 (type 1, variant 0, flags 0) to userspace
>> > [ 1.138247] rust_virtio_rtc: Got response! Ok(RespRead { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, clock_reading: Le64(1777890485032086075) })
>> > [ 1.138264] rust_virtio_rtc: #1 clock reading = 1777890485032086075
>> > [ 1.138730] rust_virtio_rtc: Got response! Ok(RespClockCap { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, clock_type: 2, leap_second_smearing: 0, flags: 0, reserved: [0, 0, 0, 0, 0] })
>> > [ 1.138751] rust_virtio_rtc virtio0: cannot expose clock 2 (type 2, variant 0, flags 0) to userspace
>> > [ 1.139253] rust_virtio_rtc: Got response! Ok(RespRead { head: ReqHead { msg_type: Le16(0), reserved: [0, 0, 0, 0, 0, 0] }, clock_reading: Le64(338567896865557) })
>> > [ 1.139270] rust_virtio_rtc: #2 clock reading = 338567896865557
>> >
>> >Concerns - Notes - TODOs
>> >========================
>> >
>> >- Virtqueue lifetimes don't neatly apply to Rust as expected, so a lot
>> > of times we have to go through unsafe pointer dereferences (though
>> > which are guaranteed by Virtio subsystem to be valid, for example when
>> > a callback is called with the vq argument). There's a potential for
>> > misuse and definitely could use better thinking.
>> >- `struct virtio_device` is not reference-counted like other implemented
>> > device types in rust/kernel. Maybe we need to change C API first to
>> > make them reference counted, assuming this doesn't break anything?
>> >- The sample driver obviously conflicts with the C implementation, so
>> > this would either need to move out of samples/ or figure out some way
>> > to handle this in kbuild.
>> >- kernel::virtio module and its types need a few rustdoc examples that I
>> > will add in followup series
>> >- Note that the registration of RTC clocks etc in the sample driver is
>> > not done, I'm putting it off until I receive some feedback first. The
>> > sample driver otherwise does send and receive data from the virtqueue
>> > as a PoC.
>> >
>> >PS: No LLMs used so any mistakes and goofs are solely written by me.
>> >
>> >Signed-off-by: Manos Pitsidianakis <manos@pitsidianak.is>
>> >---
>> >Changes in v3:
>> >- Removed unused methods from virtio API
>> >- Clean up how scattergather lists are added to virtqueues by using
>> > owned SGTables only, and make the API safe(r)
>> >- Add RAII cleanup for find_vqs return value that calls del_vqs
>> >- Reset device after remove callback
>> >- Significantly clean up sample driver as a result of the other cleanups
>> >- Link to v2: https://lore.kernel.org/r/20260509-rust-virtio-v2-0-c1e30ec2bd21@pitsidianak.is
>> >
>> >Changes in v2:
>> >- Move helper ifdefs to helper file (thanks Alice)
>> >- Changed CONFIG checks to IS_ENABLED to allow for CONFIG_VIRTIO=m
>> >- Split all use imports to one item per line according to style guide
>> >- Fixed wait_for_completion_interruptible*() rustdocs
>> >- Use Jiffy type alias in wait_for_completion_interruptible_timeout()
>> >- Pepper and salt #[inline]s wherever appropriate as per style guide
>> >- Split probe() into probe() and init() to allow cleaning up if init
>> > fails
>> >- Remove unnecessary Send and Sync unsafe impls for
>> > kernel::virtio::Device
>> >- Remove unnecessary LeSize and BeSize
>> >- Accept Option<_> for virtqueue callback when creating a VirtqueueInfo
>> >- Made all vq buffer adding operations unsafe
>> >- Use AtomicU16 instead of Cell<u16> for sample virtio driver
>> >- Fix RespHead field types in sample virtio driver
>> >- Fix response error checking in sample virtio driver
>> >- Change some device contexts in method signatures
>> >- Link to v1: https://lore.kernel.org/r/20260505-rust-virtio-v1-0-9563383909e4@pitsidianak.is
>> >
>> >---
>> >Manos Pitsidianakis (6):
>> > rust/bindings: generate virtio bindings
>> > rust/helpers: add virtio.c
>> > rust/kernel/device: return parent at same context
>> > rust: add virtio module
>> > rust: impl interruptible waits for Completion
>> > samples/rust: Add sample virtio-rtc driver [WIP]
>> >
>> > MAINTAINERS | 9 +
>> > rust/bindings/bindings_helper.h | 5 +
>> > rust/helpers/helpers.c | 1 +
>> > rust/helpers/virtio.c | 37 ++++
>> > rust/kernel/device.rs | 2 +-
>> > rust/kernel/lib.rs | 2 +
>> > rust/kernel/sync/completion.rs | 42 +++-
>> > rust/kernel/virtio.rs | 423 ++++++++++++++++++++++++++++++++++++++++
>> > rust/kernel/virtio/utils.rs | 57 ++++++
>> > rust/kernel/virtio/virtqueue.rs | 314 +++++++++++++++++++++++++++++
>> > samples/rust/Kconfig | 15 ++
>> > samples/rust/Makefile | 1 +
>> > samples/rust/rust_virtio_rtc.rs | 403 ++++++++++++++++++++++++++++++++++++++
>> > 13 files changed, 1309 insertions(+), 2 deletions(-)
>> >---
>> >base-commit: 028ef9c96e96197026887c0f092424679298aae8
>> >change-id: 20260504-rust-virtio-8523b01dfdc2
>> >
>> >Best regards,
>> >--
>> >Manos Pitsidianakis <manos@pitsidianak.is>
>> >
^ permalink raw reply
* Re: [PATCH v5 2/5] vhost/vsock: suppress EHOSTUNREACH fast-fail during CPR pause
From: Stefano Garzarella @ 2026-07-22 9:14 UTC (permalink / raw)
To: Andrey Drobyshev
Cc: linux-kernel, kvm, virtualization, netdev, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <20260720102241.371610-3-andrey.drobyshev@virtuozzo.com>
On Mon, Jul 20, 2026 at 01:22:38PM +0300, Andrey Drobyshev wrote:
>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 an 'ever_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.
>
>The VHOST_RESET_OWNER ioctl is implemented in a following patch, and
>without RESET_OWNER the problem we fix here isn't manifesting - thus
>this patch is a preparation to support RESET_OWNER.
>
>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.
>
>Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>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(-)
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox