Hi,

vhost-user allows to drive a virtio device in a seperate
process. After vhost-user-net, we have seen
vhost-user-{scsi,blk,crypto} added more recently.

This series, initially proposed 2 years ago
(https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg01905.html)
contributes with vhost-user-input and vhost-user-gpu.

Additionally, to factor out common code and ease the usage, a
vhost-user-backend is introduced as an intermediary object between the
backend and the qemu device.

You may start a vhost-user-gpu with virgl rendering in a separate
process like this:

$ ./vhost-user-gpu --virgl -s vgpu.sock &
$ qemu...
  -chardev socket,id=chr,path=vgpu.sock
  -object vhost-user-backend,id=vug,chardev=chr
  -device vhost-user-vga,vhost-user=vug

You may also specify the backend command and the arguments as part of
vhost-user-backend qemu arguments. For example, to start a
vhost-user-input backend on input device /dev/input/event19:

-object vhost-user-backend,id=vuid,cmd="vhost-user-input /dev/input/event19"
-device virtio-input-host-pci,vhost-user=vuid

The vhost-user-gpu backend requires virgl from git.

The libvirt support is on-going work:
https://github.com/elmarco/libvirt/commits/vhost-user-gpu

The GPU benchmarks are encouraging, giving up to x5 performance on
Unigine Heaven 4.0.

Feedback welcome,

v4:
 - move qemu_write_pidfile() in util, improve it a bit
 - add --pid and --fd arguments to vhost-user to help with libvirt support
 - various bug fixes for synchronization, and tearing down

v3: deal with most comments from rfcv2 and various improvements
 - "vhost-user-backend: allow to specify binary to execute" as seperate
  patch, not for inclusion since Daniel as concerned about parsing
  shell strings with glib
 - use dmabuf to share 2d rendering result (with intel gem only atm)
 - document the vhost-user-gpu protocol
 - make vhost-user-gpu-pci and vhost-user-vga seperate devices (instead
   of adding vhost-user support to existing devices)
 - allow to specify virgl rendering, and rendernode
 - let's promote out of RFC status :)

RFCv2: (addressing some of Gerd comments digged in the archives)
 - rebased, clean ups, various small fixes, update commit messages
 - teach the vhost-user-backend to take a chardev
 - add vhost-user-input-pci, instead of adding vhost code to virtio-input-host-pci

Marc-André Lureau (29):
  chardev: avoid crash if no associated address
  chardev: remove qemu_chr_fe_read_all() counter
  chardev: unref if underlying chardev has no parent
  dmabuf: add y0_top, pass it to spice
  vhost-user: simplify vhost_user_init/vhost_user_cleanup
  libvhost-user: exit by default on VHOST_USER_NONE
  vhost-user: wrap some read/write with retry handling
  Add vhost-user-backend
  qio: add qio_channel_command_new_spawn_with_pre_exec()
  HACK: vhost-user-backend: allow to specify binary to execute
  vhost-user: split vhost_user_read()
  vhost-user: add vhost_user_input_get_config()
  libvhost-user: export vug_source_new()
  contrib: add vhost-user-input
  Add vhost-user-input-pci
  vhost-user: add vhost_user_gpu_set_socket()
  vhost-user: add vhost_user_gpu_get_num_capsets()
  virtio: add virtio-gpu bswap helpers header
  util: promote qemu_egl_rendernode_open() to libqemuutil
  util: add qemu_write_pidfile()
  util: use fcntl() for qemu_write_pidfile() locking
  contrib: add vhost-user-gpu
  virtio-gpu: remove unused qdev
  virtio-gpu: remove unused config_size
  virtio-gpu: block both 2d and 3d rendering
  virtio-gpu: remove useless 'waiting' field
  virtio-gpu: split virtio-gpu, introduce virtio-gpu-base
  virtio-gpu: split virtio-gpu-pci & virtio-vga
  hw/display: add vhost-user-vga & gpu-pci

 contrib/libvhost-user/libvhost-user-glib.h |    3 +
 contrib/libvhost-user/libvhost-user.h      |    3 +
 contrib/vhost-user-gpu/drm.h               |   63 ++
 contrib/vhost-user-gpu/virgl.h             |   25 +
 contrib/vhost-user-gpu/vugpu.h             |  168 +++
 hw/display/virtio-vga.h                    |   22 +
 hw/virtio/virtio-pci.h                     |   27 +-
 include/hw/virtio/vhost-backend.h          |    6 +
 include/hw/virtio/vhost-user-blk.h         |    2 +-
 include/hw/virtio/vhost-user-scsi.h        |    2 +-
 include/hw/virtio/vhost-user.h             |    2 +-
 include/hw/virtio/virtio-gpu-bswap.h       |   61 +
 include/hw/virtio/virtio-gpu.h             |   90 +-
 include/hw/virtio/virtio-input.h           |   14 +
 include/io/channel-command.h               |   18 +
 include/qemu/drm.h                         |    6 +
 include/qemu/osdep.h                       |    3 +-
 include/sysemu/vhost-user-backend.h        |   60 +
 include/ui/console.h                       |    1 +
 backends/cryptodev-vhost-user.c            |   18 +-
 backends/vhost-user.c                      |  320 ++++++
 chardev/char-fe.c                          |   13 +-
 chardev/char-socket.c                      |    8 +-
 contrib/libvhost-user/libvhost-user-glib.c |   15 +-
 contrib/libvhost-user/libvhost-user.c      |    3 +-
 contrib/vhost-user-gpu/drm.c               |  190 ++++
 contrib/vhost-user-gpu/main.c              | 1164 ++++++++++++++++++++
 contrib/vhost-user-gpu/virgl.c             |  579 ++++++++++
 contrib/vhost-user-input/main.c            |  379 +++++++
 hw/block/vhost-user-blk.c                  |   22 +-
 hw/display/vhost-user-gpu-pci.c            |   51 +
 hw/display/vhost-user-gpu.c                |  524 +++++++++
 hw/display/vhost-user-vga.c                |   52 +
 hw/display/virtio-gpu-3d.c                 |   70 +-
 hw/display/virtio-gpu-base.c               |  292 +++++
 hw/display/virtio-gpu-pci.c                |   39 +-
 hw/display/virtio-gpu.c                    |  368 ++-----
 hw/display/virtio-vga.c                    |  127 ++-
 hw/input/vhost-user-input.c                |  110 ++
 hw/scsi/vhost-user-scsi.c                  |   20 +-
 hw/virtio/vhost-stub.c                     |    4 +-
 hw/virtio/vhost-user.c                     |  143 ++-
 hw/virtio/virtio-pci.c                     |   20 +
 io/channel-command.c                       |   33 +-
 net/vhost-user.c                           |   13 +-
 os-posix.c                                 |   24 -
 os-win32.c                                 |   25 -
 qga/main.c                                 |   54 +-
 scsi/qemu-pr-helper.c                      |   40 +-
 ui/egl-helpers.c                           |   51 +-
 ui/spice-display.c                         |    3 +-
 util/drm.c                                 |   66 ++
 util/oslib-posix.c                         |   39 +
 util/oslib-win32.c                         |   27 +
 vl.c                                       |    8 +-
 MAINTAINERS                                |   15 +-
 Makefile                                   |    6 +
 Makefile.objs                              |    2 +
 backends/Makefile.objs                     |    3 +-
 configure                                  |   35 +
 contrib/vhost-user-gpu/Makefile.objs       |   10 +
 contrib/vhost-user-input/Makefile.objs     |    1 +
 docs/interop/vhost-user-gpu.rst            |  236 ++++
 docs/interop/vhost-user.txt                |   24 +
 hw/display/Makefile.objs                   |    5 +-
 hw/input/Makefile.objs                     |    1 +
 qemu-options.hx                            |   24 +
 util/Makefile.objs                         |    1 +
 68 files changed, 5130 insertions(+), 723 deletions(-)
 create mode 100644 contrib/vhost-user-gpu/drm.h
 create mode 100644 contrib/vhost-user-gpu/virgl.h
 create mode 100644 contrib/vhost-user-gpu/vugpu.h
 create mode 100644 hw/display/virtio-vga.h
 create mode 100644 include/hw/virtio/virtio-gpu-bswap.h
 create mode 100644 include/qemu/drm.h
 create mode 100644 include/sysemu/vhost-user-backend.h
 create mode 100644 backends/vhost-user.c
 create mode 100644 contrib/vhost-user-gpu/drm.c
 create mode 100644 contrib/vhost-user-gpu/main.c
 create mode 100644 contrib/vhost-user-gpu/virgl.c
 create mode 100644 contrib/vhost-user-input/main.c
 create mode 100644 hw/display/vhost-user-gpu-pci.c
 create mode 100644 hw/display/vhost-user-gpu.c
 create mode 100644 hw/display/vhost-user-vga.c
 create mode 100644 hw/display/virtio-gpu-base.c
 create mode 100644 hw/input/vhost-user-input.c
 create mode 100644 util/drm.c
 create mode 100644 contrib/vhost-user-gpu/Makefile.objs
 create mode 100644 contrib/vhost-user-input/Makefile.objs
 create mode 100644 docs/interop/vhost-user-gpu.rst

