Re: [Qemu-devel] [ [PATCH]] nVMX: Initialize IA32_FEATURE_CONTROL MSR in reset and migration

[Gleb Natapov]

On Mon, Jul 15, 2013 at 01:44:01PM +0800, Arthur Chunqi Li wrote:
> Hi Gleb and Paolo,
> What is the status of this patch since the relevant patch for KVM is
> accepted? These two patches must cooperate to fix the bug.
> 
Need some reviews from migration and machine type experts. Copying Juan
and Eduardo.

> Arthur
> 
> On Sun, Jul 7, 2013 at 11:13 PM, Arthur Chunqi Li <address@hidden> wrote:
> > The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
> > to clear this MSR when reset vCPU and keep the value of it when
> > migration. This patch add this feature.
> >
> > Signed-off-by: Arthur Chunqi Li <address@hidden>
> > ---
> >  target-i386/cpu.h     |    2 ++
> >  target-i386/kvm.c     |    4 ++++
> >  target-i386/machine.c |   22 ++++++++++++++++++++++
> >  3 files changed, 28 insertions(+)
> >
> > diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> > index 62e3547..a418e17 100644
> > --- a/target-i386/cpu.h
> > +++ b/target-i386/cpu.h
> > @@ -301,6 +301,7 @@
> >  #define MSR_IA32_APICBASE_BSP           (1<<8)
> >  #define MSR_IA32_APICBASE_ENABLE        (1<<11)
> >  #define MSR_IA32_APICBASE_BASE          (0xfffff<<12)
> > +#define MSR_IA32_FEATURE_CONTROL        0x0000003a
> >  #define MSR_TSC_ADJUST                  0x0000003b
> >  #define MSR_IA32_TSCDEADLINE            0x6e0
> >
> > @@ -813,6 +814,7 @@ typedef struct CPUX86State {
> >
> >      uint64_t mcg_status;
> >      uint64_t msr_ia32_misc_enable;
> > +    uint64_t msr_ia32_feature_control;
> >
> >      /* exception/interrupt handling */
> >      int error_code;
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index 39f4fbb..3cb2161 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> >          if (hyperv_vapic_recommended()) {
> >              kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
> >          }
> > +        kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, 
> > env->msr_ia32_feature_control);
> >      }
> >      if (env->mcg_cap) {
> >          int i;
> > @@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
> >      if (has_msr_misc_enable) {
> >          msrs[n++].index = MSR_IA32_MISC_ENABLE;
> >      }
> > +    msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
> >
> >      if (!env->tsc_valid) {
> >          msrs[n++].index = MSR_IA32_TSC;
> > @@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
> >          case MSR_IA32_MISC_ENABLE:
> >              env->msr_ia32_misc_enable = msrs[i].data;
> >              break;
> > +        case MSR_IA32_FEATURE_CONTROL:
> > +            env->msr_ia32_feature_control = msrs[i].data;
> >          default:
> >              if (msrs[i].index >= MSR_MC0_CTL &&
> >                  msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
> > diff --git a/target-i386/machine.c b/target-i386/machine.c
> > index 3659db9..94ca914 100644
> > --- a/target-i386/machine.c
> > +++ b/target-i386/machine.c
> > @@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
> >      return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
> >  }
> >
> > +static bool feature_control_needed(void *opaque)
> > +{
> > +    X86CPU *cpu = opaque;
> > +    CPUX86State *env = &cpu->env;
> > +
> > +    return env->msr_ia32_feature_control != 0;
> > +}
> > +
> >  static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> >      .name = "cpu/msr_ia32_misc_enable",
> >      .version_id = 1,
> > @@ -410,6 +418,17 @@ static const VMStateDescription 
> > vmstate_msr_ia32_misc_enable = {
> >      }
> >  };
> >
> > +static const VMStateDescription vmstate_msr_ia32_feature_control = {
> > +    .name = "cpu/msr_ia32_feature_control",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .minimum_version_id_old = 1,
> > +    .fields      = (VMStateField []) {
> > +        VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> >  const VMStateDescription vmstate_x86_cpu = {
> >      .name = "cpu",
> >      .version_id = 12,
> > @@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
> >          }, {
> >              .vmsd = &vmstate_msr_ia32_misc_enable,
> >              .needed = misc_enable_needed,
> > +        }, {
> > +            .vmsd = &vmstate_msr_ia32_feature_control,
> > +            .needed = feature_control_needed,
> >          } , {
> >              /* empty */
> >          }
> > --
> > 1.7.9.5
> >

--
                        Gleb.