Patchwork qemu-kvm bugfix for IA32_FEATURE_CONTROL

login
register
mail settings
Submitter Liu, Jinsong
Date Aug. 18, 2013, 6:23 p.m.
Message ID <DE8DF0795D48FD4CA783C40EC829233501338EA4@SHSMSX101.ccr.corp.intel.com>
Download mbox | patch
Permalink /patch/268048/
State New
Headers show

Comments

Liu, Jinsong - Aug. 18, 2013, 6:23 p.m.
From 1273f8b2e5464ec987facf9942fd3ccc0b69087e Mon Sep 17 00:00:00 2001
From: Liu Jinsong <jinsong.liu@intel.com>
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 <jinsong.liu@intel.com>
---
 target-i386/kvm.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)
Stefan Weil - Aug. 18, 2013, 6:37 p.m.
Am 18.08.2013 20:23, schrieb Liu, Jinsong:
> From 1273f8b2e5464ec987facf9942fd3ccc0b69087e Mon Sep 17 00:00:00 2001
> From: Liu Jinsong <jinsong.liu@intel.com>
> 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 <jinsong.liu@intel.com>
> ---
>  target-i386/kvm.c |   16 ++++++++++++++--
>  1 files changed, 14 insertions(+), 2 deletions(-)
>

Hello,

please check your patch with scripts/checkpatch.pl
before sending it to the list.

For boolean operands, the boolean or (||) is better than
the bitwise or (|).

Regards,
Stefan
Paolo Bonzini - Aug. 19, 2013, 9:22 a.m.
Il 18/08/2013 20:23, Liu, Jinsong ha scritto:
> From 1273f8b2e5464ec987facf9942fd3ccc0b69087e Mon Sep 17 00:00:00 2001
> From: Liu Jinsong <jinsong.liu@intel.com>
> 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 <jinsong.liu@intel.com>
> ---
>  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

Patch

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) {