Re: [RFC PATCH] virtio-mmio: add xenbus probing

[Teddy Astie <teddy.astie@xxxxxxxxxx>]

Hello,

Le 29/04/2026 à 16:18, Val Packett a écrit :
> 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,
> 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 <val@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> 
> 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 :)

Is it HVM specific, or can we also make it work for PVH (we can actually 
attach a ioreq server to PVH guests) ?

> 
> 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?
> * 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.

Could that be done through evtchn_send (or its userland counterpart) ?

> 
> 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_DEVICES
>   
>        If unsure, say 'N'.
>   
> +config VIRTIO_MMIO_XENBUS
> +     bool "Memory mapped virtio devices parameter parsing"

that text seems to miss the xenbus aspect

> +     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 */
> +     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);
> +     if (err) {
> +             xenbus_dev_fatal(dev, err, "starting transaction");
> +             goto error_irq;
> +     }
> +
> +     err = xenbus_printf(xbt, dev->nodename, "event-channel", "%u",
> +                         info->evtchn);
> +     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;
> +     }
> +}
> +

In some way, we're defining a new "PV driver" which is a virtio-mmio 
one, I guess we can eventually specific some form of protocol that 
backend/frontend would need to follow ?

> +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" },
> +     { "" },
> +};
> +
> +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();
>   }



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech