[libvirt] [PATCH 0/6] Add support for Direct Mode for Hyper-V Synthetic timers

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. Because of an existing imcompatibility between libvirt-5.5+ and QEMU-4.1-git (see https://www.redhat.com/archives/libvir-list/2019-July/msg01435.html) I had to backport hv-stimer-direct to QEMU-4.0 to test this. If someone wants to do it too here is a list of QEMU patches needed: 128531d9e1 i386/kvm: add support for Direct Mode for Hyper-V synthetic timers 8caba36db5 i386/kvm: hv-evmcs requires hv-vapic bd59fbdf4f i386/kvm: hv-tlbflush/ipi require hv-vpindex c686193072 i386/kvm: hv-stimer requires hv-time and hv-synic e48ddcc6ce i386/kvm: implement 'hv-passthrough' mode fb19f72b77 i386/kvm: document existing Hyper-V enlightenments 2344d22e50 i386/kvm: move Hyper-V CPUID filling to hyperv_handle_properties() 6760bd2002 i386/kvm: add support for KVM_GET_SUPPORTED_HV_CPUID 2d384d7c83 i386/kvm: convert hyperv enlightenments properties from bools to bits Vitaly Kuznetsov (6): docs: formatdomain: add synic flag to the example xml docs: formatdomain: fix 'SynIC' spelling docs: formatdomain: move 'msrs' out of Hyper-V Enlightenments 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 | 13 ++- docs/news.xml | 9 ++ docs/schemas/domaincommon.rng | 16 +++- src/conf/domain_conf.c | 138 +++++++++++++++++++++++++--- src/conf/domain_conf.h | 8 ++ src/cpu/cpu_x86.c | 51 +++++----- src/cpu/cpu_x86_data.h | 2 + src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 22 ++++- src/qemu/qemu_process.c | 39 +++++++- tests/qemuxml2argvdata/hyperv.args | 4 +- tests/qemuxml2argvdata/hyperv.xml | 4 +- tests/qemuxml2xmloutdata/hyperv.xml | 4 +- 13 files changed, 263 insertions(+), 49 deletions(-) -- 2.20.1

The example XML we have contains all other Hyper-V Enlightenments but 'stimer' is missing. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- docs/formatdomain.html.in | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 1d57729394..0aedd9efbb 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2033,6 +2033,7 @@ <vpindex state='on'/> <runtime state='on'/> <synic state='on'/> + <stimer state='on'/> <reset state='on'/> <vendor_id state='on' value='KVM Hv'/> <frequencies state='on'/> -- 2.20.1

s/synic/stimer/ in the summary On Thu, Jul 25, 2019 at 03:52:13PM +0200, Vitaly Kuznetsov wrote:
The example XML we have contains all other Hyper-V Enlightenments but 'stimer' is missing.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- docs/formatdomain.html.in | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

SynIC stands for 'Synthetic Interrupt Controller', it is not a NIC. Fix the spelling in accordance with Hypervisor Top Level Functional Specification. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- docs/formatdomain.html.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0aedd9efbb..ee00aa9cbd 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2142,13 +2142,13 @@ </tr> <tr> <td>synic</td> - <td>Enable Synthetic Interrupt Controller (SyNIC)</td> + <td>Enable Synthetic Interrupt Controller (SynIC)</td> <td>on, off</td> <td><span class="since">1.3.3 (QEMU 2.6)</span></td> </tr> <tr> <td>stimer</td> - <td>Enable SyNIC timers</td> + <td>Enable SynIC timers</td> <td>on, off</td> <td><span class="since">1.3.3 (QEMU 2.6)</span></td> </tr> -- 2.20.1

On Thu, Jul 25, 2019 at 03:52:14PM +0200, Vitaly Kuznetsov wrote:
SynIC stands for 'Synthetic Interrupt Controller', it is not a NIC. Fix the spelling in accordance with Hypervisor Top Level Functional Specification.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- docs/formatdomain.html.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

'msrs' is a feature unrelated to Hyper-V Enlightenments, the commit message which added it and the test have it right: <features> ... <msrs unknown='ignore'> ... </features> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- docs/formatdomain.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index ee00aa9cbd..1aaddb6d9b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2041,7 +2041,6 @@ <tlbflush state='on'/> <ipi state='on'/> <evmcs state='on'/> - <msrs unknown='ignore'/> </hyperv> <kvm> <hidden state='on'/> @@ -2057,6 +2056,7 @@ <tseg unit='MiB'>48</tseg> </smm> <htm state='on'/> + <msrs unknown='ignore'/> </features> ...</pre> -- 2.20.1

On Thu, Jul 25, 2019 at 03:52:15PM +0200, Vitaly Kuznetsov wrote:
'msrs' is a feature unrelated to Hyper-V Enlightenments, the commit message which added it and the test have it right:
Introduced by: commit e9528f41c612fff334e958d5e5df197aa8e83792
<features> ... <msrs unknown='ignore'> ... </features>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- docs/formatdomain.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
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 | 138 +++++++++++++++++++++++++++++++--- src/conf/domain_conf.h | 8 ++ src/cpu/cpu_x86.c | 51 +++++++------ src/cpu/cpu_x86_data.h | 2 + src/libvirt_private.syms | 2 + 7 files changed, 187 insertions(+), 40 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 1aaddb6d9b..a0723edad1 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.6.0 (QEMU 4.1)</span></td> </tr> <tr> <td>reset</td> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 763480440c..8cf1995748 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5896,7 +5896,7 @@ </optional> <optional> <element name="stimer"> - <ref name="featurestate"/> + <ref name="stimer"/> </element> </optional> <optional> @@ -5945,6 +5945,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 0574c69a46..779b4ed880 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -197,6 +197,11 @@ VIR_ENUM_IMPL(virDomainHyperv, "evmcs", ); +VIR_ENUM_IMPL(virDomainHypervStimer, + VIR_DOMAIN_HYPERV_STIMER_LAST, + "direct", +); + VIR_ENUM_IMPL(virDomainKVM, VIR_DOMAIN_KVM_LAST, "hidden", @@ -20359,6 +20364,51 @@ virDomainDefParseXML(xmlDocPtr xml, ctxt->node = node; } + if (def->features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) { + int feature; + int value; + if ((n = virXPathNodeSet("./features/hyperv/stimer/*", ctxt, &nodes)) < 0) + goto error; + + for (i = 0; i < n; i++) { + feature = virDomainHypervStimerTypeFromString((const char *)nodes[i]->name); + if (feature < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported Hyper-V stimer feature: %s"), + nodes[i]->name); + goto error; + } + + switch ((virDomainHypervStimer) feature) { + case VIR_DOMAIN_HYPERV_STIMER_DIRECT: + if (!(tmp = virXMLPropString(nodes[i], "state"))) { + virReportError(VIR_ERR_XML_ERROR, + _("missing 'state' attribute for " + "Hyper-V stimer feature '%s'"), + nodes[i]->name); + goto error; + } + + if ((value = virTristateSwitchTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid value of state argument " + "for Hyper-V stimer feature '%s'"), + nodes[i]->name); + goto error; + } + + VIR_FREE(tmp); + def->hyperv_stimer_features[feature] = value; + break; + + /* coverity[dead_error_begin] */ + case VIR_DOMAIN_HYPERV_STIMER_LAST: + break; + } + } + VIR_FREE(nodes); + } + if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) { int feature; int value; @@ -22583,6 +22633,29 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, } } + if (src->hyperv_features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) { + for (i = 0; i < VIR_DOMAIN_HYPERV_STIMER_LAST; i++) { + switch ((virDomainHypervStimer) i) { + case VIR_DOMAIN_HYPERV_STIMER_DIRECT: + if (src->hyperv_stimer_features[i] != dst->hyperv_stimer_features[i]) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("State of HyperV stimer feature '%s' differs: " + "source: '%s', destination: '%s'"), + virDomainHypervStimerTypeToString(i), + virTristateSwitchTypeToString(src->hyperv_stimer_features[i]), + virTristateSwitchTypeToString(dst->hyperv_stimer_features[i])); + return false; + } + + break; + + /* coverity[dead_error_begin] */ + case VIR_DOMAIN_HYPERV_STIMER_LAST: + break; + } + } + } + /* kvm */ if (src->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) { for (i = 0; i < VIR_DOMAIN_KVM_LAST; i++) { @@ -28017,6 +28090,8 @@ virDomainDefFormatFeatures(virBufferPtr buf, virBufferAddLit(&childBuf, "<hyperv>\n"); virBufferAdjustIndent(&childBuf, 2); for (j = 0; j < VIR_DOMAIN_HYPERV_LAST; j++) { + size_t k; + if (def->hyperv_features[j] == VIR_TRISTATE_SWITCH_ABSENT) continue; @@ -28031,35 +28106,76 @@ 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: 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) + case VIR_DOMAIN_HYPERV_STIMER: + if (def->hyperv_features[j] != VIR_TRISTATE_SWITCH_ON) { + virBufferAddLit(&childBuf, "/>\n"); + break; + } + + for (k = 0; k < VIR_DOMAIN_HYPERV_STIMER_LAST; k++) { + if (def->hyperv_stimer_features[k]) + break; + } + + /* Omit long <stimer></stimer> form when no features are enabled */ + if (k == VIR_DOMAIN_HYPERV_STIMER_LAST) { + virBufferAddLit(&childBuf, "/>\n"); break; - virBufferAsprintf(&childBuf, " retries='%d'", - def->hyperv_spinlocks); + } + + virBufferAddLit(&childBuf, ">\n"); + virBufferAdjustIndent(&childBuf, 2); + + for (k = 0; k < VIR_DOMAIN_HYPERV_STIMER_LAST; k++) { + switch ((virDomainHypervStimer) k) { + case VIR_DOMAIN_HYPERV_STIMER_DIRECT: + if (def->hyperv_stimer_features[k]) + virBufferAsprintf(&childBuf, "<%s state='%s'/>\n", + virDomainHypervStimerTypeToString(k), + virTristateSwitchTypeToString( + def->hyperv_stimer_features[k])); + break; + + /* coverity[dead_error_begin] */ + case VIR_DOMAIN_HYPERV_STIMER_LAST: + break; + } + } + + virBufferAdjustIndent(&childBuf, -2); + virBufferAddLit(&childBuf, "</stimer>\n"); + + break; + + case VIR_DOMAIN_HYPERV_SPINLOCKS: + if (def->hyperv_features[j] == VIR_TRISTATE_SWITCH_ON) { + virBufferAsprintf(&childBuf, " retries='%d'", + def->hyperv_spinlocks); + } + virBufferAddLit(&childBuf, "/>\n"); break; case VIR_DOMAIN_HYPERV_VENDOR_ID: - if (def->hyperv_features[j] != VIR_TRISTATE_SWITCH_ON) - break; - virBufferEscapeString(&childBuf, " value='%s'", - def->hyperv_vendor_id); + if (def->hyperv_features[j] == VIR_TRISTATE_SWITCH_ON) { + virBufferEscapeString(&childBuf, " value='%s'", + def->hyperv_vendor_id); + } + virBufferAddLit(&childBuf, "/>\n"); break; /* coverity[dead_error_begin] */ case VIR_DOMAIN_HYPERV_LAST: break; } - - virBufferAddLit(&childBuf, "/>\n"); } virBufferAdjustIndent(&childBuf, -2); virBufferAddLit(&childBuf, "</hyperv>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 48b0af4b04..fc12887fc3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1762,6 +1762,12 @@ typedef enum { VIR_DOMAIN_HYPERV_LAST } virDomainHyperv; +typedef enum { + VIR_DOMAIN_HYPERV_STIMER_DIRECT = 0, + + VIR_DOMAIN_HYPERV_STIMER_LAST +} virDomainHypervStimer; + typedef enum { VIR_DOMAIN_KVM_HIDDEN = 0, @@ -2400,6 +2406,7 @@ struct _virDomainDef { int kvm_features[VIR_DOMAIN_KVM_LAST]; int msrs_features[VIR_DOMAIN_MSRS_LAST]; unsigned int hyperv_spinlocks; + int hyperv_stimer_features[VIR_DOMAIN_HYPERV_STIMER_LAST]; virGICVersion gic_version; virDomainHPTResizing hpt_resizing; unsigned long long hpt_maxpagesize; /* Stored in KiB */ @@ -3420,6 +3427,7 @@ VIR_ENUM_DECL(virDomainGraphicsSpiceStreamingMode); VIR_ENUM_DECL(virDomainGraphicsSpiceMouseMode); VIR_ENUM_DECL(virDomainGraphicsVNCSharePolicy); VIR_ENUM_DECL(virDomainHyperv); +VIR_ENUM_DECL(virDomainHypervStimer); VIR_ENUM_DECL(virDomainKVM); VIR_ENUM_DECL(virDomainMsrsUnknown); VIR_ENUM_DECL(virDomainRNGModel); diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 55b55da784..4fb9e6a4df 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,49 +74,51 @@ struct _virCPUx86Feature { } KVM_FEATURE_DEF(VIR_CPU_x86_KVM_CLOCKSOURCE, - 0x40000001, 0x00000001); + 0x40000001, 0x00000001, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_NOP_IO_DELAY, - 0x40000001, 0x00000002); + 0x40000001, 0x00000002, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_MMU_OP, - 0x40000001, 0x00000004); + 0x40000001, 0x00000004, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_CLOCKSOURCE2, - 0x40000001, 0x00000008); + 0x40000001, 0x00000008, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_ASYNC_PF, - 0x40000001, 0x00000010); + 0x40000001, 0x00000010, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_STEAL_TIME, - 0x40000001, 0x00000020); + 0x40000001, 0x00000020, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_PV_EOI, - 0x40000001, 0x00000040); + 0x40000001, 0x00000040, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_PV_UNHALT, - 0x40000001, 0x00000080); + 0x40000001, 0x00000080, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT, - 0x40000001, 0x01000000); + 0x40000001, 0x01000000, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_RUNTIME, - 0x40000003, 0x00000001); + 0x40000003, 0x00000001, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_SYNIC, - 0x40000003, 0x00000004); + 0x40000003, 0x00000004, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_STIMER, - 0x40000003, 0x00000008); + 0x40000003, 0x00000008, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_RELAXED, - 0x40000003, 0x00000020); + 0x40000003, 0x00000020, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_SPINLOCKS, - 0x40000003, 0x00000022); + 0x40000003, 0x00000022, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_VAPIC, - 0x40000003, 0x00000030); + 0x40000003, 0x00000030, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_VPINDEX, - 0x40000003, 0x00000040); + 0x40000003, 0x00000040, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_RESET, - 0x40000003, 0x00000080); + 0x40000003, 0x00000080, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_FREQUENCIES, - 0x40000003, 0x00000800); + 0x40000003, 0x00000800, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_REENLIGHTENMENT, - 0x40000003, 0x00002000); + 0x40000003, 0x00002000, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_TLBFLUSH, - 0x40000004, 0x00000004); + 0x40000004, 0x00000004, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_IPI, - 0x40000004, 0x00000400); + 0x40000004, 0x00000400, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_EVMCS, - 0x40000004, 0x00004000); + 0x40000004, 0x00004000, 0x0); +KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_STIMER_DIRECT, + 0x40000003, 0x0, 0x00080000); static virCPUx86Feature x86_kvm_features[] = { @@ -142,6 +144,7 @@ static virCPUx86Feature x86_kvm_features[] = KVM_FEATURE(VIR_CPU_x86_KVM_HV_TLBFLUSH), KVM_FEATURE(VIR_CPU_x86_KVM_HV_IPI), KVM_FEATURE(VIR_CPU_x86_KVM_HV_EVMCS), + KVM_FEATURE(VIR_CPU_x86_KVM_HV_STIMER_DIRECT), }; typedef struct _virCPUx86Model virCPUx86Model; diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h index f3f4d7ab9c..198f19e037 100644 --- a/src/cpu/cpu_x86_data.h +++ b/src/cpu/cpu_x86_data.h @@ -72,6 +72,8 @@ struct _virCPUx86MSR { #define VIR_CPU_x86_KVM_HV_IPI "__kvm_hv_ipi" #define VIR_CPU_x86_KVM_HV_EVMCS "__kvm_hv_evmcs" +/* Hyper-V Synthetic Timer (virDomainHypervStimer) features */ +#define VIR_CPU_x86_KVM_HV_STIMER_DIRECT "__kvm_hv_stimer_direct" #define VIR_CPU_X86_DATA_INIT { 0 } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dff97bd82a..ef8e6cd183 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -418,6 +418,8 @@ virDomainHostdevSubsysTypeToString; virDomainHPTResizingTypeToString; virDomainHubTypeFromString; virDomainHubTypeToString; +virDomainHypervStimerTypeFromString; +virDomainHypervStimerTypeToString; virDomainHypervTypeFromString; virDomainHypervTypeToString; virDomainInputBusTypeToString; -- 2.20.1

On Thu, Jul 25, 2019 at 03:52:16PM +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 | 138 +++++++++++++++++++++++++++++++--- src/conf/domain_conf.h | 8 ++ src/cpu/cpu_x86.c | 51 +++++++------ src/cpu/cpu_x86_data.h | 2 + src/libvirt_private.syms | 2 + 7 files changed, 187 insertions(+), 40 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 1aaddb6d9b..a0723edad1 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.6.0 (QEMU 4.1)</span></td> </tr> <tr> <td>reset</td> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 763480440c..8cf1995748 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5896,7 +5896,7 @@ </optional> <optional> <element name="stimer"> - <ref name="featurestate"/> + <ref name="stimer"/> </element> </optional> <optional> @@ -5945,6 +5945,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 0574c69a46..779b4ed880 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -197,6 +197,11 @@ VIR_ENUM_IMPL(virDomainHyperv, "evmcs", );
+VIR_ENUM_IMPL(virDomainHypervStimer, + VIR_DOMAIN_HYPERV_STIMER_LAST, + "direct", +);
Do you anticipate more stimer "sub"-features in the future? Having an enum with one value just to loop over an array with one element and then switch()-ing across all the possible value seems like overkill.
+ VIR_ENUM_IMPL(virDomainKVM, VIR_DOMAIN_KVM_LAST, "hidden", @@ -20359,6 +20364,51 @@ virDomainDefParseXML(xmlDocPtr xml, ctxt->node = node; }
+ if (def->features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) { + int feature; + int value; + if ((n = virXPathNodeSet("./features/hyperv/stimer/*", ctxt, &nodes)) < 0) + goto error; + + for (i = 0; i < n; i++) { + feature = virDomainHypervStimerTypeFromString((const char *)nodes[i]->name); + if (feature < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported Hyper-V stimer feature: %s"), + nodes[i]->name); + goto error; + } + + switch ((virDomainHypervStimer) feature) { + case VIR_DOMAIN_HYPERV_STIMER_DIRECT: + if (!(tmp = virXMLPropString(nodes[i], "state"))) { + virReportError(VIR_ERR_XML_ERROR, + _("missing 'state' attribute for " + "Hyper-V stimer feature '%s'"), + nodes[i]->name); + goto error; + } + + if ((value = virTristateSwitchTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid value of state argument " + "for Hyper-V stimer feature '%s'"), + nodes[i]->name); + goto error; + } + + VIR_FREE(tmp); + def->hyperv_stimer_features[feature] = value; + break; + + /* coverity[dead_error_begin] */ + case VIR_DOMAIN_HYPERV_STIMER_LAST: + break; + } + } + VIR_FREE(nodes); + } + if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) { int feature; int value; @@ -22583,6 +22633,29 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, } }
+ if (src->hyperv_features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) { + for (i = 0; i < VIR_DOMAIN_HYPERV_STIMER_LAST; i++) { + switch ((virDomainHypervStimer) i) { + case VIR_DOMAIN_HYPERV_STIMER_DIRECT: + if (src->hyperv_stimer_features[i] != dst->hyperv_stimer_features[i]) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("State of HyperV stimer feature '%s' differs: " + "source: '%s', destination: '%s'"), + virDomainHypervStimerTypeToString(i), + virTristateSwitchTypeToString(src->hyperv_stimer_features[i]), + virTristateSwitchTypeToString(dst->hyperv_stimer_features[i])); + return false; + } + + break; + + /* coverity[dead_error_begin] */ + case VIR_DOMAIN_HYPERV_STIMER_LAST: + break; + } + } + } + /* kvm */ if (src->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) { for (i = 0; i < VIR_DOMAIN_KVM_LAST; i++) { @@ -28017,6 +28090,8 @@ virDomainDefFormatFeatures(virBufferPtr buf, virBufferAddLit(&childBuf, "<hyperv>\n"); virBufferAdjustIndent(&childBuf, 2); for (j = 0; j < VIR_DOMAIN_HYPERV_LAST; j++) { + size_t k; + if (def->hyperv_features[j] == VIR_TRISTATE_SWITCH_ABSENT) continue;
@@ -28031,35 +28106,76 @@ 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: case VIR_DOMAIN_HYPERV_TLBFLUSH: case VIR_DOMAIN_HYPERV_IPI: case VIR_DOMAIN_HYPERV_EVMCS: + virBufferAddLit(&childBuf, "/>\n");
Changing all the cases to print the ending tag themselves in a separate commit first would make this one look nicer.
break;
- case VIR_DOMAIN_HYPERV_SPINLOCKS: - if (def->hyperv_features[j] != VIR_TRISTATE_SWITCH_ON) + case VIR_DOMAIN_HYPERV_STIMER: + if (def->hyperv_features[j] != VIR_TRISTATE_SWITCH_ON) { + virBufferAddLit(&childBuf, "/>\n"); + break; + } + diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 48b0af4b04..fc12887fc3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1762,6 +1762,12 @@ typedef enum { VIR_DOMAIN_HYPERV_LAST } virDomainHyperv;
+typedef enum { + VIR_DOMAIN_HYPERV_STIMER_DIRECT = 0, + + VIR_DOMAIN_HYPERV_STIMER_LAST +} virDomainHypervStimer; + typedef enum { VIR_DOMAIN_KVM_HIDDEN = 0,
@@ -2400,6 +2406,7 @@ struct _virDomainDef { int kvm_features[VIR_DOMAIN_KVM_LAST]; int msrs_features[VIR_DOMAIN_MSRS_LAST]; unsigned int hyperv_spinlocks; + int hyperv_stimer_features[VIR_DOMAIN_HYPERV_STIMER_LAST];
How about: int hyperv_stimer_direct;
virGICVersion gic_version; virDomainHPTResizing hpt_resizing; unsigned long long hpt_maxpagesize; /* Stored in KiB */ @@ -3420,6 +3427,7 @@ VIR_ENUM_DECL(virDomainGraphicsSpiceStreamingMode); VIR_ENUM_DECL(virDomainGraphicsSpiceMouseMode); VIR_ENUM_DECL(virDomainGraphicsVNCSharePolicy); VIR_ENUM_DECL(virDomainHyperv); +VIR_ENUM_DECL(virDomainHypervStimer); VIR_ENUM_DECL(virDomainKVM); VIR_ENUM_DECL(virDomainMsrsUnknown); VIR_ENUM_DECL(virDomainRNGModel); diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 55b55da784..4fb9e6a4df 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), \
Another change that can be separated.
}
#define KVM_FEATURE(Name) \ @@ -74,49 +74,51 @@ struct _virCPUx86Feature { }
KVM_FEATURE_DEF(VIR_CPU_x86_KVM_CLOCKSOURCE, - 0x40000001, 0x00000001); + 0x40000001, 0x00000001, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_NOP_IO_DELAY, - 0x40000001, 0x00000002); + 0x40000001, 0x00000002, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_MMU_OP, - 0x40000001, 0x00000004); + 0x40000001, 0x00000004, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_CLOCKSOURCE2, - 0x40000001, 0x00000008); + 0x40000001, 0x00000008, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_ASYNC_PF, - 0x40000001, 0x00000010); + 0x40000001, 0x00000010, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_STEAL_TIME, - 0x40000001, 0x00000020); + 0x40000001, 0x00000020, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_PV_EOI, - 0x40000001, 0x00000040); + 0x40000001, 0x00000040, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_PV_UNHALT, - 0x40000001, 0x00000080); + 0x40000001, 0x00000080, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT, - 0x40000001, 0x01000000); + 0x40000001, 0x01000000, 0x0);
Take a look at Jirka's series which removes most of these.
KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_RUNTIME, - 0x40000003, 0x00000001); + 0x40000003, 0x00000001, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_SYNIC, - 0x40000003, 0x00000004); + 0x40000003, 0x00000004, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_STIMER, - 0x40000003, 0x00000008); + 0x40000003, 0x00000008, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_RELAXED, - 0x40000003, 0x00000020); + 0x40000003, 0x00000020, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_SPINLOCKS, - 0x40000003, 0x00000022); + 0x40000003, 0x00000022, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_VAPIC, - 0x40000003, 0x00000030); + 0x40000003, 0x00000030, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_VPINDEX, - 0x40000003, 0x00000040); + 0x40000003, 0x00000040, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_RESET, - 0x40000003, 0x00000080); + 0x40000003, 0x00000080, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_FREQUENCIES, - 0x40000003, 0x00000800); + 0x40000003, 0x00000800, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_REENLIGHTENMENT, - 0x40000003, 0x00002000); + 0x40000003, 0x00002000, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_TLBFLUSH, - 0x40000004, 0x00000004); + 0x40000004, 0x00000004, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_IPI, - 0x40000004, 0x00000400); + 0x40000004, 0x00000400, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_EVMCS, - 0x40000004, 0x00004000); + 0x40000004, 0x00004000, 0x0); +KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_STIMER_DIRECT, + 0x40000003, 0x0, 0x00080000);
static virCPUx86Feature x86_kvm_features[] = { @@ -142,6 +144,7 @@ static virCPUx86Feature x86_kvm_features[] = KVM_FEATURE(VIR_CPU_x86_KVM_HV_TLBFLUSH), KVM_FEATURE(VIR_CPU_x86_KVM_HV_IPI), KVM_FEATURE(VIR_CPU_x86_KVM_HV_EVMCS), + KVM_FEATURE(VIR_CPU_x86_KVM_HV_STIMER_DIRECT), };
typedef struct _virCPUx86Model virCPUx86Model; diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h index f3f4d7ab9c..198f19e037 100644 --- a/src/cpu/cpu_x86_data.h +++ b/src/cpu/cpu_x86_data.h @@ -72,6 +72,8 @@ struct _virCPUx86MSR { #define VIR_CPU_x86_KVM_HV_IPI "__kvm_hv_ipi" #define VIR_CPU_x86_KVM_HV_EVMCS "__kvm_hv_evmcs"
+/* Hyper-V Synthetic Timer (virDomainHypervStimer) features */ +#define VIR_CPU_x86_KVM_HV_STIMER_DIRECT "__kvm_hv_stimer_direct"
"hv-stimer-direct" to correctly detect its availability
#define VIR_CPU_X86_DATA_INIT { 0 }
Jano

Ján Tomko <jtomko@redhat.com> writes:
On Thu, Jul 25, 2019 at 03:52:16PM +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 | 138 +++++++++++++++++++++++++++++++--- src/conf/domain_conf.h | 8 ++ src/cpu/cpu_x86.c | 51 +++++++------ src/cpu/cpu_x86_data.h | 2 + src/libvirt_private.syms | 2 + 7 files changed, 187 insertions(+), 40 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 1aaddb6d9b..a0723edad1 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.6.0 (QEMU 4.1)</span></td> </tr> <tr> <td>reset</td> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 763480440c..8cf1995748 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5896,7 +5896,7 @@ </optional> <optional> <element name="stimer"> - <ref name="featurestate"/> + <ref name="stimer"/> </element> </optional> <optional> @@ -5945,6 +5945,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 0574c69a46..779b4ed880 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -197,6 +197,11 @@ VIR_ENUM_IMPL(virDomainHyperv, "evmcs", );
+VIR_ENUM_IMPL(virDomainHypervStimer, + VIR_DOMAIN_HYPERV_STIMER_LAST, + "direct", +);
Do you anticipate more stimer "sub"-features in the future? Having an enum with one value just to loop over an array with one element and then switch()-ing across all the possible value seems like overkill.
I don't anticipate any sub-features for stimer for the time being, however, I wanted to make code look like what we already have (e.g. virDomainKVM). We can, of course, simplify things if we consider 'direct' being the one and only.
+ VIR_ENUM_IMPL(virDomainKVM, VIR_DOMAIN_KVM_LAST, "hidden", @@ -20359,6 +20364,51 @@ virDomainDefParseXML(xmlDocPtr xml, ctxt->node = node; }
+ if (def->features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) { + int feature; + int value; + if ((n = virXPathNodeSet("./features/hyperv/stimer/*", ctxt, &nodes)) < 0) + goto error; + + for (i = 0; i < n; i++) { + feature = virDomainHypervStimerTypeFromString((const char *)nodes[i]->name); + if (feature < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported Hyper-V stimer feature: %s"), + nodes[i]->name); + goto error; + } + + switch ((virDomainHypervStimer) feature) { + case VIR_DOMAIN_HYPERV_STIMER_DIRECT: + if (!(tmp = virXMLPropString(nodes[i], "state"))) { + virReportError(VIR_ERR_XML_ERROR, + _("missing 'state' attribute for " + "Hyper-V stimer feature '%s'"), + nodes[i]->name); + goto error; + } + + if ((value = virTristateSwitchTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid value of state argument " + "for Hyper-V stimer feature '%s'"), + nodes[i]->name); + goto error; + } + + VIR_FREE(tmp); + def->hyperv_stimer_features[feature] = value; + break; + + /* coverity[dead_error_begin] */ + case VIR_DOMAIN_HYPERV_STIMER_LAST: + break; + } + } + VIR_FREE(nodes); + } + if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) { int feature; int value; @@ -22583,6 +22633,29 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, } }
+ if (src->hyperv_features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) { + for (i = 0; i < VIR_DOMAIN_HYPERV_STIMER_LAST; i++) { + switch ((virDomainHypervStimer) i) { + case VIR_DOMAIN_HYPERV_STIMER_DIRECT: + if (src->hyperv_stimer_features[i] != dst->hyperv_stimer_features[i]) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("State of HyperV stimer feature '%s' differs: " + "source: '%s', destination: '%s'"), + virDomainHypervStimerTypeToString(i), + virTristateSwitchTypeToString(src->hyperv_stimer_features[i]), + virTristateSwitchTypeToString(dst->hyperv_stimer_features[i])); + return false; + } + + break; + + /* coverity[dead_error_begin] */ + case VIR_DOMAIN_HYPERV_STIMER_LAST: + break; + } + } + } + /* kvm */ if (src->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) { for (i = 0; i < VIR_DOMAIN_KVM_LAST; i++) { @@ -28017,6 +28090,8 @@ virDomainDefFormatFeatures(virBufferPtr buf, virBufferAddLit(&childBuf, "<hyperv>\n"); virBufferAdjustIndent(&childBuf, 2); for (j = 0; j < VIR_DOMAIN_HYPERV_LAST; j++) { + size_t k; + if (def->hyperv_features[j] == VIR_TRISTATE_SWITCH_ABSENT) continue;
@@ -28031,35 +28106,76 @@ 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: case VIR_DOMAIN_HYPERV_TLBFLUSH: case VIR_DOMAIN_HYPERV_IPI: case VIR_DOMAIN_HYPERV_EVMCS: + virBufferAddLit(&childBuf, "/>\n");
Changing all the cases to print the ending tag themselves in a separate commit first would make this one look nicer.
Ok.
break;
- case VIR_DOMAIN_HYPERV_SPINLOCKS: - if (def->hyperv_features[j] != VIR_TRISTATE_SWITCH_ON) + case VIR_DOMAIN_HYPERV_STIMER: + if (def->hyperv_features[j] != VIR_TRISTATE_SWITCH_ON) { + virBufferAddLit(&childBuf, "/>\n"); + break; + } + diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 48b0af4b04..fc12887fc3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1762,6 +1762,12 @@ typedef enum { VIR_DOMAIN_HYPERV_LAST } virDomainHyperv;
+typedef enum { + VIR_DOMAIN_HYPERV_STIMER_DIRECT = 0, + + VIR_DOMAIN_HYPERV_STIMER_LAST +} virDomainHypervStimer; + typedef enum { VIR_DOMAIN_KVM_HIDDEN = 0,
@@ -2400,6 +2406,7 @@ struct _virDomainDef { int kvm_features[VIR_DOMAIN_KVM_LAST]; int msrs_features[VIR_DOMAIN_MSRS_LAST]; unsigned int hyperv_spinlocks; + int hyperv_stimer_features[VIR_DOMAIN_HYPERV_STIMER_LAST];
How about: int hyperv_stimer_direct;
Yes, if we decide to drop the virDomainHypervStimer.
virGICVersion gic_version; virDomainHPTResizing hpt_resizing; unsigned long long hpt_maxpagesize; /* Stored in KiB */ @@ -3420,6 +3427,7 @@ VIR_ENUM_DECL(virDomainGraphicsSpiceStreamingMode); VIR_ENUM_DECL(virDomainGraphicsSpiceMouseMode); VIR_ENUM_DECL(virDomainGraphicsVNCSharePolicy); VIR_ENUM_DECL(virDomainHyperv); +VIR_ENUM_DECL(virDomainHypervStimer); VIR_ENUM_DECL(virDomainKVM); VIR_ENUM_DECL(virDomainMsrsUnknown); VIR_ENUM_DECL(virDomainRNGModel); diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 55b55da784..4fb9e6a4df 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), \
Another change that can be separated.
Yes if we still need it) I haven't looked at Jirka's series to check if we still check CPUID for Hyper-V features or we only check QEMU command line now.
}
#define KVM_FEATURE(Name) \ @@ -74,49 +74,51 @@ struct _virCPUx86Feature { }
KVM_FEATURE_DEF(VIR_CPU_x86_KVM_CLOCKSOURCE, - 0x40000001, 0x00000001); + 0x40000001, 0x00000001, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_NOP_IO_DELAY, - 0x40000001, 0x00000002); + 0x40000001, 0x00000002, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_MMU_OP, - 0x40000001, 0x00000004); + 0x40000001, 0x00000004, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_CLOCKSOURCE2, - 0x40000001, 0x00000008); + 0x40000001, 0x00000008, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_ASYNC_PF, - 0x40000001, 0x00000010); + 0x40000001, 0x00000010, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_STEAL_TIME, - 0x40000001, 0x00000020); + 0x40000001, 0x00000020, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_PV_EOI, - 0x40000001, 0x00000040); + 0x40000001, 0x00000040, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_PV_UNHALT, - 0x40000001, 0x00000080); + 0x40000001, 0x00000080, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT, - 0x40000001, 0x01000000); + 0x40000001, 0x01000000, 0x0);
Take a look at Jirka's series which removes most of these.
KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_RUNTIME, - 0x40000003, 0x00000001); + 0x40000003, 0x00000001, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_SYNIC, - 0x40000003, 0x00000004); + 0x40000003, 0x00000004, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_STIMER, - 0x40000003, 0x00000008); + 0x40000003, 0x00000008, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_RELAXED, - 0x40000003, 0x00000020); + 0x40000003, 0x00000020, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_SPINLOCKS, - 0x40000003, 0x00000022); + 0x40000003, 0x00000022, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_VAPIC, - 0x40000003, 0x00000030); + 0x40000003, 0x00000030, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_VPINDEX, - 0x40000003, 0x00000040); + 0x40000003, 0x00000040, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_RESET, - 0x40000003, 0x00000080); + 0x40000003, 0x00000080, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_FREQUENCIES, - 0x40000003, 0x00000800); + 0x40000003, 0x00000800, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_REENLIGHTENMENT, - 0x40000003, 0x00002000); + 0x40000003, 0x00002000, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_TLBFLUSH, - 0x40000004, 0x00000004); + 0x40000004, 0x00000004, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_IPI, - 0x40000004, 0x00000400); + 0x40000004, 0x00000400, 0x0); KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_EVMCS, - 0x40000004, 0x00004000); + 0x40000004, 0x00004000, 0x0); +KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_STIMER_DIRECT, + 0x40000003, 0x0, 0x00080000);
static virCPUx86Feature x86_kvm_features[] = { @@ -142,6 +144,7 @@ static virCPUx86Feature x86_kvm_features[] = KVM_FEATURE(VIR_CPU_x86_KVM_HV_TLBFLUSH), KVM_FEATURE(VIR_CPU_x86_KVM_HV_IPI), KVM_FEATURE(VIR_CPU_x86_KVM_HV_EVMCS), + KVM_FEATURE(VIR_CPU_x86_KVM_HV_STIMER_DIRECT), };
typedef struct _virCPUx86Model virCPUx86Model; diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h index f3f4d7ab9c..198f19e037 100644 --- a/src/cpu/cpu_x86_data.h +++ b/src/cpu/cpu_x86_data.h @@ -72,6 +72,8 @@ struct _virCPUx86MSR { #define VIR_CPU_x86_KVM_HV_IPI "__kvm_hv_ipi" #define VIR_CPU_x86_KVM_HV_EVMCS "__kvm_hv_evmcs"
+/* Hyper-V Synthetic Timer (virDomainHypervStimer) features */ +#define VIR_CPU_x86_KVM_HV_STIMER_DIRECT "__kvm_hv_stimer_direct"
"hv-stimer-direct"
to correctly detect its availability
Yes, I'll rebase on top of Jirka's series. Thanks!
#define VIR_CPU_X86_DATA_INIT { 0 }
Jano
-- Vitaly

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 | 22 ++++++++++++++-- src/qemu/qemu_process.c | 39 +++++++++++++++++++++++++++-- tests/qemuxml2argvdata/hyperv.args | 4 +-- tests/qemuxml2argvdata/hyperv.xml | 4 ++- tests/qemuxml2xmloutdata/hyperv.xml | 4 ++- 5 files changed, 65 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1cf165079f..12229e879e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7096,7 +7096,7 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, int ret = -1; virBuffer cpu_buf = VIR_BUFFER_INITIALIZER; virBuffer buf = VIR_BUFFER_INITIALIZER; - size_t i; + size_t i, j; if (def->cpu && (def->cpu->mode != VIR_CPU_MODE_CUSTOM || def->cpu->model)) { @@ -7158,7 +7158,6 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, 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: @@ -7170,6 +7169,25 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, virDomainHypervTypeToString(i)); break; + case VIR_DOMAIN_HYPERV_STIMER: + if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) + virBufferAsprintf(&buf, ",hv_%s", + virDomainHypervTypeToString(i)); + for (j = 0; j < VIR_DOMAIN_HYPERV_STIMER_LAST; j++) { + switch ((virDomainHypervStimer) j) { + case VIR_DOMAIN_HYPERV_STIMER_DIRECT: + if (def->hyperv_stimer_features[j] == VIR_TRISTATE_SWITCH_ON) + virBufferAsprintf(&buf, ",hv_stimer_%s", + virDomainHypervStimerTypeToString(j)); + break; + + /* coverity[dead_error_begin] */ + case VIR_DOMAIN_HYPERV_STIMER_LAST: + break; + } + } + break; + case VIR_DOMAIN_HYPERV_SPINLOCKS: if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) virBufferAsprintf(&buf, ",hv_spinlocks=0x%x", diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 75205bc121..0235cfd022 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4111,10 +4111,45 @@ 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)) { + size_t j; + + for (j = 0; j < VIR_DOMAIN_HYPERV_STIMER_LAST; j++) { + switch ((virDomainHypervStimer) j) { + case VIR_DOMAIN_HYPERV_STIMER_DIRECT: + if (def->hyperv_stimer_features[j] != VIR_TRISTATE_SWITCH_ON) + continue; + + if (virAsprintf(&cpuFeature, "__kvm_hv_stimer_%s", + virDomainHypervStimerTypeToString(j)) < 0) + return -1; + + rc = virCPUDataCheckFeature(cpu, cpuFeature); + VIR_FREE(cpuFeature); + + if (rc < 0) + return -1; + else if (rc == 1) + continue; + + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("host doesn't support hyperv stimer '%s' feature"), + virDomainHypervStimerTypeToString(i)); + return -1; + + /* coverity[dead_error_begin] */ + case VIR_DOMAIN_HYPERV_STIMER_LAST: + break; + } + } + + } + continue; + } switch ((virDomainHyperv) i) { case VIR_DOMAIN_HYPERV_RELAXED: diff --git a/tests/qemuxml2argvdata/hyperv.args b/tests/qemuxml2argvdata/hyperv.args index c557b6d8fe..a8f47ebb21 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 Thu, Jul 25, 2019 at 03:52:17PM +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 | 22 ++++++++++++++-- src/qemu/qemu_process.c | 39 +++++++++++++++++++++++++++-- tests/qemuxml2argvdata/hyperv.args | 4 +-- tests/qemuxml2argvdata/hyperv.xml | 4 ++- tests/qemuxml2xmloutdata/hyperv.xml | 4 ++- 5 files changed, 65 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1cf165079f..12229e879e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7096,7 +7096,7 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, int ret = -1; virBuffer cpu_buf = VIR_BUFFER_INITIALIZER; virBuffer buf = VIR_BUFFER_INITIALIZER; - size_t i; + size_t i, j;
if (def->cpu && (def->cpu->mode != VIR_CPU_MODE_CUSTOM || def->cpu->model)) { @@ -7158,7 +7158,6 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, 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: @@ -7170,6 +7169,25 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, virDomainHypervTypeToString(i)); break;
+ case VIR_DOMAIN_HYPERV_STIMER: + if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) + virBufferAsprintf(&buf, ",hv_%s", + virDomainHypervTypeToString(i));
+ for (j = 0; j < VIR_DOMAIN_HYPERV_STIMER_LAST; j++) { + switch ((virDomainHypervStimer) j) { + case VIR_DOMAIN_HYPERV_STIMER_DIRECT: + if (def->hyperv_stimer_features[j] == VIR_TRISTATE_SWITCH_ON) + virBufferAsprintf(&buf, ",hv_stimer_%s", + virDomainHypervStimerTypeToString(j)); + break; + + /* coverity[dead_error_begin] */ + case VIR_DOMAIN_HYPERV_STIMER_LAST: + break; + } + }
if (def->hyperv_stimer_direct == VIR_TRISTATE_SWITCH_ON) virBufferAddLit(&buf, "hv-stimer-direct");
+ break; + case VIR_DOMAIN_HYPERV_SPINLOCKS: if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) virBufferAsprintf(&buf, ",hv_spinlocks=0x%x", diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 75205bc121..0235cfd022 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4111,10 +4111,45 @@ 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)) { + size_t j; +
Fails compilation with my Clang: qemu/qemu_process.c:4117:20: error: equality comparison with extraneous parentheses [-Werror,-Wparentheses-equality] if ((i == VIR_DOMAIN_HYPERV_STIMER)) { ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ qemu/qemu_process.c:4117:20: note: remove extraneous parentheses around the comparison to silence this warning if ((i == VIR_DOMAIN_HYPERV_STIMER)) { ~ ^ ~ qemu/qemu_process.c:4117:20: note: use '=' to turn this equality comparison into an assignment if ((i == VIR_DOMAIN_HYPERV_STIMER)) { ^~ = 1 error generated. make[5]: *** [Makefile:11191: qemu/libvirt_driver_qemu_impl_la-qemu_process.lo] Error 1
+ for (j = 0; j < VIR_DOMAIN_HYPERV_STIMER_LAST; j++) { + switch ((virDomainHypervStimer) j) { + case VIR_DOMAIN_HYPERV_STIMER_DIRECT: + if (def->hyperv_stimer_features[j] != VIR_TRISTATE_SWITCH_ON) + continue; + + if (virAsprintf(&cpuFeature, "__kvm_hv_stimer_%s", + virDomainHypervStimerTypeToString(j)) < 0) + return -1; + + rc = virCPUDataCheckFeature(cpu, cpuFeature); + VIR_FREE(cpuFeature); + + if (rc < 0) + return -1; + else if (rc == 1) + continue; + + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("host doesn't support hyperv stimer '%s' feature"), + virDomainHypervStimerTypeToString(i)); + return -1; + + /* coverity[dead_error_begin] */ + case VIR_DOMAIN_HYPERV_STIMER_LAST: + break; + } + } + + } + continue; + }
switch ((virDomainHyperv) i) { case VIR_DOMAIN_HYPERV_RELAXED:
Jano

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 1134309ec2..e4d8f1ecac 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -54,6 +54,15 @@ <libvirt> <release version="v5.6.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> <change> <summary> qemu: Introduce a new video model of type 'bochs' -- 2.20.1
participants (2)
-
Ján Tomko
-
Vitaly Kuznetsov