[PATCH v2 0/6] Vhost-vdpa Shadow Virtqueue multiqueue support.

[PATCH v2 0/6] Vhost-vdpa Shadow Virtqueue multiqueue support.

[Eugenio Pérez]

This series enables shadowed CVQ to intercept multiqueue commands through
shadowed CVQ, update the virtio NIC device model so qemu send it in a
migration, and the restore of that MQ state in the destination.

It needs to be applied on top of [1].

[1] https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg02965.html

v2:
* Add vhost_vdpa_net_load_cmd helper to avoid out buffers castings.
* Make cvq_cmd_in_buffer virtio_net_ctrl_ack type.

Eugenio Pérez (6):
  vdpa: Make VhostVDPAState cvq_cmd_out_buffer control ack type
  vdpa: extract vhost_vdpa_net_load_mac from vhost_vdpa_net_load
  vdpa: Add vhost_vdpa_net_load_mq
  vdpa: validate MQ CVQ commands
  virtio-net: Update virtio-net curr_queue_pairs in vdpa backends
  vdpa: Allow MQ feture in SVQ

 hw/net/virtio-net.c |  17 +++-----
 net/vhost-vdpa.c    | 101 ++++++++++++++++++++++++++++++++++----------
 2 files changed, 85 insertions(+), 33 deletions(-)

--
2.31.1

[PATCH v2 1/6] vdpa: Make VhostVDPAState cvq_cmd_out_buffer control ack type

[Eugenio Pérez]

This allows to simplify the code.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 net/vhost-vdpa.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 6ce68fcd3f..468e460ac2 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -35,7 +35,9 @@ typedef struct VhostVDPAState {
     VHostNetState *vhost_net;
 
     /* Control commands shadow buffers */
-    void *cvq_cmd_out_buffer, *cvq_cmd_in_buffer;
+    void *cvq_cmd_out_buffer;
+    virtio_net_ctrl_ack *cvq_cmd_in_buffer;
+
     bool started;
 } VhostVDPAState;
 
@@ -396,7 +398,7 @@ static int vhost_vdpa_net_load(NetClientState *nc)
             return dev_written;
         }
 
-        return *((virtio_net_ctrl_ack *)s->cvq_cmd_in_buffer) != VIRTIO_NET_OK;
+        return *s->cvq_cmd_in_buffer != VIRTIO_NET_OK;
     }
 
     return 0;
@@ -491,8 +493,7 @@ static int vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
         goto out;
     }
 
