| Message ID | 20170804091403.13478-4-lprosek@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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; > } > > } >
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
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; } }
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(-)