[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 00/12] virtio: add vhost-user-generic and reduce copy and pas
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH 00/12] virtio: add vhost-user-generic and reduce copy and paste |
Date: |
Mon, 17 Apr 2023 08:32:58 -0400 |
On Fri, 14 Apr 2023 at 12:06, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> A lot of our vhost-user stubs are large chunks of boilerplate that do
> (mostly) the same thing. This series attempts to fix that by defining
> a new base class for vhost-user devices and then converting the rng
> and gpio devices to be based off them. You can even use
> vhost-user-device directly if you supply it with the right magic
> numbers (which is helpful for development).
>
> However the final patch runs into the weeds because I don't yet have a
> clean way to represent in QOM the fixing of certain properties for the
> specialised classes.
>
> The series is a net reduction in code and an increase in
> documentation but obviously needs to iron out a few more warts. I'm
> open to suggestions on the best way to tweak the QOM stuff.
--device vhost-user-device is not really possible because vhost-user
devices are not full VIRTIO devices. vhost-user devices depend on
device-specific code in the VMM by design.
The "subset of a VIRTIO device" design made sense for vhost_net.
Nowadays there are other device types that are close to full VIRTIO
devices, although the vhost-user protocol doesn't support the full
VIRTIO device lifecycle.
I think a user-creatable --device vhost-user-device is not a good idea
today. It creates confusion. Many people aren't aware of the
architectural difference between vhost-user and VIRTIO devices. The
result is that VMMs and vhost-user backends implement increasingly
brittle VIRTIO configuration space and feature bit logic as they
knowingly or unknowingly try to paper over the fact that a traditional
vhost-user device isn't a full VIRTIO device.
It is possible to resolve this difference and make --device
vhost-user-device work properly for devices that want to be full
VIRTIO devices. See "Making VMM device shims optional" here:
https://blog.vmsplice.net/2020/09/on-unifying-vhost-user-and-virtio.html
Even after extending the vhost-user protocol to solve the current
limitations, existing backends would still only be partial VIRTIO
devices that wouldn't work with --device vhost-user-device.
Reducing boilerplate is helpful, but I think --device
vhost-user-device should not be user-creatable.
Stefan
>
> Alex.
>
> Alex Bennée (12):
> hw/virtio: fix typo in VIRTIO_CONFIG_IRQ_IDX comments
> include/hw/virtio: document virtio_notify_config
> include/hw/virtio: add kerneldoc for virtio_init
> include/hw/virtio: document some more usage of notifiers
> virtio: add generic vhost-user-device
> virtio: add PCI stub for vhost-user-device
> include: attempt to document device_class_set_props
> qom: allow for properties to become "fixed"
> hw/virtio: derive vhost-user-rng from vhost-user-device
> hw/virtio: add config support to vhost-user-device
> hw/virtio: derive vhost-user-gpio from vhost-user-device (!BROKEN)
> docs/system: add a basic enumeration of vhost-user devices
>
> docs/system/devices/vhost-user-rng.rst | 2 +
> docs/system/devices/vhost-user.rst | 41 +++
> qapi/qom.json | 2 +
> include/hw/qdev-core.h | 9 +
> include/hw/virtio/vhost-user-device.h | 33 ++
> include/hw/virtio/vhost-user-gpio.h | 23 +-
> include/hw/virtio/vhost-user-rng.h | 11 +-
> include/hw/virtio/virtio.h | 21 ++
> include/qom/object.h | 16 +-
> hw/display/vhost-user-gpu.c | 4 +-
> hw/net/virtio-net.c | 4 +-
> hw/virtio/vhost-user-device-pci.c | 71 +++++
> hw/virtio/vhost-user-device.c | 359 ++++++++++++++++++++++
> hw/virtio/vhost-user-fs.c | 4 +-
> hw/virtio/vhost-user-gpio.c | 405 +------------------------
> hw/virtio/vhost-user-rng.c | 264 +---------------
> hw/virtio/vhost-vsock-common.c | 4 +-
> hw/virtio/virtio-crypto.c | 4 +-
> qom/object.c | 14 +
> qom/object_interfaces.c | 9 +-
> qom/qom-qmp-cmds.c | 1 +
> softmmu/qdev-monitor.c | 1 +
> hw/virtio/meson.build | 3 +
> 23 files changed, 613 insertions(+), 692 deletions(-)
> create mode 100644 include/hw/virtio/vhost-user-device.h
> create mode 100644 hw/virtio/vhost-user-device-pci.c
> create mode 100644 hw/virtio/vhost-user-device.c
>
> --
> 2.39.2
>
>