[PATCH v2 00/12] virtio pci support VIRTIO_F_RING_RESET

[PATCH v2 00/12] virtio pci support VIRTIO_F_RING_RESET

[Xuan Zhuo]

The virtio spec already supports the virtio queue reset function. This patch set
is to add this function to the kernel. The relevant virtio spec information is
here:

    https://github.com/oasis-tcs/virtio-spec/issues/124

Also regarding MMIO support for queue reset, I plan to support it after this
patch is passed.

#9-#12 is the disable/enable function of rx/tx pair implemented by virtio-net
using the new helper. This function is not currently referenced by other
functions. It is more to show the usage of the new helper, I not sure if they
are going to be merged together.

Please review. Thanks.

Xuan Zhuo (12):
  virtio: pci: struct virtio_pci_common_cfg add queue_notify_data
  virtio: queue_reset: add VIRTIO_F_RING_RESET
  virtio: queue_reset: struct virtio_config_ops add callbacks for
    queue_reset
  virtio: queue_reset: pci: update struct virtio_pci_common_cfg and
    option functions
  virito: queue_reset: pci: move the per queue irq logic from vp_del_vqs
    to vp_del_vq
  virtio: queue_reset: pci: add independent function to enable msix vq
  virtio: queue_reset: pci: support VIRTIO_F_RING_RESET
  virtio: queue_reset: add helper
  virtio_net: virtnet_tx_timeout() fix style
  virtio_net: virtnet_tx_timeout() stop ref sq->vq
  virtio_net: split free_unused_bufs()
  virtio-net: support pair disable/enable

 drivers/net/virtio_net.c               | 200 ++++++++++++++++++++++---
 drivers/virtio/virtio_pci_common.c     | 135 +++++++++++++----
 drivers/virtio/virtio_pci_common.h     |   4 +
 drivers/virtio/virtio_pci_modern.c     |  73 +++++++++
 drivers/virtio/virtio_pci_modern_dev.c |  28 ++++
 include/linux/virtio_config.h          |  63 ++++++++
 include/linux/virtio_pci_modern.h      |   2 +
 include/uapi/linux/virtio_config.h     |   7 +-
 include/uapi/linux/virtio_pci.h        |   2 +
 9 files changed, 460 insertions(+), 54 deletions(-)

--
2.31.0

[PATCH v2 01/12] virtio: pci: struct virtio_pci_common_cfg add queue_notify_data

[Xuan Zhuo]

Add queue_notify_data in struct virtio_pci_common_cfg, which comes from
here https://github.com/oasis-tcs/virtio-spec/issues/89

Since I want to add queue_reset after it, I submitted this patch first.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 include/uapi/linux/virtio_pci.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
index 3a86f36d7e3d..492c89f56c6a 100644
--- a/include/uapi/linux/virtio_pci.h
+++ b/include/uapi/linux/virtio_pci.h
@@ -164,6 +164,7 @@ struct virtio_pci_common_cfg {
 	__le32 queue_avail_hi;		/* read-write */
 	__le32 queue_used_lo;		/* read-write */
 	__le32 queue_used_hi;		/* read-write */
+	__le16 queue_notify_data;	/* read-write */
 };
 
 /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
-- 
2.31.0

[PATCH v2 02/12] virtio: queue_reset: add VIRTIO_F_RING_RESET

[Xuan Zhuo]

Added VIRTIO_F_RING_RESET, it came from here
https://github.com/oasis-tcs/virtio-spec/issues/124

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 include/uapi/linux/virtio_config.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
index b5eda06f0d57..0862be802ff8 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		38
+#define VIRTIO_TRANSPORT_F_END		41
 
 #ifndef VIRTIO_CONFIG_NO_LEGACY
 /* Do we get callbacks when the ring is completely used, even if we've
@@ -92,4 +92,9 @@
  * Does the device support Single Root I/O Virtualization?
  */
 #define VIRTIO_F_SR_IOV			37
+
+/*
+ * This feature indicates that the driver can reset a queue individually.
+ */
+#define VIRTIO_F_RING_RESET		40
 #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
-- 
2.31.0

[PATCH v2 03/12] virtio: queue_reset: struct virtio_config_ops add callbacks for queue_reset

[Xuan Zhuo]

Performing reset on a queue is divided into two steps:

1. reset_vq: reset one vq
2. enable_reset_vq: re-enable the reset queue

In the first step, these tasks will be completed:
    1. notify the hardware queue to reset
    2. recycle the buffer from vq
    3. delete the vq

So add two callbacks reset_vq, enable_reset_vq to struct
virtio_config_ops. The parameters of enable_reset_vq are similar to
those of find_vqs.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 include/linux/virtio_config.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 4d107ad31149..e50a377c27a0 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -74,8 +74,22 @@ struct virtio_shm_region {
  * @set_vq_affinity: set the affinity for a virtqueue (optional).
  * @get_vq_affinity: get the affinity for a virtqueue (optional).
  * @get_shm_region: get a shared memory region based on the index.
+ * @reset_vq: reset a queue individually
+ *	vdev: the device
+ *	queue_index: the queue index
+ *	callback: callback to free unused bufs
+ *	data: pass to callback
+ *	returns 0 on success or error status
+ * @enable_reset_vq: enable a reset queue
+ *	vdev: the device
+ *	queue_index: the queue index
+ *	callback: callback for the virtqueue, NULL for vq that do not need a callback
+ *	name: virtqueue names (mainly for debugging), NULL for vq unused by driver
+ *	ctx: ctx
+ *	returns vq on success or error status
  */
 typedef void vq_callback_t(struct virtqueue *);
+typedef void vq_reset_callback_t(struct virtio_device *vdev, void *buf, void *data);
 struct virtio_config_ops {
 	void (*enable_cbs)(struct virtio_device *vdev);
 	void (*get)(struct virtio_device *vdev, unsigned offset,
@@ -100,6 +114,12 @@ struct virtio_config_ops {
 			int index);
 	bool (*get_shm_region)(struct virtio_device *vdev,
 			       struct virtio_shm_region *region, u8 id);
+	int (*reset_vq)(struct virtio_device *vdev, u16 queue_index,
+			vq_reset_callback_t *callback, void *data);
+	struct virtqueue *(*enable_reset_vq)(struct virtio_device *vdev,
+					     u16 queue_index,
+					     vq_callback_t *callback,
+					     const char *name, const bool *ctx);
 };
 
 /* If driver didn't advertise the feature, it will never appear. */
-- 
2.31.0

[PATCH v2 04/12] virtio: queue_reset: pci: update struct virtio_pci_common_cfg and option functions

[Xuan Zhuo]

Add queue_reset in virtio_pci_common_cfg, and add related operation
functions.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/virtio/virtio_pci_modern_dev.c | 28 ++++++++++++++++++++++++++
 include/linux/virtio_pci_modern.h      |  2 ++
 include/uapi/linux/virtio_pci.h        |  1 +
 3 files changed, 31 insertions(+)

diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
index e11ed748e661..be02f812a2f6 100644
--- a/drivers/virtio/virtio_pci_modern_dev.c
+++ b/drivers/virtio/virtio_pci_modern_dev.c
@@ -463,6 +463,34 @@ void vp_modern_set_status(struct virtio_pci_modern_device *mdev,
 }
 EXPORT_SYMBOL_GPL(vp_modern_set_status);
 
+/*
+ * vp_modern_get_queue_reset - get the queue reset status
+ * @mdev: the modern virtio-pci device
+ * @index: queue index
+ */
+int vp_modern_get_queue_reset(struct virtio_pci_modern_device *mdev, u16 index)
+{
+	struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+
+	vp_iowrite16(index, &cfg->queue_select);
+	return vp_ioread16(&cfg->queue_reset);
+}
+EXPORT_SYMBOL_GPL(vp_modern_get_queue_reset);
+
+/*
+ * vp_modern_set_queue_reset - reset the queue
+ * @mdev: the modern virtio-pci device
+ * @index: queue index
+ */
+void vp_modern_set_queue_reset(struct virtio_pci_modern_device *mdev, u16 index)
+{
+	struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+
+	vp_iowrite16(index, &cfg->queue_select);
+	vp_iowrite16(1, &cfg->queue_reset);
+}
+EXPORT_SYMBOL_GPL(vp_modern_set_queue_reset);
+
 /*
  * vp_modern_queue_vector - set the MSIX vector for a specific virtqueue
  * @mdev: the modern virtio-pci device
diff --git a/include/linux/virtio_pci_modern.h b/include/linux/virtio_pci_modern.h
index eb2bd9b4077d..cc4154dd7b28 100644
--- a/include/linux/virtio_pci_modern.h
+++ b/include/linux/virtio_pci_modern.h
@@ -106,4 +106,6 @@ void __iomem * vp_modern_map_vq_notify(struct virtio_pci_modern_device *mdev,
 				       u16 index, resource_size_t *pa);
 int vp_modern_probe(struct virtio_pci_modern_device *mdev);
 void vp_modern_remove(struct virtio_pci_modern_device *mdev);
+int vp_modern_get_queue_reset(struct virtio_pci_modern_device *mdev, u16 index);
+void vp_modern_set_queue_reset(struct virtio_pci_modern_device *mdev, u16 index);
 #endif
diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
index 492c89f56c6a..bf10f349087f 100644
--- a/include/uapi/linux/virtio_pci.h
+++ b/include/uapi/linux/virtio_pci.h
@@ -165,6 +165,7 @@ struct virtio_pci_common_cfg {
 	__le32 queue_used_lo;		/* read-write */
 	__le32 queue_used_hi;		/* read-write */
 	__le16 queue_notify_data;	/* read-write */
+	__le16 queue_reset;		/* read-write */
 };
 
 /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
-- 
2.31.0

[PATCH v2 05/12] virito: queue_reset: pci: move the per queue irq logic from vp_del_vqs to vp_del_vq

[Xuan Zhuo]

Move irq's processing logic into vp_del_vq(), so that this function can
handle a vq's del operation independently.

In the subsequent patches that supports queue reset, I have the
need to delete a vq separately.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/virtio/virtio_pci_common.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index fdbde1db5ec5..cb1eec0a6bf3 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -248,6 +248,17 @@ static void vp_del_vq(struct virtqueue *vq)
 	struct virtio_pci_vq_info *info = vp_dev->vqs[vq->index];
 	unsigned long flags;
 
+	if (vp_dev->per_vq_vectors) {
+		int v = vp_dev->vqs[vq->index]->msix_vector;
+
+		if (v != VIRTIO_MSI_NO_VECTOR) {
+			int irq = pci_irq_vector(vp_dev->pci_dev, v);
+
+			irq_set_affinity_hint(irq, NULL);
+			free_irq(irq, vq);
+		}
+	}
+
 	spin_lock_irqsave(&vp_dev->lock, flags);
 	list_del(&info->node);
 	spin_unlock_irqrestore(&vp_dev->lock, flags);
@@ -263,19 +274,9 @@ void vp_del_vqs(struct virtio_device *vdev)
 	struct virtqueue *vq, *n;
 	int i;
 
-	list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
-		if (vp_dev->per_vq_vectors) {
-			int v = vp_dev->vqs[vq->index]->msix_vector;
-
-			if (v != VIRTIO_MSI_NO_VECTOR) {
-				int irq = pci_irq_vector(vp_dev->pci_dev, v);
-
-				irq_set_affinity_hint(irq, NULL);
-				free_irq(irq, vq);
-			}
-		}
+	list_for_each_entry_safe(vq, n, &vdev->vqs, list)
 		vp_del_vq(vq);
-	}
+
 	vp_dev->per_vq_vectors = false;
 
 	if (vp_dev->intx_enabled) {
-- 
2.31.0

[PATCH v2 06/12] virtio: queue_reset: pci: add independent function to enable msix vq

[Xuan Zhuo]

Extract the vp_enable_vq_msix() function from vp_find_vqs_msix() . Used
to enable a msix vq individually.

In the subsequent patches that supports queue reset, I have the
need to enable a vq separately.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/virtio/virtio_pci_common.c | 61 ++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 20 deletions(-)

diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index cb1eec0a6bf3..5afe207ce28a 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -311,6 +311,40 @@ void vp_del_vqs(struct virtio_device *vdev)
 	vp_dev->vqs = NULL;
 }
 
+static struct virtqueue *vp_enable_vq_msix(struct virtio_device *vdev,
+					   int queue_index,
+					   vq_callback_t *callback,
+					   const char * const name,
+					   bool ctx,
+					   u16 msix_vec)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	struct virtqueue *vq;
+	int err;
+
+	vq = vp_setup_vq(vdev, queue_index, callback, name, ctx, msix_vec);
+	if (IS_ERR(vq))
+		return vq;
+
+	if (!vp_dev->per_vq_vectors || msix_vec == VIRTIO_MSI_NO_VECTOR)
+		return vq;
+
+	/* allocate per-vq irq if available and necessary */
+	snprintf(vp_dev->msix_names[msix_vec],
+		 sizeof *vp_dev->msix_names,
+		 "%s-%s", dev_name(&vp_dev->vdev.dev), name);
+
+	err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
+			  vring_interrupt, IRQF_NO_AUTOEN,
+			  vp_dev->msix_names[msix_vec], vq);
+	if (err) {
+		vp_del_vq(vq);
+		return ERR_PTR(err);
+	}
+
+	return vq;
+}
+
 static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
 		struct virtqueue *vqs[], vq_callback_t *callbacks[],
 		const char * const names[], bool per_vq_vectors,
@@ -320,6 +354,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
 	u16 msix_vec;
 	int i, err, nvectors, allocated_vectors, queue_idx = 0;
+	struct virtqueue *vq;
 
 	vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL);
 	if (!vp_dev->vqs)
@@ -355,28 +390,14 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
 			msix_vec = allocated_vectors++;
 		else
 			msix_vec = VP_MSIX_VQ_VECTOR;
-		vqs[i] = vp_setup_vq(vdev, queue_idx++, callbacks[i], names[i],
-				     ctx ? ctx[i] : false,
-				     msix_vec);
-		if (IS_ERR(vqs[i])) {
-			err = PTR_ERR(vqs[i]);
-			goto error_find;
-		}
-
-		if (!vp_dev->per_vq_vectors || msix_vec == VIRTIO_MSI_NO_VECTOR)
-			continue;
 
-		/* allocate per-vq irq if available and necessary */
-		snprintf(vp_dev->msix_names[msix_vec],
-			 sizeof *vp_dev->msix_names,
-			 "%s-%s",
-			 dev_name(&vp_dev->vdev.dev), names[i]);
-		err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
-				  vring_interrupt, IRQF_NO_AUTOEN,
-				  vp_dev->msix_names[msix_vec],
-				  vqs[i]);
-		if (err)
+		vq = vp_enable_vq_msix(vdev, queue_idx++, callbacks[i],
+				       names[i], ctx ? ctx[i] : false, msix_vec);
+		if (IS_ERR(vq)) {
+			err = PTR_ERR(vq);
 			goto error_find;
+		}
+		vqs[i] = vq;
 	}
 	return 0;
 
-- 
2.31.0

[PATCH v2 07/12] virtio: queue_reset: pci: support VIRTIO_F_RING_RESET

[Xuan Zhuo]

This patch implements virtio pci support for QUEUE RESET.

Performing reset on a queue is divided into two steps:

1. reset_vq: reset one vq
2. enable_reset_vq: re-enable the reset queue

In the first step, these tasks will be completed:
   1. notify the hardware queue to reset
   2. recycle the buffer from vq
   3. delete the vq

When deleting a vq, vp_del_vq() will be called to release all the memory
of the vq. But this does not affect the process of deleting vqs, because
that is based on the queue to release all the vqs. During this process,
the vq has been removed from the queue.

When deleting vq, info and vq will be released, and I save msix_vec in
vp_dev->vqs[queue_index]. When re-enable, the current msix_vec can be
reused. And based on intx_enabled to determine which method to use to
enable this queue.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/virtio/virtio_pci_common.c | 49 ++++++++++++++++++++
 drivers/virtio/virtio_pci_common.h |  4 ++
 drivers/virtio/virtio_pci_modern.c | 73 ++++++++++++++++++++++++++++++
 3 files changed, 126 insertions(+)

diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 5afe207ce28a..28b5ffde4621 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -464,6 +464,55 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
 	return vp_find_vqs_intx(vdev, nvqs, vqs, callbacks, names, ctx);
 }
 
