[Qemu-devel] [PATCH RESEND v4 00/18] target-i386: CPU feature flag queue

[Qemu-devel] [PATCH RESEND v4 00/18] target-i386: CPU feature flag queue

[Eduardo Habkost]

(Resending due to complete lack of feedback on v4 submission from 15 days ago.)

This started as a TCG vs KVM feature flag code cleanup, but now it is a queue
which includes other feature-flag-related patches that depend on each other.

Changes v3 -> v4:
 * New patch: target-i386: kvm: Don't enable MONITOR by default on any CPU model
 * New patch: target-i386: Add "migratable" property to "host" CPU model
 * New patch: target-i386: Set migratable=yes by default
 * New patch: savevm: check vmsd for migratability status
 * New patch: target-i386: Loop-based copying and setting/unsetting of feature words
 * Patch changed to use the new .migratable_flags field:
   * target-i386: support "invariant tsc" flag

Changes v2 -> v3:
 * Rebase after QEMU v2.0.0 (onto commit 2d03b49)
 * Added new patch: target-i386: support "invariant tsc" flag
 * Added new patch: target-i386: Support "-cpu host" in TCG mode

Changes v1 -> v2:
 * Rebase to latest qom-cpu (commit 90c5d39c)

Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Andreas Färber <afaerber@suse.de>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Marcelo Tosatti <mtosatti@redhat.com>

Eduardo Habkost (15):
  target-i386: kvm: Don't enable MONITOR by default on any CPU model
  target-i386: Simplify reporting of unavailable features
  target-i386: Merge feature filtering/checking functions
  target-i386: Pass FeatureWord argument to
    report_unavailable_features()
  target-i386: Isolate KVM-specific code on CPU feature filtering logic
  target-i386: Make TCG feature filtering more readable
  target-i386: Filter FEAT_7_0_EBX TCG features too
  target-i386: Filter KVM and 0xC0000001 features on TCG
  target-i386: Define TCG_*_FEATURES earlier on cpu.c
  target-i386: Loop-based copying and setting/unsetting of feature words
  target-i386: Loop-based feature word filtering in TCG mode
  target-i386: Support check/enforce flags in TCG mode, too
  target-i386: Support "-cpu host" in TCG mode
  target-i386: Add "migratable" property to "host" CPU model
  target-i386: Set migratable=yes by default

Marcelo Tosatti (3):
  savevm: check vmsd for migratability status
  target-i386: block migration and savevm if invariant tsc is exposed
  target-i386: support "invariant tsc" flag

 savevm.c              |   5 +-
 target-i386/cpu-qom.h |   7 +-
 target-i386/cpu.c     | 358 ++++++++++++++++++++++++++++++--------------------
 target-i386/cpu.h     |   4 +
 target-i386/kvm.c     |  13 ++
 target-i386/machine.c |   2 +-
 6 files changed, 240 insertions(+), 149 deletions(-)

-- 
1.9.0

[Qemu-devel] [PATCH RESEND v4 01/18] target-i386: kvm: Don't enable MONITOR by default on any CPU model

[Eduardo Habkost]

KVM never supported the MONITOR flag so it doesn't make sense to have it
enabled by default when KVM is enabled.

The rationale here is similar to the cases where it makes sense to have
a feature enabled by default on all CPU models when on KVM mode (e.g.
x2apic). In this case we are having a feature disabled by default for
the same reasons.

In this case we don't need machine-type compat code because it is
currently impossible to run a KVM VM with the MONITOR flag set.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8f193a9..694348e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -372,6 +372,12 @@ static uint32_t kvm_default_features[FEATURE_WORDS] = {
     [FEAT_1_ECX] = CPUID_EXT_X2APIC,
 };
 
+/* Features that are not added by default to any CPU model when KVM is enabled.
+ */
+static uint32_t kvm_default_unset_features[FEATURE_WORDS] = {
+    [FEAT_1_ECX] = CPUID_EXT_MONITOR,
+};
+
 void x86_cpu_compat_disable_kvm_features(FeatureWord w, uint32_t features)
 {
     kvm_default_features[w] &= ~features;
@@ -1893,6 +1899,7 @@ static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp)
         FeatureWord w;
         for (w = 0; w < FEATURE_WORDS; w++) {
             env->features[w] |= kvm_default_features[w];
+            env->features[w] &= ~kvm_default_unset_features[w];
         }
     }
 
-- 
1.9.0

[Qemu-devel] [PATCH RESEND v4 02/18] target-i386: Simplify reporting of unavailable features

[Eduardo Habkost]

Instead of checking and calling unavailable_host_feature() once for each
bit, simply call the function (now renamed to
report_unavailable_features()) once for each feature word.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
 * Rebase to latest qom-cpu (commit 90c5d39c)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
---
 target-i386/cpu.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 694348e..3c4f327 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1236,11 +1236,11 @@ static const TypeInfo host_x86_cpu_type_info = {
 
 #endif
 
-static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
+static int report_unavailable_features(FeatureWordInfo *f, uint32_t mask)
 {
     int i;
 
-    for (i = 0; i < 32; ++i)
+    for (i = 0; i < 32; ++i) {
         if (1 << i & mask) {
             const char *reg = get_register_name_32(f->cpuid_reg);
             assert(reg);
@@ -1249,8 +1249,8 @@ static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
                 f->cpuid_eax, reg,
                 f->feat_names[i] ? "." : "",
                 f->feat_names[i] ? f->feat_names[i] : "", i);
-            break;
         }
+    }
     return 0;
 }
 
@@ -1274,12 +1274,10 @@ static int kvm_check_features_against_host(KVMState *s, X86CPU *cpu)
         uint32_t host_feat = kvm_arch_get_supported_cpuid(s, wi->cpuid_eax,
                                                              wi->cpuid_ecx,
                                                              wi->cpuid_reg);
-        uint32_t mask;
-        for (mask = 1; mask; mask <<= 1) {
-            if (guest_feat & mask && !(host_feat & mask)) {
-                unavailable_host_feature(wi, mask);
-                rv = 1;
-            }
+        uint32_t unavailable_features = guest_feat & ~host_feat;
+        if (unavailable_features) {
+            report_unavailable_features(wi, unavailable_features);
+            rv = 1;
         }
     }
     return rv;
-- 
1.9.0

[Qemu-devel] [PATCH RESEND v4 03/18] target-i386: Merge feature filtering/checking functions

[Eduardo Habkost]

Merge filter_features_for_kvm() and kvm_check_features_against_host().

Both functions made exactly the same calculations, the only difference
was that filter_features_for_kvm() changed the bits on cpu->features[],
and kvm_check_features_against_host() did error reporting.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
 * Trivial rebase to latest qom-cpu (commit 90c5d39c)
   (Reviewed-by line kept)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
   (Reviewed-by line kept)
---
 target-i386/cpu.c | 53 +++++++++++++++++++----------------------------------
 1 file changed, 19 insertions(+), 34 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3c4f327..370da81 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1254,35 +1254,6 @@ static int report_unavailable_features(FeatureWordInfo *f, uint32_t mask)
     return 0;
 }
 
-/* Check if all requested cpu flags are making their way to the guest
- *
- * Returns 0 if all flags are supported by the host, non-zero otherwise.
- *
- * This function may be called only if KVM is enabled.
- */
-static int kvm_check_features_against_host(KVMState *s, X86CPU *cpu)
-{
-    CPUX86State *env = &cpu->env;
-    int rv = 0;
-    FeatureWord w;
-
-    assert(kvm_enabled());
-
-    for (w = 0; w < FEATURE_WORDS; w++) {
-        FeatureWordInfo *wi = &feature_word_info[w];
-        uint32_t guest_feat = env->features[w];
-        uint32_t host_feat = kvm_arch_get_supported_cpuid(s, wi->cpuid_eax,
-                                                             wi->cpuid_ecx,
-                                                             wi->cpuid_reg);
-        uint32_t unavailable_features = guest_feat & ~host_feat;
-        if (unavailable_features) {
-            report_unavailable_features(wi, unavailable_features);
-            rv = 1;
-        }
-    }
-    return rv;
-}
-
 static void x86_cpuid_version_get_family(Object *obj, Visitor *v, void *opaque,
                                          const char *name, Error **errp)
 {
@@ -1849,11 +1820,20 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
     return cpu_list;
 }
 
