* [PATCH 0/6] virtio pci support VIRTIO_F_RING_RESET
@ 2022-01-19 2:35 Xuan Zhuo
2022-01-19 2:35 ` [PATCH 1/6] virtio: pci: struct virtio_pci_common_cfg add queue_notify_data Xuan Zhuo
` (6 more replies)
0 siblings, 7 replies; 17+ messages in thread
From: Xuan Zhuo @ 2022-01-19 2:35 UTC (permalink / raw)
To: virtualization; +Cc: Michael S. Tsirkin
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
virtio-net's queue disable/enable function based on virtio queue reset is here
https://github.com/fengidri/linux/commit/021165b4eda44f87c4b7771ef637bab2512d066f
I plan to include this patch in the subsequent virtio-net support AF_XDP
patch set.
Also regarding MMIO support for queue reset, I plan to support it after this
patch is passed.
Please review. Thanks.
Xuan Zhuo (6):
virtio: pci: struct virtio_pci_common_cfg add queue_notify_data
virtio: queue_reset: add VIRTIO_F_RING_RESET
virtio: queue_reset: pci: update struct virtio_pci_common_cfg and
option functions
virtio: queue_reset: pci: extract some functions for subsequent
patches
virtio: queue_reset: pci: support VIRTIO_F_RING_RESET
virtio: queue_reset: add helper
drivers/virtio/virtio_pci_common.c | 140 +++++++++++++++++++------
drivers/virtio/virtio_pci_common.h | 5 +
drivers/virtio/virtio_pci_modern.c | 81 ++++++++++++++
drivers/virtio/virtio_pci_modern_dev.c | 28 +++++
include/linux/virtio_config.h | 78 ++++++++++++++
include/linux/virtio_pci_modern.h | 2 +
include/uapi/linux/virtio_config.h | 7 +-
include/uapi/linux/virtio_pci.h | 2 +
8 files changed, 310 insertions(+), 33 deletions(-)
--
2.31.0
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/6] virtio: pci: struct virtio_pci_common_cfg add queue_notify_data
2022-01-19 2:35 [PATCH 0/6] virtio pci support VIRTIO_F_RING_RESET Xuan Zhuo
@ 2022-01-19 2:35 ` Xuan Zhuo
2022-01-19 6:00 ` Jason Wang
2022-01-19 2:35 ` [PATCH 2/6] virtio: queue_reset: add VIRTIO_F_RING_RESET Xuan Zhuo
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Xuan Zhuo @ 2022-01-19 2:35 UTC (permalink / raw)
To: virtualization; +Cc: Michael S. Tsirkin
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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/6] virtio: queue_reset: add VIRTIO_F_RING_RESET
2022-01-19 2:35 [PATCH 0/6] virtio pci support VIRTIO_F_RING_RESET Xuan Zhuo
2022-01-19 2:35 ` [PATCH 1/6] virtio: pci: struct virtio_pci_common_cfg add queue_notify_data Xuan Zhuo
@ 2022-01-19 2:35 ` Xuan Zhuo
2022-01-19 2:35 ` [PATCH 3/6] virtio: queue_reset: pci: update struct virtio_pci_common_cfg and option functions Xuan Zhuo
` (4 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Xuan Zhuo @ 2022-01-19 2:35 UTC (permalink / raw)
To: virtualization; +Cc: Michael S. Tsirkin
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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/6] virtio: queue_reset: pci: update struct virtio_pci_common_cfg and option functions
2022-01-19 2:35 [PATCH 0/6] virtio pci support VIRTIO_F_RING_RESET Xuan Zhuo
2022-01-19 2:35 ` [PATCH 1/6] virtio: pci: struct virtio_pci_common_cfg add queue_notify_data Xuan Zhuo
2022-01-19 2:35 ` [PATCH 2/6] virtio: queue_reset: add VIRTIO_F_RING_RESET Xuan Zhuo
@ 2022-01-19 2:35 ` Xuan Zhuo
2022-01-19 2:35 ` [PATCH 4/6] virtio: queue_reset: pci: extract some functions for subsequent patches Xuan Zhuo
` (3 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Xuan Zhuo @ 2022-01-19 2:35 UTC (permalink / raw)
To: virtualization; +Cc: Michael S. Tsirkin
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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/6] virtio: queue_reset: pci: extract some functions for subsequent patches
2022-01-19 2:35 [PATCH 0/6] virtio pci support VIRTIO_F_RING_RESET Xuan Zhuo
` (2 preceding siblings ...)
2022-01-19 2:35 ` [PATCH 3/6] virtio: queue_reset: pci: update struct virtio_pci_common_cfg and option functions Xuan Zhuo
@ 2022-01-19 2:35 ` Xuan Zhuo
2022-01-19 6:04 ` Jason Wang
2022-01-19 2:35 ` [PATCH 5/6] virtio: queue_reset: pci: support VIRTIO_F_RING_RESET Xuan Zhuo
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Xuan Zhuo @ 2022-01-19 2:35 UTC (permalink / raw)
To: virtualization; +Cc: Michael S. Tsirkin
Add a vq_enable_vq_msix() function to enable a vq alone.
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/enable a vq separately.
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
drivers/virtio/virtio_pci_common.c | 86 +++++++++++++++++++-----------
1 file changed, 54 insertions(+), 32 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index fdbde1db5ec5..5afe207ce28a 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) {
@@ -310,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,
@@ -319,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)
@@ -354,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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/6] virtio: queue_reset: pci: support VIRTIO_F_RING_RESET
2022-01-19 2:35 [PATCH 0/6] virtio pci support VIRTIO_F_RING_RESET Xuan Zhuo
` (3 preceding siblings ...)
2022-01-19 2:35 ` [PATCH 4/6] virtio: queue_reset: pci: extract some functions for subsequent patches Xuan Zhuo
@ 2022-01-19 2:35 ` Xuan Zhuo
2022-01-19 6:11 ` Jason Wang
2022-01-19 2:35 ` [PATCH 6/6] virtio: queue_reset: add helper Xuan Zhuo
2022-01-19 6:13 ` [PATCH 0/6] virtio pci support VIRTIO_F_RING_RESET Jason Wang
6 siblings, 1 reply; 17+ messages in thread
From: Xuan Zhuo @ 2022-01-19 2:35 UTC (permalink / raw)
To: virtualization; +Cc: Michael S. Tsirkin
This patch implements virtio pci support for QUEUE RESET.
Performing reset on a queue is divided into three steps:
1. reset_vq: notify the hardware queue to reset
2. del_reset_vq: delete the reset queue
3. enable_reset_vq: re-enable the reset queue
Between steps 1 and 2, generally call virtqueue_detach_unused_buf() to
recycle the buffer.
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 | 54 ++++++++++++++++++++
drivers/virtio/virtio_pci_common.h | 5 ++
drivers/virtio/virtio_pci_modern.c | 81 ++++++++++++++++++++++++++++++
include/linux/virtio_config.h | 21 ++++++++
4 files changed, 161 insertions(+)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 5afe207ce28a..615a5a5f757c 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -464,6 +464,60 @@ 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))
+
+int 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;
+
+ if (VQ_IS_DELETED(vp_dev, queue_index))
+ return -EPERM;
+
+ 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);
+
+ return 0;
+}
+
+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..a92c99cc010e 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -115,6 +115,11 @@ 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_reset_vq(struct virtio_device *vdev, u16 queue_index);
+int 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..74e0b2b2f4dc 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,78 @@ 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)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
+ u16 msix_vec;
+
+ 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);
+
+ msix_vec = vp_dev->vqs[queue_index]->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));
+
+ return 0;
+}
+
+static int vp_modern_del_reset_vq(struct virtio_device *vdev, u16 queue_index)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
+
+ if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
+ return -ENOENT;
+
+ /* check queue reset status */
+ if (vp_modern_get_queue_reset(mdev, queue_index) != 1)
+ return -EBUSY;
+
+ return vp_del_reset_vq(vdev, queue_index);
+}
+
+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 +470,9 @@ 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,
+ .del_reset_vq = vp_modern_del_reset_vq,
+ .enable_reset_vq = vp_modern_enable_reset_vq,
};
static const struct virtio_config_ops virtio_pci_config_ops = {
@@ -413,6 +491,9 @@ 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,
+ .del_reset_vq = vp_modern_del_reset_vq,
+ .enable_reset_vq = vp_modern_enable_reset_vq,
};
/* the PCI probing function */
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 4d107ad31149..6e94f7d0e153 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -74,6 +74,21 @@ 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
+ * returns 0 on success or error status
+ * @del_reset_vq: del a reset queue
+ * vdev: the device
+ * queue_index: the queue index
+ * 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 *);
struct virtio_config_ops {
@@ -100,6 +115,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);
+ int (*del_reset_vq)(struct virtio_device *vdev, u16 queue_index);
+ 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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 6/6] virtio: queue_reset: add helper
2022-01-19 2:35 [PATCH 0/6] virtio pci support VIRTIO_F_RING_RESET Xuan Zhuo
` (4 preceding siblings ...)
2022-01-19 2:35 ` [PATCH 5/6] virtio: queue_reset: pci: support VIRTIO_F_RING_RESET Xuan Zhuo
@ 2022-01-19 2:35 ` Xuan Zhuo
2022-01-19 6:13 ` [PATCH 0/6] virtio pci support VIRTIO_F_RING_RESET Jason Wang
6 siblings, 0 replies; 17+ messages in thread
From: Xuan Zhuo @ 2022-01-19 2:35 UTC (permalink / raw)
To: virtualization; +Cc: Michael S. Tsirkin
Add helper for virtio queue reset.
* virtio_reset_vq: reset a queue individually
* virtio_del_resetq: del a reset queue
* virtio_enable_resetq: enable a reset queue
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
include/linux/virtio_config.h | 57 +++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 6e94f7d0e153..ddc26a5e74fd 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -240,6 +240,63 @@ 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
+ *
+ * returns 0 on success or error status
+ *
+ */
+static inline
+int virtio_reset_vq(struct virtio_device *vdev, u16 queue_index)
+{
+ if (!vdev->config->reset_vq)
+ return -ENOENT;
+
+ return vdev->config->reset_vq(vdev, queue_index);
+}
+
+/**
+ * virtio_del_resetq - del a reset queue
+ * @vdev: the device
+ * @queue_index: the queue index
+ *
+ * returns 0 on success or error status.
+ *
+ */
+static inline
+int virtio_del_resetq(struct virtio_device *vdev, u16 queue_index)
+{
+ if (!vdev->config->del_reset_vq)
+ return -ENOENT;
+
+ return vdev->config->del_reset_vq(vdev, queue_index);
+}
+
+/**
+ * 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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6] virtio: pci: struct virtio_pci_common_cfg add queue_notify_data
2022-01-19 2:35 ` [PATCH 1/6] virtio: pci: struct virtio_pci_common_cfg add queue_notify_data Xuan Zhuo
@ 2022-01-19 6:00 ` Jason Wang
2022-01-19 6:31 ` Xuan Zhuo
0 siblings, 1 reply; 17+ messages in thread
From: Jason Wang @ 2022-01-19 6:00 UTC (permalink / raw)
To: Xuan Zhuo; +Cc: Michael S. Tsirkin, virtualization
On Wed, Jan 19, 2022 at 10:35 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> 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 */
> };
Since it's uAPI, do we need another which embed virtio_pci_common_cfg?
Thanks
>
> /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
> --
> 2.31.0
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6] virtio: queue_reset: pci: extract some functions for subsequent patches
2022-01-19 2:35 ` [PATCH 4/6] virtio: queue_reset: pci: extract some functions for subsequent patches Xuan Zhuo
@ 2022-01-19 6:04 ` Jason Wang
2022-01-19 6:33 ` Xuan Zhuo
0 siblings, 1 reply; 17+ messages in thread
From: Jason Wang @ 2022-01-19 6:04 UTC (permalink / raw)
To: Xuan Zhuo; +Cc: Michael S. Tsirkin, virtualization
On Wed, Jan 19, 2022 at 10:35 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> Add a vq_enable_vq_msix() function to enable a vq alone.
>
> 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/enable a vq separately.
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
> drivers/virtio/virtio_pci_common.c | 86 +++++++++++++++++++-----------
> 1 file changed, 54 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index fdbde1db5ec5..5afe207ce28a 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);
> - }
I think we need a better subject of the patch and it's better to even
split the patch into two.
This part could be "move the per queue irq logic from vp_del_vqs to vp_del_vq".
> +
> vp_dev->per_vq_vectors = false;
>
> if (vp_dev->intx_enabled) {
> @@ -310,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;
> +}
And this deserves a separate patch as well.
Thanks
> +
> 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,
> @@ -319,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)
> @@ -354,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
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/6] virtio: queue_reset: pci: support VIRTIO_F_RING_RESET
2022-01-19 2:35 ` [PATCH 5/6] virtio: queue_reset: pci: support VIRTIO_F_RING_RESET Xuan Zhuo
@ 2022-01-19 6:11 ` Jason Wang
2022-01-19 6:12 ` Jason Wang
2022-01-19 6:36 ` Xuan Zhuo
0 siblings, 2 replies; 17+ messages in thread
From: Jason Wang @ 2022-01-19 6:11 UTC (permalink / raw)
To: Xuan Zhuo; +Cc: Michael S. Tsirkin, virtualization
On Wed, Jan 19, 2022 at 10:35 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> This patch implements virtio pci support for QUEUE RESET.
>
> Performing reset on a queue is divided into three steps:
>
> 1. reset_vq: notify the hardware queue to reset
> 2. del_reset_vq: delete the reset queue
> 3. enable_reset_vq: re-enable the reset queue
>
> Between steps 1 and 2, generally call virtqueue_detach_unused_buf() to
> recycle the buffer.
I wonder if it's better to squash 1, 2 and
virtqueue_detach_unused_buf() into a single helper.
>
> 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 | 54 ++++++++++++++++++++
> drivers/virtio/virtio_pci_common.h | 5 ++
> drivers/virtio/virtio_pci_modern.c | 81 ++++++++++++++++++++++++++++++
> include/linux/virtio_config.h | 21 ++++++++
> 4 files changed, 161 insertions(+)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 5afe207ce28a..615a5a5f757c 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -464,6 +464,60 @@ 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))
> +
> +int 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;
> +
> + if (VQ_IS_DELETED(vp_dev, queue_index))
> + return -EPERM;
> +
> + 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);
> +
> + return 0;
> +}
> +
> +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..a92c99cc010e 100644
> --- a/drivers/virtio/virtio_pci_common.h
> +++ b/drivers/virtio/virtio_pci_common.h
> @@ -115,6 +115,11 @@ 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_reset_vq(struct virtio_device *vdev, u16 queue_index);
> +int 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..74e0b2b2f4dc 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,78 @@ 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)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> + u16 msix_vec;
> +
> + 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);
> +
> + msix_vec = vp_dev->vqs[queue_index]->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));
What happens if the irq is shared among virtqueues?
> +
> + return 0;
> +}
> +
> +static int vp_modern_del_reset_vq(struct virtio_device *vdev, u16 queue_index)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> +
> + if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
> + return -ENOENT;
> +
> + /* check queue reset status */
> + if (vp_modern_get_queue_reset(mdev, queue_index) != 1)
> + return -EBUSY;
> +
> + return vp_del_reset_vq(vdev, queue_index);
> +}
> +
> +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 +470,9 @@ 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,
> + .del_reset_vq = vp_modern_del_reset_vq,
> + .enable_reset_vq = vp_modern_enable_reset_vq,
> };
>
> static const struct virtio_config_ops virtio_pci_config_ops = {
> @@ -413,6 +491,9 @@ 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,
> + .del_reset_vq = vp_modern_del_reset_vq,
> + .enable_reset_vq = vp_modern_enable_reset_vq,
> };
>
> /* the PCI probing function */
> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index 4d107ad31149..6e94f7d0e153 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -74,6 +74,21 @@ 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
> + * returns 0 on success or error status
> + * @del_reset_vq: del a reset queue
> + * vdev: the device
> + * queue_index: the queue index
> + * 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 *);
> struct virtio_config_ops {
> @@ -100,6 +115,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);
> + int (*del_reset_vq)(struct virtio_device *vdev, u16 queue_index);
> + 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
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/6] virtio: queue_reset: pci: support VIRTIO_F_RING_RESET
2022-01-19 6:11 ` Jason Wang
@ 2022-01-19 6:12 ` Jason Wang
2022-01-19 6:33 ` Xuan Zhuo
2022-01-19 6:36 ` Xuan Zhuo
1 sibling, 1 reply; 17+ messages in thread
From: Jason Wang @ 2022-01-19 6:12 UTC (permalink / raw)
To: Xuan Zhuo; +Cc: Michael S. Tsirkin, virtualization
On Wed, Jan 19, 2022 at 2:11 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Wed, Jan 19, 2022 at 10:35 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > This patch implements virtio pci support for QUEUE RESET.
> >
> > Performing reset on a queue is divided into three steps:
> >
> > 1. reset_vq: notify the hardware queue to reset
> > 2. del_reset_vq: delete the reset queue
> > 3. enable_reset_vq: re-enable the reset queue
> >
> > Between steps 1 and 2, generally call virtqueue_detach_unused_buf() to
> > recycle the buffer.
>
> I wonder if it's better to squash 1, 2 and
> virtqueue_detach_unused_buf() into a single helper.
>
> >
> > 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 | 54 ++++++++++++++++++++
> > drivers/virtio/virtio_pci_common.h | 5 ++
> > drivers/virtio/virtio_pci_modern.c | 81 ++++++++++++++++++++++++++++++
> > include/linux/virtio_config.h | 21 ++++++++
> > 4 files changed, 161 insertions(+)
> >
> > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> > index 5afe207ce28a..615a5a5f757c 100644
> > --- a/drivers/virtio/virtio_pci_common.c
> > +++ b/drivers/virtio/virtio_pci_common.c
> > @@ -464,6 +464,60 @@ 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))
> > +
> > +int 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;
> > +
> > + if (VQ_IS_DELETED(vp_dev, queue_index))
> > + return -EPERM;
> > +
> > + 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);
> > +
> > + return 0;
> > +}
> > +
> > +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..a92c99cc010e 100644
> > --- a/drivers/virtio/virtio_pci_common.h
> > +++ b/drivers/virtio/virtio_pci_common.h
> > @@ -115,6 +115,11 @@ 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_reset_vq(struct virtio_device *vdev, u16 queue_index);
> > +int 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..74e0b2b2f4dc 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,78 @@ 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)
> > +{
> > + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > + struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> > + u16 msix_vec;
> > +
> > + 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);
> > +
> > + msix_vec = vp_dev->vqs[queue_index]->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));
>
> What happens if the irq is shared among virtqueues?
>
> > +
> > + return 0;
> > +}
> > +
> > +static int vp_modern_del_reset_vq(struct virtio_device *vdev, u16 queue_index)
> > +{
> > + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > + struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> > +
> > + if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
> > + return -ENOENT;
> > +
> > + /* check queue reset status */
> > + if (vp_modern_get_queue_reset(mdev, queue_index) != 1)
> > + return -EBUSY;
> > +
> > + return vp_del_reset_vq(vdev, queue_index);
> > +}
> > +
> > +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 +470,9 @@ 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,
> > + .del_reset_vq = vp_modern_del_reset_vq,
> > + .enable_reset_vq = vp_modern_enable_reset_vq,
> > };
> >
> > static const struct virtio_config_ops virtio_pci_config_ops = {
> > @@ -413,6 +491,9 @@ 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,
> > + .del_reset_vq = vp_modern_del_reset_vq,
> > + .enable_reset_vq = vp_modern_enable_reset_vq,
> > };
> >
> > /* the PCI probing function */
> > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> > index 4d107ad31149..6e94f7d0e153 100644
> > --- a/include/linux/virtio_config.h
> > +++ b/include/linux/virtio_config.h
> > @@ -74,6 +74,21 @@ 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
> > + * returns 0 on success or error status
> > + * @del_reset_vq: del a reset queue
> > + * vdev: the device
> > + * queue_index: the queue index
> > + * 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 *);
> > struct virtio_config_ops {
> > @@ -100,6 +115,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);
> > + int (*del_reset_vq)(struct virtio_device *vdev, u16 queue_index);
Btw, it's better to do this in a separate patch.
Thanks
> > + 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
> >
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/6] virtio pci support VIRTIO_F_RING_RESET
2022-01-19 2:35 [PATCH 0/6] virtio pci support VIRTIO_F_RING_RESET Xuan Zhuo
` (5 preceding siblings ...)
2022-01-19 2:35 ` [PATCH 6/6] virtio: queue_reset: add helper Xuan Zhuo
@ 2022-01-19 6:13 ` Jason Wang
2022-01-19 6:22 ` Xuan Zhuo
6 siblings, 1 reply; 17+ messages in thread
From: Jason Wang @ 2022-01-19 6:13 UTC (permalink / raw)
To: Xuan Zhuo; +Cc: Michael S. Tsirkin, virtualization
On Wed, Jan 19, 2022 at 10:35 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> 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
>
> virtio-net's queue disable/enable function based on virtio queue reset is here
>
> https://github.com/fengidri/linux/commit/021165b4eda44f87c4b7771ef637bab2512d066f
>
> I plan to include this patch in the subsequent virtio-net support AF_XDP
> patch set.
It's better to send them all to see how the new helpers are actually used.
Thanks
>
> Also regarding MMIO support for queue reset, I plan to support it after this
> patch is passed.
>
> Please review. Thanks.
>
> Xuan Zhuo (6):
> virtio: pci: struct virtio_pci_common_cfg add queue_notify_data
> virtio: queue_reset: add VIRTIO_F_RING_RESET
> virtio: queue_reset: pci: update struct virtio_pci_common_cfg and
> option functions
> virtio: queue_reset: pci: extract some functions for subsequent
> patches
> virtio: queue_reset: pci: support VIRTIO_F_RING_RESET
> virtio: queue_reset: add helper
>
> drivers/virtio/virtio_pci_common.c | 140 +++++++++++++++++++------
> drivers/virtio/virtio_pci_common.h | 5 +
> drivers/virtio/virtio_pci_modern.c | 81 ++++++++++++++
> drivers/virtio/virtio_pci_modern_dev.c | 28 +++++
> include/linux/virtio_config.h | 78 ++++++++++++++
> include/linux/virtio_pci_modern.h | 2 +
> include/uapi/linux/virtio_config.h | 7 +-
> include/uapi/linux/virtio_pci.h | 2 +
> 8 files changed, 310 insertions(+), 33 deletions(-)
>
> --
> 2.31.0
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/6] virtio pci support VIRTIO_F_RING_RESET
2022-01-19 6:13 ` [PATCH 0/6] virtio pci support VIRTIO_F_RING_RESET Jason Wang
@ 2022-01-19 6:22 ` Xuan Zhuo
0 siblings, 0 replies; 17+ messages in thread
From: Xuan Zhuo @ 2022-01-19 6:22 UTC (permalink / raw)
To: Jason Wang; +Cc: Michael S. Tsirkin, virtualization
On Wed, 19 Jan 2022 14:13:39 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Wed, Jan 19, 2022 at 10:35 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > 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
> >
> > virtio-net's queue disable/enable function based on virtio queue reset is here
> >
> > https://github.com/fengidri/linux/commit/021165b4eda44f87c4b7771ef637bab2512d066f
> >
> > I plan to include this patch in the subsequent virtio-net support AF_XDP
> > patch set.
>
> It's better to send them all to see how the new helpers are actually used.
If this patch is sent together with virtio-net + AF_XDP, I feel that this
patch set will be too big.
I can add the disable/enable function of tx/rx pair to virtio-net. But no one
uses this function for now, so I put it on github.
I'll include these patches for virtio-net using these new helpers in the next patch
set.
Thanks.
>
> Thanks
>
> >
> > Also regarding MMIO support for queue reset, I plan to support it after this
> > patch is passed.
> >
> > Please review. Thanks.
> >
> > Xuan Zhuo (6):
> > virtio: pci: struct virtio_pci_common_cfg add queue_notify_data
> > virtio: queue_reset: add VIRTIO_F_RING_RESET
> > virtio: queue_reset: pci: update struct virtio_pci_common_cfg and
> > option functions
> > virtio: queue_reset: pci: extract some functions for subsequent
> > patches
> > virtio: queue_reset: pci: support VIRTIO_F_RING_RESET
> > virtio: queue_reset: add helper
> >
> > drivers/virtio/virtio_pci_common.c | 140 +++++++++++++++++++------
> > drivers/virtio/virtio_pci_common.h | 5 +
> > drivers/virtio/virtio_pci_modern.c | 81 ++++++++++++++
> > drivers/virtio/virtio_pci_modern_dev.c | 28 +++++
> > include/linux/virtio_config.h | 78 ++++++++++++++
> > include/linux/virtio_pci_modern.h | 2 +
> > include/uapi/linux/virtio_config.h | 7 +-
> > include/uapi/linux/virtio_pci.h | 2 +
> > 8 files changed, 310 insertions(+), 33 deletions(-)
> >
> > --
> > 2.31.0
> >
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6] virtio: pci: struct virtio_pci_common_cfg add queue_notify_data
2022-01-19 6:00 ` Jason Wang
@ 2022-01-19 6:31 ` Xuan Zhuo
0 siblings, 0 replies; 17+ messages in thread
From: Xuan Zhuo @ 2022-01-19 6:31 UTC (permalink / raw)
To: Jason Wang; +Cc: Michael S. Tsirkin, virtualization
On Wed, 19 Jan 2022 14:00:35 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Wed, Jan 19, 2022 at 10:35 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > 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 */
> > };
>
> Since it's uAPI, do we need another which embed virtio_pci_common_cfg?
If I'm not mistaken, the struct virtio_pci_common_cfg used by
virtio_pci_modern_dev.c is indeed this.
Thanks.
>
> Thanks
>
> >
> > /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
> > --
> > 2.31.0
> >
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6] virtio: queue_reset: pci: extract some functions for subsequent patches
2022-01-19 6:04 ` Jason Wang
@ 2022-01-19 6:33 ` Xuan Zhuo
0 siblings, 0 replies; 17+ messages in thread
From: Xuan Zhuo @ 2022-01-19 6:33 UTC (permalink / raw)
To: Jason Wang; +Cc: Michael S. Tsirkin, virtualization
On Wed, 19 Jan 2022 14:04:51 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Wed, Jan 19, 2022 at 10:35 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > Add a vq_enable_vq_msix() function to enable a vq alone.
> >
> > 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/enable a vq separately.
> >
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > ---
> > drivers/virtio/virtio_pci_common.c | 86 +++++++++++++++++++-----------
> > 1 file changed, 54 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> > index fdbde1db5ec5..5afe207ce28a 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);
> > - }
>
> I think we need a better subject of the patch and it's better to even
> split the patch into two.
>
> This part could be "move the per queue irq logic from vp_del_vqs to vp_del_vq".
OK.
>
> > +
> > vp_dev->per_vq_vectors = false;
> >
> > if (vp_dev->intx_enabled) {
> > @@ -310,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;
> > +}
>
> And this deserves a separate patch as well.
OK
Thanks.
>
> Thanks
>
> > +
> > 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,
> > @@ -319,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)
> > @@ -354,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
> >
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/6] virtio: queue_reset: pci: support VIRTIO_F_RING_RESET
2022-01-19 6:12 ` Jason Wang
@ 2022-01-19 6:33 ` Xuan Zhuo
0 siblings, 0 replies; 17+ messages in thread
From: Xuan Zhuo @ 2022-01-19 6:33 UTC (permalink / raw)
To: Jason Wang; +Cc: Michael S. Tsirkin, virtualization
On Wed, 19 Jan 2022 14:12:55 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Wed, Jan 19, 2022 at 2:11 PM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Wed, Jan 19, 2022 at 10:35 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > >
> > > This patch implements virtio pci support for QUEUE RESET.
> > >
> > > Performing reset on a queue is divided into three steps:
> > >
> > > 1. reset_vq: notify the hardware queue to reset
> > > 2. del_reset_vq: delete the reset queue
> > > 3. enable_reset_vq: re-enable the reset queue
> > >
> > > Between steps 1 and 2, generally call virtqueue_detach_unused_buf() to
> > > recycle the buffer.
> >
> > I wonder if it's better to squash 1, 2 and
> > virtqueue_detach_unused_buf() into a single helper.
> >
> > >
> > > 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 | 54 ++++++++++++++++++++
> > > drivers/virtio/virtio_pci_common.h | 5 ++
> > > drivers/virtio/virtio_pci_modern.c | 81 ++++++++++++++++++++++++++++++
> > > include/linux/virtio_config.h | 21 ++++++++
> > > 4 files changed, 161 insertions(+)
> > >
> > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> > > index 5afe207ce28a..615a5a5f757c 100644
> > > --- a/drivers/virtio/virtio_pci_common.c
> > > +++ b/drivers/virtio/virtio_pci_common.c
> > > @@ -464,6 +464,60 @@ 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))
> > > +
> > > +int 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;
> > > +
> > > + if (VQ_IS_DELETED(vp_dev, queue_index))
> > > + return -EPERM;
> > > +
> > > + 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);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +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..a92c99cc010e 100644
> > > --- a/drivers/virtio/virtio_pci_common.h
> > > +++ b/drivers/virtio/virtio_pci_common.h
> > > @@ -115,6 +115,11 @@ 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_reset_vq(struct virtio_device *vdev, u16 queue_index);
> > > +int 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..74e0b2b2f4dc 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,78 @@ 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)
> > > +{
> > > + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > + struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> > > + u16 msix_vec;
> > > +
> > > + 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);
> > > +
> > > + msix_vec = vp_dev->vqs[queue_index]->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));
> >
> > What happens if the irq is shared among virtqueues?
> >
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int vp_modern_del_reset_vq(struct virtio_device *vdev, u16 queue_index)
> > > +{
> > > + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > > + struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> > > +
> > > + if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
> > > + return -ENOENT;
> > > +
> > > + /* check queue reset status */
> > > + if (vp_modern_get_queue_reset(mdev, queue_index) != 1)
> > > + return -EBUSY;
> > > +
> > > + return vp_del_reset_vq(vdev, queue_index);
> > > +}
> > > +
> > > +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 +470,9 @@ 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,
> > > + .del_reset_vq = vp_modern_del_reset_vq,
> > > + .enable_reset_vq = vp_modern_enable_reset_vq,
> > > };
> > >
> > > static const struct virtio_config_ops virtio_pci_config_ops = {
> > > @@ -413,6 +491,9 @@ 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,
> > > + .del_reset_vq = vp_modern_del_reset_vq,
> > > + .enable_reset_vq = vp_modern_enable_reset_vq,
> > > };
> > >
> > > /* the PCI probing function */
> > > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> > > index 4d107ad31149..6e94f7d0e153 100644
> > > --- a/include/linux/virtio_config.h
> > > +++ b/include/linux/virtio_config.h
> > > @@ -74,6 +74,21 @@ 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
> > > + * returns 0 on success or error status
> > > + * @del_reset_vq: del a reset queue
> > > + * vdev: the device
> > > + * queue_index: the queue index
> > > + * 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 *);
> > > struct virtio_config_ops {
> > > @@ -100,6 +115,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);
> > > + int (*del_reset_vq)(struct virtio_device *vdev, u16 queue_index);
>
> Btw, it's better to do this in a separate patch.
>
OK, my next version will come out with a separate patch.
Thanks.
> Thanks
>
> > > + 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
> > >
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/6] virtio: queue_reset: pci: support VIRTIO_F_RING_RESET
2022-01-19 6:11 ` Jason Wang
2022-01-19 6:12 ` Jason Wang
@ 2022-01-19 6:36 ` Xuan Zhuo
1 sibling, 0 replies; 17+ messages in thread
From: Xuan Zhuo @ 2022-01-19 6:36 UTC (permalink / raw)
To: Jason Wang; +Cc: Michael S. Tsirkin, virtualization
On Wed, 19 Jan 2022 14:11:29 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Wed, Jan 19, 2022 at 10:35 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > This patch implements virtio pci support for QUEUE RESET.
> >
> > Performing reset on a queue is divided into three steps:
> >
> > 1. reset_vq: notify the hardware queue to reset
> > 2. del_reset_vq: delete the reset queue
> > 3. enable_reset_vq: re-enable the reset queue
> >
> > Between steps 1 and 2, generally call virtqueue_detach_unused_buf() to
> > recycle the buffer.
>
> I wonder if it's better to squash 1, 2 and
> virtqueue_detach_unused_buf() into a single helper.
Oh, that's a good note. The only trouble is that the caller has to provide a
callback to handle the reclaimed buffer.
Thanks.
>
> >
> > 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 | 54 ++++++++++++++++++++
> > drivers/virtio/virtio_pci_common.h | 5 ++
> > drivers/virtio/virtio_pci_modern.c | 81 ++++++++++++++++++++++++++++++
> > include/linux/virtio_config.h | 21 ++++++++
> > 4 files changed, 161 insertions(+)
> >
> > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> > index 5afe207ce28a..615a5a5f757c 100644
> > --- a/drivers/virtio/virtio_pci_common.c
> > +++ b/drivers/virtio/virtio_pci_common.c
> > @@ -464,6 +464,60 @@ 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))
> > +
> > +int 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;
> > +
> > + if (VQ_IS_DELETED(vp_dev, queue_index))
> > + return -EPERM;
> > +
> > + 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);
> > +
> > + return 0;
> > +}
> > +
> > +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..a92c99cc010e 100644
> > --- a/drivers/virtio/virtio_pci_common.h
> > +++ b/drivers/virtio/virtio_pci_common.h
> > @@ -115,6 +115,11 @@ 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_reset_vq(struct virtio_device *vdev, u16 queue_index);
> > +int 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..74e0b2b2f4dc 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,78 @@ 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)
> > +{
> > + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > + struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> > + u16 msix_vec;
> > +
> > + 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);
> > +
> > + msix_vec = vp_dev->vqs[queue_index]->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));
>
> What happens if the irq is shared among virtqueues?
>
> > +
> > + return 0;
> > +}
> > +
> > +static int vp_modern_del_reset_vq(struct virtio_device *vdev, u16 queue_index)
> > +{
> > + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > + struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> > +
> > + if (!virtio_has_feature(vdev, VIRTIO_F_RING_RESET))
> > + return -ENOENT;
> > +
> > + /* check queue reset status */
> > + if (vp_modern_get_queue_reset(mdev, queue_index) != 1)
> > + return -EBUSY;
> > +
> > + return vp_del_reset_vq(vdev, queue_index);
> > +}
> > +
> > +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 +470,9 @@ 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,
> > + .del_reset_vq = vp_modern_del_reset_vq,
> > + .enable_reset_vq = vp_modern_enable_reset_vq,
> > };
> >
> > static const struct virtio_config_ops virtio_pci_config_ops = {
> > @@ -413,6 +491,9 @@ 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,
> > + .del_reset_vq = vp_modern_del_reset_vq,
> > + .enable_reset_vq = vp_modern_enable_reset_vq,
> > };
> >
> > /* the PCI probing function */
> > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> > index 4d107ad31149..6e94f7d0e153 100644
> > --- a/include/linux/virtio_config.h
> > +++ b/include/linux/virtio_config.h
> > @@ -74,6 +74,21 @@ 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
> > + * returns 0 on success or error status
> > + * @del_reset_vq: del a reset queue
> > + * vdev: the device
> > + * queue_index: the queue index
> > + * 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 *);
> > struct virtio_config_ops {
> > @@ -100,6 +115,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);
> > + int (*del_reset_vq)(struct virtio_device *vdev, u16 queue_index);
> > + 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
> >
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-01-19 6:37 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-19 2:35 [PATCH 0/6] virtio pci support VIRTIO_F_RING_RESET Xuan Zhuo
2022-01-19 2:35 ` [PATCH 1/6] virtio: pci: struct virtio_pci_common_cfg add queue_notify_data Xuan Zhuo
2022-01-19 6:00 ` Jason Wang
2022-01-19 6:31 ` Xuan Zhuo
2022-01-19 2:35 ` [PATCH 2/6] virtio: queue_reset: add VIRTIO_F_RING_RESET Xuan Zhuo
2022-01-19 2:35 ` [PATCH 3/6] virtio: queue_reset: pci: update struct virtio_pci_common_cfg and option functions Xuan Zhuo
2022-01-19 2:35 ` [PATCH 4/6] virtio: queue_reset: pci: extract some functions for subsequent patches Xuan Zhuo
2022-01-19 6:04 ` Jason Wang
2022-01-19 6:33 ` Xuan Zhuo
2022-01-19 2:35 ` [PATCH 5/6] virtio: queue_reset: pci: support VIRTIO_F_RING_RESET Xuan Zhuo
2022-01-19 6:11 ` Jason Wang
2022-01-19 6:12 ` Jason Wang
2022-01-19 6:33 ` Xuan Zhuo
2022-01-19 6:36 ` Xuan Zhuo
2022-01-19 2:35 ` [PATCH 6/6] virtio: queue_reset: add helper Xuan Zhuo
2022-01-19 6:13 ` [PATCH 0/6] virtio pci support VIRTIO_F_RING_RESET Jason Wang
2022-01-19 6:22 ` Xuan Zhuo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.