All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] i386/kvm: advertise Hyper-V frequency MSRs
@ 2017-08-04  9:14 ` Ladi Prosek
  0 siblings, 0 replies; 14+ messages in thread
From: Ladi Prosek @ 2017-08-04  9:14 UTC (permalink / raw)
  To: qemu-devel, kvm; +Cc: pbonzini, mtosatti, rkrcmar

This is the QEMU part of the changes required for nested Hyper-V to read
timestamps with RDTSC + TSC page. Without exposing the frequency MSRs,
Windows with the Hyper-V role enabled use the much slower
HV_X64_MSR_TIME_REF_COUNT (0x40000020) RDMSR to read timestamps.

The new registers are exposed only if the TSC frequency is stable across
migration and known, as suggested by Paolo.

Ladi Prosek (3):
  i386/kvm: use a switch statement for MSR detection
  i386/kvm: set tsc_khz before configuring Hyper-V CPUID
  i386/kvm: advertise Hyper-V frequency MSRs

 target/i386/kvm.c | 130 +++++++++++++++++++++++++++---------------------------
 1 file changed, 66 insertions(+), 64 deletions(-)

-- 
2.9.3

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

* [Qemu-devel] [PATCH 0/3] i386/kvm: advertise Hyper-V frequency MSRs
@ 2017-08-04  9:14 ` Ladi Prosek
  0 siblings, 0 replies; 14+ messages in thread
From: Ladi Prosek @ 2017-08-04  9:14 UTC (permalink / raw)
  To: qemu-devel, kvm; +Cc: pbonzini, mtosatti, rkrcmar

This is the QEMU part of the changes required for nested Hyper-V to read
timestamps with RDTSC + TSC page. Without exposing the frequency MSRs,
Windows with the Hyper-V role enabled use the much slower
HV_X64_MSR_TIME_REF_COUNT (0x40000020) RDMSR to read timestamps.

The new registers are exposed only if the TSC frequency is stable across
migration and known, as suggested by Paolo.

Ladi Prosek (3):
  i386/kvm: use a switch statement for MSR detection
  i386/kvm: set tsc_khz before configuring Hyper-V CPUID
  i386/kvm: advertise Hyper-V frequency MSRs

 target/i386/kvm.c | 130 +++++++++++++++++++++++++++---------------------------
 1 file changed, 66 insertions(+), 64 deletions(-)

-- 
2.9.3

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

* [PATCH 1/3] i386/kvm: use a switch statement for MSR detection
  2017-08-04  9:14 ` [Qemu-devel] " Ladi Prosek
@ 2017-08-04  9:14   ` Ladi Prosek
  -1 siblings, 0 replies; 14+ messages in thread
From: Ladi Prosek @ 2017-08-04  9:14 UTC (permalink / raw)
  To: qemu-devel, kvm; +Cc: pbonzini, mtosatti, rkrcmar

Switch is easier on the eye and might lead to better codegen.

