All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/9] i386/kvm/hyper-v: refactor and implement 'hv-stimer-direct' and 'hv-passthrough' enlightenments
@ 2019-05-17 14:19 Vitaly Kuznetsov
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 1/9] i386/kvm: convert hyperv enlightenments properties from bools to bits Vitaly Kuznetsov
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Vitaly Kuznetsov @ 2019-05-17 14:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P . Berrangé,
	Eduardo Habkost, Marcelo Tosatti, Dr . David Alan Gilbert,
	Roman Kagan, Paolo Bonzini, Richard Henderson

It has been a while since my 'v1' and I (again) apologize for that.

Changes since v1:
- Existing Hyper-V properties are converted from BOOL to BIT64, this allows
  us to express dependencies between them in a more natural way as well as
  simplifies search in kvm_hyperv_properties [Roman Kagan] (hope I got the
  idea right, but in any case this should be an improvement). PATCH1 added
  to the series.
- 'hv-all' renamed to 'hv-passthrough' [Roman Kagan]
- minor changes mostly related to support the addition of PATCH1.

Original description:

The recently introduced Direct Mode for Hyper-V synthetic timers
enlightenment is only exposed through KVM_GET_SUPPORTED_HV_CPUID ioctl.
Take the opportunity and re-implement the way we handle Hyper-V
enlightenments in QEMU, add support for hv-stimer-direct and 'hv-all'
pass-through mode, add missing dependencies between enlightenments.

Vitaly Kuznetsov (9):
  i386/kvm: convert hyperv enlightenments properties from bools to bits
  i386/kvm: add support for KVM_GET_SUPPORTED_HV_CPUID
  i386/kvm: move Hyper-V CPUID filling to hyperv_handle_properties()
  i386/kvm: document existing Hyper-V enlightenments
  i386/kvm: implement 'hv-passthrough' mode
  i386/kvm: hv-stimer requires hv-time and hv-synic
  i386/kvm: hv-tlbflush/ipi require hv-vpindex
  i386/kvm: hv-evmcs requires hv-vapic
  i386/kvm: add support for Direct Mode for Hyper-V synthetic timers

 docs/hyperv.txt            | 201 ++++++++++
 hw/i386/pc.c               |   3 +-
 target/i386/cpu.c          |  47 ++-
 target/i386/cpu.h          |  39 +-
 target/i386/hyperv-proto.h |   1 +
 target/i386/hyperv.c       |   2 +-
 target/i386/kvm.c          | 770 ++++++++++++++++++++++++++-----------
 target/i386/machine.c      |   2 +-
 8 files changed, 813 insertions(+), 252 deletions(-)
 create mode 100644 docs/hyperv.txt

-- 
2.20.1



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v2 1/9] i386/kvm: convert hyperv enlightenments properties from bools to bits
  2019-05-17 14:19 [Qemu-devel] [PATCH v2 0/9] i386/kvm/hyper-v: refactor and implement 'hv-stimer-direct' and 'hv-passthrough' enlightenments Vitaly Kuznetsov
@ 2019-05-17 14:19 ` Vitaly Kuznetsov
  2019-05-27 15:46   ` Roman Kagan
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 2/9] i386/kvm: add support for KVM_GET_SUPPORTED_HV_CPUID Vitaly Kuznetsov
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Vitaly Kuznetsov @ 2019-05-17 14:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P . Berrangé,
	Eduardo Habkost, Marcelo Tosatti, Dr . David Alan Gilbert,
	Roman Kagan, Paolo Bonzini, Richard Henderson

Representing Hyper-V properties as bits will allow us to check features
and dependencies between them in a natural way.

Suggested-by: Roman Kagan <rkagan@virtuozzo.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 hw/i386/pc.c          |  3 +-
 target/i386/cpu.c     | 44 +++++++++++++++--------
 target/i386/cpu.h     | 37 +++++++++++--------
 target/i386/hyperv.c  |  2 +-
 target/i386/kvm.c     | 83 ++++++++++++++++++-------------------------
 target/i386/machine.c |  2 +-
 6 files changed, 91 insertions(+), 80 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d98b737b8f..77c479e667 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2387,7 +2387,8 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
     }
     cpu->thread_id = topo.smt_id;
 
-    if (cpu->hyperv_vpindex && !kvm_hv_vpindex_settable()) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX) &&
+        !kvm_hv_vpindex_settable()) {
         error_setg(errp, "kernel doesn't allow setting HyperV VP_INDEX");
         return;
     }
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 722c5514d4..9530b28d42 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5819,21 +5819,37 @@ static Property x86_cpu_properties[] = {
 #endif
     DEFINE_PROP_INT32("node-id", X86CPU, node_id, CPU_UNSET_NUMA_NODE_ID),
     DEFINE_PROP_BOOL("pmu", X86CPU, enable_pmu, false),
+
     { .name  = "hv-spinlocks", .info  = &qdev_prop_spinlocks },
-    DEFINE_PROP_BOOL("hv-relaxed", X86CPU, hyperv_relaxed_timing, false),
-    DEFINE_PROP_BOOL("hv-vapic", X86CPU, hyperv_vapic, false),
-    DEFINE_PROP_BOOL("hv-time", X86CPU, hyperv_time, false),
-    DEFINE_PROP_BOOL("hv-crash", X86CPU, hyperv_crash, false),
-    DEFINE_PROP_BOOL("hv-reset", X86CPU, hyperv_reset, false),
-    DEFINE_PROP_BOOL("hv-vpindex", X86CPU, hyperv_vpindex, false),
-    DEFINE_PROP_BOOL("hv-runtime", X86CPU, hyperv_runtime, false),
-    DEFINE_PROP_BOOL("hv-synic", X86CPU, hyperv_synic, false),
-    DEFINE_PROP_BOOL("hv-stimer", X86CPU, hyperv_stimer, false),
-    DEFINE_PROP_BOOL("hv-frequencies", X86CPU, hyperv_frequencies, false),
-    DEFINE_PROP_BOOL("hv-reenlightenment", X86CPU, hyperv_reenlightenment, false),
-    DEFINE_PROP_BOOL("hv-tlbflush", X86CPU, hyperv_tlbflush, false),
-    DEFINE_PROP_BOOL("hv-evmcs", X86CPU, hyperv_evmcs, false),
-    DEFINE_PROP_BOOL("hv-ipi", X86CPU, hyperv_ipi, false),
+    DEFINE_PROP_BIT64("hv-relaxed", X86CPU, hyperv_features,
+                      HYPERV_FEAT_RELAXED, 0),
+    DEFINE_PROP_BIT64("hv-vapic", X86CPU, hyperv_features,
+                      HYPERV_FEAT_VAPIC, 0),
+    DEFINE_PROP_BIT64("hv-time", X86CPU, hyperv_features,
+                      HYPERV_FEAT_TIME, 0),
+    DEFINE_PROP_BIT64("hv-crash", X86CPU, hyperv_features,
+                      HYPERV_FEAT_CRASH, 0),
+    DEFINE_PROP_BIT64("hv-reset", X86CPU, hyperv_features,
+                      HYPERV_FEAT_RESET, 0),
+    DEFINE_PROP_BIT64("hv-vpindex", X86CPU, hyperv_features,
+                      HYPERV_FEAT_VPINDEX, 0),
+    DEFINE_PROP_BIT64("hv-runtime", X86CPU, hyperv_features,
+                      HYPERV_FEAT_RUNTIME, 0),
+    DEFINE_PROP_BIT64("hv-synic", X86CPU, hyperv_features,
+                      HYPERV_FEAT_SYNIC, 0),
+    DEFINE_PROP_BIT64("hv-stimer", X86CPU, hyperv_features,
+                      HYPERV_FEAT_STIMER, 0),
+    DEFINE_PROP_BIT64("hv-frequencies", X86CPU, hyperv_features,
+                      HYPERV_FEAT_FREQUENCIES, 0),
+    DEFINE_PROP_BIT64("hv-reenlightenment", X86CPU, hyperv_features,
+                      HYPERV_FEAT_REENLIGHTENMENT, 0),
+    DEFINE_PROP_BIT64("hv-tlbflush", X86CPU, hyperv_features,
+                      HYPERV_FEAT_TLBFLUSH, 0),
+    DEFINE_PROP_BIT64("hv-evmcs", X86CPU, hyperv_features,
+                      HYPERV_FEAT_EVMCS, 0),
+    DEFINE_PROP_BIT64("hv-ipi", X86CPU, hyperv_features,
+                      HYPERV_FEAT_IPI, 0),
+
     DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
     DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
     DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 0128910661..11fa9e643e 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -743,6 +743,22 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
 #define MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY (1U << 3)
 #define MSR_ARCH_CAP_SSB_NO     (1U << 4)
 
+/* Supported Hyper-V Enlightenments */
+#define HYPERV_FEAT_RELAXED             0
+#define HYPERV_FEAT_VAPIC               1
+#define HYPERV_FEAT_TIME                2
+#define HYPERV_FEAT_CRASH               3
+#define HYPERV_FEAT_RESET               4
+#define HYPERV_FEAT_VPINDEX             5
+#define HYPERV_FEAT_RUNTIME             6
+#define HYPERV_FEAT_SYNIC               7
+#define HYPERV_FEAT_STIMER              8
+#define HYPERV_FEAT_FREQUENCIES         9
+#define HYPERV_FEAT_REENLIGHTENMENT     10
+#define HYPERV_FEAT_TLBFLUSH            11
+#define HYPERV_FEAT_EVMCS               12
+#define HYPERV_FEAT_IPI                 13
+
 #ifndef HYPERV_SPINLOCK_NEVER_RETRY
 #define HYPERV_SPINLOCK_NEVER_RETRY             0xFFFFFFFF
 #endif
@@ -1381,23 +1397,11 @@ struct X86CPU {
 
     CPUX86State env;
 
-    bool hyperv_vapic;
-    bool hyperv_relaxed_timing;
     int hyperv_spinlock_attempts;
     char *hyperv_vendor_id;
-    bool hyperv_time;
-    bool hyperv_crash;
-    bool hyperv_reset;
-    bool hyperv_vpindex;
-    bool hyperv_runtime;
-    bool hyperv_synic;
     bool hyperv_synic_kvm_only;
-    bool hyperv_stimer;
-    bool hyperv_frequencies;
-    bool hyperv_reenlightenment;
-    bool hyperv_tlbflush;
-    bool hyperv_evmcs;
-    bool hyperv_ipi;
+    uint64_t hyperv_features;
+
     bool check_cpuid;
     bool enforce_cpuid;
     bool expose_kvm;
@@ -1934,4 +1938,9 @@ void x86_cpu_xrstor_all_areas(X86CPU *cpu, const X86XSaveArea *buf);
 void x86_cpu_xsave_all_areas(X86CPU *cpu, X86XSaveArea *buf);
 void x86_update_hflags(CPUX86State* env);
 
+static inline bool hyperv_feat_enabled(X86CPU *cpu, int feat)
+{
+    return !!(cpu->hyperv_features & BIT(feat));
+}
+
 #endif /* I386_CPU_H */
diff --git a/target/i386/hyperv.c b/target/i386/hyperv.c
index b264a28620..26efc1e0e6 100644
--- a/target/i386/hyperv.c
+++ b/target/i386/hyperv.c
@@ -52,7 +52,7 @@ int kvm_hv_handle_exit(X86CPU *cpu, struct kvm_hyperv_exit *exit)
 
     switch (exit->type) {
     case KVM_EXIT_HYPERV_SYNIC:
-        if (!cpu->hyperv_synic) {
+        if (!hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC)) {
             return -1;
         }
 
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 3b29ce5c0d..3daac1e4f4 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -635,28 +635,12 @@ unsigned long kvm_arch_vcpu_id(CPUState *cs)
 #define KVM_CPUID_SIGNATURE_NEXT                0x40000100
 #endif
 
-static bool hyperv_hypercall_available(X86CPU *cpu)
-{
-    return cpu->hyperv_vapic ||
-           (cpu->hyperv_spinlock_attempts != HYPERV_SPINLOCK_NEVER_RETRY);
-}
-
 static bool hyperv_enabled(X86CPU *cpu)
 {
     CPUState *cs = CPU(cpu);
     return kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV) > 0 &&
-           (hyperv_hypercall_available(cpu) ||
-            cpu->hyperv_time  ||
-            cpu->hyperv_relaxed_timing ||
-            cpu->hyperv_crash ||
-            cpu->hyperv_reset ||
-            cpu->hyperv_vpindex ||
-            cpu->hyperv_runtime ||
-            cpu->hyperv_synic ||
-            cpu->hyperv_stimer ||
-            cpu->hyperv_reenlightenment ||
-            cpu->hyperv_tlbflush ||
-            cpu->hyperv_ipi);
+        ((cpu->hyperv_spinlock_attempts != HYPERV_SPINLOCK_NEVER_RETRY) ||
+         cpu->hyperv_features);
 }
 
 static int kvm_arch_set_tsc_khz(CPUState *cs)
@@ -705,14 +689,14 @@ static int hyperv_handle_properties(CPUState *cs)
     X86CPU *cpu = X86_CPU(cs);
     CPUX86State *env = &cpu->env;
 
-    if (cpu->hyperv_relaxed_timing) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_RELAXED)) {
         env->features[FEAT_HYPERV_EAX] |= HV_HYPERCALL_AVAILABLE;
     }
-    if (cpu->hyperv_vapic) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC)) {
         env->features[FEAT_HYPERV_EAX] |= HV_HYPERCALL_AVAILABLE;
         env->features[FEAT_HYPERV_EAX] |= HV_APIC_ACCESS_AVAILABLE;
     }
-    if (cpu->hyperv_time) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_TIME)) {
         if (kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_TIME) <= 0) {
             fprintf(stderr, "Hyper-V clocksources "
                     "(requested by 'hv-time' cpu flag) "
@@ -723,7 +707,7 @@ static int hyperv_handle_properties(CPUState *cs)
         env->features[FEAT_HYPERV_EAX] |= HV_TIME_REF_COUNT_AVAILABLE;
         env->features[FEAT_HYPERV_EAX] |= HV_REFERENCE_TSC_AVAILABLE;
     }
-    if (cpu->hyperv_frequencies) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_FREQUENCIES)) {
         if (!has_msr_hv_frequencies) {
             fprintf(stderr, "Hyper-V frequency MSRs "
                     "(requested by 'hv-frequencies' cpu flag) "
@@ -733,7 +717,7 @@ static int hyperv_handle_properties(CPUState *cs)
         env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS;
         env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE;
     }
-    if (cpu->hyperv_crash) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_CRASH)) {
         if (!has_msr_hv_crash) {
             fprintf(stderr, "Hyper-V crash MSRs "
                     "(requested by 'hv-crash' cpu flag) "
@@ -742,7 +726,7 @@ static int hyperv_handle_properties(CPUState *cs)
         }
         env->features[FEAT_HYPERV_EDX] |= HV_GUEST_CRASH_MSR_AVAILABLE;
     }
-    if (cpu->hyperv_reenlightenment) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_REENLIGHTENMENT)) {
         if (!has_msr_hv_reenlightenment) {
             fprintf(stderr,
                     "Hyper-V Reenlightenment MSRs "
@@ -753,7 +737,7 @@ static int hyperv_handle_properties(CPUState *cs)
         env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_REENLIGHTENMENTS_CONTROL;
     }
     env->features[FEAT_HYPERV_EDX] |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
-    if (cpu->hyperv_reset) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_RESET)) {
         if (!has_msr_hv_reset) {
             fprintf(stderr, "Hyper-V reset MSR "
                     "(requested by 'hv-reset' cpu flag) "
@@ -762,7 +746,7 @@ static int hyperv_handle_properties(CPUState *cs)
         }
         env->features[FEAT_HYPERV_EAX] |= HV_RESET_AVAILABLE;
     }
-    if (cpu->hyperv_vpindex) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
         if (!has_msr_hv_vpindex) {
             fprintf(stderr, "Hyper-V VP_INDEX MSR "
                     "(requested by 'hv-vpindex' cpu flag) "
@@ -771,7 +755,7 @@ static int hyperv_handle_properties(CPUState *cs)
         }
         env->features[FEAT_HYPERV_EAX] |= HV_VP_INDEX_AVAILABLE;
     }
-    if (cpu->hyperv_runtime) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_RUNTIME)) {
         if (!has_msr_hv_runtime) {
             fprintf(stderr, "Hyper-V VP_RUNTIME MSR "
                     "(requested by 'hv-runtime' cpu flag) "
@@ -780,10 +764,10 @@ static int hyperv_handle_properties(CPUState *cs)
         }
         env->features[FEAT_HYPERV_EAX] |= HV_VP_RUNTIME_AVAILABLE;
     }
-    if (cpu->hyperv_synic) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC)) {
         unsigned int cap = KVM_CAP_HYPERV_SYNIC;
         if (!cpu->hyperv_synic_kvm_only) {
-            if (!cpu->hyperv_vpindex) {
+            if (!hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
                 fprintf(stderr, "Hyper-V SynIC "
                         "(requested by 'hv-synic' cpu flag) "
                         "requires Hyper-V VP_INDEX ('hv-vpindex')\n");
@@ -800,20 +784,20 @@ static int hyperv_handle_properties(CPUState *cs)
 
         env->features[FEAT_HYPERV_EAX] |= HV_SYNIC_AVAILABLE;
     }
-    if (cpu->hyperv_stimer) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_STIMER)) {
         if (!has_msr_hv_stimer) {
             fprintf(stderr, "Hyper-V timers aren't supported by kernel\n");
             return -ENOSYS;
         }
         env->features[FEAT_HYPERV_EAX] |= HV_SYNTIMERS_AVAILABLE;
     }
-    if (cpu->hyperv_relaxed_timing) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_RELAXED)) {
         env->features[FEAT_HV_RECOMM_EAX] |= HV_RELAXED_TIMING_RECOMMENDED;
     }
-    if (cpu->hyperv_vapic) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC)) {
         env->features[FEAT_HV_RECOMM_EAX] |= HV_APIC_ACCESS_RECOMMENDED;
     }
