Re: [Qemu-devel] [PATCH 1/1] target-i386: get/put MSR_TSC_AUX across reset and migration

[Paolo Bonzini]

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 <address@hidden>
> Signed-off-by: Amit Shah <address@hidden>
> CC: Paolo Bonzini <address@hidden>
> CC: Juan Quintela <address@hidden>
> CC: "Dr. David Alan Gilbert" <address@hidden>
> CC: Marcelo Tosatti <address@hidden>
> CC: Richard Henderson <address@hidden>
> CC: Eduardo Habkost <address@hidden>
> ---
>  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
>      }
>  };
>