-static void filter_features_for_kvm(X86CPU *cpu)
+/* Filters CPU feature words based on host availability of each feature
+ *
+ * Returns 0 if all flags are supported by the host, non-zero otherwise.
+ *
+ * This function may be called only if KVM is enabled.
+ */
+static int filter_features_for_kvm(X86CPU *cpu)
 {
     CPUX86State *env = &cpu->env;
     KVMState *s = kvm_state;
     FeatureWord w;
+    int rv = 0;
+
+    assert(kvm_enabled());
 
     for (w = 0; w < FEATURE_WORDS; w++) {
         FeatureWordInfo *wi = &feature_word_info[w];
@@ -1863,7 +1843,15 @@ static void filter_features_for_kvm(X86CPU *cpu)
         uint32_t requested_features = env->features[w];
         env->features[w] &= host_feat;
         cpu->filtered_features[w] = requested_features & ~env->features[w];
+        if (cpu->filtered_features[w]) {
+            if (cpu->check_cpuid || cpu->enforce_cpuid) {
+                report_unavailable_features(wi, cpu->filtered_features[w]);
+            }
+            rv = 1;
+        }
     }
+
+    return rv;
 }
 
 /* Load data from X86CPUDefinition
@@ -2612,14 +2600,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES;
         env->features[FEAT_SVM] &= TCG_SVM_FEATURES;
     } else {
-        KVMState *s = kvm_state;
-        if ((cpu->check_cpuid || cpu->enforce_cpuid)
-            && kvm_check_features_against_host(s, cpu) && cpu->enforce_cpuid) {
+        if (filter_features_for_kvm(cpu) && cpu->enforce_cpuid) {
             error_setg(&local_err,
                        "Host's CPU doesn't support requested features");
             goto out;
         }
-        filter_features_for_kvm(cpu);
     }
 
 #ifndef CONFIG_USER_ONLY
-- 
1.9.0

[Qemu-devel] [PATCH RESEND v4 04/18] target-i386: Pass FeatureWord argument to report_unavailable_features()

[Eduardo Habkost]

This will help us simplify the code that calls
report_unavailable_features() later.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
 * Rebase to latest qom-cpu (commit 90c5d39c)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
---
 target-i386/cpu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 370da81..b097c0d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1236,8 +1236,9 @@ static const TypeInfo host_x86_cpu_type_info = {
 
 #endif
 
-static int report_unavailable_features(FeatureWordInfo *f, uint32_t mask)
+static int report_unavailable_features(FeatureWord w, uint32_t mask)
 {
+    FeatureWordInfo *f = &feature_word_info[w];
     int i;
 
     for (i = 0; i < 32; ++i) {
@@ -1845,7 +1846,7 @@ static int filter_features_for_kvm(X86CPU *cpu)
         cpu->filtered_features[w] = requested_features & ~env->features[w];
         if (cpu->filtered_features[w]) {
             if (cpu->check_cpuid || cpu->enforce_cpuid) {
-                report_unavailable_features(wi, cpu->filtered_features[w]);
+                report_unavailable_features(w, cpu->filtered_features[w]);
             }
             rv = 1;
         }
-- 
1.9.0

[Qemu-devel] [PATCH RESEND v4 05/18] target-i386: Isolate KVM-specific code on CPU feature filtering logic

[Eduardo Habkost]

This will allow us to re-use the feature filtering logic (and the
check/enforce flag logic) for TCG.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
 * Trivial rebase to latest qom-cpu (commit 90c5d39c)
   (Reviewed-by line kept)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
   (Reviewed-by line kept)
---
 target-i386/cpu.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index b097c0d..4dd522a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1821,26 +1821,29 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
     return cpu_list;
 }
 
+static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w)
+{
+    FeatureWordInfo *wi = &feature_word_info[w];
+    assert(kvm_enabled());
+    return kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid_eax,
+                                                   wi->cpuid_ecx,
+                                                   wi->cpuid_reg);
+}
+
 /* Filters CPU feature words based on host availability of each feature
  *
  * Returns 0 if all flags are supported by the host, non-zero otherwise.
  *
  * This function may be called only if KVM is enabled.
  */