-    if (cpu->hyperv_tlbflush) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_TLBFLUSH)) {
         if (kvm_check_extension(cs->kvm_state,
                                 KVM_CAP_HYPERV_TLBFLUSH) <= 0) {
             fprintf(stderr, "Hyper-V TLB flush support "
@@ -824,7 +808,7 @@ static int hyperv_handle_properties(CPUState *cs)
         env->features[FEAT_HV_RECOMM_EAX] |= HV_REMOTE_TLB_FLUSH_RECOMMENDED;
         env->features[FEAT_HV_RECOMM_EAX] |= HV_EX_PROCESSOR_MASKS_RECOMMENDED;
     }
-    if (cpu->hyperv_ipi) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_IPI)) {
         if (kvm_check_extension(cs->kvm_state,
                                 KVM_CAP_HYPERV_SEND_IPI) <= 0) {
             fprintf(stderr, "Hyper-V IPI send support "
@@ -835,7 +819,7 @@ static int hyperv_handle_properties(CPUState *cs)
         env->features[FEAT_HV_RECOMM_EAX] |= HV_CLUSTER_IPI_RECOMMENDED;
         env->features[FEAT_HV_RECOMM_EAX] |= HV_EX_PROCESSOR_MASKS_RECOMMENDED;
     }
-    if (cpu->hyperv_evmcs) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS)) {
         uint16_t evmcs_version;
 
         if (kvm_vcpu_enable_cap(cs, KVM_CAP_HYPERV_ENLIGHTENED_VMCS, 0,
@@ -857,7 +841,7 @@ static int hyperv_init_vcpu(X86CPU *cpu)
     CPUState *cs = CPU(cpu);
     int ret;
 
-    if (cpu->hyperv_vpindex && !hv_vpindex_settable) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX) && !hv_vpindex_settable) {
         /*
          * the kernel doesn't support setting vp_index; assert that its value
          * is in sync
@@ -882,7 +866,7 @@ static int hyperv_init_vcpu(X86CPU *cpu)
         }
     }
 
-    if (cpu->hyperv_synic) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC)) {
         uint32_t synic_cap = cpu->hyperv_synic_kvm_only ?
             KVM_CAP_HYPERV_SYNIC : KVM_CAP_HYPERV_SYNIC2;
         ret = kvm_vcpu_enable_cap(cs, synic_cap, 0);
@@ -973,7 +957,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
             memset(signature, 0, 12);
             memcpy(signature, cpu->hyperv_vendor_id, len);
         }
-        c->eax = cpu->hyperv_evmcs ?
+        c->eax = hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS) ?
             HV_CPUID_NESTED_FEATURES : HV_CPUID_IMPLEMENT_LIMITS;
         c->ebx = signature[0];
         c->ecx = signature[1];
@@ -1017,7 +1001,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
         kvm_base = KVM_CPUID_SIGNATURE_NEXT;
         has_msr_hv_hypercall = true;
 
-        if (cpu->hyperv_evmcs) {
+        if (hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS)) {
             __u32 function;
 
             /* Create zeroed 0x40000006..0x40000009 leaves */
@@ -1361,7 +1345,7 @@ void kvm_arch_reset_vcpu(X86CPU *cpu)
         env->mp_state = KVM_MP_STATE_RUNNABLE;
     }
 
-    if (cpu->hyperv_synic) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC)) {
         int i;
         for (i = 0; i < ARRAY_SIZE(env->msr_hv_synic_sint); i++) {
             env->msr_hv_synic_sint[i] = HV_SINT_MASKED;
@@ -2101,11 +2085,11 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
                 kvm_msr_entry_add(cpu, HV_X64_MSR_HYPERCALL,
                                   env->msr_hv_hypercall);
             }
-            if (cpu->hyperv_time) {
+            if (hyperv_feat_enabled(cpu, HYPERV_FEAT_TIME)) {
                 kvm_msr_entry_add(cpu, HV_X64_MSR_REFERENCE_TSC,
                                   env->msr_hv_tsc);
             }
-            if (cpu->hyperv_reenlightenment) {
+            if (hyperv_feat_enabled(cpu, HYPERV_FEAT_REENLIGHTENMENT)) {
                 kvm_msr_entry_add(cpu, HV_X64_MSR_REENLIGHTENMENT_CONTROL,
                                   env->msr_hv_reenlightenment_control);
                 kvm_msr_entry_add(cpu, HV_X64_MSR_TSC_EMULATION_CONTROL,
@@ -2114,7 +2098,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
                                   env->msr_hv_tsc_emulation_status);
             }
         }
-        if (cpu->hyperv_vapic) {
+        if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC)) {
             kvm_msr_entry_add(cpu, HV_X64_MSR_APIC_ASSIST_PAGE,
                               env->msr_hv_vapic);
         }
@@ -2130,11 +2114,12 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
         if (has_msr_hv_runtime) {
             kvm_msr_entry_add(cpu, HV_X64_MSR_VP_RUNTIME, env->msr_hv_runtime);
         }
-        if (cpu->hyperv_vpindex && hv_vpindex_settable) {
+        if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)
+            && hv_vpindex_settable) {
             kvm_msr_entry_add(cpu, HV_X64_MSR_VP_INDEX,
                               hyperv_vp_index(CPU(cpu)));
         }
-        if (cpu->hyperv_synic) {
+        if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC)) {
             int j;
 
             kvm_msr_entry_add(cpu, HV_X64_MSR_SVERSION, HV_SYNIC_VERSION);
@@ -2474,13 +2459,13 @@ static int kvm_get_msrs(X86CPU *cpu)
         kvm_msr_entry_add(cpu, HV_X64_MSR_HYPERCALL, 0);
         kvm_msr_entry_add(cpu, HV_X64_MSR_GUEST_OS_ID, 0);
     }
-    if (cpu->hyperv_vapic) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC)) {
         kvm_msr_entry_add(cpu, HV_X64_MSR_APIC_ASSIST_PAGE, 0);
     }
-    if (cpu->hyperv_time) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_TIME)) {
         kvm_msr_entry_add(cpu, HV_X64_MSR_REFERENCE_TSC, 0);
     }
-    if (cpu->hyperv_reenlightenment) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_REENLIGHTENMENT)) {
         kvm_msr_entry_add(cpu, HV_X64_MSR_REENLIGHTENMENT_CONTROL, 0);
         kvm_msr_entry_add(cpu, HV_X64_MSR_TSC_EMULATION_CONTROL, 0);
         kvm_msr_entry_add(cpu, HV_X64_MSR_TSC_EMULATION_STATUS, 0);
@@ -2495,7 +2480,7 @@ static int kvm_get_msrs(X86CPU *cpu)
     if (has_msr_hv_runtime) {
         kvm_msr_entry_add(cpu, HV_X64_MSR_VP_RUNTIME, 0);
     }
-    if (cpu->hyperv_synic) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC)) {
         uint32_t msr;
 
         kvm_msr_entry_add(cpu, HV_X64_MSR_SCONTROL, 0);
diff --git a/target/i386/machine.c b/target/i386/machine.c
index 225b5d433b..ef4da3828d 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -634,7 +634,7 @@ static bool hyperv_runtime_enable_needed(void *opaque)
     X86CPU *cpu = opaque;
     CPUX86State *env = &cpu->env;
 
-    if (!cpu->hyperv_runtime) {
+    if (!hyperv_feat_enabled(cpu, HYPERV_FEAT_RUNTIME)) {
         return false;
     }
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v2 2/9] i386/kvm: add support for KVM_GET_SUPPORTED_HV_CPUID
  2019-05-17 14:19 [Qemu-devel] [PATCH v2 0/9] i386/kvm/hyper-v: refactor and implement 'hv-stimer-direct' and 'hv-passthrough' enlightenments Vitaly Kuznetsov
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 1/9] i386/kvm: convert hyperv enlightenments properties from bools to bits Vitaly Kuznetsov
@ 2019-05-17 14:19 ` Vitaly Kuznetsov
  2019-05-27 15:52   ` Roman Kagan
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 3/9] i386/kvm: move Hyper-V CPUID filling to hyperv_handle_properties() Vitaly Kuznetsov
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Vitaly Kuznetsov @ 2019-05-17 14:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P . Berrangé,
	Eduardo Habkost, Marcelo Tosatti, Dr . David Alan Gilbert,
	Roman Kagan, Paolo Bonzini, Richard Henderson

KVM now supports reporting supported Hyper-V features through CPUID
(KVM_GET_SUPPORTED_HV_CPUID ioctl). Going forward, this is going to be
the only way to announce new functionality and this has already happened
with Direct Mode stimers.

While we could just support KVM_GET_SUPPORTED_HV_CPUID for new features,
it seems to be beneficial to use it for all Hyper-V enlightenments when
possible. This way we can implement 'hv-all' pass-through mode giving the
guest all supported Hyper-V features even when QEMU knows nothing about
them.

Implementation-wise we create a new kvm_hyperv_properties structure
defining Hyper-V features, get_supported_hv_cpuid()/
get_supported_hv_cpuid_legacy() returning the supported CPUID set and
a bit over-engineered hv_cpuid_check_and_set() which we will also be
used to set cpu->hyperv_* properties for 'hv-all' mode.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 target/i386/kvm.c | 474 ++++++++++++++++++++++++++++++++++------------
 1 file changed, 356 insertions(+), 118 deletions(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 3daac1e4f4..6ead422efa 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -684,156 +684,394 @@ static bool tsc_is_stable_and_known(CPUX86State *env)
         || env->user_tsc_khz;
 }
 
-static int hyperv_handle_properties(CPUState *cs)
+static struct {
+    const char *desc;
+    struct {
+        uint32_t fw;
+        uint32_t bits;
+    } flags[2];
+} kvm_hyperv_properties[] = {
+    [HYPERV_FEAT_RELAXED] = {
+        .desc = "relaxed timing (hv-relaxed)",
+        .flags = {
+            {.fw = FEAT_HYPERV_EAX,
+             .bits = HV_HYPERCALL_AVAILABLE},
+            {.fw = FEAT_HV_RECOMM_EAX,
+             .bits = HV_RELAXED_TIMING_RECOMMENDED}
+        }
+    },
+    [HYPERV_FEAT_VAPIC] = {
+        .desc = "virtual APIC (hv-vapic)",
+        .flags = {
+            {.fw = FEAT_HYPERV_EAX,
+             .bits = HV_HYPERCALL_AVAILABLE | HV_APIC_ACCESS_AVAILABLE},
+            {.fw = FEAT_HV_RECOMM_EAX,
+             .bits = HV_APIC_ACCESS_RECOMMENDED}
+        }
+    },
+    [HYPERV_FEAT_TIME] = {
+        .desc = "clocksources (hv-time)",
+        .flags = {
+            {.fw = FEAT_HYPERV_EAX,
+             .bits = HV_HYPERCALL_AVAILABLE | HV_TIME_REF_COUNT_AVAILABLE |
+             HV_REFERENCE_TSC_AVAILABLE}
+        }
+    },
+    [HYPERV_FEAT_CRASH] = {
+        .desc = "crash MSRs (hv-crash)",
+        .flags = {
+            {.fw = FEAT_HYPERV_EDX,
+             .bits = HV_GUEST_CRASH_MSR_AVAILABLE}
+        }
+    },
+    [HYPERV_FEAT_RESET] = {
+        .desc = "reset MSR (hv-reset)",
+        .flags = {
+            {.fw = FEAT_HYPERV_EAX,
+             .bits = HV_RESET_AVAILABLE}
+        }
+    },
+    [HYPERV_FEAT_VPINDEX] = {
+        .desc = "VP_INDEX MSR (hv-vpindex)",
+        .flags = {
+            {.fw = FEAT_HYPERV_EAX,
+             .bits = HV_VP_INDEX_AVAILABLE}
+        }
+    },
+    [HYPERV_FEAT_RUNTIME] = {
+        .desc = "VP_RUNTIME MSR (hv-runtime)",
+        .flags = {
+            {.fw = FEAT_HYPERV_EAX,
+             .bits = HV_VP_RUNTIME_AVAILABLE}
+        }
+    },
+    [HYPERV_FEAT_SYNIC] = {
+        .desc = "synthetic interrupt controller (hv-synic)",
+        .flags = {
+            {.fw = FEAT_HYPERV_EAX,
+             .bits = HV_SYNIC_AVAILABLE}
+        }
+    },
+    [HYPERV_FEAT_STIMER] = {
+        .desc = "synthetic timers (hv-stimer)",
+        .flags = {
+            {.fw = FEAT_HYPERV_EAX,
+             .bits = HV_SYNTIMERS_AVAILABLE}
+        }
+    },
+    [HYPERV_FEAT_FREQUENCIES] = {
+        .desc = "frequency MSRs (hv-frequencies)",
+        .flags = {
+            {.fw = FEAT_HYPERV_EAX,
+             .bits = HV_ACCESS_FREQUENCY_MSRS},
+            {.fw = FEAT_HYPERV_EDX,
+             .bits = HV_FREQUENCY_MSRS_AVAILABLE}
+        }
+    },
+    [HYPERV_FEAT_REENLIGHTENMENT] = {
+        .desc = "reenlightenment MSRs (hv-reenlightenment)",
+        .flags = {
+            {.fw = FEAT_HYPERV_EAX,
+             .bits = HV_ACCESS_REENLIGHTENMENTS_CONTROL}
+        }
+    },
+    [HYPERV_FEAT_TLBFLUSH] = {
+        .desc = "paravirtualized TLB flush (hv-tlbflush)",
+        .flags = {
+            {.fw = FEAT_HV_RECOMM_EAX,
+             .bits = HV_REMOTE_TLB_FLUSH_RECOMMENDED |
+             HV_EX_PROCESSOR_MASKS_RECOMMENDED}
+        }
+    },
+    [HYPERV_FEAT_EVMCS] = {
+        .desc = "enlightened VMCS (hv-evmcs)",
+        .flags = {
+            {.fw = FEAT_HV_RECOMM_EAX,
+             .bits = HV_ENLIGHTENED_VMCS_RECOMMENDED}
+        }
+    },
+    [HYPERV_FEAT_IPI] = {
+        .desc = "paravirtualized IPI (hv-ipi)",
+        .flags = {
+            {.fw = FEAT_HV_RECOMM_EAX,
+             .bits = HV_CLUSTER_IPI_RECOMMENDED |
+             HV_EX_PROCESSOR_MASKS_RECOMMENDED}
+        }
+    },
+};
+
+static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max)
+{
+    struct kvm_cpuid2 *cpuid;
+    int r, size;
+
+    size = sizeof(*cpuid) + max * sizeof(*cpuid->entries);
+    cpuid = g_malloc0(size);
+    cpuid->nent = max;
+
+    r = kvm_vcpu_ioctl(cs, KVM_GET_SUPPORTED_HV_CPUID, cpuid);
+    if (r == 0 && cpuid->nent >= max) {
+        r = -E2BIG;
+    }
+    if (r < 0) {
+        if (r == -E2BIG) {
+            g_free(cpuid);
+            return NULL;
+        } else {
+            fprintf(stderr, "KVM_GET_SUPPORTED_HV_CPUID failed: %s\n",
+                    strerror(-r));
+            exit(1);
+        }
+    }
+    return cpuid;
+}
+
+/*
+ * Run KVM_GET_SUPPORTED_HV_CPUID ioctl(), allocating a buffer large enough
+ * for all entries.
+ */
+static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs)
+{
+    struct kvm_cpuid2 *cpuid;
+    int max = 7; /* 0x40000000..0x40000005, 0x4000000A */
+
+    /*
+     * When the buffer is too small, KVM_GET_SUPPORTED_HV_CPUID fails with
+     * -E2BIG, however, it doesn't report back the right size. Keep increasing
+     * it and re-trying until we succeed.
+     */
+    while ((cpuid = try_get_hv_cpuid(cs, max)) == NULL) {
+        max++;
+    }
+    return cpuid;
+}
+
+/*
+ * When KVM_GET_SUPPORTED_HV_CPUID is not supported we fill CPUID feature
+ * leaves from KVM_CAP_HYPERV* and present MSRs data.
+ */
+static struct kvm_cpuid2 *get_supported_hv_cpuid_legacy(CPUState *cs)
 {
     X86CPU *cpu = X86_CPU(cs);
-    CPUX86State *env = &cpu->env;
+    struct kvm_cpuid2 *cpuid;
+    struct kvm_cpuid_entry2 *entry_feat, *entry_recomm;
+
+    /* HV_CPUID_FEATURES, HV_CPUID_ENLIGHTMENT_INFO */
+    cpuid = g_malloc0(sizeof(*cpuid) + 2 * sizeof(*cpuid->entries));
+    cpuid->nent = 2;
+
+    /* HV_CPUID_VENDOR_AND_MAX_FUNCTIONS */
+    entry_feat = &cpuid->entries[0];
+    entry_feat->function = HV_CPUID_FEATURES;
 
-    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_RELAXED)) {
-        env->features[FEAT_HYPERV_EAX] |= HV_HYPERCALL_AVAILABLE;
+    entry_recomm = &cpuid->entries[1];
+    entry_recomm->function = HV_CPUID_ENLIGHTMENT_INFO;
+    entry_recomm->ebx = cpu->hyperv_spinlock_attempts;
+
+    if (kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV) > 0) {
+        entry_feat->eax |= HV_HYPERCALL_AVAILABLE;
+        entry_feat->eax |= HV_APIC_ACCESS_AVAILABLE;
+        entry_feat->edx |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
+        entry_recomm->eax |= HV_RELAXED_TIMING_RECOMMENDED;
+        entry_recomm->eax |= HV_APIC_ACCESS_RECOMMENDED;
     }
-    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC)) {
-        env->features[FEAT_HYPERV_EAX] |= HV_HYPERCALL_AVAILABLE;
-        env->features[FEAT_HYPERV_EAX] |= HV_APIC_ACCESS_AVAILABLE;
+
+    if (kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_TIME) > 0) {
+        entry_feat->eax |= HV_TIME_REF_COUNT_AVAILABLE;
+        entry_feat->eax |= HV_REFERENCE_TSC_AVAILABLE;
     }
-    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_TIME)) {
-        if (kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_TIME) <= 0) {
-            fprintf(stderr, "Hyper-V clocksources "
-                    "(requested by 'hv-time' cpu flag) "
-                    "are not supported by kernel\n");
-            return -ENOSYS;
-        }
-        env->features[FEAT_HYPERV_EAX] |= HV_HYPERCALL_AVAILABLE;
-        env->features[FEAT_HYPERV_EAX] |= HV_TIME_REF_COUNT_AVAILABLE;
-        env->features[FEAT_HYPERV_EAX] |= HV_REFERENCE_TSC_AVAILABLE;
+
+    if (has_msr_hv_frequencies) {
+        entry_feat->eax |= HV_ACCESS_FREQUENCY_MSRS;
+        entry_feat->edx |= HV_FREQUENCY_MSRS_AVAILABLE;
     }
