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

[John Ferlan]

On 03/27/2017 09:57 AM, Andrea Bolognani wrote:
> On Mon, 2017-03-27 at 08:03 -0400, John Ferlan wrote:
> [...]
>>> +    /* 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"
> 
> Good catch!
> 
>>> +     * 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".
> 
> Probably not, but I don't think having different
> arch-specific handling in the two code paths is a good
> idea: we should stay consistent, if anything not to confuse
> our future selves :)
> 
> [...]

That's fine...

>>> @@ -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.
> 
> HPET and KVM PIT are not firmware-related, though.
> 
> How about I move setting the arch based on the monitor to
> a separate virQEMUCapsInitQMPArch() and leave only setting
> the actual arch-dependent capabilities in this function?
> 

I think if "all" the lines were in a single API it would reduce the
chance that some future self would have to have to (or be told to) keep
this in sync with testUpdateQEMUCaps.

While PIT (Programmable? Interval Timer) and HPET (High Precision Event
Timer) aren't necessarily "firmware" type things they got lumped
together with the APCI and well are at least "related" to a degree...
One gsearch lands on http://wiki.osdev.org/HPET

Maybe "Firmware" is a poor selection of names on my part, but it was as
close as I could get.

John