Signed-off-by: Ladi Prosek <lprosek@redhat.com>
---
 target/i386/kvm.c | 76 +++++++++++++++++++++++--------------------------------
 1 file changed, 32 insertions(+), 44 deletions(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 6db7783..ed119ca 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1081,66 +1081,54 @@ static int kvm_get_supported_msrs(KVMState *s)
             int i;
 
             for (i = 0; i < kvm_msr_list->nmsrs; i++) {
-                if (kvm_msr_list->indices[i] == MSR_STAR) {
+                switch (kvm_msr_list->indices[i]) {
+                case MSR_STAR:
                     has_msr_star = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == MSR_VM_HSAVE_PA) {
+                    break;
+                case MSR_VM_HSAVE_PA:
                     has_msr_hsave_pa = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == MSR_TSC_AUX) {
+                    break;
+                case MSR_TSC_AUX:
                     has_msr_tsc_aux = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == MSR_TSC_ADJUST) {
+                    break;
+                case MSR_TSC_ADJUST:
                     has_msr_tsc_adjust = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == MSR_IA32_TSCDEADLINE) {
+                    break;
+                case MSR_IA32_TSCDEADLINE:
                     has_msr_tsc_deadline = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == MSR_IA32_SMBASE) {
+                    break;
+                case MSR_IA32_SMBASE:
                     has_msr_smbase = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == MSR_IA32_MISC_ENABLE) {
+                    break;
+                case MSR_IA32_MISC_ENABLE:
                     has_msr_misc_enable = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == MSR_IA32_BNDCFGS) {
+                    break;
+                case MSR_IA32_BNDCFGS:
                     has_msr_bndcfgs = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == MSR_IA32_XSS) {
+                    break;
+                case MSR_IA32_XSS:
                     has_msr_xss = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == HV_X64_MSR_CRASH_CTL) {
+                    break;;
+                case HV_X64_MSR_CRASH_CTL:
                     has_msr_hv_crash = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == HV_X64_MSR_RESET) {
+                    break;
+                case HV_X64_MSR_RESET:
                     has_msr_hv_reset = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == HV_X64_MSR_VP_INDEX) {
+                    break;
+                case HV_X64_MSR_VP_INDEX:
                     has_msr_hv_vpindex = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == HV_X64_MSR_VP_RUNTIME) {
+                    break;
+                case HV_X64_MSR_VP_RUNTIME:
                     has_msr_hv_runtime = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == HV_X64_MSR_SCONTROL) {
+                    break;
+                case HV_X64_MSR_SCONTROL:
                     has_msr_hv_synic = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == HV_X64_MSR_STIMER0_CONFIG) {
+                    break;
+                case HV_X64_MSR_STIMER0_CONFIG:
                     has_msr_hv_stimer = true;
-                    continue;
+                    break;
                 }
+
             }
         }
 
-- 
2.9.3

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

* [Qemu-devel] [PATCH 1/3] i386/kvm: use a switch statement for MSR detection
@ 2017-08-04  9:14   ` Ladi Prosek
  0 siblings, 0 replies; 14+ messages in thread
From: Ladi Prosek @ 2017-08-04  9:14 UTC (permalink / raw)
  To: qemu-devel, kvm; +Cc: pbonzini, mtosatti, rkrcmar

Switch is easier on the eye and might lead to better codegen.

Signed-off-by: Ladi Prosek <lprosek@redhat.com>
---
 target/i386/kvm.c | 76 +++++++++++++++++++++++--------------------------------
 1 file changed, 32 insertions(+), 44 deletions(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 6db7783..ed119ca 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1081,66 +1081,54 @@ static int kvm_get_supported_msrs(KVMState *s)
             int i;
 
             for (i = 0; i < kvm_msr_list->nmsrs; i++) {
-                if (kvm_msr_list->indices[i] == MSR_STAR) {
+                switch (kvm_msr_list->indices[i]) {
+                case MSR_STAR:
                     has_msr_star = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == MSR_VM_HSAVE_PA) {
+                    break;
+                case MSR_VM_HSAVE_PA:
                     has_msr_hsave_pa = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == MSR_TSC_AUX) {
+                    break;
+                case MSR_TSC_AUX:
                     has_msr_tsc_aux = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == MSR_TSC_ADJUST) {
+                    break;
+                case MSR_TSC_ADJUST:
                     has_msr_tsc_adjust = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == MSR_IA32_TSCDEADLINE) {
+                    break;
+                case MSR_IA32_TSCDEADLINE:
                     has_msr_tsc_deadline = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == MSR_IA32_SMBASE) {
+                    break;
+                case MSR_IA32_SMBASE:
                     has_msr_smbase = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == MSR_IA32_MISC_ENABLE) {
+                    break;
+                case MSR_IA32_MISC_ENABLE:
                     has_msr_misc_enable = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == MSR_IA32_BNDCFGS) {
+                    break;
+                case MSR_IA32_BNDCFGS:
                     has_msr_bndcfgs = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == MSR_IA32_XSS) {
+                    break;
+                case MSR_IA32_XSS:
                     has_msr_xss = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == HV_X64_MSR_CRASH_CTL) {
+                    break;;
+                case HV_X64_MSR_CRASH_CTL:
                     has_msr_hv_crash = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == HV_X64_MSR_RESET) {
+                    break;
+                case HV_X64_MSR_RESET:
                     has_msr_hv_reset = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == HV_X64_MSR_VP_INDEX) {
+                    break;
+                case HV_X64_MSR_VP_INDEX:
                     has_msr_hv_vpindex = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == HV_X64_MSR_VP_RUNTIME) {
+                    break;
+                case HV_X64_MSR_VP_RUNTIME:
                     has_msr_hv_runtime = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == HV_X64_MSR_SCONTROL) {
+                    break;
+                case HV_X64_MSR_SCONTROL:
                     has_msr_hv_synic = true;
-                    continue;
-                }
-                if (kvm_msr_list->indices[i] == HV_X64_MSR_STIMER0_CONFIG) {
+                    break;
+                case HV_X64_MSR_STIMER0_CONFIG:
                     has_msr_hv_stimer = true;
-                    continue;
+                    break;
                 }
+
             }
         }
 
-- 
2.9.3

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

* [PATCH 2/3] i386/kvm: set tsc_khz before configuring Hyper-V CPUID
  2017-08-04  9:14 ` [Qemu-devel] " Ladi Prosek
@ 2017-08-04  9:14   ` Ladi Prosek
  -1 siblings, 0 replies; 14+ messages in thread
From: Ladi Prosek @ 2017-08-04  9:14 UTC (permalink / raw)
  To: qemu-devel, kvm; +Cc: pbonzini, mtosatti, rkrcmar

Timing-related Hyper-V enlightenments will benefit from knowing the final
tsc_khz value. This commit just moves the code in preparation for further
changes.

Signed-off-by: Ladi Prosek <lprosek@redhat.com>
---
 target/i386/kvm.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index ed119ca..77b6373 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -695,6 +695,25 @@ int kvm_arch_init_vcpu(CPUState *cs)
 
     cpuid_i = 0;
 
+    r = kvm_arch_set_tsc_khz(cs);
+    if (r < 0) {
+        goto fail;
+    }
+
+    /* vcpu's TSC frequency is either specified by user, or following
+     * the value used by KVM if the former is not present. In the
+     * latter case, we query it from KVM and record in env->tsc_khz,
+     * so that vcpu's TSC frequency can be migrated later via this field.
+     */
+    if (!env->tsc_khz) {
+        r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
+            kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ) :
+            -ENOTSUP;
+        if (r > 0) {
+            env->tsc_khz = r;
+        }
+    }
+
     /* Paravirtualization CPUIDs */
     if (hyperv_enabled(cpu)) {
         c = &cpuid_data.entries[cpuid_i++];
@@ -961,25 +980,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
         }
     }
 
-    r = kvm_arch_set_tsc_khz(cs);
-    if (r < 0) {
-        goto fail;
-    }
-
-    /* vcpu's TSC frequency is either specified by user, or following
-     * the value used by KVM if the former is not present. In the
-     * latter case, we query it from KVM and record in env->tsc_khz,
-     * so that vcpu's TSC frequency can be migrated later via this field.
-     */
-    if (!env->tsc_khz) {
-        r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
-            kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ) :
-            -ENOTSUP;
-        if (r > 0) {
-            env->tsc_khz = r;
-        }
-    }
-
     if (cpu->vmware_cpuid_freq
         /* Guests depend on 0x40000000 to detect this feature, so only expose
          * it if KVM exposes leaf 0x40000000. (Conflicts with Hyper-V) */
-- 
2.9.3

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

* [Qemu-devel] [PATCH 2/3] i386/kvm: set tsc_khz before configuring Hyper-V CPUID
@ 2017-08-04  9:14   ` Ladi Prosek
  0 siblings, 0 replies; 14+ messages in thread
From: Ladi Prosek @ 2017-08-04  9:14 UTC (permalink / raw)
  To: qemu-devel, kvm; +Cc: pbonzini, mtosatti, rkrcmar

Timing-related Hyper-V enlightenments will benefit from knowing the final
tsc_khz value. This commit just moves the code in preparation for further
changes.

Signed-off-by: Ladi Prosek <lprosek@redhat.com>
---
 target/i386/kvm.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index ed119ca..77b6373 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -695,6 +695,25 @@ int kvm_arch_init_vcpu(CPUState *cs)
 
     cpuid_i = 0;
 
+    r = kvm_arch_set_tsc_khz(cs);
+    if (r < 0) {
+        goto fail;
+    }
+
+    /* vcpu's TSC frequency is either specified by user, or following
+     * the value used by KVM if the former is not present. In the
+     * latter case, we query it from KVM and record in env->tsc_khz,
+     * so that vcpu's TSC frequency can be migrated later via this field.
+     */
+    if (!env->tsc_khz) {
+        r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
+            kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ) :
+            -ENOTSUP;
+        if (r > 0) {
+            env->tsc_khz = r;
+        }
+    }
+
     /* Paravirtualization CPUIDs */
     if (hyperv_enabled(cpu)) {
         c = &cpuid_data.entries[cpuid_i++];
@@ -961,25 +980,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
         }
     }
 