-    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_FREQUENCIES)) {
-        if (!has_msr_hv_frequencies) {
-            fprintf(stderr, "Hyper-V frequency MSRs "
-                    "(requested by 'hv-frequencies' cpu flag) "
-                    "are not supported by kernel\n");
-            return -ENOSYS;
-        }
-        env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS;
-        env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE;
+
+    if (has_msr_hv_crash) {
+        entry_feat->edx |= HV_GUEST_CRASH_MSR_AVAILABLE;
     }
-    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_CRASH)) {
-        if (!has_msr_hv_crash) {
-            fprintf(stderr, "Hyper-V crash MSRs "
-                    "(requested by 'hv-crash' cpu flag) "
-                    "are not supported by kernel\n");
-            return -ENOSYS;
-        }
-        env->features[FEAT_HYPERV_EDX] |= HV_GUEST_CRASH_MSR_AVAILABLE;
+
+    if (has_msr_hv_reenlightenment) {
+        entry_feat->eax |= HV_ACCESS_REENLIGHTENMENTS_CONTROL;
     }
-    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_REENLIGHTENMENT)) {
-        if (!has_msr_hv_reenlightenment) {
-            fprintf(stderr,
-                    "Hyper-V Reenlightenment MSRs "
-                    "(requested by 'hv-reenlightenment' cpu flag) "
-                    "are not supported by kernel\n");
-            return -ENOSYS;
-        }
-        env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_REENLIGHTENMENTS_CONTROL;
+
+    if (has_msr_hv_reset) {
+        entry_feat->eax |= HV_RESET_AVAILABLE;
     }
-    env->features[FEAT_HYPERV_EDX] |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
-    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_RESET)) {
-        if (!has_msr_hv_reset) {
-            fprintf(stderr, "Hyper-V reset MSR "
-                    "(requested by 'hv-reset' cpu flag) "
-                    "is not supported by kernel\n");
-            return -ENOSYS;
-        }
-        env->features[FEAT_HYPERV_EAX] |= HV_RESET_AVAILABLE;
+
+    if (has_msr_hv_vpindex) {
+        entry_feat->eax |= HV_VP_INDEX_AVAILABLE;
     }
-    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
-        if (!has_msr_hv_vpindex) {
-            fprintf(stderr, "Hyper-V VP_INDEX MSR "
-                    "(requested by 'hv-vpindex' cpu flag) "
-                    "is not supported by kernel\n");
-            return -ENOSYS;
-        }
-        env->features[FEAT_HYPERV_EAX] |= HV_VP_INDEX_AVAILABLE;
+
+    if (has_msr_hv_runtime) {
+        entry_feat->eax |= HV_VP_RUNTIME_AVAILABLE;
     }
-    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_RUNTIME)) {
-        if (!has_msr_hv_runtime) {
-            fprintf(stderr, "Hyper-V VP_RUNTIME MSR "
-                    "(requested by 'hv-runtime' cpu flag) "
-                    "is not supported by kernel\n");
-            return -ENOSYS;
+
+    if (has_msr_hv_synic) {
+        unsigned int cap = cpu->hyperv_synic_kvm_only ?
+            KVM_CAP_HYPERV_SYNIC : KVM_CAP_HYPERV_SYNIC2;
+
+        if (kvm_check_extension(cs->kvm_state, cap) > 0) {
+            entry_feat->eax |= HV_SYNIC_AVAILABLE;
         }
-        env->features[FEAT_HYPERV_EAX] |= HV_VP_RUNTIME_AVAILABLE;
     }
-    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC)) {
-        unsigned int cap = KVM_CAP_HYPERV_SYNIC;
-        if (!cpu->hyperv_synic_kvm_only) {
-            if (!hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
-                fprintf(stderr, "Hyper-V SynIC "
-                        "(requested by 'hv-synic' cpu flag) "
-                        "requires Hyper-V VP_INDEX ('hv-vpindex')\n");
-            return -ENOSYS;
-            }
-            cap = KVM_CAP_HYPERV_SYNIC2;
-        }
 
-        if (!has_msr_hv_synic || !kvm_check_extension(cs->kvm_state, cap)) {
-            fprintf(stderr, "Hyper-V SynIC (requested by 'hv-synic' cpu flag) "
-                    "is not supported by kernel\n");
-            return -ENOSYS;
-        }
+    if (has_msr_hv_stimer) {
+        entry_feat->eax |= HV_SYNTIMERS_AVAILABLE;
+    }
 
-        env->features[FEAT_HYPERV_EAX] |= HV_SYNIC_AVAILABLE;
+    if (kvm_check_extension(cs->kvm_state,
+                            KVM_CAP_HYPERV_TLBFLUSH) > 0) {
+        entry_recomm->eax |= HV_REMOTE_TLB_FLUSH_RECOMMENDED;
+        entry_recomm->eax |= HV_EX_PROCESSOR_MASKS_RECOMMENDED;
     }
-    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_STIMER)) {
-        if (!has_msr_hv_stimer) {
-            fprintf(stderr, "Hyper-V timers aren't supported by kernel\n");
-            return -ENOSYS;
-        }
-        env->features[FEAT_HYPERV_EAX] |= HV_SYNTIMERS_AVAILABLE;
+
+    if (kvm_check_extension(cs->kvm_state,
+                            KVM_CAP_HYPERV_ENLIGHTENED_VMCS) > 0) {
+        entry_recomm->eax |= HV_ENLIGHTENED_VMCS_RECOMMENDED;
     }
-    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_RELAXED)) {
-        env->features[FEAT_HV_RECOMM_EAX] |= HV_RELAXED_TIMING_RECOMMENDED;
+
+    if (kvm_check_extension(cs->kvm_state,
+                            KVM_CAP_HYPERV_SEND_IPI) > 0) {
+        entry_recomm->eax |= HV_CLUSTER_IPI_RECOMMENDED;
+        entry_recomm->eax |= HV_EX_PROCESSOR_MASKS_RECOMMENDED;
     }
-    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC)) {
-        env->features[FEAT_HV_RECOMM_EAX] |= HV_APIC_ACCESS_RECOMMENDED;
-    }
-    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_TLBFLUSH)) {
-        if (kvm_check_extension(cs->kvm_state,
-                                KVM_CAP_HYPERV_TLBFLUSH) <= 0) {
-            fprintf(stderr, "Hyper-V TLB flush support "
-                    "(requested by 'hv-tlbflush' cpu flag) "
-                    " is not supported by kernel\n");
-            return -ENOSYS;
-        }
-        env->features[FEAT_HV_RECOMM_EAX] |= HV_REMOTE_TLB_FLUSH_RECOMMENDED;
-        env->features[FEAT_HV_RECOMM_EAX] |= HV_EX_PROCESSOR_MASKS_RECOMMENDED;
+
+    return cpuid;
+}
+
+static int hv_cpuid_get_fw(struct kvm_cpuid2 *cpuid, int fw, uint32_t *r)
+{
+    struct kvm_cpuid_entry2 *entry;
+    uint32_t func;
+    int reg;
+
+    switch (fw) {
+    case FEAT_HYPERV_EAX:
+        reg = R_EAX;
+        func = HV_CPUID_FEATURES;
+        break;
+    case FEAT_HYPERV_EDX:
+        reg = R_EDX;
+        func = HV_CPUID_FEATURES;
+        break;
+    case FEAT_HV_RECOMM_EAX:
+        reg = R_EAX;
+        func = HV_CPUID_ENLIGHTMENT_INFO;
+        break;
+    default:
+        return -EINVAL;
     }
-    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_IPI)) {
-        if (kvm_check_extension(cs->kvm_state,
-                                KVM_CAP_HYPERV_SEND_IPI) <= 0) {
-            fprintf(stderr, "Hyper-V IPI send support "
-                    "(requested by 'hv-ipi' cpu flag) "
-                    " is not supported by kernel\n");
-            return -ENOSYS;
+
+    entry = cpuid_find_entry(cpuid, func, 0);
+    if (!entry) {
+        return -ENOENT;
+    }
+
+    switch (reg) {
+    case R_EAX:
+        *r = entry->eax;
+        break;
+    case R_EDX:
+        *r = entry->edx;
+        break;
+    default:
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+static int hv_cpuid_check_and_set(CPUState *cs, struct kvm_cpuid2 *cpuid,
+                                  int feature)
+{
+    X86CPU *cpu = X86_CPU(cs);
+    CPUX86State *env = &cpu->env;
+    uint32_t r, fw, bits;;
+    int i;
+
+    if (!hyperv_feat_enabled(cpu, feature)) {
+        return 0;
+    }
+
+    for (i = 0; i < ARRAY_SIZE(kvm_hyperv_properties[feature].flags); i++) {
+        fw = kvm_hyperv_properties[feature].flags[i].fw;
+        bits = kvm_hyperv_properties[feature].flags[i].bits;
+
+        if (!fw) {
+            continue;
         }
-        env->features[FEAT_HV_RECOMM_EAX] |= HV_CLUSTER_IPI_RECOMMENDED;
-        env->features[FEAT_HV_RECOMM_EAX] |= HV_EX_PROCESSOR_MASKS_RECOMMENDED;
+
+        if (hv_cpuid_get_fw(cpuid, fw, &r) || (r & bits) != bits) {
+            fprintf(stderr,
+                    "Hyper-V %s is not supported by kernel\n",
+                    kvm_hyperv_properties[feature].desc);
+            return 1;
+        }
+
+        env->features[fw] |= bits;
     }
+
+    return 0;
+}
+
+static int hyperv_handle_properties(CPUState *cs)
+{
+    X86CPU *cpu = X86_CPU(cs);
+    CPUX86State *env = &cpu->env;
+    struct kvm_cpuid2 *cpuid;
+    int r = 0;
+
     if (hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS)) {
         uint16_t evmcs_version;
 
         if (kvm_vcpu_enable_cap(cs, KVM_CAP_HYPERV_ENLIGHTENED_VMCS, 0,
                                 (uintptr_t)&evmcs_version)) {
-            fprintf(stderr, "Hyper-V Enlightened VMCS "
-                    "(requested by 'hv-evmcs' cpu flag) "
-                    "is not supported by kernel\n");
+            fprintf(stderr, "Hyper-V %s is not supported by kernel\n",
+                    kvm_hyperv_properties[HYPERV_FEAT_EVMCS].desc);
             return -ENOSYS;
         }
         env->features[FEAT_HV_RECOMM_EAX] |= HV_ENLIGHTENED_VMCS_RECOMMENDED;
         env->features[FEAT_HV_NESTED_EAX] = evmcs_version;
     }
 
-    return 0;
+    if (kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_CPUID) > 0) {
+        cpuid = get_supported_hv_cpuid(cs);
+    } else {
+        cpuid = get_supported_hv_cpuid_legacy(cs);
+    }
+
+    /* Features */
+    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_RELAXED);
+    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_VAPIC);
+    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_TIME);
+    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_CRASH);
+    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_RESET);
+    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_VPINDEX);
+    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_RUNTIME);
+    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_SYNIC);
+    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_STIMER);
+    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_FREQUENCIES);
+    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_REENLIGHTENMENT);
+    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_TLBFLUSH);
+    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_EVMCS);
+    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_IPI);
+
+    /* Dependencies */
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC) &&
+        !cpu->hyperv_synic_kvm_only &&
+        !hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
+        fprintf(stderr, "Hyper-V %s requires %s\n",
+                kvm_hyperv_properties[HYPERV_FEAT_SYNIC].desc,
+                kvm_hyperv_properties[HYPERV_FEAT_VPINDEX].desc);
+        r |= 1;
+    }
+
+    /* Not exposed by KVM but needed to make CPU hotplug in Windows work */
+    env->features[FEAT_HYPERV_EDX] |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
+
+    g_free(cpuid);
+
+    return r ? -ENOSYS : 0;
 }
 
 static int hyperv_init_vcpu(X86CPU *cpu)
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v2 3/9] i386/kvm: move Hyper-V CPUID filling to hyperv_handle_properties()
  2019-05-17 14:19 [Qemu-devel] [PATCH v2 0/9] i386/kvm/hyper-v: refactor and implement 'hv-stimer-direct' and 'hv-passthrough' enlightenments Vitaly Kuznetsov
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 1/9] i386/kvm: convert hyperv enlightenments properties from bools to bits Vitaly Kuznetsov
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 2/9] i386/kvm: add support for KVM_GET_SUPPORTED_HV_CPUID Vitaly Kuznetsov
@ 2019-05-17 14:19 ` Vitaly Kuznetsov
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 4/9] i386/kvm: document existing Hyper-V enlightenments Vitaly Kuznetsov
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Vitaly Kuznetsov @ 2019-05-17 14:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P . Berrangé,
	Eduardo Habkost, Marcelo Tosatti, Dr . David Alan Gilbert,
	Roman Kagan, Paolo Bonzini, Richard Henderson

Let's consolidate Hyper-V features handling in hyperv_handle_properties().
The change is necessary to support 'hv-passthrough' mode as we'll be just
copying CPUIDs from KVM instead of filling them in.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 target/i386/kvm.c | 163 +++++++++++++++++++++++++---------------------
 1 file changed, 90 insertions(+), 73 deletions(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 6ead422efa..2b13757441 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1014,13 +1014,25 @@ static int hv_cpuid_check_and_set(CPUState *cs, struct kvm_cpuid2 *cpuid,
     return 0;
 }
 
-static int hyperv_handle_properties(CPUState *cs)
+/*
+ * Fill in Hyper-V CPUIDs. Returns the number of entries filled in cpuid_ent in
+ * case of success, errno < 0 in case of failure and 0 when no Hyper-V
+ * extentions are enabled.
+ */
+static int hyperv_handle_properties(CPUState *cs,
+                                    struct kvm_cpuid_entry2 *cpuid_ent)
 {
     X86CPU *cpu = X86_CPU(cs);
     CPUX86State *env = &cpu->env;
     struct kvm_cpuid2 *cpuid;
+    struct kvm_cpuid_entry2 *c;
+    uint32_t signature[3];
+    uint32_t cpuid_i = 0;
     int r = 0;
 
+    if (!hyperv_enabled(cpu))
+        return 0;
+
     if (hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS)) {
         uint16_t evmcs_version;
 
@@ -1069,9 +1081,80 @@ static int hyperv_handle_properties(CPUState *cs)
     /* Not exposed by KVM but needed to make CPU hotplug in Windows work */
     env->features[FEAT_HYPERV_EDX] |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
 
+    if (r) {
+        r = -ENOSYS;
+        goto free;
+    }
+
+    c = &cpuid_ent[cpuid_i++];
+    c->function = HV_CPUID_VENDOR_AND_MAX_FUNCTIONS;
+    if (!cpu->hyperv_vendor_id) {
+        memcpy(signature, "Microsoft Hv", 12);
+    } else {
+        size_t len = strlen(cpu->hyperv_vendor_id);
+
+        if (len > 12) {
+            error_report("hv-vendor-id truncated to 12 characters");
+            len = 12;
+        }
+        memset(signature, 0, 12);
+        memcpy(signature, cpu->hyperv_vendor_id, len);
+    }
+    c->eax = hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS) ?
+        HV_CPUID_NESTED_FEATURES : HV_CPUID_IMPLEMENT_LIMITS;
+    c->ebx = signature[0];
+    c->ecx = signature[1];
+    c->edx = signature[2];
+
+    c = &cpuid_ent[cpuid_i++];
+    c->function = HV_CPUID_INTERFACE;
+    memcpy(signature, "Hv#1\0\0\0\0\0\0\0\0", 12);
+    c->eax = signature[0];
+    c->ebx = 0;
+    c->ecx = 0;
+    c->edx = 0;
+
+    c = &cpuid_ent[cpuid_i++];
+    c->function = HV_CPUID_VERSION;
+    c->eax = 0x00001bbc;
+    c->ebx = 0x00060001;
+
+    c = &cpuid_ent[cpuid_i++];
+    c->function = HV_CPUID_FEATURES;
+    c->eax = env->features[FEAT_HYPERV_EAX];
+    c->ebx = env->features[FEAT_HYPERV_EBX];
+    c->edx = env->features[FEAT_HYPERV_EDX];
+
+    c = &cpuid_ent[cpuid_i++];
+    c->function = HV_CPUID_ENLIGHTMENT_INFO;
+    c->eax = env->features[FEAT_HV_RECOMM_EAX];
+    c->ebx = cpu->hyperv_spinlock_attempts;
+
+    c = &cpuid_ent[cpuid_i++];
+    c->function = HV_CPUID_IMPLEMENT_LIMITS;
+    c->eax = cpu->hv_max_vps;
+    c->ebx = 0x40;
+
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS)) {
+        __u32 function;
+
+        /* Create zeroed 0x40000006..0x40000009 leaves */
+        for (function = HV_CPUID_IMPLEMENT_LIMITS + 1;
+             function < HV_CPUID_NESTED_FEATURES; function++) {
+            c = &cpuid_ent[cpuid_i++];
+            c->function = function;
+        }
+
+        c = &cpuid_ent[cpuid_i++];
+        c->function = HV_CPUID_NESTED_FEATURES;
+        c->eax = env->features[FEAT_HV_NESTED_EAX];
+    }
+    r = cpuid_i;
+
+free:
     g_free(cpuid);
 
-    return r ? -ENOSYS : 0;
+    return r;
 }
 
 static int hyperv_init_vcpu(X86CPU *cpu)
@@ -1180,79 +1263,13 @@ int kvm_arch_init_vcpu(CPUState *cs)
     }
 
     /* Paravirtualization CPUIDs */
-    if (hyperv_enabled(cpu)) {
-        c = &cpuid_data.entries[cpuid_i++];
-        c->function = HV_CPUID_VENDOR_AND_MAX_FUNCTIONS;
-        if (!cpu->hyperv_vendor_id) {
-            memcpy(signature, "Microsoft Hv", 12);
-        } else {
-            size_t len = strlen(cpu->hyperv_vendor_id);
-
-            if (len > 12) {
-                error_report("hv-vendor-id truncated to 12 characters");
-                len = 12;
-            }
-            memset(signature, 0, 12);
-            memcpy(signature, cpu->hyperv_vendor_id, len);
-        }
-        c->eax = hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS) ?
-            HV_CPUID_NESTED_FEATURES : HV_CPUID_IMPLEMENT_LIMITS;
-        c->ebx = signature[0];
-        c->ecx = signature[1];
-        c->edx = signature[2];
-
-        c = &cpuid_data.entries[cpuid_i++];
-        c->function = HV_CPUID_INTERFACE;
-        memcpy(signature, "Hv#1\0\0\0\0\0\0\0\0", 12);
-        c->eax = signature[0];
-        c->ebx = 0;
-        c->ecx = 0;
-        c->edx = 0;
-
-        c = &cpuid_data.entries[cpuid_i++];
-        c->function = HV_CPUID_VERSION;
-        c->eax = 0x00001bbc;
-        c->ebx = 0x00060001;
-
-        c = &cpuid_data.entries[cpuid_i++];
-        c->function = HV_CPUID_FEATURES;
-        r = hyperv_handle_properties(cs);
-        if (r) {
-            return r;
-        }
-        c->eax = env->features[FEAT_HYPERV_EAX];
-        c->ebx = env->features[FEAT_HYPERV_EBX];
-        c->edx = env->features[FEAT_HYPERV_EDX];
-
-        c = &cpuid_data.entries[cpuid_i++];
-        c->function = HV_CPUID_ENLIGHTMENT_INFO;
-
-        c->eax = env->features[FEAT_HV_RECOMM_EAX];
-        c->ebx = cpu->hyperv_spinlock_attempts;
-
-        c = &cpuid_data.entries[cpuid_i++];
-        c->function = HV_CPUID_IMPLEMENT_LIMITS;
-
-        c->eax = cpu->hv_max_vps;
-        c->ebx = 0x40;
-
+    r = hyperv_handle_properties(cs, cpuid_data.entries);
+    if (r < 0) {
+        return r;
+    } else if (r > 0) {
+        cpuid_i = r;
         kvm_base = KVM_CPUID_SIGNATURE_NEXT;
         has_msr_hv_hypercall = true;
-
-        if (hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS)) {
-            __u32 function;
-
-            /* Create zeroed 0x40000006..0x40000009 leaves */
-            for (function = HV_CPUID_IMPLEMENT_LIMITS + 1;
-                 function < HV_CPUID_NESTED_FEATURES; function++) {
-                c = &cpuid_data.entries[cpuid_i++];
-                c->function = function;
-            }
-
-            c = &cpuid_data.entries[cpuid_i++];
-            c->function = HV_CPUID_NESTED_FEATURES;
-            c->eax = env->features[FEAT_HV_NESTED_EAX];
-        }
     }
 
     if (cpu->expose_kvm) {
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v2 4/9] i386/kvm: document existing Hyper-V enlightenments
  2019-05-17 14:19 [Qemu-devel] [PATCH v2 0/9] i386/kvm/hyper-v: refactor and implement 'hv-stimer-direct' and 'hv-passthrough' enlightenments Vitaly Kuznetsov
                   ` (2 preceding siblings ...)
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 3/9] i386/kvm: move Hyper-V CPUID filling to hyperv_handle_properties() Vitaly Kuznetsov
@ 2019-05-17 14:19 ` Vitaly Kuznetsov
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 5/9] i386/kvm: implement 'hv-passthrough' mode Vitaly Kuznetsov
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Vitaly Kuznetsov @ 2019-05-17 14:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P . Berrangé,
	Eduardo Habkost, Marcelo Tosatti, Dr . David Alan Gilbert,
	Roman Kagan, Paolo Bonzini, Richard Henderson

Currently, there is no doc describing hv-* CPU flags, people are
encouraged to get the information from Microsoft Hyper-V Top Level
Functional specification (TLFS). There is, however, a bit of QEMU
specifics.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 docs/hyperv.txt | 181 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 181 insertions(+)
 create mode 100644 docs/hyperv.txt

diff --git a/docs/hyperv.txt b/docs/hyperv.txt
new file mode 100644
index 0000000000..c423e0fca2
--- /dev/null
+++ b/docs/hyperv.txt
@@ -0,0 +1,181 @@
+Hyper-V Enlightenments
+======================
+
+
+1. Description
+===============
+In some cases when implementing a hardware interface in software is slow, KVM
+implements its own paravirtualized interfaces. This works well for Linux as
+guest support for such features is added simultaneously with the feature itself.
+It may, however, be hard-to-impossible to add support for these interfaces to
+proprietary OSes, namely, Microsoft Windows.
+
+KVM on x86 implements Hyper-V Enlightenments for Windows guests. These features
+make Windows and Hyper-V guests think they're running on top of a Hyper-V
+compatible hypervisor and use Hyper-V specific features.
+
+
+2. Setup
+=========
+No Hyper-V enlightenments are enabled by default by either KVM or QEMU. In
+QEMU, individual enlightenments can be enabled through CPU flags, e.g:
+
+  qemu-system-x86_64 --enable-kvm --cpu host,hv_relaxed,hv_vpindex,hv_time, ...
+
+Sometimes there are dependencies between enlightenments, QEMU is supposed to
+check that the supplied configuration is sane.
+
+When any set of the Hyper-V enlightenments is enabled, QEMU changes hypervisor
+identification (CPUID 0x40000000..0x4000000A) to Hyper-V. KVM identification
+and features are kept in leaves 0x40000100..0x40000101.
+
+
+3. Existing enlightenments
+===========================
+
+3.1. hv-relaxed
+================
+This feature tells guest OS to disable watchdog timeouts as it is running on a
+hypervisor. It is known that some Windows versions will do this even when they
+see 'hypervisor' CPU flag.
+
+3.2. hv-vapic
+==============
+Provides so-called VP Assist page MSR to guest allowing it to work with APIC
+more efficiently. In particular, this enlightenment allows paravirtualized
+(exit-less) EOI processing.
+
+3.3. hv-spinlocks=xxx
+======================
+Enables paravirtualized spinlocks. The parameter indicates how many times
+spinlock acquisition should be attempted before indicating the situation to the
+hypervisor. A special value 0xffffffff indicates "never to retry".
+
+3.4. hv-vpindex
+================
+Provides HV_X64_MSR_VP_INDEX (0x40000002) MSR to the guest which has Virtual
+processor index information. This enlightenment makes sense in conjunction with
+hv-synic, hv-stimer and other enlightenments which require the guest to know its
+Virtual Processor indices (e.g. when VP index needs to be passed in a
+hypercall).
+
+3.5. hv-runtime
+================
+Provides HV_X64_MSR_VP_RUNTIME (0x40000010) MSR to the guest. The MSR keeps the
+virtual processor run time in 100ns units. This gives guest operating system an
+idea of how much time was 'stolen' from it (when the virtual CPU was preempted
+to perform some other work).
+
+3.6. hv-crash
+==============
+Provides HV_X64_MSR_CRASH_P0..HV_X64_MSR_CRASH_P5 (0x40000100..0x40000105) and
+HV_X64_MSR_CRASH_CTL (0x40000105) MSRs to the guest. These MSRs are written to
+by the guest when it crashes, HV_X64_MSR_CRASH_P0..HV_X64_MSR_CRASH_P5 MSRs
+contain additional crash information. This information is outputted in QEMU log
+and through QAPI.
+Note: unlike under genuine Hyper-V, write to HV_X64_MSR_CRASH_CTL causes guest
+to shutdown. This effectively blocks crash dump generation by Windows.
+
+3.7. hv-time
+=============
+Enables two Hyper-V-specific clocksources available to the guest: MSR-based
+Hyper-V clocksource (HV_X64_MSR_TIME_REF_COUNT, 0x40000020) and Reference TSC
+page (enabled via MSR HV_X64_MSR_REFERENCE_TSC, 0x40000021). Both clocksources
+are per-guest, Reference TSC page clocksource allows for exit-less time stamp
+readings. Using this enlightenment leads to significant speedup of all timestamp
+related operations.
+
+3.8. hv-synic
+==============
+Enables Hyper-V Synthetic interrupt controller - an extension of a local APIC.
+When enabled, this enlightenment provides additional communication facilities
+to the guest: SynIC messages and Events. This is a pre-requisite for
+implementing VMBus devices (not yet in QEMU). Additionally, this enlightenment
+is needed to enable Hyper-V synthetic timers. SynIC is controlled through MSRs
+HV_X64_MSR_SCONTROL..HV_X64_MSR_EOM (0x40000080..0x40000084) and
+HV_X64_MSR_SINT0..HV_X64_MSR_SINT15 (0x40000090..0x4000009F)
+
+Requires: hv-vpindex
+
+3.9. hv-stimer
+===============
+Enables Hyper-V synthetic timers. There are four synthetic timers per virtual
+CPU controlled through HV_X64_MSR_STIMER0_CONFIG..HV_X64_MSR_STIMER3_COUNT
+(0x400000B0..0x400000B7) MSRs. These timers can work either in single-shot or
+periodic mode. It is known that certain Windows versions revert to using HPET
+(or even RTC when HPET is unavailable) extensively when this enlightenment is
+not provided; this can lead to significant CPU consumption, even when virtual
+CPU is idle.
+
+Requires: hv-vpindex, hv-synic, hv-time
+
+3.10. hv-tlbflush
+==================
+Enables paravirtualized TLB shoot-down mechanism. On x86 architecture, remote
+TLB flush procedure requires sending IPIs and waiting for other CPUs to perform
+local TLB flush. In virtualized environment some virtual CPUs may not even be
+scheduled at the time of the call and may not require flushing (or, flushing
+may be postponed until the virtual CPU is scheduled). hv-tlbflush enlightenment
+implements TLB shoot-down through hypervisor enabling the optimization.
+
+Requires: hv-vpindex
+
+3.11. hv-ipi
+=============
+Enables paravirtualized IPI send mechanism. HvCallSendSyntheticClusterIpi
+hypercall may target more than 64 virtual CPUs simultaneously, doing the same
+through APIC requires more than one access (and thus exit to the hypervisor).
+
+Requires: hv-vpindex
+
+3.12. hv-vendor-id=xxx
+=======================
+This changes Hyper-V identification in CPUID 0x40000000.EBX-EDX from the default
+"Microsoft Hv". The parameter should be no longer than 12 characters. According
+to the specification, guests shouldn't use this information and it is unknown
+if there is a Windows version which acts differently.
+Note: hv-vendor-id is not an enlightenment and thus doesn't enable Hyper-V
+identification when specified without some other enlightenment.
+
+3.13. hv-reset
+===============
+Provides HV_X64_MSR_RESET (0x40000003) MSR to the guest allowing it to reset
+itself by writing to it. Even when this MSR is enabled, it is not a recommended
+way for Windows to perform system reboot and thus it may not be used.
+
+3.14. hv-frequencies
+============================================
+Provides HV_X64_MSR_TSC_FREQUENCY (0x40000022) and HV_X64_MSR_APIC_FREQUENCY
+(0x40000023) allowing the guest to get its TSC/APIC frequencies without doing
+measurements.
+
+3.15 hv-reenlightenment
+========================
+The enlightenment is nested specific, it targets Hyper-V on KVM guests. When
+enabled, it provides HV_X64_MSR_REENLIGHTENMENT_CONTROL (0x40000106),
+HV_X64_MSR_TSC_EMULATION_CONTROL (0x40000107)and HV_X64_MSR_TSC_EMULATION_STATUS
+(0x40000108) MSRs allowing the guest to get notified when TSC frequency changes
+(only happens on migration) and keep using old frequency (through emulation in
+the hypervisor) until it is ready to switch to the new one. This, in conjunction
+with hv-frequencies, allows Hyper-V on KVM to pass stable clocksource (Reference
+TSC page) to its own guests.
+
+Recommended: hv-frequencies
+
+3.16. hv-evmcs
+===============
+The enlightenment is nested specific, it targets Hyper-V on KVM guests. When
+enabled, it provides Enlightened VMCS feature to the guest. The feature
+implements paravirtualized protocol between L0 (KVM) and L1 (Hyper-V)
+hypervisors making L2 exits to the hypervisor faster. The feature is Intel-only.
+Note: some virtualization features (e.g. Posted Interrupts) are disabled when
+hv-evmcs is enabled. It may make sense to measure your nested workload with and
+without the feature to find out if enabling it is beneficial.
+
+Requires: hv-vapic
+
+
+4. Useful links
+================
+Hyper-V Top Level Functional specification and other information:
+https://github.com/MicrosoftDocs/Virtualization-Documentation
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v2 5/9] i386/kvm: implement 'hv-passthrough' mode
  2019-05-17 14:19 [Qemu-devel] [PATCH v2 0/9] i386/kvm/hyper-v: refactor and implement 'hv-stimer-direct' and 'hv-passthrough' enlightenments Vitaly Kuznetsov
                   ` (3 preceding siblings ...)
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 4/9] i386/kvm: document existing Hyper-V enlightenments Vitaly Kuznetsov
@ 2019-05-17 14:19 ` Vitaly Kuznetsov
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 6/9] i386/kvm: hv-stimer requires hv-time and hv-synic Vitaly Kuznetsov
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Vitaly Kuznetsov @ 2019-05-17 14:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P . Berrangé,
	Eduardo Habkost, Marcelo Tosatti, Dr . David Alan Gilbert,
	Roman Kagan, Paolo Bonzini, Richard Henderson

In many case we just want to give Windows guests all currently supported
Hyper-V enlightenments and that's where this new mode may come handy. We
pass through what was returned by KVM_GET_SUPPORTED_HV_CPUID.

hv_cpuid_check_and_set() is modified to also set cpu->hyperv_* flags as
we may want to check them later (and we actually do for hv_runtime,
hv_synic,...).

'hv-passthrough' is a development only feature, a migration blocker is
added to prevent issues while migrating between hosts with different
feature sets.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 docs/hyperv.txt   | 10 ++++++
 target/i386/cpu.c |  1 +
 target/i386/cpu.h |  1 +
 target/i386/kvm.c | 89 +++++++++++++++++++++++++++++++++++++++--------
 4 files changed, 87 insertions(+), 14 deletions(-)

diff --git a/docs/hyperv.txt b/docs/hyperv.txt
index c423e0fca2..beadb2d0d4 100644
--- a/docs/hyperv.txt
+++ b/docs/hyperv.txt
@@ -175,6 +175,16 @@ without the feature to find out if enabling it is beneficial.
 Requires: hv-vapic
 
 
+4. Development features
+========================
+In some cases (e.g. during development) it may make sense to use QEMU in
+'pass-through' mode and give Windows guests all enlightenments currently
+supported by KVM. This pass-through mode is enabled by "hv-passthrough" CPU
+flag.
+Note: enabling this flag effectively prevents migration as supported features
+may differ between target and destination.
+
+
 4. Useful links
 ================
 Hyper-V Top Level Functional specification and other information:
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9530b28d42..063551ef55 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5849,6 +5849,7 @@ static Property x86_cpu_properties[] = {
                       HYPERV_FEAT_EVMCS, 0),
     DEFINE_PROP_BIT64("hv-ipi", X86CPU, hyperv_features,
                       HYPERV_FEAT_IPI, 0),
+    DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, false),
 
     DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
     DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 11fa9e643e..1f1f8969b4 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1401,6 +1401,7 @@ struct X86CPU {
     char *hyperv_vendor_id;
     bool hyperv_synic_kvm_only;
     uint64_t hyperv_features;
+    bool hyperv_passthrough;
 
     bool check_cpuid;
     bool enforce_cpuid;
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 2b13757441..e876dc6118 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -640,7 +640,7 @@ static bool hyperv_enabled(X86CPU *cpu)
     CPUState *cs = CPU(cpu);
     return kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV) > 0 &&
         ((cpu->hyperv_spinlock_attempts != HYPERV_SPINLOCK_NEVER_RETRY) ||
-         cpu->hyperv_features);
+         cpu->hyperv_features || cpu->hyperv_passthrough);
 }
 
 static int kvm_arch_set_tsc_khz(CPUState *cs)
@@ -986,10 +986,10 @@ static int hv_cpuid_check_and_set(CPUState *cs, struct kvm_cpuid2 *cpuid,
 {
     X86CPU *cpu = X86_CPU(cs);
     CPUX86State *env = &cpu->env;
-    uint32_t r, fw, bits;;
+    uint32_t r, fw, bits;
     int i;
 
-    if (!hyperv_feat_enabled(cpu, feature)) {
+    if (!hyperv_feat_enabled(cpu, feature) && !cpu->hyperv_passthrough) {
         return 0;
     }
 
@@ -1002,15 +1002,23 @@ static int hv_cpuid_check_and_set(CPUState *cs, struct kvm_cpuid2 *cpuid,
         }
 
         if (hv_cpuid_get_fw(cpuid, fw, &r) || (r & bits) != bits) {
-            fprintf(stderr,
-                    "Hyper-V %s is not supported by kernel\n",
-                    kvm_hyperv_properties[feature].desc);
-            return 1;
+            if (hyperv_feat_enabled(cpu, feature)) {
+                fprintf(stderr,
+                        "Hyper-V %s is not supported by kernel\n",
+                        kvm_hyperv_properties[feature].desc);
+                return 1;
+            } else {
+                return 0;
+            }
         }
 
         env->features[fw] |= bits;
     }
 
+    if (cpu->hyperv_passthrough) {
+        cpu->hyperv_features |= BIT(feature);
+    }
+
     return 0;
 }
 
@@ -1028,22 +1036,29 @@ static int hyperv_handle_properties(CPUState *cs,
     struct kvm_cpuid_entry2 *c;
     uint32_t signature[3];
     uint32_t cpuid_i = 0;
-    int r = 0;
+    int r;
 
     if (!hyperv_enabled(cpu))
         return 0;
 
-    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS)) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS) ||
+        cpu->hyperv_passthrough) {
         uint16_t evmcs_version;
 
-        if (kvm_vcpu_enable_cap(cs, KVM_CAP_HYPERV_ENLIGHTENED_VMCS, 0,
-                                (uintptr_t)&evmcs_version)) {
+        r = kvm_vcpu_enable_cap(cs, KVM_CAP_HYPERV_ENLIGHTENED_VMCS, 0,
+                                (uintptr_t)&evmcs_version);
+
+        if (hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS) && r) {
             fprintf(stderr, "Hyper-V %s is not supported by kernel\n",
                     kvm_hyperv_properties[HYPERV_FEAT_EVMCS].desc);
             return -ENOSYS;
         }
-        env->features[FEAT_HV_RECOMM_EAX] |= HV_ENLIGHTENED_VMCS_RECOMMENDED;
-        env->features[FEAT_HV_NESTED_EAX] = evmcs_version;
+
+        if (!r) {
+            env->features[FEAT_HV_RECOMM_EAX] |=
+                HV_ENLIGHTENED_VMCS_RECOMMENDED;
+            env->features[FEAT_HV_NESTED_EAX] = evmcs_version;
+        }
     }
 
     if (kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_CPUID) > 0) {
@@ -1052,8 +1067,33 @@ static int hyperv_handle_properties(CPUState *cs,
         cpuid = get_supported_hv_cpuid_legacy(cs);
     }
 
+    if (cpu->hyperv_passthrough) {
+        memcpy(cpuid_ent, &cpuid->entries[0],
+               cpuid->nent * sizeof(cpuid->entries[0]));
+
+        c = cpuid_find_entry(cpuid, HV_CPUID_FEATURES, 0);
+        if (c) {
+            env->features[FEAT_HYPERV_EAX] = c->eax;
+            env->features[FEAT_HYPERV_EBX] = c->ebx;
+            env->features[FEAT_HYPERV_EDX] = c->eax;
+        }
+        c = cpuid_find_entry(cpuid, HV_CPUID_ENLIGHTMENT_INFO, 0);
+        if (c) {
+            env->features[FEAT_HV_RECOMM_EAX] = c->eax;
+
+            /* hv-spinlocks may have been overriden */
+            if (cpu->hyperv_spinlock_attempts != HYPERV_SPINLOCK_NEVER_RETRY) {
+                c->ebx = cpu->hyperv_spinlock_attempts;
+            }
+        }
+        c = cpuid_find_entry(cpuid, HV_CPUID_NESTED_FEATURES, 0);
+        if (c) {
+            env->features[FEAT_HV_NESTED_EAX] = c->eax;
+        }
+    }
+
     /* Features */
-    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_RELAXED);
+    r = hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_RELAXED);
     r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_VAPIC);
     r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_TIME);
     r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_CRASH);
@@ -1086,6 +1126,12 @@ static int hyperv_handle_properties(CPUState *cs,
         goto free;
     }
 
+    if (cpu->hyperv_passthrough) {
+        /* We already copied all feature words from KVM as is */
+        r = cpuid->nent;
+        goto free;
+    }
+
     c = &cpuid_ent[cpuid_i++];
     c->function = HV_CPUID_VENDOR_AND_MAX_FUNCTIONS;
     if (!cpu->hyperv_vendor_id) {
@@ -1157,11 +1203,26 @@ free:
     return r;
 }
 
+static Error *hv_passthrough_mig_blocker;
+
 static int hyperv_init_vcpu(X86CPU *cpu)
 {
     CPUState *cs = CPU(cpu);
+    Error *local_err = NULL;
     int ret;
 
+    if (cpu->hyperv_passthrough && hv_passthrough_mig_blocker == NULL) {
+        error_setg(&hv_passthrough_mig_blocker,
+                   "'hv-passthrough' CPU flag prevents migration, use explicit"
+                   " set of hv-* flags instead");
+        ret = migrate_add_blocker(hv_passthrough_mig_blocker, &local_err);
+        if (local_err) {
+            error_report_err(local_err);
+            error_free(hv_passthrough_mig_blocker);
+            return ret;
+        }
+    }
+
     if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX) && !hv_vpindex_settable) {
         /*
          * the kernel doesn't support setting vp_index; assert that its value
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v2 6/9] i386/kvm: hv-stimer requires hv-time and hv-synic
  2019-05-17 14:19 [Qemu-devel] [PATCH v2 0/9] i386/kvm/hyper-v: refactor and implement 'hv-stimer-direct' and 'hv-passthrough' enlightenments Vitaly Kuznetsov
                   ` (4 preceding siblings ...)
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 5/9] i386/kvm: implement 'hv-passthrough' mode Vitaly Kuznetsov
@ 2019-05-17 14:19 ` Vitaly Kuznetsov
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 7/9] i386/kvm: hv-tlbflush/ipi require hv-vpindex Vitaly Kuznetsov
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Vitaly Kuznetsov @ 2019-05-17 14:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P . Berrangé,
	Eduardo Habkost, Marcelo Tosatti, Dr . David Alan Gilbert,
	Roman Kagan, Paolo Bonzini, Richard Henderson

Synthetic timers operate in hv-time time and Windows won't use these
without SynIC.

Add .dependencies field to kvm_hyperv_properties[] and a generic mechanism
to check dependencies between features.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 target/i386/kvm.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index e876dc6118..d8b83031a5 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -690,6 +690,7 @@ static struct {
         uint32_t fw;
         uint32_t bits;
     } flags[2];
+    uint64_t dependencies;
 } kvm_hyperv_properties[] = {
     [HYPERV_FEAT_RELAXED] = {
         .desc = "relaxed timing (hv-relaxed)",
@@ -757,7 +758,8 @@ static struct {
         .flags = {
             {.fw = FEAT_HYPERV_EAX,
              .bits = HV_SYNTIMERS_AVAILABLE}
-        }
+        },
+        .dependencies = BIT(HYPERV_FEAT_SYNIC) | BIT(HYPERV_FEAT_TIME)
     },
     [HYPERV_FEAT_FREQUENCIES] = {
         .desc = "frequency MSRs (hv-frequencies)",
@@ -987,12 +989,25 @@ static int hv_cpuid_check_and_set(CPUState *cs, struct kvm_cpuid2 *cpuid,
     X86CPU *cpu = X86_CPU(cs);
     CPUX86State *env = &cpu->env;
     uint32_t r, fw, bits;
-    int i;
+    uint64_t deps;
+    int i, dep_feat = 0;
 
     if (!hyperv_feat_enabled(cpu, feature) && !cpu->hyperv_passthrough) {
         return 0;
     }
 
+    deps = kvm_hyperv_properties[feature].dependencies;
+    while ((dep_feat = find_next_bit(&deps, 64, dep_feat)) < 64) {
+        if (!(hyperv_feat_enabled(cpu, dep_feat))) {
+                fprintf(stderr,
+                        "Hyper-V %s requires Hyper-V %s\n",
+                        kvm_hyperv_properties[feature].desc,
+                        kvm_hyperv_properties[dep_feat].desc);
+                return 1;
+        }
+        dep_feat++;
+    }
+
     for (i = 0; i < ARRAY_SIZE(kvm_hyperv_properties[feature].flags); i++) {
         fw = kvm_hyperv_properties[feature].flags[i].fw;
         bits = kvm_hyperv_properties[feature].flags[i].bits;
@@ -1108,11 +1123,11 @@ static int hyperv_handle_properties(CPUState *cs,
     r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_EVMCS);
     r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_IPI);
 
-    /* Dependencies */
+    /* Additional dependencies not covered by kvm_hyperv_properties[] */
     if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC) &&
         !cpu->hyperv_synic_kvm_only &&
         !hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
-        fprintf(stderr, "Hyper-V %s requires %s\n",
+        fprintf(stderr, "Hyper-V %s requires Hyper-V %s\n",
                 kvm_hyperv_properties[HYPERV_FEAT_SYNIC].desc,
                 kvm_hyperv_properties[HYPERV_FEAT_VPINDEX].desc);
         r |= 1;
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v2 7/9] i386/kvm: hv-tlbflush/ipi require hv-vpindex
  2019-05-17 14:19 [Qemu-devel] [PATCH v2 0/9] i386/kvm/hyper-v: refactor and implement 'hv-stimer-direct' and 'hv-passthrough' enlightenments Vitaly Kuznetsov
                   ` (5 preceding siblings ...)
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 6/9] i386/kvm: hv-stimer requires hv-time and hv-synic Vitaly Kuznetsov
@ 2019-05-17 14:19 ` Vitaly Kuznetsov
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 8/9] i386/kvm: hv-evmcs requires hv-vapic Vitaly Kuznetsov
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 9/9] i386/kvm: add support for Direct Mode for Hyper-V synthetic timers Vitaly Kuznetsov
  8 siblings, 0 replies; 15+ messages in thread
From: Vitaly Kuznetsov @ 2019-05-17 14:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P . Berrangé,
	Eduardo Habkost, Marcelo Tosatti, Dr . David Alan Gilbert,
	Roman Kagan, Paolo Bonzini, Richard Henderson

The corresponding hypercalls require using VP indexes.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 target/i386/kvm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index d8b83031a5..7fc97b749e 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -783,7 +783,8 @@ static struct {
             {.fw = FEAT_HV_RECOMM_EAX,
              .bits = HV_REMOTE_TLB_FLUSH_RECOMMENDED |
              HV_EX_PROCESSOR_MASKS_RECOMMENDED}
-        }
+        },
+        .dependencies = BIT(HYPERV_FEAT_VPINDEX)
     },
     [HYPERV_FEAT_EVMCS] = {
         .desc = "enlightened VMCS (hv-evmcs)",
@@ -798,7 +799,8 @@ static struct {
             {.fw = FEAT_HV_RECOMM_EAX,
              .bits = HV_CLUSTER_IPI_RECOMMENDED |
              HV_EX_PROCESSOR_MASKS_RECOMMENDED}
-        }
+        },
+        .dependencies = BIT(HYPERV_FEAT_VPINDEX)
     },
 };
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v2 8/9] i386/kvm: hv-evmcs requires hv-vapic
  2019-05-17 14:19 [Qemu-devel] [PATCH v2 0/9] i386/kvm/hyper-v: refactor and implement 'hv-stimer-direct' and 'hv-passthrough' enlightenments Vitaly Kuznetsov
                   ` (6 preceding siblings ...)
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 7/9] i386/kvm: hv-tlbflush/ipi require hv-vpindex Vitaly Kuznetsov
@ 2019-05-17 14:19 ` Vitaly Kuznetsov
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 9/9] i386/kvm: add support for Direct Mode for Hyper-V synthetic timers Vitaly Kuznetsov
  8 siblings, 0 replies; 15+ messages in thread
From: Vitaly Kuznetsov @ 2019-05-17 14:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P . Berrangé,
	Eduardo Habkost, Marcelo Tosatti, Dr . David Alan Gilbert,
	Roman Kagan, Paolo Bonzini, Richard Henderson

Enlightened VMCS is enabled by writing to a field in VP assist page and
these require virtual APIC.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 target/i386/kvm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 7fc97b749e..7ae2f63f72 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -791,7 +791,8 @@ static struct {
         .flags = {
             {.fw = FEAT_HV_RECOMM_EAX,
              .bits = HV_ENLIGHTENED_VMCS_RECOMMENDED}
-        }
+        },
+        .dependencies = BIT(HYPERV_FEAT_VAPIC)
     },
     [HYPERV_FEAT_IPI] = {
         .desc = "paravirtualized IPI (hv-ipi)",
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v2 9/9] i386/kvm: add support for Direct Mode for Hyper-V synthetic timers
  2019-05-17 14:19 [Qemu-devel] [PATCH v2 0/9] i386/kvm/hyper-v: refactor and implement 'hv-stimer-direct' and 'hv-passthrough' enlightenments Vitaly Kuznetsov
                   ` (7 preceding siblings ...)
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 8/9] i386/kvm: hv-evmcs requires hv-vapic Vitaly Kuznetsov
@ 2019-05-17 14:19 ` Vitaly Kuznetsov
  8 siblings, 0 replies; 15+ messages in thread
From: Vitaly Kuznetsov @ 2019-05-17 14:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P . Berrangé,
	Eduardo Habkost, Marcelo Tosatti, Dr . David Alan Gilbert,
	Roman Kagan, Paolo Bonzini, Richard Henderson

Hyper-V on KVM can only use Synthetic timers with Direct Mode (opting for
an interrupt instead of VMBus message). This new capability is only
announced in KVM_GET_SUPPORTED_HV_CPUID.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 docs/hyperv.txt            | 10 ++++++++++
 target/i386/cpu.c          |  2 ++
 target/i386/cpu.h          |  1 +
 target/i386/hyperv-proto.h |  1 +
 target/i386/kvm.c          |  9 +++++++++
 5 files changed, 23 insertions(+)

diff --git a/docs/hyperv.txt b/docs/hyperv.txt
index beadb2d0d4..8fdf25c829 100644
--- a/docs/hyperv.txt
+++ b/docs/hyperv.txt
@@ -174,6 +174,16 @@ without the feature to find out if enabling it is beneficial.
 
 Requires: hv-vapic
 
+3.17. hv-stimer-direct
+=======================
+Hyper-V specification allows synthetic timer operation in two modes: "classic",
+when expiration event is delivered as SynIC message and "direct", when the event
+is delivered via normal interrupt. It is known that nested Hyper-V can only
+use synthetic timers in direct mode and thus 'hv-stimer-direct' needs to be
+enabled.
+
+Requires: hv-vpindex, hv-synic, hv-time, hv-stimer
+
 
 4. Development features
 ========================
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 063551ef55..3cfd85758c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5849,6 +5849,8 @@ static Property x86_cpu_properties[] = {
                       HYPERV_FEAT_EVMCS, 0),
     DEFINE_PROP_BIT64("hv-ipi", X86CPU, hyperv_features,
                       HYPERV_FEAT_IPI, 0),
+    DEFINE_PROP_BIT64("hv-stimer-direct", X86CPU, hyperv_features,
+                      HYPERV_FEAT_STIMER_DIRECT, 0),
     DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, false),
 
     DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 1f1f8969b4..0b6b781ecb 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -758,6 +758,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
 #define HYPERV_FEAT_TLBFLUSH            11
 #define HYPERV_FEAT_EVMCS               12
 #define HYPERV_FEAT_IPI                 13
+#define HYPERV_FEAT_STIMER_DIRECT       14
 
 #ifndef HYPERV_SPINLOCK_NEVER_RETRY
 #define HYPERV_SPINLOCK_NEVER_RETRY             0xFFFFFFFF
diff --git a/target/i386/hyperv-proto.h b/target/i386/hyperv-proto.h
index c0272b3a01..cffac10b45 100644
--- a/target/i386/hyperv-proto.h
+++ b/target/i386/hyperv-proto.h
@@ -49,6 +49,7 @@
 #define HV_GUEST_IDLE_STATE_AVAILABLE           (1u << 5)
 #define HV_FREQUENCY_MSRS_AVAILABLE             (1u << 8)
 #define HV_GUEST_CRASH_MSR_AVAILABLE            (1u << 10)
+#define HV_STIMER_DIRECT_MODE_AVAILABLE         (1u << 19)
 
 /*
  * HV_CPUID_ENLIGHTMENT_INFO.EAX bits
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 7ae2f63f72..fb29a3057b 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -803,6 +803,14 @@ static struct {
         },
         .dependencies = BIT(HYPERV_FEAT_VPINDEX)
     },
+    [HYPERV_FEAT_STIMER_DIRECT] = {
+        .desc = "direct mode synthetic timers (hv-stimer-direct)",
+        .flags = {
+            {.fw = FEAT_HYPERV_EDX,
+             .bits = HV_STIMER_DIRECT_MODE_AVAILABLE}
+        },
+        .dependencies = BIT(HYPERV_FEAT_STIMER)
+    },
 };
 
 static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max)
@@ -1125,6 +1133,7 @@ static int hyperv_handle_properties(CPUState *cs,
     r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_TLBFLUSH);
     r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_EVMCS);
     r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_IPI);
+    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_STIMER_DIRECT);
 
     /* Additional dependencies not covered by kvm_hyperv_properties[] */
     if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC) &&
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/9] i386/kvm: convert hyperv enlightenments properties from bools to bits
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 1/9] i386/kvm: convert hyperv enlightenments properties from bools to bits Vitaly Kuznetsov
@ 2019-05-27 15:46   ` Roman Kagan
  0 siblings, 0 replies; 15+ messages in thread
