| Message ID | 7e1d1d52d0599c8c3d3e043b5d96ff8732c91250.1442989653.git.amit.shah@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 > } > }; >
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!
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 --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 } };
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(+)