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

[Qemu-devel] [PATCH] 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.

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>
---
 hw/virtio/vhost.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 1794f0d..3121e19 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 MMIO eventfds");
+        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] vhost: Fix aborting if KVM does not support eventfds

[Michael S. Tsirkin]

On Fri, Nov 13, 2015 at 12:28:10PM +0300, Pavel Fedin wrote:
> 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.
> 
> 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>

How come you have /dev/vhost-net though?
That was supposed to only be there if you have vhost-net,
and that never existed on kernels without eventfd.

> ---
>  hw/virtio/vhost.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 1794f0d..3121e19 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 MMIO eventfds");
> +        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] vhost: Fix aborting if KVM does not support eventfds

[Pavel Fedin]

 Hello!

> > 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.
> >
> > 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>
> 
> How come you have /dev/vhost-net though?

 Easily. Just enable CONFIG_VHOST_NET for the kernel. I happened to have it because i just copied over .config from another kernel
version, which had ioeventfds working.
 CONFIG_VHOST_NET in the kernel depends on CONFIG_VHOST, which in turn depends on CONFIG_EVENTFD. But there's no direct dependency
between it and CONFIG_HAVE_KVM_EVENTFD. So, you can just enable CONFIG_IOEVENTFD in "Configure standard kernel features", and get
ioeventfds by themselves with vhost-net. Which, technically speaking, has no direct dependency on KVM's ability to bind ioeventfd to
memory accesses. Because, theoretically, you could have some other use for it.

> That was supposed to only be there if you have vhost-net,
> and that never existed on kernels without eventfd.

 I believe distro maintainers (should) have taken care of it and disable it. But, as you can see, in some circumstances you could
have it enabled. IMHO it's better to be more flexible and process this situation correctly. The fix is trivial.

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

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

[Michael S. Tsirkin]

On Mon, Nov 16, 2015 at 10:02:07AM +0300, Pavel Fedin wrote:
>  Hello!
> 
> > > 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.
> > >
> > > 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>
> > 
> > How come you have /dev/vhost-net though?
> 
>  Easily. Just enable CONFIG_VHOST_NET for the kernel. I happened to have it because i just copied over .config from another kernel
> version, which had ioeventfds working.
>  CONFIG_VHOST_NET in the kernel depends on CONFIG_VHOST, which in turn depends on CONFIG_EVENTFD. But there's no direct dependency
> between it and CONFIG_HAVE_KVM_EVENTFD. So, you can just enable CONFIG_IOEVENTFD in "Configure standard kernel features", and get
> ioeventfds by themselves with vhost-net. Which, technically speaking, has no direct dependency on KVM's ability to bind ioeventfd to
> memory accesses. Because, theoretically, you could have some other use for it.
> 
> > That was supposed to only be there if you have vhost-net,
> > and that never existed on kernels without eventfd.
> 
>  I believe distro maintainers (should) have taken care of it and disable it. But, as you can see, in some circumstances you could
> have it enabled. IMHO it's better to be more flexible and process this situation correctly. The fix is trivial.
> 
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
> 

OK so it's a misconfigured kernel.
Fine but I'm not happy with silently using userspace instead.
If people ask for vhost they should get it.
How about
- kicking vhost from userspace
or
- failing vhost init
?

-- 
MST

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

[Pavel Fedin]

 Hello!

> OK so it's a misconfigured kernel.
> Fine but I'm not happy with silently using userspace instead.

 It's not silent. You get two warnings in the log:
--- cut ---
2015-11-13T08:43:51.146802Z qemu-system-aarch64: KVM does not support MMIO eventfds
2015-11-13T08:43:51.146915Z qemu-system-aarch64: unable to start vhost net: 38: falling back on userspace virtio
--- cut ---

> If people ask for vhost they should get it.
> How about
> - kicking vhost from userspace

 This can be interesting, this would be similar to forwarding IRQ through the userspace if we don't have irqfds. But i believe this
would require a lot to implement. To tell the truth, i'm not ready for this.
 Actually, for now i just want to provide some way to switch kernels and use the same userspace configuration, and make all possible
conbinations working somehow, without critical faults. Anyway, if we want to use vhost, i suppose we want to get maximum
performance, which can not be achieved with userspace event forwarding, so it's IMHO okay just to fallback to userspace (which we
already do, just we miss to catch some situations), just explain why.
 I even plan to add another warning, when eventfds work and irqfds don't. Currently we just silently fall back to userspace IRQ
forwarding.

> or
> - failing vhost init

 That's what my patch does. vhost_dev_enable_notifiers () is called from within vhost-net init routine. I intentionally patch
vhost.c so that it should also have the same effect on vhost-scsi.

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

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

[Christian Borntraeger]

On 11/13/2015 10:28 AM, Pavel Fedin wrote:
> 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.
> 
> 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>
> ---
>  hw/virtio/vhost.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 1794f0d..3121e19 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 MMIO eventfds");

We also have virtio-ccw. what about something without MMIO like

           error_report("KVM does not support eventfds");