From: Roman Kagan @ 2019-05-27 15:46 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: Daniel P . Berrangé,
	Eduardo Habkost, Marcelo Tosatti, Dr . David Alan Gilbert,
	qemu-devel, Paolo Bonzini, Richard Henderson

On Fri, May 17, 2019 at 04:19:16PM +0200, Vitaly Kuznetsov wrote:
> Representing Hyper-V properties as bits will allow us to check features
> and dependencies between them in a natural way.
> 
> Suggested-by: Roman Kagan <rkagan@virtuozzo.com>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  hw/i386/pc.c          |  3 +-
>  target/i386/cpu.c     | 44 +++++++++++++++--------
>  target/i386/cpu.h     | 37 +++++++++++--------
>  target/i386/hyperv.c  |  2 +-
>  target/i386/kvm.c     | 83 ++++++++++++++++++-------------------------
>  target/i386/machine.c |  2 +-
>  6 files changed, 91 insertions(+), 80 deletions(-)

Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/9] i386/kvm: add support for KVM_GET_SUPPORTED_HV_CPUID
  2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 2/9] i386/kvm: add support for KVM_GET_SUPPORTED_HV_CPUID Vitaly Kuznetsov
@ 2019-05-27 15:52   ` Roman Kagan
  2019-05-27 16:39     ` Vitaly Kuznetsov
  0 siblings, 1 reply; 15+ messages in thread
