qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/1] target-i386: get/put MSR_TSC_AUX across reset and migration
@ 2015-09-23  6:27 Amit Shah
  2015-09-23  7:47 ` Paolo Bonzini
  2015-09-24 15:00 ` Eduardo Habkost
  0 siblings, 2 replies; 4+ messages in thread
From: Amit Shah @ 2015-09-23  6:27 UTC (permalink / raw)
  To: qemu list, kvm list
  Cc: Eduardo Habkost, Juan Quintela, Marcelo Tosatti,
	Dr. David Alan Gilbert, Amit Shah, Paolo Bonzini,
	Richard Henderson

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(+)

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
     }
 };
-- 
2.4.3

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

* Re: [Qemu-devel] [PATCH 1/1] target-i386: get/put MSR_TSC_AUX across reset and migration
  2015-09-23  6:27 [Qemu-devel] [PATCH 1/1] target-i386: get/put MSR_TSC_AUX across reset and migration Amit Shah
@ 2015-09-23  7:47 ` Paolo Bonzini
  2015-09-24 14:50   ` Eduardo Habkost
  2015-09-24 15:00 ` Eduardo Habkost
  1 sibling, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2015-09-23  7:47 UTC (permalink / raw)
  To: Amit Shah, qemu list, kvm list
  Cc: Richard Henderson, Marcelo Tosatti, Dr. David Alan Gilbert,
	Eduardo Habkost, Juan Quintela



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
>      }
>  };
> 

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

* Re: [Qemu-devel] [PATCH 1/1] target-i386: get/put MSR_TSC_AUX across reset and migration
  2015-09-23  7:47 ` Paolo Bonzini
@ 2015-09-24 14:50   ` Eduardo Habkost
  0 siblings, 0 replies; 4+ messages in thread
From: Eduardo Habkost @ 2015-09-24 14:50 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kvm list, Juan Quintela, Marcelo Tosatti, Dr. David Alan Gilbert,
	qemu list, Amit Shah, Richard Henderson

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

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

* Re: [Qemu-devel] [PATCH 1/1] target-i386: get/put MSR_TSC_AUX across reset and migration
  2015-09-23  6:27 [Qemu-devel] [PATCH 1/1] target-i386: get/put MSR_TSC_AUX across reset and migration Amit Shah
  2015-09-23  7:47 ` Paolo Bonzini
@ 2015-09-24 15:00 ` Eduardo Habkost
  1 sibling, 0 replies; 4+ messages in thread
From: Eduardo Habkost @ 2015-09-24 15:00 UTC (permalink / raw)
  To: Amit Shah
  Cc: kvm list, Juan Quintela, Marcelo Tosatti, Dr. David Alan Gilbert,
	qemu list, Paolo Bonzini, Richard Henderson

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!

-- 
Eduardo

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

end of thread, other threads:[~2015-09-24 15:00 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-23  6:27 [Qemu-devel] [PATCH 1/1] target-i386: get/put MSR_TSC_AUX across reset and migration Amit Shah
2015-09-23  7:47 ` Paolo Bonzini
2015-09-24 14:50   ` Eduardo Habkost
2015-09-24 15:00 ` Eduardo Habkost

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).