From: Eugenio Pérez <eperezma@redhat.com>

Qemu falls back on userland handlers even if vhost-user and vhost-vdpa
cases. These assumes a tap device can handle the packets.

If a vdpa device fail to start, it can trigger a sigsegv because of
that. Do not resort on them unless actually possible.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 hw/net/virtio-net.c        |  4 ++++
 hw/virtio/virtio.c         | 21 +++++++++++++--------
 include/hw/virtio/virtio.h |  2 ++
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index ffaf481..9cdf777 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3523,6 +3523,10 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
     nc = qemu_get_queue(n->nic);
     nc->rxfilter_notify_enabled = 1;
 
+    if (!nc->peer || nc->peer->info->type != NET_CLIENT_DRIVER_TAP) {
+        /* Only tap can use userspace networking */
+        vdev->disable_ioeventfd_handler = true;
+    }
     if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
         struct virtio_net_config netcfg = {};
         memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 9d637e0..806603b 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3708,17 +3708,22 @@ static int virtio_device_start_ioeventfd_impl(VirtIODevice *vdev)
             err = r;
             goto assign_error;
         }
-        event_notifier_set_handler(&vq->host_notifier,
-                                   virtio_queue_host_notifier_read);
+
+        if (!vdev->disable_ioeventfd_handler) {
+            event_notifier_set_handler(&vq->host_notifier,
+                                       virtio_queue_host_notifier_read);
+        }
     }
 
-    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
-        /* Kick right away to begin processing requests already in vring */
-        VirtQueue *vq = &vdev->vq[n];
-        if (!vq->vring.num) {
-            continue;
+    if (!vdev->disable_ioeventfd_handler) {
+        for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
+            /* Kick right away to begin processing requests already in vring */
+            VirtQueue *vq = &vdev->vq[n];
+            if (!vq->vring.num) {
+                continue;
+            }
+            event_notifier_set(&vq->host_notifier);
         }
-        event_notifier_set(&vq->host_notifier);
     }
     memory_region_transaction_commit();
     return 0;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index b31c450..b6ce5f0 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -105,6 +105,8 @@ struct VirtIODevice
     VMChangeStateEntry *vmstate;
     char *bus_name;
     uint8_t device_endian;
+    /* backend does not support userspace handler */
+    bool disable_ioeventfd_handler;
     bool use_guest_notifier_mask;
     AddressSpace *dma_as;
     QLIST_HEAD(, VirtQueue) *vector_queues;
-- 
1.8.3.1

On Wed, Mar 30, 2022 at 2:33 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>
> From: Eugenio Pérez <eperezma@redhat.com>
>
> Qemu falls back on userland handlers even if vhost-user and vhost-vdpa
> cases. These assumes a tap device can handle the packets.
>
> If a vdpa device fail to start, it can trigger a sigsegv because of
> that. Do not resort on them unless actually possible.

What kind of segsev you've seen. If my memory is correct we finally
choose to have a dummy receive() for vhost-vDPA in

commit 846a1e85da646c6006db429648389fc110f92d75
Author: Eugenio Pérez <eperezma@redhat.com>
Date:   Thu Nov 25 11:16:13 2021 +0100

    vdpa: Add dummy receive callback

    Qemu falls back on userland handlers even if vhost-user and vhost-vdpa
    cases. These assumes a tap device can handle the packets.

    If a vdpa device fail to start, it can trigger a sigsegv because of
    that. Add dummy receiver that returns no progress so it can keep
    running.

    Fixes: 1e0a84ea49 ("vhost-vdpa: introduce vhost-vdpa net client")
    Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
    Message-Id: <20211125101614.76927-2-eperezma@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>

(Technically, we can have a vhost-vDPA networking backend (not vhost backend))

>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>  hw/net/virtio-net.c        |  4 ++++
>  hw/virtio/virtio.c         | 21 +++++++++++++--------
>  include/hw/virtio/virtio.h |  2 ++
>  3 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index ffaf481..9cdf777 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -3523,6 +3523,10 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>      nc = qemu_get_queue(n->nic);
>      nc->rxfilter_notify_enabled = 1;
>
> +    if (!nc->peer || nc->peer->info->type != NET_CLIENT_DRIVER_TAP) {
> +        /* Only tap can use userspace networking */
> +        vdev->disable_ioeventfd_handler = true;
> +    }
>      if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
>          struct virtio_net_config netcfg = {};
>          memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 9d637e0..806603b 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3708,17 +3708,22 @@ static int virtio_device_start_ioeventfd_impl(VirtIODevice *vdev)
>              err = r;
>              goto assign_error;
>          }
> -        event_notifier_set_handler(&vq->host_notifier,
> -                                   virtio_queue_host_notifier_read);
> +
> +        if (!vdev->disable_ioeventfd_handler) {
> +            event_notifier_set_handler(&vq->host_notifier,
> +                                       virtio_queue_host_notifier_read);
> +        }
>      }
>
> -    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
> -        /* Kick right away to begin processing requests already in vring */
> -        VirtQueue *vq = &vdev->vq[n];
> -        if (!vq->vring.num) {
> -            continue;
> +    if (!vdev->disable_ioeventfd_handler) {
> +        for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
> +            /* Kick right away to begin processing requests already in vring */
> +            VirtQueue *vq = &vdev->vq[n];
> +            if (!vq->vring.num) {
> +                continue;
> +            }
> +            event_notifier_set(&vq->host_notifier);
>          }
> -        event_notifier_set(&vq->host_notifier);
>      }
>      memory_region_transaction_commit();
>      return 0;
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index b31c450..b6ce5f0 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -105,6 +105,8 @@ struct VirtIODevice
>      VMChangeStateEntry *vmstate;
>      char *bus_name;
>      uint8_t device_endian;
> +    /* backend does not support userspace handler */
> +    bool disable_ioeventfd_handler;
>      bool use_guest_notifier_mask;
>      AddressSpace *dma_as;
>      QLIST_HEAD(, VirtQueue) *vector_queues;
> --
> 1.8.3.1
>