From: Roman Kagan @ 2019-05-27 15:52 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: Daniel P . Berrangé,
	Eduardo Habkost, Marcelo Tosatti, Dr . David Alan Gilbert,
	qemu-devel, Paolo Bonzini, Richard Henderson

On Fri, May 17, 2019 at 04:19:17PM +0200, Vitaly Kuznetsov wrote:
> KVM now supports reporting supported Hyper-V features through CPUID
> (KVM_GET_SUPPORTED_HV_CPUID ioctl). Going forward, this is going to be
> the only way to announce new functionality and this has already happened
> with Direct Mode stimers.
> 
> While we could just support KVM_GET_SUPPORTED_HV_CPUID for new features,
> it seems to be beneficial to use it for all Hyper-V enlightenments when
> possible. This way we can implement 'hv-all' pass-through mode giving the
> guest all supported Hyper-V features even when QEMU knows nothing about
> them.
> 
> Implementation-wise we create a new kvm_hyperv_properties structure
> defining Hyper-V features, get_supported_hv_cpuid()/
> get_supported_hv_cpuid_legacy() returning the supported CPUID set and
> a bit over-engineered hv_cpuid_check_and_set() which we will also be
> used to set cpu->hyperv_* properties for 'hv-all' mode.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  target/i386/kvm.c | 474 ++++++++++++++++++++++++++++++++++------------
>  1 file changed, 356 insertions(+), 118 deletions(-)
> 
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 3daac1e4f4..6ead422efa 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -684,156 +684,394 @@ static bool tsc_is_stable_and_known(CPUX86State *env)
>          || env->user_tsc_khz;
>  }
>  
> -static int hyperv_handle_properties(CPUState *cs)
> +static struct {
> +    const char *desc;
> +    struct {
> +        uint32_t fw;
> +        uint32_t bits;
> +    } flags[2];
> +} kvm_hyperv_properties[] = {
> +    [HYPERV_FEAT_RELAXED] = {
> +        .desc = "relaxed timing (hv-relaxed)",

I'd still like to avoid repeating the feature names.

> +        .flags = {
> +            {.fw = FEAT_HYPERV_EAX,
> +             .bits = HV_HYPERCALL_AVAILABLE},
> +            {.fw = FEAT_HV_RECOMM_EAX,
> +             .bits = HV_RELAXED_TIMING_RECOMMENDED}
> +        }
> +    },
> +    [HYPERV_FEAT_VAPIC] = {
> +        .desc = "virtual APIC (hv-vapic)",
> +        .flags = {
> +            {.fw = FEAT_HYPERV_EAX,
> +             .bits = HV_HYPERCALL_AVAILABLE | HV_APIC_ACCESS_AVAILABLE},
> +            {.fw = FEAT_HV_RECOMM_EAX,
> +             .bits = HV_APIC_ACCESS_RECOMMENDED}
> +        }
> +    },
> +    [HYPERV_FEAT_TIME] = {
> +        .desc = "clocksources (hv-time)",
> +        .flags = {
> +            {.fw = FEAT_HYPERV_EAX,
> +             .bits = HV_HYPERCALL_AVAILABLE | HV_TIME_REF_COUNT_AVAILABLE |
> +             HV_REFERENCE_TSC_AVAILABLE}
> +        }
> +    },
> +    [HYPERV_FEAT_CRASH] = {
> +        .desc = "crash MSRs (hv-crash)",
> +        .flags = {
> +            {.fw = FEAT_HYPERV_EDX,
> +             .bits = HV_GUEST_CRASH_MSR_AVAILABLE}
> +        }
> +    },
> +    [HYPERV_FEAT_RESET] = {
> +        .desc = "reset MSR (hv-reset)",
> +        .flags = {
> +            {.fw = FEAT_HYPERV_EAX,
> +             .bits = HV_RESET_AVAILABLE}
> +        }
> +    },
> +    [HYPERV_FEAT_VPINDEX] = {
> +        .desc = "VP_INDEX MSR (hv-vpindex)",
> +        .flags = {
> +            {.fw = FEAT_HYPERV_EAX,
> +             .bits = HV_VP_INDEX_AVAILABLE}
> +        }
> +    },
> +    [HYPERV_FEAT_RUNTIME] = {
> +        .desc = "VP_RUNTIME MSR (hv-runtime)",
> +        .flags = {
> +            {.fw = FEAT_HYPERV_EAX,
> +             .bits = HV_VP_RUNTIME_AVAILABLE}
> +        }
> +    },
> +    [HYPERV_FEAT_SYNIC] = {
> +        .desc = "synthetic interrupt controller (hv-synic)",
> +        .flags = {
> +            {.fw = FEAT_HYPERV_EAX,
> +             .bits = HV_SYNIC_AVAILABLE}
> +        }
> +    },
> +    [HYPERV_FEAT_STIMER] = {
> +        .desc = "synthetic timers (hv-stimer)",
> +        .flags = {
> +            {.fw = FEAT_HYPERV_EAX,
> +             .bits = HV_SYNTIMERS_AVAILABLE}
> +        }
> +    },
> +    [HYPERV_FEAT_FREQUENCIES] = {
> +        .desc = "frequency MSRs (hv-frequencies)",
> +        .flags = {
> +            {.fw = FEAT_HYPERV_EAX,
> +             .bits = HV_ACCESS_FREQUENCY_MSRS},
> +            {.fw = FEAT_HYPERV_EDX,
> +             .bits = HV_FREQUENCY_MSRS_AVAILABLE}
> +        }
> +    },
> +    [HYPERV_FEAT_REENLIGHTENMENT] = {
> +        .desc = "reenlightenment MSRs (hv-reenlightenment)",
> +        .flags = {
> +            {.fw = FEAT_HYPERV_EAX,
> +             .bits = HV_ACCESS_REENLIGHTENMENTS_CONTROL}
> +        }
> +    },
> +    [HYPERV_FEAT_TLBFLUSH] = {
> +        .desc = "paravirtualized TLB flush (hv-tlbflush)",
> +        .flags = {
> +            {.fw = FEAT_HV_RECOMM_EAX,
> +             .bits = HV_REMOTE_TLB_FLUSH_RECOMMENDED |
> +             HV_EX_PROCESSOR_MASKS_RECOMMENDED}
> +        }
> +    },
> +    [HYPERV_FEAT_EVMCS] = {
> +        .desc = "enlightened VMCS (hv-evmcs)",
> +        .flags = {
> +            {.fw = FEAT_HV_RECOMM_EAX,
> +             .bits = HV_ENLIGHTENED_VMCS_RECOMMENDED}
> +        }
> +    },
> +    [HYPERV_FEAT_IPI] = {
> +        .desc = "paravirtualized IPI (hv-ipi)",
> +        .flags = {
> +            {.fw = FEAT_HV_RECOMM_EAX,
> +             .bits = HV_CLUSTER_IPI_RECOMMENDED |
> +             HV_EX_PROCESSOR_MASKS_RECOMMENDED}
> +        }
> +    },
> +};
> +
> +static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max)
> +{
> +    struct kvm_cpuid2 *cpuid;
> +    int r, size;
> +
> +    size = sizeof(*cpuid) + max * sizeof(*cpuid->entries);
> +    cpuid = g_malloc0(size);
> +    cpuid->nent = max;
> +
> +    r = kvm_vcpu_ioctl(cs, KVM_GET_SUPPORTED_HV_CPUID, cpuid);
> +    if (r == 0 && cpuid->nent >= max) {
> +        r = -E2BIG;
> +    }
> +    if (r < 0) {
> +        if (r == -E2BIG) {
> +            g_free(cpuid);
> +            return NULL;
> +        } else {
> +            fprintf(stderr, "KVM_GET_SUPPORTED_HV_CPUID failed: %s\n",
> +                    strerror(-r));
> +            exit(1);
> +        }
> +    }
> +    return cpuid;
> +}
> +
> +/*
> + * Run KVM_GET_SUPPORTED_HV_CPUID ioctl(), allocating a buffer large enough
> + * for all entries.
> + */
> +static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs)
> +{
> +    struct kvm_cpuid2 *cpuid;
> +    int max = 7; /* 0x40000000..0x40000005, 0x4000000A */
> +
> +    /*
> +     * When the buffer is too small, KVM_GET_SUPPORTED_HV_CPUID fails with
> +     * -E2BIG, however, it doesn't report back the right size. Keep increasing
> +     * it and re-trying until we succeed.
> +     */

I'm still missing the idea of reiterating more than once: the ioctl
returns the actual size of the array.

> +    while ((cpuid = try_get_hv_cpuid(cs, max)) == NULL) {
> +        max++;
> +    }
> +    return cpuid;
> +}
> +
> +/*
> + * When KVM_GET_SUPPORTED_HV_CPUID is not supported we fill CPUID feature
> + * leaves from KVM_CAP_HYPERV* and present MSRs data.
> + */
> +static struct kvm_cpuid2 *get_supported_hv_cpuid_legacy(CPUState *cs)
>  {
>      X86CPU *cpu = X86_CPU(cs);
> -    CPUX86State *env = &cpu->env;
> +    struct kvm_cpuid2 *cpuid;
> +    struct kvm_cpuid_entry2 *entry_feat, *entry_recomm;
> +
> +    /* HV_CPUID_FEATURES, HV_CPUID_ENLIGHTMENT_INFO */
> +    cpuid = g_malloc0(sizeof(*cpuid) + 2 * sizeof(*cpuid->entries));
> +    cpuid->nent = 2;
> +
> +    /* HV_CPUID_VENDOR_AND_MAX_FUNCTIONS */
> +    entry_feat = &cpuid->entries[0];
> +    entry_feat->function = HV_CPUID_FEATURES;
>  
> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_RELAXED)) {
> -        env->features[FEAT_HYPERV_EAX] |= HV_HYPERCALL_AVAILABLE;
> +    entry_recomm = &cpuid->entries[1];
> +    entry_recomm->function = HV_CPUID_ENLIGHTMENT_INFO;
> +    entry_recomm->ebx = cpu->hyperv_spinlock_attempts;
> +
> +    if (kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV) > 0) {
> +        entry_feat->eax |= HV_HYPERCALL_AVAILABLE;
> +        entry_feat->eax |= HV_APIC_ACCESS_AVAILABLE;
> +        entry_feat->edx |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
> +        entry_recomm->eax |= HV_RELAXED_TIMING_RECOMMENDED;
> +        entry_recomm->eax |= HV_APIC_ACCESS_RECOMMENDED;
>      }
> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC)) {
> -        env->features[FEAT_HYPERV_EAX] |= HV_HYPERCALL_AVAILABLE;
> -        env->features[FEAT_HYPERV_EAX] |= HV_APIC_ACCESS_AVAILABLE;
> +
> +    if (kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_TIME) > 0) {
> +        entry_feat->eax |= HV_TIME_REF_COUNT_AVAILABLE;
> +        entry_feat->eax |= HV_REFERENCE_TSC_AVAILABLE;
>      }
> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_TIME)) {
> -        if (kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_TIME) <= 0) {
> -            fprintf(stderr, "Hyper-V clocksources "
> -                    "(requested by 'hv-time' cpu flag) "
> -                    "are not supported by kernel\n");
> -            return -ENOSYS;
> -        }
> -        env->features[FEAT_HYPERV_EAX] |= HV_HYPERCALL_AVAILABLE;
> -        env->features[FEAT_HYPERV_EAX] |= HV_TIME_REF_COUNT_AVAILABLE;
> -        env->features[FEAT_HYPERV_EAX] |= HV_REFERENCE_TSC_AVAILABLE;
> +
> +    if (has_msr_hv_frequencies) {
> +        entry_feat->eax |= HV_ACCESS_FREQUENCY_MSRS;
> +        entry_feat->edx |= HV_FREQUENCY_MSRS_AVAILABLE;
>      }
> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_FREQUENCIES)) {
> -        if (!has_msr_hv_frequencies) {
> -            fprintf(stderr, "Hyper-V frequency MSRs "
> -                    "(requested by 'hv-frequencies' cpu flag) "
> -                    "are not supported by kernel\n");
> -            return -ENOSYS;
> -        }
> -        env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS;
> -        env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE;
> +
> +    if (has_msr_hv_crash) {
> +        entry_feat->edx |= HV_GUEST_CRASH_MSR_AVAILABLE;
>      }
> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_CRASH)) {
> -        if (!has_msr_hv_crash) {
> -            fprintf(stderr, "Hyper-V crash MSRs "
> -                    "(requested by 'hv-crash' cpu flag) "
> -                    "are not supported by kernel\n");
> -            return -ENOSYS;
> -        }
> -        env->features[FEAT_HYPERV_EDX] |= HV_GUEST_CRASH_MSR_AVAILABLE;
> +
> +    if (has_msr_hv_reenlightenment) {
> +        entry_feat->eax |= HV_ACCESS_REENLIGHTENMENTS_CONTROL;
>      }
> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_REENLIGHTENMENT)) {
> -        if (!has_msr_hv_reenlightenment) {
> -            fprintf(stderr,
> -                    "Hyper-V Reenlightenment MSRs "
> -                    "(requested by 'hv-reenlightenment' cpu flag) "
> -                    "are not supported by kernel\n");
> -            return -ENOSYS;
> -        }
> -        env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_REENLIGHTENMENTS_CONTROL;
> +
> +    if (has_msr_hv_reset) {
> +        entry_feat->eax |= HV_RESET_AVAILABLE;
>      }
> -    env->features[FEAT_HYPERV_EDX] |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_RESET)) {
> -        if (!has_msr_hv_reset) {
> -            fprintf(stderr, "Hyper-V reset MSR "
> -                    "(requested by 'hv-reset' cpu flag) "
> -                    "is not supported by kernel\n");
> -            return -ENOSYS;
> -        }
> -        env->features[FEAT_HYPERV_EAX] |= HV_RESET_AVAILABLE;
> +
> +    if (has_msr_hv_vpindex) {
> +        entry_feat->eax |= HV_VP_INDEX_AVAILABLE;
>      }
> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
> -        if (!has_msr_hv_vpindex) {
> -            fprintf(stderr, "Hyper-V VP_INDEX MSR "
> -                    "(requested by 'hv-vpindex' cpu flag) "
> -                    "is not supported by kernel\n");
> -            return -ENOSYS;
> -        }
> -        env->features[FEAT_HYPERV_EAX] |= HV_VP_INDEX_AVAILABLE;
> +
> +    if (has_msr_hv_runtime) {
> +        entry_feat->eax |= HV_VP_RUNTIME_AVAILABLE;
>      }
> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_RUNTIME)) {
> -        if (!has_msr_hv_runtime) {
> -            fprintf(stderr, "Hyper-V VP_RUNTIME MSR "
> -                    "(requested by 'hv-runtime' cpu flag) "
> -                    "is not supported by kernel\n");
> -            return -ENOSYS;
> +
> +    if (has_msr_hv_synic) {
> +        unsigned int cap = cpu->hyperv_synic_kvm_only ?
> +            KVM_CAP_HYPERV_SYNIC : KVM_CAP_HYPERV_SYNIC2;
> +
> +        if (kvm_check_extension(cs->kvm_state, cap) > 0) {
> +            entry_feat->eax |= HV_SYNIC_AVAILABLE;
>          }
> -        env->features[FEAT_HYPERV_EAX] |= HV_VP_RUNTIME_AVAILABLE;
>      }
> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC)) {
> -        unsigned int cap = KVM_CAP_HYPERV_SYNIC;
> -        if (!cpu->hyperv_synic_kvm_only) {
> -            if (!hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
> -                fprintf(stderr, "Hyper-V SynIC "
> -                        "(requested by 'hv-synic' cpu flag) "
> -                        "requires Hyper-V VP_INDEX ('hv-vpindex')\n");
> -            return -ENOSYS;
> -            }
> -            cap = KVM_CAP_HYPERV_SYNIC2;
> -        }
>  
> -        if (!has_msr_hv_synic || !kvm_check_extension(cs->kvm_state, cap)) {
> -            fprintf(stderr, "Hyper-V SynIC (requested by 'hv-synic' cpu flag) "
> -                    "is not supported by kernel\n");
> -            return -ENOSYS;
> -        }
> +    if (has_msr_hv_stimer) {
> +        entry_feat->eax |= HV_SYNTIMERS_AVAILABLE;
> +    }
>  
> -        env->features[FEAT_HYPERV_EAX] |= HV_SYNIC_AVAILABLE;
> +    if (kvm_check_extension(cs->kvm_state,
> +                            KVM_CAP_HYPERV_TLBFLUSH) > 0) {
> +        entry_recomm->eax |= HV_REMOTE_TLB_FLUSH_RECOMMENDED;
> +        entry_recomm->eax |= HV_EX_PROCESSOR_MASKS_RECOMMENDED;
>      }
> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_STIMER)) {
> -        if (!has_msr_hv_stimer) {
> -            fprintf(stderr, "Hyper-V timers aren't supported by kernel\n");
> -            return -ENOSYS;
> -        }
> -        env->features[FEAT_HYPERV_EAX] |= HV_SYNTIMERS_AVAILABLE;
> +
> +    if (kvm_check_extension(cs->kvm_state,
> +                            KVM_CAP_HYPERV_ENLIGHTENED_VMCS) > 0) {
> +        entry_recomm->eax |= HV_ENLIGHTENED_VMCS_RECOMMENDED;
>      }
> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_RELAXED)) {
> -        env->features[FEAT_HV_RECOMM_EAX] |= HV_RELAXED_TIMING_RECOMMENDED;
> +
> +    if (kvm_check_extension(cs->kvm_state,
> +                            KVM_CAP_HYPERV_SEND_IPI) > 0) {
> +        entry_recomm->eax |= HV_CLUSTER_IPI_RECOMMENDED;
> +        entry_recomm->eax |= HV_EX_PROCESSOR_MASKS_RECOMMENDED;
>      }
> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC)) {
> -        env->features[FEAT_HV_RECOMM_EAX] |= HV_APIC_ACCESS_RECOMMENDED;
> -    }
> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_TLBFLUSH)) {
> -        if (kvm_check_extension(cs->kvm_state,
> -                                KVM_CAP_HYPERV_TLBFLUSH) <= 0) {
> -            fprintf(stderr, "Hyper-V TLB flush support "
> -                    "(requested by 'hv-tlbflush' cpu flag) "
> -                    " is not supported by kernel\n");
> -            return -ENOSYS;
> -        }
> -        env->features[FEAT_HV_RECOMM_EAX] |= HV_REMOTE_TLB_FLUSH_RECOMMENDED;
> -        env->features[FEAT_HV_RECOMM_EAX] |= HV_EX_PROCESSOR_MASKS_RECOMMENDED;
> +
> +    return cpuid;
> +}
> +
> +static int hv_cpuid_get_fw(struct kvm_cpuid2 *cpuid, int fw, uint32_t *r)
> +{
> +    struct kvm_cpuid_entry2 *entry;
> +    uint32_t func;
> +    int reg;
> +
> +    switch (fw) {
> +    case FEAT_HYPERV_EAX:
> +        reg = R_EAX;
> +        func = HV_CPUID_FEATURES;
> +        break;
> +    case FEAT_HYPERV_EDX:
> +        reg = R_EDX;
> +        func = HV_CPUID_FEATURES;
> +        break;
> +    case FEAT_HV_RECOMM_EAX:
> +        reg = R_EAX;
> +        func = HV_CPUID_ENLIGHTMENT_INFO;
> +        break;
> +    default:
> +        return -EINVAL;
>      }
> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_IPI)) {
> -        if (kvm_check_extension(cs->kvm_state,
> -                                KVM_CAP_HYPERV_SEND_IPI) <= 0) {
> -            fprintf(stderr, "Hyper-V IPI send support "
> -                    "(requested by 'hv-ipi' cpu flag) "
> -                    " is not supported by kernel\n");
> -            return -ENOSYS;
> +
> +    entry = cpuid_find_entry(cpuid, func, 0);
> +    if (!entry) {
> +        return -ENOENT;
> +    }
> +
> +    switch (reg) {
> +    case R_EAX:
> +        *r = entry->eax;
> +        break;
> +    case R_EDX:
> +        *r = entry->edx;
> +        break;
> +    default:
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}
> +
> +static int hv_cpuid_check_and_set(CPUState *cs, struct kvm_cpuid2 *cpuid,
> +                                  int feature)
> +{
> +    X86CPU *cpu = X86_CPU(cs);
> +    CPUX86State *env = &cpu->env;
> +    uint32_t r, fw, bits;;
> +    int i;
> +
> +    if (!hyperv_feat_enabled(cpu, feature)) {
> +        return 0;
> +    }
> +
> +    for (i = 0; i < ARRAY_SIZE(kvm_hyperv_properties[feature].flags); i++) {
> +        fw = kvm_hyperv_properties[feature].flags[i].fw;
> +        bits = kvm_hyperv_properties[feature].flags[i].bits;
> +
> +        if (!fw) {
> +            continue;
>          }
> -        env->features[FEAT_HV_RECOMM_EAX] |= HV_CLUSTER_IPI_RECOMMENDED;
> -        env->features[FEAT_HV_RECOMM_EAX] |= HV_EX_PROCESSOR_MASKS_RECOMMENDED;
> +
> +        if (hv_cpuid_get_fw(cpuid, fw, &r) || (r & bits) != bits) {
> +            fprintf(stderr,
> +                    "Hyper-V %s is not supported by kernel\n",
> +                    kvm_hyperv_properties[feature].desc);
> +            return 1;
> +        }
> +
> +        env->features[fw] |= bits;
>      }
> +
> +    return 0;
> +}
> +
> +static int hyperv_handle_properties(CPUState *cs)
> +{
> +    X86CPU *cpu = X86_CPU(cs);
> +    CPUX86State *env = &cpu->env;
> +    struct kvm_cpuid2 *cpuid;
> +    int r = 0;
> +
>      if (hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS)) {
>          uint16_t evmcs_version;
>  
>          if (kvm_vcpu_enable_cap(cs, KVM_CAP_HYPERV_ENLIGHTENED_VMCS, 0,
>                                  (uintptr_t)&evmcs_version)) {
> -            fprintf(stderr, "Hyper-V Enlightened VMCS "
> -                    "(requested by 'hv-evmcs' cpu flag) "
> -                    "is not supported by kernel\n");
> +            fprintf(stderr, "Hyper-V %s is not supported by kernel\n",
> +                    kvm_hyperv_properties[HYPERV_FEAT_EVMCS].desc);
>              return -ENOSYS;
>          }
>          env->features[FEAT_HV_RECOMM_EAX] |= HV_ENLIGHTENED_VMCS_RECOMMENDED;
>          env->features[FEAT_HV_NESTED_EAX] = evmcs_version;
>      }
>  
> -    return 0;
> +    if (kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_CPUID) > 0) {
> +        cpuid = get_supported_hv_cpuid(cs);
> +    } else {
> +        cpuid = get_supported_hv_cpuid_legacy(cs);
> +    }
> +
> +    /* Features */
> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_RELAXED);
> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_VAPIC);
> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_TIME);
> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_CRASH);
> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_RESET);
> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_VPINDEX);
> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_RUNTIME);
> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_SYNIC);
> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_STIMER);
> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_FREQUENCIES);
> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_REENLIGHTENMENT);
> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_TLBFLUSH);
> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_EVMCS);
> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_IPI);

