Re: [PULL 40/41] vhost-vdpa: introduce vhost-vdpa backend

[Cindy Lu]

On Wed, Jul 8, 2020 at 8:08 AM Bruce Rogers <brogers@suse.com> wrote:
>
> On Fri, 2020-07-03 at 05:05 -0400, Michael S. Tsirkin wrote:
> > From: Cindy Lu <lulu@redhat.com>
> >
> > Currently we have 2 types of vhost backends in QEMU: vhost kernel and
> > vhost-user. The above patch provides a generic device for vDPA
> > purpose,
> > this vDPA device exposes to user space a non-vendor-specific
> > configuration
> > interface for setting up a vhost HW accelerator, this patch set
> > introduces
> > a third vhost backend called vhost-vdpa based on the vDPA interface.
> >
> > Vhost-vdpa usage:
> >
> > qemu-system-x86_64 -cpu host -enable-kvm \
> >     ......
> >     -netdev type=vhost-vdpa,vhostdev=/dev/vhost-vdpa-id,id=vhost-
> > vdpa0 \
> >     -device virtio-net-pci,netdev=vhost-vdpa0,page-per-vq=on \
> >
> > Signed-off-by: Lingshan zhu <lingshan.zhu@intel.com>
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > Message-Id: <20200701145538.22333-14-lulu@redhat.com>
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Acked-by: Jason Wang <jasowang@redhat.com>
> > ---
> >  configure                         |  21 ++
> >  include/hw/virtio/vhost-backend.h |   4 +-
> >  include/hw/virtio/vhost-vdpa.h    |  26 ++
> >  include/hw/virtio/vhost.h         |   7 +
> >  hw/net/vhost_net.c                |  19 +-
> >  hw/net/virtio-net.c               |  19 ++
> >  hw/virtio/vhost-backend.c         |   6 +
> >  hw/virtio/vhost-vdpa.c            | 475
> > ++++++++++++++++++++++++++++++
> >  docs/interop/index.rst            |   1 +
> >  docs/interop/vhost-vdpa.rst       |  17 ++
> >  hw/virtio/Makefile.objs           |   1 +
> >  qemu-options.hx                   |  12 +
> >  12 files changed, 601 insertions(+), 7 deletions(-)
> >  create mode 100644 include/hw/virtio/vhost-vdpa.h
> >  create mode 100644 hw/virtio/vhost-vdpa.c
> >  create mode 100644 docs/interop/vhost-vdpa.rst
> >
> > diff --git a/configure b/configure
> > index 4a22dcd563..3db7f20185 100755
> > --- a/configure
> > +++ b/configure
> > @@ -1575,6 +1575,10 @@ for opt do
> >    ;;
> >    --enable-vhost-user) vhost_user="yes"
> >    ;;
> > +  --disable-vhost-vdpa) vhost_vdpa="no"
> > +  ;;
> > +  --enable-vhost-vdpa) vhost_vdpa="yes"
> > +  ;;
> >    --disable-vhost-kernel) vhost_kernel="no"
> >    ;;
> >    --enable-vhost-kernel) vhost_kernel="yes"
> > @@ -1883,6 +1887,7 @@ disabled with --disable-FEATURE, default is
> > enabled if available:
> >    vhost-crypto    vhost-user-crypto backend support
> >    vhost-kernel    vhost kernel backend support
> >    vhost-user      vhost-user backend support
> > +  vhost-vdpa      vhost-vdpa kernel backend support
> >    spice           spice
> >    rbd             rados block device (rbd)
> >    libiscsi        iscsi support
> > @@ -2394,6 +2399,10 @@ test "$vhost_user" = "" && vhost_user=yes
> >  if test "$vhost_user" = "yes" && test "$mingw32" = "yes"; then
> >    error_exit "vhost-user isn't available on win32"
> >  fi
> > +test "$vhost_vdpa" = "" && vhost_vdpa=$linux
> > +if test "$vhost_vdpa" = "yes" && test "$linux" != "yes"; then
> > +  error_exit "vhost-vdpa is only available on Linux"
> > +fi
> >  test "$vhost_kernel" = "" && vhost_kernel=$linux
> >  if test "$vhost_kernel" = "yes" && test "$linux" != "yes"; then
> >    error_exit "vhost-kernel is only available on Linux"
> > @@ -2422,6 +2431,11 @@ test "$vhost_user_fs" = "" &&
> > vhost_user_fs=$vhost_user
> >  if test "$vhost_user_fs" = "yes" && test "$vhost_user" = "no"; then
> >    error_exit "--enable-vhost-user-fs requires --enable-vhost-user"
> >  fi
> > +#vhost-vdpa backends
> > +test "$vhost_net_vdpa" = "" && vhost_net_vdpa=$vhost_vdpa
> > +if test "$vhost_net_vdpa" = "yes" && test "$vhost_vdpa" = "no"; then
> > +  error_exit "--enable-vhost-net-vdpa requires --enable-vhost-vdpa"
> > +fi
> >
> >  # OR the vhost-kernel and vhost-user values for simplicity
> >  if test "$vhost_net" = ""; then
> > @@ -6936,6 +6950,7 @@ echo "vhost-scsi support $vhost_scsi"
> >  echo "vhost-vsock support $vhost_vsock"
> >  echo "vhost-user support $vhost_user"
> >  echo "vhost-user-fs support $vhost_user_fs"
> > +echo "vhost-vdpa support $vhost_vdpa"
> >  echo "Trace backends    $trace_backends"
> >  if have_backend "simple"; then
> >  echo "Trace output file $trace_file-<pid>"
> > @@ -7437,6 +7452,9 @@ fi
> >  if test "$vhost_net_user" = "yes" ; then
> >    echo "CONFIG_VHOST_NET_USER=y" >> $config_host_mak
> >  fi
> > +if test "$vhost_net_vdpa" = "yes" ; then
> > +  echo "CONFIG_VHOST_NET_VDPA=y" >> $config_host_mak
> > +fi
> >  if test "$vhost_crypto" = "yes" ; then
> >    echo "CONFIG_VHOST_CRYPTO=y" >> $config_host_mak
> >  fi
> > @@ -7452,6 +7470,9 @@ fi
> >  if test "$vhost_user" = "yes" ; then
> >    echo "CONFIG_VHOST_USER=y" >> $config_host_mak
> >  fi
> > +if test "$vhost_vdpa" = "yes" ; then
> > +  echo "CONFIG_VHOST_VDPA=y" >> $config_host_mak
> > +fi
> >  if test "$vhost_user_fs" = "yes" ; then
> >    echo "CONFIG_VHOST_USER_FS=y" >> $config_host_mak
> >  fi
> > diff --git a/include/hw/virtio/vhost-backend.h
> > b/include/hw/virtio/vhost-backend.h
> > index e7cb8d028c..8825bd278f 100644
> > --- a/include/hw/virtio/vhost-backend.h
> > +++ b/include/hw/virtio/vhost-backend.h
> > @@ -17,7 +17,8 @@ typedef enum VhostBackendType {
> >      VHOST_BACKEND_TYPE_NONE = 0,
> >      VHOST_BACKEND_TYPE_KERNEL = 1,
> >      VHOST_BACKEND_TYPE_USER = 2,
> > -    VHOST_BACKEND_TYPE_MAX = 3,
> > +    VHOST_BACKEND_TYPE_VDPA = 3,
> > +    VHOST_BACKEND_TYPE_MAX = 4,
> >  } VhostBackendType;
> >
> >  typedef enum VhostSetConfigType {
> > @@ -170,6 +171,7 @@ typedef struct VhostOps {
> >  } VhostOps;
> >
> >  extern const VhostOps user_ops;
> > +extern const VhostOps vdpa_ops;
> >
> >  int vhost_set_backend_type(struct vhost_dev *dev,
> >                             VhostBackendType backend_type);
> > diff --git a/include/hw/virtio/vhost-vdpa.h
> > b/include/hw/virtio/vhost-vdpa.h
> > new file mode 100644
> > index 0000000000..6455663388
> > --- /dev/null
> > +++ b/include/hw/virtio/vhost-vdpa.h
> > @@ -0,0 +1,26 @@
> > +/*
> > + * vhost-vdpa.h
> > + *
> > + * Copyright(c) 2017-2018 Intel Corporation.
> > + * Copyright(c) 2020 Red Hat, Inc.
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2
> > or later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#ifndef HW_VIRTIO_VHOST_VDPA_H
> > +#define HW_VIRTIO_VHOST_VDPA_H
> > +
> > +#include "hw/virtio/virtio.h"
> > +
> > +typedef struct vhost_vdpa {
> > +    int device_fd;
> > +    uint32_t msg_type;
> > +    MemoryListener listener;
> > +} VhostVDPA;
> > +
> > +extern AddressSpace address_space_memory;
> > +extern int vhost_vdpa_get_device_id(struct vhost_dev *dev,
> > +                                   uint32_t *device_id);
> > +#endif
> > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> > index 085450c6f8..767a95ec0b 100644
> > --- a/include/hw/virtio/vhost.h
> > +++ b/include/hw/virtio/vhost.h
> > @@ -92,6 +92,13 @@ struct vhost_dev {
> >      const VhostDevConfigOps *config_ops;
> >  };
> >
> > +struct vhost_net {
> > +    struct vhost_dev dev;
> > +    struct vhost_virtqueue vqs[2];
> > +    int backend;
> > +    NetClientState *nc;
> > +};
> > +
> >  int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> >                     VhostBackendType backend_type,
> >                     uint32_t busyloop_timeout);
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index 4561665f6b..e165594cf3 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -17,8 +17,10 @@
> >  #include "net/net.h"
> >  #include "net/tap.h"
> >  #include "net/vhost-user.h"
> > +#include "net/vhost-vdpa.h"
> >
> >  #include "standard-headers/linux/vhost_types.h"
> > +#include "linux-headers/linux/vhost.h"
> >  #include "hw/virtio/virtio-net.h"
> >  #include "net/vhost_net.h"
> >  #include "qemu/error-report.h"
> > @@ -33,12 +35,6 @@
> >  #include "hw/virtio/vhost.h"
> >  #include "hw/virtio/virtio-bus.h"
> >
> > -struct vhost_net {
> > -    struct vhost_dev dev;
> > -    struct vhost_virtqueue vqs[2];
> > -    int backend;
> > -    NetClientState *nc;
> > -};
> >
> >  /* Features supported by host kernel. */
> >  static const int kernel_feature_bits[] = {
> > @@ -96,6 +92,11 @@ static const int
> > *vhost_net_get_feature_bits(struct vhost_net *net)
> >      case NET_CLIENT_DRIVER_VHOST_USER:
> >          feature_bits = user_feature_bits;
> >          break;
> > +#ifdef CONFIG_VHOST_NET_VDPA
> > +    case NET_CLIENT_DRIVER_VHOST_VDPA:
> > +        feature_bits = vdpa_feature_bits;
> > +        break;
> > +#endif
> >      default:
> >          error_report("Feature bits not defined for this type: %d",
> >                  net->nc->info->type);
> > @@ -443,6 +444,12 @@ VHostNetState *get_vhost_net(NetClientState *nc)
> >          vhost_net = vhost_user_get_vhost_net(nc);
> >          assert(vhost_net);
> >          break;
> > +#endif
> > +#ifdef CONFIG_VHOST_NET_VDPA
> > +    case NET_CLIENT_DRIVER_VHOST_VDPA:
> > +        vhost_net = vhost_vdpa_get_vhost_net(nc);
> > +        assert(vhost_net);
> > +        break;
> >  #endif
> >      default:
> >          break;
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index aff67a92df..0a54b1910a 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -43,6 +43,7 @@
> >  #include "monitor/qdev.h"
> >  #include "hw/pci/pci.h"
> >  #include "net_rx_pkt.h"
> > +#include "hw/virtio/vhost.h"
> >
> >  #define VIRTIO_NET_VM_VERSION    11
> >
> > @@ -125,6 +126,8 @@ static void virtio_net_get_config(VirtIODevice
> > *vdev, uint8_t *config)
> >      VirtIONet *n = VIRTIO_NET(vdev);
> >      struct virtio_net_config netcfg;
> >
> > +    int ret = 0;
> > +    memset(&netcfg, 0 , sizeof(struct virtio_net_config));
> >      virtio_stw_p(vdev, &netcfg.status, n->status);
> >      virtio_stw_p(vdev, &netcfg.max_virtqueue_pairs, n->max_queues);
> >      virtio_stw_p(vdev, &netcfg.mtu, n->net_conf.mtu);
> > @@ -138,6 +141,15 @@ static void virtio_net_get_config(VirtIODevice
> > *vdev, uint8_t *config)
> >      virtio_stl_p(vdev, &netcfg.supported_hash_types,
> >                   VIRTIO_NET_RSS_SUPPORTED_HASHES);
> >      memcpy(config, &netcfg, n->config_size);
> > +
> > +    NetClientState *nc = qemu_get_queue(n->nic);
> > +    if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> > +        ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t
> > *)&netcfg,
> > +                             n->config_size);
> > +    if (ret != -1) {
> > +        memcpy(config, &netcfg, n->config_size);
> > +    }
> > +    }
> >  }
> >
> >  static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t
> > *config)
> > @@ -153,6 +165,13 @@ static void virtio_net_set_config(VirtIODevice
> > *vdev, const uint8_t *config)
> >          memcpy(n->mac, netcfg.mac, ETH_ALEN);
> >          qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
> >      }
> > +
> > +    NetClientState *nc = qemu_get_queue(n->nic);
> > +    if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> > +        vhost_net_set_config(get_vhost_net(nc->peer), (uint8_t
> > *)&netcfg,
> > +                               0, n->config_size,
> > +                        VHOST_SET_CONFIG_TYPE_MASTER);
> > +      }
> >  }
> >
> >  static bool virtio_net_started(VirtIONet *n, uint8_t status)
> > diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
> > index 48905383f8..782b1d67d9 100644
> > --- a/hw/virtio/vhost-backend.c
> > +++ b/hw/virtio/vhost-backend.c
> > @@ -15,6 +15,7 @@
> >  #include "qemu/main-loop.h"
> >  #include "standard-headers/linux/vhost_types.h"
> >
> > +#include "hw/virtio/vhost-vdpa.h"
> >  #ifdef CONFIG_VHOST_KERNEL
> >  #include <linux/vhost.h>
> >  #include <sys/ioctl.h>
> > @@ -285,6 +286,11 @@ int vhost_set_backend_type(struct vhost_dev
> > *dev, VhostBackendType backend_type)
> >      case VHOST_BACKEND_TYPE_USER:
> >          dev->vhost_ops = &user_ops;
> >          break;
> > +#endif
> > +#ifdef CONFIG_VHOST_VDPA
> > +    case VHOST_BACKEND_TYPE_VDPA:
> > +        dev->vhost_ops = &vdpa_ops;
> > +        break;
> >  #endif
> >      default:
> >          error_report("Unknown vhost backend type");
> > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > new file mode 100644
> > index 0000000000..6f2b01c25e
> > --- /dev/null
> > +++ b/hw/virtio/vhost-vdpa.c
> > @@ -0,0 +1,475 @@
> > +/*
> > + * vhost-vdpa
> > + *
> > + *  Copyright(c) 2017-2018 Intel Corporation.
> > + *  Copyright(c) 2020 Red Hat, Inc.
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2
> > or later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include <linux/vhost.h>
> > +#include <linux/vfio.h>
> > +#include <sys/eventfd.h>
> > +#include <sys/ioctl.h>
> > +#include "hw/virtio/vhost.h"
> > +#include "hw/virtio/vhost-backend.h"
> > +#include "hw/virtio/virtio-net.h"
> > +#include "hw/virtio/vhost-vdpa.h"
> > +#include "qemu/main-loop.h"
> > +#include <linux/kvm.h>
> > +#include "sysemu/kvm.h"
> There is an issue here ^^^
> Now that this patch is in mainline, I find that hw/virtio/vhost-vdpa.c
> does not compile on a RISCV host (I'm using the openSUSE Build Service,
> which provides a number of architectures, including RISCV64). This host
> doesn't have KVM support.
>
> The failure is as follows:
> In file included from /home/abuild/rpmbuild/BUILD/qemu-
> 5.0.50/hw/virtio/vhost-vdpa.c:22:
> /home/abuild/rpmbuild/BUILD/qemu-5.0.50/linux-
> headers/linux/kvm.h:14:10: fatal error: asm/kvm.h: No such file or
> directory
>    14 | #include <asm/kvm.h>
>
> As I wondered why we have a kvm reference in this file at all, I
> removed the last two #include lines above and recompiled, I find that
> TARGET_PAGE_MASK and TARGET_PAGE_ALIGN are not defined, so poking
> around a bit it seemed that including "cpu.h" might be the solution
> there, and it indeed compiles ok that way.
>
> I'll leave it to the patch author or sub-maintainer to identify the
> best solution.
>
> Thanks,
>
> Bruce
>
Thanks a lot Bruce, I will send out a patch soon