Re: [PATCH v2 19/19] vdpa: Add x-svq to NetdevVhostVDPAOptions

[Jason Wang]

On Fri, Jul 15, 2022 at 12:32 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> Finally offering the possibility to enable SVQ from the command line.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> Acked-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qapi/net.json    |  9 +++++-
>  net/vhost-vdpa.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 77 insertions(+), 4 deletions(-)
>
> diff --git a/qapi/net.json b/qapi/net.json
> index 9af11e9a3b..75ba2cb989 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -445,12 +445,19 @@
>  # @queues: number of queues to be created for multiqueue vhost-vdpa
>  #          (default: 1)
>  #
> +# @x-svq: Start device with (experimental) shadow virtqueue. (Since 7.1)
> +#         (default: false)
> +#
> +# Features:
> +# @unstable: Member @x-svq is experimental.
> +#
>  # Since: 5.1
>  ##
>  { 'struct': 'NetdevVhostVDPAOptions',
>    'data': {
>      '*vhostdev':     'str',
> -    '*queues':       'int' } }
> +    '*queues':       'int',
> +    '*x-svq':        {'type': 'bool', 'features' : [ 'unstable'] } } }
>
>  ##
>  # @NetdevVmnetHostOptions:
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 7ccf9eaf4d..85148a5114 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -75,6 +75,28 @@ const int vdpa_feature_bits[] = {
>      VHOST_INVALID_FEATURE_BIT
>  };
>
> +/** Supported device specific feature bits with SVQ */
> +static const uint64_t vdpa_svq_device_features =
> +    BIT_ULL(VIRTIO_NET_F_CSUM) |
> +    BIT_ULL(VIRTIO_NET_F_GUEST_CSUM) |
> +    BIT_ULL(VIRTIO_NET_F_MTU) |
> +    BIT_ULL(VIRTIO_NET_F_MAC) |
> +    BIT_ULL(VIRTIO_NET_F_GUEST_TSO4) |
> +    BIT_ULL(VIRTIO_NET_F_GUEST_TSO6) |
> +    BIT_ULL(VIRTIO_NET_F_GUEST_ECN) |
> +    BIT_ULL(VIRTIO_NET_F_GUEST_UFO) |
> +    BIT_ULL(VIRTIO_NET_F_HOST_TSO4) |
> +    BIT_ULL(VIRTIO_NET_F_HOST_TSO6) |
> +    BIT_ULL(VIRTIO_NET_F_HOST_ECN) |
> +    BIT_ULL(VIRTIO_NET_F_HOST_UFO) |
> +    BIT_ULL(VIRTIO_NET_F_MRG_RXBUF) |
> +    BIT_ULL(VIRTIO_NET_F_STATUS) |
> +    BIT_ULL(VIRTIO_NET_F_CTRL_VQ) |
> +    BIT_ULL(VIRTIO_F_ANY_LAYOUT) |
> +    BIT_ULL(VIRTIO_NET_F_CTRL_MAC_ADDR) |
> +    BIT_ULL(VIRTIO_NET_F_RSC_EXT) |
> +    BIT_ULL(VIRTIO_NET_F_STANDBY);

We need to have a plan for the full feature support like

indirect, event_index, and packed.

I can help in developing some of these if you wish.

Thanks

