qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH for 8.0] backends/vhost-user: relax the ioeventfd check
@ 2022-12-02 13:22 Alex Bennée
  2022-12-19 17:48 ` Alex Bennée
  2022-12-19 18:49 ` Stefan Hajnoczi
  0 siblings, 2 replies; 4+ messages in thread
From: Alex Bennée @ 2022-12-02 13:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: slp, mst, marcandre.lureau, stefanha, mathieu.poirier,
	viresh.kumar, sgarzare, Alex Bennée

While you certainly need ioeventfds to work for KVM guests it
shouldn't be limited to that. We can run vhost-user backends for TCG
guests and either use ioeventfds or in band signalling.

Maybe we should apply the same fix as b0aa77d36d (vhost-user: fix
ioeventfd_enabled)?

With this change I can run:

  $QEMU $OPTS \
    -display gtk,gl=on \
    -device vhost-user-gpu-pci,chardev=vhgpu \
    -chardev socket,id=vhgpu,path=vhgpu.sock

with:

  ./contrib/vhost-user-gpu/vhost-user-gpu \
    -s vhgpu.sock \
    -v

and at least see things start-up (although the display gets rotated by
180 degrees).

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 backends/vhost-user.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/backends/vhost-user.c b/backends/vhost-user.c
index 5dedb2d987..87d43fb03a 100644
--- a/backends/vhost-user.c
+++ b/backends/vhost-user.c
@@ -21,12 +21,6 @@
 #include "io/channel-command.h"
 #include "hw/virtio/virtio-bus.h"
 
-static bool
-ioeventfd_enabled(void)
-{
-    return kvm_enabled() && kvm_eventfds_enabled();
-}
-
 int
 vhost_user_backend_dev_init(VhostUserBackend *b, VirtIODevice *vdev,
                             unsigned nvqs, Error **errp)
