* [Qemu-devel] [PATCH 1/3] i386/kvm: use a switch statement for MSR detection
2017-08-04 9:14 [Qemu-devel] [PATCH 0/3] i386/kvm: advertise Hyper-V frequency MSRs Ladi Prosek
@ 2017-08-04 9:14 ` Ladi Prosek
2017-08-04 13:30 ` David Hildenbrand
2017-08-04 9:14 ` [Qemu-devel] [PATCH 2/3] i386/kvm: set tsc_khz before configuring Hyper-V CPUID Ladi Prosek
2017-08-04 9:14 ` [Qemu-devel] [PATCH 3/3] i386/kvm: advertise Hyper-V frequency MSRs Ladi Prosek
2 siblings, 1 reply; 7+ 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] 7+ messages in thread* [Qemu-devel] [PATCH 2/3] i386/kvm: set tsc_khz before configuring Hyper-V CPUID
2017-08-04 9:14 [Qemu-devel] [PATCH 0/3] i386/kvm: advertise Hyper-V frequency MSRs Ladi Prosek
2017-08-04 9:14 ` [Qemu-devel] [PATCH 1/3] i386/kvm: use a switch statement for MSR detection Ladi Prosek
@ 2017-08-04 9:14 ` Ladi Prosek
2017-08-04 9:14 ` [Qemu-devel] [PATCH 3/3] i386/kvm: advertise Hyper-V frequency MSRs Ladi Prosek
2 siblings, 0 replies; 7+ 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] 7+ messages in thread* [Qemu-devel] [PATCH 3/3] i386/kvm: advertise Hyper-V frequency MSRs
2017-08-04 9:14 [Qemu-devel] [PATCH 0/3] i386/kvm: advertise Hyper-V frequency MSRs Ladi Prosek
2017-08-04 9:14 ` [Qemu-devel] [PATCH 1/3] i386/kvm: use a switch statement for MSR detection Ladi Prosek
2017-08-04 9:14 ` [Qemu-devel] [PATCH 2/3] i386/kvm: set tsc_khz before configuring Hyper-V CPUID Ladi Prosek
@ 2017-08-04 9:14 ` Ladi Prosek
2017-08-04 13:39 ` David Hildenbrand
2 siblings, 1 reply; 7+ 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] 7+ messages in thread* Re: [Qemu-devel] [PATCH 3/3] i386/kvm: advertise Hyper-V frequency MSRs
2017-08-04 9:14 ` [Qemu-devel] [PATCH 3/3] i386/kvm: advertise Hyper-V frequency MSRs Ladi Prosek
@ 2017-08-04 13:39 ` David Hildenbrand
2017-08-04 13:45 ` Ladi Prosek
0 siblings, 1 reply; 7+ 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] 7+ messages in thread* Re: [Qemu-devel] [PATCH 3/3] i386/kvm: advertise Hyper-V frequency MSRs
2017-08-04 13:39 ` David Hildenbrand
@ 2017-08-04 13:45 ` Ladi Prosek
0 siblings, 0 replies; 7+ 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] 7+ messages in thread