[libvirt] [PATCH 0/5] qemu: Unbreak aarch64/virt TCG guests
This issue was reported on the list a while ago, and now bug reports are popping up in Bugzilla as well. Since the proper fix (implementing the MSI controller for GICv3 in QEMU) has been confirmed to be a long way off, let's deal with it in libvirt. Andrea Bolognani (5): qemu: Use qemuDomainMachineIsVirt() more tests: Check default GIC version for aarch64/virt TCG guests qemu: Use GICv2 for aarch64/virt TCG guests gic: Remove VIR_GIC_VERSION_DEFAULT news: Update for GIC version on TCG changes docs/news.xml | 11 ++++++ src/qemu/qemu_capabilities.c | 7 +--- src/qemu/qemu_command.c | 6 +-- src/qemu/qemu_domain.c | 44 +++++++++++++++------- src/util/virgic.h | 3 -- .../qemuxml2argv-aarch64-gic-none-tcg.args | 19 ++++++++++ .../qemuxml2argv-aarch64-gic-none-tcg.xml | 17 +++++++++ tests/qemuxml2argvtest.c | 3 ++ .../qemuxml2xmlout-aarch64-gic-none-tcg.xml | 25 ++++++++++++ tests/qemuxml2xmltest.c | 1 + 10 files changed, 110 insertions(+), 26 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml -- 2.7.4
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_capabilities.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 71951e6..cf4dc74 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5824,12 +5824,7 @@ virQEMUCapsFillDomainFeatureGICCaps(virQEMUCapsPtr qemuCaps, virDomainCapsFeatureGICPtr gic = &domCaps->gic; virGICVersion version; - if (domCaps->arch != VIR_ARCH_ARMV7L && - domCaps->arch != VIR_ARCH_AARCH64) - return 0; - - if (STRNEQ(domCaps->machine, "virt") && - !STRPREFIX(domCaps->machine, "virt-")) + if (!qemuDomainMachineIsVirt(domCaps->machine, domCaps->arch)) return 0; for (version = VIR_GIC_VERSION_LAST - 1; -- 2.7.4
On Fri, May 12, 2017 at 16:14:43 +0200, Andrea Bolognani wrote:
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_capabilities.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
ACK
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- .../qemuxml2argv-aarch64-gic-none-tcg.args | 19 ++++++++++++++++ .../qemuxml2argv-aarch64-gic-none-tcg.xml | 17 +++++++++++++++ tests/qemuxml2argvtest.c | 3 +++ .../qemuxml2xmlout-aarch64-gic-none-tcg.xml | 25 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 5 files changed, 65 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args new file mode 100644 index 0000000..975a014 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args @@ -0,0 +1,19 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-aarch64 \ +-name guest \ +-S \ +-machine virt,accel=tcg,gic-version=3 \ +-cpu cortex-a57 \ +-m 1024 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 6ba410c5-1e5c-4d57-bee7-2228e7ffa32f \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-guest/monitor.sock,server,nowait \ +-no-acpi \ +-boot c diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.xml b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.xml new file mode 100644 index 0000000..0aa33db --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.xml @@ -0,0 +1,17 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>6ba410c5-1e5c-4d57-bee7-2228e7ffa32f</uuid> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom'> + <model>cortex-a57</model> + </cpu> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ce87938..8273725 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2251,6 +2251,9 @@ mymain(void) DO_TEST_GIC("aarch64-gic-none-both", GIC_BOTH, QEMU_CAPS_KVM, QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACH_VIRT_GIC_VERSION); + DO_TEST_GIC("aarch64-gic-none-tcg", GIC_BOTH, + QEMU_CAPS_MACHINE_OPT, + QEMU_CAPS_MACH_VIRT_GIC_VERSION); DO_TEST_GIC("aarch64-gic-default", GIC_NONE, QEMU_CAPS_KVM, QEMU_CAPS_MACHINE_OPT); DO_TEST_GIC("aarch64-gic-default", GIC_NONE, diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml new file mode 100644 index 0000000..69510e2 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml @@ -0,0 +1,25 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>6ba410c5-1e5c-4d57-bee7-2228e7ffa32f</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <gic version='3'/> + </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='allow'>cortex-a57</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 2dccde7..c75199e 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1070,6 +1070,7 @@ mymain(void) DO_TEST_FULL("aarch64-gic-none-v2", WHEN_BOTH, GIC_V2, NONE); DO_TEST_FULL("aarch64-gic-none-v3", WHEN_BOTH, GIC_V3, NONE); DO_TEST_FULL("aarch64-gic-none-both", WHEN_BOTH, GIC_BOTH, NONE); + DO_TEST_FULL("aarch64-gic-none-tcg", WHEN_BOTH, GIC_BOTH, NONE); DO_TEST_FULL("aarch64-gic-default", WHEN_BOTH, GIC_NONE, NONE); DO_TEST_FULL("aarch64-gic-default", WHEN_BOTH, GIC_V2, NONE); DO_TEST_FULL("aarch64-gic-default", WHEN_BOTH, GIC_V3, NONE); -- 2.7.4
On Fri, May 12, 2017 at 16:14:44 +0200, Andrea Bolognani wrote:
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- .../qemuxml2argv-aarch64-gic-none-tcg.args | 19 ++++++++++++++++ .../qemuxml2argv-aarch64-gic-none-tcg.xml | 17 +++++++++++++++ tests/qemuxml2argvtest.c | 3 +++ .../qemuxml2xmlout-aarch64-gic-none-tcg.xml | 25 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 5 files changed, 65 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml
ACK
There are currently some limitations in the emulated GICv3 that make it unsuitable as a default. Use GICv2 instead. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1450433 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_domain.c | 41 +++++++++++++++------- .../qemuxml2argv-aarch64-gic-none-tcg.args | 2 +- .../qemuxml2xmlout-aarch64-gic-none-tcg.xml | 2 +- 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cc02c80..31ed391 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2559,17 +2559,31 @@ qemuDomainDefEnableDefaultFeatures(virDomainDefPtr def, if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ABSENT && qemuDomainIsVirt(def)) { - VIR_DEBUG("Looking for usable GIC version in domain capabilities"); - for (version = VIR_GIC_VERSION_LAST - 1; - version > VIR_GIC_VERSION_NONE; - version--) { - if (virQEMUCapsSupportsGICVersion(qemuCaps, - def->virtType, - version)) { - VIR_DEBUG("Using GIC version %s", - virGICVersionTypeToString(version)); - def->gic_version = version; - break; + /* We want to use the highest available GIC version for guests; + * however, the emulated GICv3 is currently lacking a MSI controller, + * making it unsuitable for the pure PCIe topology we aim for. + * + * For that reason, we skip this step entirely for TCG guests, + * and rely on the code below to pick the default version, GICv2, + * which supports all the features we need. + * + * We'll want to revisit this once MSI support for GICv3 has been + * implemented in QEMU. + * + * See https://bugzilla.redhat.com/show_bug.cgi?id=1414081 */ + if (def->virtType == VIR_DOMAIN_VIRT_KVM) { + VIR_DEBUG("Looking for usable GIC version in domain capabilities"); + for (version = VIR_GIC_VERSION_LAST - 1; + version > VIR_GIC_VERSION_NONE; + version--) { + if (virQEMUCapsSupportsGICVersion(qemuCaps, + def->virtType, + version)) { + VIR_DEBUG("Using GIC version %s", + virGICVersionTypeToString(version)); + def->gic_version = version; + break; + } } } @@ -2580,8 +2594,11 @@ qemuDomainDefEnableDefaultFeatures(virDomainDefPtr def, /* Use the default GIC version if no version was specified */ if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ON && - def->gic_version == VIR_GIC_VERSION_NONE) + def->gic_version == VIR_GIC_VERSION_NONE) { + VIR_DEBUG("Using GIC version %s (default)", + virGICVersionTypeToString(VIR_GIC_VERSION_DEFAULT)); def->gic_version = VIR_GIC_VERSION_DEFAULT; + } } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args index 975a014..52b6996 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args @@ -7,7 +7,7 @@ QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-aarch64 \ -name guest \ -S \ --machine virt,accel=tcg,gic-version=3 \ +-machine virt,accel=tcg \ -cpu cortex-a57 \ -m 1024 \ -smp 1,sockets=1,cores=1,threads=1 \ diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml index 69510e2..a0cd0b7 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml @@ -9,7 +9,7 @@ <boot dev='hd'/> </os> <features> - <gic version='3'/> + <gic version='2'/> </features> <cpu mode='custom' match='exact' check='none'> <model fallback='allow'>cortex-a57</model> -- 2.7.4
On Fri, May 12, 2017 at 16:14:45 +0200, Andrea Bolognani wrote:
There are currently some limitations in the emulated GICv3 that make it unsuitable as a default. Use GICv2 instead.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1450433
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_domain.c | 41 +++++++++++++++------- .../qemuxml2argv-aarch64-gic-none-tcg.args | 2 +- .../qemuxml2xmlout-aarch64-gic-none-tcg.xml | 2 +- 3 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cc02c80..31ed391 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2559,17 +2559,31 @@ qemuDomainDefEnableDefaultFeatures(virDomainDefPtr def, if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ABSENT && qemuDomainIsVirt(def)) {
- VIR_DEBUG("Looking for usable GIC version in domain capabilities"); - for (version = VIR_GIC_VERSION_LAST - 1; - version > VIR_GIC_VERSION_NONE; - version--) { - if (virQEMUCapsSupportsGICVersion(qemuCaps, - def->virtType, - version)) { - VIR_DEBUG("Using GIC version %s", - virGICVersionTypeToString(version)); - def->gic_version = version; - break; + /* We want to use the highest available GIC version for guests; + * however, the emulated GICv3 is currently lacking a MSI controller, + * making it unsuitable for the pure PCIe topology we aim for. + * + * For that reason, we skip this step entirely for TCG guests, + * and rely on the code below to pick the default version, GICv2, + * which supports all the features we need. + * + * We'll want to revisit this once MSI support for GICv3 has been + * implemented in QEMU. + * + * See https://bugzilla.redhat.com/show_bug.cgi?id=1414081 */ + if (def->virtType == VIR_DOMAIN_VIRT_KVM) {
Currently it does not matter that much, since there are only two versions but this looks very non-future-proof to me. When qemu adds the feature you'll need to add a capability, where you also enable the code below for TCG guests. If there will be another version or something the condition will need to be altered. I'd rather see that v3 is specifically disqualified for TCG guests (which will be later relaxed using the capability.). That way you'll still run the detection process.
+ VIR_DEBUG("Looking for usable GIC version in domain capabilities"); + for (version = VIR_GIC_VERSION_LAST - 1; + version > VIR_GIC_VERSION_NONE; + version--) { + if (virQEMUCapsSupportsGICVersion(qemuCaps, + def->virtType, + version)) { + VIR_DEBUG("Using GIC version %s", + virGICVersionTypeToString(version)); + def->gic_version = version; + break; + } } }
On Mon, 2017-05-15 at 12:53 +0200, Peter Krempa wrote:
+ /* We want to use the highest available GIC version for guests; + * however, the emulated GICv3 is currently lacking a MSI controller, + * making it unsuitable for the pure PCIe topology we aim for. + * + * For that reason, we skip this step entirely for TCG guests, + * and rely on the code below to pick the default version, GICv2, + * which supports all the features we need. + * + * We'll want to revisit this once MSI support for GICv3 has been + * implemented in QEMU. + * + * See https://bugzilla.redhat.com/show_bug.cgi?id=1414081 */ + if (def->virtType == VIR_DOMAIN_VIRT_KVM) { Currently it does not matter that much, since there are only two versions but this looks very non-future-proof to me. When qemu adds the feature you'll need to add a capability, where you also enable the code below for TCG guests. If there will be another version or something the condition will need to be altered. I'd rather see that v3 is specifically disqualified for TCG guests (which will be later relaxed using the capability.). That way you'll still run the detection process.
Can do. I'll post a respin shortly. -- Andrea Bolognani / Red Hat / Virtualization
The QEMU default is GICv2, and some of the code in libvirt relies on the exact value. Stop pretending that's not the case and use GICv2 explicitly where needed. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_command.c | 6 +++--- src/qemu/qemu_domain.c | 7 +++---- src/util/virgic.h | 3 --- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 813a851..c2a9415 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7374,9 +7374,9 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, goto cleanup; } - /* The default GIC version should not be specified on the - * QEMU commandline for backwards compatibility reasons */ - if (def->gic_version != VIR_GIC_VERSION_DEFAULT) { + /* The default GIC version (GICv2) should not be specified on + * the QEMU commandline for backwards compatibility reasons */ + if (def->gic_version != VIR_GIC_VERSION_2) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACH_VIRT_GIC_VERSION)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 31ed391..2dae837 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2592,12 +2592,11 @@ qemuDomainDefEnableDefaultFeatures(virDomainDefPtr def, def->features[VIR_DOMAIN_FEATURE_GIC] = VIR_TRISTATE_SWITCH_ON; } - /* Use the default GIC version if no version was specified */ + /* Use the default GIC version (GICv2) if no version was specified */ if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ON && def->gic_version == VIR_GIC_VERSION_NONE) { - VIR_DEBUG("Using GIC version %s (default)", - virGICVersionTypeToString(VIR_GIC_VERSION_DEFAULT)); - def->gic_version = VIR_GIC_VERSION_DEFAULT; + VIR_DEBUG("Using GIC version 2 (default)"); + def->gic_version = VIR_GIC_VERSION_2; } } diff --git a/src/util/virgic.h b/src/util/virgic.h index 1c9efd6..2d77fdd 100644 --- a/src/util/virgic.h +++ b/src/util/virgic.h @@ -35,9 +35,6 @@ typedef enum { VIR_ENUM_DECL(virGICVersion); -/* Consider GIC v2 the default */ -# define VIR_GIC_VERSION_DEFAULT VIR_GIC_VERSION_2 - typedef enum { VIR_GIC_IMPLEMENTATION_NONE = 0, VIR_GIC_IMPLEMENTATION_KERNEL = (1 << 1), -- 2.7.4
On Fri, May 12, 2017 at 16:14:46 +0200, Andrea Bolognani wrote:
The QEMU default is GICv2, and some of the code in libvirt relies on the exact value. Stop pretending that's not the case and use GICv2 explicitly where needed.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_command.c | 6 +++--- src/qemu/qemu_domain.c | 7 +++---- src/util/virgic.h | 3 --- 3 files changed, 6 insertions(+), 10 deletions(-)
ACK
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 2f01449..4cf14b0 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -37,6 +37,17 @@ <section title="New features"> </section> <section title="Improvements"> + <change> + <summary> + qemu: Use GICv2 by default for aarch64/virt TCG guests + </summary> + <description> + The emulated GICv3 has some limitations that make it unusable as a + default; use GICv2 until they're sorted out. This change makes it + once again possible to run aarch64/virt guests on a x86_64 host + without having to tweak their configuration. + </description> + </change> </section> <section title="Bug fixes"> </section> -- 2.7.4
On Fri, May 12, 2017 at 16:14:47 +0200, Andrea Bolognani wrote:
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+)
ACK
participants (2)
-
Andrea Bolognani -
Peter Krempa