[PATCH 0/8] virtio: Fix virtqueue teardown and probe cleanup

[PATCH 0/8] virtio: Fix virtqueue teardown and probe cleanup

[Bin Meng]

This series fixes virtqueue teardown across MMIO and PCI transports, as
well as a queue leak on failed virtio block probes.


Bin Meng (8):
  virtio: mmio: Fix private data lookup when deleting queues
  virtio: pci: Fix legacy private data lookup when deleting queues
  virtio: pci: Fix modern private data lookup when deleting queues
  virtio: Release virtqueues after device reset
  virtio: 9p: Reset the device before releasing its queues
  virtio: blk: Validate configuration before creating the virtqueue
  MAINTAINERS: Cover all virtio driver-model tests
  test: dm: Check virtio queue cleanup on device removal

 MAINTAINERS                        |  2 +-
 drivers/virtio/virtio-uclass.c     | 18 +++++++++++++++++-
 drivers/virtio/virtio_9p.c         |  8 +-------
 drivers/virtio/virtio_blk.c        |  7 +------
 drivers/virtio/virtio_mmio.c       |  2 +-
 drivers/virtio/virtio_pci_legacy.c |  2 +-
 drivers/virtio/virtio_pci_modern.c | 14 ++------------
 include/virtio.h                   |  7 ++++++-
 test/dm/virtio_device.c            | 19 ++++++++++++++-----
 9 files changed, 44 insertions(+), 35 deletions(-)

---
base-commit: 32ebe922e574a733d01c3f106fa77d9176bfecad
branch: virtio

-- 
2.53.0

[PATCH 1/8] virtio: mmio: Fix private data lookup when deleting queues

[Bin Meng]

The virtqueue's vdev points to the functional device, while the MMIO
register base and version are stored in its parent transport device.
Using the child's private data can make queue deletion write to queue
memory instead of MMIO registers.

Get the parent device's private data in virtio_mmio_del_vq().

