[PATCH v3 00/15] Support VIRTIO_F_RING_RESET for virtio-net, vhost-net kernel in virtio pci-modern

[PATCH v3 00/15] Support VIRTIO_F_RING_RESET for virtio-net, vhost-net kernel in virtio pci-modern

[Kangjie Xu]

The virtio queue reset function has already been defined in the virtio spec 1.2.
The relevant virtio spec information is here:

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

This patch set is to support this function in QEMU. It consists of several parts:
1. Patches 1-7 are the basic interfaces for vq reset in virtio and virtio-pci.
2. Patches 8-11 support vq reset and vq restart for vhost-kernel.
3. Patches 12-14 support vq reset and vq restart for virtio-net.
5. Patch 15 enables the vq reset feature for vhost-kernel.

The process of virtqueue reset can be concluded as:
1. The virtqueue is disabled when VIRTIO_PCI_COMMON_Q_RESET is written.
2. Then the virtqueue can be optionally restarted(re-enabled).

Since this patch set involves multiple modules and seems a bit messy, we briefly describe the
calling process for different modes below.
virtio-net:
1. VIRTIO_PCI_COMMON_Q_RESET is written [virtio-pci]
    -> virtio_queue_reset() [virtio]
        -> virtio_net_queue_reset() [virtio-net]
        -> __virtio_queue_reset()
2. VIRTIO_PCI_COMMON_Q_ENABLE is written [virtio-pci]
    -> set enabled, reset status of vq.

vhost-kernel:
1. VIRTIO_PCI_COMMON_Q_RESET is written [virtio-pci]
    -> virtio_queue_reset() [virtio]
        -> virtio_net_queue_reset() [virtio-net]
            -> vhost_net_virtqueue_stop() [vhost-net]
                -> vhost_net_set_backend() [vhost]
                -> vhost_virtqueue_unmap()
        -> __virtio_queue_reset()
2. VIRTIO_PCI_COMMON_Q_ENABLE is written [virtio-pci]
    -> virtio_queue_enable() [virtio]
        -> virtio_net_queue_enable() [virtio-net]
            -> vhost_net_virtqueue_restart() [vhost-net]
                -> vhost_virtqueue_start() [vhost]
                -> vhost_net_set_backend()
    -> set enabled, reset status of vq.


Test environment and method:
    Host: 5.19.0-rc3 (With vq reset support)
    Qemu: QEMU emulator version 7.0.50
    Guest: 5.19.0-rc3 (With vq reset support)
    Test Cmd: ethtool -g eth1; ethtool -G eth1 rx $1 tx $2; ethtool -g eth1;

    The drvier can resize the virtio queue, then virtio queue reset function should
    be triggered.

    The default is split mode, modify Qemu virtio-net to add PACKED feature to 
    test packed mode.

Guest Kernel Patch:
    https://lore.kernel.org/bpf/20220801063902.129329-1-xuanzhuo@linux.alibaba.com/

Host Kernel Patch:
    https://github.com/middaywords/linux/commit/a845098f0df6b8bdf7e1e5db57af6ebd1c8eaf47

Looking forward to your review and comments. Thanks.

changelog:
v3:
  1. Remove support for vhost-user in this series and refactor the code.
  2. Rename 'vhost_net_virtqueue_stop' to 'vhost_net_virtqueue_reset'.
  3. Make PCI transport ready before device ready when queue_enabled is set to true.
  3. Add some comments.

v2:
  1. Add support for vhost-net kernel scenario.
  2. Add a new vhost-user message VHOST_USER_RESET_VRING.
  3. Add migration compatibility for virtqueue reset.

Kangjie Xu (10):
  virtio: introduce virtio_queue_enable()
  virtio: core: vq reset feature negotation support
  virtio-pci: support queue enable
  vhost: extract the logic of unmapping the vrings and desc
  vhost: expose vhost_virtqueue_start()
  vhost-net: vhost-kernel: introduce vhost_net_virtqueue_reset()
  vhost-net: vhost-kernel: introduce vhost_net_virtqueue_restart()
  virtio-net: introduce flush_or_purge_queued_packets()
  virtio-net: support queue_enable
  vhost: vhost-kernel: enable vq reset feature

Xuan Zhuo (5):
  virtio: sync relevant definitions with linux
  virtio: introduce __virtio_queue_reset()
  virtio: introduce virtio_queue_reset()
  virtio-pci: support queue reset
  virtio-net: support queue reset

 hw/core/machine.c                             |  1 +
 hw/net/vhost_net.c                            | 75 +++++++++++++++++++
 hw/net/virtio-net.c                           | 56 ++++++++++++--
 hw/virtio/vhost.c                             | 28 ++++---
 hw/virtio/virtio-pci.c                        | 16 ++++
 hw/virtio/virtio.c                            | 62 +++++++++++----
 include/hw/virtio/vhost.h                     |  5 ++
 include/hw/virtio/virtio-pci.h                |  5 ++
 include/hw/virtio/virtio.h                    |  8 +-
 include/net/vhost_net.h                       |  4 +
 .../standard-headers/linux/virtio_config.h    |  5 ++
 include/standard-headers/linux/virtio_pci.h   |  2 +
 12 files changed, 234 insertions(+), 33 deletions(-)

-- 
2.32.0

[PATCH v3 01/15] virtio: sync relevant definitions with linux

[Kangjie Xu]

From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>

This is updated using scripts/update-linux-headers.sh.

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

Add VIRTIO_PCI_COMMON_Q_NDATA, which comes from here:
https://github.com/oasis-tcs/virtio-spec/issues/89

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
---
 include/standard-headers/linux/virtio_config.h | 5 +++++
 include/standard-headers/linux/virtio_pci.h    | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/include/standard-headers/linux/virtio_config.h b/include/standard-headers/linux/virtio_config.h
index 7acd8d4abc..47a7eef5e4 100644
--- a/include/standard-headers/linux/virtio_config.h
+++ b/include/standard-headers/linux/virtio_config.h
@@ -96,4 +96,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 /* _LINUX_VIRTIO_CONFIG_H */
diff --git a/include/standard-headers/linux/virtio_pci.h b/include/standard-headers/linux/virtio_pci.h
index db7a8e2fcb..be912cfc95 100644
--- a/include/standard-headers/linux/virtio_pci.h
+++ b/include/standard-headers/linux/virtio_pci.h
@@ -202,6 +202,8 @@ struct virtio_pci_cfg_cap {
 #define VIRTIO_PCI_COMMON_Q_AVAILHI	44
 #define VIRTIO_PCI_COMMON_Q_USEDLO	48
 #define VIRTIO_PCI_COMMON_Q_USEDHI	52
+#define VIRTIO_PCI_COMMON_Q_NDATA	56
+#define VIRTIO_PCI_COMMON_Q_RESET	58
 
 #endif /* VIRTIO_PCI_NO_MODERN */
 
-- 
2.32.0

Re: [PATCH v3 01/15] virtio: sync relevant definitions with linux

[Jason Wang]

在 2022/8/25 16:08, Kangjie Xu 写道:
> From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>
> This is updated using scripts/update-linux-headers.sh.
>
> Added VIRTIO_F_RING_RESET, VIRTIO_PCI_COMMON_Q_RESET. It came from here:
> https://github.com/oasis-tcs/virtio-spec/issues/124
> https://github.com/oasis-tcs/virtio-spec/issues/139
>
> Add VIRTIO_PCI_COMMON_Q_NDATA, which comes from here:
> https://github.com/oasis-tcs/virtio-spec/issues/89
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>


Acked-by: Jason Wang <jasowang@redhat.com>


> ---
>   include/standard-headers/linux/virtio_config.h | 5 +++++
>   include/standard-headers/linux/virtio_pci.h    | 2 ++
>   2 files changed, 7 insertions(+)
>
> diff --git a/include/standard-headers/linux/virtio_config.h b/include/standard-headers/linux/virtio_config.h
> index 7acd8d4abc..47a7eef5e4 100644
> --- a/include/standard-headers/linux/virtio_config.h
> +++ b/include/standard-headers/linux/virtio_config.h
> @@ -96,4 +96,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 /* _LINUX_VIRTIO_CONFIG_H */
> diff --git a/include/standard-headers/linux/virtio_pci.h b/include/standard-headers/linux/virtio_pci.h
> index db7a8e2fcb..be912cfc95 100644
> --- a/include/standard-headers/linux/virtio_pci.h
> +++ b/include/standard-headers/linux/virtio_pci.h
> @@ -202,6 +202,8 @@ struct virtio_pci_cfg_cap {
>   #define VIRTIO_PCI_COMMON_Q_AVAILHI	44
>   #define VIRTIO_PCI_COMMON_Q_USEDLO	48
>   #define VIRTIO_PCI_COMMON_Q_USEDHI	52
> +#define VIRTIO_PCI_COMMON_Q_NDATA	56
> +#define VIRTIO_PCI_COMMON_Q_RESET	58
>   
>   #endif /* VIRTIO_PCI_NO_MODERN */
>   

[PATCH v3 02/15] virtio: introduce __virtio_queue_reset()

[Kangjie Xu]

From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>

Separate the logic of vq reset. This logic will be called directly
later.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio/virtio.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 5d607aeaa0..67d54832a9 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2019,6 +2019,26 @@ static enum virtio_device_endian virtio_current_cpu_endian(void)
     }
 }
 
