* [PATCH v3 39/41] x86/paravirt: Move using_native_sched_clock() stub into timer.h
From: Sean Christopherson @ 2026-05-15 19:19 UTC (permalink / raw)
To: Kiryl Shutsemau, Paolo Bonzini, Sean Christopherson,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner, David Woodhouse
In-Reply-To: <20260515191942.1892718-1-seanjc@google.com>
Now that timer.h ended up with CONFIG_PARAVIRT #ifdeffery anyways, move the
PARAVIRT=n using_native_sched_clock() stub into timer.h as a "free"
optimization.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/timer.h | 5 +++--
arch/x86/kernel/tsc.c | 2 --
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/timer.h b/arch/x86/include/asm/timer.h
index ab1271bd9c3b..d8cb9c84f2c7 100644
--- a/arch/x86/include/asm/timer.h
+++ b/arch/x86/include/asm/timer.h
@@ -11,9 +11,9 @@ extern void recalibrate_cpu_khz(void);
extern int no_timer_check;
-extern bool using_native_sched_clock(void);
-
#ifdef CONFIG_PARAVIRT
+extern bool using_native_sched_clock(void);
+
int __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
void (*save)(void), void (*restore)(void),
void (*start_secondary));
@@ -27,6 +27,7 @@ static __always_inline void paravirt_set_sched_clock(u64 (*func)(void),
void paravirt_sched_clock_start_secondary(void);
#else
static inline void paravirt_sched_clock_start_secondary(void) { }
+static inline bool using_native_sched_clock(void) { return true; }
#endif
/*
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index f78e86494dec..1b569954ae5e 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -316,8 +316,6 @@ int __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
}
#else
u64 sched_clock_noinstr(void) __attribute__((alias("native_sched_clock")));
-
-bool using_native_sched_clock(void) { return true; }
#endif
notrace u64 sched_clock(void)
--
2.54.0.563.g4f69b47b94-goog
^ permalink raw reply related
* [PATCH v3 40/41] x86/tsc: Add standalone helper for getting CPU frequency from CPUID
From: Sean Christopherson @ 2026-05-15 19:19 UTC (permalink / raw)
To: Kiryl Shutsemau, Paolo Bonzini, Sean Christopherson,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner, David Woodhouse
In-Reply-To: <20260515191942.1892718-1-seanjc@google.com>
Extract the guts of cpu_khz_from_cpuid() to a standalone helper that
doesn't restrict the usage to Intel CPUs. This will allow sharing the
core logic with kvmclock, as (a) CPUID.0x16 may be enumerated alongside
kvmclock, and (b) KVM generally doesn't restrict CPUID based on vendor.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/tsc.h | 1 +
arch/x86/kernel/tsc.c | 37 +++++++++++++++++++++++--------------
2 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/arch/x86/include/asm/tsc.h b/arch/x86/include/asm/tsc.h
index f458be688512..c145f5707b52 100644
--- a/arch/x86/include/asm/tsc.h
+++ b/arch/x86/include/asm/tsc.h
@@ -91,6 +91,7 @@ struct cpuid_tsc_info {
};
extern int cpuid_get_tsc_info(struct cpuid_tsc_info *info);
extern int cpuid_get_tsc_freq(struct cpuid_tsc_info *info);
+extern int cpuid_get_cpu_freq(unsigned int *cpu_khz);
extern void tsc_early_init(void);
extern void tsc_init(void);
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 1b569954ae5e..745fa2052c74 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -719,6 +719,24 @@ int cpuid_get_tsc_freq(struct cpuid_tsc_info *info)
return 0;
}
+int cpuid_get_cpu_freq(unsigned int *cpu_khz)
+{
+ unsigned int eax_base_mhz, ebx, ecx, edx;
+
+ *cpu_khz = 0;
+
+ if (boot_cpu_data.cpuid_level < CPUID_LEAF_FREQ)
+ return -ENOENT;
+
+ cpuid(CPUID_LEAF_FREQ, &eax_base_mhz, &ebx, &ecx, &edx);
+
+ if (!eax_base_mhz)
+ return -ENOENT;
+
+ *cpu_khz = eax_base_mhz * 1000;
+ return 0;
+}
+
/**
* native_calibrate_tsc - determine TSC frequency
* Determine TSC frequency via CPUID, else return 0.
@@ -754,13 +772,8 @@ unsigned long native_calibrate_tsc(void)
* clock, but we can easily calculate it to a high degree of accuracy
* by considering the crystal ratio and the CPU speed.
*/
- if (!info.crystal_khz && boot_cpu_data.cpuid_level >= CPUID_LEAF_FREQ) {
- unsigned int eax_base_mhz, ebx, ecx, edx;
-
- cpuid(CPUID_LEAF_FREQ, &eax_base_mhz, &ebx, &ecx, &edx);
- info.crystal_khz = eax_base_mhz * 1000 *
- info.denominator / info.numerator;
- }
+ if (!info.crystal_khz && !cpuid_get_cpu_freq(&cpu_khz))
+ info.crystal_khz = cpu_khz * info.denominator / info.numerator;
if (!info.crystal_khz)
return 0;
@@ -787,19 +800,15 @@ unsigned long native_calibrate_tsc(void)
static unsigned long cpu_khz_from_cpuid(void)
{
- unsigned int eax_base_mhz, ebx_max_mhz, ecx_bus_mhz, edx;
+ unsigned int cpu_khz;
if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
return 0;
- if (boot_cpu_data.cpuid_level < CPUID_LEAF_FREQ)
+ if (cpuid_get_cpu_freq(&cpu_khz))
return 0;
- eax_base_mhz = ebx_max_mhz = ecx_bus_mhz = edx = 0;
-
- cpuid(CPUID_LEAF_FREQ, &eax_base_mhz, &ebx_max_mhz, &ecx_bus_mhz, &edx);
-
- return eax_base_mhz * 1000;
+ return cpu_khz;
}
/*
--
2.54.0.563.g4f69b47b94-goog
^ permalink raw reply related
* [PATCH v3 41/41] x86/kvmclock: Get CPU base frequency from CPUID when it's available
From: Sean Christopherson @ 2026-05-15 19:19 UTC (permalink / raw)
To: Kiryl Shutsemau, Paolo Bonzini, Sean Christopherson,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner, David Woodhouse
In-Reply-To: <20260515191942.1892718-1-seanjc@google.com>
If CPUID.0x16 is present and valid, use the CPU frequency provided by
CPUID instead of assuming that the virtual CPU runs at the same
frequency as TSC and/or kvmclock. Back before constant TSCs were a
thing, treating the TSC and CPU frequencies as one and the same was
somewhat reasonable, but now it's nonsensical, especially if the
hypervisor explicitly enumerates the CPU frequency.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kernel/kvmclock.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 62c8ea2e6769..7607920ae386 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -190,6 +190,20 @@ void kvmclock_cpu_action(enum kvm_guest_cpu_action action)
}
}
+static unsigned long kvm_get_cpu_khz(void)
+{
+ unsigned int cpu_khz;
+
+ /*
+ * Prefer CPUID over kvmclock when possible, as the base CPU frequency
+ * isn't necessarily the same as the kvmlock "TSC" frequency.
+ */
+ if (!cpuid_get_cpu_freq(&cpu_khz))
+ return cpu_khz;
+
+ return pvclock_tsc_khz(this_cpu_pvti());
+}
+
/*
* If we don't do that, there is the possibility that the guest
* will calibrate under heavy load - thus, getting a lower lpj -
@@ -434,7 +448,7 @@ void __init kvmclock_init(void)
kvm_sched_clock_init(stable);
}
- tsc_register_calibration_routines(kvm_get_tsc_khz, kvm_get_tsc_khz,
+ tsc_register_calibration_routines(kvm_get_tsc_khz, kvm_get_cpu_khz,
tsc_properties);
x86_platform.get_wallclock = kvm_get_wallclock;
--
2.54.0.563.g4f69b47b94-goog
^ permalink raw reply related
* [PATCH] virtio_console: fix race between hvc put_chars and virtqueue teardown on freeze
From: Sungho Bae @ 2026-05-15 22:52 UTC (permalink / raw)
To: amit, arnd, gregkh; +Cc: virtualization, linux-kernel, Sungho Bae
From: Sungho Bae <baver.bae@lge.com>
With no_console_suspend enabled, hvc console output can continue while
virtio_console is freezing. In that window, put_chars can still enqueue
buffers to the output virtqueue while virtcons_freeze is tearing queues
down.
This races with virtqueue_detach_unused_buf_split, which expects all
descriptors to be reclaimed during teardown and can hit:
BUG_ON(vq->vq.num_free != vq->split.vring.num)
Root cause is concurrent virtqueue_add_outbuf activity from the console
TX path during freeze.
Add a pm_freezing flag in ports_device and set it at the beginning of
virtcons_freeze. Check the flag in put_chars and __send_to_port to drop
output while freeze is in progress, preventing new buffer enqueue before
vq teardown. Clear the flag on restore only after all port->out_vq
pointers have been reassigned to the newly allocated virtqueues, which
prevents a use-after-free window where TX paths could dereference
already-freed virtqueues.
Place the pm_freezing check inside outvq_lock in __send_to_port so that
it serializes with remove_port_data, which also takes outvq_lock during
freeze. This provides a formal guarantee: once remove_port_data returns
for a given port, no concurrent __send_to_port call can still be adding
buffers to the vq, eliminating the race window with remove_vqs.
Use smp_store_release/smp_load_acquire for the flag to guarantee correct
cross-CPU visibility on weakly ordered architectures.
Signed-off-by: Sungho Bae <baver.bae@lge.com>
---
drivers/char/virtio_console.c | 46 +++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 9a33217c68d9..0fbe40a7f1e7 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -157,6 +157,12 @@ struct ports_device {
/* Major number for this device. Ports will be created as minors. */
int chr_major;
+
+ /*
+ * Set to true during PM freeze to block TX paths that may race
+ * with virtqueue teardown (e.g. hvc put_chars with no_console_suspend).
+ */
+ bool pm_freezing;
};
struct port_stats {
@@ -606,6 +612,20 @@ static ssize_t __send_to_port(struct port *port, struct scatterlist *sg,
spin_lock_irqsave(&port->outvq_lock, flags);
+ /*
+ * Check freeze flag under the lock so that the flag check and
+ * virtqueue_add_outbuf() are atomic with respect to
+ * remove_port_data() which also takes outvq_lock. This
+ * guarantees that once remove_port_data() returns, no new
+ * buffers can be added before remove_vqs() tears down the vq.
+ * Pairs with smp_store_release() in virtcons_freeze/restore.
+ */
+ if (!port->portdev ||
+ smp_load_acquire(&port->portdev->pm_freezing)) { /* pairs with freeze/restore */
+ in_count = 0;
+ goto done;
+ }
+
reclaim_consumed_buffers(port);
err = virtqueue_add_outbuf(out_vq, sg, nents, data, GFP_ATOMIC);
@@ -1115,6 +1135,17 @@ static ssize_t put_chars(u32 vtermno, const u8 *buf, size_t count)
if (!port)
return -EPIPE;
+ /*
+ * Silently drop output during freeze: the console is still active
+ * with no_console_suspend but the virtqueues are being torn down.
+ * This early check avoids a pointless GFP_ATOMIC allocation;
+ * __send_to_port() rechecks under outvq_lock for correctness.
+ * Pairs with smp_store_release() in virtcons_freeze/restore.
+ */
+ if (!port->portdev ||
+ smp_load_acquire(&port->portdev->pm_freezing)) /* pairs with freeze/restore */
+ return count;
+
data = kmemdup(buf, count, GFP_ATOMIC);
if (!data)
return -ENOMEM;
@@ -1972,6 +2003,7 @@ static int virtcons_probe(struct virtio_device *vdev)
/* Attach this portdev to this virtio_device, and vice-versa. */
portdev->vdev = vdev;
vdev->priv = portdev;
+ portdev->pm_freezing = false;
portdev->chr_major = register_chrdev(0, "virtio-portsdev",
&portdev_fops);
@@ -2092,6 +2124,13 @@ static int virtcons_freeze(struct virtio_device *vdev)
portdev = vdev->priv;
+ /*
+ * Block TX paths (put_chars, __send_to_port) before resetting the
+ * device and tearing down virtqueues. This prevents races with
+ * hvc console writes that remain active under no_console_suspend.
+ */
+ smp_store_release(&portdev->pm_freezing, true);
+
virtio_reset_device(vdev);
if (use_multiport(portdev))
@@ -2153,6 +2192,13 @@ static int virtcons_restore(struct virtio_device *vdev)
if (port->guest_connected)
send_control_msg(port, VIRTIO_CONSOLE_PORT_OPEN, 1);
}
+
+ /*
+ * Allow TX paths only after all port->out_vq pointers have
+ * been reassigned to the newly allocated virtqueues.
+ */
+ smp_store_release(&portdev->pm_freezing, false);
+
return 0;
}
#endif
--
2.34.1
^ permalink raw reply related
* Re: [PATCH net] vsock/virtio: fix zerocopy completion for multi-skb sends
From: patchwork-bot+netdevbpf @ 2026-05-16 0:50 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, kuba, pabeni, horms, avkrasnov, stefanha, kvm, edumazet,
eperezma, mst, xuanzhuo, virtualization, davem, jasowang,
linux-kernel, maherazz04
In-Reply-To: <20260514092948.268720-1-sgarzare@redhat.com>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 14 May 2026 11:29:48 +0200 you wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
>
> When a large message is fragmented into multiple skbs, the zerocopy
> uarg is only allocated and attached to the last skb in the loop.
> Non-final skbs carry pinned user pages with no completion tracking,
> so the kernel has no way to notify userspace when those pages are safe
> to reuse. If the loop breaks early the uarg is never allocated at all,
> leaking pinned pages with no completion notification.
>
> [...]
Here is the summary with links:
- [net] vsock/virtio: fix zerocopy completion for multi-skb sends
https://git.kernel.org/netdev/net/c/ae38d9179190
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [RFC PATCH v9 0/5] virtio: add noirq system sleep PM callbacks for virtio-mmio
From: Sungho Bae @ 2026-05-16 1:57 UTC (permalink / raw)
To: mst, jasowang
Cc: xuanzhuo, eperezma, stephan.gerhold, virtualization, linux-kernel,
Sungho Bae
Hi all,
Some virtio-mmio based devices, such as virtio-clock or virtio-regulator,
must become operational before other devices have their regular PM restore
callbacks invoked, because those other devices depend on them.
Generally, the PM framework provides the three phases (freeze, freeze_late,
freeze_noirq) for the system sleep sequence, and the corresponding resume
phases. But, the virtio core only supports the normal freeze/restore phase,
so virtio drivers have no way to participate in the noirq phase, which runs
with IRQs disabled and is guaranteed to run before any normal-phase restore
callbacks.
This series adds the infrastructure and the virtio-mmio transport
wiring so that virtio drivers can implement freeze_noirq/restore_noirq
callbacks.
Design overview
===============
The noirq phase runs with device IRQ handlers disabled and must avoid
sleepable operations. The main constraints addressed are:
- might_sleep() in virtio_add_status() and virtio_features_ok().
- virtio_synchronize_cbs() in virtio_reset_device() (IRQs are already
quiesced).
- spin_lock_irq() in virtio_config_core_enable() (not safe to call
with interrupts already disabled).
- Memory allocation during vq setup (virtqueue_reinit_vring() reuses
existing buffers instead).
The series provides noirq-safe variants for each of these, plus a new
config_ops->reset_vqs() callback that lets the transport reprogram
queue registers without freeing/reallocating vring memory.
Not all transports can safely perform these operations in the noirq phase.
Transports like virtio-ccw issue channel commands and wait for a completion
interrupt, which will never arrive while device interrupts are masked at
the interrupt controller. A new boolean field config_ops->noirq_safe marks
transports that implement reset/status operations via simple MMIO
reads/writes and are therefore safe to use in noirq context. The noirq
helpers assert this flag at runtime, and virtio_device_freeze_noirq()
enforces it at freeze time, returning -EOPNOTSUPP early to prevent
a deadlock on resume.
When a driver implements restore_noirq, the device bring-up (reset ->
ACKNOWLEDGE -> DRIVER -> finalize_features -> FEATURES_OK) happens in
the noirq phase. The subsequent normal-phase virtio_device_restore()
detects this and skips the redundant re-initialization.
Patch breakdown
===============
Patch 1 fixes a bug that configures the guest page size before
the reset step of the initialization sequence on virtio-mmio
legacy devices.
Patch 2 is a preparatory refactoring with no functional change.
Patches 3-4 add the core infrastructure. Patch 5 wires it up for
virtio-mmio.
1. virtio-mmio: move guest page size setting into vm_reset()
Moves the GuestPageSize configuration into vm_reset() immediately
after the status register reset to fully comply with the virtio-mmio
legacy specification. It prevents the configuration from being
cleared during device resets and eliminates duplicate write
operations across probe and restore paths.
https://lore.kernel.org/all/20260507183807.22007-1-baver.bae@gmail.com/
2. virtio: separate PM restore and reset_done paths
Splits virtio_device_restore_priv() into independent
virtio_device_restore() and virtio_device_reset_done() paths,
using a shared virtio_device_reinit() helper. This is a pure
refactoring to make the restore path independently extensible
without complicating the boolean dispatch.
3. virtio_ring: export virtqueue_reinit_vring() for noirq restore
Adds virtqueue_reinit_vring(), an exported wrapper that resets
vring indices and descriptor state in place without any memory
allocation, making it safe to call from noirq context. Also
resets IN_ORDER-specific state (free_head, batch_last.id) in
virtqueue_init() to keep the ring consistent after reinit, and
adds runtime WARN_ON checks for unexpected in-flight descriptor
state and split-ring index consistency.
4. virtio: add noirq system sleep PM infrastructure
Adds noirq-safe helpers (virtio_add_status_noirq,
virtio_features_ok_noirq, virtio_reset_device_noirq,
virtio_config_core_enable_noirq, virtio_device_ready_noirq) and
the freeze_noirq/restore_noirq driver callbacks plus the
config_ops->reset_vqs() transport hook. Introduces
config_ops->noirq_safe to mark transports whose reset/status
operations are safe in noirq context (e.g. simple MMIO), and
enforces early -EOPNOTSUPP in virtio_device_freeze() when
the transport does not meet noirq requirements. Modifies
virtio_device_restore() to skip bring-up when restore_noirq
already ran.
5. virtio-mmio: wire up noirq system sleep PM callbacks
Implements vm_reset_vqs() which iterates existing virtqueues,
reinitializes the vring state via virtqueue_reinit_vring(), and
reprograms the MMIO queue registers. Adds
virtio_mmio_freeze_noirq/virtio_mmio_restore_noirq and registers
them via SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(). Sets .noirq_safe = true
in virtio_mmio_config_ops to declare that MMIO-based status and
reset operations are safe during the noirq PM phase.
Testing
=======
Build-tested with arm64 cross-compilation.
(make ARCH=arm64 M=drivers/virtio)
Runtime-tested on an internal virtio-mmio platform with virtio-clock,
confirming that the clock device works well before other devices' normal
restore() callbacks run.
Notes
=====
- Although the author of the fixed commit (e0c2ce821795) in Patch 1 is
included in the CC list, please note that their email address returned
a delivery failure error during previous submissions.
Changes
=======
v9:
virtio-mmio: move guest page size setting into vm_reset()
- Newly introduced in this series as a prerequisite for Patch 5.
virtio-mmio: wire up noirq system sleep PM callbacks
- Fixed a bug where VIRTIO_MMIO_GUEST_PAGE_SIZE was set before calling
virtio_device_restore_noirq().
v8:
virtio: add noirq system sleep PM infrastructure
- Enforced freeze/restore and freeze_noirq/restore_noirq callback
pairing in virtio_device_freeze() via virtio_has_valid_pm_cbs()
- Skipped virtio_check_mem_acc_cb() in noirq path (may sleep)
- Set VIRTIO_NOIRQ_ENTERED only on freeze_noirq success to allow
restore() to proceed on failure.
v7:
virtio: add noirq system sleep PM infrastructure
- Configured virtio_noirq_state to have 3 states to differentiate
between restore_noirq failures and skipping the noirq phase.
- Re-verified the PM callback combinations.
virtio-mmio: wire up noirq system sleep PM callbacks
- Aligned both conditions for GUEST_PAGE_SIZE and virtio_device_reinit()
v6:
virtio_ring: export virtqueue_reinit_vring() for noirq restore
- Made virtqueue_reinit_vring() fail with -EBUSY on precondition
violations and propagate that error.
virtio: add noirq system sleep PM infrastructure
- Make noirq restore failure terminal for the same device:
.restore is not a same-device fallback for .restore_noirq failure.
- Decouple noirq failure from pre-freeze dev->failed snapshot.
- Add noirq_safe validation in virtio_device_freeze() to catch transport
mismatch early.
virtio-mmio: wire up noirq system sleep PM callbacks
- Make vm_reset_vqs() handle virtqueue_reinit_vring() failures
to prevent continuing noirq restore with a potentially corrupted
split free-list state.
v5:
virtio: add noirq system sleep PM infrastructure
- Preserve FAILED across restore_noirq() failure by recording the
failure in dev->failed before falling back to the normal restore path.
- Document the restore/restore_noirq fallback contract more clearly,
especially for drivers that preserve virtqueues across suspend.
v4:
virtio_ring: export virtqueue_reinit_vring() for noirq restore
- Reinit safety was tightened by resetting IN_ORDER-specific state.
- Added extra split-ring consistency WARN_ON checks.
- Clarified caller preconditions for noirq-safe vring reinit.
virtio: add noirq system sleep PM infrastructure
- Added config_ops->noirq_safe to explicitly mark transports that are
safe in noirq PM phase.
- Enforced early -EOPNOTSUPP checks in freeze_noirq for unsupported
transport combinations (noirq_safe/reset_vqs requirements).
- Added defensive runtime guards/warnings in noirq helper and restore
paths.
- Discussed the freeze-before-freeze_noirq abort scenario raised in
review and concluded that the fallback restore path is intentionally
handled by regular restore after core reinit/reset, so reset_vqs is
kept limited to the noirq restore flow.
virtio-mmio: wire up noirq system sleep PM callbacks
- Marked virtio-mmio transport as noirq-capable by setting .noirq_safe
in virtio_mmio_config_ops.
v3:
virtio: separate PM restore and reset_done paths
- Refined restore flow to explicitly handle the no-driver case after
reinit, improving clarity and avoiding unnecessary driver-path
assumptions.
virtio_ring: export virtqueue_reinit_vring() for noirq restore
- Hardened virtqueue_reinit_vring() with stronger safety notes and
a runtime WARN_ON check to catch reinit with unexpected free-list
state.
virtio: add noirq system sleep PM infrastructure
- Added explicit noirq restore completion tracking noirq_restore_done
and updated PM sequencing to use it, plus early freeze_noirq
validation for missing reset_vqs support.
virtio-mmio: wire up noirq system sleep PM callbacks
- Updated virtio-mmio restore path to skip legacy GUEST_PAGE_SIZE
rewrite when noirq restore already completed.
v2:
virtio-mmio: wire up noirq system sleep PM callbacks
- The code that was duplicated in vm_setup_vq() and vm_reset_vqs() has
been moved to vm_active_vq() function.
Sungho Bae (5):
virtio-mmio: move guest page size setting into vm_reset()
virtio: separate PM restore and reset_done paths
virtio_ring: export virtqueue_reinit_vring() for noirq restore
virtio: add noirq system sleep PM infrastructure
virtio-mmio: wire up noirq system sleep PM callbacks
drivers/virtio/virtio.c | 368 +++++++++++++++++++++++++++++++---
drivers/virtio/virtio_mmio.c | 146 ++++++++++----
drivers/virtio/virtio_ring.c | 58 ++++++
include/linux/virtio.h | 42 ++++
include/linux/virtio_config.h | 39 ++++
include/linux/virtio_ring.h | 3 +
6 files changed, 586 insertions(+), 70 deletions(-)
--
2.34.1
^ permalink raw reply
* [RFC PATCH v9 1/5] virtio-mmio: move guest page size setting into vm_reset()
From: Sungho Bae @ 2026-05-16 1:57 UTC (permalink / raw)
To: mst, jasowang
Cc: xuanzhuo, eperezma, stephan.gerhold, virtualization, linux-kernel,
Sungho Bae
In-Reply-To: <20260516015756.20948-1-baver.bae@gmail.com>
From: Sungho Bae <baver.bae@lge.com>
The virtio-mmio legacy spec (Section 4.2.4) requires the driver to
write the guest page size "during initialization, before any queues
are used". Reset is step 1 of the initialization sequence
(Section 3.1), so setting GuestPageSize immediately after the status
register reset in vm_reset() is more proper.
Currently the GuestPageSize write lives in two separate call sites:
- virtio_mmio_probe(), before register_virtio_device()
- virtio_mmio_restore(), before virtio_device_restore()
Both of these write the value *before* the reset that happens inside
register_virtio_device()/virtio_device_restore(), so a device
implementation that clears GuestPageSize on reset would lose the
value. QEMU's virtio_mmio_reset() for example zeroes guest_page_shift
on a full device reset.
The current code happens to work because the Linux driver triggers
only a "soft reset" (STATUS register write of 0), and QEMU's
soft-reset path does not clear guest_page_shift. But relying on
this is fragile and not guaranteed by the spec.
Move the GuestPageSize write into vm_reset(), right after the status
reset. This ensures the value is set:
- at the correct point in the initialization sequence per spec,
- after every reset (probe, restore, or any future path), and
- exactly once, removing the duplication.
Fixes: e0c2ce821795 ("virtio_mmio: Restore guest page size on resume")
Signed-off-by: Sungho Bae <baver.bae@lge.com>
---
drivers/virtio/virtio_mmio.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 595c2274fbb5..daa65b269a36 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -254,6 +254,16 @@ static void vm_reset(struct virtio_device *vdev)
/* 0 status means a reset. */
writel(0, vm_dev->base + VIRTIO_MMIO_STATUS);
+
+ /*
+ * The virtio-mmio legacy spec requires the driver to write the
+ * guest page size during initialization, before any queues are
+ * used. Since reset is step 1 of initialization (Section 3.1),
+ * set it here so it is always in place for subsequent queue setup
+ * in every code path (probe, restore, etc.).
+ */
+ if (vm_dev->version == 1)
+ writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
}
@@ -547,9 +557,6 @@ static int virtio_mmio_restore(struct device *dev)
{
struct virtio_mmio_device *vm_dev = dev_get_drvdata(dev);
- if (vm_dev->version == 1)
- writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
-
return virtio_device_restore(&vm_dev->vdev);
}
@@ -619,8 +626,6 @@ static int virtio_mmio_probe(struct platform_device *pdev)
vm_dev->vdev.id.vendor = readl(vm_dev->base + VIRTIO_MMIO_VENDOR_ID);
if (vm_dev->version == 1) {
- writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
-
rc = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
/*
* In the legacy case, ensure our coherently-allocated virtio
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v9 2/5] virtio: separate PM restore and reset_done paths
From: Sungho Bae @ 2026-05-16 1:57 UTC (permalink / raw)
To: mst, jasowang
Cc: xuanzhuo, eperezma, stephan.gerhold, virtualization, linux-kernel,
Sungho Bae
In-Reply-To: <20260516015756.20948-1-baver.bae@gmail.com>
From: Sungho Bae <baver.bae@lge.com>
Refactor virtio_device_restore_priv() by extracting the common device
re-initialization sequence into virtio_device_reinit(). This helper
performs the full bring-up sequence: reset, status acknowledgment,
feature finalization, and feature negotiation.
virtio_device_restore() and virtio_device_reset_done() now each call
virtio_device_reinit() directly instead of going through a boolean-
dispatched wrapper. This makes each path independently readable and
extensible without further complicating the dispatch logic.
A follow-up series will add noirq PM callbacks that only affect the
restore path; having the two paths separated avoids adding more
conditionals to a shared function.
No functional change.
Signed-off-by: Sungho Bae <baver.bae@lge.com>
---
drivers/virtio/virtio.c | 81 +++++++++++++++++++++++++----------------
1 file changed, 50 insertions(+), 31 deletions(-)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 5bdc6b82b30b..98f1875f8df1 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -588,7 +588,7 @@ void unregister_virtio_device(struct virtio_device *dev)
}
EXPORT_SYMBOL_GPL(unregister_virtio_device);
-static int virtio_device_restore_priv(struct virtio_device *dev, bool restore)
+static int virtio_device_reinit(struct virtio_device *dev)
{
struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
int ret;
@@ -613,35 +613,9 @@ static int virtio_device_restore_priv(struct virtio_device *dev, bool restore)
ret = dev->config->finalize_features(dev);
if (ret)
- goto err;
-
- ret = virtio_features_ok(dev);
- if (ret)
- goto err;
-
- if (restore) {
- if (drv->restore) {
- ret = drv->restore(dev);
- if (ret)
- goto err;
- }
- } else {
- ret = drv->reset_done(dev);
- if (ret)
- goto err;
- }
-
- /* If restore didn't do it, mark device DRIVER_OK ourselves. */
- if (!(dev->config->get_status(dev) & VIRTIO_CONFIG_S_DRIVER_OK))
- virtio_device_ready(dev);
-
- virtio_config_core_enable(dev);
-
- return 0;
+ return ret;
-err:
- virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
- return ret;
+ return virtio_features_ok(dev);
}
#ifdef CONFIG_PM_SLEEP
@@ -668,7 +642,33 @@ EXPORT_SYMBOL_GPL(virtio_device_freeze);
int virtio_device_restore(struct virtio_device *dev)
{
- return virtio_device_restore_priv(dev, true);
+ struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+ int ret;
+
+ ret = virtio_device_reinit(dev);
+ if (ret)
+ goto err;
+
+ if (!drv)
+ return 0;
+
+ if (drv->restore) {
+ ret = drv->restore(dev);
+ if (ret)
+ goto err;
+ }
+
+ /* If restore didn't do it, mark device DRIVER_OK ourselves. */
+ if (!(dev->config->get_status(dev) & VIRTIO_CONFIG_S_DRIVER_OK))
+ virtio_device_ready(dev);
+
+ virtio_config_core_enable(dev);
+
+ return 0;
+
+err:
+ virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
+ return ret;
}
EXPORT_SYMBOL_GPL(virtio_device_restore);
#endif
@@ -698,11 +698,30 @@ EXPORT_SYMBOL_GPL(virtio_device_reset_prepare);
int virtio_device_reset_done(struct virtio_device *dev)
{
struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+ int ret;
if (!drv || !drv->reset_done)
return -EOPNOTSUPP;
- return virtio_device_restore_priv(dev, false);
+ ret = virtio_device_reinit(dev);
+ if (ret)
+ goto err;
+
+ ret = drv->reset_done(dev);
+ if (ret)
+ goto err;
+
+ /* If reset_done didn't do it, mark device DRIVER_OK ourselves. */
+ if (!(dev->config->get_status(dev) & VIRTIO_CONFIG_S_DRIVER_OK))
+ virtio_device_ready(dev);
+
+ virtio_config_core_enable(dev);
+
+ return 0;
+
+err:
+ virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
+ return ret;
}
EXPORT_SYMBOL_GPL(virtio_device_reset_done);
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v9 3/5] virtio_ring: export virtqueue_reinit_vring() for noirq restore
From: Sungho Bae @ 2026-05-16 1:57 UTC (permalink / raw)
To: mst, jasowang
Cc: xuanzhuo, eperezma, stephan.gerhold, virtualization, linux-kernel,
Sungho Bae
In-Reply-To: <20260516015756.20948-1-baver.bae@gmail.com>
From: Sungho Bae <baver.bae@lge.com>
After a device reset in noirq context the existing vrings must be
re-initialized without any memory allocation, because GFP_KERNEL is
not available.
The internal helpers virtqueue_reset_split() and
virtqueue_reset_packed() already reset vring indices and descriptor
state in place. Add a thin exported wrapper, virtqueue_reinit_vring(),
that dispatches to the appropriate helper based on the ring layout.
This will be used by a subsequent patch that adds noirq system-sleep
PM callbacks for virtio-mmio.
Signed-off-by: Sungho Bae <baver.bae@lge.com>
---
drivers/virtio/virtio_ring.c | 58 ++++++++++++++++++++++++++++++++++++
include/linux/virtio_ring.h | 3 ++
2 files changed, 61 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index fbca7ce1c6bf..d3339b820f6b 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -506,6 +506,15 @@ static void virtqueue_init(struct vring_virtqueue *vq, u32 num)
vq->event_triggered = false;
vq->num_added = 0;
+ /*
+ * Keep IN_ORDER state aligned with a freshly initialized/reset queue.
+ * For packed IN_ORDER, free_head is unused but harmlessly reset.
+ */
+ if (virtqueue_is_in_order(vq)) {
+ vq->free_head = 0;
+ vq->batch_last.id = UINT_MAX;
+ }
+
#ifdef DEBUG
vq->in_use = false;
vq->last_add_time_valid = false;
@@ -3936,5 +3945,54 @@ void virtqueue_map_sync_single_range_for_device(const struct virtqueue *_vq,
}
EXPORT_SYMBOL_GPL(virtqueue_map_sync_single_range_for_device);
+/**
+ * virtqueue_reinit_vring - reinitialize vring state without reallocation
+ * @_vq: the virtqueue
+ *
+ * Reset the avail/used indices and descriptor state of an existing
+ * virtqueue so it can be reused after a device reset. No memory is
+ * allocated or freed, making this safe for use in noirq context.
+ *
+ * Preconditions for callers:
+ * 1) The vq must be fully quiesced (no concurrent add/get/kick/IRQ callback).
+ * 2) Transport/device side must already have stopped/reset this queue.
+ * 3) All in-flight buffers must already be completed or detached.
+ *
+ * If called with outstanding descriptors, free-list state can be corrupted:
+ * num_free is restored to full capacity while desc_extra next-chain/free_head
+ * may still represent a partially consumed list.
+ *
+ * Return:
+ * 0 on success, or -EBUSY if preconditions are not met.
+ */
+int virtqueue_reinit_vring(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ unsigned int num = virtqueue_is_packed(vq) ?
+ vq->packed.vring.num : vq->split.vring.num;
+
+ /* All in-flight descriptors must be completed or detached */
+ if (WARN_ON(vq->vq.num_free != num))
+ return -EBUSY;
+
+ if (virtqueue_is_packed(vq)) {
+ virtqueue_reset_packed(vq);
+ } else {
+ /*
+ * Split queue shadow index should match the visible avail
+ * index when the queue is fully quiesced.
+ */
+ if (WARN_ON(vq->split.avail_idx_shadow !=
+ virtio16_to_cpu(vq->vq.vdev,
+ vq->split.vring.avail->idx)))
+ return -EBUSY;
+
+ virtqueue_reset_split(vq);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(virtqueue_reinit_vring);
+
MODULE_DESCRIPTION("Virtio ring implementation");
MODULE_LICENSE("GPL");
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index c97a12c1cda3..8b421fef4fef 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -118,6 +118,9 @@ void vring_del_virtqueue(struct virtqueue *vq);
/* Filter out transport-specific feature bits. */
void vring_transport_features(struct virtio_device *vdev);
+/* Reinitialize a virtqueue without reallocation (safe in noirq context) */
+int virtqueue_reinit_vring(struct virtqueue *_vq);
+
irqreturn_t vring_interrupt(int irq, void *_vq);
u32 vring_notification_data(struct virtqueue *_vq);
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v9 4/5] virtio: add noirq system sleep PM infrastructure
From: Sungho Bae @ 2026-05-16 1:57 UTC (permalink / raw)
To: mst, jasowang
Cc: xuanzhuo, eperezma, stephan.gerhold, virtualization, linux-kernel,
Sungho Bae
In-Reply-To: <20260516015756.20948-1-baver.bae@gmail.com>
From: Sungho Bae <baver.bae@lge.com>
Some virtio-mmio devices, such as virtio-clock or virtio-regulator,
must become operational before the regular PM restore callback runs
because other devices may depend on them.
Add the core infrastructure needed to support noirq system-sleep PM
callbacks for virtio transports:
- virtio_add_status_noirq(): status helper without might_sleep().
- virtio_features_ok_noirq(): feature negotiation without might_sleep().
- virtio_reset_device_noirq(): device reset that skips
virtio_synchronize_cbs() (IRQ handlers are already quiesced in the
noirq phase).
- virtio_device_reinit_noirq(): full noirq bring-up sequence using the
above helpers.
- virtio_config_core_enable_noirq(): config enable with irqsave
locking.
- virtio_device_ready_noirq(): marks DRIVER_OK without
virtio_synchronize_cbs().
Not all transports can safely call reset, get_status, set_status, or
finalize_features during the noirq phase: transports like virtio-ccw
issue channel commands and wait for a completion interrupt, which will
never be delivered because device interrupts are masked at the interrupt
controller during noirq suspend/resume. To address this, introduce a
boolean field noirq_safe in struct virtio_config_ops. Transports that
implement the above operations via simple MMIO reads/writes (e.g.
virtio-mmio) set this flag; all others leave it at the default false.
The noirq helpers assert noirq_safe via WARN_ON at runtime.
virtio_device_freeze() enforces the contract at freeze time, returning
-EOPNOTSUPP early if the driver provides restore_noirq but the transport
does not meet the requirements, to prevent a deadlock on resume.
virtio_device_freeze_noirq() and virtio_device_restore_noirq() perform
secondary checks as safety nets.
Add freeze_noirq/restore_noirq callbacks to struct virtio_driver and
provide matching helper wrappers in the virtio core:
- virtio_device_freeze_noirq(): validates noirq_safe and reset_vqs
requirements, then forwards to drv->freeze_noirq().
- virtio_device_restore_noirq(): guards against unsafe transports,
runs the noirq bring-up sequence, resets existing vrings via the
new config_ops->reset_vqs() hook, then calls drv->restore_noirq().
Modify virtio_device_restore() so that when a driver provides
restore_noirq, the normal-phase restore skips the re-initialization
that was already done in the noirq phase.
Signed-off-by: Sungho Bae <baver.bae@lge.com>
---
drivers/virtio/virtio.c | 305 +++++++++++++++++++++++++++++++++-
include/linux/virtio.h | 42 +++++
include/linux/virtio_config.h | 39 +++++
3 files changed, 382 insertions(+), 4 deletions(-)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 98f1875f8df1..97a3ed5c2985 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -193,6 +193,17 @@ static void virtio_config_core_enable(struct virtio_device *dev)
spin_unlock_irq(&dev->config_lock);
}
+static void virtio_config_core_enable_noirq(struct virtio_device *dev)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&dev->config_lock, flags);
+ dev->config_core_enabled = true;
+ if (dev->config_change_pending)
+ __virtio_config_changed(dev);
+ spin_unlock_irqrestore(&dev->config_lock, flags);
+}
+
void virtio_add_status(struct virtio_device *dev, unsigned int status)
{
might_sleep();
@@ -200,6 +211,21 @@ void virtio_add_status(struct virtio_device *dev, unsigned int status)
}
EXPORT_SYMBOL_GPL(virtio_add_status);
+/*
+ * Same as virtio_add_status() but without the might_sleep() assertion,
+ * so it is safe to call from noirq context.
+ *
+ * Requires the transport to have set config_ops->noirq_safe, which declares
+ * that reset, get_status, and set_status do not wait for a completion
+ * interrupt and are therefore safe during the noirq PM phase.
+ */
+void virtio_add_status_noirq(struct virtio_device *dev, unsigned int status)
+{
+ WARN_ON(!dev->config->noirq_safe);
+ dev->config->set_status(dev, dev->config->get_status(dev) | status);
+}
+EXPORT_SYMBOL_GPL(virtio_add_status_noirq);
+
/* Do some validation, then set FEATURES_OK */
static int virtio_features_ok(struct virtio_device *dev)
{
@@ -234,6 +260,32 @@ static int virtio_features_ok(struct virtio_device *dev)
return 0;
}
+/* noirq-safe variant: no might_sleep(), uses virtio_add_status_noirq() */
+static int virtio_features_ok_noirq(struct virtio_device *dev)
+{
+ unsigned int status;
+
+ /*
+ * Skip virtio_check_mem_acc_cb() here: it may sleep (e.g. Xen's
+ * xen_virtio_restricted_mem_acc() calls devm_kzalloc with GFP_KERNEL).
+ * The check was already performed during probe in virtio_features_ok();
+ * features cannot change across suspend/resume so the constraint is
+ * still satisfied.
+ */
+
+ if (!virtio_has_feature(dev, VIRTIO_F_VERSION_1))
+ return 0;
+
+ virtio_add_status_noirq(dev, VIRTIO_CONFIG_S_FEATURES_OK);
+ status = dev->config->get_status(dev);
+ if (!(status & VIRTIO_CONFIG_S_FEATURES_OK)) {
+ dev_err(&dev->dev, "virtio: device refuses features: %x\n",
+ status);
+ return -ENODEV;
+ }
+ return 0;
+}
+
/**
* virtio_reset_device - quiesce device for removal
* @dev: the device to reset
@@ -267,6 +319,28 @@ void virtio_reset_device(struct virtio_device *dev)
}
EXPORT_SYMBOL_GPL(virtio_reset_device);
+/**
+ * virtio_reset_device_noirq - noirq-safe variant of virtio_reset_device()
+ * @dev: the device to reset
+ *
+ * Requires the transport to have set config_ops->noirq_safe.
+ */
+void virtio_reset_device_noirq(struct virtio_device *dev)
+{
+ WARN_ON(!dev->config->noirq_safe);
+
+#ifdef CONFIG_VIRTIO_HARDEN_NOTIFICATION
+ /*
+ * The noirq stage runs with device IRQ handlers disabled, so
+ * virtio_synchronize_cbs() must not be called here.
+ */
+ virtio_break_device(dev);
+#endif
+
+ dev->config->reset(dev);
+}
+EXPORT_SYMBOL_GPL(virtio_reset_device_noirq);
+
static int virtio_dev_probe(struct device *_d)
{
int err, i;
@@ -539,6 +613,7 @@ int register_virtio_device(struct virtio_device *dev)
dev->config_driver_disabled = false;
dev->config_core_enabled = false;
dev->config_change_pending = false;
+ dev->noirq_state = VIRTIO_NOIRQ_NONE;
INIT_LIST_HEAD(&dev->vqs);
spin_lock_init(&dev->vqs_list_lock);
@@ -618,7 +693,63 @@ static int virtio_device_reinit(struct virtio_device *dev)
return virtio_features_ok(dev);
}
+/*
+ * noirq-safe variant of virtio_device_reinit().
+ *
+ * Requires the transport to declare config_ops->noirq_safe, which means
+ * reset, get_status, set_status, and finalize_features are safe to call
+ * during the noirq PM phase.
+ */
+static int virtio_device_reinit_noirq(struct virtio_device *dev)
+{
+ struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+ int ret;
+
+ /*
+ * We always start by resetting the device, in case a previous
+ * driver messed it up.
+ */
+ virtio_reset_device_noirq(dev);
+
+ /* Acknowledge that we've seen the device. */
+ virtio_add_status_noirq(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
+
+ /*
+ * Maybe driver failed before freeze.
+ * Restore the failed status, for debugging.
+ */
+ if (dev->failed)
+ virtio_add_status_noirq(dev, VIRTIO_CONFIG_S_FAILED);
+
+ if (!drv)
+ return 0;
+
+ /* We have a driver! */
+ virtio_add_status_noirq(dev, VIRTIO_CONFIG_S_DRIVER);
+
+ ret = dev->config->finalize_features(dev);
+ if (ret)
+ return ret;
+
+ return virtio_features_ok_noirq(dev);
+}
+
#ifdef CONFIG_PM_SLEEP
+static inline bool virtio_has_valid_pm_cbs(struct virtio_driver *drv)
+{
+ /* Each callback pair must be fully implemented or fully absent. */
+ bool has_freeze = drv->freeze;
+ bool has_restore = drv->restore;
+ bool has_freeze_noirq = drv->freeze_noirq;
+ bool has_restore_noirq = drv->restore_noirq;
+
+ if (has_freeze != has_restore)
+ return false;
+ if (has_freeze_noirq != has_restore_noirq)
+ return false;
+ return true;
+}
+
int virtio_device_freeze(struct virtio_device *dev)
{
struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
@@ -627,6 +758,34 @@ int virtio_device_freeze(struct virtio_device *dev)
virtio_config_core_disable(dev);
dev->failed = dev->config->get_status(dev) & VIRTIO_CONFIG_S_FAILED;
+ dev->noirq_state = VIRTIO_NOIRQ_NONE;
+
+ /*
+ * freeze_noirq and restore_noirq must be implemented as a pair.
+ * freeze_noirq performs full device teardown that only
+ * restore_noirq knows how to undo, and restore_noirq assumes
+ * freeze_noirq prepared the device for re-initialization.
+ */
+ if (drv && !virtio_has_valid_pm_cbs(drv)) {
+ dev_warn(&dev->dev,
+ "freeze/restore and freeze_noirq/restore_noirq must each be paired\n");
+ virtio_config_core_enable(dev);
+ return -EINVAL;
+ }
+
+ /*
+ * If the driver provides noirq callbacks, verify that the
+ * transport supports noirq PM. The driver's freeze_noirq or
+ * restore_noirq may call transport ops (reset, get_status,
+ * set_status) that could wait for an interrupt that will never
+ * arrive if the transport is not noirq-safe.
+ */
+ if (drv && drv->restore_noirq && !dev->config->noirq_safe) {
+ dev_warn(&dev->dev,
+ "transport does not support noirq PM\n");
+ virtio_config_core_enable(dev);
+ return -EOPNOTSUPP;
+ }
if (drv && drv->freeze) {
ret = drv->freeze(dev);
@@ -645,12 +804,42 @@ int virtio_device_restore(struct virtio_device *dev)
struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
int ret;
- ret = virtio_device_reinit(dev);
- if (ret)
+ /*
+ * If the driver implements restore_noirq and the noirq phase was
+ * actually entered (freeze_noirq ran), but restore_noirq did not
+ * complete successfully, the noirq phase must have failed. PM core
+ * may continue later resume phases for global recovery, but virtio
+ * does not use the normal restore path as an implicit same-device
+ * fallback.
+ */
+ if (drv && drv->restore_noirq &&
+ dev->noirq_state == VIRTIO_NOIRQ_ENTERED) {
+ ret = -EIO;
goto err;
+ }
- if (!drv)
- return 0;
+ /*
+ * Re-initialization is needed only for drivers that do not
+ * implement restore_noirq. When restore_noirq exists, either:
+ * - NOIRQ_NONE: noirq phase was never entered, so no noirq-specific
+ * teardown occurred and the device is still live.
+ * - NOIRQ_RESTORED: noirq phase already performed reinit.
+ * (NOIRQ_ENTERED is caught above as -EIO.)
+ *
+ * Note: when a driver implements restore_noirq, freeze() must NOT
+ * perform full device teardown (e.g., must not destroy virtqueues).
+ * Full teardown is deferred to freeze_noirq(). If suspend is aborted
+ * before the noirq phase, restore() is called to undo only the
+ * partial quiesce performed by freeze(), with the device still live
+ * and in DRIVER_OK state.
+ */
+ if (!drv || !drv->restore_noirq) {
+ ret = virtio_device_reinit(dev);
+ if (ret)
+ goto err;
+ if (!drv)
+ return 0;
+ }
if (drv->restore) {
ret = drv->restore(dev);
@@ -671,6 +860,114 @@ int virtio_device_restore(struct virtio_device *dev)
return ret;
}
EXPORT_SYMBOL_GPL(virtio_device_restore);
+
+int virtio_device_freeze_noirq(struct virtio_device *dev)
+{
+ struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+
+ if (!drv)
+ return 0;
+
+ /*
+ * Pairing is validated in virtio_device_freeze(); since both
+ * callbacks must co-exist, checking one implies the other.
+ *
+ * Verify the transport supports noirq PM. This should normally
+ * have been caught at freeze time, but guard here as well.
+ */
+ if (drv->freeze_noirq && !dev->config->noirq_safe) {
+ dev_warn(&dev->dev,
+ "transport does not support noirq PM\n");
+ return -EOPNOTSUPP;
+ }
+
+ /*
+ * If the driver provides noirq callbacks and has active vqs,
+ * the transport must support reset_vqs to restore them.
+ * Fail here so the PM core can abort the transition gracefully,
+ * rather than hitting -EOPNOTSUPP on resume.
+ */
+ if (drv->freeze_noirq && !list_empty(&dev->vqs) &&
+ !dev->config->reset_vqs) {
+ dev_warn(&dev->dev,
+ "transport does not support noirq PM restore with active vqs (missing reset_vqs)\n");
+ return -EOPNOTSUPP;
+ }
+
+ /*
+ * Invoke the driver's freeze_noirq callback and mark noirq
+ * phase entered on success. Pairing is enforced in
+ * virtio_device_freeze(), so restore_noirq also exists.
+ *
+ * If freeze_noirq fails, the driver must have rolled back to
+ * the pre-call state (per kernel PM convention), so
+ * noirq_state remains NONE to allow restore() to proceed.
+ */
+ if (drv->freeze_noirq) {
+ int ret = drv->freeze_noirq(dev);
+
+ if (!ret)
+ dev->noirq_state = VIRTIO_NOIRQ_ENTERED;
+
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(virtio_device_freeze_noirq);
+
+int virtio_device_restore_noirq(struct virtio_device *dev)
+{
+ struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+ int ret;
+
+ if (!drv || !drv->restore_noirq)
+ return 0;
+
+ /*
+ * All transport ops called below (reset, get_status, set_status) must
+ * be noirq-safe. Return early if not - this should normally have
+ * been caught at freeze_noirq time.
+ */
+ if (!dev->config->noirq_safe) {
+ dev_warn(&dev->dev,
+ "transport does not support noirq PM; skipping restore\n");
+ return -EOPNOTSUPP;
+ }
+
+ ret = virtio_device_reinit_noirq(dev);
+ if (ret)
+ goto err;
+
+ if (!list_empty(&dev->vqs)) {
+ if (!dev->config->reset_vqs) {
+ ret = -EOPNOTSUPP;
+ goto err;
+ }
+
+ ret = dev->config->reset_vqs(dev);
+ if (ret)
+ goto err;
+ }
+
+ ret = drv->restore_noirq(dev);
+ if (ret)
+ goto err;
+
+ /* Mark that noirq restore has completed successfully. */
+ dev->noirq_state = VIRTIO_NOIRQ_RESTORED;
+
+ /* If restore_noirq set DRIVER_OK, enable config now. */
+ if (dev->config->get_status(dev) & VIRTIO_CONFIG_S_DRIVER_OK)
+ virtio_config_core_enable_noirq(dev);
+
+ return 0;
+
+err:
+ virtio_add_status_noirq(dev, VIRTIO_CONFIG_S_FAILED);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(virtio_device_restore_noirq);
#endif
int virtio_device_reset_prepare(struct virtio_device *dev)
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 3bbc4cb6a672..937bc3c56bb8 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -143,6 +143,18 @@ struct virtio_admin_cmd {
int ret;
};
+/**
+ * enum virtio_noirq_state - tracks noirq PM phase progress
+ * @VIRTIO_NOIRQ_NONE: noirq phase was not entered (only freeze ran)
+ * @VIRTIO_NOIRQ_ENTERED: freeze_noirq ran; restore_noirq is expected
+ * @VIRTIO_NOIRQ_RESTORED: restore_noirq completed successfully
+ */
+enum virtio_noirq_state {
+ VIRTIO_NOIRQ_NONE,
+ VIRTIO_NOIRQ_ENTERED,
+ VIRTIO_NOIRQ_RESTORED,
+};
+
/**
* struct virtio_device - representation of a device using virtio
* @index: unique position on the virtio bus
@@ -151,6 +163,7 @@ struct virtio_admin_cmd {
* @config_driver_disabled: configuration change reporting disabled by
* a driver
* @config_change_pending: configuration change reported while disabled
+ * @noirq_state: tracks noirq PM phase progress for restore coordination
* @config_lock: protects configuration change reporting
* @vqs_list_lock: protects @vqs.
* @dev: underlying device.
@@ -171,6 +184,7 @@ struct virtio_device {
bool config_core_enabled;
bool config_driver_disabled;
bool config_change_pending;
+ enum virtio_noirq_state noirq_state;
spinlock_t config_lock;
spinlock_t vqs_list_lock;
struct device dev;
@@ -209,8 +223,12 @@ void virtio_config_driver_enable(struct virtio_device *dev);
#ifdef CONFIG_PM_SLEEP
int virtio_device_freeze(struct virtio_device *dev);
int virtio_device_restore(struct virtio_device *dev);
+int virtio_device_freeze_noirq(struct virtio_device *dev);
+int virtio_device_restore_noirq(struct virtio_device *dev);
#endif
void virtio_reset_device(struct virtio_device *dev);
+void virtio_reset_device_noirq(struct virtio_device *dev);
+void virtio_add_status_noirq(struct virtio_device *dev, unsigned int status);
int virtio_device_reset_prepare(struct virtio_device *dev);
int virtio_device_reset_done(struct virtio_device *dev);
@@ -237,6 +255,28 @@ size_t virtio_max_dma_size(const struct virtio_device *vdev);
* changes; may be called in interrupt context.
* @freeze: optional function to call during suspend/hibernation.
* @restore: optional function to call on resume.
+ * When @restore_noirq is not implemented, core resets and reinitializes
+ * the device before calling this. When @restore_noirq succeeded, core
+ * skips reinitialization; drivers should avoid calling virtio_device_ready()
+ * if DRIVER_OK was already set in the noirq phase.
+ * When @restore_noirq failed, this callback is not invoked for same-device
+ * recovery; the saved noirq error is propagated instead.
+ * When the noirq phase was entirely skipped (e.g. suspend aborted before
+ * suspend_noirq), core skips reinitialization for drivers that implement
+ * @restore_noirq and calls @restore (if provided) to undo the freeze()
+ * quiesce. Drivers without @restore_noirq follow the normal reinit +
+ * restore path.
+ * @freeze_noirq: optional function to call during noirq suspend/hibernation.
+ * @restore_noirq: optional function to call on noirq resume.
+ * If this callback fails, PM core may still continue later resume phases
+ * for global system recovery. Virtio does not treat @restore as an
+ * implicit same-device fallback for @restore_noirq failure; drivers should
+ * only implement @restore_noirq when noirq resume is their required
+ * recovery point.
+ * A noirq restore failure is detected by the normal restore path
+ * (noirq_state == VIRTIO_NOIRQ_ENTERED, meaning freeze_noirq ran but
+ * restore_noirq did not complete) and returns -EIO instead of attempting
+ * same-device recovery.
* @reset_prepare: optional function to call when a transport specific reset
* occurs.
* @reset_done: optional function to call after transport specific reset
@@ -258,6 +298,8 @@ struct virtio_driver {
void (*config_changed)(struct virtio_device *dev);
int (*freeze)(struct virtio_device *dev);
int (*restore)(struct virtio_device *dev);
+ int (*freeze_noirq)(struct virtio_device *dev);
+ int (*restore_noirq)(struct virtio_device *dev);
int (*reset_prepare)(struct virtio_device *dev);
int (*reset_done)(struct virtio_device *dev);
void (*shutdown)(struct virtio_device *dev);
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 69f84ea85d71..0110b091f634 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -70,6 +70,9 @@ struct virtqueue_info {
* vqs_info: array of virtqueue info structures
* Returns 0 on success or error status
* @del_vqs: free virtqueues found by find_vqs().
+ * @reset_vqs: reinitialize existing virtqueues without allocating or
+ * freeing them (optional). Used during noirq restore.
+ * Returns 0 on success or error status.
* @synchronize_cbs: synchronize with the virtqueue callbacks (optional)
* The function guarantees that all memory operations on the
* queue before it are visible to the vring_interrupt() that is
@@ -108,6 +111,14 @@ struct virtqueue_info {
* Returns 0 on success or error status
* If disable_vq_and_reset is set, then enable_vq_after_reset must also be
* set.
+ * @noirq_safe: set to true if @reset, @get_status, @set_status, and
+ * @finalize_features are safe to call during the noirq phase of system
+ * suspend/resume. Transports that implement these operations via simple
+ * MMIO reads/writes (e.g. virtio-mmio) can set this flag. Transports
+ * that issue channel commands and wait for a completion interrupt (e.g.
+ * virtio-ccw) must NOT set it, because device interrupts are masked at
+ * the interrupt controller during the noirq phase, which would cause the
+ * wait to hang.
*/
struct virtio_config_ops {
void (*get)(struct virtio_device *vdev, unsigned offset,
@@ -123,6 +134,7 @@ struct virtio_config_ops {
struct virtqueue_info vqs_info[],
struct irq_affinity *desc);
void (*del_vqs)(struct virtio_device *);
+ int (*reset_vqs)(struct virtio_device *vdev);
void (*synchronize_cbs)(struct virtio_device *);
u64 (*get_features)(struct virtio_device *vdev);
void (*get_extended_features)(struct virtio_device *vdev,
@@ -137,6 +149,7 @@ struct virtio_config_ops {
struct virtio_shm_region *region, u8 id);
int (*disable_vq_and_reset)(struct virtqueue *vq);
int (*enable_vq_after_reset)(struct virtqueue *vq);
+ bool noirq_safe;
};
/**
@@ -371,6 +384,32 @@ void virtio_device_ready(struct virtio_device *dev)
dev->config->set_status(dev, status | VIRTIO_CONFIG_S_DRIVER_OK);
}
+/**
+ * virtio_device_ready_noirq - noirq-safe variant of virtio_device_ready()
+ * @dev: the virtio device
+ *
+ * Requires the transport to have set config_ops->noirq_safe, which declares
+ * that get_status and set_status do not wait for a completion interrupt.
+ */
+static inline
+void virtio_device_ready_noirq(struct virtio_device *dev)
+{
+ unsigned int status = dev->config->get_status(dev);
+
+ WARN_ON(!dev->config->noirq_safe);
+ WARN_ON(status & VIRTIO_CONFIG_S_DRIVER_OK);
+
+#ifdef CONFIG_VIRTIO_HARDEN_NOTIFICATION
+ /*
+ * The noirq stage runs with device IRQ handlers disabled, so
+ * virtio_synchronize_cbs() must not be called here.
+ */
+ __virtio_unbreak_device(dev);
+#endif
+
+ dev->config->set_status(dev, status | VIRTIO_CONFIG_S_DRIVER_OK);
+}
+
static inline
const char *virtio_bus_name(struct virtio_device *vdev)
{
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v9 5/5] virtio-mmio: wire up noirq system sleep PM callbacks
From: Sungho Bae @ 2026-05-16 1:57 UTC (permalink / raw)
To: mst, jasowang
Cc: xuanzhuo, eperezma, stephan.gerhold, virtualization, linux-kernel,
Sungho Bae
In-Reply-To: <20260516015756.20948-1-baver.bae@gmail.com>
From: Sungho Bae <baver.bae@lge.com>
Add noirq system-sleep PM support to the virtio-mmio transport.
This change wires noirq freeze/restore callbacks into virtio-mmio and
hooks queue reset/reactivation into the transport config ops so virtqueues
can be reinitialized and reused across suspend/resume.
This enables virtio-mmio based devices to participate safely in the noirq
PM phase, which is required for early-restore users.
Signed-off-by: Sungho Bae <baver.bae@lge.com>
---
drivers/virtio/virtio_mmio.c | 131 ++++++++++++++++++++++++-----------
1 file changed, 92 insertions(+), 39 deletions(-)
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index daa65b269a36..fbad0ee8312f 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -346,6 +346,77 @@ static void vm_del_vqs(struct virtio_device *vdev)
free_irq(platform_get_irq(vm_dev->pdev, 0), vm_dev);
}
+static int vm_active_vq(struct virtio_device *vdev, struct virtqueue *vq)
+{
+ struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
+ int q_num = virtqueue_get_vring_size(vq);
+
+ writel(q_num, vm_dev->base + VIRTIO_MMIO_QUEUE_NUM);
+ if (vm_dev->version == 1) {
+ u64 q_pfn = virtqueue_get_desc_addr(vq) >> PAGE_SHIFT;
+
+ /*
+ * virtio-mmio v1 uses a 32bit QUEUE PFN. If we have something
+ * that doesn't fit in 32bit, fail the setup rather than
+ * pretending to be successful.
+ */
+ if (q_pfn >> 32) {
+ dev_err(&vdev->dev,
+ "platform bug: legacy virtio-mmio must not be used with RAM above 0x%llxGB\n",
+ 0x1ULL << (32 + PAGE_SHIFT - 30));
+ return -E2BIG;
+ }
+
+ writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_QUEUE_ALIGN);
+ writel(q_pfn, vm_dev->base + VIRTIO_MMIO_QUEUE_PFN);
+ } else {
+ u64 addr;
+
+ addr = virtqueue_get_desc_addr(vq);
+ writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_DESC_LOW);
+ writel((u32)(addr >> 32),
+ vm_dev->base + VIRTIO_MMIO_QUEUE_DESC_HIGH);
+
+ addr = virtqueue_get_avail_addr(vq);
+ writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_AVAIL_LOW);
+ writel((u32)(addr >> 32),
+ vm_dev->base + VIRTIO_MMIO_QUEUE_AVAIL_HIGH);
+
+ addr = virtqueue_get_used_addr(vq);
+ writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_USED_LOW);
+ writel((u32)(addr >> 32),
+ vm_dev->base + VIRTIO_MMIO_QUEUE_USED_HIGH);
+
+ writel(1, vm_dev->base + VIRTIO_MMIO_QUEUE_READY);
+ }
+
+ return 0;
+}
+
+static int vm_reset_vqs(struct virtio_device *vdev)
+{
+ struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
+ struct virtqueue *vq;
+ int err;
+
+ virtio_device_for_each_vq(vdev, vq) {
+ /* Re-initialize vring state */
+ err = virtqueue_reinit_vring(vq);
+ if (err < 0)
+ return err;
+
+ /* Select the queue we're interested in */
+ writel(vq->index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL);
+
+ /* Activate the queue */
+ err = vm_active_vq(vdev, vq);
+ if (err < 0)
+ return err;
+ }
+
+ return 0;
+}
+
static void vm_synchronize_cbs(struct virtio_device *vdev)
{
struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
@@ -398,45 +469,9 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in
vq->num_max = num;
/* Activate the queue */
- writel(virtqueue_get_vring_size(vq), vm_dev->base + VIRTIO_MMIO_QUEUE_NUM);
- if (vm_dev->version == 1) {
- u64 q_pfn = virtqueue_get_desc_addr(vq) >> PAGE_SHIFT;
-
- /*
- * virtio-mmio v1 uses a 32bit QUEUE PFN. If we have something
- * that doesn't fit in 32bit, fail the setup rather than
- * pretending to be successful.
- */
- if (q_pfn >> 32) {
- dev_err(&vdev->dev,
- "platform bug: legacy virtio-mmio must not be used with RAM above 0x%llxGB\n",
- 0x1ULL << (32 + PAGE_SHIFT - 30));
- err = -E2BIG;
- goto error_bad_pfn;
- }
-
- writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_QUEUE_ALIGN);
- writel(q_pfn, vm_dev->base + VIRTIO_MMIO_QUEUE_PFN);
- } else {
- u64 addr;
-
- addr = virtqueue_get_desc_addr(vq);
- writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_DESC_LOW);
- writel((u32)(addr >> 32),
- vm_dev->base + VIRTIO_MMIO_QUEUE_DESC_HIGH);
-
- addr = virtqueue_get_avail_addr(vq);
- writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_AVAIL_LOW);
- writel((u32)(addr >> 32),
- vm_dev->base + VIRTIO_MMIO_QUEUE_AVAIL_HIGH);
-
- addr = virtqueue_get_used_addr(vq);
- writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_USED_LOW);
- writel((u32)(addr >> 32),
- vm_dev->base + VIRTIO_MMIO_QUEUE_USED_HIGH);
-
- writel(1, vm_dev->base + VIRTIO_MMIO_QUEUE_READY);
- }
+ err = vm_active_vq(vdev, vq);
+ if (err < 0)
+ goto error_bad_pfn;
return vq;
@@ -538,11 +573,13 @@ static const struct virtio_config_ops virtio_mmio_config_ops = {
.reset = vm_reset,
.find_vqs = vm_find_vqs,
.del_vqs = vm_del_vqs,
+ .reset_vqs = vm_reset_vqs,
.get_features = vm_get_features,
.finalize_features = vm_finalize_features,
.bus_name = vm_bus_name,
.get_shm_region = vm_get_shm_region,
.synchronize_cbs = vm_synchronize_cbs,
+ .noirq_safe = true,
};
#ifdef CONFIG_PM_SLEEP
@@ -560,8 +597,24 @@ static int virtio_mmio_restore(struct device *dev)
return virtio_device_restore(&vm_dev->vdev);
}
+static int virtio_mmio_freeze_noirq(struct device *dev)
+{
+ struct virtio_mmio_device *vm_dev = dev_get_drvdata(dev);
+
+ return virtio_device_freeze_noirq(&vm_dev->vdev);
+}
+
+static int virtio_mmio_restore_noirq(struct device *dev)
+{
+ struct virtio_mmio_device *vm_dev = dev_get_drvdata(dev);
+
+ return virtio_device_restore_noirq(&vm_dev->vdev);
+}
+
static const struct dev_pm_ops virtio_mmio_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(virtio_mmio_freeze, virtio_mmio_restore)
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(virtio_mmio_freeze_noirq,
+ virtio_mmio_restore_noirq)
};
#endif
--
2.34.1
^ permalink raw reply related
* [PATCH net] vsock: keep poll shutdown state consistent
From: Ziyu Zhang @ 2026-05-16 3:47 UTC (permalink / raw)
To: Stefano Garzarella, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Andy King, George Zhang, Dmitry Torokhov,
virtualization, netdev, linux-kernel, baijiaju1990, r33s3n6,
gality369, zhenghaoran154, hanguidong02, zzzccc427, Ziyu Zhang
vsock_poll() reads vsk->peer_shutdown before taking the socket
lock to set EPOLLHUP and EPOLLRDHUP, then reads it again under the
lock to report EOF readability. A shutdown packet can update
peer_shutdown while poll is waiting for the lock, so one poll invocation
can report EPOLLIN without the corresponding HUP/RDHUP bits.
Keep non-connectible sockets on a single lockless READ_ONCE()
snapshot. For connectible sockets, defer shutdown-derived poll bits
until after lock_sock() and use one READ_ONCE() snapshot for both EOF
readability and HUP/RDHUP. This preserves shutdowns that arrive before
the lock is acquired and keeps all peer-shutdown-derived bits consistent
for a poll pass.
Fixes: d021c344051a ("VSOCK: Introduce VM Sockets")
Signed-off-by: Ziyu Zhang <ziyuzhang201@gmail.com>
---
net/vmw_vsock/af_vsock.c | 40 ++++++++++++++++++++++++++--------------
1 file changed, 26 insertions(+), 14 deletions(-)
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index adcba1b7b..bed42347b 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -1122,6 +1122,25 @@ static int vsock_shutdown(struct socket *sock, int mode)
return err;
}
+static __poll_t vsock_poll_shutdown(struct sock *sk, u32 peer_shutdown)
+{
+ __poll_t mask = 0;
+
+ /* INET sockets treat local write shutdown and peer write shutdown as a
+ * case of EPOLLHUP set.
+ */
+ if (sk->sk_shutdown == SHUTDOWN_MASK ||
+ ((sk->sk_shutdown & SEND_SHUTDOWN) &&
+ (peer_shutdown & SEND_SHUTDOWN)))
+ mask |= EPOLLHUP;
+
+ if (sk->sk_shutdown & RCV_SHUTDOWN ||
+ peer_shutdown & SEND_SHUTDOWN)
+ mask |= EPOLLRDHUP;
+
+ return mask;
+}
+
static __poll_t vsock_poll(struct file *file, struct socket *sock,
poll_table *wait)
{
@@ -1139,19 +1158,9 @@ static __poll_t vsock_poll(struct file *file, struct socket *sock,
/* Signify that there has been an error on this socket. */
mask |= EPOLLERR;
- /* INET sockets treat local write shutdown and peer write shutdown as a
- * case of EPOLLHUP set.
- */
- if ((sk->sk_shutdown == SHUTDOWN_MASK) ||
- ((sk->sk_shutdown & SEND_SHUTDOWN) &&
- (vsk->peer_shutdown & SEND_SHUTDOWN))) {
- mask |= EPOLLHUP;
- }
-
- if (sk->sk_shutdown & RCV_SHUTDOWN ||
- vsk->peer_shutdown & SEND_SHUTDOWN) {
- mask |= EPOLLRDHUP;
- }
+ if (!sock_type_connectible(sk->sk_type))
+ mask |= vsock_poll_shutdown(sk,
+ READ_ONCE(vsk->peer_shutdown));
if (sk_is_readable(sk))
mask |= EPOLLIN | EPOLLRDNORM;
@@ -1171,6 +1180,7 @@ static __poll_t vsock_poll(struct file *file, struct socket *sock,
} else if (sock_type_connectible(sk->sk_type)) {
const struct vsock_transport *transport;
+ u32 peer_shutdown;
lock_sock(sk);
@@ -1203,10 +1213,12 @@ static __poll_t vsock_poll(struct file *file, struct socket *sock,
* terminated should also be considered read, and we check the
* shutdown flag for that.
*/
+ peer_shutdown = READ_ONCE(vsk->peer_shutdown);
if (sk->sk_shutdown & RCV_SHUTDOWN ||
- vsk->peer_shutdown & SEND_SHUTDOWN) {
+ peer_shutdown & SEND_SHUTDOWN) {
mask |= EPOLLIN | EPOLLRDNORM;
}
+ mask |= vsock_poll_shutdown(sk, peer_shutdown);
/* Connected sockets that can produce data can be written. */
if (transport && sk->sk_state == TCP_ESTABLISHED) {
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v3 40/41] x86/tsc: Add standalone helper for getting CPU frequency from CPUID
From: Paolo Bonzini @ 2026-05-16 7:42 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Dave Hansen, Andy Lutomirski,
Peter Zijlstra, Juergen Gross, Daniel Lezcano, Thomas Gleixner,
John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner, David Woodhouse
In-Reply-To: <20260515191942.1892718-41-seanjc@google.com>
On 5/15/26 21:19, Sean Christopherson wrote:
> Extract the guts of cpu_khz_from_cpuid() to a standalone helper that
> doesn't restrict the usage to Intel CPUs. This will allow sharing the
> core logic with kvmclock, as (a) CPUID.0x16 may be enumerated alongside
> kvmclock, and (b) KVM generally doesn't restrict CPUID based on vendor.
Even for native there's no real reason to restrict to Intel, I think.
native_calibrate_tsc() only limits itself because historically (prior to
commit 604dc9170f24, "x86/tsc: Use CPUID.0x16 to calculate missing
crystal frequency", 2019-05-09) it used a hardcoded table of crystal
frequencies.
Of course paranoia applies, but for virtualization, if the leaf exists
there is no reason not to trust it.
Thanks,
Paolo
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/include/asm/tsc.h | 1 +
> arch/x86/kernel/tsc.c | 37 +++++++++++++++++++++++--------------
> 2 files changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/include/asm/tsc.h b/arch/x86/include/asm/tsc.h
> index f458be688512..c145f5707b52 100644
> --- a/arch/x86/include/asm/tsc.h
> +++ b/arch/x86/include/asm/tsc.h
> @@ -91,6 +91,7 @@ struct cpuid_tsc_info {
> };
> extern int cpuid_get_tsc_info(struct cpuid_tsc_info *info);
> extern int cpuid_get_tsc_freq(struct cpuid_tsc_info *info);
> +extern int cpuid_get_cpu_freq(unsigned int *cpu_khz);
>
> extern void tsc_early_init(void);
> extern void tsc_init(void);
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 1b569954ae5e..745fa2052c74 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -719,6 +719,24 @@ int cpuid_get_tsc_freq(struct cpuid_tsc_info *info)
> return 0;
> }
>
> +int cpuid_get_cpu_freq(unsigned int *cpu_khz)
> +{
> + unsigned int eax_base_mhz, ebx, ecx, edx;
> +
> + *cpu_khz = 0;
> +
> + if (boot_cpu_data.cpuid_level < CPUID_LEAF_FREQ)
> + return -ENOENT;
> +
> + cpuid(CPUID_LEAF_FREQ, &eax_base_mhz, &ebx, &ecx, &edx);
> +
> + if (!eax_base_mhz)
> + return -ENOENT;
> +
> + *cpu_khz = eax_base_mhz * 1000;
> + return 0;
> +}
> +
> /**
> * native_calibrate_tsc - determine TSC frequency
> * Determine TSC frequency via CPUID, else return 0.
> @@ -754,13 +772,8 @@ unsigned long native_calibrate_tsc(void)
> * clock, but we can easily calculate it to a high degree of accuracy
> * by considering the crystal ratio and the CPU speed.
> */
> - if (!info.crystal_khz && boot_cpu_data.cpuid_level >= CPUID_LEAF_FREQ) {
> - unsigned int eax_base_mhz, ebx, ecx, edx;
> -
> - cpuid(CPUID_LEAF_FREQ, &eax_base_mhz, &ebx, &ecx, &edx);
> - info.crystal_khz = eax_base_mhz * 1000 *
> - info.denominator / info.numerator;
> - }
> + if (!info.crystal_khz && !cpuid_get_cpu_freq(&cpu_khz))
> + info.crystal_khz = cpu_khz * info.denominator / info.numerator;
>
> if (!info.crystal_khz)
> return 0;
> @@ -787,19 +800,15 @@ unsigned long native_calibrate_tsc(void)
>
> static unsigned long cpu_khz_from_cpuid(void)
> {
> - unsigned int eax_base_mhz, ebx_max_mhz, ecx_bus_mhz, edx;
> + unsigned int cpu_khz;
>
> if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
> return 0;
>
> - if (boot_cpu_data.cpuid_level < CPUID_LEAF_FREQ)
> + if (cpuid_get_cpu_freq(&cpu_khz))
> return 0;
>
> - eax_base_mhz = ebx_max_mhz = ecx_bus_mhz = edx = 0;
> -
> - cpuid(CPUID_LEAF_FREQ, &eax_base_mhz, &ebx_max_mhz, &ecx_bus_mhz, &edx);
> -
> - return eax_base_mhz * 1000;
> + return cpu_khz;
> }
>
> /*
^ permalink raw reply
* Re: [PATCH net] vsock/virtio: fix zerocopy completion for multi-skb sends
From: David Laight @ 2026-05-16 11:53 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Jakub Kicinski, Paolo Abeni, Simon Horman,
Arseniy Krasnov, Stefan Hajnoczi, kvm, Eric Dumazet,
Eugenio Pérez, Michael S. Tsirkin, Xuan Zhuo, virtualization,
David S. Miller, Jason Wang, linux-kernel, Maher Azzouzi
In-Reply-To: <20260514092948.268720-1-sgarzare@redhat.com>
On Thu, 14 May 2026 11:29:48 +0200
Stefano Garzarella <sgarzare@redhat.com> wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
>
> When a large message is fragmented into multiple skbs, the zerocopy
> uarg is only allocated and attached to the last skb in the loop.
> Non-final skbs carry pinned user pages with no completion tracking,
> so the kernel has no way to notify userspace when those pages are safe
> to reuse. If the loop breaks early the uarg is never allocated at all,
> leaking pinned pages with no completion notification.
>
> Fix this by following the approach used by TCP: allocate the zerocopy
> uarg (if not provided by the caller) before the send loop and attach
> it to every skb via skb_zcopy_set(), which takes a reference per skb.
> Each skb's completion properly decrements the refcount, and the
> notification only fires after the last skb is freed.
> On failure, if no data was sent, the uarg is cleanly aborted via
> net_zcopy_put_abort().
>
> This issue was initially discovered by sashiko while reviewing commit
> 1cb36e252211 ("vsock/virtio: fix MSG_ZEROCOPY pinned-pages accounting")
> but was pre-existing.
>
> Fixes: 581512a6dc93 ("vsock/virtio: MSG_ZEROCOPY flag support")
> Cc: Arseniy Krasnov <avkrasnov@salutedevices.com>
> Closes: https://sashiko.dev/#/patchset/20260420132051.217589-1-sgarzare%40redhat.com
> Reported-by: Maher Azzouzi <maherazz04@gmail.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> net/vmw_vsock/virtio_transport_common.c | 83 ++++++++++---------------
> 1 file changed, 34 insertions(+), 49 deletions(-)
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 989cc252d3d3..1e3409d28164 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -70,34 +70,6 @@ static bool virtio_transport_can_zcopy(const struct virtio_transport *t_ops,
> return true;
> }
>
> -static int virtio_transport_init_zcopy_skb(struct vsock_sock *vsk,
> - struct sk_buff *skb,
> - struct msghdr *msg,
> - size_t pkt_len,
> - bool zerocopy)
> -{
> - struct ubuf_info *uarg;
> -
> - if (msg->msg_ubuf) {
> - uarg = msg->msg_ubuf;
> - net_zcopy_get(uarg);
> - } else {
> - struct ubuf_info_msgzc *uarg_zc;
> -
> - uarg = msg_zerocopy_realloc(sk_vsock(vsk),
> - pkt_len, NULL, false);
> - if (!uarg)
> - return -1;
> -
> - uarg_zc = uarg_to_msgzc(uarg);
> - uarg_zc->zerocopy = zerocopy ? 1 : 0;
> - }
> -
> - skb_zcopy_init(skb, uarg);
> -
> - return 0;
> -}
> -
> static int virtio_transport_fill_skb(struct sk_buff *skb,
> struct virtio_vsock_pkt_info *info,
> size_t len,
> @@ -317,8 +289,10 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> u32 src_cid, src_port, dst_cid, dst_port;
> const struct virtio_transport *t_ops;
> struct virtio_vsock_sock *vvs;
> + struct ubuf_info *uarg = NULL;
> u32 pkt_len = info->pkt_len;
> bool can_zcopy = false;
> + bool have_uref = false;
> u32 rest_len;
> int ret;
>
> @@ -360,6 +334,25 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> if (can_zcopy)
> max_skb_len = min_t(u32, VIRTIO_VSOCK_MAX_PKT_BUF_SIZE,
> (MAX_SKB_FRAGS * PAGE_SIZE));
> +
> + if (info->msg->msg_flags & MSG_ZEROCOPY &&
> + info->op == VIRTIO_VSOCK_OP_RW) {
> + uarg = info->msg->msg_ubuf;
> +
> + if (!uarg) {
> + uarg = msg_zerocopy_realloc(sk_vsock(vsk),
> + pkt_len, NULL, false);
> + if (!uarg) {
> + virtio_transport_put_credit(vvs, pkt_len);
> + return -ENOMEM;
> + }
> +
> + if (!can_zcopy)
> + uarg_to_msgzc(uarg)->zerocopy = 0;
> +
> + have_uref = true;
> + }
> + }
Surely that block should only be done if can_zcopy is true?
And shouldn't something unset it if info->op != VIRTIO_VSOCK_OP_RW ?
If the msg_zerocopy_realloc() fails then can't you just set can_zcopy to false.
It info->msg->msg_buf is already set then I think you have to disable zero-copy.
The caller has already requested a callback - and you can't add another.
In any case by the end of this can_zcopy and have_uref are really the same flag.
> }
>
> rest_len = pkt_len;
> @@ -378,27 +371,7 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> break;
> }
>
> - /* We process buffer part by part, allocating skb on
> - * each iteration. If this is last skb for this buffer
> - * and MSG_ZEROCOPY mode is in use - we must allocate
> - * completion for the current syscall.
> - *
> - * Pass pkt_len because msg iter is already consumed
> - * by virtio_transport_fill_skb(), so iter->count
> - * can not be used for RLIMIT_MEMLOCK pinned-pages
> - * accounting done by msg_zerocopy_realloc().
> - */
> - if (info->msg && info->msg->msg_flags & MSG_ZEROCOPY &&
> - skb_len == rest_len && info->op == VIRTIO_VSOCK_OP_RW) {
> - if (virtio_transport_init_zcopy_skb(vsk, skb,
> - info->msg,
> - pkt_len,
> - can_zcopy)) {
> - kfree_skb(skb);
> - ret = -ENOMEM;
> - break;
> - }
> - }
> + skb_zcopy_set(skb, uarg, NULL);
>
> virtio_transport_inc_tx_pkt(vvs, skb);
>
> @@ -422,6 +395,18 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>
> virtio_transport_put_credit(vvs, rest_len);
>
> + /* msg_zerocopy_realloc() initializes the ubuf_info refcnt to 1.
> + * skb_zcopy_set() increases it for each skb, so we can drop that
^ must
> + * initial reference to keep it balanced.
> + */
> + if (have_uref) {
> + if (rest_len == pkt_len)
> + /* No data sent, abort the notification. */
> + net_zcopy_put_abort(uarg, true);
Is it worth optimising for the 'nothing sent' case ?
-- David
> + else
> + net_zcopy_put(uarg);
> + }
> +
> /* Return number of bytes, if any data has been sent. */
> if (rest_len != pkt_len)
> ret = pkt_len - rest_len;
^ permalink raw reply
* Re: [PATCH RESEND] ALSA: virtio: Add missing 384 kHz PCM rate mapping
From: Takashi Iwai @ 2026-05-16 13:42 UTC (permalink / raw)
To: Cássio Gabriel
Cc: Takashi Iwai, Anton Yakovlev, Michael S. Tsirkin, Jaroslav Kysela,
virtualization, linux-sound, linux-kernel, stable
In-Reply-To: <20260515-alsa-virtio-384k-rate-v1-1-35ecb5df835c@gmail.com>
On Fri, 15 May 2026 15:32:25 +0200,
Cássio Gabriel wrote:
>
> The VirtIO sound UAPI defines VIRTIO_SND_PCM_RATE_384000, and ALSA
> has SNDRV_PCM_RATE_384000. However, virtio-snd's rate conversion
> tables stop at 192 kHz.
>
> A device advertising only 384 kHz is rejected as having no supported
> PCM frame rates. A device advertising 384 kHz together with lower rates
> does not expose 384 kHz through the ALSA hardware constraints. The
> selected ALSA rate also needs a reverse mapping for SET_PARAMS.
>
> Add the missing 384 kHz entries to both conversion tables.
>
> Fixes: 29b96bf50ba9 ("ALSA: virtio: build PCM devices and substream hardware descriptors")
> Fixes: da76e9f3e43a ("ALSA: virtio: PCM substream operators")
> Cc: stable@vger.kernel.org
> Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
Applied to for-next branch now. Thanks.
Takashi
^ permalink raw reply
* Re: Patch "vsock/virtio: fix potential unbounded skb queue" has been added to the 6.6-stable tree
From: Sasha Levin @ 2026-05-17 13:33 UTC (permalink / raw)
To: Greg KH
Cc: Sasha Levin, Michael S. Tsirkin, AVKrasnov, edumazet, eperezma,
jasowang, kuba, leonardi, sgarzare, stefanha, virtualization,
xuanzhuo, stable-commits, stable
In-Reply-To: <20260515114521-mutt-send-email-mst@kernel.org>
> > What's the status of that fix?
>
> Stefano posted v3 and is working on v4.
>
> > Should it be reverted elsewhere?
>
> Donnu. With the change we have no DoS but the socket gets silently
> broken. Eric felt given the brokenness is upstream already it's better
> to work on a fix on top, not revert.
Dropped from the 6.6, 6.12, 6.18, and 7.0 queues. We'll pick up Stefano's
follow-up once it lands upstream.
Thanks.
--
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH v3] drm/virtio: use uninterruptible resv lock for plane updates
From: Dmitry Osipenko @ 2026-05-17 16:14 UTC (permalink / raw)
To: Deepanshu Kartikey, airlied, kraxel, gurchetansingh, olvaffe,
maarten.lankhorst, mripard, tzimmermann, simona, sumit.semwal,
christian.koenig
Cc: dri-devel, virtualization, linux-kernel, linux-media,
linaro-mm-sig, syzbot+72bd3dd3a5d5f39a0271, stable
In-Reply-To: <20260515084030.21986-1-kartikey406@gmail.com>
On 5/15/26 11:40, Deepanshu Kartikey wrote:
> +int virtio_gpu_array_lock_resv_uninterruptible(struct virtio_gpu_object_array *objs)
> +{
> + unsigned int i;
> + int ret = 0;
> +
> + if (objs->nents == 1) {
> + dma_resv_lock(objs->objs[0]->resv, NULL);
> + } else {
> + ret = drm_gem_lock_reservations(objs->objs, objs->nents,
> + &objs->ticket);
drm_gem_lock_reservations() is interruptible. Given that only one BO
needs to be locked for the fix, make it
virtio_gpu_lock_one_resv_uninterruptible() and fail with -EINVAL if
objs->nents > 1
--
Best regards,
Dmitry
^ permalink raw reply
* Re: [PATCH] drm/virtio: add timeout to virtqueue wait to avoid hung task
From: Ryosuke Yasuoka @ 2026-05-18 6:45 UTC (permalink / raw)
To: Dmitry Osipenko, David Airlie, Gerd Hoffmann, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter
Cc: dri-devel, virtualization, linux-kernel
In-Reply-To: <8a986c52-964f-42a5-b063-fbe2b242ca36@collabora.com>
Hi Dmitry,
Thank you for your review.
On 14/05/2026 00:54, Dmitry Osipenko wrote:
> On 5/14/26 00:40, Dmitry Osipenko wrote:
>> Hi,
>>
>> On 5/12/26 11:59, Ryosuke Yasuoka wrote:
>>> virtio_gpu_queue_ctrl_sgs() and virtio_gpu_queue_cursor() use
>>> wait_event() without timeout when waiting for virtqueue space. If the
>>> host device stops processing commands, these waits block indefinitely.
>>> Since callers may hold DRM locks, this can make the entire system
>>> unresponsive.
>>>
>>> Replace wait_event() with wait_event_timeout() using a 5-second timeout,
>>> consistent with the existing timeout pattern in the driver. On timeout,
>>> clean up and return -ENODEV, following the same error path as
>>> drm_dev_enter() failure.
>>>
>>> Reported-by: syzbot+d6dd6f86d3aaf7eebe7406e45c1c6e549453f224@syzkaller.appspotmail.com
>>> Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
>>> Reported-by: syzbot+908bd910da5dd79b88de4cf7baf376cc873a922e@syzkaller.appspotmail.com
>>> Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
>>> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
>>> ---
>>> drivers/gpu/drm/virtio/virtgpu_vq.c | 20 ++++++++++++++++++--
>>> 1 file changed, 18 insertions(+), 2 deletions(-)
>>
>> If host stops processing commands, this is a problem on host side. Isn't it?
Yes, it is. But the guest has no way to recover from this situation on
its own. The wait_event{,_timeout}() is inside the device critical
section between drm_dev_enter/exit(). Removing the device via sysfs and
graceful shutdown call drm_dev_unplug(), which is blocked until the
critical section completes, so they cannot proceed either. Also, IIUC
the virtio-gpu device cannot be hot-unplugged from the host side. The
only option left is a forced reboot.
> It may be acceptable to have wait_event_timeout() in a loop, printing
> warnings about unresponsive host.
I considered this approach, but it does not solve the recovery problem
described above. The guest would still be stuck in the loop with no way
to remove the device or shut down gracefully.
> Don't think we can assume that 5 seconds is enough to say that host is
> busted, unless spec says so.
>
> There could be a driver module parameter, specifying the timeoout. This
> will be acceptable as user takes responsibility for the special timeout
> behaviour.
Agreed. In v2, the default behavior is preserved (wait_event, wait
indefinitely). A new 'timeout' module parameter (in seconds) lets the
user specify when to give up. When set to a non-zero value, the driver
returns -ENODEV after the specified duration with a warning message.
Also, I'm considering proposing a host response timeout in the
virtio-gpu specificaiton. If a spec-defined timeout is accepted in the
future, the driver could use it as the default instead of relying on the
module parameter.
Best regards,
Ryosuke
^ permalink raw reply
* [PATCH net v4 0/2] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
From: Stefano Garzarella @ 2026-05-18 9:06 UTC (permalink / raw)
To: netdev
Cc: Simon Horman, Jakub Kicinski, Michael S. Tsirkin, Paolo Abeni,
Jason Wang, Stefano Garzarella, David S. Miller, kvm,
Stefan Hajnoczi, linux-kernel, Eric Dumazet, Xuan Zhuo,
virtualization, Eugenio Pérez
Patch 1 resets the connection when we can no longer queue packets,
this prevents silent data loss, and both peers are notified.
Patch 2 increases the total budget to `buf_alloc * 2` for payload
plus skb overhead similar to how SO_RCVBUF is doubled to reserve
space for sk_buff metadata. This preserves the full buf_alloc for
payload under normal operation, while still bounding the skb queue
growth.
In the future, we plan to improve how we handle the merging of packets
to minimize overhead and avoid closing connections.
v4:
- Split the buf_alloc check to be sure the credit is still respected and
to avoid overflow of buf_used [sashiko]
- call virtio_transport_do_close() and vsock_remove_sock() to properly
close the connection and remove the socket from the connect table
[sashiko]
v3: https://lore.kernel.org/netdev/20260513105417.56761-1-sgarzare@redhat.com/
- Split in 2 patches [MST]
v2: https://lore.kernel.org/netdev/20260512080737.36787-1-sgarzare@redhat.com/
- Close the connection when we can no longer queue new packets instead
of losing data.
- No longer announce the reduced buf_alloc to avoid violating the
spec. [MST]
v1: https://lore.kernel.org/netdev/20260508092330.69690-1-sgarzare@redhat.com/
Stefano Garzarella (2):
vsock/virtio: reset connection on receiving queue overflow
vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
net/vmw_vsock/virtio_transport_common.c | 29 ++++++++++++++++++++-----
1 file changed, 23 insertions(+), 6 deletions(-)
--
2.54.0
^ permalink raw reply
* [PATCH net v4 1/2] vsock/virtio: reset connection on receiving queue overflow
From: Stefano Garzarella @ 2026-05-18 9:06 UTC (permalink / raw)
To: netdev
Cc: Simon Horman, Jakub Kicinski, Michael S. Tsirkin, Paolo Abeni,
Jason Wang, Stefano Garzarella, David S. Miller, kvm,
Stefan Hajnoczi, linux-kernel, Eric Dumazet, Xuan Zhuo,
virtualization, Eugenio Pérez, stable
In-Reply-To: <20260518090656.134588-1-sgarzare@redhat.com>
From: Stefano Garzarella <sgarzare@redhat.com>
When there is no more space to queue an incoming packet, the packet is
silently dropped. This causes data loss without any notification to
either peer, since there is no retransmission.
Under normal circumstances, this should never happen. However, it could
happen if the other peer doesn't respect the credit, or if the skb
overhead, which we recently began to take into account with commit
059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue"),
is too high.
Fix this by resetting the connection and setting the local socket error
to ENOBUFS when virtio_transport_recv_enqueue() can no longer queue a
packet, so both peers are explicitly notified of the failure rather than
silently losing data.
Fixes: ae6fcfbf5f03 ("vsock/virtio: discard packets if credit is not respected")
Cc: stable@vger.kernel.org
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
net/vmw_vsock/virtio_transport_common.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 1e3409d28164..5028ff534888 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1335,7 +1335,7 @@ virtio_transport_recv_connecting(struct sock *sk,
return err;
}
-static void
+static bool
virtio_transport_recv_enqueue(struct vsock_sock *vsk,
struct sk_buff *skb)
{
@@ -1350,10 +1350,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
spin_lock_bh(&vvs->rx_lock);
can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
- if (!can_enqueue) {
- free_pkt = true;
+ if (!can_enqueue)
goto out;
- }
if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)
vvs->msg_count++;
@@ -1393,6 +1391,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
spin_unlock_bh(&vvs->rx_lock);
if (free_pkt)
kfree_skb(skb);
+
+ return can_enqueue;
}
static int
@@ -1405,7 +1405,17 @@ virtio_transport_recv_connected(struct sock *sk,
switch (le16_to_cpu(hdr->op)) {
case VIRTIO_VSOCK_OP_RW:
- virtio_transport_recv_enqueue(vsk, skb);
+ if (!virtio_transport_recv_enqueue(vsk, skb)) {
+ /* There is no more space to queue the packet, so let's
+ * close the connection; otherwise, we'll lose data.
+ */
+ (void)virtio_transport_reset(vsk, skb);
+ virtio_transport_do_close(vsk, true);
+ sk->sk_err = ENOBUFS;
+ sk_error_report(sk);
+ vsock_remove_sock(vsk);
+ break;
+ }
vsock_data_ready(sk);
return err;
case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
--
2.54.0
^ permalink raw reply related
* [PATCH net v4 2/2] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
From: Stefano Garzarella @ 2026-05-18 9:06 UTC (permalink / raw)
To: netdev
Cc: Simon Horman, Jakub Kicinski, Michael S. Tsirkin, Paolo Abeni,
Jason Wang, Stefano Garzarella, David S. Miller, kvm,
Stefan Hajnoczi, linux-kernel, Eric Dumazet, Xuan Zhuo,
virtualization, Eugenio Pérez, stable
In-Reply-To: <20260518090656.134588-1-sgarzare@redhat.com>
From: Stefano Garzarella <sgarzare@redhat.com>
After commit 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb
queue"), virtio_transport_inc_rx_pkt() subtracts per-skb overhead from
buf_alloc when checking whether a new packet fits. This reduces the
effective receive buffer below what the user configured via
SO_VM_SOCKETS_BUFFER_SIZE, causing legitimate data packets to be
silently dropped and applications that rely on the full buffer size
to deadlock.
Also, the reduced space is not communicated to the remote peer, so
its credit calculation accounts more credit than the receiver will
actually accept, causing data loss (there is no retransmission).
With this approach we currently have failures in
tools/testing/vsock/vsock_test.c. Test 18 sometimes fails, while
test 22 always fails in this way:
18 - SOCK_STREAM MSG_ZEROCOPY...hash mismatch
22 - SOCK_STREAM virtio credit update + SO_RCVLOWAT...send failed:
Resource temporarily unavailable
Fix by allowing at most `buf_alloc * 2` as the total budget for payload
plus skb overhead in virtio_transport_inc_rx_pkt(), similar to how
SO_RCVBUF is doubled to reserve space for sk_buff metadata.
This preserves the full buf_alloc for payload under normal operation,
while still bounding the skb queue growth.
With this patch, all tests in tools/testing/vsock/vsock_test.c are
now passing again.
Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
Cc: stable@vger.kernel.org
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
net/vmw_vsock/virtio_transport_common.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 5028ff534888..df3b418e0392 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -419,7 +419,14 @@ static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
{
u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
- if (skb_overhead + vvs->buf_used + len > vvs->buf_alloc)
+ /* Allow at most buf_alloc * 2 total budget (payload + overhead),
+ * similar to how SO_RCVBUF is doubled to reserve space for sk_buff
+ * metadata. Check payload against buf_alloc to be sure the other
+ * peer is respecting the credit, and sk_buff overhead to bound
+ * queue growth.
+ */
+ if ((u64)vvs->buf_used + len > vvs->buf_alloc ||
+ skb_overhead > vvs->buf_alloc)
return false;
vvs->rx_bytes += len;
--
2.54.0
^ permalink raw reply related
* Re: [PATCH net] vsock/virtio: fix zerocopy completion for multi-skb sends
From: Stefano Garzarella @ 2026-05-18 9:18 UTC (permalink / raw)
To: David Laight
Cc: netdev, Jakub Kicinski, Paolo Abeni, Simon Horman,
Arseniy Krasnov, Stefan Hajnoczi, kvm, Eric Dumazet,
Eugenio Pérez, Michael S. Tsirkin, Xuan Zhuo, virtualization,
David S. Miller, Jason Wang, linux-kernel, Maher Azzouzi
In-Reply-To: <20260516125329.7b699c6f@pumpkin>
On Sat, May 16, 2026 at 12:53:29PM +0100, David Laight wrote:
>On Thu, 14 May 2026 11:29:48 +0200
>Stefano Garzarella <sgarzare@redhat.com> wrote:
>
>> From: Stefano Garzarella <sgarzare@redhat.com>
>>
>> When a large message is fragmented into multiple skbs, the zerocopy
>> uarg is only allocated and attached to the last skb in the loop.
>> Non-final skbs carry pinned user pages with no completion tracking,
>> so the kernel has no way to notify userspace when those pages are safe
>> to reuse. If the loop breaks early the uarg is never allocated at all,
>> leaking pinned pages with no completion notification.
>>
>> Fix this by following the approach used by TCP: allocate the zerocopy
>> uarg (if not provided by the caller) before the send loop and attach
>> it to every skb via skb_zcopy_set(), which takes a reference per skb.
>> Each skb's completion properly decrements the refcount, and the
>> notification only fires after the last skb is freed.
>> On failure, if no data was sent, the uarg is cleanly aborted via
>> net_zcopy_put_abort().
>>
>> This issue was initially discovered by sashiko while reviewing commit
>> 1cb36e252211 ("vsock/virtio: fix MSG_ZEROCOPY pinned-pages accounting")
>> but was pre-existing.
>>
>> Fixes: 581512a6dc93 ("vsock/virtio: MSG_ZEROCOPY flag support")
>> Cc: Arseniy Krasnov <avkrasnov@salutedevices.com>
>> Closes: https://sashiko.dev/#/patchset/20260420132051.217589-1-sgarzare%40redhat.com
>> Reported-by: Maher Azzouzi <maherazz04@gmail.com>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> ---
>> net/vmw_vsock/virtio_transport_common.c | 83 ++++++++++---------------
>> 1 file changed, 34 insertions(+), 49 deletions(-)
>>
>> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>> index 989cc252d3d3..1e3409d28164 100644
>> --- a/net/vmw_vsock/virtio_transport_common.c
>> +++ b/net/vmw_vsock/virtio_transport_common.c
>> @@ -70,34 +70,6 @@ static bool virtio_transport_can_zcopy(const struct virtio_transport *t_ops,
>> return true;
>> }
>>
>> -static int virtio_transport_init_zcopy_skb(struct vsock_sock *vsk,
>> - struct sk_buff *skb,
>> - struct msghdr *msg,
>> - size_t pkt_len,
>> - bool zerocopy)
>> -{
>> - struct ubuf_info *uarg;
>> -
>> - if (msg->msg_ubuf) {
>> - uarg = msg->msg_ubuf;
>> - net_zcopy_get(uarg);
>> - } else {
>> - struct ubuf_info_msgzc *uarg_zc;
>> -
>> - uarg = msg_zerocopy_realloc(sk_vsock(vsk),
>> - pkt_len, NULL, false);
>> - if (!uarg)
>> - return -1;
>> -
>> - uarg_zc = uarg_to_msgzc(uarg);
>> - uarg_zc->zerocopy = zerocopy ? 1 : 0;
>> - }
>> -
>> - skb_zcopy_init(skb, uarg);
>> -
>> - return 0;
>> -}
>> -
>> static int virtio_transport_fill_skb(struct sk_buff *skb,
>> struct virtio_vsock_pkt_info *info,
>> size_t len,
>> @@ -317,8 +289,10 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>> u32 src_cid, src_port, dst_cid, dst_port;
>> const struct virtio_transport *t_ops;
>> struct virtio_vsock_sock *vvs;
>> + struct ubuf_info *uarg = NULL;
>> u32 pkt_len = info->pkt_len;
>> bool can_zcopy = false;
>> + bool have_uref = false;
>> u32 rest_len;
>> int ret;
>>
>> @@ -360,6 +334,25 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>> if (can_zcopy)
>> max_skb_len = min_t(u32, VIRTIO_VSOCK_MAX_PKT_BUF_SIZE,
>> (MAX_SKB_FRAGS * PAGE_SIZE));
>> +
>> + if (info->msg->msg_flags & MSG_ZEROCOPY &&
>> + info->op == VIRTIO_VSOCK_OP_RW) {
>> + uarg = info->msg->msg_ubuf;
>> +
>> + if (!uarg) {
>> + uarg = msg_zerocopy_realloc(sk_vsock(vsk),
>> + pkt_len, NULL, false);
>> + if (!uarg) {
>> + virtio_transport_put_credit(vvs, pkt_len);
>> + return -ENOMEM;
>> + }
>> +
>> + if (!can_zcopy)
>> + uarg_to_msgzc(uarg)->zerocopy = 0;
>> +
>> + have_uref = true;
>> + }
>> + }
>
>Surely that block should only be done if can_zcopy is true?
>And shouldn't something unset it if info->op != VIRTIO_VSOCK_OP_RW ?
>If the msg_zerocopy_realloc() fails then can't you just set can_zcopy to false.
>
>It info->msg->msg_buf is already set then I think you have to disable zero-copy.
>The caller has already requested a callback - and you can't add another.
>
>In any case by the end of this can_zcopy and have_uref are really the same flag.
I kept the same approach we had before, trying to make as few changes as
possible.
All these potential issues seem to be pre-existing and should be
eventually addressed in other patches IMHO. This patch one only
resolves the main issue of calling `skb_zcopy_set()` for every skb to
avoid leaking pages, etc.
@Arseniy can you help on this?
>
>> }
>>
>> rest_len = pkt_len;
>> @@ -378,27 +371,7 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>> break;
>> }
>>
>> - /* We process buffer part by part, allocating skb on
>> - * each iteration. If this is last skb for this buffer
>> - * and MSG_ZEROCOPY mode is in use - we must allocate
>> - * completion for the current syscall.
>> - *
>> - * Pass pkt_len because msg iter is already consumed
>> - * by virtio_transport_fill_skb(), so iter->count
>> - * can not be used for RLIMIT_MEMLOCK pinned-pages
>> - * accounting done by msg_zerocopy_realloc().
>> - */
>> - if (info->msg && info->msg->msg_flags & MSG_ZEROCOPY &&
>> - skb_len == rest_len && info->op == VIRTIO_VSOCK_OP_RW) {
>> - if (virtio_transport_init_zcopy_skb(vsk, skb,
>> - info->msg,
>> - pkt_len,
>> - can_zcopy)) {
>> - kfree_skb(skb);
>> - ret = -ENOMEM;
>> - break;
>> - }
>> - }
>> + skb_zcopy_set(skb, uarg, NULL);
>>
>> virtio_transport_inc_tx_pkt(vvs, skb);
>>
>> @@ -422,6 +395,18 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>>
>> virtio_transport_put_credit(vvs, rest_len);
>>
>> + /* msg_zerocopy_realloc() initializes the ubuf_info refcnt to 1.
>> + * skb_zcopy_set() increases it for each skb, so we can drop that
> ^ must
>
>> + * initial reference to keep it balanced.
>> + */
>> + if (have_uref) {
>> + if (rest_len == pkt_len)
>> + /* No data sent, abort the notification. */
>> + net_zcopy_put_abort(uarg, true);
>
>Is it worth optimising for the 'nothing sent' case ?
What do you suggest doing?
I followed what TCP does.
Thanks,
Stefano
>
>-- David
>
>> + else
>> + net_zcopy_put(uarg);
>> + }
>> +
>> /* Return number of bytes, if any data has been sent. */
>> if (rest_len != pkt_len)
>> ret = pkt_len - rest_len;
>
^ permalink raw reply
* Re: [PATCH net] vsock/virtio: fix zerocopy completion for multi-skb sends
From: Michael S. Tsirkin @ 2026-05-18 9:33 UTC (permalink / raw)
To: Stefano Garzarella
Cc: David Laight, netdev, Jakub Kicinski, Paolo Abeni, Simon Horman,
Arseniy Krasnov, Stefan Hajnoczi, kvm, Eric Dumazet,
Eugenio Pérez, Xuan Zhuo, virtualization, David S. Miller,
Jason Wang, linux-kernel, Maher Azzouzi
In-Reply-To: <agrXvCtNRlySlwgx@sgarzare-redhat>
On Mon, May 18, 2026 at 11:18:24AM +0200, Stefano Garzarella wrote:
> On Sat, May 16, 2026 at 12:53:29PM +0100, David Laight wrote:
> > On Thu, 14 May 2026 11:29:48 +0200
> > Stefano Garzarella <sgarzare@redhat.com> wrote:
> >
> > > From: Stefano Garzarella <sgarzare@redhat.com>
> > >
> > > When a large message is fragmented into multiple skbs, the zerocopy
> > > uarg is only allocated and attached to the last skb in the loop.
> > > Non-final skbs carry pinned user pages with no completion tracking,
> > > so the kernel has no way to notify userspace when those pages are safe
> > > to reuse. If the loop breaks early the uarg is never allocated at all,
> > > leaking pinned pages with no completion notification.
> > >
> > > Fix this by following the approach used by TCP: allocate the zerocopy
> > > uarg (if not provided by the caller) before the send loop and attach
> > > it to every skb via skb_zcopy_set(), which takes a reference per skb.
> > > Each skb's completion properly decrements the refcount, and the
> > > notification only fires after the last skb is freed.
> > > On failure, if no data was sent, the uarg is cleanly aborted via
> > > net_zcopy_put_abort().
> > >
> > > This issue was initially discovered by sashiko while reviewing commit
> > > 1cb36e252211 ("vsock/virtio: fix MSG_ZEROCOPY pinned-pages accounting")
> > > but was pre-existing.
> > >
> > > Fixes: 581512a6dc93 ("vsock/virtio: MSG_ZEROCOPY flag support")
> > > Cc: Arseniy Krasnov <avkrasnov@salutedevices.com>
> > > Closes: https://sashiko.dev/#/patchset/20260420132051.217589-1-sgarzare%40redhat.com
> > > Reported-by: Maher Azzouzi <maherazz04@gmail.com>
> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > ---
> > > net/vmw_vsock/virtio_transport_common.c | 83 ++++++++++---------------
> > > 1 file changed, 34 insertions(+), 49 deletions(-)
> > >
> > > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > > index 989cc252d3d3..1e3409d28164 100644
> > > --- a/net/vmw_vsock/virtio_transport_common.c
> > > +++ b/net/vmw_vsock/virtio_transport_common.c
> > > @@ -70,34 +70,6 @@ static bool virtio_transport_can_zcopy(const struct virtio_transport *t_ops,
> > > return true;
> > > }
> > >
> > > -static int virtio_transport_init_zcopy_skb(struct vsock_sock *vsk,
> > > - struct sk_buff *skb,
> > > - struct msghdr *msg,
> > > - size_t pkt_len,
> > > - bool zerocopy)
> > > -{
> > > - struct ubuf_info *uarg;
> > > -
> > > - if (msg->msg_ubuf) {
> > > - uarg = msg->msg_ubuf;
> > > - net_zcopy_get(uarg);
> > > - } else {
> > > - struct ubuf_info_msgzc *uarg_zc;
> > > -
> > > - uarg = msg_zerocopy_realloc(sk_vsock(vsk),
> > > - pkt_len, NULL, false);
> > > - if (!uarg)
> > > - return -1;
> > > -
> > > - uarg_zc = uarg_to_msgzc(uarg);
> > > - uarg_zc->zerocopy = zerocopy ? 1 : 0;
> > > - }
> > > -
> > > - skb_zcopy_init(skb, uarg);
> > > -
> > > - return 0;
> > > -}
> > > -
> > > static int virtio_transport_fill_skb(struct sk_buff *skb,
> > > struct virtio_vsock_pkt_info *info,
> > > size_t len,
> > > @@ -317,8 +289,10 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> > > u32 src_cid, src_port, dst_cid, dst_port;
> > > const struct virtio_transport *t_ops;
> > > struct virtio_vsock_sock *vvs;
> > > + struct ubuf_info *uarg = NULL;
> > > u32 pkt_len = info->pkt_len;
> > > bool can_zcopy = false;
> > > + bool have_uref = false;
> > > u32 rest_len;
> > > int ret;
> > >
> > > @@ -360,6 +334,25 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> > > if (can_zcopy)
> > > max_skb_len = min_t(u32, VIRTIO_VSOCK_MAX_PKT_BUF_SIZE,
> > > (MAX_SKB_FRAGS * PAGE_SIZE));
> > > +
> > > + if (info->msg->msg_flags & MSG_ZEROCOPY &&
> > > + info->op == VIRTIO_VSOCK_OP_RW) {
> > > + uarg = info->msg->msg_ubuf;
> > > +
> > > + if (!uarg) {
> > > + uarg = msg_zerocopy_realloc(sk_vsock(vsk),
> > > + pkt_len, NULL, false);
> > > + if (!uarg) {
> > > + virtio_transport_put_credit(vvs, pkt_len);
> > > + return -ENOMEM;
> > > + }
> > > +
> > > + if (!can_zcopy)
> > > + uarg_to_msgzc(uarg)->zerocopy = 0;
> > > +
> > > + have_uref = true;
> > > + }
> > > + }
> >
> > Surely that block should only be done if can_zcopy is true?
> > And shouldn't something unset it if info->op != VIRTIO_VSOCK_OP_RW ?
> > If the msg_zerocopy_realloc() fails then can't you just set can_zcopy to false.
> >
> > It info->msg->msg_buf is already set then I think you have to disable zero-copy.
> > The caller has already requested a callback - and you can't add another.
> >
> > In any case by the end of this can_zcopy and have_uref are really the same flag.
>
> I kept the same approach we had before, trying to make as few changes as
> possible.
>
> All these potential issues seem to be pre-existing and should be eventually
> addressed in other patches IMHO. This patch one only resolves the main issue
> of calling `skb_zcopy_set()` for every skb to avoid leaking pages, etc.
the patch is upstream now, right? So pretty much have to be patches on
top.
> @Arseniy can you help on this?
>
> >
> > > }
> > >
> > > rest_len = pkt_len;
> > > @@ -378,27 +371,7 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> > > break;
> > > }
> > >
> > > - /* We process buffer part by part, allocating skb on
> > > - * each iteration. If this is last skb for this buffer
> > > - * and MSG_ZEROCOPY mode is in use - we must allocate
> > > - * completion for the current syscall.
> > > - *
> > > - * Pass pkt_len because msg iter is already consumed
> > > - * by virtio_transport_fill_skb(), so iter->count
> > > - * can not be used for RLIMIT_MEMLOCK pinned-pages
> > > - * accounting done by msg_zerocopy_realloc().
> > > - */
> > > - if (info->msg && info->msg->msg_flags & MSG_ZEROCOPY &&
> > > - skb_len == rest_len && info->op == VIRTIO_VSOCK_OP_RW) {
> > > - if (virtio_transport_init_zcopy_skb(vsk, skb,
> > > - info->msg,
> > > - pkt_len,
> > > - can_zcopy)) {
> > > - kfree_skb(skb);
> > > - ret = -ENOMEM;
> > > - break;
> > > - }
> > > - }
> > > + skb_zcopy_set(skb, uarg, NULL);
> > >
> > > virtio_transport_inc_tx_pkt(vvs, skb);
> > >
> > > @@ -422,6 +395,18 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> > >
> > > virtio_transport_put_credit(vvs, rest_len);
> > >
> > > + /* msg_zerocopy_realloc() initializes the ubuf_info refcnt to 1.
> > > + * skb_zcopy_set() increases it for each skb, so we can drop that
> > ^ must
> >
> > > + * initial reference to keep it balanced.
> > > + */
> > > + if (have_uref) {
> > > + if (rest_len == pkt_len)
> > > + /* No data sent, abort the notification. */
> > > + net_zcopy_put_abort(uarg, true);
> >
> > Is it worth optimising for the 'nothing sent' case ?
>
> What do you suggest doing?
>
> I followed what TCP does.
>
> Thanks,
> Stefano
>
> >
> > -- David
> >
> > > + else
> > > + net_zcopy_put(uarg);
> > > + }
> > > +
> > > /* Return number of bytes, if any data has been sent. */
> > > if (rest_len != pkt_len)
> > > ret = pkt_len - rest_len;
> >
^ permalink raw reply
* Re: [PATCH net] vsock/virtio: fix zerocopy completion for multi-skb sends
From: Stefano Garzarella @ 2026-05-18 9:54 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David Laight, netdev, Jakub Kicinski, Paolo Abeni, Simon Horman,
Arseniy Krasnov, Stefan Hajnoczi, kvm, Eric Dumazet,
Eugenio Pérez, Xuan Zhuo, virtualization, David S. Miller,
Jason Wang, linux-kernel, Maher Azzouzi
In-Reply-To: <20260518053223-mutt-send-email-mst@kernel.org>
On Mon, May 18, 2026 at 05:33:08AM -0400, Michael S. Tsirkin wrote:
>On Mon, May 18, 2026 at 11:18:24AM +0200, Stefano Garzarella wrote:
>> On Sat, May 16, 2026 at 12:53:29PM +0100, David Laight wrote:
>> > On Thu, 14 May 2026 11:29:48 +0200
>> > Stefano Garzarella <sgarzare@redhat.com> wrote:
>> >
>> > > From: Stefano Garzarella <sgarzare@redhat.com>
>> > >
>> > > When a large message is fragmented into multiple skbs, the zerocopy
>> > > uarg is only allocated and attached to the last skb in the loop.
>> > > Non-final skbs carry pinned user pages with no completion tracking,
>> > > so the kernel has no way to notify userspace when those pages are safe
>> > > to reuse. If the loop breaks early the uarg is never allocated at all,
>> > > leaking pinned pages with no completion notification.
>> > >
>> > > Fix this by following the approach used by TCP: allocate the zerocopy
>> > > uarg (if not provided by the caller) before the send loop and attach
>> > > it to every skb via skb_zcopy_set(), which takes a reference per skb.
>> > > Each skb's completion properly decrements the refcount, and the
>> > > notification only fires after the last skb is freed.
>> > > On failure, if no data was sent, the uarg is cleanly aborted via
>> > > net_zcopy_put_abort().
>> > >
>> > > This issue was initially discovered by sashiko while reviewing commit
>> > > 1cb36e252211 ("vsock/virtio: fix MSG_ZEROCOPY pinned-pages accounting")
>> > > but was pre-existing.
>> > >
>> > > Fixes: 581512a6dc93 ("vsock/virtio: MSG_ZEROCOPY flag support")
>> > > Cc: Arseniy Krasnov <avkrasnov@salutedevices.com>
>> > > Closes: https://sashiko.dev/#/patchset/20260420132051.217589-1-sgarzare%40redhat.com
>> > > Reported-by: Maher Azzouzi <maherazz04@gmail.com>
>> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> > > ---
>> > > net/vmw_vsock/virtio_transport_common.c | 83 ++++++++++---------------
>> > > 1 file changed, 34 insertions(+), 49 deletions(-)
>> > >
>> > > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>> > > index 989cc252d3d3..1e3409d28164 100644
>> > > --- a/net/vmw_vsock/virtio_transport_common.c
>> > > +++ b/net/vmw_vsock/virtio_transport_common.c
>> > > @@ -70,34 +70,6 @@ static bool virtio_transport_can_zcopy(const struct virtio_transport *t_ops,
>> > > return true;
>> > > }
>> > >
>> > > -static int virtio_transport_init_zcopy_skb(struct vsock_sock *vsk,
>> > > - struct sk_buff *skb,
>> > > - struct msghdr *msg,
>> > > - size_t pkt_len,
>> > > - bool zerocopy)
>> > > -{
>> > > - struct ubuf_info *uarg;
>> > > -
>> > > - if (msg->msg_ubuf) {
>> > > - uarg = msg->msg_ubuf;
>> > > - net_zcopy_get(uarg);
>> > > - } else {
>> > > - struct ubuf_info_msgzc *uarg_zc;
>> > > -
>> > > - uarg = msg_zerocopy_realloc(sk_vsock(vsk),
>> > > - pkt_len, NULL, false);
>> > > - if (!uarg)
>> > > - return -1;
>> > > -
>> > > - uarg_zc = uarg_to_msgzc(uarg);
>> > > - uarg_zc->zerocopy = zerocopy ? 1 : 0;
>> > > - }
>> > > -
>> > > - skb_zcopy_init(skb, uarg);
>> > > -
>> > > - return 0;
>> > > -}
>> > > -
>> > > static int virtio_transport_fill_skb(struct sk_buff *skb,
>> > > struct virtio_vsock_pkt_info *info,
>> > > size_t len,
>> > > @@ -317,8 +289,10 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>> > > u32 src_cid, src_port, dst_cid, dst_port;
>> > > const struct virtio_transport *t_ops;
>> > > struct virtio_vsock_sock *vvs;
>> > > + struct ubuf_info *uarg = NULL;
>> > > u32 pkt_len = info->pkt_len;
>> > > bool can_zcopy = false;
>> > > + bool have_uref = false;
>> > > u32 rest_len;
>> > > int ret;
>> > >
>> > > @@ -360,6 +334,25 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>> > > if (can_zcopy)
>> > > max_skb_len = min_t(u32, VIRTIO_VSOCK_MAX_PKT_BUF_SIZE,
>> > > (MAX_SKB_FRAGS * PAGE_SIZE));
>> > > +
>> > > + if (info->msg->msg_flags & MSG_ZEROCOPY &&
>> > > + info->op == VIRTIO_VSOCK_OP_RW) {
>> > > + uarg = info->msg->msg_ubuf;
>> > > +
>> > > + if (!uarg) {
>> > > + uarg = msg_zerocopy_realloc(sk_vsock(vsk),
>> > > + pkt_len, NULL, false);
>> > > + if (!uarg) {
>> > > + virtio_transport_put_credit(vvs, pkt_len);
>> > > + return -ENOMEM;
>> > > + }
>> > > +
>> > > + if (!can_zcopy)
>> > > + uarg_to_msgzc(uarg)->zerocopy = 0;
>> > > +
>> > > + have_uref = true;
>> > > + }
>> > > + }
>> >
>> > Surely that block should only be done if can_zcopy is true?
>> > And shouldn't something unset it if info->op != VIRTIO_VSOCK_OP_RW ?
>> > If the msg_zerocopy_realloc() fails then can't you just set can_zcopy to false.
>> >
>> > It info->msg->msg_buf is already set then I think you have to disable zero-copy.
>> > The caller has already requested a callback - and you can't add another.
>> >
>> > In any case by the end of this can_zcopy and have_uref are really the same flag.
>>
>> I kept the same approach we had before, trying to make as few changes as
>> possible.
>>
>> All these potential issues seem to be pre-existing and should be eventually
>> addressed in other patches IMHO. This patch one only resolves the main issue
>> of calling `skb_zcopy_set()` for every skb to avoid leaking pages, etc.
>
>the patch is upstream now, right? So pretty much have to be patches on
>top.
If those are actual issues, then yes. TBH, I didn’t look into that
aspect and left it as it was before. We should take a closer look at how
MSG_ZEROCOPY is handled.
David, if you think it needs fixing and you have time, feel free to send
patches on top.
Thanks,
Stefano
>
>> @Arseniy can you help on this?
>>
>> >
>> > > }
>> > >
>> > > rest_len = pkt_len;
>> > > @@ -378,27 +371,7 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>> > > break;
>> > > }
>> > >
>> > > - /* We process buffer part by part, allocating skb on
>> > > - * each iteration. If this is last skb for this buffer
>> > > - * and MSG_ZEROCOPY mode is in use - we must allocate
>> > > - * completion for the current syscall.
>> > > - *
>> > > - * Pass pkt_len because msg iter is already consumed
>> > > - * by virtio_transport_fill_skb(), so iter->count
>> > > - * can not be used for RLIMIT_MEMLOCK pinned-pages
>> > > - * accounting done by msg_zerocopy_realloc().
>> > > - */
>> > > - if (info->msg && info->msg->msg_flags & MSG_ZEROCOPY &&
>> > > - skb_len == rest_len && info->op == VIRTIO_VSOCK_OP_RW) {
>> > > - if (virtio_transport_init_zcopy_skb(vsk, skb,
>> > > - info->msg,
>> > > - pkt_len,
>> > > - can_zcopy)) {
>> > > - kfree_skb(skb);
>> > > - ret = -ENOMEM;
>> > > - break;
>> > > - }
>> > > - }
>> > > + skb_zcopy_set(skb, uarg, NULL);
>> > >
>> > > virtio_transport_inc_tx_pkt(vvs, skb);
>> > >
>> > > @@ -422,6 +395,18 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>> > >
>> > > virtio_transport_put_credit(vvs, rest_len);
>> > >
>> > > + /* msg_zerocopy_realloc() initializes the ubuf_info refcnt to 1.
>> > > + * skb_zcopy_set() increases it for each skb, so we can drop that
>> > ^ must
>> >
>> > > + * initial reference to keep it balanced.
>> > > + */
>> > > + if (have_uref) {
>> > > + if (rest_len == pkt_len)
>> > > + /* No data sent, abort the notification. */
>> > > + net_zcopy_put_abort(uarg, true);
>> >
>> > Is it worth optimising for the 'nothing sent' case ?
>>
>> What do you suggest doing?
>>
>> I followed what TCP does.
>>
>> Thanks,
>> Stefano
>>
>> >
>> > -- David
>> >
>> > > + else
>> > > + net_zcopy_put(uarg);
>> > > + }
>> > > +
>> > > /* Return number of bytes, if any data has been sent. */
>> > > if (rest_len != pkt_len)
>> > > ret = pkt_len - rest_len;
>> >
>
^ permalink raw reply
* Re: [PATCH net] vsock: keep poll shutdown state consistent
From: Stefano Garzarella @ 2026-05-18 10:16 UTC (permalink / raw)
To: Ziyu Zhang
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Andy King, George Zhang, Dmitry Torokhov,
virtualization, netdev, linux-kernel, baijiaju1990, r33s3n6,
gality369, zhenghaoran154, hanguidong02, zzzccc427
In-Reply-To: <20260516034745.260442-1-ziyuzhang201@gmail.com>
On Sat, May 16, 2026 at 11:47:45AM +0800, Ziyu Zhang wrote:
>vsock_poll() reads vsk->peer_shutdown before taking the socket
>lock to set EPOLLHUP and EPOLLRDHUP, then reads it again under the
>lock to report EOF readability. A shutdown packet can update
>peer_shutdown while poll is waiting for the lock, so one poll invocation
>can report EPOLLIN without the corresponding HUP/RDHUP bits.
>
>Keep non-connectible sockets on a single lockless READ_ONCE()
Should this be paired with WRITE_ONCE() on writers?
>snapshot. For connectible sockets, defer shutdown-derived poll bits
>until after lock_sock() and use one READ_ONCE() snapshot for both EOF
>readability and HUP/RDHUP. This preserves shutdowns that arrive before
>the lock is acquired and keeps all peer-shutdown-derived bits consistent
>for a poll pass.
>
>Fixes: d021c344051a ("VSOCK: Introduce VM Sockets")
>Signed-off-by: Ziyu Zhang <ziyuzhang201@gmail.com>
>---
> net/vmw_vsock/af_vsock.c | 40 ++++++++++++++++++++++++++--------------
> 1 file changed, 26 insertions(+), 14 deletions(-)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index adcba1b7b..bed42347b 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -1122,6 +1122,25 @@ static int vsock_shutdown(struct socket *sock, int mode)
> return err;
> }
>
>+static __poll_t vsock_poll_shutdown(struct sock *sk, u32 peer_shutdown)
>+{
>+ __poll_t mask = 0;
>+
>+ /* INET sockets treat local write shutdown and peer write shutdown as a
>+ * case of EPOLLHUP set.
>+ */
>+ if (sk->sk_shutdown == SHUTDOWN_MASK ||
>+ ((sk->sk_shutdown & SEND_SHUTDOWN) &&
>+ (peer_shutdown & SEND_SHUTDOWN)))
>+ mask |= EPOLLHUP;
>+
>+ if (sk->sk_shutdown & RCV_SHUTDOWN ||
>+ peer_shutdown & SEND_SHUTDOWN)
>+ mask |= EPOLLRDHUP;
>+
>+ return mask;
>+}
>+
> static __poll_t vsock_poll(struct file *file, struct socket *sock,
> poll_table *wait)
> {
>@@ -1139,19 +1158,9 @@ static __poll_t vsock_poll(struct file *file, struct socket *sock,
> /* Signify that there has been an error on this socket. */
> mask |= EPOLLERR;
>
>- /* INET sockets treat local write shutdown and peer write shutdown as a
>- * case of EPOLLHUP set.
>- */
>- if ((sk->sk_shutdown == SHUTDOWN_MASK) ||
>- ((sk->sk_shutdown & SEND_SHUTDOWN) &&
>- (vsk->peer_shutdown & SEND_SHUTDOWN))) {
>- mask |= EPOLLHUP;
>- }
>-
>- if (sk->sk_shutdown & RCV_SHUTDOWN ||
>- vsk->peer_shutdown & SEND_SHUTDOWN) {
>- mask |= EPOLLRDHUP;
>- }
>+ if (!sock_type_connectible(sk->sk_type))
>+ mask |= vsock_poll_shutdown(sk,
>+ READ_ONCE(vsk->peer_shutdown));
Can we move this in the `if (sock->type == SOCK_DGRAM)` branch ?
Not a strong opinion about that, but in any case IMO we should add a
comment here to explain why we are doing only for not connectible
sockets.
That said, if we use WRITE_ONCE in the writers, do we really need to
move this after the lock_sock for the connectable ones?
>
> if (sk_is_readable(sk))
> mask |= EPOLLIN | EPOLLRDNORM;
>@@ -1171,6 +1180,7 @@ static __poll_t vsock_poll(struct file *file, struct socket *sock,
>
> } else if (sock_type_connectible(sk->sk_type)) {
> const struct vsock_transport *transport;
>+ u32 peer_shutdown;
>
> lock_sock(sk);
>
>@@ -1203,10 +1213,12 @@ static __poll_t vsock_poll(struct file *file, struct socket *sock,
> * terminated should also be considered read, and we check the
> * shutdown flag for that.
> */
>+ peer_shutdown = READ_ONCE(vsk->peer_shutdown);
> if (sk->sk_shutdown & RCV_SHUTDOWN ||
>- vsk->peer_shutdown & SEND_SHUTDOWN) {
>+ peer_shutdown & SEND_SHUTDOWN) {
> mask |= EPOLLIN | EPOLLRDNORM;
> }
>+ mask |= vsock_poll_shutdown(sk, peer_shutdown);
nit: to keep the order the same as before, I would move this call just
before this `if` block, but I don't think it makes any difference in the
end.
>
> /* Connected sockets that can produce data can be written. */
> if (transport && sk->sk_state == TCP_ESTABLISHED) {
>--
>2.43.0
>
^ 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