-- 
2.18.0.129.ge3331758f1

On Fri, Jul 13, 2018 at 3:12 PM Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
>
> Hi,
>
> vhost-user allows to drive a virtio device in a seperate
> process. After vhost-user-net, we have seen
> vhost-user-{scsi,blk,crypto} added more recently.
>
> This series, initially proposed 2 years ago
> (https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg01905.html)
> contributes with vhost-user-input and vhost-user-gpu.
>
> Additionally, to factor out common code and ease the usage, a
> vhost-user-backend is introduced as an intermediary object between the
> backend and the qemu device.
>
> You may start a vhost-user-gpu with virgl rendering in a separate
> process like this:
>
> $ ./vhost-user-gpu --virgl -s vgpu.sock &
> $ qemu...
>   -chardev socket,id=chr,path=vgpu.sock
>   -object vhost-user-backend,id=vug,chardev=chr
>   -device vhost-user-vga,vhost-user=vug
>
> You may also specify the backend command and the arguments as part of
> vhost-user-backend qemu arguments. For example, to start a
> vhost-user-input backend on input device /dev/input/event19:
>
> -object vhost-user-backend,id=vuid,cmd="vhost-user-input /dev/input/event19"
> -device virtio-input-host-pci,vhost-user=vuid
>
> The vhost-user-gpu backend requires virgl from git.
>
> The libvirt support is on-going work:
> https://github.com/elmarco/libvirt/commits/vhost-user-gpu
>
> The GPU benchmarks are encouraging, giving up to x5 performance on
> Unigine Heaven 4.0.
>
> Feedback welcome,

ping

fwiw, there is a related series for libvirt support: "[libvirt] [RFC
PATCH 00/17] Add vhost-user-gpu support"

thanks

>
> v4:
>  - move qemu_write_pidfile() in util, improve it a bit
>  - add --pid and --fd arguments to vhost-user to help with libvirt support
>  - various bug fixes for synchronization, and tearing down
>
> v3: deal with most comments from rfcv2 and various improvements
>  - "vhost-user-backend: allow to specify binary to execute" as seperate
>   patch, not for inclusion since Daniel as concerned about parsing
>   shell strings with glib
>  - use dmabuf to share 2d rendering result (with intel gem only atm)
>  - document the vhost-user-gpu protocol
>  - make vhost-user-gpu-pci and vhost-user-vga seperate devices (instead
>    of adding vhost-user support to existing devices)
>  - allow to specify virgl rendering, and rendernode
>  - let's promote out of RFC status :)
>
> RFCv2: (addressing some of Gerd comments digged in the archives)
>  - rebased, clean ups, various small fixes, update commit messages
>  - teach the vhost-user-backend to take a chardev
>  - add vhost-user-input-pci, instead of adding vhost code to virtio-input-host-pci
>
> Marc-André Lureau (29):
>   chardev: avoid crash if no associated address
>   chardev: remove qemu_chr_fe_read_all() counter
>   chardev: unref if underlying chardev has no parent
>   dmabuf: add y0_top, pass it to spice
>   vhost-user: simplify vhost_user_init/vhost_user_cleanup
>   libvhost-user: exit by default on VHOST_USER_NONE
>   vhost-user: wrap some read/write with retry handling
>   Add vhost-user-backend
>   qio: add qio_channel_command_new_spawn_with_pre_exec()
>   HACK: vhost-user-backend: allow to specify binary to execute
>   vhost-user: split vhost_user_read()
>   vhost-user: add vhost_user_input_get_config()
>   libvhost-user: export vug_source_new()
>   contrib: add vhost-user-input
>   Add vhost-user-input-pci
>   vhost-user: add vhost_user_gpu_set_socket()
>   vhost-user: add vhost_user_gpu_get_num_capsets()
>   virtio: add virtio-gpu bswap helpers header
>   util: promote qemu_egl_rendernode_open() to libqemuutil
>   util: add qemu_write_pidfile()
>   util: use fcntl() for qemu_write_pidfile() locking
>   contrib: add vhost-user-gpu
>   virtio-gpu: remove unused qdev
>   virtio-gpu: remove unused config_size
>   virtio-gpu: block both 2d and 3d rendering
>   virtio-gpu: remove useless 'waiting' field
>   virtio-gpu: split virtio-gpu, introduce virtio-gpu-base
>   virtio-gpu: split virtio-gpu-pci & virtio-vga
>   hw/display: add vhost-user-vga & gpu-pci
>
>  contrib/libvhost-user/libvhost-user-glib.h |    3 +
>  contrib/libvhost-user/libvhost-user.h      |    3 +
>  contrib/vhost-user-gpu/drm.h               |   63 ++
>  contrib/vhost-user-gpu/virgl.h             |   25 +
>  contrib/vhost-user-gpu/vugpu.h             |  168 +++
>  hw/display/virtio-vga.h                    |   22 +
>  hw/virtio/virtio-pci.h                     |   27 +-
>  include/hw/virtio/vhost-backend.h          |    6 +
>  include/hw/virtio/vhost-user-blk.h         |    2 +-
>  include/hw/virtio/vhost-user-scsi.h        |    2 +-
>  include/hw/virtio/vhost-user.h             |    2 +-
>  include/hw/virtio/virtio-gpu-bswap.h       |   61 +
>  include/hw/virtio/virtio-gpu.h             |   90 +-
>  include/hw/virtio/virtio-input.h           |   14 +
>  include/io/channel-command.h               |   18 +
>  include/qemu/drm.h                         |    6 +
>  include/qemu/osdep.h                       |    3 +-
>  include/sysemu/vhost-user-backend.h        |   60 +
>  include/ui/console.h                       |    1 +
>  backends/cryptodev-vhost-user.c            |   18 +-
>  backends/vhost-user.c                      |  320 ++++++
>  chardev/char-fe.c                          |   13 +-
>  chardev/char-socket.c                      |    8 +-
>  contrib/libvhost-user/libvhost-user-glib.c |   15 +-
>  contrib/libvhost-user/libvhost-user.c      |    3 +-
>  contrib/vhost-user-gpu/drm.c               |  190 ++++
>  contrib/vhost-user-gpu/main.c              | 1164 ++++++++++++++++++++
>  contrib/vhost-user-gpu/virgl.c             |  579 ++++++++++
>  contrib/vhost-user-input/main.c            |  379 +++++++
>  hw/block/vhost-user-blk.c                  |   22 +-
>  hw/display/vhost-user-gpu-pci.c            |   51 +
>  hw/display/vhost-user-gpu.c                |  524 +++++++++
>  hw/display/vhost-user-vga.c                |   52 +
>  hw/display/virtio-gpu-3d.c                 |   70 +-
>  hw/display/virtio-gpu-base.c               |  292 +++++
>  hw/display/virtio-gpu-pci.c                |   39 +-
>  hw/display/virtio-gpu.c                    |  368 ++-----
>  hw/display/virtio-vga.c                    |  127 ++-
>  hw/input/vhost-user-input.c                |  110 ++
>  hw/scsi/vhost-user-scsi.c                  |   20 +-
>  hw/virtio/vhost-stub.c                     |    4 +-
>  hw/virtio/vhost-user.c                     |  143 ++-
>  hw/virtio/virtio-pci.c                     |   20 +
>  io/channel-command.c                       |   33 +-
>  net/vhost-user.c                           |   13 +-
>  os-posix.c                                 |   24 -
>  os-win32.c                                 |   25 -
>  qga/main.c                                 |   54 +-
>  scsi/qemu-pr-helper.c                      |   40 +-
>  ui/egl-helpers.c                           |   51 +-
>  ui/spice-display.c                         |    3 +-
>  util/drm.c                                 |   66 ++
>  util/oslib-posix.c                         |   39 +
>  util/oslib-win32.c                         |   27 +
>  vl.c                                       |    8 +-
>  MAINTAINERS                                |   15 +-
>  Makefile                                   |    6 +
>  Makefile.objs                              |    2 +
>  backends/Makefile.objs                     |    3 +-
>  configure                                  |   35 +
>  contrib/vhost-user-gpu/Makefile.objs       |   10 +
>  contrib/vhost-user-input/Makefile.objs     |    1 +
>  docs/interop/vhost-user-gpu.rst            |  236 ++++
>  docs/interop/vhost-user.txt                |   24 +
>  hw/display/Makefile.objs                   |    5 +-
>  hw/input/Makefile.objs                     |    1 +
>  qemu-options.hx                            |   24 +
>  util/Makefile.objs                         |    1 +
>  68 files changed, 5130 insertions(+), 723 deletions(-)
>  create mode 100644 contrib/vhost-user-gpu/drm.h
>  create mode 100644 contrib/vhost-user-gpu/virgl.h
>  create mode 100644 contrib/vhost-user-gpu/vugpu.h
>  create mode 100644 hw/display/virtio-vga.h
>  create mode 100644 include/hw/virtio/virtio-gpu-bswap.h
>  create mode 100644 include/qemu/drm.h
>  create mode 100644 include/sysemu/vhost-user-backend.h
>  create mode 100644 backends/vhost-user.c
>  create mode 100644 contrib/vhost-user-gpu/drm.c
>  create mode 100644 contrib/vhost-user-gpu/main.c
>  create mode 100644 contrib/vhost-user-gpu/virgl.c
>  create mode 100644 contrib/vhost-user-input/main.c
>  create mode 100644 hw/display/vhost-user-gpu-pci.c
>  create mode 100644 hw/display/vhost-user-gpu.c
>  create mode 100644 hw/display/vhost-user-vga.c
>  create mode 100644 hw/display/virtio-gpu-base.c
>  create mode 100644 hw/input/vhost-user-input.c
>  create mode 100644 util/drm.c
>  create mode 100644 contrib/vhost-user-gpu/Makefile.objs
>  create mode 100644 contrib/vhost-user-input/Makefile.objs
>  create mode 100644 docs/interop/vhost-user-gpu.rst
>
> --
> 2.18.0.129.ge3331758f1
>
>