+static void __virtio_queue_reset(VirtIODevice *vdev, uint32_t i)
+{
+    vdev->vq[i].vring.desc = 0;
+    vdev->vq[i].vring.avail = 0;
+    vdev->vq[i].vring.used = 0;
+    vdev->vq[i].last_avail_idx = 0;
+    vdev->vq[i].shadow_avail_idx = 0;
+    vdev->vq[i].used_idx = 0;
+    vdev->vq[i].last_avail_wrap_counter = true;
+    vdev->vq[i].shadow_avail_wrap_counter = true;
+    vdev->vq[i].used_wrap_counter = true;
+    virtio_queue_set_vector(vdev, i, VIRTIO_NO_VECTOR);
+    vdev->vq[i].signalled_used = 0;
+    vdev->vq[i].signalled_used_valid = false;
+    vdev->vq[i].notification = true;
+    vdev->vq[i].vring.num = vdev->vq[i].vring.num_default;
+    vdev->vq[i].inuse = 0;
+    virtio_virtqueue_reset_region_cache(&vdev->vq[i]);
+}
+
 void virtio_reset(void *opaque)
 {
     VirtIODevice *vdev = opaque;
@@ -2050,22 +2070,7 @@ void virtio_reset(void *opaque)
     virtio_notify_vector(vdev, vdev->config_vector);
 
     for(i = 0; i < VIRTIO_QUEUE_MAX; i++) {
-        vdev->vq[i].vring.desc = 0;
-        vdev->vq[i].vring.avail = 0;
-        vdev->vq[i].vring.used = 0;
-        vdev->vq[i].last_avail_idx = 0;
-        vdev->vq[i].shadow_avail_idx = 0;
-        vdev->vq[i].used_idx = 0;
-        vdev->vq[i].last_avail_wrap_counter = true;
-        vdev->vq[i].shadow_avail_wrap_counter = true;
-        vdev->vq[i].used_wrap_counter = true;
-        virtio_queue_set_vector(vdev, i, VIRTIO_NO_VECTOR);
-        vdev->vq[i].signalled_used = 0;
-        vdev->vq[i].signalled_used_valid = false;
-        vdev->vq[i].notification = true;
-        vdev->vq[i].vring.num = vdev->vq[i].vring.num_default;
-        vdev->vq[i].inuse = 0;
-        virtio_virtqueue_reset_region_cache(&vdev->vq[i]);
+        __virtio_queue_reset(vdev, i);
     }
 }
 
-- 
2.32.0

[PATCH v3 03/15] virtio: introduce virtio_queue_reset()

[Kangjie Xu]

From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>

Introduce a new interface function virtio_queue_reset() to implement
reset for vq.

Add a new callback to VirtioDeviceClass for queue reset operation for
each child device.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio/virtio.c         | 11 +++++++++++
 include/hw/virtio/virtio.h |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 67d54832a9..0e9d41366f 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2039,6 +2039,17 @@ static void __virtio_queue_reset(VirtIODevice *vdev, uint32_t i)
     virtio_virtqueue_reset_region_cache(&vdev->vq[i]);
 }
 
+void virtio_queue_reset(VirtIODevice *vdev, uint32_t queue_index)
+{
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+
+    if (k->queue_reset) {
+        k->queue_reset(vdev, queue_index);
+    }
+
+    __virtio_queue_reset(vdev, queue_index);
+}
+
 void virtio_reset(void *opaque)
 {
     VirtIODevice *vdev = opaque;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index db1c0ddf6b..879394299b 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -130,6 +130,7 @@ struct VirtioDeviceClass {
     void (*set_config)(VirtIODevice *vdev, const uint8_t *config);
     void (*reset)(VirtIODevice *vdev);
     void (*set_status)(VirtIODevice *vdev, uint8_t val);
+    void (*queue_reset)(VirtIODevice *vdev, uint32_t queue_index);
     /* For transitional devices, this is a bitmap of features
      * that are only exposed on the legacy interface but not
      * the modern one.
@@ -268,6 +269,7 @@ int virtio_queue_set_host_notifier_mr(VirtIODevice *vdev, int n,
                                       MemoryRegion *mr, bool assign);
 int virtio_set_status(VirtIODevice *vdev, uint8_t val);
 void virtio_reset(void *opaque);
+void virtio_queue_reset(VirtIODevice *vdev, uint32_t queue_index);
 void virtio_update_irq(VirtIODevice *vdev);
 int virtio_set_features(VirtIODevice *vdev, uint64_t val);
 
-- 
2.32.0

[PATCH v3 04/15] virtio: introduce virtio_queue_enable()

[Kangjie Xu]

Introduce the interface queue_enable() in VirtioDeviceClass and the
fucntion virtio_queue_enable() in virtio, it can be called when
VIRTIO_PCI_COMMON_Q_ENABLE is written and related virtqueue can be
started. It only supports the devices of virtio 1 or later. The
not-supported devices can only start the virtqueue when DRIVER_OK.

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio/virtio.c         | 14 ++++++++++++++
 include/hw/virtio/virtio.h |  2 ++
 2 files changed, 16 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 0e9d41366f..141f18c633 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2050,6 +2050,20 @@ void virtio_queue_reset(VirtIODevice *vdev, uint32_t queue_index)
     __virtio_queue_reset(vdev, queue_index);
 }
 
+void virtio_queue_enable(VirtIODevice *vdev, uint32_t queue_index)
+{
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+
+    if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+        error_report("queue_enable is only suppported in devices of virtio "
+                     "1.0 or later.");
+    }
+
+    if (k->queue_enable) {
+        k->queue_enable(vdev, queue_index);
+    }
+}
+
 void virtio_reset(void *opaque)
 {
     VirtIODevice *vdev = opaque;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 879394299b..085997d8f3 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -131,6 +131,7 @@ struct VirtioDeviceClass {
     void (*reset)(VirtIODevice *vdev);
     void (*set_status)(VirtIODevice *vdev, uint8_t val);
     void (*queue_reset)(VirtIODevice *vdev, uint32_t queue_index);
+    void (*queue_enable)(VirtIODevice *vdev, uint32_t queue_index);
     /* For transitional devices, this is a bitmap of features
      * that are only exposed on the legacy interface but not
      * the modern one.
@@ -270,6 +271,7 @@ int virtio_queue_set_host_notifier_mr(VirtIODevice *vdev, int n,
 int virtio_set_status(VirtIODevice *vdev, uint8_t val);
 void virtio_reset(void *opaque);
 void virtio_queue_reset(VirtIODevice *vdev, uint32_t queue_index);
+void virtio_queue_enable(VirtIODevice *vdev, uint32_t queue_index);
 void virtio_update_irq(VirtIODevice *vdev);
 int virtio_set_features(VirtIODevice *vdev, uint64_t val);
 
-- 
2.32.0

[PATCH v3 05/15] virtio: core: vq reset feature negotation support

[Kangjie Xu]

A a new command line parameter "queue_reset" is added.

Meanwhile, the vq reset feature is disabled for pre-7.1 machines.

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 hw/core/machine.c          | 1 +
 include/hw/virtio/virtio.h | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index a673302cce..8b22b4647f 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -43,6 +43,7 @@
 GlobalProperty hw_compat_7_0[] = {
     { "arm-gicv3-common", "force-8-bit-prio", "on" },
     { "nvme-ns", "eui64-default", "on"},
+    { "virtio-device", "queue_reset", "false" },
 };
 const size_t hw_compat_7_0_len = G_N_ELEMENTS(hw_compat_7_0);
 
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 085997d8f3..ed3ecbef80 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -295,7 +295,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
     DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
                       VIRTIO_F_IOMMU_PLATFORM, false), \
     DEFINE_PROP_BIT64("packed", _state, _field, \
-                      VIRTIO_F_RING_PACKED, false)
+                      VIRTIO_F_RING_PACKED, false), \
+    DEFINE_PROP_BIT64("queue_reset", _state, _field, \
+                      VIRTIO_F_RING_RESET, true)
 
 hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
 bool virtio_queue_enabled_legacy(VirtIODevice *vdev, int n);
-- 
2.32.0

[PATCH v3 06/15] virtio-pci: support queue reset

[Kangjie Xu]

From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>

PCI devices support vq reset.

Based on this function, the driver can adjust the size of the ring, and
quickly recycle the buffer in the ring.

The migration of the virtio devices will not happen during a reset
operation. This is becuase the global iothread lock is held. Migration
thread also needs the lock. As a result, when migration of virtio
devices starts, the 'reset' status of VirtIOPCIQueue will always be 0.
Thus, we do not need to add it in vmstate_virtio_pci_modern_queue_state.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
---
 hw/virtio/virtio-pci.c         | 15 +++++++++++++++
 include/hw/virtio/virtio-pci.h |  5 +++++
 2 files changed, 20 insertions(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index a50c5a57d7..79b9e641dd 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1251,6 +1251,9 @@ static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
     case VIRTIO_PCI_COMMON_Q_USEDHI:
         val = proxy->vqs[vdev->queue_sel].used[1];
         break;
+    case VIRTIO_PCI_COMMON_Q_RESET:
+        val = proxy->vqs[vdev->queue_sel].reset;
+        break;
     default:
         val = 0;
     }
@@ -1338,6 +1341,7 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr,
                        ((uint64_t)proxy->vqs[vdev->queue_sel].used[1]) << 32 |
                        proxy->vqs[vdev->queue_sel].used[0]);
             proxy->vqs[vdev->queue_sel].enabled = 1;
+            proxy->vqs[vdev->queue_sel].reset = 0;
         } else {
             virtio_error(vdev, "wrong value for queue_enable %"PRIx64, val);
         }
@@ -1360,6 +1364,16 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr,
     case VIRTIO_PCI_COMMON_Q_USEDHI:
         proxy->vqs[vdev->queue_sel].used[1] = val;
         break;
+    case VIRTIO_PCI_COMMON_Q_RESET:
+        if (val == 1) {
+            proxy->vqs[vdev->queue_sel].reset = 1;
+
+            virtio_queue_reset(vdev, vdev->queue_sel);
+
+            proxy->vqs[vdev->queue_sel].reset = 0;
+            proxy->vqs[vdev->queue_sel].enabled = 0;
+        }
+        break;
     default:
         break;
     }
@@ -1954,6 +1968,7 @@ static void virtio_pci_reset(DeviceState *qdev)
 
     for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
         proxy->vqs[i].enabled = 0;
+        proxy->vqs[i].reset = 0;
         proxy->vqs[i].num = 0;
         proxy->vqs[i].desc[0] = proxy->vqs[i].desc[1] = 0;
         proxy->vqs[i].avail[0] = proxy->vqs[i].avail[1] = 0;
diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 2446dcd9ae..938799e8f6 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -117,6 +117,11 @@ typedef struct VirtIOPCIRegion {
 typedef struct VirtIOPCIQueue {
   uint16_t num;
   bool enabled;
+  /*
+   * No need to migrate the reset status, because it is always 0
+   * when the migration starts.
+   */
+  bool reset;
   uint32_t desc[2];
   uint32_t avail[2];
   uint32_t used[2];
-- 
2.32.0

Re: [PATCH v3 06/15] virtio-pci: support queue reset

[Jason Wang]

在 2022/8/25 16:08, Kangjie Xu 写道:
> From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>
> PCI devices support vq reset.
>
> Based on this function, the driver can adjust the size of the ring, and
> quickly recycle the buffer in the ring.
>
> The migration of the virtio devices will not happen during a reset
> operation. This is becuase the global iothread lock is held. Migration
> thread also needs the lock. As a result, when migration of virtio
> devices starts, the 'reset' status of VirtIOPCIQueue will always be 0.
> Thus, we do not need to add it in vmstate_virtio_pci_modern_queue_state.
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>


Acked-by: Jason Wang <jasowang@redhat.com>


> ---
>   hw/virtio/virtio-pci.c         | 15 +++++++++++++++
>   include/hw/virtio/virtio-pci.h |  5 +++++
>   2 files changed, 20 insertions(+)
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index a50c5a57d7..79b9e641dd 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1251,6 +1251,9 @@ static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
>       case VIRTIO_PCI_COMMON_Q_USEDHI:
>           val = proxy->vqs[vdev->queue_sel].used[1];
>           break;
> +    case VIRTIO_PCI_COMMON_Q_RESET:
> +        val = proxy->vqs[vdev->queue_sel].reset;
> +        break;
>       default:
>           val = 0;
>       }
> @@ -1338,6 +1341,7 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr,
>                          ((uint64_t)proxy->vqs[vdev->queue_sel].used[1]) << 32 |
>                          proxy->vqs[vdev->queue_sel].used[0]);
>               proxy->vqs[vdev->queue_sel].enabled = 1;
> +            proxy->vqs[vdev->queue_sel].reset = 0;
>           } else {
>               virtio_error(vdev, "wrong value for queue_enable %"PRIx64, val);
>           }
> @@ -1360,6 +1364,16 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr,
>       case VIRTIO_PCI_COMMON_Q_USEDHI:
>           proxy->vqs[vdev->queue_sel].used[1] = val;
>           break;
> +    case VIRTIO_PCI_COMMON_Q_RESET:
> +        if (val == 1) {
> +            proxy->vqs[vdev->queue_sel].reset = 1;
> +
> +            virtio_queue_reset(vdev, vdev->queue_sel);
> +
> +            proxy->vqs[vdev->queue_sel].reset = 0;
> +            proxy->vqs[vdev->queue_sel].enabled = 0;
> +        }
> +        break;
>       default:
>           break;
>       }
> @@ -1954,6 +1968,7 @@ static void virtio_pci_reset(DeviceState *qdev)
>   
>       for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
>           proxy->vqs[i].enabled = 0;
> +        proxy->vqs[i].reset = 0;
>           proxy->vqs[i].num = 0;
>           proxy->vqs[i].desc[0] = proxy->vqs[i].desc[1] = 0;
>           proxy->vqs[i].avail[0] = proxy->vqs[i].avail[1] = 0;
> diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
> index 2446dcd9ae..938799e8f6 100644
> --- a/include/hw/virtio/virtio-pci.h
> +++ b/include/hw/virtio/virtio-pci.h
> @@ -117,6 +117,11 @@ typedef struct VirtIOPCIRegion {
>   typedef struct VirtIOPCIQueue {
>     uint16_t num;
>     bool enabled;
> +  /*
> +   * No need to migrate the reset status, because it is always 0
> +   * when the migration starts.
> +   */
> +  bool reset;
>     uint32_t desc[2];
>     uint32_t avail[2];
>     uint32_t used[2];

[PATCH v3 07/15] virtio-pci: support queue enable

[Kangjie Xu]

PCI devices support device specific vq enable.

Based on this function, the driver can re-enable the virtqueue after the
virtqueue is reset.

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 hw/virtio/virtio-pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 79b9e641dd..a53b5d9f1e 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1342,6 +1342,7 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr,
                        proxy->vqs[vdev->queue_sel].used[0]);
             proxy->vqs[vdev->queue_sel].enabled = 1;
             proxy->vqs[vdev->queue_sel].reset = 0;
+            virtio_queue_enable(vdev, vdev->queue_sel);
         } else {
             virtio_error(vdev, "wrong value for queue_enable %"PRIx64, val);
         }