-    memcpy(&status, s->cvq_cmd_in_buffer, sizeof(status));
-    if (status != VIRTIO_NET_OK) {
+    if (*s->cvq_cmd_in_buffer != VIRTIO_NET_OK) {
         return VIRTIO_NET_ERR;
     }
 
-- 
2.31.1

Re: [PATCH v2 1/6] vdpa: Make VhostVDPAState cvq_cmd_out_buffer control ack type

[Jason Wang]

On Thu, Aug 25, 2022 at 2:36 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> This allows to simplify the code.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>  net/vhost-vdpa.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 6ce68fcd3f..468e460ac2 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -35,7 +35,9 @@ typedef struct VhostVDPAState {
>      VHostNetState *vhost_net;
>
>      /* Control commands shadow buffers */
> -    void *cvq_cmd_out_buffer, *cvq_cmd_in_buffer;
> +    void *cvq_cmd_out_buffer;
> +    virtio_net_ctrl_ack *cvq_cmd_in_buffer;

Nit, let's simply rename this to 'status'.

Thanks

> +
>      bool started;
>  } VhostVDPAState;
>
> @@ -396,7 +398,7 @@ static int vhost_vdpa_net_load(NetClientState *nc)
>              return dev_written;
>          }
>
> -        return *((virtio_net_ctrl_ack *)s->cvq_cmd_in_buffer) != VIRTIO_NET_OK;
> +        return *s->cvq_cmd_in_buffer != VIRTIO_NET_OK;
>      }
>
>      return 0;
> @@ -491,8 +493,7 @@ static int vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
>          goto out;
>      }
>
> -    memcpy(&status, s->cvq_cmd_in_buffer, sizeof(status));
> -    if (status != VIRTIO_NET_OK) {
> +    if (*s->cvq_cmd_in_buffer != VIRTIO_NET_OK) {
>          return VIRTIO_NET_ERR;
>      }
>
> --
> 2.31.1
>

[PATCH v2 2/6] vdpa: extract vhost_vdpa_net_load_mac from vhost_vdpa_net_load

[Eugenio Pérez]

Since there may be many commands we need to issue to load the NIC
state, let's split them in individual functions

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
--
v2: Add vhost_vdpa_net_load_cmd helper
---
 net/vhost-vdpa.c | 54 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 18 deletions(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 468e460ac2..c89e2262d9 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -365,35 +365,31 @@ static ssize_t vhost_vdpa_net_cvq_add(VhostVDPAState *s, size_t out_len,
     return vhost_svq_poll(svq);
 }
 
-static int vhost_vdpa_net_load(NetClientState *nc)
+static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s,
+                                       const struct virtio_net_ctrl_hdr *ctrl,
+                                       const void *data, size_t data_size)
 {
-    VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
-    const struct vhost_vdpa *v = &s->vhost_vdpa;
-    const VirtIONet *n;
-    uint64_t features;
+    assert(data_size < vhost_vdpa_net_cvq_cmd_page_len() - sizeof(*ctrl));
 
-    assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
+    memcpy(s->cvq_cmd_out_buffer, ctrl, sizeof(*ctrl));
+    memcpy(s->cvq_cmd_out_buffer + sizeof(ctrl), data, data_size);
 
-    if (!v->shadow_vqs_enabled) {
-        return 0;
-    }
+    return vhost_vdpa_net_cvq_add(s, sizeof(ctrl) + data_size,
+                                  sizeof(virtio_net_ctrl_ack));
+}
 
-    n = VIRTIO_NET(v->dev->vdev);
-    features = n->parent_obj.guest_features;
+static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n)
+{
+    uint64_t features = n->parent_obj.guest_features;
     if (features & BIT_ULL(VIRTIO_NET_F_CTRL_MAC_ADDR)) {
         const struct virtio_net_ctrl_hdr ctrl = {
             .class = VIRTIO_NET_CTRL_MAC,
             .cmd = VIRTIO_NET_CTRL_MAC_ADDR_SET,
         };
-        char *cursor = s->cvq_cmd_out_buffer;
         ssize_t dev_written;
 
-        memcpy(cursor, &ctrl, sizeof(ctrl));
-        cursor += sizeof(ctrl);
-        memcpy(cursor, n->mac, sizeof(n->mac));
-
-        dev_written = vhost_vdpa_net_cvq_add(s, sizeof(ctrl) + sizeof(n->mac),
-                                             sizeof(virtio_net_ctrl_ack));
+        dev_written = vhost_vdpa_net_load_cmd(s, &ctrl, n->mac,
+                                              sizeof(n->mac));
         if (unlikely(dev_written < 0)) {
             return dev_written;
         }
@@ -404,6 +400,28 @@ static int vhost_vdpa_net_load(NetClientState *nc)
     return 0;
 }
 
+static int vhost_vdpa_net_load(NetClientState *nc)
+{
+    VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
+    struct vhost_vdpa *v = &s->vhost_vdpa;
+    const VirtIONet *n;
+    int r;
+
+    assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
+
+    if (!v->shadow_vqs_enabled) {
+        return 0;
+    }
+
+    n = VIRTIO_NET(v->dev->vdev);
+    r = vhost_vdpa_net_load_mac(s, n);
+    if (unlikely(r < 0)) {
+        return r;
+    }
+
+    return 0;
+}
+
 static NetClientInfo net_vhost_vdpa_cvq_info = {
     .type = NET_CLIENT_DRIVER_VHOST_VDPA,
     .size = sizeof(VhostVDPAState),
-- 
2.31.1

Re: [PATCH v2 2/6] vdpa: extract vhost_vdpa_net_load_mac from vhost_vdpa_net_load

[Jason Wang]

On Thu, Aug 25, 2022 at 2:36 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> Since there may be many commands we need to issue to load the NIC
> state, let's split them in individual functions
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> --
> v2: Add vhost_vdpa_net_load_cmd helper
> ---
>  net/vhost-vdpa.c | 54 ++++++++++++++++++++++++++++++++----------------
>  1 file changed, 36 insertions(+), 18 deletions(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 468e460ac2..c89e2262d9 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -365,35 +365,31 @@ static ssize_t vhost_vdpa_net_cvq_add(VhostVDPAState *s, size_t out_len,
>      return vhost_svq_poll(svq);
>  }
>
> -static int vhost_vdpa_net_load(NetClientState *nc)
> +static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s,
> +                                       const struct virtio_net_ctrl_hdr *ctrl,
> +                                       const void *data, size_t data_size)
>  {
> -    VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> -    const struct vhost_vdpa *v = &s->vhost_vdpa;
> -    const VirtIONet *n;
> -    uint64_t features;
> +    assert(data_size < vhost_vdpa_net_cvq_cmd_page_len() - sizeof(*ctrl));
>
> -    assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
> +    memcpy(s->cvq_cmd_out_buffer, ctrl, sizeof(*ctrl));
> +    memcpy(s->cvq_cmd_out_buffer + sizeof(ctrl), data, data_size);
>
> -    if (!v->shadow_vqs_enabled) {
> -        return 0;
> -    }
> +    return vhost_vdpa_net_cvq_add(s, sizeof(ctrl) + data_size,
> +                                  sizeof(virtio_net_ctrl_ack));
> +}
>
> -    n = VIRTIO_NET(v->dev->vdev);
> -    features = n->parent_obj.guest_features;
> +static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n)
> +{
> +    uint64_t features = n->parent_obj.guest_features;
>      if (features & BIT_ULL(VIRTIO_NET_F_CTRL_MAC_ADDR)) {
>          const struct virtio_net_ctrl_hdr ctrl = {
>              .class = VIRTIO_NET_CTRL_MAC,
>              .cmd = VIRTIO_NET_CTRL_MAC_ADDR_SET,
>          };
> -        char *cursor = s->cvq_cmd_out_buffer;
>          ssize_t dev_written;
>
> -        memcpy(cursor, &ctrl, sizeof(ctrl));
> -        cursor += sizeof(ctrl);
> -        memcpy(cursor, n->mac, sizeof(n->mac));
> -
> -        dev_written = vhost_vdpa_net_cvq_add(s, sizeof(ctrl) + sizeof(n->mac),
> -                                             sizeof(virtio_net_ctrl_ack));
> +        dev_written = vhost_vdpa_net_load_cmd(s, &ctrl, n->mac,
> +                                              sizeof(n->mac));

Patch looks good, if there's another respin, I'd accept class/cmd as
parameter then there's no need for the caller to prepare
virtio_net_ctrl_hdr.

Thanks

>          if (unlikely(dev_written < 0)) {
>              return dev_written;
>          }
> @@ -404,6 +400,28 @@ static int vhost_vdpa_net_load(NetClientState *nc)
>      return 0;
>  }
>
> +static int vhost_vdpa_net_load(NetClientState *nc)
> +{
> +    VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> +    struct vhost_vdpa *v = &s->vhost_vdpa;
> +    const VirtIONet *n;
> +    int r;
> +
> +    assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
> +
> +    if (!v->shadow_vqs_enabled) {
> +        return 0;
> +    }
> +
> +    n = VIRTIO_NET(v->dev->vdev);
> +    r = vhost_vdpa_net_load_mac(s, n);
> +    if (unlikely(r < 0)) {
> +        return r;
> +    }
> +
> +    return 0;
> +}
> +
>  static NetClientInfo net_vhost_vdpa_cvq_info = {
>      .type = NET_CLIENT_DRIVER_VHOST_VDPA,
>      .size = sizeof(VhostVDPAState),
> --
> 2.31.1
>

[PATCH v2 3/6] vdpa: Add vhost_vdpa_net_load_mq

[Eugenio Pérez]

Same way as with the MAC, restore the expected number of queues at
device's start.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 net/vhost-vdpa.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index c89e2262d9..77c85f4ddd 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -400,6 +400,30 @@ static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n)
     return 0;
 }
 
+static int vhost_vdpa_net_load_mq(VhostVDPAState *s,
+                                  const VirtIONet *n)
+{
+    const struct virtio_net_ctrl_hdr ctrl = {
+        .class = VIRTIO_NET_CTRL_MQ,
+        .cmd = VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET,
+    };
+    struct virtio_net_ctrl_mq mq;
+    uint64_t features = n->parent_obj.guest_features;
+    ssize_t dev_written;
+
+    if (!(features & BIT_ULL(VIRTIO_NET_F_MQ))) {
+        return 0;
+    }
+
+    mq.virtqueue_pairs = cpu_to_le16(n->curr_queue_pairs);
+    dev_written = vhost_vdpa_net_load_cmd(s, &ctrl, &mq, sizeof(mq));
+    if (unlikely(dev_written < 0)) {
+        return dev_written;
+    }
+
+    return *s->cvq_cmd_in_buffer != VIRTIO_NET_OK;
+}
+
 static int vhost_vdpa_net_load(NetClientState *nc)
 {
     VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
@@ -418,6 +442,10 @@ static int vhost_vdpa_net_load(NetClientState *nc)
     if (unlikely(r < 0)) {
         return r;
     }
+    r = vhost_vdpa_net_load_mq(s, n);
+    if (unlikely(r)) {
+        return r;
+    }
 
     return 0;
 }
-- 
2.31.1

[PATCH v2 4/6] vdpa: validate MQ CVQ commands

[Eugenio Pérez]

So we are sure we can update the device model properly before sending to
the device.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 net/vhost-vdpa.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 77c85f4ddd..b070c029e7 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -488,6 +488,15 @@ static bool vhost_vdpa_net_cvq_validate_cmd(const void *out_buf, size_t len)
                           __func__, ctrl.cmd);
         };
         break;
