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

[Gleb Natapov]

On Sun, Jul 07, 2013 at 11:13:37PM +0800, Arthur Chunqi Li 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.
> 
So what happens if we migrate from qemu that does not have this patch
to qemu that does? Since msr_ia32_feature_control will not be migrated
it will not be set on the destination so destination will not be able to
use nested vmx. Since nested vmx is experimental it may be to early for
us to care about it though, and nested vmx does not work with migration
anyway.

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