-- 
2.32.0

Re: [PATCH v3 07/15] virtio-pci: support queue enable

[Jason Wang]

在 2022/8/25 16:08, Kangjie Xu 写道:
> PCI devices support device specific vq enable.
>
> Based on this function, the driver can re-enable the virtqueue after the
> virtqueue is reset.
>
> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>


Acked-by: Jason Wang <jasowang@redhat.com>


> ---
>   hw/virtio/virtio-pci.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 79b9e641dd..a53b5d9f1e 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1342,6 +1342,7 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr,
>                          proxy->vqs[vdev->queue_sel].used[0]);
>               proxy->vqs[vdev->queue_sel].enabled = 1;
>               proxy->vqs[vdev->queue_sel].reset = 0;
> +            virtio_queue_enable(vdev, vdev->queue_sel);
>           } else {
>               virtio_error(vdev, "wrong value for queue_enable %"PRIx64, val);
>           }

[PATCH v3 08/15] vhost: extract the logic of unmapping the vrings and desc

[Kangjie Xu]

Introduce vhost_virtqueue_unmap() to ummap the vrings and desc
of a virtqueue.

The function will be re-used when resetting a virtqueue.

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio/vhost.c         | 20 ++++++++++++++------
 include/hw/virtio/vhost.h |  3 +++
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index f758f177bb..2419521c36 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1201,6 +1201,19 @@ fail_alloc_desc:
     return r;
 }
 
+void vhost_virtqueue_unmap(struct vhost_dev *dev,
+                           struct VirtIODevice *vdev,
+                           struct vhost_virtqueue *vq,
+                           unsigned idx)
+{
+    vhost_memory_unmap(dev, vq->used, virtio_queue_get_used_size(vdev, idx),
+                       1, virtio_queue_get_used_size(vdev, idx));
+    vhost_memory_unmap(dev, vq->avail, virtio_queue_get_avail_size(vdev, idx),
+                       0, virtio_queue_get_avail_size(vdev, idx));
+    vhost_memory_unmap(dev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
+                       0, virtio_queue_get_desc_size(vdev, idx));
+}
+
 static void vhost_virtqueue_stop(struct vhost_dev *dev,
                                     struct VirtIODevice *vdev,
                                     struct vhost_virtqueue *vq,
@@ -1239,12 +1252,7 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
                                                 vhost_vq_index);
     }
 
-    vhost_memory_unmap(dev, vq->used, virtio_queue_get_used_size(vdev, idx),
-                       1, virtio_queue_get_used_size(vdev, idx));
-    vhost_memory_unmap(dev, vq->avail, virtio_queue_get_avail_size(vdev, idx),
-                       0, virtio_queue_get_avail_size(vdev, idx));
-    vhost_memory_unmap(dev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
-                       0, virtio_queue_get_desc_size(vdev, idx));
+    vhost_virtqueue_unmap(dev, vdev, vq, idx);
 }
 
 static void vhost_eventfd_add(MemoryListener *listener,
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index a346f23d13..d9adbca584 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -279,6 +279,9 @@ int vhost_net_set_backend(struct vhost_dev *hdev,
 
 int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write);
 
+void vhost_virtqueue_unmap(struct vhost_dev *dev, struct VirtIODevice *vdev,
+                           struct vhost_virtqueue *vq, unsigned idx);
+
 void vhost_dev_reset_inflight(struct vhost_inflight *inflight);
 void vhost_dev_free_inflight(struct vhost_inflight *inflight);
 void vhost_dev_save_inflight(struct vhost_inflight *inflight, QEMUFile *f);
-- 
2.32.0

[PATCH v3 09/15] vhost: expose vhost_virtqueue_start()

[Kangjie Xu]

Expose vhost_virtqueue_start(), we need to use it when restarting a
virtqueue.

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 hw/virtio/vhost.c         | 8 ++++----
 include/hw/virtio/vhost.h | 2 ++
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 2419521c36..8bf9ed952e 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1081,10 +1081,10 @@ out:
     return ret;
 }
 
-static int vhost_virtqueue_start(struct vhost_dev *dev,
-                                struct VirtIODevice *vdev,
-                                struct vhost_virtqueue *vq,
-                                unsigned idx)
+int vhost_virtqueue_start(struct vhost_dev *dev,
+                          struct VirtIODevice *vdev,
+                          struct vhost_virtqueue *vq,
+                          unsigned idx)
 {
     BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
     VirtioBusState *vbus = VIRTIO_BUS(qbus);
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index d9adbca584..7b4ffc522b 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -279,6 +279,8 @@ int vhost_net_set_backend(struct vhost_dev *hdev,
 
 int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write);
 
+int vhost_virtqueue_start(struct vhost_dev *dev, struct VirtIODevice *vdev,
+                          struct vhost_virtqueue *vq, unsigned idx);
 void vhost_virtqueue_unmap(struct vhost_dev *dev, struct VirtIODevice *vdev,
                            struct vhost_virtqueue *vq, unsigned idx);
 
-- 
2.32.0

[PATCH v3 10/15] vhost-net: vhost-kernel: introduce vhost_net_virtqueue_reset()

[Kangjie Xu]

Introduce vhost_virtqueue_reset(), which can reset the specific
virtqueue in the device. Then it will unmap vrings and the desc
of the virtqueue.

Here we do not reuse the vhost_net_stop_one() or vhost_dev_stop(),
because they work at queue pair level. We do not use
vhost_virtqueue_stop() because it may stop the device in the
backend.

This patch only considers the case of vhost-kernel, when
NetClientDriver is NET_CLIENT_DRIVER_TAP.

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 hw/net/vhost_net.c      | 22 ++++++++++++++++++++++
 include/net/vhost_net.h |  2 ++
 2 files changed, 24 insertions(+)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index ccac5b7a64..be51be98b3 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -514,3 +514,25 @@ int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu)
 
     return vhost_ops->vhost_net_set_mtu(&net->dev, mtu);
 }