> +
>  VHostNetState *vhost_vdpa_get_vhost_net(NetClientState *nc)
>  {
>      VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> @@ -133,9 +155,13 @@ err_init:
>  static void vhost_vdpa_cleanup(NetClientState *nc)
>  {
>      VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> +    struct vhost_dev *dev = &s->vhost_net->dev;
>
>      qemu_vfree(s->cvq_cmd_out_buffer);
>      qemu_vfree(s->cvq_cmd_in_buffer);
> +    if (dev->vq_index + dev->nvqs == dev->vq_index_end) {
> +        g_clear_pointer(&s->vhost_vdpa.iova_tree, vhost_iova_tree_delete);
> +    }
>      if (s->vhost_net) {
>          vhost_net_cleanup(s->vhost_net);
>          g_free(s->vhost_net);
> @@ -437,7 +463,9 @@ static NetClientState *net_vhost_vdpa_init(NetClientState 
> *peer,
>                                             int vdpa_device_fd,
>                                             int queue_pair_index,
>                                             int nvqs,
> -                                           bool is_datapath)
> +                                           bool is_datapath,
> +                                           bool svq,
> +                                           VhostIOVATree *iova_tree)
>  {
>      NetClientState *nc = NULL;
>      VhostVDPAState *s;
> @@ -455,6 +483,8 @@ static NetClientState *net_vhost_vdpa_init(NetClientState 
> *peer,
>
>      s->vhost_vdpa.device_fd = vdpa_device_fd;
>      s->vhost_vdpa.index = queue_pair_index;
> +    s->vhost_vdpa.shadow_vqs_enabled = svq;
> +    s->vhost_vdpa.iova_tree = iova_tree;
>      if (!is_datapath) {
>          s->cvq_cmd_out_buffer = qemu_memalign(qemu_real_host_page_size(),
>                                              
> vhost_vdpa_net_cvq_cmd_page_len());
> @@ -465,6 +495,8 @@ static NetClientState *net_vhost_vdpa_init(NetClientState 
> *peer,
>
>          s->vhost_vdpa.shadow_vq_ops = &vhost_vdpa_net_svq_ops;
>          s->vhost_vdpa.shadow_vq_ops_opaque = s;
> +        error_setg(&s->vhost_vdpa.migration_blocker,
> +                   "Migration disabled: vhost-vdpa uses CVQ.");
>      }
>      ret = vhost_vdpa_add(nc, (void *)&s->vhost_vdpa, queue_pair_index, nvqs);
>      if (ret) {
> @@ -474,6 +506,14 @@ static NetClientState 
> *net_vhost_vdpa_init(NetClientState *peer,
>      return nc;
>  }
>
> +static int vhost_vdpa_get_iova_range(int fd,
> +                                     struct vhost_vdpa_iova_range 
> *iova_range)
> +{
> +    int ret = ioctl(fd, VHOST_VDPA_GET_IOVA_RANGE, iova_range);
> +
> +    return ret < 0 ? -errno : 0;
> +}
> +
>  static int vhost_vdpa_get_features(int fd, uint64_t *features, Error **errp)
>  {
>      int ret = ioctl(fd, VHOST_GET_FEATURES, features);
> @@ -524,6 +564,7 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char 
> *name,
>      uint64_t features;
>      int vdpa_device_fd;
>      g_autofree NetClientState **ncs = NULL;
> +    g_autoptr(VhostIOVATree) iova_tree = NULL;
>      NetClientState *nc;
>      int queue_pairs, r, i, has_cvq = 0;
>
> @@ -551,22 +592,45 @@ int net_init_vhost_vdpa(const Netdev *netdev, const 
> char *name,
>          return queue_pairs;
>      }
>
> +    if (opts->x_svq) {
> +        struct vhost_vdpa_iova_range iova_range;
> +
> +        uint64_t invalid_dev_features =
> +            features & ~vdpa_svq_device_features &
> +            /* Transport are all accepted at this point */
> +            ~MAKE_64BIT_MASK(VIRTIO_TRANSPORT_F_START,
> +                             VIRTIO_TRANSPORT_F_END - 
> VIRTIO_TRANSPORT_F_START);
> +
> +        if (invalid_dev_features) {
> +            error_setg(errp, "vdpa svq does not work with features 0x%" 
> PRIx64,
> +                       invalid_dev_features);
> +            goto err_svq;
> +        }
> +
> +        vhost_vdpa_get_iova_range(vdpa_device_fd, &iova_range);
> +        iova_tree = vhost_iova_tree_new(iova_range.first, iova_range.last);
> +    }
> +
>      ncs = g_malloc0(sizeof(*ncs) * queue_pairs);
>
>      for (i = 0; i < queue_pairs; i++) {
>          ncs[i] = net_vhost_vdpa_init(peer, TYPE_VHOST_VDPA, name,
> -                                     vdpa_device_fd, i, 2, true);
> +                                     vdpa_device_fd, i, 2, true, opts->x_svq,
> +                                     iova_tree);
>          if (!ncs[i])
>              goto err;
>      }
>
>      if (has_cvq) {
>          nc = net_vhost_vdpa_init(peer, TYPE_VHOST_VDPA, name,
> -                                 vdpa_device_fd, i, 1, false);
> +                                 vdpa_device_fd, i, 1, false,
> +                                 opts->x_svq, iova_tree);
>          if (!nc)
>              goto err;
>      }
>
> +    /* iova_tree ownership belongs to last NetClientState */
> +    g_steal_pointer(&iova_tree);
>      return 0;
>
>  err:
> @@ -575,6 +639,8 @@ err:
>              qemu_del_net_client(ncs[i]);
>          }
>      }
> +
> +err_svq:
>      qemu_close(vdpa_device_fd);
>
>      return -1;
> --
> 2.31.1
>