[Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

[-- Attachment #1: Type: text/plain, Size: 4323 bytes --]

>From 3a78adf8006ec6189bfe2f55f7ae213e75bf3815 Mon Sep 17 00:00:00 2001
From: Liu Jinsong <jinsong.liu@intel.com>
Date: Thu, 29 Dec 2011 05:28:12 +0800
Subject: [PATCH 2/2] Expose tsc deadline timer cpuid to guest

Depend on several factors:
1. Considering live migration, user enable/disable tsc deadline timer;
2. If guest use kvm apic and kvm emulate tsc deadline timer, expose it;
3. If in the future qemu support tsc deadline timer emulation,
   and guest use qemu apic, add cpuid exposing case then.

Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
---
 target-i386/cpu.h   |    2 ++
 target-i386/cpuid.c |    7 ++++++-
 target-i386/kvm.c   |   13 +++++++++++++
 3 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 177d8aa..f2d0ad5 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -399,6 +399,7 @@
 #define CPUID_EXT_X2APIC   (1 << 21)
 #define CPUID_EXT_MOVBE    (1 << 22)
 #define CPUID_EXT_POPCNT   (1 << 23)
+#define CPUID_EXT_TSC_DEADLINE_TIMER (1 << 24)
 #define CPUID_EXT_XSAVE    (1 << 26)
 #define CPUID_EXT_OSXSAVE  (1 << 27)
 #define CPUID_EXT_HYPERVISOR  (1 << 31)
@@ -693,6 +694,7 @@ typedef struct CPUX86State {
 
     uint64_t tsc;
     uint64_t tsc_deadline;
+    bool tsc_deadline_timer_enabled;
 
     uint64_t mcg_status;
     uint64_t msr_ia32_misc_enable;
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 0b3af90..fe749e0 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -48,7 +48,7 @@ static const char *ext_feature_name[] = {
     "fma", "cx16", "xtpr", "pdcm",
     NULL, NULL, "dca", "sse4.1|sse4_1",
     "sse4.2|sse4_2", "x2apic", "movbe", "popcnt",
-    NULL, "aes", "xsave", "osxsave",
+    "tsc_deadline", "aes", "xsave", "osxsave",
     "avx", NULL, NULL, "hypervisor",
 };
 static const char *ext2_feature_name[] = {
@@ -225,6 +225,7 @@ typedef struct x86_def_t {
     int model;
     int stepping;
     int tsc_khz;
+    bool tsc_deadline_timer_enabled;
     uint32_t features, ext_features, ext2_features, ext3_features;
     uint32_t kvm_features, svm_features;
     uint32_t xlevel;
@@ -742,6 +743,9 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
     x86_cpu_def->ext3_features &= ~minus_ext3_features;
     x86_cpu_def->kvm_features &= ~minus_kvm_features;
     x86_cpu_def->svm_features &= ~minus_svm_features;
+    /* Defaultly user don't against tsc_deadline_timer */
+    x86_cpu_def->tsc_deadline_timer_enabled =
+        !(minus_ext_features & CPUID_EXT_TSC_DEADLINE_TIMER);
     if (check_cpuid) {
         if (check_features_against_host(x86_cpu_def) && enforce_cpuid)
             goto error;
@@ -885,6 +889,7 @@ int cpu_x86_register (CPUX86State *env, const char *cpu_model)
     env->cpuid_ext4_features = def->ext4_features;
     env->cpuid_xlevel2 = def->xlevel2;
     env->tsc_khz = def->tsc_khz;
+    env->tsc_deadline_timer_enabled = def->tsc_deadline_timer_enabled;
     if (!kvm_enabled()) {
         env->cpuid_features &= TCG_FEATURES;
         env->cpuid_ext_features &= TCG_EXT_FEATURES;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index d50de90..79baf0b 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -370,6 +370,19 @@ int kvm_arch_init_vcpu(CPUState *env)
     i = env->cpuid_ext_features & CPUID_EXT_HYPERVISOR;
     env->cpuid_ext_features &= kvm_arch_get_supported_cpuid(s, 1, 0, R_ECX);
     env->cpuid_ext_features |= i;
+    /*
+     * 1. Considering live migration, user enable/disable tsc deadline timer;
+     * 2. If guest use kvm apic and kvm emulate tsc deadline timer, expose it;
+     * 3. If in the future qemu support tsc deadline timer emulation,
+     *    and guest use qemu apic, add cpuid exposing case then.
+     */
+    env->cpuid_ext_features &= ~CPUID_EXT_TSC_DEADLINE_TIMER;
+    if (env->tsc_deadline_timer_enabled) {
+        if (kvm_irqchip_in_kernel() &&
+            kvm_check_extension(s, KVM_CAP_TSC_DEADLINE_TIMER)) {
+            env->cpuid_ext_features |= CPUID_EXT_TSC_DEADLINE_TIMER;
+        }
+    }
 
     env->cpuid_ext2_features &= kvm_arch_get_supported_cpuid(s, 0x80000001,
                                                              0, R_EDX);
-- 
1.6.5.6

[-- Attachment #2: 0002-Expose-tsc-deadline-timer-cpuid-to-guest.patch --]
[-- Type: application/octet-stream, Size: 4220 bytes --]

From 3a78adf8006ec6189bfe2f55f7ae213e75bf3815 Mon Sep 17 00:00:00 2001
From: Liu Jinsong <jinsong.liu@intel.com>
Date: Thu, 29 Dec 2011 05:28:12 +0800
Subject: [PATCH 2/2] Expose tsc deadline timer cpuid to guest

Depend on several factors:
1. Considering live migration, user enable/disable tsc deadline timer;
2. If guest use kvm apic and kvm emulate tsc deadline timer, expose it;
3. If in the future qemu support tsc deadline timer emulation,
   and guest use qemu apic, add cpuid exposing case then.

Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
---
 target-i386/cpu.h   |    2 ++
 target-i386/cpuid.c |    7 ++++++-
 target-i386/kvm.c   |   13 +++++++++++++
 3 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 177d8aa..f2d0ad5 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -399,6 +399,7 @@
 #define CPUID_EXT_X2APIC   (1 << 21)
 #define CPUID_EXT_MOVBE    (1 << 22)
 #define CPUID_EXT_POPCNT   (1 << 23)
+#define CPUID_EXT_TSC_DEADLINE_TIMER (1 << 24)
 #define CPUID_EXT_XSAVE    (1 << 26)
 #define CPUID_EXT_OSXSAVE  (1 << 27)
 #define CPUID_EXT_HYPERVISOR  (1 << 31)
@@ -693,6 +694,7 @@ typedef struct CPUX86State {
 
     uint64_t tsc;
     uint64_t tsc_deadline;
+    bool tsc_deadline_timer_enabled;
 
     uint64_t mcg_status;
     uint64_t msr_ia32_misc_enable;
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 0b3af90..fe749e0 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -48,7 +48,7 @@ static const char *ext_feature_name[] = {
     "fma", "cx16", "xtpr", "pdcm",
     NULL, NULL, "dca", "sse4.1|sse4_1",
     "sse4.2|sse4_2", "x2apic", "movbe", "popcnt",
-    NULL, "aes", "xsave", "osxsave",
+    "tsc_deadline", "aes", "xsave", "osxsave",
     "avx", NULL, NULL, "hypervisor",
 };
 static const char *ext2_feature_name[] = {
@@ -225,6 +225,7 @@ typedef struct x86_def_t {
     int model;
     int stepping;
     int tsc_khz;
+    bool tsc_deadline_timer_enabled;
     uint32_t features, ext_features, ext2_features, ext3_features;
     uint32_t kvm_features, svm_features;
     uint32_t xlevel;
@@ -742,6 +743,9 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
     x86_cpu_def->ext3_features &= ~minus_ext3_features;
     x86_cpu_def->kvm_features &= ~minus_kvm_features;
     x86_cpu_def->svm_features &= ~minus_svm_features;
+    /* Defaultly user don't against tsc_deadline_timer */
+    x86_cpu_def->tsc_deadline_timer_enabled =
+        !(minus_ext_features & CPUID_EXT_TSC_DEADLINE_TIMER);
     if (check_cpuid) {
         if (check_features_against_host(x86_cpu_def) && enforce_cpuid)
             goto error;
@@ -885,6 +889,7 @@ int cpu_x86_register (CPUX86State *env, const char *cpu_model)
     env->cpuid_ext4_features = def->ext4_features;
     env->cpuid_xlevel2 = def->xlevel2;
     env->tsc_khz = def->tsc_khz;
+    env->tsc_deadline_timer_enabled = def->tsc_deadline_timer_enabled;
     if (!kvm_enabled()) {
         env->cpuid_features &= TCG_FEATURES;
         env->cpuid_ext_features &= TCG_EXT_FEATURES;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index d50de90..79baf0b 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -370,6 +370,19 @@ int kvm_arch_init_vcpu(CPUState *env)
     i = env->cpuid_ext_features & CPUID_EXT_HYPERVISOR;
     env->cpuid_ext_features &= kvm_arch_get_supported_cpuid(s, 1, 0, R_ECX);
     env->cpuid_ext_features |= i;
+    /*
+     * 1. Considering live migration, user enable/disable tsc deadline timer;
+     * 2. If guest use kvm apic and kvm emulate tsc deadline timer, expose it;
+     * 3. If in the future qemu support tsc deadline timer emulation,
+     *    and guest use qemu apic, add cpuid exposing case then.
+     */
+    env->cpuid_ext_features &= ~CPUID_EXT_TSC_DEADLINE_TIMER;
+    if (env->tsc_deadline_timer_enabled) {
+        if (kvm_irqchip_in_kernel() &&
+            kvm_check_extension(s, KVM_CAP_TSC_DEADLINE_TIMER)) {
+            env->cpuid_ext_features |= CPUID_EXT_TSC_DEADLINE_TIMER;
+        }
+    }
 
     env->cpuid_ext2_features &= kvm_arch_get_supported_cpuid(s, 0x80000001,
                                                              0, R_EDX);
-- 
1.6.5.6

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 5639 bytes --]

On 2011-12-28 19:55, Liu, Jinsong wrote:
>>From 3a78adf8006ec6189bfe2f55f7ae213e75bf3815 Mon Sep 17 00:00:00 2001
> From: Liu Jinsong <jinsong.liu@intel.com>
> Date: Thu, 29 Dec 2011 05:28:12 +0800
> Subject: [PATCH 2/2] Expose tsc deadline timer cpuid to guest
> 
> Depend on several factors:
> 1. Considering live migration, user enable/disable tsc deadline timer;
> 2. If guest use kvm apic and kvm emulate tsc deadline timer, expose it;
> 3. If in the future qemu support tsc deadline timer emulation,
>    and guest use qemu apic, add cpuid exposing case then.

This requires some logic change and then rewording:

- enable TSC deadline timer support by default if in-kernel irqchip is
  used
- disable it on user request via a cpu feature flag
- disable it for older machine types (see below) by default

TSC deadline timer emulation in user space is a different story to be
told once we have a patch for it.

> 
> Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
> ---
>  target-i386/cpu.h   |    2 ++
>  target-i386/cpuid.c |    7 ++++++-
>  target-i386/kvm.c   |   13 +++++++++++++
>  3 files changed, 21 insertions(+), 1 deletions(-)
> 
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 177d8aa..f2d0ad5 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -399,6 +399,7 @@
>  #define CPUID_EXT_X2APIC   (1 << 21)
>  #define CPUID_EXT_MOVBE    (1 << 22)
>  #define CPUID_EXT_POPCNT   (1 << 23)
> +#define CPUID_EXT_TSC_DEADLINE_TIMER (1 << 24)
>  #define CPUID_EXT_XSAVE    (1 << 26)
>  #define CPUID_EXT_OSXSAVE  (1 << 27)
>  #define CPUID_EXT_HYPERVISOR  (1 << 31)
> @@ -693,6 +694,7 @@ typedef struct CPUX86State {
>  
>      uint64_t tsc;
>      uint64_t tsc_deadline;
> +    bool tsc_deadline_timer_enabled;
>  
>      uint64_t mcg_status;
>      uint64_t msr_ia32_misc_enable;
> diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
> index 0b3af90..fe749e0 100644
> --- a/target-i386/cpuid.c
> +++ b/target-i386/cpuid.c
> @@ -48,7 +48,7 @@ static const char *ext_feature_name[] = {
>      "fma", "cx16", "xtpr", "pdcm",
>      NULL, NULL, "dca", "sse4.1|sse4_1",
>      "sse4.2|sse4_2", "x2apic", "movbe", "popcnt",
> -    NULL, "aes", "xsave", "osxsave",
> +    "tsc_deadline", "aes", "xsave", "osxsave",
>      "avx", NULL, NULL, "hypervisor",
>  };
>  static const char *ext2_feature_name[] = {
> @@ -225,6 +225,7 @@ typedef struct x86_def_t {
>      int model;
>      int stepping;
>      int tsc_khz;
> +    bool tsc_deadline_timer_enabled;
>      uint32_t features, ext_features, ext2_features, ext3_features;
>      uint32_t kvm_features, svm_features;
>      uint32_t xlevel;
> @@ -742,6 +743,9 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
>      x86_cpu_def->ext3_features &= ~minus_ext3_features;
>      x86_cpu_def->kvm_features &= ~minus_kvm_features;
>      x86_cpu_def->svm_features &= ~minus_svm_features;
> +    /* Defaultly user don't against tsc_deadline_timer */
> +    x86_cpu_def->tsc_deadline_timer_enabled =
> +        !(minus_ext_features & CPUID_EXT_TSC_DEADLINE_TIMER);
>      if (check_cpuid) {
>          if (check_features_against_host(x86_cpu_def) && enforce_cpuid)
>              goto error;
> @@ -885,6 +889,7 @@ int cpu_x86_register (CPUX86State *env, const char *cpu_model)
>      env->cpuid_ext4_features = def->ext4_features;
>      env->cpuid_xlevel2 = def->xlevel2;
>      env->tsc_khz = def->tsc_khz;
> +    env->tsc_deadline_timer_enabled = def->tsc_deadline_timer_enabled;
>      if (!kvm_enabled()) {
>          env->cpuid_features &= TCG_FEATURES;
>          env->cpuid_ext_features &= TCG_EXT_FEATURES;
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index d50de90..79baf0b 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -370,6 +370,19 @@ int kvm_arch_init_vcpu(CPUState *env)
>      i = env->cpuid_ext_features & CPUID_EXT_HYPERVISOR;
>      env->cpuid_ext_features &= kvm_arch_get_supported_cpuid(s, 1, 0, R_ECX);
>      env->cpuid_ext_features |= i;
> +    /*
> +     * 1. Considering live migration, user enable/disable tsc deadline timer;
> +     * 2. If guest use kvm apic and kvm emulate tsc deadline timer, expose it;
> +     * 3. If in the future qemu support tsc deadline timer emulation,
> +     *    and guest use qemu apic, add cpuid exposing case then.
> +     */

See above. Also, I don't think this comment applies very well to this
function.

> +    env->cpuid_ext_features &= ~CPUID_EXT_TSC_DEADLINE_TIMER;

Can that feature possibly be set in cpuid_ext_features? I thought the
kernel now refrains from this.

> +    if (env->tsc_deadline_timer_enabled) {
> +        if (kvm_irqchip_in_kernel() &&
> +            kvm_check_extension(s, KVM_CAP_TSC_DEADLINE_TIMER)) {
> +            env->cpuid_ext_features |= CPUID_EXT_TSC_DEADLINE_TIMER;
> +        }
> +    }
>  
>      env->cpuid_ext2_features &= kvm_arch_get_supported_cpuid(s, 0x80000001,
>                                                               0, R_EDX);

Sorry, it remains bogus to expose the tsc deadline timer feature on
machines < pc-1.1. That's just like we introduced kvmclock only to
pc-0.14 onward. The reason is that guest OSes so far running on qemu-1.0
or older without deadline timer support must not find that feature when
being migrated to a host with qemu-1.1 in pc-1.0 compat mode. Yes, the
user can explicitly disable it, but that is not the idea of legacy
machine models. They should provide the very same environment that older
qemu versions offered.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

> This requires some logic change and then rewording:
> 
> - enable TSC deadline timer support by default if in-kernel irqchip is
>   used
> - disable it on user request via a cpu feature flag

Yes, the logic has been implemented by the former patch as:
+    if (env->tsc_deadline_timer_enabled) {	// user control, default is to authorize tsc deadline timer feature
+        if (kvm_irqchip_in_kernel() &&		// in-kerenl irqchip is used
+            kvm_check_extension(s, KVM_CAP_TSC_DEADLINE_TIMER)) {
+            env->cpuid_ext_features |= CPUID_EXT_TSC_DEADLINE_TIMER;
+        }
+    }

> - disable it for older machine types (see below) by default
> 
> TSC deadline timer emulation in user space is a different story to be
> told once we have a patch for it.
> 
>> 
>> Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com> ---
>>  target-i386/cpu.h   |    2 ++
>>  target-i386/cpuid.c |    7 ++++++-
>>  target-i386/kvm.c   |   13 +++++++++++++
>>  3 files changed, 21 insertions(+), 1 deletions(-)
>> 
>> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
>> index 177d8aa..f2d0ad5 100644
>> --- a/target-i386/cpu.h
>> +++ b/target-i386/cpu.h
>> @@ -399,6 +399,7 @@
>>  #define CPUID_EXT_X2APIC   (1 << 21)
>>  #define CPUID_EXT_MOVBE    (1 << 22)
>>  #define CPUID_EXT_POPCNT   (1 << 23)
>> +#define CPUID_EXT_TSC_DEADLINE_TIMER (1 << 24)
>>  #define CPUID_EXT_XSAVE    (1 << 26)
>>  #define CPUID_EXT_OSXSAVE  (1 << 27)
>>  #define CPUID_EXT_HYPERVISOR  (1 << 31)
>> @@ -693,6 +694,7 @@ typedef struct CPUX86State {
>> 
>>      uint64_t tsc;
>>      uint64_t tsc_deadline;
>> +    bool tsc_deadline_timer_enabled;
>> 
>>      uint64_t mcg_status;
>>      uint64_t msr_ia32_misc_enable;
>> diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
>> index 0b3af90..fe749e0 100644
>> --- a/target-i386/cpuid.c
>> +++ b/target-i386/cpuid.c
>> @@ -48,7 +48,7 @@ static const char *ext_feature_name[] = {
>>      "fma", "cx16", "xtpr", "pdcm",
>>      NULL, NULL, "dca", "sse4.1|sse4_1",
>>      "sse4.2|sse4_2", "x2apic", "movbe", "popcnt",
>> -    NULL, "aes", "xsave", "osxsave",
>> +    "tsc_deadline", "aes", "xsave", "osxsave",
>>      "avx", NULL, NULL, "hypervisor",
>>  };
>>  static const char *ext2_feature_name[] = {
>> @@ -225,6 +225,7 @@ typedef struct x86_def_t {
>>      int model;
>>      int stepping;
>>      int tsc_khz;
>> +    bool tsc_deadline_timer_enabled;
>>      uint32_t features, ext_features, ext2_features, ext3_features;
>>      uint32_t kvm_features, svm_features;
>>      uint32_t xlevel;
>> @@ -742,6 +743,9 @@ static int cpu_x86_find_by_name(x86_def_t
>>      *x86_cpu_def, const char *cpu_model) x86_cpu_def->ext3_features
>>      &= ~minus_ext3_features; x86_cpu_def->kvm_features &=
>>      ~minus_kvm_features; x86_cpu_def->svm_features &=
>> ~minus_svm_features; +    /* Defaultly user don't against
>> tsc_deadline_timer */ +    x86_cpu_def->tsc_deadline_timer_enabled =
>> +        !(minus_ext_features & CPUID_EXT_TSC_DEADLINE_TIMER);     
>>          if (check_cpuid) { if
>> (check_features_against_host(x86_cpu_def) && enforce_cpuid)         
>>      goto error; @@ -885,6 +889,7 @@ int cpu_x86_register
>>      (CPUX86State *env, const char *cpu_model)
>>      env->cpuid_ext4_features = def->ext4_features;
>> env->cpuid_xlevel2 = def->xlevel2; env->tsc_khz = def->tsc_khz; +   
>>          env->tsc_deadline_timer_enabled =
>>          def->tsc_deadline_timer_enabled;      if (!kvm_enabled()) {
>> env->cpuid_features &= TCG_FEATURES; env->cpuid_ext_features &=
>> TCG_EXT_FEATURES;  
>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>> index d50de90..79baf0b 100644
>> --- a/target-i386/kvm.c
>> +++ b/target-i386/kvm.c
>> @@ -370,6 +370,19 @@ int kvm_arch_init_vcpu(CPUState *env)
>>      i = env->cpuid_ext_features & CPUID_EXT_HYPERVISOR;
>>      env->cpuid_ext_features &= kvm_arch_get_supported_cpuid(s, 1,
>>      0, R_ECX); env->cpuid_ext_features |= i;
>> +    /*
>> +     * 1. Considering live migration, user enable/disable tsc
>> deadline timer; +     * 2. If guest use kvm apic and kvm emulate tsc
>> deadline timer, expose it; +     * 3. If in the future qemu support
>> tsc deadline timer emulation, +     *    and guest use qemu apic,
>> add cpuid exposing case then. +     */
> 
> See above. Also, I don't think this comment applies very well to this
> function.

Yes, the comment is indeed ambiguous. Would elaborate more clear.

> 
>> +    env->cpuid_ext_features &= ~CPUID_EXT_TSC_DEADLINE_TIMER;
> 
> Can that feature possibly be set in cpuid_ext_features? I thought the
> kernel now refrains from this.

Yes, it's possible. Kernel didn't refrain it, just let qemu to make decision.

> 
>> +    if (env->tsc_deadline_timer_enabled) {
>> +        if (kvm_irqchip_in_kernel() &&
>> +            kvm_check_extension(s, KVM_CAP_TSC_DEADLINE_TIMER)) {
>> +            env->cpuid_ext_features |=
>> CPUID_EXT_TSC_DEADLINE_TIMER; +        } +    }
>> 
>>      env->cpuid_ext2_features &= kvm_arch_get_supported_cpuid(s,
>>                                                              
>> 0x80000001, 0, R_EDX); 
> 
> Sorry, it remains bogus to expose the tsc deadline timer feature on
> machines < pc-1.1. That's just like we introduced kvmclock only to
> pc-0.14 onward. The reason is that guest OSes so far running on
> qemu-1.0 or older without deadline timer support must not find that
> feature when being migrated to a host with qemu-1.1 in pc-1.0 compat
> mode. Yes, the user can explicitly disable it, but that is not the
> idea of legacy machine models. They should provide the very same
> environment that older qemu versions offered.
> 

Not quite clear about this point.
Per my understanding, if a kvm guest running on an older qemu without tsc deadline timer support, 
then after migrate, the guest would still cannot find tsc deadline feature, no matter older or newer host/qemu/pc-xx it migrate to.

Thanks,
Jinsong

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1066 bytes --]

On 2012-01-05 18:07, Liu, Jinsong wrote:
>> Sorry, it remains bogus to expose the tsc deadline timer feature on
>> machines < pc-1.1. That's just like we introduced kvmclock only to
>> pc-0.14 onward. The reason is that guest OSes so far running on
>> qemu-1.0 or older without deadline timer support must not find that
>> feature when being migrated to a host with qemu-1.1 in pc-1.0 compat
>> mode. Yes, the user can explicitly disable it, but that is not the
>> idea of legacy machine models. They should provide the very same
>> environment that older qemu versions offered.
>>
> 
> Not quite clear about this point.
> Per my understanding, if a kvm guest running on an older qemu without tsc deadline timer support, 
> then after migrate, the guest would still cannot find tsc deadline feature, no matter older or newer host/qemu/pc-xx it migrate to.

What should prevent this? The feature flags are not part of the vmstate.
They are part of the vm configuration which is not migrated but defined
by starting qemu on the target host.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

Jan Kiszka wrote:
> On 2012-01-05 18:07, Liu, Jinsong wrote:
>>> Sorry, it remains bogus to expose the tsc deadline timer feature on
>>> machines < pc-1.1. That's just like we introduced kvmclock only to
>>> pc-0.14 onward. The reason is that guest OSes so far running on
>>> qemu-1.0 or older without deadline timer support must not find that
>>> feature when being migrated to a host with qemu-1.1 in pc-1.0 compat
>>> mode. Yes, the user can explicitly disable it, but that is not the
>>> idea of legacy machine models. They should provide the very same
>>> environment that older qemu versions offered.
>>> 
>> 
>> Not quite clear about this point.
>> Per my understanding, if a kvm guest running on an older qemu
>> without tsc deadline timer support, 
>> then after migrate, the guest would still cannot find tsc deadline
>> feature, no matter older or newer host/qemu/pc-xx it migrate to. 
> 
> What should prevent this? The feature flags are not part of the
> vmstate. They are part of the vm configuration which is not migrated
> but defined by starting qemu on the target host.
> 

Thanks! understand this point ("They are part of the vm configuration which is not migrated but defined by starting qemu on the target host").

But kvmclock example still cannot satisfy the purpose "guest running on old qemu/pc-0.13 without kvmclock support must not find kvmclock feature when being migrated to a host with new qemu/pc-0.13 compat mode". After migration, guest can possibily find kvmclock feature CPUID.0x40000001.KVM_FEATURE_CLOCKSOURCE:
pc_init1(..., kvmclock_enabled) 
{
    pc_cpus_init(cpu_model);    // the point to decide and expose cpuid features to guest

    if (kvmclock_enabled) {        // the difference point between pc-0.13 vs. pc-0.14, related nothing to cpuid features.
        kvmclock_create();
    }
}

Seems currently there is no good way to satisfy "guest running on old qemu/pc-xx without feature A support must not find feature A when being migrated to a host with new qemu/pc-xx compat mode", i.e. considering
* if running with '-cpu host' then migrate;
* each time we add a new cpuid feature it need add one or more new machine model? is it necessary to bind pc-xx with cpuid feature?
* logically cpuid features should better be controlled by cpu model, not by machine model.

Regards,
Jinsong

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2877 bytes --]

On 2012-01-07 19:23, Liu, Jinsong wrote:
> Jan Kiszka wrote:
>> On 2012-01-05 18:07, Liu, Jinsong wrote:
>>>> Sorry, it remains bogus to expose the tsc deadline timer feature on
>>>> machines < pc-1.1. That's just like we introduced kvmclock only to
>>>> pc-0.14 onward. The reason is that guest OSes so far running on
>>>> qemu-1.0 or older without deadline timer support must not find that
>>>> feature when being migrated to a host with qemu-1.1 in pc-1.0 compat
>>>> mode. Yes, the user can explicitly disable it, but that is not the
>>>> idea of legacy machine models. They should provide the very same
>>>> environment that older qemu versions offered.
>>>>
>>>
>>> Not quite clear about this point.
>>> Per my understanding, if a kvm guest running on an older qemu
>>> without tsc deadline timer support, 
>>> then after migrate, the guest would still cannot find tsc deadline
>>> feature, no matter older or newer host/qemu/pc-xx it migrate to. 
>>
>> What should prevent this? The feature flags are not part of the
>> vmstate. They are part of the vm configuration which is not migrated
>> but defined by starting qemu on the target host.
>>
> 
> Thanks! understand this point ("They are part of the vm configuration which is not migrated but defined by starting qemu on the target host").
> 
> But kvmclock example still cannot satisfy the purpose "guest running on old qemu/pc-0.13 without kvmclock support must not find kvmclock feature when being migrated to a host with new qemu/pc-0.13 compat mode". After migration, guest can possibily find kvmclock feature CPUID.0x40000001.KVM_FEATURE_CLOCKSOURCE:
> pc_init1(..., kvmclock_enabled) 
> {
>     pc_cpus_init(cpu_model);    // the point to decide and expose cpuid features to guest
> 
>     if (kvmclock_enabled) {        // the difference point between pc-0.13 vs. pc-0.14, related nothing to cpuid features.
>         kvmclock_create();
>     }
> }

Right, not a perfect example: the cpuid feature is not influenced by
this mechanism, only the fact if a kvmclock device (for save/restore)
should be created. I guess we ignored this back then, only focusing on
the more obvious issue of the addition device.

> 
> Seems currently there is no good way to satisfy "guest running on old qemu/pc-xx without feature A support must not find feature A when being migrated to a host with new qemu/pc-xx compat mode", i.e. considering
> * if running with '-cpu host' then migrate;
> * each time we add a new cpuid feature it need add one or more new machine model? is it necessary to bind pc-xx with cpuid feature?
> * logically cpuid features should better be controlled by cpu model, not by machine model.

The compatibility machines define the possible cpu models. If I select
pc-0.14, e.g. -cpu kvm64 should not give me features that 0.14 was not
exposing.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

Jan Kiszka wrote:
> On 2012-01-07 19:23, Liu, Jinsong wrote:
>> Jan Kiszka wrote:
>>> On 2012-01-05 18:07, Liu, Jinsong wrote:
>>>>> Sorry, it remains bogus to expose the tsc deadline timer feature
>>>>> on machines < pc-1.1. That's just like we introduced kvmclock
>>>>> only to pc-0.14 onward. The reason is that guest OSes so far
>>>>> running on qemu-1.0 or older without deadline timer support must
>>>>> not find that feature when being migrated to a host with qemu-1.1
>>>>> in pc-1.0 compat mode. Yes, the user can explicitly disable it,
>>>>> but that is not the idea of legacy machine models. They should
>>>>> provide the very same environment that older qemu versions
>>>>> offered. 
>>>>> 
>>>> 
>>>> Not quite clear about this point.
>>>> Per my understanding, if a kvm guest running on an older qemu
>>>> without tsc deadline timer support,
>>>> then after migrate, the guest would still cannot find tsc deadline
>>>> feature, no matter older or newer host/qemu/pc-xx it migrate to.
>>> 
>>> What should prevent this? The feature flags are not part of the
>>> vmstate. They are part of the vm configuration which is not migrated
>>> but defined by starting qemu on the target host.
>>> 
>> 
>> Thanks! understand this point ("They are part of the vm
>> configuration which is not migrated but defined by starting qemu on
>> the target host").  
>> 
>> But kvmclock example still cannot satisfy the purpose "guest running
>> on old qemu/pc-0.13 without kvmclock support must not find kvmclock
>> feature when being migrated to a host with new qemu/pc-0.13 compat
>>     mode". After migration, guest can possibily find kvmclock
>> feature CPUID.0x40000001.KVM_FEATURE_CLOCKSOURCE: pc_init1(...,
>> kvmclock_enabled) { pc_cpus_init(cpu_model);    // the point to
>> decide and expose cpuid features to guest   
>> 
>>     if (kvmclock_enabled) {        // the difference point between
>>     pc-0.13 vs. pc-0.14, related nothing to cpuid features.        
>> kvmclock_create(); } }
> 
> Right, not a perfect example: the cpuid feature is not influenced by
> this mechanism, only the fact if a kvmclock device (for save/restore)
> should be created. I guess we ignored this back then, only focusing on
> the more obvious issue of the addition device.
> 
>> 
>> Seems currently there is no good way to satisfy "guest running on
>> old qemu/pc-xx without feature A support must not find feature A
>> when being migrated to a host with new qemu/pc-xx compat mode", i.e.
>> considering   
>> * if running with '-cpu host' then migrate;
>> * each time we add a new cpuid feature it need add one or more new
>> machine model? is it necessary to bind pc-xx with cpuid feature? 
>> * logically cpuid features should better be controlled by cpu model,
>> not by machine model. 
> 
> The compatibility machines define the possible cpu models. If I select

How does machine define possible cpu models?
cpu model defined by qemu option '-cpu ...', while machine model defined by '-machine ...'

> pc-0.14, e.g. -cpu kvm64 should not give me features that 0.14 was not
> exposing.
> 

in such case, it's '-cpu kvm64' who take effect to decide what cpuid features would exposed to guest, not '-machine pc-0.14'.

IMO, what our patch need to do is to expose a cpuid feature to guest (CPUID.01H:ECX.TSC_Deadline[bit 24]), it decided by cpu model, not machine model:
pc_init1(..., cpu_model, ...)
{
    pc_cpus_init(cpu_model);   // this is the whole logic exposing cpuid features to guest
    ...
}

Do I misunderstanding something?

Thanks,
Jinsong

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Jan Kiszka]

On 2012-02-27 17:05, Liu, Jinsong wrote:
> Jan Kiszka wrote:
>> On 2012-01-07 19:23, Liu, Jinsong wrote:
>>> Jan Kiszka wrote:
>>>> On 2012-01-05 18:07, Liu, Jinsong wrote:
>>>>>> Sorry, it remains bogus to expose the tsc deadline timer feature
>>>>>> on machines < pc-1.1. That's just like we introduced kvmclock
>>>>>> only to pc-0.14 onward. The reason is that guest OSes so far
>>>>>> running on qemu-1.0 or older without deadline timer support must
>>>>>> not find that feature when being migrated to a host with qemu-1.1
>>>>>> in pc-1.0 compat mode. Yes, the user can explicitly disable it,
>>>>>> but that is not the idea of legacy machine models. They should
>>>>>> provide the very same environment that older qemu versions
>>>>>> offered. 
>>>>>>
>>>>>
>>>>> Not quite clear about this point.
>>>>> Per my understanding, if a kvm guest running on an older qemu
>>>>> without tsc deadline timer support,
>>>>> then after migrate, the guest would still cannot find tsc deadline
>>>>> feature, no matter older or newer host/qemu/pc-xx it migrate to.
>>>>
>>>> What should prevent this? The feature flags are not part of the
>>>> vmstate. They are part of the vm configuration which is not migrated
>>>> but defined by starting qemu on the target host.
>>>>
>>>
>>> Thanks! understand this point ("They are part of the vm
>>> configuration which is not migrated but defined by starting qemu on
>>> the target host").  
>>>
>>> But kvmclock example still cannot satisfy the purpose "guest running
>>> on old qemu/pc-0.13 without kvmclock support must not find kvmclock
>>> feature when being migrated to a host with new qemu/pc-0.13 compat
>>>     mode". After migration, guest can possibily find kvmclock
>>> feature CPUID.0x40000001.KVM_FEATURE_CLOCKSOURCE: pc_init1(...,
>>> kvmclock_enabled) { pc_cpus_init(cpu_model);    // the point to
>>> decide and expose cpuid features to guest   
>>>
>>>     if (kvmclock_enabled) {        // the difference point between
>>>     pc-0.13 vs. pc-0.14, related nothing to cpuid features.        
>>> kvmclock_create(); } }
>>
>> Right, not a perfect example: the cpuid feature is not influenced by
>> this mechanism, only the fact if a kvmclock device (for save/restore)
>> should be created. I guess we ignored this back then, only focusing on
>> the more obvious issue of the addition device.
>>
>>>
>>> Seems currently there is no good way to satisfy "guest running on
>>> old qemu/pc-xx without feature A support must not find feature A
>>> when being migrated to a host with new qemu/pc-xx compat mode", i.e.
>>> considering   
>>> * if running with '-cpu host' then migrate;
>>> * each time we add a new cpuid feature it need add one or more new
>>> machine model? is it necessary to bind pc-xx with cpuid feature? 
>>> * logically cpuid features should better be controlled by cpu model,
>>> not by machine model. 
>>
>> The compatibility machines define the possible cpu models. If I select
> 
> How does machine define possible cpu models?
> cpu model defined by qemu option '-cpu ...', while machine model defined by '-machine ...'
> 
>> pc-0.14, e.g. -cpu kvm64 should not give me features that 0.14 was not
>> exposing.
>>
> 
> in such case, it's '-cpu kvm64' who take effect to decide what cpuid features would exposed to guest, not '-machine pc-0.14'.
> 
> IMO, what our patch need to do is to expose a cpuid feature to guest (CPUID.01H:ECX.TSC_Deadline[bit 24]), it decided by cpu model, not machine model:
> pc_init1(..., cpu_model, ...)
> {
>     pc_cpus_init(cpu_model);   // this is the whole logic exposing cpuid features to guest
>     ...
> }
> 
> Do I misunderstanding something?

My point is that

  qemu-version-A [-cpu whatever]

should provide the same VM as

  qemu-version-B -machine pc-A [-cpu whatever]

specifically if you leave out the cpu specification.

So the compat machine could establish a feature mask (e.g. append some
"-tsc_deadline" in this case). But, indeed, we need a new channel for this.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

[-- Attachment #1: Type: text/plain, Size: 3123 bytes --]

> 
> My point is that
> 
>   qemu-version-A [-cpu whatever]
> 
> should provide the same VM as
> 
>   qemu-version-B -machine pc-A [-cpu whatever]
> 
> specifically if you leave out the cpu specification.
> 
> So the compat machine could establish a feature mask (e.g. append some
> "-tsc_deadline" in this case). But, indeed, we need a new channel for
> this. 
> 

Yes, if such requirement need to be satisfied, I agree we need a new channel to solve this kind of common issue.

As for tsc deadline timer feature exposing, I write an updated patch as attached.
1). It exposes tsc deadline timer feature to guest if in-kernel irqchip is used and kvm has emulated tsc deadline timer;
2). It also authorizes user to control the feature exposing via a cpu feature flag;

Thanks,
Jinsong

====================
>From 5b7d5f459b621686e78e437010ce34748bcb9e8e Mon Sep 17 00:00:00 2001
From: Liu, Jinsong <jinsong.liu@intel.com>
Date: Wed, 29 Feb 2012 01:53:15 +0800
Subject: [PATCH] Expose tsc deadline timer feature to guest

It exposes tsc deadline timer feature to guest if in-kernel irqchip is used
and kvm has emulated tsc deadline timer.
It also authorizes user to control the feature exposing via a cpu feature flag.

Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
---
 target-i386/cpu.h   |    1 +
 target-i386/cpuid.c |    2 +-
 target-i386/kvm.c   |    4 ++++
 3 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index d92be5d..3409afe 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -399,6 +399,7 @@
 #define CPUID_EXT_X2APIC   (1 << 21)
 #define CPUID_EXT_MOVBE    (1 << 22)
 #define CPUID_EXT_POPCNT   (1 << 23)
+#define CPUID_EXT_TSC_DEADLINE_TIMER (1 << 24)
 #define CPUID_EXT_XSAVE    (1 << 26)
 #define CPUID_EXT_OSXSAVE  (1 << 27)
 #define CPUID_EXT_HYPERVISOR  (1 << 31)
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index b9bfeaf..ac4b79c 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -50,7 +50,7 @@ static const char *ext_feature_name[] = {
     "fma", "cx16", "xtpr", "pdcm",
     NULL, NULL, "dca", "sse4.1|sse4_1",
     "sse4.2|sse4_2", "x2apic", "movbe", "popcnt",
-    NULL, "aes", "xsave", "osxsave",
+    "tsc_deadline", "aes", "xsave", "osxsave",
     "avx", NULL, NULL, "hypervisor",
 };
 static const char *ext2_feature_name[] = {
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 7079e87..2639699 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -370,6 +370,10 @@ int kvm_arch_init_vcpu(CPUState *env)
     i = env->cpuid_ext_features & CPUID_EXT_HYPERVISOR;
     env->cpuid_ext_features &= kvm_arch_get_supported_cpuid(s, 1, 0, R_ECX);
     env->cpuid_ext_features |= i;
+    if (!kvm_irqchip_in_kernel() ||
+        !kvm_check_extension(s, KVM_CAP_TSC_DEADLINE_TIMER)) {
+        env->cpuid_ext_features &= ~CPUID_EXT_TSC_DEADLINE_TIMER;
+    }
 
     env->cpuid_ext2_features &= kvm_arch_get_supported_cpuid(s, 0x80000001,
                                                              0, R_EDX);
-- 
1.7.1

[-- Attachment #2: 0001-Expose-tsc-deadline-timer-feature-to-guest.patch --]
[-- Type: application/octet-stream, Size: 2233 bytes --]

From 5b7d5f459b621686e78e437010ce34748bcb9e8e Mon Sep 17 00:00:00 2001
From: Liu, Jinsong <jinsong.liu@intel.com>
Date: Wed, 29 Feb 2012 01:53:15 +0800
Subject: [PATCH] Expose tsc deadline timer feature to guest

It exposes tsc deadline timer feature to guest if in-kernel irqchip is used
and kvm has emulated tsc deadline timer.
It also authorizes user to control the feature exposing via a cpu feature flag.

Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
---
 target-i386/cpu.h   |    1 +
 target-i386/cpuid.c |    2 +-
 target-i386/kvm.c   |    4 ++++
 3 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index d92be5d..3409afe 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -399,6 +399,7 @@
 #define CPUID_EXT_X2APIC   (1 << 21)
 #define CPUID_EXT_MOVBE    (1 << 22)
 #define CPUID_EXT_POPCNT   (1 << 23)
+#define CPUID_EXT_TSC_DEADLINE_TIMER (1 << 24)
 #define CPUID_EXT_XSAVE    (1 << 26)
 #define CPUID_EXT_OSXSAVE  (1 << 27)
 #define CPUID_EXT_HYPERVISOR  (1 << 31)
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index b9bfeaf..ac4b79c 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -50,7 +50,7 @@ static const char *ext_feature_name[] = {
     "fma", "cx16", "xtpr", "pdcm",
     NULL, NULL, "dca", "sse4.1|sse4_1",
     "sse4.2|sse4_2", "x2apic", "movbe", "popcnt",
-    NULL, "aes", "xsave", "osxsave",
+    "tsc_deadline", "aes", "xsave", "osxsave",
     "avx", NULL, NULL, "hypervisor",
 };
 static const char *ext2_feature_name[] = {
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 7079e87..2639699 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -370,6 +370,10 @@ int kvm_arch_init_vcpu(CPUState *env)
     i = env->cpuid_ext_features & CPUID_EXT_HYPERVISOR;
     env->cpuid_ext_features &= kvm_arch_get_supported_cpuid(s, 1, 0, R_ECX);
     env->cpuid_ext_features |= i;
+    if (!kvm_irqchip_in_kernel() ||
+        !kvm_check_extension(s, KVM_CAP_TSC_DEADLINE_TIMER)) {
+        env->cpuid_ext_features &= ~CPUID_EXT_TSC_DEADLINE_TIMER;
+    }
 
     env->cpuid_ext2_features &= kvm_arch_get_supported_cpuid(s, 0x80000001,
                                                              0, R_EDX);
-- 
1.7.1

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

Jan,

Any comments? I feel some confused about your point 'disable cpuid feature for older machine types by default': are you planning a common approach for this common issue, or, you just ask me a specific solution for the tsc deadline timer case?

Thanks,
Jinsong


Liu, Jinsong wrote:
>> My point is that
>> 
>>   qemu-version-A [-cpu whatever]
>> 
>> should provide the same VM as
>> 
>>   qemu-version-B -machine pc-A [-cpu whatever]
>> 
>> specifically if you leave out the cpu specification.
>> 
>> So the compat machine could establish a feature mask (e.g. append
>> some "-tsc_deadline" in this case). But, indeed, we need a new
>> channel for this. 
>> 
> 
> Yes, if such requirement need to be satisfied, I agree we need a new
> channel to solve this kind of common issue. 
> 
> As for tsc deadline timer feature exposing, I write an updated patch
> as attached. 1). It exposes tsc deadline timer feature to guest if
> in-kernel irqchip is used and kvm has emulated tsc deadline timer;
> 2). It also authorizes user to control the feature exposing via a cpu
> feature flag;  
> 
> Thanks,
> Jinsong
> 
> ====================
> From 5b7d5f459b621686e78e437010ce34748bcb9e8e Mon Sep 17 00:00:00 2001
> From: Liu, Jinsong <jinsong.liu@intel.com>
> Date: Wed, 29 Feb 2012 01:53:15 +0800
> Subject: [PATCH] Expose tsc deadline timer feature to guest
> 
> It exposes tsc deadline timer feature to guest if in-kernel irqchip
> is used 
> and kvm has emulated tsc deadline timer.
> It also authorizes user to control the feature exposing via a cpu
> feature flag. 
> 
> Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
> ---
>  target-i386/cpu.h   |    1 +
>  target-i386/cpuid.c |    2 +-
>  target-i386/kvm.c   |    4 ++++
>  3 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index d92be5d..3409afe 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -399,6 +399,7 @@
>  #define CPUID_EXT_X2APIC   (1 << 21)
>  #define CPUID_EXT_MOVBE    (1 << 22)
>  #define CPUID_EXT_POPCNT   (1 << 23)
> +#define CPUID_EXT_TSC_DEADLINE_TIMER (1 << 24)
>  #define CPUID_EXT_XSAVE    (1 << 26)
>  #define CPUID_EXT_OSXSAVE  (1 << 27)
>  #define CPUID_EXT_HYPERVISOR  (1 << 31)
> diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
> index b9bfeaf..ac4b79c 100644
> --- a/target-i386/cpuid.c
> +++ b/target-i386/cpuid.c
> @@ -50,7 +50,7 @@ static const char *ext_feature_name[] = {
>      "fma", "cx16", "xtpr", "pdcm",
>      NULL, NULL, "dca", "sse4.1|sse4_1",
>      "sse4.2|sse4_2", "x2apic", "movbe", "popcnt",
> -    NULL, "aes", "xsave", "osxsave",
> +    "tsc_deadline", "aes", "xsave", "osxsave",
>      "avx", NULL, NULL, "hypervisor",
>  };
>  static const char *ext2_feature_name[] = {
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 7079e87..2639699 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -370,6 +370,10 @@ int kvm_arch_init_vcpu(CPUState *env)
>      i = env->cpuid_ext_features & CPUID_EXT_HYPERVISOR;
>      env->cpuid_ext_features &= kvm_arch_get_supported_cpuid(s, 1, 0,
>      R_ECX); env->cpuid_ext_features |= i;
> +    if (!kvm_irqchip_in_kernel() ||
> +        !kvm_check_extension(s, KVM_CAP_TSC_DEADLINE_TIMER)) {
> +        env->cpuid_ext_features &= ~CPUID_EXT_TSC_DEADLINE_TIMER;
> +    }
> 
>      env->cpuid_ext2_features &= kvm_arch_get_supported_cpuid(s,
>                                                              
> 0x80000001, 0, R_EDX); 

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Jan Kiszka]

On 2012-03-06 08:49, Liu, Jinsong wrote:
> Jan,
> 
> Any comments? I feel some confused about your point 'disable cpuid feature for older machine types by default': are you planning a common approach for this common issue, or, you just ask me a specific solution for the tsc deadline timer case?

I think a generic solution for this can be as simple as passing a
feature exclusion mask to cpu_init. You could simple append a string of
"-feature1,-feature2" to the cpu model that is specified on creation.
And that string could be defined in the compat machine descriptions.
Does this make sense?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

[-- Attachment #1: Type: text/plain, Size: 2177 bytes --]

Jan Kiszka wrote:
> On 2012-03-06 08:49, Liu, Jinsong wrote:
>> Jan,
>> 
>> Any comments? I feel some confused about your point 'disable cpuid
>> feature for older machine types by default': are you planning a
>> common approach for this common issue, or, you just ask me a
>> specific solution for the tsc deadline timer case?   
> 
> I think a generic solution for this can be as simple as passing a
> feature exclusion mask to cpu_init. You could simple append a string
> of "-feature1,-feature2" to the cpu model that is specified on
> creation. And that string could be defined in the compat machine
> descriptions. Does this make sense?
> 

Jan, to prevent misunderstanding, I elaborate my understanding of your points below (if any misunderstanding please point out to me):
=====================
Your target is, to migrate from A(old qemu) to B(new qemu) by
1. at A: qemu-version-A [-cpu whatever]      // currently the default machine type is pc-A
2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1 -feature2

B run new qemu-version-B (w/ new features 'feature1' and 'feature2'), but when B runs w/ compat '-machine pc-A', vm should not see 'feature1' and 'feature2', so commandline append string to cpu model '-cpu whatever -feature1 -feature2' to hidden new feature1 and feature2 to vm, hence vm can see same cpuid features (at B) as those at A (which means, no feature1, no feature2)
=====================

If my understanding of your thoughts is right, I think currently qemu has satisfied your target, code refer to
     pc_cpus_init(cpu_model)
     ......
     cpu_init(cpu_model)
--> cpu_x86_register(*env, cpu_model)
--> cpu_x86_find_by_name(*def, cpu_model)     // parse '+/- features', generate feature masks plus_features... 
                                                                     // and minus_features...(this is feature exclusion masks you want)
I think your point 'define in the compat machine description' is unnecessary.

As for 'tsc deadline' feature exposing, my patch (as attached) just obey qemu general cpuid exposing method, and also satisfied your target I think.


Thanks,
Jinsong

[-- Attachment #2: 0001-Expose-tsc-deadline-timer-feature-to-guest.patch --]
[-- Type: application/octet-stream, Size: 2233 bytes --]

From 5b7d5f459b621686e78e437010ce34748bcb9e8e Mon Sep 17 00:00:00 2001
From: Liu, Jinsong <jinsong.liu@intel.com>
Date: Wed, 29 Feb 2012 01:53:15 +0800
Subject: [PATCH] Expose tsc deadline timer feature to guest

It exposes tsc deadline timer feature to guest if in-kernel irqchip is used
and kvm has emulated tsc deadline timer.
It also authorizes user to control the feature exposing via a cpu feature flag.

Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
---
 target-i386/cpu.h   |    1 +
 target-i386/cpuid.c |    2 +-
 target-i386/kvm.c   |    4 ++++
 3 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index d92be5d..3409afe 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -399,6 +399,7 @@
 #define CPUID_EXT_X2APIC   (1 << 21)
 #define CPUID_EXT_MOVBE    (1 << 22)
 #define CPUID_EXT_POPCNT   (1 << 23)
+#define CPUID_EXT_TSC_DEADLINE_TIMER (1 << 24)
 #define CPUID_EXT_XSAVE    (1 << 26)
 #define CPUID_EXT_OSXSAVE  (1 << 27)
 #define CPUID_EXT_HYPERVISOR  (1 << 31)
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index b9bfeaf..ac4b79c 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -50,7 +50,7 @@ static const char *ext_feature_name[] = {
     "fma", "cx16", "xtpr", "pdcm",
     NULL, NULL, "dca", "sse4.1|sse4_1",
     "sse4.2|sse4_2", "x2apic", "movbe", "popcnt",
-    NULL, "aes", "xsave", "osxsave",
+    "tsc_deadline", "aes", "xsave", "osxsave",
     "avx", NULL, NULL, "hypervisor",
 };
 static const char *ext2_feature_name[] = {
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 7079e87..2639699 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -370,6 +370,10 @@ int kvm_arch_init_vcpu(CPUState *env)
     i = env->cpuid_ext_features & CPUID_EXT_HYPERVISOR;
     env->cpuid_ext_features &= kvm_arch_get_supported_cpuid(s, 1, 0, R_ECX);
     env->cpuid_ext_features |= i;
+    if (!kvm_irqchip_in_kernel() ||
+        !kvm_check_extension(s, KVM_CAP_TSC_DEADLINE_TIMER)) {
+        env->cpuid_ext_features &= ~CPUID_EXT_TSC_DEADLINE_TIMER;
+    }
 
     env->cpuid_ext2_features &= kvm_arch_get_supported_cpuid(s, 0x80000001,
                                                              0, R_EDX);
-- 
1.7.1

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Jan Kiszka]

On 2012-03-09 19:27, Liu, Jinsong wrote:
> Jan Kiszka wrote:
>> On 2012-03-06 08:49, Liu, Jinsong wrote:
>>> Jan,
>>>
>>> Any comments? I feel some confused about your point 'disable cpuid
>>> feature for older machine types by default': are you planning a
>>> common approach for this common issue, or, you just ask me a
>>> specific solution for the tsc deadline timer case?   
>>
>> I think a generic solution for this can be as simple as passing a
>> feature exclusion mask to cpu_init. You could simple append a string
>> of "-feature1,-feature2" to the cpu model that is specified on
>> creation. And that string could be defined in the compat machine
>> descriptions. Does this make sense?
>>
> 
> Jan, to prevent misunderstanding, I elaborate my understanding of your points below (if any misunderstanding please point out to me):
> =====================
> Your target is, to migrate from A(old qemu) to B(new qemu) by
> 1. at A: qemu-version-A [-cpu whatever]      // currently the default machine type is pc-A
> 2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1 -feature2
> 
> B run new qemu-version-B (w/ new features 'feature1' and 'feature2'), but when B runs w/ compat '-machine pc-A', vm should not see 'feature1' and 'feature2', so commandline append string to cpu model '-cpu whatever -feature1 -feature2' to hidden new feature1 and feature2 to vm, hence vm can see same cpuid features (at B) as those at A (which means, no feature1, no feature2)
> =====================
> 
> If my understanding of your thoughts is right, I think currently qemu has satisfied your target, code refer to
>      pc_cpus_init(cpu_model)
>      ......
>      cpu_init(cpu_model)
> --> cpu_x86_register(*env, cpu_model)
> --> cpu_x86_find_by_name(*def, cpu_model)     // parse '+/- features', generate feature masks plus_features... 
>                                                                      // and minus_features...(this is feature exclusion masks you want)
> I think your point 'define in the compat machine description' is unnecessary.

The user would have to specify the new feature as exclusions *manually*
on the command line if -machine pc-A doesn't inject them
*automatically*. So it is necessary to enhance qemu in this regard.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

Jan Kiszka wrote:
> On 2012-03-09 19:27, Liu, Jinsong wrote:
>> Jan Kiszka wrote:
>>> On 2012-03-06 08:49, Liu, Jinsong wrote:
>>>> Jan,
>>>> 
>>>> Any comments? I feel some confused about your point 'disable cpuid
>>>> feature for older machine types by default': are you planning a
>>>> common approach for this common issue, or, you just ask me a
>>>> specific solution for the tsc deadline timer case?
>>> 
>>> I think a generic solution for this can be as simple as passing a
>>> feature exclusion mask to cpu_init. You could simple append a string
>>> of "-feature1,-feature2" to the cpu model that is specified on
>>> creation. And that string could be defined in the compat machine
>>> descriptions. Does this make sense?
>>> 
>> 
>> Jan, to prevent misunderstanding, I elaborate my understanding of
>> your points below (if any misunderstanding please point out to me):
>> =====================  
>> Your target is, to migrate from A(old qemu) to B(new qemu) by
>> 1. at A: qemu-version-A [-cpu whatever]      // currently the
>> default machine type is pc-A 
>> 2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1
>> -feature2 
>> 
>> B run new qemu-version-B (w/ new features 'feature1' and
>> 'feature2'), but when B runs w/ compat '-machine pc-A', vm should
>> not see 'feature1' and 'feature2', so commandline append string to
>> cpu model '-cpu whatever -feature1 -feature2' to hidden new feature1
>> and feature2 to vm, hence vm can see same cpuid features (at B) as
>> those at A (which means, no feature1, no feature2)
>> =====================      
>> 
>> If my understanding of your thoughts is right, I think currently
>>      qemu has satisfied your target, code refer to     
>>      pc_cpus_init(cpu_model) ...... cpu_init(cpu_model)
>> --> cpu_x86_register(*env, cpu_model)
>> --> cpu_x86_find_by_name(*def, cpu_model)     // parse '+/-
>>                                                                     
>> features', generate feature masks plus_features... // and
>> minus_features...(this is feature exclusion masks you want)  
>> I think your point 'define in the compat machine description' is
>> unnecessary. 
> 
> The user would have to specify the new feature as exclusions
> *manually* on the command line if -machine pc-A doesn't inject them
> *automatically*. So it is necessary to enhance qemu in this regard.
> 

... You suggest 'append a string of "-feature1,-feature2" to the cpu model that is specified on creation' at your last email.
Could you tell me other way user exclude features? I only know qemu command line :-(

Thanks,
Jinsong

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 4644 bytes --]

On 2012-03-09 20:09, Liu, Jinsong wrote:
> Jan Kiszka wrote:
>> On 2012-03-09 19:27, Liu, Jinsong wrote:
>>> Jan Kiszka wrote:
>>>> On 2012-03-06 08:49, Liu, Jinsong wrote:
>>>>> Jan,
>>>>>
>>>>> Any comments? I feel some confused about your point 'disable cpuid
>>>>> feature for older machine types by default': are you planning a
>>>>> common approach for this common issue, or, you just ask me a
>>>>> specific solution for the tsc deadline timer case?
>>>>
>>>> I think a generic solution for this can be as simple as passing a
>>>> feature exclusion mask to cpu_init. You could simple append a string
>>>> of "-feature1,-feature2" to the cpu model that is specified on
>>>> creation. And that string could be defined in the compat machine
>>>> descriptions. Does this make sense?
>>>>
>>>
>>> Jan, to prevent misunderstanding, I elaborate my understanding of
>>> your points below (if any misunderstanding please point out to me):
>>> =====================  
>>> Your target is, to migrate from A(old qemu) to B(new qemu) by
>>> 1. at A: qemu-version-A [-cpu whatever]      // currently the
>>> default machine type is pc-A 
>>> 2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1
>>> -feature2 
>>>
>>> B run new qemu-version-B (w/ new features 'feature1' and
>>> 'feature2'), but when B runs w/ compat '-machine pc-A', vm should
>>> not see 'feature1' and 'feature2', so commandline append string to
>>> cpu model '-cpu whatever -feature1 -feature2' to hidden new feature1
>>> and feature2 to vm, hence vm can see same cpuid features (at B) as
>>> those at A (which means, no feature1, no feature2)
>>> =====================      
>>>
>>> If my understanding of your thoughts is right, I think currently
>>>      qemu has satisfied your target, code refer to     
>>>      pc_cpus_init(cpu_model) ...... cpu_init(cpu_model)
>>> --> cpu_x86_register(*env, cpu_model)
>>> --> cpu_x86_find_by_name(*def, cpu_model)     // parse '+/-
>>>                                                                     
>>> features', generate feature masks plus_features... // and
>>> minus_features...(this is feature exclusion masks you want)  
>>> I think your point 'define in the compat machine description' is
>>> unnecessary. 
>>
>> The user would have to specify the new feature as exclusions
>> *manually* on the command line if -machine pc-A doesn't inject them
>> *automatically*. So it is necessary to enhance qemu in this regard.
>>
> 
> ... You suggest 'append a string of "-feature1,-feature2" to the cpu model that is specified on creation' at your last email.
> Could you tell me other way user exclude features? I only know qemu command line :-(

I was thinking of something like

diff --git a/hw/boards.h b/hw/boards.h
index 667177d..2bae071 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -28,6 +28,7 @@ typedef struct QEMUMachine {
     int is_default;
     const char *default_machine_opts;
     GlobalProperty *compat_props;
+    const char *compat_cpu_features;
     struct QEMUMachine *next;
 } QEMUMachine;
 
diff --git a/hw/pc.c b/hw/pc.c
index bb9867b..4d11559 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -949,8 +949,9 @@ static CPUState *pc_new_cpu(const char *cpu_model)
     return env;
 }
 
-void pc_cpus_init(const char *cpu_model)
+void pc_cpus_init(const char *cpu_model, const char *append_features)
 {
+    char *model_and_features;
     int i;
 
     /* init CPUs */
@@ -961,10 +962,13 @@ void pc_cpus_init(const char *cpu_model)
         cpu_model = "qemu32";
 #endif
     }
+    model_and_features = g_strconcat(cpu_model, ",", append_features, NULL);
 
     for(i = 0; i < smp_cpus; i++) {
-        pc_new_cpu(cpu_model);
+        pc_new_cpu(model_and_features);
     }
+
+    g_free(model_and_features);
 }
 
 void pc_memory_init(MemoryRegion *system_memory,


However, getting machine.compat_cpu_features to pc_cpus_init is rather
ugly. And we will have CPU devices with real properties soon. Then the
compat feature string could be passed that way, without changing any
machine init function.

Andreas, do you expect CPU devices to be ready for qemu 1.1? We would
need them to pass a feature exclusion mask from machine.compat_props to
the (x86) CPU init code.

Well, given that introducing some intermediate solution for this would
be complex and hacky and that there is a way to configure tsc_deadline
for old machines away, though only an explicit one, I could live with
postponing the feature mask after the CPU device conversion. But the
last word will have the maintainers.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Andreas Färber]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 09.03.2012 21:52, schrieb Jan Kiszka:
> Andreas, do you expect CPU devices to be ready for qemu 1.1? We 
> would need them to pass a feature exclusion mask from 
> machine.compat_props to the (x86) CPU init code.

I was sure hoping to!

Marcelo and Avi both going into vacation have let me down with the
kvmclock patch that you wanted to go through uq/master.

http://patchwork.ozlabs.org/patch/141721/

After it didn't make it into Avi's next PULL, I resubmitted it as part
of the qom-user series. That's still on the list. Review and tags
appreciated.

http://patchwork.ozlabs.org/patch/144529/

Also on the list is a patch for a new object_class_get_list() function
for sensible -cpu ? handling.

http://patchwork.ozlabs.org/patch/143076/

I have been waiting for progress on these prerequisites before posting
huge follow-on batches, but I can spam the list right away...
The remaining issue was how to solve the struct CPU vs. CPU() issue -
my solution on qom-cpu branch is to free the identifier CPUState in a
quite invasive but half-scripted renaming. Based on struct CPUState,
all targets are long successfully converted and keep needing to be
rebased whenever someone moves code to a new microblaze file, adds a
new arm board or in-kernel device using "CPUState", etc.
Since there's a configure change involved in *-user QOM support,
virtually all files get rebuilt on each rebase and that takes quite
some time, so getting that in soonish would be appreciated.

http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-cpu

Andreas

- -- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.18 (GNU/Linux)

iQIcBAEBAgAGBQJPWqljAAoJEPou0S0+fgE/u+kQAInqv/OYlL2zU+hyeKXdJcZ4
RJ9Ne7J1+MOYcOI0p2T4lTImawHmZbF1HQyM9gTYv3EcGoSLm/SgYnHhFOulvPaE
dbpuwk3KPHU+1ekvMVhEX89F62rXvMTz8JxO7hSmNAwIU2fIeKKWEAo0tED2tY16
OC2ExnE46+xLpandGJmfnx8vUJ1/PaO6fpd/FLOZbACT75zmdDxjDpF85e6NNsct
ysMQ3kNMWcc0RfTU02dPgrhCPc5irEbalaX6BtOnquVQZDH4ypwzKHOZpMfoTF4u
QWyxy+NaOVMVBHgHJeGek0wQ4nbihIhrQAn+Hg/h0P+NPuxQ8s7zX4xwqlYwhAHn
cdDfN1sVbIvRWkGCXSRJR5eIkPKV+81Afm7/pL26eNlVtSrVV9UnsOJyfyDrtFs/
tSE90n8ZecjARGXUaACboZRalt7Pxl4EGWJwBhsNd5+3xaYvc9/WuTYP1FK1nltk
KiBETQXnSY5aTh13erKLjbtYGR5YexXiLIOXvnBD0qQ/LDryHNjslgM8HcShrx4S
U9LFAqEq78fgpCXZlzwtnzed341Wuerw3z4jELrSW8GH+R21jMfT9L8lwaHexabh
7YlulgdSpsmCxzhHOUCMeUTFFY6TlPxVk2NhEhQxlzDXdDmm6nd67R9qJIf7d42z
CPzkIX+OpKBmorMQREIg
=a+ZQ
-----END PGP SIGNATURE-----

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

Jan Kiszka wrote:
> On 2012-03-09 20:09, Liu, Jinsong wrote:
>> Jan Kiszka wrote:
>>> On 2012-03-09 19:27, Liu, Jinsong wrote:
>>>> Jan Kiszka wrote:
>>>>> On 2012-03-06 08:49, Liu, Jinsong wrote:
>>>>>> Jan,
>>>>>> 
>>>>>> Any comments? I feel some confused about your point 'disable
>>>>>> cpuid feature for older machine types by default': are you
>>>>>> planning a common approach for this common issue, or, you just
>>>>>> ask me a specific solution for the tsc deadline timer case?
>>>>> 
>>>>> I think a generic solution for this can be as simple as passing a
>>>>> feature exclusion mask to cpu_init. You could simple append a
>>>>> string of "-feature1,-feature2" to the cpu model that is
>>>>> specified on creation. And that string could be defined in the
>>>>> compat machine descriptions. Does this make sense?
>>>>> 
>>>> 
>>>> Jan, to prevent misunderstanding, I elaborate my understanding of
>>>> your points below (if any misunderstanding please point out to
>>>> me): ===================== Your target is, to migrate from A(old
>>>> qemu) to B(new qemu) by 
>>>> 1. at A: qemu-version-A [-cpu whatever]      // currently the
>>>> default machine type is pc-A 
>>>> 2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1
>>>> -feature2 
>>>> 
>>>> B run new qemu-version-B (w/ new features 'feature1' and
>>>> 'feature2'), but when B runs w/ compat '-machine pc-A', vm should
>>>> not see 'feature1' and 'feature2', so commandline append string to
>>>> cpu model '-cpu whatever -feature1 -feature2' to hidden new
>>>> feature1 and feature2 to vm, hence vm can see same cpuid features
>>>> (at B) as those at A (which means, no feature1, no feature2)
>>>> =====================
>>>> 
>>>> If my understanding of your thoughts is right, I think currently
>>>>      qemu has satisfied your target, code refer to
>>>>      pc_cpus_init(cpu_model) ...... cpu_init(cpu_model)
>>>> --> cpu_x86_register(*env, cpu_model)
>>>> --> cpu_x86_find_by_name(*def, cpu_model)     // parse '+/-
>>>> 
>>>> features', generate feature masks plus_features... // and
>>>> minus_features...(this is feature exclusion masks you want)
>>>> I think your point 'define in the compat machine description' is
>>>> unnecessary.
>>> 
>>> The user would have to specify the new feature as exclusions
>>> *manually* on the command line if -machine pc-A doesn't inject them
>>> *automatically*. So it is necessary to enhance qemu in this regard.
>>> 
>> 
>> ... You suggest 'append a string of "-feature1,-feature2" to the cpu
>> model that is specified on creation' at your last email. Could you
>> tell me other way user exclude features? I only know qemu command
>> line :-(  
> 
> I was thinking of something like
> 
> diff --git a/hw/boards.h b/hw/boards.h
> index 667177d..2bae071 100644
> --- a/hw/boards.h
> +++ b/hw/boards.h
> @@ -28,6 +28,7 @@ typedef struct QEMUMachine {
>      int is_default;
>      const char *default_machine_opts;
>      GlobalProperty *compat_props;
> +    const char *compat_cpu_features;
>      struct QEMUMachine *next;
>  } QEMUMachine;
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index bb9867b..4d11559 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -949,8 +949,9 @@ static CPUState *pc_new_cpu(const char *cpu_model)
>      return env;
>  }
> 
> -void pc_cpus_init(const char *cpu_model)
> +void pc_cpus_init(const char *cpu_model, const char *append_features)
>  {
> +    char *model_and_features;
>      int i;
> 
>      /* init CPUs */
> @@ -961,10 +962,13 @@ void pc_cpus_init(const char *cpu_model)
>          cpu_model = "qemu32";
>  #endif
>      }
> +    model_and_features = g_strconcat(cpu_model, ",",
> append_features, NULL); 
> 
>      for(i = 0; i < smp_cpus; i++) {
> -        pc_new_cpu(cpu_model);
> +        pc_new_cpu(model_and_features);
>      }
> +
> +    g_free(model_and_features);
>  }
> 
>  void pc_memory_init(MemoryRegion *system_memory,
> 
> 
> However, getting machine.compat_cpu_features to pc_cpus_init is rather
> ugly. And we will have CPU devices with real properties soon. Then the
> compat feature string could be passed that way, without changing any
> machine init function.
> 
> Andreas, do you expect CPU devices to be ready for qemu 1.1? We would
> need them to pass a feature exclusion mask from machine.compat_props
> to 
> the (x86) CPU init code.

cpu devices is just another format of current cpu_model. It helps nothing to our problem.
Again, the point is, by what method the feature exclusion mask can be generated, if user not give hint manually?

Thanks,
Jinsong

> 
> Well, given that introducing some intermediate solution for this would
> be complex and hacky and that there is a way to configure tsc_deadline
> for old machines away, though only an explicit one, I could live with
> postponing the feature mask after the CPU device conversion. But the
> last word will have the maintainers.
> 
> Jan

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Eduardo Habkost]

On Fri, Mar 09, 2012 at 09:52:29PM +0100, Jan Kiszka wrote:
> On 2012-03-09 20:09, Liu, Jinsong wrote:
> > Jan Kiszka wrote:
> >> On 2012-03-09 19:27, Liu, Jinsong wrote:
> >>> Jan Kiszka wrote:
> >>>> On 2012-03-06 08:49, Liu, Jinsong wrote:
> >>>>> Jan,
> >>>>>
> >>>>> Any comments? I feel some confused about your point 'disable cpuid
> >>>>> feature for older machine types by default': are you planning a
> >>>>> common approach for this common issue, or, you just ask me a
> >>>>> specific solution for the tsc deadline timer case?
> >>>>
> >>>> I think a generic solution for this can be as simple as passing a
> >>>> feature exclusion mask to cpu_init. You could simple append a string
> >>>> of "-feature1,-feature2" to the cpu model that is specified on
> >>>> creation. And that string could be defined in the compat machine
> >>>> descriptions. Does this make sense?
> >>>>
> >>>
> >>> Jan, to prevent misunderstanding, I elaborate my understanding of
> >>> your points below (if any misunderstanding please point out to me):
> >>> =====================  
> >>> Your target is, to migrate from A(old qemu) to B(new qemu) by
> >>> 1. at A: qemu-version-A [-cpu whatever]      // currently the
> >>> default machine type is pc-A 
> >>> 2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1
> >>> -feature2 
> >>>
> >>> B run new qemu-version-B (w/ new features 'feature1' and
> >>> 'feature2'), but when B runs w/ compat '-machine pc-A', vm should
> >>> not see 'feature1' and 'feature2', so commandline append string to
> >>> cpu model '-cpu whatever -feature1 -feature2' to hidden new feature1
> >>> and feature2 to vm, hence vm can see same cpuid features (at B) as
> >>> those at A (which means, no feature1, no feature2)
> >>> =====================      
> >>>
> >>> If my understanding of your thoughts is right, I think currently
> >>>      qemu has satisfied your target, code refer to     
> >>>      pc_cpus_init(cpu_model) ...... cpu_init(cpu_model)
> >>> --> cpu_x86_register(*env, cpu_model)
> >>> --> cpu_x86_find_by_name(*def, cpu_model)     // parse '+/-
> >>>                                                                     
> >>> features', generate feature masks plus_features... // and
> >>> minus_features...(this is feature exclusion masks you want)  
> >>> I think your point 'define in the compat machine description' is
> >>> unnecessary. 
> >>
> >> The user would have to specify the new feature as exclusions
> >> *manually* on the command line if -machine pc-A doesn't inject them
> >> *automatically*. So it is necessary to enhance qemu in this regard.
> >>
> > 
> > ... You suggest 'append a string of "-feature1,-feature2" to the cpu model that is specified on creation' at your last email.
> > Could you tell me other way user exclude features? I only know qemu command line :-(
> 
> I was thinking of something like
> 
> diff --git a/hw/boards.h b/hw/boards.h
> index 667177d..2bae071 100644
> --- a/hw/boards.h
> +++ b/hw/boards.h
> @@ -28,6 +28,7 @@ typedef struct QEMUMachine {
>      int is_default;
>      const char *default_machine_opts;
>      GlobalProperty *compat_props;
> +    const char *compat_cpu_features;
>      struct QEMUMachine *next;
>  } QEMUMachine;
>  
> diff --git a/hw/pc.c b/hw/pc.c
> index bb9867b..4d11559 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -949,8 +949,9 @@ static CPUState *pc_new_cpu(const char *cpu_model)
>      return env;
>  }
>  
> -void pc_cpus_init(const char *cpu_model)
> +void pc_cpus_init(const char *cpu_model, const char *append_features)
>  {
> +    char *model_and_features;
>      int i;
>  
>      /* init CPUs */
> @@ -961,10 +962,13 @@ void pc_cpus_init(const char *cpu_model)
>          cpu_model = "qemu32";
>  #endif
>      }
> +    model_and_features = g_strconcat(cpu_model, ",", append_features, NULL);
>  
>      for(i = 0; i < smp_cpus; i++) {
> -        pc_new_cpu(cpu_model);
> +        pc_new_cpu(model_and_features);
>      }
> +
> +    g_free(model_and_features);
>  }
>  
>  void pc_memory_init(MemoryRegion *system_memory,
> 
> 
> However, getting machine.compat_cpu_features to pc_cpus_init is rather
> ugly. And we will have CPU devices with real properties soon. Then the
> compat feature string could be passed that way, without changing any
> machine init function.

What if one cpudef had the wrong flags set but another cpudef was
correct, and we had to fix it on Qemu 1.1 for only one model? What if
the user _really_ wanted to edit the config file to add or remove a
given flag?

I think the best approach would be:
- Having versioned CPU model names;
- Specifying per-machine-type aliases.

See also the "[libvirt] Modern CPU models cannot be used with libvirt"
for related discussion.

The config file would look like:

[cpudef]
name = "Westmere-1.0"
features = "[...]" # no tsc-deadline
[...]

[cpudef]
name = "Westmere-1.1"
# so we don't have to copy everything from Westmere-1.0 manually:
base_cpudef = "Westemere-1.0"
# we could simply copy & extend:
features = "[...] tsc-deadline"
# or, even better, if we had a "append" mechanism. e.g.:
#features_append = "tsc-deadline"


Then, on the machine-type table:

- Machine-types "pc-1.0" and older would have:
  .cpudef_aliases = {
    {"Westmere", "Westmere-1.0"},
    [...]
  }

- Machine-type "pc-1.1" would have:
  .cpudef_aliases = {
    {"Westmere", "Westmere-1.1"},
    [...]
  }


-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

Eduardo Habkost wrote:
> On Fri, Mar 09, 2012 at 09:52:29PM +0100, Jan Kiszka wrote:
>> On 2012-03-09 20:09, Liu, Jinsong wrote:
>>> Jan Kiszka wrote:
>>>> On 2012-03-09 19:27, Liu, Jinsong wrote:
>>>>> Jan Kiszka wrote:
>>>>>> On 2012-03-06 08:49, Liu, Jinsong wrote:
>>>>>>> Jan,
>>>>>>> 
>>>>>>> Any comments? I feel some confused about your point 'disable
>>>>>>> cpuid feature for older machine types by default': are you
>>>>>>> planning a common approach for this common issue, or, you just
>>>>>>> ask me a specific solution for the tsc deadline timer case?
>>>>>> 
>>>>>> I think a generic solution for this can be as simple as passing a
>>>>>> feature exclusion mask to cpu_init. You could simple append a
>>>>>> string of "-feature1,-feature2" to the cpu model that is
>>>>>> specified on creation. And that string could be defined in the
>>>>>> compat machine descriptions. Does this make sense?
>>>>>> 
>>>>> 
>>>>> Jan, to prevent misunderstanding, I elaborate my understanding of
>>>>> your points below (if any misunderstanding please point out to
>>>>> me): ===================== Your target is, to migrate from A(old
>>>>> qemu) to B(new qemu) by 
>>>>> 1. at A: qemu-version-A [-cpu whatever]      // currently the
>>>>> default machine type is pc-A 
>>>>> 2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1
>>>>> -feature2 
>>>>> 
>>>>> B run new qemu-version-B (w/ new features 'feature1' and
>>>>> 'feature2'), but when B runs w/ compat '-machine pc-A', vm should
>>>>> not see 'feature1' and 'feature2', so commandline append string to
>>>>> cpu model '-cpu whatever -feature1 -feature2' to hidden new
>>>>> feature1 and feature2 to vm, hence vm can see same cpuid features
>>>>> (at B) as those at A (which means, no feature1, no feature2)
>>>>> =====================
>>>>> 
>>>>> If my understanding of your thoughts is right, I think currently
>>>>>      qemu has satisfied your target, code refer to
>>>>>      pc_cpus_init(cpu_model) ...... cpu_init(cpu_model)
>>>>> --> cpu_x86_register(*env, cpu_model)
>>>>> --> cpu_x86_find_by_name(*def, cpu_model)     // parse '+/-
>>>>> 
>>>>> features', generate feature masks plus_features... // and
>>>>> minus_features...(this is feature exclusion masks you want)
>>>>> I think your point 'define in the compat machine description' is
>>>>> unnecessary.
>>>> 
>>>> The user would have to specify the new feature as exclusions
>>>> *manually* on the command line if -machine pc-A doesn't inject them
>>>> *automatically*. So it is necessary to enhance qemu in this regard.
>>>> 
>>> 
>>> ... You suggest 'append a string of "-feature1,-feature2" to the
>>> cpu model that is specified on creation' at your last email. Could
>>> you tell me other way user exclude features? I only know qemu
>>> command line :-(  
>> 
>> I was thinking of something like
>> 
>> diff --git a/hw/boards.h b/hw/boards.h
>> index 667177d..2bae071 100644
>> --- a/hw/boards.h
>> +++ b/hw/boards.h
>> @@ -28,6 +28,7 @@ typedef struct QEMUMachine {
>>      int is_default;
>>      const char *default_machine_opts;
>>      GlobalProperty *compat_props;
>> +    const char *compat_cpu_features;
>>      struct QEMUMachine *next;
>>  } QEMUMachine;
>> 
>> diff --git a/hw/pc.c b/hw/pc.c
>> index bb9867b..4d11559 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -949,8 +949,9 @@ static CPUState *pc_new_cpu(const char
>>  *cpu_model)      return env; }
>> 
>> -void pc_cpus_init(const char *cpu_model)
>> +void pc_cpus_init(const char *cpu_model, const char
>> *append_features)  { +    char *model_and_features;
>>      int i;
>> 
>>      /* init CPUs */
>> @@ -961,10 +962,13 @@ void pc_cpus_init(const char *cpu_model)
>>          cpu_model = "qemu32";
>>  #endif
>>      }
>> +    model_and_features = g_strconcat(cpu_model, ",",
>> append_features, NULL); 
>> 
>>      for(i = 0; i < smp_cpus; i++) {
>> -        pc_new_cpu(cpu_model);
>> +        pc_new_cpu(model_and_features);
>>      }
>> +
>> +    g_free(model_and_features);
>>  }
>> 
>>  void pc_memory_init(MemoryRegion *system_memory,
>> 
>> 
>> However, getting machine.compat_cpu_features to pc_cpus_init is
>> rather 
>> ugly. And we will have CPU devices with real properties soon. Then
>> the compat feature string could be passed that way, without changing
>> any 
>> machine init function.
> 
> What if one cpudef had the wrong flags set but another cpudef was
> correct, and we had to fix it on Qemu 1.1 for only one model? What if
> the user _really_ wanted to edit the config file to add or remove a
> given flag?
> 
> I think the best approach would be:
> - Having versioned CPU model names;
> - Specifying per-machine-type aliases.
> 
> See also the "[libvirt] Modern CPU models cannot be used with libvirt"
> for related discussion.
> 
> The config file would look like:
> 
> [cpudef]
> name = "Westmere-1.0"
> features = "[...]" # no tsc-deadline
> [...]
> 
> [cpudef]
> name = "Westmere-1.1"
> # so we don't have to copy everything from Westmere-1.0 manually:
> base_cpudef = "Westemere-1.0"
> # we could simply copy & extend:
> features = "[...] tsc-deadline"
> # or, even better, if we had a "append" mechanism. e.g.:
> #features_append = "tsc-deadline"
> 
> 
> Then, on the machine-type table:
> 
> - Machine-types "pc-1.0" and older would have:
>   .cpudef_aliases = {
>     {"Westmere", "Westmere-1.0"},
>     [...]
>   }
> 
> - Machine-type "pc-1.1" would have:
>   .cpudef_aliases = {
>     {"Westmere", "Westmere-1.1"},
>     [...]
>   }

Yes, it's reasonable in this way (or something like this). Considering currently qemu1.1 is changing cpu_device/per-machine-type, we'd better wait until it done and then update tsc deadline timer accordingly.

Thanks,
Jinsong

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

Liu, Jinsong wrote:
> Jan Kiszka wrote:
>> On 2012-03-06 08:49, Liu, Jinsong wrote:
>>> Jan,
>>> 
>>> Any comments? I feel some confused about your point 'disable cpuid
>>> feature for older machine types by default': are you planning a
>>> common approach for this common issue, or, you just ask me a
>>> specific solution for the tsc deadline timer case?
>> 
>> I think a generic solution for this can be as simple as passing a
>> feature exclusion mask to cpu_init. You could simple append a string
>> of "-feature1,-feature2" to the cpu model that is specified on
>> creation. And that string could be defined in the compat machine
>> descriptions. Does this make sense?
>> 
> 
> Jan, to prevent misunderstanding, I elaborate my understanding of
> your points below (if any misunderstanding please point out to me):
> ===================== 
> Your target is, to migrate from A(old qemu) to B(new qemu) by
> 1. at A: qemu-version-A [-cpu whatever]      // currently the default
> machine type is pc-A 
> 2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1
> -feature2 
> 
> B run new qemu-version-B (w/ new features 'feature1' and 'feature2'),
> but when B runs w/ compat '-machine pc-A', vm should not see
> 'feature1' and 'feature2', so commandline append string to cpu model
> '-cpu whatever -feature1 -feature2' to hidden new feature1 and
> feature2 to vm, hence vm can see same cpuid features (at B) as those
> at A (which means, no feature1, no feature2) =====================    
> 

BTW, any misunderstanding or something wrong about my understanding of your target? please help me confirm. I want to make sure we are talking same thing. 

Thanks,
Jinsong

> If my understanding of your thoughts is right, I think currently qemu
>      has satisfied your target, code refer to pc_cpus_init(cpu_model)
>      ......
>      cpu_init(cpu_model)
> --> cpu_x86_register(*env, cpu_model)
> --> cpu_x86_find_by_name(*def, cpu_model)     // parse '+/-
>                                                                     
> features', generate feature masks plus_features... // and
> minus_features...(this is feature exclusion masks you want)  
> I think your point 'define in the compat machine description' is
> unnecessary. 
> 
> As for 'tsc deadline' feature exposing, my patch (as attached) just
> obey qemu general cpuid exposing method, and also satisfied your
> target I think.  
> 
> 
> Thanks,
> Jinsong

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Rik van Riel]

On 03/09/2012 01:27 PM, Liu, Jinsong wrote:

> As for 'tsc deadline' feature exposing, my patch (as attached) just obey qemu general cpuid exposing method, and also satisfied your target I think.

One question.

Why is TSC_DEADLINE not exposed in the cpuid allowed feature
bits in do_cpuid_ent() in arch/x86/kvm/x86.c ?

         /* cpuid 1.ecx */
         const u32 kvm_supported_word4_x86_features =
                 F(XMM3) | F(PCLMULQDQ) | 0 /* DTES64, MONITOR */ |
                 0 /* DS-CPL, VMX, SMX, EST */ |
                 0 /* TM2 */ | F(SSSE3) | 0 /* CNXT-ID */ | 0 /* 
Reserved */ |
                 F(FMA) | F(CX16) | 0 /* xTPR Update, PDCM */ |
                 0 /* Reserved, DCA */ | F(XMM4_1) |
                 F(XMM4_2) | F(X2APIC) | F(MOVBE) | F(POPCNT) |
                 0 /* Reserved*/ | F(AES) | F(XSAVE) | 0 /* OSXSAVE */ | 
F(AVX) |
                 F(F16C) | F(RDRAND);

Would it make sense to expose F(TSC_DEADLINE) above?

Or is there something truly special about tsc deadline
that means it should be different from everything else?

-- 
All rights reversed

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

Rik van Riel wrote:
> On 03/09/2012 01:27 PM, Liu, Jinsong wrote:
> 
>> As for 'tsc deadline' feature exposing, my patch (as attached) just
>> obey qemu general cpuid exposing method, and also satisfied your
>> target I think.  
> 
> One question.
> 
> Why is TSC_DEADLINE not exposed in the cpuid allowed feature
> bits in do_cpuid_ent() in arch/x86/kvm/x86.c ?
> 
>          /* cpuid 1.ecx */
>          const u32 kvm_supported_word4_x86_features =
>                  F(XMM3) | F(PCLMULQDQ) | 0 /* DTES64, MONITOR */ |
>                  0 /* DS-CPL, VMX, SMX, EST */ |
>                  0 /* TM2 */ | F(SSSE3) | 0 /* CNXT-ID */ | 0 /*
> Reserved */ |
>                  F(FMA) | F(CX16) | 0 /* xTPR Update, PDCM */ |
>                  0 /* Reserved, DCA */ | F(XMM4_1) |
>                  F(XMM4_2) | F(X2APIC) | F(MOVBE) | F(POPCNT) |
>                  0 /* Reserved*/ | F(AES) | F(XSAVE) | 0 /* OSXSAVE
> */ | F(AVX) |
>                  F(F16C) | F(RDRAND);
> 
> Would it make sense to expose F(TSC_DEADLINE) above?
> 
> Or is there something truly special about tsc deadline
> that means it should be different from everything else?

Because the feature depends on KVM_CREATE_IRQCHIP, which we cannot guarantee will be called, we expose it via a KVM_CAP_TSC_DEADLINE_TIMER and not KVM_GET_SUPPORTED_CPUID.
Refer changeset 4d25a066b69fb749a39d0d4c610689dd765a0b0e.

BTW, your question remind me qemu-kvm side patch, and I update it a little (would be sent out later).

Thanks,
Jinsong

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Eduardo Habkost]

On Tue, Mar 20, 2012 at 12:53:57PM +0000, Liu, Jinsong wrote:
> Rik van Riel wrote:
> > On 03/09/2012 01:27 PM, Liu, Jinsong wrote:
> > 
> >> As for 'tsc deadline' feature exposing, my patch (as attached) just
> >> obey qemu general cpuid exposing method, and also satisfied your
> >> target I think.  
> > 
> > One question.
> > 
> > Why is TSC_DEADLINE not exposed in the cpuid allowed feature
> > bits in do_cpuid_ent() in arch/x86/kvm/x86.c ?
> > 
> >          /* cpuid 1.ecx */
> >          const u32 kvm_supported_word4_x86_features =
> >                  F(XMM3) | F(PCLMULQDQ) | 0 /* DTES64, MONITOR */ |
> >                  0 /* DS-CPL, VMX, SMX, EST */ |
> >                  0 /* TM2 */ | F(SSSE3) | 0 /* CNXT-ID */ | 0 /*
> > Reserved */ |
> >                  F(FMA) | F(CX16) | 0 /* xTPR Update, PDCM */ |
> >                  0 /* Reserved, DCA */ | F(XMM4_1) |
> >                  F(XMM4_2) | F(X2APIC) | F(MOVBE) | F(POPCNT) |
> >                  0 /* Reserved*/ | F(AES) | F(XSAVE) | 0 /* OSXSAVE
> > */ | F(AVX) |
> >                  F(F16C) | F(RDRAND);
> > 
> > Would it make sense to expose F(TSC_DEADLINE) above?
> > 
> > Or is there something truly special about tsc deadline
> > that means it should be different from everything else?
> 
> Because the feature depends on KVM_CREATE_IRQCHIP, which we cannot guarantee will be called, we expose it via a KVM_CAP_TSC_DEADLINE_TIMER and not KVM_GET_SUPPORTED_CPUID.

We have many other features that depend on proper support from userspace
otherwise they wouldn't work, but are listed on GET_SUPPORTED_CPUID,
don't we? Why is TSC-deadline special?

GET_SUPPORTED_CPUID just means "KVM supports it as long as userspace
supports it too and enables it", it doesn't mean "CPUID bit that will be
enabled by default"[1].

> Refer changeset 4d25a066b69fb749a39d0d4c610689dd765a0b0e.

That changeset was necessary because the kernel was doing this on
update_cpu

	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
		best->function == 0x1) {
		best->ecx |= bit(X86_FEATURE_TSC_DEADLINE_TIMER);

And that was really wrong, because it enabled the bit unconditionally.
But I don't understand why KVM_CAP_TSC_DEADLINE_TIMER was created if we
already have KVM_GET_SUPPORTED_CPUID to tell userspace which bits are
supported by KVM.


[1] From Documentation/virtual/kvm/api.txt:

"KVM_GET_SUPPORTED_CPUID
[...]
This ioctl returns x86 cpuid features which are supported by both the
hardware and kvm.  Userspace can use the information returned by this
ioctl to construct cpuid information (for KVM_SET_CPUID2) that is
consistent with hardware, kernel, and userspace capabilities, and with
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^
user requirements (for example, the user may wish to constrain cpuid to
emulate older hardware, or for feature consistency across a cluster)."


-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

Eduardo Habkost wrote:
> On Tue, Mar 20, 2012 at 12:53:57PM +0000, Liu, Jinsong wrote:
>> Rik van Riel wrote:
>>> On 03/09/2012 01:27 PM, Liu, Jinsong wrote:
>>> 
>>>> As for 'tsc deadline' feature exposing, my patch (as attached) just
>>>> obey qemu general cpuid exposing method, and also satisfied your
>>>> target I think.
>>> 
>>> One question.
>>> 
>>> Why is TSC_DEADLINE not exposed in the cpuid allowed feature
>>> bits in do_cpuid_ent() in arch/x86/kvm/x86.c ?
>>> 
>>>          /* cpuid 1.ecx */
>>>          const u32 kvm_supported_word4_x86_features =
>>>                  F(XMM3) | F(PCLMULQDQ) | 0 /* DTES64, MONITOR */ |
>>>                  0 /* DS-CPL, VMX, SMX, EST */ |
>>>                  0 /* TM2 */ | F(SSSE3) | 0 /* CNXT-ID */ | 0 /*
>>>                  Reserved */ | F(FMA) | F(CX16) | 0 /* xTPR Update,
>>>                  PDCM */ | 0 /* Reserved, DCA */ | F(XMM4_1) |
>>>                  F(XMM4_2) | F(X2APIC) | F(MOVBE) | F(POPCNT) |
>>>                  0 /* Reserved*/ | F(AES) | F(XSAVE) | 0 /* OSXSAVE
>>>                  */ | F(AVX) | F(F16C) | F(RDRAND);
>>> 
>>> Would it make sense to expose F(TSC_DEADLINE) above?
>>> 
>>> Or is there something truly special about tsc deadline
>>> that means it should be different from everything else?
>> 
>> Because the feature depends on KVM_CREATE_IRQCHIP, which we cannot
>> guarantee will be called, we expose it via a
>> KVM_CAP_TSC_DEADLINE_TIMER and not KVM_GET_SUPPORTED_CPUID.  
> 
> We have many other features that depend on proper support from
> userspace otherwise they wouldn't work, but are listed on
> GET_SUPPORTED_CPUID, don't we? Why is TSC-deadline special?
> 
> GET_SUPPORTED_CPUID just means "KVM supports it as long as userspace
> supports it too and enables it", it doesn't mean "CPUID bit that will
> be enabled by default"[1].
> 
>> Refer changeset 4d25a066b69fb749a39d0d4c610689dd765a0b0e.
> 
> That changeset was necessary because the kernel was doing this on
> update_cpu
> 
> 	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
> 		best->function == 0x1) {
> 		best->ecx |= bit(X86_FEATURE_TSC_DEADLINE_TIMER);
> 
> And that was really wrong, because it enabled the bit unconditionally.
> But I don't understand why KVM_CAP_TSC_DEADLINE_TIMER was created if
> we already have KVM_GET_SUPPORTED_CPUID to tell userspace which bits
> are supported by KVM.
> 

Yes, exactly. That's why we need this patch.

> 
> [1] From Documentation/virtual/kvm/api.txt:
> 
> "KVM_GET_SUPPORTED_CPUID
> [...]
> This ioctl returns x86 cpuid features which are supported by both the
> hardware and kvm.  Userspace can use the information returned by this
> ioctl to construct cpuid information (for KVM_SET_CPUID2) that is
> consistent with hardware, kernel, and userspace capabilities, and with
>                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^
> user requirements (for example, the user may wish to constrain cpuid
> to emulate older hardware, or for feature consistency across a
> cluster)." 

The fixbug patch is implemented by Jan and Avi, I reply per my understanding.

I think for tsc deadline timer feature, KVM_CAP_TSC_DEADLINE_TIMER is slightly better than KVM_GET_SUPPORTED_CPUID. If use KVM_GET_SUPPORTED_CPUID, it means tsc deadline features bind to host cpuid, while it fact it could be pure software emulated by kvm (though currently it implemented as bound to hareware). For the sake of extension, it choose KVM_CAP_TSC_DEADLINE_TIMER.

Thanks,
Jinsong

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Eduardo Habkost]

On Fri, Mar 23, 2012 at 03:49:27AM +0000, Liu, Jinsong wrote:
> Eduardo Habkost wrote:
> > [1] From Documentation/virtual/kvm/api.txt:
> > 
> > "KVM_GET_SUPPORTED_CPUID
> > [...]
> > This ioctl returns x86 cpuid features which are supported by both the
> > hardware and kvm.  Userspace can use the information returned by this
> > ioctl to construct cpuid information (for KVM_SET_CPUID2) that is
> > consistent with hardware, kernel, and userspace capabilities, and with
> >                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > user requirements (for example, the user may wish to constrain cpuid
> > to emulate older hardware, or for feature consistency across a
> > cluster)." 
> 
> The fixbug patch is implemented by Jan and Avi, I reply per my understanding.

No problem. I hope Jan or Avi can clarify this.

> 
> I think for tsc deadline timer feature, KVM_CAP_TSC_DEADLINE_TIMER is
> slightly better than KVM_GET_SUPPORTED_CPUID. If use
> KVM_GET_SUPPORTED_CPUID, it means tsc deadline features bind to host
> cpuid, while it fact it could be pure software emulated by kvm (though
> currently it implemented as bound to hareware). For the sake of
> extension, it choose KVM_CAP_TSC_DEADLINE_TIMER.

There's no requirement for GET_SUPPORTED_CPUID to be a subset of the
host CPU features. If KVM can completely emulate the feature by
software, then it can return the feature on GET_SUPPORTED_CPUID even if
the host CPU doesn't have the feature. That's the case for x2apic, for
example (see commit 0d1de2d901f4ba0972a3886496a44fb1d3300dbd).

-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

Eduardo Habkost wrote:
> On Fri, Mar 23, 2012 at 03:49:27AM +0000, Liu, Jinsong wrote:
>> Eduardo Habkost wrote:
>>> [1] From Documentation/virtual/kvm/api.txt:
>>> 
>>> "KVM_GET_SUPPORTED_CPUID
>>> [...]
>>> This ioctl returns x86 cpuid features which are supported by both
>>> the hardware and kvm.  Userspace can use the information returned
>>> by this ioctl to construct cpuid information (for KVM_SET_CPUID2)
>>> that is consistent with hardware, kernel, and userspace
>>>                                   capabilities, and with
>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^ 
>>> user requirements (for example, the user may wish to constrain cpuid
>>> to emulate older hardware, or for feature consistency across a
>>> cluster)."
>> 
>> The fixbug patch is implemented by Jan and Avi, I reply per my
>> understanding. 
> 
> No problem. I hope Jan or Avi can clarify this.
> 
>> 
>> I think for tsc deadline timer feature, KVM_CAP_TSC_DEADLINE_TIMER is
>> slightly better than KVM_GET_SUPPORTED_CPUID. If use
>> KVM_GET_SUPPORTED_CPUID, it means tsc deadline features bind to host
>> cpuid, while it fact it could be pure software emulated by kvm
>> (though currently it implemented as bound to hareware). For the sake
>> of 
>> extension, it choose KVM_CAP_TSC_DEADLINE_TIMER.
> 
> There's no requirement for GET_SUPPORTED_CPUID to be a subset of the
> host CPU features. If KVM can completely emulate the feature by
> software, then it can return the feature on GET_SUPPORTED_CPUID even
> if the host CPU doesn't have the feature. That's the case for x2apic,
> for example (see commit 0d1de2d901f4ba0972a3886496a44fb1d3300dbd).


Jan/Avi,

Could you elaborate more your thought? 

Thanks,
Jinsong

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

Eduardo, Jan

I will update tsc deadline timer patch (at qemu-kvm side) recently.
Have you made a final agreement of the issue 'KVM_CAP_TSC_DEADLINE_TIMER' vs. 'GET_SUPPORTED_CPUID'?

Thanks,
Jinsong


Eduardo Habkost wrote:
> (CCing Andre Przywara, in case he can help to clarify what's the
> expected meaning of "-cpu host")
> 
> On Tue, Apr 24, 2012 at 06:06:55PM +0200, Jan Kiszka wrote:
>> On 2012-04-23 22:02, Eduardo Habkost wrote:
>>> On Mon, Apr 23, 2012 at 06:31:25PM +0200, Jan Kiszka wrote:
>>>> However, that was how I interpreted this GET_SUPPORTED_CPUID. In
>>>> fact, 
>>>> it is used as "kernel or hardware does not _prevent_" already. And
>>>> in that sense, it's ok to enable even features that are not in
>>>> kernel/hardware hands. We should point out this fact in the
>>>> documentation. 
>>> 
>>> I see GET_SUPPORTED_CPUID as just a "what userspace can enable
>>> because the kernel and the hardware support it (= don't prevent
>>> it), as long as userspace has the required support" (meaning A+B).
>>> It's a bit like KVM_CHECK_EXTENSION, but with the nice feature that
>>> that the capabilities map directly to CPUID bits.
>>> 
>>> So, it's not clear to me: now you are OK with adding TSC_DEADLINE
>>> to GET_SUPPORTED_CPUID? 
>>> 
>>> But we still have the issue of "-cpu host" not knowing what can be
>>> safely enabled (without userspace feature-specific setup code), or
>>> not. Do you have any suggestion for that? Avi, do you have any
>>> suggestion? 
>> 
>> First of all, I bet this was already broken with the introduction of
>> x2apic. So TSC deadline won't make it worse. I guess we need to
>> address this in userspace, first by masking those features out,
>> later by actually emulating them.
> 
> I am not sure I understand what you are proposing. Let me explain the
> use case I am thinking about:
> 
> - Feature FOO is of type (A) (e.g. just a new instruction set that
>   doesn't require additional userspace support)
> - User has a Qemu vesion that doesn't know anything about feature FOO
> - User gets a new CPU that supports feature FOO
> - User gets a new kernel that supports feature FOO (i.e. has FOO in
>   GET_SUPPORTED_CPUID)
> - User does _not_ upgrade Qemu.
> - User expects to get feature FOO enabled if using "-cpu host",
>   without upgrading Qemu.
> 
> The problem here is: to support the above use-case, userspace need a
> probing mechanism that can differentiate _new_ (previously unknown)
> features that are in group (A) (safe to blindly enable) from features
> that are in group (B) (that can't be enabled without an userspace
> upgrade).
> 
> In short, it becomes a problem if we consider the following case:
> 
> - Feature BAR is of type (B) (it can't be enabled without extra
>   userspace support)
> - User has a Qemu version that doesn't know anything about feature BAR
> - User gets a new CPU that supports feature BAR
> - User gets a new kernel that supports feature BAR (i.e. has BAR in
>   GET_SUPPORTED_CPUID)
> - User does _not_ upgrade Qemu.
> - User simply shouldn't get feature BAR enabled, even if using "-cpu
>   host", otherwise Qemu would break.
> 
> If userspace always limited itself to features it knows about, it
> would be really easy to implement the feature without any new probing
> mechanism from the kernel. But that's not how I think users expect
> "-cpu host" to work. Maybe I am wrong, I don't know. I am CCing
> Andre, who introduced the "-cpu host" feature, in case he can explain
> what's the expected semantics on the cases above.
> 
>> 
>>> 
>>> And I still don't know the answer to:
>>> 
>>>>> - How to precisely define the groups (A) and (B)?
>>>>>   - "requires additional code only if migration is required"
>>>>>     qualifies as (B) or (A)?
>>> 
>>> 
>>> Re: documentation, isn't the following paragraph (already present
>>> on api.txt) sufficient? 
>>> 
>>> "The entries returned are the host cpuid as returned by the cpuid
>>> instruction, with unknown or unsupported features masked out.  Some
>>> features (for example, x2apic), may not be present in the host cpu,
>>> but are exposed by kvm if it can emulate them efficiently."
>> 
>> That suggests such features are always emulated - which is not true.
>> They are either emulated, or nothing _prevents_ their emulation by
>> user space.
> 
> Well... it's a bit more complicated than that: the current semantics
> are a bit more than "doesn't prevent", as in theory every single
> feature can be emulated by userspace, without any help from the
> kernel. So, if "doesn't prevent" were the only criteria, the kernel
> would set every single feature bit on GET_SUPPORTED_CPUID, making it
> not very useful. 
> 
> At least in the case of x2apic, the kernel is using
> GET_SUPPORTED_CPUID to expose a _capability_ too: when x2apic is
> present on GET_SUPPORTED_CPUID, userspace knows that in addition to
> "not preventing" the feature from being enabled, the kernel is now
> able to emulate x2apic (if proper setup is made by userspace). A
> kernel that can't emulate x2apic (even if userspace was allowed to
> emulate it completely in userspace) would never have x2apic enabled on
> GET_SUPPORTED_CPUID.
> 
> Like I said previously, in the end GET_SUPPORTED_CPUID is just a
> capability querying mechanism like KVM_CHECK_EXTENSION (where each
> extension have a specific kernel-capability meaning), but with the
> nice feature of being automatically mapped to CPUID bits (with no
> need for additional KVM_CAP_* => CPUID translation in userspace).

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Eduardo Habkost]

On Thu, Jun 14, 2012 at 07:02:03PM +0000, Liu, Jinsong wrote:
> Eduardo, Jan
> 
> I will update tsc deadline timer patch (at qemu-kvm side) recently.
> Have you made a final agreement of the issue 'KVM_CAP_TSC_DEADLINE_TIMER' vs. 'GET_SUPPORTED_CPUID'?

I don't think there's a final agreement, but I was convinced later that
it's probably better to _not_ have TSC-deadline on GET_SUPPORTED_CPUID,
at least not by default.

Even if this is changed in the future, it's a good idea to make qemu
support the KVM_CAP_TSC_DEADLINE_TIMER method if running on older
kernels, anyway.


> Eduardo Habkost wrote:
> > (CCing Andre Przywara, in case he can help to clarify what's the
> > expected meaning of "-cpu host")
> > 
> > On Tue, Apr 24, 2012 at 06:06:55PM +0200, Jan Kiszka wrote:
> >> On 2012-04-23 22:02, Eduardo Habkost wrote:
> >>> On Mon, Apr 23, 2012 at 06:31:25PM +0200, Jan Kiszka wrote:
> >>>> However, that was how I interpreted this GET_SUPPORTED_CPUID. In
> >>>> fact, 
> >>>> it is used as "kernel or hardware does not _prevent_" already. And
> >>>> in that sense, it's ok to enable even features that are not in
> >>>> kernel/hardware hands. We should point out this fact in the
> >>>> documentation. 
> >>> 
> >>> I see GET_SUPPORTED_CPUID as just a "what userspace can enable
> >>> because the kernel and the hardware support it (= don't prevent
> >>> it), as long as userspace has the required support" (meaning A+B).
> >>> It's a bit like KVM_CHECK_EXTENSION, but with the nice feature that
> >>> that the capabilities map directly to CPUID bits.
> >>> 
> >>> So, it's not clear to me: now you are OK with adding TSC_DEADLINE
> >>> to GET_SUPPORTED_CPUID? 
> >>> 
> >>> But we still have the issue of "-cpu host" not knowing what can be
> >>> safely enabled (without userspace feature-specific setup code), or
> >>> not. Do you have any suggestion for that? Avi, do you have any
> >>> suggestion? 
> >> 
> >> First of all, I bet this was already broken with the introduction of
> >> x2apic. So TSC deadline won't make it worse. I guess we need to
> >> address this in userspace, first by masking those features out,
> >> later by actually emulating them.
> > 
> > I am not sure I understand what you are proposing. Let me explain the
> > use case I am thinking about:
> > 
> > - Feature FOO is of type (A) (e.g. just a new instruction set that
> >   doesn't require additional userspace support)
> > - User has a Qemu vesion that doesn't know anything about feature FOO
> > - User gets a new CPU that supports feature FOO
> > - User gets a new kernel that supports feature FOO (i.e. has FOO in
> >   GET_SUPPORTED_CPUID)
> > - User does _not_ upgrade Qemu.
> > - User expects to get feature FOO enabled if using "-cpu host",
> >   without upgrading Qemu.
> > 
> > The problem here is: to support the above use-case, userspace need a
> > probing mechanism that can differentiate _new_ (previously unknown)
> > features that are in group (A) (safe to blindly enable) from features
> > that are in group (B) (that can't be enabled without an userspace
> > upgrade).
> > 
> > In short, it becomes a problem if we consider the following case:
> > 
> > - Feature BAR is of type (B) (it can't be enabled without extra
> >   userspace support)
> > - User has a Qemu version that doesn't know anything about feature BAR
> > - User gets a new CPU that supports feature BAR
> > - User gets a new kernel that supports feature BAR (i.e. has BAR in
> >   GET_SUPPORTED_CPUID)
> > - User does _not_ upgrade Qemu.
> > - User simply shouldn't get feature BAR enabled, even if using "-cpu
> >   host", otherwise Qemu would break.
> > 
> > If userspace always limited itself to features it knows about, it
> > would be really easy to implement the feature without any new probing
> > mechanism from the kernel. But that's not how I think users expect
> > "-cpu host" to work. Maybe I am wrong, I don't know. I am CCing
> > Andre, who introduced the "-cpu host" feature, in case he can explain
> > what's the expected semantics on the cases above.
> > 
> >> 
> >>> 
> >>> And I still don't know the answer to:
> >>> 
> >>>>> - How to precisely define the groups (A) and (B)?
> >>>>>   - "requires additional code only if migration is required"
> >>>>>     qualifies as (B) or (A)?
> >>> 
> >>> 
> >>> Re: documentation, isn't the following paragraph (already present
> >>> on api.txt) sufficient? 
> >>> 
> >>> "The entries returned are the host cpuid as returned by the cpuid
> >>> instruction, with unknown or unsupported features masked out.  Some
> >>> features (for example, x2apic), may not be present in the host cpu,
> >>> but are exposed by kvm if it can emulate them efficiently."
> >> 
> >> That suggests such features are always emulated - which is not true.
> >> They are either emulated, or nothing _prevents_ their emulation by
> >> user space.
> > 
> > Well... it's a bit more complicated than that: the current semantics
> > are a bit more than "doesn't prevent", as in theory every single
> > feature can be emulated by userspace, without any help from the
> > kernel. So, if "doesn't prevent" were the only criteria, the kernel
> > would set every single feature bit on GET_SUPPORTED_CPUID, making it
> > not very useful. 
> > 
> > At least in the case of x2apic, the kernel is using
> > GET_SUPPORTED_CPUID to expose a _capability_ too: when x2apic is
> > present on GET_SUPPORTED_CPUID, userspace knows that in addition to
> > "not preventing" the feature from being enabled, the kernel is now
> > able to emulate x2apic (if proper setup is made by userspace). A
> > kernel that can't emulate x2apic (even if userspace was allowed to
> > emulate it completely in userspace) would never have x2apic enabled on
> > GET_SUPPORTED_CPUID.
> > 
> > Like I said previously, in the end GET_SUPPORTED_CPUID is just a
> > capability querying mechanism like KVM_CHECK_EXTENSION (where each
> > extension have a specific kernel-capability meaning), but with the
> > nice feature of being automatically mapped to CPUID bits (with no
> > need for additional KVM_CAP_* => CPUID translation in userspace).
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

Eduardo Habkost wrote:
> On Thu, Jun 14, 2012 at 07:02:03PM +0000, Liu, Jinsong wrote:
>> Eduardo, Jan
>> 
>> I will update tsc deadline timer patch (at qemu-kvm side) recently.
>> Have you made a final agreement of the issue
>> 'KVM_CAP_TSC_DEADLINE_TIMER' vs. 'GET_SUPPORTED_CPUID'? 
> 
> I don't think there's a final agreement, but I was convinced later
> that it's probably better to _not_ have TSC-deadline on
> GET_SUPPORTED_CPUID, at least not by default.
> 
> Even if this is changed in the future, it's a good idea to make qemu
> support the KVM_CAP_TSC_DEADLINE_TIMER method if running on older
> kernels, anyway.

OK, so I will coding based on current KVM_CAP_TSC_DEADLINE_TIMER method.

Thanks for clarifying!


> 
> 
>> Eduardo Habkost wrote:
>>> (CCing Andre Przywara, in case he can help to clarify what's the
>>> expected meaning of "-cpu host")
>>> 
>>> On Tue, Apr 24, 2012 at 06:06:55PM +0200, Jan Kiszka wrote:
>>>> On 2012-04-23 22:02, Eduardo Habkost wrote:
>>>>> On Mon, Apr 23, 2012 at 06:31:25PM +0200, Jan Kiszka wrote:
>>>>>> However, that was how I interpreted this GET_SUPPORTED_CPUID. In
>>>>>> fact, it is used as "kernel or hardware does not _prevent_"
>>>>>> already. And in that sense, it's ok to enable even features that
>>>>>> are not in kernel/hardware hands. We should point out this fact
>>>>>> in the documentation.
>>>>> 
>>>>> I see GET_SUPPORTED_CPUID as just a "what userspace can enable
>>>>> because the kernel and the hardware support it (= don't prevent
>>>>> it), as long as userspace has the required support" (meaning A+B).
>>>>> It's a bit like KVM_CHECK_EXTENSION, but with the nice feature
>>>>> that that the capabilities map directly to CPUID bits.
>>>>> 
>>>>> So, it's not clear to me: now you are OK with adding TSC_DEADLINE
>>>>> to GET_SUPPORTED_CPUID? 
>>>>> 
>>>>> But we still have the issue of "-cpu host" not knowing what can be
>>>>> safely enabled (without userspace feature-specific setup code), or
>>>>> not. Do you have any suggestion for that? Avi, do you have any
>>>>> suggestion?
>>>> 
>>>> First of all, I bet this was already broken with the introduction
>>>> of x2apic. So TSC deadline won't make it worse. I guess we need to
>>>> address this in userspace, first by masking those features out,
>>>> later by actually emulating them.
>>> 
>>> I am not sure I understand what you are proposing. Let me explain
>>> the use case I am thinking about: 
>>> 
>>> - Feature FOO is of type (A) (e.g. just a new instruction set that
>>>   doesn't require additional userspace support)
>>> - User has a Qemu vesion that doesn't know anything about feature
>>> FOO 
>>> - User gets a new CPU that supports feature FOO
>>> - User gets a new kernel that supports feature FOO (i.e. has FOO in
>>> GET_SUPPORTED_CPUID) 
>>> - User does _not_ upgrade Qemu.
>>> - User expects to get feature FOO enabled if using "-cpu host",  
>>> without upgrading Qemu. 
>>> 
>>> The problem here is: to support the above use-case, userspace need a
>>> probing mechanism that can differentiate _new_ (previously unknown)
>>> features that are in group (A) (safe to blindly enable) from
>>> features that are in group (B) (that can't be enabled without an
>>> userspace upgrade). 
>>> 
>>> In short, it becomes a problem if we consider the following case:
>>> 
>>> - Feature BAR is of type (B) (it can't be enabled without extra  
>>> userspace support) 
>>> - User has a Qemu version that doesn't know anything about feature
>>> BAR 
>>> - User gets a new CPU that supports feature BAR
>>> - User gets a new kernel that supports feature BAR (i.e. has BAR in
>>> GET_SUPPORTED_CPUID) 
>>> - User does _not_ upgrade Qemu.
>>> - User simply shouldn't get feature BAR enabled, even if using "-cpu
>>>   host", otherwise Qemu would break.
>>> 
>>> If userspace always limited itself to features it knows about, it
>>> would be really easy to implement the feature without any new
>>> probing mechanism from the kernel. But that's not how I think users
>>> expect "-cpu host" to work. Maybe I am wrong, I don't know. I am
>>> CCing Andre, who introduced the "-cpu host" feature, in case he can
>>> explain what's the expected semantics on the cases above.
>>> 
>>>> 
>>>>> 
>>>>> And I still don't know the answer to:
>>>>> 
>>>>>>> - How to precisely define the groups (A) and (B)?
>>>>>>>   - "requires additional code only if migration is required"
>>>>>>>     qualifies as (B) or (A)?
>>>>> 
>>>>> 
>>>>> Re: documentation, isn't the following paragraph (already present
>>>>> on api.txt) sufficient? 
>>>>> 
>>>>> "The entries returned are the host cpuid as returned by the cpuid
>>>>> instruction, with unknown or unsupported features masked out. 
>>>>> Some features (for example, x2apic), may not be present in the
>>>>> host cpu, but are exposed by kvm if it can emulate them
>>>>> efficiently." 
>>>> 
>>>> That suggests such features are always emulated - which is not
>>>> true. They are either emulated, or nothing _prevents_ their
>>>> emulation by user space.
>>> 
>>> Well... it's a bit more complicated than that: the current semantics
>>> are a bit more than "doesn't prevent", as in theory every single
>>> feature can be emulated by userspace, without any help from the
>>> kernel. So, if "doesn't prevent" were the only criteria, the kernel
>>> would set every single feature bit on GET_SUPPORTED_CPUID, making
>>> it not very useful. 
>>> 
>>> At least in the case of x2apic, the kernel is using
>>> GET_SUPPORTED_CPUID to expose a _capability_ too: when x2apic is
>>> present on GET_SUPPORTED_CPUID, userspace knows that in addition to
>>> "not preventing" the feature from being enabled, the kernel is now
>>> able to emulate x2apic (if proper setup is made by userspace). A
>>> kernel that can't emulate x2apic (even if userspace was allowed to
>>> emulate it completely in userspace) would never have x2apic enabled
>>> on GET_SUPPORTED_CPUID. 
>>> 
>>> Like I said previously, in the end GET_SUPPORTED_CPUID is just a
>>> capability querying mechanism like KVM_CHECK_EXTENSION (where each
>>> extension have a specific kernel-capability meaning), but with the
>>> nice feature of being automatically mapped to CPUID bits (with no
>>> need for additional KVM_CAP_* => CPUID translation in userspace).