+
+void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
+                               int vq_index)
+{
+    VHostNetState *net = get_vhost_net(nc->peer);
+    const VhostOps *vhost_ops = net->dev.vhost_ops;
+    struct vhost_vring_file file = { .fd = -1 };
+    int idx;
+
+    /* should only be called after backend is connected */
+    assert(vhost_ops);
+
+    idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
+
+    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
+        file.index = idx;
+        int r = vhost_net_set_backend(&net->dev, &file);
+        assert(r >= 0);
+    }
+
+    vhost_virtqueue_unmap(&net->dev, vdev, net->dev.vqs + idx, idx);
+}
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 387e913e4e..85d85a4957 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -48,4 +48,6 @@ uint64_t vhost_net_get_acked_features(VHostNetState *net);
 
 int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu);
 
+void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
+                               int vq_index);
 #endif
-- 
2.32.0

Re: [PATCH v3 10/15] vhost-net: vhost-kernel: introduce vhost_net_virtqueue_reset()

[Jason Wang]

在 2022/8/25 16:08, Kangjie Xu 写道:
> Introduce vhost_virtqueue_reset(), which can reset the specific
> virtqueue in the device. Then it will unmap vrings and the desc
> of the virtqueue.
>
> Here we do not reuse the vhost_net_stop_one() or vhost_dev_stop(),
> because they work at queue pair level. We do not use
> vhost_virtqueue_stop() because it may stop the device in the
> backend.


So I think this is not true at least for vhost-net kernel baceknd.


>
> This patch only considers the case of vhost-kernel, when
> NetClientDriver is NET_CLIENT_DRIVER_TAP.
>
> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>   hw/net/vhost_net.c      | 22 ++++++++++++++++++++++
>   include/net/vhost_net.h |  2 ++
>   2 files changed, 24 insertions(+)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index ccac5b7a64..be51be98b3 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -514,3 +514,25 @@ int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu)
>   
>       return vhost_ops->vhost_net_set_mtu(&net->dev, mtu);
>   }
> +
> +void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
> +                               int vq_index)
> +{
> +    VHostNetState *net = get_vhost_net(nc->peer);
> +    const VhostOps *vhost_ops = net->dev.vhost_ops;
> +    struct vhost_vring_file file = { .fd = -1 };
> +    int idx;
> +
> +    /* should only be called after backend is connected */
> +    assert(vhost_ops);
> +
> +    idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
> +
> +    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
> +        file.index = idx;
> +        int r = vhost_net_set_backend(&net->dev, &file);
> +        assert(r >= 0);
> +    }


Do we need to reset e.g last_avail_idx here?

Thanks


> +
> +    vhost_virtqueue_unmap(&net->dev, vdev, net->dev.vqs + idx, idx);
> +}
> diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
> index 387e913e4e..85d85a4957 100644
> --- a/include/net/vhost_net.h
> +++ b/include/net/vhost_net.h
> @@ -48,4 +48,6 @@ uint64_t vhost_net_get_acked_features(VHostNetState *net);
>   
>   int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu);
>   
> +void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
> +                               int vq_index);
>   #endif

Re: [PATCH v3 10/15] vhost-net: vhost-kernel: introduce vhost_net_virtqueue_reset()

[Kangjie Xu]

在 2022/9/5 16:03, Jason Wang 写道:
>
> 在 2022/8/25 16:08, Kangjie Xu 写道:
>> Introduce vhost_virtqueue_reset(), which can reset the specific
>> virtqueue in the device. Then it will unmap vrings and the desc
>> of the virtqueue.
>>
>> Here we do not reuse the vhost_net_stop_one() or vhost_dev_stop(),
>> because they work at queue pair level. We do not use
>> vhost_virtqueue_stop() because it may stop the device in the
>> backend.
>
>
> So I think this is not true at least for vhost-net kernel baceknd.
>
>
But vhost-user(OVS-DPDK) will stop the device.

When DPDK vhost received VHOST_USER_GET_VRING_BASE message, it will call 
vhost_destroy_device_notify() to destroy the device.

It seems like it is a inconsistency error in DPDK. Maybe I should submit 
a patch to DPDK. We can stop the device only when all the virtqueues in 
one device are destroyed.

>>
>> This patch only considers the case of vhost-kernel, when
>> NetClientDriver is NET_CLIENT_DRIVER_TAP.
>>
>> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>> ---
>>   hw/net/vhost_net.c      | 22 ++++++++++++++++++++++
>>   include/net/vhost_net.h |  2 ++
>>   2 files changed, 24 insertions(+)
>>
>> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
>> index ccac5b7a64..be51be98b3 100644
>> --- a/hw/net/vhost_net.c
>> +++ b/hw/net/vhost_net.c
>> @@ -514,3 +514,25 @@ int vhost_net_set_mtu(struct vhost_net *net, 
>> uint16_t mtu)
>>         return vhost_ops->vhost_net_set_mtu(&net->dev, mtu);
>>   }
>> +
>> +void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
>> +                               int vq_index)
>> +{
>> +    VHostNetState *net = get_vhost_net(nc->peer);
>> +    const VhostOps *vhost_ops = net->dev.vhost_ops;
>> +    struct vhost_vring_file file = { .fd = -1 };
>> +    int idx;
>> +
>> +    /* should only be called after backend is connected */
>> +    assert(vhost_ops);
>> +
>> +    idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
>> +
>> +    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
>> +        file.index = idx;
>> +        int r = vhost_net_set_backend(&net->dev, &file);
>> +        assert(r >= 0);
>> +    }
>
>
> Do we need to reset e.g last_avail_idx here?
>
> Thanks
>
I did not reset it because we will re-configure them when we restart 
virtqueue.

Thanks

>
>> +
>> +    vhost_virtqueue_unmap(&net->dev, vdev, net->dev.vqs + idx, idx);
>> +}
>> diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
>> index 387e913e4e..85d85a4957 100644
>> --- a/include/net/vhost_net.h
>> +++ b/include/net/vhost_net.h
>> @@ -48,4 +48,6 @@ uint64_t vhost_net_get_acked_features(VHostNetState 
>> *net);
>>     int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu);
>>   +void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState 
>> *nc,
>> +                               int vq_index);
>>   #endif

Re: [PATCH v3 10/15] vhost-net: vhost-kernel: introduce vhost_net_virtqueue_reset()

[Jason Wang]

On Mon, Sep 5, 2022 at 6:15 PM Kangjie Xu <kangjie.xu@linux.alibaba.com> wrote:
>
>
> 在 2022/9/5 16:03, Jason Wang 写道:
> >
> > 在 2022/8/25 16:08, Kangjie Xu 写道:
> >> Introduce vhost_virtqueue_reset(), which can reset the specific
> >> virtqueue in the device. Then it will unmap vrings and the desc
> >> of the virtqueue.
> >>
> >> Here we do not reuse the vhost_net_stop_one() or vhost_dev_stop(),
> >> because they work at queue pair level. We do not use
> >> vhost_virtqueue_stop() because it may stop the device in the
> >> backend.
> >
> >
> > So I think this is not true at least for vhost-net kernel baceknd.
> >
> >
> But vhost-user(OVS-DPDK) will stop the device.

Yes, what I meant is that, considering this series only deal with
vhost-net. We can leave any "fix" or "workaround" until e.g the
vhost-user support is posted?

>
> When DPDK vhost received VHOST_USER_GET_VRING_BASE message, it will call
> vhost_destroy_device_notify() to destroy the device.

Right, actually this trick is used even in some hardware.

>
> It seems like it is a inconsistency error in DPDK. Maybe I should submit
> a patch to DPDK. We can stop the device only when all the virtqueues in
> one device are destroyed.

That would be fine.

>
> >>
> >> This patch only considers the case of vhost-kernel, when
> >> NetClientDriver is NET_CLIENT_DRIVER_TAP.
> >>
> >> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> >> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> >> ---
> >>   hw/net/vhost_net.c      | 22 ++++++++++++++++++++++
> >>   include/net/vhost_net.h |  2 ++
> >>   2 files changed, 24 insertions(+)
> >>
> >> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> >> index ccac5b7a64..be51be98b3 100644
> >> --- a/hw/net/vhost_net.c
> >> +++ b/hw/net/vhost_net.c
> >> @@ -514,3 +514,25 @@ int vhost_net_set_mtu(struct vhost_net *net,
> >> uint16_t mtu)
> >>         return vhost_ops->vhost_net_set_mtu(&net->dev, mtu);
> >>   }
> >> +
> >> +void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
> >> +                               int vq_index)
> >> +{
> >> +    VHostNetState *net = get_vhost_net(nc->peer);
> >> +    const VhostOps *vhost_ops = net->dev.vhost_ops;
> >> +    struct vhost_vring_file file = { .fd = -1 };
> >> +    int idx;
> >> +
> >> +    /* should only be called after backend is connected */
> >> +    assert(vhost_ops);
> >> +
> >> +    idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
> >> +
> >> +    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
> >> +        file.index = idx;
> >> +        int r = vhost_net_set_backend(&net->dev, &file);
> >> +        assert(r >= 0);
> >> +    }
> >
> >
> > Do we need to reset e.g last_avail_idx here?
> >
> > Thanks
> >
> I did not reset it because we will re-configure them when we restart
> virtqueue.