-static int filter_features_for_kvm(X86CPU *cpu)
+static int x86_cpu_filter_features(X86CPU *cpu)
 {
     CPUX86State *env = &cpu->env;
-    KVMState *s = kvm_state;
     FeatureWord w;
     int rv = 0;
 
-    assert(kvm_enabled());
-
     for (w = 0; w < FEATURE_WORDS; w++) {
-        FeatureWordInfo *wi = &feature_word_info[w];
-        uint32_t host_feat = kvm_arch_get_supported_cpuid(s, wi->cpuid_eax,
-                                                             wi->cpuid_ecx,
-                                                             wi->cpuid_reg);
+        uint32_t host_feat = x86_cpu_get_supported_feature_word(w);
         uint32_t requested_features = env->features[w];
         env->features[w] &= host_feat;
         cpu->filtered_features[w] = requested_features & ~env->features[w];
@@ -2601,7 +2604,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES;
         env->features[FEAT_SVM] &= TCG_SVM_FEATURES;
     } else {
-        if (filter_features_for_kvm(cpu) && cpu->enforce_cpuid) {
+        if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) {
             error_setg(&local_err,
                        "Host's CPU doesn't support requested features");
             goto out;
-- 
1.9.0

[Qemu-devel] [PATCH RESEND v4 06/18] target-i386: Make TCG feature filtering more readable

[Eduardo Habkost]

Instead of an #ifdef in the middle of the code, just set
TCG_EXT2_FEATURES to a different value depending on TARGET_X86_64.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
 * Trivial rebase to latest qom-cpu (commit 90c5d39c)
   (Reviewed-by line kept)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
   (Reviewed-by line kept)
---
 target-i386/cpu.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4dd522a..1a1e390 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -572,9 +572,17 @@ struct X86CPUDefinition {
           CPUID_EXT_X2APIC, CPUID_EXT_TSC_DEADLINE_TIMER, CPUID_EXT_XSAVE,
           CPUID_EXT_OSXSAVE, CPUID_EXT_AVX, CPUID_EXT_F16C,
           CPUID_EXT_RDRAND */
+
+#ifdef TARGET_X86_64
+#define TCG_EXT2_X86_64_FEATURES (CPUID_EXT2_SYSCALL | CPUID_EXT2_LM)
+#else
+#define TCG_EXT2_X86_64_FEATURES 0
+#endif
+
 #define TCG_EXT2_FEATURES ((TCG_FEATURES & CPUID_EXT2_AMD_ALIASES) | \
           CPUID_EXT2_NX | CPUID_EXT2_MMXEXT | CPUID_EXT2_RDTSCP | \
-          CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT)
+          CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT | \
+          TCG_EXT2_X86_64_FEATURES)
           /* missing:
           CPUID_EXT2_PDPE1GB */
 #define TCG_EXT3_FEATURES (CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | \
@@ -2596,11 +2604,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
     if (!kvm_enabled()) {
         env->features[FEAT_1_EDX] &= TCG_FEATURES;
         env->features[FEAT_1_ECX] &= TCG_EXT_FEATURES;
-        env->features[FEAT_8000_0001_EDX] &= (TCG_EXT2_FEATURES
-#ifdef TARGET_X86_64
-            | CPUID_EXT2_SYSCALL | CPUID_EXT2_LM
-#endif
-            );
+        env->features[FEAT_8000_0001_EDX] &= TCG_EXT2_FEATURES;
         env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES;
         env->features[FEAT_SVM] &= TCG_SVM_FEATURES;
     } else {
-- 
1.9.0

[Qemu-devel] [PATCH RESEND v4 07/18] target-i386: Filter FEAT_7_0_EBX TCG features too

[Eduardo Habkost]

The TCG_7_0_EBX_FEATURES macro was defined but never used (it even had a
typo that was never noticed). Make the existing TCG feature filtering
code use it.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
 * Trivial rebase to latest qom-cpu (commit 90c5d39c)
   (Reviewed-by line kept)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
   (Reviewed-by line kept)
---
 target-i386/cpu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1a1e390..8d0ae12 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -588,7 +588,7 @@ struct X86CPUDefinition {
 #define TCG_EXT3_FEATURES (CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | \
           CPUID_EXT3_CR8LEG | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A)
 #define TCG_SVM_FEATURES 0
-#define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP \
+#define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP | \
           CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ADX)
           /* missing:
           CPUID_7_0_EBX_FSGSBASE, CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2,
@@ -2604,6 +2604,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
     if (!kvm_enabled()) {
         env->features[FEAT_1_EDX] &= TCG_FEATURES;
         env->features[FEAT_1_ECX] &= TCG_EXT_FEATURES;
+        env->features[FEAT_7_0_EBX] &= TCG_7_0_EBX_FEATURES;
         env->features[FEAT_8000_0001_EDX] &= TCG_EXT2_FEATURES;
         env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES;
         env->features[FEAT_SVM] &= TCG_SVM_FEATURES;
-- 
1.9.0

[Qemu-devel] [PATCH RESEND v4 08/18] target-i386: Filter KVM and 0xC0000001 features on TCG

[Eduardo Habkost]

TCG doesn't support any of the feature flags on FEAT_KVM and
FEAT_C000_0001_EDX feature words, so clear all bits on those feature
words.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
 * Trivial rebase to latest qom-cpu (commit 90c5d39c)
   (Reviewed-by line kept)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
   (Reviewed-by line kept)
---
 target-i386/cpu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8d0ae12..0e9f3ea 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -587,7 +587,9 @@ struct X86CPUDefinition {
           CPUID_EXT2_PDPE1GB */
 #define TCG_EXT3_FEATURES (CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | \
           CPUID_EXT3_CR8LEG | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A)
+#define TCG_EXT4_FEATURES 0
 #define TCG_SVM_FEATURES 0
+#define TCG_KVM_FEATURES 0
 #define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP | \
           CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ADX)
           /* missing:
@@ -2608,6 +2610,8 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         env->features[FEAT_8000_0001_EDX] &= TCG_EXT2_FEATURES;
         env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES;
         env->features[FEAT_SVM] &= TCG_SVM_FEATURES;
+        env->features[FEAT_KVM] &= TCG_KVM_FEATURES;
+        env->features[FEAT_C000_0001_EDX] &= TCG_EXT4_FEATURES;
     } else {
         if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) {
             error_setg(&local_err,
-- 
1.9.0

[Qemu-devel] [PATCH RESEND v4 09/18] target-i386: Define TCG_*_FEATURES earlier on cpu.c

[Eduardo Habkost]

Those macros will be used in the feature_word_info array data, so need
to be defined earlier.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
 * Rebase to latest qom-cpu (commit 90c5d39c)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
---
 target-i386/cpu.c | 117 +++++++++++++++++++++++++++---------------------------
 1 file changed, 59 insertions(+), 58 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0e9f3ea..ccd05ad 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -262,6 +262,65 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
     NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 };
 
+#define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
+#define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
+          CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC)
+#define PENTIUM2_FEATURES (PENTIUM_FEATURES | CPUID_PAE | CPUID_SEP | \
+          CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \
+          CPUID_PSE36 | CPUID_FXSR)
+#define PENTIUM3_FEATURES (PENTIUM2_FEATURES | CPUID_SSE)
+#define PPRO_FEATURES (CPUID_FP87 | CPUID_DE | CPUID_PSE | CPUID_TSC | \
+          CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
+          CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
+          CPUID_PAE | CPUID_SEP | CPUID_APIC)
+
+#define TCG_FEATURES (CPUID_FP87 | CPUID_PSE | CPUID_TSC | CPUID_MSR | \
+          CPUID_PAE | CPUID_MCE | CPUID_CX8 | CPUID_APIC | CPUID_SEP | \
+          CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \
+          CPUID_PSE36 | CPUID_CLFLUSH | CPUID_ACPI | CPUID_MMX | \
+          CPUID_FXSR | CPUID_SSE | CPUID_SSE2 | CPUID_SS)
+          /* partly implemented:
+          CPUID_MTRR, CPUID_MCA, CPUID_CLFLUSH (needed for Win64)
+          CPUID_PSE36 (needed for Solaris) */
+          /* missing:
+          CPUID_VME, CPUID_DTS, CPUID_SS, CPUID_HT, CPUID_TM, CPUID_PBE */
+#define TCG_EXT_FEATURES (CPUID_EXT_SSE3 | CPUID_EXT_PCLMULQDQ | \
+          CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 | CPUID_EXT_CX16 | \
+          CPUID_EXT_SSE41 | CPUID_EXT_SSE42 | CPUID_EXT_POPCNT | \
+          CPUID_EXT_MOVBE | CPUID_EXT_AES | CPUID_EXT_HYPERVISOR)
+          /* missing:
+          CPUID_EXT_DTES64, CPUID_EXT_DSCPL, CPUID_EXT_VMX, CPUID_EXT_SMX,
+          CPUID_EXT_EST, CPUID_EXT_TM2, CPUID_EXT_CID, CPUID_EXT_FMA,
+          CPUID_EXT_XTPR, CPUID_EXT_PDCM, CPUID_EXT_PCID, CPUID_EXT_DCA,
+          CPUID_EXT_X2APIC, CPUID_EXT_TSC_DEADLINE_TIMER, CPUID_EXT_XSAVE,
+          CPUID_EXT_OSXSAVE, CPUID_EXT_AVX, CPUID_EXT_F16C,
+          CPUID_EXT_RDRAND */
+
+#ifdef TARGET_X86_64
+#define TCG_EXT2_X86_64_FEATURES (CPUID_EXT2_SYSCALL | CPUID_EXT2_LM)
+#else
+#define TCG_EXT2_X86_64_FEATURES 0
+#endif
+
+#define TCG_EXT2_FEATURES ((TCG_FEATURES & CPUID_EXT2_AMD_ALIASES) | \
+          CPUID_EXT2_NX | CPUID_EXT2_MMXEXT | CPUID_EXT2_RDTSCP | \
+          CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT | \
+          TCG_EXT2_X86_64_FEATURES)
+          /* missing:
+          CPUID_EXT2_PDPE1GB */
+#define TCG_EXT3_FEATURES (CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | \
+          CPUID_EXT3_CR8LEG | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A)
+#define TCG_EXT4_FEATURES 0
+#define TCG_SVM_FEATURES 0
+#define TCG_KVM_FEATURES 0
+#define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP | \
+          CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ADX)
+          /* missing:
+          CPUID_7_0_EBX_FSGSBASE, CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2,
+          CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM,
+          CPUID_7_0_EBX_RDSEED */
+
+
 typedef struct FeatureWordInfo {
     const char **feat_names;
     uint32_t cpuid_eax;   /* Input EAX for CPUID */
@@ -539,64 +598,6 @@ struct X86CPUDefinition {
     bool cache_info_passthrough;
 };
 
-#define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
-#define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
-          CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC)
-#define PENTIUM2_FEATURES (PENTIUM_FEATURES | CPUID_PAE | CPUID_SEP | \
-          CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \
-          CPUID_PSE36 | CPUID_FXSR)
-#define PENTIUM3_FEATURES (PENTIUM2_FEATURES | CPUID_SSE)
-#define PPRO_FEATURES (CPUID_FP87 | CPUID_DE | CPUID_PSE | CPUID_TSC | \
-          CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
-          CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
-          CPUID_PAE | CPUID_SEP | CPUID_APIC)
-
-#define TCG_FEATURES (CPUID_FP87 | CPUID_PSE | CPUID_TSC | CPUID_MSR | \
-          CPUID_PAE | CPUID_MCE | CPUID_CX8 | CPUID_APIC | CPUID_SEP | \
-          CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \
-          CPUID_PSE36 | CPUID_CLFLUSH | CPUID_ACPI | CPUID_MMX | \
-          CPUID_FXSR | CPUID_SSE | CPUID_SSE2 | CPUID_SS)
-          /* partly implemented:
-          CPUID_MTRR, CPUID_MCA, CPUID_CLFLUSH (needed for Win64)
-          CPUID_PSE36 (needed for Solaris) */
-          /* missing:
-          CPUID_VME, CPUID_DTS, CPUID_SS, CPUID_HT, CPUID_TM, CPUID_PBE */
-#define TCG_EXT_FEATURES (CPUID_EXT_SSE3 | CPUID_EXT_PCLMULQDQ | \
-          CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 | CPUID_EXT_CX16 | \
-          CPUID_EXT_SSE41 | CPUID_EXT_SSE42 | CPUID_EXT_POPCNT | \
-          CPUID_EXT_MOVBE | CPUID_EXT_AES | CPUID_EXT_HYPERVISOR)
-          /* missing:
-          CPUID_EXT_DTES64, CPUID_EXT_DSCPL, CPUID_EXT_VMX, CPUID_EXT_SMX,
-          CPUID_EXT_EST, CPUID_EXT_TM2, CPUID_EXT_CID, CPUID_EXT_FMA,
-          CPUID_EXT_XTPR, CPUID_EXT_PDCM, CPUID_EXT_PCID, CPUID_EXT_DCA,
-          CPUID_EXT_X2APIC, CPUID_EXT_TSC_DEADLINE_TIMER, CPUID_EXT_XSAVE,
-          CPUID_EXT_OSXSAVE, CPUID_EXT_AVX, CPUID_EXT_F16C,
-          CPUID_EXT_RDRAND */
-
-#ifdef TARGET_X86_64
-#define TCG_EXT2_X86_64_FEATURES (CPUID_EXT2_SYSCALL | CPUID_EXT2_LM)
-#else
-#define TCG_EXT2_X86_64_FEATURES 0
-#endif
-
-#define TCG_EXT2_FEATURES ((TCG_FEATURES & CPUID_EXT2_AMD_ALIASES) | \
-          CPUID_EXT2_NX | CPUID_EXT2_MMXEXT | CPUID_EXT2_RDTSCP | \
-          CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT | \
-          TCG_EXT2_X86_64_FEATURES)
-          /* missing:
-          CPUID_EXT2_PDPE1GB */
-#define TCG_EXT3_FEATURES (CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | \
-          CPUID_EXT3_CR8LEG | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A)
-#define TCG_EXT4_FEATURES 0
-#define TCG_SVM_FEATURES 0
-#define TCG_KVM_FEATURES 0
-#define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP | \
-          CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ADX)
-          /* missing:
-          CPUID_7_0_EBX_FSGSBASE, CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2,
-          CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM,
-          CPUID_7_0_EBX_RDSEED */
-
 static X86CPUDefinition builtin_x86_defs[] = {
     {
         .name = "qemu64",
-- 
1.9.0

[Qemu-devel] [PATCH RESEND v4 10/18] target-i386: Loop-based copying and setting/unsetting of feature words

[Eduardo Habkost]

Now that we have the feature word arrays, we don't need to manually copy
each array item, we can simply iterate through each feature word.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 35 ++++++++++-------------------------
 1 file changed, 10 insertions(+), 25 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ccd05ad..816adc2 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1652,6 +1652,7 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features,
 {
     X86CPU *cpu = X86_CPU(cs);
     char *featurestr; /* Single 'key=value" string being parsed */
+    FeatureWord w;
     /* Features to be added */
     FeatureWordArray plus_features = { 0 };
     /* Features to be removed */
@@ -1731,22 +1732,11 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features,
         }
         featurestr = strtok(NULL, ",");
     }
-    env->features[FEAT_1_EDX] |= plus_features[FEAT_1_EDX];
-    env->features[FEAT_1_ECX] |= plus_features[FEAT_1_ECX];
-    env->features[FEAT_8000_0001_EDX] |= plus_features[FEAT_8000_0001_EDX];
-    env->features[FEAT_8000_0001_ECX] |= plus_features[FEAT_8000_0001_ECX];
-    env->features[FEAT_C000_0001_EDX] |= plus_features[FEAT_C000_0001_EDX];
-    env->features[FEAT_KVM] |= plus_features[FEAT_KVM];
-    env->features[FEAT_SVM] |= plus_features[FEAT_SVM];
-    env->features[FEAT_7_0_EBX] |= plus_features[FEAT_7_0_EBX];
-    env->features[FEAT_1_EDX] &= ~minus_features[FEAT_1_EDX];
-    env->features[FEAT_1_ECX] &= ~minus_features[FEAT_1_ECX];
-    env->features[FEAT_8000_0001_EDX] &= ~minus_features[FEAT_8000_0001_EDX];
-    env->features[FEAT_8000_0001_ECX] &= ~minus_features[FEAT_8000_0001_ECX];
-    env->features[FEAT_C000_0001_EDX] &= ~minus_features[FEAT_C000_0001_EDX];
-    env->features[FEAT_KVM] &= ~minus_features[FEAT_KVM];
-    env->features[FEAT_SVM] &= ~minus_features[FEAT_SVM];
-    env->features[FEAT_7_0_EBX] &= ~minus_features[FEAT_7_0_EBX];
+
+    for (w = 0; w < FEATURE_WORDS; w++) {
+        env->features[w] |= plus_features[w];
+        env->features[w] &= ~minus_features[w];
+    }
 
 out:
     return;
@@ -1876,24 +1866,19 @@ static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp)
     CPUX86State *env = &cpu->env;
     const char *vendor;
     char host_vendor[CPUID_VENDOR_SZ + 1];
