[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH V7 1/2] libxl: Add support for Virtio disk configuration
On 08.04.22 20:21, Oleksandr Tyshchenko wrote: From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx> This patch adds basic support for configuring and assisting virtio-mmio based virtio-disk backend (emualator) which is intended to run out of Qemu and could be run in any domain. Although the Virtio block device is quite different from traditional Xen PV block device (vbd) from the toolstack's point of view: - as the frontend is virtio-blk which is not a Xenbus driver, nothing written to Xenstore are fetched by the frontend (the vdev is not passed to the frontend) I thought about the future support on x86. There we don't have a device tree (and I don't want to introduce it), so the only ways to specify the backend domain id would be to: - add some information to ACPI tables - use boot parameters - use Xenstore Thinking further of hotplugging virtio devices, Xenstore seems to be the only real suitable alternative. Using virtio mechanisms doesn't seem appropriate, as such information should be retrieved in "platform specific" ways (see e.g. specifying an "endpoint" in the virtio IOMMU device [1], [2]). I think the Xenstore information for that purpose could be rather minimal and it should be device-type agnostic. Having just a directory with endpoints and associated backend domids would probably be enough (not needed in this series, of course). This doesn't preclude the device tree variant you are using, as this would be required for dom0less systems anyway. OTOH I'd like you to modify the commit message to make it more clear that in future frontend data might be written to Xenstore in order to support other use cases. - the ring-ref/event-channel are not used for the backend<->frontend communication, the proposed IPC for Virtio is IOREQ/DM it is still a "block device" and ought to be integrated in existing "disk" handling. So, re-use (and adapt) "disk" parsing/configuration logic to deal with Virtio devices as well. For the immediate purpose and an ability to extend that support for other use-cases in future (Qemu, virtio-pci, etc) perform the following actions: - Add new disk backend type (LIBXL_DISK_BACKEND_OTHER) and reflect that in the configuration - Introduce new disk protocol field to libxl_device_disk struct (with LIBXL_DISK_PROTOCOL_XEN and LIBXL_DISK_PROTOCOL_VIRTIO_MMIO types) and reflect that in the configuration (new "protocol" option with "xen" protocol being default value) And with the hotplug option in mind I start to feel unueasy with naming the new Xenstore node "protocol", as the frontend disk nodes for "normal" disks already have a "protocol" entry specifying 64- or 32-bit protocol. Maybe we should really name it "transport" instead? - Add new device kind (LIBXL__DEVICE_KIND_VIRTIO_DISK) as current one (LIBXL__DEVICE_KIND_VBD) doesn't fit into Virtio disk model An example of domain configuration for Virtio disk: disk = [ 'phy:/dev/mmcblk0p3, xvda1, backendtype=other, protocol=virtio-mmio'] With Roger's feedback this would then be "transport=virtio", the "mmio" part should then be something like "adapter=mmio" (in contrast to "adapter=pci"), and "adapter" only needed in case of a device tree and PCI being available. Nothing has changed for default Xen disk configuration. Please note, this patch is not enough for virtio-disk to work on Xen (Arm), as for every Virtio device (including disk) we need to allocate Virtio MMIO params (IRQ and memory region) and pass them to the backend, also update Guest device-tree. The subsequent patch will add these missing bits. For the current patch, the default "irq" and "base" are just written to the Xenstore. This is not an ideal splitting, but this way we avoid breaking the bisectability. Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx> I'm fine with the overall approach and couldn't spot any real issues in the code. Juergen [1]: https://github.com/oasis-tcs/virtio-spec/blob/master/virtio-iommu.tex [2]: https://medium.com/@michael2012zhao_67085/virtio-iommu-789369049443 Attachment:
OpenPGP_0xB0DE9DD628BF132F.asc Attachment:
OpenPGP_signature
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |