diff mbox

[1/1] target-i386: get/put MSR_TSC_AUX across reset and migration

Message ID 7e1d1d52d0599c8c3d3e043b5d96ff8732c91250.1442989653.git.amit.shah@redhat.com
State New
Headers show

Commit Message

Amit Shah Sept. 23, 2015, 6:27 a.m. UTC
There's one report of migration breaking due to missing MSR_TSC_AUX
save/restore.  Fix this by adding a new subsection that saves the state
of this MSR.

https://bugzilla.redhat.com/show_bug.cgi?id=1261797

Reported-by: Xiaoqing Wei <xwei@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
CC: Marcelo Tosatti <mtosatti@redhat.com>
CC: Richard Henderson <rth@twiddle.net>
CC: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/kvm.c     | 14 ++++++++++++++
 target-i386/machine.c | 20 ++++++++++++++++++++
 2 files changed, 34 insertions(+)

Comments

Paolo Bonzini Sept. 23, 2015, 7:47 a.m. UTC | #1
On 23/09/2015 08:27, Amit Shah wrote:
> There's one report of migration breaking due to missing MSR_TSC_AUX
> save/restore.  Fix this by adding a new subsection that saves the state
> of this MSR.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1261797

It turns out that the MSR is already saved/restored into the migration
stream!  However, the commit that introduced RDTSCP support (commit
1b05007, "target-i386: add RDTSCP support", 2009-09-19) was written for
TCG, and we ended up forgetting to fish the value out of KVM and send it
back in.

The KVM bits are okay.  Eduardo, can you undo the machine.c hunk or
should Amit send v2?

Paolo

> Reported-by: Xiaoqing Wei <xwei@redhat.com>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Juan Quintela <quintela@redhat.com>
> CC: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> CC: Marcelo Tosatti <mtosatti@redhat.com>
> CC: Richard Henderson <rth@twiddle.net>
> CC: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target-i386/kvm.c     | 14 ++++++++++++++
>  target-i386/machine.c | 20 ++++++++++++++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 7b0ba17..80d1a7e 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -67,6 +67,7 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>  
>  static bool has_msr_star;
>  static bool has_msr_hsave_pa;
> +static bool has_msr_tsc_aux;
>  static bool has_msr_tsc_adjust;
>  static bool has_msr_tsc_deadline;
>  static bool has_msr_feature_control;
> @@ -825,6 +826,10 @@ static int kvm_get_supported_msrs(KVMState *s)
>                      has_msr_hsave_pa = true;
>                      continue;
>                  }
> +                if (kvm_msr_list->indices[i] == MSR_TSC_AUX) {
> +                    has_msr_tsc_aux = true;
> +                    continue;
> +                }
>                  if (kvm_msr_list->indices[i] == MSR_TSC_ADJUST) {
>                      has_msr_tsc_adjust = true;
>                      continue;
> @@ -1299,6 +1304,9 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
>      if (has_msr_hsave_pa) {
>          kvm_msr_entry_set(&msrs[n++], MSR_VM_HSAVE_PA, env->vm_hsave);
>      }
> +    if (has_msr_tsc_aux) {
> +        kvm_msr_entry_set(&msrs[n++], MSR_TSC_AUX, env->tsc_aux);
> +    }
>      if (has_msr_tsc_adjust) {
>          kvm_msr_entry_set(&msrs[n++], MSR_TSC_ADJUST, env->tsc_adjust);
>      }
> @@ -1671,6 +1679,9 @@ static int kvm_get_msrs(X86CPU *cpu)
>      if (has_msr_hsave_pa) {
>          msrs[n++].index = MSR_VM_HSAVE_PA;
>      }
> +    if (has_msr_tsc_aux) {
> +        msrs[n++].index = MSR_TSC_AUX;
> +    }
>      if (has_msr_tsc_adjust) {
>          msrs[n++].index = MSR_TSC_ADJUST;
>      }
> @@ -1820,6 +1831,9 @@ static int kvm_get_msrs(X86CPU *cpu)
>          case MSR_IA32_TSC:
>              env->tsc = msrs[i].data;
>              break;
> +        case MSR_TSC_AUX:
> +            env->tsc_aux = msrs[i].data;
> +            break;
>          case MSR_TSC_ADJUST:
>              env->tsc_adjust = msrs[i].data;
>              break;
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index 9fa0563..116693d 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -453,6 +453,25 @@ static const VMStateDescription vmstate_fpop_ip_dp = {
>      }
>  };
>  
> +static bool tsc_aux_needed(void *opaque)
> +{
> +    X86CPU *cpu = opaque;
> +    CPUX86State *env = &cpu->env;
> +
> +    return env->tsc_aux != 0;
> +}
> +
> +static const VMStateDescription vmstate_msr_tsc_aux = {
> +    .name = "cpu/msr_tsc_aux",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = tsc_aux_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT64(env.tsc_aux, X86CPU),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static bool tsc_adjust_needed(void *opaque)
>  {
>      X86CPU *cpu = opaque;
> @@ -871,6 +890,7 @@ VMStateDescription vmstate_x86_cpu = {
>          &vmstate_msr_hyperv_crash,
>          &vmstate_avx512,
>          &vmstate_xss,
> +        &vmstate_msr_tsc_aux,
>          NULL
>      }
>  };
>
Eduardo Habkost Sept. 24, 2015, 2:50 p.m. UTC | #2
On Wed, Sep 23, 2015 at 09:47:43AM +0200, Paolo Bonzini wrote:
> 
> 
> On 23/09/2015 08:27, Amit Shah wrote:
> > There's one report of migration breaking due to missing MSR_TSC_AUX
> > save/restore.  Fix this by adding a new subsection that saves the state
> > of this MSR.
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1261797
> 
> It turns out that the MSR is already saved/restored into the migration
> stream!  However, the commit that introduced RDTSCP support (commit
> 1b05007, "target-i386: add RDTSCP support", 2009-09-19) was written for
> TCG, and we ended up forgetting to fish the value out of KVM and send it
> back in.
> 
> The KVM bits are okay.  Eduardo, can you undo the machine.c hunk or
> should Amit send v2?

I can remove the machine.c hunk manually when applying. Thanks!
Eduardo Habkost Sept. 24, 2015, 3 p.m. UTC | #3
On Wed, Sep 23, 2015 at 11:57:33AM +0530, Amit Shah wrote:
> There's one report of migration breaking due to missing MSR_TSC_AUX
> save/restore.  Fix this by adding a new subsection that saves the state
> of this MSR.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1261797
> 
> Reported-by: Xiaoqing Wei <xwei@redhat.com>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Juan Quintela <quintela@redhat.com>
> CC: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> CC: Marcelo Tosatti <mtosatti@redhat.com>
> CC: Richard Henderson <rth@twiddle.net>
> CC: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target-i386/kvm.c     | 14 ++++++++++++++
>  target-i386/machine.c | 20 ++++++++++++++++++++
>  2 files changed, 34 insertions(+)

For the target-i386/kvm.c hunk:
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

Applied to x86 tree without the machine.c hunk. Thanks!
diff mbox

Patch

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 7b0ba17..80d1a7e 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -67,6 +67,7 @@  const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 
 static bool has_msr_star;
 static bool has_msr_hsave_pa;
+static bool has_msr_tsc_aux;
 static bool has_msr_tsc_adjust;
 static bool has_msr_tsc_deadline;
 static bool has_msr_feature_control;
@@ -825,6 +826,10 @@  static int kvm_get_supported_msrs(KVMState *s)
                     has_msr_hsave_pa = true;
                     continue;
                 }
