qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] i386: Make migration fail when Hyper-V reenlightenment was enabled but 'user_tsc_khz' is unset
@ 2021-03-19 12:38 Vitaly Kuznetsov
  2021-03-29 12:21 ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 5+ messages in thread
From: Vitaly Kuznetsov @ 2021-03-19 12:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Marcelo Tosatti, Richard Henderson,
	Eduardo Habkost, Dr . David Alan Gilbert

KVM doesn't fully support Hyper-V reenlightenment notifications on
migration. In particular, it doesn't support emulating TSC frequency
of the source host by trapping all TSC accesses so unless TSC scaling
is supported on the destination host and KVM_SET_TSC_KHZ succeeds, it
is unsafe to proceed with migration.

KVM_SET_TSC_KHZ is called from two sites: kvm_arch_init_vcpu() and
kvm_arch_put_registers(). The later (intentionally) doesn't propagate
errors allowing migrations to succeed even when TSC scaling is not
supported on the destination. This doesn't suit 're-enlightenment'
use-case as we have to guarantee that TSC frequency stays constant.

Require 'tsc-frequency=' command line option to be specified for successful
migration when re-enlightenment was enabled by the guest.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
This patch is a successor of "[PATCH 3/3] i386: Make sure
kvm_arch_set_tsc_khz() succeeds on migration when 'hv-reenlightenment'
was exposed" taking a different approach suggested by Paolo.
---
 docs/hyperv.txt                |  5 +++++
 target/i386/kvm/hyperv-proto.h |  1 +
 target/i386/machine.c          | 20 ++++++++++++++++++++
 3 files changed, 26 insertions(+)

diff --git a/docs/hyperv.txt b/docs/hyperv.txt
index 5df00da54fc4..e53c581f4586 100644
--- a/docs/hyperv.txt
+++ b/docs/hyperv.txt
@@ -160,6 +160,11 @@ the hypervisor) until it is ready to switch to the new one. This, in conjunction
 with hv-frequencies, allows Hyper-V on KVM to pass stable clocksource (Reference
 TSC page) to its own guests.
 
+Note, KVM doesn't fully support re-enlightenment notifications and doesn't
+emulate TSC accesses after migration so 'tsc-frequency=' CPU option also has to
+be specified to make migration succeed. The destination host has to either have
+the same TSC frequency or support TSC scaling CPU feature.
+
 Recommended: hv-frequencies
 
 3.16. hv-evmcs
diff --git a/target/i386/kvm/hyperv-proto.h b/target/i386/kvm/hyperv-proto.h
index 056a305be38c..e30d64b4ade4 100644
--- a/target/i386/kvm/hyperv-proto.h
+++ b/target/i386/kvm/hyperv-proto.h
@@ -139,6 +139,7 @@
  * Reenlightenment notification MSRs
  */
 #define HV_X64_MSR_REENLIGHTENMENT_CONTROL      0x40000106
+#define HV_REENLIGHTENMENT_ENABLE_BIT           (1u << 16)
 #define HV_X64_MSR_TSC_EMULATION_CONTROL        0x40000107
 #define HV_X64_MSR_TSC_EMULATION_STATUS         0x40000108
 
diff --git a/target/i386/machine.c b/target/i386/machine.c
index 7259fe6868c6..137604ddb898 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -883,11 +883,31 @@ static bool hyperv_reenlightenment_enable_needed(void *opaque)
         env->msr_hv_tsc_emulation_status != 0;
 }
 