@@ -35,8 +29,8 @@ vhost_user_backend_dev_init(VhostUserBackend *b, VirtIODevice *vdev,
 
     assert(!b->vdev && vdev);
 
-    if (!ioeventfd_enabled()) {
-        error_setg(errp, "vhost initialization failed: requires kvm");
+    if (kvm_enabled() && !kvm_eventfds_enabled()) {
+        error_setg(errp, "vhost initialization failed: kvm required ioeventfds");
         return -1;
     }
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH for 8.0] backends/vhost-user: relax the ioeventfd check
  2022-12-02 13:22 [RFC PATCH for 8.0] backends/vhost-user: relax the ioeventfd check Alex Bennée
@ 2022-12-19 17:48 ` Alex Bennée
  2022-12-19 17:53   ` Michael S. Tsirkin
  2022-12-19 18:49 ` Stefan Hajnoczi
  1 sibling, 1 reply; 4+ messages in thread
From: Alex Bennée @ 2022-12-19 17:48 UTC (permalink / raw)
  To: qemu-devel
  Cc: slp, mst, marcandre.lureau, stefanha, mathieu.poirier,
	viresh.kumar, sgarzare, Alex Bennée


Alex Bennée <alex.bennee@linaro.org> writes:

> While you certainly need ioeventfds to work for KVM guests it
> shouldn't be limited to that. We can run vhost-user backends for TCG
> guests and either use ioeventfds or in band signalling.
>
> Maybe we should apply the same fix as b0aa77d36d (vhost-user: fix
> ioeventfd_enabled)?
>
> With this change I can run:
>
>   $QEMU $OPTS \
>     -display gtk,gl=on \
>     -device vhost-user-gpu-pci,chardev=vhgpu \
>     -chardev socket,id=vhgpu,path=vhgpu.sock
>
> with:
>
>   ./contrib/vhost-user-gpu/vhost-user-gpu \
>     -s vhgpu.sock \
>     -v
>
> and at least see things start-up (although the display gets rotated by
> 180 degrees).
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  backends/vhost-user.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/backends/vhost-user.c b/backends/vhost-user.c
> index 5dedb2d987..87d43fb03a 100644
> --- a/backends/vhost-user.c
> +++ b/backends/vhost-user.c
> @@ -21,12 +21,6 @@
>  #include "io/channel-command.h"
>  #include "hw/virtio/virtio-bus.h"
>  
> -static bool
> -ioeventfd_enabled(void)
> -{
> -    return kvm_enabled() && kvm_eventfds_enabled();
> -}
> -
>  int
>  vhost_user_backend_dev_init(VhostUserBackend *b, VirtIODevice *vdev,
>                              unsigned nvqs, Error **errp)
> @@ -35,8 +29,8 @@ vhost_user_backend_dev_init(VhostUserBackend *b, VirtIODevice *vdev,
>  
>      assert(!b->vdev && vdev);
>  
> -    if (!ioeventfd_enabled()) {
> -        error_setg(errp, "vhost initialization failed: requires kvm");
> +    if (kvm_enabled() && !kvm_eventfds_enabled()) {
> +        error_setg(errp, "vhost initialization failed: kvm required ioeventfds");
>          return -1;
>      }

Gentle ping?

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH for 8.0] backends/vhost-user: relax the ioeventfd check
  2022-12-19 17:48 ` Alex Bennée
@ 2022-12-19 17:53   ` Michael S. Tsirkin
  0 siblings, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2022-12-19 17:53 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, slp, marcandre.lureau, stefanha, mathieu.poirier,
	viresh.kumar, sgarzare

On Mon, Dec 19, 2022 at 05:48:50PM +0000, Alex Bennée wrote:
> 
> Alex Bennée <alex.bennee@linaro.org> writes:
> 
> > While you certainly need ioeventfds to work for KVM guests it
> > shouldn't be limited to that. We can run vhost-user backends for TCG
> > guests and either use ioeventfds or in band signalling.
> >
> > Maybe we should apply the same fix as b0aa77d36d (vhost-user: fix
> > ioeventfd_enabled)?
> >
> > With this change I can run:
> >
> >   $QEMU $OPTS \
> >     -display gtk,gl=on \
> >     -device vhost-user-gpu-pci,chardev=vhgpu \
> >     -chardev socket,id=vhgpu,path=vhgpu.sock
> >
> > with:
> >
> >   ./contrib/vhost-user-gpu/vhost-user-gpu \
> >     -s vhgpu.sock \
> >     -v
> >
> > and at least see things start-up (although the display gets rotated by
> > 180 degrees).
> >
> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> > ---
> >  backends/vhost-user.c | 10 ++--------
> >  1 file changed, 2 insertions(+), 8 deletions(-)
> >
> > diff --git a/backends/vhost-user.c b/backends/vhost-user.c
> > index 5dedb2d987..87d43fb03a 100644
> > --- a/backends/vhost-user.c
> > +++ b/backends/vhost-user.c
> > @@ -21,12 +21,6 @@
> >  #include "io/channel-command.h"
> >  #include "hw/virtio/virtio-bus.h"
> >  
> > -static bool
> > -ioeventfd_enabled(void)
> > -{
> > -    return kvm_enabled() && kvm_eventfds_enabled();
> > -}
> > -
> >  int
> >  vhost_user_backend_dev_init(VhostUserBackend *b, VirtIODevice *vdev,
> >                              unsigned nvqs, Error **errp)
> > @@ -35,8 +29,8 @@ vhost_user_backend_dev_init(VhostUserBackend *b, VirtIODevice *vdev,
> >  
> >      assert(!b->vdev && vdev);
> >  
> > -    if (!ioeventfd_enabled()) {
> > -        error_setg(errp, "vhost initialization failed: requires kvm");
> > +    if (kvm_enabled() && !kvm_eventfds_enabled()) {
> > +        error_setg(errp, "vhost initialization failed: kvm required ioeventfds");
> >          return -1;
> >      }
> 
> Gentle ping?
> 

Yea why not. Pls post a non RFC patch.

-- 
MST



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH for 8.0] backends/vhost-user: relax the ioeventfd check
  2022-12-02 13:22 [RFC PATCH for 8.0] backends/vhost-user: relax the ioeventfd check Alex Bennée
  2022-12-19 17:48 ` Alex Bennée
@ 2022-12-19 18:49 ` Stefan Hajnoczi
  1 sibling, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2022-12-19 18:49 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, slp, mst, marcandre.lureau, stefanha, mathieu.poirier,
	viresh.kumar, sgarzare

On Fri, 2 Dec 2022 at 08:23, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> While you certainly need ioeventfds to work for KVM guests it
> shouldn't be limited to that. We can run vhost-user backends for TCG
> guests and either use ioeventfds or in band signalling.

I suggest tweaking this sentence so it's clear that you mean QEMU's
simulated eventfds rather than kvm.ko's real ioeventfds:
s/use eventfds or in band signalling/simulate ioeventfds or use in
band signalling/

>
> Maybe we should apply the same fix as b0aa77d36d (vhost-user: fix
> ioeventfd_enabled)?
>
> With this change I can run:
>
>   $QEMU $OPTS \
>     -display gtk,gl=on \
>     -device vhost-user-gpu-pci,chardev=vhgpu \
>     -chardev socket,id=vhgpu,path=vhgpu.sock
>
> with:
>
>   ./contrib/vhost-user-gpu/vhost-user-gpu \
>     -s vhgpu.sock \
>     -v
>
> and at least see things start-up (although the display gets rotated by
> 180 degrees).
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  backends/vhost-user.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/backends/vhost-user.c b/backends/vhost-user.c
> index 5dedb2d987..87d43fb03a 100644
> --- a/backends/vhost-user.c
> +++ b/backends/vhost-user.c
> @@ -21,12 +21,6 @@
>  #include "io/channel-command.h"
>  #include "hw/virtio/virtio-bus.h"
>
> -static bool
> -ioeventfd_enabled(void)
> -{
> -    return kvm_enabled() && kvm_eventfds_enabled();
> -}
> -
>  int
>  vhost_user_backend_dev_init(VhostUserBackend *b, VirtIODevice *vdev,
>                              unsigned nvqs, Error **errp)
> @@ -35,8 +29,8 @@ vhost_user_backend_dev_init(VhostUserBackend *b, VirtIODevice *vdev,
>
>      assert(!b->vdev && vdev);
>
> -    if (!ioeventfd_enabled()) {
> -        error_setg(errp, "vhost initialization failed: requires kvm");
> +    if (kvm_enabled() && !kvm_eventfds_enabled()) {
> +        error_setg(errp, "vhost initialization failed: kvm required ioeventfds");
>          return -1;
>      }

The check can be dropped completely. If kvm.ko doesn't support
ioeventfds QEMU can simulate them just like it does with TCG. In
practice I don't think this happens with a reasonably modern kernel
version.

Stefan


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-12-19 18:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-02 13:22 [RFC PATCH for 8.0] backends/vhost-user: relax the ioeventfd check Alex Bennée
2022-12-19 17:48 ` Alex Bennée
2022-12-19 17:53   ` Michael S. Tsirkin
2022-12-19 18:49 ` Stefan Hajnoczi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).