On Wed, Jul 19, 2023 at 9:54 AM Hawkins Jiawei <yin31...@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 <yin31...@gmail.com>
> ---
> net/vhost-vdpa.c | 38 ++++++++++++++++++++------------------
> 1 file changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index d1dd140bf6..6b16c8ece0 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,
> + struct iovec *out_sg, size_t out_num,
> + 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 */
> + struct iovec out = {
> + .iov_base = s->cvq_cmd_out_buffer,
> + .iov_len = sizeof(ctrl) + data_size,
> + };
> + 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,9 +1222,7 @@ static int
> vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
> struct iovec out = {
> .iov_base = s->cvq_cmd_out_buffer,
> };
> - /* in buffer used for device model */
> - const struct iovec in = {
> - .iov_base = &status,
> + struct iovec in = {
What if instead of reusing "in" we declare a new struct iovec in the
condition that calls vhost_vdpa_net_cvq_add? Something in the line of
"device_in".
I'm also ok with this code, but splitting them would reduce the
possibility of sending the wrong one to the device / virtio device
model by mistake.
Thanks!
> .iov_len = sizeof(status),
> };
> ssize_t dev_written = -EINVAL;
> @@ -1232,6 +1230,8 @@ static int
> vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
> out.iov_len = iov_to_buf(elem->out_sg, elem->out_num, 0,
> s->cvq_cmd_out_buffer,
> vhost_vdpa_net_cvq_cmd_page_len());
> + /* In buffer used for the vdpa device */
> + in.iov_base = s->status;
>
> ctrl = s->cvq_cmd_out_buffer;
> if (ctrl->class == VIRTIO_NET_CTRL_ANNOUNCE) {
> @@ -1260,7 +1260,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, &in, 1);
> if (unlikely(dev_written < 0)) {
> goto out;
> }
> @@ -1276,6 +1276,8 @@ static int
> vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
> }
>
> status = VIRTIO_NET_ERR;
> + /* In buffer used for the device model */
> + in.iov_base = &status;
> virtio_net_handle_ctrl_iov(svq->vdev, &in, 1, &out, 1);
> if (status != VIRTIO_NET_OK) {
> error_report("Bad CVQ processing in model");
> --
> 2.25.1
>
