Re: [PATCH v4 2/8] vdpa: Use iovec for vhost_vdpa_net_cvq_add()

[Eugenio Perez Martin]

On Tue, Aug 29, 2023 at 7:55 AM Hawkins Jiawei <yin31149@gmail.com> wrote:
>
> Next patches in this series will no longer perform an
> immediate poll and check of the device's used buffers
> for each CVQ state load command. Consequently, there
> will be multiple pending buffers in the shadow VirtQueue,
> making it a must for every control command to have its
> own buffer.
>
> To achieve this, this patch refactor vhost_vdpa_net_cvq_add()
> to accept `struct iovec`, which eliminates the coupling of
> control commands to `s->cvq_cmd_out_buffer` and `s->status`,
> allowing them to use their own buffer.
>
> Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>

Acked-by: Eugenio Pérez <eperezma@redhat.com>

> ---
> v4:
>   - split `in` to `vdpa_in` and `model_in` instead of reusing `in`
> in vhost_vdpa_net_handle_ctrl_avail() suggested by Eugenio
>
> v3: 
> https://lore.kernel.org/all/b1d473772ec4bcb254ab0d12430c9b1efe758606.1689748694.git.yin31149@gmail.com/
>
>  net/vhost-vdpa.c | 39 ++++++++++++++++++++++-----------------
>  1 file changed, 22 insertions(+), 17 deletions(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 3acda8591a..a875767ee9 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -596,22 +596,14 @@ static void vhost_vdpa_net_cvq_stop(NetClientState *nc)
>      vhost_vdpa_net_client_stop(nc);
>  }
>
> -static ssize_t vhost_vdpa_net_cvq_add(VhostVDPAState *s, size_t out_len,
> -                                      size_t in_len)
> +static ssize_t vhost_vdpa_net_cvq_add(VhostVDPAState *s,
> +                                    const struct iovec *out_sg, size_t 
> out_num,
> +                                    const struct iovec *in_sg, size_t in_num)
>  {
> -    /* Buffers for the device */
> -    const struct iovec out = {
> -        .iov_base = s->cvq_cmd_out_buffer,
> -        .iov_len = out_len,
> -    };
> -    const struct iovec in = {
> -        .iov_base = s->status,
> -        .iov_len = sizeof(virtio_net_ctrl_ack),
> -    };
>      VhostShadowVirtqueue *svq = g_ptr_array_index(s->vhost_vdpa.shadow_vqs, 
> 0);
>      int r;
>
> -    r = vhost_svq_add(svq, &out, 1, &in, 1, NULL);
> +    r = vhost_svq_add(svq, out_sg, out_num, in_sg, in_num, NULL);
>      if (unlikely(r != 0)) {
>          if (unlikely(r == -ENOSPC)) {
>              qemu_log_mask(LOG_GUEST_ERROR, "%s: No space on device queue\n",
> @@ -637,6 +629,15 @@ static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState 
> *s, uint8_t class,
>          .cmd = cmd,
>      };
>      size_t data_size = iov_size(data_sg, data_num);
> +    /* Buffers for the device */
> +    const struct iovec out = {
> +        .iov_base = s->cvq_cmd_out_buffer,
> +        .iov_len = sizeof(ctrl) + data_size,
> +    };
> +    const struct iovec in = {
> +        .iov_base = s->status,
> +        .iov_len = sizeof(*s->status),
> +    };
>
>      assert(data_size < vhost_vdpa_net_cvq_cmd_page_len() - sizeof(ctrl));
>
> @@ -647,8 +648,7 @@ static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s, 
> uint8_t class,
>      iov_to_buf(data_sg, data_num, 0,
>                 s->cvq_cmd_out_buffer + sizeof(ctrl), data_size);
>
> -    return vhost_vdpa_net_cvq_add(s, data_size + sizeof(ctrl),
> -                                  sizeof(virtio_net_ctrl_ack));
> +    return vhost_vdpa_net_cvq_add(s, &out, 1, &in, 1);
>  }
>
>  static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n)
> @@ -1222,10 +1222,15 @@ static int 
> vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
>          .iov_base = s->cvq_cmd_out_buffer,
>      };
>      /* in buffer used for device model */
> -    const struct iovec in = {
> +    const struct iovec model_in = {
>          .iov_base = &status,
>          .iov_len = sizeof(status),
>      };
> +    /* in buffer used for vdpa device */
> +    const struct iovec vdpa_in = {
> +        .iov_base = s->status,
> +        .iov_len = sizeof(*s->status),
> +    };
>      ssize_t dev_written = -EINVAL;
>
>      out.iov_len = iov_to_buf(elem->out_sg, elem->out_num, 0,
> @@ -1259,7 +1264,7 @@ static int 
> vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
>              goto out;
>          }
>      } else {
> -        dev_written = vhost_vdpa_net_cvq_add(s, out.iov_len, sizeof(status));
> +        dev_written = vhost_vdpa_net_cvq_add(s, &out, 1, &vdpa_in, 1);
>          if (unlikely(dev_written < 0)) {
>              goto out;
>          }
> @@ -1275,7 +1280,7 @@ static int 
> vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
>      }
>
>      status = VIRTIO_NET_ERR;
> -    virtio_net_handle_ctrl_iov(svq->vdev, &in, 1, &out, 1);
> +    virtio_net_handle_ctrl_iov(svq->vdev, &model_in, 1, &out, 1);
>      if (status != VIRTIO_NET_OK) {
>          error_report("Bad CVQ processing in model");
>      }
> --
> 2.25.1
>