+    case VIRTIO_NET_CTRL_MQ:
+        switch (ctrl.cmd) {
+        case VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET:
+            return true;
+        default:
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid mq cmd %u\n",
+                          __func__, ctrl.cmd);
+        };
+        break;
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid control class %u\n",
                       __func__, ctrl.class);
-- 
2.31.1

[PATCH v2 5/6] virtio-net: Update virtio-net curr_queue_pairs in vdpa backends

[Eugenio Pérez]

It was returned as error before. Instead of it, simply update the
corresponding field so qemu can send it in the migration data.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 hw/net/virtio-net.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index dd0d056fde..63a8332cd0 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1412,19 +1412,14 @@ static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
         return VIRTIO_NET_ERR;
     }
 
-    /* Avoid changing the number of queue_pairs for vdpa device in
-     * userspace handler. A future fix is needed to handle the mq
-     * change in userspace handler with vhost-vdpa. Let's disable
-     * the mq handling from userspace for now and only allow get
-     * done through the kernel. Ripples may be seen when falling
-     * back to userspace, but without doing it qemu process would
-     * crash on a recursive entry to virtio_net_set_status().
-     */
+    n->curr_queue_pairs = queue_pairs;
     if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
-        return VIRTIO_NET_ERR;
+        /*
+         * Avoid updating the backend for a vdpa device: We're only interested
+         * in updating the device model queues.
+         */
+        return VIRTIO_NET_OK;
     }
-
-    n->curr_queue_pairs = queue_pairs;
     /* stop the backend before changing the number of queue_pairs to avoid handling a
      * disabled queue */
     virtio_net_set_status(vdev, vdev->status);
-- 
2.31.1

[PATCH v2 6/6] vdpa: Allow MQ feture in SVQ

[Eugenio Pérez]

Finally enable SVQ with MQ feature.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 net/vhost-vdpa.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index b070c029e7..0376151b60 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -94,6 +94,7 @@ static const uint64_t vdpa_svq_device_features =
     BIT_ULL(VIRTIO_NET_F_MRG_RXBUF) |
     BIT_ULL(VIRTIO_NET_F_STATUS) |
     BIT_ULL(VIRTIO_NET_F_CTRL_VQ) |
+    BIT_ULL(VIRTIO_NET_F_MQ) |
     BIT_ULL(VIRTIO_F_ANY_LAYOUT) |
     BIT_ULL(VIRTIO_NET_F_CTRL_MAC_ADDR) |
     BIT_ULL(VIRTIO_NET_F_RSC_EXT) |
-- 
2.31.1

Re: [PATCH v2 0/6] Vhost-vdpa Shadow Virtqueue multiqueue support.

[Jason Wang]

On Thu, Aug 25, 2022 at 2:35 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> This series enables shadowed CVQ to intercept multiqueue commands through
> shadowed CVQ, update the virtio NIC device model so qemu send it in a
> migration, and the restore of that MQ state in the destination.
>
> It needs to be applied on top of [1].
>
> [1] https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg02965.html
>
> v2:
> * Add vhost_vdpa_net_load_cmd helper to avoid out buffers castings.
> * Make cvq_cmd_in_buffer virtio_net_ctrl_ack type.
>
> Eugenio Pérez (6):
>   vdpa: Make VhostVDPAState cvq_cmd_out_buffer control ack type
>   vdpa: extract vhost_vdpa_net_load_mac from vhost_vdpa_net_load
>   vdpa: Add vhost_vdpa_net_load_mq
>   vdpa: validate MQ CVQ commands
>   virtio-net: Update virtio-net curr_queue_pairs in vdpa backends
>   vdpa: Allow MQ feture in SVQ

(Typo here).

Looks good, let's wait for confirmation from Si Wei. Then I can queue
this series.

Thanks

>
>  hw/net/virtio-net.c |  17 +++-----
>  net/vhost-vdpa.c    | 101 ++++++++++++++++++++++++++++++++++----------
>  2 files changed, 85 insertions(+), 33 deletions(-)
>
> --
> 2.31.1
>
>