[Libguestfs] [PATCH] aarch64: Enable virtio-pci, replacing virtio-mmio.
Laine Stump
laine at laine.org
Mon Oct 10 18:15:32 UTC 2016
On 10/10/2016 11:47 AM, Richard W.M. Jones wrote:
> This patch causes aarch64 to use virtio-pci instead of virtio-mmio.
> Virtio-pci is considerably faster than virtio-mmio, it's more like how
> other architectures work, and it supports hotplugging (although it's
> not likely we'd use the latter feature).
>
> I'm not necessarily suggesting that we apply this. Laine (CC'd) has
> some further patches to libvirt lined up which AIUI would make it
> simpler to do this.
Since you're placing the devices directly on pcie-root, my libvirt
patches won't make it any easier. (If, on the other hand, you were
willing to have each device on its own pcie-root-port, then my patches
would mean that you wouldn't have to do anything special at all). So I
don't see a problem with pushing this. (It's possible that in the future
we might further simplify by making it possible to specify "bus='0'"
without needing to specify exactly which slot, but that's very iffy, and
not likely to happen soon even if it does)
BTW, you can use *exactly* the same device config for a Q35 machine.
On 10/10/2016 11:47 AM, Richard W.M. Jones wrote:
> Thanks: Laine Stump, Andrea Bolognani, Marcel Apfelbaum.
> ---
> src/guestfs-internal.h | 6 +++---
> src/launch-libvirt.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 44 insertions(+), 3 deletions(-)
>
> diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
> index d437b9a..428da7f 100644
> --- a/src/guestfs-internal.h
> +++ b/src/guestfs-internal.h
> @@ -137,17 +137,17 @@
> /* Differences in device names on ARM (virtio-mmio) vs normal
> * hardware with PCI.
> */
> -#if !defined(__arm__) && !defined(__aarch64__)
> +#if !defined(__arm__)
> #define VIRTIO_BLK "virtio-blk-pci"
> #define VIRTIO_SCSI "virtio-scsi-pci"
> #define VIRTIO_SERIAL "virtio-serial-pci"
> #define VIRTIO_NET "virtio-net-pci"
> -#else /* ARM */
> +#else /* ARMv7 */
> #define VIRTIO_BLK "virtio-blk-device"
> #define VIRTIO_SCSI "virtio-scsi-device"
> #define VIRTIO_SERIAL "virtio-serial-device"
> #define VIRTIO_NET "virtio-net-device"
> -#endif /* ARM */
> +#endif /* ARMv7 */
>
> /* Machine types. */
> #ifdef __arm__
> diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
> index d8479dc..1ac5604 100644
> --- a/src/launch-libvirt.c
> +++ b/src/launch-libvirt.c
> @@ -950,6 +950,13 @@ static int construct_libvirt_xml_disk_source_hosts (guestfs_h *g, xmlTextWriterP
> static int construct_libvirt_xml_disk_source_seclabel (guestfs_h *g, const struct backend_libvirt_data *data, xmlTextWriterPtr xo);
> static int construct_libvirt_xml_appliance (guestfs_h *g, const struct libvirt_xml_params *params, xmlTextWriterPtr xo);
>
> +static int construct_libvirt_xml_virtio_pci_address (guestfs_h *g, xmlTextWriterPtr xo, int slot);
> +/* Don't use slot 1, since can be used by video. */
> +#define VIRTIO_PCI_SLOT_RNG 2
> +#define VIRTIO_PCI_SLOT_SCSI 3
> +#define VIRTIO_PCI_SLOT_SERIAL 4
> +#define VIRTIO_PCI_SLOT_NETWORK 5
> +
> /* These macros make it easier to write XML, but they also make a lot
> * of assumptions:
> *
> @@ -1337,6 +1344,9 @@ construct_libvirt_xml_devices (guestfs_h *g,
> attribute ("model", "random");
> string ("/dev/urandom");
> } end_element ();
> + if (construct_libvirt_xml_virtio_pci_address (g, xo,
> + VIRTIO_PCI_SLOT_RNG) == -1)
> + return -1;
> } end_element ();
> }
>
> @@ -1345,6 +1355,9 @@ construct_libvirt_xml_devices (guestfs_h *g,
> attribute ("type", "scsi");
> attribute ("index", "0");
> attribute ("model", "virtio-scsi");
> + if (construct_libvirt_xml_virtio_pci_address (g, xo,
> + VIRTIO_PCI_SLOT_SCSI) == -1)
> + return -1;
> } end_element ();
>
> /* Disks. */
> @@ -1382,6 +1395,9 @@ construct_libvirt_xml_devices (guestfs_h *g,
> attribute ("type", "virtio");
> attribute ("name", "org.libguestfs.channel.0");
> } end_element ();
> + if (construct_libvirt_xml_virtio_pci_address (g, xo,
> + VIRTIO_PCI_SLOT_SERIAL) == -1)
> + return -1;
> } end_element ();
>
> /* Connect to libvirt bridge (see: RHBZ#1148012). */
> @@ -1394,6 +1410,9 @@ construct_libvirt_xml_devices (guestfs_h *g,
> start_element ("model") {
> attribute ("type", "virtio");
> } end_element ();
> + if (construct_libvirt_xml_virtio_pci_address (g, xo,
> + VIRTIO_PCI_SLOT_NETWORK) == -1)
> + return -1;
> } end_element ();
> }
>
> @@ -1986,6 +2005,28 @@ find_secret (guestfs_h *g,
> return 0;
> }
>
> +/**
> + * On aarch64 only, to force libvirt to use virtio-pci instead of
> + * virtio-mmio, we assign every virtio device to a unique function
s/function/slot/
> + * within the (implicitly created) pcie-root bus. Every virtio device
> + * must have a unique slot number.
Not exactly true. It would also be possible to have multiple devices on
the same slot, as long as each was on a different function within that
slot (the reason it didn't work when you tried to assign them to
different functions on the same slot was that you were using slot 0 of
pcie-root, which is reserved. If you had used slot='1' that would have
worked to.
So anyway, if you wanted to be pendantic, you could say that each device
must have a unique address, and that you make the addresses unique by
giving each a different slot.
If I knew the libguestfs code at all, I would ACK this. Since I don't,
I'll give an ACK to the resulting config, for whatever that's worth :-)
> + */
> +static int
> +construct_libvirt_xml_virtio_pci_address (guestfs_h *g,
> + xmlTextWriterPtr xo,
> + int slot)
> +{
> +#if defined(__aarch64__)
> + start_element ("address") {
> + attribute ("type", "pci");
> + attribute ("bus", "0");
> + attribute_format ("slot", "%d", slot);
> + } end_element ();
> +#endif
> +
> + return 0;
> +}
> +
> static int
> is_blk (const char *path)
> {
More information about the Libguestfs
mailing list