On Wed, Mar 09 2022, Alex Bennée <alex.ben...@linaro.org> wrote: > While writing my own VirtIO devices I've gotten confused with how > things are structured and what sort of shared infrastructure there is. > If we can document how everything is supposed to work we can then > maybe start cleaning up inconsistencies in the code.
I agree that we could use some documentation here; OTOH, I'm a bit confused in turn by your patch :) Let me comment below. > > Based-on: 20220309135355.4149689-1-alex.ben...@linaro.org > Signed-off-by: Alex Bennée <alex.ben...@linaro.org> > Cc: Stefan Hajnoczi <stefa...@redhat.com> > Cc: "Michael S. Tsirkin" <m...@redhat.com> > Cc: Gerd Hoffmann <kra...@redhat.com> > Cc: Marc-André Lureau <marcandre.lur...@redhat.com> > Cc: Viresh Kumar <viresh.ku...@linaro.org> > Cc: Mathieu Poirier <mathieu.poir...@linaro.org> > Cc: Dr. David Alan Gilbert <dgilb...@redhat.com> > --- > docs/devel/index-internals.rst | 2 +- > docs/devel/virtio-backends.rst | 139 +++++++++++++++++++++++++++++++++ > 2 files changed, 140 insertions(+), 1 deletion(-) > create mode 100644 docs/devel/virtio-backends.rst (...) > diff --git a/docs/devel/virtio-backends.rst b/docs/devel/virtio-backends.rst > new file mode 100644 > index 0000000000..230538f46b > --- /dev/null > +++ b/docs/devel/virtio-backends.rst > @@ -0,0 +1,139 @@ > +.. > + Copyright (c) 2022, Linaro Limited > + Written by Alex Bennée > + > +Writing VirtIO backends for QEMU > +================================ > + > +This document attempts to outline the information a developer needs to > +know to write backends for QEMU. It is specifically focused on > +implementing VirtIO devices. I think you first need to define a bit more clearly what you consider a "backend". For virtio, it is probably "everything a device needs to function as a specific device type like net, block, etc., which may be implemented by different methods" (as you describe further below). > + > +Front End Transports > +-------------------- > + > +VirtIO supports a number of different front end transports. The > +details of the device remain the same but there are differences in > +command line for specifying the device (e.g. -device virtio-foo > +and -device virtio-foo-pci). For example: > + > +.. code:: c > + > + static const TypeInfo vhost_user_blk_info = { > + .name = TYPE_VHOST_USER_BLK, > + .parent = TYPE_VIRTIO_DEVICE, > + .instance_size = sizeof(VHostUserBlk), > + .instance_init = vhost_user_blk_instance_init, > + .class_init = vhost_user_blk_class_init, > + }; > + > +defines ``TYPE_VHOST_USER_BLK`` as a child of the generic > +``TYPE_VIRTIO_DEVICE``. That's not what I'd consider a "front end", though? > And then for the PCI device it wraps around the > +base device (although explicitly initialising via > +virtio_instance_init_common): > + > +.. code:: c > + > + struct VHostUserBlkPCI { > + VirtIOPCIProxy parent_obj; > + VHostUserBlk vdev; > + }; The VirtIOPCIProxy seems to materialize a bit out of thin air here... maybe the information simply needs to be structured in a different way? Perhaps: - describe that virtio devices consist of a part that implements the device functionality, which ultimately derives from VirtIODevice (the "backend"), and a part that exposes a way for the operating system to discover and use the device (the "frontend", what the virtio spec calls a "transport") - decribe how the "frontend" part works (maybe mention VirtIOPCIProxy, VirtIOMMIOProxy, and VirtioCcwDevice as specialized proxy devices for PCI, MMIO, and CCW devices) - list the different types of "backends" (as you did below), and give two examples of how VirtIODevice is extended (a plain one, and a vhost-user one) - explain how frontend and backend together create an actual device (with the two device examples, and maybe also with the plain one plugged as both PCI and CCW?); maybe also mention that MMIO is a bit different? (it always confuses me) > + > + static void vhost_user_blk_pci_instance_init(Object *obj) > + { > + VHostUserBlkPCI *dev = VHOST_USER_BLK_PCI(obj); > + > + virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev), > + TYPE_VHOST_USER_BLK); > + object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev), > + "bootindex"); > + } > + > + static const VirtioPCIDeviceTypeInfo vhost_user_blk_pci_info = { > + .base_name = TYPE_VHOST_USER_BLK_PCI, > + .generic_name = "vhost-user-blk-pci", > + .transitional_name = "vhost-user-blk-pci-transitional", > + .non_transitional_name = "vhost-user-blk-pci-non-transitional", > + .instance_size = sizeof(VHostUserBlkPCI), > + .instance_init = vhost_user_blk_pci_instance_init, > + .class_init = vhost_user_blk_pci_class_init, > + }; > + > +Back End Implementations > +------------------------ > + > +There are a number of places where the implementation of the backend > +can be done: > + > +* in QEMU itself > +* in the host kernel (a.k.a vhost) > +* in a separate process (a.k.a. vhost-user) > + > +where a vhost-user implementation is being done the code in QEMU is > +mainly boilerplate to handle the command line definition and > +connection to the separate process with a socket (using the ``chardev`` > +functionality). > + > +Implementing a vhost-user wrapper > +--------------------------------- > + > +There are some classes defined that can wrap a lot of the common > +vhost-user code in a ``VhostUserBackend``. For example: Is VhostUserBackend something that is expected to be commonly used? I think gpu and input use it, but not virtiofs (unless I misread the code). > + > +.. code:: c > + > + struct VhostUserGPU { > + VirtIOGPUBase parent_obj; > + > + VhostUserBackend *vhost; > + ... > + }; > + > + static void > + vhost_user_gpu_instance_init(Object *obj) > + { > + VhostUserGPU *g = VHOST_USER_GPU(obj); > + > + g->vhost = VHOST_USER_BACKEND(object_new(TYPE_VHOST_USER_BACKEND)); > + object_property_add_alias(obj, "chardev", > + OBJECT(g->vhost), "chardev"); > + } > + > + static void > + vhost_user_gpu_device_realize(DeviceState *qdev, Error **errp) > + { > + VhostUserGPU *g = VHOST_USER_GPU(qdev); > + VirtIODevice *vdev = VIRTIO_DEVICE(g); > + > + vhost_dev_set_config_notifier(&g->vhost->dev, &config_ops); > + if (vhost_user_backend_dev_init(g->vhost, vdev, 2, errp) < 0) { > + return; > + } > + ... > + } > + > + static void > + vhost_user_gpu_class_init(ObjectClass *klass, void *data) > + { > + DeviceClass *dc = DEVICE_CLASS(klass); > + VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass); > + > + vdc->realize = vhost_user_gpu_device_realize; > + ... > + } > + > + static const TypeInfo vhost_user_gpu_info = { > + .name = TYPE_VHOST_USER_GPU, > + .parent = TYPE_VIRTIO_GPU_BASE, > + .instance_size = sizeof(VhostUserGPU), > + .instance_init = vhost_user_gpu_instance_init, > + .class_init = vhost_user_gpu_class_init, > + ... > + }; > + > +Here the ``TYPE_VHOST_USER_GPU`` is based off a shared base class > +(``TYPE_VIRTIO_GPU_BASE`` which itself is based on > +``TYPE_VIRTIO_DEVICE``). The chardev property is aliased to the > +VhostUserBackend chardev so it can be specified on the command line > +for this device. > + I think using a "base" device is something that is device-specific; for gpu, it makes sense as it can be implemented in different ways, but e.g. virtiofs does not have a "plain" implementation, and some device types have only "plain" implementations.