Re: [Qemu-devel] [PATCH] qemu-kvm bugfix for IA32_FEATURE_CONTROL

[Paolo Bonzini]

Il 18/08/2013 20:23, Liu, Jinsong ha scritto:
> From 1273f8b2e5464ec987facf9942fd3ccc0b69087e Mon Sep 17 00:00:00 2001
> From: Liu Jinsong <address@hidden>
> Date: Mon, 19 Aug 2013 09:33:30 +0800
> Subject: [PATCH] qemu-kvm bugfix for IA32_FEATURE_CONTROL
> 
> This patch is to fix the bug https://bugs.launchpad.net/qemu-kvm/+bug/1207623
> 
> IA32_FEATURE_CONTROL is pointless if not expose VMX or SMX bits to
> cpuid.1.ecx of vcpu. Current qemu-kvm will error return when kvm_put_msrs
> or kvm_get_msrs.
> 
> Signed-off-by: Liu Jinsong <address@hidden>
> ---
>  target-i386/kvm.c |   16 ++++++++++++++--
>  1 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 84ac00a..7facbfe 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -65,6 +65,7 @@ static bool has_msr_star;
>  static bool has_msr_hsave_pa;
>  static bool has_msr_tsc_adjust;
>  static bool has_msr_tsc_deadline;
> +static bool has_msr_feature_control;
>  static bool has_msr_async_pf_en;
>  static bool has_msr_pv_eoi_en;
>  static bool has_msr_misc_enable;
> @@ -644,6 +645,11 @@ int kvm_arch_init_vcpu(CPUState *cs)
>  
>      qemu_add_vm_change_state_handler(cpu_update_state, env);
>  
> +    c = cpuid_find_entry(&cpuid_data.cpuid, 1, 0);
> +    if (c)
> +        has_msr_feature_control = !!(c->ecx & CPUID_EXT_VMX) |
> +                                  !!(c->ecx & CPUID_EXT_SMX);
> +
>      cpuid_data.cpuid.padding = 0;
>      r = kvm_vcpu_ioctl(cs, KVM_SET_CPUID2, &cpuid_data);
>      if (r) {
> @@ -1121,7 +1127,10 @@ 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 (has_msr_feature_control) {
> +            kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL,
> +                              env->msr_ia32_feature_control);
> +        }
>      }
>      if (env->mcg_cap) {
>          int i;
> @@ -1346,7 +1355,9 @@ 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 (has_msr_feature_control) {
> +        msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
> +    }
>  
>      if (!env->tsc_valid) {
>          msrs[n++].index = MSR_IA32_TSC;
> @@ -1447,6 +1458,7 @@ static int kvm_get_msrs(X86CPU *cpu)
>              break;
>          case MSR_IA32_FEATURE_CONTROL:
>              env->msr_ia32_feature_control = msrs[i].data;
> +            break;
>          default:
>              if (msrs[i].index >= MSR_MC0_CTL &&
>                  msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
> 

The patch looks good.  Please repost it with checkpatch.pl failures fixed.

Paolo