Can't this be expressed by a regualr for() loop?

Thanks,
Roman.

> +
> +    /* Dependencies */
> +    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC) &&
> +        !cpu->hyperv_synic_kvm_only &&
> +        !hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
> +        fprintf(stderr, "Hyper-V %s requires %s\n",
> +                kvm_hyperv_properties[HYPERV_FEAT_SYNIC].desc,
> +                kvm_hyperv_properties[HYPERV_FEAT_VPINDEX].desc);
> +        r |= 1;
> +    }
> +
> +    /* Not exposed by KVM but needed to make CPU hotplug in Windows work */
> +    env->features[FEAT_HYPERV_EDX] |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
> +
> +    g_free(cpuid);
> +
> +    return r ? -ENOSYS : 0;
>  }
>  
>  static int hyperv_init_vcpu(X86CPU *cpu)
> -- 
> 2.20.1
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/9] i386/kvm: add support for KVM_GET_SUPPORTED_HV_CPUID
  2019-05-27 15:52   ` Roman Kagan
@ 2019-05-27 16:39     ` Vitaly Kuznetsov
  2019-05-30 17:55       ` Roman Kagan
  0 siblings, 1 reply; 15+ messages in thread
From: Vitaly Kuznetsov @ 2019-05-27 16:39 UTC (permalink / raw)
  To: Roman Kagan
  Cc: Daniel P . Berrangé,
	Eduardo Habkost, Marcelo Tosatti, Dr . David Alan Gilbert,
	qemu-devel, Paolo Bonzini, Richard Henderson

Roman Kagan <rkagan@virtuozzo.com> writes:

> On Fri, May 17, 2019 at 04:19:17PM +0200, Vitaly Kuznetsov wrote:
>> KVM now supports reporting supported Hyper-V features through CPUID
>> (KVM_GET_SUPPORTED_HV_CPUID ioctl). Going forward, this is going to be
>> the only way to announce new functionality and this has already happened
>> with Direct Mode stimers.
>> 
>> While we could just support KVM_GET_SUPPORTED_HV_CPUID for new features,
>> it seems to be beneficial to use it for all Hyper-V enlightenments when
>> possible. This way we can implement 'hv-all' pass-through mode giving the
>> guest all supported Hyper-V features even when QEMU knows nothing about
>> them.
>> 
>> Implementation-wise we create a new kvm_hyperv_properties structure
>> defining Hyper-V features, get_supported_hv_cpuid()/
>> get_supported_hv_cpuid_legacy() returning the supported CPUID set and
>> a bit over-engineered hv_cpuid_check_and_set() which we will also be
>> used to set cpu->hyperv_* properties for 'hv-all' mode.
>> 
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>>  target/i386/kvm.c | 474 ++++++++++++++++++++++++++++++++++------------
>>  1 file changed, 356 insertions(+), 118 deletions(-)
>> 
>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> index 3daac1e4f4..6ead422efa 100644
>> --- a/target/i386/kvm.c
>> +++ b/target/i386/kvm.c
>> @@ -684,156 +684,394 @@ static bool tsc_is_stable_and_known(CPUX86State *env)
>>          || env->user_tsc_khz;
>>  }
>>  
>> -static int hyperv_handle_properties(CPUState *cs)
>> +static struct {
>> +    const char *desc;
>> +    struct {
>> +        uint32_t fw;
>> +        uint32_t bits;
>> +    } flags[2];
>> +} kvm_hyperv_properties[] = {
>> +    [HYPERV_FEAT_RELAXED] = {
>> +        .desc = "relaxed timing (hv-relaxed)",
>
> I'd still like to avoid repeating the feature names.

I didn't find a convenient way to extract this from
x86_cpu_properties[]. This can be done but to me it looked like
over-engineering as the only reason we have this here is to have a
somewhat nicer message when something is unsupported.

>
>> +        .flags = {
>> +            {.fw = FEAT_HYPERV_EAX,
>> +             .bits = HV_HYPERCALL_AVAILABLE},
>> +            {.fw = FEAT_HV_RECOMM_EAX,
>> +             .bits = HV_RELAXED_TIMING_RECOMMENDED}
>> +        }
>> +    },
>> +    [HYPERV_FEAT_VAPIC] = {
>> +        .desc = "virtual APIC (hv-vapic)",
>> +        .flags = {
>> +            {.fw = FEAT_HYPERV_EAX,
>> +             .bits = HV_HYPERCALL_AVAILABLE | HV_APIC_ACCESS_AVAILABLE},
>> +            {.fw = FEAT_HV_RECOMM_EAX,
>> +             .bits = HV_APIC_ACCESS_RECOMMENDED}
>> +        }
>> +    },
>> +    [HYPERV_FEAT_TIME] = {
>> +        .desc = "clocksources (hv-time)",
>> +        .flags = {
>> +            {.fw = FEAT_HYPERV_EAX,
>> +             .bits = HV_HYPERCALL_AVAILABLE | HV_TIME_REF_COUNT_AVAILABLE |
>> +             HV_REFERENCE_TSC_AVAILABLE}
>> +        }
>> +    },
>> +    [HYPERV_FEAT_CRASH] = {
>> +        .desc = "crash MSRs (hv-crash)",
>> +        .flags = {
>> +            {.fw = FEAT_HYPERV_EDX,
>> +             .bits = HV_GUEST_CRASH_MSR_AVAILABLE}
>> +        }
>> +    },
>> +    [HYPERV_FEAT_RESET] = {
>> +        .desc = "reset MSR (hv-reset)",
>> +        .flags = {
>> +            {.fw = FEAT_HYPERV_EAX,
>> +             .bits = HV_RESET_AVAILABLE}
>> +        }
>> +    },
>> +    [HYPERV_FEAT_VPINDEX] = {
>> +        .desc = "VP_INDEX MSR (hv-vpindex)",
>> +        .flags = {
>> +            {.fw = FEAT_HYPERV_EAX,
>> +             .bits = HV_VP_INDEX_AVAILABLE}
>> +        }
>> +    },
>> +    [HYPERV_FEAT_RUNTIME] = {
>> +        .desc = "VP_RUNTIME MSR (hv-runtime)",
>> +        .flags = {
>> +            {.fw = FEAT_HYPERV_EAX,
>> +             .bits = HV_VP_RUNTIME_AVAILABLE}
>> +        }
>> +    },
>> +    [HYPERV_FEAT_SYNIC] = {
>> +        .desc = "synthetic interrupt controller (hv-synic)",
>> +        .flags = {
>> +            {.fw = FEAT_HYPERV_EAX,
>> +             .bits = HV_SYNIC_AVAILABLE}
>> +        }
>> +    },
>> +    [HYPERV_FEAT_STIMER] = {
>> +        .desc = "synthetic timers (hv-stimer)",
>> +        .flags = {
>> +            {.fw = FEAT_HYPERV_EAX,
>> +             .bits = HV_SYNTIMERS_AVAILABLE}
>> +        }
>> +    },
>> +    [HYPERV_FEAT_FREQUENCIES] = {
>> +        .desc = "frequency MSRs (hv-frequencies)",
>> +        .flags = {
>> +            {.fw = FEAT_HYPERV_EAX,
>> +             .bits = HV_ACCESS_FREQUENCY_MSRS},
>> +            {.fw = FEAT_HYPERV_EDX,
>> +             .bits = HV_FREQUENCY_MSRS_AVAILABLE}
>> +        }
>> +    },
>> +    [HYPERV_FEAT_REENLIGHTENMENT] = {
>> +        .desc = "reenlightenment MSRs (hv-reenlightenment)",
>> +        .flags = {
>> +            {.fw = FEAT_HYPERV_EAX,
>> +             .bits = HV_ACCESS_REENLIGHTENMENTS_CONTROL}
>> +        }
>> +    },
>> +    [HYPERV_FEAT_TLBFLUSH] = {
>> +        .desc = "paravirtualized TLB flush (hv-tlbflush)",
>> +        .flags = {
>> +            {.fw = FEAT_HV_RECOMM_EAX,
>> +             .bits = HV_REMOTE_TLB_FLUSH_RECOMMENDED |
>> +             HV_EX_PROCESSOR_MASKS_RECOMMENDED}
>> +        }
>> +    },
>> +    [HYPERV_FEAT_EVMCS] = {
>> +        .desc = "enlightened VMCS (hv-evmcs)",
>> +        .flags = {
>> +            {.fw = FEAT_HV_RECOMM_EAX,
>> +             .bits = HV_ENLIGHTENED_VMCS_RECOMMENDED}
>> +        }
>> +    },
>> +    [HYPERV_FEAT_IPI] = {
>> +        .desc = "paravirtualized IPI (hv-ipi)",
>> +        .flags = {
>> +            {.fw = FEAT_HV_RECOMM_EAX,
>> +             .bits = HV_CLUSTER_IPI_RECOMMENDED |
>> +             HV_EX_PROCESSOR_MASKS_RECOMMENDED}
>> +        }
>> +    },
>> +};
>> +
>> +static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max)
>> +{
>> +    struct kvm_cpuid2 *cpuid;
>> +    int r, size;
>> +
>> +    size = sizeof(*cpuid) + max * sizeof(*cpuid->entries);
>> +    cpuid = g_malloc0(size);
>> +    cpuid->nent = max;
>> +
>> +    r = kvm_vcpu_ioctl(cs, KVM_GET_SUPPORTED_HV_CPUID, cpuid);
>> +    if (r == 0 && cpuid->nent >= max) {
>> +        r = -E2BIG;
>> +    }
>> +    if (r < 0) {
>> +        if (r == -E2BIG) {
>> +            g_free(cpuid);
>> +            return NULL;
>> +        } else {
>> +            fprintf(stderr, "KVM_GET_SUPPORTED_HV_CPUID failed: %s\n",
>> +                    strerror(-r));
>> +            exit(1);
>> +        }
>> +    }
>> +    return cpuid;
>> +}
>> +
>> +/*
>> + * Run KVM_GET_SUPPORTED_HV_CPUID ioctl(), allocating a buffer large enough
>> + * for all entries.
>> + */
>> +static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs)
>> +{
>> +    struct kvm_cpuid2 *cpuid;
>> +    int max = 7; /* 0x40000000..0x40000005, 0x4000000A */
>> +
>> +    /*
>> +     * When the buffer is too small, KVM_GET_SUPPORTED_HV_CPUID fails with
>> +     * -E2BIG, however, it doesn't report back the right size. Keep increasing
>> +     * it and re-trying until we succeed.
>> +     */
>
> I'm still missing the idea of reiterating more than once: the ioctl
> returns the actual size of the array.

Hm, I think I checked that and it doesn't seem to be the case.

The code in kvm_vcpu_ioctl_get_hv_cpuid():

	if (cpuid->nent < nent)
		return -E2BIG;

	if (cpuid->nent > nent)
		cpuid->nent = nent;

(I think I even ran a test after your comment on v1 and it it
confirmed nent is not set on E2BIG). Am I missing something obvious?

>
>> +    while ((cpuid = try_get_hv_cpuid(cs, max)) == NULL) {
>> +        max++;
>> +    }
>> +    return cpuid;
>> +}
>> +
>> +/*
>> + * When KVM_GET_SUPPORTED_HV_CPUID is not supported we fill CPUID feature
>> + * leaves from KVM_CAP_HYPERV* and present MSRs data.
>> + */
>> +static struct kvm_cpuid2 *get_supported_hv_cpuid_legacy(CPUState *cs)
>>  {
>>      X86CPU *cpu = X86_CPU(cs);
>> -    CPUX86State *env = &cpu->env;
>> +    struct kvm_cpuid2 *cpuid;
>> +    struct kvm_cpuid_entry2 *entry_feat, *entry_recomm;
>> +
>> +    /* HV_CPUID_FEATURES, HV_CPUID_ENLIGHTMENT_INFO */
>> +    cpuid = g_malloc0(sizeof(*cpuid) + 2 * sizeof(*cpuid->entries));
>> +    cpuid->nent = 2;
>> +
>> +    /* HV_CPUID_VENDOR_AND_MAX_FUNCTIONS */
>> +    entry_feat = &cpuid->entries[0];
>> +    entry_feat->function = HV_CPUID_FEATURES;
>>  
>> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_RELAXED)) {
>> -        env->features[FEAT_HYPERV_EAX] |= HV_HYPERCALL_AVAILABLE;
>> +    entry_recomm = &cpuid->entries[1];
>> +    entry_recomm->function = HV_CPUID_ENLIGHTMENT_INFO;
>> +    entry_recomm->ebx = cpu->hyperv_spinlock_attempts;
>> +
>> +    if (kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV) > 0) {
>> +        entry_feat->eax |= HV_HYPERCALL_AVAILABLE;
>> +        entry_feat->eax |= HV_APIC_ACCESS_AVAILABLE;
>> +        entry_feat->edx |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
>> +        entry_recomm->eax |= HV_RELAXED_TIMING_RECOMMENDED;
>> +        entry_recomm->eax |= HV_APIC_ACCESS_RECOMMENDED;
>>      }
>> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC)) {
>> -        env->features[FEAT_HYPERV_EAX] |= HV_HYPERCALL_AVAILABLE;
>> -        env->features[FEAT_HYPERV_EAX] |= HV_APIC_ACCESS_AVAILABLE;
>> +
>> +    if (kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_TIME) > 0) {
>> +        entry_feat->eax |= HV_TIME_REF_COUNT_AVAILABLE;
>> +        entry_feat->eax |= HV_REFERENCE_TSC_AVAILABLE;
>>      }
>> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_TIME)) {
>> -        if (kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_TIME) <= 0) {
>> -            fprintf(stderr, "Hyper-V clocksources "
>> -                    "(requested by 'hv-time' cpu flag) "
>> -                    "are not supported by kernel\n");
>> -            return -ENOSYS;
>> -        }
>> -        env->features[FEAT_HYPERV_EAX] |= HV_HYPERCALL_AVAILABLE;
>> -        env->features[FEAT_HYPERV_EAX] |= HV_TIME_REF_COUNT_AVAILABLE;
>> -        env->features[FEAT_HYPERV_EAX] |= HV_REFERENCE_TSC_AVAILABLE;
>> +
>> +    if (has_msr_hv_frequencies) {
>> +        entry_feat->eax |= HV_ACCESS_FREQUENCY_MSRS;
>> +        entry_feat->edx |= HV_FREQUENCY_MSRS_AVAILABLE;
>>      }
>> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_FREQUENCIES)) {
>> -        if (!has_msr_hv_frequencies) {
>> -            fprintf(stderr, "Hyper-V frequency MSRs "
>> -                    "(requested by 'hv-frequencies' cpu flag) "
>> -                    "are not supported by kernel\n");
>> -            return -ENOSYS;
>> -        }
>> -        env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS;
>> -        env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE;
>> +
>> +    if (has_msr_hv_crash) {
>> +        entry_feat->edx |= HV_GUEST_CRASH_MSR_AVAILABLE;
>>      }
>> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_CRASH)) {
>> -        if (!has_msr_hv_crash) {
>> -            fprintf(stderr, "Hyper-V crash MSRs "
>> -                    "(requested by 'hv-crash' cpu flag) "
>> -                    "are not supported by kernel\n");
>> -            return -ENOSYS;
>> -        }
>> -        env->features[FEAT_HYPERV_EDX] |= HV_GUEST_CRASH_MSR_AVAILABLE;
>> +
>> +    if (has_msr_hv_reenlightenment) {
>> +        entry_feat->eax |= HV_ACCESS_REENLIGHTENMENTS_CONTROL;
>>      }
>> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_REENLIGHTENMENT)) {
>> -        if (!has_msr_hv_reenlightenment) {
>> -            fprintf(stderr,
>> -                    "Hyper-V Reenlightenment MSRs "
>> -                    "(requested by 'hv-reenlightenment' cpu flag) "
>> -                    "are not supported by kernel\n");
>> -            return -ENOSYS;
>> -        }
>> -        env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_REENLIGHTENMENTS_CONTROL;
>> +
>> +    if (has_msr_hv_reset) {
>> +        entry_feat->eax |= HV_RESET_AVAILABLE;
>>      }
>> -    env->features[FEAT_HYPERV_EDX] |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
>> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_RESET)) {
>> -        if (!has_msr_hv_reset) {
>> -            fprintf(stderr, "Hyper-V reset MSR "
>> -                    "(requested by 'hv-reset' cpu flag) "
>> -                    "is not supported by kernel\n");
>> -            return -ENOSYS;
>> -        }
>> -        env->features[FEAT_HYPERV_EAX] |= HV_RESET_AVAILABLE;
>> +
>> +    if (has_msr_hv_vpindex) {
>> +        entry_feat->eax |= HV_VP_INDEX_AVAILABLE;
>>      }
>> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
>> -        if (!has_msr_hv_vpindex) {
>> -            fprintf(stderr, "Hyper-V VP_INDEX MSR "
>> -                    "(requested by 'hv-vpindex' cpu flag) "
>> -                    "is not supported by kernel\n");
>> -            return -ENOSYS;
>> -        }
>> -        env->features[FEAT_HYPERV_EAX] |= HV_VP_INDEX_AVAILABLE;
>> +
>> +    if (has_msr_hv_runtime) {
>> +        entry_feat->eax |= HV_VP_RUNTIME_AVAILABLE;
>>      }
>> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_RUNTIME)) {
>> -        if (!has_msr_hv_runtime) {
>> -            fprintf(stderr, "Hyper-V VP_RUNTIME MSR "
>> -                    "(requested by 'hv-runtime' cpu flag) "
>> -                    "is not supported by kernel\n");
>> -            return -ENOSYS;
>> +
>> +    if (has_msr_hv_synic) {
>> +        unsigned int cap = cpu->hyperv_synic_kvm_only ?
>> +            KVM_CAP_HYPERV_SYNIC : KVM_CAP_HYPERV_SYNIC2;
>> +
>> +        if (kvm_check_extension(cs->kvm_state, cap) > 0) {
>> +            entry_feat->eax |= HV_SYNIC_AVAILABLE;
>>          }
>> -        env->features[FEAT_HYPERV_EAX] |= HV_VP_RUNTIME_AVAILABLE;
>>      }
>> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC)) {
>> -        unsigned int cap = KVM_CAP_HYPERV_SYNIC;
>> -        if (!cpu->hyperv_synic_kvm_only) {
>> -            if (!hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
>> -                fprintf(stderr, "Hyper-V SynIC "
>> -                        "(requested by 'hv-synic' cpu flag) "
>> -                        "requires Hyper-V VP_INDEX ('hv-vpindex')\n");
>> -            return -ENOSYS;
>> -            }
>> -            cap = KVM_CAP_HYPERV_SYNIC2;
>> -        }
>>  
>> -        if (!has_msr_hv_synic || !kvm_check_extension(cs->kvm_state, cap)) {
>> -            fprintf(stderr, "Hyper-V SynIC (requested by 'hv-synic' cpu flag) "
>> -                    "is not supported by kernel\n");
>> -            return -ENOSYS;
>> -        }
>> +    if (has_msr_hv_stimer) {
>> +        entry_feat->eax |= HV_SYNTIMERS_AVAILABLE;
>> +    }
>>  
>> -        env->features[FEAT_HYPERV_EAX] |= HV_SYNIC_AVAILABLE;
>> +    if (kvm_check_extension(cs->kvm_state,
>> +                            KVM_CAP_HYPERV_TLBFLUSH) > 0) {
>> +        entry_recomm->eax |= HV_REMOTE_TLB_FLUSH_RECOMMENDED;
>> +        entry_recomm->eax |= HV_EX_PROCESSOR_MASKS_RECOMMENDED;
>>      }
>> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_STIMER)) {
>> -        if (!has_msr_hv_stimer) {
>> -            fprintf(stderr, "Hyper-V timers aren't supported by kernel\n");
>> -            return -ENOSYS;
>> -        }
>> -        env->features[FEAT_HYPERV_EAX] |= HV_SYNTIMERS_AVAILABLE;
>> +
>> +    if (kvm_check_extension(cs->kvm_state,
>> +                            KVM_CAP_HYPERV_ENLIGHTENED_VMCS) > 0) {
>> +        entry_recomm->eax |= HV_ENLIGHTENED_VMCS_RECOMMENDED;
>>      }
>> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_RELAXED)) {
>> -        env->features[FEAT_HV_RECOMM_EAX] |= HV_RELAXED_TIMING_RECOMMENDED;
>> +
>> +    if (kvm_check_extension(cs->kvm_state,
>> +                            KVM_CAP_HYPERV_SEND_IPI) > 0) {
>> +        entry_recomm->eax |= HV_CLUSTER_IPI_RECOMMENDED;
>> +        entry_recomm->eax |= HV_EX_PROCESSOR_MASKS_RECOMMENDED;
>>      }
>> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC)) {
>> -        env->features[FEAT_HV_RECOMM_EAX] |= HV_APIC_ACCESS_RECOMMENDED;
>> -    }
>> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_TLBFLUSH)) {
>> -        if (kvm_check_extension(cs->kvm_state,
>> -                                KVM_CAP_HYPERV_TLBFLUSH) <= 0) {
>> -            fprintf(stderr, "Hyper-V TLB flush support "
>> -                    "(requested by 'hv-tlbflush' cpu flag) "
>> -                    " is not supported by kernel\n");
>> -            return -ENOSYS;
>> -        }
>> -        env->features[FEAT_HV_RECOMM_EAX] |= HV_REMOTE_TLB_FLUSH_RECOMMENDED;
>> -        env->features[FEAT_HV_RECOMM_EAX] |= HV_EX_PROCESSOR_MASKS_RECOMMENDED;
>> +
>> +    return cpuid;
>> +}
>> +
>> +static int hv_cpuid_get_fw(struct kvm_cpuid2 *cpuid, int fw, uint32_t *r)
>> +{
>> +    struct kvm_cpuid_entry2 *entry;
>> +    uint32_t func;
>> +    int reg;
>> +
>> +    switch (fw) {
>> +    case FEAT_HYPERV_EAX:
>> +        reg = R_EAX;
>> +        func = HV_CPUID_FEATURES;
>> +        break;
>> +    case FEAT_HYPERV_EDX:
>> +        reg = R_EDX;
>> +        func = HV_CPUID_FEATURES;
>> +        break;
>> +    case FEAT_HV_RECOMM_EAX:
>> +        reg = R_EAX;
>> +        func = HV_CPUID_ENLIGHTMENT_INFO;
>> +        break;
>> +    default:
>> +        return -EINVAL;
>>      }
>> -    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_IPI)) {
>> -        if (kvm_check_extension(cs->kvm_state,
>> -                                KVM_CAP_HYPERV_SEND_IPI) <= 0) {
>> -            fprintf(stderr, "Hyper-V IPI send support "
>> -                    "(requested by 'hv-ipi' cpu flag) "
>> -                    " is not supported by kernel\n");
>> -            return -ENOSYS;
>> +
>> +    entry = cpuid_find_entry(cpuid, func, 0);
>> +    if (!entry) {
>> +        return -ENOENT;
>> +    }
>> +
>> +    switch (reg) {
>> +    case R_EAX:
>> +        *r = entry->eax;
>> +        break;
>> +    case R_EDX:
>> +        *r = entry->edx;
>> +        break;
>> +    default:
>> +        return -EINVAL;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int hv_cpuid_check_and_set(CPUState *cs, struct kvm_cpuid2 *cpuid,
>> +                                  int feature)
>> +{
>> +    X86CPU *cpu = X86_CPU(cs);
>> +    CPUX86State *env = &cpu->env;
>> +    uint32_t r, fw, bits;;
>> +    int i;
>> +
>> +    if (!hyperv_feat_enabled(cpu, feature)) {
>> +        return 0;
>> +    }
>> +
>> +    for (i = 0; i < ARRAY_SIZE(kvm_hyperv_properties[feature].flags); i++) {
>> +        fw = kvm_hyperv_properties[feature].flags[i].fw;
>> +        bits = kvm_hyperv_properties[feature].flags[i].bits;
>> +
>> +        if (!fw) {
>> +            continue;
>>          }
>> -        env->features[FEAT_HV_RECOMM_EAX] |= HV_CLUSTER_IPI_RECOMMENDED;
>> -        env->features[FEAT_HV_RECOMM_EAX] |= HV_EX_PROCESSOR_MASKS_RECOMMENDED;
>> +
>> +        if (hv_cpuid_get_fw(cpuid, fw, &r) || (r & bits) != bits) {
>> +            fprintf(stderr,
>> +                    "Hyper-V %s is not supported by kernel\n",
>> +                    kvm_hyperv_properties[feature].desc);
>> +            return 1;
>> +        }
>> +
>> +        env->features[fw] |= bits;
>>      }
>> +
>> +    return 0;
>> +}
>> +
>> +static int hyperv_handle_properties(CPUState *cs)
>> +{
>> +    X86CPU *cpu = X86_CPU(cs);
>> +    CPUX86State *env = &cpu->env;
>> +    struct kvm_cpuid2 *cpuid;
>> +    int r = 0;
>> +
>>      if (hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS)) {
>>          uint16_t evmcs_version;
>>  
>>          if (kvm_vcpu_enable_cap(cs, KVM_CAP_HYPERV_ENLIGHTENED_VMCS, 0,
>>                                  (uintptr_t)&evmcs_version)) {
>> -            fprintf(stderr, "Hyper-V Enlightened VMCS "
>> -                    "(requested by 'hv-evmcs' cpu flag) "
>> -                    "is not supported by kernel\n");
>> +            fprintf(stderr, "Hyper-V %s is not supported by kernel\n",
>> +                    kvm_hyperv_properties[HYPERV_FEAT_EVMCS].desc);
>>              return -ENOSYS;
>>          }
>>          env->features[FEAT_HV_RECOMM_EAX] |= HV_ENLIGHTENED_VMCS_RECOMMENDED;
>>          env->features[FEAT_HV_NESTED_EAX] = evmcs_version;
>>      }
>>  
>> -    return 0;
>> +    if (kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_CPUID) > 0) {
>> +        cpuid = get_supported_hv_cpuid(cs);
>> +    } else {
>> +        cpuid = get_supported_hv_cpuid_legacy(cs);
>> +    }
>> +
>> +    /* Features */
>> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_RELAXED);
>> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_VAPIC);
>> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_TIME);
>> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_CRASH);
>> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_RESET);
>> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_VPINDEX);
>> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_RUNTIME);
>> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_SYNIC);
>> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_STIMER);
>> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_FREQUENCIES);
>> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_REENLIGHTENMENT);
>> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_TLBFLUSH);
>> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_EVMCS);
>> +    r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_IPI);
>
> Can't this be expressed by a regualr for() loop?

Yes, it can. In future we can add a special flag to skip something here
if we need to (but there's no need for it now).

>
> Thanks,
> Roman.
>
>> +
>> +    /* Dependencies */
>> +    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC) &&
>> +        !cpu->hyperv_synic_kvm_only &&
>> +        !hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
>> +        fprintf(stderr, "Hyper-V %s requires %s\n",
>> +                kvm_hyperv_properties[HYPERV_FEAT_SYNIC].desc,
>> +                kvm_hyperv_properties[HYPERV_FEAT_VPINDEX].desc);
>> +        r |= 1;
>> +    }
>> +
>> +    /* Not exposed by KVM but needed to make CPU hotplug in Windows work */
>> +    env->features[FEAT_HYPERV_EDX] |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
>> +
>> +    g_free(cpuid);
>> +
>> +    return r ? -ENOSYS : 0;
>>  }
>>  
>>  static int hyperv_init_vcpu(X86CPU *cpu)
>> -- 
>> 2.20.1
>> 

-- 
Vitaly


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/9] i386/kvm: add support for KVM_GET_SUPPORTED_HV_CPUID
  2019-05-27 16:39     ` Vitaly Kuznetsov
