[libvirt] [PATCH v3 0/5] Add support for Direct Mode for Hyper-V Synthetic timers
Changes since v2: - Post 5.6.0 rebase, patches 1-3 are dropped as they were already merged. Original description: QEMU-4.1 will bring us Direct Mode for Hyper-V Synthetic timers support, we need to support it in libvirt too. As this is not a new enlightenment but rather an enhancement of an existing one ('stimer'), support it in <stimer state='on'> <direct state='on'/> </stimer> form. Backwards compatibility is (hopefully) preserved. Vitaly Kuznetsov (5): cpu_x86: add Edx to KVM_FEATURE_DEF() conf: change the way how Hyper-V features are printed out conf: add support for Direct Mode for Hyper-V Synthetic timers qemu: add support for Direct Mode for Hyper-V Synthetic timers news: mention Direct Mode for Hyper-V Synthetic timers support docs/formatdomain.html.in | 10 ++-- docs/news.xml | 9 ++++ docs/schemas/domaincommon.rng | 16 +++++- src/conf/domain_conf.c | 77 ++++++++++++++++++++++++++--- src/conf/domain_conf.h | 1 + src/cpu/cpu_x86.c | 33 +++++++------ src/cpu/cpu_x86_data.h | 2 + src/qemu/qemu_command.c | 12 +++-- src/qemu/qemu_process.c | 20 +++++++- tests/qemuxml2argvdata/hyperv.args | 4 +- tests/qemuxml2argvdata/hyperv.xml | 4 +- tests/qemuxml2xmloutdata/hyperv.xml | 4 +- 12 files changed, 155 insertions(+), 37 deletions(-) -- 2.20.1
Some Hyper-V features (like the upcoming Direct Synthetic timers) are announced by feature bits in Edx but KVM_FEATURE_DEF() supports only Eax. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- src/cpu/cpu_x86.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index b58eb2c9d8..d2d9537c32 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -59,9 +59,9 @@ struct _virCPUx86Feature { { .type = VIR_CPU_X86_DATA_CPUID, \ .data = { .cpuid = {__VA_ARGS__} } } -#define KVM_FEATURE_DEF(Name, Eax_in, Eax) \ +#define KVM_FEATURE_DEF(Name, Eax_in, Eax, Edx) \ static virCPUx86DataItem Name ## _data[] = { \ - CPUID(.eax_in = Eax_in, .eax = Eax), \ + CPUID(.eax_in = Eax_in, .eax = Eax, .edx = Edx), \ } #define KVM_FEATURE(Name) \ @@ -74,32 +74,32 @@ struct _virCPUx86Feature { } KVM_FEATURE_DEF(VIR_CPU_x86_KVM_PV_UNHALT, - 0x40000001, 0x00000080); + 0x40000001, 0x00000080, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_HV_RUNTIME, - 0x40000003, 0x00000001); + 0x40000003, 0x00000001, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_HV_SYNIC, - 0x40000003, 0x00000004); + 0x40000003, 0x00000004, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_HV_STIMER, - 0x40000003, 0x00000008); + 0x40000003, 0x00000008, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_HV_RELAXED, - 0x40000003, 0x00000020); + 0x40000003, 0x00000020, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_HV_VAPIC, - 0x40000003, 0x00000030); + 0x40000003, 0x00000030, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_HV_VPINDEX, - 0x40000003, 0x00000040); + 0x40000003, 0x00000040, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_HV_RESET, - 0x40000003, 0x00000080); + 0x40000003, 0x00000080, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_HV_FREQUENCIES, - 0x40000003, 0x00000800); + 0x40000003, 0x00000800, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_HV_REENLIGHTENMENT, - 0x40000003, 0x00002000); + 0x40000003, 0x00002000, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_HV_TLBFLUSH, - 0x40000004, 0x00000004); + 0x40000004, 0x00000004, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_HV_IPI, - 0x40000004, 0x00000400); + 0x40000004, 0x00000400, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_HV_EVMCS, - 0x40000004, 0x00004000); + 0x40000004, 0x00004000, 0x0); static virCPUx86Feature x86_kvm_features[] = { -- 2.20.1
On Tue, Aug 06, 2019 at 07:52:32AM +0200, Vitaly Kuznetsov wrote:
Some Hyper-V features (like the upcoming Direct Synthetic timers) are announced by feature bits in Edx but KVM_FEATURE_DEF() supports only Eax.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- src/cpu/cpu_x86.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Current code doesn't allow us to add sub-features as we always print the closing '/>'. As a preparatory change to implementing 'direct' sub-feature for 'stimer' feature switch to printing closing tag individually. No functional change. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- src/conf/domain_conf.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5cd9939031..f75ee03e9b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -28078,19 +28078,24 @@ virDomainDefFormatFeatures(virBufferPtr buf, case VIR_DOMAIN_HYPERV_TLBFLUSH: case VIR_DOMAIN_HYPERV_IPI: case VIR_DOMAIN_HYPERV_EVMCS: + virBufferAddLit(&childBuf, "/>\n"); break; case VIR_DOMAIN_HYPERV_SPINLOCKS: - if (def->hyperv_features[j] != VIR_TRISTATE_SWITCH_ON) + if (def->hyperv_features[j] != VIR_TRISTATE_SWITCH_ON) { + virBufferAddLit(&childBuf, "/>\n"); break; - virBufferAsprintf(&childBuf, " retries='%d'", + } + virBufferAsprintf(&childBuf, " retries='%d'/>\n", def->hyperv_spinlocks); break; case VIR_DOMAIN_HYPERV_VENDOR_ID: - if (def->hyperv_features[j] != VIR_TRISTATE_SWITCH_ON) + if (def->hyperv_features[j] != VIR_TRISTATE_SWITCH_ON) { + virBufferAddLit(&childBuf, "/>\n"); break; - virBufferEscapeString(&childBuf, " value='%s'", + } + virBufferEscapeString(&childBuf, " value='%s'/>\n", def->hyperv_vendor_id); break; @@ -28098,8 +28103,6 @@ virDomainDefFormatFeatures(virBufferPtr buf, case VIR_DOMAIN_HYPERV_LAST: break; } - - virBufferAddLit(&childBuf, "/>\n"); } virBufferAdjustIndent(&childBuf, -2); virBufferAddLit(&childBuf, "</hyperv>\n"); -- 2.20.1
On Tue, Aug 06, 2019 at 07:52:33AM +0200, Vitaly Kuznetsov wrote:
Current code doesn't allow us to add sub-features as we always print the closing '/>'. As a preparatory change to implementing 'direct' sub-feature for 'stimer' feature switch to printing closing tag individually.
No functional change.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- src/conf/domain_conf.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Support 'Direct Mode' for Hyper-V Synthetic Timers in domain config. Make it 'stimer' enlightenment option as it is not a separate thing. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- docs/formatdomain.html.in | 10 +++--- docs/schemas/domaincommon.rng | 16 ++++++++- src/conf/domain_conf.c | 62 ++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 1 + src/cpu/cpu_x86.c | 3 ++ src/cpu/cpu_x86_data.h | 2 ++ 6 files changed, 88 insertions(+), 6 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f5c882141a..de577e5755 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2033,7 +2033,9 @@ <vpindex state='on'/> <runtime state='on'/> <synic state='on'/> - <stimer state='on'/> + <stimer state='on'> + <direct state='on'/> + </stimer> <reset state='on'/> <vendor_id state='on' value='KVM Hv'/> <frequencies state='on'/> @@ -2148,9 +2150,9 @@ </tr> <tr> <td>stimer</td> - <td>Enable SynIC timers</td> - <td>on, off</td> - <td><span class="since">1.3.3 (QEMU 2.6)</span></td> + <td>Enable SynIC timers, optionally with Direct Mode support</td> + <td>on, off; direct - on,off</td> + <td><span class="since">1.3.3 (QEMU 2.6), direct mode 5.7.0 (QEMU 4.1)</span></td> </tr> <tr> <td>reset</td> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index a0771da45b..6707dcc634 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5907,7 +5907,7 @@ </optional> <optional> <element name="stimer"> - <ref name="featurestate"/> + <ref name="stimer"/> </element> </optional> <optional> @@ -5956,6 +5956,20 @@ </element> </define> + <!-- Hyper-V stimer features --> + <define name="stimer"> + <interleave> + <optional> + <ref name="featurestate"/> + </optional> + <optional> + <element name="direct"> + <ref name="featurestate"/> + </element> + </optional> + </interleave> + </define> + <!-- Optional KVM features --> <define name="kvm"> <element name="kvm"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f75ee03e9b..4d95be8c08 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -20388,6 +20388,39 @@ virDomainDefParseXML(xmlDocPtr xml, ctxt->node = node; } + if (def->features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) { + int value; + if ((n = virXPathNodeSet("./features/hyperv/stimer/*", ctxt, &nodes)) < 0) + goto error; + + for (i = 0; i < n; i++) { + if (STRNEQ((const char *)nodes[i]->name, "direct")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported Hyper-V stimer feature: %s"), + nodes[i]->name); + goto error; + } + + if (!(tmp = virXMLPropString(nodes[i], "state"))) { + virReportError(VIR_ERR_XML_ERROR, + _("missing 'state' attribute for " + "Hyper-V stimer '%s' feature"), "direct"); + goto error; + } + + if ((value = virTristateSwitchTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid value of state argument " + "for Hyper-V stimer '%s' feature"), "direct"); + goto error; + } + + VIR_FREE(tmp); + def->hyperv_stimer_direct = value; + } + VIR_FREE(nodes); + } + if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) { int feature; int value; @@ -22612,6 +22645,17 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, } } + if (src->hyperv_features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) { + if (src->hyperv_stimer_direct != dst->hyperv_stimer_direct) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("State of HyperV stimer direct feature differs: " + "source: '%s', destination: '%s'"), + virTristateSwitchTypeToString(src->hyperv_stimer_direct), + virTristateSwitchTypeToString(dst->hyperv_stimer_direct)); + return false; + } + } + /* kvm */ if (src->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) { for (i = 0; i < VIR_DOMAIN_KVM_LAST; i++) { @@ -28071,7 +28115,6 @@ virDomainDefFormatFeatures(virBufferPtr buf, case VIR_DOMAIN_HYPERV_VPINDEX: case VIR_DOMAIN_HYPERV_RUNTIME: case VIR_DOMAIN_HYPERV_SYNIC: - case VIR_DOMAIN_HYPERV_STIMER: case VIR_DOMAIN_HYPERV_RESET: case VIR_DOMAIN_HYPERV_FREQUENCIES: case VIR_DOMAIN_HYPERV_REENLIGHTENMENT: @@ -28090,6 +28133,23 @@ virDomainDefFormatFeatures(virBufferPtr buf, def->hyperv_spinlocks); break; + case VIR_DOMAIN_HYPERV_STIMER: + if (def->hyperv_features[j] != VIR_TRISTATE_SWITCH_ON) { + virBufferAddLit(&childBuf, "/>\n"); + break; + } + if (def->hyperv_stimer_direct == VIR_TRISTATE_SWITCH_ON) { + virBufferAddLit(&childBuf, ">\n"); + virBufferAdjustIndent(&childBuf, 2); + virBufferAddLit(&childBuf, "<direct state='on'/>\n"); + virBufferAdjustIndent(&childBuf, -2); + virBufferAddLit(&childBuf, "</stimer>\n"); + } else { + virBufferAddLit(&childBuf, "/>\n"); + } + + break; + case VIR_DOMAIN_HYPERV_VENDOR_ID: if (def->hyperv_features[j] != VIR_TRISTATE_SWITCH_ON) { virBufferAddLit(&childBuf, "/>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8a4425df64..7a2a9e16c0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2400,6 +2400,7 @@ struct _virDomainDef { int kvm_features[VIR_DOMAIN_KVM_LAST]; int msrs_features[VIR_DOMAIN_MSRS_LAST]; unsigned int hyperv_spinlocks; + int hyperv_stimer_direct; virGICVersion gic_version; virDomainHPTResizing hpt_resizing; unsigned long long hpt_maxpagesize; /* Stored in KiB */ diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index d2d9537c32..32b9836dd5 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -100,6 +100,8 @@ KVM_FEATURE_DEF(VIR_CPU_x86_HV_IPI, 0x40000004, 0x00000400, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_HV_EVMCS, 0x40000004, 0x00004000, 0x0); +KVM_FEATURE_DEF(VIR_CPU_x86_HV_STIMER_DIRECT, + 0x40000003, 0x0, 0x00080000); static virCPUx86Feature x86_kvm_features[] = { @@ -116,6 +118,7 @@ static virCPUx86Feature x86_kvm_features[] = KVM_FEATURE(VIR_CPU_x86_HV_TLBFLUSH), KVM_FEATURE(VIR_CPU_x86_HV_IPI), KVM_FEATURE(VIR_CPU_x86_HV_EVMCS), + KVM_FEATURE(VIR_CPU_x86_HV_STIMER_DIRECT), }; typedef struct _virCPUx86Model virCPUx86Model; diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h index cc0c93dff0..cebf3b6669 100644 --- a/src/cpu/cpu_x86_data.h +++ b/src/cpu/cpu_x86_data.h @@ -64,6 +64,8 @@ struct _virCPUx86MSR { #define VIR_CPU_x86_HV_IPI "hv-ipi" #define VIR_CPU_x86_HV_EVMCS "hv-evmcs" +/* Hyper-V Synthetic Timer option */ +#define VIR_CPU_x86_HV_STIMER_DIRECT "hv-stimer-direct" #define VIR_CPU_X86_DATA_INIT { 0 } -- 2.20.1
On Tue, Aug 06, 2019 at 07:52:34AM +0200, Vitaly Kuznetsov wrote:
Support 'Direct Mode' for Hyper-V Synthetic Timers in domain config. Make it 'stimer' enlightenment option as it is not a separate thing.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- docs/formatdomain.html.in | 10 +++--- docs/schemas/domaincommon.rng | 16 ++++++++- src/conf/domain_conf.c | 62 ++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 1 + src/cpu/cpu_x86.c | 3 ++ src/cpu/cpu_x86_data.h | 2 ++ 6 files changed, 88 insertions(+), 6 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
QEMU-4.1 supports 'Direct Mode' for Hyper-V synthetic timers (hv-stimer-direct CPU flag): Windows guests can request that timer expiration notifications are delivered as normal interrupts (and not VMBus messages). This is used by Hyper-V on KVM. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- src/qemu/qemu_command.c | 12 ++++++++---- src/qemu/qemu_process.c | 20 ++++++++++++++++++-- tests/qemuxml2argvdata/hyperv.args | 4 ++-- tests/qemuxml2argvdata/hyperv.xml | 4 +++- tests/qemuxml2xmloutdata/hyperv.xml | 4 +++- 5 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c8494de785..af913dba34 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7152,10 +7152,10 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, } if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) { - const char *hvPrefix = "hv-"; + const char *hvDelimiter = "-"; if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CANONICAL_CPU_FEATURES)) - hvPrefix = "hv_"; + hvDelimiter = "_"; for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) { switch ((virDomainHyperv) i) { @@ -7172,9 +7172,13 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, case VIR_DOMAIN_HYPERV_IPI: case VIR_DOMAIN_HYPERV_EVMCS: if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) - virBufferAsprintf(&buf, ",%s%s", - hvPrefix, + virBufferAsprintf(&buf, ",hv%s%s", + hvDelimiter, virDomainHypervTypeToString(i)); + if ((i == VIR_DOMAIN_HYPERV_STIMER) && + (def->hyperv_stimer_direct == VIR_TRISTATE_SWITCH_ON)) + virBufferAsprintf(&buf, ",hv%sstimer%sdirect", hvDelimiter, + hvDelimiter); break; case VIR_DOMAIN_HYPERV_SPINLOCKS: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1ed56457b1..792fa33327 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4112,10 +4112,26 @@ qemuProcessVerifyHypervFeatures(virDomainDefPtr def, rc = virCPUDataCheckFeature(cpu, cpuFeature); VIR_FREE(cpuFeature); - if (rc < 0) + if (rc < 0) { return -1; - else if (rc == 1) + } else if (rc == 1) { + if (i == VIR_DOMAIN_HYPERV_STIMER) { + if (def->hyperv_stimer_direct != VIR_TRISTATE_SWITCH_ON) + continue; + + rc = virCPUDataCheckFeature(cpu, "hv-stimer-direct"); + if (rc < 0) + return -1; + else if (rc == 1) + continue; + + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("host doesn't support hyperv stimer '%s' feature"), + "direct"); + return -1; + } continue; + } switch ((virDomainHyperv) i) { case VIR_DOMAIN_HYPERV_RELAXED: diff --git a/tests/qemuxml2argvdata/hyperv.args b/tests/qemuxml2argvdata/hyperv.args index 086adaa349..8040da9caa 100644 --- a/tests/qemuxml2argvdata/hyperv.args +++ b/tests/qemuxml2argvdata/hyperv.args @@ -12,8 +12,8 @@ QEMU_AUDIO_DRV=none \ -S \ -machine pc,accel=tcg,usb=off,dump-guest-core=off \ -cpu 'qemu32,hv_relaxed,hv_vapic,hv-spinlocks=0x2fff,hv_vpindex,hv_runtime,\ -hv_synic,hv_stimer,hv_reset,hv-vendor-id=KVM Hv,hv_frequencies,\ -hv_reenlightenment,hv_tlbflush,hv_ipi,hv_evmcs' \ +hv_synic,hv_stimer,hv_stimer_direct,hv_reset,hv-vendor-id=KVM Hv,\ +hv_frequencies,hv_reenlightenment,hv_tlbflush,hv_ipi,hv_evmcs' \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/hyperv.xml b/tests/qemuxml2argvdata/hyperv.xml index c6feaed528..ae0f934f76 100644 --- a/tests/qemuxml2argvdata/hyperv.xml +++ b/tests/qemuxml2argvdata/hyperv.xml @@ -17,7 +17,9 @@ <vpindex state='on'/> <runtime state='on'/> <synic state='on'/> - <stimer state='on'/> + <stimer state='on'> + <direct state='on'/> + </stimer> <reset state='on'/> <vendor_id state='on' value='KVM Hv'/> <frequencies state='on'/> diff --git a/tests/qemuxml2xmloutdata/hyperv.xml b/tests/qemuxml2xmloutdata/hyperv.xml index 5510d3dfad..2e4b43d4c6 100644 --- a/tests/qemuxml2xmloutdata/hyperv.xml +++ b/tests/qemuxml2xmloutdata/hyperv.xml @@ -17,7 +17,9 @@ <vpindex state='on'/> <runtime state='on'/> <synic state='on'/> - <stimer state='on'/> + <stimer state='on'> + <direct state='on'/> + </stimer> <reset state='on'/> <vendor_id state='on' value='KVM Hv'/> <frequencies state='on'/> -- 2.20.1
On Tue, Aug 06, 2019 at 07:52:35AM +0200, Vitaly Kuznetsov wrote:
QEMU-4.1 supports 'Direct Mode' for Hyper-V synthetic timers (hv-stimer-direct CPU flag): Windows guests can request that timer expiration notifications are delivered as normal interrupts (and not VMBus messages). This is used by Hyper-V on KVM.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- src/qemu/qemu_command.c | 12 ++++++++---- src/qemu/qemu_process.c | 20 ++++++++++++++++++-- tests/qemuxml2argvdata/hyperv.args | 4 ++-- tests/qemuxml2argvdata/hyperv.xml | 4 +++- tests/qemuxml2xmloutdata/hyperv.xml | 4 +++- 5 files changed, 34 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c8494de785..af913dba34 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7152,10 +7152,10 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, }
if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) { - const char *hvPrefix = "hv-"; + const char *hvDelimiter = "-";
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CANONICAL_CPU_FEATURES)) - hvPrefix = "hv_"; + hvDelimiter = "_";
for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) { switch ((virDomainHyperv) i) { @@ -7172,9 +7172,13 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, case VIR_DOMAIN_HYPERV_IPI: case VIR_DOMAIN_HYPERV_EVMCS: if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) - virBufferAsprintf(&buf, ",%s%s", - hvPrefix, + virBufferAsprintf(&buf, ",hv%s%s", + hvDelimiter, virDomainHypervTypeToString(i)); + if ((i == VIR_DOMAIN_HYPERV_STIMER) && + (def->hyperv_stimer_direct == VIR_TRISTATE_SWITCH_ON)) + virBufferAsprintf(&buf, ",hv%sstimer%sdirect", hvDelimiter, + hvDelimiter);
IIUC this feature will be introduced in QEMU 4.1.0 which is the one where we stop using the aliases with underscores, so this can be ",hv-stimer-direct" and the hvDelimiter changes are not needed. With that changed: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On Tue, Aug 06, 2019 at 01:25:37PM +0200, Ján Tomko wrote:
On Tue, Aug 06, 2019 at 07:52:35AM +0200, Vitaly Kuznetsov wrote:
QEMU-4.1 supports 'Direct Mode' for Hyper-V synthetic timers (hv-stimer-direct CPU flag): Windows guests can request that timer expiration notifications are delivered as normal interrupts (and not VMBus messages). This is used by Hyper-V on KVM.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- src/qemu/qemu_command.c | 12 ++++++++---- src/qemu/qemu_process.c | 20 ++++++++++++++++++-- tests/qemuxml2argvdata/hyperv.args | 4 ++-- tests/qemuxml2argvdata/hyperv.xml | 4 +++- tests/qemuxml2xmloutdata/hyperv.xml | 4 +++- 5 files changed, 34 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c8494de785..af913dba34 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7152,10 +7152,10 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, }
if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) { - const char *hvPrefix = "hv-"; + const char *hvDelimiter = "-";
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CANONICAL_CPU_FEATURES)) - hvPrefix = "hv_"; + hvDelimiter = "_";
for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) { switch ((virDomainHyperv) i) { @@ -7172,9 +7172,13 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, case VIR_DOMAIN_HYPERV_IPI: case VIR_DOMAIN_HYPERV_EVMCS: if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) - virBufferAsprintf(&buf, ",%s%s", - hvPrefix, + virBufferAsprintf(&buf, ",hv%s%s", + hvDelimiter, virDomainHypervTypeToString(i)); + if ((i == VIR_DOMAIN_HYPERV_STIMER) && + (def->hyperv_stimer_direct == VIR_TRISTATE_SWITCH_ON)) + virBufferAsprintf(&buf, ",hv%sstimer%sdirect", hvDelimiter, + hvDelimiter);
IIUC this feature will be introduced in QEMU 4.1.0 which is the one where we stop using the aliases with underscores, so this can be ",hv-stimer-direct" and the hvDelimiter changes are not needed.
Or even the VIR_CPU_x86_HV_STIMER_DIRECT string instead. One more thing, qemuxml2argvtest is using DO_TEST: DO_TEST("hyperv", NONE); DO_TEST("hyperv-off", NONE); DO_TEST("hyperv-panic", NONE); which manually enumarate the used QEMU capabilities (although there are none here) DO_TEST_CAPS_LATEST is the way to test against QEMU capabilites gathered from a real binary - if you use that macro for testing, it will have the QEMU_CAPS_CANONICAL_CPU_FEATURES capability and all the included features will use '-' instead of '_' as the separator. Jano
With that changed:
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Ján Tomko <jtomko@redhat.com> writes:
One more thing, qemuxml2argvtest is using DO_TEST: DO_TEST("hyperv", NONE); DO_TEST("hyperv-off", NONE); DO_TEST("hyperv-panic", NONE); which manually enumarate the used QEMU capabilities (although there are none here)
DO_TEST_CAPS_LATEST is the way to test against QEMU capabilites gathered from a real binary - if you use that macro for testing, it will have the QEMU_CAPS_CANONICAL_CPU_FEATURES capability and all the included features will use '-' instead of '_' as the separator.
Forgive me my ignorance (and long reply times), is the idea aimed at removing "hvPrefix" machinery from qemuBuildCpuCommandLine() completely - probably in favor of hardcorded '-' delimiters? I tried switching to DO_TEST_CAPS_LATEST in qemuxml2argvtest and after renaming tests/qemuxml2argvdata/hyperv{-off,-panic}.args to tests/qemuxml2argvdata/hyperv{,-off,-panic}.x86_64-latest.args to make the test run, I'm getting the following: In '/home/vitty/workspace/Upstream/libvirt/tests/qemuxml2argvdata/hyperv.x86_64-latest.args': Offset 292 Expect [QEMUGuest1 -S -machine pc,accel=tcg,usb=off,dump-guest-core=off -cpu 'qemu32,hv-relaxed,hv-vapic,hv-spinlocks=0x2fff,hv-vpindex,hv-runtime,hv-synic,hv-stimer,hv-reset,hv-vendor-id=KVM Hv,hv-frequencies,hv-reenlightenment,hv-tlbflush,hv-ipi,hv-evmcs' -m 214 -realtime mlock=off -smp 6,sockets=6,cores=1,threads=1 -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 -display none -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -usb] Actual [guest=QEMUGuest1,debug-threads=on -S -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes -machine pc,accel=tcg,usb=off,dump-guest-core=off -cpu 'qemu32,hv-relaxed,hv-vapic,hv-spinlocks=0x2fff,hv-vpindex,hv-runtime,hv-synic,hv-stimer,hv-reset,hv-vendor-id=KVM Hv,hv-frequencies,hv-reenlightenment,hv-tlbflush,hv-ipi,hv-evmcs' -m 214 -overcommit mem-lock=off -smp 6,sockets=6,cores=1,threads=1 -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 -display none -no-user-config -nodefaults -chardev socket,id=charmonitor,fd=1729,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -boot strict=on -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny -msg timestamp=on] I can, probably, add all the missing stuff but I'm not sure it's the right way to go in the first place :-) What if we just add QEMU_CAPS_CANONICAL_CPU_FEATURES to DO_TEST: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 71a36ff63a..f88d45f4e6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7152,11 +7152,6 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, } if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) { - const char *hvPrefix = "hv-"; - - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CANONICAL_CPU_FEATURES)) - hvPrefix = "hv_"; - for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) { switch ((virDomainHyperv) i) { case VIR_DOMAIN_HYPERV_RELAXED: @@ -7172,8 +7167,7 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, case VIR_DOMAIN_HYPERV_IPI: case VIR_DOMAIN_HYPERV_EVMCS: if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) - virBufferAsprintf(&buf, ",%s%s", - hvPrefix, + virBufferAsprintf(&buf, ",hv-%s", virDomainHypervTypeToString(i)); break; diff --git a/tests/qemuxml2argvdata/hyperv.args b/tests/qemuxml2argvdata/hyperv.args index 086adaa349..d09d8acdff 100644 --- a/tests/qemuxml2argvdata/hyperv.args +++ b/tests/qemuxml2argvdata/hyperv.args @@ -11,9 +11,9 @@ QEMU_AUDIO_DRV=none \ -name QEMUGuest1 \ -S \ -machine pc,accel=tcg,usb=off,dump-guest-core=off \ --cpu 'qemu32,hv_relaxed,hv_vapic,hv-spinlocks=0x2fff,hv_vpindex,hv_runtime,\ -hv_synic,hv_stimer,hv_reset,hv-vendor-id=KVM Hv,hv_frequencies,\ -hv_reenlightenment,hv_tlbflush,hv_ipi,hv_evmcs' \ +-cpu 'qemu32,hv-relaxed,hv-vapic,hv-spinlocks=0x2fff,hv-vpindex,hv-runtime,\ +hv-synic,hv-stimer,hv-reset,hv-vendor-id=KVM Hv,hv-frequencies,\ +hv-reenlightenment,hv-tlbflush,hv-ipi,hv-evmcs' \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c166fd18d6..d13c7c8b56 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -911,9 +911,9 @@ mymain(void) DO_TEST_CAPS_VER("kvmclock+eoi-disabled", "4.0.0"); DO_TEST_CAPS_LATEST("kvmclock+eoi-disabled"); - DO_TEST("hyperv", NONE); - DO_TEST("hyperv-off", NONE); - DO_TEST("hyperv-panic", NONE); + DO_TEST("hyperv", QEMU_CAPS_CANONICAL_CPU_FEATURES); + DO_TEST("hyperv-off", QEMU_CAPS_CANONICAL_CPU_FEATURES); + DO_TEST("hyperv-panic", QEMU_CAPS_CANONICAL_CPU_FEATURES); DO_TEST("kvm-features", NONE); DO_TEST("kvm-features-off", NONE); -- Vitaly
On Fri, Aug 09, 2019 at 12:13:45PM +0200, Vitaly Kuznetsov wrote:
Ján Tomko <jtomko@redhat.com> writes:
One more thing, qemuxml2argvtest is using DO_TEST: DO_TEST("hyperv", NONE); DO_TEST("hyperv-off", NONE); DO_TEST("hyperv-panic", NONE); which manually enumarate the used QEMU capabilities (although there are none here)
DO_TEST_CAPS_LATEST is the way to test against QEMU capabilites gathered from a real binary - if you use that macro for testing, it will have the QEMU_CAPS_CANONICAL_CPU_FEATURES capability and all the included features will use '-' instead of '_' as the separator.
Forgive me my ignorance (and long reply times), is the idea aimed at removing "hvPrefix" machinery from qemuBuildCpuCommandLine() completely - probably in favor of hardcorded '-' delimiters?
For new features that will not be used with QEMU <4.1, which does not have QEMU_CAPS_CANONICAL_CPU_FEATURES we should hardcode the version with the '-' delimiter.
I tried switching to DO_TEST_CAPS_LATEST in qemuxml2argvtest and after renaming tests/qemuxml2argvdata/hyperv{-off,-panic}.args to tests/qemuxml2argvdata/hyperv{,-off,-panic}.x86_64-latest.args to make the test run, I'm getting the following:
In '/home/vitty/workspace/Upstream/libvirt/tests/qemuxml2argvdata/hyperv.x86_64-latest.args': Offset 292 Expect [QEMUGuest1 -S -machine pc,accel=tcg,usb=off,dump-guest-core=off -cpu 'qemu32,hv-relaxed,hv-vapic,hv-spinlocks=0x2fff,hv-vpindex,hv-runtime,hv-synic,hv-stimer,hv-reset,hv-vendor-id=KVM Hv,hv-frequencies,hv-reenlightenment,hv-tlbflush,hv-ipi,hv-evmcs' -m 214 -realtime mlock=off -smp 6,sockets=6,cores=1,threads=1 -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 -display none -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -usb] Actual [guest=QEMUGuest1,debug-threads=on -S -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes -machine pc,accel=tcg,usb=off,dump-guest-core=off -cpu 'qemu32,hv-relaxed,hv-vapic,hv-spinlocks=0x2fff,hv-vpindex,hv-runtime,hv-synic,hv-stimer,hv-reset,hv-vendor-id=KVM Hv,hv-frequencies,hv-reenlightenment,hv-tlbflush,hv-ipi,hv-evmcs' -m 214 -overcommit mem-lock=off -smp 6,sockets=6,cores=1,threads=1 -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 -display none -no-user-config -nodefaults -chardev socket,id=charmonitor,fd=1729,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -boot strict=on -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny -msg timestamp=on]
From a glance all these changes look OK and are a result of using newer syntax for the same options. To easily regenerate the output, set the env variable VIR_TEST_REGENERATE_OUTPUT=1 before running the test.
I can, probably, add all the missing stuff but I'm not sure it's the right way to go in the first place :-)
What if we just add QEMU_CAPS_CANONICAL_CPU_FEATURES to DO_TEST:
We try to steer away from enumerating capabilites in the tests.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 71a36ff63a..f88d45f4e6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7152,11 +7152,6 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, }
if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) { - const char *hvPrefix = "hv-"; - - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CANONICAL_CPU_FEATURES)) - hvPrefix = "hv_"; -
IIUC we still need this code to deal with older QEMU. It's just new QEMUs where we can use the unified '-' delimiter.
for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) { switch ((virDomainHyperv) i) { case VIR_DOMAIN_HYPERV_RELAXED: @@ -7172,8 +7167,7 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, case VIR_DOMAIN_HYPERV_IPI: case VIR_DOMAIN_HYPERV_EVMCS: if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) - virBufferAsprintf(&buf, ",%s%s", - hvPrefix, + virBufferAsprintf(&buf, ",hv-%s", virDomainHypervTypeToString(i)); break;
diff --git a/tests/qemuxml2argvdata/hyperv.args b/tests/qemuxml2argvdata/hyperv.args index 086adaa349..d09d8acdff 100644 --- a/tests/qemuxml2argvdata/hyperv.args +++ b/tests/qemuxml2argvdata/hyperv.args @@ -11,9 +11,9 @@ QEMU_AUDIO_DRV=none \ -name QEMUGuest1 \ -S \ -machine pc,accel=tcg,usb=off,dump-guest-core=off \ --cpu 'qemu32,hv_relaxed,hv_vapic,hv-spinlocks=0x2fff,hv_vpindex,hv_runtime,\ -hv_synic,hv_stimer,hv_reset,hv-vendor-id=KVM Hv,hv_frequencies,\ -hv_reenlightenment,hv_tlbflush,hv_ipi,hv_evmcs' \ +-cpu 'qemu32,hv-relaxed,hv-vapic,hv-spinlocks=0x2fff,hv-vpindex,hv-runtime,\ +hv-synic,hv-stimer,hv-reset,hv-vendor-id=KVM Hv,hv-frequencies,\ +hv-reenlightenment,hv-tlbflush,hv-ipi,hv-evmcs' \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c166fd18d6..d13c7c8b56 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -911,9 +911,9 @@ mymain(void) DO_TEST_CAPS_VER("kvmclock+eoi-disabled", "4.0.0"); DO_TEST_CAPS_LATEST("kvmclock+eoi-disabled");
- DO_TEST("hyperv", NONE); - DO_TEST("hyperv-off", NONE); - DO_TEST("hyperv-panic", NONE);
This would remove the coverage of the syntax for old QEMUs. Either leave this as-is and add the new feature in a separate test, or you can branch the test based on QEMU versions like we do for other features: DO_TEST_CAPS_VER("pv-spinlock-disabled", "2.7.0"); DO_TEST_CAPS_VER("pv-spinlock-disabled", "4.0.0"); DO_TEST_CAPS_LATEST("pv-spinlock-disabled"); (After 2.7.0 we stopped using +|-feature in favor of feature=on|off, and 4.0.0 is the last one where we use underscores) Jano
+ DO_TEST("hyperv", QEMU_CAPS_CANONICAL_CPU_FEATURES); + DO_TEST("hyperv-off", QEMU_CAPS_CANONICAL_CPU_FEATURES); + DO_TEST("hyperv-panic", QEMU_CAPS_CANONICAL_CPU_FEATURES);
DO_TEST("kvm-features", NONE); DO_TEST("kvm-features-off", NONE);
-- Vitaly
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Ján Tomko <jtomko@redhat.com> writes:
Either leave this as-is and add the new feature in a separate test, or you can branch the test based on QEMU versions like we do for other features:
DO_TEST_CAPS_VER("pv-spinlock-disabled", "2.7.0"); DO_TEST_CAPS_VER("pv-spinlock-disabled", "4.0.0"); DO_TEST_CAPS_LATEST("pv-spinlock-disabled");
I think I slightly prefer a new test: this way we can test both <stimer state='on'/> and <stimer state='on'> <direct state='on'/> </stimer> (and it makes no sense to test hv-stimer-direct for old QEMU versions and 4.1 is "the latest" atm). Thank you for your review, v4 is comming soon (I promise)! -- Vitaly
The QEMU driver now supports Direct Mode for Hyper-V Synthetic timers for Hyper-V guests. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- docs/news.xml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index a4d727b9c5..d63fca3b48 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -41,6 +41,15 @@ <libvirt> <release version="v5.7.0" date="unreleased"> <section title="New features"> + <change> + <summary> + qemu: Support Direct Mode for Hyper-V Synthetic timers + </summary> + <description> + The QEMU driver now supports Direct Mode for Hyper-V Synthetic timers + for Hyper-V guests. + </description> + </change> </section> <section title="Improvements"> </section> -- 2.20.1
On Tue, Aug 06, 2019 at 07:52:36AM +0200, Vitaly Kuznetsov wrote:
The QEMU driver now supports Direct Mode for Hyper-V Synthetic timers for Hyper-V guests.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- docs/news.xml | 9 +++++++++ 1 file changed, 9 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On Tue, Aug 06, 2019 at 07:52:31AM +0200, Vitaly Kuznetsov wrote:
Changes since v2: - Post 5.6.0 rebase, patches 1-3 are dropped as they were already merged.
Original description:
QEMU-4.1 will bring us Direct Mode for Hyper-V Synthetic timers support, we need to support it in libvirt too. As this is not a new enlightenment but rather an enhancement of an existing one ('stimer'), support it in
<stimer state='on'> <direct state='on'/> </stimer>
form. Backwards compatibility is (hopefully) preserved.
Vitaly Kuznetsov (5): cpu_x86: add Edx to KVM_FEATURE_DEF() conf: change the way how Hyper-V features are printed out
Pushed the first two. Jano
conf: add support for Direct Mode for Hyper-V Synthetic timers qemu: add support for Direct Mode for Hyper-V Synthetic timers news: mention Direct Mode for Hyper-V Synthetic timers support
docs/formatdomain.html.in | 10 ++-- docs/news.xml | 9 ++++ docs/schemas/domaincommon.rng | 16 +++++- src/conf/domain_conf.c | 77 ++++++++++++++++++++++++++--- src/conf/domain_conf.h | 1 + src/cpu/cpu_x86.c | 33 +++++++------ src/cpu/cpu_x86_data.h | 2 + src/qemu/qemu_command.c | 12 +++-- src/qemu/qemu_process.c | 20 +++++++- tests/qemuxml2argvdata/hyperv.args | 4 +- tests/qemuxml2argvdata/hyperv.xml | 4 +- tests/qemuxml2xmloutdata/hyperv.xml | 4 +- 12 files changed, 155 insertions(+), 37 deletions(-)
-- 2.20.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
Ján Tomko -
Vitaly Kuznetsov