-- 
Marc-André Lureau

  Hi,

> > Feedback welcome,
> 
> ping

Cherry-picked two patches for the ui pull request.

What is the plan for the command line handling, specifically the patch
tagged with HACK?

cheers,
  Gerd

Hi

On Tue, Aug 21, 2018 at 9:51 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>   Hi,
>
> > > Feedback welcome,
> >
> > ping
>
> Cherry-picked two patches for the ui pull request.

thanks

>
> What is the plan for the command line handling, specifically the patch
> tagged with HACK?

The RFC libvirt series doesn't use it.

However, it's very convenient for qemu command line users. Daniel
doesn't want g_shell_parse_argv(), but he would like the -object json:
command line form instead. I don't know what is the status about
supporting that syntax.

You may skip patch 9 & 10 for review & merge. But for easy testing, I
recommend to use them atm :)

thanks



--
Marc-André Lureau

On Tue, Aug 21, 2018 at 12:10:46PM +0200, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Aug 21, 2018 at 9:51 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> >   Hi,
> >
> > > > Feedback welcome,
> > >
> > > ping
> >
> > Cherry-picked two patches for the ui pull request.
> 
> thanks
> 
> >
> > What is the plan for the command line handling, specifically the patch
> > tagged with HACK?
> 
> The RFC libvirt series doesn't use it.
> 
> However, it's very convenient for qemu command line users. Daniel
> doesn't want g_shell_parse_argv(), but he would like the -object json:
> command line form instead. I don't know what is the status about
> supporting that syntax.

IIRC the last suggestion I saw was to not allow custom ARGV at all ?
ie define a standard contract that all scripts must follow, as we
have done with the ifup scripts.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Fri, Jul 13, 2018 at 03:08:47PM +0200, Marc-André Lureau wrote:
> Hi,
> 
> vhost-user allows to drive a virtio device in a seperate
> process. After vhost-user-net, we have seen
> vhost-user-{scsi,blk,crypto} added more recently.
> 
> This series, initially proposed 2 years ago
> (https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg01905.html)
> contributes with vhost-user-input and vhost-user-gpu.
> 
> Additionally, to factor out common code and ease the usage, a
> vhost-user-backend is introduced as an intermediary object between the
> backend and the qemu device.
> 
> You may start a vhost-user-gpu with virgl rendering in a separate
> process like this:
> 
> $ ./vhost-user-gpu --virgl -s vgpu.sock &
> $ qemu...
>   -chardev socket,id=chr,path=vgpu.sock
>   -object vhost-user-backend,id=vug,chardev=chr
>   -device vhost-user-vga,vhost-user=vug
> 
> You may also specify the backend command and the arguments as part of
> vhost-user-backend qemu arguments. For example, to start a
> vhost-user-input backend on input device /dev/input/event19:
> 
> -object vhost-user-backend,id=vuid,cmd="vhost-user-input /dev/input/event19"
> -device virtio-input-host-pci,vhost-user=vuid
> 
> The vhost-user-gpu backend requires virgl from git.
> 
> The libvirt support is on-going work:
> https://github.com/elmarco/libvirt/commits/vhost-user-gpu
> 
> The GPU benchmarks are encouraging, giving up to x5 performance on
> Unigine Heaven 4.0.

What is the main driving motivation behind this featureset ? Is it aimed
at providing performance, or security, or allowing arbitrary out of tree
backends, or all three ?

Although we've got a number of vhost-user backends I'm pretty concerned
about the direction this is taking QEMU overall.

Managing QEMU is non-trivial for a number of reasons. We've done a lot of
work to provide standardized modelling of CLI args, guest ABI stability
via association with machine types, migration data stream stability,
QEMU feature capabilities detection and so on.

The move to outsource backends to external binaries is discarding some
or all of these items, rewinding alot of progress we've made in the
managability of QEMU. Each external binary has to now reinvent the
things that are already solved in QEMU, and you can be sure each impl
will reinvent the concepts differently.