Is last_avail_idx reset to 0 in this case?

Thanks

>
> Thanks
>
> >
> >> +
> >> +    vhost_virtqueue_unmap(&net->dev, vdev, net->dev.vqs + idx, idx);
> >> +}
> >> diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
> >> index 387e913e4e..85d85a4957 100644
> >> --- a/include/net/vhost_net.h
> >> +++ b/include/net/vhost_net.h
> >> @@ -48,4 +48,6 @@ uint64_t vhost_net_get_acked_features(VHostNetState
> >> *net);
> >>     int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu);
> >>   +void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState
> >> *nc,
> >> +                               int vq_index);
> >>   #endif
>

[PATCH v3 11/15] vhost-net: vhost-kernel: introduce vhost_net_virtqueue_restart()

[Kangjie Xu]

Introduce vhost_net_virtqueue_restart(), which can restart the
specific virtqueue when the vhost net started running before.
If it fails to restart the virtqueue, the device will be stopped.

Here we do not reuse vhost_net_start_one() or vhost_dev_start()
because they work at queue pair level. The mem table and features
do not change, so we can call the vhost_virtqueue_start() to
restart a specific queue.

This patch only considers the case of vhost-kernel, when
NetClientDriver is NET_CLIENT_DRIVER_TAP.

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 hw/net/vhost_net.c      | 52 +++++++++++++++++++++++++++++++++++++++++
 include/net/vhost_net.h |  2 ++
 2 files changed, 54 insertions(+)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index be51be98b3..0716f6cd96 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -536,3 +536,55 @@ void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
 
     vhost_virtqueue_unmap(&net->dev, vdev, net->dev.vqs + idx, idx);
 }
+
+int vhost_net_virtqueue_restart(VirtIODevice *vdev, NetClientState *nc,
+                                int vq_index)
+{
+    VHostNetState *net = get_vhost_net(nc->peer);
+    const VhostOps *vhost_ops = net->dev.vhost_ops;
+    struct vhost_vring_file file = { };
+    int idx, r;
+
+    if (!net->dev.started) {
+        return 0;
+    }
+
+    /* should only be called after backend is connected */
+    assert(vhost_ops);
+
+    idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
+
+    r = vhost_virtqueue_start(&net->dev,
+                              vdev,
+                              net->dev.vqs + idx,
+                              net->dev.vq_index + idx);
+    if (r < 0) {
+        goto err_start;
+    }
+
+    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
+        file.index = idx;
+        file.fd = net->backend;
+        r = vhost_net_set_backend(&net->dev, &file);
+        if (r < 0) {
+            r = -errno;
+            goto err_start;
+        }
+    }
+
+    return 0;
+
+err_start:
+    error_report("Error when restarting the queue.");
+
+    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
+        file.fd = -1;
+        file.index = idx;
+        int r = vhost_net_set_backend(&net->dev, &file);
+        assert(r >= 0);
+    }
+
+    vhost_dev_stop(&net->dev, vdev);
+
+    return r;
+}
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 85d85a4957..40b9a40074 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -50,4 +50,6 @@ int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu);
 
 void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
                                int vq_index);
+int vhost_net_virtqueue_restart(VirtIODevice *vdev, NetClientState *nc,
+                                int vq_index);
 #endif
-- 
2.32.0

Re: [PATCH v3 11/15] vhost-net: vhost-kernel: introduce vhost_net_virtqueue_restart()

[Jason Wang]

在 2022/8/25 16:08, Kangjie Xu 写道:
> Introduce vhost_net_virtqueue_restart(), which can restart the
> specific virtqueue when the vhost net started running before.
> If it fails to restart the virtqueue, the device will be stopped.
>
> Here we do not reuse vhost_net_start_one() or vhost_dev_start()
> because they work at queue pair level. The mem table and features
> do not change, so we can call the vhost_virtqueue_start() to
> restart a specific queue.
>
> This patch only considers the case of vhost-kernel, when
> NetClientDriver is NET_CLIENT_DRIVER_TAP.
>
> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>   hw/net/vhost_net.c      | 52 +++++++++++++++++++++++++++++++++++++++++
>   include/net/vhost_net.h |  2 ++
>   2 files changed, 54 insertions(+)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index be51be98b3..0716f6cd96 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -536,3 +536,55 @@ void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
>   
>       vhost_virtqueue_unmap(&net->dev, vdev, net->dev.vqs + idx, idx);
>   }
> +
> +int vhost_net_virtqueue_restart(VirtIODevice *vdev, NetClientState *nc,
> +                                int vq_index)
> +{
> +    VHostNetState *net = get_vhost_net(nc->peer);
> +    const VhostOps *vhost_ops = net->dev.vhost_ops;
> +    struct vhost_vring_file file = { };
> +    int idx, r;
> +
> +    if (!net->dev.started) {
> +        return 0;
> +    }


Should we return error in this case?

Thanks


> +
> +    /* should only be called after backend is connected */
> +    assert(vhost_ops);
> +
> +    idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
> +
> +    r = vhost_virtqueue_start(&net->dev,
> +                              vdev,
> +                              net->dev.vqs + idx,
> +                              net->dev.vq_index + idx);
> +    if (r < 0) {
> +        goto err_start;
> +    }
> +
> +    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
> +        file.index = idx;
> +        file.fd = net->backend;
> +        r = vhost_net_set_backend(&net->dev, &file);
> +        if (r < 0) {
> +            r = -errno;
> +            goto err_start;
> +        }
> +    }
> +
> +    return 0;
> +
> +err_start:
> +    error_report("Error when restarting the queue.");
> +
> +    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
> +        file.fd = -1;
> +        file.index = idx;
> +        int r = vhost_net_set_backend(&net->dev, &file);
> +        assert(r >= 0);
> +    }
> +
> +    vhost_dev_stop(&net->dev, vdev);
> +
> +    return r;
> +}
> diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
> index 85d85a4957..40b9a40074 100644
> --- a/include/net/vhost_net.h
> +++ b/include/net/vhost_net.h
> @@ -50,4 +50,6 @@ int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu);
>   
>   void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
>                                  int vq_index);
> +int vhost_net_virtqueue_restart(VirtIODevice *vdev, NetClientState *nc,
> +                                int vq_index);
>   #endif

Re: [PATCH v3 11/15] vhost-net: vhost-kernel: introduce vhost_net_virtqueue_restart()

[Kangjie Xu]

在 2022/9/5 16:24, Jason Wang 写道:
>
> 在 2022/8/25 16:08, Kangjie Xu 写道:
>> Introduce vhost_net_virtqueue_restart(), which can restart the
>> specific virtqueue when the vhost net started running before.
>> If it fails to restart the virtqueue, the device will be stopped.
>>
>> Here we do not reuse vhost_net_start_one() or vhost_dev_start()
>> because they work at queue pair level. The mem table and features
>> do not change, so we can call the vhost_virtqueue_start() to
>> restart a specific queue.
>>
>> This patch only considers the case of vhost-kernel, when
>> NetClientDriver is NET_CLIENT_DRIVER_TAP.
>>
>> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>> ---
>>   hw/net/vhost_net.c      | 52 +++++++++++++++++++++++++++++++++++++++++
>>   include/net/vhost_net.h |  2 ++
>>   2 files changed, 54 insertions(+)
>>
>> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
>> index be51be98b3..0716f6cd96 100644
>> --- a/hw/net/vhost_net.c
>> +++ b/hw/net/vhost_net.c
>> @@ -536,3 +536,55 @@ void vhost_net_virtqueue_reset(VirtIODevice 
>> *vdev, NetClientState *nc,
>>         vhost_virtqueue_unmap(&net->dev, vdev, net->dev.vqs + idx, idx);
>>   }
>> +
>> +int vhost_net_virtqueue_restart(VirtIODevice *vdev, NetClientState *nc,
>> +                                int vq_index)
>> +{
>> +    VHostNetState *net = get_vhost_net(nc->peer);
>> +    const VhostOps *vhost_ops = net->dev.vhost_ops;
>> +    struct vhost_vring_file file = { };
>> +    int idx, r;
>> +
>> +    if (!net->dev.started) {
>> +        return 0;
>> +    }
>
>
> Should we return error in this case?
>
> Thanks

Yes, I think so. Will fix.

Thanks.

>
>> +
>> +    /* should only be called after backend is connected */
>> +    assert(vhost_ops);
>> +
>> +    idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
>> +
>> +    r = vhost_virtqueue_start(&net->dev,
>> +                              vdev,
>> +                              net->dev.vqs + idx,
>> +                              net->dev.vq_index + idx);
>> +    if (r < 0) {
>> +        goto err_start;
>> +    }
>> +
>> +    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
>> +        file.index = idx;
>> +        file.fd = net->backend;
>> +        r = vhost_net_set_backend(&net->dev, &file);
>> +        if (r < 0) {
>> +            r = -errno;
>> +            goto err_start;
>> +        }
>> +    }
>> +
>> +    return 0;
>> +
>> +err_start:
>> +    error_report("Error when restarting the queue.");
>> +
>> +    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
>> +        file.fd = -1;
>> +        file.index = idx;
>> +        int r = vhost_net_set_backend(&net->dev, &file);
>> +        assert(r >= 0);
>> +    }
>> +
>> +    vhost_dev_stop(&net->dev, vdev);
>> +
>> +    return r;
>> +}
>> diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
>> index 85d85a4957..40b9a40074 100644
>> --- a/include/net/vhost_net.h
>> +++ b/include/net/vhost_net.h
>> @@ -50,4 +50,6 @@ int vhost_net_set_mtu(struct vhost_net *net, 
>> uint16_t mtu);
>>     void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState 
>> *nc,
>>                                  int vq_index);
>> +int vhost_net_virtqueue_restart(VirtIODevice *vdev, NetClientState *nc,
>> +                                int vq_index);
>>   #endif

