| Message ID | 002301d11df5$9d440a10$d7cc1e30$@samsung.com |
|---|---|
| State | New |
| Headers | show |
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 >
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
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 ?
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
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");
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);
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(+)