+#define VQ_IS_DELETED(vp_dev, idx) ((unsigned long)vp_dev->vqs[idx] & 1)
+#define VQ_RESET_MSIX_VEC(vp_dev, idx) ((unsigned long)vp_dev->vqs[idx] >> 2)
+#define VQ_RESET_MARK(msix_vec) ((void *)(long)((msix_vec << 2) + 1))
+
+void vp_del_reset_vq(struct virtio_device *vdev, u16 queue_index)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	struct virtio_pci_vq_info *info;
+	u16 msix_vec;
+
+	info = vp_dev->vqs[queue_index];
+
+	msix_vec = info->msix_vector;
+
+	/* delete vq */
+	vp_del_vq(info->vq);
+
+	/* Mark the vq has been deleted, and save the msix_vec. */
+	vp_dev->vqs[queue_index] = VQ_RESET_MARK(msix_vec);
+}
+
+struct virtqueue *vp_enable_reset_vq(struct virtio_device *vdev,
+				     int queue_index,
+				     vq_callback_t *callback,
+				     const char *name,
+				     const bool ctx)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	struct virtqueue *vq;
+	u16 msix_vec;
+
+	if (!VQ_IS_DELETED(vp_dev, queue_index))
+		return ERR_PTR(-EPERM);
+
+	msix_vec = VQ_RESET_MSIX_VEC(vp_dev, queue_index);
+
+	if (vp_dev->intx_enabled)
+		vq = vp_setup_vq(vdev, queue_index, callback, name, ctx,
+				 VIRTIO_MSI_NO_VECTOR);
+	else
+		vq = vp_enable_vq_msix(vdev, queue_index, callback, name, ctx,
+				       msix_vec);
+
+	if (IS_ERR(vq))
+		vp_dev->vqs[queue_index] = VQ_RESET_MARK(msix_vec);
+
+	return vq;
+}
+
 const char *vp_bus_name(struct virtio_device *vdev)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index 23f6c5c678d5..96c13b1398f8 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -115,6 +115,10 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
 		struct virtqueue *vqs[], vq_callback_t *callbacks[],
 		const char * const names[], const bool *ctx,
 		struct irq_affinity *desc);
+void vp_del_reset_vq(struct virtio_device *vdev, u16 queue_index);
+struct virtqueue *vp_enable_reset_vq(struct virtio_device *vdev, int queue_index,
+				     vq_callback_t *callback, const char *name,
+				     const bool ctx);
 const char *vp_bus_name(struct virtio_device *vdev);
 
 /* Setup the affinity for a virtqueue:
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 5455bc041fb6..fbf87239c920 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -34,6 +34,9 @@ static void vp_transport_features(struct virtio_device *vdev, u64 features)
 	if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
 			pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
 		__virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
+
+	if (features & BIT_ULL(VIRTIO_F_RING_RESET))
+		__virtio_set_bit(vdev, VIRTIO_F_RING_RESET);
 }
 
 /* virtio config->finalize_features() implementation */
@@ -176,6 +179,72 @@ static void vp_reset(struct virtio_device *vdev)
 	vp_disable_cbs(vdev);
 }
 
+static int vp_modern_reset_vq(struct virtio_device *vdev, u16 queue_index,
+			      vq_reset_callback_t *callback, void *data)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
+	struct virtio_pci_vq_info *info;
+	u16 msix_vec;
+	void *buf;
+
+	if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
+		return -ENOENT;
+
+	vp_modern_set_queue_reset(mdev, queue_index);
+
+	/* After write 1 to queue reset, the driver MUST wait for a read of
+	 * queue reset to return 1.
+	 */
+	while (vp_modern_get_queue_reset(mdev, queue_index) != 1)
+		msleep(1);
+
+	info = vp_dev->vqs[queue_index];
+	msix_vec = info->msix_vector;
+
+	/* Disable VQ callback. */
+	if (vp_dev->per_vq_vectors && msix_vec != VIRTIO_MSI_NO_VECTOR)
+		disable_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec));
+
+	while ((buf = virtqueue_detach_unused_buf(info->vq)) != NULL)
+		callback(vdev, buf, data);
+
+	vp_del_reset_vq(vdev, queue_index);
+
+	return 0;
+}
+
+static struct virtqueue *vp_modern_enable_reset_vq(struct virtio_device *vdev,
+						   u16 queue_index,
+						   vq_callback_t *callback,
+						   const char *name,
+						   const bool *ctx)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
+	struct virtqueue *vq;
+	u16 msix_vec;
+
+	if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
+		return ERR_PTR(-ENOENT);
+
+	/* check queue reset status */
+	if (vp_modern_get_queue_reset(mdev, queue_index) != 1)
+		return ERR_PTR(-EBUSY);
+
+	vq = vp_enable_reset_vq(vdev, queue_index, callback, name, ctx);
+	if (IS_ERR(vq))
+		return vq;
+
+	vp_modern_set_queue_enable(&vp_dev->mdev, vq->index, true);
+
+	msix_vec = vp_dev->vqs[queue_index]->msix_vector;
+	if (vp_dev->per_vq_vectors && msix_vec != VIRTIO_MSI_NO_VECTOR)
+		enable_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec));
+
+	return vq;
+}
+
 static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
 {
 	return vp_modern_config_vector(&vp_dev->mdev, vector);
@@ -395,6 +464,8 @@ static const struct virtio_config_ops virtio_pci_config_nodev_ops = {
 	.set_vq_affinity = vp_set_vq_affinity,
 	.get_vq_affinity = vp_get_vq_affinity,
 	.get_shm_region  = vp_get_shm_region,
+	.reset_vq	 = vp_modern_reset_vq,
+	.enable_reset_vq = vp_modern_enable_reset_vq,
 };
 
 static const struct virtio_config_ops virtio_pci_config_ops = {
@@ -413,6 +484,8 @@ static const struct virtio_config_ops virtio_pci_config_ops = {
 	.set_vq_affinity = vp_set_vq_affinity,
 	.get_vq_affinity = vp_get_vq_affinity,
 	.get_shm_region  = vp_get_shm_region,
+	.reset_vq	 = vp_modern_reset_vq,
+	.enable_reset_vq = vp_modern_enable_reset_vq,
 };
 
 /* the PCI probing function */
-- 
2.31.0

[PATCH v2 08/12] virtio: queue_reset: add helper

[Xuan Zhuo]

Add helper for virtio queue reset.

* virtio_reset_vq: reset a queue individually
* virtio_enable_resetq: enable a reset queue

In the virtio_reset_vq(), these tasks will be completed:
   1. notify the hardware queue to reset
   2. recycle the buffer from vq
   3. delete the vq

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 include/linux/virtio_config.h | 43 +++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index e50a377c27a0..f84347f4e4ee 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -239,6 +239,49 @@ int virtio_find_vqs_ctx(struct virtio_device *vdev, unsigned nvqs,
 				      desc);
 }
 
+/**
+ * virtio_reset_vq - reset a queue individually
+ * @vdev: the device
+ * @queue_index: the queue index
+ * @callback: callback to free unused bufs
+ * @data: pass to callback
+ *
+ * returns 0 on success or error status
+ *
+ */
+static inline
+int virtio_reset_vq(struct virtio_device *vdev, u16 queue_index,
+		    vq_reset_callback_t *callback, void *data)
+{
+	if (!vdev->config->reset_vq)
+		return -ENOENT;
+
+	return vdev->config->reset_vq(vdev, queue_index, callback, data);
+}
+
+/**
+ * virtio_enable_resetq - enable a reset queue
+ * @vdev: the device
+ * @queue_index: the queue index
+ * @callback: callback for the virtqueue, NULL for vq that do not need a callback
+ * @name: virtqueue names (mainly for debugging), NULL for vq unused by driver
+ * @ctx: ctx
+ *
+ * returns vq on success or error status
+ *
+ */
+static inline
+struct virtqueue *virtio_enable_resetq(struct virtio_device *vdev,
+				       u16 queue_index, vq_callback_t *callback,
+				       const char *name, const bool *ctx)
+{
+	if (!vdev->config->enable_reset_vq)
+		return ERR_PTR(-ENOENT);
+
+	return vdev->config->enable_reset_vq(vdev, queue_index, callback,
+					     name, ctx);
+}
+
 /**
  * virtio_device_ready - enable vq use in probe function
  * @vdev: the device
-- 
2.31.0

[PATCH v2 09/12] virtio_net: virtnet_tx_timeout() fix style

[Xuan Zhuo]

Replace priv with vi which is commonly used in the virtio-net module.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio_net.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 569eecfbc2cd..97eb4dddba1f 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2683,8 +2683,8 @@ static int virtnet_set_features(struct net_device *dev,
 
 static void virtnet_tx_timeout(struct net_device *dev, unsigned int txqueue)
 {
-	struct virtnet_info *priv = netdev_priv(dev);
-	struct send_queue *sq = &priv->sq[txqueue];
+	struct virtnet_info *vi = netdev_priv(dev);
+	struct send_queue *sq = &vi->sq[txqueue];
 	struct netdev_queue *txq = netdev_get_tx_queue(dev, txqueue);
 
 	u64_stats_update_begin(&sq->stats.syncp);
-- 
2.31.0

[PATCH v2 10/12] virtio_net: virtnet_tx_timeout() stop ref sq->vq

[Xuan Zhuo]

Make virtnet_tx_timeout() no longer refer to vq directly. Because sq->vq
may be equal to NULL after implementing rx/tx queue disable/enable.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 97eb4dddba1f..2add7fe749b8 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2692,7 +2692,7 @@ static void virtnet_tx_timeout(struct net_device *dev, unsigned int txqueue)
 	u64_stats_update_end(&sq->stats.syncp);
 
 	netdev_err(dev, "TX timeout on queue: %u, sq: %s, vq: 0x%x, name: %s, %u usecs ago\n",
-		   txqueue, sq->name, sq->vq->index, sq->vq->name,
+		   txqueue, sq->name, txq2vq(sq - vi->sq), sq->name,
 		   jiffies_to_usecs(jiffies - READ_ONCE(txq->trans_start)));
 }
 
-- 
2.31.0

[PATCH v2 11/12] virtio_net: split free_unused_bufs()

[Xuan Zhuo]

This patch separates two functions for freeing sq buf and rq buf from
free_unused_bufs().

When supporting the enable/disable tx/rq queue in the future, it is
necessary to support separate recovery of a sq buf or a rq buf.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio_net.c | 46 ++++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 18 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 2add7fe749b8..ea90a1a57c9e 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2804,33 +2804,43 @@ static void free_receive_page_frags(struct virtnet_info *vi)
 			put_page(vi->rq[i].alloc_frag.page);
 }
 
+static void virtnet_sq_free_unused_buf(struct virtnet_info *vi,
+				       struct send_queue *sq, void *buf)
+{
+	if (!is_xdp_frame(buf))
+		dev_kfree_skb(buf);
+	else
+		xdp_return_frame(ptr_to_xdp(buf));
+}
+
+static void virtnet_rq_free_unused_buf(struct virtnet_info *vi,
+				       struct receive_queue *rq, void *buf)
+{
+	if (vi->mergeable_rx_bufs)
+		put_page(virt_to_head_page(buf));
+	else if (vi->big_packets)
+		give_pages(rq, buf);
+	else
+		put_page(virt_to_head_page(buf));
+}
+
 static void free_unused_bufs(struct virtnet_info *vi)
 {
+	struct receive_queue *rq;
+	struct send_queue *sq;
 	void *buf;
 	int i;
 
 	for (i = 0; i < vi->max_queue_pairs; i++) {
-		struct virtqueue *vq = vi->sq[i].vq;
-		while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
-			if (!is_xdp_frame(buf))
-				dev_kfree_skb(buf);
-			else
-				xdp_return_frame(ptr_to_xdp(buf));
-		}
+		sq = &vi->sq[i];
+		while ((buf = virtqueue_detach_unused_buf(sq->vq)) != NULL)
+			virtnet_sq_free_unused_buf(vi, sq, buf);
 	}
 
 	for (i = 0; i < vi->max_queue_pairs; i++) {
-		struct virtqueue *vq = vi->rq[i].vq;
-
-		while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
-			if (vi->mergeable_rx_bufs) {
-				put_page(virt_to_head_page(buf));
-			} else if (vi->big_packets) {
-				give_pages(&vi->rq[i], buf);
-			} else {
-				put_page(virt_to_head_page(buf));
-			}
-		}
+		rq = &vi->rq[i];
+		while ((buf = virtqueue_detach_unused_buf(rq->vq)) != NULL)
+			virtnet_rq_free_unused_buf(vi, rq, buf);
 	}
 }
 
-- 
2.31.0

[PATCH v2 12/12] virtio-net: support pair disable/enable

[Xuan Zhuo]

This patch implements virtio-net rx/tx pair disable/enable functionality
based on virtio queue reset. The purpose of the current implementation
is to quickly recycle the buffer submitted to vq.

In the process of pair disable, in theory, as long as virtio supports
queue reset, there will be no exceptions.

However, in the process of pari enable, there may be exceptions due to
memory allocation. In this case, vq == NULL, but napi will still
be enabled. Because napi_disable is similar to a lock, napi_enable must
be called after calling napi_disable.

Since enable fails, the driver will not receive an interrupt from the
device to wake up napi, so the driver is safe. But we still need to add
vq checks in some places to ensure safety, such as refill_work().

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio_net.c | 148 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 148 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ea90a1a57c9e..dba95553247c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1369,6 +1369,9 @@ static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
 {
 	napi_enable(napi);
 
+	if (!vq)
+		return;
+
 	/* If all buffers were filled by other side before we napi_enabled, we
 	 * won't get another interrupt, so process any outstanding packets now.
 	 * Call local_bh_enable after to trigger softIRQ processing.
@@ -1413,6 +1416,10 @@ static void refill_work(struct work_struct *work)
 		struct receive_queue *rq = &vi->rq[i];
 
 		napi_disable(&rq->napi);
+		if (!rq->vq) {
+			virtnet_napi_enable(rq->vq, &rq->napi);
+			continue;
+		}
 		still_empty = !try_fill_recv(vi, rq, GFP_KERNEL);
 		virtnet_napi_enable(rq->vq, &rq->napi);
 
@@ -2871,6 +2878,147 @@ static unsigned int mergeable_min_buf_len(struct virtnet_info *vi, struct virtqu
 		   (unsigned int)GOOD_PACKET_LEN);
 }
 
+static void virtnet_rq_free_unused_buf_cb(struct virtio_device *vdev,
+					  void *buf, void *data)
+{
+	virtnet_rq_free_unused_buf(vdev->priv, data, buf);
+}
+
+static void virtnet_sq_free_unused_buf_cb(struct virtio_device *vdev,
+					  void *buf, void *data)
+{
+	virtnet_rq_free_unused_buf(vdev->priv, data, buf);
+}
+
+static int __virtnet_rx_vq_disable(struct virtnet_info *vi,
+				   struct receive_queue *rq)
+{
+	int err, qnum;
+
+	qnum = rxq2vq(rq - vi->rq);
+
+	napi_disable(&rq->napi);
+
+	err = virtio_reset_vq(vi->vdev, qnum, virtnet_rq_free_unused_buf_cb, rq);
+	if (err) {
+		virtnet_napi_enable(rq->vq, &rq->napi);
+		return err;
+	}
+
+	rq->vq = NULL;
+
+	return err;
+}
+
+static int __virtnet_tx_vq_disable(struct virtnet_info *vi,
+				   struct send_queue *sq)
+{
+	struct netdev_queue *txq;
+	int err, qnum;
+
+	qnum = txq2vq(sq - vi->sq);
+
+	netif_stop_subqueue(vi->dev, sq - vi->sq);
+	virtnet_napi_tx_disable(&sq->napi);
+
+	/* wait xmit done */
+	txq = netdev_get_tx_queue(vi->dev, qnum);
+	__netif_tx_lock(txq, raw_smp_processor_id());
+	__netif_tx_unlock(txq);
+
+	err = virtio_reset_vq(vi->vdev, qnum, virtnet_sq_free_unused_buf_cb, sq);
+	if (err) {
+		virtnet_napi_tx_enable(vi, sq->vq, &sq->napi);
+		netif_start_subqueue(vi->dev, sq - vi->sq);
+		return err;
+	}
+
+	sq->vq = NULL;
+
+	return err;
+}
+
+static int virtnet_pair_disable(struct virtnet_info *vi, int i)
+{
+	int err;
+
+	err = __virtnet_rx_vq_disable(vi, vi->rq + i);
+	if (err)
+		return err;
+
+	return __virtnet_tx_vq_disable(vi, vi->sq + i);
+}
+
+static int virtnet_enable_resetq(struct virtnet_info *vi,
+				 struct receive_queue *rq,
+				 struct send_queue *sq)
+{
+	vq_callback_t *callback;
+	struct virtqueue *vq;
+	const char *name;
+	int vq_idx;
+	bool ctx;
+
+	if (rq) {
+		vq = rq->vq;
+		vq_idx = rxq2vq(rq - vi->rq);
+		callback = skb_recv_done;
+		name = rq->name;
+
+	} else {
+		vq = sq->vq;
+		vq_idx = txq2vq(sq - vi->sq);
+		callback = skb_xmit_done;
+		name = sq->name;
+	}
+
+	if (vq)
+		return -EBUSY;
+
+	if (!vi->big_packets || vi->mergeable_rx_bufs)
+		ctx = true;
+	else
+		ctx = false;
+
+	vq = virtio_enable_resetq(vi->vdev, vq_idx, callback, name, &ctx);
+	if (IS_ERR(vq))
+		return PTR_ERR(vq);
+
+	if (rq)
+		rq->vq = vq;
+	else
+		sq->vq = vq;
+
+	return 0;
+}
+
+static int virtnet_pair_enable(struct virtnet_info *vi, int i)
+{
+	struct receive_queue *rq;
+	struct send_queue *sq;
+	int err;
+
+	rq = vi->rq + i;
+	sq = vi->sq + i;
+
+	/* tx */
+	err = virtnet_enable_resetq(vi, NULL, sq);
+	if (err)
+		goto err;
+	else
+		netif_start_subqueue(vi->dev, sq - vi->sq);
+
+	/* rx */
+	err = virtnet_enable_resetq(vi, rq, NULL);
+	if (err)
+		return err;
+
+err:
+	virtnet_napi_tx_enable(vi, sq->vq, &sq->napi);
+	virtnet_napi_enable(rq->vq, &rq->napi);
+	return 0;
+}
+
 static int virtnet_find_vqs(struct virtnet_info *vi)
 {
 	vq_callback_t **callbacks;
-- 
2.31.0

Re: [PATCH v2 07/12] virtio: queue_reset: pci: support VIRTIO_F_RING_RESET

[Michael S. Tsirkin]

On Thu, Jan 20, 2022 at 02:42:58PM +0800, Xuan Zhuo wrote:
> This patch implements virtio pci support for QUEUE RESET.
> 
> Performing reset on a queue is divided into two steps:
> 
> 1. reset_vq: reset one vq
> 2. enable_reset_vq: re-enable the reset queue
> 
> In the first step, these tasks will be completed:
>    1. notify the hardware queue to reset
>    2. recycle the buffer from vq
>    3. delete the vq
> 
> When deleting a vq, vp_del_vq() will be called to release all the memory
> of the vq. But this does not affect the process of deleting vqs, because
> that is based on the queue to release all the vqs. During this process,
> the vq has been removed from the queue.
> 
> When deleting vq, info and vq will be released, and I save msix_vec in
> vp_dev->vqs[queue_index]. When re-enable, the current msix_vec can be
> reused. And based on intx_enabled to determine which method to use to
> enable this queue.
> 
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>

There's something I don't understand here. It looks like
you assume that when you reset a queue, you also
reset the mapping from queue to event vector.
The spec does not say it should, and I don't think it's
useful to extend spec to do it - we already have a simple
way to tweak the mapping.

Avoid doing that, and things will be much easier, with no need
to interact with a transport, won't they?


> ---
>  drivers/virtio/virtio_pci_common.c | 49 ++++++++++++++++++++
>  drivers/virtio/virtio_pci_common.h |  4 ++
>  drivers/virtio/virtio_pci_modern.c | 73 ++++++++++++++++++++++++++++++
>  3 files changed, 126 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 5afe207ce28a..28b5ffde4621 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -464,6 +464,55 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
>  	return vp_find_vqs_intx(vdev, nvqs, vqs, callbacks, names, ctx);
>  }
>  
> +#define VQ_IS_DELETED(vp_dev, idx) ((unsigned long)vp_dev->vqs[idx] & 1)
> +#define VQ_RESET_MSIX_VEC(vp_dev, idx) ((unsigned long)vp_dev->vqs[idx] >> 2)
> +#define VQ_RESET_MARK(msix_vec) ((void *)(long)((msix_vec << 2) + 1))
> +
> +void vp_del_reset_vq(struct virtio_device *vdev, u16 queue_index)
> +{
> +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> +	struct virtio_pci_vq_info *info;
> +	u16 msix_vec;
> +
> +	info = vp_dev->vqs[queue_index];
> +
> +	msix_vec = info->msix_vector;
> +
> +	/* delete vq */
> +	vp_del_vq(info->vq);
> +
> +	/* Mark the vq has been deleted, and save the msix_vec. */
> +	vp_dev->vqs[queue_index] = VQ_RESET_MARK(msix_vec);
> +}
> +
> +struct virtqueue *vp_enable_reset_vq(struct virtio_device *vdev,
> +				     int queue_index,
> +				     vq_callback_t *callback,
> +				     const char *name,
> +				     const bool ctx)
> +{
> +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> +	struct virtqueue *vq;
> +	u16 msix_vec;
> +
> +	if (!VQ_IS_DELETED(vp_dev, queue_index))
> +		return ERR_PTR(-EPERM);
> +
> +	msix_vec = VQ_RESET_MSIX_VEC(vp_dev, queue_index);
> +
> +	if (vp_dev->intx_enabled)
> +		vq = vp_setup_vq(vdev, queue_index, callback, name, ctx,
> +				 VIRTIO_MSI_NO_VECTOR);
> +	else
> +		vq = vp_enable_vq_msix(vdev, queue_index, callback, name, ctx,
> +				       msix_vec);
> +
> +	if (IS_ERR(vq))
> +		vp_dev->vqs[queue_index] = VQ_RESET_MARK(msix_vec);
> +
> +	return vq;
> +}
> +
>  const char *vp_bus_name(struct virtio_device *vdev)
>  {
>  	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
> index 23f6c5c678d5..96c13b1398f8 100644
> --- a/drivers/virtio/virtio_pci_common.h
> +++ b/drivers/virtio/virtio_pci_common.h
> @@ -115,6 +115,10 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
>  		struct virtqueue *vqs[], vq_callback_t *callbacks[],
>  		const char * const names[], const bool *ctx,
>  		struct irq_affinity *desc);
> +void vp_del_reset_vq(struct virtio_device *vdev, u16 queue_index);
> +struct virtqueue *vp_enable_reset_vq(struct virtio_device *vdev, int queue_index,
> +				     vq_callback_t *callback, const char *name,
> +				     const bool ctx);
>  const char *vp_bus_name(struct virtio_device *vdev);
>  
>  /* Setup the affinity for a virtqueue:
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 5455bc041fb6..fbf87239c920 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -34,6 +34,9 @@ static void vp_transport_features(struct virtio_device *vdev, u64 features)
>  	if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
>  			pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
>  		__virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> +
> +	if (features & BIT_ULL(VIRTIO_F_RING_RESET))
> +		__virtio_set_bit(vdev, VIRTIO_F_RING_RESET);
>  }
>  
>  /* virtio config->finalize_features() implementation */
> @@ -176,6 +179,72 @@ static void vp_reset(struct virtio_device *vdev)
>  	vp_disable_cbs(vdev);
>  }
>  
> +static int vp_modern_reset_vq(struct virtio_device *vdev, u16 queue_index,
> +			      vq_reset_callback_t *callback, void *data)
> +{
> +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> +	struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> +	struct virtio_pci_vq_info *info;
> +	u16 msix_vec;
> +	void *buf;
> +
> +	if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
> +		return -ENOENT;
> +
> +	vp_modern_set_queue_reset(mdev, queue_index);
> +
> +	/* After write 1 to queue reset, the driver MUST wait for a read of
> +	 * queue reset to return 1.
> +	 */
> +	while (vp_modern_get_queue_reset(mdev, queue_index) != 1)
> +		msleep(1);
> +
> +	info = vp_dev->vqs[queue_index];
> +	msix_vec = info->msix_vector;
> +
> +	/* Disable VQ callback. */
> +	if (vp_dev->per_vq_vectors && msix_vec != VIRTIO_MSI_NO_VECTOR)
> +		disable_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec));
> +
> +	while ((buf = virtqueue_detach_unused_buf(info->vq)) != NULL)
> +		callback(vdev, buf, data);
> +
> +	vp_del_reset_vq(vdev, queue_index);
> +
> +	return 0;
> +}
> +
> +static struct virtqueue *vp_modern_enable_reset_vq(struct virtio_device *vdev,
> +						   u16 queue_index,
> +						   vq_callback_t *callback,
> +						   const char *name,
> +						   const bool *ctx)
> +{
> +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> +	struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> +	struct virtqueue *vq;
> +	u16 msix_vec;
> +
> +	if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
> +		return ERR_PTR(-ENOENT);
> +
> +	/* check queue reset status */
> +	if (vp_modern_get_queue_reset(mdev, queue_index) != 1)
> +		return ERR_PTR(-EBUSY);
> +
> +	vq = vp_enable_reset_vq(vdev, queue_index, callback, name, ctx);
> +	if (IS_ERR(vq))
> +		return vq;
> +
> +	vp_modern_set_queue_enable(&vp_dev->mdev, vq->index, true);
> +
> +	msix_vec = vp_dev->vqs[queue_index]->msix_vector;
> +	if (vp_dev->per_vq_vectors && msix_vec != VIRTIO_MSI_NO_VECTOR)
> +		enable_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec));
> +
> +	return vq;
> +}
> +
>  static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
>  {
>  	return vp_modern_config_vector(&vp_dev->mdev, vector);
> @@ -395,6 +464,8 @@ static const struct virtio_config_ops virtio_pci_config_nodev_ops = {
>  	.set_vq_affinity = vp_set_vq_affinity,
>  	.get_vq_affinity = vp_get_vq_affinity,
>  	.get_shm_region  = vp_get_shm_region,
> +	.reset_vq	 = vp_modern_reset_vq,
> +	.enable_reset_vq = vp_modern_enable_reset_vq,
>  };
>  
>  static const struct virtio_config_ops virtio_pci_config_ops = {
> @@ -413,6 +484,8 @@ static const struct virtio_config_ops virtio_pci_config_ops = {
>  	.set_vq_affinity = vp_set_vq_affinity,
>  	.get_vq_affinity = vp_get_vq_affinity,
>  	.get_shm_region  = vp_get_shm_region,
> +	.reset_vq	 = vp_modern_reset_vq,
> +	.enable_reset_vq = vp_modern_enable_reset_vq,
>  };
>  
>  /* the PCI probing function */
> -- 
> 2.31.0

Re: [PATCH v2 07/12] virtio: queue_reset: pci: support VIRTIO_F_RING_RESET

[Michael S. Tsirkin]

On Thu, Jan 20, 2022 at 07:46:20PM +0800, Xuan Zhuo wrote:
> On Thu, 20 Jan 2022 05:55:14 -0500, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Thu, Jan 20, 2022 at 02:42:58PM +0800, Xuan Zhuo wrote:
> > > This patch implements virtio pci support for QUEUE RESET.
> > >
> > > Performing reset on a queue is divided into two steps:
> > >
> > > 1. reset_vq: reset one vq
> > > 2. enable_reset_vq: re-enable the reset queue
> > >
> > > In the first step, these tasks will be completed:
> > >    1. notify the hardware queue to reset
> > >    2. recycle the buffer from vq
> > >    3. delete the vq
> > >
> > > When deleting a vq, vp_del_vq() will be called to release all the memory
> > > of the vq. But this does not affect the process of deleting vqs, because
> > > that is based on the queue to release all the vqs. During this process,
> > > the vq has been removed from the queue.
> > >
> > > When deleting vq, info and vq will be released, and I save msix_vec in
> > > vp_dev->vqs[queue_index]. When re-enable, the current msix_vec can be
> > > reused. And based on intx_enabled to determine which method to use to
> > > enable this queue.
> > >
> > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> >
> > There's something I don't understand here. It looks like
> > you assume that when you reset a queue, you also
> > reset the mapping from queue to event vector.
> > The spec does not say it should, and I don't think it's
> > useful to extend spec to do it - we already have a simple
> > way to tweak the mapping.
> >
> 
> Sorry, what is the already existing method you are referring to, I didn't find
> it.


Write 0xffff into vector number.

> I think you mean that we don't have to reset the event vector, I think you are
> right.
> 
> 
> 
> Thanks.
> 
> > Avoid doing that, and things will be much easier, with no need
> > to interact with a transport, won't they?
> >
> >
> > > ---
> > >  drivers/virtio/virtio_pci_common.c | 49 ++++++++++++++++++++
> > >  drivers/virtio/virtio_pci_common.h |  4 ++
> > >  drivers/virtio/virtio_pci_modern.c | 73 ++++++++++++++++++++++++++++++
> > >  3 files changed, 126 insertions(+)
> > >
> > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> > > index 5afe207ce28a..28b5ffde4621 100644
> > > --- a/drivers/virtio/virtio_pci_common.c
> > > +++ b/drivers/virtio/virtio_pci_common.c
> > > @@ -464,6 +464,55 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> > >  	return vp_find_vqs_intx(vdev, nvqs, vqs, callbacks, names, ctx);
> > >  }
> > >
> > > +#define VQ_IS_DELETED(vp_dev, idx) ((unsigned long)vp_dev->vqs[idx] & 1)
> > > +#define VQ_RESET_MSIX_VEC(vp_dev, idx) ((unsigned long)vp_dev->vqs[idx] >> 2)
> > > +#define VQ_RESET_MARK(msix_vec) ((void *)(long)((msix_vec << 2) + 1))
> > > +
> > > +void vp_del_reset_vq(struct virtio_device *vdev, u16 queue_index)
> > > +{
> > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > +	struct virtio_pci_vq_info *info;
> > > +	u16 msix_vec;
> > > +
> > > +	info = vp_dev->vqs[queue_index];
> > > +
> > > +	msix_vec = info->msix_vector;
> > > +
> > > +	/* delete vq */
> > > +	vp_del_vq(info->vq);
> > > +
> > > +	/* Mark the vq has been deleted, and save the msix_vec. */
> > > +	vp_dev->vqs[queue_index] = VQ_RESET_MARK(msix_vec);
> > > +}
> > > +
> > > +struct virtqueue *vp_enable_reset_vq(struct virtio_device *vdev,
> > > +				     int queue_index,
> > > +				     vq_callback_t *callback,
> > > +				     const char *name,
> > > +				     const bool ctx)
> > > +{
> > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > +	struct virtqueue *vq;
> > > +	u16 msix_vec;
> > > +
> > > +	if (!VQ_IS_DELETED(vp_dev, queue_index))
> > > +		return ERR_PTR(-EPERM);
> > > +
> > > +	msix_vec = VQ_RESET_MSIX_VEC(vp_dev, queue_index);
> > > +
> > > +	if (vp_dev->intx_enabled)
> > > +		vq = vp_setup_vq(vdev, queue_index, callback, name, ctx,
> > > +				 VIRTIO_MSI_NO_VECTOR);
> > > +	else
> > > +		vq = vp_enable_vq_msix(vdev, queue_index, callback, name, ctx,
> > > +				       msix_vec);
> > > +
> > > +	if (IS_ERR(vq))
> > > +		vp_dev->vqs[queue_index] = VQ_RESET_MARK(msix_vec);
> > > +
> > > +	return vq;
> > > +}
> > > +
> > >  const char *vp_bus_name(struct virtio_device *vdev)
> > >  {
> > >  	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
> > > index 23f6c5c678d5..96c13b1398f8 100644
> > > --- a/drivers/virtio/virtio_pci_common.h
> > > +++ b/drivers/virtio/virtio_pci_common.h
> > > @@ -115,6 +115,10 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> > >  		struct virtqueue *vqs[], vq_callback_t *callbacks[],
> > >  		const char * const names[], const bool *ctx,
> > >  		struct irq_affinity *desc);
> > > +void vp_del_reset_vq(struct virtio_device *vdev, u16 queue_index);
> > > +struct virtqueue *vp_enable_reset_vq(struct virtio_device *vdev, int queue_index,
> > > +				     vq_callback_t *callback, const char *name,
> > > +				     const bool ctx);
> > >  const char *vp_bus_name(struct virtio_device *vdev);
> > >
> > >  /* Setup the affinity for a virtqueue:
> > > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> > > index 5455bc041fb6..fbf87239c920 100644
> > > --- a/drivers/virtio/virtio_pci_modern.c
> > > +++ b/drivers/virtio/virtio_pci_modern.c
> > > @@ -34,6 +34,9 @@ static void vp_transport_features(struct virtio_device *vdev, u64 features)
> > >  	if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> > >  			pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> > >  		__virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> > > +
> > > +	if (features & BIT_ULL(VIRTIO_F_RING_RESET))
> > > +		__virtio_set_bit(vdev, VIRTIO_F_RING_RESET);
> > >  }
> > >
> > >  /* virtio config->finalize_features() implementation */
> > > @@ -176,6 +179,72 @@ static void vp_reset(struct virtio_device *vdev)
> > >  	vp_disable_cbs(vdev);
> > >  }
> > >
> > > +static int vp_modern_reset_vq(struct virtio_device *vdev, u16 queue_index,
> > > +			      vq_reset_callback_t *callback, void *data)
> > > +{
> > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > +	struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> > > +	struct virtio_pci_vq_info *info;
> > > +	u16 msix_vec;
> > > +	void *buf;
> > > +
> > > +	if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
> > > +		return -ENOENT;
> > > +
> > > +	vp_modern_set_queue_reset(mdev, queue_index);
> > > +
> > > +	/* After write 1 to queue reset, the driver MUST wait for a read of
> > > +	 * queue reset to return 1.
> > > +	 */
> > > +	while (vp_modern_get_queue_reset(mdev, queue_index) != 1)
> > > +		msleep(1);
> > > +
> > > +	info = vp_dev->vqs[queue_index];
> > > +	msix_vec = info->msix_vector;
> > > +
> > > +	/* Disable VQ callback. */
> > > +	if (vp_dev->per_vq_vectors && msix_vec != VIRTIO_MSI_NO_VECTOR)
> > > +		disable_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec));
> > > +
> > > +	while ((buf = virtqueue_detach_unused_buf(info->vq)) != NULL)
> > > +		callback(vdev, buf, data);
> > > +
> > > +	vp_del_reset_vq(vdev, queue_index);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static struct virtqueue *vp_modern_enable_reset_vq(struct virtio_device *vdev,
> > > +						   u16 queue_index,
> > > +						   vq_callback_t *callback,
> > > +						   const char *name,
> > > +						   const bool *ctx)
> > > +{
> > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > +	struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> > > +	struct virtqueue *vq;
> > > +	u16 msix_vec;
> > > +
> > > +	if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
> > > +		return ERR_PTR(-ENOENT);
> > > +
> > > +	/* check queue reset status */
> > > +	if (vp_modern_get_queue_reset(mdev, queue_index) != 1)
> > > +		return ERR_PTR(-EBUSY);
> > > +
> > > +	vq = vp_enable_reset_vq(vdev, queue_index, callback, name, ctx);
> > > +	if (IS_ERR(vq))
> > > +		return vq;
> > > +
> > > +	vp_modern_set_queue_enable(&vp_dev->mdev, vq->index, true);
> > > +
> > > +	msix_vec = vp_dev->vqs[queue_index]->msix_vector;
> > > +	if (vp_dev->per_vq_vectors && msix_vec != VIRTIO_MSI_NO_VECTOR)
> > > +		enable_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec));
> > > +
> > > +	return vq;
> > > +}
> > > +
> > >  static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
> > >  {
> > >  	return vp_modern_config_vector(&vp_dev->mdev, vector);
> > > @@ -395,6 +464,8 @@ static const struct virtio_config_ops virtio_pci_config_nodev_ops = {
> > >  	.set_vq_affinity = vp_set_vq_affinity,
> > >  	.get_vq_affinity = vp_get_vq_affinity,
> > >  	.get_shm_region  = vp_get_shm_region,
> > > +	.reset_vq	 = vp_modern_reset_vq,
> > > +	.enable_reset_vq = vp_modern_enable_reset_vq,
> > >  };
> > >
> > >  static const struct virtio_config_ops virtio_pci_config_ops = {
> > > @@ -413,6 +484,8 @@ static const struct virtio_config_ops virtio_pci_config_ops = {
> > >  	.set_vq_affinity = vp_set_vq_affinity,
> > >  	.get_vq_affinity = vp_get_vq_affinity,
> > >  	.get_shm_region  = vp_get_shm_region,
> > > +	.reset_vq	 = vp_modern_reset_vq,
> > > +	.enable_reset_vq = vp_modern_enable_reset_vq,
> > >  };
> > >
> > >  /* the PCI probing function */
> > > --
> > > 2.31.0
> >

Re: [PATCH v2 07/12] virtio: queue_reset: pci: support VIRTIO_F_RING_RESET

[Michael S. Tsirkin]

On Fri, Jan 21, 2022 at 10:22:59AM +0800, Xuan Zhuo wrote:
> On Thu, 20 Jan 2022 10:03:45 -0500, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Thu, Jan 20, 2022 at 07:46:20PM +0800, Xuan Zhuo wrote:
> > > On Thu, 20 Jan 2022 05:55:14 -0500, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > On Thu, Jan 20, 2022 at 02:42:58PM +0800, Xuan Zhuo wrote:
> > > > > This patch implements virtio pci support for QUEUE RESET.
> > > > >
> > > > > Performing reset on a queue is divided into two steps:
> > > > >
> > > > > 1. reset_vq: reset one vq
> > > > > 2. enable_reset_vq: re-enable the reset queue
> > > > >
> > > > > In the first step, these tasks will be completed:
> > > > >    1. notify the hardware queue to reset
> > > > >    2. recycle the buffer from vq
> > > > >    3. delete the vq
> > > > >
> > > > > When deleting a vq, vp_del_vq() will be called to release all the memory
> > > > > of the vq. But this does not affect the process of deleting vqs, because
> > > > > that is based on the queue to release all the vqs. During this process,
> > > > > the vq has been removed from the queue.
> > > > >
> > > > > When deleting vq, info and vq will be released, and I save msix_vec in
> > > > > vp_dev->vqs[queue_index]. When re-enable, the current msix_vec can be
> > > > > reused. And based on intx_enabled to determine which method to use to
> > > > > enable this queue.
> > > > >
> > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > >
> > > > There's something I don't understand here. It looks like
> > > > you assume that when you reset a queue, you also
> > > > reset the mapping from queue to event vector.
> > > > The spec does not say it should, and I don't think it's
> > > > useful to extend spec to do it - we already have a simple
> > > > way to tweak the mapping.
> > > >
> > >
> > > Sorry, what is the already existing method you are referring to, I didn't find
> > > it.
> >
> >
> > Write 0xffff into vector number.
> 
> I wonder if there is some misunderstanding here.
> 
> My purpose is to release vq, then for the vector used by vq, I hope that it can
> be reused when re-enable.
> 
> But the vector number is not in a fixed order. When I re-enable it, I don't know
> what the original vector number is. So I found a place to save this number.
> 
> The queue reset I implemented is divided into the following steps:
> 	1. notify the driver to queue reset
> 	2. disable_irq()
> 	3. free unused bufs
> 	4. free irq, free vq, free info
> 
> The process of enable is divided into the following steps:
> 	1. Get the vector number used by the original vq and re-setup vq
> 	2. enable vq
> 	3. enable irq
> 
> If there is anything unreasonable please let me know.
> 
> Thanks.

Why do you free irq?

> >
> > > I think you mean that we don't have to reset the event vector, I think you are
> > > right.
> > >
> > >
> > >
> > > Thanks.
> > >
> > > > Avoid doing that, and things will be much easier, with no need
> > > > to interact with a transport, won't they?
> > > >
> > > >
> > > > > ---
> > > > >  drivers/virtio/virtio_pci_common.c | 49 ++++++++++++++++++++
> > > > >  drivers/virtio/virtio_pci_common.h |  4 ++
> > > > >  drivers/virtio/virtio_pci_modern.c | 73 ++++++++++++++++++++++++++++++
> > > > >  3 files changed, 126 insertions(+)
> > > > >
> > > > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> > > > > index 5afe207ce28a..28b5ffde4621 100644
> > > > > --- a/drivers/virtio/virtio_pci_common.c
> > > > > +++ b/drivers/virtio/virtio_pci_common.c
> > > > > @@ -464,6 +464,55 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> > > > >  	return vp_find_vqs_intx(vdev, nvqs, vqs, callbacks, names, ctx);
> > > > >  }
> > > > >
> > > > > +#define VQ_IS_DELETED(vp_dev, idx) ((unsigned long)vp_dev->vqs[idx] & 1)
> > > > > +#define VQ_RESET_MSIX_VEC(vp_dev, idx) ((unsigned long)vp_dev->vqs[idx] >> 2)
> > > > > +#define VQ_RESET_MARK(msix_vec) ((void *)(long)((msix_vec << 2) + 1))
> > > > > +
> > > > > +void vp_del_reset_vq(struct virtio_device *vdev, u16 queue_index)
> > > > > +{
> > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > +	struct virtio_pci_vq_info *info;
> > > > > +	u16 msix_vec;
> > > > > +
> > > > > +	info = vp_dev->vqs[queue_index];
> > > > > +
> > > > > +	msix_vec = info->msix_vector;
> > > > > +
> > > > > +	/* delete vq */
> > > > > +	vp_del_vq(info->vq);
> > > > > +
> > > > > +	/* Mark the vq has been deleted, and save the msix_vec. */
> > > > > +	vp_dev->vqs[queue_index] = VQ_RESET_MARK(msix_vec);
> > > > > +}
> > > > > +
> > > > > +struct virtqueue *vp_enable_reset_vq(struct virtio_device *vdev,
> > > > > +				     int queue_index,
> > > > > +				     vq_callback_t *callback,
> > > > > +				     const char *name,
> > > > > +				     const bool ctx)
> > > > > +{
> > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > +	struct virtqueue *vq;
> > > > > +	u16 msix_vec;
> > > > > +
> > > > > +	if (!VQ_IS_DELETED(vp_dev, queue_index))
> > > > > +		return ERR_PTR(-EPERM);
> > > > > +
> > > > > +	msix_vec = VQ_RESET_MSIX_VEC(vp_dev, queue_index);
> > > > > +
> > > > > +	if (vp_dev->intx_enabled)
> > > > > +		vq = vp_setup_vq(vdev, queue_index, callback, name, ctx,
> > > > > +				 VIRTIO_MSI_NO_VECTOR);
> > > > > +	else
> > > > > +		vq = vp_enable_vq_msix(vdev, queue_index, callback, name, ctx,
> > > > > +				       msix_vec);
> > > > > +
> > > > > +	if (IS_ERR(vq))
> > > > > +		vp_dev->vqs[queue_index] = VQ_RESET_MARK(msix_vec);
> > > > > +
> > > > > +	return vq;
> > > > > +}
> > > > > +
> > > > >  const char *vp_bus_name(struct virtio_device *vdev)
> > > > >  {
> > > > >  	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
> > > > > index 23f6c5c678d5..96c13b1398f8 100644
> > > > > --- a/drivers/virtio/virtio_pci_common.h
> > > > > +++ b/drivers/virtio/virtio_pci_common.h
> > > > > @@ -115,6 +115,10 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> > > > >  		struct virtqueue *vqs[], vq_callback_t *callbacks[],
> > > > >  		const char * const names[], const bool *ctx,
> > > > >  		struct irq_affinity *desc);
> > > > > +void vp_del_reset_vq(struct virtio_device *vdev, u16 queue_index);
> > > > > +struct virtqueue *vp_enable_reset_vq(struct virtio_device *vdev, int queue_index,
> > > > > +				     vq_callback_t *callback, const char *name,
> > > > > +				     const bool ctx);
> > > > >  const char *vp_bus_name(struct virtio_device *vdev);
> > > > >
> > > > >  /* Setup the affinity for a virtqueue:
> > > > > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> > > > > index 5455bc041fb6..fbf87239c920 100644
> > > > > --- a/drivers/virtio/virtio_pci_modern.c
> > > > > +++ b/drivers/virtio/virtio_pci_modern.c
> > > > > @@ -34,6 +34,9 @@ static void vp_transport_features(struct virtio_device *vdev, u64 features)
> > > > >  	if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> > > > >  			pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> > > > >  		__virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> > > > > +
> > > > > +	if (features & BIT_ULL(VIRTIO_F_RING_RESET))
> > > > > +		__virtio_set_bit(vdev, VIRTIO_F_RING_RESET);
> > > > >  }
> > > > >
> > > > >  /* virtio config->finalize_features() implementation */
> > > > > @@ -176,6 +179,72 @@ static void vp_reset(struct virtio_device *vdev)
> > > > >  	vp_disable_cbs(vdev);
> > > > >  }
> > > > >
> > > > > +static int vp_modern_reset_vq(struct virtio_device *vdev, u16 queue_index,
> > > > > +			      vq_reset_callback_t *callback, void *data)
> > > > > +{
> > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > +	struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> > > > > +	struct virtio_pci_vq_info *info;
> > > > > +	u16 msix_vec;
> > > > > +	void *buf;
> > > > > +
> > > > > +	if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
> > > > > +		return -ENOENT;
> > > > > +
> > > > > +	vp_modern_set_queue_reset(mdev, queue_index);
> > > > > +
> > > > > +	/* After write 1 to queue reset, the driver MUST wait for a read of
> > > > > +	 * queue reset to return 1.
> > > > > +	 */
> > > > > +	while (vp_modern_get_queue_reset(mdev, queue_index) != 1)
> > > > > +		msleep(1);
> > > > > +
> > > > > +	info = vp_dev->vqs[queue_index];
> > > > > +	msix_vec = info->msix_vector;
> > > > > +
> > > > > +	/* Disable VQ callback. */
> > > > > +	if (vp_dev->per_vq_vectors && msix_vec != VIRTIO_MSI_NO_VECTOR)
> > > > > +		disable_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec));
> > > > > +
> > > > > +	while ((buf = virtqueue_detach_unused_buf(info->vq)) != NULL)
> > > > > +		callback(vdev, buf, data);
> > > > > +
> > > > > +	vp_del_reset_vq(vdev, queue_index);
> > > > > +
> > > > > +	return 0;
> > > > > +}
> > > > > +
> > > > > +static struct virtqueue *vp_modern_enable_reset_vq(struct virtio_device *vdev,
> > > > > +						   u16 queue_index,
> > > > > +						   vq_callback_t *callback,
> > > > > +						   const char *name,
> > > > > +						   const bool *ctx)
> > > > > +{
> > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > +	struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> > > > > +	struct virtqueue *vq;
> > > > > +	u16 msix_vec;
> > > > > +
> > > > > +	if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
> > > > > +		return ERR_PTR(-ENOENT);
> > > > > +
> > > > > +	/* check queue reset status */
> > > > > +	if (vp_modern_get_queue_reset(mdev, queue_index) != 1)
> > > > > +		return ERR_PTR(-EBUSY);
> > > > > +
> > > > > +	vq = vp_enable_reset_vq(vdev, queue_index, callback, name, ctx);
> > > > > +	if (IS_ERR(vq))
> > > > > +		return vq;
> > > > > +
> > > > > +	vp_modern_set_queue_enable(&vp_dev->mdev, vq->index, true);
> > > > > +
> > > > > +	msix_vec = vp_dev->vqs[queue_index]->msix_vector;
> > > > > +	if (vp_dev->per_vq_vectors && msix_vec != VIRTIO_MSI_NO_VECTOR)
> > > > > +		enable_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec));
> > > > > +
> > > > > +	return vq;
> > > > > +}
> > > > > +
> > > > >  static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
> > > > >  {
> > > > >  	return vp_modern_config_vector(&vp_dev->mdev, vector);
> > > > > @@ -395,6 +464,8 @@ static const struct virtio_config_ops virtio_pci_config_nodev_ops = {
> > > > >  	.set_vq_affinity = vp_set_vq_affinity,
> > > > >  	.get_vq_affinity = vp_get_vq_affinity,
> > > > >  	.get_shm_region  = vp_get_shm_region,
> > > > > +	.reset_vq	 = vp_modern_reset_vq,
> > > > > +	.enable_reset_vq = vp_modern_enable_reset_vq,
> > > > >  };
> > > > >
> > > > >  static const struct virtio_config_ops virtio_pci_config_ops = {
> > > > > @@ -413,6 +484,8 @@ static const struct virtio_config_ops virtio_pci_config_ops = {
> > > > >  	.set_vq_affinity = vp_set_vq_affinity,
> > > > >  	.get_vq_affinity = vp_get_vq_affinity,
> > > > >  	.get_shm_region  = vp_get_shm_region,
> > > > > +	.reset_vq	 = vp_modern_reset_vq,
> > > > > +	.enable_reset_vq = vp_modern_enable_reset_vq,
> > > > >  };
> > > > >
> > > > >  /* the PCI probing function */
> > > > > --
> > > > > 2.31.0
> > > >
> >

Re: [PATCH v2 07/12] virtio: queue_reset: pci: support VIRTIO_F_RING_RESET

[Michael S. Tsirkin]

On Fri, Jan 21, 2022 at 05:22:29AM -0500, Michael S. Tsirkin wrote:
> On Fri, Jan 21, 2022 at 10:22:59AM +0800, Xuan Zhuo wrote:
> > On Thu, 20 Jan 2022 10:03:45 -0500, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > On Thu, Jan 20, 2022 at 07:46:20PM +0800, Xuan Zhuo wrote:
> > > > On Thu, 20 Jan 2022 05:55:14 -0500, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > > On Thu, Jan 20, 2022 at 02:42:58PM +0800, Xuan Zhuo wrote:
> > > > > > This patch implements virtio pci support for QUEUE RESET.
> > > > > >
> > > > > > Performing reset on a queue is divided into two steps:
> > > > > >
> > > > > > 1. reset_vq: reset one vq
> > > > > > 2. enable_reset_vq: re-enable the reset queue
> > > > > >
> > > > > > In the first step, these tasks will be completed:
> > > > > >    1. notify the hardware queue to reset
> > > > > >    2. recycle the buffer from vq
> > > > > >    3. delete the vq
> > > > > >
> > > > > > When deleting a vq, vp_del_vq() will be called to release all the memory
> > > > > > of the vq. But this does not affect the process of deleting vqs, because
> > > > > > that is based on the queue to release all the vqs. During this process,
> > > > > > the vq has been removed from the queue.
> > > > > >
> > > > > > When deleting vq, info and vq will be released, and I save msix_vec in
> > > > > > vp_dev->vqs[queue_index]. When re-enable, the current msix_vec can be
> > > > > > reused. And based on intx_enabled to determine which method to use to
> > > > > > enable this queue.
> > > > > >
> > > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > >
> > > > > There's something I don't understand here. It looks like
> > > > > you assume that when you reset a queue, you also
> > > > > reset the mapping from queue to event vector.
> > > > > The spec does not say it should, and I don't think it's
> > > > > useful to extend spec to do it - we already have a simple
> > > > > way to tweak the mapping.
> > > > >
> > > >
> > > > Sorry, what is the already existing method you are referring to, I didn't find
> > > > it.
> > >
> > >
> > > Write 0xffff into vector number.
> > 
> > I wonder if there is some misunderstanding here.
> > 
> > My purpose is to release vq, then for the vector used by vq, I hope that it can
> > be reused when re-enable.
> > 
> > But the vector number is not in a fixed order. When I re-enable it, I don't know
> > what the original vector number is. So I found a place to save this number.
> > 
> > The queue reset I implemented is divided into the following steps:
> > 	1. notify the driver to queue reset
> > 	2. disable_irq()
> > 	3. free unused bufs
> > 	4. free irq, free vq, free info

step 4 here seems pointless.

> > The process of enable is divided into the following steps:
> > 	1. Get the vector number used by the original vq and re-setup vq
> > 	2. enable vq
> > 	3. enable irq
> > 
> > If there is anything unreasonable please let me know.
> > 
> > Thanks.
> 
> Why do you free irq?
> 
> > >
> > > > I think you mean that we don't have to reset the event vector, I think you are
> > > > right.
> > > >
> > > >
> > > >
> > > > Thanks.
> > > >
> > > > > Avoid doing that, and things will be much easier, with no need
> > > > > to interact with a transport, won't they?
> > > > >
> > > > >
> > > > > > ---
> > > > > >  drivers/virtio/virtio_pci_common.c | 49 ++++++++++++++++++++
> > > > > >  drivers/virtio/virtio_pci_common.h |  4 ++
> > > > > >  drivers/virtio/virtio_pci_modern.c | 73 ++++++++++++++++++++++++++++++
> > > > > >  3 files changed, 126 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> > > > > > index 5afe207ce28a..28b5ffde4621 100644
> > > > > > --- a/drivers/virtio/virtio_pci_common.c
> > > > > > +++ b/drivers/virtio/virtio_pci_common.c
> > > > > > @@ -464,6 +464,55 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> > > > > >  	return vp_find_vqs_intx(vdev, nvqs, vqs, callbacks, names, ctx);
> > > > > >  }
> > > > > >
> > > > > > +#define VQ_IS_DELETED(vp_dev, idx) ((unsigned long)vp_dev->vqs[idx] & 1)
> > > > > > +#define VQ_RESET_MSIX_VEC(vp_dev, idx) ((unsigned long)vp_dev->vqs[idx] >> 2)
> > > > > > +#define VQ_RESET_MARK(msix_vec) ((void *)(long)((msix_vec << 2) + 1))
> > > > > > +
> > > > > > +void vp_del_reset_vq(struct virtio_device *vdev, u16 queue_index)
> > > > > > +{
> > > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > +	struct virtio_pci_vq_info *info;
> > > > > > +	u16 msix_vec;
> > > > > > +
> > > > > > +	info = vp_dev->vqs[queue_index];
> > > > > > +
> > > > > > +	msix_vec = info->msix_vector;
> > > > > > +
> > > > > > +	/* delete vq */
> > > > > > +	vp_del_vq(info->vq);
> > > > > > +
> > > > > > +	/* Mark the vq has been deleted, and save the msix_vec. */
> > > > > > +	vp_dev->vqs[queue_index] = VQ_RESET_MARK(msix_vec);
> > > > > > +}
> > > > > > +
> > > > > > +struct virtqueue *vp_enable_reset_vq(struct virtio_device *vdev,
> > > > > > +				     int queue_index,
> > > > > > +				     vq_callback_t *callback,
> > > > > > +				     const char *name,
> > > > > > +				     const bool ctx)
> > > > > > +{
> > > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > +	struct virtqueue *vq;
> > > > > > +	u16 msix_vec;
> > > > > > +
> > > > > > +	if (!VQ_IS_DELETED(vp_dev, queue_index))
> > > > > > +		return ERR_PTR(-EPERM);
> > > > > > +
> > > > > > +	msix_vec = VQ_RESET_MSIX_VEC(vp_dev, queue_index);
> > > > > > +
> > > > > > +	if (vp_dev->intx_enabled)
> > > > > > +		vq = vp_setup_vq(vdev, queue_index, callback, name, ctx,
> > > > > > +				 VIRTIO_MSI_NO_VECTOR);
> > > > > > +	else
> > > > > > +		vq = vp_enable_vq_msix(vdev, queue_index, callback, name, ctx,
> > > > > > +				       msix_vec);
> > > > > > +
> > > > > > +	if (IS_ERR(vq))
> > > > > > +		vp_dev->vqs[queue_index] = VQ_RESET_MARK(msix_vec);
> > > > > > +
> > > > > > +	return vq;
> > > > > > +}
> > > > > > +
> > > > > >  const char *vp_bus_name(struct virtio_device *vdev)
> > > > > >  {
> > > > > >  	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
> > > > > > index 23f6c5c678d5..96c13b1398f8 100644
> > > > > > --- a/drivers/virtio/virtio_pci_common.h
> > > > > > +++ b/drivers/virtio/virtio_pci_common.h
> > > > > > @@ -115,6 +115,10 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> > > > > >  		struct virtqueue *vqs[], vq_callback_t *callbacks[],
> > > > > >  		const char * const names[], const bool *ctx,
> > > > > >  		struct irq_affinity *desc);
> > > > > > +void vp_del_reset_vq(struct virtio_device *vdev, u16 queue_index);
> > > > > > +struct virtqueue *vp_enable_reset_vq(struct virtio_device *vdev, int queue_index,
> > > > > > +				     vq_callback_t *callback, const char *name,
> > > > > > +				     const bool ctx);
> > > > > >  const char *vp_bus_name(struct virtio_device *vdev);
> > > > > >
> > > > > >  /* Setup the affinity for a virtqueue:
> > > > > > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> > > > > > index 5455bc041fb6..fbf87239c920 100644
> > > > > > --- a/drivers/virtio/virtio_pci_modern.c
> > > > > > +++ b/drivers/virtio/virtio_pci_modern.c
> > > > > > @@ -34,6 +34,9 @@ static void vp_transport_features(struct virtio_device *vdev, u64 features)
> > > > > >  	if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> > > > > >  			pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> > > > > >  		__virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> > > > > > +
> > > > > > +	if (features & BIT_ULL(VIRTIO_F_RING_RESET))
> > > > > > +		__virtio_set_bit(vdev, VIRTIO_F_RING_RESET);
> > > > > >  }
> > > > > >
> > > > > >  /* virtio config->finalize_features() implementation */
> > > > > > @@ -176,6 +179,72 @@ static void vp_reset(struct virtio_device *vdev)
> > > > > >  	vp_disable_cbs(vdev);
> > > > > >  }
> > > > > >
> > > > > > +static int vp_modern_reset_vq(struct virtio_device *vdev, u16 queue_index,
> > > > > > +			      vq_reset_callback_t *callback, void *data)
> > > > > > +{
> > > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > +	struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> > > > > > +	struct virtio_pci_vq_info *info;
> > > > > > +	u16 msix_vec;
> > > > > > +	void *buf;
> > > > > > +
> > > > > > +	if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
> > > > > > +		return -ENOENT;
> > > > > > +
> > > > > > +	vp_modern_set_queue_reset(mdev, queue_index);
> > > > > > +
> > > > > > +	/* After write 1 to queue reset, the driver MUST wait for a read of
> > > > > > +	 * queue reset to return 1.
> > > > > > +	 */
> > > > > > +	while (vp_modern_get_queue_reset(mdev, queue_index) != 1)
> > > > > > +		msleep(1);
> > > > > > +
> > > > > > +	info = vp_dev->vqs[queue_index];
> > > > > > +	msix_vec = info->msix_vector;
> > > > > > +
> > > > > > +	/* Disable VQ callback. */
> > > > > > +	if (vp_dev->per_vq_vectors && msix_vec != VIRTIO_MSI_NO_VECTOR)
> > > > > > +		disable_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec));
> > > > > > +
> > > > > > +	while ((buf = virtqueue_detach_unused_buf(info->vq)) != NULL)
> > > > > > +		callback(vdev, buf, data);
> > > > > > +
> > > > > > +	vp_del_reset_vq(vdev, queue_index);
> > > > > > +
> > > > > > +	return 0;
> > > > > > +}
> > > > > > +
> > > > > > +static struct virtqueue *vp_modern_enable_reset_vq(struct virtio_device *vdev,
> > > > > > +						   u16 queue_index,
> > > > > > +						   vq_callback_t *callback,
> > > > > > +						   const char *name,
> > > > > > +						   const bool *ctx)
> > > > > > +{
> > > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > +	struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> > > > > > +	struct virtqueue *vq;
> > > > > > +	u16 msix_vec;
> > > > > > +
> > > > > > +	if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
> > > > > > +		return ERR_PTR(-ENOENT);
> > > > > > +
> > > > > > +	/* check queue reset status */
> > > > > > +	if (vp_modern_get_queue_reset(mdev, queue_index) != 1)
> > > > > > +		return ERR_PTR(-EBUSY);
> > > > > > +
> > > > > > +	vq = vp_enable_reset_vq(vdev, queue_index, callback, name, ctx);
> > > > > > +	if (IS_ERR(vq))
> > > > > > +		return vq;
> > > > > > +
> > > > > > +	vp_modern_set_queue_enable(&vp_dev->mdev, vq->index, true);
> > > > > > +
> > > > > > +	msix_vec = vp_dev->vqs[queue_index]->msix_vector;
> > > > > > +	if (vp_dev->per_vq_vectors && msix_vec != VIRTIO_MSI_NO_VECTOR)
> > > > > > +		enable_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec));
> > > > > > +
> > > > > > +	return vq;
> > > > > > +}
> > > > > > +
> > > > > >  static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
> > > > > >  {
> > > > > >  	return vp_modern_config_vector(&vp_dev->mdev, vector);
> > > > > > @@ -395,6 +464,8 @@ static const struct virtio_config_ops virtio_pci_config_nodev_ops = {
> > > > > >  	.set_vq_affinity = vp_set_vq_affinity,
> > > > > >  	.get_vq_affinity = vp_get_vq_affinity,
> > > > > >  	.get_shm_region  = vp_get_shm_region,
> > > > > > +	.reset_vq	 = vp_modern_reset_vq,
> > > > > > +	.enable_reset_vq = vp_modern_enable_reset_vq,
> > > > > >  };
> > > > > >
> > > > > >  static const struct virtio_config_ops virtio_pci_config_ops = {
> > > > > > @@ -413,6 +484,8 @@ static const struct virtio_config_ops virtio_pci_config_ops = {
> > > > > >  	.set_vq_affinity = vp_set_vq_affinity,
> > > > > >  	.get_vq_affinity = vp_get_vq_affinity,
> > > > > >  	.get_shm_region  = vp_get_shm_region,
> > > > > > +	.reset_vq	 = vp_modern_reset_vq,
> > > > > > +	.enable_reset_vq = vp_modern_enable_reset_vq,
> > > > > >  };
> > > > > >
> > > > > >  /* the PCI probing function */
> > > > > > --
> > > > > > 2.31.0
> > > > >
> > >

Re: [PATCH v2 07/12] virtio: queue_reset: pci: support VIRTIO_F_RING_RESET

[Michael S. Tsirkin]

On Fri, Jan 21, 2022 at 06:26:33PM +0800, Xuan Zhuo wrote:
> On Fri, 21 Jan 2022 05:22:59 -0500, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Fri, Jan 21, 2022 at 05:22:29AM -0500, Michael S. Tsirkin wrote:
> > > On Fri, Jan 21, 2022 at 10:22:59AM +0800, Xuan Zhuo wrote:
> > > > On Thu, 20 Jan 2022 10:03:45 -0500, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > > On Thu, Jan 20, 2022 at 07:46:20PM +0800, Xuan Zhuo wrote:
> > > > > > On Thu, 20 Jan 2022 05:55:14 -0500, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > > > > On Thu, Jan 20, 2022 at 02:42:58PM +0800, Xuan Zhuo wrote:
> > > > > > > > This patch implements virtio pci support for QUEUE RESET.
> > > > > > > >
> > > > > > > > Performing reset on a queue is divided into two steps:
> > > > > > > >
> > > > > > > > 1. reset_vq: reset one vq
> > > > > > > > 2. enable_reset_vq: re-enable the reset queue
> > > > > > > >
> > > > > > > > In the first step, these tasks will be completed:
> > > > > > > >    1. notify the hardware queue to reset
> > > > > > > >    2. recycle the buffer from vq
> > > > > > > >    3. delete the vq
> > > > > > > >
> > > > > > > > When deleting a vq, vp_del_vq() will be called to release all the memory
> > > > > > > > of the vq. But this does not affect the process of deleting vqs, because
> > > > > > > > that is based on the queue to release all the vqs. During this process,
> > > > > > > > the vq has been removed from the queue.
> > > > > > > >
> > > > > > > > When deleting vq, info and vq will be released, and I save msix_vec in
> > > > > > > > vp_dev->vqs[queue_index]. When re-enable, the current msix_vec can be
> > > > > > > > reused. And based on intx_enabled to determine which method to use to
> > > > > > > > enable this queue.
> > > > > > > >
> > > > > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > > > >
> > > > > > > There's something I don't understand here. It looks like
> > > > > > > you assume that when you reset a queue, you also
> > > > > > > reset the mapping from queue to event vector.
> > > > > > > The spec does not say it should, and I don't think it's
> > > > > > > useful to extend spec to do it - we already have a simple
> > > > > > > way to tweak the mapping.
> > > > > > >
> > > > > >
> > > > > > Sorry, what is the already existing method you are referring to, I didn't find
> > > > > > it.
> > > > >
> > > > >
> > > > > Write 0xffff into vector number.
> > > >
> > > > I wonder if there is some misunderstanding here.
> > > >
> > > > My purpose is to release vq, then for the vector used by vq, I hope that it can
> > > > be reused when re-enable.
> > > >
> > > > But the vector number is not in a fixed order. When I re-enable it, I don't know
> > > > what the original vector number is. So I found a place to save this number.
> > > >
> > > > The queue reset I implemented is divided into the following steps:
> > > > 	1. notify the driver to queue reset
> > > > 	2. disable_irq()
> > > > 	3. free unused bufs
> > > > 	4. free irq, free vq, free info
> >
> > step 4 here seems pointless.
> 
> 
> The core operation is to release the vq. The release of the irq is indeed by the
> way. We can leave the irq untouched.
> 
> 	1. notify the driver to queue reset
> 	2. disable_irq()
> 	3. free unused bufs
> 	4. free vq, free info
> 
> Thanks.

OK. why free the vq and info though?

> >
> > > > The process of enable is divided into the following steps:
> > > > 	1. Get the vector number used by the original vq and re-setup vq
> > > > 	2. enable vq
> > > > 	3. enable irq
> > > >
> > > > If there is anything unreasonable please let me know.
> > > >
> > > > Thanks.
> > >
> > > Why do you free irq?
> > >
> > > > >
> > > > > > I think you mean that we don't have to reset the event vector, I think you are
> > > > > > right.
> > > > > >
> > > > > >
> > > > > >
> > > > > > Thanks.
> > > > > >
> > > > > > > Avoid doing that, and things will be much easier, with no need
> > > > > > > to interact with a transport, won't they?
> > > > > > >
> > > > > > >
> > > > > > > > ---
> > > > > > > >  drivers/virtio/virtio_pci_common.c | 49 ++++++++++++++++++++
> > > > > > > >  drivers/virtio/virtio_pci_common.h |  4 ++
> > > > > > > >  drivers/virtio/virtio_pci_modern.c | 73 ++++++++++++++++++++++++++++++
> > > > > > > >  3 files changed, 126 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> > > > > > > > index 5afe207ce28a..28b5ffde4621 100644
> > > > > > > > --- a/drivers/virtio/virtio_pci_common.c
> > > > > > > > +++ b/drivers/virtio/virtio_pci_common.c
> > > > > > > > @@ -464,6 +464,55 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> > > > > > > >  	return vp_find_vqs_intx(vdev, nvqs, vqs, callbacks, names, ctx);
> > > > > > > >  }
> > > > > > > >
> > > > > > > > +#define VQ_IS_DELETED(vp_dev, idx) ((unsigned long)vp_dev->vqs[idx] & 1)
> > > > > > > > +#define VQ_RESET_MSIX_VEC(vp_dev, idx) ((unsigned long)vp_dev->vqs[idx] >> 2)
> > > > > > > > +#define VQ_RESET_MARK(msix_vec) ((void *)(long)((msix_vec << 2) + 1))
> > > > > > > > +
> > > > > > > > +void vp_del_reset_vq(struct virtio_device *vdev, u16 queue_index)
> > > > > > > > +{
> > > > > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > > > +	struct virtio_pci_vq_info *info;
> > > > > > > > +	u16 msix_vec;
> > > > > > > > +
> > > > > > > > +	info = vp_dev->vqs[queue_index];
> > > > > > > > +
> > > > > > > > +	msix_vec = info->msix_vector;
> > > > > > > > +
> > > > > > > > +	/* delete vq */
> > > > > > > > +	vp_del_vq(info->vq);
> > > > > > > > +
> > > > > > > > +	/* Mark the vq has been deleted, and save the msix_vec. */
> > > > > > > > +	vp_dev->vqs[queue_index] = VQ_RESET_MARK(msix_vec);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +struct virtqueue *vp_enable_reset_vq(struct virtio_device *vdev,
> > > > > > > > +				     int queue_index,
> > > > > > > > +				     vq_callback_t *callback,
> > > > > > > > +				     const char *name,
> > > > > > > > +				     const bool ctx)
> > > > > > > > +{
> > > > > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > > > +	struct virtqueue *vq;
> > > > > > > > +	u16 msix_vec;
> > > > > > > > +
> > > > > > > > +	if (!VQ_IS_DELETED(vp_dev, queue_index))
> > > > > > > > +		return ERR_PTR(-EPERM);
> > > > > > > > +
> > > > > > > > +	msix_vec = VQ_RESET_MSIX_VEC(vp_dev, queue_index);
> > > > > > > > +
> > > > > > > > +	if (vp_dev->intx_enabled)
> > > > > > > > +		vq = vp_setup_vq(vdev, queue_index, callback, name, ctx,
> > > > > > > > +				 VIRTIO_MSI_NO_VECTOR);
> > > > > > > > +	else
> > > > > > > > +		vq = vp_enable_vq_msix(vdev, queue_index, callback, name, ctx,
> > > > > > > > +				       msix_vec);
> > > > > > > > +
> > > > > > > > +	if (IS_ERR(vq))
> > > > > > > > +		vp_dev->vqs[queue_index] = VQ_RESET_MARK(msix_vec);
> > > > > > > > +
> > > > > > > > +	return vq;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > >  const char *vp_bus_name(struct virtio_device *vdev)
> > > > > > > >  {
> > > > > > > >  	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > > > diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
> > > > > > > > index 23f6c5c678d5..96c13b1398f8 100644
> > > > > > > > --- a/drivers/virtio/virtio_pci_common.h
> > > > > > > > +++ b/drivers/virtio/virtio_pci_common.h
> > > > > > > > @@ -115,6 +115,10 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> > > > > > > >  		struct virtqueue *vqs[], vq_callback_t *callbacks[],
> > > > > > > >  		const char * const names[], const bool *ctx,
> > > > > > > >  		struct irq_affinity *desc);
> > > > > > > > +void vp_del_reset_vq(struct virtio_device *vdev, u16 queue_index);
> > > > > > > > +struct virtqueue *vp_enable_reset_vq(struct virtio_device *vdev, int queue_index,
> > > > > > > > +				     vq_callback_t *callback, const char *name,
> > > > > > > > +				     const bool ctx);
> > > > > > > >  const char *vp_bus_name(struct virtio_device *vdev);
> > > > > > > >
> > > > > > > >  /* Setup the affinity for a virtqueue:
> > > > > > > > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> > > > > > > > index 5455bc041fb6..fbf87239c920 100644
> > > > > > > > --- a/drivers/virtio/virtio_pci_modern.c
> > > > > > > > +++ b/drivers/virtio/virtio_pci_modern.c
> > > > > > > > @@ -34,6 +34,9 @@ static void vp_transport_features(struct virtio_device *vdev, u64 features)
> > > > > > > >  	if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> > > > > > > >  			pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> > > > > > > >  		__virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> > > > > > > > +
> > > > > > > > +	if (features & BIT_ULL(VIRTIO_F_RING_RESET))
> > > > > > > > +		__virtio_set_bit(vdev, VIRTIO_F_RING_RESET);
> > > > > > > >  }
> > > > > > > >
> > > > > > > >  /* virtio config->finalize_features() implementation */
> > > > > > > > @@ -176,6 +179,72 @@ static void vp_reset(struct virtio_device *vdev)
> > > > > > > >  	vp_disable_cbs(vdev);
> > > > > > > >  }
> > > > > > > >
> > > > > > > > +static int vp_modern_reset_vq(struct virtio_device *vdev, u16 queue_index,
> > > > > > > > +			      vq_reset_callback_t *callback, void *data)
> > > > > > > > +{
> > > > > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > > > +	struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> > > > > > > > +	struct virtio_pci_vq_info *info;
> > > > > > > > +	u16 msix_vec;
> > > > > > > > +	void *buf;
> > > > > > > > +
> > > > > > > > +	if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
> > > > > > > > +		return -ENOENT;
> > > > > > > > +
> > > > > > > > +	vp_modern_set_queue_reset(mdev, queue_index);
> > > > > > > > +
> > > > > > > > +	/* After write 1 to queue reset, the driver MUST wait for a read of
> > > > > > > > +	 * queue reset to return 1.
> > > > > > > > +	 */
> > > > > > > > +	while (vp_modern_get_queue_reset(mdev, queue_index) != 1)
> > > > > > > > +		msleep(1);
> > > > > > > > +
> > > > > > > > +	info = vp_dev->vqs[queue_index];
> > > > > > > > +	msix_vec = info->msix_vector;
> > > > > > > > +
> > > > > > > > +	/* Disable VQ callback. */
> > > > > > > > +	if (vp_dev->per_vq_vectors && msix_vec != VIRTIO_MSI_NO_VECTOR)
> > > > > > > > +		disable_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec));
> > > > > > > > +
> > > > > > > > +	while ((buf = virtqueue_detach_unused_buf(info->vq)) != NULL)
> > > > > > > > +		callback(vdev, buf, data);
> > > > > > > > +
> > > > > > > > +	vp_del_reset_vq(vdev, queue_index);
> > > > > > > > +
> > > > > > > > +	return 0;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static struct virtqueue *vp_modern_enable_reset_vq(struct virtio_device *vdev,
> > > > > > > > +						   u16 queue_index,
> > > > > > > > +						   vq_callback_t *callback,
> > > > > > > > +						   const char *name,
> > > > > > > > +						   const bool *ctx)
> > > > > > > > +{
> > > > > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > > > +	struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> > > > > > > > +	struct virtqueue *vq;
> > > > > > > > +	u16 msix_vec;
> > > > > > > > +
> > > > > > > > +	if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
> > > > > > > > +		return ERR_PTR(-ENOENT);
> > > > > > > > +
> > > > > > > > +	/* check queue reset status */
> > > > > > > > +	if (vp_modern_get_queue_reset(mdev, queue_index) != 1)
> > > > > > > > +		return ERR_PTR(-EBUSY);
> > > > > > > > +
> > > > > > > > +	vq = vp_enable_reset_vq(vdev, queue_index, callback, name, ctx);
> > > > > > > > +	if (IS_ERR(vq))
> > > > > > > > +		return vq;
> > > > > > > > +
> > > > > > > > +	vp_modern_set_queue_enable(&vp_dev->mdev, vq->index, true);
> > > > > > > > +
> > > > > > > > +	msix_vec = vp_dev->vqs[queue_index]->msix_vector;
> > > > > > > > +	if (vp_dev->per_vq_vectors && msix_vec != VIRTIO_MSI_NO_VECTOR)
> > > > > > > > +		enable_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec));
> > > > > > > > +
> > > > > > > > +	return vq;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > >  static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
> > > > > > > >  {
> > > > > > > >  	return vp_modern_config_vector(&vp_dev->mdev, vector);
> > > > > > > > @@ -395,6 +464,8 @@ static const struct virtio_config_ops virtio_pci_config_nodev_ops = {
> > > > > > > >  	.set_vq_affinity = vp_set_vq_affinity,
> > > > > > > >  	.get_vq_affinity = vp_get_vq_affinity,
> > > > > > > >  	.get_shm_region  = vp_get_shm_region,
> > > > > > > > +	.reset_vq	 = vp_modern_reset_vq,
> > > > > > > > +	.enable_reset_vq = vp_modern_enable_reset_vq,
> > > > > > > >  };
> > > > > > > >
> > > > > > > >  static const struct virtio_config_ops virtio_pci_config_ops = {
> > > > > > > > @@ -413,6 +484,8 @@ static const struct virtio_config_ops virtio_pci_config_ops = {
> > > > > > > >  	.set_vq_affinity = vp_set_vq_affinity,
> > > > > > > >  	.get_vq_affinity = vp_get_vq_affinity,
> > > > > > > >  	.get_shm_region  = vp_get_shm_region,
> > > > > > > > +	.reset_vq	 = vp_modern_reset_vq,
> > > > > > > > +	.enable_reset_vq = vp_modern_enable_reset_vq,
> > > > > > > >  };
> > > > > > > >
> > > > > > > >  /* the PCI probing function */
> > > > > > > > --
> > > > > > > > 2.31.0
> > > > > > >
> > > > >
> >

Re: [PATCH v2 07/12] virtio: queue_reset: pci: support VIRTIO_F_RING_RESET

[Michael S. Tsirkin]

On Fri, Jan 21, 2022 at 10:09:31PM +0800, Xuan Zhuo wrote:
> On Fri, 21 Jan 2022 08:19:55 -0500, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Fri, Jan 21, 2022 at 06:26:33PM +0800, Xuan Zhuo wrote:
> > > On Fri, 21 Jan 2022 05:22:59 -0500, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > On Fri, Jan 21, 2022 at 05:22:29AM -0500, Michael S. Tsirkin wrote:
> > > > > On Fri, Jan 21, 2022 at 10:22:59AM +0800, Xuan Zhuo wrote:
> > > > > > On Thu, 20 Jan 2022 10:03:45 -0500, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > > > > On Thu, Jan 20, 2022 at 07:46:20PM +0800, Xuan Zhuo wrote:
> > > > > > > > On Thu, 20 Jan 2022 05:55:14 -0500, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > > > > > > On Thu, Jan 20, 2022 at 02:42:58PM +0800, Xuan Zhuo wrote:
> > > > > > > > > > This patch implements virtio pci support for QUEUE RESET.
> > > > > > > > > >
> > > > > > > > > > Performing reset on a queue is divided into two steps:
> > > > > > > > > >
> > > > > > > > > > 1. reset_vq: reset one vq
> > > > > > > > > > 2. enable_reset_vq: re-enable the reset queue
> > > > > > > > > >
> > > > > > > > > > In the first step, these tasks will be completed:
> > > > > > > > > >    1. notify the hardware queue to reset
> > > > > > > > > >    2. recycle the buffer from vq
> > > > > > > > > >    3. delete the vq
> > > > > > > > > >
> > > > > > > > > > When deleting a vq, vp_del_vq() will be called to release all the memory
> > > > > > > > > > of the vq. But this does not affect the process of deleting vqs, because
> > > > > > > > > > that is based on the queue to release all the vqs. During this process,
> > > > > > > > > > the vq has been removed from the queue.
> > > > > > > > > >
> > > > > > > > > > When deleting vq, info and vq will be released, and I save msix_vec in
> > > > > > > > > > vp_dev->vqs[queue_index]. When re-enable, the current msix_vec can be
> > > > > > > > > > reused. And based on intx_enabled to determine which method to use to
> > > > > > > > > > enable this queue.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > > > > > >
> > > > > > > > > There's something I don't understand here. It looks like
> > > > > > > > > you assume that when you reset a queue, you also
> > > > > > > > > reset the mapping from queue to event vector.
> > > > > > > > > The spec does not say it should, and I don't think it's
> > > > > > > > > useful to extend spec to do it - we already have a simple
> > > > > > > > > way to tweak the mapping.
> > > > > > > > >
> > > > > > > >
> > > > > > > > Sorry, what is the already existing method you are referring to, I didn't find
> > > > > > > > it.
> > > > > > >
> > > > > > >
> > > > > > > Write 0xffff into vector number.
> > > > > >
> > > > > > I wonder if there is some misunderstanding here.
> > > > > >
> > > > > > My purpose is to release vq, then for the vector used by vq, I hope that it can
> > > > > > be reused when re-enable.
> > > > > >
> > > > > > But the vector number is not in a fixed order. When I re-enable it, I don't know
> > > > > > what the original vector number is. So I found a place to save this number.
> > > > > >
> > > > > > The queue reset I implemented is divided into the following steps:
> > > > > > 	1. notify the driver to queue reset
> > > > > > 	2. disable_irq()
> > > > > > 	3. free unused bufs
> > > > > > 	4. free irq, free vq, free info
> > > >
> > > > step 4 here seems pointless.
> > >
> > >
> > > The core operation is to release the vq. The release of the irq is indeed by the
> > > way. We can leave the irq untouched.
> > >
> > > 	1. notify the driver to queue reset
> > > 	2. disable_irq()
> > > 	3. free unused bufs
> > > 	4. free vq, free info
> > >
> > > Thanks.
> >
> > OK. why free the vq and info though?
> 
> 
> I guess what you mean may be that we do not release the memory of vq, but
> reinitialize it and map it to the backend. In this way, the ring size of vq is
> also unchanged.
> 
> +After the queue has been successfully reset, the driver MAY release any
> +resource associated with that virtqueue.
> 
> [...]
> 
> +
> +\devicenormative{\paragraph}{Virtqueue Re-enable}{Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset / Virtqueue Re-enable}
> +
> +The device MUST observe any queue configuration that may have been
> +changed by the driver, like the maximum queue size.
> +
> +\drivernormative{\paragraph}{Virtqueue Re-enable}{Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset / Virtqueue Re-enable}
> +
> +When re-enabling a queue, the driver MUST configure the queue resources
> +as during initial virtqueue discovery, but optionally with different
> +parameters.
> +
> 
> Based on the above spec definition, reset can modify, for example, the length of
> the queue.  So I chose to release memory here. Of course, I have not yet
> supported the modification of ring size.
> 
> I wonder if we can provide two methods when implementing queue reset:
> 
> 	1. Lightweight reset, do not release vq memory, re-initialize vq. Then
>        	map to the backend (virtio-net supports AF_XDP, this can be satisfied)
> 	2. Release vq to support modification of ring size. (virtio-net can
> 	modify ring size based on this)
> 
> Thanks.

I'm not sure I understand completely. I guess the simpler the
reset code the better so ... 1 is simpler I guess?

> 
> 
> >
> > > >
> > > > > > The process of enable is divided into the following steps:
> > > > > > 	1. Get the vector number used by the original vq and re-setup vq
> > > > > > 	2. enable vq
> > > > > > 	3. enable irq
> > > > > >
> > > > > > If there is anything unreasonable please let me know.
> > > > > >
> > > > > > Thanks.
> > > > >
> > > > > Why do you free irq?
> > > > >
> > > > > > >
> > > > > > > > I think you mean that we don't have to reset the event vector, I think you are
> > > > > > > > right.
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > Thanks.
> > > > > > > >
> > > > > > > > > Avoid doing that, and things will be much easier, with no need
> > > > > > > > > to interact with a transport, won't they?
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > > ---
> > > > > > > > > >  drivers/virtio/virtio_pci_common.c | 49 ++++++++++++++++++++
> > > > > > > > > >  drivers/virtio/virtio_pci_common.h |  4 ++
> > > > > > > > > >  drivers/virtio/virtio_pci_modern.c | 73 ++++++++++++++++++++++++++++++
> > > > > > > > > >  3 files changed, 126 insertions(+)
> > > > > > > > > >
> > > > > > > > > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> > > > > > > > > > index 5afe207ce28a..28b5ffde4621 100644
> > > > > > > > > > --- a/drivers/virtio/virtio_pci_common.c
> > > > > > > > > > +++ b/drivers/virtio/virtio_pci_common.c
> > > > > > > > > > @@ -464,6 +464,55 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> > > > > > > > > >  	return vp_find_vqs_intx(vdev, nvqs, vqs, callbacks, names, ctx);
> > > > > > > > > >  }
> > > > > > > > > >
> > > > > > > > > > +#define VQ_IS_DELETED(vp_dev, idx) ((unsigned long)vp_dev->vqs[idx] & 1)
> > > > > > > > > > +#define VQ_RESET_MSIX_VEC(vp_dev, idx) ((unsigned long)vp_dev->vqs[idx] >> 2)
> > > > > > > > > > +#define VQ_RESET_MARK(msix_vec) ((void *)(long)((msix_vec << 2) + 1))
> > > > > > > > > > +
> > > > > > > > > > +void vp_del_reset_vq(struct virtio_device *vdev, u16 queue_index)
> > > > > > > > > > +{
> > > > > > > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > > > > > +	struct virtio_pci_vq_info *info;
> > > > > > > > > > +	u16 msix_vec;
> > > > > > > > > > +
> > > > > > > > > > +	info = vp_dev->vqs[queue_index];
> > > > > > > > > > +
> > > > > > > > > > +	msix_vec = info->msix_vector;
> > > > > > > > > > +
> > > > > > > > > > +	/* delete vq */
> > > > > > > > > > +	vp_del_vq(info->vq);
> > > > > > > > > > +
> > > > > > > > > > +	/* Mark the vq has been deleted, and save the msix_vec. */
> > > > > > > > > > +	vp_dev->vqs[queue_index] = VQ_RESET_MARK(msix_vec);
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +struct virtqueue *vp_enable_reset_vq(struct virtio_device *vdev,
> > > > > > > > > > +				     int queue_index,
> > > > > > > > > > +				     vq_callback_t *callback,
> > > > > > > > > > +				     const char *name,
> > > > > > > > > > +				     const bool ctx)
> > > > > > > > > > +{
> > > > > > > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > > > > > +	struct virtqueue *vq;
> > > > > > > > > > +	u16 msix_vec;
> > > > > > > > > > +
> > > > > > > > > > +	if (!VQ_IS_DELETED(vp_dev, queue_index))
> > > > > > > > > > +		return ERR_PTR(-EPERM);
> > > > > > > > > > +
> > > > > > > > > > +	msix_vec = VQ_RESET_MSIX_VEC(vp_dev, queue_index);
> > > > > > > > > > +
> > > > > > > > > > +	if (vp_dev->intx_enabled)
> > > > > > > > > > +		vq = vp_setup_vq(vdev, queue_index, callback, name, ctx,
> > > > > > > > > > +				 VIRTIO_MSI_NO_VECTOR);
> > > > > > > > > > +	else
> > > > > > > > > > +		vq = vp_enable_vq_msix(vdev, queue_index, callback, name, ctx,
> > > > > > > > > > +				       msix_vec);
> > > > > > > > > > +
> > > > > > > > > > +	if (IS_ERR(vq))
> > > > > > > > > > +		vp_dev->vqs[queue_index] = VQ_RESET_MARK(msix_vec);
> > > > > > > > > > +
> > > > > > > > > > +	return vq;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > >  const char *vp_bus_name(struct virtio_device *vdev)
> > > > > > > > > >  {
> > > > > > > > > >  	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > > > > > diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
> > > > > > > > > > index 23f6c5c678d5..96c13b1398f8 100644
> > > > > > > > > > --- a/drivers/virtio/virtio_pci_common.h
> > > > > > > > > > +++ b/drivers/virtio/virtio_pci_common.h
> > > > > > > > > > @@ -115,6 +115,10 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> > > > > > > > > >  		struct virtqueue *vqs[], vq_callback_t *callbacks[],
> > > > > > > > > >  		const char * const names[], const bool *ctx,
> > > > > > > > > >  		struct irq_affinity *desc);
> > > > > > > > > > +void vp_del_reset_vq(struct virtio_device *vdev, u16 queue_index);
> > > > > > > > > > +struct virtqueue *vp_enable_reset_vq(struct virtio_device *vdev, int queue_index,
> > > > > > > > > > +				     vq_callback_t *callback, const char *name,
> > > > > > > > > > +				     const bool ctx);
> > > > > > > > > >  const char *vp_bus_name(struct virtio_device *vdev);
> > > > > > > > > >
> > > > > > > > > >  /* Setup the affinity for a virtqueue:
> > > > > > > > > > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> > > > > > > > > > index 5455bc041fb6..fbf87239c920 100644
> > > > > > > > > > --- a/drivers/virtio/virtio_pci_modern.c
> > > > > > > > > > +++ b/drivers/virtio/virtio_pci_modern.c
> > > > > > > > > > @@ -34,6 +34,9 @@ static void vp_transport_features(struct virtio_device *vdev, u64 features)
> > > > > > > > > >  	if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> > > > > > > > > >  			pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> > > > > > > > > >  		__virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> > > > > > > > > > +
> > > > > > > > > > +	if (features & BIT_ULL(VIRTIO_F_RING_RESET))
> > > > > > > > > > +		__virtio_set_bit(vdev, VIRTIO_F_RING_RESET);
> > > > > > > > > >  }
> > > > > > > > > >
> > > > > > > > > >  /* virtio config->finalize_features() implementation */
> > > > > > > > > > @@ -176,6 +179,72 @@ static void vp_reset(struct virtio_device *vdev)
> > > > > > > > > >  	vp_disable_cbs(vdev);
> > > > > > > > > >  }
> > > > > > > > > >
> > > > > > > > > > +static int vp_modern_reset_vq(struct virtio_device *vdev, u16 queue_index,
> > > > > > > > > > +			      vq_reset_callback_t *callback, void *data)
> > > > > > > > > > +{
> > > > > > > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > > > > > +	struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> > > > > > > > > > +	struct virtio_pci_vq_info *info;
> > > > > > > > > > +	u16 msix_vec;
> > > > > > > > > > +	void *buf;
> > > > > > > > > > +
> > > > > > > > > > +	if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
> > > > > > > > > > +		return -ENOENT;
> > > > > > > > > > +
> > > > > > > > > > +	vp_modern_set_queue_reset(mdev, queue_index);
> > > > > > > > > > +
> > > > > > > > > > +	/* After write 1 to queue reset, the driver MUST wait for a read of
> > > > > > > > > > +	 * queue reset to return 1.
> > > > > > > > > > +	 */
> > > > > > > > > > +	while (vp_modern_get_queue_reset(mdev, queue_index) != 1)
> > > > > > > > > > +		msleep(1);
> > > > > > > > > > +
> > > > > > > > > > +	info = vp_dev->vqs[queue_index];
> > > > > > > > > > +	msix_vec = info->msix_vector;
> > > > > > > > > > +
> > > > > > > > > > +	/* Disable VQ callback. */
> > > > > > > > > > +	if (vp_dev->per_vq_vectors && msix_vec != VIRTIO_MSI_NO_VECTOR)
> > > > > > > > > > +		disable_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec));
> > > > > > > > > > +
> > > > > > > > > > +	while ((buf = virtqueue_detach_unused_buf(info->vq)) != NULL)
> > > > > > > > > > +		callback(vdev, buf, data);
> > > > > > > > > > +
> > > > > > > > > > +	vp_del_reset_vq(vdev, queue_index);
> > > > > > > > > > +
> > > > > > > > > > +	return 0;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static struct virtqueue *vp_modern_enable_reset_vq(struct virtio_device *vdev,
> > > > > > > > > > +						   u16 queue_index,
> > > > > > > > > > +						   vq_callback_t *callback,
> > > > > > > > > > +						   const char *name,
> > > > > > > > > > +						   const bool *ctx)
> > > > > > > > > > +{
> > > > > > > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > > > > > +	struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> > > > > > > > > > +	struct virtqueue *vq;
> > > > > > > > > > +	u16 msix_vec;
> > > > > > > > > > +
> > > > > > > > > > +	if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
> > > > > > > > > > +		return ERR_PTR(-ENOENT);
> > > > > > > > > > +
> > > > > > > > > > +	/* check queue reset status */
> > > > > > > > > > +	if (vp_modern_get_queue_reset(mdev, queue_index) != 1)
> > > > > > > > > > +		return ERR_PTR(-EBUSY);
> > > > > > > > > > +
> > > > > > > > > > +	vq = vp_enable_reset_vq(vdev, queue_index, callback, name, ctx);
> > > > > > > > > > +	if (IS_ERR(vq))
> > > > > > > > > > +		return vq;
> > > > > > > > > > +
> > > > > > > > > > +	vp_modern_set_queue_enable(&vp_dev->mdev, vq->index, true);
> > > > > > > > > > +
> > > > > > > > > > +	msix_vec = vp_dev->vqs[queue_index]->msix_vector;
> > > > > > > > > > +	if (vp_dev->per_vq_vectors && msix_vec != VIRTIO_MSI_NO_VECTOR)
> > > > > > > > > > +		enable_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec));
> > > > > > > > > > +
> > > > > > > > > > +	return vq;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > >  static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
> > > > > > > > > >  {
> > > > > > > > > >  	return vp_modern_config_vector(&vp_dev->mdev, vector);
> > > > > > > > > > @@ -395,6 +464,8 @@ static const struct virtio_config_ops virtio_pci_config_nodev_ops = {
> > > > > > > > > >  	.set_vq_affinity = vp_set_vq_affinity,
> > > > > > > > > >  	.get_vq_affinity = vp_get_vq_affinity,
> > > > > > > > > >  	.get_shm_region  = vp_get_shm_region,
> > > > > > > > > > +	.reset_vq	 = vp_modern_reset_vq,
> > > > > > > > > > +	.enable_reset_vq = vp_modern_enable_reset_vq,
> > > > > > > > > >  };
> > > > > > > > > >
> > > > > > > > > >  static const struct virtio_config_ops virtio_pci_config_ops = {
> > > > > > > > > > @@ -413,6 +484,8 @@ static const struct virtio_config_ops virtio_pci_config_ops = {
> > > > > > > > > >  	.set_vq_affinity = vp_set_vq_affinity,
> > > > > > > > > >  	.get_vq_affinity = vp_get_vq_affinity,
> > > > > > > > > >  	.get_shm_region  = vp_get_shm_region,
> > > > > > > > > > +	.reset_vq	 = vp_modern_reset_vq,
> > > > > > > > > > +	.enable_reset_vq = vp_modern_enable_reset_vq,
> > > > > > > > > >  };
> > > > > > > > > >
> > > > > > > > > >  /* the PCI probing function */
> > > > > > > > > > --
> > > > > > > > > > 2.31.0
> > > > > > > > >
> > > > > > >
> > > >
> >

Re: [PATCH v2 07/12] virtio: queue_reset: pci: support VIRTIO_F_RING_RESET

[Michael S. Tsirkin]

On Fri, Jan 21, 2022 at 11:34:25PM +0800, Xuan Zhuo wrote:
> On Fri, 21 Jan 2022 10:28:43 -0500, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Fri, Jan 21, 2022 at 10:09:31PM +0800, Xuan Zhuo wrote:
> > > On Fri, 21 Jan 2022 08:19:55 -0500, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > On Fri, Jan 21, 2022 at 06:26:33PM +0800, Xuan Zhuo wrote:
> > > > > On Fri, 21 Jan 2022 05:22:59 -0500, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > > > On Fri, Jan 21, 2022 at 05:22:29AM -0500, Michael S. Tsirkin wrote:
> > > > > > > On Fri, Jan 21, 2022 at 10:22:59AM +0800, Xuan Zhuo wrote:
> > > > > > > > On Thu, 20 Jan 2022 10:03:45 -0500, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > > > > > > On Thu, Jan 20, 2022 at 07:46:20PM +0800, Xuan Zhuo wrote:
> > > > > > > > > > On Thu, 20 Jan 2022 05:55:14 -0500, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > > > > > > > > On Thu, Jan 20, 2022 at 02:42:58PM +0800, Xuan Zhuo wrote:
> > > > > > > > > > > > This patch implements virtio pci support for QUEUE RESET.
> > > > > > > > > > > >
> > > > > > > > > > > > Performing reset on a queue is divided into two steps:
> > > > > > > > > > > >
> > > > > > > > > > > > 1. reset_vq: reset one vq
> > > > > > > > > > > > 2. enable_reset_vq: re-enable the reset queue
> > > > > > > > > > > >
> > > > > > > > > > > > In the first step, these tasks will be completed:
> > > > > > > > > > > >    1. notify the hardware queue to reset
> > > > > > > > > > > >    2. recycle the buffer from vq
> > > > > > > > > > > >    3. delete the vq
> > > > > > > > > > > >
> > > > > > > > > > > > When deleting a vq, vp_del_vq() will be called to release all the memory
> > > > > > > > > > > > of the vq. But this does not affect the process of deleting vqs, because
> > > > > > > > > > > > that is based on the queue to release all the vqs. During this process,
> > > > > > > > > > > > the vq has been removed from the queue.
> > > > > > > > > > > >
> > > > > > > > > > > > When deleting vq, info and vq will be released, and I save msix_vec in
> > > > > > > > > > > > vp_dev->vqs[queue_index]. When re-enable, the current msix_vec can be
> > > > > > > > > > > > reused. And based on intx_enabled to determine which method to use to
> > > > > > > > > > > > enable this queue.
> > > > > > > > > > > >
> > > > > > > > > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > > > > > > > >
> > > > > > > > > > > There's something I don't understand here. It looks like
> > > > > > > > > > > you assume that when you reset a queue, you also
> > > > > > > > > > > reset the mapping from queue to event vector.
> > > > > > > > > > > The spec does not say it should, and I don't think it's
> > > > > > > > > > > useful to extend spec to do it - we already have a simple
> > > > > > > > > > > way to tweak the mapping.
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Sorry, what is the already existing method you are referring to, I didn't find
> > > > > > > > > > it.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > Write 0xffff into vector number.
> > > > > > > >
> > > > > > > > I wonder if there is some misunderstanding here.
> > > > > > > >
> > > > > > > > My purpose is to release vq, then for the vector used by vq, I hope that it can
> > > > > > > > be reused when re-enable.
> > > > > > > >
> > > > > > > > But the vector number is not in a fixed order. When I re-enable it, I don't know
> > > > > > > > what the original vector number is. So I found a place to save this number.
> > > > > > > >
> > > > > > > > The queue reset I implemented is divided into the following steps:
> > > > > > > > 	1. notify the driver to queue reset
> > > > > > > > 	2. disable_irq()
> > > > > > > > 	3. free unused bufs
> > > > > > > > 	4. free irq, free vq, free info
> > > > > >
> > > > > > step 4 here seems pointless.
> > > > >
> > > > >
> > > > > The core operation is to release the vq. The release of the irq is indeed by the
> > > > > way. We can leave the irq untouched.
> > > > >
> > > > > 	1. notify the driver to queue reset
> > > > > 	2. disable_irq()
> > > > > 	3. free unused bufs
> > > > > 	4. free vq, free info
> > > > >
> > > > > Thanks.
> > > >
> > > > OK. why free the vq and info though?
> > >
> > >
> > > I guess what you mean may be that we do not release the memory of vq, but
> > > reinitialize it and map it to the backend. In this way, the ring size of vq is
> > > also unchanged.
> > >
> > > +After the queue has been successfully reset, the driver MAY release any
> > > +resource associated with that virtqueue.
> > >
> > > [...]
> > >
> > > +
> > > +\devicenormative{\paragraph}{Virtqueue Re-enable}{Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset / Virtqueue Re-enable}
> > > +
> > > +The device MUST observe any queue configuration that may have been
> > > +changed by the driver, like the maximum queue size.
> > > +
> > > +\drivernormative{\paragraph}{Virtqueue Re-enable}{Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset / Virtqueue Re-enable}
> > > +
> > > +When re-enabling a queue, the driver MUST configure the queue resources
> > > +as during initial virtqueue discovery, but optionally with different
> > > +parameters.
> > > +
> > >
> > > Based on the above spec definition, reset can modify, for example, the length of
> > > the queue.  So I chose to release memory here. Of course, I have not yet
> > > supported the modification of ring size.
> > >
> > > I wonder if we can provide two methods when implementing queue reset:
> > >
> > > 	1. Lightweight reset, do not release vq memory, re-initialize vq. Then
> > >        	map to the backend (virtio-net supports AF_XDP, this can be satisfied)
> > > 	2. Release vq to support modification of ring size. (virtio-net can
> > > 	modify ring size based on this)
> > >
> > > Thanks.
> >
> > I'm not sure I understand completely. I guess the simpler the
> > reset code the better so ... 1 is simpler I guess?
> 
> Yes.
> 
> I will first implement the next version based on 1. If there is a need to modify
> the ring size in the future, we will consider implementing solution 2.
> 
> Thanks.


Hmm resize is worth supporting. Is it much more code? If not maybe do
2 really.

> >
> > >
> > >
> > > >
> > > > > >
> > > > > > > > The process of enable is divided into the following steps:
> > > > > > > > 	1. Get the vector number used by the original vq and re-setup vq
> > > > > > > > 	2. enable vq
> > > > > > > > 	3. enable irq
> > > > > > > >
> > > > > > > > If there is anything unreasonable please let me know.
> > > > > > > >
> > > > > > > > Thanks.
> > > > > > >
> > > > > > > Why do you free irq?
> > > > > > >
> > > > > > > > >
> > > > > > > > > > I think you mean that we don't have to reset the event vector, I think you are
> > > > > > > > > > right.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Thanks.
> > > > > > > > > >
> > > > > > > > > > > Avoid doing that, and things will be much easier, with no need
> > > > > > > > > > > to interact with a transport, won't they?
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > > ---
> > > > > > > > > > > >  drivers/virtio/virtio_pci_common.c | 49 ++++++++++++++++++++
> > > > > > > > > > > >  drivers/virtio/virtio_pci_common.h |  4 ++
> > > > > > > > > > > >  drivers/virtio/virtio_pci_modern.c | 73 ++++++++++++++++++++++++++++++
> > > > > > > > > > > >  3 files changed, 126 insertions(+)
> > > > > > > > > > > >
> > > > > > > > > > > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> > > > > > > > > > > > index 5afe207ce28a..28b5ffde4621 100644
> > > > > > > > > > > > --- a/drivers/virtio/virtio_pci_common.c
> > > > > > > > > > > > +++ b/drivers/virtio/virtio_pci_common.c
> > > > > > > > > > > > @@ -464,6 +464,55 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> > > > > > > > > > > >  	return vp_find_vqs_intx(vdev, nvqs, vqs, callbacks, names, ctx);
> > > > > > > > > > > >  }
> > > > > > > > > > > >
> > > > > > > > > > > > +#define VQ_IS_DELETED(vp_dev, idx) ((unsigned long)vp_dev->vqs[idx] & 1)
> > > > > > > > > > > > +#define VQ_RESET_MSIX_VEC(vp_dev, idx) ((unsigned long)vp_dev->vqs[idx] >> 2)
> > > > > > > > > > > > +#define VQ_RESET_MARK(msix_vec) ((void *)(long)((msix_vec << 2) + 1))
> > > > > > > > > > > > +
> > > > > > > > > > > > +void vp_del_reset_vq(struct virtio_device *vdev, u16 queue_index)
> > > > > > > > > > > > +{
> > > > > > > > > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > > > > > > > +	struct virtio_pci_vq_info *info;
> > > > > > > > > > > > +	u16 msix_vec;
> > > > > > > > > > > > +
> > > > > > > > > > > > +	info = vp_dev->vqs[queue_index];
> > > > > > > > > > > > +
> > > > > > > > > > > > +	msix_vec = info->msix_vector;
> > > > > > > > > > > > +
> > > > > > > > > > > > +	/* delete vq */
> > > > > > > > > > > > +	vp_del_vq(info->vq);
> > > > > > > > > > > > +
> > > > > > > > > > > > +	/* Mark the vq has been deleted, and save the msix_vec. */
> > > > > > > > > > > > +	vp_dev->vqs[queue_index] = VQ_RESET_MARK(msix_vec);
> > > > > > > > > > > > +}
> > > > > > > > > > > > +
> > > > > > > > > > > > +struct virtqueue *vp_enable_reset_vq(struct virtio_device *vdev,
> > > > > > > > > > > > +				     int queue_index,
> > > > > > > > > > > > +				     vq_callback_t *callback,
> > > > > > > > > > > > +				     const char *name,
> > > > > > > > > > > > +				     const bool ctx)
> > > > > > > > > > > > +{
> > > > > > > > > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > > > > > > > +	struct virtqueue *vq;
> > > > > > > > > > > > +	u16 msix_vec;
> > > > > > > > > > > > +
> > > > > > > > > > > > +	if (!VQ_IS_DELETED(vp_dev, queue_index))
> > > > > > > > > > > > +		return ERR_PTR(-EPERM);
> > > > > > > > > > > > +
> > > > > > > > > > > > +	msix_vec = VQ_RESET_MSIX_VEC(vp_dev, queue_index);
> > > > > > > > > > > > +
> > > > > > > > > > > > +	if (vp_dev->intx_enabled)
> > > > > > > > > > > > +		vq = vp_setup_vq(vdev, queue_index, callback, name, ctx,
> > > > > > > > > > > > +				 VIRTIO_MSI_NO_VECTOR);
> > > > > > > > > > > > +	else
> > > > > > > > > > > > +		vq = vp_enable_vq_msix(vdev, queue_index, callback, name, ctx,
> > > > > > > > > > > > +				       msix_vec);
> > > > > > > > > > > > +
> > > > > > > > > > > > +	if (IS_ERR(vq))
> > > > > > > > > > > > +		vp_dev->vqs[queue_index] = VQ_RESET_MARK(msix_vec);
> > > > > > > > > > > > +
> > > > > > > > > > > > +	return vq;
> > > > > > > > > > > > +}
> > > > > > > > > > > > +
> > > > > > > > > > > >  const char *vp_bus_name(struct virtio_device *vdev)
> > > > > > > > > > > >  {
> > > > > > > > > > > >  	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > > > > > > > diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
> > > > > > > > > > > > index 23f6c5c678d5..96c13b1398f8 100644
> > > > > > > > > > > > --- a/drivers/virtio/virtio_pci_common.h
> > > > > > > > > > > > +++ b/drivers/virtio/virtio_pci_common.h
> > > > > > > > > > > > @@ -115,6 +115,10 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> > > > > > > > > > > >  		struct virtqueue *vqs[], vq_callback_t *callbacks[],
> > > > > > > > > > > >  		const char * const names[], const bool *ctx,
> > > > > > > > > > > >  		struct irq_affinity *desc);
> > > > > > > > > > > > +void vp_del_reset_vq(struct virtio_device *vdev, u16 queue_index);
> > > > > > > > > > > > +struct virtqueue *vp_enable_reset_vq(struct virtio_device *vdev, int queue_index,
> > > > > > > > > > > > +				     vq_callback_t *callback, const char *name,
> > > > > > > > > > > > +				     const bool ctx);
> > > > > > > > > > > >  const char *vp_bus_name(struct virtio_device *vdev);
> > > > > > > > > > > >
> > > > > > > > > > > >  /* Setup the affinity for a virtqueue:
> > > > > > > > > > > > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> > > > > > > > > > > > index 5455bc041fb6..fbf87239c920 100644
> > > > > > > > > > > > --- a/drivers/virtio/virtio_pci_modern.c
> > > > > > > > > > > > +++ b/drivers/virtio/virtio_pci_modern.c
> > > > > > > > > > > > @@ -34,6 +34,9 @@ static void vp_transport_features(struct virtio_device *vdev, u64 features)
> > > > > > > > > > > >  	if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> > > > > > > > > > > >  			pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> > > > > > > > > > > >  		__virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> > > > > > > > > > > > +
> > > > > > > > > > > > +	if (features & BIT_ULL(VIRTIO_F_RING_RESET))
> > > > > > > > > > > > +		__virtio_set_bit(vdev, VIRTIO_F_RING_RESET);
> > > > > > > > > > > >  }
> > > > > > > > > > > >
> > > > > > > > > > > >  /* virtio config->finalize_features() implementation */
> > > > > > > > > > > > @@ -176,6 +179,72 @@ static void vp_reset(struct virtio_device *vdev)
> > > > > > > > > > > >  	vp_disable_cbs(vdev);
> > > > > > > > > > > >  }
> > > > > > > > > > > >
> > > > > > > > > > > > +static int vp_modern_reset_vq(struct virtio_device *vdev, u16 queue_index,
> > > > > > > > > > > > +			      vq_reset_callback_t *callback, void *data)
> > > > > > > > > > > > +{
> > > > > > > > > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > > > > > > > +	struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> > > > > > > > > > > > +	struct virtio_pci_vq_info *info;
> > > > > > > > > > > > +	u16 msix_vec;
> > > > > > > > > > > > +	void *buf;
> > > > > > > > > > > > +
> > > > > > > > > > > > +	if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
> > > > > > > > > > > > +		return -ENOENT;
> > > > > > > > > > > > +
> > > > > > > > > > > > +	vp_modern_set_queue_reset(mdev, queue_index);
> > > > > > > > > > > > +
> > > > > > > > > > > > +	/* After write 1 to queue reset, the driver MUST wait for a read of
> > > > > > > > > > > > +	 * queue reset to return 1.
> > > > > > > > > > > > +	 */
> > > > > > > > > > > > +	while (vp_modern_get_queue_reset(mdev, queue_index) != 1)
> > > > > > > > > > > > +		msleep(1);
> > > > > > > > > > > > +
> > > > > > > > > > > > +	info = vp_dev->vqs[queue_index];
> > > > > > > > > > > > +	msix_vec = info->msix_vector;
> > > > > > > > > > > > +
> > > > > > > > > > > > +	/* Disable VQ callback. */
> > > > > > > > > > > > +	if (vp_dev->per_vq_vectors && msix_vec != VIRTIO_MSI_NO_VECTOR)
> > > > > > > > > > > > +		disable_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec));
> > > > > > > > > > > > +
> > > > > > > > > > > > +	while ((buf = virtqueue_detach_unused_buf(info->vq)) != NULL)
> > > > > > > > > > > > +		callback(vdev, buf, data);
> > > > > > > > > > > > +
> > > > > > > > > > > > +	vp_del_reset_vq(vdev, queue_index);
> > > > > > > > > > > > +
> > > > > > > > > > > > +	return 0;
> > > > > > > > > > > > +}
> > > > > > > > > > > > +
> > > > > > > > > > > > +static struct virtqueue *vp_modern_enable_reset_vq(struct virtio_device *vdev,
> > > > > > > > > > > > +						   u16 queue_index,
> > > > > > > > > > > > +						   vq_callback_t *callback,
> > > > > > > > > > > > +						   const char *name,
> > > > > > > > > > > > +						   const bool *ctx)
> > > > > > > > > > > > +{
> > > > > > > > > > > > +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > > > > > > > > > > +	struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> > > > > > > > > > > > +	struct virtqueue *vq;
> > > > > > > > > > > > +	u16 msix_vec;
> > > > > > > > > > > > +
> > > > > > > > > > > > +	if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
> > > > > > > > > > > > +		return ERR_PTR(-ENOENT);
> > > > > > > > > > > > +
> > > > > > > > > > > > +	/* check queue reset status */
> > > > > > > > > > > > +	if (vp_modern_get_queue_reset(mdev, queue_index) != 1)
> > > > > > > > > > > > +		return ERR_PTR(-EBUSY);
> > > > > > > > > > > > +
> > > > > > > > > > > > +	vq = vp_enable_reset_vq(vdev, queue_index, callback, name, ctx);
> > > > > > > > > > > > +	if (IS_ERR(vq))
> > > > > > > > > > > > +		return vq;
> > > > > > > > > > > > +
> > > > > > > > > > > > +	vp_modern_set_queue_enable(&vp_dev->mdev, vq->index, true);
> > > > > > > > > > > > +
> > > > > > > > > > > > +	msix_vec = vp_dev->vqs[queue_index]->msix_vector;
> > > > > > > > > > > > +	if (vp_dev->per_vq_vectors && msix_vec != VIRTIO_MSI_NO_VECTOR)
> > > > > > > > > > > > +		enable_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec));
> > > > > > > > > > > > +
> > > > > > > > > > > > +	return vq;
> > > > > > > > > > > > +}
> > > > > > > > > > > > +
> > > > > > > > > > > >  static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
> > > > > > > > > > > >  {
> > > > > > > > > > > >  	return vp_modern_config_vector(&vp_dev->mdev, vector);
> > > > > > > > > > > > @@ -395,6 +464,8 @@ static const struct virtio_config_ops virtio_pci_config_nodev_ops = {
> > > > > > > > > > > >  	.set_vq_affinity = vp_set_vq_affinity,
> > > > > > > > > > > >  	.get_vq_affinity = vp_get_vq_affinity,
> > > > > > > > > > > >  	.get_shm_region  = vp_get_shm_region,
> > > > > > > > > > > > +	.reset_vq	 = vp_modern_reset_vq,
> > > > > > > > > > > > +	.enable_reset_vq = vp_modern_enable_reset_vq,
> > > > > > > > > > > >  };
> > > > > > > > > > > >
> > > > > > > > > > > >  static const struct virtio_config_ops virtio_pci_config_ops = {
> > > > > > > > > > > > @@ -413,6 +484,8 @@ static const struct virtio_config_ops virtio_pci_config_ops = {
> > > > > > > > > > > >  	.set_vq_affinity = vp_set_vq_affinity,
> > > > > > > > > > > >  	.get_vq_affinity = vp_get_vq_affinity,
> > > > > > > > > > > >  	.get_shm_region  = vp_get_shm_region,
> > > > > > > > > > > > +	.reset_vq	 = vp_modern_reset_vq,
> > > > > > > > > > > > +	.enable_reset_vq = vp_modern_enable_reset_vq,
> > > > > > > > > > > >  };
> > > > > > > > > > > >
> > > > > > > > > > > >  /* the PCI probing function */
> > > > > > > > > > > > --
> > > > > > > > > > > > 2.31.0
> > > > > > > > > > >
> > > > > > > > >
> > > > > >
> > > >
> >