Some minor details from the Xen side of things: On 29.04.26 15:52, Val Packett wrote:
The experimental virtio-mmio support for Xen was initially developed on aarch64, so device trees were used to configure the mmio devices, with arbitrary vGIC interrupts used by the hypervisor. On x86_64 however, the only reasonable way to interrupt the guest is over Xen event channels, which can only be acquired by children of xenbus,
More exact: interdomain event channels need to be connected to a xenbus device. But you are needing those, so for your use case the above statement is correct.
the virtual bus driven by Xen's configuration database, XenStore. It is also a more convenient and "Xen-ish" way to provision devices. Implement a xenbus client for virtio-mmio which negotiates an event channel and provides it as a platform IRQ to the virtio-mmio driver. Signed-off-by: Val Packett <[email protected]> --- Hi, I've been working on porting virtio-mmio support from Arm to x86_64, with the goal of running vhost-user-gpu to power Wayland/GPU integration for Qubes OS. (I'm aware of various proposals for alternative virtio transports but virtio-mmio seems to be the only one that *is* upstream already and just Works..) Setting up virtio-mmio through xenbus, initially motivated just by event channels being the only real way to get interrupts working on HVM, turned out to generally be quite pleasant and nice :) I'd like to get some early feedback for this patch, particularly the general stuff: * is this whole thing acceptable in general? * should it be extracted into a different file? * (from the Xen side) any input on the xenstore keys, what goes where?
You should add some documentation in the Xen source tree regarding the Xenstore keys (see docs/misc/xenstore-paths.pandoc there).
* anything else to keep in mind? It does seem simple enough, so hopefully this can be done? The corresponding userspace-side WIP is available at: https://github.com/QubesOS/xen-vhost-frontend And the required DMOP for firing the evtchn events will be sent to xen-devel shortly as well. Thanks, ~val --- drivers/virtio/Kconfig | 7 ++ drivers/virtio/virtio_mmio.c | 177 ++++++++++++++++++++++++++++++++++- 2 files changed, 183 insertions(+), 1 deletion(-) diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig index ce5bc0d9ea28..56bc2b10526b 100644 --- a/drivers/virtio/Kconfig +++ b/drivers/virtio/Kconfig @@ -171,6 +171,13 @@ config VIRTIO_MMIO_CMDLINE_DEVICESIf unsure, say 'N'. +config VIRTIO_MMIO_XENBUS+ bool "Memory mapped virtio devices parameter parsing" + depends on VIRTIO_MMIO && XEN + select XEN_XENBUS_FRONTEND + help + Allow virtio-mmio devices instantiation for Xen guests via xenbus. + config VIRTIO_DMA_SHARED_BUFFER tristate depends on DMA_SHARED_BUFFER diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c index 595c2274fbb5..32295284bdbf 100644 --- a/drivers/virtio/virtio_mmio.c +++ b/drivers/virtio/virtio_mmio.c @@ -70,6 +70,11 @@ #include <uapi/linux/virtio_mmio.h> #include <linux/virtio_ring.h>+#ifdef CONFIG_VIRTIO_MMIO_XENBUS+#include <xen/xen.h> +#include <xen/xenbus.h> +#include <xen/events.h> +#endif/* The alignment to use between consumer and producer parts of vring.@@ -810,13 +815,183 @@ static struct platform_driver virtio_mmio_driver = { }, };+#ifdef CONFIG_VIRTIO_MMIO_XENBUS+struct virtio_mmio_xen_info { + struct resource resources[2]; + unsigned int evtchn; + struct platform_device *pdev; +}; + +static int virtio_mmio_xen_probe(struct xenbus_device *dev, + const struct xenbus_device_id *id) +{ + int err; + long long base, size; + char *mem; + struct virtio_mmio_xen_info *info; + struct xenbus_transaction xbt; + + /* TODO: allocate an unused address here and pass it to the host instead */
Indeed.
+ err = xenbus_scanf(XBT_NIL, dev->otherend, "base", "0x%llx",
+ &base);
+ if (err < 0) {
+ xenbus_dev_fatal(dev, err, "reading base");
+ return -EINVAL;
+ }
+
+ mem = xenbus_read(XBT_NIL, dev->otherend, "size", NULL);
+ if (XENBUS_IS_ERR_READ(mem))
+ return PTR_ERR(mem);
+ size = memparse(mem, NULL);
+ kfree(mem);
+
+ info = kzalloc_obj(*info);
+ if (!info) {
+ xenbus_dev_fatal(dev, -ENOMEM, "allocating info structure");
+ return -ENOMEM;
+ }
+
+ info->resources[0].flags = IORESOURCE_MEM;
+ info->resources[0].start = base;
+ info->resources[0].end = base + size - 1;
+
+ err = xenbus_alloc_evtchn(dev, &info->evtchn);
+ if (err) {
+ xenbus_dev_fatal(dev, err, "xenbus_alloc_evtchn");
+ goto error_info;
+ }
+
+ err = bind_evtchn_to_irq(info->evtchn);
+ if (err <= 0) {
+ xenbus_dev_fatal(dev, err, "bind_evtchn_to_irq");
+ goto error_evtchan;
+ }
+
+ info->resources[1].flags = IORESOURCE_IRQ;
+ info->resources[1].start = info->resources[1].end = err;
+
+again:
+ err = xenbus_transaction_start(&xbt);
No need to use a Xenstore transaction here. The written node(s) are regarded to be valid only after calling xenbus_switch_state() to set the frontend state to XenbusStateInitialised.
+ if (err) {
+ xenbus_dev_fatal(dev, err, "starting transaction");
+ goto error_irq;
+ }
+
+ err = xenbus_printf(xbt, dev->nodename, "event-channel", "%u",
+ info->evtchn);
With allocation of the base address you'd want to write it to another node, of course.
+ if (err) {
+ xenbus_transaction_end(xbt, 1);
+ xenbus_dev_fatal(dev, err, "%s", "writing event-channel");
+ goto error_irq;
+ }
+
+ err = xenbus_transaction_end(xbt, 0);
+ if (err) {
+ if (err == -EAGAIN)
+ goto again;
+ xenbus_dev_fatal(dev, err, "completing transaction");
+ goto error_irq;
+ }
+
+ dev_set_drvdata(&dev->dev, info);
+ xenbus_switch_state(dev, XenbusStateInitialised);
+ return 0;
+
+error_irq:
+ unbind_from_irqhandler(info->resources[1].start, info);
+error_evtchan:
+ xenbus_free_evtchn(dev, info->evtchn);
+error_info:
+ kfree(info);
+
+ return err;
+}
+
+static void virtio_mmio_xen_backend_changed(struct xenbus_device *dev,
+ enum xenbus_state backend_state)
+{
+ struct virtio_mmio_xen_info *info = dev_get_drvdata(&dev->dev);
+
+ switch (backend_state) {
+ case XenbusStateInitialising:
+ case XenbusStateInitWait:
+ case XenbusStateInitialised:
+ case XenbusStateReconfiguring:
+ case XenbusStateReconfigured:
+ case XenbusStateUnknown:
+ break;
+
+ case XenbusStateConnected:
+ if (dev->state != XenbusStateInitialised) {
+ dev_warn(&dev->dev, "state %d on connect", dev->state);
+ break;
+ }
+ info->pdev = platform_device_register_resndata(&dev->dev,
+ "virtio-mmio", PLATFORM_DEVID_AUTO,
+ info->resources, ARRAY_SIZE(info->resources),
NULL, 0);
+ xenbus_switch_state(dev, XenbusStateConnected);
+ break;
+
+ case XenbusStateClosed:
+ if (dev->state == XenbusStateClosed)
+ break;
+ fallthrough; /* Missed the backend's Closing state. */
+ case XenbusStateClosing:
+ platform_device_unregister(info->pdev);
+ xenbus_switch_state(dev, XenbusStateClosed);
+ break;
+
+ default:
+ xenbus_dev_fatal(dev, -EINVAL, "saw state %d at frontend",
+ backend_state);
+ break;
+ }
+}
+
+static void virtio_mmio_xen_remove(struct xenbus_device *dev)
+{
+ struct virtio_mmio_xen_info *info = dev_get_drvdata(&dev->dev);
+
+ kfree(info);
+ dev_set_drvdata(&dev->dev, NULL);
+}
+
+static const struct xenbus_device_id virtio_mmio_xen_ids[] = {
+ { "virtio" },
Please use "virtio-mmio" here, as I could imagine "virtio-pci" devices, too. Juergen
+ { "" }, +}; + +static struct xenbus_driver virtio_mmio_xen_driver = { + .ids = virtio_mmio_xen_ids, + .probe = virtio_mmio_xen_probe, + .otherend_changed = virtio_mmio_xen_backend_changed, + .remove = virtio_mmio_xen_remove, +}; +#endif + static int __init virtio_mmio_init(void) { - return platform_driver_register(&virtio_mmio_driver); + int ret; + + ret = platform_driver_register(&virtio_mmio_driver); + if (ret) + return ret; + +#ifdef CONFIG_VIRTIO_MMIO_XENBUS + if (xen_domain()) + ret = xenbus_register_frontend(&virtio_mmio_xen_driver); +#endif + + return ret; }static void __exit virtio_mmio_exit(void){ +#ifdef CONFIG_VIRTIO_MMIO_XENBUS + if (xen_domain()) + xenbus_unregister_driver(&virtio_mmio_xen_driver); +#endif + platform_driver_unregister(&virtio_mmio_driver); vm_unregister_cmdline_devices(); }
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