[PATCH v3 12/15] virtio-net: introduce flush_or_purge_queued_packets()

[Kangjie Xu]

Introduce the fucntion flush_or_purge_queued_packets(), it will be
used in device reset and virtqueue reset. Therefore, we extract the
common logic as a new function.

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 hw/net/virtio-net.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index dd0d056fde..27b59c0ad6 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -118,6 +118,16 @@ static int vq2q(int queue_index)
     return queue_index / 2;
 }
 
+static void flush_or_purge_queued_packets(NetClientState *nc)
+{
+    if (!nc->peer) {
+        return;
+    }
+
+    qemu_flush_or_purge_queued_packets(nc->peer, true);
+    assert(!virtio_net_get_subqueue(nc)->async_tx.elem);
+}
+
 /* TODO
  * - we could suppress RX interrupt if we were so inclined.
  */
@@ -560,12 +570,7 @@ static void virtio_net_reset(VirtIODevice *vdev)
 
     /* Flush any async TX */
     for (i = 0;  i < n->max_queue_pairs; i++) {
-        NetClientState *nc = qemu_get_subqueue(n->nic, i);
-
-        if (nc->peer) {
-            qemu_flush_or_purge_queued_packets(nc->peer, true);
-            assert(!virtio_net_get_subqueue(nc)->async_tx.elem);
-        }
+        flush_or_purge_queued_packets(qemu_get_subqueue(n->nic, i));
     }
 }
 
-- 
2.32.0

Re: [PATCH v3 12/15] virtio-net: introduce flush_or_purge_queued_packets()

[Jason Wang]

在 2022/8/25 16:08, Kangjie Xu 写道:
> Introduce the fucntion flush_or_purge_queued_packets(), it will be
> used in device reset and virtqueue reset. Therefore, we extract the
> common logic as a new function.
>
> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>


Acked-by: Jason Wang <jasowang@redhat.com>


> ---
>   hw/net/virtio-net.c | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index dd0d056fde..27b59c0ad6 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -118,6 +118,16 @@ static int vq2q(int queue_index)
>       return queue_index / 2;
>   }
>   
> +static void flush_or_purge_queued_packets(NetClientState *nc)
> +{
> +    if (!nc->peer) {
> +        return;
> +    }
> +
> +    qemu_flush_or_purge_queued_packets(nc->peer, true);
> +    assert(!virtio_net_get_subqueue(nc)->async_tx.elem);
> +}
> +
>   /* TODO
>    * - we could suppress RX interrupt if we were so inclined.
>    */
> @@ -560,12 +570,7 @@ static void virtio_net_reset(VirtIODevice *vdev)
>   
>       /* Flush any async TX */
>       for (i = 0;  i < n->max_queue_pairs; i++) {
> -        NetClientState *nc = qemu_get_subqueue(n->nic, i);
> -
> -        if (nc->peer) {
> -            qemu_flush_or_purge_queued_packets(nc->peer, true);
> -            assert(!virtio_net_get_subqueue(nc)->async_tx.elem);
> -        }
> +        flush_or_purge_queued_packets(qemu_get_subqueue(n->nic, i));
>       }
>   }
>   

[PATCH v3 13/15] virtio-net: support queue reset

[Kangjie Xu]

From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>

virtio-net and vhost-kernel implement queue reset.
Queued packets in the corresponding queue pair are flushed
or purged.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
---
 hw/net/virtio-net.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 27b59c0ad6..d774a3e652 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -540,6 +540,23 @@ static RxFilterInfo *virtio_net_query_rxfilter(NetClientState *nc)
     return info;
 }
 
+static void virtio_net_queue_reset(VirtIODevice *vdev, uint32_t queue_index)
+{
+    VirtIONet *n = VIRTIO_NET(vdev);
+    NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(queue_index));
+
+    if (!nc->peer) {
+        return;
+    }
+
+    if (get_vhost_net(nc->peer) &&
+        nc->peer->info->type == NET_CLIENT_DRIVER_TAP) {
+        vhost_net_virtqueue_reset(vdev, nc, queue_index);
+    }
+
+    flush_or_purge_queued_packets(nc);
+}
+
 static void virtio_net_reset(VirtIODevice *vdev)
 {
     VirtIONet *n = VIRTIO_NET(vdev);
@@ -3784,6 +3801,7 @@ static void virtio_net_class_init(ObjectClass *klass, void *data)
     vdc->set_features = virtio_net_set_features;
     vdc->bad_features = virtio_net_bad_features;
     vdc->reset = virtio_net_reset;
+    vdc->queue_reset = virtio_net_queue_reset;
     vdc->set_status = virtio_net_set_status;
     vdc->guest_notifier_mask = virtio_net_guest_notifier_mask;
     vdc->guest_notifier_pending = virtio_net_guest_notifier_pending;
-- 
2.32.0

Re: [PATCH v3 13/15] virtio-net: support queue reset

[Jason Wang]

在 2022/8/25 16:08, Kangjie Xu 写道:
> From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>
> virtio-net and vhost-kernel implement queue reset.
> Queued packets in the corresponding queue pair are flushed
> or purged.
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> ---
>   hw/net/virtio-net.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 27b59c0ad6..d774a3e652 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -540,6 +540,23 @@ static RxFilterInfo *virtio_net_query_rxfilter(NetClientState *nc)
>       return info;
>   }
>   
> +static void virtio_net_queue_reset(VirtIODevice *vdev, uint32_t queue_index)
> +{
> +    VirtIONet *n = VIRTIO_NET(vdev);
> +    NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(queue_index));
> +
> +    if (!nc->peer) {
> +        return;
> +    }
> +
> +    if (get_vhost_net(nc->peer) &&
> +        nc->peer->info->type == NET_CLIENT_DRIVER_TAP) {
> +        vhost_net_virtqueue_reset(vdev, nc, queue_index);
> +    }
> +
> +    flush_or_purge_queued_packets(nc);


But the codes doesn't prevent the usersapce datapath from being used? 
(e.g vhost=off)

E.g vhost_net_start_one() had:

     if (net->nc->info->poll) {
         net->nc->info->poll(net->nc, false);
     }

And I will wonder if it's better to consider to:

1) factor out the per virtqueue start/stop from vhost_net_start/stop_one()

2) simply use the helper factored out via step 1)

Thanks


> +}
> +
>   static void virtio_net_reset(VirtIODevice *vdev)
>   {
>       VirtIONet *n = VIRTIO_NET(vdev);
> @@ -3784,6 +3801,7 @@ static void virtio_net_class_init(ObjectClass *klass, void *data)
>       vdc->set_features = virtio_net_set_features;
>       vdc->bad_features = virtio_net_bad_features;
>       vdc->reset = virtio_net_reset;
> +    vdc->queue_reset = virtio_net_queue_reset;
>       vdc->set_status = virtio_net_set_status;
>       vdc->guest_notifier_mask = virtio_net_guest_notifier_mask;
>       vdc->guest_notifier_pending = virtio_net_guest_notifier_pending;

Re: [PATCH v3 13/15] virtio-net: support queue reset

[Kangjie Xu]

在 2022/9/5 16:30, Jason Wang 写道:
>
> 在 2022/8/25 16:08, Kangjie Xu 写道:
>> From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>>
>> virtio-net and vhost-kernel implement queue reset.
>> Queued packets in the corresponding queue pair are flushed
>> or purged.
>>
>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
>> ---
>>   hw/net/virtio-net.c | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 27b59c0ad6..d774a3e652 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -540,6 +540,23 @@ static RxFilterInfo 
>> *virtio_net_query_rxfilter(NetClientState *nc)
>>       return info;
>>   }
>>   +static void virtio_net_queue_reset(VirtIODevice *vdev, uint32_t 
>> queue_index)
>> +{
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>> +    NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(queue_index));
>> +
>> +    if (!nc->peer) {
>> +        return;
>> +    }
>> +
>> +    if (get_vhost_net(nc->peer) &&
>> +        nc->peer->info->type == NET_CLIENT_DRIVER_TAP) {
>> +        vhost_net_virtqueue_reset(vdev, nc, queue_index);
>> +    }
>> +
>> +    flush_or_purge_queued_packets(nc);
>
>
> But the codes doesn't prevent the usersapce datapath from being used? 
> (e.g vhost=off)
>
> E.g vhost_net_start_one() had:
>
>     if (net->nc->info->poll) {
>         net->nc->info->poll(net->nc, false);
>     }
>
> And I will wonder if it's better to consider to:
>
> 1) factor out the per virtqueue start/stop from 
> vhost_net_start/stop_one()
>
> 2) simply use the helper factored out via step 1)
>
> Thanks
>
Will update it based on your suggestions.

Thanks