I can't help feeling that we are shooting ourselves in the foot here
long term.

We've always rejected the idea of having loadable modules in QEMU, but
as a result we've end up with outsourcing the backend entirely via the
vhost-user framework, so the end result is even more opaque than if we
had loadable modules, and is unable to take advantage of our standardized
modelling frameworks & capabilities :-(

If we just look at performance & security, and ignore 3rd party impls
for a minute, I really don't like that to gain perf + security  we have
to change from:

 $ qemu...
   -device virtio-vga
 
To

 $ ./vhost-user-gpu --virgl -s vgpu.sock &
 $ qemu...
   -chardev socket,id=chr,path=vgpu.sock
   -object vhost-user-backend,id=vug,chardev=chr
   -device vhost-user-vga,vhost-user=vug

Once we have the ability to run the backend via an external process,
to gain performance & security benefits, assuming feature parity is
achieved, I question why anyone would want to continue with the old
in-process approach ? IOW the goal should be that the original args 

 $ qemu...
   -device virtio-vga

should "do the right thing" to launch the external process when you
have upgraded to a new enough QEMU, so that everyone immediately
benefits from the more secure & performant architecture. 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Fri, Jul 13, 2018 at 03:08:47PM +0200, Marc-André Lureau wrote:
> > Hi,
> > 
> > vhost-user allows to drive a virtio device in a seperate
> > process. After vhost-user-net, we have seen
> > vhost-user-{scsi,blk,crypto} added more recently.
> > 
> > This series, initially proposed 2 years ago
> > (https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg01905.html)
> > contributes with vhost-user-input and vhost-user-gpu.
> > 
> > Additionally, to factor out common code and ease the usage, a
> > vhost-user-backend is introduced as an intermediary object between the
> > backend and the qemu device.
> > 
> > You may start a vhost-user-gpu with virgl rendering in a separate
> > process like this:
> > 
> > $ ./vhost-user-gpu --virgl -s vgpu.sock &
> > $ qemu...
> >   -chardev socket,id=chr,path=vgpu.sock
> >   -object vhost-user-backend,id=vug,chardev=chr
> >   -device vhost-user-vga,vhost-user=vug
> > 
> > You may also specify the backend command and the arguments as part of
> > vhost-user-backend qemu arguments. For example, to start a
> > vhost-user-input backend on input device /dev/input/event19:
> > 
> > -object vhost-user-backend,id=vuid,cmd="vhost-user-input /dev/input/event19"
> > -device virtio-input-host-pci,vhost-user=vuid
> > 
> > The vhost-user-gpu backend requires virgl from git.
> > 
> > The libvirt support is on-going work:
> > https://github.com/elmarco/libvirt/commits/vhost-user-gpu
> > 
> > The GPU benchmarks are encouraging, giving up to x5 performance on
> > Unigine Heaven 4.0.
> 
> What is the main driving motivation behind this featureset ? Is it aimed
> at providing performance, or security, or allowing arbitrary out of tree
> backends, or all three ?
> 
> Although we've got a number of vhost-user backends I'm pretty concerned
> about the direction this is taking QEMU overall.
> 
> Managing QEMU is non-trivial for a number of reasons. We've done a lot of
> work to provide standardized modelling of CLI args, guest ABI stability
> via association with machine types, migration data stream stability,
> QEMU feature capabilities detection and so on.
> 
> The move to outsource backends to external binaries is discarding some
> or all of these items, rewinding alot of progress we've made in the
> managability of QEMU. Each external binary has to now reinvent the
> things that are already solved in QEMU, and you can be sure each impl
> will reinvent the concepts differently.
> 
> I can't help feeling that we are shooting ourselves in the foot here
> long term.
> 
> We've always rejected the idea of having loadable modules in QEMU, but
> as a result we've end up with outsourcing the backend entirely via the
> vhost-user framework, so the end result is even more opaque than if we
> had loadable modules, and is unable to take advantage of our standardized
> modelling frameworks & capabilities :-(
> 
> If we just look at performance & security, and ignore 3rd party impls
> for a minute, I really don't like that to gain perf + security  we have
> to change from:
> 
>  $ qemu...
>    -device virtio-vga
>  
> To
> 
>  $ ./vhost-user-gpu --virgl -s vgpu.sock &
>  $ qemu...
>    -chardev socket,id=chr,path=vgpu.sock
>    -object vhost-user-backend,id=vug,chardev=chr
>    -device vhost-user-vga,vhost-user=vug
> 
> Once we have the ability to run the backend via an external process,
> to gain performance & security benefits, assuming feature parity is
> achieved, I question why anyone would want to continue with the old
> in-process approach ? IOW the goal should be that the original args 
> 
>  $ qemu...
>    -device virtio-vga
> 
> should "do the right thing" to launch the external process when you
> have upgraded to a new enough QEMU, so that everyone immediately
> benefits from the more secure & performant architecture. 

But which external process should it run, under what privilieges
and with sockets placed where?
While it's true it would be good to have a nice simple way, often
the qemu process might not have the privs to run that external process
or know where to put the sockets; that's exactly the type of thing
we're happy to leave to libvirt to wire up.

Dave

> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

On Wed, Aug 29, 2018 at 11:22:01AM +0100, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > On Fri, Jul 13, 2018 at 03:08:47PM +0200, Marc-André Lureau wrote:
> > > Hi,
> > > 
> > > vhost-user allows to drive a virtio device in a seperate
> > > process. After vhost-user-net, we have seen
> > > vhost-user-{scsi,blk,crypto} added more recently.
> > > 
> > > This series, initially proposed 2 years ago
> > > (https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg01905.html)
> > > contributes with vhost-user-input and vhost-user-gpu.
> > > 
> > > Additionally, to factor out common code and ease the usage, a
> > > vhost-user-backend is introduced as an intermediary object between the
> > > backend and the qemu device.
> > > 
> > > You may start a vhost-user-gpu with virgl rendering in a separate
> > > process like this:
> > > 
> > > $ ./vhost-user-gpu --virgl -s vgpu.sock &
> > > $ qemu...
> > >   -chardev socket,id=chr,path=vgpu.sock
> > >   -object vhost-user-backend,id=vug,chardev=chr
> > >   -device vhost-user-vga,vhost-user=vug
> > > 
> > > You may also specify the backend command and the arguments as part of
> > > vhost-user-backend qemu arguments. For example, to start a
> > > vhost-user-input backend on input device /dev/input/event19:
> > > 
> > > -object vhost-user-backend,id=vuid,cmd="vhost-user-input /dev/input/event19"
> > > -device virtio-input-host-pci,vhost-user=vuid
> > > 
> > > The vhost-user-gpu backend requires virgl from git.
> > > 
> > > The libvirt support is on-going work:
> > > https://github.com/elmarco/libvirt/commits/vhost-user-gpu
> > > 
> > > The GPU benchmarks are encouraging, giving up to x5 performance on
> > > Unigine Heaven 4.0.
> > 
> > What is the main driving motivation behind this featureset ? Is it aimed
> > at providing performance, or security, or allowing arbitrary out of tree
> > backends, or all three ?
> > 
> > Although we've got a number of vhost-user backends I'm pretty concerned
> > about the direction this is taking QEMU overall.
> > 
> > Managing QEMU is non-trivial for a number of reasons. We've done a lot of
> > work to provide standardized modelling of CLI args, guest ABI stability
> > via association with machine types, migration data stream stability,
> > QEMU feature capabilities detection and so on.
> > 
> > The move to outsource backends to external binaries is discarding some
> > or all of these items, rewinding alot of progress we've made in the
> > managability of QEMU. Each external binary has to now reinvent the
> > things that are already solved in QEMU, and you can be sure each impl
> > will reinvent the concepts differently.
> > 
> > I can't help feeling that we are shooting ourselves in the foot here
> > long term.
> > 
> > We've always rejected the idea of having loadable modules in QEMU, but
> > as a result we've end up with outsourcing the backend entirely via the
> > vhost-user framework, so the end result is even more opaque than if we
> > had loadable modules, and is unable to take advantage of our standardized
> > modelling frameworks & capabilities :-(
> > 
> > If we just look at performance & security, and ignore 3rd party impls
> > for a minute, I really don't like that to gain perf + security  we have
> > to change from:
> > 
> >  $ qemu...
> >    -device virtio-vga
> >  
> > To
> > 
> >  $ ./vhost-user-gpu --virgl -s vgpu.sock &
> >  $ qemu...
> >    -chardev socket,id=chr,path=vgpu.sock
> >    -object vhost-user-backend,id=vug,chardev=chr
> >    -device vhost-user-vga,vhost-user=vug
> > 
> > Once we have the ability to run the backend via an external process,
> > to gain performance & security benefits, assuming feature parity is
> > achieved, I question why anyone would want to continue with the old
> > in-process approach ? IOW the goal should be that the original args 
> > 
> >  $ qemu...
> >    -device virtio-vga
> > 
> > should "do the right thing" to launch the external process when you
> > have upgraded to a new enough QEMU, so that everyone immediately
> > benefits from the more secure & performant architecture. 
> 
> But which external process should it run, under what privilieges
> and with sockets placed where?
> While it's true it would be good to have a nice simple way, often
> the qemu process might not have the privs to run that external process
> or know where to put the sockets; that's exactly the type of thing
> we're happy to leave to libvirt to wire up.

We can still do both - the simple case where QEMU just spawns a process,

  $ qemu...
    -device virtio-vga

while also allowing connections to a pre-spawned process to have extra
privs via:

  $ qemu...
    -device virtio-vga,vhost=chardevID

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Hi

On Wed, Aug 29, 2018 at 11:50 AM, Daniel P. Berrangé
<berrange@redhat.com> wrote:
> On Fri, Jul 13, 2018 at 03:08:47PM +0200, Marc-André Lureau wrote:
>> Hi,
>>
>> vhost-user allows to drive a virtio device in a seperate
>> process. After vhost-user-net, we have seen
>> vhost-user-{scsi,blk,crypto} added more recently.
>>
>> This series, initially proposed 2 years ago
>> (https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg01905.html)
>> contributes with vhost-user-input and vhost-user-gpu.
>>
>> Additionally, to factor out common code and ease the usage, a
>> vhost-user-backend is introduced as an intermediary object between the
>> backend and the qemu device.
>>
>> You may start a vhost-user-gpu with virgl rendering in a separate
>> process like this:
>>
>> $ ./vhost-user-gpu --virgl -s vgpu.sock &
>> $ qemu...
>>   -chardev socket,id=chr,path=vgpu.sock
>>   -object vhost-user-backend,id=vug,chardev=chr
>>   -device vhost-user-vga,vhost-user=vug
>>
>> You may also specify the backend command and the arguments as part of
>> vhost-user-backend qemu arguments. For example, to start a
>> vhost-user-input backend on input device /dev/input/event19:
>>
>> -object vhost-user-backend,id=vuid,cmd="vhost-user-input /dev/input/event19"
>> -device virtio-input-host-pci,vhost-user=vuid
>>
>> The vhost-user-gpu backend requires virgl from git.
>>
>> The libvirt support is on-going work:
>> https://github.com/elmarco/libvirt/commits/vhost-user-gpu
>>
>> The GPU benchmarks are encouraging, giving up to x5 performance on
>> Unigine Heaven 4.0.
>
> What is the main driving motivation behind this featureset ? Is it aimed
> at providing performance, or security, or allowing arbitrary out of tree
> backends, or all three ?

Mainly security/stability & performance. Allowing arbitrary out of
tree backends is a bonus for me, I don't care much if we don't allow
it.

> Although we've got a number of vhost-user backends I'm pretty concerned
> about the direction this is taking QEMU overall.
>
> Managing QEMU is non-trivial for a number of reasons. We've done a lot of
> work to provide standardized modelling of CLI args, guest ABI stability
> via association with machine types, migration data stream stability,
> QEMU feature capabilities detection and so on.
>
> The move to outsource backends to external binaries is discarding some
> or all of these items, rewinding alot of progress we've made in the
> managability of QEMU. Each external binary has to now reinvent the
> things that are already solved in QEMU, and you can be sure each impl
> will reinvent the concepts differently.
>
> I can't help feeling that we are shooting ourselves in the foot here
> long term.

I have a bit of the same feeling. For ex, I was a bit reluctant having
the TPM emulator as an external process (new protocol, external
binaries, with various management work, "opaque" migration). But in
the end, the integration with libvirt makes thing quite easy for the
user. So I changed a bit my mind, and I think this is one task that
libvirt can be really good at.

>
> We've always rejected the idea of having loadable modules in QEMU, but
> as a result we've end up with outsourcing the backend entirely via the
> vhost-user framework, so the end result is even more opaque than if we
> had loadable modules, and is unable to take advantage of our standardized
> modelling frameworks & capabilities :-(

I agree, that's one of the reason I put together libvhostuser in the
first place. If qemu ships its own backend, there is no reason we
don't share modelling & code etc.

For ex, I hope more vhost-users will use the -object
vhost-user-backend to do basic connection and virtio setup. (in the
series, -gpu and -input, I didn't really investigate -crypto, -net,
-blk etc)

> If we just look at performance & security, and ignore 3rd party impls
> for a minute, I really don't like that to gain perf + security  we have
> to change from:
>
>  $ qemu...
>    -device virtio-vga
>
> To
>
>  $ ./vhost-user-gpu --virgl -s vgpu.sock &
>  $ qemu...
>    -chardev socket,id=chr,path=vgpu.sock
>    -object vhost-user-backend,id=vug,chardev=chr
>    -device vhost-user-vga,vhost-user=vug
>

That's a bit incovenient for qemu command line users. But who runs
qemu this way but some developers? (others have higher-level scripts /
tools or libvirt)

> Once we have the ability to run the backend via an external process,
> to gain performance & security benefits, assuming feature parity is
> achieved, I question why anyone would want to continue with the old
> in-process approach ? IOW the goal should be that the original args
>
>  $ qemu...
>    -device virtio-vga
>
> should "do the right thing" to launch the external process when you
> have upgraded to a new enough QEMU, so that everyone immediately
> benefits from the more secure & performant architecture.


That's not incompatible with having the lengthy version. But this
comes with some downside atm, since migration is not implemented for
ex (for 2d, virgl doesn't have migration).

And seccomp spawn rule disable forking.

And libvirt will probably want to manage the external process for
security/resource/tweaking reasons.

thanks for the feedback!

 Hi

On Wed, Aug 29, 2018 at 4:00 PM Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
>
> Hi
>
> On Wed, Aug 29, 2018 at 11:50 AM, Daniel P. Berrangé
> <berrange@redhat.com> wrote:
> > On Fri, Jul 13, 2018 at 03:08:47PM +0200, Marc-André Lureau wrote:
> >> Hi,
> >>
> >> vhost-user allows to drive a virtio device in a seperate
> >> process. After vhost-user-net, we have seen
> >> vhost-user-{scsi,blk,crypto} added more recently.
> >>
> >> This series, initially proposed 2 years ago
> >> (https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg01905.html)
> >> contributes with vhost-user-input and vhost-user-gpu.
> >>
> >> Additionally, to factor out common code and ease the usage, a
> >> vhost-user-backend is introduced as an intermediary object between the
> >> backend and the qemu device.
> >>
> >> You may start a vhost-user-gpu with virgl rendering in a separate
> >> process like this:
> >>
> >> $ ./vhost-user-gpu --virgl -s vgpu.sock &
> >> $ qemu...
> >>   -chardev socket,id=chr,path=vgpu.sock
> >>   -object vhost-user-backend,id=vug,chardev=chr
> >>   -device vhost-user-vga,vhost-user=vug
> >>
> >> You may also specify the backend command and the arguments as part of
> >> vhost-user-backend qemu arguments. For example, to start a
> >> vhost-user-input backend on input device /dev/input/event19:
> >>
> >> -object vhost-user-backend,id=vuid,cmd="vhost-user-input /dev/input/event19"
> >> -device virtio-input-host-pci,vhost-user=vuid
> >>
> >> The vhost-user-gpu backend requires virgl from git.
> >>
> >> The libvirt support is on-going work:
> >> https://github.com/elmarco/libvirt/commits/vhost-user-gpu
> >>
> >> The GPU benchmarks are encouraging, giving up to x5 performance on
> >> Unigine Heaven 4.0.
> >
> > What is the main driving motivation behind this featureset ? Is it aimed
> > at providing performance, or security, or allowing arbitrary out of tree
> > backends, or all three ?
>
> Mainly security/stability & performance. Allowing arbitrary out of
> tree backends is a bonus for me, I don't care much if we don't allow
> it.
>
> > Although we've got a number of vhost-user backends I'm pretty concerned
> > about the direction this is taking QEMU overall.
> >
> > Managing QEMU is non-trivial for a number of reasons. We've done a lot of
> > work to provide standardized modelling of CLI args, guest ABI stability
> > via association with machine types, migration data stream stability,
> > QEMU feature capabilities detection and so on.
> >
> > The move to outsource backends to external binaries is discarding some
> > or all of these items, rewinding alot of progress we've made in the
> > managability of QEMU. Each external binary has to now reinvent the
> > things that are already solved in QEMU, and you can be sure each impl
> > will reinvent the concepts differently.
> >
> > I can't help feeling that we are shooting ourselves in the foot here
> > long term.
>
> I have a bit of the same feeling. For ex, I was a bit reluctant having
> the TPM emulator as an external process (new protocol, external
> binaries, with various management work, "opaque" migration). But in
> the end, the integration with libvirt makes thing quite easy for the
> user. So I changed a bit my mind, and I think this is one task that
> libvirt can be really good at.
>
> >
> > We've always rejected the idea of having loadable modules in QEMU, but
> > as a result we've end up with outsourcing the backend entirely via the
> > vhost-user framework, so the end result is even more opaque than if we
> > had loadable modules, and is unable to take advantage of our standardized
> > modelling frameworks & capabilities :-(
>
> I agree, that's one of the reason I put together libvhostuser in the
> first place. If qemu ships its own backend, there is no reason we
> don't share modelling & code etc.
>
> For ex, I hope more vhost-users will use the -object
> vhost-user-backend to do basic connection and virtio setup. (in the
> series, -gpu and -input, I didn't really investigate -crypto, -net,
> -blk etc)
>
> > If we just look at performance & security, and ignore 3rd party impls
> > for a minute, I really don't like that to gain perf + security  we have
> > to change from:
> >
> >  $ qemu...
> >    -device virtio-vga
> >
> > To
> >
> >  $ ./vhost-user-gpu --virgl -s vgpu.sock &
> >  $ qemu...
> >    -chardev socket,id=chr,path=vgpu.sock
> >    -object vhost-user-backend,id=vug,chardev=chr
> >    -device vhost-user-vga,vhost-user=vug
> >
>
> That's a bit incovenient for qemu command line users. But who runs
> qemu this way but some developers? (others have higher-level scripts /
> tools or libvirt)
>
> > Once we have the ability to run the backend via an external process,
> > to gain performance & security benefits, assuming feature parity is
> > achieved, I question why anyone would want to continue with the old
> > in-process approach ? IOW the goal should be that the original args
> >
> >  $ qemu...
> >    -device virtio-vga
> >
> > should "do the right thing" to launch the external process when you
> > have upgraded to a new enough QEMU, so that everyone immediately
> > benefits from the more secure & performant architecture.
>
>
> That's not incompatible with having the lengthy version. But this
> comes with some downside atm, since migration is not implemented for
> ex (for 2d, virgl doesn't have migration).
>
> And seccomp spawn rule disable forking.
>
> And libvirt will probably want to manage the external process for
> security/resource/tweaking reasons.

I would like to make progress on this, but I feel a bit stuck. As
explained earlier, libvirt will have to manage the external processes.
So what we would like to see, is a stable interface for the various
vhost-user backends.

I suggest the vhost-user binary (vhost-user-gpu or vhost-user-input
etc), the default binary be in the system PATH, so that libvirt & co
can find it. Host administrator can use update-alternative or such if
they are other implementations. (other selections mechanisms can be
added later)

I thinks the vhost-user backends could have the following common
options handling:
- take a "--fd=FDNUM" argument, that would indicate which fd the
socket has been passed on.
- OR take a "--socket-path=PATH"
- optionally? take a "--pid=PATH" argument, to write out the process PID

We probably need a way to list the capabilities of the backend.:
--dump-caps, could list known keywords, one per line? (grab, virgl,
opengles, etc...)

Other options may vary based on the backend type, for ex:
- vhost-user-input EVDEV-PATH (required)
- vhost-user-gpu --render-node=PATH (optional)

I am not sure how this should be documented.

Any help appreciated!
thanks

-- 
Marc-André Lureau

> > >  $ ./vhost-user-gpu --virgl -s vgpu.sock &
> > >  $ qemu...
> > >    -chardev socket,id=chr,path=vgpu.sock
> > >    -object vhost-user-backend,id=vug,chardev=chr
> > >    -device vhost-user-vga,vhost-user=vug
> >
> > That's a bit incovenient for qemu command line users. But who runs
> > qemu this way but some developers? (others have higher-level scripts /
> > tools or libvirt)

In practice this will *allways* run over a unix socket, right?
So I'm wondering whenever it makes sense to take the chardev
indirection here ...

> I suggest the vhost-user binary (vhost-user-gpu or vhost-user-input
> etc), the default binary be in the system PATH, so that libvirt & co
> can find it. Host administrator can use update-alternative or such if
> they are other implementations. (other selections mechanisms can be
> added later)

Might make sense to place them in /usr/libexec instead.

> I thinks the vhost-user backends could have the following common
> options handling:
> - take a "--fd=FDNUM" argument, that would indicate which fd the
> socket has been passed on.

Which is what libvirt would use, right?

> - OR take a "--socket-path=PATH"

Convinient when not using libvirt, looks ok to me.

> - optionally? take a "--pid=PATH" argument, to write out the process PID

Do we need that?  When libvirt forks off the process it knows the PID
without pidfile, right?

> We probably need a way to list the capabilities of the backend.:
> --dump-caps, could list known keywords, one per line? (grab, virgl,
> opengles, etc...)

Hmm, good question.

I'd tend include more information here.  Such as a description of the
capability.  The command line switch to enable it.  Print as json, we
have support in both qemu and libvirt.  Something like

  [ 'name'   : 'vulkan',
    'help'   : 'enable virgl vulkan support',
    'switch' : '--enable-vulkan' ]

What would libvirt like to consume here?  Would it actually be able to
use whatever it finds there?  Or would libvirt look for specific known
features only, matching them with domain xml attributes?

> Other options may vary based on the backend type, for ex:
> - vhost-user-input EVDEV-PATH (required)

I'd go for "--evdev-path=/dev/input/..." syntax even for mandatory
arguments.

> - vhost-user-gpu --render-node=PATH (optional)
> 
> I am not sure how this should be documented.

docs/specs/ ?

cheers,
  Gerd

Hi

On Tue, Sep 11, 2018 at 12:59 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> > > >  $ ./vhost-user-gpu --virgl -s vgpu.sock &
> > > >  $ qemu...
> > > >    -chardev socket,id=chr,path=vgpu.sock
> > > >    -object vhost-user-backend,id=vug,chardev=chr
> > > >    -device vhost-user-vga,vhost-user=vug
> > >
> > > That's a bit incovenient for qemu command line users. But who runs
> > > qemu this way but some developers? (others have higher-level scripts /
> > > tools or libvirt)
>
> In practice this will *allways* run over a unix socket, right?
> So I'm wondering whenever it makes sense to take the chardev
> indirection here ...

The management layer can set things up and pass fd around, avoiding
socket paths etc.

>
> > I suggest the vhost-user binary (vhost-user-gpu or vhost-user-input
> > etc), the default binary be in the system PATH, so that libvirt & co
> > can find it. Host administrator can use update-alternative or such if
> > they are other implementations. (other selections mechanisms can be
> > added later)
>
> Might make sense to place them in /usr/libexec instead.

ok (slightly less convenient for command line users though)

>
> > I thinks the vhost-user backends could have the following common
> > options handling:
> > - take a "--fd=FDNUM" argument, that would indicate which fd the
> > socket has been passed on.
>
> Which is what libvirt would use, right?

yes (see the related libvirt series / branch)

>
> > - OR take a "--socket-path=PATH"
>
> Convinient when not using libvirt, looks ok to me.
>
> > - optionally? take a "--pid=PATH" argument, to write out the process PID
>
> Do we need that?  When libvirt forks off the process it knows the PID
> without pidfile, right?

Yes I am not sure it is strictly necessary (that's why I put
optionally). The helper could fork itself, for various reasons.

>
> > We probably need a way to list the capabilities of the backend.:
> > --dump-caps, could list known keywords, one per line? (grab, virgl,
> > opengles, etc...)
>
> Hmm, good question.
>
> I'd tend include more information here.  Such as a description of the
> capability.  The command line switch to enable it.  Print as json, we
> have support in both qemu and libvirt.  Something like
>
>   [ 'name'   : 'vulkan',
>     'help'   : 'enable virgl vulkan support',
>     'switch' : '--enable-vulkan' ]
>

I wouldn't go so far as describing the command line arguments here. Or
else, we will have to specify the argument type etc.
Instead, I would use a  list of well-known keywords, described in the
specification.

"vulkan" caps would mean that --enable-vulkan (without argument) is
accepted and has an expected behaviour (to handle vulkan commands /
rendering etc)

I don't see a problem putting the list in a json array, but I don't
see much benefit either doing it.

> What would libvirt like to consume here?  Would it actually be able to
> use whatever it finds there?  Or would libvirt look for specific known
> features only, matching them with domain xml attributes?

I am not sure I understand the qestion. libvirt would deal with the
binary capabilities like it deals with qemu capabilities.

It would use the best combination of options given a domain XML and
the capabilities.

If the domain XML requests a feature that the backend is not able to
support, it would provide a sensible error.

Unknown capabilities would be ignored.

>
> > Other options may vary based on the backend type, for ex:
> > - vhost-user-input EVDEV-PATH (required)
>
> I'd go for "--evdev-path=/dev/input/..." syntax even for mandatory
> arguments.

ok

>
> > - vhost-user-gpu --render-node=PATH (optional)
> >
> > I am not sure how this should be documented.
>
> docs/specs/ ?

ok, I suppose I can start some document and include libvir-list for discussion.


-- 
Marc-André Lureau

On Tue, Sep 11, 2018 at 01:16:22PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Sep 11, 2018 at 12:59 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > > > >  $ ./vhost-user-gpu --virgl -s vgpu.sock &
> > > > >  $ qemu...
> > > > >    -chardev socket,id=chr,path=vgpu.sock
> > > > >    -object vhost-user-backend,id=vug,chardev=chr
> > > > >    -device vhost-user-vga,vhost-user=vug
> > > >
> > > > That's a bit incovenient for qemu command line users. But who runs
> > > > qemu this way but some developers? (others have higher-level scripts /
> > > > tools or libvirt)
> >
> > In practice this will *allways* run over a unix socket, right?
> > So I'm wondering whenever it makes sense to take the chardev
> > indirection here ...
> 
> The management layer can set things up and pass fd around, avoiding
> socket paths etc.

But that fd will be a socket too ...

> > > - optionally? take a "--pid=PATH" argument, to write out the process PID
> >
> > Do we need that?  When libvirt forks off the process it knows the PID
> > without pidfile, right?
> 
> Yes I am not sure it is strictly necessary (that's why I put
> optionally). The helper could fork itself, for various reasons.

Ok, good point.  But then I'd make this mandatory, and maybe name it
--pidfile.  Shouldn't a big burden as you probably have code for that in
the vhost-user helper lib

> Unknown capabilities would be ignored.

Ok, so when libvirt ignores unknown stuff anyway a list of keywords is
good enough I guess.  And for humans having --help is more useful.

I'd include the protocol into the names, for robustness reasons, i.e.
use "input-grab" or "gpu-vulkan".

cheers,
  Gerd

Hi

On Fri, Jul 13, 2018 at 3:12 PM Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
>
> Hi,
>
> vhost-user allows to drive a virtio device in a seperate
> process. After vhost-user-net, we have seen
> vhost-user-{scsi,blk,crypto} added more recently.
>
> This series, initially proposed 2 years ago
> (https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg01905.html)
> contributes with vhost-user-input and vhost-user-gpu.
>
> Additionally, to factor out common code and ease the usage, a
> vhost-user-backend is introduced as an intermediary object between the
> backend and the qemu device.
>
> You may start a vhost-user-gpu with virgl rendering in a separate
> process like this:
>
> $ ./vhost-user-gpu --virgl -s vgpu.sock &
> $ qemu...
>   -chardev socket,id=chr,path=vgpu.sock
>   -object vhost-user-backend,id=vug,chardev=chr
>   -device vhost-user-vga,vhost-user=vug
>
> You may also specify the backend command and the arguments as part of
> vhost-user-backend qemu arguments. For example, to start a
> vhost-user-input backend on input device /dev/input/event19:
>
> -object vhost-user-backend,id=vuid,cmd="vhost-user-input /dev/input/event19"
> -device virtio-input-host-pci,vhost-user=vuid
>
> The vhost-user-gpu backend requires virgl from git.
>
> The libvirt support is on-going work:
> https://github.com/elmarco/libvirt/commits/vhost-user-gpu
>
> The GPU benchmarks are encouraging, giving up to x5 performance on
> Unigine Heaven 4.0.
>

I rebased the series: (on top of virtio-vga: fix reset)

https://github.com/elmarco/qemu/ vhost-user-gpu

> Feedback welcome,
>
> v4:
>  - move qemu_write_pidfile() in util, improve it a bit
>  - add --pid and --fd arguments to vhost-user to help with libvirt support
>  - various bug fixes for synchronization, and tearing down
>
> v3: deal with most comments from rfcv2 and various improvements
>  - "vhost-user-backend: allow to specify binary to execute" as seperate
>   patch, not for inclusion since Daniel as concerned about parsing
>   shell strings with glib
>  - use dmabuf to share 2d rendering result (with intel gem only atm)
>  - document the vhost-user-gpu protocol
>  - make vhost-user-gpu-pci and vhost-user-vga seperate devices (instead
>    of adding vhost-user support to existing devices)
>  - allow to specify virgl rendering, and rendernode
>  - let's promote out of RFC status :)
>
> RFCv2: (addressing some of Gerd comments digged in the archives)
>  - rebased, clean ups, various small fixes, update commit messages
>  - teach the vhost-user-backend to take a chardev
>  - add vhost-user-input-pci, instead of adding vhost code to virtio-input-host-pci
>
> Marc-André Lureau (29):
>   chardev: avoid crash if no associated address
>   chardev: remove qemu_chr_fe_read_all() counter
>   chardev: unref if underlying chardev has no parent
>   dmabuf: add y0_top, pass it to spice
>   vhost-user: simplify vhost_user_init/vhost_user_cleanup
>   libvhost-user: exit by default on VHOST_USER_NONE
>   vhost-user: wrap some read/write with retry handling
>   Add vhost-user-backend
>   qio: add qio_channel_command_new_spawn_with_pre_exec()
>   HACK: vhost-user-backend: allow to specify binary to execute
>   vhost-user: split vhost_user_read()
>   vhost-user: add vhost_user_input_get_config()
>   libvhost-user: export vug_source_new()
>   contrib: add vhost-user-input
>   Add vhost-user-input-pci
>   vhost-user: add vhost_user_gpu_set_socket()
>   vhost-user: add vhost_user_gpu_get_num_capsets()
>   virtio: add virtio-gpu bswap helpers header
>   util: promote qemu_egl_rendernode_open() to libqemuutil
>   util: add qemu_write_pidfile()
>   util: use fcntl() for qemu_write_pidfile() locking
>   contrib: add vhost-user-gpu
>   virtio-gpu: remove unused qdev
>   virtio-gpu: remove unused config_size
>   virtio-gpu: block both 2d and 3d rendering
>   virtio-gpu: remove useless 'waiting' field
>   virtio-gpu: split virtio-gpu, introduce virtio-gpu-base
>   virtio-gpu: split virtio-gpu-pci & virtio-vga
>   hw/display: add vhost-user-vga & gpu-pci
>
>  contrib/libvhost-user/libvhost-user-glib.h |    3 +
>  contrib/libvhost-user/libvhost-user.h      |    3 +
>  contrib/vhost-user-gpu/drm.h               |   63 ++
>  contrib/vhost-user-gpu/virgl.h             |   25 +
>  contrib/vhost-user-gpu/vugpu.h             |  168 +++
>  hw/display/virtio-vga.h                    |   22 +
>  hw/virtio/virtio-pci.h                     |   27 +-
>  include/hw/virtio/vhost-backend.h          |    6 +
>  include/hw/virtio/vhost-user-blk.h         |    2 +-
>  include/hw/virtio/vhost-user-scsi.h        |    2 +-
>  include/hw/virtio/vhost-user.h             |    2 +-
>  include/hw/virtio/virtio-gpu-bswap.h       |   61 +
>  include/hw/virtio/virtio-gpu.h             |   90 +-
>  include/hw/virtio/virtio-input.h           |   14 +
>  include/io/channel-command.h               |   18 +
>  include/qemu/drm.h                         |    6 +
>  include/qemu/osdep.h                       |    3 +-
>  include/sysemu/vhost-user-backend.h        |   60 +
>  include/ui/console.h                       |    1 +
>  backends/cryptodev-vhost-user.c            |   18 +-
>  backends/vhost-user.c                      |  320 ++++++
>  chardev/char-fe.c                          |   13 +-
>  chardev/char-socket.c                      |    8 +-
>  contrib/libvhost-user/libvhost-user-glib.c |   15 +-
>  contrib/libvhost-user/libvhost-user.c      |    3 +-
>  contrib/vhost-user-gpu/drm.c               |  190 ++++
>  contrib/vhost-user-gpu/main.c              | 1164 ++++++++++++++++++++
>  contrib/vhost-user-gpu/virgl.c             |  579 ++++++++++
>  contrib/vhost-user-input/main.c            |  379 +++++++
>  hw/block/vhost-user-blk.c                  |   22 +-
>  hw/display/vhost-user-gpu-pci.c            |   51 +
>  hw/display/vhost-user-gpu.c                |  524 +++++++++
>  hw/display/vhost-user-vga.c                |   52 +
>  hw/display/virtio-gpu-3d.c                 |   70 +-
>  hw/display/virtio-gpu-base.c               |  292 +++++
>  hw/display/virtio-gpu-pci.c                |   39 +-
>  hw/display/virtio-gpu.c                    |  368 ++-----
>  hw/display/virtio-vga.c                    |  127 ++-
>  hw/input/vhost-user-input.c                |  110 ++
>  hw/scsi/vhost-user-scsi.c                  |   20 +-
>  hw/virtio/vhost-stub.c                     |    4 +-
>  hw/virtio/vhost-user.c                     |  143 ++-
>  hw/virtio/virtio-pci.c                     |   20 +
>  io/channel-command.c                       |   33 +-
>  net/vhost-user.c                           |   13 +-
>  os-posix.c                                 |   24 -
>  os-win32.c                                 |   25 -
>  qga/main.c                                 |   54 +-
>  scsi/qemu-pr-helper.c                      |   40 +-
>  ui/egl-helpers.c                           |   51 +-
>  ui/spice-display.c                         |    3 +-
>  util/drm.c                                 |   66 ++
>  util/oslib-posix.c                         |   39 +
>  util/oslib-win32.c                         |   27 +
>  vl.c                                       |    8 +-
>  MAINTAINERS                                |   15 +-
>  Makefile                                   |    6 +
>  Makefile.objs                              |    2 +
>  backends/Makefile.objs                     |    3 +-
>  configure                                  |   35 +
>  contrib/vhost-user-gpu/Makefile.objs       |   10 +
>  contrib/vhost-user-input/Makefile.objs     |    1 +
>  docs/interop/vhost-user-gpu.rst            |  236 ++++
>  docs/interop/vhost-user.txt                |   24 +
>  hw/display/Makefile.objs                   |    5 +-
>  hw/input/Makefile.objs                     |    1 +
>  qemu-options.hx                            |   24 +
>  util/Makefile.objs                         |    1 +
>  68 files changed, 5130 insertions(+), 723 deletions(-)
>  create mode 100644 contrib/vhost-user-gpu/drm.h
>  create mode 100644 contrib/vhost-user-gpu/virgl.h
>  create mode 100644 contrib/vhost-user-gpu/vugpu.h
>  create mode 100644 hw/display/virtio-vga.h
>  create mode 100644 include/hw/virtio/virtio-gpu-bswap.h
>  create mode 100644 include/qemu/drm.h
>  create mode 100644 include/sysemu/vhost-user-backend.h
>  create mode 100644 backends/vhost-user.c
>  create mode 100644 contrib/vhost-user-gpu/drm.c
>  create mode 100644 contrib/vhost-user-gpu/main.c
>  create mode 100644 contrib/vhost-user-gpu/virgl.c
>  create mode 100644 contrib/vhost-user-input/main.c
>  create mode 100644 hw/display/vhost-user-gpu-pci.c
>  create mode 100644 hw/display/vhost-user-gpu.c
>  create mode 100644 hw/display/vhost-user-vga.c
>  create mode 100644 hw/display/virtio-gpu-base.c
>  create mode 100644 hw/input/vhost-user-input.c
>  create mode 100644 util/drm.c
>  create mode 100644 contrib/vhost-user-gpu/Makefile.objs
>  create mode 100644 contrib/vhost-user-input/Makefile.objs
>  create mode 100644 docs/interop/vhost-user-gpu.rst
>
> --
> 2.18.0.129.ge3331758f1
>
>


-- 
Marc-André Lureau