+static int hyperv_reenlightenment_post_load(void *opaque, int version_id)
+{
+    X86CPU *cpu = opaque;
+    CPUX86State *env = &cpu->env;
+
+    /*
+     * KVM doesn't fully support re-enlightenment notifications so we need to
+     * make sure TSC frequency doesn't change upon migration.
+     */
+    if ((env->msr_hv_reenlightenment_control & HV_REENLIGHTENMENT_ENABLE_BIT) &&
+        !env->user_tsc_khz) {
+        error_report("Guest enabled re-enlightenment notifications, "
+                     "'tsc-frequency=' has to be specified");
+        return -EINVAL;
+    }
+
+    return 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,
+    .post_load = hyperv_reenlightenment_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64(env.msr_hv_reenlightenment_control, X86CPU),
         VMSTATE_UINT64(env.msr_hv_tsc_emulation_control, X86CPU),
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] i386: Make migration fail when Hyper-V reenlightenment was enabled but 'user_tsc_khz' is unset
  2021-03-19 12:38 [PATCH v2] i386: Make migration fail when Hyper-V reenlightenment was enabled but 'user_tsc_khz' is unset Vitaly Kuznetsov
@ 2021-03-29 12:21 ` Dr. David Alan Gilbert
  2021-03-29 12:46   ` Vitaly Kuznetsov
  0 siblings, 1 reply; 5+ messages in thread
From: Dr. David Alan Gilbert @ 2021-03-29 12:21 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: Paolo Bonzini, Marcelo Tosatti, Richard Henderson, qemu-devel,
	Eduardo Habkost

* Vitaly Kuznetsov (vkuznets@redhat.com) wrote:
> KVM doesn't fully support Hyper-V reenlightenment notifications on
> migration. In particular, it doesn't support emulating TSC frequency
> of the source host by trapping all TSC accesses so unless TSC scaling
> is supported on the destination host and KVM_SET_TSC_KHZ succeeds, it
> is unsafe to proceed with migration.
> 
> KVM_SET_TSC_KHZ is called from two sites: kvm_arch_init_vcpu() and
> kvm_arch_put_registers(). The later (intentionally) doesn't propagate
> errors allowing migrations to succeed even when TSC scaling is not
> supported on the destination. This doesn't suit 're-enlightenment'
> use-case as we have to guarantee that TSC frequency stays constant.
> 
> Require 'tsc-frequency=' command line option to be specified for successful
> migration when re-enlightenment was enabled by the guest.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> This patch is a successor of "[PATCH 3/3] i386: Make sure
> kvm_arch_set_tsc_khz() succeeds on migration when 'hv-reenlightenment'
> was exposed" taking a different approach suggested by Paolo.
> ---
>  docs/hyperv.txt                |  5 +++++
>  target/i386/kvm/hyperv-proto.h |  1 +
>  target/i386/machine.c          | 20 ++++++++++++++++++++
>  3 files changed, 26 insertions(+)
> 
> diff --git a/docs/hyperv.txt b/docs/hyperv.txt
> index 5df00da54fc4..e53c581f4586 100644
> --- a/docs/hyperv.txt
> +++ b/docs/hyperv.txt
> @@ -160,6 +160,11 @@ the hypervisor) until it is ready to switch to the new one. This, in conjunction
>  with hv-frequencies, allows Hyper-V on KVM to pass stable clocksource (Reference
>  TSC page) to its own guests.
>  
> +Note, KVM doesn't fully support re-enlightenment notifications and doesn't
> +emulate TSC accesses after migration so 'tsc-frequency=' CPU option also has to
> +be specified to make migration succeed. The destination host has to either have
> +the same TSC frequency or support TSC scaling CPU feature.
> +
>  Recommended: hv-frequencies
>  
>  3.16. hv-evmcs
> diff --git a/target/i386/kvm/hyperv-proto.h b/target/i386/kvm/hyperv-proto.h
> index 056a305be38c..e30d64b4ade4 100644
> --- a/target/i386/kvm/hyperv-proto.h
> +++ b/target/i386/kvm/hyperv-proto.h
> @@ -139,6 +139,7 @@
>   * Reenlightenment notification MSRs
>   */
>  #define HV_X64_MSR_REENLIGHTENMENT_CONTROL      0x40000106
> +#define HV_REENLIGHTENMENT_ENABLE_BIT           (1u << 16)
>  #define HV_X64_MSR_TSC_EMULATION_CONTROL        0x40000107
>  #define HV_X64_MSR_TSC_EMULATION_STATUS         0x40000108
>  
> diff --git a/target/i386/machine.c b/target/i386/machine.c
> index 7259fe6868c6..137604ddb898 100644
> --- a/target/i386/machine.c
> +++ b/target/i386/machine.c
> @@ -883,11 +883,31 @@ static bool hyperv_reenlightenment_enable_needed(void *opaque)
>          env->msr_hv_tsc_emulation_status != 0;
>  }
>  
> +static int hyperv_reenlightenment_post_load(void *opaque, int version_id)
> +{
> +    X86CPU *cpu = opaque;
> +    CPUX86State *env = &cpu->env;
> +
> +    /*
> +     * KVM doesn't fully support re-enlightenment notifications so we need to
> +     * make sure TSC frequency doesn't change upon migration.
> +     */
> +    if ((env->msr_hv_reenlightenment_control & HV_REENLIGHTENMENT_ENABLE_BIT) &&
> +        !env->user_tsc_khz) {
> +        error_report("Guest enabled re-enlightenment notifications, "
> +                     "'tsc-frequency=' has to be specified");

It's unusual to fail on the destination for a valid configuration but
guest state;  wouldn't it be better to always insist on tsc-frequency if
that hv feature is exposed; failing early before reeiving the state?

Dave

> +        return -EINVAL;
> +    }
> +
> +    return 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,
> +    .post_load = hyperv_reenlightenment_post_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64(env.msr_hv_reenlightenment_control, X86CPU),
>          VMSTATE_UINT64(env.msr_hv_tsc_emulation_control, X86CPU),
> -- 
> 2.30.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] i386: Make migration fail when Hyper-V reenlightenment was enabled but 'user_tsc_khz' is unset
  2021-03-29 12:21 ` Dr. David Alan Gilbert
@ 2021-03-29 12:46   ` Vitaly Kuznetsov
  2021-03-29 14:49     ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 5+ messages in thread
