[PATCH 0/2] domain, qemu: add support for clearing TSC on reset
Some versions of Windows hang on reboot if their TSC value is greater than 2^54. The workaround is to reset the TSC to a small value, and is implemented by QEMU and ESXi. This series adds a domain attribute for this, and implements it in QEMU. Paolo Paolo Bonzini (2): domain: add tsc.on_reboot element qemu: add support for tsc.on_reboot element docs/formatdomain.rst | 4 +++ src/conf/domain_conf.c | 22 ++++++++++++ src/conf/domain_conf.h | 10 ++++++ src/conf/schemas/domaincommon.rng | 9 +++++ src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 4 +++ src/qemu/qemu_validate.c | 36 +++++++++++++------ .../caps_7.0.0.x86_64.replies | 4 +++ .../caps_7.0.0.x86_64.xml | 1 + ...uency.args => cpu-tsc-clear-on-reset.args} | 2 +- ... cpu-tsc-clear-on-reset.x86_64-7.0.0.args} | 6 ++-- ...equency.xml => cpu-tsc-clear-on-reset.xml} | 2 +- tests/qemuxml2argvtest.c | 2 ++ 14 files changed, 89 insertions(+), 16 deletions(-) copy tests/qemuxml2argvdata/{cpu-tsc-frequency.args => cpu-tsc-clear-on-reset.args} (97%) copy tests/qemuxml2argvdata/{virtio-rng-builtin.x86_64-latest.args => cpu-tsc-clear-on-reset.x86_64-7.0.0.args} (77%) copy tests/qemuxml2argvdata/{cpu-tsc-frequency.xml => cpu-tsc-clear-on-reset.xml} (95%) -- 2.35.1
Some versions of Windows hang on reboot if their TSC value is greater than 2^54. The workaround is to reset the TSC to a small value. Add to the domain configuration an attribute for this. It can be used by QEMU and in principle also by ESXi, which has a property called monitor_control.enable_softResetClearTSC as well. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- docs/formatdomain.rst | 4 ++++ src/conf/domain_conf.c | 22 ++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++++++ src/conf/schemas/domaincommon.rng | 9 +++++++++ 4 files changed, 45 insertions(+) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index d188de4858..c9319e42ac 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2241,6 +2241,10 @@ Windows, however, expects it to be in so called 'localtime'. ``frequency`` The ``frequency`` attribute is an unsigned integer specifying the frequency at which ``name="tsc"`` runs. + ``on_reboot`` + The ``on_reboot`` attribute controls how the ``name="tsc"`` timer behaves + when the VM is reset, and can be "default", "clear" or "keep". The reset + behavior of other timers is hardcoded, and depends on the type of timer. ``mode`` The ``mode`` attribute controls how the ``name="tsc"`` timer is managed, and can be "auto", "native", "emulate", "paravirt", or "smpsafe". Other diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 153954a0b0..1893b8bbca 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1217,6 +1217,13 @@ VIR_ENUM_IMPL(virDomainTimerMode, "smpsafe", ); +VIR_ENUM_IMPL(virDomainTimerRebootMode, + VIR_DOMAIN_TIMER_REBOOT_MODE_LAST, + "default", + "keep", + "clear", +); + VIR_ENUM_IMPL(virDomainStartupPolicy, VIR_DOMAIN_STARTUP_POLICY_LAST, "default", @@ -12022,6 +12029,7 @@ virDomainTimerDefParseXML(xmlNodePtr node, g_autofree char *tickpolicy = NULL; g_autofree char *track = NULL; g_autofree char *mode = NULL; + g_autofree char *reboot = NULL; def = g_new0(virDomainTimerDef, 1); @@ -12080,6 +12088,15 @@ virDomainTimerDefParseXML(xmlNodePtr node, } } + reboot = virXMLPropString(node, "on_reboot"); + if (reboot != NULL) { + if ((def->reboot = virDomainTimerRebootModeTypeFromString(reboot)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown timer reboot mode '%s'"), reboot); + goto error; + } + } + catchup = virXPathNode("./catchup", ctxt); if (catchup != NULL) { ret = virXPathULong("string(./catchup/@threshold)", ctxt, @@ -26151,6 +26168,11 @@ virDomainTimerDefFormat(virBuffer *buf, virBufferAsprintf(&timerAttr, " mode='%s'", virDomainTimerModeTypeToString(def->mode)); } + + if (def->reboot) { + virBufferAsprintf(&timerAttr, " on_reboot='%s'", + virDomainTimerRebootModeTypeToString(def->mode)); + } } if (def->catchup.threshold > 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b69abfa270..3618410f6c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2408,6 +2408,14 @@ typedef enum { VIR_DOMAIN_TIMER_MODE_LAST } virDomainTimerModeType; +typedef enum { + VIR_DOMAIN_TIMER_REBOOT_MODE_DEFAULT = 0, + VIR_DOMAIN_TIMER_REBOOT_MODE_KEEP, + VIR_DOMAIN_TIMER_REBOOT_MODE_CLEAR, + + VIR_DOMAIN_TIMER_REBOOT_MODE_LAST +} virDomainTimerRebootModeType; + typedef enum { VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC = 0, VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO, @@ -2439,6 +2447,7 @@ struct _virDomainTimerDef { /* frequency & mode are only valid for name='tsc' */ unsigned long long frequency; /* in Hz, unspecified = 0 */ int mode; /* enum virDomainTimerModeType */ + int reboot; /* enum virDomainTimerRebootModeType */ }; typedef enum { @@ -4032,6 +4041,7 @@ VIR_ENUM_DECL(virDomainClockBasis); VIR_ENUM_DECL(virDomainTimerName); VIR_ENUM_DECL(virDomainTimerTrack); VIR_ENUM_DECL(virDomainTimerTickpolicy); +VIR_ENUM_DECL(virDomainTimerRebootMode); VIR_ENUM_DECL(virDomainTimerMode); VIR_ENUM_DECL(virDomainCpuPlacementMode); diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 9c1b64a644..10d0404889 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -1285,6 +1285,15 @@ <ref name="unsignedLong"/> </attribute> </optional> + <optional> + <attribute name="on_reboot"> + <choice> + <value>default</value> + <value>clear</value> + <value>keep</value> + </choice> + </attribute> + </optional> <optional> <attribute name="mode"> <choice> -- 2.35.1
On 3/24/22 10:48, Paolo Bonzini wrote:
Some versions of Windows hang on reboot if their TSC value is greater than 2^54. The workaround is to reset the TSC to a small value. Add to the domain configuration an attribute for this. It can be used by QEMU and in principle also by ESXi, which has a property called monitor_control.enable_softResetClearTSC as well.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- docs/formatdomain.rst | 4 ++++ src/conf/domain_conf.c | 22 ++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++++++ src/conf/schemas/domaincommon.rng | 9 +++++++++ 4 files changed, 45 insertions(+)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index d188de4858..c9319e42ac 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2241,6 +2241,10 @@ Windows, however, expects it to be in so called 'localtime'. ``frequency`` The ``frequency`` attribute is an unsigned integer specifying the frequency at which ``name="tsc"`` runs. + ``on_reboot`` + The ``on_reboot`` attribute controls how the ``name="tsc"`` timer behaves + when the VM is reset, and can be "default", "clear" or "keep". The reset + behavior of other timers is hardcoded, and depends on the type of timer. ``mode`` The ``mode`` attribute controls how the ``name="tsc"`` timer is managed, and can be "auto", "native", "emulate", "paravirt", or "smpsafe". Other diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 153954a0b0..1893b8bbca 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1217,6 +1217,13 @@ VIR_ENUM_IMPL(virDomainTimerMode, "smpsafe", );
+VIR_ENUM_IMPL(virDomainTimerRebootMode, + VIR_DOMAIN_TIMER_REBOOT_MODE_LAST, + "default", + "keep", + "clear", +); + VIR_ENUM_IMPL(virDomainStartupPolicy, VIR_DOMAIN_STARTUP_POLICY_LAST, "default", @@ -12022,6 +12029,7 @@ virDomainTimerDefParseXML(xmlNodePtr node, g_autofree char *tickpolicy = NULL; g_autofree char *track = NULL; g_autofree char *mode = NULL; + g_autofree char *reboot = NULL;
def = g_new0(virDomainTimerDef, 1);
@@ -12080,6 +12088,15 @@ virDomainTimerDefParseXML(xmlNodePtr node, } }
+ reboot = virXMLPropString(node, "on_reboot"); + if (reboot != NULL) { + if ((def->reboot = virDomainTimerRebootModeTypeFromString(reboot)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown timer reboot mode '%s'"), reboot); + goto error; + } + }
I know you just mimicked what is done for @mode attribute, but we have this nice brand new virXMLPropEnum() which fits perfectly here as it encapsulates these lines.
+ catchup = virXPathNode("./catchup", ctxt); if (catchup != NULL) { ret = virXPathULong("string(./catchup/@threshold)", ctxt, @@ -26151,6 +26168,11 @@ virDomainTimerDefFormat(virBuffer *buf, virBufferAsprintf(&timerAttr, " mode='%s'", virDomainTimerModeTypeToString(def->mode)); } + + if (def->reboot) { + virBufferAsprintf(&timerAttr, " on_reboot='%s'", + virDomainTimerRebootModeTypeToString(def->mode)); + } }
if (def->catchup.threshold > 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b69abfa270..3618410f6c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2408,6 +2408,14 @@ typedef enum { VIR_DOMAIN_TIMER_MODE_LAST } virDomainTimerModeType;
+typedef enum { + VIR_DOMAIN_TIMER_REBOOT_MODE_DEFAULT = 0, + VIR_DOMAIN_TIMER_REBOOT_MODE_KEEP, + VIR_DOMAIN_TIMER_REBOOT_MODE_CLEAR, + + VIR_DOMAIN_TIMER_REBOOT_MODE_LAST +} virDomainTimerRebootModeType; + typedef enum { VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC = 0, VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO, @@ -2439,6 +2447,7 @@ struct _virDomainTimerDef { /* frequency & mode are only valid for name='tsc' */ unsigned long long frequency; /* in Hz, unspecified = 0 */ int mode; /* enum virDomainTimerModeType */ + int reboot; /* enum virDomainTimerRebootModeType */
This can be then the actual enum instead of int.
};
typedef enum { @@ -4032,6 +4041,7 @@ VIR_ENUM_DECL(virDomainClockBasis); VIR_ENUM_DECL(virDomainTimerName); VIR_ENUM_DECL(virDomainTimerTrack); VIR_ENUM_DECL(virDomainTimerTickpolicy); +VIR_ENUM_DECL(virDomainTimerRebootMode); VIR_ENUM_DECL(virDomainTimerMode); VIR_ENUM_DECL(virDomainCpuPlacementMode);
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 9c1b64a644..10d0404889 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -1285,6 +1285,15 @@ <ref name="unsignedLong"/> </attribute> </optional> + <optional> + <attribute name="on_reboot"> + <choice> + <value>default</value>
And "default" is not accepted (because _DEFAULT = 0 and parser checks <= 0).
+ <value>clear</value> + <value>keep</value> + </choice> + </attribute> + </optional> <optional> <attribute name="mode"> <choice>
Michal
On 3/25/22 16:35, Michal Prívozník wrote:
@@ -12080,6 +12088,15 @@ virDomainTimerDefParseXML(xmlNodePtr node, } }
+ reboot = virXMLPropString(node, "on_reboot"); + if (reboot != NULL) { + if ((def->reboot = virDomainTimerRebootModeTypeFromString(reboot)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown timer reboot mode '%s'"), reboot); + goto error; + } + }
I know you just mimicked what is done for @mode attribute, but we have this nice brand new virXMLPropEnum() which fits perfectly here as it encapsulates these lines.
Huh, after I've merged your patches and started rebasing my local branches I've realized I have a branch that switches this code to virXMLPropEnum(). I don't remember why I haven't sent it yet. Michal
QEMU 7.0.0 adds a new property tsc-clear-on-reset to x86 CPU, corresponding to Libvirt's <tsc on_reboot="clear"/> element. Plumb it in the validation, command line handling and tests. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 4 +++ src/qemu/qemu_validate.c | 36 +++++++++++++------ .../caps_7.0.0.x86_64.replies | 4 +++ .../caps_7.0.0.x86_64.xml | 1 + ...uency.args => cpu-tsc-clear-on-reset.args} | 2 +- ... cpu-tsc-clear-on-reset.x86_64-7.0.0.args} | 6 ++-- ...equency.xml => cpu-tsc-clear-on-reset.xml} | 2 +- tests/qemuxml2argvtest.c | 2 ++ 10 files changed, 44 insertions(+), 16 deletions(-) copy tests/qemuxml2argvdata/{cpu-tsc-frequency.args => cpu-tsc-clear-on-reset.args} (97%) copy tests/qemuxml2argvdata/{virtio-rng-builtin.x86_64-latest.args => cpu-tsc-clear-on-reset.x86_64-7.0.0.args} (77%) copy tests/qemuxml2argvdata/{cpu-tsc-frequency.xml => cpu-tsc-clear-on-reset.xml} (95%) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 32980e7330..d22bbee70e 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -668,6 +668,7 @@ VIR_ENUM_IMPL(virQEMUCaps, /* 425 */ "blockdev.nbd.tls-hostname", /* QEMU_CAPS_BLOCKDEV_NBD_TLS_HOSTNAME */ + "x86-cpu.tsc-clear-on-reset", /* QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET */ ); @@ -1775,6 +1776,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsMemoryBackendMemfd[] static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsMaxCPU[] = { { "unavailable-features", QEMU_CAPS_CPU_UNAVAILABLE_FEATURES }, { "kvm-no-adjvtime", QEMU_CAPS_CPU_KVM_NO_ADJVTIME }, + { "tsc-clear-on-reset", QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET }, { "migratable", QEMU_CAPS_CPU_MIGRATABLE }, }; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 0a215a11d5..7aee73a725 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -643,6 +643,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 425 */ QEMU_CAPS_BLOCKDEV_NBD_TLS_HOSTNAME, /* tls hostname can be overriden for NBD clients */ + QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET, /* x86-cpu.tsc-clear-on-reset */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c836799888..8ecede0ade 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6613,6 +6613,10 @@ qemuBuildCpuCommandLine(virCommand *cmd, case VIR_DOMAIN_TIMER_NAME_TSC: if (timer->frequency > 0) virBufferAsprintf(&buf, ",tsc-frequency=%llu", timer->frequency); + if (timer->reboot == VIR_DOMAIN_TIMER_REBOOT_MODE_CLEAR) + virBufferAddLit(&buf, ",tsc-clear-on-reset=on"); + else if (timer->reboot == VIR_DOMAIN_TIMER_REBOOT_MODE_KEEP) + virBufferAddLit(&buf, ",tsc-clear-on-reset=off"); break; case VIR_DOMAIN_TIMER_NAME_ARMVTIMER: switch (timer->tickpolicy) { diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index f27e480696..c4ab2976dc 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -396,10 +396,11 @@ static int qemuValidateDomainDefClockTimers(const virDomainDef *def, virQEMUCaps *qemuCaps) { + virDomainTimerDef *timer; size_t i; for (i = 0; i < def->clock.ntimers; i++) { - virDomainTimerDef *timer = def->clock.timers[i]; + timer = def->clock.timers[i]; switch ((virDomainTimerNameType)timer->name) { case VIR_DOMAIN_TIMER_NAME_PLATFORM: @@ -409,20 +410,25 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, return -1; case VIR_DOMAIN_TIMER_NAME_TSC: - case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: - case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: - if (!ARCH_IS_X86(def->os.arch) && timer->present == VIR_TRISTATE_BOOL_YES) { + if (!ARCH_IS_X86(def->os.arch) && timer->present == VIR_TRISTATE_BOOL_YES) + goto no_support; + + if (timer->reboot != VIR_DOMAIN_TIMER_REBOOT_MODE_DEFAULT && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Configuring the '%s' timer is not supported " - "for virtType=%s arch=%s machine=%s guests"), - virDomainTimerNameTypeToString(timer->name), - virDomainVirtTypeToString(def->virtType), - virArchToString(def->os.arch), - def->os.machine); + _("Configuring the '%s' timer to reset on reboot is not supported " + "with this QEMU binary"), + virDomainTimerNameTypeToString(timer->name)); return -1; } break; + case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: + case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: + if (!ARCH_IS_X86(def->os.arch) && timer->present == VIR_TRISTATE_BOOL_YES) + goto no_support; + break; + case VIR_DOMAIN_TIMER_NAME_LAST: break; @@ -540,6 +546,16 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, } return 0; + + no_support: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Configuring the '%s' timer is not supported " + "for virtType=%s arch=%s machine=%s guests"), + virDomainTimerNameTypeToString(timer->name), + virDomainVirtTypeToString(def->virtType), + virArchToString(def->os.arch), + def->os.machine); + return -1; } diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.replies index 82ccbab6eb..d919caf7f3 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.replies @@ -28849,6 +28849,10 @@ { "name": "sse4_1", "type": "bool" + }, + { + "name": "tsc-clear-on-reset", + "type": "bool" } ], "id": "libvirt-41" diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml index 0f34a341af..d36237364f 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml @@ -244,6 +244,7 @@ <flag name='calc-dirty-rate'/> <flag name='dirtyrate-param.mode'/> <flag name='blockdev.nbd.tls-hostname'/> + <flag name='x86-cpu.tsc-clear-on-reset'/> <version>6002050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> diff --git a/tests/qemuxml2argvdata/cpu-tsc-frequency.args b/tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.args similarity index 97% copy from tests/qemuxml2argvdata/cpu-tsc-frequency.args copy to tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.args index 4a032f5d85..f2b8a98968 100644 --- a/tests/qemuxml2argvdata/cpu-tsc-frequency.args +++ b/tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.args @@ -13,7 +13,7 @@ QEMU_AUDIO_DRV=none \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,usb=off,dump-guest-core=off \ -accel kvm \ --cpu Haswell,vme=on,ds=on,acpi=on,ss=on,ht=on,tm=on,pbe=on,dtes64=on,monitor=on,ds-cpl=on,vmx=on,smx=on,est=on,tm2=on,xtpr=on,pdcm=on,f16c=on,rdrand=on,pdpe1gb=on,abm=on,lahf-lm=on,invtsc=on,tsc-frequency=4567890000 \ +-cpu Haswell,vme=on,ds=on,acpi=on,ss=on,ht=on,tm=on,pbe=on,dtes64=on,monitor=on,ds-cpl=on,vmx=on,smx=on,est=on,tm2=on,xtpr=on,pdcm=on,f16c=on,rdrand=on,pdpe1gb=on,abm=on,lahf-lm=on,invtsc=on,tsc-clear-on-reset=on \ -m 214 \ -realtime mlock=off \ -smp 1,sockets=1,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/virtio-rng-builtin.x86_64-latest.args b/tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.x86_64-7.0.0.args similarity index 77% copy from tests/qemuxml2argvdata/virtio-rng-builtin.x86_64-latest.args copy to tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.x86_64-7.0.0.args index a0aeb74319..59f1d66ed4 100644 --- a/tests/qemuxml2argvdata/virtio-rng-builtin.x86_64-latest.args +++ b/tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.x86_64-7.0.0.args @@ -10,9 +10,9 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -name guest=QEMUGuest1,debug-threads=on \ -S \ -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ --machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-machine pc-i440fx-7.0,usb=off,dump-guest-core=off,memory-backend=pc.ram \ -accel kvm \ --cpu qemu64 \ +-cpu EPYC-Rome,x2apic=on,tsc-deadline=on,hypervisor=on,tsc-adjust=on,stibp=on,arch-capabilities=on,ssbd=on,xsaves=on,cmp-legacy=on,amd-ssbd=on,virt-ssbd=on,svme-addr-chk=on,rdctl-no=on,skip-l1dfl-vmentry=on,mds-no=on,pschange-mc-no=on,invtsc=on,tsc-clear-on-reset=on \ -m 214 \ -object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ -overcommit mem-lock=off \ @@ -30,7 +30,5 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ -audiodev '{"id":"audio1","driver":"none"}' \ -device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \ --object '{"qom-type":"rng-builtin","id":"objrng0"}' \ --device '{"driver":"virtio-rng-pci","rng":"objrng0","id":"rng0","bus":"pci.0","addr":"0x3"}' \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ -msg timestamp=on diff --git a/tests/qemuxml2argvdata/cpu-tsc-frequency.xml b/tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.xml similarity index 95% copy from tests/qemuxml2argvdata/cpu-tsc-frequency.xml copy to tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.xml index afaef6e8e5..2521349ce7 100644 --- a/tests/qemuxml2argvdata/cpu-tsc-frequency.xml +++ b/tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.xml @@ -18,7 +18,7 @@ <feature policy='require' name='invtsc'/> </cpu> <clock offset='utc'> - <timer name='tsc' frequency='4567890000'/> + <timer name='tsc' on_reboot='clear'/> </clock> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e7fecb24d3..9d5aba85a1 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2094,6 +2094,8 @@ mymain(void) DO_TEST("cpu-Haswell-noTSX", QEMU_CAPS_KVM); DO_TEST_NOCAPS("cpu-host-model-cmt"); DO_TEST_CAPS_VER("cpu-host-model-cmt", "4.0.0"); + DO_TEST("cpu-tsc-clear-on-reset", QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET); + DO_TEST_CAPS_VER("cpu-tsc-clear-on-reset", "7.0.0"); DO_TEST("cpu-tsc-frequency", QEMU_CAPS_KVM); DO_TEST_CAPS_VER("cpu-tsc-frequency", "4.0.0"); DO_TEST_CAPS_VER("cpu-translation", "4.0.0"); -- 2.35.1
On 3/24/22 10:48, Paolo Bonzini wrote:
QEMU 7.0.0 adds a new property tsc-clear-on-reset to x86 CPU, corresponding to Libvirt's <tsc on_reboot="clear"/> element. Plumb it in the validation, command line handling and tests.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 4 +++ src/qemu/qemu_validate.c | 36 +++++++++++++------ .../caps_7.0.0.x86_64.replies | 4 +++ .../caps_7.0.0.x86_64.xml | 1 + ...uency.args => cpu-tsc-clear-on-reset.args} | 2 +- ... cpu-tsc-clear-on-reset.x86_64-7.0.0.args} | 6 ++-- ...equency.xml => cpu-tsc-clear-on-reset.xml} | 2 +- tests/qemuxml2argvtest.c | 2 ++ 10 files changed, 44 insertions(+), 16 deletions(-) copy tests/qemuxml2argvdata/{cpu-tsc-frequency.args => cpu-tsc-clear-on-reset.args} (97%) copy tests/qemuxml2argvdata/{virtio-rng-builtin.x86_64-latest.args => cpu-tsc-clear-on-reset.x86_64-7.0.0.args} (77%) copy tests/qemuxml2argvdata/{cpu-tsc-frequency.xml => cpu-tsc-clear-on-reset.xml} (95%)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 32980e7330..d22bbee70e 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -668,6 +668,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
/* 425 */ "blockdev.nbd.tls-hostname", /* QEMU_CAPS_BLOCKDEV_NBD_TLS_HOSTNAME */ + "x86-cpu.tsc-clear-on-reset", /* QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET */ );
@@ -1775,6 +1776,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsMemoryBackendMemfd[] static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsMaxCPU[] = { { "unavailable-features", QEMU_CAPS_CPU_UNAVAILABLE_FEATURES }, { "kvm-no-adjvtime", QEMU_CAPS_CPU_KVM_NO_ADJVTIME }, + { "tsc-clear-on-reset", QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET }, { "migratable", QEMU_CAPS_CPU_MIGRATABLE }, };
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 0a215a11d5..7aee73a725 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -643,6 +643,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
/* 425 */ QEMU_CAPS_BLOCKDEV_NBD_TLS_HOSTNAME, /* tls hostname can be overriden for NBD clients */ + QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET, /* x86-cpu.tsc-clear-on-reset */
QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c836799888..8ecede0ade 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6613,6 +6613,10 @@ qemuBuildCpuCommandLine(virCommand *cmd, case VIR_DOMAIN_TIMER_NAME_TSC: if (timer->frequency > 0) virBufferAsprintf(&buf, ",tsc-frequency=%llu", timer->frequency); + if (timer->reboot == VIR_DOMAIN_TIMER_REBOOT_MODE_CLEAR) + virBufferAddLit(&buf, ",tsc-clear-on-reset=on"); + else if (timer->reboot == VIR_DOMAIN_TIMER_REBOOT_MODE_KEEP) + virBufferAddLit(&buf, ",tsc-clear-on-reset=off");
Here, I'd prefer switch(), esp. after the reboot member is declared as corresponding enum instead of int so that compiler warns us about this place when a new enum value is added.
break; case VIR_DOMAIN_TIMER_NAME_ARMVTIMER: switch (timer->tickpolicy) { diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index f27e480696..c4ab2976dc 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -396,10 +396,11 @@ static int qemuValidateDomainDefClockTimers(const virDomainDef *def, virQEMUCaps *qemuCaps) { + virDomainTimerDef *timer; size_t i;
for (i = 0; i < def->clock.ntimers; i++) { - virDomainTimerDef *timer = def->clock.timers[i]; + timer = def->clock.timers[i];
switch ((virDomainTimerNameType)timer->name) { case VIR_DOMAIN_TIMER_NAME_PLATFORM: @@ -409,20 +410,25 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, return -1;
case VIR_DOMAIN_TIMER_NAME_TSC: - case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: - case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: - if (!ARCH_IS_X86(def->os.arch) && timer->present == VIR_TRISTATE_BOOL_YES) { + if (!ARCH_IS_X86(def->os.arch) && timer->present == VIR_TRISTATE_BOOL_YES) + goto no_support; + + if (timer->reboot != VIR_DOMAIN_TIMER_REBOOT_MODE_DEFAULT && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Configuring the '%s' timer is not supported " - "for virtType=%s arch=%s machine=%s guests"), - virDomainTimerNameTypeToString(timer->name), - virDomainVirtTypeToString(def->virtType), - virArchToString(def->os.arch), - def->os.machine); + _("Configuring the '%s' timer to reset on reboot is not supported " + "with this QEMU binary"),
Error messages are exempt from the 80 chars rule, so that they can be easily 'git grep'-ped.
+ virDomainTimerNameTypeToString(timer->name)); return -1; } break;
+ case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: + case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: + if (!ARCH_IS_X86(def->os.arch) && timer->present == VIR_TRISTATE_BOOL_YES) + goto no_support; + break; + case VIR_DOMAIN_TIMER_NAME_LAST: break;
@@ -540,6 +546,16 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, }
return 0; + + no_support: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Configuring the '%s' timer is not supported " + "for virtType=%s arch=%s machine=%s guests"), + virDomainTimerNameTypeToString(timer->name), + virDomainVirtTypeToString(def->virtType), + virArchToString(def->os.arch), + def->os.machine); + return -1; }
Michal
On 3/24/22 10:48, Paolo Bonzini wrote:
Some versions of Windows hang on reboot if their TSC value is greater than 2^54. The workaround is to reset the TSC to a small value, and is implemented by QEMU and ESXi. This series adds a domain attribute for this, and implements it in QEMU.
Paolo
Paolo Bonzini (2): domain: add tsc.on_reboot element qemu: add support for tsc.on_reboot element
docs/formatdomain.rst | 4 +++ src/conf/domain_conf.c | 22 ++++++++++++ src/conf/domain_conf.h | 10 ++++++ src/conf/schemas/domaincommon.rng | 9 +++++ src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 4 +++ src/qemu/qemu_validate.c | 36 +++++++++++++------ .../caps_7.0.0.x86_64.replies | 4 +++ .../caps_7.0.0.x86_64.xml | 1 + ...uency.args => cpu-tsc-clear-on-reset.args} | 2 +- ... cpu-tsc-clear-on-reset.x86_64-7.0.0.args} | 6 ++-- ...equency.xml => cpu-tsc-clear-on-reset.xml} | 2 +- tests/qemuxml2argvtest.c | 2 ++ 14 files changed, 89 insertions(+), 16 deletions(-) copy tests/qemuxml2argvdata/{cpu-tsc-frequency.args => cpu-tsc-clear-on-reset.args} (97%) copy tests/qemuxml2argvdata/{virtio-rng-builtin.x86_64-latest.args => cpu-tsc-clear-on-reset.x86_64-7.0.0.args} (77%) copy tests/qemuxml2argvdata/{cpu-tsc-frequency.xml => cpu-tsc-clear-on-reset.xml} (95%)
I'm fixing all the small issues I've raised in review and pushing these. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Thanks! Michal
On 3/25/22 16:35, Michal Prívozník wrote:
I'm fixing all the small issues I've raised in review and pushing these.
Reviewed-by: Michal Privoznik<mprivozn@redhat.com>
No, please don't! The property is not yet part of QEMU, this should have been tagged RFC. Sorry about the confusion. Paolo
On 3/25/22 17:49, Paolo Bonzini wrote:
On 3/25/22 16:35, Michal Prívozník wrote:
I'm fixing all the small issues I've raised in review and pushing these.
Reviewed-by: Michal Privoznik<mprivozn@redhat.com>
No, please don't! The property is not yet part of QEMU, this should have been tagged RFC.
Sorry about the confusion.
Oops, should have followed conversation on qemu-devel more closely: https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg06077.html So let me revert these. Michal
participants (2)
-
Michal Prívozník -
Paolo Bonzini