+                if (kvm_msr_list->indices[i] == MSR_TSC_AUX) {
+                    has_msr_tsc_aux = true;
+                    continue;
+                }
                 if (kvm_msr_list->indices[i] == MSR_TSC_ADJUST) {
                     has_msr_tsc_adjust = true;
                     continue;
@@ -1299,6 +1304,9 @@  static int kvm_put_msrs(X86CPU *cpu, int level)
     if (has_msr_hsave_pa) {
         kvm_msr_entry_set(&msrs[n++], MSR_VM_HSAVE_PA, env->vm_hsave);
     }
+    if (has_msr_tsc_aux) {
+        kvm_msr_entry_set(&msrs[n++], MSR_TSC_AUX, env->tsc_aux);
+    }
     if (has_msr_tsc_adjust) {
         kvm_msr_entry_set(&msrs[n++], MSR_TSC_ADJUST, env->tsc_adjust);
     }
@@ -1671,6 +1679,9 @@  static int kvm_get_msrs(X86CPU *cpu)
     if (has_msr_hsave_pa) {
         msrs[n++].index = MSR_VM_HSAVE_PA;
     }
+    if (has_msr_tsc_aux) {
+        msrs[n++].index = MSR_TSC_AUX;
+    }
     if (has_msr_tsc_adjust) {
         msrs[n++].index = MSR_TSC_ADJUST;
     }
@@ -1820,6 +1831,9 @@  static int kvm_get_msrs(X86CPU *cpu)
         case MSR_IA32_TSC:
             env->tsc = msrs[i].data;
             break;
+        case MSR_TSC_AUX:
+            env->tsc_aux = msrs[i].data;
+            break;
         case MSR_TSC_ADJUST:
             env->tsc_adjust = msrs[i].data;
             break;
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 9fa0563..116693d 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -453,6 +453,25 @@  static const VMStateDescription vmstate_fpop_ip_dp = {
     }
 };
 
+static bool tsc_aux_needed(void *opaque)
+{
+    X86CPU *cpu = opaque;
+    CPUX86State *env = &cpu->env;
+
+    return env->tsc_aux != 0;
+}
+
+static const VMStateDescription vmstate_msr_tsc_aux = {
+    .name = "cpu/msr_tsc_aux",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = tsc_aux_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64(env.tsc_aux, X86CPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static bool tsc_adjust_needed(void *opaque)
 {
     X86CPU *cpu = opaque;
@@ -871,6 +890,7 @@  VMStateDescription vmstate_x86_cpu = {
         &vmstate_msr_hyperv_crash,
         &vmstate_avx512,
         &vmstate_xss,
+        &vmstate_msr_tsc_aux,
         NULL
     }
 };