@ 2019-05-30 17:55       ` Roman Kagan
  2019-05-31  9:22         ` Vitaly Kuznetsov
  0 siblings, 1 reply; 15+ messages in thread
From: Roman Kagan @ 2019-05-30 17:55 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: Daniel P . Berrangé,
	Eduardo Habkost, Marcelo Tosatti, Dr . David Alan Gilbert,
	qemu-devel, Paolo Bonzini, Richard Henderson

On Mon, May 27, 2019 at 06:39:53PM +0200, Vitaly Kuznetsov wrote:
> Roman Kagan <rkagan@virtuozzo.com> writes:
> > On Fri, May 17, 2019 at 04:19:17PM +0200, Vitaly Kuznetsov wrote:
> >> +static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max)
> >> +{
> >> +    struct kvm_cpuid2 *cpuid;
> >> +    int r, size;
> >> +
> >> +    size = sizeof(*cpuid) + max * sizeof(*cpuid->entries);
> >> +    cpuid = g_malloc0(size);
> >> +    cpuid->nent = max;
> >> +
> >> +    r = kvm_vcpu_ioctl(cs, KVM_GET_SUPPORTED_HV_CPUID, cpuid);
> >> +    if (r == 0 && cpuid->nent >= max) {
> >> +        r = -E2BIG;
> >> +    }
> >> +    if (r < 0) {
> >> +        if (r == -E2BIG) {
> >> +            g_free(cpuid);
> >> +            return NULL;
> >> +        } else {
> >> +            fprintf(stderr, "KVM_GET_SUPPORTED_HV_CPUID failed: %s\n",
> >> +                    strerror(-r));
> >> +            exit(1);
> >> +        }
> >> +    }
> >> +    return cpuid;
> >> +}
> >> +
> >> +/*
> >> + * Run KVM_GET_SUPPORTED_HV_CPUID ioctl(), allocating a buffer large enough
> >> + * for all entries.
> >> + */
> >> +static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs)
> >> +{
> >> +    struct kvm_cpuid2 *cpuid;
> >> +    int max = 7; /* 0x40000000..0x40000005, 0x4000000A */
> >> +
> >> +    /*
> >> +     * When the buffer is too small, KVM_GET_SUPPORTED_HV_CPUID fails with
> >> +     * -E2BIG, however, it doesn't report back the right size. Keep increasing
> >> +     * it and re-trying until we succeed.
> >> +     */
> >
> > I'm still missing the idea of reiterating more than once: the ioctl
> > returns the actual size of the array.
> 
> Hm, I think I checked that and it doesn't seem to be the case.
> 
> The code in kvm_vcpu_ioctl_get_hv_cpuid():
> 
> 	if (cpuid->nent < nent)
> 		return -E2BIG;
> 
> 	if (cpuid->nent > nent)
> 		cpuid->nent = nent;
> 
> (I think I even ran a test after your comment on v1 and it it
> confirmed nent is not set on E2BIG). Am I missing something obvious?

Indeed, I saw kvm_vcpu_ioctl_get_cpuid2() always setting ->nent on
return and assumed so did kvm_vcpu_ioctl_get_hv_cpuid().  I stand
corrected, please disregard this comment.
(What was the reason for not following this pattern in
kvm_vcpu_ioctl_get_hv_cpuid BTW?)

Thanks, and sorry for confusion,
Roman.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/9] i386/kvm: add support for KVM_GET_SUPPORTED_HV_CPUID
  2019-05-30 17:55       ` Roman Kagan
@ 2019-05-31  9:22         ` Vitaly Kuznetsov
  0 siblings, 0 replies; 15+ messages in thread
