[libvirt] [RFC PATCH] Add support for invtsc timer
Not yet merged in upstream QEMU: https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg05024.html Add support for invariant TSC timer running at constant rate in all ACPI P-, C- and T-states. It can be enabled by specifying: <clock> <timer name='invtsc' present='yes'/> </clock> in the domain XML. Migration and saving the domain does not work with this timer. The support for this timer is indicated by bit 8 of EDX after calling CPUID with 0x80000007. It does not show up in /proc/cpuinfo [1] and since we're calling qemu without 'enforce', it doesn't error out if the host doesn't support this. Alternatively, we could expose it in libvirt as a cpu flag: <cpu mode='custom' match='exact'> <model fallback='forbid'>qemu64</model> <feature policy='require' name='invtsc'/> </cpu> or maybe add +invtsc to qemu args when the 'nonstop_tsc' flag is requested? [1]: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/x8... --- docs/formatdomain.html.in | 9 ++++++-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 6 +++-- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 6 +++++ src/qemu/qemu_migration.c | 14 ++++++++++++ .../qemuxml2argv-clock-timer-inv-tsc.args | 5 +++++ .../qemuxml2argv-clock-timer-inv-tsc.xml | 26 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 1 + 10 files changed, 66 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 4f19473..1d3fd93 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1368,13 +1368,18 @@ being modified, and can be one of "platform" (currently unsupported), "hpet" (libxl, xen, qemu), "kvmclock" (qemu), - "pit" (qemu), "rtc" (qemu), "tsc" (libxl) or "hypervclock" - (qemu - <span class="since">since 1.2.2</span>). + "pit" (qemu), "rtc" (qemu), "tsc" (libxl), "hypervclock" + (qemu - <span class="since">since 1.2.2</span>) or + "invtsc" (qemu - <span class="since">since 1.2.5</span>). The <code>hypervclock</code> timer adds support for the reference time counter and the reference page for iTSC feature for guests running the Microsoft Windows operating system. + + The <code>invtsc</code> timer adds support for the invariant + TSC. It runs at a constant rate in all ACPI P- C- and T-states. + A guest with this timer enabled cannot be migrated or saved. </dd> <dt><code>track</code></dt> <dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4249ed5..5154826 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -918,6 +918,7 @@ <choice> <value>kvmclock</value> <value>hypervclock</value> + <value>invtsc</value> </choice> </attribute> </group> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6c3bdad..893d904 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -706,7 +706,8 @@ VIR_ENUM_IMPL(virDomainTimerName, VIR_DOMAIN_TIMER_NAME_LAST, "hpet", "tsc", "kvmclock", - "hypervclock"); + "hypervclock", + "invtsc"); VIR_ENUM_IMPL(virDomainTimerTrack, VIR_DOMAIN_TIMER_TRACK_LAST, "boot", @@ -2931,7 +2932,8 @@ virDomainDefPostParseInternal(virDomainDefPtr def, virDomainTimerDefPtr timer = def->clock.timers[i]; if (timer->name == VIR_DOMAIN_TIMER_NAME_KVMCLOCK || - timer->name == VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK) { + timer->name == VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK || + timer->name == VIR_DOMAIN_TIMER_NAME_INVTSC) { if (timer->tickpolicy != -1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("timer %s doesn't support setting of " diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a92f0f3..53c02e6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1635,6 +1635,7 @@ enum virDomainTimerNameType { VIR_DOMAIN_TIMER_NAME_TSC, VIR_DOMAIN_TIMER_NAME_KVMCLOCK, VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK, + VIR_DOMAIN_TIMER_NAME_INVTSC, VIR_DOMAIN_TIMER_NAME_LAST }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6c1e17d..2994427 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6736,6 +6736,11 @@ qemuBuildCpuArgStr(virQEMUDriverPtr driver, have_cpu ? "" : default_model, timer->present ? '+' : '-'); have_cpu = true; + } else if (timer->name == VIR_DOMAIN_TIMER_NAME_INVTSC) { + virBufferAsprintf(&buf, "%s,%cinvtsc", + have_cpu ? "" : default_model, + timer->present ? '+' : '-'); + have_cpu = true; } else if (timer->name == VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK && timer->present) { virBufferAsprintf(&buf, "%s,hv_time", @@ -8063,6 +8068,7 @@ qemuBuildCommandLine(virConnectPtr conn, case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: + case VIR_DOMAIN_TIMER_NAME_INVTSC: /* Timers above are handled when building -cpu. */ case VIR_DOMAIN_TIMER_NAME_LAST: break; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index a9f7fea..c1ffc0f 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->clock.ntimers; i++) { + virDomainTimerDefPtr timer = def->clock.timers[i]; + + if (timer->present != 1) + continue; + + if (timer->name == VIR_DOMAIN_TIMER_NAME_INVTSC) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("domain has '%s' timer"), + virDomainTimerNameTypeToString(timer->name)); + return false; + } + } + return true; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.args b/tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.args new file mode 100644 index 0000000..ae74ae8 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.args @@ -0,0 +1,5 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/kvm -S -M pc \ +-cpu qemu32,+invtsc -m 214 -smp 6 \ +-nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -usb -net \ +none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.xml b/tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.xml new file mode 100644 index 0000000..b4a82e8 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.xml @@ -0,0 +1,26 @@ +<domain type='kvm'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>6</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <features> + <pae/> + </features> + <clock offset='utc'> + <timer name='invtsc' present='yes'/> + </clock> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/kvm</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 14482fd..3c5024e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -641,6 +641,7 @@ mymain(void) DO_TEST("cpu-host-kvmclock", QEMU_CAPS_ENABLE_KVM, QEMU_CAPS_CPU_HOST); DO_TEST("kvmclock", QEMU_CAPS_KVM); DO_TEST("clock-timer-hyperv-rtc", QEMU_CAPS_KVM); + DO_TEST("clock-timer-inv-tsc", QEMU_CAPS_KVM); DO_TEST("cpu-eoi-disabled", QEMU_CAPS_ENABLE_KVM); DO_TEST("cpu-eoi-enabled", QEMU_CAPS_ENABLE_KVM); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 3ea03e6..a659cd0 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -185,6 +185,7 @@ mymain(void) DO_TEST("clock-catchup"); DO_TEST("kvmclock"); DO_TEST("clock-timer-hyperv-rtc"); + DO_TEST("clock-timer-inv-tsc"); DO_TEST("cpu-eoi-disabled"); DO_TEST("cpu-eoi-enabled"); -- 1.8.3.2
On 05/06/14 15:27, Ján Tomko wrote:
Not yet merged in upstream QEMU: https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg05024.html
Add support for invariant TSC timer running at constant rate in all ACPI P-, C- and T-states.
It can be enabled by specifying: <clock> <timer name='invtsc' present='yes'/> </clock> in the domain XML.
Migration and saving the domain does not work with this timer.
The support for this timer is indicated by bit 8 of EDX after calling CPUID with 0x80000007. It does not show up in /proc/cpuinfo [1] and since we're calling qemu without 'enforce', it doesn't error out if the host doesn't support this.
Alternatively, we could expose it in libvirt as a cpu flag: <cpu mode='custom' match='exact'> <model fallback='forbid'>qemu64</model> <feature policy='require' name='invtsc'/> </cpu> or maybe add +invtsc to qemu args when the 'nonstop_tsc' flag is requested?
[1]: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/x8... --- docs/formatdomain.html.in | 9 ++++++-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 6 +++-- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 6 +++++ src/qemu/qemu_migration.c | 14 ++++++++++++ .../qemuxml2argv-clock-timer-inv-tsc.args | 5 +++++ .../qemuxml2argv-clock-timer-inv-tsc.xml | 26 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 1 + 10 files changed, 66 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.xml
...
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4249ed5..5154826 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -918,6 +918,7 @@ <choice> <value>kvmclock</value> <value>hypervclock</value> + <value>invtsc</value>
I'd prefer to change the name of the feature to "invarianttsc" in libvirt's representation, but that's just bikeshedding.
</choice> </attribute> </group>
...
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index a9f7fea..c1ffc0f 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->clock.ntimers; i++) { + virDomainTimerDefPtr timer = def->clock.timers[i]; + + if (timer->present != 1) + continue; + + if (timer->name == VIR_DOMAIN_TIMER_NAME_INVTSC) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("domain has '%s' timer"), + virDomainTimerNameTypeToString(timer->name)); + return false; + } + } + return true; }
It's a shame that this doesn't work across migration in a way HyperV has designed it. ACK once the qemu functionality will be released Peter
On Tue, May 06, 2014 at 03:27:20PM +0200, Ján Tomko wrote:
Not yet merged in upstream QEMU: https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg05024.html
Add support for invariant TSC timer running at constant rate in all ACPI P-, C- and T-states.
What does this do at the hardware level ? This doesn't seem to be creating a second TSC timer source in the guest, rather it is setting a property related to the existing TSC timer. So I think it might make more sense as an attribute for <timer name='tsc'> element instead.
It can be enabled by specifying: <clock> <timer name='invtsc' present='yes'/> </clock> in the domain XML.
Migration and saving the domain does not work with this timer.
Why is that ? The QEMU patch doesn't mention this restriction.
The support for this timer is indicated by bit 8 of EDX after calling CPUID with 0x80000007. It does not show up in /proc/cpuinfo [1] and since we're calling qemu without 'enforce', it doesn't error out if the host doesn't support this.
Maybe I'm mis-interpreting the kernel source, but my reading of that was that this *does* show up in /proc/cpuinfo, but it is given the name 'constant_tsc' instead of 'invtsc'. On my host I see 'constant_tsc' and 'nonstop_tsc' in /proc/cpuinfo
Alternatively, we could expose it in libvirt as a cpu flag: <cpu mode='custom' match='exact'> <model fallback='forbid'>qemu64</model> <feature policy='require' name='invtsc'/> </cpu> or maybe add +invtsc to qemu args when the 'nonstop_tsc' flag is requested?
Yep, I could see that as a valid option. If it is visible in /proc/cpuinfo, then I think that's a compelling reason for libvirt to model it as a CPU flag too, rather than pretend it is a new type of timer when it is just an attribute of the base TSC timer.
[1]: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/x8...
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 Tue, May 06, 2014 at 03:50:48PM +0100, Daniel P. Berrange wrote:
On Tue, May 06, 2014 at 03:27:20PM +0200, Ján Tomko wrote:
Not yet merged in upstream QEMU: https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg05024.html
Add support for invariant TSC timer running at constant rate in all ACPI P-, C- and T-states.
What does this do at the hardware level ? This doesn't seem to be creating a second TSC timer source in the guest, rather it is setting a property related to the existing TSC timer. So I think it might make more sense as an attribute for <timer name='tsc'> element instead.
It can be enabled by specifying: <clock> <timer name='invtsc' present='yes'/> </clock> in the domain XML.
Migration and saving the domain does not work with this timer.
Why is that ? The QEMU patch doesn't mention this restriction.
See http://marc.info/?l=qemu-devel&m=139828711719342&w=2
The support for this timer is indicated by bit 8 of EDX after calling CPUID with 0x80000007. It does not show up in /proc/cpuinfo [1] and since we're calling qemu without 'enforce', it doesn't error out if the host doesn't support this.
libvirt really needs to address this bug (lack of "enforce" flag). It was not so serious a few years ago, but now we have lots of features that depend on support on KVM kernel code. And now we have this feature that is unlikely to be included in a CPU model by default and likely to be configured explicitly, so existing "CPU level" or family/model checks won't be enough. QEMU now has the "filtered-features" X86CPU property that can be used by libvirt to emulate "enforce" behavior with appropriate error reporting.
Maybe I'm mis-interpreting the kernel source, but my reading of that was that this *does* show up in /proc/cpuinfo, but it is given the name 'constant_tsc' instead of 'invtsc'.
On my host I see 'constant_tsc' and 'nonstop_tsc' in /proc/cpuinfo
invtsc itself doesn't show up on /proc/cpuinfo directly, though. nonstop_tsc and constant-tsc are Linux-specific CPU capability flags that are set because invtsc is present. I find it confusing, I don't know why Linux doesn't simply show the feature directly like it does for all other feature flag bits.
Alternatively, we could expose it in libvirt as a cpu flag: <cpu mode='custom' match='exact'> <model fallback='forbid'>qemu64</model> <feature policy='require' name='invtsc'/> </cpu> or maybe add +invtsc to qemu args when the 'nonstop_tsc' flag is requested?
Yep, I could see that as a valid option. If it is visible in /proc/cpuinfo, then I think that's a compelling reason for libvirt to model it as a CPU flag too, rather than pretend it is a new type of timer when it is just an attribute of the base TSC timer.
Even if the guest doesn't show it on /proc/cpuinfo, it is a CPUID feature flag, so this approach seems valid to me.
[1]: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/x8...
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 :|
-- Eduardo
On 05/06/2014 04:50 PM, Daniel P. Berrange wrote:
On Tue, May 06, 2014 at 03:27:20PM +0200, Ján Tomko wrote:
The support for this timer is indicated by bit 8 of EDX after calling CPUID with 0x80000007. It does not show up in /proc/cpuinfo [1] and since we're calling qemu without 'enforce', it doesn't error out if the host doesn't support this.
Maybe I'm mis-interpreting the kernel source, but my reading of that was that this *does* show up in /proc/cpuinfo, but it is given the name 'constant_tsc' instead of 'invtsc'.
On my host I see 'constant_tsc' and 'nonstop_tsc' in /proc/cpuinfo
I see them too, despite not having this invariant TSC feature - the less 'enhanced' timer is enough for these flags to appear.
Alternatively, we could expose it in libvirt as a cpu flag: <cpu mode='custom' match='exact'> <model fallback='forbid'>qemu64</model> <feature policy='require' name='invtsc'/> </cpu> or maybe add +invtsc to qemu args when the 'nonstop_tsc' flag is requested?
Yep, I could see that as a valid option. If it is visible in /proc/cpuinfo, then I think that's a compelling reason for libvirt to model it as a CPU flag too, rather than pretend it is a new type of timer when it is just an attribute of the base TSC timer.
The lack of a name in /proc/cpuinfo for this feature is the reason I posted the <timer> version first. Jan
On Tue, May 06, 2014 at 03:27:20PM +0200, Ján Tomko wrote:
Not yet merged in upstream QEMU: https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg05024.html
Add support for invariant TSC timer running at constant rate in all ACPI P-, C- and T-states.
It can be enabled by specifying: <clock> <timer name='invtsc' present='yes'/> </clock> in the domain XML.
Migration and saving the domain does not work with this timer.
The support for this timer is indicated by bit 8 of EDX after calling CPUID with 0x80000007. It does not show up in /proc/cpuinfo [1] and since we're calling qemu without 'enforce', it doesn't error out if the host doesn't support this.
It is not a timer, really, but a CPU flag.
Alternatively, we could expose it in libvirt as a cpu flag: <cpu mode='custom' match='exact'> <model fallback='forbid'>qemu64</model> <feature policy='require' name='invtsc'/> </cpu>
Would prefer that option. Can't one modify QEMU's "-cpu" parameters, via libvirt XML?
or maybe add +invtsc to qemu args when the 'nonstop_tsc' flag is requested?
No, since it blocks migration better not condition "+invtsc" on presence of 'nonstop_tsc' flag.
[1]: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/x8... --- docs/formatdomain.html.in | 9 ++++++-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 6 +++-- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 6 +++++ src/qemu/qemu_migration.c | 14 ++++++++++++ .../qemuxml2argv-clock-timer-inv-tsc.args | 5 +++++ .../qemuxml2argv-clock-timer-inv-tsc.xml | 26 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 1 + 10 files changed, 66 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 4f19473..1d3fd93 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1368,13 +1368,18 @@ being modified, and can be one of "platform" (currently unsupported), "hpet" (libxl, xen, qemu), "kvmclock" (qemu), - "pit" (qemu), "rtc" (qemu), "tsc" (libxl) or "hypervclock" - (qemu - <span class="since">since 1.2.2</span>). + "pit" (qemu), "rtc" (qemu), "tsc" (libxl), "hypervclock" + (qemu - <span class="since">since 1.2.2</span>) or + "invtsc" (qemu - <span class="since">since 1.2.5</span>).
The <code>hypervclock</code> timer adds support for the reference time counter and the reference page for iTSC feature for guests running the Microsoft Windows operating system. + + The <code>invtsc</code> timer adds support for the invariant + TSC. It runs at a constant rate in all ACPI P- C- and T-states. + A guest with this timer enabled cannot be migrated or saved. </dd> <dt><code>track</code></dt> <dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4249ed5..5154826 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -918,6 +918,7 @@ <choice> <value>kvmclock</value> <value>hypervclock</value> + <value>invtsc</value> </choice> </attribute> </group> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6c3bdad..893d904 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -706,7 +706,8 @@ VIR_ENUM_IMPL(virDomainTimerName, VIR_DOMAIN_TIMER_NAME_LAST, "hpet", "tsc", "kvmclock", - "hypervclock"); + "hypervclock", + "invtsc");
VIR_ENUM_IMPL(virDomainTimerTrack, VIR_DOMAIN_TIMER_TRACK_LAST, "boot", @@ -2931,7 +2932,8 @@ virDomainDefPostParseInternal(virDomainDefPtr def, virDomainTimerDefPtr timer = def->clock.timers[i];
if (timer->name == VIR_DOMAIN_TIMER_NAME_KVMCLOCK || - timer->name == VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK) { + timer->name == VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK || + timer->name == VIR_DOMAIN_TIMER_NAME_INVTSC) { if (timer->tickpolicy != -1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("timer %s doesn't support setting of " diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a92f0f3..53c02e6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1635,6 +1635,7 @@ enum virDomainTimerNameType { VIR_DOMAIN_TIMER_NAME_TSC, VIR_DOMAIN_TIMER_NAME_KVMCLOCK, VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK, + VIR_DOMAIN_TIMER_NAME_INVTSC,
VIR_DOMAIN_TIMER_NAME_LAST }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6c1e17d..2994427 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6736,6 +6736,11 @@ qemuBuildCpuArgStr(virQEMUDriverPtr driver, have_cpu ? "" : default_model, timer->present ? '+' : '-'); have_cpu = true; + } else if (timer->name == VIR_DOMAIN_TIMER_NAME_INVTSC) { + virBufferAsprintf(&buf, "%s,%cinvtsc", + have_cpu ? "" : default_model, + timer->present ? '+' : '-'); + have_cpu = true; } else if (timer->name == VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK && timer->present) { virBufferAsprintf(&buf, "%s,hv_time", @@ -8063,6 +8068,7 @@ qemuBuildCommandLine(virConnectPtr conn,
case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: + case VIR_DOMAIN_TIMER_NAME_INVTSC: /* Timers above are handled when building -cpu. */ case VIR_DOMAIN_TIMER_NAME_LAST: break; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index a9f7fea..c1ffc0f 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->clock.ntimers; i++) { + virDomainTimerDefPtr timer = def->clock.timers[i]; + + if (timer->present != 1) + continue; + + if (timer->name == VIR_DOMAIN_TIMER_NAME_INVTSC) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("domain has '%s' timer"), + virDomainTimerNameTypeToString(timer->name)); + return false; + } + } + return true; }
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.args b/tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.args new file mode 100644 index 0000000..ae74ae8 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.args @@ -0,0 +1,5 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/kvm -S -M pc \ +-cpu qemu32,+invtsc -m 214 -smp 6 \ +-nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -usb -net \ +none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.xml b/tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.xml new file mode 100644 index 0000000..b4a82e8 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.xml @@ -0,0 +1,26 @@ +<domain type='kvm'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>6</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <features> + <pae/> + </features> + <clock offset='utc'> + <timer name='invtsc' present='yes'/> + </clock> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/kvm</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 14482fd..3c5024e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -641,6 +641,7 @@ mymain(void) DO_TEST("cpu-host-kvmclock", QEMU_CAPS_ENABLE_KVM, QEMU_CAPS_CPU_HOST); DO_TEST("kvmclock", QEMU_CAPS_KVM); DO_TEST("clock-timer-hyperv-rtc", QEMU_CAPS_KVM); + DO_TEST("clock-timer-inv-tsc", QEMU_CAPS_KVM);
DO_TEST("cpu-eoi-disabled", QEMU_CAPS_ENABLE_KVM); DO_TEST("cpu-eoi-enabled", QEMU_CAPS_ENABLE_KVM); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 3ea03e6..a659cd0 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -185,6 +185,7 @@ mymain(void) DO_TEST("clock-catchup"); DO_TEST("kvmclock"); DO_TEST("clock-timer-hyperv-rtc"); + DO_TEST("clock-timer-inv-tsc");
DO_TEST("cpu-eoi-disabled"); DO_TEST("cpu-eoi-enabled"); -- 1.8.3.2
On Wed, May 07, 2014 at 07:35:13PM -0300, Marcelo Tosatti wrote:
On Tue, May 06, 2014 at 03:27:20PM +0200, Ján Tomko wrote:
Not yet merged in upstream QEMU: https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg05024.html
Add support for invariant TSC timer running at constant rate in all ACPI P-, C- and T-states.
It can be enabled by specifying: <clock> <timer name='invtsc' present='yes'/> </clock> in the domain XML.
Migration and saving the domain does not work with this timer.
The support for this timer is indicated by bit 8 of EDX after calling CPUID with 0x80000007. It does not show up in /proc/cpuinfo [1] and since we're calling qemu without 'enforce', it doesn't error out if the host doesn't support this.
It is not a timer, really, but a CPU flag.
Alternatively, we could expose it in libvirt as a cpu flag: <cpu mode='custom' match='exact'> <model fallback='forbid'>qemu64</model> <feature policy='require' name='invtsc'/> </cpu>
Would prefer that option. Can't one modify QEMU's "-cpu" parameters, via libvirt XML?
or maybe add +invtsc to qemu args when the 'nonstop_tsc' flag is requested?
No, since it blocks migration better not condition "+invtsc" on presence of 'nonstop_tsc' flag.
Simply adding <feature name='invtsc'> <cpuid function='0x80000007' edx='0x00000100'/> </feature> To cpu_map.xml is sufficient to allow users to enable this feature on demand ? Haven't thought about all the corner cases, though.
On 05/09/2014 08:58 AM, Marcelo Tosatti wrote:
On Wed, May 07, 2014 at 07:35:13PM -0300, Marcelo Tosatti wrote:
Simply adding
<feature name='invtsc'> <cpuid function='0x80000007' edx='0x00000100'/> </feature>
To cpu_map.xml is sufficient to allow users to enable this feature on demand ?
Yes, this is what I posted in v2 of the patch: https://www.redhat.com/archives/libvir-list/2014-May/msg00297.html Jan
Haven't thought about all the corner cases, though.
participants (5)
-
Daniel P. Berrange -
Eduardo Habkost -
Ján Tomko -
Marcelo Tosatti -
Peter Krempa