+    FeatureWord w;
 
     object_property_set_int(OBJECT(cpu), def->level, "level", errp);
     object_property_set_int(OBJECT(cpu), def->family, "family", errp);
     object_property_set_int(OBJECT(cpu), def->model, "model", errp);
     object_property_set_int(OBJECT(cpu), def->stepping, "stepping", errp);
-    env->features[FEAT_1_EDX] = def->features[FEAT_1_EDX];
-    env->features[FEAT_1_ECX] = def->features[FEAT_1_ECX];
-    env->features[FEAT_8000_0001_EDX] = def->features[FEAT_8000_0001_EDX];
-    env->features[FEAT_8000_0001_ECX] = def->features[FEAT_8000_0001_ECX];
     object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", errp);
-    env->features[FEAT_KVM] = def->features[FEAT_KVM];
-    env->features[FEAT_SVM] = def->features[FEAT_SVM];
-    env->features[FEAT_C000_0001_EDX] = def->features[FEAT_C000_0001_EDX];
-    env->features[FEAT_7_0_EBX] = def->features[FEAT_7_0_EBX];
     env->cpuid_xlevel2 = def->xlevel2;
     cpu->cache_info_passthrough = def->cache_info_passthrough;
-
     object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
+    for (w = 0; w < FEATURE_WORDS; w++) {
+        env->features[w] = def->features[w];
+    }
 
     /* Special cases not set in the X86CPUDefinition structs: */
     if (kvm_enabled()) {
-- 
1.9.0

[Qemu-devel] [PATCH RESEND v4 11/18] target-i386: Loop-based feature word filtering in TCG mode

[Eduardo Habkost]

Instead of manually filtering each feature word, add a tcg_features
field to FeatureWordInfo, and use that field to filter all feature words
in TCG mode.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
 * Trivial rebase to latest qom-cpu (commit 90c5d39c)
   (Reviewed-by line kept)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
   (Reviewed-by line kept)
---
 target-i386/cpu.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 816adc2..8ebf8c5 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -327,42 +327,51 @@ typedef struct FeatureWordInfo {
     bool cpuid_needs_ecx; /* CPUID instruction uses ECX as input */
     uint32_t cpuid_ecx;   /* Input ECX value for CPUID */
     int cpuid_reg;        /* output register (R_* constant) */
+    uint32_t tcg_features; /* Feature flags supported by TCG */
 } FeatureWordInfo;
 
 static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
     [FEAT_1_EDX] = {
         .feat_names = feature_name,
         .cpuid_eax = 1, .cpuid_reg = R_EDX,
+        .tcg_features = TCG_FEATURES,
     },
     [FEAT_1_ECX] = {
         .feat_names = ext_feature_name,
         .cpuid_eax = 1, .cpuid_reg = R_ECX,
+        .tcg_features = TCG_EXT_FEATURES,
     },
     [FEAT_8000_0001_EDX] = {
         .feat_names = ext2_feature_name,
         .cpuid_eax = 0x80000001, .cpuid_reg = R_EDX,
+        .tcg_features = TCG_EXT2_FEATURES,
     },
     [FEAT_8000_0001_ECX] = {
         .feat_names = ext3_feature_name,
         .cpuid_eax = 0x80000001, .cpuid_reg = R_ECX,
+        .tcg_features = TCG_EXT3_FEATURES,
     },
     [FEAT_C000_0001_EDX] = {
         .feat_names = ext4_feature_name,
         .cpuid_eax = 0xC0000001, .cpuid_reg = R_EDX,
+        .tcg_features = TCG_EXT4_FEATURES,
     },
     [FEAT_KVM] = {
         .feat_names = kvm_feature_name,
         .cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EAX,
+        .tcg_features = TCG_KVM_FEATURES,
     },
     [FEAT_SVM] = {
         .feat_names = svm_feature_name,
         .cpuid_eax = 0x8000000A, .cpuid_reg = R_EDX,
+        .tcg_features = TCG_SVM_FEATURES,
     },
     [FEAT_7_0_EBX] = {
         .feat_names = cpuid_7_0_ebx_feature_name,
         .cpuid_eax = 7,
         .cpuid_needs_ecx = true, .cpuid_ecx = 0,
         .cpuid_reg = R_EBX,
+        .tcg_features = TCG_7_0_EBX_FEATURES,
     },
 };
 