From: Vitaly Kuznetsov @ 2021-03-29 12:46 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: Paolo Bonzini, Marcelo Tosatti, Richard Henderson, qemu-devel,
	Eduardo Habkost

"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Vitaly Kuznetsov (vkuznets@redhat.com) wrote:
>> KVM doesn't fully support Hyper-V reenlightenment notifications on
>> migration. In particular, it doesn't support emulating TSC frequency
>> of the source host by trapping all TSC accesses so unless TSC scaling
>> is supported on the destination host and KVM_SET_TSC_KHZ succeeds, it
>> is unsafe to proceed with migration.
>> 
>> KVM_SET_TSC_KHZ is called from two sites: kvm_arch_init_vcpu() and
>> kvm_arch_put_registers(). The later (intentionally) doesn't propagate
>> errors allowing migrations to succeed even when TSC scaling is not
>> supported on the destination. This doesn't suit 're-enlightenment'
>> use-case as we have to guarantee that TSC frequency stays constant.
>> 
>> Require 'tsc-frequency=' command line option to be specified for successful
>> migration when re-enlightenment was enabled by the guest.
>> 
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>> This patch is a successor of "[PATCH 3/3] i386: Make sure
>> kvm_arch_set_tsc_khz() succeeds on migration when 'hv-reenlightenment'
>> was exposed" taking a different approach suggested by Paolo.
>> ---
>>  docs/hyperv.txt                |  5 +++++
>>  target/i386/kvm/hyperv-proto.h |  1 +
>>  target/i386/machine.c          | 20 ++++++++++++++++++++
>>  3 files changed, 26 insertions(+)
>> 
>> diff --git a/docs/hyperv.txt b/docs/hyperv.txt
>> index 5df00da54fc4..e53c581f4586 100644
>> --- a/docs/hyperv.txt
>> +++ b/docs/hyperv.txt
>> @@ -160,6 +160,11 @@ the hypervisor) until it is ready to switch to the new one. This, in conjunction
>>  with hv-frequencies, allows Hyper-V on KVM to pass stable clocksource (Reference
>>  TSC page) to its own guests.
>>  
>> +Note, KVM doesn't fully support re-enlightenment notifications and doesn't
>> +emulate TSC accesses after migration so 'tsc-frequency=' CPU option also has to
>> +be specified to make migration succeed. The destination host has to either have
>> +the same TSC frequency or support TSC scaling CPU feature.
>> +
>>  Recommended: hv-frequencies
>>  
>>  3.16. hv-evmcs
>> diff --git a/target/i386/kvm/hyperv-proto.h b/target/i386/kvm/hyperv-proto.h
>> index 056a305be38c..e30d64b4ade4 100644
>> --- a/target/i386/kvm/hyperv-proto.h
>> +++ b/target/i386/kvm/hyperv-proto.h
>> @@ -139,6 +139,7 @@
>>   * Reenlightenment notification MSRs
>>   */
>>  #define HV_X64_MSR_REENLIGHTENMENT_CONTROL      0x40000106
>> +#define HV_REENLIGHTENMENT_ENABLE_BIT           (1u << 16)
>>  #define HV_X64_MSR_TSC_EMULATION_CONTROL        0x40000107
>>  #define HV_X64_MSR_TSC_EMULATION_STATUS         0x40000108
>>  
>> diff --git a/target/i386/machine.c b/target/i386/machine.c
>> index 7259fe6868c6..137604ddb898 100644
>> --- a/target/i386/machine.c
>> +++ b/target/i386/machine.c
>> @@ -883,11 +883,31 @@ static bool hyperv_reenlightenment_enable_needed(void *opaque)
>>          env->msr_hv_tsc_emulation_status != 0;
>>  }
>>  
>> +static int hyperv_reenlightenment_post_load(void *opaque, int version_id)
>> +{
>> +    X86CPU *cpu = opaque;
>> +    CPUX86State *env = &cpu->env;
>> +
>> +    /*
>> +     * KVM doesn't fully support re-enlightenment notifications so we need to
>> +     * make sure TSC frequency doesn't change upon migration.
>> +     */
>> +    if ((env->msr_hv_reenlightenment_control & HV_REENLIGHTENMENT_ENABLE_BIT) &&
>> +        !env->user_tsc_khz) {
>> +        error_report("Guest enabled re-enlightenment notifications, "
>> +                     "'tsc-frequency=' has to be specified");
>
> It's unusual to fail on the destination for a valid configuration but
> guest state;  wouldn't it be better to always insist on tsc-frequency if
> that hv feature is exposed; failing early before reeiving the state?
>

Doing so would make a number of currently existing configurations
invalid, even when re-enlightenment is not to be used by the
guest. AFAIR Windows without Hyper-V doesn't enable it. Generally, we
just advise people to 'enable all currently supported hyper-v
enlightenments' to make things easier so reenlightenment may end up
being added for no particular reason.

FWIW, v1 of this patch used a slightly different approach: I was
requiring kvm_arch_set_tsc_khz() to succeed. This is not exactly the
same as e.g. even when the destination host doesn't support TSC scaling,
it may happen to have the same TSC frequency.

-- 
Vitaly



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] i386: Make migration fail when Hyper-V reenlightenment was enabled but 'user_tsc_khz' is unset
  2021-03-29 12:46   ` Vitaly Kuznetsov
@ 2021-03-29 14:49     ` Dr. David Alan Gilbert
  2021-03-30 11:51       ` Vitaly Kuznetsov
  0 siblings, 1 reply; 5+ messages in thread
From: Dr. David Alan Gilbert @ 2021-03-29 14:49 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: Paolo Bonzini, Marcelo Tosatti, Richard Henderson, qemu-devel,
	Eduardo Habkost

* Vitaly Kuznetsov (vkuznets@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> 
> > * Vitaly Kuznetsov (vkuznets@redhat.com) wrote:
> >> KVM doesn't fully support Hyper-V reenlightenment notifications on
> >> migration. In particular, it doesn't support emulating TSC frequency
> >> of the source host by trapping all TSC accesses so unless TSC scaling
> >> is supported on the destination host and KVM_SET_TSC_KHZ succeeds, it
> >> is unsafe to proceed with migration.
> >> 
> >> KVM_SET_TSC_KHZ is called from two sites: kvm_arch_init_vcpu() and
> >> kvm_arch_put_registers(). The later (intentionally) doesn't propagate
> >> errors allowing migrations to succeed even when TSC scaling is not
> >> supported on the destination. This doesn't suit 're-enlightenment'
> >> use-case as we have to guarantee that TSC frequency stays constant.
> >> 
> >> Require 'tsc-frequency=' command line option to be specified for successful
> >> migration when re-enlightenment was enabled by the guest.
> >> 
> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> >> ---
> >> This patch is a successor of "[PATCH 3/3] i386: Make sure
> >> kvm_arch_set_tsc_khz() succeeds on migration when 'hv-reenlightenment'
> >> was exposed" taking a different approach suggested by Paolo.
> >> ---
> >>  docs/hyperv.txt                |  5 +++++
> >>  target/i386/kvm/hyperv-proto.h |  1 +
> >>  target/i386/machine.c          | 20 ++++++++++++++++++++
> >>  3 files changed, 26 insertions(+)
> >> 
> >> diff --git a/docs/hyperv.txt b/docs/hyperv.txt
> >> index 5df00da54fc4..e53c581f4586 100644
> >> --- a/docs/hyperv.txt
> >> +++ b/docs/hyperv.txt
> >> @@ -160,6 +160,11 @@ the hypervisor) until it is ready to switch to the new one. This, in conjunction
> >>  with hv-frequencies, allows Hyper-V on KVM to pass stable clocksource (Reference
> >>  TSC page) to its own guests.
> >>  
> >> +Note, KVM doesn't fully support re-enlightenment notifications and doesn't
> >> +emulate TSC accesses after migration so 'tsc-frequency=' CPU option also has to
> >> +be specified to make migration succeed. The destination host has to either have
> >> +the same TSC frequency or support TSC scaling CPU feature.
> >> +
> >>  Recommended: hv-frequencies
> >>  
> >>  3.16. hv-evmcs
> >> diff --git a/target/i386/kvm/hyperv-proto.h b/target/i386/kvm/hyperv-proto.h
> >> index 056a305be38c..e30d64b4ade4 100644
> >> --- a/target/i386/kvm/hyperv-proto.h
> >> +++ b/target/i386/kvm/hyperv-proto.h
> >> @@ -139,6 +139,7 @@
> >>   * Reenlightenment notification MSRs
> >>   */
> >>  #define HV_X64_MSR_REENLIGHTENMENT_CONTROL      0x40000106
> >> +#define HV_REENLIGHTENMENT_ENABLE_BIT           (1u << 16)
> >>  #define HV_X64_MSR_TSC_EMULATION_CONTROL        0x40000107
> >>  #define HV_X64_MSR_TSC_EMULATION_STATUS         0x40000108
> >>  
> >> diff --git a/target/i386/machine.c b/target/i386/machine.c
> >> index 7259fe6868c6..137604ddb898 100644
> >> --- a/target/i386/machine.c
> >> +++ b/target/i386/machine.c
> >> @@ -883,11 +883,31 @@ static bool hyperv_reenlightenment_enable_needed(void *opaque)
> >>          env->msr_hv_tsc_emulation_status != 0;
> >>  }
> >>  
> >> +static int hyperv_reenlightenment_post_load(void *opaque, int version_id)
> >> +{
> >> +    X86CPU *cpu = opaque;
> >> +    CPUX86State *env = &cpu->env;
> >> +
> >> +    /*
> >> +     * KVM doesn't fully support re-enlightenment notifications so we need to
> >> +     * make sure TSC frequency doesn't change upon migration.
> >> +     */
> >> +    if ((env->msr_hv_reenlightenment_control & HV_REENLIGHTENMENT_ENABLE_BIT) &&
> >> +        !env->user_tsc_khz) {
> >> +        error_report("Guest enabled re-enlightenment notifications, "
> >> +                     "'tsc-frequency=' has to be specified");
> >
> > It's unusual to fail on the destination for a valid configuration but
> > guest state;  wouldn't it be better to always insist on tsc-frequency if
> > that hv feature is exposed; failing early before reeiving the state?
> >
> 
> Doing so would make a number of currently existing configurations
> invalid, even when re-enlightenment is not to be used by the
> guest. AFAIR Windows without Hyper-V doesn't enable it. Generally, we
> just advise people to 'enable all currently supported hyper-v
> enlightenments' to make things easier so reenlightenment may end up
> being added for no particular reason.

Ouch, that's difficult - the problem with testing this late is that the
migration fails right at the end so it's an unpleasent surprise.

Could you disallow re-enlightenment without tsc-frequency on new machine
types?

Dave

> FWIW, v1 of this patch used a slightly different approach: I was
> requiring kvm_arch_set_tsc_khz() to succeed. This is not exactly the
> same as e.g. even when the destination host doesn't support TSC scaling,
> it may happen to have the same TSC frequency.
> 
> -- 
> Vitaly
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] i386: Make migration fail when Hyper-V reenlightenment was enabled but 'user_tsc_khz' is unset
  2021-03-29 14:49     ` Dr. David Alan Gilbert
@ 2021-03-30 11:51       ` Vitaly Kuznetsov
  0 siblings, 0 replies; 5+ messages in thread
From: Vitaly Kuznetsov @ 2021-03-30 11:51 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: Paolo Bonzini, Marcelo Tosatti, Richard Henderson, qemu-devel,
	Eduardo Habkost

"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Vitaly Kuznetsov (vkuznets@redhat.com) wrote:
>> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
>> 
>> > * Vitaly Kuznetsov (vkuznets@redhat.com) wrote:
>> >> KVM doesn't fully support Hyper-V reenlightenment notifications on
>> >> migration. In particular, it doesn't support emulating TSC frequency
>> >> of the source host by trapping all TSC accesses so unless TSC scaling
>> >> is supported on the destination host and KVM_SET_TSC_KHZ succeeds, it
>> >> is unsafe to proceed with migration.
>> >> 
>> >> KVM_SET_TSC_KHZ is called from two sites: kvm_arch_init_vcpu() and
>> >> kvm_arch_put_registers(). The later (intentionally) doesn't propagate
>> >> errors allowing migrations to succeed even when TSC scaling is not
>> >> supported on the destination. This doesn't suit 're-enlightenment'
>> >> use-case as we have to guarantee that TSC frequency stays constant.
>> >> 
>> >> Require 'tsc-frequency=' command line option to be specified for successful
>> >> migration when re-enlightenment was enabled by the guest.
>> >> 
>> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> >> ---
>> >> This patch is a successor of "[PATCH 3/3] i386: Make sure
>> >> kvm_arch_set_tsc_khz() succeeds on migration when 'hv-reenlightenment'
>> >> was exposed" taking a different approach suggested by Paolo.
>> >> ---
>> >>  docs/hyperv.txt                |  5 +++++
>> >>  target/i386/kvm/hyperv-proto.h |  1 +
>> >>  target/i386/machine.c          | 20 ++++++++++++++++++++
>> >>  3 files changed, 26 insertions(+)
>> >> 
>> >> diff --git a/docs/hyperv.txt b/docs/hyperv.txt
>> >> index 5df00da54fc4..e53c581f4586 100644
>> >> --- a/docs/hyperv.txt
>> >> +++ b/docs/hyperv.txt
>> >> @@ -160,6 +160,11 @@ the hypervisor) until it is ready to switch to the new one. This, in conjunction
>> >>  with hv-frequencies, allows Hyper-V on KVM to pass stable clocksource (Reference
>> >>  TSC page) to its own guests.
>> >>  
>> >> +Note, KVM doesn't fully support re-enlightenment notifications and doesn't
>> >> +emulate TSC accesses after migration so 'tsc-frequency=' CPU option also has to
>> >> +be specified to make migration succeed. The destination host has to either have
>> >> +the same TSC frequency or support TSC scaling CPU feature.
>> >> +
>> >>  Recommended: hv-frequencies
>> >>  
>> >>  3.16. hv-evmcs
>> >> diff --git a/target/i386/kvm/hyperv-proto.h b/target/i386/kvm/hyperv-proto.h
>> >> index 056a305be38c..e30d64b4ade4 100644
>> >> --- a/target/i386/kvm/hyperv-proto.h
>> >> +++ b/target/i386/kvm/hyperv-proto.h
>> >> @@ -139,6 +139,7 @@
>> >>   * Reenlightenment notification MSRs
>> >>   */
>> >>  #define HV_X64_MSR_REENLIGHTENMENT_CONTROL      0x40000106
>> >> +#define HV_REENLIGHTENMENT_ENABLE_BIT           (1u << 16)
>> >>  #define HV_X64_MSR_TSC_EMULATION_CONTROL        0x40000107
>> >>  #define HV_X64_MSR_TSC_EMULATION_STATUS         0x40000108
>> >>  
>> >> diff --git a/target/i386/machine.c b/target/i386/machine.c
>> >> index 7259fe6868c6..137604ddb898 100644
>> >> --- a/target/i386/machine.c
>> >> +++ b/target/i386/machine.c
>> >> @@ -883,11 +883,31 @@ static bool hyperv_reenlightenment_enable_needed(void *opaque)
>> >>          env->msr_hv_tsc_emulation_status != 0;
>> >>  }
>> >>  
>> >> +static int hyperv_reenlightenment_post_load(void *opaque, int version_id)
>> >> +{
>> >> +    X86CPU *cpu = opaque;
>> >> +    CPUX86State *env = &cpu->env;
>> >> +
>> >> +    /*
>> >> +     * KVM doesn't fully support re-enlightenment notifications so we need to
>> >> +     * make sure TSC frequency doesn't change upon migration.
>> >> +     */
>> >> +    if ((env->msr_hv_reenlightenment_control & HV_REENLIGHTENMENT_ENABLE_BIT) &&
>> >> +        !env->user_tsc_khz) {
>> >> +        error_report("Guest enabled re-enlightenment notifications, "
>> >> +                     "'tsc-frequency=' has to be specified");
>> >
>> > It's unusual to fail on the destination for a valid configuration but
>> > guest state;  wouldn't it be better to always insist on tsc-frequency if
>> > that hv feature is exposed; failing early before reeiving the state?
>> >
>> 
>> Doing so would make a number of currently existing configurations
>> invalid, even when re-enlightenment is not to be used by the
>> guest. AFAIR Windows without Hyper-V doesn't enable it. Generally, we
>> just advise people to 'enable all currently supported hyper-v
>> enlightenments' to make things easier so reenlightenment may end up
>> being added for no particular reason.
>
> Ouch, that's difficult - the problem with testing this late is that the
> migration fails right at the end so it's an unpleasent surprise.
>
> Could you disallow re-enlightenment without tsc-frequency on new machine
> types?
>

Will do. I'm not exactly sure if I should target 6.0 or 6.1 atm, let's
try the former first.

-- 
Vitaly



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-03-30 11:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-19 12:38 [PATCH v2] i386: Make migration fail when Hyper-V reenlightenment was enabled but 'user_tsc_khz' is unset Vitaly Kuznetsov
2021-03-29 12:21 ` Dr. David Alan Gilbert
2021-03-29 12:46   ` Vitaly Kuznetsov
2021-03-29 14:49     ` Dr. David Alan Gilbert
2021-03-30 11:51       ` 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).