* [Qemu-devel] [PATCH v4 0/2] i386/kvm: TSC page clocksource for Hyper-V-on-KVM fixes @ 2018-03-22 13:13 Vitaly Kuznetsov 2018-03-22 13:13 ` [Qemu-devel] [PATCH v4 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs Vitaly Kuznetsov 2018-03-22 13:13 ` [Qemu-devel] [PATCH v4 2/2] i386/kvm: expose Hyper-V frequency MSRs with reenlightenment Vitaly Kuznetsov 0 siblings, 2 replies; 9+ messages in thread From: Vitaly Kuznetsov @ 2018-03-22 13:13 UTC (permalink / raw) To: qemu-devel Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost, Marcelo Tosatti, Roman Kagan Changes since v3: - check cpu->hyperv_reenlightenment instead of has_msr_reenlightenment [Eduardo Habkost, Roman Kagan] - expose frequency MSRs only when either INVTSC or Reenlightenment is provided [Paolo Bonzini] (I'm not doing 'hv_frequency' property for now as the discussion around it seems to be inconclusive.) Previously, Ladi was working on enabling TSC page clocksource for nested Hyper-V-on-KVM workloads. He found out that if Hyper-V frequency MSRs are exposed to L1 as well as INVTSC flag Hyper-V enables TSC page clocksource to its guests. Qemu doesn't pass INVTSC by default as it is a migration blocker. I found out there's a different way to make Hyper-V like us: expose Reenlightenment MSRs to it. KVM doesn't fully support the feature as we're still unable to migrate nested environments but rudimentary support we have there (kvm/queue currently) is enough. Enable Hyper-V reenlightenment MSRs and expose frequency MSRs even without INVTSC to make things work. Vitaly Kuznetsov (2): i386/kvm: add support for Hyper-V reenlightenment MSRs i386/kvm: expose Hyper-V frequency MSRs with reenlightenment target/i386/cpu.c | 4 +++- target/i386/cpu.h | 4 ++++ target/i386/hyperv-proto.h | 9 ++++++++- target/i386/kvm.c | 41 +++++++++++++++++++++++++++++++++++++++-- target/i386/machine.c | 24 ++++++++++++++++++++++++ 5 files changed, 78 insertions(+), 4 deletions(-) -- 2.14.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v4 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs 2018-03-22 13:13 [Qemu-devel] [PATCH v4 0/2] i386/kvm: TSC page clocksource for Hyper-V-on-KVM fixes Vitaly Kuznetsov @ 2018-03-22 13:13 ` Vitaly Kuznetsov 2018-03-22 13:13 ` [Qemu-devel] [PATCH v4 2/2] i386/kvm: expose Hyper-V frequency MSRs with reenlightenment Vitaly Kuznetsov 1 sibling, 0 replies; 9+ messages in thread From: Vitaly Kuznetsov @ 2018-03-22 13:13 UTC (permalink / raw) To: qemu-devel Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost, Marcelo Tosatti, Roman Kagan KVM recently gained support for Hyper-V Reenlightenment MSRs which are required to make KVM-on-Hyper-V enable TSC page clocksource to its guests when INVTSC is not passed to it (and it is not passed by default in Qemu as it effectively blocks migration). Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- Changes since v3: - check cpu->hyperv_reenlightenment instead of has_msr_reenlightenment [Eduardo Habkost, Roman Kagan] --- target/i386/cpu.c | 4 +++- target/i386/cpu.h | 4 ++++ target/i386/hyperv-proto.h | 9 ++++++++- target/i386/kvm.c | 38 +++++++++++++++++++++++++++++++++++++- target/i386/machine.c | 24 ++++++++++++++++++++++++ 5 files changed, 76 insertions(+), 3 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 6bb4ce8719..02579f8234 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -407,7 +407,8 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { NULL /* hv_vpindex_access */, NULL /* hv_msr_reset_access */, NULL /* hv_msr_stats_access */, NULL /* hv_reftsc_access */, NULL /* hv_msr_idle_access */, NULL /* hv_msr_frequency_access */, - NULL, NULL, NULL, NULL, + NULL /* hv_msr_debug_access */, NULL /* hv_msr_reenlightenment_access */, + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, @@ -4764,6 +4765,7 @@ static Property x86_cpu_properties[] = { 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-reenlightenment", X86CPU, hyperv_reenlightenment, false), 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 2e2bab5ff3..98eed72937 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1174,6 +1174,9 @@ typedef struct CPUX86State { uint64_t msr_hv_synic_sint[HV_SINT_COUNT]; uint64_t msr_hv_stimer_config[HV_STIMER_COUNT]; uint64_t msr_hv_stimer_count[HV_STIMER_COUNT]; + uint64_t msr_hv_reenlightenment_control; + uint64_t msr_hv_tsc_emulation_control; + uint64_t msr_hv_tsc_emulation_status; uint64_t msr_rtit_ctrl; uint64_t msr_rtit_status; @@ -1296,6 +1299,7 @@ struct X86CPU { bool hyperv_runtime; bool hyperv_synic; bool hyperv_stimer; + bool hyperv_reenlightenment; bool check_cpuid; bool enforce_cpuid; bool expose_kvm; diff --git a/target/i386/hyperv-proto.h b/target/i386/hyperv-proto.h index cb4d7f2b7a..93352ebd2a 100644 --- a/target/i386/hyperv-proto.h +++ b/target/i386/hyperv-proto.h @@ -35,7 +35,7 @@ #define HV_RESET_AVAILABLE (1u << 7) #define HV_REFERENCE_TSC_AVAILABLE (1u << 9) #define HV_ACCESS_FREQUENCY_MSRS (1u << 11) - +#define HV_ACCESS_REENLIGHTENMENTS_CONTROL (1u << 13) /* * HV_CPUID_FEATURES.EDX bits @@ -129,6 +129,13 @@ #define HV_X64_MSR_CRASH_CTL 0x40000105 #define HV_CRASH_CTL_NOTIFY (1ull << 63) +/* + * Reenlightenment notification MSRs + */ +#define HV_X64_MSR_REENLIGHTENMENT_CONTROL 0x40000106 +#define HV_X64_MSR_TSC_EMULATION_CONTROL 0x40000107 +#define HV_X64_MSR_TSC_EMULATION_STATUS 0x40000108 + /* * Hypercall status code */ diff --git a/target/i386/kvm.c b/target/i386/kvm.c index d23fff12f5..75f4e1d69e 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -90,6 +90,7 @@ 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_hv_reenlightenment; static bool has_msr_xss; static bool has_msr_spec_ctrl; static bool has_msr_smi_count; @@ -583,7 +584,8 @@ static bool hyperv_enabled(X86CPU *cpu) cpu->hyperv_vpindex || cpu->hyperv_runtime || cpu->hyperv_synic || - cpu->hyperv_stimer); + cpu->hyperv_stimer || + cpu->hyperv_reenlightenment); } static int kvm_arch_set_tsc_khz(CPUState *cs) @@ -654,6 +656,14 @@ static int hyperv_handle_properties(CPUState *cs) env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE; } } + if (cpu->hyperv_reenlightenment) { + if (!has_msr_hv_reenlightenment) { + fprintf(stderr, + "Hyper-V Reenlightenment is not supported by kernel\n"); + return -ENOSYS; + } + env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_REENLIGHTENMENTS_CONTROL; + } if (cpu->hyperv_crash && has_msr_hv_crash) { env->features[FEAT_HYPERV_EDX] |= HV_GUEST_CRASH_MSR_AVAILABLE; } @@ -1185,6 +1195,9 @@ static int kvm_get_supported_msrs(KVMState *s) case HV_X64_MSR_TSC_FREQUENCY: has_msr_hv_frequencies = true; break; + case HV_X64_MSR_REENLIGHTENMENT_CONTROL: + has_msr_hv_reenlightenment = true; + break; case MSR_IA32_SPEC_CTRL: has_msr_spec_ctrl = true; break; @@ -1748,6 +1761,14 @@ static int kvm_put_msrs(X86CPU *cpu, int level) kvm_msr_entry_add(cpu, HV_X64_MSR_REFERENCE_TSC, env->msr_hv_tsc); } + if (cpu->hyperv_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, + env->msr_hv_tsc_emulation_control); + kvm_msr_entry_add(cpu, HV_X64_MSR_TSC_EMULATION_STATUS, + env->msr_hv_tsc_emulation_status); + } } if (cpu->hyperv_vapic) { kvm_msr_entry_add(cpu, HV_X64_MSR_APIC_ASSIST_PAGE, @@ -2109,6 +2130,12 @@ static int kvm_get_msrs(X86CPU *cpu) } if (cpu->hyperv_time) { kvm_msr_entry_add(cpu, HV_X64_MSR_REFERENCE_TSC, 0); + + } + if (cpu->hyperv_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); } if (has_msr_hv_crash) { int j; @@ -2367,6 +2394,15 @@ static int kvm_get_msrs(X86CPU *cpu) env->msr_hv_stimer_count[(index - HV_X64_MSR_STIMER0_COUNT)/2] = msrs[i].data; break; + case HV_X64_MSR_REENLIGHTENMENT_CONTROL: + env->msr_hv_reenlightenment_control = msrs[i].data; + break; + case HV_X64_MSR_TSC_EMULATION_CONTROL: + env->msr_hv_tsc_emulation_control = msrs[i].data; + break; + case HV_X64_MSR_TSC_EMULATION_STATUS: + env->msr_hv_tsc_emulation_status = msrs[i].data; + break; case MSR_MTRRdefType: env->mtrr_deftype = msrs[i].data; break; diff --git a/target/i386/machine.c b/target/i386/machine.c index bd2d82e91b..fd99c0bbb4 100644 --- a/target/i386/machine.c +++ b/target/i386/machine.c @@ -713,6 +713,29 @@ static const VMStateDescription vmstate_msr_hyperv_stimer = { } }; +static bool hyperv_reenlightenment_enable_needed(void *opaque) +{ + X86CPU *cpu = opaque; + CPUX86State *env = &cpu->env; + + return env->msr_hv_reenlightenment_control != 0 || + env->msr_hv_tsc_emulation_control != 0 || + env->msr_hv_tsc_emulation_status != 0; +} + +static const VMStateDescription vmstate_msr_hyperv_reenlightenment = { + .name = "cpu/msr_hyperv_reenlightenment", + .version_id = 1, + .minimum_version_id = 1, + .needed = hyperv_reenlightenment_enable_needed, + .fields = (VMStateField[]) { + VMSTATE_UINT64(env.msr_hv_reenlightenment_control, X86CPU), + VMSTATE_UINT64(env.msr_hv_tsc_emulation_control, X86CPU), + VMSTATE_UINT64(env.msr_hv_tsc_emulation_status, X86CPU), + VMSTATE_END_OF_LIST() + } +}; + static bool avx512_needed(void *opaque) { X86CPU *cpu = opaque; @@ -1005,6 +1028,7 @@ VMStateDescription vmstate_x86_cpu = { &vmstate_msr_hyperv_runtime, &vmstate_msr_hyperv_synic, &vmstate_msr_hyperv_stimer, + &vmstate_msr_hyperv_reenlightenment, &vmstate_avx512, &vmstate_xss, &vmstate_tsc_khz, -- 2.14.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v4 2/2] i386/kvm: expose Hyper-V frequency MSRs with reenlightenment 2018-03-22 13:13 [Qemu-devel] [PATCH v4 0/2] i386/kvm: TSC page clocksource for Hyper-V-on-KVM fixes Vitaly Kuznetsov 2018-03-22 13:13 ` [Qemu-devel] [PATCH v4 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs Vitaly Kuznetsov @ 2018-03-22 13:13 ` Vitaly Kuznetsov 2018-03-22 18:49 ` Eduardo Habkost 2018-03-22 19:01 ` Eduardo Habkost 1 sibling, 2 replies; 9+ messages in thread From: Vitaly Kuznetsov @ 2018-03-22 13:13 UTC (permalink / raw) To: qemu-devel Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost, Marcelo Tosatti, Roman Kagan We can also expose Hyper-V frequency MSRs when reenlightenment feature is enabled and TSC frequency is known, Hyper-V on KVM will provide stable TSC page clocksources to its guests. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- - Expose frequency MSRs only when either INVTSC or Reenlightenment is provided [Paolo Bonzini] --- 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 75f4e1d69e..2c3c19d690 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -651,7 +651,8 @@ 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 (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) { + if (has_msr_hv_frequencies && env->tsc_khz && + (tsc_is_stable_and_known(env) || cpu->hyperv_reenlightenment)) { env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS; env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE; } -- 2.14.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] i386/kvm: expose Hyper-V frequency MSRs with reenlightenment 2018-03-22 13:13 ` [Qemu-devel] [PATCH v4 2/2] i386/kvm: expose Hyper-V frequency MSRs with reenlightenment Vitaly Kuznetsov @ 2018-03-22 18:49 ` Eduardo Habkost 2018-03-23 14:45 ` Vitaly Kuznetsov 2018-03-22 19:01 ` Eduardo Habkost 1 sibling, 1 reply; 9+ messages in thread From: Eduardo Habkost @ 2018-03-22 18:49 UTC (permalink / raw) To: Vitaly Kuznetsov Cc: qemu-devel, Paolo Bonzini, Richard Henderson, Marcelo Tosatti, Roman Kagan On Thu, Mar 22, 2018 at 02:13:58PM +0100, Vitaly Kuznetsov wrote: > We can also expose Hyper-V frequency MSRs when reenlightenment feature is > enabled and TSC frequency is known, Hyper-V on KVM will provide stable TSC > page clocksources to its guests. > > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > - Expose frequency MSRs only when either INVTSC or Reenlightenment is > provided [Paolo Bonzini] > --- > 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 75f4e1d69e..2c3c19d690 100644 > --- a/target/i386/kvm.c > +++ b/target/i386/kvm.c > @@ -651,7 +651,8 @@ 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 (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) { > + if (has_msr_hv_frequencies && env->tsc_khz && Why is the check for env->tsc_khz necessary? Are there known circumstances where HV_X64_MSR_TSC_FREQUENCY will be supported by KVM but ioctl(KVM_GET_TSC_KHZ) will return 0, or this is just for extra safety? > + (tsc_is_stable_and_known(env) || cpu->hyperv_reenlightenment)) { > env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS; > env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE; > } > -- > 2.14.3 > -- Eduardo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] i386/kvm: expose Hyper-V frequency MSRs with reenlightenment 2018-03-22 18:49 ` Eduardo Habkost @ 2018-03-23 14:45 ` Vitaly Kuznetsov 2018-03-26 14:25 ` Roman Kagan 0 siblings, 1 reply; 9+ messages in thread From: Vitaly Kuznetsov @ 2018-03-23 14:45 UTC (permalink / raw) To: Eduardo Habkost Cc: qemu-devel, Paolo Bonzini, Richard Henderson, Marcelo Tosatti, Roman Kagan Eduardo Habkost <ehabkost@redhat.com> writes: > On Thu, Mar 22, 2018 at 02:13:58PM +0100, Vitaly Kuznetsov wrote: >> We can also expose Hyper-V frequency MSRs when reenlightenment feature is >> enabled and TSC frequency is known, Hyper-V on KVM will provide stable TSC >> page clocksources to its guests. >> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> >> --- >> - Expose frequency MSRs only when either INVTSC or Reenlightenment is >> provided [Paolo Bonzini] >> --- >> 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 75f4e1d69e..2c3c19d690 100644 >> --- a/target/i386/kvm.c >> +++ b/target/i386/kvm.c >> @@ -651,7 +651,8 @@ 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 (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) { >> + if (has_msr_hv_frequencies && env->tsc_khz && > > Why is the check for env->tsc_khz necessary? > > Are there known circumstances where HV_X64_MSR_TSC_FREQUENCY will be supported > by KVM but ioctl(KVM_GET_TSC_KHZ) will return 0, or this is just for extra > safety? > Yes, I didn't experiment with passing '0' to Windows but in general it doesn't sound like a good idea. >> + (tsc_is_stable_and_known(env) || cpu->hyperv_reenlightenment)) { >> env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS; >> env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE; >> } >> -- >> 2.14.3 >> -- Vitaly ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] i386/kvm: expose Hyper-V frequency MSRs with reenlightenment 2018-03-23 14:45 ` Vitaly Kuznetsov @ 2018-03-26 14:25 ` Roman Kagan 2018-03-28 14:09 ` Eduardo Habkost 0 siblings, 1 reply; 9+ messages in thread From: Roman Kagan @ 2018-03-26 14:25 UTC (permalink / raw) To: Vitaly Kuznetsov Cc: Eduardo Habkost, qemu-devel, Paolo Bonzini, Richard Henderson, Marcelo Tosatti On Fri, Mar 23, 2018 at 03:45:49PM +0100, Vitaly Kuznetsov wrote: > Eduardo Habkost <ehabkost@redhat.com> writes: > > > On Thu, Mar 22, 2018 at 02:13:58PM +0100, Vitaly Kuznetsov wrote: > >> We can also expose Hyper-V frequency MSRs when reenlightenment feature is > >> enabled and TSC frequency is known, Hyper-V on KVM will provide stable TSC > >> page clocksources to its guests. > >> > >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > >> --- > >> - Expose frequency MSRs only when either INVTSC or Reenlightenment is > >> provided [Paolo Bonzini] > >> --- > >> 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 75f4e1d69e..2c3c19d690 100644 > >> --- a/target/i386/kvm.c > >> +++ b/target/i386/kvm.c > >> @@ -651,7 +651,8 @@ 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 (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) { > >> + if (has_msr_hv_frequencies && env->tsc_khz && > > > > Why is the check for env->tsc_khz necessary? > > > > Are there known circumstances where HV_X64_MSR_TSC_FREQUENCY will be supported > > by KVM but ioctl(KVM_GET_TSC_KHZ) will return 0, or this is just for extra > > safety? > > > > Yes, > > I didn't experiment with passing '0' to Windows but in general it > doesn't sound like a good idea. AFAICS the value of ->tsc_khz that QEMU pushes into KVM is the one that it first obtains from KVM via ioctl(KVM_GET_TSC_KHZ), or receives in the migration stream (obtained in a similar way on the source VM). So for all relevant configurations this check seems indeed redundant, and I went ahead and dropped it in the patch I posted. Did I miss any case where it is not? Thanks, Roman. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] i386/kvm: expose Hyper-V frequency MSRs with reenlightenment 2018-03-26 14:25 ` Roman Kagan @ 2018-03-28 14:09 ` Eduardo Habkost 0 siblings, 0 replies; 9+ messages in thread From: Eduardo Habkost @ 2018-03-28 14:09 UTC (permalink / raw) To: Roman Kagan, Vitaly Kuznetsov, qemu-devel, Paolo Bonzini, Richard Henderson, Marcelo Tosatti On Mon, Mar 26, 2018 at 05:25:47PM +0300, Roman Kagan wrote: > On Fri, Mar 23, 2018 at 03:45:49PM +0100, Vitaly Kuznetsov wrote: > > Eduardo Habkost <ehabkost@redhat.com> writes: > > > > > On Thu, Mar 22, 2018 at 02:13:58PM +0100, Vitaly Kuznetsov wrote: > > >> We can also expose Hyper-V frequency MSRs when reenlightenment feature is > > >> enabled and TSC frequency is known, Hyper-V on KVM will provide stable TSC > > >> page clocksources to its guests. > > >> > > >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > > >> --- > > >> - Expose frequency MSRs only when either INVTSC or Reenlightenment is > > >> provided [Paolo Bonzini] > > >> --- > > >> 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 75f4e1d69e..2c3c19d690 100644 > > >> --- a/target/i386/kvm.c > > >> +++ b/target/i386/kvm.c > > >> @@ -651,7 +651,8 @@ 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 (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) { > > >> + if (has_msr_hv_frequencies && env->tsc_khz && > > > > > > Why is the check for env->tsc_khz necessary? > > > > > > Are there known circumstances where HV_X64_MSR_TSC_FREQUENCY will be supported > > > by KVM but ioctl(KVM_GET_TSC_KHZ) will return 0, or this is just for extra > > > safety? > > > > > > > Yes, > > > > I didn't experiment with passing '0' to Windows but in general it > > doesn't sound like a good idea. > > AFAICS the value of ->tsc_khz that QEMU pushes into KVM is the one that > it first obtains from KVM via ioctl(KVM_GET_TSC_KHZ), or receives in the > migration stream (obtained in a similar way on the source VM). So for > all relevant configurations this check seems indeed redundant, and I > went ahead and dropped it in the patch I posted. Did I miss any case > where it is not? That's my impression as well. -- Eduardo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] i386/kvm: expose Hyper-V frequency MSRs with reenlightenment 2018-03-22 13:13 ` [Qemu-devel] [PATCH v4 2/2] i386/kvm: expose Hyper-V frequency MSRs with reenlightenment Vitaly Kuznetsov 2018-03-22 18:49 ` Eduardo Habkost @ 2018-03-22 19:01 ` Eduardo Habkost 2018-03-23 10:11 ` Roman Kagan 1 sibling, 1 reply; 9+ messages in thread From: Eduardo Habkost @ 2018-03-22 19:01 UTC (permalink / raw) To: Vitaly Kuznetsov Cc: qemu-devel, Paolo Bonzini, Marcelo Tosatti, Roman Kagan, Richard Henderson On Thu, Mar 22, 2018 at 02:13:58PM +0100, Vitaly Kuznetsov wrote: > We can also expose Hyper-V frequency MSRs when reenlightenment feature is > enabled and TSC frequency is known, Hyper-V on KVM will provide stable TSC > page clocksources to its guests. > > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > - Expose frequency MSRs only when either INVTSC or Reenlightenment is > provided [Paolo Bonzini] > --- > 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 75f4e1d69e..2c3c19d690 100644 > --- a/target/i386/kvm.c > +++ b/target/i386/kvm.c > @@ -651,7 +651,8 @@ 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 (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) { > + if (has_msr_hv_frequencies && env->tsc_khz && > + (tsc_is_stable_and_known(env) || cpu->hyperv_reenlightenment)) { Continuing the discussion we had in v3 about being possible to crash when migrating to a host running an older kernel: This patch doesn't fix the issue, because it is still implicitly enabling hv-frequencies. But I don't think this patch will make the situation any worse, because we don't have any KVM versions that support HV_X64_MSR_REENLIGHTENMENT_CONTROL but not HV_X64_MSR_TSC_FREQUENCY. This means that we can safely make "hv-reenlightenment=on" automatically set "hv-frequencies=on", when we finally introduce a hv-frequencies property. Roman, do you agree? The next question is: do we need to fix this and introduce a hv-frequencies property in 2.12, or can this wait for 2.13? > env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS; > env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE; > } > -- > 2.14.3 > > -- Eduardo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] i386/kvm: expose Hyper-V frequency MSRs with reenlightenment 2018-03-22 19:01 ` Eduardo Habkost @ 2018-03-23 10:11 ` Roman Kagan 0 siblings, 0 replies; 9+ messages in thread From: Roman Kagan @ 2018-03-23 10:11 UTC (permalink / raw) To: Eduardo Habkost Cc: Vitaly Kuznetsov, qemu-devel, Paolo Bonzini, Marcelo Tosatti, Richard Henderson On Thu, Mar 22, 2018 at 04:01:46PM -0300, Eduardo Habkost wrote: > On Thu, Mar 22, 2018 at 02:13:58PM +0100, Vitaly Kuznetsov wrote: > > We can also expose Hyper-V frequency MSRs when reenlightenment feature is > > enabled and TSC frequency is known, Hyper-V on KVM will provide stable TSC > > page clocksources to its guests. > > > > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > > --- > > - Expose frequency MSRs only when either INVTSC or Reenlightenment is > > provided [Paolo Bonzini] > > --- > > 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 75f4e1d69e..2c3c19d690 100644 > > --- a/target/i386/kvm.c > > +++ b/target/i386/kvm.c > > @@ -651,7 +651,8 @@ 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 (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) { > > + if (has_msr_hv_frequencies && env->tsc_khz && > > + (tsc_is_stable_and_known(env) || cpu->hyperv_reenlightenment)) { > > Continuing the discussion we had in v3 about being possible to > crash when migrating to a host running an older kernel: > > This patch doesn't fix the issue, because it is still implicitly > enabling hv-frequencies. > > But I don't think this patch will make the situation any worse, > because we don't have any KVM versions that support > HV_X64_MSR_REENLIGHTENMENT_CONTROL but not > HV_X64_MSR_TSC_FREQUENCY. > > This means that we can safely make "hv-reenlightenment=on" > automatically set "hv-frequencies=on", when we finally introduce > a hv-frequencies property. Do I get you right that there's no controversy about the need for hv-frequencies? > Roman, do you agree? Well, this strictly depends on the answer to your next question, because hv-reenlightenment is most certainly 2.13 material. > The next question is: do we need to fix this and introduce a > hv-frequencies property in 2.12, or can this wait for 2.13? I'm tempted to stick hv-frequencies into 2.12 and retrospectively into 2.11-stable, to minimize the exposure to the problem but to keep the effort and added complexity reasonable. But I'm not sure this will be ok from the compatibility policy standpoint. So I'd appreciate an advice on this. [Should I better just post a patch doing that and continue this discussion there?] Thanks, Roman. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-03-28 14:09 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-03-22 13:13 [Qemu-devel] [PATCH v4 0/2] i386/kvm: TSC page clocksource for Hyper-V-on-KVM fixes Vitaly Kuznetsov 2018-03-22 13:13 ` [Qemu-devel] [PATCH v4 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs Vitaly Kuznetsov 2018-03-22 13:13 ` [Qemu-devel] [PATCH v4 2/2] i386/kvm: expose Hyper-V frequency MSRs with reenlightenment Vitaly Kuznetsov 2018-03-22 18:49 ` Eduardo Habkost 2018-03-23 14:45 ` Vitaly Kuznetsov 2018-03-26 14:25 ` Roman Kagan 2018-03-28 14:09 ` Eduardo Habkost 2018-03-22 19:01 ` Eduardo Habkost 2018-03-23 10:11 ` Roman Kagan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).