-    r = kvm_arch_set_tsc_khz(cs);
-    if (r < 0) {
-        goto fail;
-    }
-
-    /* vcpu's TSC frequency is either specified by user, or following
-     * the value used by KVM if the former is not present. In the
-     * latter case, we query it from KVM and record in env->tsc_khz,
-     * so that vcpu's TSC frequency can be migrated later via this field.
-     */
-    if (!env->tsc_khz) {
-        r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
-            kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ) :
-            -ENOTSUP;
-        if (r > 0) {
-            env->tsc_khz = r;
-        }
-    }
-
     if (cpu->vmware_cpuid_freq
         /* Guests depend on 0x40000000 to detect this feature, so only expose
          * it if KVM exposes leaf 0x40000000. (Conflicts with Hyper-V) */
-- 
2.9.3

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

* [PATCH 3/3] i386/kvm: advertise Hyper-V frequency MSRs
  2017-08-04  9:14 ` [Qemu-devel] " Ladi Prosek
@ 2017-08-04  9:14   ` Ladi Prosek
  -1 siblings, 0 replies; 14+ messages in thread
From: Ladi Prosek @ 2017-08-04  9:14 UTC (permalink / raw)
  To: qemu-devel, kvm; +Cc: pbonzini, mtosatti, rkrcmar

As of kernel commit eb82feea59d6 ("KVM: hyperv: support HV_X64_MSR_TSC_FREQUENCY
and HV_X64_MSR_APIC_FREQUENCY"), KVM supports two new MSRs which are required
for nested Hyper-V to read timestamps with RDTSC + TSC page.

This commit makes QEMU advertise the MSRs with CPUID.40000003H:EAX[11] and
CPUID.40000003H:EDX[8] as specified in the Hyper-V TLFS and experimentally
verified on a Hyper-V host. The feature is enabled with the existing hv-time CPU
flag, and only if the TSC frequency is stable across migration and known.

Signed-off-by: Ladi Prosek <lprosek@redhat.com>
---
 target/i386/kvm.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 77b6373..7e484a7 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -89,6 +89,7 @@ static bool has_msr_hv_vpindex;
 static bool has_msr_hv_runtime;
 static bool has_msr_hv_synic;
 static bool has_msr_hv_stimer;
+static bool has_msr_hv_frequencies;
 static bool has_msr_xss;
 
 static bool has_msr_architectural_pmu;
@@ -631,7 +632,17 @@ static int hyperv_handle_properties(CPUState *cs)
     if (cpu->hyperv_time) {
         env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_HYPERCALL_AVAILABLE;
         env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_TIME_REF_COUNT_AVAILABLE;
-        env->features[FEAT_HYPERV_EAX] |= 0x200;
+        env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_REFERENCE_TSC_AVAILABLE;
+        if (has_msr_hv_frequencies
+            /* TSC clock must be stable and known for this feature. */
+            && ((env->features[FEAT_8000_0007_EDX] & CPUID_APM_INVTSC)
+                || env->user_tsc_khz != 0)
+            && env->tsc_khz != 0) {
+
+            env->features[FEAT_HYPERV_EAX] |= HV_X64_ACCESS_FREQUENCY_MSRS;
+            env->features[FEAT_HYPERV_EDX] |=
+                HV_FEATURE_FREQUENCY_MSRS_AVAILABLE;
+        }
     }
     if (cpu->hyperv_crash && has_msr_hv_crash) {
         env->features[FEAT_HYPERV_EDX] |= HV_X64_GUEST_CRASH_MSR_AVAILABLE;
@@ -1127,6 +1138,9 @@ static int kvm_get_supported_msrs(KVMState *s)
                 case HV_X64_MSR_STIMER0_CONFIG:
                     has_msr_hv_stimer = true;
                     break;
+                case HV_X64_MSR_TSC_FREQUENCY:
+                    has_msr_hv_frequencies = true;
+                    break;
                 }
 
             }
-- 
2.9.3

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

* [Qemu-devel] [PATCH 3/3] i386/kvm: advertise Hyper-V frequency MSRs
@ 2017-08-04  9:14   ` Ladi Prosek
  0 siblings, 0 replies; 14+ messages in thread
From: Ladi Prosek @ 2017-08-04  9:14 UTC (permalink / raw)
  To: qemu-devel, kvm; +Cc: pbonzini, mtosatti, rkrcmar

As of kernel commit eb82feea59d6 ("KVM: hyperv: support HV_X64_MSR_TSC_FREQUENCY
and HV_X64_MSR_APIC_FREQUENCY"), KVM supports two new MSRs which are required
for nested Hyper-V to read timestamps with RDTSC + TSC page.

This commit makes QEMU advertise the MSRs with CPUID.40000003H:EAX[11] and
CPUID.40000003H:EDX[8] as specified in the Hyper-V TLFS and experimentally
verified on a Hyper-V host. The feature is enabled with the existing hv-time CPU
flag, and only if the TSC frequency is stable across migration and known.

Signed-off-by: Ladi Prosek <lprosek@redhat.com>
---
 target/i386/kvm.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 77b6373..7e484a7 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -89,6 +89,7 @@ static bool has_msr_hv_vpindex;
 static bool has_msr_hv_runtime;
 static bool has_msr_hv_synic;
 static bool has_msr_hv_stimer;
+static bool has_msr_hv_frequencies;
 static bool has_msr_xss;
 
 static bool has_msr_architectural_pmu;
@@ -631,7 +632,17 @@ static int hyperv_handle_properties(CPUState *cs)
     if (cpu->hyperv_time) {
         env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_HYPERCALL_AVAILABLE;
         env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_TIME_REF_COUNT_AVAILABLE;
-        env->features[FEAT_HYPERV_EAX] |= 0x200;
+        env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_REFERENCE_TSC_AVAILABLE;
+        if (has_msr_hv_frequencies
+            /* TSC clock must be stable and known for this feature. */
+            && ((env->features[FEAT_8000_0007_EDX] & CPUID_APM_INVTSC)
+                || env->user_tsc_khz != 0)
+            && env->tsc_khz != 0) {
+
+            env->features[FEAT_HYPERV_EAX] |= HV_X64_ACCESS_FREQUENCY_MSRS;
+            env->features[FEAT_HYPERV_EDX] |=
+                HV_FEATURE_FREQUENCY_MSRS_AVAILABLE;
+        }
     }
     if (cpu->hyperv_crash && has_msr_hv_crash) {
         env->features[FEAT_HYPERV_EDX] |= HV_X64_GUEST_CRASH_MSR_AVAILABLE;
@@ -1127,6 +1138,9 @@ static int kvm_get_supported_msrs(KVMState *s)
                 case HV_X64_MSR_STIMER0_CONFIG:
                     has_msr_hv_stimer = true;
                     break;
+                case HV_X64_MSR_TSC_FREQUENCY:
+                    has_msr_hv_frequencies = true;
+                    break;
                 }
 
             }
-- 
2.9.3

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

* Re: [PATCH 1/3] i386/kvm: use a switch statement for MSR detection
  2017-08-04  9:14   ` [Qemu-devel] " Ladi Prosek
@ 2017-08-04 13:30     ` David Hildenbrand
  -1 siblings, 0 replies; 14+ messages in thread
From: David Hildenbrand @ 2017-08-04 13:30 UTC (permalink / raw)
  To: Ladi Prosek, qemu-devel, kvm; +Cc: pbonzini, mtosatti, rkrcmar

On 04.08.2017 11:14, Ladi Prosek wrote:
> Switch is easier on the eye and might lead to better codegen.
> 
> Signed-off-by: Ladi Prosek <lprosek@redhat.com>


Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David

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

* Re: [Qemu-devel] [PATCH 1/3] i386/kvm: use a switch statement for MSR detection
@ 2017-08-04 13:30     ` David Hildenbrand
  0 siblings, 0 replies; 14+ messages in thread
From: David Hildenbrand @ 2017-08-04 13:30 UTC (permalink / raw)
  To: Ladi Prosek, qemu-devel, kvm; +Cc: pbonzini, mtosatti, rkrcmar

On 04.08.2017 11:14, Ladi Prosek wrote:
> Switch is easier on the eye and might lead to better codegen.
> 
> Signed-off-by: Ladi Prosek <lprosek@redhat.com>


Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David

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

* Re: [PATCH 3/3] i386/kvm: advertise Hyper-V frequency MSRs
  2017-08-04  9:14   ` [Qemu-devel] " Ladi Prosek
@ 2017-08-04 13:39     ` David Hildenbrand
  -1 siblings, 0 replies; 14+ messages in thread
From: David Hildenbrand @ 2017-08-04 13:39 UTC (permalink / raw)
  To: Ladi Prosek, qemu-devel, kvm; +Cc: pbonzini, mtosatti, rkrcmar

On 04.08.2017 11:14, Ladi Prosek wrote:
> As of kernel commit eb82feea59d6 ("KVM: hyperv: support HV_X64_MSR_TSC_FREQUENCY
> and HV_X64_MSR_APIC_FREQUENCY"), KVM supports two new MSRs which are required
> for nested Hyper-V to read timestamps with RDTSC + TSC page.
> 
> This commit makes QEMU advertise the MSRs with CPUID.40000003H:EAX[11] and
> CPUID.40000003H:EDX[8] as specified in the Hyper-V TLFS and experimentally
> verified on a Hyper-V host. The feature is enabled with the existing hv-time CPU
> flag, and only if the TSC frequency is stable across migration and known.
> 
> Signed-off-by: Ladi Prosek <lprosek@redhat.com>
> ---
>  target/i386/kvm.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 77b6373..7e484a7 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -89,6 +89,7 @@ static bool has_msr_hv_vpindex;
>  static bool has_msr_hv_runtime;
>  static bool has_msr_hv_synic;
>  static bool has_msr_hv_stimer;
> +static bool has_msr_hv_frequencies;
>  static bool has_msr_xss;
>  
>  static bool has_msr_architectural_pmu;
> @@ -631,7 +632,17 @@ static int hyperv_handle_properties(CPUState *cs)
>      if (cpu->hyperv_time) {
>          env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_HYPERCALL_AVAILABLE;
>          env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_TIME_REF_COUNT_AVAILABLE;
> -        env->features[FEAT_HYPERV_EAX] |= 0x200;
> +        env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_REFERENCE_TSC_AVAILABLE;
> +        if (has_msr_hv_frequencies> +            /* TSC clock must be stable and known for this feature. */
> +            && ((env->features[FEAT_8000_0007_EDX] & CPUID_APM_INVTSC)
> +                || env->user_tsc_khz != 0)
> +            && env->tsc_khz != 0) {

I'd drop the != 0 in both cases and move the env->tsc_khz check up to
has_msr_hv_frequencies.

if (has_msr_hv_frequencies && env->tsc_khz && ...

Wonder if it even would make sense to move some parts of this check into
a helper function, to beautify this a bit. tsc_stable(env)
tsc_known(env) ...

> +
> +            env->features[FEAT_HYPERV_EAX] |= HV_X64_ACCESS_FREQUENCY_MSRS;
> +            env->features[FEAT_HYPERV_EDX] |=
> +                HV_FEATURE_FREQUENCY_MSRS_AVAILABLE;
> +        }
>      }
>      if (cpu->hyperv_crash && has_msr_hv_crash) {
>          env->features[FEAT_HYPERV_EDX] |= HV_X64_GUEST_CRASH_MSR_AVAILABLE;
> @@ -1127,6 +1138,9 @@ static int kvm_get_supported_msrs(KVMState *s)
>                  case HV_X64_MSR_STIMER0_CONFIG:
>                      has_msr_hv_stimer = true;
>                      break;
> +                case HV_X64_MSR_TSC_FREQUENCY:
> +                    has_msr_hv_frequencies = true;
> +                    break;
>                  }
>  
>              }
> 


-- 

Thanks,

David

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

* Re: [Qemu-devel] [PATCH 3/3] i386/kvm: advertise Hyper-V frequency MSRs
@ 2017-08-04 13:39     ` David Hildenbrand
  0 siblings, 0 replies; 14+ messages in thread
From: David Hildenbrand @ 2017-08-04 13:39 UTC (permalink / raw)
  To: Ladi Prosek, qemu-devel, kvm; +Cc: pbonzini, mtosatti, rkrcmar

On 04.08.2017 11:14, Ladi Prosek wrote:
> As of kernel commit eb82feea59d6 ("KVM: hyperv: support HV_X64_MSR_TSC_FREQUENCY
> and HV_X64_MSR_APIC_FREQUENCY"), KVM supports two new MSRs which are required
> for nested Hyper-V to read timestamps with RDTSC + TSC page.
> 
> This commit makes QEMU advertise the MSRs with CPUID.40000003H:EAX[11] and
> CPUID.40000003H:EDX[8] as specified in the Hyper-V TLFS and experimentally
> verified on a Hyper-V host. The feature is enabled with the existing hv-time CPU
> flag, and only if the TSC frequency is stable across migration and known.
> 
> Signed-off-by: Ladi Prosek <lprosek@redhat.com>
> ---
>  target/i386/kvm.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 77b6373..7e484a7 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -89,6 +89,7 @@ static bool has_msr_hv_vpindex;
>  static bool has_msr_hv_runtime;
>  static bool has_msr_hv_synic;
>  static bool has_msr_hv_stimer;
> +static bool has_msr_hv_frequencies;
>  static bool has_msr_xss;
>  
>  static bool has_msr_architectural_pmu;
> @@ -631,7 +632,17 @@ static int hyperv_handle_properties(CPUState *cs)
>      if (cpu->hyperv_time) {
>          env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_HYPERCALL_AVAILABLE;
>          env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_TIME_REF_COUNT_AVAILABLE;
> -        env->features[FEAT_HYPERV_EAX] |= 0x200;
> +        env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_REFERENCE_TSC_AVAILABLE;
> +        if (has_msr_hv_frequencies> +            /* TSC clock must be stable and known for this feature. */
> +            && ((env->features[FEAT_8000_0007_EDX] & CPUID_APM_INVTSC)
> +                || env->user_tsc_khz != 0)
> +            && env->tsc_khz != 0) {

I'd drop the != 0 in both cases and move the env->tsc_khz check up to
has_msr_hv_frequencies.

if (has_msr_hv_frequencies && env->tsc_khz && ...

Wonder if it even would make sense to move some parts of this check into
a helper function, to beautify this a bit. tsc_stable(env)
tsc_known(env) ...

> +
> +            env->features[FEAT_HYPERV_EAX] |= HV_X64_ACCESS_FREQUENCY_MSRS;
> +            env->features[FEAT_HYPERV_EDX] |=
> +                HV_FEATURE_FREQUENCY_MSRS_AVAILABLE;
> +        }
>      }
>      if (cpu->hyperv_crash && has_msr_hv_crash) {
>          env->features[FEAT_HYPERV_EDX] |= HV_X64_GUEST_CRASH_MSR_AVAILABLE;
> @@ -1127,6 +1138,9 @@ static int kvm_get_supported_msrs(KVMState *s)
>                  case HV_X64_MSR_STIMER0_CONFIG:
>                      has_msr_hv_stimer = true;
>                      break;
> +                case HV_X64_MSR_TSC_FREQUENCY:
> +                    has_msr_hv_frequencies = true;
> +                    break;
>                  }
>  
>              }
> 


-- 

Thanks,

David

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

* Re: [PATCH 3/3] i386/kvm: advertise Hyper-V frequency MSRs
  2017-08-04 13:39     ` [Qemu-devel] " David Hildenbrand
@ 2017-08-04 13:45       ` Ladi Prosek
  -1 siblings, 0 replies; 14+ messages in thread
From: Ladi Prosek @ 2017-08-04 13:45 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: qemu-devel, KVM list, Paolo Bonzini, Marcelo Tosatti, Radim Krcmar

On Fri, Aug 4, 2017 at 3:39 PM, David Hildenbrand <david@redhat.com> wrote:
> On 04.08.2017 11:14, Ladi Prosek wrote:
>> As of kernel commit eb82feea59d6 ("KVM: hyperv: support HV_X64_MSR_TSC_FREQUENCY
>> and HV_X64_MSR_APIC_FREQUENCY"), KVM supports two new MSRs which are required
>> for nested Hyper-V to read timestamps with RDTSC + TSC page.
>>
>> This commit makes QEMU advertise the MSRs with CPUID.40000003H:EAX[11] and
>> CPUID.40000003H:EDX[8] as specified in the Hyper-V TLFS and experimentally
>> verified on a Hyper-V host. The feature is enabled with the existing hv-time CPU
>> flag, and only if the TSC frequency is stable across migration and known.
>>
>> Signed-off-by: Ladi Prosek <lprosek@redhat.com>
>> ---
>>  target/i386/kvm.c | 16 +++++++++++++++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> index 77b6373..7e484a7 100644
>> --- a/target/i386/kvm.c
>> +++ b/target/i386/kvm.c
>> @@ -89,6 +89,7 @@ static bool has_msr_hv_vpindex;
>>  static bool has_msr_hv_runtime;
>>  static bool has_msr_hv_synic;
>>  static bool has_msr_hv_stimer;
>> +static bool has_msr_hv_frequencies;
>>  static bool has_msr_xss;
>>
>>  static bool has_msr_architectural_pmu;
>> @@ -631,7 +632,17 @@ static int hyperv_handle_properties(CPUState *cs)
>>      if (cpu->hyperv_time) {
>>          env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_HYPERCALL_AVAILABLE;
>>          env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_TIME_REF_COUNT_AVAILABLE;
>> -        env->features[FEAT_HYPERV_EAX] |= 0x200;
>> +        env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_REFERENCE_TSC_AVAILABLE;
>> +        if (has_msr_hv_frequencies> +            /* TSC clock must be stable and known for this feature. */
>> +            && ((env->features[FEAT_8000_0007_EDX] & CPUID_APM_INVTSC)
>> +                || env->user_tsc_khz != 0)
>> +            && env->tsc_khz != 0) {
>
> I'd drop the != 0 in both cases and move the env->tsc_khz check up to
> has_msr_hv_frequencies.
>
> if (has_msr_hv_frequencies && env->tsc_khz && ...
>
> Wonder if it even would make sense to move some parts of this check into
> a helper function, to beautify this a bit. tsc_stable(env)
> tsc_known(env) ...

Yup, especially since I copied these conditions from "if
(cpu->vmware_cpuid_freq .." further down in the file. So maybe one
helper called by both would be the best. Thanks!

>> +
>> +            env->features[FEAT_HYPERV_EAX] |= HV_X64_ACCESS_FREQUENCY_MSRS;
>> +            env->features[FEAT_HYPERV_EDX] |=
>> +                HV_FEATURE_FREQUENCY_MSRS_AVAILABLE;
>> +        }
>>      }
>>      if (cpu->hyperv_crash && has_msr_hv_crash) {
>>          env->features[FEAT_HYPERV_EDX] |= HV_X64_GUEST_CRASH_MSR_AVAILABLE;
>> @@ -1127,6 +1138,9 @@ static int kvm_get_supported_msrs(KVMState *s)
>>                  case HV_X64_MSR_STIMER0_CONFIG:
>>                      has_msr_hv_stimer = true;
>>                      break;
>> +                case HV_X64_MSR_TSC_FREQUENCY:
>> +                    has_msr_hv_frequencies = true;
>> +                    break;
>>                  }
>>
>>              }
>>
>
>
> --
>
> Thanks,
>
> David

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

* Re: [Qemu-devel] [PATCH 3/3] i386/kvm: advertise Hyper-V frequency MSRs
@ 2017-08-04 13:45       ` Ladi Prosek
  0 siblings, 0 replies; 14+ messages in thread
From: Ladi Prosek @ 2017-08-04 13:45 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: qemu-devel, KVM list, Paolo Bonzini, Marcelo Tosatti, Radim Krcmar

On Fri, Aug 4, 2017 at 3:39 PM, David Hildenbrand <david@redhat.com> wrote:
> On 04.08.2017 11:14, Ladi Prosek wrote:
>> As of kernel commit eb82feea59d6 ("KVM: hyperv: support HV_X64_MSR_TSC_FREQUENCY
>> and HV_X64_MSR_APIC_FREQUENCY"), KVM supports two new MSRs which are required
>> for nested Hyper-V to read timestamps with RDTSC + TSC page.
>>
>> This commit makes QEMU advertise the MSRs with CPUID.40000003H:EAX[11] and
>> CPUID.40000003H:EDX[8] as specified in the Hyper-V TLFS and experimentally
>> verified on a Hyper-V host. The feature is enabled with the existing hv-time CPU
>> flag, and only if the TSC frequency is stable across migration and known.
>>
>> Signed-off-by: Ladi Prosek <lprosek@redhat.com>
>> ---
>>  target/i386/kvm.c | 16 +++++++++++++++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> index 77b6373..7e484a7 100644
>> --- a/target/i386/kvm.c
>> +++ b/target/i386/kvm.c
>> @@ -89,6 +89,7 @@ static bool has_msr_hv_vpindex;
>>  static bool has_msr_hv_runtime;
>>  static bool has_msr_hv_synic;
>>  static bool has_msr_hv_stimer;
>> +static bool has_msr_hv_frequencies;
>>  static bool has_msr_xss;
>>
>>  static bool has_msr_architectural_pmu;
>> @@ -631,7 +632,17 @@ static int hyperv_handle_properties(CPUState *cs)
>>      if (cpu->hyperv_time) {
>>          env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_HYPERCALL_AVAILABLE;
>>          env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_TIME_REF_COUNT_AVAILABLE;
>> -        env->features[FEAT_HYPERV_EAX] |= 0x200;
>> +        env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_REFERENCE_TSC_AVAILABLE;
>> +        if (has_msr_hv_frequencies> +            /* TSC clock must be stable and known for this feature. */
>> +            && ((env->features[FEAT_8000_0007_EDX] & CPUID_APM_INVTSC)
>> +                || env->user_tsc_khz != 0)
>> +            && env->tsc_khz != 0) {
>
> I'd drop the != 0 in both cases and move the env->tsc_khz check up to
> has_msr_hv_frequencies.
>
> if (has_msr_hv_frequencies && env->tsc_khz && ...
>
> Wonder if it even would make sense to move some parts of this check into
> a helper function, to beautify this a bit. tsc_stable(env)
> tsc_known(env) ...

Yup, especially since I copied these conditions from "if
(cpu->vmware_cpuid_freq .." further down in the file. So maybe one
helper called by both would be the best. Thanks!

>> +
>> +            env->features[FEAT_HYPERV_EAX] |= HV_X64_ACCESS_FREQUENCY_MSRS;
>> +            env->features[FEAT_HYPERV_EDX] |=
>> +                HV_FEATURE_FREQUENCY_MSRS_AVAILABLE;
>> +        }
>>      }
>>      if (cpu->hyperv_crash && has_msr_hv_crash) {
>>          env->features[FEAT_HYPERV_EDX] |= HV_X64_GUEST_CRASH_MSR_AVAILABLE;
>> @@ -1127,6 +1138,9 @@ static int kvm_get_supported_msrs(KVMState *s)
>>                  case HV_X64_MSR_STIMER0_CONFIG:
>>                      has_msr_hv_stimer = true;
>>                      break;
>> +                case HV_X64_MSR_TSC_FREQUENCY:
>> +                    has_msr_hv_frequencies = true;
>> +                    break;
>>                  }
>>
>>              }
>>
>
>
> --
>
> Thanks,
>
> David

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

end of thread, other threads:[~2017-08-04 13:47 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-04  9:14 [PATCH 0/3] i386/kvm: advertise Hyper-V frequency MSRs Ladi Prosek
2017-08-04  9:14 ` [Qemu-devel] " Ladi Prosek
2017-08-04  9:14 ` [PATCH 1/3] i386/kvm: use a switch statement for MSR detection Ladi Prosek
2017-08-04  9:14   ` [Qemu-devel] " Ladi Prosek
2017-08-04 13:30   ` David Hildenbrand
2017-08-04 13:30     ` [Qemu-devel] " David Hildenbrand
2017-08-04  9:14 ` [PATCH 2/3] i386/kvm: set tsc_khz before configuring Hyper-V CPUID Ladi Prosek
2017-08-04  9:14   ` [Qemu-devel] " Ladi Prosek
2017-08-04  9:14 ` [PATCH 3/3] i386/kvm: advertise Hyper-V frequency MSRs Ladi Prosek
2017-08-04  9:14   ` [Qemu-devel] " Ladi Prosek
2017-08-04 13:39   ` David Hildenbrand
2017-08-04 13:39     ` [Qemu-devel] " David Hildenbrand
2017-08-04 13:45     ` Ladi Prosek
2017-08-04 13:45       ` [Qemu-devel] " Ladi Prosek

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.