From: Vitaly Kuznetsov @ 2019-05-31  9:22 UTC (permalink / raw)
  To: Roman Kagan
  Cc: Daniel P . Berrangé,
	Eduardo Habkost, Marcelo Tosatti, Dr . David Alan Gilbert,
	qemu-devel, Paolo Bonzini, Richard Henderson

Roman Kagan <rkagan@virtuozzo.com> writes:

> On Mon, May 27, 2019 at 06:39:53PM +0200, Vitaly Kuznetsov wrote:
>> Roman Kagan <rkagan@virtuozzo.com> writes:
>> > On Fri, May 17, 2019 at 04:19:17PM +0200, Vitaly Kuznetsov wrote:
>> >> +static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max)
>> >> +{
>> >> +    struct kvm_cpuid2 *cpuid;
>> >> +    int r, size;
>> >> +
>> >> +    size = sizeof(*cpuid) + max * sizeof(*cpuid->entries);
>> >> +    cpuid = g_malloc0(size);
>> >> +    cpuid->nent = max;
>> >> +
>> >> +    r = kvm_vcpu_ioctl(cs, KVM_GET_SUPPORTED_HV_CPUID, cpuid);
>> >> +    if (r == 0 && cpuid->nent >= max) {
>> >> +        r = -E2BIG;
>> >> +    }
>> >> +    if (r < 0) {
>> >> +        if (r == -E2BIG) {
>> >> +            g_free(cpuid);
>> >> +            return NULL;
>> >> +        } else {
>> >> +            fprintf(stderr, "KVM_GET_SUPPORTED_HV_CPUID failed: %s\n",
>> >> +                    strerror(-r));
>> >> +            exit(1);
>> >> +        }
>> >> +    }
>> >> +    return cpuid;
>> >> +}
>> >> +
>> >> +/*
>> >> + * Run KVM_GET_SUPPORTED_HV_CPUID ioctl(), allocating a buffer large enough
>> >> + * for all entries.
>> >> + */
>> >> +static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs)
>> >> +{
>> >> +    struct kvm_cpuid2 *cpuid;
>> >> +    int max = 7; /* 0x40000000..0x40000005, 0x4000000A */
>> >> +
>> >> +    /*
>> >> +     * When the buffer is too small, KVM_GET_SUPPORTED_HV_CPUID fails with
>> >> +     * -E2BIG, however, it doesn't report back the right size. Keep increasing
>> >> +     * it and re-trying until we succeed.
>> >> +     */
>> >
>> > I'm still missing the idea of reiterating more than once: the ioctl
>> > returns the actual size of the array.
>> 
>> Hm, I think I checked that and it doesn't seem to be the case.
>> 
>> The code in kvm_vcpu_ioctl_get_hv_cpuid():
>> 
>> 	if (cpuid->nent < nent)
>> 		return -E2BIG;
>> 
>> 	if (cpuid->nent > nent)
>> 		cpuid->nent = nent;
>> 
>> (I think I even ran a test after your comment on v1 and it it
>> confirmed nent is not set on E2BIG). Am I missing something obvious?
>
> Indeed, I saw kvm_vcpu_ioctl_get_cpuid2() always setting ->nent on
> return and assumed so did kvm_vcpu_ioctl_get_hv_cpuid().  I stand
> corrected, please disregard this comment.

No problem at all!

> (What was the reason for not following this pattern in
> kvm_vcpu_ioctl_get_hv_cpuid BTW?)

The opportunity to set nent in E2BIG case was probabbly overlooked. I
was looking at QEMU's get_supported_cpuid() implementation which
iterates trying to find the right number and used this as a pattern.

While setting nent in E2BIG case seems to be very convenient, this is an
unobvious side-effect: usually, where the return value indicates an
error we don't inspect the payload. I'm, however, not at all against
changing kvm_vcpu_ioctl_get_hv_cpuid(). Unfortunately, this won't help
QEMU and we'll still have to iterate to support legacy kernels.

-- 
Vitaly


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2019-05-31  9:24 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-17 14:19 [Qemu-devel] [PATCH v2 0/9] i386/kvm/hyper-v: refactor and implement 'hv-stimer-direct' and 'hv-passthrough' enlightenments Vitaly Kuznetsov
2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 1/9] i386/kvm: convert hyperv enlightenments properties from bools to bits Vitaly Kuznetsov
2019-05-27 15:46   ` Roman Kagan
2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 2/9] i386/kvm: add support for KVM_GET_SUPPORTED_HV_CPUID Vitaly Kuznetsov
2019-05-27 15:52   ` Roman Kagan
2019-05-27 16:39     ` Vitaly Kuznetsov
2019-05-30 17:55       ` Roman Kagan
2019-05-31  9:22         ` Vitaly Kuznetsov
2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 3/9] i386/kvm: move Hyper-V CPUID filling to hyperv_handle_properties() Vitaly Kuznetsov
2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 4/9] i386/kvm: document existing Hyper-V enlightenments Vitaly Kuznetsov
2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 5/9] i386/kvm: implement 'hv-passthrough' mode Vitaly Kuznetsov
2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 6/9] i386/kvm: hv-stimer requires hv-time and hv-synic Vitaly Kuznetsov
2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 7/9] i386/kvm: hv-tlbflush/ipi require hv-vpindex Vitaly Kuznetsov
2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 8/9] i386/kvm: hv-evmcs requires hv-vapic Vitaly Kuznetsov
2019-05-17 14:19 ` [Qemu-devel] [PATCH v2 9/9] i386/kvm: add support for Direct Mode for Hyper-V synthetic timers Vitaly Kuznetsov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.