[Qemu-devel] [PATCH v2] vhost: Fix aborting if KVM does not support eventfds

[Qemu-devel] [PATCH v2] vhost: Fix aborting if KVM does not support eventfds

[Pavel Fedin]

If you happen to have a stock kernel of old version, like 3.x, and you
attempt to enable vhost by setting vhost=on, qemu aborts with error:

kvm_mem_ioeventfd_add: error adding ioeventfd: Function not implemented

This patch adds capability check, so that vhost gets disabled instead. A
warning is displayed, explaining the reason:

2015-11-13T08:43:51.146802Z qemu-system-aarch64: KVM does not support eventfd binding
2015-11-13T08:43:51.146915Z qemu-system-aarch64: unable to start vhost net: 38: falling back on userspace virtio

This problem can be observed with libvirt, which checks for /dev/vhost-net
availability and just inserts "vhost=on" automatically in this case.

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
v1 => v2:
- Removed "MMIO" from warning message, because it applies not only to MMIO
- Add note about warning to the commit message
---
 hw/virtio/vhost.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 1794f0d..50b8171 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -24,6 +24,7 @@
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/virtio-access.h"
 #include "migration/migration.h"
+#include "sysemu/kvm.h"
 
 static struct vhost_log *vhost_log;
 static struct vhost_log *vhost_log_shm;
@@ -1083,6 +1084,11 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
         r = -ENOSYS;
         goto fail;
     }
+    if (!kvm_eventfds_enabled()) {
+        error_report("KVM does not support eventfd binding");
+        r = -ENOSYS;
+        goto fail;
+    }
 
     for (i = 0; i < hdev->nvqs; ++i) {
         r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, true);
-- 
1.9.5.msysgit.0

Re: [Qemu-devel] [PATCH v2] vhost: Fix aborting if KVM does not support eventfds

[Michael S. Tsirkin]

On Tue, Nov 17, 2015 at 10:17:37AM +0300, Pavel Fedin wrote:
> If you happen to have a stock kernel of old version, like 3.x,

On ARM

> and you
> attempt to enable vhost by setting vhost=on, qemu aborts with error:
> kvm_mem_ioeventfd_add: error adding ioeventfd: Function not implemented
> 
> This patch adds capability check, so that vhost gets disabled instead. A
> warning is displayed, explaining the reason:
> 
> 2015-11-13T08:43:51.146802Z qemu-system-aarch64: KVM does not support eventfd binding
> 2015-11-13T08:43:51.146915Z qemu-system-aarch64: unable to start vhost net: 38: falling back on userspace virtio
> 
> This problem can be observed with libvirt, which checks for /dev/vhost-net
> availability and just inserts "vhost=on" automatically in this case.
> 
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> ---
> v1 => v2:
> - Removed "MMIO" from warning message, because it applies not only to MMIO
> - Add note about warning to the commit message
> ---
>  hw/virtio/vhost.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 1794f0d..50b8171 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -24,6 +24,7 @@
>  #include "hw/virtio/virtio-bus.h"
>  #include "hw/virtio/virtio-access.h"
>  #include "migration/migration.h"
> +#include "sysemu/kvm.h"
>  
>  static struct vhost_log *vhost_log;
>  static struct vhost_log *vhost_log_shm;
> @@ -1083,6 +1084,11 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
>          r = -ENOSYS;
>          goto fail;
>      }
> +    if (!kvm_eventfds_enabled()) {
> +        error_report("KVM does not support eventfd binding");
> +        r = -ENOSYS;
> +        goto fail;
> +    }
>  
>      for (i = 0; i < hdev->nvqs; ++i) {
>          r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, true);

That's a wrong thing for vhost to do - we don't want it
poking at kvm because it does not work with either kvm or
eventfd.  It uses host notifiers, there should be an
API to check for that.

Or better yet, actually implement host notifiers
in userspace - should not be that hard to do, just
do write into eventfd.

> -- 
> 1.9.5.msysgit.0
> 

Re: [Qemu-devel] [PATCH v2] vhost: Fix aborting if KVM does not support eventfds

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2673 bytes --]

On Wed, Nov 18, 2015 at 11:25:32PM +0200, Michael S. Tsirkin wrote:
> On Tue, Nov 17, 2015 at 10:17:37AM +0300, Pavel Fedin wrote:
> > If you happen to have a stock kernel of old version, like 3.x,
> 
> On ARM
> 
> > and you
> > attempt to enable vhost by setting vhost=on, qemu aborts with error:
> > kvm_mem_ioeventfd_add: error adding ioeventfd: Function not implemented
> > 
> > This patch adds capability check, so that vhost gets disabled instead. A
> > warning is displayed, explaining the reason:
> > 
> > 2015-11-13T08:43:51.146802Z qemu-system-aarch64: KVM does not support eventfd binding
> > 2015-11-13T08:43:51.146915Z qemu-system-aarch64: unable to start vhost net: 38: falling back on userspace virtio
> > 
> > This problem can be observed with libvirt, which checks for /dev/vhost-net
> > availability and just inserts "vhost=on" automatically in this case.
> > 
> > Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> > ---
> > v1 => v2:
> > - Removed "MMIO" from warning message, because it applies not only to MMIO
> > - Add note about warning to the commit message
> > ---
> >  hw/virtio/vhost.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > index 1794f0d..50b8171 100644
> > --- a/hw/virtio/vhost.c
> > +++ b/hw/virtio/vhost.c
> > @@ -24,6 +24,7 @@
> >  #include "hw/virtio/virtio-bus.h"
> >  #include "hw/virtio/virtio-access.h"
> >  #include "migration/migration.h"
> > +#include "sysemu/kvm.h"
> >  
> >  static struct vhost_log *vhost_log;
> >  static struct vhost_log *vhost_log_shm;
> > @@ -1083,6 +1084,11 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
> >          r = -ENOSYS;
> >          goto fail;
> >      }
> > +    if (!kvm_eventfds_enabled()) {
> > +        error_report("KVM does not support eventfd binding");
> > +        r = -ENOSYS;
> > +        goto fail;
> > +    }
> >  
> >      for (i = 0; i < hdev->nvqs; ++i) {
> >          r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, true);
> 
> That's a wrong thing for vhost to do - we don't want it
> poking at kvm because it does not work with either kvm or
> eventfd.  It uses host notifiers, there should be an
> API to check for that.
> 
> Or better yet, actually implement host notifiers
> in userspace - should not be that hard to do, just
> do write into eventfd.

I sent this RFC patch a while back:
https://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg04710.html

My intent was to write a qtest for a vhost device but I didn't complete
the work.  Anyway, this patch might be useful.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v2] vhost: Fix aborting if KVM does not support eventfds

[Pavel Fedin]

 Hello!

> > Or better yet, actually implement host notifiers
> > in userspace - should not be that hard to do, just
> > do write into eventfd.
> 
> I sent this RFC patch a while back:

 Thank you very much, but Paolo already pointed it out for me, and i have picked it up:
https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg04626.html

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia