[libvirt] [PATCH 4/6] conf: add support for Direct Mode for Hyper-V Synthetic timers

Ján Tomko jtomko at redhat.com
Mon Jul 29 11:51:56 UTC 2019


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 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190729/a8ae6fd2/attachment-0001.sig>


More information about the libvir-list mailing list