@@ -2590,14 +2599,10 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
     }
 
     if (!kvm_enabled()) {
-        env->features[FEAT_1_EDX] &= TCG_FEATURES;
-        env->features[FEAT_1_ECX] &= TCG_EXT_FEATURES;
-        env->features[FEAT_7_0_EBX] &= TCG_7_0_EBX_FEATURES;
-        env->features[FEAT_8000_0001_EDX] &= TCG_EXT2_FEATURES;
-        env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES;
-        env->features[FEAT_SVM] &= TCG_SVM_FEATURES;
-        env->features[FEAT_KVM] &= TCG_KVM_FEATURES;
-        env->features[FEAT_C000_0001_EDX] &= TCG_EXT4_FEATURES;
+        FeatureWord w;
+        for (w = 0; w < FEATURE_WORDS; w++) {
+            env->features[w] &= feature_word_info[w].tcg_features;
+        }
     } else {
         if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) {
             error_setg(&local_err,
-- 
1.9.0

[Qemu-devel] [PATCH RESEND v4 12/18] target-i386: Support check/enforce flags in TCG mode, too

[Eduardo Habkost]

If enforce/check is specified in TCG mode, QEMU will ensure all CPU
features are supported by TCG, so no CPU feature is silently disabled.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
 * Trivial rebase to latest qom-cpu (commit 90c5d39c)
   (Reviewed-by line kept)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
   (Reviewed-by line kept)
---
 target-i386/cpu.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8ebf8c5..d3c1663 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1265,8 +1265,9 @@ static int report_unavailable_features(FeatureWord w, uint32_t mask)
         if (1 << i & mask) {
             const char *reg = get_register_name_32(f->cpuid_reg);
             assert(reg);
-            fprintf(stderr, "warning: host doesn't support requested feature: "
+            fprintf(stderr, "warning: %s doesn't support requested feature: "
                 "CPUID.%02XH:%s%s%s [bit %d]\n",
+                kvm_enabled() ? "host" : "TCG",
                 f->cpuid_eax, reg,
                 f->feat_names[i] ? "." : "",
                 f->feat_names[i] ? f->feat_names[i] : "", i);
@@ -1834,17 +1835,18 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
 static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w)
 {
     FeatureWordInfo *wi = &feature_word_info[w];
-    assert(kvm_enabled());
-    return kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid_eax,
-                                                   wi->cpuid_ecx,
-                                                   wi->cpuid_reg);
+    if (kvm_enabled()) {
+        return kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid_eax,
+                                                       wi->cpuid_ecx,
+                                                       wi->cpuid_reg);
+    } else {
+        return wi->tcg_features;
+    }
 }
 
 /* Filters CPU feature words based on host availability of each feature
  *
  * Returns 0 if all flags are supported by the host, non-zero otherwise.
- *
- * This function may be called only if KVM is enabled.
  */
 static int x86_cpu_filter_features(X86CPU *cpu)
 {
@@ -2598,17 +2600,13 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
            & CPUID_EXT2_AMD_ALIASES);
     }
 
-    if (!kvm_enabled()) {
-        FeatureWord w;
-        for (w = 0; w < FEATURE_WORDS; w++) {
-            env->features[w] &= feature_word_info[w].tcg_features;
-        }
-    } else {
-        if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) {
-            error_setg(&local_err,
-                       "Host's CPU doesn't support requested features");
-            goto out;
-        }
+
+    if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) {
+        error_setg(&local_err,
+                   kvm_enabled() ?
+                       "Host doesn't support requested features" :
+                       "TCG doesn't support requested features");
+        goto out;
     }
 
 #ifndef CONFIG_USER_ONLY
-- 
1.9.0

[Qemu-devel] [PATCH RESEND v4 13/18] target-i386: Support "-cpu host" in TCG mode

[Eduardo Habkost]

As "-cpu host" simply means "enable every bit that can be enabled on
this host", we can emulate similar behavior even if KVM is not enabled.
We just need to set all feature bits supported by TCG, accordingly.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v2:
 * Coding style fix (break long lines)
---
 target-i386/cpu.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index d3c1663..77d6d3c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -274,6 +274,16 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
           CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
           CPUID_PAE | CPUID_SEP | CPUID_APIC)
 
+/* Maximum CPUID level values for TCG: */
+
+/* CPUID level 7 is needed for TCG_7_0_EBX_FEATURES */
+#define TCG_MAX_LEVEL    7
+/* 0x8000000A is needed for CPUID_EXT3_SVM */
+#define TCG_MAX_XLEVEL   0x8000000A
+/* TCG_EXT4_FEATURES is 0 */
+#define TCG_MAX_XLEVEL2  0
+
+
 #define TCG_FEATURES (CPUID_FP87 | CPUID_PSE | CPUID_TSC | CPUID_MSR | \
           CPUID_PAE | CPUID_MCE | CPUID_CX8 | CPUID_APIC | CPUID_SEP | \
           CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \
@@ -1205,8 +1215,6 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void *data)
     X86CPUClass *xcc = X86_CPU_CLASS(oc);
     uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
 
-    xcc->kvm_required = true;
-
     host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
     x86_cpu_vendor_words2str(host_cpudef.vendor, ebx, edx, ecx);
 
@@ -1225,6 +1233,8 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void *data)
      */
 }
 
+static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w);
+
 static void host_x86_cpu_initfn(Object *obj)
 {
     X86CPU *cpu = X86_CPU(obj);
@@ -1232,17 +1242,22 @@ static void host_x86_cpu_initfn(Object *obj)
     KVMState *s = kvm_state;
     FeatureWord w;
 
-    assert(kvm_enabled());
-
-    env->cpuid_level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
-    env->cpuid_xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
-    env->cpuid_xlevel2 = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
+    if (kvm_enabled()) {
+        env->cpuid_level =
+            kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
+        env->cpuid_xlevel =
+            kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
+        env->cpuid_xlevel2 =
+            kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
+    } else {
+        env->cpuid_level = TCG_MAX_LEVEL;
+        env->cpuid_xlevel = TCG_MAX_XLEVEL;
+        env->cpuid_xlevel2 = TCG_MAX_XLEVEL2;
+    }
 
     for (w = 0; w < FEATURE_WORDS; w++) {
-        FeatureWordInfo *wi = &feature_word_info[w];
         env->features[w] =
-            kvm_arch_get_supported_cpuid(s, wi->cpuid_eax, wi->cpuid_ecx,
-                                         wi->cpuid_reg);
+            x86_cpu_get_supported_feature_word(w);
     }
     object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);
 }