>
>> +}
>> +
>>   static void virtio_net_reset(VirtIODevice *vdev)
>>   {
>>       VirtIONet *n = VIRTIO_NET(vdev);
>> @@ -3784,6 +3801,7 @@ static void virtio_net_class_init(ObjectClass 
>> *klass, void *data)
>>       vdc->set_features = virtio_net_set_features;
>>       vdc->bad_features = virtio_net_bad_features;
>>       vdc->reset = virtio_net_reset;
>> +    vdc->queue_reset = virtio_net_queue_reset;
>>       vdc->set_status = virtio_net_set_status;
>>       vdc->guest_notifier_mask = virtio_net_guest_notifier_mask;
>>       vdc->guest_notifier_pending = virtio_net_guest_notifier_pending;

Re: [PATCH v3 13/15] virtio-net: support queue reset

[Kangjie Xu]

在 2022/9/5 16:30, Jason Wang 写道:
>
> 在 2022/8/25 16:08, Kangjie Xu 写道:
>> From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>>
>> virtio-net and vhost-kernel implement queue reset.
>> Queued packets in the corresponding queue pair are flushed
>> or purged.
>>
>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
>> ---
>>   hw/net/virtio-net.c | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 27b59c0ad6..d774a3e652 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -540,6 +540,23 @@ static RxFilterInfo 
>> *virtio_net_query_rxfilter(NetClientState *nc)
>>       return info;
>>   }
>>   +static void virtio_net_queue_reset(VirtIODevice *vdev, uint32_t 
>> queue_index)
>> +{
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>> +    NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(queue_index));
>> +
>> +    if (!nc->peer) {
>> +        return;
>> +    }
>> +
>> +    if (get_vhost_net(nc->peer) &&
>> +        nc->peer->info->type == NET_CLIENT_DRIVER_TAP) {
>> +        vhost_net_virtqueue_reset(vdev, nc, queue_index);
>> +    }
>> +
>> +    flush_or_purge_queued_packets(nc);
>
>
> But the codes doesn't prevent the usersapce datapath from being used? 
> (e.g vhost=off)
>
> E.g vhost_net_start_one() had:
>
>     if (net->nc->info->poll) {
>         net->nc->info->poll(net->nc, false);
>     }
I review this part in vhost_net_start/stop_one().

I didn't take the usersapce datapath into consideration. If we don't 
prevent it, the userspace datapath may access it and causes some 
problems. From this point, we should disable it.

But if we add net->nc->info->poll in vq reset, it can only operate at 
queue-pair level, which obeys "per-virtqueue".

What's your opinion on this point? I think it's ok to add it in vq reset.

>
> And I will wonder if it's better to consider to:
>
> 1) factor out the per virtqueue start/stop from 
> vhost_net_start/stop_one()
>
> 2) simply use the helper factored out via step 1)
>
> Thanks
>
I have a different idea about it, vhost_virtqueue_start/stop()(in 
hw/virtio/vhost.c) are already good abstractions of per virtqueue 
start/stop.

These two functions are used in vhost_dev_start. It's just because 
vhost-net needs some configuration, so we add net->nc->info->poll and 
set_backend for it. But for other devices using vhost_dev_start, 
set_backend and net->nc->info->poll may be not necessary.

I think your apporach to abstract per virtqueue start/stop from 
vhost_net_start/stop_one() will break the generality of 
vhost_dev_start(), which is a common interface for different devices.

To conclude my opinions

1. I think we need to add net->nc->info->poll in our 
vhost_net_virtqueue_reset() and vhost_net_virtqueue_restart()

2. We still need vhost_net_virtqueue_reset() and 
vhost_net_virtqueue_restart().

     a. vhost_net_virtqueue_reset() is a wrapper for vhost_virtqueue_stop().

     b. vhost_net_virtqueue_restart() is a wrapper for 
vhost_virtqueue_start().

Thanks

>
>> +}
>> +
>>   static void virtio_net_reset(VirtIODevice *vdev)
>>   {
>>       VirtIONet *n = VIRTIO_NET(vdev);
>> @@ -3784,6 +3801,7 @@ static void virtio_net_class_init(ObjectClass 
>> *klass, void *data)
>>       vdc->set_features = virtio_net_set_features;
>>       vdc->bad_features = virtio_net_bad_features;
>>       vdc->reset = virtio_net_reset;
>> +    vdc->queue_reset = virtio_net_queue_reset;
>>       vdc->set_status = virtio_net_set_status;
>>       vdc->guest_notifier_mask = virtio_net_guest_notifier_mask;
>>       vdc->guest_notifier_pending = virtio_net_guest_notifier_pending;

Re: [PATCH v3 13/15] virtio-net: support queue reset

[Jason Wang]

On Tue, Sep 6, 2022 at 12:25 AM Kangjie Xu <kangjie.xu@linux.alibaba.com> wrote:
>
>
> 在 2022/9/5 16:30, Jason Wang 写道:
> >
> > 在 2022/8/25 16:08, Kangjie Xu 写道:
> >> From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> >>
> >> virtio-net and vhost-kernel implement queue reset.
> >> Queued packets in the corresponding queue pair are flushed
> >> or purged.
> >>
> >> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> >> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> >> ---
> >>   hw/net/virtio-net.c | 18 ++++++++++++++++++
> >>   1 file changed, 18 insertions(+)
> >>
> >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >> index 27b59c0ad6..d774a3e652 100644
> >> --- a/hw/net/virtio-net.c
> >> +++ b/hw/net/virtio-net.c
> >> @@ -540,6 +540,23 @@ static RxFilterInfo
> >> *virtio_net_query_rxfilter(NetClientState *nc)
> >>       return info;
> >>   }
> >>   +static void virtio_net_queue_reset(VirtIODevice *vdev, uint32_t
> >> queue_index)
> >> +{
> >> +    VirtIONet *n = VIRTIO_NET(vdev);
> >> +    NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(queue_index));
> >> +
> >> +    if (!nc->peer) {
> >> +        return;
> >> +    }
> >> +
> >> +    if (get_vhost_net(nc->peer) &&
> >> +        nc->peer->info->type == NET_CLIENT_DRIVER_TAP) {
> >> +        vhost_net_virtqueue_reset(vdev, nc, queue_index);
> >> +    }
> >> +
> >> +    flush_or_purge_queued_packets(nc);
> >
> >
> > But the codes doesn't prevent the usersapce datapath from being used?
> > (e.g vhost=off)
> >
> > E.g vhost_net_start_one() had:
> >
> >     if (net->nc->info->poll) {
> >         net->nc->info->poll(net->nc, false);
> >     }
> I review this part in vhost_net_start/stop_one().
>
> I didn't take the usersapce datapath into consideration.

Because it's used for vhost-net. So I think maybe we should first
implement the rest in the userspace datapath first then try to
implement them in the vhost?

> If we don't
> prevent it, the userspace datapath may access it and causes some
> problems. From this point, we should disable it.
>
> But if we add net->nc->info->poll in vq reset, it can only operate at
> queue-pair level, which obeys "per-virtqueue".
>
> What's your opinion on this point? I think it's ok to add it in vq reset.

See above, I'd suggest to implement the vq reset in qemu usersapce
datapath first. Then we can do vhost part on top.

>
> >
> > And I will wonder if it's better to consider to:
> >
> > 1) factor out the per virtqueue start/stop from
> > vhost_net_start/stop_one()
> >
> > 2) simply use the helper factored out via step 1)
> >
> > Thanks
> >
> I have a different idea about it, vhost_virtqueue_start/stop()(in
> hw/virtio/vhost.c) are already good abstractions of per virtqueue
> start/stop.
>
> These two functions are used in vhost_dev_start. It's just because
> vhost-net needs some configuration, so we add net->nc->info->poll and
> set_backend for it. But for other devices using vhost_dev_start,
> set_backend and net->nc->info->poll may be not necessary.
>
> I think your apporach to abstract per virtqueue start/stop from
> vhost_net_start/stop_one() will break the generality of
> vhost_dev_start(), which is a common interface for different devices.
>
> To conclude my opinions
>
> 1. I think we need to add net->nc->info->poll in our
> vhost_net_virtqueue_reset() and vhost_net_virtqueue_restart()
>
> 2. We still need vhost_net_virtqueue_reset() and
> vhost_net_virtqueue_restart().
>
>      a. vhost_net_virtqueue_reset() is a wrapper for vhost_virtqueue_stop().
>
>      b. vhost_net_virtqueue_restart() is a wrapper for
> vhost_virtqueue_start().

Sounds good, let's do and see.

Thanks

>
> Thanks
>
> >
> >> +}
> >> +
> >>   static void virtio_net_reset(VirtIODevice *vdev)
> >>   {
> >>       VirtIONet *n = VIRTIO_NET(vdev);
> >> @@ -3784,6 +3801,7 @@ static void virtio_net_class_init(ObjectClass
> >> *klass, void *data)
> >>       vdc->set_features = virtio_net_set_features;
> >>       vdc->bad_features = virtio_net_bad_features;
> >>       vdc->reset = virtio_net_reset;
> >> +    vdc->queue_reset = virtio_net_queue_reset;
> >>       vdc->set_status = virtio_net_set_status;
> >>       vdc->guest_notifier_mask = virtio_net_guest_notifier_mask;
> >>       vdc->guest_notifier_pending = virtio_net_guest_notifier_pending;
>

Re: [PATCH v3 13/15] virtio-net: support queue reset

[Kangjie Xu]

在 2022/9/5 16:30, Jason Wang 写道:
>
> 在 2022/8/25 16:08, Kangjie Xu 写道:
>> From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>>
>> virtio-net and vhost-kernel implement queue reset.
>> Queued packets in the corresponding queue pair are flushed
>> or purged.
>>
>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
>> ---
>>   hw/net/virtio-net.c | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 27b59c0ad6..d774a3e652 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -540,6 +540,23 @@ static RxFilterInfo 
>> *virtio_net_query_rxfilter(NetClientState *nc)
>>       return info;
>>   }
>>   +static void virtio_net_queue_reset(VirtIODevice *vdev, uint32_t 
>> queue_index)
>> +{
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>> +    NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(queue_index));
>> +
>> +    if (!nc->peer) {
>> +        return;
>> +    }
>> +
>> +    if (get_vhost_net(nc->peer) &&
>> +        nc->peer->info->type == NET_CLIENT_DRIVER_TAP) {
>> +        vhost_net_virtqueue_reset(vdev, nc, queue_index);
>> +    }
>> +
>> +    flush_or_purge_queued_packets(nc);
>
>
> But the codes doesn't prevent the usersapce datapath from being used? 
> (e.g vhost=off)

I think we do not need to prevent it for vhost=off, because the 
virtio-net device is in control of the tap device.

