* [RFC PATCH 4/4] virtio: add SQ/CQ polling mode support for vhost-scsi
From: rom.wang @ 2026-07-20 14:10 UTC (permalink / raw)
To: mst, jasowangio, eperezma, stefanha
Cc: virtualization, linux-kernel, linux-scsi, kvm, qemu-devel,
Yufeng Wang
In-Reply-To: <20260720141040.181946-1-r4o5m6e8o@163.com>
From: Yufeng Wang <wangyufeng@kylinos.cn>
Implement the QEMU-side bridge layer for virtio SQ/CQ polling mode,
which replaces interrupt-based virtio notifications with io_uring-style
doorbell polling to eliminate VM exits in high-throughput scenarios.
UAPI headers: sync VIRTIO_F_SQCQ_POLL (bit 42), PCI common config
offsets (64-76), struct vring_sq/vring_cq, struct vhost_vring_sqcq_addr,
and VHOST_SET_VRING_SQCQ_ADDR ioctl (0x27) from Linux kernel.
VirtIO core: add sq/cq fields to VRing, with getter/setter functions.
PCI transport: handle SQE/CQE register read/write in common config,
gate SQ/CQ address forwarding on VIRTIO_F_SQCQ_POLL negotiation in
Q_ENABLE handler.
Vhost backend: add vhost_set_vring_sqcq_addr_op to VhostOps, implement
kernel backend via VHOST_SET_VRING_SQCQ_ADDR ioctl.
Vhost core: map SQ/CQ GPAs to HVAs in vhost_virtqueue_start() after
VHOST_SET_VRING_ADDR, call the new ioctl before VHOST_SET_VRING_KICK;
unmap in do_vhost_virtqueue_stop().
vhost-scsi: advertise VIRTIO_F_SQCQ_POLL in kernel_feature_bits[].
Signed-off-by: Yufeng Wang <wangyufeng@kylinos.cn>
---
hw/scsi/vhost-scsi.c | 4 ++
hw/virtio/vhost-backend.c | 7 +++
hw/virtio/vhost.c | 54 +++++++++++++++++++
hw/virtio/virtio-pci.c | 31 +++++++++++
hw/virtio/virtio.c | 18 +++++++
include/hw/virtio/vhost-backend.h | 4 ++
include/hw/virtio/vhost.h | 6 +++
include/hw/virtio/virtio-pci.h | 2 +
include/hw/virtio/virtio.h | 3 ++
include/standard-headers/linux/vhost_types.h | 7 +++
.../standard-headers/linux/virtio_config.h | 5 +-
include/standard-headers/linux/virtio_pci.h | 4 ++
include/standard-headers/linux/virtio_ring.h | 30 +++++++++++
| 4 ++
14 files changed, 178 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 699863c..bb7f3e8 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -40,6 +40,7 @@ static const int kernel_feature_bits[] = {
VIRTIO_F_RING_RESET,
VIRTIO_F_IN_ORDER,
VIRTIO_F_NOTIFICATION_DATA,
+ VIRTIO_F_SQCQ_POLL,
VHOST_INVALID_FEATURE_BIT
};
@@ -361,6 +362,9 @@ static const Property vhost_scsi_properties[] = {
DEFINE_PROP_BIT64("hotplug", VHostSCSICommon, host_features,
VIRTIO_SCSI_F_HOTPLUG,
false),
+ DEFINE_PROP_BIT64("sqcq_poll", VHostSCSICommon, host_features,
+ VIRTIO_F_SQCQ_POLL,
+ true),
DEFINE_PROP_BOOL("migratable", VHostSCSICommon, migratable, false),
DEFINE_PROP_BOOL("worker_per_virtqueue", VirtIOSCSICommon,
conf.worker_per_virtqueue, false),
diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 4367db0..a71b30b 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -115,6 +115,12 @@ static int vhost_kernel_set_vring_addr(struct vhost_dev *dev,
return vhost_kernel_call(dev, VHOST_SET_VRING_ADDR, addr);
}
+static int vhost_kernel_set_vring_sqcq_addr(struct vhost_dev *dev,
+ struct vhost_vring_sqcq_addr *addr)
+{
+ return vhost_kernel_call(dev, VHOST_SET_VRING_SQCQ_ADDR, addr);
+}
+
static int vhost_kernel_set_vring_endian(struct vhost_dev *dev,
struct vhost_vring_state *ring)
{
@@ -368,6 +374,7 @@ const VhostOps kernel_ops = {
.vhost_set_log_base = vhost_kernel_set_log_base,
.vhost_set_mem_table = vhost_kernel_set_mem_table,
.vhost_set_vring_addr = vhost_kernel_set_vring_addr,
+ .vhost_set_vring_sqcq_addr = vhost_kernel_set_vring_sqcq_addr,
.vhost_set_vring_endian = vhost_kernel_set_vring_endian,
.vhost_set_vring_num = vhost_kernel_set_vring_num,
.vhost_set_vring_base = vhost_kernel_set_vring_base,
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 31e9704..8c82f98 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -22,6 +22,7 @@
#include "qemu/memfd.h"
#include "qemu/log.h"
#include "standard-headers/linux/vhost_types.h"
+#include "standard-headers/linux/virtio_ring.h"
#include "hw/virtio/virtio-bus.h"
#include "hw/mem/memory-device.h"
#include "migration/blocker.h"
@@ -1337,6 +1338,49 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
goto fail_alloc;
}
+ /* SQ/CQ polling: map and pass to vhost if feature negotiated */
+ vq->sq = NULL;
+ vq->cq = NULL;
+ vq->sq_phys = virtio_queue_get_sq_addr(vdev, idx);
+ vq->cq_phys = virtio_queue_get_cq_addr(vdev, idx);
+
+ if (vq->sq_phys && vq->cq_phys &&
+ dev->vhost_ops->vhost_set_vring_sqcq_addr &&
+ virtio_has_feature(vdev->guest_features, VIRTIO_F_SQCQ_POLL)) {
+ struct vhost_vring_sqcq_addr sqcq_addr;
+
+ l = sizeof(struct vring_sq);
+ vq->sq = vhost_memory_map(dev, vq->sq_phys, &l, false);
+ if (!vq->sq) {
+ r = -ENOMEM;
+ goto fail_alloc;
+ }
+ vq->sq_size = l;
+
+ l = sizeof(struct vring_cq);
+ vq->cq = vhost_memory_map(dev, vq->cq_phys, &l, false);
+ if (!vq->cq) {
+ r = -ENOMEM;
+ vhost_memory_unmap(dev, vq->sq, vq->sq_size, 0, 0);
+ goto fail_alloc;
+ }
+ vq->cq_size = l;
+
+ memset(&sqcq_addr, 0, sizeof(sqcq_addr));
+ sqcq_addr.index = vhost_vq_index;
+ sqcq_addr.sq_user_addr = (uint64_t)(unsigned long)vq->sq;
+ sqcq_addr.cq_user_addr = (uint64_t)(unsigned long)vq->cq;
+ r = dev->vhost_ops->vhost_set_vring_sqcq_addr(dev, &sqcq_addr);
+ if (r < 0) {
+ VHOST_OPS_DEBUG(r, "vhost_set_vring_sqcq_addr failed");
+ vhost_memory_unmap(dev, vq->cq, vq->cq_size, 0, 0);
+ vhost_memory_unmap(dev, vq->sq, vq->sq_size, 0, 0);
+ vq->sq = NULL;
+ vq->cq = NULL;
+ goto fail_alloc;
+ }
+ }
+
file.fd = event_notifier_get_fd(virtio_queue_get_host_notifier(vvq));
r = dev->vhost_ops->vhost_set_vring_kick(dev, &file);
if (r) {
@@ -1425,6 +1469,16 @@ static int do_vhost_virtqueue_stop(struct vhost_dev *dev,
vhost_vq_index);
}
+ /* Unmap SQ/CQ if mapped */
+ if (vq->sq) {
+ vhost_memory_unmap(dev, vq->sq, vq->sq_size, 0, 0);
+ vq->sq = NULL;
+ }
+ if (vq->cq) {
+ vhost_memory_unmap(dev, vq->cq, vq->cq_size, 0, 0);
+ vq->cq = NULL;
+ }
+
vhost_memory_unmap(dev, vq->used, virtio_queue_get_used_size(vdev, idx),
1, virtio_queue_get_used_size(vdev, idx));
vhost_memory_unmap(dev, vq->avail, virtio_queue_get_avail_size(vdev, idx),
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index b273eb2..c88cba3 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1623,6 +1623,18 @@ static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
case VIRTIO_PCI_COMMON_Q_RESET:
val = proxy->vqs[vdev->queue_sel].reset;
break;
+ case VIRTIO_PCI_COMMON_SQE_LO:
+ val = proxy->vqs[vdev->queue_sel].sq[0];
+ break;
+ case VIRTIO_PCI_COMMON_SQE_HI:
+ val = proxy->vqs[vdev->queue_sel].sq[1];
+ break;
+ case VIRTIO_PCI_COMMON_CQE_LO:
+ val = proxy->vqs[vdev->queue_sel].cq[0];
+ break;
+ case VIRTIO_PCI_COMMON_CQE_HI:
+ val = proxy->vqs[vdev->queue_sel].cq[1];
+ break;
default:
val = 0;
}
@@ -1727,6 +1739,13 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr,
proxy->vqs[vdev->queue_sel].avail[0],
((uint64_t)proxy->vqs[vdev->queue_sel].used[1]) << 32 |
proxy->vqs[vdev->queue_sel].used[0]);
+ if (virtio_has_feature(vdev->guest_features, VIRTIO_F_SQCQ_POLL)) {
+ virtio_queue_set_sqcq(vdev, vdev->queue_sel,
+ ((uint64_t)proxy->vqs[vdev->queue_sel].sq[1] << 32) |
+ proxy->vqs[vdev->queue_sel].sq[0],
+ ((uint64_t)proxy->vqs[vdev->queue_sel].cq[1] << 32) |
+ proxy->vqs[vdev->queue_sel].cq[0]);
+ }
proxy->vqs[vdev->queue_sel].enabled = 1;
proxy->vqs[vdev->queue_sel].reset = 0;
virtio_queue_enable(vdev, vdev->queue_sel);
@@ -1752,6 +1771,18 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr,
case VIRTIO_PCI_COMMON_Q_USEDHI:
proxy->vqs[vdev->queue_sel].used[1] = val;
break;
+ case VIRTIO_PCI_COMMON_SQE_LO:
+ proxy->vqs[vdev->queue_sel].sq[0] = val;
+ break;
+ case VIRTIO_PCI_COMMON_SQE_HI:
+ proxy->vqs[vdev->queue_sel].sq[1] = val;
+ break;
+ case VIRTIO_PCI_COMMON_CQE_LO:
+ proxy->vqs[vdev->queue_sel].cq[0] = val;
+ break;
+ case VIRTIO_PCI_COMMON_CQE_HI:
+ proxy->vqs[vdev->queue_sel].cq[1] = val;
+ break;
case VIRTIO_PCI_COMMON_Q_RESET:
if (val == 1) {
proxy->vqs[vdev->queue_sel].reset = 1;
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 3dc9423..42e39d9 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -111,6 +111,8 @@ typedef struct VRing
hwaddr desc;
hwaddr avail;
hwaddr used;
+ hwaddr sq;
+ hwaddr cq;
VRingMemoryRegionCaches *caches;
} VRing;
@@ -2400,6 +2402,12 @@ void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc,
virtio_init_region_cache(vdev, n);
}
+void virtio_queue_set_sqcq(VirtIODevice *vdev, int n, hwaddr sq, hwaddr cq)
+{
+ vdev->vq[n].vring.sq = sq;
+ vdev->vq[n].vring.cq = cq;
+}
+
void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
{
/* Don't allow guest to flip queue between existent and
@@ -3661,6 +3669,16 @@ hwaddr virtio_queue_get_used_addr(VirtIODevice *vdev, int n)
return vdev->vq[n].vring.used;
}
+hwaddr virtio_queue_get_sq_addr(VirtIODevice *vdev, int n)
+{
+ return vdev->vq[n].vring.sq;
+}
+
+hwaddr virtio_queue_get_cq_addr(VirtIODevice *vdev, int n)
+{
+ return vdev->vq[n].vring.cq;
+}
+
hwaddr virtio_queue_get_desc_size(VirtIODevice *vdev, int n)
{
return sizeof(VRingDesc) * vdev->vq[n].vring.num;
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index ff94fa1..911cd15 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -45,6 +45,7 @@ struct vhost_memory;
struct vhost_vring_file;
struct vhost_vring_state;
struct vhost_vring_addr;
+struct vhost_vring_sqcq_addr;
struct vhost_vring_worker;
struct vhost_worker_state;
struct vhost_scsi_target;
@@ -71,6 +72,8 @@ typedef int (*vhost_set_mem_table_op)(struct vhost_dev *dev,
struct vhost_memory *mem);
typedef int (*vhost_set_vring_addr_op)(struct vhost_dev *dev,
struct vhost_vring_addr *addr);
+typedef int (*vhost_set_vring_sqcq_addr_op)(struct vhost_dev *dev,
+ struct vhost_vring_sqcq_addr *addr);
typedef int (*vhost_set_vring_endian_op)(struct vhost_dev *dev,
struct vhost_vring_state *ring);
typedef int (*vhost_set_vring_num_op)(struct vhost_dev *dev,
@@ -178,6 +181,7 @@ typedef struct VhostOps {
vhost_set_log_base_op vhost_set_log_base;
vhost_set_mem_table_op vhost_set_mem_table;
vhost_set_vring_addr_op vhost_set_vring_addr;
+ vhost_set_vring_sqcq_addr_op vhost_set_vring_sqcq_addr;
vhost_set_vring_endian_op vhost_set_vring_endian;
vhost_set_vring_num_op vhost_set_vring_num;
vhost_set_vring_base_op vhost_set_vring_base;
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 08bbb4d..65056a0 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -27,6 +27,8 @@ struct vhost_virtqueue {
void *desc;
void *avail;
void *used;
+ void *sq; /* Mapped SQ doorbell */
+ void *cq; /* Mapped CQ doorbell */
int num;
unsigned long long desc_phys;
unsigned desc_size;
@@ -34,6 +36,10 @@ struct vhost_virtqueue {
unsigned avail_size;
unsigned long long used_phys;
unsigned used_size;
+ unsigned long long sq_phys; /* GPA of SQ doorbell */
+ unsigned long long cq_phys; /* GPA of CQ doorbell */
+ unsigned sq_size; /* Size of mapped SQ region */
+ unsigned cq_size; /* Size of mapped CQ region */
EventNotifier masked_notifier;
EventNotifier error_notifier;
EventNotifier masked_config_notifier;
diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 6397529..37103c7 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -122,6 +122,8 @@ typedef struct VirtIOPCIQueue {
uint32_t desc[2];
uint32_t avail[2];
uint32_t used[2];
+ uint32_t sq[2];
+ uint32_t cq[2];
} VirtIOPCIQueue;
struct VirtIOPCIProxy {
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 27cd98d..29f21f4 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -361,6 +361,7 @@ int virtio_queue_get_max_num(VirtIODevice *vdev, int n);
int virtio_get_num_queues(VirtIODevice *vdev);
void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc,
hwaddr avail, hwaddr used);
+void virtio_queue_set_sqcq(VirtIODevice *vdev, int n, hwaddr sq, hwaddr cq);
void virtio_queue_update_rings(VirtIODevice *vdev, int n);
void virtio_init_region_cache(VirtIODevice *vdev, int n);
void virtio_queue_set_align(VirtIODevice *vdev, int n, int align);
@@ -408,6 +409,8 @@ bool virtio_queue_enabled_legacy(VirtIODevice *vdev, int n);
bool virtio_queue_enabled(VirtIODevice *vdev, int n);
hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
hwaddr virtio_queue_get_used_addr(VirtIODevice *vdev, int n);
+hwaddr virtio_queue_get_sq_addr(VirtIODevice *vdev, int n);
+hwaddr virtio_queue_get_cq_addr(VirtIODevice *vdev, int n);
hwaddr virtio_queue_get_desc_size(VirtIODevice *vdev, int n);
hwaddr virtio_queue_get_avail_size(VirtIODevice *vdev, int n);
hwaddr virtio_queue_get_used_size(VirtIODevice *vdev, int n);
diff --git a/include/standard-headers/linux/vhost_types.h b/include/standard-headers/linux/vhost_types.h
index 79b53a9..ea96feb 100644
--- a/include/standard-headers/linux/vhost_types.h
+++ b/include/standard-headers/linux/vhost_types.h
@@ -151,6 +151,13 @@ struct vhost_scsi_target {
unsigned short reserved;
};
+/* SQ/CQ polling mode */
+struct vhost_vring_sqcq_addr {
+ unsigned int index;
+ __u64 sq_user_addr; /* SQ doorbell HVA */
+ __u64 cq_user_addr; /* CQ doorbell HVA */
+};
+
/* VHOST_VDPA specific definitions */
struct vhost_vdpa_config {
diff --git a/include/standard-headers/linux/virtio_config.h b/include/standard-headers/linux/virtio_config.h
index 45be0fa..d73938e 100644
--- a/include/standard-headers/linux/virtio_config.h
+++ b/include/standard-headers/linux/virtio_config.h
@@ -52,7 +52,7 @@
* rest are per-device feature bits.
*/
#define VIRTIO_TRANSPORT_F_START 28
-#define VIRTIO_TRANSPORT_F_END 42
+#define VIRTIO_TRANSPORT_F_END 43
#ifndef VIRTIO_CONFIG_NO_LEGACY
/* Do we get callbacks when the ring is completely used, even if we've
@@ -118,4 +118,7 @@
*/
#define VIRTIO_F_ADMIN_VQ 41
+/* SQ/CQ polling mode for io_uring-style doorbell polling */
+#define VIRTIO_F_SQCQ_POLL 42
+
#endif /* _LINUX_VIRTIO_CONFIG_H */
diff --git a/include/standard-headers/linux/virtio_pci.h b/include/standard-headers/linux/virtio_pci.h
index 4c82513..cc585b6 100644
--- a/include/standard-headers/linux/virtio_pci.h
+++ b/include/standard-headers/linux/virtio_pci.h
@@ -235,6 +235,10 @@ struct virtio_pci_cfg_cap {
#define VIRTIO_PCI_COMMON_Q_RESET 58
#define VIRTIO_PCI_COMMON_ADM_Q_IDX 60
#define VIRTIO_PCI_COMMON_ADM_Q_NUM 62
+#define VIRTIO_PCI_COMMON_SQE_LO 64
+#define VIRTIO_PCI_COMMON_SQE_HI 68
+#define VIRTIO_PCI_COMMON_CQE_LO 72
+#define VIRTIO_PCI_COMMON_CQE_HI 76
#endif /* VIRTIO_PCI_NO_MODERN */
diff --git a/include/standard-headers/linux/virtio_ring.h b/include/standard-headers/linux/virtio_ring.h
index 22f6eb8..520b758 100644
--- a/include/standard-headers/linux/virtio_ring.h
+++ b/include/standard-headers/linux/virtio_ring.h
@@ -245,4 +245,34 @@ struct vring_packed_desc {
uint16_t flags;
};
+/*
+ * struct vring_sq - Submission Queue doorbell for polling mode.
+ * Placed alongside the standard virtio split ring to enable
+ * kick-less notification from guest to host.
+ *
+ * @idx: Producer index (guest updates, corresponds to avail->idx)
+ * @sq_need_wakeup: device requests guest to kick when sleeping
+ * @reserved: Reserved for future use, pad to cache-line
+ */
+struct vring_sq {
+ __virtio64 idx;
+ uint8_t sq_need_wakeup;
+ uint8_t reserved[55];
+} __attribute__((aligned(128)));
+
+/*
+ * struct vring_cq - Completion Queue doorbell for polling mode.
+ * Placed alongside the standard virtio split ring to enable
+ * interrupt-less notification from host to guest.
+ *
+ * @idx: Producer index (device updates, corresponds to used->idx)
+ * @cq_need_wakeup: guest requests host to signal when sleeping
+ * @reserved: Reserved for future use, pad to cache-line
+ */
+struct vring_cq {
+ __virtio64 idx;
+ uint8_t cq_need_wakeup;
+ uint8_t reserved[55];
+} __attribute__((aligned(128)));
+
#endif /* _LINUX_VIRTIO_RING_H */
--git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h
index c57674a..746d71f 100644
--- a/linux-headers/linux/vhost.h
+++ b/linux-headers/linux/vhost.h
@@ -123,6 +123,10 @@
#define VHOST_SET_BACKEND_FEATURES _IOW(VHOST_VIRTIO, 0x25, __u64)
#define VHOST_GET_BACKEND_FEATURES _IOR(VHOST_VIRTIO, 0x26, __u64)
+/* Set SQ/CQ doorbell addresses for polling mode */
+#define VHOST_SET_VRING_SQCQ_ADDR _IOW(VHOST_VIRTIO, 0x27, \
+ struct vhost_vring_sqcq_addr)
+
/* VHOST_NET specific defines */
/* Attach virtio net ring to a raw socket, or tap device.
--
2.34.1
^ permalink raw reply related
* [RFC PATCH 3/4] virtio: guest driver support for SQ/CQ polling
From: rom.wang @ 2026-07-20 14:10 UTC (permalink / raw)
To: mst, jasowangio, eperezma, stefanha
Cc: virtualization, linux-kernel, linux-scsi, kvm, qemu-devel,
Yufeng Wang
In-Reply-To: <20260720141040.181946-1-r4o5m6e8o@163.com>
From: Yufeng Wang <wangyufeng@kylinos.cn>
Implement SQ/CQ doorbell polling in the guest kernel:
- virtio_ring.c: SQ/CQ DMA allocation, virtqueue_notify() writes
sq->idx via smp_store_release instead of MMIO kick; completion
detection via more_used() (direct used-ring check)
- virtio_sqcq_poll.c: per-device CQ poll thread with need_resched()
cooperative spin, time-based spin budget, 1ms idle sleep
- virtio_pci_modern.c: feature negotiation, PCI config space write
for SQ/CQ DMA addresses, poll thread lifecycle
- virtio.c: reject VIRTIO_F_SQCQ_POLL + VIRTIO_F_RING_PACKED
- virtio_scsi.c: 10s window io_stats diagnostics output
- do_softirq() after each completion callback (no hardware interrupt)
Signed-off-by: Yufeng Wang <wangyufeng@kylinos.cn>
---
drivers/scsi/virtio_scsi.c | 113 ++++++++-
drivers/virtio/Makefile | 2 +-
drivers/virtio/virtio.c | 10 +
drivers/virtio/virtio_pci_common.c | 3 +
drivers/virtio/virtio_pci_modern.c | 39 +++
drivers/virtio/virtio_pci_modern_dev.c | 34 ++-
drivers/virtio/virtio_ring.c | 262 +++++++++++++++++++
drivers/virtio/virtio_sqcq_poll.c | 338 +++++++++++++++++++++++++
include/linux/virtio.h | 25 ++
include/linux/virtio_pci_modern.h | 2 +
10 files changed, 825 insertions(+), 3 deletions(-)
create mode 100644 drivers/virtio/virtio_sqcq_poll.c
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 0ed8558dad72..f803bb62d22a 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -30,6 +30,7 @@
#include <scsi/scsi_devinfo.h>
#include <linux/seqlock.h>
#include <linux/dma-mapping.h>
+#include <linux/ktime.h>
#include "sd.h"
@@ -46,6 +47,8 @@ MODULE_PARM_DESC(virtscsi_poll_queues,
struct virtio_scsi_cmd {
struct scsi_cmnd *sc;
struct completion *comp;
+ u64 submit_ns; /* ktime_get_ns() at submission */
+ u64 notify_ns; /* ktime_get_ns() after SQ doorbell */
union {
struct virtio_scsi_cmd_req cmd;
struct virtio_scsi_cmd_req_pi cmd_pi;
@@ -95,6 +98,22 @@ struct virtio_scsi {
struct virtio_scsi_event events[VIRTIO_SCSI_EVENT_LEN];
__dma_from_device_group_end();
+ /* I/O latency statistics (debugging) */
+ spinlock_t lat_lock;
+ u64 lat_sum_ns;
+ u64 lat_min_ns;
+ u64 lat_max_ns;
+ u64 lat_count;
+ u64 host_sum_ns; /* notify → cq_detect (host transit) */
+ u64 cb_wait_sum_ns; /* cq_detect → process (poll sched latency) */
+ u64 det_count;
+ u64 last_cq_detect_ns; /* timestamp of last CQ detection */
+ unsigned long lat_last_print;
+
+ /* Completion interval tracking (matches host interval metric) */
+ u64 last_complete_ns; /* ktime of last completion */
+ u64 ema_cmp_int_ns; /* EMA of completion-to-completion interval */
+
struct virtio_scsi_vq req_vqs[];
};
@@ -122,6 +141,7 @@ static void virtscsi_complete_cmd(struct virtio_scsi *vscsi, void *buf)
struct virtio_scsi_cmd *cmd = buf;
struct scsi_cmnd *sc = cmd->sc;
struct virtio_scsi_cmd_resp *resp = &cmd->resp.cmd;
+ u64 cb_entry_ns = ktime_get_ns();
dev_dbg(&sc->device->sdev_gendev,
"cmd %p response %u status %#02x sense_len %u\n",
@@ -175,6 +195,88 @@ static void virtscsi_complete_cmd(struct virtio_scsi *vscsi, void *buf)
VIRTIO_SCSI_SENSE_SIZE));
}
+ /* I/O latency tracking */
+ if (cmd->submit_ns) {
+ u64 now = ktime_get_ns();
+ u64 lat = now - cmd->submit_ns;
+ unsigned long flags;
+
+ spin_lock_irqsave(&vscsi->lat_lock, flags);
+ vscsi->lat_sum_ns += lat;
+ vscsi->lat_count++;
+ if (lat < vscsi->lat_min_ns || vscsi->lat_min_ns == 0)
+ vscsi->lat_min_ns = lat;
+ if (lat > vscsi->lat_max_ns)
+ vscsi->lat_max_ns = lat;
+
+ /* Completion interval EMA (matches host interval metric) */
+ if (vscsi->last_complete_ns) {
+ u64 cmp_int = now - vscsi->last_complete_ns;
+
+ if (vscsi->ema_cmp_int_ns == 0)
+ vscsi->ema_cmp_int_ns = cmp_int;
+ else
+ vscsi->ema_cmp_int_ns =
+ vscsi->ema_cmp_int_ns -
+ vscsi->ema_cmp_int_ns / 8 +
+ cmp_int / 8;
+ }
+ vscsi->last_complete_ns = now;
+
+ if (cmd->notify_ns && vscsi->last_cq_detect_ns) {
+ u64 host_lat = vscsi->last_cq_detect_ns -
+ cmd->notify_ns;
+ u64 cb_wait = cb_entry_ns -
+ vscsi->last_cq_detect_ns;
+
+ if (host_lat < 10ULL * NSEC_PER_SEC) {
+ vscsi->host_sum_ns += host_lat;
+ vscsi->cb_wait_sum_ns += cb_wait;
+ vscsi->det_count++;
+ }
+ }
+
+ /* 10s window — single consolidated output */
+ if (time_after(jiffies, vscsi->lat_last_print +
+ msecs_to_jiffies(10000)) &&
+ vscsi->lat_count > 0) {
+ u64 avg_lat = vscsi->lat_sum_ns / vscsi->lat_count;
+ u64 host_lat = vscsi->det_count > 0 ?
+ vscsi->host_sum_ns / vscsi->det_count : 0;
+ u64 poll_lat = vscsi->det_count > 0 ?
+ vscsi->cb_wait_sum_ns / vscsi->det_count : 0;
+ u64 interval = 0;
+ unsigned int wk_kick;
+ unsigned long sleep_cnt;
+
+ wk_kick = virtqueue_get_sq_notify_stats(
+ vscsi->req_vqs[0].vq, &interval);
+ sleep_cnt = virtio_sqcq_poll_get_sleep_count(vscsi->vdev);
+
+ dev_info(&vscsi->vdev->dev,
+ "io_stats: cq=%llu avg_lat=%lluns host_lat=%lluns poll_lat=%lluns min_lat=%lluns max_lat=%lluns interval=%lluns cmp_interval=%lluns wk_kick=%u sleep=%lu\n",
+ vscsi->lat_count,
+ avg_lat,
+ host_lat,
+ poll_lat,
+ vscsi->lat_min_ns,
+ vscsi->lat_max_ns,
+ interval,
+ vscsi->ema_cmp_int_ns,
+ wk_kick,
+ sleep_cnt);
+ vscsi->lat_sum_ns = 0;
+ vscsi->lat_count = 0;
+ vscsi->lat_min_ns = 0;
+ vscsi->lat_max_ns = 0;
+ vscsi->host_sum_ns = 0;
+ vscsi->cb_wait_sum_ns = 0;
+ vscsi->det_count = 0;
+ vscsi->lat_last_print = jiffies;
+ }
+ spin_unlock_irqrestore(&vscsi->lat_lock, flags);
+ }
+
scsi_done(sc);
}
@@ -204,6 +306,7 @@ static void virtscsi_req_done(struct virtqueue *vq)
int index = vq->index - VIRTIO_SCSI_VQ_BASE;
struct virtio_scsi_vq *req_vq = &vscsi->req_vqs[index];
+ vscsi->last_cq_detect_ns = virtqueue_get_cq_detect_ns(vq);
virtscsi_vq_done(vscsi, req_vq, virtscsi_complete_cmd);
};
@@ -514,8 +617,12 @@ static int virtscsi_add_cmd(struct virtio_scsi_vq *vq,
spin_unlock_irqrestore(&vq->vq_lock, flags);
- if (needs_kick)
+ if (needs_kick) {
+ cmd->notify_ns = ktime_get_ns();
virtqueue_notify(vq->vq);
+ } else {
+ cmd->notify_ns = 0;
+ }
return err;
}
@@ -605,6 +712,7 @@ static enum scsi_qc_status virtscsi_queuecommand(struct Scsi_Host *shost,
}
kick = (sc->flags & SCMD_LAST) != 0;
+ cmd->submit_ns = ktime_get_ns();
ret = virtscsi_add_cmd(req_vq, cmd, req_size, sizeof(cmd->resp.cmd), kick);
if (ret == -EIO) {
cmd->resp.cmd.response = VIRTIO_SCSI_S_BAD_TARGET;
@@ -895,6 +1003,9 @@ static int virtscsi_init(struct virtio_device *vdev,
virtscsi_init_vq(&vscsi->req_vqs[i - VIRTIO_SCSI_VQ_BASE],
vqs[i]);
+ spin_lock_init(&vscsi->lat_lock);
+ vscsi->lat_last_print = jiffies;
+
virtscsi_config_set(vdev, cdb_size, VIRTIO_SCSI_CDB_SIZE);
virtscsi_config_set(vdev, sense_size, VIRTIO_SCSI_SENSE_SIZE);
diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
index eefcfe90d6b8..769e95ee5be4 100644
--- a/drivers/virtio/Makefile
+++ b/drivers/virtio/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o
+obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o virtio_sqcq_poll.o
obj-$(CONFIG_VIRTIO_ANCHOR) += virtio_anchor.o
obj-$(CONFIG_VIRTIO_PCI_LIB) += virtio_pci_modern_dev.o
obj-$(CONFIG_VIRTIO_PCI_LIB_LEGACY) += virtio_pci_legacy_dev.o
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 5bdc6b82b30b..d440e920f8e6 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -221,6 +221,16 @@ static int virtio_features_ok(struct virtio_device *dev)
}
}
+ /* SQ/CQ poll only supports split ring; reject packed ring
+ * to avoid split-specific field access in virtqueue_notify().
+ */
+ if (virtio_has_feature(dev, VIRTIO_F_SQCQ_POLL) &&
+ virtio_has_feature(dev, VIRTIO_F_RING_PACKED)) {
+ dev_warn(&dev->dev,
+ "VIRTIO_F_SQCQ_POLL is incompatible with VIRTIO_F_RING_PACKED\n");
+ return -ENODEV;
+ }
+
if (!virtio_has_feature(dev, VIRTIO_F_VERSION_1))
return 0;
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index da97b6a988de..cd22651c73e2 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -268,6 +268,9 @@ void vp_del_vqs(struct virtio_device *vdev)
struct virtqueue *vq, *n;
int i;
+ /* Stop SQ/CQ poll thread before deleting virtqueues */
+ virtio_stop_sqcq_poll(vdev);
+
list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
info = vp_is_avq(vdev, vq->index) ? vp_dev->admin_vq.info :
vp_dev->vqs[vq->index];
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 6d8ae2a6a8ca..7a36ec4584a6 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -18,6 +18,7 @@
#include <linux/virtio_pci_admin.h>
#define VIRTIO_PCI_NO_LEGACY
#define VIRTIO_RING_NO_LEGACY
+
#include "virtio_pci_common.h"
#define VIRTIO_AVQ_SGS_MAX 4
@@ -378,6 +379,9 @@ static void vp_transport_features(struct virtio_device *vdev, u64 features)
if (features & BIT_ULL(VIRTIO_F_ADMIN_VQ))
__virtio_set_bit(vdev, VIRTIO_F_ADMIN_VQ);
+
+ if (features & BIT_ULL(VIRTIO_F_SQCQ_POLL))
+ __virtio_set_bit(vdev, VIRTIO_F_SQCQ_POLL);
}
static int __vp_check_common_size_one_feature(struct virtio_device *vdev, u32 fbit,
@@ -413,6 +417,9 @@ static int vp_check_common_size(struct virtio_device *vdev)
if (vp_check_common_size_one_feature(vdev, VIRTIO_F_ADMIN_VQ, admin_queue_num))
return -EINVAL;
+ if (vp_check_common_size_one_feature(vdev, VIRTIO_F_SQCQ_POLL, cqe_ring_hi))
+ return -EINVAL;
+
return 0;
}
@@ -578,6 +585,11 @@ static int vp_active_vq(struct virtqueue *vq, u16 msix_vec)
virtqueue_get_avail_addr(vq),
virtqueue_get_used_addr(vq));
+ if (virtqueue_sqcq_poll_active(vq))
+ vp_modern_queue_address_sqcq(mdev, index,
+ virtqueue_get_sq_dma_addr(vq),
+ virtqueue_get_cq_dma_addr(vq));
+
if (msix_vec != VIRTIO_MSI_NO_VECTOR) {
msix_vec = vp_modern_queue_vector(mdev, index, msix_vec);
if (msix_vec == VIRTIO_MSI_NO_VECTOR)
@@ -753,6 +765,27 @@ static int vp_modern_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
if (rc)
return rc;
+ /* Start SQ/CQ poll thread if feature negotiated */
+ if (virtio_has_feature(vdev, VIRTIO_F_SQCQ_POLL)) {
+ dev_info(&vdev->dev, "VIRTIO_F_SQCQ_POLL negotiated, starting poll thread\n");
+ rc = virtio_start_sqcq_poll(vdev, 1000, 1000);
+ if (rc)
+ goto err_del_vqs;
+
+ list_for_each_entry(vq, &vdev->vqs, list) {
+ if (!virtqueue_sqcq_poll_active(vq))
+ continue;
+ rc = virtio_sqcq_poll_register_vq(vdev, vq);
+ if (rc) {
+ dev_err(&vdev->dev,
+ "failed to register vq %d (%s) with poll thread: %d\n",
+ vq->index,
+ vq->name ? vq->name : "?", rc);
+ goto err_stop_poll;
+ }
+ }
+ }
+
/* Select and activate all queues. Has to be done last: once we do
* this, there's no way to go back except reset.
*/
@@ -760,6 +793,12 @@ static int vp_modern_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
vp_modern_set_queue_enable(&vp_dev->mdev, vq->index, true);
return 0;
+
+err_stop_poll:
+ virtio_stop_sqcq_poll(vdev);
+err_del_vqs:
+ vp_del_vqs(vdev);
+ return rc;
}
static void del_vq(struct virtio_pci_vq_info *info)
diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
index 413a8c353463..642b9a58751c 100644
--- a/drivers/virtio/virtio_pci_modern_dev.c
+++ b/drivers/virtio/virtio_pci_modern_dev.c
@@ -211,6 +211,14 @@ static inline void check_offsets(void)
offsetof(struct virtio_pci_modern_common_cfg, admin_queue_index));
BUILD_BUG_ON(VIRTIO_PCI_COMMON_ADM_Q_NUM !=
offsetof(struct virtio_pci_modern_common_cfg, admin_queue_num));
+ BUILD_BUG_ON(VIRTIO_PCI_COMMON_SQE_LO !=
+ offsetof(struct virtio_pci_modern_common_cfg, sqe_ring_lo));
+ BUILD_BUG_ON(VIRTIO_PCI_COMMON_SQE_HI !=
+ offsetof(struct virtio_pci_modern_common_cfg, sqe_ring_hi));
+ BUILD_BUG_ON(VIRTIO_PCI_COMMON_CQE_LO !=
+ offsetof(struct virtio_pci_modern_common_cfg, cqe_ring_lo));
+ BUILD_BUG_ON(VIRTIO_PCI_COMMON_CQE_HI !=
+ offsetof(struct virtio_pci_modern_common_cfg, cqe_ring_hi));
}
/*
@@ -300,7 +308,7 @@ int vp_modern_probe(struct virtio_pci_modern_device *mdev)
mdev->common = vp_modern_map_capability(mdev, common,
sizeof(struct virtio_pci_common_cfg), 4, 0,
offsetofend(struct virtio_pci_modern_common_cfg,
- admin_queue_num),
+ cqe_ring_hi),
&mdev->common_len, NULL);
if (!mdev->common)
goto err_map_common;
@@ -607,6 +615,30 @@ void vp_modern_queue_address(struct virtio_pci_modern_device *mdev,
}
EXPORT_SYMBOL_GPL(vp_modern_queue_address);
+/*
+ * vp_modern_queue_address_sqcq - set the SQ/CQ doorbell DMA addresses
+ * @mdev: the modern virtio-pci device
+ * @index: the queue index
+ * @sq_addr: DMA address of the SQ doorbell
+ * @cq_addr: DMA address of the CQ doorbell
+ */
+void vp_modern_queue_address_sqcq(struct virtio_pci_modern_device *mdev,
+ u16 index, u64 sq_addr, u64 cq_addr)
+{
+ struct virtio_pci_modern_common_cfg __iomem *cfg;
+
+ cfg = (struct virtio_pci_modern_common_cfg __iomem *)mdev->common;
+
+ vp_iowrite16(index, &cfg->cfg.queue_select);
+
+ vp_iowrite64_twopart(sq_addr, &cfg->sqe_ring_lo,
+ &cfg->sqe_ring_hi);
+
+ vp_iowrite64_twopart(cq_addr, &cfg->cqe_ring_lo,
+ &cfg->cqe_ring_hi);
+}
+EXPORT_SYMBOL_GPL(vp_modern_queue_address_sqcq);
+
/*
* vp_modern_set_queue_enable - enable a virtqueue
* @mdev: the modern virtio-pci device
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 335692d41617..a3aaf123ee74 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -270,10 +270,25 @@ struct vring_virtqueue {
bool last_add_time_valid;
ktime_t last_add_time;
#endif
+
+ /* SQ/CQ polling state (VIRTIO_F_SQCQ_POLL) */
+ bool sqcq_poll; /* Is SQ/CQ polling active? */
+ struct vring_sq *sq; /* DMA-coherent SQ doorbell */
+ struct vring_cq *cq; /* DMA-coherent CQ doorbell */
+ dma_addr_t sq_dma_addr; /* DMA address of SQ */
+ dma_addr_t cq_dma_addr; /* DMA address of CQ */
+ atomic_t sq_mmio_count; /* MMIO kicks (sq_need_wakeup=1) */
+ u64 last_cq_detect_ns; /* ktime_get_ns() when CQ work first detected */
+ u64 sq_last_notify_ns; /* ktime of last sq->idx store */
+ u64 sq_ema_interval_ns; /* EMA of sq->idx update interval (ns) */
};
static struct vring_desc_extra *vring_alloc_desc_extra(unsigned int num);
static void vring_free(struct virtqueue *_vq);
+static int virtio_alloc_sqcq(struct virtio_device *vdev,
+ struct vring_virtqueue *vq);
+static void virtio_free_sqcq(struct virtio_device *vdev,
+ struct vring_virtqueue *vq);
/*
* Helpers.
@@ -1334,6 +1349,10 @@ static struct virtqueue *__vring_new_virtqueue_split(unsigned int index,
vq->layout = virtio_has_feature(vdev, VIRTIO_F_IN_ORDER) ?
VQ_LAYOUT_SPLIT_IN_ORDER : VQ_LAYOUT_SPLIT;
+ vq->sqcq_poll = false;
+ vq->sq = NULL;
+ vq->cq = NULL;
+
if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
vq->weak_barriers = false;
@@ -1385,6 +1404,17 @@ static struct virtqueue *vring_create_virtqueue_split(
to_vvq(vq)->we_own_ring = true;
+ if (virtio_has_feature(vdev, VIRTIO_F_SQCQ_POLL)) {
+ if (virtio_alloc_sqcq(vdev, to_vvq(vq))) {
+ /* vq is on vdev->vqs list; use vring_del_virtqueue()
+ * to remove and free. vring_free_split() would
+ * double-free via later vp_del_vqs().
+ */
+ vring_del_virtqueue(vq);
+ return NULL;
+ }
+ }
+
return vq;
}
@@ -2554,6 +2584,10 @@ static struct virtqueue *__vring_new_virtqueue_packed(unsigned int index,
vq->layout = virtio_has_feature(vdev, VIRTIO_F_IN_ORDER) ?
VQ_LAYOUT_PACKED_IN_ORDER : VQ_LAYOUT_PACKED;
+ vq->sqcq_poll = false;
+ vq->sq = NULL;
+ vq->cq = NULL;
+
if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
vq->weak_barriers = false;
@@ -3013,6 +3047,9 @@ bool virtqueue_kick_prepare(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
+ if (vq->sqcq_poll)
+ return true;
+
return VIRTQUEUE_CALL(vq, kick_prepare);
}
EXPORT_SYMBOL_GPL(virtqueue_kick_prepare);
@@ -3032,6 +3069,45 @@ bool virtqueue_notify(struct virtqueue *_vq)
if (unlikely(vq->broken))
return false;
+ /* SQ/CQ polling mode: update SQ idx instead of MMIO kick. */
+ if (vq->sqcq_poll) {
+ u16 new_sq_idx = vq->split.avail_idx_shadow;
+ u8 need_wakeup;
+
+ /* Release: make descriptor writes visible before sq->idx */
+ smp_store_release(&vq->sq->idx, new_sq_idx);
+
+ /* Track submission interval (matches host's interval metric) */
+ {
+ u64 _now = ktime_get_ns();
+
+ if (vq->sq_last_notify_ns) {
+ u64 _int = _now - vq->sq_last_notify_ns;
+
+ if (vq->sq_ema_interval_ns == 0)
+ vq->sq_ema_interval_ns = _int;
+ else
+ vq->sq_ema_interval_ns =
+ vq->sq_ema_interval_ns -
+ vq->sq_ema_interval_ns / 8 +
+ _int / 8;
+ }
+ vq->sq_last_notify_ns = _now;
+ }
+
+ /* Acquire: order sq->idx store before reading sq_need_wakeup.
+ * Prevents stale read on ARM/RISC-V causing missed kick
+ * when host has set sq_need_wakeup=1 and gone to sleep.
+ */
+ need_wakeup = smp_load_acquire(&vq->sq->sq_need_wakeup);
+
+ /* Device poll thread is sleeping -- need MMIO kick to wake it */
+ if (!need_wakeup)
+ return true;
+ atomic_inc(&vq->sq_mmio_count);
+ /* Fall through to MMIO notify */
+ }
+
/* Prod other side to tell it about changes. */
if (!vq->notify(_vq)) {
vq->broken = true;
@@ -3230,6 +3306,12 @@ irqreturn_t vring_interrupt(int irq, void *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
+ /* SQ/CQ poll mode: wake the poll thread, let it handle completions */
+ if (vq->sqcq_poll) {
+ virtio_sqcq_poll_wake_thread(vq->vq.vdev);
+ return IRQ_HANDLED;
+ }
+
if (!more_used(vq)) {
pr_debug("virtqueue interrupt with no work for %p\n", vq);
return IRQ_NONE;
@@ -3250,6 +3332,7 @@ irqreturn_t vring_interrupt(int irq, void *_vq)
data_race(vq->event_triggered = true);
pr_debug("virtqueue callback for %p (%p)\n", vq, vq->vq.callback);
+
if (vq->vq.callback)
vq->vq.callback(&vq->vq);
@@ -3468,6 +3551,8 @@ static void vring_free(struct virtqueue *_vq)
kfree(vq->split.desc_state);
kfree(vq->split.desc_extra);
}
+ if (vq->sqcq_poll)
+ virtio_free_sqcq(vq->vq.vdev, vq);
}
void vring_del_virtqueue(struct virtqueue *_vq)
@@ -3524,6 +3609,8 @@ void vring_transport_features(struct virtio_device *vdev)
break;
case VIRTIO_F_IN_ORDER:
break;
+ case VIRTIO_F_SQCQ_POLL:
+ break;
default:
/* We don't understand this bit. */
__virtio_clear_bit(vdev, i);
@@ -3663,6 +3750,181 @@ dma_addr_t virtqueue_get_used_addr(const struct virtqueue *_vq)
}
EXPORT_SYMBOL_GPL(virtqueue_get_used_addr);
+bool virtqueue_sqcq_poll_active(const struct virtqueue *_vq)
+{
+ const struct vring_virtqueue *vq = to_vvq(_vq);
+
+ return vq->sqcq_poll;
+}
+EXPORT_SYMBOL_GPL(virtqueue_sqcq_poll_active);
+
+/**
+ * virtqueue_disable_sqcq_poll - disable SQ/CQ poll mode for a virtqueue.
+ * @_vq: the virtqueue
+ *
+ * Clears sqcq_poll flag; restores MMIO kick + interrupt for this vq.
+ * Used for control/event queues that must not use the poll thread.
+ */
+void virtqueue_disable_sqcq_poll(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ vq->sqcq_poll = false;
+}
+EXPORT_SYMBOL_GPL(virtqueue_disable_sqcq_poll);
+
+dma_addr_t virtqueue_get_sq_dma_addr(const struct virtqueue *_vq)
+{
+ const struct vring_virtqueue *vq = to_vvq(_vq);
+
+ return vq->sq_dma_addr;
+}
+EXPORT_SYMBOL_GPL(virtqueue_get_sq_dma_addr);
+
+dma_addr_t virtqueue_get_cq_dma_addr(const struct virtqueue *_vq)
+{
+ const struct vring_virtqueue *vq = to_vvq(_vq);
+
+ return vq->cq_dma_addr;
+}
+EXPORT_SYMBOL_GPL(virtqueue_get_cq_dma_addr);
+
+/* Completion detection and sleep/wake accessors for the poll thread. */
+
+bool virtqueue_cq_poll_has_work(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ if (!vq->cq)
+ return false;
+
+ if (more_used(vq)) {
+ WRITE_ONCE(vq->last_cq_detect_ns, ktime_get_ns());
+ return true;
+ }
+
+ return false;
+}
+EXPORT_SYMBOL_GPL(virtqueue_cq_poll_has_work);
+
+unsigned int virtqueue_get_sq_notify_stats(struct virtqueue *_vq, u64 *interval)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ unsigned int mmio;
+
+ mmio = atomic_xchg(&vq->sq_mmio_count, 0);
+ if (interval)
+ *interval = vq->sq_ema_interval_ns;
+ return mmio;
+}
+EXPORT_SYMBOL_GPL(virtqueue_get_sq_notify_stats);
+
+void virtqueue_cq_set_need_wakeup(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ if (vq->cq)
+ /* Release: order prior used-ring reads before wakeup flag */
+ smp_store_release(&vq->cq->cq_need_wakeup, 1);
+}
+EXPORT_SYMBOL_GPL(virtqueue_cq_set_need_wakeup);
+
+void virtqueue_cq_clear_need_wakeup(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ if (vq->cq)
+ WRITE_ONCE(vq->cq->cq_need_wakeup, 0);
+}
+EXPORT_SYMBOL_GPL(virtqueue_cq_clear_need_wakeup);
+
+bool virtqueue_cq_recheck(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ if (!vq->cq)
+ return false;
+
+ /* Full barrier: order cq_need_wakeup store before used ring read */
+ smp_mb();
+ return more_used(vq);
+}
+EXPORT_SYMBOL_GPL(virtqueue_cq_recheck);
+
+u64 virtqueue_get_cq_detect_ns(const struct virtqueue *_vq)
+{
+ const struct vring_virtqueue *vq = to_vvq(_vq);
+
+ return READ_ONCE(vq->last_cq_detect_ns);
+}
+EXPORT_SYMBOL_GPL(virtqueue_get_cq_detect_ns);
+
+/*
+ * virtio_alloc_sqcq - allocate SQ/CQ doorbell structures for polling mode
+ * @vdev: the virtio device
+ * @vq: the vring_virtqueue to allocate for
+ *
+ * Called when VIRTIO_F_SQCQ_POLL is negotiated. Returns 0 on success.
+ */
+static int virtio_alloc_sqcq(struct virtio_device *vdev,
+ struct vring_virtqueue *vq)
+{
+ struct device *dma_dev = vdev->dev.parent;
+
+ /* Control and event queues use traditional interrupt-driven I/O. */
+ if (vq->vq.name &&
+ (strcmp(vq->vq.name, "control") == 0 ||
+ strcmp(vq->vq.name, "event") == 0))
+ return 0;
+ size_t total = sizeof(struct vring_sq) + sizeof(struct vring_cq);
+ void *buf;
+
+ buf = dma_alloc_coherent(dma_dev, total, &vq->sq_dma_addr, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ memset(buf, 0, total);
+
+ vq->sq = buf;
+ vq->cq = buf + sizeof(struct vring_sq);
+ vq->cq_dma_addr = vq->sq_dma_addr + sizeof(struct vring_sq);
+ vq->sqcq_poll = true;
+ atomic_set(&vq->sq_mmio_count, 0);
+ vq->last_cq_detect_ns = 0;
+ vq->sq_last_notify_ns = 0;
+ vq->sq_ema_interval_ns = 0;
+
+ /* Diagnostic: dump SQ/CQ addresses for debug */
+ dev_dbg(&vdev->dev,
+ "vq %s: SQ/CQ doorbells kvirt=%p sq_dma=0x%llx cq_dma=0x%llx\n",
+ vq->vq.name ? vq->vq.name : "?",
+ buf,
+ (unsigned long long)vq->sq_dma_addr,
+ (unsigned long long)vq->cq_dma_addr);
+
+ return 0;
+}
+
+/*
+ * virtio_free_sqcq - free SQ/CQ doorbell structures
+ * @vdev: the virtio device
+ * @vq: the vring_virtqueue to free for
+ */
+static void virtio_free_sqcq(struct virtio_device *vdev,
+ struct vring_virtqueue *vq)
+{
+ struct device *dma_dev = vdev->dev.parent;
+ size_t total = sizeof(struct vring_sq) + sizeof(struct vring_cq);
+
+ if (vq->sq) {
+ dma_free_coherent(dma_dev, total,
+ vq->sq, vq->sq_dma_addr);
+ vq->sq = NULL;
+ vq->cq = NULL;
+ }
+ vq->sqcq_poll = false;
+}
+
/* Only available for split ring */
const struct vring *virtqueue_get_vring(const struct virtqueue *vq)
{
diff --git a/drivers/virtio/virtio_sqcq_poll.c b/drivers/virtio/virtio_sqcq_poll.c
new file mode 100644
index 000000000000..411493218e93
--- /dev/null
+++ b/drivers/virtio/virtio_sqcq_poll.c
@@ -0,0 +1,338 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * virtio SQ/CQ polling thread.
+ *
+ * Replaces interrupt-based completion notification with a dedicated
+ * kernel thread that checks the used ring for completions. Uses the
+ * io_uring sqpoll pattern: busy-wait while active, then set
+ * NEED_WAKEUP and sleep when idle for a configurable timeout.
+ *
+ * Copyright (C) 2026
+ */
+
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/sched.h>
+#include <linux/completion.h>
+#include <linux/mutex.h>
+#include <linux/wait.h>
+#include <linux/sched/clock.h>
+#include <linux/slab.h>
+#include <linux/list.h>
+#include <linux/kthread.h>
+#include <linux/virtio.h>
+#include <linux/virtio_config.h>
+#include <linux/virtio_ring.h>
+#include <linux/interrupt.h>
+
+/**
+ * struct virtio_sqcq_poll_data - per-device poll thread state.
+ *
+ * Follows the Host vhost_sqcq_poll_thread pattern.
+ */
+struct virtio_sqcq_poll_data {
+ struct mutex lock;
+
+ /* List of virtqueues served by this poll thread */
+ struct list_head vq_list;
+
+ struct task_struct *thread;
+ struct wait_queue_head wait;
+
+ unsigned int idle_timeout_us; /* Sleep timeout (us) */
+ unsigned int busy_spin_us; /* Busy-spin budget (us) */
+
+ unsigned long state;
+#define VIRTIO_SQ_THREAD_SHOULD_STOP 0
+
+ struct completion exited;
+ struct device *dev; /* for dev_info() logging */
+
+ /* Poll thread diagnostic statistics */
+ unsigned long stat_sleep_count;
+};
+
+/**
+ * struct virtio_sqcq_vq_node - links a virtqueue to its poll_data.
+ */
+struct virtio_sqcq_vq_node {
+ struct virtqueue *vq;
+ struct list_head node;
+};
+
+static void virtio_sqcq_poll_wake(struct virtio_sqcq_poll_data *sqd)
+{
+ wake_up(&sqd->wait);
+}
+
+/**
+ * virtio_sqcq_poll_wake_thread - wake the CQ poll thread from IRQ context.
+ * @vdev: the virtio device
+ *
+ * Called by vring_interrupt() when SQ/CQ poll mode is active. Wakes the
+ * poll thread so it can check the used ring and invoke completion
+ * callbacks in its own context, instead of handling completions directly
+ * from the hard-IRQ handler.
+ */
+void virtio_sqcq_poll_wake_thread(struct virtio_device *vdev)
+{
+ struct virtio_sqcq_poll_data *sqd = vdev->sqcq_poll_data;
+
+ if (sqd)
+ virtio_sqcq_poll_wake(sqd);
+}
+EXPORT_SYMBOL_GPL(virtio_sqcq_poll_wake_thread);
+
+unsigned long virtio_sqcq_poll_get_sleep_count(struct virtio_device *vdev)
+{
+ struct virtio_sqcq_poll_data *sqd = vdev->sqcq_poll_data;
+
+ if (!sqd)
+ return 0;
+
+ return xchg(&sqd->stat_sleep_count, 0);
+}
+EXPORT_SYMBOL_GPL(virtio_sqcq_poll_get_sleep_count);
+
+static int virtio_sqcq_poll_thread(void *data)
+{
+ struct virtio_sqcq_poll_data *sqd = data;
+ struct virtio_sqcq_vq_node *node;
+ ktime_t last_io_ts = ktime_get();
+ u64 spin_budget_ns = (u64)sqd->busy_spin_us * NSEC_PER_USEC;
+ int vq_count = 0;
+
+ /* Count registered vqs for the startup message */
+ mutex_lock(&sqd->lock);
+ list_for_each_entry(node, &sqd->vq_list, node)
+ vq_count++;
+ mutex_unlock(&sqd->lock);
+
+ dev_info(sqd->dev, "poll thread running, %d vqs, spin=%uus, idle=%uus\n",
+ vq_count, sqd->busy_spin_us, sqd->idle_timeout_us);
+
+ while (!kthread_should_stop()) {
+ bool did_work = false;
+
+ if (test_bit(VIRTIO_SQ_THREAD_SHOULD_STOP, &sqd->state)) {
+ dev_info(sqd->dev, "poll thread stopping\n");
+ break;
+ }
+
+ if (signal_pending(current)) {
+ flush_signals(current);
+ continue;
+ }
+
+ mutex_lock(&sqd->lock);
+
+ /* Process completions on all registered virtqueues */
+ list_for_each_entry(node, &sqd->vq_list, node) {
+ while (virtqueue_cq_poll_has_work(node->vq)) {
+ if (node->vq->callback)
+ node->vq->callback(node->vq);
+ if (local_softirq_pending())
+ do_softirq();
+ did_work = true;
+ }
+ }
+
+ if (did_work) {
+ mutex_unlock(&sqd->lock);
+ last_io_ts = ktime_get();
+ continue;
+ }
+
+ /* No work — spin within the time budget from last_io_ts.
+ * need_resched() + cpu_relax() for cooperative yielding,
+ * same pattern as Host vhost_sqcq_poll_thread.
+ */
+ if (spin_budget_ns > 0 &&
+ ktime_to_ns(ktime_sub(ktime_get(), last_io_ts)) <
+ (s64)spin_budget_ns) {
+ mutex_unlock(&sqd->lock);
+ if (need_resched())
+ cond_resched();
+ cpu_relax();
+ continue;
+ }
+
+ /* Spin budget expired — set cq_need_wakeup on all VQs
+ * and sleep. Double-check for work before sleeping to
+ * close the lost-wakeup race.
+ */
+ {
+ bool found_work = false;
+ DEFINE_WAIT(wait);
+
+ list_for_each_entry(node, &sqd->vq_list, node)
+ virtqueue_cq_set_need_wakeup(node->vq);
+
+ prepare_to_wait(&sqd->wait, &wait,
+ TASK_INTERRUPTIBLE);
+
+ list_for_each_entry(node, &sqd->vq_list, node) {
+ if (virtqueue_cq_recheck(node->vq)) {
+ found_work = true;
+ break;
+ }
+ }
+
+ if (!found_work) {
+ mutex_unlock(&sqd->lock);
+ schedule_timeout(
+ usecs_to_jiffies(sqd->idle_timeout_us));
+ sqd->stat_sleep_count++;
+ mutex_lock(&sqd->lock);
+ }
+
+ finish_wait(&sqd->wait, &wait);
+
+ list_for_each_entry(node, &sqd->vq_list, node)
+ virtqueue_cq_clear_need_wakeup(node->vq);
+ }
+
+ mutex_unlock(&sqd->lock);
+ }
+
+ complete(&sqd->exited);
+ return 0;
+}
+
+/**
+ * virtio_start_sqcq_poll - start the SQ/CQ poll thread for a device.
+ * @vdev: the virtio device
+ * @idle_timeout_us: idle timeout in microseconds before sleeping
+ * @busy_spin_us: busy-spin duration in microseconds before yielding
+ *
+ * Returns 0 on success, negative error on failure.
+ */
+int virtio_start_sqcq_poll(struct virtio_device *vdev,
+ unsigned int idle_timeout_us,
+ unsigned int busy_spin_us)
+{
+ struct virtio_sqcq_poll_data *sqd;
+
+ sqd = kzalloc(sizeof(*sqd), GFP_KERNEL);
+ if (!sqd)
+ return -ENOMEM;
+
+ mutex_init(&sqd->lock);
+ init_waitqueue_head(&sqd->wait);
+ INIT_LIST_HEAD(&sqd->vq_list);
+ init_completion(&sqd->exited);
+ sqd->idle_timeout_us = idle_timeout_us ? idle_timeout_us : 1000;
+ sqd->busy_spin_us = busy_spin_us;
+ sqd->dev = &vdev->dev;
+
+ sqd->thread = kthread_create(virtio_sqcq_poll_thread, sqd,
+ "virtio-sqcq/%s", dev_name(&vdev->dev));
+ if (IS_ERR(sqd->thread)) {
+ kfree(sqd);
+ return PTR_ERR(sqd->thread);
+ }
+
+ vdev->sqcq_poll_data = sqd;
+ wake_up_process(sqd->thread);
+
+ dev_info(sqd->dev, "SQ/CQ poll thread started, idle_timeout=%uus, spin_us=%u\n",
+ sqd->idle_timeout_us, sqd->busy_spin_us);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(virtio_start_sqcq_poll);
+
+/**
+ * virtio_stop_sqcq_poll - stop the SQ/CQ poll thread for a device.
+ * @vdev: the virtio device
+ */
+void virtio_stop_sqcq_poll(struct virtio_device *vdev)
+{
+ struct virtio_sqcq_poll_data *sqd = vdev->sqcq_poll_data;
+ struct virtio_sqcq_vq_node *node, *tmp;
+
+ if (!sqd)
+ return;
+
+ set_bit(VIRTIO_SQ_THREAD_SHOULD_STOP, &sqd->state);
+
+ dev_info(sqd->dev, "SQ/CQ poll thread stopping\n");
+ virtio_sqcq_poll_wake(sqd);
+ wait_for_completion(&sqd->exited);
+
+ /* Reap kthread task_struct to avoid zombie thread leak.
+ * Thread already exited via complete(); safe to call on
+ * already-exited kthread.
+ */
+ kthread_stop(sqd->thread);
+
+ /* Free all vq nodes */
+ list_for_each_entry_safe(node, tmp, &sqd->vq_list, node) {
+ list_del(&node->node);
+ kfree(node);
+ }
+
+ vdev->sqcq_poll_data = NULL;
+ kfree(sqd);
+}
+EXPORT_SYMBOL_GPL(virtio_stop_sqcq_poll);
+
+/**
+ * virtio_sqcq_poll_register_vq - register a virtqueue with the poll thread.
+ * @vdev: the virtio device
+ * @vq: the virtqueue to register
+ *
+ * The virtqueue must have VIRTIO_F_SQCQ_POLL active.
+ * Returns 0 on success, negative error on failure.
+ */
+int virtio_sqcq_poll_register_vq(struct virtio_device *vdev,
+ struct virtqueue *vq)
+{
+ struct virtio_sqcq_poll_data *sqd = vdev->sqcq_poll_data;
+ struct virtio_sqcq_vq_node *node;
+
+ if (!sqd || !virtqueue_sqcq_poll_active(vq))
+ return -EINVAL;
+
+ node = kmalloc(sizeof(*node), GFP_KERNEL);
+ if (!node)
+ return -ENOMEM;
+
+ node->vq = vq;
+
+ mutex_lock(&sqd->lock);
+ list_add_tail(&node->node, &sqd->vq_list);
+ mutex_unlock(&sqd->lock);
+
+ dev_info(sqd->dev, "vq index=%d (%s) registered with poll thread\n",
+ vq->index, vq->name ? vq->name : "?");
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(virtio_sqcq_poll_register_vq);
+
+/**
+ * virtio_sqcq_poll_unregister_vq - unregister a virtqueue from the poll thread.
+ * @vdev: the virtio device
+ * @vq: the virtqueue to unregister
+ */
+void virtio_sqcq_poll_unregister_vq(struct virtio_device *vdev,
+ struct virtqueue *vq)
+{
+ struct virtio_sqcq_poll_data *sqd = vdev->sqcq_poll_data;
+ struct virtio_sqcq_vq_node *node, *tmp;
+
+ if (!sqd)
+ return;
+
+ mutex_lock(&sqd->lock);
+ list_for_each_entry_safe(node, tmp, &sqd->vq_list, node) {
+ if (node->vq == vq) {
+ list_del(&node->node);
+ kfree(node);
+ break;
+ }
+ }
+ mutex_unlock(&sqd->lock);
+}
+EXPORT_SYMBOL_GPL(virtio_sqcq_poll_unregister_vq);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 3bbc4cb6a672..8300e0946719 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -125,6 +125,28 @@ dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *vq);
dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *vq);
dma_addr_t virtqueue_get_used_addr(const struct virtqueue *vq);
+/* SQ/CQ polling (VIRTIO_F_SQCQ_POLL) */
+bool virtqueue_sqcq_poll_active(const struct virtqueue *vq);
+void virtqueue_disable_sqcq_poll(struct virtqueue *vq);
+dma_addr_t virtqueue_get_sq_dma_addr(const struct virtqueue *vq);
+dma_addr_t virtqueue_get_cq_dma_addr(const struct virtqueue *vq);
+u64 virtqueue_get_cq_detect_ns(const struct virtqueue *vq);
+int virtio_start_sqcq_poll(struct virtio_device *vdev,
+ unsigned int idle_timeout_us,
+ unsigned int busy_spin_us);
+void virtio_stop_sqcq_poll(struct virtio_device *vdev);
+void virtio_sqcq_poll_wake_thread(struct virtio_device *vdev);
+int virtio_sqcq_poll_register_vq(struct virtio_device *vdev,
+ struct virtqueue *vq);
+void virtio_sqcq_poll_unregister_vq(struct virtio_device *vdev,
+ struct virtqueue *vq);
+bool virtqueue_cq_poll_has_work(struct virtqueue *vq);
+unsigned int virtqueue_get_sq_notify_stats(struct virtqueue *vq, u64 *interval);
+unsigned long virtio_sqcq_poll_get_sleep_count(struct virtio_device *vdev);
+void virtqueue_cq_set_need_wakeup(struct virtqueue *vq);
+void virtqueue_cq_clear_need_wakeup(struct virtqueue *vq);
+bool virtqueue_cq_recheck(struct virtqueue *vq);
+
int virtqueue_resize(struct virtqueue *vq, u32 num,
void (*recycle)(struct virtqueue *vq, void *buf),
void (*recycle_done)(struct virtqueue *vq));
@@ -165,6 +187,8 @@ struct virtio_admin_cmd {
* @debugfs_dir: debugfs directory entry.
* @debugfs_filter_features: features to be filtered set by debugfs.
*/
+struct virtio_sqcq_poll_data;
+
struct virtio_device {
int index;
bool failed;
@@ -182,6 +206,7 @@ struct virtio_device {
VIRTIO_DECLARE_FEATURES(features);
void *priv;
union virtio_map vmap;
+ struct virtio_sqcq_poll_data *sqcq_poll_data; /* SQ/CQ poll thread state */
#ifdef CONFIG_VIRTIO_DEBUG
struct dentry *debugfs_dir;
u64 debugfs_filter_features[VIRTIO_FEATURES_U64S];
diff --git a/include/linux/virtio_pci_modern.h b/include/linux/virtio_pci_modern.h
index 9a3f2fc53bd6..36810f9d23f3 100644
--- a/include/linux/virtio_pci_modern.h
+++ b/include/linux/virtio_pci_modern.h
@@ -145,6 +145,8 @@ u16 vp_modern_config_vector(struct virtio_pci_modern_device *mdev,
void vp_modern_queue_address(struct virtio_pci_modern_device *mdev,
u16 index, u64 desc_addr, u64 driver_addr,
u64 device_addr);
+void vp_modern_queue_address_sqcq(struct virtio_pci_modern_device *mdev,
+ u16 index, u64 sq_addr, u64 cq_addr);
void vp_modern_set_queue_enable(struct virtio_pci_modern_device *mdev,
u16 idx, bool enable);
bool vp_modern_get_queue_enable(struct virtio_pci_modern_device *mdev,
--
2.34.1
^ permalink raw reply related
* [RFC PATCH 0/4] virtio: SQ/CQ doorbell polling for vhost-scsi
From: rom.wang @ 2026-07-20 14:10 UTC (permalink / raw)
To: mst, jasowangio, eperezma, stefanha
Cc: virtualization, linux-kernel, linux-scsi, kvm, qemu-devel,
Yufeng Wang
From: Yufeng Wang <wangyufeng@kylinos.cn>
This RFC proposes a new virtio transport feature, VIRTIO_F_SQCQ_POLL,
that eliminates VM exits on both submission and completion paths for
vhost-scsi by using shared-memory doorbells and kernel polling
threads, following the io_uring SQPOLL model.
This is an early RFC to gather design feedback. The implementation is
functional and has been tested on arm64 and x86_64. We are not
requesting merge at this time.
Problem
-------
vhost-scsi uses MMIO writes (Guest -> Host) and MSI-X interrupts
(Host -> Guest) for notification. Each notification involves a VM exit,
which becomes a bottleneck at high IOPS:
- 4K random read, QD32, 8 jobs: 341K IOPS baseline
- With ~340K VM exits/second, the exit overhead dominates
Existing mitigations (vhost-net's tx polling, blk-mq iopoll) only
address one direction or require the submitting task to poll. Neither
eliminates VM exits on both paths simultaneously.
Solution
--------
Introduce two cache-line-aligned doorbell structures, SQ (Submission
Queue) and CQ (Completion Queue), placed alongside the standard split
virtqueue:
- Guest writes sq->idx instead of MMIO kick; Host poll thread
detects the change and processes submissions.
- Host writes cq->idx instead of MSI-X interrupt; Guest poll
thread detects the change and invokes completion callbacks.
A NEED_WAKEUP protocol (mirroring io_uring's SQ_NEED_WAKEUP) allows
either side to sleep when idle, with the other side responsible for
waking it via eventfd.
Feature negotiation via VIRTIO_F_SQCQ_POLL (bit 42) ensures zero
overhead when not negotiated — the driver falls back to traditional
MMIO kick + MSI-X interrupt.
Performance
-----------
Benchmark: fio, 4K random I/O
Test configuration:
arm64:
CPU: Kunpeng 920 (2.6GHz), 8 vCPUs
Disk: NVMe INTEL SSDPED1K375GA (375GB)
x86_64:
CPU: Intel Xeon E5-2680 v4 @ 2.40GHz, 8 vCPUs
Disk: NVMe SAMSUNG MZ1LB960HAJQ-000MV (960GB)
Backend: vhost-scsi with TCM loopback to NVMe device
QEMU: vhost-scsi-pci with VIRTIO_F_SQCQ_POLL negotiated
arm64 results:
Test Baseline SQ/CQ Poll Change
----------- ---------- ---------- -------
randread QD1 22,427 28,289 +26%
randread QD32 NJ1 89,910 75,665 -16%
randread QD32 NJ4 186,763 379,549 +103%
randread QD32 NJ8 199,967 550,633 +175%
randwrite QD1 21,912 27,261 +24%
randwrite QD32 NJ1 85,349 81,389 -5%
randwrite QD32 NJ4 190,443 355,811 +87%
randwrite QD32 NJ8 196,552 566,640 +188%
x86_64 results:
Test Baseline SQ/CQ Poll Change
----------- ---------- ---------- -------
randread QD1 8,263 9,552 +16%
randread QD32 NJ1 127,412 162,805 +28%
randread QD32 NJ4 303,208 375,056 +24%
randread QD32 NJ8 341,625 371,193 +9%
randwrite QD1 20,773 30,332 +46%
randwrite QD32 NJ1 133,316 159,207 +19%
randwrite QD32 NJ4 233,373 229,224 -2%
randwrite QD32 NJ8 231,442 231,676 +0%
Multi-queue workloads (NJ4/NJ8) see significant improvement on arm64
(87-188%) and moderate improvement on x86_64 (9-24%). Single-VQ
high-queue-depth workloads show a minor regression on arm64 due to
polling overhead vs. VM-exit savings trade-off, while x86_64 shows
improvement across most configurations (16-46% for QD1, 19-28%
for QD32-NJ1) due to lower per-VM-exit cost on x86.
Why Not vDPA?
-------------
vhost-vDPA already provides doorbell mmap and polling. A reasonable
reviewer would ask: why not extend vhost-vDPA instead?
Three reasons:
1. No vdpa-scsi device exists. The vDPA framework
(drivers/vdpa/) currently has hardware devices for net (mlx5,
ifcvf, etc.) and software devices for net and blk (vdpa_sim).
There is no virtio-scsi vDPA device, hardware or software.
Building one means re-implementing vhost-scsi's TCM integration
(SCSI CDB processing, ALUA, persistent reservations) under the
vDPA device abstraction — 3-5x the work of extending vhost-scsi.
2. vhost-scsi is a deployed interface. libvirt, QEMU, and
OpenStack have vhost-scsi configuration APIs and operational
tooling. Switching to vhost-vdpa requires a new backend, user
migration, and toolchain updates. SQ/CQ poll as a vhost-scsi
feature is fully backward-compatible — no existing deployments
break.
3. The protocol is transport-agnostic. The SQ/CQ doorbell design
(struct vring_sq, struct vring_cq, NEED_WAKEUP handshake) is
orthogonal to vhost vs. vDPA. The same UAPI can be consumed by
vhost-scsi today and a future vdpa-scsi device. Implementing in
vhost-scsi first does not block future vDPA integration.
We acknowledge that vDPA is the long-term direction for virtio
backends. If this SQ/CQ poll protocol is accepted, it can be ported
to the vDPA framework; a vdpa-scsi device is independent work.
Patch Structure
---------------
Patch 1: UAPI definitions (virtio_config.h, virtio_ring.h,
virtio_pci.h) — shared interface for all components
Patch 2: vhost kernel support (vhost.c, vhost.h, scsi.c,
vhost.h UAPI, vhost_types.h UAPI) — Host poll thread
Patch 3: virtio guest driver (virtio_ring.c, virtio_sqcq_poll.c,
virtio_pci_modern.c, virtio.c, virtio_scsi.c) — Guest
poll thread and submission path
Patch 4: QEMU support (virtio-pci.c, vhost.c) — PCI config
forwarding and vhost ioctl bridge
Patches 1-3 apply to the Linux kernel tree. Patch 4 applies to
the QEMU tree separately.
Spec Status
-----------
A virtio-spec format document has been prepared and will be submitted
to the OASIS virtio TC as a proposal. This RFC stage seeks design
feedback before initiating the formal spec process.
Known Limitations (Future Work)
-------------------------------
- CPU hotplug: no notifier registered; poll thread may be
migrated when its CPU goes offline. Planned: kthread_park +
dynamic rebind.
- Live migration: no explicit stop/flush coordination during
migration. Planned: VHOST_BACKEND_F_SUSPEND/RESUME integration.
- SMAP overhead: Host uses get_user/put_user for doorbell access.
Future optimization: GUP + kmap to map pages into kernel space.
- Backpressure: no "slow down" signal from device to driver.
Future: CQ throttle flag or avail_event reuse.
- Packed ring: not supported; explicitly rejected at feature
negotiation. Future: add packed ring doorbell support.
RFC Goals
---------
1. Validate the overall design direction (doorbell + polling
model, NEED_WAKEUP protocol)
2. Get feedback on the feature bit allocation (42) and UAPI
structure design
3. Understand whether vDPA concern is a blocker or can be
addressed with the transport-agnostic argument
4. Collect guidance on prioritizing future work items
(hotplug, migration, spec process)
We welcome all feedback, especially on:
- Whether the NEED_WAKEUP protocol design is sound
- Whether the per-device Guest poll thread model (vs per-VQ)
is acceptable
- Whether feature bit 42 is appropriate or if a different
allocation is needed
- Any concerns about the SMAP overhead in the Host poll path
How to Test
-----------
Patch usage:
Guest Kernel: apply patch 1 (UAPI) + patch 3 (guest driver)
Host Kernel: apply patch 1 (UAPI) + patch 2 (vhost support)
QEMU: apply patch 4 (vhost-scsi bridge)
1. Set up vhost-scsi target on the host (see:
https://wiki.libvirt.org/Vhost-scsi_target.html#Host_Setup)
using targetcli to create a TCM loopback device, e.g.:
targetcli /backstores/loopback create dev=/dev/sda
targetcli /vhost create naa.5001405376e34400
targetcli /vhost/naa.5001405376e34400/lun create \
/backstores/loopback/dev,/dev/sda
2. Boot VM with vhost-scsi using patched QEMU:
qemu-system-aarch64 ... \
-device vhost-scsi-pci,wwpn=naa.5001405376e34400
3. Verify SQ/CQ poll mode is active on the host:
dmesg | grep "vhost-sqcq"
# Expected: "vhost-sqcq: vq[N] poll thread bound to cpuN"
# and 10s stats: "vhost-sqcq: vq[N] cq=... ema_lat=... interval=..."
4. Verify feature negotiated in guest:
dmesg | grep "VIRTIO_F_SQCQ_POLL"
# Expected: "VIRTIO_F_SQCQ_POLL negotiated, starting poll thread"
# and 10s stats: "io_stats: cq=... avg_lat=... interval=..."
5. Run fio benchmark (compare with unpatched QEMU/kernel baseline):
fio --name=randread --rw=randread --bs=4k --iodepth=32 \
--numjobs=4 --runtime=60 --time_based --direct=1 \
--filename=/dev/sda
Test scripts (run-sqcq-compare.sh, compare-sqcq-results.sh) are
available and will be sent as a follow-up to this RFC.
Thank you for your time.
---
Yufeng Wang (4):
common: add UAPI for SQ/CQ doorbell polling
vhost: host kernel support for SQ/CQ polling
virtio: guest driver support for SQ/CQ polling
qemu: add SQ/CQ polling mode support for vhost-scsi
Patch 1 (UAPI, 3 files): +41 -1
Patch 2 (vhost, 5 files): +794 -20
Patch 3 (virtio guest, 10 files): +840 -3
Patch 4 (QEMU, 14 files): +179 -3
Total: 32 files changed, 1854 insertions(+), 27 deletions(-)
--
2.34.1
^ permalink raw reply
* [RFC PATCH 1/4] common: add UAPI for SQ/CQ doorbell polling
From: rom.wang @ 2026-07-20 14:10 UTC (permalink / raw)
To: mst, jasowangio, eperezma, stefanha
Cc: virtualization, linux-kernel, linux-scsi, kvm, qemu-devel,
Yufeng Wang
In-Reply-To: <20260720141040.181946-1-r4o5m6e8o@163.com>
From: Yufeng Wang <wangyufeng@kylinos.cn>
Define VIRTIO_F_SQCQ_POLL (bit 42) and the associated SQ/CQ doorbell
structures for split virtqueue polling mode.
- include/uapi/linux/virtio_config.h: feature bit 42
- include/uapi/linux/virtio_ring.h: struct vring_sq, struct vring_cq
(128-byte aligned for cache-line isolation on ARM Neoverse N2/V2)
- include/uapi/linux/virtio_pci.h: sqe/cqe_ring PCI config registers
at offsets 64-76
Signed-off-by: Yufeng Wang <wangyufeng@kylinos.cn>
---
include/uapi/linux/virtio_config.h | 8 +++++++-
include/uapi/linux/virtio_pci.h | 9 +++++++++
include/uapi/linux/virtio_ring.h | 25 +++++++++++++++++++++++++
3 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
index 2445f365bce7..dbda23e44b62 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -52,7 +52,7 @@
* rest are per-device feature bits.
*/
#define VIRTIO_TRANSPORT_F_START 28
-#define VIRTIO_TRANSPORT_F_END 42
+#define VIRTIO_TRANSPORT_F_END 43
#ifndef VIRTIO_CONFIG_NO_LEGACY
/* Do we get callbacks when the ring is completely used, even if we've
@@ -120,4 +120,10 @@
*/
#define VIRTIO_F_ADMIN_VQ 41
+/*
+ * This feature indicates that both driver and device support
+ * SQ/CQ polling mode to replace interrupt-based notifications.
+ */
+#define VIRTIO_F_SQCQ_POLL 42
+
#endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
index e732e3456e27..f5058d1b4b16 100644
--- a/include/uapi/linux/virtio_pci.h
+++ b/include/uapi/linux/virtio_pci.h
@@ -193,6 +193,11 @@ struct virtio_pci_modern_common_cfg {
__le16 admin_queue_index; /* read-only */
__le16 admin_queue_num; /* read-only */
+
+ __le32 sqe_ring_lo; /* read-write: SQ DMA addr low */
+ __le32 sqe_ring_hi; /* read-write: SQ DMA addr high */
+ __le32 cqe_ring_lo; /* read-write: CQ DMA addr low */
+ __le32 cqe_ring_hi; /* read-write: CQ DMA addr high */
};
/* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
@@ -235,6 +240,10 @@ struct virtio_pci_cfg_cap {
#define VIRTIO_PCI_COMMON_Q_RESET 58
#define VIRTIO_PCI_COMMON_ADM_Q_IDX 60
#define VIRTIO_PCI_COMMON_ADM_Q_NUM 62
+#define VIRTIO_PCI_COMMON_SQE_LO 64
+#define VIRTIO_PCI_COMMON_SQE_HI 68
+#define VIRTIO_PCI_COMMON_CQE_LO 72
+#define VIRTIO_PCI_COMMON_CQE_HI 76
#endif /* VIRTIO_PCI_NO_MODERN */
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 3c478582a3c2..d1f2b9584d7c 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -131,6 +131,31 @@ struct vring_used {
vring_used_elem_t ring[];
};
+/*
+ * SQ/CQ doorbell structures for polling mode (VIRTIO_F_SQCQ_POLL).
+ * These are allocated separately from the vring and their DMA addresses
+ * are communicated via PCI config space registers.
+ *
+ * Ownership:
+ * SQ: Guest writes idx, Device reads it.
+ * CQ: Device writes idx, Guest reads it.
+ *
+ * Flags (8-bit):
+ * Set to 1 before sleeping, cleared to 0 after waking up.
+ * Direct assignment — no read-modify-write.
+ */
+struct vring_sq {
+ __virtio64 idx; /* Producer position (updated by guest) */
+ __u8 sq_need_wakeup; /* 1 = device should be woken on next kick */
+ __u8 reserved[55]; /* Reserved for future use */
+} __attribute__((aligned(128)));
+
+struct vring_cq {
+ __virtio64 idx; /* Producer position (updated by device) */
+ __u8 cq_need_wakeup; /* 1 = guest should be woken on completion */
+ __u8 reserved[55]; /* Reserved for future use */
+} __attribute__((aligned(128)));
+
/*
* The ring element addresses are passed between components with different
* alignments assumptions. Thus, we might need to decrease the compiler-selected
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v4 6/8] media: virtio: Add virtio_media_driver
From: Alistair Delva @ 2026-07-20 14:07 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Mauro Carvalho Chehab, Brian Daniels, Mauro Carvalho Chehab,
acourbot, aesteve, changyeon, daniel.almeida, eperezma, gnurou,
gurchetansingh, hverkuil, jasowang, linux-kernel, linux-media,
nicolas.dufresne, virtualization, xuanzhuo
In-Reply-To: <20260720090703-mutt-send-email-mst@kernel.org>
On Mon, Jul 20, 2026 at 6:09 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Jul 20, 2026 at 05:35:19AM -0700, Alistair Delva wrote:
> > On Sat, Jul 18, 2026 at 10:29 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Sun, Jul 12, 2026 at 08:57:26AM +0200, Mauro Carvalho Chehab wrote:
> > > > On Thu, 25 Jun 2026 16:18:48 -0400
> > > > Brian Daniels <briandaniels@google.com> wrote:
> > > >
> > > > > > > From: Alexandre Courbot <gnurou@gmail.com>
> > > > > > >
> > > > > > > virtio_media_driver.c provides the expected driver hooks, and support
> > > > > > > for mmapping and polling.
> > > > > > >
> > > > > > > Signed-off-by: Alexandre Courbot <gnurou@gmail.com>
> > > > > > > Co-developed-by: Brian Daniels <briandaniels@google.com>
> > > > > > > Signed-off-by: Brian Daniels <briandaniels@google.com>
> > > > > > > ---
> > > > > > > drivers/media/virtio/virtio_media_driver.c | 959 +++++++++++++++++++++
> > > > > > > 1 file changed, 959 insertions(+)
> > > > > > > create mode 100644 drivers/media/virtio/virtio_media_driver.c
> > > > > > >
> > > > > > > diff --git a/drivers/media/virtio/virtio_media_driver.c b/drivers/media/virtio/virtio_media_driver.c
> > > > > > > new file mode 100644
> > > > > > > index 000000000..d6363c673
> > > > > > > --- /dev/null
> > > > > > > +++ b/drivers/media/virtio/virtio_media_driver.c
> > > > > > > @@ -0,0 +1,959 @@
> > > > > > > +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0+
> > > > > > > +
> > > > > > > +/*
> > > > > > > + * Virtio-media driver.
> > > > > > > + *
> > > > > > > + * Copyright (c) 2024-2025 Google LLC.
> > > > > > > + */
> > > > > > > +
> > > > > > > +#include <linux/delay.h>
> > > > > > > +#include <linux/device.h>
> > > > > > > +#include <linux/dev_printk.h>
> > > > > > > +#include <linux/mm.h>
> > > > > > > +#include <linux/mutex.h>
> > > > > > > +#include <linux/scatterlist.h>
> > > > > > > +#include <linux/types.h>
> > > > > > > +#include <linux/videodev2.h>
> > > > > > > +#include <linux/vmalloc.h>
> > > > > > > +#include <linux/wait.h>
> > > > > > > +#include <linux/workqueue.h>
> > > > > > > +#include <linux/module.h>
> > > > > > > +#include <linux/moduleparam.h>
> > > > > > > +#include <linux/virtio.h>
> > > > > > > +#include <linux/virtio_config.h>
> > > > > > > +#include <linux/virtio_ids.h>
> > > > > > > +
> > > > > > > +#include <media/frame_vector.h>
> > > > > > > +#include <media/v4l2-dev.h>
> > > > > > > +#include <media/v4l2-event.h>
> > > > > > > +#include <media/videobuf2-memops.h>
> > > > > > > +#include <media/v4l2-device.h>
> > > > > > > +#include <media/v4l2-ioctl.h>
> > > > > > > +
> > > > > > > +#include "protocol.h"
> > > > > > > +#include "session.h"
> > > > > > > +#include "virtio_media.h"
> > > > > > > +
> > > > > > > +#define VIRTIO_MEDIA_NUM_EVENT_BUFS 16
> > > > > > > +
> > > > > > > +/* ID of the SHM region into which MMAP buffer will be mapped. */
> > > > > > > +#define VIRTIO_MEDIA_SHM_MMAP 0
> > > > > > > +
> > > > > > > +/*
> > > > > > > + * Name of the driver to expose to user-space.
> > > > > > > + *
> > > > > > > + * This is configurable because v4l2-compliance has workarounds specific to
> > > > > > > + * some drivers. When proxying these directly from the host, this allows it to
> > > > > > > + * apply them as needed.
> > > > > > > + */
> > > > > > > +char *virtio_media_driver_name;
> > > > > > > +module_param_named(driver_name, virtio_media_driver_name, charp, 0660);
> > > > > >
> > > > > >
> > > > > > Um. What? Not how it should be handled.
> > > > >
> > > > > I can remove this module param. I didn't end up using this when compliance testing.
> > > > > Instead, I patched v4l-utils:
> > > > > https://lore.kernel.org/all/20260528163448.4031965-1-briandaniels@google.com/
> > > > >
> > > > > Let me know if you think the v4l-utils patch is a good approach, otherwise let
> > > > > me know how you'd prefer to address the v4l2-compliance driver-specific workounds
> > > > > when they're being proxied with virtio-media.
> > > >
> > > > This kind of discussion should happen on a separate PR for v4l2-compliance,
> > > > c/c to the proper developers and maintainers of it.
> > >
> > >
> > > I wonder how migration can work when guest is tied to host driver model like
> > > this.
> >
> > We haven't given migration much thought yet, but I think it wouldn't
> > be so different to GPU, where to do snapshot/migration we have to
> > record all initialization / context setup state and replay it against
> > the GPU driver on restore (see https://github.com/google/gfxstream).
> > Some migrations will be possible, some will not, and the host should
> > be able to decide.
> >
> > Also, Brian mentioned the host device pass-through use case, but we
> > also have device implementations on the host that work with camera
> > emulators or some other data source like video/webrtc, which will
> > support snapshot/migration more easily. This use case will probably
> > will see more real-world use and migration will be more relevant.
>
> This is par for the course.
>
>
> But I'm afraid I wasn't clear enough. This thread mentions
> supplying the host driver name to guest userspace
> in order to implement "driver specific work arounds".
Pardon my ignorance, but do a lot of v4l2 apps actually do things
differently for different drivers?
IIUC, this data is so far only used as a command line flag specified
to v4l2-compliance in the guest to pass some tests. It seems like this
information could be provided out of band for that use case.
> But given such, for migration to work userspace needs
> to be notified about driver change? And what to do about
> the things it started after migration but before the
> notification?
If this was used in a local development environment or lab, I think an
equipment change is unlikely or could be avoided by the setup. IMO,
the driver actually changing during a migration is an unlikely use
case and the host could just fail to restore the guest if a driver
change is detected.
That's why I brought up GPU - we might not support e.g. migrating from
nvidia to amd, but that's probably fine and migration is still useful
most of the time.
> > > > >
> > > > > > > +
> > > > > > > +/*
> > > > > > > + * Whether USERPTR buffers are allowed.
> > > > > > > + *
> > > > > > > + * This is disabled by default as USERPTR buffers are dangerous, but the option
> > > > > > > + * is left to enable them if desired.
> > > > > > > + */
> > > > > > > +bool virtio_media_allow_userptr;
> > > > > > > +module_param_named(allow_userptr, virtio_media_allow_userptr, bool, 0660);
> > > > > >
> > > > > >
> > > > > > is this kind of thing common?
> > > >
> > > > There is one old media device that has it (saa7134).
> > > >
> > > > >
> > > > > To be honest, I don't really know. I'm also not that familiar with the USERPTR
> > > > > issues. I see a few references online about their use being discouraged due to
> > > > > possible race conditions, perhaps that was the original motivation for this
> > > > > parameter (I'm not the original author of this driver).
> > > > >
> > > > > I'm open to alternatives, feel free to let me know if you have a preference.
> > > >
> > > > We tend to not implement USERPTR on newer drivers. I suggest you
> > > > to place the logic with regards to V4L2_MEMORY_USERPTR on a separate
> > > > patch for further discussions.
> > > >
> > > > >
> > > > > > > +
> > > > > > > +/**
> > > > > > > + * virtio_media_session_alloc - Allocate a new session.
> > > > > > > + * @vv: virtio-media device the session belongs to.
> > > > > > > + * @id: ID of the session.
> > > > > > > + * @nonblocking_dequeue: whether dequeuing of buffers should be blocking or
> > > > > > > + * not.
> > > > > > > + *
> > > > > > > + * The ``id`` and ``list`` fields must still be set by the caller.
> > > > > >
> > > > > > still in what sense?
> > > > >
> > > > > Based on the code below, I'm not so sure that the caller is responsible for
> > > > > setting these values. They seem to be initialized in the function.
> > > > >
> > > > > Perhaps Alexandre Courbot (the original author) would know more. Unless he
> > > > > says otherwise though I'm inclined to remove this comment.
> > > > >
> > > > > > > + */
> > > > > > > +static struct virtio_media_session *
> > > > > > > +virtio_media_session_alloc(struct virtio_media *vv, u32 id,
> > > > > > > + struct file *file)
> > > > > > > +{
> > > > > > > + struct virtio_media_session *session;
> > > > > > > + int i;
> > > > > > > + int ret;
> > > > > > > +
> > > > > > > + session = kzalloc_obj(*session, GFP_KERNEL);
> > > > > > > + if (!session)
> > > > > > > + goto err_session;
> > > > > > > +
> > > > > > > + session->shadow_buf = kzalloc(VIRTIO_SHADOW_BUF_SIZE, GFP_KERNEL);
> > > > > > > + if (!session->shadow_buf)
> > > > > > > + goto err_shadow_buf;
> > > > > > > +
> > > > > > > + ret = sg_alloc_table(&session->command_sgs, DESC_CHAIN_MAX_LEN,
> > > > > > > + GFP_KERNEL);
> > > > > > > + if (ret)
> > > > > > > + goto err_payload_sgs;
> > > > > > > +
> > > > > > > + session->id = id;
> > > > > > > + session->nonblocking_dequeue = file->f_flags & O_NONBLOCK;
> > > > > > > +
> > > > > > > + INIT_LIST_HEAD(&session->list);
> > > > > > > + v4l2_fh_init(&session->fh, &vv->video_dev);
> > > > > > > + virtio_media_session_fh_add(session, file);
> > > > > > > +
> > > > > > > + for (i = 0; i <= VIRTIO_MEDIA_LAST_QUEUE; i++)
> > > > > > > + INIT_LIST_HEAD(&session->queues[i].pending_dqbufs);
> > > > > > > + mutex_init(&session->queues_lock);
> > > > > > > +
> > > > > > > + init_waitqueue_head(&session->dqbuf_wait);
> > > > > > > +
> > > > > > > + mutex_lock(&vv->sessions_lock);
> > > > > > > + list_add_tail(&session->list, &vv->sessions);
> > > > > > > + mutex_unlock(&vv->sessions_lock);
> > > > > > > +
> > > > > > > + return session;
> > > > > > > +
> > > > > > > +err_payload_sgs:
> > > > > > > + kfree(session->shadow_buf);
> > > > > > > +err_shadow_buf:
> > > > > > > + kfree(session);
> > > > > > > +err_session:
> > > > > > > + return ERR_PTR(-ENOMEM);
> > > > > > > +}
> > > > > > > +
> > > > > > > +/**
> > > > > > > + * virtio_media_session_free - Free all resources of a session.
> > > > > > > + * @vv: virtio-media device the session belongs to.
> > > > > > > + * @session: session to destroy.
> > > > > > > + *
> > > > > > > + * All the resources of @sesssion, as well as the backing memory of @session
> > > > > > > + * itself, are freed.
> > > > > >
> > > > > > why @ here and `` above? And typo in the name.
> > > > >
> > > > > The `@` here was an attempt to follow the guide here for referencing function
> > > > > parameters:
> > > > > https://docs.kernel.org/doc-guide/kernel-doc.html#highlights-and-cross-references
> > > >
> > > > Yes. This is part of Linux Kernel kernel-doc markup: when referring to
> > > > struct fields, you should use @field (or ``field`` if one wants to place an
> > > > asterisk on it, like ``*field``).
> > > >
> > > > >
> > > > > That being said, I don't believe this file is 100% consistent with that. I will
> > > > > spend some time cleaning up the comments throughout this patch set to get them
> > > > > consistent for v5. Thanks!
> > > >
> > > > Please use it on a consistent way along the driver.
> > > >
> > > >
> > > > Thanks,
> > > > Mauro
> > >
>
^ permalink raw reply
* Re: [PATCH v4 6/8] media: virtio: Add virtio_media_driver
From: Michael S. Tsirkin @ 2026-07-20 13:09 UTC (permalink / raw)
To: Alistair Delva
Cc: Mauro Carvalho Chehab, Brian Daniels, Mauro Carvalho Chehab,
acourbot, aesteve, changyeon, daniel.almeida, eperezma, gnurou,
gurchetansingh, hverkuil, jasowang, linux-kernel, linux-media,
nicolas.dufresne, virtualization, xuanzhuo
In-Reply-To: <CANDihLFeF_nKi5z=eH5Mwaa4s8fEU1chS4K2VQZ+0b-0OLybjg@mail.gmail.com>
On Mon, Jul 20, 2026 at 05:35:19AM -0700, Alistair Delva wrote:
> On Sat, Jul 18, 2026 at 10:29 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Sun, Jul 12, 2026 at 08:57:26AM +0200, Mauro Carvalho Chehab wrote:
> > > On Thu, 25 Jun 2026 16:18:48 -0400
> > > Brian Daniels <briandaniels@google.com> wrote:
> > >
> > > > > > From: Alexandre Courbot <gnurou@gmail.com>
> > > > > >
> > > > > > virtio_media_driver.c provides the expected driver hooks, and support
> > > > > > for mmapping and polling.
> > > > > >
> > > > > > Signed-off-by: Alexandre Courbot <gnurou@gmail.com>
> > > > > > Co-developed-by: Brian Daniels <briandaniels@google.com>
> > > > > > Signed-off-by: Brian Daniels <briandaniels@google.com>
> > > > > > ---
> > > > > > drivers/media/virtio/virtio_media_driver.c | 959 +++++++++++++++++++++
> > > > > > 1 file changed, 959 insertions(+)
> > > > > > create mode 100644 drivers/media/virtio/virtio_media_driver.c
> > > > > >
> > > > > > diff --git a/drivers/media/virtio/virtio_media_driver.c b/drivers/media/virtio/virtio_media_driver.c
> > > > > > new file mode 100644
> > > > > > index 000000000..d6363c673
> > > > > > --- /dev/null
> > > > > > +++ b/drivers/media/virtio/virtio_media_driver.c
> > > > > > @@ -0,0 +1,959 @@
> > > > > > +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0+
> > > > > > +
> > > > > > +/*
> > > > > > + * Virtio-media driver.
> > > > > > + *
> > > > > > + * Copyright (c) 2024-2025 Google LLC.
> > > > > > + */
> > > > > > +
> > > > > > +#include <linux/delay.h>
> > > > > > +#include <linux/device.h>
> > > > > > +#include <linux/dev_printk.h>
> > > > > > +#include <linux/mm.h>
> > > > > > +#include <linux/mutex.h>
> > > > > > +#include <linux/scatterlist.h>
> > > > > > +#include <linux/types.h>
> > > > > > +#include <linux/videodev2.h>
> > > > > > +#include <linux/vmalloc.h>
> > > > > > +#include <linux/wait.h>
> > > > > > +#include <linux/workqueue.h>
> > > > > > +#include <linux/module.h>
> > > > > > +#include <linux/moduleparam.h>
> > > > > > +#include <linux/virtio.h>
> > > > > > +#include <linux/virtio_config.h>
> > > > > > +#include <linux/virtio_ids.h>
> > > > > > +
> > > > > > +#include <media/frame_vector.h>
> > > > > > +#include <media/v4l2-dev.h>
> > > > > > +#include <media/v4l2-event.h>
> > > > > > +#include <media/videobuf2-memops.h>
> > > > > > +#include <media/v4l2-device.h>
> > > > > > +#include <media/v4l2-ioctl.h>
> > > > > > +
> > > > > > +#include "protocol.h"
> > > > > > +#include "session.h"
> > > > > > +#include "virtio_media.h"
> > > > > > +
> > > > > > +#define VIRTIO_MEDIA_NUM_EVENT_BUFS 16
> > > > > > +
> > > > > > +/* ID of the SHM region into which MMAP buffer will be mapped. */
> > > > > > +#define VIRTIO_MEDIA_SHM_MMAP 0
> > > > > > +
> > > > > > +/*
> > > > > > + * Name of the driver to expose to user-space.
> > > > > > + *
> > > > > > + * This is configurable because v4l2-compliance has workarounds specific to
> > > > > > + * some drivers. When proxying these directly from the host, this allows it to
> > > > > > + * apply them as needed.
> > > > > > + */
> > > > > > +char *virtio_media_driver_name;
> > > > > > +module_param_named(driver_name, virtio_media_driver_name, charp, 0660);
> > > > >
> > > > >
> > > > > Um. What? Not how it should be handled.
> > > >
> > > > I can remove this module param. I didn't end up using this when compliance testing.
> > > > Instead, I patched v4l-utils:
> > > > https://lore.kernel.org/all/20260528163448.4031965-1-briandaniels@google.com/
> > > >
> > > > Let me know if you think the v4l-utils patch is a good approach, otherwise let
> > > > me know how you'd prefer to address the v4l2-compliance driver-specific workounds
> > > > when they're being proxied with virtio-media.
> > >
> > > This kind of discussion should happen on a separate PR for v4l2-compliance,
> > > c/c to the proper developers and maintainers of it.
> >
> >
> > I wonder how migration can work when guest is tied to host driver model like
> > this.
>
> We haven't given migration much thought yet, but I think it wouldn't
> be so different to GPU, where to do snapshot/migration we have to
> record all initialization / context setup state and replay it against
> the GPU driver on restore (see https://github.com/google/gfxstream).
> Some migrations will be possible, some will not, and the host should
> be able to decide.
>
> Also, Brian mentioned the host device pass-through use case, but we
> also have device implementations on the host that work with camera
> emulators or some other data source like video/webrtc, which will
> support snapshot/migration more easily. This use case will probably
> will see more real-world use and migration will be more relevant.
This is par for the course.
But I'm afraid I wasn't clear enough. This thread mentions
supplying the host driver name to guest userspace
in order to implement "driver specific work arounds".
But given such, for migration to work userspace needs
to be notified about driver change? And what to do about
the things it started after migration but before the
notification?
> > > >
> > > > > > +
> > > > > > +/*
> > > > > > + * Whether USERPTR buffers are allowed.
> > > > > > + *
> > > > > > + * This is disabled by default as USERPTR buffers are dangerous, but the option
> > > > > > + * is left to enable them if desired.
> > > > > > + */
> > > > > > +bool virtio_media_allow_userptr;
> > > > > > +module_param_named(allow_userptr, virtio_media_allow_userptr, bool, 0660);
> > > > >
> > > > >
> > > > > is this kind of thing common?
> > >
> > > There is one old media device that has it (saa7134).
> > >
> > > >
> > > > To be honest, I don't really know. I'm also not that familiar with the USERPTR
> > > > issues. I see a few references online about their use being discouraged due to
> > > > possible race conditions, perhaps that was the original motivation for this
> > > > parameter (I'm not the original author of this driver).
> > > >
> > > > I'm open to alternatives, feel free to let me know if you have a preference.
> > >
> > > We tend to not implement USERPTR on newer drivers. I suggest you
> > > to place the logic with regards to V4L2_MEMORY_USERPTR on a separate
> > > patch for further discussions.
> > >
> > > >
> > > > > > +
> > > > > > +/**
> > > > > > + * virtio_media_session_alloc - Allocate a new session.
> > > > > > + * @vv: virtio-media device the session belongs to.
> > > > > > + * @id: ID of the session.
> > > > > > + * @nonblocking_dequeue: whether dequeuing of buffers should be blocking or
> > > > > > + * not.
> > > > > > + *
> > > > > > + * The ``id`` and ``list`` fields must still be set by the caller.
> > > > >
> > > > > still in what sense?
> > > >
> > > > Based on the code below, I'm not so sure that the caller is responsible for
> > > > setting these values. They seem to be initialized in the function.
> > > >
> > > > Perhaps Alexandre Courbot (the original author) would know more. Unless he
> > > > says otherwise though I'm inclined to remove this comment.
> > > >
> > > > > > + */
> > > > > > +static struct virtio_media_session *
> > > > > > +virtio_media_session_alloc(struct virtio_media *vv, u32 id,
> > > > > > + struct file *file)
> > > > > > +{
> > > > > > + struct virtio_media_session *session;
> > > > > > + int i;
> > > > > > + int ret;
> > > > > > +
> > > > > > + session = kzalloc_obj(*session, GFP_KERNEL);
> > > > > > + if (!session)
> > > > > > + goto err_session;
> > > > > > +
> > > > > > + session->shadow_buf = kzalloc(VIRTIO_SHADOW_BUF_SIZE, GFP_KERNEL);
> > > > > > + if (!session->shadow_buf)
> > > > > > + goto err_shadow_buf;
> > > > > > +
> > > > > > + ret = sg_alloc_table(&session->command_sgs, DESC_CHAIN_MAX_LEN,
> > > > > > + GFP_KERNEL);
> > > > > > + if (ret)
> > > > > > + goto err_payload_sgs;
> > > > > > +
> > > > > > + session->id = id;
> > > > > > + session->nonblocking_dequeue = file->f_flags & O_NONBLOCK;
> > > > > > +
> > > > > > + INIT_LIST_HEAD(&session->list);
> > > > > > + v4l2_fh_init(&session->fh, &vv->video_dev);
> > > > > > + virtio_media_session_fh_add(session, file);
> > > > > > +
> > > > > > + for (i = 0; i <= VIRTIO_MEDIA_LAST_QUEUE; i++)
> > > > > > + INIT_LIST_HEAD(&session->queues[i].pending_dqbufs);
> > > > > > + mutex_init(&session->queues_lock);
> > > > > > +
> > > > > > + init_waitqueue_head(&session->dqbuf_wait);
> > > > > > +
> > > > > > + mutex_lock(&vv->sessions_lock);
> > > > > > + list_add_tail(&session->list, &vv->sessions);
> > > > > > + mutex_unlock(&vv->sessions_lock);
> > > > > > +
> > > > > > + return session;
> > > > > > +
> > > > > > +err_payload_sgs:
> > > > > > + kfree(session->shadow_buf);
> > > > > > +err_shadow_buf:
> > > > > > + kfree(session);
> > > > > > +err_session:
> > > > > > + return ERR_PTR(-ENOMEM);
> > > > > > +}
> > > > > > +
> > > > > > +/**
> > > > > > + * virtio_media_session_free - Free all resources of a session.
> > > > > > + * @vv: virtio-media device the session belongs to.
> > > > > > + * @session: session to destroy.
> > > > > > + *
> > > > > > + * All the resources of @sesssion, as well as the backing memory of @session
> > > > > > + * itself, are freed.
> > > > >
> > > > > why @ here and `` above? And typo in the name.
> > > >
> > > > The `@` here was an attempt to follow the guide here for referencing function
> > > > parameters:
> > > > https://docs.kernel.org/doc-guide/kernel-doc.html#highlights-and-cross-references
> > >
> > > Yes. This is part of Linux Kernel kernel-doc markup: when referring to
> > > struct fields, you should use @field (or ``field`` if one wants to place an
> > > asterisk on it, like ``*field``).
> > >
> > > >
> > > > That being said, I don't believe this file is 100% consistent with that. I will
> > > > spend some time cleaning up the comments throughout this patch set to get them
> > > > consistent for v5. Thanks!
> > >
> > > Please use it on a consistent way along the driver.
> > >
> > >
> > > Thanks,
> > > Mauro
> >
^ permalink raw reply
* Re: [PATCH v4 6/8] media: virtio: Add virtio_media_driver
From: Alistair Delva @ 2026-07-20 12:35 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Mauro Carvalho Chehab, Brian Daniels, Mauro Carvalho Chehab,
acourbot, aesteve, changyeon, daniel.almeida, eperezma, gnurou,
gurchetansingh, hverkuil, jasowang, linux-kernel, linux-media,
nicolas.dufresne, virtualization, xuanzhuo
In-Reply-To: <20260718132759-mutt-send-email-mst@kernel.org>
On Sat, Jul 18, 2026 at 10:29 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sun, Jul 12, 2026 at 08:57:26AM +0200, Mauro Carvalho Chehab wrote:
> > On Thu, 25 Jun 2026 16:18:48 -0400
> > Brian Daniels <briandaniels@google.com> wrote:
> >
> > > > > From: Alexandre Courbot <gnurou@gmail.com>
> > > > >
> > > > > virtio_media_driver.c provides the expected driver hooks, and support
> > > > > for mmapping and polling.
> > > > >
> > > > > Signed-off-by: Alexandre Courbot <gnurou@gmail.com>
> > > > > Co-developed-by: Brian Daniels <briandaniels@google.com>
> > > > > Signed-off-by: Brian Daniels <briandaniels@google.com>
> > > > > ---
> > > > > drivers/media/virtio/virtio_media_driver.c | 959 +++++++++++++++++++++
> > > > > 1 file changed, 959 insertions(+)
> > > > > create mode 100644 drivers/media/virtio/virtio_media_driver.c
> > > > >
> > > > > diff --git a/drivers/media/virtio/virtio_media_driver.c b/drivers/media/virtio/virtio_media_driver.c
> > > > > new file mode 100644
> > > > > index 000000000..d6363c673
> > > > > --- /dev/null
> > > > > +++ b/drivers/media/virtio/virtio_media_driver.c
> > > > > @@ -0,0 +1,959 @@
> > > > > +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0+
> > > > > +
> > > > > +/*
> > > > > + * Virtio-media driver.
> > > > > + *
> > > > > + * Copyright (c) 2024-2025 Google LLC.
> > > > > + */
> > > > > +
> > > > > +#include <linux/delay.h>
> > > > > +#include <linux/device.h>
> > > > > +#include <linux/dev_printk.h>
> > > > > +#include <linux/mm.h>
> > > > > +#include <linux/mutex.h>
> > > > > +#include <linux/scatterlist.h>
> > > > > +#include <linux/types.h>
> > > > > +#include <linux/videodev2.h>
> > > > > +#include <linux/vmalloc.h>
> > > > > +#include <linux/wait.h>
> > > > > +#include <linux/workqueue.h>
> > > > > +#include <linux/module.h>
> > > > > +#include <linux/moduleparam.h>
> > > > > +#include <linux/virtio.h>
> > > > > +#include <linux/virtio_config.h>
> > > > > +#include <linux/virtio_ids.h>
> > > > > +
> > > > > +#include <media/frame_vector.h>
> > > > > +#include <media/v4l2-dev.h>
> > > > > +#include <media/v4l2-event.h>
> > > > > +#include <media/videobuf2-memops.h>
> > > > > +#include <media/v4l2-device.h>
> > > > > +#include <media/v4l2-ioctl.h>
> > > > > +
> > > > > +#include "protocol.h"
> > > > > +#include "session.h"
> > > > > +#include "virtio_media.h"
> > > > > +
> > > > > +#define VIRTIO_MEDIA_NUM_EVENT_BUFS 16
> > > > > +
> > > > > +/* ID of the SHM region into which MMAP buffer will be mapped. */
> > > > > +#define VIRTIO_MEDIA_SHM_MMAP 0
> > > > > +
> > > > > +/*
> > > > > + * Name of the driver to expose to user-space.
> > > > > + *
> > > > > + * This is configurable because v4l2-compliance has workarounds specific to
> > > > > + * some drivers. When proxying these directly from the host, this allows it to
> > > > > + * apply them as needed.
> > > > > + */
> > > > > +char *virtio_media_driver_name;
> > > > > +module_param_named(driver_name, virtio_media_driver_name, charp, 0660);
> > > >
> > > >
> > > > Um. What? Not how it should be handled.
> > >
> > > I can remove this module param. I didn't end up using this when compliance testing.
> > > Instead, I patched v4l-utils:
> > > https://lore.kernel.org/all/20260528163448.4031965-1-briandaniels@google.com/
> > >
> > > Let me know if you think the v4l-utils patch is a good approach, otherwise let
> > > me know how you'd prefer to address the v4l2-compliance driver-specific workounds
> > > when they're being proxied with virtio-media.
> >
> > This kind of discussion should happen on a separate PR for v4l2-compliance,
> > c/c to the proper developers and maintainers of it.
>
>
> I wonder how migration can work when guest is tied to host driver model like
> this.
We haven't given migration much thought yet, but I think it wouldn't
be so different to GPU, where to do snapshot/migration we have to
record all initialization / context setup state and replay it against
the GPU driver on restore (see https://github.com/google/gfxstream).
Some migrations will be possible, some will not, and the host should
be able to decide.
Also, Brian mentioned the host device pass-through use case, but we
also have device implementations on the host that work with camera
emulators or some other data source like video/webrtc, which will
support snapshot/migration more easily. This use case will probably
will see more real-world use and migration will be more relevant.
> > >
> > > > > +
> > > > > +/*
> > > > > + * Whether USERPTR buffers are allowed.
> > > > > + *
> > > > > + * This is disabled by default as USERPTR buffers are dangerous, but the option
> > > > > + * is left to enable them if desired.
> > > > > + */
> > > > > +bool virtio_media_allow_userptr;
> > > > > +module_param_named(allow_userptr, virtio_media_allow_userptr, bool, 0660);
> > > >
> > > >
> > > > is this kind of thing common?
> >
> > There is one old media device that has it (saa7134).
> >
> > >
> > > To be honest, I don't really know. I'm also not that familiar with the USERPTR
> > > issues. I see a few references online about their use being discouraged due to
> > > possible race conditions, perhaps that was the original motivation for this
> > > parameter (I'm not the original author of this driver).
> > >
> > > I'm open to alternatives, feel free to let me know if you have a preference.
> >
> > We tend to not implement USERPTR on newer drivers. I suggest you
> > to place the logic with regards to V4L2_MEMORY_USERPTR on a separate
> > patch for further discussions.
> >
> > >
> > > > > +
> > > > > +/**
> > > > > + * virtio_media_session_alloc - Allocate a new session.
> > > > > + * @vv: virtio-media device the session belongs to.
> > > > > + * @id: ID of the session.
> > > > > + * @nonblocking_dequeue: whether dequeuing of buffers should be blocking or
> > > > > + * not.
> > > > > + *
> > > > > + * The ``id`` and ``list`` fields must still be set by the caller.
> > > >
> > > > still in what sense?
> > >
> > > Based on the code below, I'm not so sure that the caller is responsible for
> > > setting these values. They seem to be initialized in the function.
> > >
> > > Perhaps Alexandre Courbot (the original author) would know more. Unless he
> > > says otherwise though I'm inclined to remove this comment.
> > >
> > > > > + */
> > > > > +static struct virtio_media_session *
> > > > > +virtio_media_session_alloc(struct virtio_media *vv, u32 id,
> > > > > + struct file *file)
> > > > > +{
> > > > > + struct virtio_media_session *session;
> > > > > + int i;
> > > > > + int ret;
> > > > > +
> > > > > + session = kzalloc_obj(*session, GFP_KERNEL);
> > > > > + if (!session)
> > > > > + goto err_session;
> > > > > +
> > > > > + session->shadow_buf = kzalloc(VIRTIO_SHADOW_BUF_SIZE, GFP_KERNEL);
> > > > > + if (!session->shadow_buf)
> > > > > + goto err_shadow_buf;
> > > > > +
> > > > > + ret = sg_alloc_table(&session->command_sgs, DESC_CHAIN_MAX_LEN,
> > > > > + GFP_KERNEL);
> > > > > + if (ret)
> > > > > + goto err_payload_sgs;
> > > > > +
> > > > > + session->id = id;
> > > > > + session->nonblocking_dequeue = file->f_flags & O_NONBLOCK;
> > > > > +
> > > > > + INIT_LIST_HEAD(&session->list);
> > > > > + v4l2_fh_init(&session->fh, &vv->video_dev);
> > > > > + virtio_media_session_fh_add(session, file);
> > > > > +
> > > > > + for (i = 0; i <= VIRTIO_MEDIA_LAST_QUEUE; i++)
> > > > > + INIT_LIST_HEAD(&session->queues[i].pending_dqbufs);
> > > > > + mutex_init(&session->queues_lock);
> > > > > +
> > > > > + init_waitqueue_head(&session->dqbuf_wait);
> > > > > +
> > > > > + mutex_lock(&vv->sessions_lock);
> > > > > + list_add_tail(&session->list, &vv->sessions);
> > > > > + mutex_unlock(&vv->sessions_lock);
> > > > > +
> > > > > + return session;
> > > > > +
> > > > > +err_payload_sgs:
> > > > > + kfree(session->shadow_buf);
> > > > > +err_shadow_buf:
> > > > > + kfree(session);
> > > > > +err_session:
> > > > > + return ERR_PTR(-ENOMEM);
> > > > > +}
> > > > > +
> > > > > +/**
> > > > > + * virtio_media_session_free - Free all resources of a session.
> > > > > + * @vv: virtio-media device the session belongs to.
> > > > > + * @session: session to destroy.
> > > > > + *
> > > > > + * All the resources of @sesssion, as well as the backing memory of @session
> > > > > + * itself, are freed.
> > > >
> > > > why @ here and `` above? And typo in the name.
> > >
> > > The `@` here was an attempt to follow the guide here for referencing function
> > > parameters:
> > > https://docs.kernel.org/doc-guide/kernel-doc.html#highlights-and-cross-references
> >
> > Yes. This is part of Linux Kernel kernel-doc markup: when referring to
> > struct fields, you should use @field (or ``field`` if one wants to place an
> > asterisk on it, like ``*field``).
> >
> > >
> > > That being said, I don't believe this file is 100% consistent with that. I will
> > > spend some time cleaning up the comments throughout this patch set to get them
> > > consistent for v5. Thanks!
> >
> > Please use it on a consistent way along the driver.
> >
> >
> > Thanks,
> > Mauro
>
^ permalink raw reply
* Re: [PATCH 01/15] drm/exynos: remove dependency on DRM simple helpers
From: Thomas Zimmermann @ 2026-07-20 11:05 UTC (permalink / raw)
To: Diogo Silva, Jingoo Han, Inki Dae, Seung-Woo Kim, Kyungmin Park,
David Airlie, Simona Vetter, Krzysztof Kozlowski, Peter Griffin,
Alim Akhtar, Laurent Pinchart, Tomi Valkeinen, Maarten Lankhorst,
Maxime Ripard, Michal Simek, Thierry Reding, Mikko Perttunen,
Jonathan Hunter, Stefan Agner, Alison Wang, Anitha Chrisanthus,
David Airlie, Gerd Hoffmann, Dmitry Osipenko, Gurchetan Singh,
Chia-I Wu, Jyri Sarha, Liu Ying, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Chun-Kuang Hu,
Philipp Zabel, Matthias Brugger, AngeloGioacchino Del Regno,
Geert Uytterhoeven, Xinliang Liu, Sumit Semwal, Yongqin Liu,
John Stultz, Liviu Dudau, Neil Armstrong, Kevin Hilman,
Jerome Brunet, Martin Blumenstingl, Jonathan Corbet, Shuah Khan
Cc: dri-devel, linux-arm-kernel, linux-samsung-soc, linux-kernel,
linux-tegra, virtualization, imx, linux-mediatek,
linux-renesas-soc, linux-amlogic, linux-doc
In-Reply-To: <20260719-drm_simple_encoder_init-v1-1-a78c509e3062@gmail.com>
Hi
Am 19.07.26 um 01:35 schrieb Diogo Silva:
> The simple KMS helpers are deprecated because they only add an
> intermediate layer between drivers and atomic modesetting.
>
> Open-code drm_simple_encoder_init() by calling drm_encoder_init()
> directly and providing driver-local drm_encoder_funcs.
> Also check the return value from drm_encoder_init() to avoid silent
> failures.
>
> Signed-off-by: Diogo Silva <diogompaissilva@gmail.com>
> ---
> drivers/gpu/drm/exynos/exynos_dp.c | 13 +++++++++++--
> drivers/gpu/drm/exynos/exynos_drm_dpi.c | 14 ++++++++++++--
> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 11 +++++++++--
> drivers/gpu/drm/exynos/exynos_drm_vidi.c | 14 ++++++++++++--
> drivers/gpu/drm/exynos/exynos_hdmi.c | 15 +++++++++++++--
> 5 files changed, 57 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
> index b80540328150..1598892c602b 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp.c
> @@ -24,11 +24,11 @@
> #include <drm/drm_bridge.h>
> #include <drm/drm_bridge_connector.h>
> #include <drm/drm_crtc.h>
> +#include <drm/drm_encoder.h>
> #include <drm/drm_of.h>
> #include <drm/drm_panel.h>
> #include <drm/drm_print.h>
> #include <drm/drm_probe_helper.h>
> -#include <drm/drm_simple_kms_helper.h>
> #include <drm/exynos_drm.h>
>
> #include "exynos_drm_crtc.h"
> @@ -79,6 +79,10 @@ static void exynos_dp_nop(struct drm_encoder *encoder)
> /* do nothing */
> }
>
> +static const struct drm_encoder_funcs exynos_dp_encoder_funcs = {
> + .destroy = drm_encoder_cleanup,
> +};
> +
> static const struct drm_encoder_helper_funcs exynos_dp_encoder_helper_funcs = {
> .mode_set = exynos_dp_mode_set,
> .enable = exynos_dp_nop,
> @@ -95,7 +99,12 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
>
> dp->drm_dev = drm_dev;
>
> - drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_TMDS);
> + ret = drm_encoder_init(drm_dev, encoder, &exynos_dp_encoder_funcs,
> + DRM_MODE_ENCODER_TMDS, NULL);
> + if (ret) {
> + dev_err(dp->dev, "Failed to initialize encoder\n");
Preferably use drm_err and drm_dev.
> + return ret;
> + }
>
> drm_encoder_helper_add(encoder, &exynos_dp_encoder_helper_funcs);
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> index 0dc36df6ada3..4e42a1da81d1 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> @@ -12,10 +12,10 @@
> #include <linux/regulator/consumer.h>
>
> #include <drm/drm_atomic_helper.h>
> +#include <drm/drm_encoder.h>
> #include <drm/drm_panel.h>
> #include <drm/drm_print.h>
> #include <drm/drm_probe_helper.h>
> -#include <drm/drm_simple_kms_helper.h>
>
> #include <video/of_videomode.h>
> #include <video/videomode.h>
> @@ -140,6 +140,10 @@ static void exynos_dpi_disable(struct drm_encoder *encoder)
> }
> }
>
> +static const struct drm_encoder_funcs exynos_dpi_encoder_funcs = {
> + .destroy = drm_encoder_cleanup,
> +};
> +
> static const struct drm_encoder_helper_funcs exynos_dpi_encoder_helper_funcs = {
> .mode_set = exynos_dpi_mode_set,
> .enable = exynos_dpi_enable,
> @@ -194,7 +198,13 @@ int exynos_dpi_bind(struct drm_device *dev, struct drm_encoder *encoder)
> {
> int ret;
>
> - drm_simple_encoder_init(dev, encoder, DRM_MODE_ENCODER_TMDS);
> + ret = drm_encoder_init(dev, encoder, &exynos_dpi_encoder_funcs,
> + DRM_MODE_ENCODER_TMDS, NULL);
> + if (ret) {
> + DRM_DEV_ERROR(encoder_to_dpi(encoder)->dev,
It just failed to init the encoder, so better not use it here. The
device should be the same as passed to the function via 'dev'.
> + "failed to create encoder ret = %d\n", ret);
> + return ret;
> + }
>
> drm_encoder_helper_add(encoder, &exynos_dpi_encoder_helper_funcs);
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index c4d098ab7863..6b7561ac9bb0 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -13,7 +13,7 @@
>
> #include <drm/bridge/samsung-dsim.h>
> #include <drm/drm_probe_helper.h>
> -#include <drm/drm_simple_kms_helper.h>
> +#include <drm/drm_encoder.h>
>
> #include "exynos_drm_crtc.h"
> #include "exynos_drm_drv.h"
> @@ -22,6 +22,10 @@ struct exynos_dsi {
> struct drm_encoder encoder;
> };
>
> +static const struct drm_encoder_funcs exynos_drm_dsi_encoder_funcs = {
> + .destroy = drm_encoder_cleanup,
> +};
> +
> static irqreturn_t exynos_dsi_te_irq_handler(struct samsung_dsim *dsim)
> {
> struct exynos_dsi *dsi = dsim->priv;
> @@ -79,7 +83,10 @@ static int exynos_dsi_bind(struct device *dev, struct device *master, void *data
> struct drm_device *drm_dev = data;
> int ret;
>
> - drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_TMDS);
> + ret = drm_encoder_init(drm_dev, encoder, &exynos_drm_dsi_encoder_funcs,
> + DRM_MODE_ENCODER_TMDS, NULL);
> + if (ret)
> + return ret;
>
> ret = exynos_drm_set_possible_crtcs(encoder, EXYNOS_DISPLAY_TYPE_LCD);
> if (ret < 0)
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> index 67bbf9b8bc0e..59dea853d364 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> @@ -13,10 +13,10 @@
>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_edid.h>
> +#include <drm/drm_encoder.h>
> #include <drm/drm_framebuffer.h>
> #include <drm/drm_print.h>
> #include <drm/drm_probe_helper.h>
> -#include <drm/drm_simple_kms_helper.h>
> #include <drm/drm_vblank.h>
> #include <drm/exynos_drm.h>
>
> @@ -403,6 +403,10 @@ static void exynos_vidi_disable(struct drm_encoder *encoder)
> {
> }
>
> +static const struct drm_encoder_funcs exynos_vidi_encoder_funcs = {
> + .destroy = drm_encoder_cleanup,
> +};
> +
> static const struct drm_encoder_helper_funcs exynos_vidi_encoder_helper_funcs = {
> .mode_set = exynos_vidi_mode_set,
> .enable = exynos_vidi_enable,
> @@ -445,7 +449,13 @@ static int vidi_bind(struct device *dev, struct device *master, void *data)
> return PTR_ERR(ctx->crtc);
> }
>
> - drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_TMDS);
> + ret = drm_encoder_init(drm_dev, encoder, &exynos_vidi_encoder_funcs,
> + DRM_MODE_ENCODER_TMDS, NULL);
> + if (ret) {
> + DRM_DEV_ERROR(dev, "failed to initialize encoder ret = %d\n",
> + ret);
Again, you rather want drm_err() with drm_dev here.
I've briefly looked over the series and many patches seem affected.
Please prefer drm_ logging functions and DRM devices over the plain
device equivalents.
Best regards
Thomas
> + return ret;
> + }
>
> drm_encoder_helper_add(encoder, &exynos_vidi_encoder_helper_funcs);
>
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 09b2cabb236f..f44586ce0fdf 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -36,9 +36,9 @@
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_bridge.h>
> #include <drm/drm_edid.h>
> +#include <drm/drm_encoder.h>
> #include <drm/drm_print.h>
> #include <drm/drm_probe_helper.h>
> -#include <drm/drm_simple_kms_helper.h>
>
> #include "exynos_drm_crtc.h"
> #include "regs-hdmi.h"
> @@ -1575,6 +1575,11 @@ static void hdmi_disable(struct drm_encoder *encoder)
> mutex_unlock(&hdata->mutex);
> }
>
> +static const struct drm_encoder_funcs exynos_hdmi_encoder_funcs = {
> + .destroy = drm_encoder_cleanup,
> +};
> +
> +
> static const struct drm_encoder_helper_funcs exynos_hdmi_encoder_helper_funcs = {
> .mode_fixup = hdmi_mode_fixup,
> .enable = hdmi_enable,
> @@ -1862,7 +1867,13 @@ static int hdmi_bind(struct device *dev, struct device *master, void *data)
>
> hdata->phy_clk.enable = hdmiphy_clk_enable;
>
> - drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_TMDS);
> + ret = drm_encoder_init(drm_dev, encoder, &exynos_hdmi_encoder_funcs,
> + DRM_MODE_ENCODER_TMDS, NULL);
> + if (ret) {
> + DRM_DEV_ERROR(dev, "failed to initialize encoder ret = %d\n",
> + ret);
> + return ret;
> + }
>
> drm_encoder_helper_add(encoder, &exynos_hdmi_encoder_helper_funcs);
>
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)
^ permalink raw reply
* Re: [PATCH 2/3] sched: Convert paravirt_steal to new static key APIs
From: Jürgen Groß @ 2026-07-20 10:35 UTC (permalink / raw)
To: Hongyan Xia, mingo@redhat.com, peterz@infradead.org,
juri.lelli@redhat.com, vincent.guittot@linaro.org,
dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com,
mgorman@suse.de, vschneid@redhat.com, kprateek.nayak@amd.com,
Ajay Kaher, Alexey Makhalov, Broadcom internal kernel review list,
Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Thomas Gleixner, Borislav Petkov,
Dave Hansen, x86@kernel.org, H. Peter Anvin, Paolo Bonzini,
Vitaly Kuznetsov, Stefano Stabellini, Oleksandr Tyshchenko
Cc: Jiazi Li, linux-kernel@vger.kernel.org,
virtualization@lists.linux.dev,
linux-arm-kernel@lists.infradead.org, loongarch@lists.linux.dev,
linuxppc-dev@lists.ozlabs.org, linux-riscv@lists.infradead.org,
kvm@vger.kernel.org, xen-devel@lists.xenproject.org
In-Reply-To: <773bbd8fcd5ef2ebfe1c64303c7a71a56368de69.1784538478.git.hongyan.xia@transsion.com>
[-- Attachment #1.1.1: Type: text/plain, Size: 375 bytes --]
On 20.07.26 11:38, Hongyan Xia wrote:
> From: Hongyan Xia <hongyan.xia@transsion.com>
>
> paravirt_steal_rq_enabled and paravirt_steal_enabled use raw static_key
> APIs which are now deprecated. Use the new API instead.
>
> No functional change.
>
> Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
Acked-by: Juergen Gross <jgross@suse.com>
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply
* [PATCH v5 4/5] vhost: synchronize with RCU readers when freeing workers
From: Andrey Drobyshev @ 2026-07-20 10:22 UTC (permalink / raw)
To: linux-kernel
Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den, andrey.drobyshev
In-Reply-To: <20260720102241.371610-1-andrey.drobyshev@virtuozzo.com>
vhost_vq_work_queue() only holds the RCU read lock while it dereferences
vq->worker and queues work on it. vhost_workers_free() however clears
the vq->worker pointers and immediately frees the workers, without
waiting for a grace period. A caller that fetched the worker right
before the pointer was cleared can therefore still be queueing work on
it while it is freed. And even when the queueing itself wins the race,
the work is never run, so its VHOST_WORK_QUEUED bit stays set and all
future attempts to queue it are silently skipped.
None of the current callers can actually hit this: net and scsi stop
their virtqueues before the workers are freed, and vsock unhashes the
device and does synchronize_rcu() of its own in vhost_vsock_dev_release()
before the workers go away. But the upcoming VHOST_RESET_OWNER support
in vhost-vsock keeps the device hashed while its workers are freed, so
the lockless send/cancel paths become able to race with the teardown.
Fix this by clearing the vq->worker pointers, waiting for a grace
period, and then flushing the workers so any work the last readers
queued runs before the workers are freed.
Fixes: 228a27cf78af ("vhost: Allow worker switching while work is queueing")
Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
drivers/vhost/vhost.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 4c525b3e16ea..d6e235c25254 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -729,6 +729,17 @@ static void vhost_workers_free(struct vhost_dev *dev)
for (i = 0; i < dev->nvqs; i++)
rcu_assign_pointer(dev->vqs[i]->worker, NULL);
+
+ /*
+ * vhost_vq_work_queue() reads vq->worker under rcu_read_lock(), so a
+ * reader that fetched a worker before we cleared the pointers above
+ * may still be queueing work on it. Wait for those readers to
+ * finish, then flush so any work they queued runs (clearing
+ * VHOST_WORK_QUEUED) before the workers are freed.
+ */
+ synchronize_rcu();
+ vhost_dev_flush(dev);
+
/*
* Free the default worker we created and cleanup workers userspace
* created but couldn't clean up (it forgot or crashed).
--
2.47.1
^ permalink raw reply related
* [PATCH v5 5/5] vhost/vsock: add VHOST_RESET_OWNER ioctl
From: Andrey Drobyshev @ 2026-07-20 10:22 UTC (permalink / raw)
To: linux-kernel
Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den, andrey.drobyshev
In-Reply-To: <20260720102241.371610-1-andrey.drobyshev@virtuozzo.com>
From: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
This ioctl is needed for QEMU's CPR (checkpoint-restore) migration of
the guest with vhost-vsock device. For this to work, we need to reset
the device ownership on the source side by calling RESET_OWNER, and then
claim it on the dest side by calling SET_OWNER. We expect not to lose any
AF_VSOCK connection while this happens.
To that end, unlike the release path, RESET_OWNER keeps the guest CID
hashed: established connections survive, and host sends issued while
the device is between owners simply stay on send_pkt_queue until the
next device start drains them.
Since the device stays reachable through the CID hash, the lockless
send/cancel paths can race with the worker teardown in
vhost_workers_free(). The previous commit ("vhost: synchronize with
RCU readers when freeing workers") makes that safe.
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
drivers/vhost/vsock.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index d5022d21120b..86f25ff80722 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -903,6 +903,29 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
return -EFAULT;
}
+static long vhost_vsock_reset_owner(struct vhost_vsock *vsock)
+{
+ struct vhost_iotlb *umem;
+ long err;
+
+ mutex_lock(&vsock->dev.mutex);
+ err = vhost_dev_check_owner(&vsock->dev);
+ if (err)
+ goto done;
+ umem = vhost_dev_reset_owner_prepare();
+ if (!umem) {
+ err = -ENOMEM;
+ goto done;
+ }
+ vhost_vsock_drop_backends(vsock);
+ vhost_vsock_flush(vsock);
+ vhost_dev_stop(&vsock->dev);
+ vhost_dev_reset_owner(&vsock->dev, umem);
+done:
+ mutex_unlock(&vsock->dev.mutex);
+ return err;
+}
+
static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl,
unsigned long arg)
{
@@ -946,6 +969,8 @@ static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl,
return -EOPNOTSUPP;
vhost_set_backend_features(&vsock->dev, features);
return 0;
+ case VHOST_RESET_OWNER:
+ return vhost_vsock_reset_owner(vsock);
default:
mutex_lock(&vsock->dev.mutex);
r = vhost_dev_ioctl(&vsock->dev, ioctl, argp);
--
2.47.1
^ permalink raw reply related
* [PATCH v5 3/5] vhost/vsock: re-scan TX virtqueue on device start
From: Andrey Drobyshev @ 2026-07-20 10:22 UTC (permalink / raw)
To: linux-kernel
Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den, andrey.drobyshev
In-Reply-To: <20260720102241.371610-1-andrey.drobyshev@virtuozzo.com>
During QEMU CPR live-update (and VHOST_RESET_OWNER in general) the guest
keeps running while the host drops and later re-attaches vhost backends.
If the guest adds a buffer to the TX virtqueue (guest->host) and kicks
while the backend is temporarily NULL (between vhost_vsock_drop_backends()
and the next vhost_vsock_start()), then the kick is delivered to the
vhost worker, handle_tx_kick() sees a NULL backend and returns, and the
kick signal is consumed. The buffer is then left in the ring.
Then upon device start vhost_vsock_start() only re-kicks the RX send
worker, never the TX VQ, so the buffer is processed only if the guest
happens to kick again. But if the guest itself is now waiting for data
from the host, it will never kick TX VQ again, and we end up in a
deadlock.
The issue itself is pre-existing, but it only manifests during a device
pause caused by VHOST_RESET_OWNER. Namely, the deadlock is reproduced
during active host->guest socat data transfer under multiple consecutive
CPR live-update's.
To fix this, in vhost_vsock_start(), after kicking the RX send worker, also
queue the TX vq poll so any buffers the guest enqueued while we were paused
get scanned.
The VHOST_RESET_OWNER ioctl itself is implemented in the following
patch, thus this patch is a preparation to support VHOST_RESET_OWNER.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
drivers/vhost/vsock.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 27169a09e87e..d5022d21120b 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -646,6 +646,13 @@ static int vhost_vsock_start(struct vhost_vsock *vsock)
*/
vhost_vq_work_queue(&vsock->vqs[VSOCK_VQ_RX], &vsock->send_pkt_work);
+ /* The guest may have added TX buffers while the device was stopped
+ * (e.g. across VHOST_RESET_OWNER) and their kicks got consumed by
+ * the NULL-backend window. Re-scan the TX VQ, mirroring the RX
+ * send-worker kick above.
+ */
+ vhost_poll_queue(&vsock->vqs[VSOCK_VQ_TX].poll);
+
mutex_unlock(&vsock->dev.mutex);
return 0;
--
2.47.1
^ permalink raw reply related
* [PATCH v5 2/5] vhost/vsock: suppress EHOSTUNREACH fast-fail during CPR pause
From: Andrey Drobyshev @ 2026-07-20 10:22 UTC (permalink / raw)
To: linux-kernel
Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den, andrey.drobyshev
In-Reply-To: <20260720102241.371610-1-andrey.drobyshev@virtuozzo.com>
Earlier commit bb26ed5f3a8b ("vhost/vsock: Refuse the connection
immediately when guest isn't ready") added a fast-fail in
vhost_transport_send_pkt(). It rejects every host send with -EHOSTUNREACH
until the destination calls SET_RUNNING(1). The fast-fail condition checks
whether device's backends are dropped, and if they're, the guest is
considered to be not ready.
However, there might be other reasons for backends to be nulled. In
particular, when QEMU is performing CPR (checkpoint-restore) migration,
device ownership is being RESET and SET again, which leads to backends
drop and reattach. If we end up connecting during this window, an
AF_VSOCK client gets -EHOSTUNREACH, which is wrong.
Add an 'ever_started' flag which is set once in vhost_vsock_start() and is
never cleared. The behaviour changes to:
* When device was never started -> flag is unset -> no listener can
exist yet -> fast-fail;
* Once the device starts -> flag is set -> we don't fast-fail ->
we queue and preserve during any later stop / CPR pause.
The VHOST_RESET_OWNER ioctl is implemented in a following patch, and
without RESET_OWNER the problem we fix here isn't manifesting - thus
this patch is a preparation to support RESET_OWNER.
Important caveat: after the first start, a connect during any stopped
window is queued instead of fast-failed. That was the behaviour before
the patch bb26ed5f3a8b, and we're restoring it now. However we still
keep the behaviour originally intended by that commit (i.e. fast-fail if
there's no real listener yet) while fixing the CPR path.
Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
drivers/vhost/vsock.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index b12221ce6faf..27169a09e87e 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -61,6 +61,7 @@ struct vhost_vsock {
u32 guest_cid;
bool seqpacket_allow;
+ bool ever_started; /* set on first SET_RUNNING(1); never cleared */
};
static u32 vhost_transport_get_local_cid(void)
@@ -302,17 +303,12 @@ vhost_transport_send_pkt(struct sk_buff *skb, struct net *net)
return -ENODEV;
}
- /* Fast-fail if the guest hasn't enabled the RX vq yet. Queuing the packet
- * and making the caller wait is pointless: even if the guest manages to init
- * within the timeout, it'll immediately reply with RST, because there's no
- * listener on the port yet.
- *
- * vhost_vq_get_backend() without vq->mutex is acceptable here: locking
- * the mutex would be too expensive in this hot path, and we already have
- * all the outcomes covered: if the backend becomes NULL right after the check,
- * vhost_transport_do_send_pkt() will check it under the mutex anyway.
+ /* Fast-fail until the guest first enables the device (SET_RUNNING(1)).
+ * Before that there is no listener, so queuing is pointless.
+ * 'ever_started' is never cleared, so once we're up we keep queuing
+ * across later stop / CPR-pause windows.
*/
- if (unlikely(!data_race(vhost_vq_get_backend(&vsock->vqs[VSOCK_VQ_RX])))) {
+ if (unlikely(!READ_ONCE(vsock->ever_started))) {
rcu_read_unlock();
kfree_skb(skb);
return -EHOSTUNREACH;
@@ -640,6 +636,11 @@ static int vhost_vsock_start(struct vhost_vsock *vsock)
mutex_unlock(&vq->mutex);
}
+ /* Set 'ever_started' flag on the first start; never cleared, so send_pkt
+ * keeps queuing (instead of fast-failing) on later stop / CPR pauses.
+ */
+ WRITE_ONCE(vsock->ever_started, true);
+
/* Some packets may have been queued before the device was started,
* let's kick the send worker to send them.
*/
@@ -728,6 +729,7 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
vsock->guest_cid = 0; /* no CID assigned yet */
vsock->seqpacket_allow = false;
+ vsock->ever_started = false;
atomic_set(&vsock->queued_replies, 0);
--
2.47.1
^ permalink raw reply related
* [PATCH v5 1/5] vhost/vsock: split out vhost_vsock_drop_backends helper
From: Andrey Drobyshev @ 2026-07-20 10:22 UTC (permalink / raw)
To: linux-kernel
Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den, andrey.drobyshev
In-Reply-To: <20260720102241.371610-1-andrey.drobyshev@virtuozzo.com>
From: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Split the actual backend dropping part from vhost_vsock_stop. We're
going to need it for the VHOST_RESET_OWNER implementation in the
following patch, when vsock->dev.mutex is already taken and owner is
checked.
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
---
drivers/vhost/vsock.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 9aaab6bb8061..b12221ce6faf 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -664,9 +664,24 @@ static int vhost_vsock_start(struct vhost_vsock *vsock)
return ret;
}
-static int vhost_vsock_stop(struct vhost_vsock *vsock, bool check_owner)
+static void vhost_vsock_drop_backends(struct vhost_vsock *vsock)
{
+ struct vhost_virtqueue *vq;
size_t i;
+
+ lockdep_assert_held(&vsock->dev.mutex);
+
+ for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) {
+ vq = &vsock->vqs[i];
+
+ mutex_lock(&vq->mutex);
+ vhost_vq_set_backend(vq, NULL);
+ mutex_unlock(&vq->mutex);
+ }
+}
+
+static int vhost_vsock_stop(struct vhost_vsock *vsock, bool check_owner)
+{
int ret = 0;
mutex_lock(&vsock->dev.mutex);
@@ -677,14 +692,7 @@ static int vhost_vsock_stop(struct vhost_vsock *vsock, bool check_owner)
goto err;
}
- for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) {
- struct vhost_virtqueue *vq = &vsock->vqs[i];
-
- mutex_lock(&vq->mutex);
- vhost_vq_set_backend(vq, NULL);
- mutex_unlock(&vq->mutex);
- }
-
+ vhost_vsock_drop_backends(vsock);
err:
mutex_unlock(&vsock->dev.mutex);
return ret;
--
2.47.1
^ permalink raw reply related
* [PATCH v5 0/5] vhost/vsock: add support for VHOST_RESET_OWNER and CPR migration
From: Andrey Drobyshev @ 2026-07-20 10:22 UTC (permalink / raw)
To: linux-kernel
Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den, andrey.drobyshev
v4 -> v5:
* Patch 4:
- call synchronize_rcu() unconditionally;
- call vhost_dev_flush(dev) instead of vhost_run_work_list(worker)
to drain remaining work;
- reword comment and commit message.
v4: https://lore.kernel.org/virtualization/20260714151638.143019-1-andrey.drobyshev@virtuozzo.com
Andrey Drobyshev (3):
vhost/vsock: suppress EHOSTUNREACH fast-fail during CPR pause
vhost/vsock: re-scan TX virtqueue on device start
vhost: synchronize with RCU readers when freeing workers
Pavel Tikhomirov (2):
vhost/vsock: split out vhost_vsock_drop_backends helper
vhost/vsock: add VHOST_RESET_OWNER ioctl
drivers/vhost/vhost.c | 11 ++++++
drivers/vhost/vsock.c | 80 +++++++++++++++++++++++++++++++++----------
2 files changed, 72 insertions(+), 19 deletions(-)
--
2.47.1
^ permalink raw reply
* [PATCH 2/3] sched: Convert paravirt_steal to new static key APIs
From: Hongyan Xia @ 2026-07-20 9:38 UTC (permalink / raw)
To: mingo@redhat.com, peterz@infradead.org, juri.lelli@redhat.com,
vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
vschneid@redhat.com, kprateek.nayak@amd.com, Juergen Gross,
Ajay Kaher, Alexey Makhalov, Broadcom internal kernel review list,
Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Thomas Gleixner, Borislav Petkov,
Dave Hansen, x86@kernel.org, H. Peter Anvin, Paolo Bonzini,
Vitaly Kuznetsov, Stefano Stabellini, Oleksandr Tyshchenko
Cc: Jiazi Li, linux-kernel@vger.kernel.org,
virtualization@lists.linux.dev,
linux-arm-kernel@lists.infradead.org, loongarch@lists.linux.dev,
linuxppc-dev@lists.ozlabs.org, linux-riscv@lists.infradead.org,
kvm@vger.kernel.org, xen-devel@lists.xenproject.org
In-Reply-To: <cover.1784538478.git.hongyan.xia@transsion.com>
From: Hongyan Xia <hongyan.xia@transsion.com>
paravirt_steal_rq_enabled and paravirt_steal_enabled use raw static_key
APIs which are now deprecated. Use the new API instead.
No functional change.
Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
---
arch/arm64/kernel/paravirt.c | 4 ++--
arch/loongarch/kernel/paravirt.c | 4 ++--
arch/powerpc/platforms/pseries/setup.c | 4 ++--
arch/riscv/kernel/paravirt.c | 4 ++--
arch/x86/kernel/cpu/vmware.c | 4 ++--
arch/x86/kernel/kvm.c | 4 ++--
drivers/xen/time.c | 4 ++--
include/linux/sched/cputime.h | 6 +++---
kernel/sched/core.c | 4 ++--
kernel/sched/cputime.c | 4 ++--
10 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/arch/arm64/kernel/paravirt.c b/arch/arm64/kernel/paravirt.c
index 572efb96b23f..30bf61d031eb 100644
--- a/arch/arm64/kernel/paravirt.c
+++ b/arch/arm64/kernel/paravirt.c
@@ -157,9 +157,9 @@ int __init pv_time_init(void)
static_call_update(pv_steal_clock, para_steal_clock);
- static_key_slow_inc(¶virt_steal_enabled);
+ static_branch_inc(¶virt_steal_enabled);
if (steal_acc)
- static_key_slow_inc(¶virt_steal_rq_enabled);
+ static_branch_inc(¶virt_steal_rq_enabled);
pr_info("using stolen time PV\n");
diff --git a/arch/loongarch/kernel/paravirt.c b/arch/loongarch/kernel/paravirt.c
index 10821cce554c..e8965a3f8082 100644
--- a/arch/loongarch/kernel/paravirt.c
+++ b/arch/loongarch/kernel/paravirt.c
@@ -308,10 +308,10 @@ int __init pv_time_init(void)
static_call_update(pv_steal_clock, paravt_steal_clock);
- static_key_slow_inc(¶virt_steal_enabled);
+ static_branch_inc(¶virt_steal_enabled);
#ifdef CONFIG_PARAVIRT_TIME_ACCOUNTING
if (steal_acc)
- static_key_slow_inc(¶virt_steal_rq_enabled);
+ static_branch_inc(¶virt_steal_rq_enabled);
#endif
if (static_key_enabled(&virt_preempt_key))
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 1223dc961242..8dcbc4bb7025 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -852,9 +852,9 @@ static void __init pSeries_setup_arch(void)
static_branch_enable(&shared_processor);
pv_spinlocks_init();
#ifdef CONFIG_PARAVIRT_TIME_ACCOUNTING
- static_key_slow_inc(¶virt_steal_enabled);
+ static_branch_inc(¶virt_steal_enabled);
if (steal_acc)
- static_key_slow_inc(¶virt_steal_rq_enabled);
+ static_branch_inc(¶virt_steal_rq_enabled);
#endif
}
diff --git a/arch/riscv/kernel/paravirt.c b/arch/riscv/kernel/paravirt.c
index 5f56be79cd06..9c13a6f1ea2a 100644
--- a/arch/riscv/kernel/paravirt.c
+++ b/arch/riscv/kernel/paravirt.c
@@ -116,9 +116,9 @@ int __init pv_time_init(void)
static_call_update(pv_steal_clock, pv_time_steal_clock);
- static_key_slow_inc(¶virt_steal_enabled);
+ static_branch_inc(¶virt_steal_enabled);
if (steal_acc)
- static_key_slow_inc(¶virt_steal_rq_enabled);
+ static_branch_inc(¶virt_steal_rq_enabled);
pr_info("Computing paravirt steal-time\n");
diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
index 34b73573b108..f7ab9e7902cf 100644
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -328,9 +328,9 @@ static int vmware_cpu_down_prepare(unsigned int cpu)
static __init int activate_jump_labels(void)
{
if (has_steal_clock) {
- static_key_slow_inc(¶virt_steal_enabled);
+ static_branch_inc(¶virt_steal_enabled);
if (steal_acc)
- static_key_slow_inc(¶virt_steal_rq_enabled);
+ static_branch_inc(¶virt_steal_rq_enabled);
}
return 0;
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index dcef84da304b..d3dcd64f22c2 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -1052,9 +1052,9 @@ const __initconst struct hypervisor_x86 x86_hyper_kvm = {
static __init int activate_jump_labels(void)
{
if (has_steal_clock) {
- static_key_slow_inc(¶virt_steal_enabled);
+ static_branch_inc(¶virt_steal_enabled);
if (steal_acc)
- static_key_slow_inc(¶virt_steal_rq_enabled);
+ static_branch_inc(¶virt_steal_rq_enabled);
}
return 0;
diff --git a/drivers/xen/time.c b/drivers/xen/time.c
index a2be0a4d45b0..a02d48a2aa68 100644
--- a/drivers/xen/time.c
+++ b/drivers/xen/time.c
@@ -169,7 +169,7 @@ void __init xen_time_setup_guest(void)
static_call_update(pv_steal_clock, xen_steal_clock);
- static_key_slow_inc(¶virt_steal_enabled);
+ static_branch_inc(¶virt_steal_enabled);
if (xen_runstate_remote)
- static_key_slow_inc(¶virt_steal_rq_enabled);
+ static_branch_inc(¶virt_steal_rq_enabled);
}
diff --git a/include/linux/sched/cputime.h b/include/linux/sched/cputime.h
index e90efaf6d26e..694126411dfe 100644
--- a/include/linux/sched/cputime.h
+++ b/include/linux/sched/cputime.h
@@ -182,9 +182,9 @@ extern unsigned long long
task_sched_runtime(struct task_struct *task);
#ifdef CONFIG_PARAVIRT
-struct static_key;
-extern struct static_key paravirt_steal_enabled;
-extern struct static_key paravirt_steal_rq_enabled;
+#include <linux/jump_label.h>
+DECLARE_STATIC_KEY_FALSE(paravirt_steal_enabled);
+DECLARE_STATIC_KEY_FALSE(paravirt_steal_rq_enabled);
#ifdef CONFIG_HAVE_PV_STEAL_CLOCK_GEN
u64 dummy_steal_clock(int cpu);
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 96226707c2f6..786975d4ca1e 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -795,7 +795,7 @@ struct rq *_task_rq_lock(struct task_struct *p, struct rq_flags *rf)
/* Use CONFIG_PARAVIRT as this will avoid more #ifdef in arch code. */
#ifdef CONFIG_PARAVIRT
-struct static_key paravirt_steal_rq_enabled;
+DEFINE_STATIC_KEY_FALSE(paravirt_steal_rq_enabled);
#endif
static void update_rq_clock_task(struct rq *rq, s64 delta)
@@ -834,7 +834,7 @@ static void update_rq_clock_task(struct rq *rq, s64 delta)
}
#endif
#ifdef CONFIG_PARAVIRT_TIME_ACCOUNTING
- if (static_key_false((¶virt_steal_rq_enabled))) {
+ if (static_branch_unlikely(¶virt_steal_rq_enabled)) {
u64 prev_steal;
steal = prev_steal = paravirt_steal_clock(cpu_of(rq));
diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index 06bddaa738e5..f16970ca81d0 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -255,7 +255,7 @@ void __account_forceidle_time(struct task_struct *p, u64 delta)
* occasion account more time than the calling functions think elapsed.
*/
#ifdef CONFIG_PARAVIRT
-struct static_key paravirt_steal_enabled;
+DEFINE_STATIC_KEY_FALSE(paravirt_steal_enabled);
#ifdef CONFIG_HAVE_PV_STEAL_CLOCK_GEN
static u64 native_steal_clock(int cpu)
@@ -270,7 +270,7 @@ DEFINE_STATIC_CALL(pv_steal_clock, native_steal_clock);
static __always_inline u64 steal_account_process_time(u64 maxtime)
{
#ifdef CONFIG_PARAVIRT
- if (static_key_false(¶virt_steal_enabled)) {
+ if (static_branch_unlikely(¶virt_steal_enabled)) {
u64 steal;
steal = paravirt_steal_clock(smp_processor_id());
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: David Hildenbrand (Arm) @ 2026-07-20 9:19 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Carlos Bilbao, Michael S. Tsirkin, Hari Mishal, Jason Wang,
Xuan Zhuo, Eugenio Pérez, virtualization, linux-kernel,
elena.reshetova, huster, mhollick, jiska.classen
In-Reply-To: <2026072036-outburst-rebel-c71b@gregkh>
>>> We've the virto-mem config struct layout and the kernel source, so for
>>> obvious fixes like a NULL check, static analysis is better than fuzzing.
>>> Claude took a few mins to find me two examples:
>>>
>>> Patch 1: virtio-mem: reject non-power-of-two device_block_size
>>> This one is for virtio_mem_init() to check if
>>> !is_power_of_2(vm->device_block_size)
>>>
>>> Patch 2: virto-mem: validate region_size and usable_region_size
>>> THis one checks region_size != 0 and vm->usable_reion_size >
>>> vm->region_size.
>>>
>>> An endless factory of "silly" checks like these are low hanging fruit.
>>
>> "silly" is the right word.
>
> "silly" in what way?
>
As in producing "silly" low-hanging fruit patches that don't move the needle
when it comes to security.
> Seriously, I'm trying to figure out what you all care about here and
> what exactly the threat model you want this driver to work in, and I'm
> getting conflicting answers.
>
> Either you all do worry about the "device" sending bad data and want to
> protect from that, or you don't and you trust it. Pick one please so
> that we know how to deal with these bug reports we are getting.
>
> For example, for USB we have said our threat model is:
>
> - we do NOT trust the device before a driver is bound to the device,
> so if a malicious device can do something to the kernel, the kernel
> needs to be fixed.
> - During the probe() call for a USB driver, the driver does NOT trust
> the device, and again, anything a malicious device can do to the
> kernel, the kernel should fix.
> - After probe() for a USB driver succeeds, it's up to the driver if it
> wants to validate all data coming from the device or not. Right
> now, in general, the kernel trusts the device at that point in time
> so additional checks are discretionary and at the whim of the
> maintainer.
>
> For that last point, I will note that some BIG users of Linux (i.e.
> billions of Android devices) still explicitly do NOT want to trust the
> USB device at this point in time, and are relying on the kernel to
> protect the system from bad devices. In that case, various patches have
> been taken to different drivers and subsystems to play whack-a-mole on
> while Android gets their act together to finally come up with a solid
> defensive plan (like ChromeOS has had for a decade.) It will be seen
> which happens first, all drivers are properly fuzzed and fixed up, or
> Android gets their act together and finally fixes their b0rked system
> trust model. I think Android management is relying on the kernel
> community to do the kernel work as they keep refusing to staff the
> userspace work that they need to do here...
Right, and for virtio devices trusting the device after probe is just extremely
questionable.
What changes during probe that the device suddenly sends us good data?
Why would a hypervisor that tried to break us before probe not try to break us
after probe?
>
> And yes, I really need to write this up in a more solid document for USB
> and get it into the tree, but at least this email thread has forced me
> to write down the above :)
>
>
> So, again, for virtio drivers, what exactly do you all want to say is
> your threat model that the drivers need to handle? Can you all agree on
> something please? Otherwise, for new developers like Hari, this is
> totaly confusion as to what they should be doing.
Well, I am also totally confused why we end up checking against some MUST
clauses in the spec, but not against others.
I am very much in favor of making virtio-mem completely safe to use even in
coco, where it is currently not used at all.
If it's really about "don't let a device trigger any unexpected kernel code
execution by sanitizing all input data", fine with me. We should do exactly
that. Try checking all MUST clauses etc.
But I don't think doing the "low hanging fruit" adds any security. It should be
done properly or not at all.
--
Cheers,
David
^ permalink raw reply
* [RFC PATCH] vhost-scsi: Serialize completion notification with callfd updates
From: Jia Jia @ 2026-07-20 8:53 UTC (permalink / raw)
To: mst, jasowangio, michael.christie
Cc: pbonzini, stefanha, eperezma, virtualization, kvm
During userspace stress testing on a KASAN-enabled host, a host-side
callfd replacement concurrent with ordinary SCSI completions exposed an
eventfd_ctx lifetime race. The test requires access to /dev/vhost-scsi and
a host process that can issue VHOST_SET_VRING_CALL; on the default device
permissions this normally means root or an explicitly delegated service.
The test used TUR completions while closing the old callfd and binding a
new one. The unbind form tends to produce a NULL pointer, while replacing
the callfd after closing the old file makes the old eventfd_ctx eligible
for release.
vhost_scsi_complete_cmd_work() drops vq->mutex before calling
vhost_signal(). VHOST_SET_VRING_CALL updates call_ctx under the same mutex
and puts the old eventfd_ctx before returning. vhost_signal() does not take
the mutex or hold a reference to call_ctx.ctx:
if (vq->call_ctx.ctx && vhost_notify(dev, vq))
eventfd_signal(vq->call_ctx.ctx);
CPU 0 (ioctl / callfd replace) CPU 1 (vhost worker)
vhost_scsi_complete_cmd_work()
mutex_unlock(&vq->mutex)
vhost_signal()
/* load old ctx */
vhost_notify() ...
VHOST_SET_VRING_CALL
mutex_lock(&vq->mutex)
swap(vq->call_ctx.ctx, new)
eventfd_ctx_put(old) /* free */
mutex_unlock(&vq->mutex)
eventfd_signal(old)
/* use-after-free */
On the reproducing run, the worker ran on CPU 1 and the callfd ioctl path
ran on CPU 0. KASAN reported:
BUG: KASAN: slab-use-after-free in eventfd_signal_mask+0x6c/0x110
The use stack was:
eventfd_signal_mask
vhost_signal
vhost_scsi_complete_cmd_work
vhost_run_work_list
vhost_task_fn
The freeing stack was:
eventfd_ctx_put
vhost_vring_ioctl
vhost_scsi_ioctl
__x64_sys_ioctl
Hold vq->mutex across vhost_signal() so SET_VRING_CALL cannot swap and
release the context while the completion worker is notifying the guest.
This gives the completion and callfd update a clear ordering without
changing the vhost API or adding a new lock. Other vhost-scsi response
paths already signal while holding the virtqueue mutex.
A private eventfd reference taken under the mutex would allow signalling
after unlock, but it needs additional reference-count plumbing. An RCU
design would require broader vhost-core changes for all call_ctx readers
and additional eventfd lifetime rules. Since callfd changes are a
control-plane operation, extending the existing per-virtqueue critical
section is the smaller complete fix.
This is an RFC because keeping vhost_notify() and eventfd_signal() inside
the virtqueue critical section extends the lock hold time slightly. I'd
appreciate feedback on whether this is acceptable or whether a
reference-pinning approach, taking the reference under the mutex and
releasing it after unlock, would be preferable.
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
drivers/vhost/scsi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 9a1253b9d8c5..7f46bc0de3c2 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -735,10 +735,9 @@ static void vhost_scsi_complete_cmd_work(struct vhost_work *work)
vhost_scsi_release_cmd_res(se_cmd);
}
- mutex_unlock(&svq->vq.mutex);
-
if (signal)
vhost_signal(&svq->vs->dev, &svq->vq);
+ mutex_unlock(&svq->vq.mutex);
}
static struct vhost_scsi_cmd *
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v4 4/5] vhost: synchronize with RCU readers when freeing workers
From: Stefano Garzarella @ 2026-07-20 8:39 UTC (permalink / raw)
To: Andrey Drobyshev
Cc: linux-kernel, kvm, virtualization, netdev, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <55d7c896-b871-4c50-a324-35f5c4a9d11a@virtuozzo.com>
On Thu, Jul 16, 2026 at 09:01:22PM +0300, Andrey Drobyshev wrote:
>On 7/16/26 7:13 PM, Stefano Garzarella wrote:
>> On Thu, Jul 16, 2026 at 06:39:48PM +0300, Andrey Drobyshev wrote:
>>> On 7/16/26 11:57 AM, Stefano Garzarella wrote:
>>>> On Tue, Jul 14, 2026 at 06:16:37PM +0300, Andrey Drobyshev wrote:
>>>>> vhost_vq_work_queue() only holds the RCU read lock while it dereferences
>>>>> vq->worker and queues work on it. vhost_workers_free() however clears
>>>>> the vq->worker pointers and immediately frees the workers, without
>>>>> waiting for a grace period. A caller that fetched the worker right
>>>>> before the pointer was cleared can therefore still be queueing work on
>>>>> it while it is freed. And even when the queueing itself wins the race,
>>>>> the work is never run, so its VHOST_WORK_QUEUED bit stays set and all
>>>>> future attempts to queue it are silently skipped.
>>>>>
>>>>> None of the current callers can actually hit this: net and scsi stop
>>>>> their virtqueues before the workers are freed, and vsock unhashes the
>>>>> device and does synchronize_rcu() of its own in vhost_vsock_dev_release()
>>>>> before the workers go away. But the upcoming VHOST_RESET_OWNER support
>>>>> in vhost-vsock keeps the device hashed while its workers are freed, so
>>>>> the lockless send/cancel paths become able to race with the teardown.
>>>>>
>>>>> Close this the way vhost_worker_killed() already does: clear the
>>>>> vq->worker pointers, wait for a grace period, run whatever the last
>>>>> readers may have queued, and only then free the workers. The
>>>>> synchronize_rcu() is skipped if the device has no workers, so cleanup of
>>>>> devices which never got an owner stays cheap.
>>>>>
>>>>
>>>> Do we need a Fixes tag for this?
>>>>
>>>
>>> I'm guessing it should be:
>>>
>>> Fixes: 228a27cf78af ("vhost: Allow worker switching while work is queueing")
>>>
>>>> Thanks for pointing out that the issue wasn't occurring, but I think we
>>>> should add it because it's a sneaky problem we discovered by chance.
>>>> IMO the code should already have `synchronize_rcu()` after
>>>> `rcu_assign_pointer()` loop.
>>>>
>>>> @Michael, what do you think?
>>>>
>>>>> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>>>>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>>>>> ---
>>>>> drivers/vhost/vhost.c | 15 +++++++++++++++
>>>>> 1 file changed, 15 insertions(+)
>>>>>
>>>>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>>>>> index 4c525b3e16ea..0d1414d40f4e 100644
>>>>> --- a/drivers/vhost/vhost.c
>>>>> +++ b/drivers/vhost/vhost.c
>>>>> @@ -729,6 +729,21 @@ static void vhost_workers_free(struct vhost_dev *dev)
>>>>>
>>>>> for (i = 0; i < dev->nvqs; i++)
>>>>> rcu_assign_pointer(dev->vqs[i]->worker, NULL);
>>>>> +
>>>>> + /*
>>>>> + * vhost_vq_work_queue() reads vq->worker under rcu_read_lock(), so a
>>>>> + * caller that fetched a worker before we cleared the pointers above
>>>>> + * may still be about to queue work on it. Wait for those RCU readers
>>>>> + * to finish before freeing the worker, then run whatever they queued
>>>>> + * so nothing is left with VHOST_WORK_QUEUED set. Mirrors
>>>>> + * vhost_worker_killed().
>>>>> + */
>>>>> + if (!xa_empty(&dev->worker_xa)) {
>>>>> + synchronize_rcu();
>>>>> + xa_for_each(&dev->worker_xa, i, worker)
>>>>> + vhost_run_work_list(worker);
>>>>> + }
>>>>> +
>>>>
>>>> Following sashiko review [1], I tried to undersand why we need this, but
>>>> TBH I'm really confused. That said, this seems wrong also because it
>>>> will work only with vhost_tasks, and not with kthreads.
>>>>
>>>> IIUC vhost_worker_killed() will be called anyway when calling
>>>> vhost_worker_destroy(). For vhost_tasks, it will call
>>>> vhost_task_do_stop() that calls vhost_task_stop(). This sets
>>>> VHOST_TASK_FLAGS_STOP and wait the worker on vtsk->exited before freeing
>>>> stuff. The worker breaks the loop and calls vtsk->handle_sigkill() that
>>>> is exactly vhost_worker_killed() you mentioned we are mirroring here.
>>>>
>>>
>>> Hmm, are we sure it's the case for our codepath? Looking at the
>>> vhost_task loop function:
>>>
>>>> static int vhost_task_fn(void *data)
>>>> {
>>>> for (;;) {
>>>> if (signal_pending(current)) {
>>>> if (get_signal(&ksig))
>>>> break;
>>>> }
>>>> ...
>>>> if (test_bit(VHOST_TASK_FLAGS_STOP, &vtsk->flags)) {
>>>> __set_current_state(TASK_RUNNING);
>>>> break;
>>>> }
>>>> did_work = vtsk->fn(vtsk->data);
>>>> ...
>>>> }
>>>>
>>>> ...
>>>>
>>>> if (!test_bit(VHOST_TASK_FLAGS_STOP, &vtsk->flags)) {
>>>> set_bit(VHOST_TASK_FLAGS_KILLED, &vtsk->flags);
>>>> vtsk->handle_sigkill(vtsk->data);
>>>> }
>>>> ...
>>>> }
>>>
>>> AFAICT, we exit the loop in 2 cases: signal delivery or STOP bit
>>> setting. Like you said, STOP is set by vhost_task_stop. E.g. for our
>>> RESET_OWNER case:
>>>
>>> vhost_vsock_reset_owner()
>>> vhost_dev_reset_owner()
>>> vhost_dev_cleanup()
>>> vhost_workers_free()
>>> vhost_worker_destroy()
>>> vhost_task_stop() // for vhost_task_ops backend
>>> set_bit(VHOST_TASK_FLAGS_STOP)
>>>
>>> So, first of all, actual work by .fn() callback is done after the exit
>>> checks, therefore we skip it - no chance to drain there.
>>>
>>> Secondly, the handle_sigkill() callback is deliberately NOT called in
>>> the STOP case and only called on fatal signal delivery. And for
>>> vhost_task backend the .handle_sigkill() callback is exactly
>>> vhost_worker_killed().
>>>
>>> So my understanding is: if we only call synchronize_rcu() here and leave
>>> this path undrained, then whatever work which was put by send_pkt() for
>>> the worker currently being freed - will be lost. Please correct me if
>>> I'm wrong.
>>
>> Yep, your right. But what will be the issue of loosing them?
>>
>> IIUC we are not loosing any data, just avoiding some works that will be
>> handled later when/if will set a new owner.
>>
>
>But will it actually be handled?
>
>vhost_transport_send_pkt() // called on every packet send
> virtio_vsock_skb_queue_tail(&send_pkt_queue, skb) // add skb to list
> vhost_vq_work_queue(&send_pkt_work) // try to arm the work
> vhost_worker_queue()
> if (!test_and_set_bit(VHOST_WORK_QUEUED, &work->flags)) {
> llist_add(&worker->work_list)
> }
>
>So send_pkt_queue is a list of skbs, it lives on the vhost_vsock device
>state, and survives RESET_OWNER. In that sense you're probably right
>that we aren't loosing any data.
>
>There's also send_pkt_work object, also living on the vhost_vsock device
>state. So we're accumulating skbs, and then send_pkg_work gets put in
>the worker task list - but only if it's NOT already armed in there, i.e.
>QUEUED bit is unset. And the bit gets cleared by the workload callback
>- for vhost_task backend it's vhost_run_work_list().
>
>The most important thing is WHERE this piece of work is being put. That
>is worker->work_list - this list does not survive RESET_OWNER, as we
>free the worker in vhost_workers_free().
>
>Now imagine we have RESET_OWNER racing with send_pkt. In
>vhost_workers_free() we acquire ptr to a worker but not NULL'ify it yet.
> Then on the send_pkt path we arm the send_pkt_work, set the QUEUED bit,
>and place it on the work_list of a DYING worker. Then the worker gets
>freed. Now we have send_pkt_work (a singleton struct) with QUEUED set
>in its flags, and with no worker to walk through this piece of work and
>clear this flag. As a result - send_pkt_work can't be placed in the
>list of any other worker, because it doesn't pass the "if
>(!test_and_set_bit(QUEUED)" check. Thus no new packets can be
>processed, and the connection is stalled.
>
>Does this make sense?
Honestly, I don't see the problem. The device is about to be stopped
because the VMM is resetting the owner, so the connection is going to
stall anyway, since I don't think the guest will never answer, isn't it?
>>>
>>> That said, I agree that vhost_run_work_list() will only work with
>>> vhost_task backend, not with kthreads backend. If we do
>>> vhost_worker_flush() instead - I guess it'll keep the drain here, yet
>>> become backend-agnostic. I.e.:
>>>
>>>> + if (!xa_empty(&dev->worker_xa)) {
>>>> + synchronize_rcu();
>>>> + xa_for_each(&dev->worker_xa, i, worker)
>>>> + vhost_worker_flush(worker);
>>>> + }
>>>
>>> With the last 2 lines being equivalent to just calling
>>> vhost_dev_flush(dev). And once we become backend-agnostic here, I'm
>>> guessing the warning reported by Sashiko should be dealt with as well.
>>
>> I'd avoid `if !xa_empty(&dev->worker_xa)` at all, and call
>> synchronize_rcu() in any case.
>>
>
>Agreed.
>
>> About vhost_dev_flush(), we are calling it in several places, and maybe
>> we should re-check them. E.g. we call in vhost_vsock_flush(), but it's
>> also called by vhost_dev_stop(), maybe we can avoid to call
>> vhost_vsock_flush() if we call vhost_dev_stop().
>>
>> I'm not sure we really need another one here, but if you think some
>> other works can be queued between the vhost_dev_stop() and the
>> synchronize_rcu() we are adding here, then okay, it may have sense.
>>
>
>Note that in our particular case we're gonna do:
>
>vhost_workers_free()
> vhost_dev_flush() // the flush we're planning to add
> xa_for_each(&dev->worker_xa, i, worker)
> vhost_worker_destroy(dev, worker)
> xa_destroy(&dev->worker_xa)
>
>So we walk through the XArray, destroy workers in it one by one, then
>destroy the XArray itself. Then the next time we call
>vhost_dev_flush(), e.g. from vhost_dev_stop() or wherever else, it tries
>iterating over the XArray which no longer exists - which is gonna be a
>no-op.
>
>Now, we can reach vhost_workers_free() via (at least) 2 paths:
>RESET_OWNER and device release path. On the former the flush is needed
>as I illustrated above. On the latter it's indeed redundant but is
>cheap as it's a no-op.
Again, I don't see the need for it TBH, but at the same time, I don't
think it's a problem to include it, so if you think it's necessary, go
ahead and add it.
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: Greg Kroah-Hartman @ 2026-07-20 8:28 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Carlos Bilbao, Michael S. Tsirkin, Hari Mishal, Jason Wang,
Xuan Zhuo, Eugenio Pérez, virtualization, linux-kernel,
elena.reshetova, huster, mhollick, jiska.classen
In-Reply-To: <9569e577-aa82-4642-b9d9-fd496fc12849@kernel.org>
On Mon, Jul 20, 2026 at 08:57:13AM +0100, David Hildenbrand (Arm) wrote:
> On 7/18/26 18:07, Carlos Bilbao wrote:
> > On 7/17/26 22:29, Greg Kroah-Hartman wrote:
> >
> >> On Fri, Jul 17, 2026 at 08:31:09PM -0700, Carlos Bilbao wrote:
> >>> Historically, one of the biggest criticisms of coco, especially around
> >>> device hardening, was that there were too many values that a
> >>> malicious/buggy device could misreport, making it a losing battle. That is
> >>> no longer the case with LLMs, and we have the advantage (and challenge) of
> >>> open-source dev, which allows us to receive many of these fixes "for free".
> >>> If others want to burn their tokens, let them :)
> >> I have lots of tokens to burn :)
> >>
> >> So along those lines, any suggestions on how best to fuzz these code
> >> paths? Any workloads you all use for testing that I can take advantage
> >> of?
> >
> >
> > We've the virtio-mem config struct layout and the kernel source, so for
> > obvious fixes like a NULL check, static analysis is better than fuzzing.
> > Claude took a few mins to find me two examples:
> >
> > Patch 1: virtio-mem: reject non-power-of-two device_block_size
> > This one is for virtio_mem_init() to check if
> > !is_power_of_2(vm->device_block_size)
> >
> > Patch 2: virto-mem: validate region_size and usable_region_size
> > THis one checks region_size != 0 and vm->usable_reion_size >
> > vm->region_size.
> >
> > An endless factory of "silly" checks like these are low hanging fruit.
> >
> > Now, for harder bugs, looking around for fuzz options, VirtFuzz [1] looks
> > like a great candidate for those interested in pursuing this direction.
> >
> >
> > Their PoC fuzzes wireless/Bluetooth stack, but nothing our AI overlords
> > can't quickly adapt for virtio-mem and other virtio drivers; the JSON
> > definition to describe device behavior is easily extensible. Their threat
> > model [2] describes an external attacker, but in the context of coco, the
> > virtio device itself is the attacker. Here's a vibe coded PR of what I mean:
> >
> > https://github.com/seemoo-lab/VirtFuzz/pull/7
> >
> > CCed the creators/authors, thanks for open sourcing this!
> >
> > Thanks,
> > Carlos
> >
> > [1] https://github.com/seemoo-lab/VirtFuzz
> >
> > On 7/17/26 22:29, Greg Kroah-Hartman wrote:
> >
> >> On Fri, Jul 17, 2026 at 08:31:09PM -0700, Carlos Bilbao wrote:
> >>> Historically, one of the biggest criticisms of coco, especially around
> >>> device hardening, was that there were too many values that a
> >>> malicious/buggy device could misreport, making it a losing battle. That is
> >>> no longer the case with LLMs, and we have the advantage (and challenge) of
> >>> open-source dev, which allows us to receive many of these fixes "for free".
> >>> If others want to burn their tokens, let them :)
> >> I have lots of tokens to burn :)
> >>
> >> So along those lines, any suggestions on how best to fuzz these code
> >> paths? Any workloads you all use for testing that I can take advantage
> >> of?
> >
> >
> > We've the virto-mem config struct layout and the kernel source, so for
> > obvious fixes like a NULL check, static analysis is better than fuzzing.
> > Claude took a few mins to find me two examples:
> >
> > Patch 1: virtio-mem: reject non-power-of-two device_block_size
> > This one is for virtio_mem_init() to check if
> > !is_power_of_2(vm->device_block_size)
> >
> > Patch 2: virto-mem: validate region_size and usable_region_size
> > THis one checks region_size != 0 and vm->usable_reion_size >
> > vm->region_size.
> >
> > An endless factory of "silly" checks like these are low hanging fruit.
>
> "silly" is the right word.
"silly" in what way?
Seriously, I'm trying to figure out what you all care about here and
what exactly the threat model you want this driver to work in, and I'm
getting conflicting answers.
Either you all do worry about the "device" sending bad data and want to
protect from that, or you don't and you trust it. Pick one please so
that we know how to deal with these bug reports we are getting.
For example, for USB we have said our threat model is:
- we do NOT trust the device before a driver is bound to the device,
so if a malicious device can do something to the kernel, the kernel
needs to be fixed.
- During the probe() call for a USB driver, the driver does NOT trust
the device, and again, anything a malicious device can do to the
kernel, the kernel should fix.
- After probe() for a USB driver succeeds, it's up to the driver if it
wants to validate all data coming from the device or not. Right
now, in general, the kernel trusts the device at that point in time
so additional checks are discretionary and at the whim of the
maintainer.
For that last point, I will note that some BIG users of Linux (i.e.
billions of Android devices) still explicitly do NOT want to trust the
USB device at this point in time, and are relying on the kernel to
protect the system from bad devices. In that case, various patches have
been taken to different drivers and subsystems to play whack-a-mole on
while Android gets their act together to finally come up with a solid
defensive plan (like ChromeOS has had for a decade.) It will be seen
which happens first, all drivers are properly fuzzed and fixed up, or
Android gets their act together and finally fixes their b0rked system
trust model. I think Android management is relying on the kernel
community to do the kernel work as they keep refusing to staff the
userspace work that they need to do here...
And yes, I really need to write this up in a more solid document for USB
and get it into the tree, but at least this email thread has forced me
to write down the above :)
So, again, for virtio drivers, what exactly do you all want to say is
your threat model that the drivers need to handle? Can you all agree on
something please? Otherwise, for new developers like Hari, this is
totaly confusion as to what they should be doing.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] vsock: use sock_error() to consume sk_err after connect timeout
From: Stefano Garzarella @ 2026-07-20 8:17 UTC (permalink / raw)
To: Nguyen Dinh Phi
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, syzbot+1b2c9c4a0f8708082678, virtualization, netdev,
linux-kernel
In-Reply-To: <20260719220103.684489-1-phind.uet@gmail.com>
On Mon, Jul 20, 2026 at 05:57:47AM +0800, Nguyen Dinh Phi wrote:
>After vsock_connect() exits the wait loop due to sk->sk_err being
>set, the error was read but not cleared. This left sk->sk_err set
>for subsequent operations.
So, is this a fix? If yes, we should put a Fixes tag.
Also, can you describe how to trigger the issue?
Because I see this in vsock_connect(), so I thought it was in some way
already handled:
/* sk_err might have been set as a result of an earlier
* (failed) connect attempt.
*/
sk->sk_err = 0;
>Switch to sock_error() which atomically reads and clears sk->sk_err,
>so the error is consumed when returned.
>
>Signed-off-by: Nguyen Dinh Phi <phind.uet@gmail.com>
>Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
Can you explain how this patch fixes that issue?
(this should be the first information to be put in the commit message
IMHO)
I'd like to understand better if this is a fix of real bug or just an
improvement to the code (which is fine by me).
Thanks,
Stefano
>---
> net/vmw_vsock/af_vsock.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 622dbd046799..43eddc33ed12 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -1847,14 +1847,11 @@ static int vsock_connect(struct socket *sock, struct sockaddr_unsized *addr,
> prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
> }
>
>- if (sk->sk_err) {
>- err = -sk->sk_err;
>+ err = sock_error(sk);
>+ if (err) {
> sk->sk_state = TCP_CLOSE;
> sock->state = SS_UNCONNECTED;
>- } else {
>- err = 0;
> }
>-
> out_wait:
> finish_wait(sk_sleep(sk), &wait);
> out:
>--
>2.53.0
>
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: David Hildenbrand (Arm) @ 2026-07-20 7:57 UTC (permalink / raw)
To: Carlos Bilbao, Greg Kroah-Hartman
Cc: Michael S. Tsirkin, Hari Mishal, Jason Wang, Xuan Zhuo,
Eugenio Pérez, virtualization, linux-kernel, elena.reshetova,
huster, mhollick, jiska.classen
In-Reply-To: <ae5bc86b-cb8d-4530-9e58-d285007deac0@gmail.com>
On 7/18/26 18:07, Carlos Bilbao wrote:
> On 7/17/26 22:29, Greg Kroah-Hartman wrote:
>
>> On Fri, Jul 17, 2026 at 08:31:09PM -0700, Carlos Bilbao wrote:
>>> Historically, one of the biggest criticisms of coco, especially around
>>> device hardening, was that there were too many values that a
>>> malicious/buggy device could misreport, making it a losing battle. That is
>>> no longer the case with LLMs, and we have the advantage (and challenge) of
>>> open-source dev, which allows us to receive many of these fixes "for free".
>>> If others want to burn their tokens, let them :)
>> I have lots of tokens to burn :)
>>
>> So along those lines, any suggestions on how best to fuzz these code
>> paths? Any workloads you all use for testing that I can take advantage
>> of?
>
>
> We've the virtio-mem config struct layout and the kernel source, so for
> obvious fixes like a NULL check, static analysis is better than fuzzing.
> Claude took a few mins to find me two examples:
>
> Patch 1: virtio-mem: reject non-power-of-two device_block_size
> This one is for virtio_mem_init() to check if
> !is_power_of_2(vm->device_block_size)
>
> Patch 2: virto-mem: validate region_size and usable_region_size
> THis one checks region_size != 0 and vm->usable_reion_size >
> vm->region_size.
>
> An endless factory of "silly" checks like these are low hanging fruit.
>
> Now, for harder bugs, looking around for fuzz options, VirtFuzz [1] looks
> like a great candidate for those interested in pursuing this direction.
>
>
> Their PoC fuzzes wireless/Bluetooth stack, but nothing our AI overlords
> can't quickly adapt for virtio-mem and other virtio drivers; the JSON
> definition to describe device behavior is easily extensible. Their threat
> model [2] describes an external attacker, but in the context of coco, the
> virtio device itself is the attacker. Here's a vibe coded PR of what I mean:
>
> https://github.com/seemoo-lab/VirtFuzz/pull/7
>
> CCed the creators/authors, thanks for open sourcing this!
>
> Thanks,
> Carlos
>
> [1] https://github.com/seemoo-lab/VirtFuzz
>
> On 7/17/26 22:29, Greg Kroah-Hartman wrote:
>
>> On Fri, Jul 17, 2026 at 08:31:09PM -0700, Carlos Bilbao wrote:
>>> Historically, one of the biggest criticisms of coco, especially around
>>> device hardening, was that there were too many values that a
>>> malicious/buggy device could misreport, making it a losing battle. That is
>>> no longer the case with LLMs, and we have the advantage (and challenge) of
>>> open-source dev, which allows us to receive many of these fixes "for free".
>>> If others want to burn their tokens, let them :)
>> I have lots of tokens to burn :)
>>
>> So along those lines, any suggestions on how best to fuzz these code
>> paths? Any workloads you all use for testing that I can take advantage
>> of?
>
>
> We've the virto-mem config struct layout and the kernel source, so for
> obvious fixes like a NULL check, static analysis is better than fuzzing.
> Claude took a few mins to find me two examples:
>
> Patch 1: virtio-mem: reject non-power-of-two device_block_size
> This one is for virtio_mem_init() to check if
> !is_power_of_2(vm->device_block_size)
>
> Patch 2: virto-mem: validate region_size and usable_region_size
> THis one checks region_size != 0 and vm->usable_reion_size >
> vm->region_size.
>
> An endless factory of "silly" checks like these are low hanging fruit.
"silly" is the right word.
--
Cheers,
David
^ permalink raw reply
* [PATCH] tools/virtio: Fix control typo in trace agent comment
From: GuoHan Zhao @ 2026-07-20 1:45 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang, virtualization
Cc: Xuan Zhuo, Eugenio Pérez, linux-kernel
Fix a misspelling of "control" in the trace agent controller description.
Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
---
tools/virtio/virtio-trace/trace-agent-ctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virtio/virtio-trace/trace-agent-ctl.c b/tools/virtio/virtio-trace/trace-agent-ctl.c
index 39860be6e2d8..9577579e86da 100644
--- a/tools/virtio/virtio-trace/trace-agent-ctl.c
+++ b/tools/virtio/virtio-trace/trace-agent-ctl.c
@@ -84,7 +84,7 @@ static int wait_order(int ctl_fd)
}
/*
- * contol read/write threads by handling global_run_operation
+ * control read/write threads by handling global_run_operation
*/
void *rw_ctl_loop(int ctl_fd)
{
--
2.43.0
^ permalink raw reply related
* [PATCH] tools/virtio: Fix userspace typo in vringh test comment
From: GuoHan Zhao @ 2026-07-20 1:44 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang, virtualization
Cc: Xuan Zhuo, Eugenio Pérez, linux-kernel
Fix a misspelling of "userspace" in the vringh test description.
Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
---
tools/virtio/vringh_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c
index 5ea6d29bc992..84961b9ab5ff 100644
--- a/tools/virtio/vringh_test.c
+++ b/tools/virtio/vringh_test.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0
-/* Simple test of virtio code, entirely in userpsace. */
+/* Simple test of virtio code, entirely in userspace. */
#define _GNU_SOURCE
#include <sched.h>
#include <err.h>
--
2.43.0
^ permalink raw reply related
* [PATCH] vsock: use sock_error() to consume sk_err after connect timeout
From: Nguyen Dinh Phi @ 2026-07-19 21:57 UTC (permalink / raw)
To: Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman
Cc: Nguyen Dinh Phi, syzbot+1b2c9c4a0f8708082678, virtualization,
netdev, linux-kernel
After vsock_connect() exits the wait loop due to sk->sk_err being
set, the error was read but not cleared. This left sk->sk_err set
for subsequent operations.
Switch to sock_error() which atomically reads and clears sk->sk_err,
so the error is consumed when returned.
Signed-off-by: Nguyen Dinh Phi <phind.uet@gmail.com>
Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
---
net/vmw_vsock/af_vsock.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 622dbd046799..43eddc33ed12 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -1847,14 +1847,11 @@ static int vsock_connect(struct socket *sock, struct sockaddr_unsized *addr,
prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
}
- if (sk->sk_err) {
- err = -sk->sk_err;
+ err = sock_error(sk);
+ if (err) {
sk->sk_state = TCP_CLOSE;
sock->state = SS_UNCONNECTED;
- } else {
- err = 0;
}
-
out_wait:
finish_wait(sk_sleep(sk), &wait);
out:
--
2.53.0
^ permalink raw reply related
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