Fixes: fdc4aca89ecb ("virtio: Add virtio over mmio transport driver")
Signed-off-by: Bin Meng <bin.meng@processmission.com>
---

 drivers/virtio/virtio_mmio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 975f98cd9e5..4b478639fb7 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -279,7 +279,7 @@ error_available:
 
 static void virtio_mmio_del_vq(struct virtqueue *vq)
 {
-	struct virtio_mmio_priv *priv = dev_get_priv(vq->vdev);
+	struct virtio_mmio_priv *priv = dev_get_priv(vq->vdev->parent);
 	unsigned int index = vq->index;
 
 	/* Select and deactivate the queue */
-- 
2.53.0

[PATCH 2/8] virtio: pci: Fix legacy private data lookup when deleting queues

[Bin Meng]

The virtqueue's vdev points to the functional device, while the PCI I/O
register mapping belongs to its parent transport. The current lookup
therefore treats the functional driver's private data as transport data
and uses a queue pointer as the register base.

Queue deletion can overwrite the queue's list links and crash when
unlinking it. Fetch the parent transport's private data instead.

Fixes: 4135e10732a0 ("virtio: Add virtio over pci transport driver")
Signed-off-by: Bin Meng <bin.meng@processmission.com>
---

 drivers/virtio/virtio_pci_legacy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
index 15f8c6e7d25..369c15bd88c 100644
--- a/drivers/virtio/virtio_pci_legacy.c
+++ b/drivers/virtio/virtio_pci_legacy.c
@@ -226,7 +226,7 @@ error_available:
 
 static void virtio_pci_del_vq(struct virtqueue *vq)
 {
-	struct virtio_pci_priv *priv = dev_get_priv(vq->vdev);
+	struct virtio_pci_priv *priv = dev_get_priv(vq->vdev->parent);
 	unsigned int index = vq->index;
 
 	iowrite16(index, priv->ioaddr + VIRTIO_PCI_QUEUE_SEL);
-- 
2.53.0

[PATCH 3/8] virtio: pci: Fix modern private data lookup when deleting queues

[Bin Meng]

The virtqueue's vdev points to the functional device, while the PCI I/O
register mapping belongs to its parent transport. The current lookup
therefore treats the functional driver's private data as transport data
and uses a queue pointer as the register base.

Queue deletion can overwrite the queue's list links and crash when
unlinking it. Fetch the parent transport's private data instead.

Fixes: 550435edf810 ("virtio: pci: Support non-legacy PCI transport device")
Signed-off-by: Bin Meng <bin.meng@processmission.com>
---

 drivers/virtio/virtio_pci_modern.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 5850e0c18c6..d309f9a9dc1 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -305,7 +305,7 @@ error_available:
 
 static void virtio_pci_del_vq(struct virtqueue *vq)
 {
-	struct virtio_pci_priv *priv = dev_get_priv(vq->vdev);
+	struct virtio_pci_priv *priv = dev_get_priv(vq->vdev->parent);
 	unsigned int index = vq->index;
 
 	iowrite16(index, &priv->common->queue_select);
-- 
2.53.0

[PATCH 4/8] virtio: Release virtqueues after device reset

[Bin Meng]

NET, BLK and RNG remove their devices through virtio_reset(). Only the
modern PCI transport currently frees its virtqueues during reset, so
MMIO and legacy PCI retain queue allocations across remove and reprobe.

Wait for reset completion and release queues in the common helper.
Keep transport reset callbacks limited to resetting hardware only.

Signed-off-by: Bin Meng <bin.meng@processmission.com>
---

 drivers/virtio/virtio-uclass.c     | 18 +++++++++++++++++-
 drivers/virtio/virtio_pci_modern.c | 12 +-----------
 include/virtio.h                   |  7 ++++++-
 3 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/drivers/virtio/virtio-uclass.c b/drivers/virtio/virtio-uclass.c
index c163b2fd7d5..978ef0d1ce7 100644
--- a/drivers/virtio/virtio-uclass.c
+++ b/drivers/virtio/virtio-uclass.c
@@ -25,6 +25,7 @@
 #include <virtio.h>
 #include <dm/lists.h>
 #include <linux/bug.h>
+#include <linux/delay.h>
 
 static const char *const virtio_drv_name[VIRTIO_ID_MAX_NUM] = {
 	[VIRTIO_ID_NET]		= VIRTIO_NET_DRV_NAME,
@@ -85,10 +86,25 @@ int virtio_set_status(struct udevice *vdev, u8 status)
 int virtio_reset(struct udevice *vdev)
 {
 	struct dm_virtio_ops *ops;
+	u8 status;
+	int ret;
 
 	ops = virtio_get_ops(vdev->parent);
 
-	return ops->reset(vdev->parent);
+	ret = ops->reset(vdev->parent);
+	if (ret)
+		return ret;
+
+	/* Wait for the device to stop accessing the queues before freeing them */
+	do {
+		ret = ops->get_status(vdev->parent, &status);
+		if (ret)
+			return ret;
+		if (status)
+			udelay(1000);
+	} while (status);
+
+	return ops->del_vqs(vdev->parent);
 }
 
 int virtio_get_features(struct udevice *vdev, u64 *features)
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index d309f9a9dc1..abc0163fa82 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -14,7 +14,6 @@
 #include <dm/device.h>
 #include <linux/bug.h>
 #include <linux/compat.h>
-#include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/io.h>
 #include "virtio_pci.h"
@@ -350,16 +349,7 @@ static int virtio_pci_reset(struct udevice *udev)
 	/* 0 status means a reset */
 	iowrite8(0, &priv->common->device_status);
 
-	/*
-	 * After writing 0 to device_status, the driver MUST wait for a read
-	 * of device_status to return 0 before reinitializing the device.
-	 * This will flush out the status write, and flush in device writes,
-	 * including MSI-X interrupts, if any.
-	 */
-	while (ioread8(&priv->common->device_status))
-		udelay(1000);
-
-	return virtio_pci_del_vqs(udev);
+	return 0;
 }
 
 static int virtio_pci_notify(struct udevice *udev, struct virtqueue *vq)
diff --git a/include/virtio.h b/include/virtio.h
index b981eead5d7..30e5005e9ad 100644
--- a/include/virtio.h
+++ b/include/virtio.h
@@ -160,6 +160,9 @@ struct dm_virtio_ops {
 	/**
 	 * reset() - reset the device
 	 *
+	 * The uclass waits for the status to become zero and frees the
+	 * virtqueues after this operation succeeds.
+	 *
 	 * @vdev:	the real virtio device
 	 * @return 0 if OK, -ve on error
 	 */
@@ -288,7 +291,9 @@ int virtio_get_status(struct udevice *vdev, u8 *status);
 int virtio_set_status(struct udevice *vdev, u8 status);
 
 /**
- * virtio_reset() - reset the device
+ * virtio_reset() - reset the device and free its virtqueues
+ *
+ * Wait for the device status to become zero before freeing the virtqueues.
  *
  * @vdev:	the real virtio device
  * Return: 0 if OK, -ve on error
-- 
2.53.0

[PATCH 5/8] virtio: 9p: Reset the device before releasing its queues

[Bin Meng]

The 9P remove callback frees virtqueues while the device can still
access them, then resets the device. Outstanding requests can therefore
race with freeing the ring memory.

Use the common reset helper, which waits for the device to stop before
releasing its queues.

Signed-off-by: Bin Meng <bin.meng@processmission.com>
---

 drivers/virtio/virtio_9p.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/virtio/virtio_9p.c b/drivers/virtio/virtio_9p.c
index fb2eb0c7223..d0fb39a1de5 100644
--- a/drivers/virtio/virtio_9p.c
+++ b/drivers/virtio/virtio_9p.c
@@ -102,19 +102,13 @@ static int virtio_9p_probe(struct udevice *dev)
 	return 0;
 }
 
-static int virtio_9p_remove(struct udevice *dev)
-{
-	virtio_del_vqs(dev);
-	return virtio_reset(dev);
-}
-
 U_BOOT_DRIVER(virtio_9p) = {
 	.name       = VIRTIO_9P_DRV_NAME,
 	.id         = UCLASS_9P,
 	.ops        = &virtio_9p_ops,
 	.bind       = virtio_9p_bind,
 	.probe      = virtio_9p_probe,
-	.remove     = virtio_9p_remove,
+	.remove     = virtio_reset,
 	.priv_auto  = sizeof(struct virtio_9p_priv),
 	.flags      = DM_FLAG_ACTIVE_DMA,
 };
-- 
2.53.0

[PATCH 6/8] virtio: blk: Validate configuration before creating the virtqueue

[Bin Meng]

The block driver allocates its virtqueue before checking the advertised
block size. An invalid size makes probe return -EIO without releasing
the queue, causing a memory leak.

Change to read and validate the configuration before allocating the
virtqueue instead.

Signed-off-by: Bin Meng <bin.meng@processmission.com>
---

 drivers/virtio/virtio_blk.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/virtio/virtio_blk.c b/drivers/virtio/virtio_blk.c
index 404d9140cb2..3d5d29f00df 100644
--- a/drivers/virtio/virtio_blk.c
+++ b/drivers/virtio/virtio_blk.c
@@ -251,13 +251,8 @@ static int virtio_blk_probe(struct udevice *dev)
 	struct virtio_blk_priv *priv = dev_get_priv(dev);
 	struct blk_desc *desc = dev_get_uclass_plat(dev);
 	u64 cap;
-	int ret;
 	u32 blk_size;
 
-	ret = virtio_find_vqs(dev, 1, &priv->vq);
-	if (ret)
-		return ret;
-
 	virtio_cread(dev, struct virtio_blk_config, capacity, &cap);
 	desc->lba = cap;
 	if (virtio_has_feature(dev, VIRTIO_BLK_F_BLK_SIZE)) {
@@ -281,7 +276,7 @@ static int virtio_blk_probe(struct udevice *dev)
 	else
 		priv->seg_max = -1U;
 
-	return 0;
+	return virtio_find_vqs(dev, 1, &priv->vq);
 }
 
 static const struct blk_ops virtio_blk_ops = {
-- 
2.53.0

[PATCH 7/8] MAINTAINERS: Cover all virtio driver-model tests

[Bin Meng]

The virtio entry only matches test/dm/virtio.c, leaving the existing
device and RNG tests outside its coverage. Use a wildcard instead.

Signed-off-by: Bin Meng <bin.meng@processmission.com>
---

 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 28d090d3c36..05963a84c0d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1869,7 +1869,7 @@ S:	Maintained
 F:	drivers/virtio/
 F:	cmd/virtio.c
 F:	include/virtio*.h
-F:	test/dm/virtio.c
+F:	test/dm/virtio*.c
 F:	doc/develop/driver-model/virtio.rst
 
 WATCHDOG
-- 
2.53.0

Re: [PATCH 7/8] MAINTAINERS: Cover all virtio driver-model tests

[Quentin Schulz]

Hi Bin Meng,

On 10/15/26 1:13 PM, Bin Meng wrote:
> The virtio entry only matches test/dm/virtio.c, leaving the existing
> device and RNG tests outside its coverage. Use a wildcard instead.
> 
> Signed-off-by: Bin Meng <bin.meng@processmission.com>
> ---
> 
>   MAINTAINERS | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 28d090d3c36..05963a84c0d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1869,7 +1869,7 @@ S:	Maintained
>   F:	drivers/virtio/
>   F:	cmd/virtio.c
>   F:	include/virtio*.h
> -F:	test/dm/virtio.c
> +F:	test/dm/virtio*.c
>   F:	doc/develop/driver-model/virtio.rst
>   

How about switching the whole MAINTAINERS entry to

N: virtio

In any case:

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin

[PATCH 8/8] test: dm: Check virtio queue cleanup on device removal

[Bin Meng]

Checking device status alone does not detect queues left allocated when
a virtio device is removed.

Probe the virtio child before removal so that it owns a real queue.
Verify that removal for DMA shutdown resets the child and releases its
queue. Reprobe it and check that the old queue is not retained, then
verify cleanup on normal removal.

Signed-off-by: Bin Meng <bin.meng@processmission.com>

---

 test/dm/virtio_device.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/test/dm/virtio_device.c b/test/dm/virtio_device.c
index c2cd0630d97..64e09049ded 100644
--- a/test/dm/virtio_device.c
+++ b/test/dm/virtio_device.c
@@ -100,6 +100,7 @@ DM_TEST(dm_test_virtio_all_ops, UTF_SCAN_PDATA | UTF_SCAN_FDT);
 static int dm_test_virtio_remove(struct unit_test_state *uts)
 {
 	struct udevice *bus, *dev;
+	struct virtio_dev_priv *uc_priv;
 	u8 status;
 
 	/* check probe success */
@@ -110,16 +111,24 @@ static int dm_test_virtio_remove(struct unit_test_state *uts)
 	ut_assertok(device_find_first_child(bus, &dev));
 	ut_assertnonnull(dev);
 
-	/* set driver status to VIRTIO_CONFIG_S_DRIVER_OK */
-	ut_assertok(virtio_set_status(dev, VIRTIO_CONFIG_S_DRIVER_OK));
+	/* Probe the driver so removal must release a real queue */
+	ut_assertok(device_probe(dev));
+	uc_priv = dev_get_uclass_priv(bus);
+	ut_asserteq(1, list_count_nodes(&uc_priv->vqs));
 
 	/* check the device can be successfully removed */
-	dev_or_flags(dev, DM_FLAG_ACTIVATED);
 	ut_asserteq(-EKEYREJECTED, device_remove(bus, DM_REMOVE_ACTIVE_ALL));
 
 	ut_asserteq(false, device_active(dev));
-	virtio_get_status(dev, &status);
-	ut_assertok(status);
+	ut_assertok(virtio_get_status(dev, &status));
+	ut_asserteq(0, status);
+	ut_assert(list_empty(&uc_priv->vqs));
+
+	/* Reprobing must not accumulate queues from the previous instance */
+	ut_assertok(device_probe(dev));
+	ut_asserteq(1, list_count_nodes(&uc_priv->vqs));
+	ut_assertok(device_remove(dev, DM_REMOVE_NORMAL));
+	ut_assert(list_empty(&uc_priv->vqs));
 
 	return 0;
 }
-- 
2.53.0