[libvirt] [PATCHv3] Add invariant TSC cpu flag

Add suport for invariant TSC flag (CPUID 0x80000007, bit 8 of EDX). If this flag is enabled, the TSC ticks at a constant rate across all ACPI P-, C- and T-states. This can be enabled by adding: <feature name='invtsc'/> to the <cpu> element. Migration and saving the domain does not work with this flag. QEMU support for this is not merged yet: https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg05024.html The feature name "invtsc" differs from the name "" used by the linux kernel: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/x8... --- v1: https://www.redhat.com/archives/libvir-list/2014-May/msg00183.html v2: https://www.redhat.com/archives/libvir-list/2014-May/msg00297.html add it as a cpu flag instead of a timer v3: check if the feature wasn't filtered out on domain startup src/cpu/cpu_map.xml | 5 +++++ src/qemu/qemu_migration.c | 14 ++++++++++++++ src/qemu/qemu_process.c | 15 +++++++++++++++ 3 files changed, 34 insertions(+) diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index 7d34d40..ffaeb92 100644 --- a/src/cpu/cpu_map.xml +++ b/src/cpu/cpu_map.xml @@ -327,6 +327,11 @@ <cpuid function='0x00000007' ebx='0x00100000'/> </feature> + <!-- Advanced Power Management edx features --> + <feature name='invtsc'> + <cpuid function='0x80000007' edx='0x00000100'/> + </feature> + <!-- models --> <model name='486'> <feature name='fpu'/> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f0df1a6..7504a38 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1513,6 +1513,20 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, virDomainObjPtr vm, return false; } + for (i = 0; i < def->cpu->nfeatures; i++) { + virCPUFeatureDefPtr feature = &def->cpu->features[i]; + + if (feature->policy != VIR_CPU_FEATURE_REQUIRE) + continue; + + if (STREQ(feature->name, "invtsc")) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("domain has CPU feature: %s"), + feature->name); + return false; + } + } + return true; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a83780f..0824afe 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3568,6 +3568,7 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver, virDomainObjPtr vm) qemuDomainObjPrivatePtr priv = vm->privateData; int rc; bool ret = false; + size_t i; switch (arch) { case VIR_ARCH_I686: @@ -3590,6 +3591,20 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver, virDomainObjPtr vm) goto cleanup; } } + + for (i = 0; i < def->cpu->nfeatures; i++) { + virCPUFeatureDefPtr feature = &def->cpu->features[i]; + + if (feature->policy != VIR_CPU_FEATURE_REQUIRE) + continue; + + if (STREQ(feature->name, "invtsc") && + !cpuHasFeature(guestcpu, feature->name)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("host doesn't support invariant TSC")); + goto cleanup; + } + } break; default: -- 1.8.3.2

On Thu, May 15, 2014 at 10:31:05AM +0200, Ján Tomko wrote:
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f0df1a6..7504a38 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1513,6 +1513,20 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, virDomainObjPtr vm, return false; }
+ for (i = 0; i < def->cpu->nfeatures; i++) { + virCPUFeatureDefPtr feature = &def->cpu->features[i]; + + if (feature->policy != VIR_CPU_FEATURE_REQUIRE) + continue; + + if (STREQ(feature->name, "invtsc")) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("domain has CPU feature: %s"), + feature->name); + return false; + } + }
Could you add a comment describing why we forbid migration with this feature set. It probably isn't obvious to some random person reading this in the future :-) ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 06/02/2014 06:35 PM, Daniel P. Berrange wrote:
On Thu, May 15, 2014 at 10:31:05AM +0200, Ján Tomko wrote:
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f0df1a6..7504a38 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1513,6 +1513,20 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, virDomainObjPtr vm, return false; }
+ for (i = 0; i < def->cpu->nfeatures; i++) { + virCPUFeatureDefPtr feature = &def->cpu->features[i]; + + if (feature->policy != VIR_CPU_FEATURE_REQUIRE) + continue; + + if (STREQ(feature->name, "invtsc")) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("domain has CPU feature: %s"), + feature->name); + return false; + } + }
Could you add a comment describing why we forbid migration with this feature set. It probably isn't obvious to some random person reading this in the future :-)
I've added this comment locally: diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3857c09..36ee718 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1519,6 +1519,7 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, virDomainObjPtr vm, if (feature->policy != VIR_CPU_FEATURE_REQUIRE) continue; + /* QEMU blocks migration and save with invariant TSC enabled */ if (STREQ(feature->name, "invtsc")) { virReportError(VIR_ERR_OPERATION_INVALID, _("domain has CPU feature: %s"),
ACK
As Eric pointed out, QEMU support is only merged in the maintainer's qom-cpu branch: https://github.com/afaerber/qemu-cpu/tree/qom-cpu I'll push this after it gets merged in qemu.git master. Thanks for the review. Jan

On 05/15/2014 02:31 AM, Ján Tomko wrote:
Add suport for invariant TSC flag (CPUID 0x80000007, bit 8 of EDX). If this flag is enabled, the TSC ticks at a constant rate across all ACPI P-, C- and T-states.
This can be enabled by adding: <feature name='invtsc'/> to the <cpu> element.
Migration and saving the domain does not work with this flag.
QEMU support for this is not merged yet: https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg05024.html
In spite of Dan's ack, let's wait until it actually lands in qemu.git before you push this. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Mon, Jun 02, 2014 at 02:17:05PM -0600, Eric Blake wrote:
On 05/15/2014 02:31 AM, Ján Tomko wrote:
Add suport for invariant TSC flag (CPUID 0x80000007, bit 8 of EDX). If this flag is enabled, the TSC ticks at a constant rate across all ACPI P-, C- and T-states.
This can be enabled by adding: <feature name='invtsc'/> to the <cpu> element.
Migration and saving the domain does not work with this flag.
QEMU support for this is not merged yet: https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg05024.html
In spite of Dan's ack, let's wait until it actually lands in qemu.git before you push this.
Doh, sorry, I missed that it wasn't merged in QEMU yet when going through old patches with no replies. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 06/02/2014 10:17 PM, Eric Blake wrote:
On 05/15/2014 02:31 AM, Ján Tomko wrote:
Add suport for invariant TSC flag (CPUID 0x80000007, bit 8 of EDX). If this flag is enabled, the TSC ticks at a constant rate across all ACPI P-, C- and T-states.
This can be enabled by adding: <feature name='invtsc'/> to the <cpu> element.
Migration and saving the domain does not work with this flag.
QEMU support for this is not merged yet: https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg05024.html
In spite of Dan's ack, let's wait until it actually lands in qemu.git before you push this.
I have pushed the patch now that it's merged in qemu.git: http://git.qemu.org/?p=qemu.git;a=commitdiff;h=303752a commit 303752a9068bfe84b9b05f1cd5ad5ff65b7f3ea6 Author: Marcelo Tosatti <mtosatti@redhat.com> AuthorDate: 2014-04-30 13:48:45 -0300 Commit: Andreas Färber <afaerber@suse.de> CommitDate: 2014-06-25 23:54:57 +0200 target-i386: Support "invariant tsc" flag Jan
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Ján Tomko