* [Qemu-devel] [PATCH 0/2] i386/kvm: TSC page clocksource for Hyper-V-on-KVM fixes @ 2018-03-12 15:12 Vitaly Kuznetsov 2018-03-12 15:12 ` [Qemu-devel] [PATCH 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs Vitaly Kuznetsov 2018-03-12 15:12 ` [Qemu-devel] [PATCH 2/2] i386/kvm: lower requirements for Hyper-V frequency MSRs exposure Vitaly Kuznetsov 0 siblings, 2 replies; 10+ messages in thread From: Vitaly Kuznetsov @ 2018-03-12 15:12 UTC (permalink / raw) To: Paolo Bonzini, Richard Henderson, Eduardo Habkost, Marcelo Tosatti Cc: Roman Kagan, qemu-devel 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 only currently) is enough. Enable Hyper-V reenlightenment MSRs and expose frequency MSRs even without INVTSC to make things work. [My first patches for Qemu, please be nice :-) ] Vitaly Kuznetsov (2): i386/kvm: add support for Hyper-V reenlightenment MSRs i386/kvm: lower requirements for Hyper-V frequency MSRs exposure target/i386/cpu.h | 3 +++ target/i386/hyperv-proto.h | 9 ++++++++- target/i386/kvm.c | 35 ++++++++++++++++++++++++++++++++++- 3 files changed, 45 insertions(+), 2 deletions(-) -- 2.14.3 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs 2018-03-12 15:12 [Qemu-devel] [PATCH 0/2] i386/kvm: TSC page clocksource for Hyper-V-on-KVM fixes Vitaly Kuznetsov @ 2018-03-12 15:12 ` Vitaly Kuznetsov 2018-03-16 14:48 ` Paolo Bonzini 2018-03-12 15:12 ` [Qemu-devel] [PATCH 2/2] i386/kvm: lower requirements for Hyper-V frequency MSRs exposure Vitaly Kuznetsov 1 sibling, 1 reply; 10+ messages in thread From: Vitaly Kuznetsov @ 2018-03-12 15:12 UTC (permalink / raw) To: Paolo Bonzini, Richard Henderson, Eduardo Habkost, Marcelo Tosatti Cc: Roman Kagan, qemu-devel 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> --- target/i386/cpu.h | 3 +++ target/i386/hyperv-proto.h | 9 ++++++++- target/i386/kvm.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/target/i386/cpu.h b/target/i386/cpu.h index faf39ec1ce..502b535be2 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1152,6 +1152,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; /* exception/interrupt handling */ int error_code; 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 ad4b159b28..21e06deaf1 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; @@ -649,6 +650,11 @@ 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 (has_msr_hv_reenlightenment) { + 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; @@ -1154,6 +1160,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; @@ -1713,6 +1722,15 @@ static int kvm_put_msrs(X86CPU *cpu, int level) if (cpu->hyperv_time) { kvm_msr_entry_add(cpu, HV_X64_MSR_REFERENCE_TSC, env->msr_hv_tsc); + + if (has_msr_hv_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) { @@ -2053,6 +2071,12 @@ static int kvm_get_msrs(X86CPU *cpu) } if (cpu->hyperv_time) { kvm_msr_entry_add(cpu, HV_X64_MSR_REFERENCE_TSC, 0); + + if (has_msr_hv_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; @@ -2294,6 +2318,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; -- 2.14.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs 2018-03-12 15:12 ` [Qemu-devel] [PATCH 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs Vitaly Kuznetsov @ 2018-03-16 14:48 ` Paolo Bonzini 2018-03-16 15:10 ` Vitaly Kuznetsov 0 siblings, 1 reply; 10+ messages in thread From: Paolo Bonzini @ 2018-03-16 14:48 UTC (permalink / raw) To: Vitaly Kuznetsov, Richard Henderson, Eduardo Habkost, Marcelo Tosatti Cc: Roman Kagan, qemu-devel On 12/03/2018 16:12, Vitaly Kuznetsov wrote: > 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> > --- > target/i386/cpu.h | 3 +++ > target/i386/hyperv-proto.h | 9 ++++++++- > target/i386/kvm.c | 33 +++++++++++++++++++++++++++++++++ > 3 files changed, 44 insertions(+), 1 deletion(-) > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > index faf39ec1ce..502b535be2 100644 > --- a/target/i386/cpu.h > +++ b/target/i386/cpu.h > @@ -1152,6 +1152,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; > > /* exception/interrupt handling */ > int error_code; > 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 ad4b159b28..21e06deaf1 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; > > @@ -649,6 +650,11 @@ 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 (has_msr_hv_reenlightenment) { > + 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; > @@ -1154,6 +1160,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; > @@ -1713,6 +1722,15 @@ static int kvm_put_msrs(X86CPU *cpu, int level) > if (cpu->hyperv_time) { > kvm_msr_entry_add(cpu, HV_X64_MSR_REFERENCE_TSC, > env->msr_hv_tsc); > + > + if (has_msr_hv_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) { > @@ -2053,6 +2071,12 @@ static int kvm_get_msrs(X86CPU *cpu) > } > if (cpu->hyperv_time) { > kvm_msr_entry_add(cpu, HV_X64_MSR_REFERENCE_TSC, 0); > + > + if (has_msr_hv_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; > @@ -2294,6 +2318,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; > Doesn't this also need a new subsection in target/i386/machine.c? Paolo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs 2018-03-16 14:48 ` Paolo Bonzini @ 2018-03-16 15:10 ` Vitaly Kuznetsov 0 siblings, 0 replies; 10+ messages in thread From: Vitaly Kuznetsov @ 2018-03-16 15:10 UTC (permalink / raw) To: Paolo Bonzini Cc: Richard Henderson, Eduardo Habkost, Marcelo Tosatti, Roman Kagan, qemu-devel Paolo Bonzini <pbonzini@redhat.com> writes: > On 12/03/2018 16:12, Vitaly Kuznetsov wrote: [snip] >> @@ -2294,6 +2318,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; >> > > Doesn't this also need a new subsection in target/i386/machine.c? > Actually yes, missed that completely! Thanks! -- Vitaly ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 2/2] i386/kvm: lower requirements for Hyper-V frequency MSRs exposure 2018-03-12 15:12 [Qemu-devel] [PATCH 0/2] i386/kvm: TSC page clocksource for Hyper-V-on-KVM fixes Vitaly Kuznetsov 2018-03-12 15:12 ` [Qemu-devel] [PATCH 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs Vitaly Kuznetsov @ 2018-03-12 15:12 ` Vitaly Kuznetsov 2018-03-16 14:49 ` Paolo Bonzini 1 sibling, 1 reply; 10+ messages in thread From: Vitaly Kuznetsov @ 2018-03-12 15:12 UTC (permalink / raw) To: Paolo Bonzini, Richard Henderson, Eduardo Habkost, Marcelo Tosatti Cc: Roman Kagan, qemu-devel Requiring tsc_is_stable_and_known() is too restrictive: even without INVTCS nested Hyper-V-on-KVM enables TSC pages for its guests e.g. when Reenlightenment MSRs are present. Presence of frequency MSRs doesn't mean these frequencies are stable, it just means they're available for reading. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- target/i386/kvm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/i386/kvm.c b/target/i386/kvm.c index 21e06deaf1..43c521f61a 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -646,7 +646,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 (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) { + if (has_msr_hv_frequencies && env->tsc_khz) { 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] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] i386/kvm: lower requirements for Hyper-V frequency MSRs exposure 2018-03-12 15:12 ` [Qemu-devel] [PATCH 2/2] i386/kvm: lower requirements for Hyper-V frequency MSRs exposure Vitaly Kuznetsov @ 2018-03-16 14:49 ` Paolo Bonzini 2018-03-16 15:05 ` Vitaly Kuznetsov 0 siblings, 1 reply; 10+ messages in thread From: Paolo Bonzini @ 2018-03-16 14:49 UTC (permalink / raw) To: Vitaly Kuznetsov, Richard Henderson, Eduardo Habkost, Marcelo Tosatti Cc: Roman Kagan, qemu-devel On 12/03/2018 16:12, Vitaly Kuznetsov wrote: > > - if (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) { > + if (has_msr_hv_frequencies && env->tsc_khz) { Should this be ((env->tsc_khz && has_msr_hv_reenlightenment) || tsc_is_stable_and_known(env)) so that you don't regress on older kernels? Paolo > env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS; > env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE; > } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] i386/kvm: lower requirements for Hyper-V frequency MSRs exposure 2018-03-16 14:49 ` Paolo Bonzini @ 2018-03-16 15:05 ` Vitaly Kuznetsov 2018-03-16 15:28 ` Paolo Bonzini 0 siblings, 1 reply; 10+ messages in thread From: Vitaly Kuznetsov @ 2018-03-16 15:05 UTC (permalink / raw) To: Paolo Bonzini Cc: Richard Henderson, Eduardo Habkost, Marcelo Tosatti, Roman Kagan, qemu-devel Paolo Bonzini <pbonzini@redhat.com> writes: > On 12/03/2018 16:12, Vitaly Kuznetsov wrote: >> >> - if (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) { >> + if (has_msr_hv_frequencies && env->tsc_khz) { > > Should this be > > ((env->tsc_khz && has_msr_hv_reenlightenment) || > tsc_is_stable_and_known(env)) > > so that you don't regress on older kernels? > I don't actually see where the regression might come from: frequency MSRs are supported regardless or reenlightenment/invtsc and there's nothing wrong with exposing them but I may be missing something.. -- Vitaly ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] i386/kvm: lower requirements for Hyper-V frequency MSRs exposure 2018-03-16 15:05 ` Vitaly Kuznetsov @ 2018-03-16 15:28 ` Paolo Bonzini 2018-03-16 15:40 ` Vitaly Kuznetsov 0 siblings, 1 reply; 10+ messages in thread From: Paolo Bonzini @ 2018-03-16 15:28 UTC (permalink / raw) To: Vitaly Kuznetsov Cc: Richard Henderson, Eduardo Habkost, Marcelo Tosatti, Roman Kagan, qemu-devel On 16/03/2018 16:05, Vitaly Kuznetsov wrote: >>> >>> - if (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) { >>> + if (has_msr_hv_frequencies && env->tsc_khz) { >> Should this be >> >> ((env->tsc_khz && has_msr_hv_reenlightenment) || >> tsc_is_stable_and_known(env)) >> >> so that you don't regress on older kernels? >> > I don't actually see where the regression might come from: frequency > MSRs are supported regardless or reenlightenment/invtsc and there's > nothing wrong with exposing them but I may be missing something.. On older kernel without re-enlightenment support, you don't want to expose the frequency MSRs unless invtsc is on, right? Paolo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] i386/kvm: lower requirements for Hyper-V frequency MSRs exposure 2018-03-16 15:28 ` Paolo Bonzini @ 2018-03-16 15:40 ` Vitaly Kuznetsov 2018-03-16 16:04 ` Paolo Bonzini 0 siblings, 1 reply; 10+ messages in thread From: Vitaly Kuznetsov @ 2018-03-16 15:40 UTC (permalink / raw) To: Paolo Bonzini Cc: Richard Henderson, Eduardo Habkost, Marcelo Tosatti, Roman Kagan, qemu-devel Paolo Bonzini <pbonzini@redhat.com> writes: > On 16/03/2018 16:05, Vitaly Kuznetsov wrote: >>>> >>>> - if (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) { >>>> + if (has_msr_hv_frequencies && env->tsc_khz) { >>> Should this be >>> >>> ((env->tsc_khz && has_msr_hv_reenlightenment) || >>> tsc_is_stable_and_known(env)) >>> >>> so that you don't regress on older kernels? >>> >> I don't actually see where the regression might come from: frequency >> MSRs are supported regardless or reenlightenment/invtsc and there's >> nothing wrong with exposing them but I may be missing something.. > > On older kernel without re-enlightenment support, you don't want to > expose the frequency MSRs unless invtsc is on, right? > Actually no, I think it's OK to expose frequency MSRs even when we don't have invtsc and don't support re-enlightenment. Nested Hyper-V won't pass stable TSC pages to its guests unless it sees either invtsc or reenlightenment. So as long as we have something to put to these MSRs (env->tsc_khz) I *think* we can expose them. I may actually be missing the reason why Ladi put tsc_is_stable_and_known() here. In case we're running Windows (and not Hyper-V) as a guest KVM will update TSC page on migration. And genuine Hyper-V also exposes these MSRs without exposing INVTSC flag by default. -- Vitaly ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] i386/kvm: lower requirements for Hyper-V frequency MSRs exposure 2018-03-16 15:40 ` Vitaly Kuznetsov @ 2018-03-16 16:04 ` Paolo Bonzini 0 siblings, 0 replies; 10+ messages in thread From: Paolo Bonzini @ 2018-03-16 16:04 UTC (permalink / raw) To: Vitaly Kuznetsov Cc: Richard Henderson, Eduardo Habkost, Marcelo Tosatti, Roman Kagan, qemu-devel On 16/03/2018 16:40, Vitaly Kuznetsov wrote: >> On older kernel without re-enlightenment support, you don't want to >> expose the frequency MSRs unless invtsc is on, right? >> > Actually no, I think it's OK to expose frequency MSRs even when we don't > have invtsc and don't support re-enlightenment. Nested Hyper-V won't > pass stable TSC pages to its guests unless it sees either invtsc or > reenlightenment. So as long as we have something to put to these MSRs > (env->tsc_khz) I *think* we can expose them. > > I may actually be missing the reason why Ladi put > tsc_is_stable_and_known() here. Probably because I asked him to. :) It looks like Hyper-V knows that you need re-enlightenment in order to really trust the frequency MSRs (of course the TSC page is special because it has the sequence count). So the patch is good. Thanks! Paolo > In case we're running Windows (and not > Hyper-V) as a guest KVM will update TSC page on migration. And genuine > Hyper-V also exposes these MSRs without exposing INVTSC flag by > default. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-03-16 16:04 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-03-12 15:12 [Qemu-devel] [PATCH 0/2] i386/kvm: TSC page clocksource for Hyper-V-on-KVM fixes Vitaly Kuznetsov 2018-03-12 15:12 ` [Qemu-devel] [PATCH 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs Vitaly Kuznetsov 2018-03-16 14:48 ` Paolo Bonzini 2018-03-16 15:10 ` Vitaly Kuznetsov 2018-03-12 15:12 ` [Qemu-devel] [PATCH 2/2] i386/kvm: lower requirements for Hyper-V frequency MSRs exposure Vitaly Kuznetsov 2018-03-16 14:49 ` Paolo Bonzini 2018-03-16 15:05 ` Vitaly Kuznetsov 2018-03-16 15:28 ` Paolo Bonzini 2018-03-16 15:40 ` Vitaly Kuznetsov 2018-03-16 16:04 ` Paolo Bonzini
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).