[libvirt] [PATCH 1/4] qemu: Advertise ACPI support for aarch64 guests

[John Ferlan]

On 03/09/2017 12:37 PM, Andrea Bolognani wrote:
> So far, libvirt has assumed that only x86 supports ACPI,
> but that's inaccurate since aarch64 supports it too.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1429509
> ---
>  src/qemu/qemu_capabilities.c                       | 28 ++++++++++++++++------
>  .../caps_2.6.0-gicv2.aarch64.xml                   |  1 +
>  .../caps_2.6.0-gicv3.aarch64.xml                   |  1 +
>  3 files changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 5a3b4ac..4ec34f8 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -1038,13 +1038,17 @@ virQEMUCapsInitGuestFromBinary(virCapsPtr caps,
>  
>          machines = NULL;
>          nmachines = 0;
> +    }
>  
> +    if ((ARCH_IS_X86(guestarch) || guestarch == VIR_ARCH_AARCH64) &&
> +        virCapabilitiesAddGuestFeature(guest, "acpi", true, true) == NULL) {
> +        goto cleanup;
>      }
>  
>      if (ARCH_IS_X86(guestarch) &&
> -        (virCapabilitiesAddGuestFeature(guest, "acpi", true, true) == NULL ||
> -         virCapabilitiesAddGuestFeature(guest, "apic", true, false) == NULL))
> +        virCapabilitiesAddGuestFeature(guest, "apic", true, false) == NULL) {
>          goto cleanup;
> +    }
>  
>      if ((guestarch == VIR_ARCH_I686) &&
>          (virCapabilitiesAddGuestFeature(guest, "pae", true, false) == NULL ||
> @@ -4122,10 +4126,15 @@ virQEMUCapsInitHelp(virQEMUCapsPtr qemuCaps, uid_t runUid, gid_t runGid, const c
>                                  qmperr) < 0)
>          goto cleanup;
>  
> -    /* -no-acpi is not supported on non-x86
> -     * even if qemu reports it in -help */
> -    if (!ARCH_IS_X86(qemuCaps->arch))
> +    /* Older QEMU versions reported -no-acpi in the output of -help even
> +     * though it was not supported by the architecture. The issue has since
> +     * been fixed, but to maintain compatibility with all release we still

"releases"

> +     * need to filter out the capability for architectures that we know
> +     * don't support the feature, eg. anything but x86 and aarch64 */
> +    if (!ARCH_IS_X86(qemuCaps->arch) &&
> +        qemuCaps->arch != VIR_ARCH_AARCH64) {
>          virQEMUCapsClear(qemuCaps, QEMU_CAPS_NO_ACPI);
> +    }

Some day maybe we'll be able to stop parsing the help output. Still
makes me wonder is AARCH64 even support on those older versions and thus
is this necessary?  IDC either way as I suppose this is preventative or
"more complete".

>  
>      /* virQEMUCapsExtractDeviceStr will only set additional caps if qemu
>       * understands the 0.13.0+ notion of "-device driver,".  */
> @@ -4222,9 +4231,14 @@ virQEMUCapsInitArchQMPBasic(virQEMUCapsPtr qemuCaps,
>          goto cleanup;
>      }
>  
> -    /* ACPI/HPET/KVM PIT are x86 specific */
> -    if (ARCH_IS_X86(qemuCaps->arch)) {
> +    /* ACPI only works on x86 and aarch64 */
> +    if (ARCH_IS_X86(qemuCaps->arch) ||
> +        qemuCaps->arch == VIR_ARCH_AARCH64) {
>          virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_ACPI);
> +    }
> +
> +    /* HPET and KVM PIT are x86 specific */
> +    if (ARCH_IS_X86(qemuCaps->arch)) {
>          virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_HPET);
>          virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_KVM_PIT);
>      }

Considering on what's coming in patch 2, this would be better as a
virQEMUCapsSetFirmwareCaps? "utility" function...  That way the added
comments in both places referencing the other place could be dropped.

John

> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
> index 0aed651..b64a6f8 100644
> --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
> +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
> @@ -40,6 +40,7 @@
>    <flag name='no-shutdown'/>
>    <flag name='cache-unsafe'/>
>    <flag name='ich9-ahci'/>
> +  <flag name='no-acpi'/>
>    <flag name='fsdev-readonly'/>
>    <flag name='virtio-blk-pci.scsi'/>
>    <flag name='drive-copy-on-read'/>
> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
> index 1041a12..46e368f 100644
> --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
> +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
> @@ -40,6 +40,7 @@
>    <flag name='no-shutdown'/>
>    <flag name='cache-unsafe'/>
>    <flag name='ich9-ahci'/>
> +  <flag name='no-acpi'/>
>    <flag name='fsdev-readonly'/>
>    <flag name='virtio-blk-pci.scsi'/>
>    <flag name='drive-copy-on-read'/>
>