* [Qemu-devel] [PATCH v2 0/2] i386/kvm: TSC page clocksource for Hyper-V-on-KVM fixes @ 2018-03-16 17:00 Vitaly Kuznetsov 2018-03-16 17:00 ` [Qemu-devel] [PATCH v2 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs Vitaly Kuznetsov 2018-03-16 17:00 ` [Qemu-devel] [PATCH v2 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-16 17:00 UTC (permalink / raw) To: Paolo Bonzini Cc: Richard Henderson, Eduardo Habkost, Marcelo Tosatti, qemu-devel, Roman Kagan Changes since v1: - add vmstate_msr_hyperv_reenlightenment subsection to vmstate_x86_cpu [Paolo Bonzini] - rebase. 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. 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 ++++++++++++++++++++++++++++++++++- target/i386/machine.c | 24 ++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 2 deletions(-) -- 2.14.3 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v2 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs 2018-03-16 17:00 [Qemu-devel] [PATCH v2 0/2] i386/kvm: TSC page clocksource for Hyper-V-on-KVM fixes Vitaly Kuznetsov @ 2018-03-16 17:00 ` Vitaly Kuznetsov 2018-03-19 17:06 ` Roman Kagan 2018-03-16 17:00 ` [Qemu-devel] [PATCH v2 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-16 17:00 UTC (permalink / raw) To: Paolo Bonzini Cc: Richard Henderson, Eduardo Habkost, Marcelo Tosatti, qemu-devel, 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 v1: - add vmstate_msr_hyperv_reenlightenment subsection to vmstate_x86_cpu [Paolo Bonzini] --- target/i386/cpu.h | 3 +++ target/i386/hyperv-proto.h | 9 ++++++++- target/i386/kvm.c | 33 +++++++++++++++++++++++++++++++++ target/i386/machine.c | 24 ++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 1 deletion(-) diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 2e2bab5ff3..0b1b556a56 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; 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..accf50eac3 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; @@ -653,6 +654,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; @@ -1185,6 +1191,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; @@ -1747,6 +1756,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) { @@ -2109,6 +2127,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; @@ -2367,6 +2391,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] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs 2018-03-16 17:00 ` [Qemu-devel] [PATCH v2 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs Vitaly Kuznetsov @ 2018-03-19 17:06 ` Roman Kagan 2018-03-19 17:29 ` Vitaly Kuznetsov 2018-03-20 12:00 ` Paolo Bonzini 0 siblings, 2 replies; 10+ messages in thread From: Roman Kagan @ 2018-03-19 17:06 UTC (permalink / raw) To: Vitaly Kuznetsov Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost, Marcelo Tosatti, qemu-devel On Fri, Mar 16, 2018 at 06:00:19PM +0100, 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> > --- > Changes since v1: > - add vmstate_msr_hyperv_reenlightenment subsection to vmstate_x86_cpu > [Paolo Bonzini] > --- > target/i386/cpu.h | 3 +++ > target/i386/hyperv-proto.h | 9 ++++++++- > target/i386/kvm.c | 33 +++++++++++++++++++++++++++++++++ > target/i386/machine.c | 24 ++++++++++++++++++++++++ > 4 files changed, 68 insertions(+), 1 deletion(-) > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > index 2e2bab5ff3..0b1b556a56 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; > 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..accf50eac3 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; > @@ -653,6 +654,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; > + } Can you please add a matching comment to the definition of feature_word_info[FEAT_HYPERV_EAX].feat_names[]? Also there appears to be no cpu property to turn this on/off, does it? It's enabled based only on the support in the KVM it's running against. So I guess we may have a problem migrating between the hosts with different KVM versions, one supporting it and the other not. (This is also a problem with has_msr_hv_frequencies, and is in general a long-standing issue of hv_* properties being done differently from the rest of CPUID features.) Thanks, Roman. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs 2018-03-19 17:06 ` Roman Kagan @ 2018-03-19 17:29 ` Vitaly Kuznetsov 2018-03-19 17:52 ` Roman Kagan 2018-03-20 18:16 ` Eduardo Habkost 2018-03-20 12:00 ` Paolo Bonzini 1 sibling, 2 replies; 10+ messages in thread From: Vitaly Kuznetsov @ 2018-03-19 17:29 UTC (permalink / raw) To: Roman Kagan Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost, Marcelo Tosatti, qemu-devel Roman Kagan <rkagan@virtuozzo.com> writes: > On Fri, Mar 16, 2018 at 06:00:19PM +0100, 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> >> --- >> Changes since v1: >> - add vmstate_msr_hyperv_reenlightenment subsection to vmstate_x86_cpu >> [Paolo Bonzini] >> --- >> target/i386/cpu.h | 3 +++ >> target/i386/hyperv-proto.h | 9 ++++++++- >> target/i386/kvm.c | 33 +++++++++++++++++++++++++++++++++ >> target/i386/machine.c | 24 ++++++++++++++++++++++++ >> 4 files changed, 68 insertions(+), 1 deletion(-) >> >> diff --git a/target/i386/cpu.h b/target/i386/cpu.h >> index 2e2bab5ff3..0b1b556a56 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; >> 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..accf50eac3 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; >> @@ -653,6 +654,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; >> + } > > Can you please add a matching comment to the definition of > feature_word_info[FEAT_HYPERV_EAX].feat_names[]? > Sure, missed that. > Also there appears to be no cpu property to turn this on/off, does it? > It's enabled based only on the support in the KVM it's running against. > So I guess we may have a problem migrating between the hosts with > different KVM versions, one supporting it and the other not. Currently nested workloads don't migrate so I decided to take the opportunity and squeeze the new feature in without adding a new hv_reenlightenment cpu property (which would have to be added to libvirt at least). > (This is also a problem with has_msr_hv_frequencies, and is in general a > long-standing issue of hv_* properties being done differently from the > rest of CPUID features.) Suggestions? (To be honest I don't really like us adding new hv_* property for every new Hyper-V feature we support. I doubt anyone needs 'partial' Hyper-V emulation. It would be nice to have a single versioned 'hv' feature implying everything. We may then forbid migrations to older hv versions. But I don't really know the history of why we decided to go with a separate hv_* for every feature we add). -- Vitaly ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs 2018-03-19 17:29 ` Vitaly Kuznetsov @ 2018-03-19 17:52 ` Roman Kagan 2018-03-20 18:16 ` Eduardo Habkost 1 sibling, 0 replies; 10+ messages in thread From: Roman Kagan @ 2018-03-19 17:52 UTC (permalink / raw) To: Vitaly Kuznetsov Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost, Marcelo Tosatti, qemu-devel On Mon, Mar 19, 2018 at 06:29:08PM +0100, Vitaly Kuznetsov wrote: > Roman Kagan <rkagan@virtuozzo.com> writes: > > > On Fri, Mar 16, 2018 at 06:00:19PM +0100, 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> > >> --- > >> Changes since v1: > >> - add vmstate_msr_hyperv_reenlightenment subsection to vmstate_x86_cpu > >> [Paolo Bonzini] > >> --- > >> target/i386/cpu.h | 3 +++ > >> target/i386/hyperv-proto.h | 9 ++++++++- > >> target/i386/kvm.c | 33 +++++++++++++++++++++++++++++++++ > >> target/i386/machine.c | 24 ++++++++++++++++++++++++ > >> 4 files changed, 68 insertions(+), 1 deletion(-) > >> > >> diff --git a/target/i386/cpu.h b/target/i386/cpu.h > >> index 2e2bab5ff3..0b1b556a56 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; > >> 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..accf50eac3 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; > >> @@ -653,6 +654,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; > >> + } > > > > Can you please add a matching comment to the definition of > > feature_word_info[FEAT_HYPERV_EAX].feat_names[]? > > > > Sure, missed that. > > > Also there appears to be no cpu property to turn this on/off, does it? > > It's enabled based only on the support in the KVM it's running against. > > So I guess we may have a problem migrating between the hosts with > > different KVM versions, one supporting it and the other not. > > Currently nested workloads don't migrate so I decided to take the > opportunity and squeeze the new feature in without adding a new > hv_reenlightenment cpu property (which would have to be added to libvirt > at least). Well, it (== L1) doesn't migrate with L2 running inside, but it certainly does without. > > > (This is also a problem with has_msr_hv_frequencies, and is in general a > > long-standing issue of hv_* properties being done differently from the > > rest of CPUID features.) > > Suggestions? (To be honest I don't really like us adding new hv_* > property for every new Hyper-V feature we support. I doubt anyone needs > 'partial' Hyper-V emulation. It would be nice to have a single versioned > 'hv' feature implying everything. We may then forbid migrations to older > hv versions. But I don't really know the history of why we decided to go > with a separate hv_* for every feature we add). IIRC those hv_* features were added before the generic CPUID feature bit naming scheme was introduced. We probably need to actually add the respective feature names to those .feat_names arrays, introduce a compatibility translation to/from the existing hv_* features, and address the conflicts, but I vaguely remember an attempt to do so ended up nowhere... I guess for the time being we can go on with extending hv_* features, but will need to do a conversion in (preferrably not too distant) future. But I'd rather have Eduardo's or Paolo's word on this. Thanks, Roman. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs 2018-03-19 17:29 ` Vitaly Kuznetsov 2018-03-19 17:52 ` Roman Kagan @ 2018-03-20 18:16 ` Eduardo Habkost 2018-03-21 10:58 ` Roman Kagan 2018-03-21 11:01 ` Paolo Bonzini 1 sibling, 2 replies; 10+ messages in thread From: Eduardo Habkost @ 2018-03-20 18:16 UTC (permalink / raw) To: Vitaly Kuznetsov Cc: Roman Kagan, Paolo Bonzini, Richard Henderson, Marcelo Tosatti, qemu-devel On Mon, Mar 19, 2018 at 06:29:08PM +0100, Vitaly Kuznetsov wrote: > Roman Kagan <rkagan@virtuozzo.com> writes: > > > On Fri, Mar 16, 2018 at 06:00:19PM +0100, 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> > >> --- > >> Changes since v1: > >> - add vmstate_msr_hyperv_reenlightenment subsection to vmstate_x86_cpu > >> [Paolo Bonzini] > >> --- [...] > >> + > >> + if (has_msr_hv_reenlightenment) { > >> + env->features[FEAT_HYPERV_EAX] |= > >> + HV_ACCESS_REENLIGHTENMENTS_CONTROL; > >> + } > > > > Can you please add a matching comment to the definition of > > feature_word_info[FEAT_HYPERV_EAX].feat_names[]? > > > > Sure, missed that. > > > Also there appears to be no cpu property to turn this on/off, does it? > > It's enabled based only on the support in the KVM it's running against. > > So I guess we may have a problem migrating between the hosts with > > different KVM versions, one supporting it and the other not. > > Currently nested workloads don't migrate so I decided to take the > opportunity and squeeze the new feature in without adding a new > hv_reenlightenment cpu property (which would have to be added to libvirt > at least). > > > (This is also a problem with has_msr_hv_frequencies, and is in general a > > long-standing issue of hv_* properties being done differently from the > > rest of CPUID features.) > > Suggestions? (To be honest I don't really like us adding new hv_* > property for every new Hyper-V feature we support. I doubt anyone needs > 'partial' Hyper-V emulation. It would be nice to have a single versioned > 'hv' feature implying everything. We may then forbid migrations to older > hv versions. But I don't really know the history of why we decided to go > with a separate hv_* for every feature we add). You will need "partial" emulation if you want to support live-migration to/from a host where the KVM or QEMU don't support all the features from the current host. Is this something the current Hyper-V code already supports, or it's something known to be broken? -- Eduardo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs 2018-03-20 18:16 ` Eduardo Habkost @ 2018-03-21 10:58 ` Roman Kagan 2018-03-21 11:01 ` Paolo Bonzini 1 sibling, 0 replies; 10+ messages in thread From: Roman Kagan @ 2018-03-21 10:58 UTC (permalink / raw) To: Eduardo Habkost Cc: Vitaly Kuznetsov, Paolo Bonzini, Richard Henderson, Marcelo Tosatti, qemu-devel On Tue, Mar 20, 2018 at 03:16:52PM -0300, Eduardo Habkost wrote: > On Mon, Mar 19, 2018 at 06:29:08PM +0100, Vitaly Kuznetsov wrote: > > Roman Kagan <rkagan@virtuozzo.com> writes: > > > (This is also a problem with has_msr_hv_frequencies, and is in general a > > > long-standing issue of hv_* properties being done differently from the > > > rest of CPUID features.) > > > > Suggestions? (To be honest I don't really like us adding new hv_* > > property for every new Hyper-V feature we support. I doubt anyone needs > > 'partial' Hyper-V emulation. It would be nice to have a single versioned > > 'hv' feature implying everything. We may then forbid migrations to older > > hv versions. But I don't really know the history of why we decided to go > > with a separate hv_* for every feature we add). > > You will need "partial" emulation if you want to support > live-migration to/from a host where the KVM or QEMU don't support > all the features from the current host. > > Is this something the current Hyper-V code already supports, or > it's something known to be broken? There's at least one case I mentioned above where it's broken, but, as Paolo pointed out, the real Windows guests wouldn't notice because they didn't use the feature due to missing INVTSC. I myself haven't tested all possible combinations of hv_* flags and KVM versions, but from code inspection I don't immediately see anything else that's obviously broken in this regard. Roman. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs 2018-03-20 18:16 ` Eduardo Habkost 2018-03-21 10:58 ` Roman Kagan @ 2018-03-21 11:01 ` Paolo Bonzini 1 sibling, 0 replies; 10+ messages in thread From: Paolo Bonzini @ 2018-03-21 11:01 UTC (permalink / raw) To: Eduardo Habkost, Vitaly Kuznetsov Cc: Roman Kagan, Richard Henderson, Marcelo Tosatti, qemu-devel On 20/03/2018 19:16, Eduardo Habkost wrote: >> Suggestions? (To be honest I don't really like us adding new hv_* >> property for every new Hyper-V feature we support. I doubt anyone needs >> 'partial' Hyper-V emulation. It would be nice to have a single versioned >> 'hv' feature implying everything. We may then forbid migrations to older >> hv versions. But I don't really know the history of why we decided to go >> with a separate hv_* for every feature we add). > You will need "partial" emulation if you want to support > live-migration to/from a host where the KVM or QEMU don't support > all the features from the current host. > > Is this something the current Hyper-V code already supports, or > it's something known to be broken? Yes, it works. Essentially, multiple Hyper-V features are (just like multiple KVM features) needed to support old kernels. The difference is that new KVM features are added quite rarely, and most of them are enabled by default. The opposite is true of Hyper-V, on both counts. Paolo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs 2018-03-19 17:06 ` Roman Kagan 2018-03-19 17:29 ` Vitaly Kuznetsov @ 2018-03-20 12:00 ` Paolo Bonzini 1 sibling, 0 replies; 10+ messages in thread From: Paolo Bonzini @ 2018-03-20 12:00 UTC (permalink / raw) To: Roman Kagan, Vitaly Kuznetsov, Richard Henderson, Eduardo Habkost, Marcelo Tosatti, qemu-devel On 19/03/2018 18:06, Roman Kagan wrote: >> + if (has_msr_hv_reenlightenment) { >> + env->features[FEAT_HYPERV_EAX] |= >> + HV_ACCESS_REENLIGHTENMENTS_CONTROL; >> + } > Can you please add a matching comment to the definition of > feature_word_info[FEAT_HYPERV_EAX].feat_names[]? > > Also there appears to be no cpu property to turn this on/off, does it? > It's enabled based only on the support in the KVM it's running against. > So I guess we may have a problem migrating between the hosts with > different KVM versions, one supporting it and the other not. > (This is also a problem with has_msr_hv_frequencies, and is in general a > long-standing issue of hv_* properties being done differently from the > rest of CPUID features.) Yeah, so far for frequencies that was okay or at least okay-ish because Windows didn't use it by default without invtsc (disables migration). However, we do need a new hv_reenlight property; if it is not set, QEMU should behave as if the reenlightenment MSRs are not provided by KVM. Thanks, Paolo ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] i386/kvm: lower requirements for Hyper-V frequency MSRs exposure 2018-03-16 17:00 [Qemu-devel] [PATCH v2 0/2] i386/kvm: TSC page clocksource for Hyper-V-on-KVM fixes Vitaly Kuznetsov 2018-03-16 17:00 ` [Qemu-devel] [PATCH v2 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs Vitaly Kuznetsov @ 2018-03-16 17:00 ` Vitaly Kuznetsov 1 sibling, 0 replies; 10+ messages in thread From: Vitaly Kuznetsov @ 2018-03-16 17:00 UTC (permalink / raw) To: Paolo Bonzini Cc: Richard Henderson, Eduardo Habkost, Marcelo Tosatti, qemu-devel, Roman Kagan 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 accf50eac3..a6d1210f46 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -650,7 +650,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
end of thread, other threads:[~2018-03-21 11:01 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-03-16 17:00 [Qemu-devel] [PATCH v2 0/2] i386/kvm: TSC page clocksource for Hyper-V-on-KVM fixes Vitaly Kuznetsov 2018-03-16 17:00 ` [Qemu-devel] [PATCH v2 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs Vitaly Kuznetsov 2018-03-19 17:06 ` Roman Kagan 2018-03-19 17:29 ` Vitaly Kuznetsov 2018-03-19 17:52 ` Roman Kagan 2018-03-20 18:16 ` Eduardo Habkost 2018-03-21 10:58 ` Roman Kagan 2018-03-21 11:01 ` Paolo Bonzini 2018-03-20 12:00 ` Paolo Bonzini 2018-03-16 17:00 ` [Qemu-devel] [PATCH v2 2/2] i386/kvm: lower requirements for Hyper-V frequency MSRs exposure Vitaly Kuznetsov
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).