After we reset the vq, the virtio-net send and recv will not use the 
userspace datapath. (virtio_net_flush_tx() and virtio_net_receive() will 
early returns because vq->vring.avail == 0)

So even if we don't prevent it using net->nc->info->poll, virtio-net 
device will prevent it. And the logic here is similar to virtio_reset(), 
I think it will not cause problems.

Thanks.

>
> E.g vhost_net_start_one() had:
>
>     if (net->nc->info->poll) {
>         net->nc->info->poll(net->nc, false);
>     }
>
> And I will wonder if it's better to consider to:
>
> 1) factor out the per virtqueue start/stop from 
> vhost_net_start/stop_one()
>
> 2) simply use the helper factored out via step 1)
>
> Thanks
>
>
>> +}
>> +
>>   static void virtio_net_reset(VirtIODevice *vdev)
>>   {
>>       VirtIONet *n = VIRTIO_NET(vdev);
>> @@ -3784,6 +3801,7 @@ static void virtio_net_class_init(ObjectClass 
>> *klass, void *data)
>>       vdc->set_features = virtio_net_set_features;
>>       vdc->bad_features = virtio_net_bad_features;
>>       vdc->reset = virtio_net_reset;
>> +    vdc->queue_reset = virtio_net_queue_reset;
>>       vdc->set_status = virtio_net_set_status;
>>       vdc->guest_notifier_mask = virtio_net_guest_notifier_mask;
>>       vdc->guest_notifier_pending = virtio_net_guest_notifier_pending;

[PATCH v3 14/15] virtio-net: support queue_enable

[Kangjie Xu]

Support queue_enable in vhost-kernel scenario. It can be called when
a vq reset operation has been performed and the vq is restared.

It should be noted that we can restart the vq when the vhost has
already started. When launching a new vhost device, the vhost is not
started and all vqs are not initalized until VIRTIO_PCI_COMMON_STATUS
is written. Thus, we should use vhost_started to differentiate the
two cases: vq reset and device start.

Currently it only supports vhost-kernel.

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 hw/net/virtio-net.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index d774a3e652..7817206596 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -557,6 +557,26 @@ static void virtio_net_queue_reset(VirtIODevice *vdev, uint32_t queue_index)
     flush_or_purge_queued_packets(nc);
 }
 
+static void virtio_net_queue_enable(VirtIODevice *vdev, uint32_t queue_index)
+{
+    VirtIONet *n = VIRTIO_NET(vdev);
+    NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(queue_index));
+    int r;
+
+    if (!nc->peer || !vdev->vhost_started) {
+        return;
+    }
+
+    if (get_vhost_net(nc->peer) &&
+        nc->peer->info->type == NET_CLIENT_DRIVER_TAP) {
+        r = vhost_net_virtqueue_restart(vdev, nc, queue_index);
+        if (r < 0) {
+            error_report("unable to restart vhost net virtqueue: %d, "
+                            "when resetting the queue", queue_index);
+        }
+    }
+}
+
 static void virtio_net_reset(VirtIODevice *vdev)
 {
     VirtIONet *n = VIRTIO_NET(vdev);
@@ -3802,6 +3822,7 @@ static void virtio_net_class_init(ObjectClass *klass, void *data)
     vdc->bad_features = virtio_net_bad_features;
     vdc->reset = virtio_net_reset;
     vdc->queue_reset = virtio_net_queue_reset;
+    vdc->queue_enable = virtio_net_queue_enable;
     vdc->set_status = virtio_net_set_status;
     vdc->guest_notifier_mask = virtio_net_guest_notifier_mask;
     vdc->guest_notifier_pending = virtio_net_guest_notifier_pending;
-- 
2.32.0

[PATCH v3 15/15] vhost: vhost-kernel: enable vq reset feature

[Kangjie Xu]

Add virtqueue reset feature for vhost-kernel.

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 hw/net/vhost_net.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 0716f6cd96..74c5147d6e 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -46,6 +46,7 @@ static const int kernel_feature_bits[] = {
     VIRTIO_NET_F_MTU,
     VIRTIO_F_IOMMU_PLATFORM,
     VIRTIO_F_RING_PACKED,
+    VIRTIO_F_RING_RESET,
     VIRTIO_NET_F_HASH_REPORT,
     VHOST_INVALID_FEATURE_BIT
 };
-- 
2.32.0

Re: [PATCH v3 00/15] Support VIRTIO_F_RING_RESET for virtio-net, vhost-net kernel in virtio pci-modern

[Kangjie Xu]

Do you have any comments about this patch set?

Thanks

在 2022/8/25 16:08, Kangjie Xu 写道:
> The virtio queue reset function has already been defined in the virtio spec 1.2.
> The relevant virtio spec information is here:
>
>      https://github.com/oasis-tcs/virtio-spec/issues/124
>      https://github.com/oasis-tcs/virtio-spec/issues/139
>
> This patch set is to support this function in QEMU. It consists of several parts:
> 1. Patches 1-7 are the basic interfaces for vq reset in virtio and virtio-pci.
> 2. Patches 8-11 support vq reset and vq restart for vhost-kernel.
> 3. Patches 12-14 support vq reset and vq restart for virtio-net.
> 5. Patch 15 enables the vq reset feature for vhost-kernel.
>
> The process of virtqueue reset can be concluded as:
> 1. The virtqueue is disabled when VIRTIO_PCI_COMMON_Q_RESET is written.
> 2. Then the virtqueue can be optionally restarted(re-enabled).
>
> Since this patch set involves multiple modules and seems a bit messy, we briefly describe the
> calling process for different modes below.
> virtio-net:
> 1. VIRTIO_PCI_COMMON_Q_RESET is written [virtio-pci]
>      -> virtio_queue_reset() [virtio]
>          -> virtio_net_queue_reset() [virtio-net]
>          -> __virtio_queue_reset()
> 2. VIRTIO_PCI_COMMON_Q_ENABLE is written [virtio-pci]
>      -> set enabled, reset status of vq.
>
> vhost-kernel:
> 1. VIRTIO_PCI_COMMON_Q_RESET is written [virtio-pci]
>      -> virtio_queue_reset() [virtio]
>          -> virtio_net_queue_reset() [virtio-net]
>              -> vhost_net_virtqueue_stop() [vhost-net]
>                  -> vhost_net_set_backend() [vhost]
>                  -> vhost_virtqueue_unmap()
>          -> __virtio_queue_reset()
> 2. VIRTIO_PCI_COMMON_Q_ENABLE is written [virtio-pci]
>      -> virtio_queue_enable() [virtio]
>          -> virtio_net_queue_enable() [virtio-net]
>              -> vhost_net_virtqueue_restart() [vhost-net]
>                  -> vhost_virtqueue_start() [vhost]
>                  -> vhost_net_set_backend()
>      -> set enabled, reset status of vq.
>
>
> Test environment and method:
>      Host: 5.19.0-rc3 (With vq reset support)
>      Qemu: QEMU emulator version 7.0.50
>      Guest: 5.19.0-rc3 (With vq reset support)
>      Test Cmd: ethtool -g eth1; ethtool -G eth1 rx $1 tx $2; ethtool -g eth1;
>
>      The drvier can resize the virtio queue, then virtio queue reset function should
>      be triggered.
>
>      The default is split mode, modify Qemu virtio-net to add PACKED feature to
>      test packed mode.
>
> Guest Kernel Patch:
>      https://lore.kernel.org/bpf/20220801063902.129329-1-xuanzhuo@linux.alibaba.com/
>
> Host Kernel Patch:
>      https://github.com/middaywords/linux/commit/a845098f0df6b8bdf7e1e5db57af6ebd1c8eaf47
>
> Looking forward to your review and comments. Thanks.
>
> changelog:
> v3:
>    1. Remove support for vhost-user in this series and refactor the code.
>    2. Rename 'vhost_net_virtqueue_stop' to 'vhost_net_virtqueue_reset'.
>    3. Make PCI transport ready before device ready when queue_enabled is set to true.
>    3. Add some comments.
>
> v2:
>    1. Add support for vhost-net kernel scenario.
>    2. Add a new vhost-user message VHOST_USER_RESET_VRING.
>    3. Add migration compatibility for virtqueue reset.
>
> Kangjie Xu (10):
>    virtio: introduce virtio_queue_enable()
>    virtio: core: vq reset feature negotation support
>    virtio-pci: support queue enable
>    vhost: extract the logic of unmapping the vrings and desc
>    vhost: expose vhost_virtqueue_start()
>    vhost-net: vhost-kernel: introduce vhost_net_virtqueue_reset()
>    vhost-net: vhost-kernel: introduce vhost_net_virtqueue_restart()
>    virtio-net: introduce flush_or_purge_queued_packets()
>    virtio-net: support queue_enable
>    vhost: vhost-kernel: enable vq reset feature
>
> Xuan Zhuo (5):
>    virtio: sync relevant definitions with linux
>    virtio: introduce __virtio_queue_reset()
>    virtio: introduce virtio_queue_reset()
>    virtio-pci: support queue reset
>    virtio-net: support queue reset
>
>   hw/core/machine.c                             |  1 +
>   hw/net/vhost_net.c                            | 75 +++++++++++++++++++
>   hw/net/virtio-net.c                           | 56 ++++++++++++--
>   hw/virtio/vhost.c                             | 28 ++++---
>   hw/virtio/virtio-pci.c                        | 16 ++++
>   hw/virtio/virtio.c                            | 62 +++++++++++----
>   include/hw/virtio/vhost.h                     |  5 ++
>   include/hw/virtio/virtio-pci.h                |  5 ++
>   include/hw/virtio/virtio.h                    |  8 +-
>   include/net/vhost_net.h                       |  4 +
>   .../standard-headers/linux/virtio_config.h    |  5 ++
>   include/standard-headers/linux/virtio_pci.h   |  2 +
>   12 files changed, 234 insertions(+), 33 deletions(-)
>