-- 
1.9.0

[Qemu-devel] [PATCH RESEND v4 14/18] target-i386: Add "migratable" property to "host" CPU model

[Eduardo Habkost]

This flag will allow the user to choose between two modes:
 * All flags that can be enabled on the host, even if unmigratable
   (migratable=no);
 * All flags that can be enabled on the host, known to QEMU,
   and migratable (migratable=yes).

The default is still migratable=false, to keep current behavior, but
this will be changed to migratable=true by another patch.

My plan was to support the "migratable" flag on all CPU classes, but
have the default to "false" on all CPU models except "host". However,
DeviceClass has no mechanism to allow a child class to have a different
property default from the parent class yet, so by now only the "host"
CPU model will support the "migratable" flag.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu-qom.h |  5 +++++
 target-i386/cpu.c     | 52 +++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index e9b3d57..016f90d 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -87,6 +87,11 @@ typedef struct X86CPU {
     bool hyperv_time;
     bool check_cpuid;
     bool enforce_cpuid;
+    /* If set, only migratable flags will be accepted when "enforce" mode is
+     * used, and only migratable flags will be included in the "host"
+     * CPU model.
+     */
+    bool migratable;
 
     /* if true the CPUID code directly forward host cache leaves to the guest */
     bool cache_info_passthrough;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 77d6d3c..105006b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -338,6 +338,7 @@ typedef struct FeatureWordInfo {
     uint32_t cpuid_ecx;   /* Input ECX value for CPUID */
     int cpuid_reg;        /* output register (R_* constant) */
     uint32_t tcg_features; /* Feature flags supported by TCG */
+    uint32_t unmigratable_flags; /* Feature flags known to be unmigratable */
 } FeatureWordInfo;
 
 static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
@@ -461,6 +462,30 @@ void x86_cpu_compat_disable_kvm_features(FeatureWord w, uint32_t features)
     kvm_default_features[w] &= ~features;
 }
 
+/* Returns the set of feature flags that are supported and migratable by
+ * QEMU, for a given FeatureWord
+ */
+static uint32_t x86_cpu_get_migratable_flags(FeatureWord w)
+{
+    uint32_t r = 0;
+    int i;
+
+    FeatureWordInfo *wi = &feature_word_info[w];
+    for (i = 0; i < 32; i++) {
+        uint32_t f = 1U << i;
+        /* If the feature name is unknown, it is not supported by QEMU yet */
+        if (!wi->feat_names[i]) {
+            continue;
+        }
+        /* Skip features known to QEMU, but explicitly marked as unmigratable */
+        if (wi->unmigratable_flags & f) {
+            continue;
+        }
+        r |= f;
+    }
+    return r;
+}
+
 void host_cpuid(uint32_t function, uint32_t count,
                 uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx)
 {
@@ -1206,6 +1231,11 @@ static int cpu_x86_fill_model_id(char *str)
 
 static X86CPUDefinition host_cpudef;
 
+static Property x86_host_cpu_properties[] = {
+    DEFINE_PROP_BOOL("migratable", X86CPU, migratable, false),
+    DEFINE_PROP_END_OF_LIST()
+};
+
 /* class_init for the "host" CPU model
  *
  * This function may be called before KVM is initialized.
@@ -1213,6 +1243,7 @@ static X86CPUDefinition host_cpudef;
 static void host_x86_cpu_class_init(ObjectClass *oc, void *data)
 {
     X86CPUClass *xcc = X86_CPU_CLASS(oc);
+    DeviceClass *dc = DEVICE_CLASS(oc);
     uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
 
     host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
@@ -1228,12 +1259,14 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void *data)
     xcc->cpu_def = &host_cpudef;
     host_cpudef.cache_info_passthrough = true;
 
+    dc->props = x86_host_cpu_properties;
     /* level, xlevel, xlevel2, and the feature words are initialized on
      * instance_init, because they require KVM to be initialized.
      */
 }
 
-static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w);
+static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
+                                                   bool migratable_only);
 
 static void host_x86_cpu_initfn(Object *obj)
 {
@@ -1257,7 +1290,7 @@ static void host_x86_cpu_initfn(Object *obj)
 
     for (w = 0; w < FEATURE_WORDS; w++) {
         env->features[w] =
-            x86_cpu_get_supported_feature_word(w);
+            x86_cpu_get_supported_feature_word(w, cpu->migratable);
     }
     object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);
 }
@@ -1847,16 +1880,22 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
     return cpu_list;
 }
 
-static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w)
+static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
+                                                   bool migratable_only)
 {
     FeatureWordInfo *wi = &feature_word_info[w];
+    uint32_t r;
     if (kvm_enabled()) {
-        return kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid_eax,
+        r =  kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid_eax,
                                                        wi->cpuid_ecx,
                                                        wi->cpuid_reg);
     } else {
-        return wi->tcg_features;
+        r =  wi->tcg_features;
+    }
+    if (migratable_only) {
+        r &= x86_cpu_get_migratable_flags(w);
     }
+    return r;
 }
 
 /* Filters CPU feature words based on host availability of each feature
@@ -1870,7 +1909,8 @@ static int x86_cpu_filter_features(X86CPU *cpu)
     int rv = 0;
 
     for (w = 0; w < FEATURE_WORDS; w++) {
-        uint32_t host_feat = x86_cpu_get_supported_feature_word(w);
+        uint32_t host_feat =
+            x86_cpu_get_supported_feature_word(w, cpu->migratable);
         uint32_t requested_features = env->features[w];
         env->features[w] &= host_feat;
         cpu->filtered_features[w] = requested_features & ~env->features[w];
-- 
1.9.0

[Qemu-devel] [PATCH RESEND v4 15/18] target-i386: Set migratable=yes by default

[Eduardo Habkost]

Having only migratable flags reported by default on the "host" CPU model
is safer for the following reasons:

 * Existing users may expect "-cpu host" to be migration-safe, if they
   take care of always using compatible host CPUs, host kernels, and
   QEMU versions.
 * Users who don't care aboug migration and want to enable all features
   supported by the host kernel can simply change their setup to use
   migratable=no.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 105006b..d43209e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1232,7 +1232,7 @@ static int cpu_x86_fill_model_id(char *str)
 static X86CPUDefinition host_cpudef;
 
 static Property x86_host_cpu_properties[] = {
-    DEFINE_PROP_BOOL("migratable", X86CPU, migratable, false),
+    DEFINE_PROP_BOOL("migratable", X86CPU, migratable, true),
     DEFINE_PROP_END_OF_LIST()
 };
 
-- 
1.9.0

[Qemu-devel] [PATCH RESEND v4 16/18] savevm: check vmsd for migratability status

[Eduardo Habkost]

From: Marcelo Tosatti <mtosatti@redhat.com>

Check vmsd for unmigratable field, allowing migratibility status
to be modified after vmstate_register.

Cc: Juan Quintela <quintela@redhat.com>
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 savevm.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/savevm.c b/savevm.c
index da8aa24..c578e42 100644
--- a/savevm.c
+++ b/savevm.c
@@ -232,7 +232,6 @@ typedef struct SaveStateEntry {
     const VMStateDescription *vmsd;
     void *opaque;
     CompatEntry *compat;
-    int no_migrate;
     int is_ram;
 } SaveStateEntry;
 
@@ -292,7 +291,6 @@ int register_savevm_live(DeviceState *dev,
     se->ops = ops;
     se->opaque = opaque;
     se->vmsd = NULL;
-    se->no_migrate = 0;
     /* if this is a live_savem then set is_ram */
     if (ops->save_live_setup != NULL) {
         se->is_ram = 1;
@@ -383,7 +381,6 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
     se->opaque = opaque;
     se->vmsd = vmsd;
     se->alias_id = alias_id;
-    se->no_migrate = vmsd->unmigratable;
 
     if (dev) {
         char *id = qdev_get_dev_path(dev);
@@ -452,7 +449,7 @@ bool qemu_savevm_state_blocked(Error **errp)
     SaveStateEntry *se;
 
     QTAILQ_FOREACH(se, &savevm_handlers, entry) {
-        if (se->no_migrate) {
+        if (se->vmsd && se->vmsd->unmigratable) {
             error_setg(errp, "State blocked by non-migratable device '%s'",
                        se->idstr);
             return true;
-- 
1.9.0

[Qemu-devel] [PATCH RESEND v4 17/18] target-i386: block migration and savevm if invariant tsc is exposed

[Eduardo Habkost]

From: Marcelo Tosatti <mtosatti@redhat.com>

Invariant TSC documentation mentions that "invariant TSC will run at a
constant rate in all ACPI P-, C-. and T-states".

This is not the case if migration to a host with different TSC frequency
is allowed, or if savevm is performed. So block migration/savevm.

Cc: Juan Quintela <quintela@redhat.com>
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu-qom.h |  2 +-
 target-i386/kvm.c     | 13 +++++++++++++
 target-i386/machine.c |  2 +-
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 016f90d..473d803 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -121,7 +121,7 @@ static inline X86CPU *x86_env_get_cpu(CPUX86State *env)
 #define ENV_OFFSET offsetof(X86CPU, env)
 
 #ifndef CONFIG_USER_ONLY
-extern const struct VMStateDescription vmstate_x86_cpu;
+extern struct VMStateDescription vmstate_x86_cpu;
 #endif
 
 /**
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 4389959..99cc7e3 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -33,6 +33,8 @@
 #include "exec/ioport.h"
 #include <asm/hyperv.h>
 #include "hw/pci/pci.h"
+#include "migration/migration.h"
+#include "qapi/qmp/qerror.h"
 
 //#define DEBUG_KVM
 
@@ -447,6 +449,8 @@ static bool hyperv_enabled(X86CPU *cpu)
             cpu->hyperv_relaxed_timing);
 }
 
+Error *invtsc_mig_blocker;
+
 #define KVM_MAX_CPUID_ENTRIES  100
 
 int kvm_arch_init_vcpu(CPUState *cs)
@@ -702,6 +706,15 @@ int kvm_arch_init_vcpu(CPUState *cs)
                                   !!(c->ecx & CPUID_EXT_SMX);
     }
 
+    c = cpuid_find_entry(&cpuid_data.cpuid, 0x80000007, 0);
+    if (c && (c->edx & 1<<8) && invtsc_mig_blocker == NULL) {
+        /* for migration */
+        error_set(&invtsc_mig_blocker, QERR_MIGRATION_NOT_SUPPORTED, "cpu");
+        migrate_add_blocker(invtsc_mig_blocker);
+        /* for savevm */
+        vmstate_x86_cpu.unmigratable = 1;
+    }
+
     cpuid_data.cpuid.padding = 0;
     r = kvm_vcpu_ioctl(cs, KVM_SET_CPUID2, &cpuid_data);
     if (r) {
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 168cab6..4d4c023 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -613,7 +613,7 @@ static const VMStateDescription vmstate_msr_hyperv_time = {
     }
 };
 
-const VMStateDescription vmstate_x86_cpu = {
+VMStateDescription vmstate_x86_cpu = {
     .name = "cpu",
     .version_id = 12,
     .minimum_version_id = 3,
-- 
1.9.0

[Qemu-devel] [PATCH RESEND v4 18/18] target-i386: support "invariant tsc" flag

[Eduardo Habkost]

From: Marcelo Tosatti <mtosatti@redhat.com>

Expose "Invariant TSC" flag, if KVM is enabled. From Intel documentation:

17.13.1 Invariant TSC The time stamp counter in newer processors may
support an enhancement, referred to as invariant TSC. Processor’s
support for invariant TSC is indicated by CPUID.80000007H:EDX[8].
The invariant TSC will run at a constant rate in all ACPI P-, C-.
and T-states. This is the architectural behavior moving forward. On
processors with invariant TSC support, the OS may use the TSC for wall
clock timer services (instead of ACPI or HPET timers). TSC reads are
much more efficient and do not incur the overhead associated with a ring
transition or access to a platform resource.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
[ehabkost: redo feature filtering to use .tcg_features]
[ehabkost: add CPUID_APM_INVTSC macro, add it to .unmigratable_flags]
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 25 +++++++++++++++++++++++++
 target-i386/cpu.h |  4 ++++
 2 files changed, 29 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index d43209e..f8e7eb1 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -262,6 +262,17 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
     NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 };
 
+static const char *cpuid_apm_edx_feature_name[] = {
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL,
+    "invtsc", NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL,
+};
+
 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
 #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
           CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC)
@@ -329,6 +340,7 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
           CPUID_7_0_EBX_FSGSBASE, CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2,
           CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM,
           CPUID_7_0_EBX_RDSEED */
+#define TCG_APM_FEATURES 0
 
 
 typedef struct FeatureWordInfo {
@@ -384,6 +396,13 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
         .cpuid_reg = R_EBX,
         .tcg_features = TCG_7_0_EBX_FEATURES,
     },
+    [FEAT_8000_0007_EDX] = {
+        .feat_names = cpuid_apm_edx_feature_name,
+        .cpuid_eax = 0x80000007,
+        .cpuid_reg = R_EDX,
+        .tcg_features = TCG_APM_FEATURES,
+        .unmigratable_flags = CPUID_APM_INVTSC,
+    },
 };
 
 typedef struct X86RegisterInfo32 {
@@ -2400,6 +2419,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
                (AMD_ENC_ASSOC(L3_ASSOCIATIVITY) << 12) | \
                (L3_LINES_PER_TAG << 8) | (L3_LINE_SIZE);
         break;
+    case 0x80000007:
+        *eax = 0;
+        *ebx = 0;
+        *ecx = 0;
+        *edx = env->features[FEAT_8000_0007_EDX];
+        break;
     case 0x80000008:
         /* virtual & phys address size in low 2 bytes. */
 /* XXX: This value must match the one used in the MMU code. */
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 2a22a7d..1bb98e6 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -398,6 +398,7 @@ typedef enum FeatureWord {
     FEAT_7_0_EBX,       /* CPUID[EAX=7,ECX=0].EBX */
     FEAT_8000_0001_EDX, /* CPUID[8000_0001].EDX */
     FEAT_8000_0001_ECX, /* CPUID[8000_0001].ECX */
+    FEAT_8000_0007_EDX, /* CPUID[8000_0007].EDX */
     FEAT_C000_0001_EDX, /* CPUID[C000_0001].EDX */
     FEAT_KVM,           /* CPUID[4000_0001].EAX (KVM_CPUID_FEATURES) */
     FEAT_SVM,           /* CPUID[8000_000A].EDX */
@@ -557,6 +558,9 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
 #define CPUID_7_0_EBX_ADX      (1U << 19)
 #define CPUID_7_0_EBX_SMAP     (1U << 20)
 
+/* CPUID[0x80000007].EDX flags: */
+#define CPUID_APM_INVTSC       (1U << 8)
+
 #define CPUID_VENDOR_SZ      12
 
 #define CPUID_VENDOR_INTEL_1 0x756e6547 /* "Genu" */
-- 
1.9.0

Re: [Qemu-devel] [PATCH RESEND v4 00/18] target-i386: CPU feature flag queue

[Andreas Färber]

Am 14.05.2014 21:29, schrieb Eduardo Habkost:
> (Resending due to complete lack of feedback on v4 submission from 15 days ago.)

Marcelo had reminded me, and I had started review of the original v4,
but not through yet. It looks as if the resend changed nothing, so I'll
continue on that one.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH RESEND v4 16/18] savevm: check vmsd for migratability status

[Juan Quintela]

Eduardo Habkost <ehabkost@redhat.com> wrote:
> From: Marcelo Tosatti <mtosatti@redhat.com>
>
> Check vmsd for unmigratable field, allowing migratibility status
> to be modified after vmstate_register.
>
> Cc: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

I agree with the patch spirt, but what happens if we want to disable
migration in a device that has no VMSD?

Later, Juan.

Re: [Qemu-devel] [PATCH RESEND v4 17/18] target-i386: block migration and savevm if invariant tsc is exposed

[Juan Quintela]

Eduardo Habkost <ehabkost@redhat.com> wrote:
> From: Marcelo Tosatti <mtosatti@redhat.com>
>
> Invariant TSC documentation mentions that "invariant TSC will run at a
> constant rate in all ACPI P-, C-. and T-states".
>
> This is not the case if migration to a host with different TSC frequency
> is allowed, or if savevm is performed. So block migration/savevm.
>
> Cc: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

I don't have a better suggestion.  Really we could allow migration to
identical machines, but I assume that there is not a way to read the tsc
frequency?
(Althought reading the model name/numbers could be enough?)

I.e. Add a subsection that includes the cpu model name, or whatever we
can have to identify the host cpu?

Later, Juan.

Re: [Qemu-devel] [PATCH RESEND v4 16/18] savevm: check vmsd for migratability status

[Andreas Färber]

Am 15.05.2014 14:14, schrieb Juan Quintela:
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>> From: Marcelo Tosatti <mtosatti@redhat.com>
>>
>> Check vmsd for unmigratable field, allowing migratibility status
>> to be modified after vmstate_register.
>>
>> Cc: Juan Quintela <quintela@redhat.com>
>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> I agree with the patch spirt, but what happens if we want to disable
> migration in a device that has no VMSD?

As the patch shows, no non-VMSD device setting no_migrate exists today.
Do you expect any to appear? I'm rather seeing more and more devices
getting converted to VMSD.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH RESEND v4 16/18] savevm: check vmsd for migratability status

[Juan Quintela]

Andreas Färber <afaerber@suse.de> wrote:
> Am 15.05.2014 14:14, schrieb Juan Quintela:
>> Eduardo Habkost <ehabkost@redhat.com> wrote:
>>> From: Marcelo Tosatti <mtosatti@redhat.com>
>>>
>>> Check vmsd for unmigratable field, allowing migratibility status
>>> to be modified after vmstate_register.
>>>
>>> Cc: Juan Quintela <quintela@redhat.com>
>>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>>> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> 
>> I agree with the patch spirt, but what happens if we want to disable
>> migration in a device that has no VMSD?
>
> As the patch shows, no non-VMSD device setting no_migrate exists today.
> Do you expect any to appear? I'm rather seeing more and more devices
> getting converted to VMSD.

I want people to notice that.  As said, I hope that everything new uses
VMSD.

Later, Juan.

Re: [Qemu-devel] [PATCH RESEND v4 16/18] savevm: check vmsd for migratability status

[Eduardo Habkost]

On Thu, May 15, 2014 at 04:05:44PM +0200, Juan Quintela wrote:
> Andreas Färber <afaerber@suse.de> wrote:
> > Am 15.05.2014 14:14, schrieb Juan Quintela:
> >> Eduardo Habkost <ehabkost@redhat.com> wrote:
> >>> From: Marcelo Tosatti <mtosatti@redhat.com>
> >>>
> >>> Check vmsd for unmigratable field, allowing migratibility status
> >>> to be modified after vmstate_register.
> >>>
> >>> Cc: Juan Quintela <quintela@redhat.com>
> >>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> >>> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> >>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> >> 
> >> I agree with the patch spirt, but what happens if we want to disable
> >> migration in a device that has no VMSD?
> >
> > As the patch shows, no non-VMSD device setting no_migrate exists today.
> > Do you expect any to appear? I'm rather seeing more and more devices
> > getting converted to VMSD.
> 
> I want people to notice that.  As said, I hope that everything new uses
> VMSD.

Note that devices that have no VMSD couldn't set no_migrate before this
patch, anyway (the only way to set no_migrate was using a VMSD). The
change is not visible outside savevm.c.

If we want to disable migration in a device that has no VMSD, we will
need a savevm API change. If that ever happens, re-adding the field to
support the API change would be trivial.

-- 
Eduardo

Re: [Qemu-devel] [PATCH RESEND v4 17/18] target-i386: block migration and savevm if invariant tsc is exposed

[Marcelo Tosatti]

On Thu, May 15, 2014 at 02:17:11PM +0200, Juan Quintela wrote:
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> > From: Marcelo Tosatti <mtosatti@redhat.com>
> >
> > Invariant TSC documentation mentions that "invariant TSC will run at a
> > constant rate in all ACPI P-, C-. and T-states".
> >
> > This is not the case if migration to a host with different TSC frequency
> > is allowed, or if savevm is performed. So block migration/savevm.
> >
> > Cc: Juan Quintela <quintela@redhat.com>
> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> 
> I don't have a better suggestion.  Really we could allow migration to
> identical machines, but I assume that there is not a way to read the tsc
> frequency?
> (Althought reading the model name/numbers could be enough?)

Even if migration to identical machine is performed, you would have 
to perform timing of downtime to compensate.

> I.e. Add a subsection that includes the cpu model name, or whatever we
> can have to identify the host cpu?

"On processors with invariant TSC support, the OS may use the TSC for
wall clock timer services (instead of ACPI or HPET timers)."

Re: [Qemu-devel] [PATCH RESEND v4 17/18] target-i386: block migration and savevm if invariant tsc is exposed

[Amit Shah]

On (Wed) 14 May 2014 [16:30:09], Eduardo Habkost wrote:
> From: Marcelo Tosatti <mtosatti@redhat.com>
> 
> Invariant TSC documentation mentions that "invariant TSC will run at a
> constant rate in all ACPI P-, C-. and T-states".
> 
> This is not the case if migration to a host with different TSC frequency
> is allowed, or if savevm is performed. So block migration/savevm.

Can you add this to the release notes, please?

		Amit

Re: [Qemu-devel] [PATCH RESEND v4 17/18] target-i386: block migration and savevm if invariant tsc is exposed

[Eduardo Habkost]

On Tue, Jun 10, 2014 at 12:42:14PM +0530, Amit Shah wrote:
> On (Wed) 14 May 2014 [16:30:09], Eduardo Habkost wrote:
> > From: Marcelo Tosatti <mtosatti@redhat.com>
> > 
> > Invariant TSC documentation mentions that "invariant TSC will run at a
> > constant rate in all ACPI P-, C-. and T-states".
> > 
> > This is not the case if migration to a host with different TSC frequency
> > is allowed, or if savevm is performed. So block migration/savevm.
> 
> Can you add this to the release notes, please?

It can be mentioned as new feature, sure, but note that it shouldn't
affect any existing users, except the ones who decide to explicitly use
"-cpu ...,+invtsc" or "-cpu host,migratable=no" on the command line.

-- 
Eduardo

Re: [Qemu-devel] [PATCH RESEND v4 17/18] target-i386: block migration and savevm if invariant tsc is exposed

[Amit Shah]

On (Tue) 10 Jun 2014 [11:43:29], Eduardo Habkost wrote:
> On Tue, Jun 10, 2014 at 12:42:14PM +0530, Amit Shah wrote:
> > On (Wed) 14 May 2014 [16:30:09], Eduardo Habkost wrote:
> > > From: Marcelo Tosatti <mtosatti@redhat.com>
> > > 
> > > Invariant TSC documentation mentions that "invariant TSC will run at a
> > > constant rate in all ACPI P-, C-. and T-states".
> > > 
> > > This is not the case if migration to a host with different TSC frequency
> > > is allowed, or if savevm is performed. So block migration/savevm.
> > 
> > Can you add this to the release notes, please?
> 
> It can be mentioned as new feature, sure, but note that it shouldn't
> affect any existing users, except the ones who decide to explicitly use
> "-cpu ...,+invtsc" or "-cpu host,migratable=no" on the command line.

Ah, yes; won't break anything.  Just that a relnote would be good for
the new functionality being added.


		Amit