[Qemu-devel] [PATCH qom-cpu 00/15 v8] target-i386: convert CPU features into properties, part 1

[Qemu-devel] [PATCH qom-cpu 00/15 v8] target-i386: convert CPU features into properties, part 1

[Igor Mammedov]

It's a rebase of v7 on current qom-cpu tree, since then some patches from it
were applied to master. Convertion of feature bits is left for part 2
since it's not ready yet.

v7-v8:
* split out dynamic properties convertion patch into per property patches
  to simplify review
* drop feature bits convertion

v6-v7:
* convert globals check_cpuid, enforce_cpuid and  hyperv_* to fields of
  CPUState
* Make PropertyInfo-s static
* maintain legacy kvmclock semantic in cpu_x86_parse_featurestr()
* existing properties code are not moved around, just fixed signatures where
  it's needed and used visitors. 

v5-v6:
* when converting feature names to property names, replace '_' with '-'
* separate patches converting existing dynamic properties into one, were
  squashed into one [1/9] and change tested with virt-test(next).
* patches that were touching +-foo features are squashed into one [9/9],
  to avoid behavior change between them(f-kvmclock property).
* the rest of conversions were basicaly rebased on top of current qom-cpu-next
  tree, with small corrections

git for testing: https://github.com/imammedo/qemu/tree/x86-cpu-properties.v8

Igor Mammedov (15):
  target-i386: cpu: convert 'family' to static property
  target-i386: cpu: convert 'model' to static property
  target-i386: cpu: convert 'stepping' to static property
  target-i386: cpu: convert 'level' to static property
  target-i386: cpu: convert 'xlevel' to static property
  target-i386: cpu: convert 'vendor' to static property
  target-i386: cpu: convert 'model-id' to static property
  target-i386: cpu: convert 'tsc-frequency' to static property
  target-i386: move hyperv_* static globals to CPUState
  target-i386: convert 'hv_spinlocks' to static property
  target-i386: convert 'hv_relaxed' to static property
  target-i386: convert 'hv_vapic' to static property
  target-i386: convert 'check' and 'enforce' to static properties
  target-i386: cleanup 'foo' feature handling'
  target-i386: cleanup 'foo=val' feature handling

 target-i386/Makefile.objs |    2 +-
 target-i386/cpu.c         |  250 +++++++++++++++++++++++++++------------------
 target-i386/cpu.h         |    9 ++
 target-i386/hyperv.c      |   64 ------------
 target-i386/hyperv.h      |   45 --------
 target-i386/kvm.c         |   36 +++++--
 6 files changed, 188 insertions(+), 218 deletions(-)
 delete mode 100644 target-i386/hyperv.c
 delete mode 100644 target-i386/hyperv.h

[Qemu-devel] [PATCH 01/15] target-i386: cpu: convert 'family' to static property

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 target-i386/cpu.c |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1a501d9..c87cc9f 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1195,6 +1195,14 @@ static void x86_cpuid_version_set_family(Object *obj, Visitor *v, void *opaque,
     }
 }
 
+static PropertyInfo qdev_prop_family = {
+    .name  = "uint32",
+    .get   = x86_cpuid_version_get_family,
+    .set   = x86_cpuid_version_set_family,
+};
+#define DEFINE_PROP_FAMILY(_n)                                                 \
+    DEFINE_PROP(_n, X86CPU, env.cpuid_version, qdev_prop_family, uint32_t)
+
 static void x86_cpuid_version_get_model(Object *obj, Visitor *v, void *opaque,
                                         const char *name, Error **errp)
 {
@@ -1475,6 +1483,11 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
     error_propagate(errp, err);
 }
 
+static Property cpu_x86_properties[] = {
+    DEFINE_PROP_FAMILY("family"),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
 {
     x86_def_t *def;
@@ -2451,9 +2464,6 @@ static void x86_cpu_initfn(Object *obj)
     cs->env_ptr = env;
     cpu_exec_init(env);
 
-    object_property_add(obj, "family", "int",
-                        x86_cpuid_version_get_family,
-                        x86_cpuid_version_set_family, NULL, NULL, NULL);
     object_property_add(obj, "model", "int",
                         x86_cpuid_version_get_model,
                         x86_cpuid_version_set_model, NULL, NULL, NULL);
@@ -2514,6 +2524,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
     xcc->parent_realize = dc->realize;
     dc->realize = x86_cpu_realizefn;
     dc->bus_type = TYPE_ICC_BUS;
+    dc->props = cpu_x86_properties;
 
     xcc->parent_reset = cc->reset;
     cc->reset = x86_cpu_reset;
-- 
1.7.1

[Qemu-devel] [PATCH 02/15] target-i386: cpu: convert 'model' to static property

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 target-i386/cpu.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index c87cc9f..5d379af 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1238,6 +1238,14 @@ static void x86_cpuid_version_set_model(Object *obj, Visitor *v, void *opaque,
     env->cpuid_version |= ((value & 0xf) << 4) | ((value >> 4) << 16);
 }
 
+static PropertyInfo qdev_prop_model = {
+    .name  = "uint32",
+    .get   = x86_cpuid_version_get_model,
+    .set   = x86_cpuid_version_set_model,
+};
+#define DEFINE_PROP_MODEL(_n)                                                  \
+    DEFINE_PROP(_n, X86CPU, env.cpuid_version, qdev_prop_model, uint32_t)
+
 static void x86_cpuid_version_get_stepping(Object *obj, Visitor *v,
                                            void *opaque, const char *name,
                                            Error **errp)
@@ -1485,6 +1493,7 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
 
 static Property cpu_x86_properties[] = {
     DEFINE_PROP_FAMILY("family"),
+    DEFINE_PROP_MODEL("model"),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2464,9 +2473,6 @@ static void x86_cpu_initfn(Object *obj)
     cs->env_ptr = env;
     cpu_exec_init(env);
 
-    object_property_add(obj, "model", "int",
-                        x86_cpuid_version_get_model,
-                        x86_cpuid_version_set_model, NULL, NULL, NULL);
     object_property_add(obj, "stepping", "int",
                         x86_cpuid_version_get_stepping,
                         x86_cpuid_version_set_stepping, NULL, NULL, NULL);
-- 
1.7.1

[Qemu-devel] [PATCH 03/15] target-i386: cpu: convert 'stepping' to static property

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 target-i386/cpu.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 5d379af..1d997ee 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1282,6 +1282,14 @@ static void x86_cpuid_version_set_stepping(Object *obj, Visitor *v,
     env->cpuid_version |= value & 0xf;
 }
 
+static PropertyInfo qdev_prop_stepping = {
+    .name  = "uint32",
+    .get   = x86_cpuid_version_get_stepping,
+    .set   = x86_cpuid_version_set_stepping,
+};
+#define DEFINE_PROP_STEPPING(_n)                                               \
+    DEFINE_PROP(_n, X86CPU, env.cpuid_version, qdev_prop_stepping, uint32_t)
+
 static void x86_cpuid_get_level(Object *obj, Visitor *v, void *opaque,
                                 const char *name, Error **errp)
 {
@@ -1494,6 +1502,7 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
 static Property cpu_x86_properties[] = {
     DEFINE_PROP_FAMILY("family"),
     DEFINE_PROP_MODEL("model"),
+    DEFINE_PROP_STEPPING("stepping"),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2473,9 +2482,6 @@ static void x86_cpu_initfn(Object *obj)
     cs->env_ptr = env;
     cpu_exec_init(env);
 
-    object_property_add(obj, "stepping", "int",
-                        x86_cpuid_version_get_stepping,
-                        x86_cpuid_version_set_stepping, NULL, NULL, NULL);
     object_property_add(obj, "level", "int",
                         x86_cpuid_get_level,
                         x86_cpuid_set_level, NULL, NULL, NULL);
-- 
1.7.1

[Qemu-devel] [PATCH 04/15] target-i386: cpu: convert 'level' to static property

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 target-i386/cpu.c |   20 +-------------------
 1 files changed, 1 insertions(+), 19 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1d997ee..bb86484 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1290,22 +1290,6 @@ static PropertyInfo qdev_prop_stepping = {
 #define DEFINE_PROP_STEPPING(_n)                                               \
     DEFINE_PROP(_n, X86CPU, env.cpuid_version, qdev_prop_stepping, uint32_t)
 
-static void x86_cpuid_get_level(Object *obj, Visitor *v, void *opaque,
-                                const char *name, Error **errp)
-{
-    X86CPU *cpu = X86_CPU(obj);
-
-    visit_type_uint32(v, &cpu->env.cpuid_level, name, errp);
-}
-
-static void x86_cpuid_set_level(Object *obj, Visitor *v, void *opaque,
-                                const char *name, Error **errp)
-{
-    X86CPU *cpu = X86_CPU(obj);
-
-    visit_type_uint32(v, &cpu->env.cpuid_level, name, errp);
-}
-
 static void x86_cpuid_get_xlevel(Object *obj, Visitor *v, void *opaque,
                                  const char *name, Error **errp)
 {
@@ -1503,6 +1487,7 @@ static Property cpu_x86_properties[] = {
     DEFINE_PROP_FAMILY("family"),
     DEFINE_PROP_MODEL("model"),
     DEFINE_PROP_STEPPING("stepping"),
+    DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2482,9 +2467,6 @@ static void x86_cpu_initfn(Object *obj)
     cs->env_ptr = env;
     cpu_exec_init(env);
 
-    object_property_add(obj, "level", "int",
-                        x86_cpuid_get_level,
-                        x86_cpuid_set_level, NULL, NULL, NULL);
     object_property_add(obj, "xlevel", "int",
                         x86_cpuid_get_xlevel,
                         x86_cpuid_set_xlevel, NULL, NULL, NULL);
-- 
1.7.1

[Qemu-devel] [PATCH 05/15] target-i386: cpu: convert 'xlevel' to static property

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 target-i386/cpu.c |   20 +-------------------
 1 files changed, 1 insertions(+), 19 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index bb86484..f42282e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1290,22 +1290,6 @@ static PropertyInfo qdev_prop_stepping = {
 #define DEFINE_PROP_STEPPING(_n)                                               \
     DEFINE_PROP(_n, X86CPU, env.cpuid_version, qdev_prop_stepping, uint32_t)
 
-static void x86_cpuid_get_xlevel(Object *obj, Visitor *v, void *opaque,
-                                 const char *name, Error **errp)
-{
-    X86CPU *cpu = X86_CPU(obj);
-
-    visit_type_uint32(v, &cpu->env.cpuid_xlevel, name, errp);
-}
-
-static void x86_cpuid_set_xlevel(Object *obj, Visitor *v, void *opaque,
-                                 const char *name, Error **errp)
-{
-    X86CPU *cpu = X86_CPU(obj);
-
-    visit_type_uint32(v, &cpu->env.cpuid_xlevel, name, errp);
-}
-
 static char *x86_cpuid_get_vendor(Object *obj, Error **errp)
 {
     X86CPU *cpu = X86_CPU(obj);
@@ -1488,6 +1472,7 @@ static Property cpu_x86_properties[] = {
     DEFINE_PROP_MODEL("model"),
     DEFINE_PROP_STEPPING("stepping"),
     DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
+    DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2467,9 +2452,6 @@ static void x86_cpu_initfn(Object *obj)
     cs->env_ptr = env;
     cpu_exec_init(env);
 
-    object_property_add(obj, "xlevel", "int",
-                        x86_cpuid_get_xlevel,
-                        x86_cpuid_set_xlevel, NULL, NULL, NULL);
     object_property_add_str(obj, "vendor",
                             x86_cpuid_get_vendor,
                             x86_cpuid_set_vendor, NULL);
-- 
1.7.1

[Qemu-devel] [PATCH 06/15] target-i386: cpu: convert 'vendor' to static property

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 target-i386/cpu.c |   31 ++++++++++++++++++++++++-------
 1 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index f42282e..21e7334 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1290,7 +1290,8 @@ static PropertyInfo qdev_prop_stepping = {
 #define DEFINE_PROP_STEPPING(_n)                                               \
     DEFINE_PROP(_n, X86CPU, env.cpuid_version, qdev_prop_stepping, uint32_t)
 
-static char *x86_cpuid_get_vendor(Object *obj, Error **errp)
+static void x86_cpuid_get_vendor(Object *obj, Visitor *v, void *opaque,
+                                 const char *name, Error **errp)
 {
     X86CPU *cpu = X86_CPU(obj);
     CPUX86State *env = &cpu->env;
@@ -1299,16 +1300,23 @@ static char *x86_cpuid_get_vendor(Object *obj, Error **errp)
     value = (char *)g_malloc(CPUID_VENDOR_SZ + 1);
     x86_cpu_vendor_words2str(value, env->cpuid_vendor1, env->cpuid_vendor2,
                              env->cpuid_vendor3);
-    return value;
+    visit_type_str(v, &value, name, errp);
+    g_free(value);
 }
 
-static void x86_cpuid_set_vendor(Object *obj, const char *value,
-                                 Error **errp)
+static void x86_cpuid_set_vendor(Object *obj, Visitor *v, void *opaque,
+                                 const char *name, Error **errp)
 {
     X86CPU *cpu = X86_CPU(obj);
     CPUX86State *env = &cpu->env;
+    char *value;
     int i;
 
+    visit_type_str(v, &value, name, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+
     if (strlen(value) != CPUID_VENDOR_SZ) {
         error_set(errp, QERR_PROPERTY_VALUE_BAD, "",
                   "vendor", value);
@@ -1323,6 +1331,17 @@ static void x86_cpuid_set_vendor(Object *obj, const char *value,
         env->cpuid_vendor2 |= ((uint8_t)value[i + 4]) << (8 * i);
         env->cpuid_vendor3 |= ((uint8_t)value[i + 8]) << (8 * i);
     }
+    g_free(value);
+}
+
+static PropertyInfo qdev_prop_vendor = {
+    .name  = "string",
+    .get   = x86_cpuid_get_vendor,
+    .set   = x86_cpuid_set_vendor,
+};
+#define DEFINE_PROP_VENDOR(_n) {                                               \
+    .name = _n,                                                                \
+    .info  = &qdev_prop_vendor                                                 \
 }
 
 static char *x86_cpuid_get_model_id(Object *obj, Error **errp)
@@ -1473,6 +1492,7 @@ static Property cpu_x86_properties[] = {
     DEFINE_PROP_STEPPING("stepping"),
     DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
     DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
+    DEFINE_PROP_VENDOR("vendor"),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2452,9 +2472,6 @@ static void x86_cpu_initfn(Object *obj)
     cs->env_ptr = env;
     cpu_exec_init(env);
 
-    object_property_add_str(obj, "vendor",
-                            x86_cpuid_get_vendor,
-                            x86_cpuid_set_vendor, NULL);
     object_property_add_str(obj, "model-id",
                             x86_cpuid_get_model_id,
                             x86_cpuid_set_model_id, NULL);
-- 
1.7.1

[Qemu-devel] [PATCH 07/15] target-i386: cpu: convert 'model-id' to static property

[Igor Mammedov]

* check "if (model_id == NULL)" looks unnecessary now, since all
builtin model-ids are not NULL and user shouldn't be able to set
it NULL (cpumodel string parsing code takes care of it, if feature
is specified as "model-id=" on command line, its parsing will
result in an empty string as value).

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 target-i386/cpu.c |   31 ++++++++++++++++++++++---------
 1 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 21e7334..9f6fe06 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1344,7 +1344,8 @@ static PropertyInfo qdev_prop_vendor = {
     .info  = &qdev_prop_vendor                                                 \
 }
 
-static char *x86_cpuid_get_model_id(Object *obj, Error **errp)
+static void x86_cpuid_get_model_id(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
 {
     X86CPU *cpu = X86_CPU(obj);
     CPUX86State *env = &cpu->env;
@@ -1356,18 +1357,21 @@ static char *x86_cpuid_get_model_id(Object *obj, Error **errp)
         value[i] = env->cpuid_model[i >> 2] >> (8 * (i & 3));
     }
     value[48] = '\0';
-    return value;
+    visit_type_str(v, &value, name, errp);
+    g_free(value);
 }
 
-static void x86_cpuid_set_model_id(Object *obj, const char *model_id,
-                                   Error **errp)
+static void x86_cpuid_set_model_id(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
 {
     X86CPU *cpu = X86_CPU(obj);
     CPUX86State *env = &cpu->env;
     int c, len, i;
+    char *model_id;
 
-    if (model_id == NULL) {
-        model_id = "";
+    visit_type_str(v, &model_id, name, errp);
+    if (error_is_set(errp)) {
+        return;
     }
     len = strlen(model_id);
     memset(env->cpuid_model, 0, 48);
@@ -1379,6 +1383,17 @@ static void x86_cpuid_set_model_id(Object *obj, const char *model_id,
         }
         env->cpuid_model[i >> 2] |= c << (8 * (i & 3));
     }
+    g_free(model_id);
+}
+
+static PropertyInfo qdev_prop_model_id = {
+    .name  = "string",
+    .get   = x86_cpuid_get_model_id,
+    .set   = x86_cpuid_set_model_id,
+};
+#define DEFINE_PROP_MODEL_ID(_n) {                                             \
+    .name  = _n,                                                               \
+    .info  = &qdev_prop_model_id                                               \
 }
 
 static void x86_cpuid_get_tsc_freq(Object *obj, Visitor *v, void *opaque,
@@ -1493,6 +1508,7 @@ static Property cpu_x86_properties[] = {
     DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
     DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
     DEFINE_PROP_VENDOR("vendor"),
+    DEFINE_PROP_MODEL_ID("model-id"),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2472,9 +2488,6 @@ static void x86_cpu_initfn(Object *obj)
     cs->env_ptr = env;
     cpu_exec_init(env);
 
-    object_property_add_str(obj, "model-id",
-                            x86_cpuid_get_model_id,
-                            x86_cpuid_set_model_id, NULL);
     object_property_add(obj, "tsc-frequency", "int",
                         x86_cpuid_get_tsc_freq,
                         x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
-- 
1.7.1

[Qemu-devel] [PATCH 08/15] target-i386: cpu: convert 'tsc-frequency' to static property

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 target-i386/cpu.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 9f6fe06..ec6d33f 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1427,6 +1427,14 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
     cpu->env.tsc_khz = value / 1000;
 }
 
+static PropertyInfo qdev_prop_tsc_freq = {
+    .name  = "int64",
+    .get   = x86_cpuid_get_tsc_freq,
+    .set   = x86_cpuid_set_tsc_freq,
+};
+#define DEFINE_PROP_TSC_FREQ(_n)                                               \
+    DEFINE_PROP(_n, X86CPU, env.tsc_khz, qdev_prop_tsc_freq, int32_t)
+
 static void x86_cpuid_get_apic_id(Object *obj, Visitor *v, void *opaque,
                                   const char *name, Error **errp)
 {
@@ -1509,6 +1517,7 @@ static Property cpu_x86_properties[] = {
     DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
     DEFINE_PROP_VENDOR("vendor"),
     DEFINE_PROP_MODEL_ID("model-id"),
+    DEFINE_PROP_TSC_FREQ("tsc-frequency"),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2488,9 +2497,6 @@ static void x86_cpu_initfn(Object *obj)
     cs->env_ptr = env;
     cpu_exec_init(env);
 
-    object_property_add(obj, "tsc-frequency", "int",
-                        x86_cpuid_get_tsc_freq,
-                        x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
     object_property_add(obj, "apic-id", "int",
                         x86_cpuid_get_apic_id,
                         x86_cpuid_set_apic_id, NULL, NULL, NULL);
-- 
1.7.1

[Qemu-devel] [PATCH 09/15] target-i386: move hyperv_* static globals to CPUState

[Igor Mammedov]

- since hyperv_* helper functions are used only in target-i386/kvm.c
  move them there as static helpers

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Requestd-By: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/Makefile.objs |    2 +-
 target-i386/cpu.c         |   16 +++++++---
 target-i386/cpu.h         |    7 +++++
 target-i386/hyperv.c      |   64 ---------------------------------------------
 target-i386/hyperv.h      |   45 -------------------------------
 target-i386/kvm.c         |   36 ++++++++++++++++++-------
 6 files changed, 45 insertions(+), 125 deletions(-)
 delete mode 100644 target-i386/hyperv.c
 delete mode 100644 target-i386/hyperv.h

diff --git a/target-i386/Makefile.objs b/target-i386/Makefile.objs
index c1d4f05..887dca7 100644
--- a/target-i386/Makefile.objs
+++ b/target-i386/Makefile.objs
@@ -2,7 +2,7 @@ obj-y += translate.o helper.o cpu.o
 obj-y += excp_helper.o fpu_helper.o cc_helper.o int_helper.o svm_helper.o
 obj-y += smm_helper.o misc_helper.o mem_helper.o seg_helper.o
 obj-$(CONFIG_SOFTMMU) += machine.o arch_memory_mapping.o arch_dump.o
-obj-$(CONFIG_KVM) += kvm.o hyperv.o
+obj-$(CONFIG_KVM) += kvm.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
 obj-$(CONFIG_LINUX_USER) += ioport-user.o
 obj-$(CONFIG_BSD_USER) += ioport-user.o
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ec6d33f..40f2a7c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -35,8 +35,6 @@
 #include "qapi/visitor.h"
 #include "sysemu/arch_init.h"
 
-#include "hyperv.h"
-
 #include "hw/hw.h"
 #if defined(CONFIG_KVM)
 #include <linux/kvm_para.h>
@@ -1633,12 +1631,19 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
                 object_property_parse(OBJECT(cpu), num, "tsc-frequency", errp);
             } else if (!strcmp(featurestr, "hv-spinlocks")) {
                 char *err;
+                const int min = 0xFFF;
                 numvalue = strtoul(val, &err, 0);
                 if (!*val || *err) {
                     error_setg(errp, "bad numerical value %s", val);
                     goto out;
                 }
-                hyperv_set_spinlock_retries(numvalue);
+                if (numvalue < min) {
+                    fprintf(stderr, "hv-spinlocks value shall always be >= 0x%x"
+                            ", fixup will be removed in future versions\n",
+                            min);
+                    numvalue = min;
+                }
+                env->hyperv_spinlock_attempts = numvalue;
             } else {
                 error_setg(errp, "unrecognized feature %s", featurestr);
                 goto out;
@@ -1648,9 +1653,9 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
         } else if (!strcmp(featurestr, "enforce")) {
             check_cpuid = enforce_cpuid = 1;
         } else if (!strcmp(featurestr, "hv_relaxed")) {
-            hyperv_enable_relaxed_timing(true);
+            env->hyperv_relaxed_timing = true;
         } else if (!strcmp(featurestr, "hv_vapic")) {
-            hyperv_enable_vapic_recommended(true);
+            env->hyperv_vapic = true;
         } else {
             error_setg(errp, "feature string `%s' not in format (+feature|"
                        "-feature|feature=xyz)", featurestr);
@@ -1797,6 +1802,7 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
         def->features[FEAT_KVM] |= kvm_default_features;
     }
     def->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
+    env->hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY;
 
     object_property_set_str(OBJECT(cpu), def->vendor, "vendor", errp);
     object_property_set_int(OBJECT(cpu), def->level, "level", errp);
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 058c57f..ea9143f 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -549,6 +549,10 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
 #define CPUID_MWAIT_IBE     (1 << 1) /* Interrupts can exit capability */
 #define CPUID_MWAIT_EMX     (1 << 0) /* enumeration supported */
 
+#ifndef HYPERV_SPINLOCK_NEVER_RETRY
+#define HYPERV_SPINLOCK_NEVER_RETRY             0xFFFFFFFF
+#endif
+
 #define EXCP00_DIVZ	0
 #define EXCP01_DB	1
 #define EXCP02_NMI	2
@@ -845,6 +849,9 @@ typedef struct CPUX86State {
     FeatureWordArray features;
     uint32_t cpuid_model[12];
     uint32_t cpuid_apic_id;
+    bool hyperv_vapic;
+    bool hyperv_relaxed_timing;
+    int hyperv_spinlock_attempts;
 
     /* MTRRs */
     uint64_t mtrr_fixed[11];
diff --git a/target-i386/hyperv.c b/target-i386/hyperv.c
deleted file mode 100644
index f284e99..0000000
--- a/target-i386/hyperv.c
+++ /dev/null
@@ -1,64 +0,0 @@
-/*
- * QEMU Hyper-V support
- *
- * Copyright Red Hat, Inc. 2011
- *
- * Author: Vadim Rozenfeld     <vrozenfe@redhat.com>
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- *
- */
-
-#include "hyperv.h"
-
-static bool hyperv_vapic;
-static bool hyperv_relaxed_timing;
-static int hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY;
-
-void hyperv_enable_vapic_recommended(bool val)
-{
-    hyperv_vapic = val;
-}
-
-void hyperv_enable_relaxed_timing(bool val)
-{
-    hyperv_relaxed_timing = val;
-}
-
-void hyperv_set_spinlock_retries(int val)
-{
-    hyperv_spinlock_attempts = val;
-    if (hyperv_spinlock_attempts < 0xFFF) {
-        hyperv_spinlock_attempts = 0xFFF;
-    }
-}
-
-bool hyperv_enabled(void)
-{
-    return hyperv_hypercall_available() || hyperv_relaxed_timing_enabled();
-}
-
-bool hyperv_hypercall_available(void)
-{
-    if (hyperv_vapic ||
-        (hyperv_spinlock_attempts != HYPERV_SPINLOCK_NEVER_RETRY)) {
-      return true;
-    }
-    return false;
-}
-
-bool hyperv_vapic_recommended(void)
-{
-    return hyperv_vapic;
-}
-
-bool hyperv_relaxed_timing_enabled(void)
-{
-    return hyperv_relaxed_timing;
-}
-
-int hyperv_get_spinlock_retries(void)
-{
-    return hyperv_spinlock_attempts;
-}
diff --git a/target-i386/hyperv.h b/target-i386/hyperv.h
deleted file mode 100644
index bacb1d4..0000000
--- a/target-i386/hyperv.h
+++ /dev/null
@@ -1,45 +0,0 @@
-/*
- * QEMU Hyper-V support
- *
- * Copyright Red Hat, Inc. 2011
- *
- * Author: Vadim Rozenfeld     <vrozenfe@redhat.com>
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- *
- */
-
-#ifndef QEMU_HW_HYPERV_H
-#define QEMU_HW_HYPERV_H 1
-
-#include "qemu-common.h"
-#ifdef CONFIG_KVM
-#include <asm/hyperv.h>
-#endif
-
-#ifndef HYPERV_SPINLOCK_NEVER_RETRY
-#define HYPERV_SPINLOCK_NEVER_RETRY             0xFFFFFFFF
-#endif
-
-#ifndef KVM_CPUID_SIGNATURE_NEXT
-#define KVM_CPUID_SIGNATURE_NEXT                0x40000100
-#endif
-
-#if !defined(CONFIG_USER_ONLY) && defined(CONFIG_KVM)
-void hyperv_enable_vapic_recommended(bool val);
-void hyperv_enable_relaxed_timing(bool val);
-void hyperv_set_spinlock_retries(int val);
-#else
-static inline void hyperv_enable_vapic_recommended(bool val) { }
-static inline void hyperv_enable_relaxed_timing(bool val) { }
-static inline void hyperv_set_spinlock_retries(int val) { }
-#endif
-
-bool hyperv_enabled(void);
-bool hyperv_hypercall_available(void);
-bool hyperv_vapic_recommended(void);
-bool hyperv_relaxed_timing_enabled(void);
-int hyperv_get_spinlock_retries(void);
-
-#endif /* QEMU_HW_HYPERV_H */
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 9ffb6ca..0e88072 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -31,7 +31,7 @@
 #include "hw/i386/pc.h"
 #include "hw/i386/apic.h"
 #include "exec/ioport.h"
-#include "hyperv.h"
+#include <asm/hyperv.h>
 #include "hw/pci/pci.h"
 
 //#define DEBUG_KVM
@@ -418,6 +418,22 @@ unsigned long kvm_arch_vcpu_id(CPUState *cs)
     return cpu->env.cpuid_apic_id;
 }
 
+#ifndef KVM_CPUID_SIGNATURE_NEXT
+#define KVM_CPUID_SIGNATURE_NEXT                0x40000100
+#endif
+
+static bool hyperv_hypercall_available(CPUX86State *env)
+{
+    return env->hyperv_vapic ||
+        (env->hyperv_spinlock_attempts != HYPERV_SPINLOCK_NEVER_RETRY);
+}
+
+static bool hyperv_enabled(CPUX86State *env)
+{
+    return hyperv_hypercall_available(env) ||
+           env->hyperv_relaxed_timing;
+}
+
 #define KVM_MAX_CPUID_ENTRIES  100
 
 int kvm_arch_init_vcpu(CPUState *cs)
@@ -440,7 +456,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
     c = &cpuid_data.entries[cpuid_i++];
     memset(c, 0, sizeof(*c));
     c->function = KVM_CPUID_SIGNATURE;
-    if (!hyperv_enabled()) {
+    if (!hyperv_enabled(env)) {
         memcpy(signature, "KVMKVMKVM\0\0\0", 12);
         c->eax = 0;
     } else {
@@ -456,7 +472,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
     c->function = KVM_CPUID_FEATURES;
     c->eax = env->features[FEAT_KVM];
 
-    if (hyperv_enabled()) {
+    if (hyperv_enabled(env)) {
         memcpy(signature, "Hv#1\0\0\0\0\0\0\0\0", 12);
         c->eax = signature[0];
 
@@ -469,10 +485,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
         c = &cpuid_data.entries[cpuid_i++];
         memset(c, 0, sizeof(*c));
         c->function = HYPERV_CPUID_FEATURES;
-        if (hyperv_relaxed_timing_enabled()) {
+        if (env->hyperv_relaxed_timing) {
             c->eax |= HV_X64_MSR_HYPERCALL_AVAILABLE;
         }
-        if (hyperv_vapic_recommended()) {
+        if (env->hyperv_vapic) {
             c->eax |= HV_X64_MSR_HYPERCALL_AVAILABLE;
             c->eax |= HV_X64_MSR_APIC_ACCESS_AVAILABLE;
         }
@@ -480,13 +496,13 @@ int kvm_arch_init_vcpu(CPUState *cs)
         c = &cpuid_data.entries[cpuid_i++];
         memset(c, 0, sizeof(*c));
         c->function = HYPERV_CPUID_ENLIGHTMENT_INFO;
-        if (hyperv_relaxed_timing_enabled()) {
+        if (env->hyperv_relaxed_timing) {
             c->eax |= HV_X64_RELAXED_TIMING_RECOMMENDED;
         }
-        if (hyperv_vapic_recommended()) {
+        if (env->hyperv_vapic) {
             c->eax |= HV_X64_APIC_ACCESS_RECOMMENDED;
         }
-        c->ebx = hyperv_get_spinlock_retries();
+        c->ebx = env->hyperv_spinlock_attempts;
 
         c = &cpuid_data.entries[cpuid_i++];
         memset(c, 0, sizeof(*c));
@@ -1115,11 +1131,11 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
             kvm_msr_entry_set(&msrs[n++], MSR_KVM_STEAL_TIME,
                               env->steal_time_msr);
         }
-        if (hyperv_hypercall_available()) {
+        if (hyperv_hypercall_available(env)) {
             kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_GUEST_OS_ID, 0);
             kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_HYPERCALL, 0);
         }
-        if (hyperv_vapic_recommended()) {
+        if (env->hyperv_vapic) {
             kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
         }
     }
-- 
1.7.1

[Qemu-devel] [PATCH 10/15] target-i386: convert 'hv_spinlocks' to static property

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 target-i386/cpu.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 40f2a7c..c2125f4 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1507,6 +1507,50 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
     error_propagate(errp, err);
 }
 
+static void x86_get_hv_spinlocks(Object *obj, Visitor *v, void *opaque,
+                                 const char *name, Error **errp)
+{
+    X86CPU *cpu = X86_CPU(obj);
+    int64_t value = cpu->env.hyperv_spinlock_attempts;
+
+    visit_type_int(v, &value, name, errp);
+}
+
+static void x86_set_hv_spinlocks(Object *obj, Visitor *v, void *opaque,
+                                 const char *name, Error **errp)
+{
+    const int64_t min = 0xFFF;
+    const int64_t max = UINT_MAX;
+    X86CPU *cpu = X86_CPU(obj);
+    int64_t value;
+
+    visit_type_int(v, &value, name, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    if (value < min || value > max) {
+        error_setg(errp, "Property %s.%s doesn't take value %" PRId64
+                  " (minimum: %" PRId64 ", maximum: %" PRId64 ")",
+                  object_get_typename(obj), name ? name : "null",
+                  value, min, max);
+        return;
+    }
+    cpu->env.hyperv_spinlock_attempts = value;
+}
+
+static PropertyInfo qdev_prop_spinlocks = {
+    .name  = "int",
+    .get   = x86_get_hv_spinlocks,
+    .set   = x86_set_hv_spinlocks,
+};
+#define DEFINE_PROP_HV_SPINLOCKS(_n, _defval) {                                \
+    .name  = _n,                                                               \
+    .info  = &qdev_prop_spinlocks,                                             \
+    .qtype = QTYPE_QINT,                                                       \
+    .defval = _defval                                                          \
+}
+
 static Property cpu_x86_properties[] = {
     DEFINE_PROP_FAMILY("family"),
     DEFINE_PROP_MODEL("model"),
@@ -1516,6 +1560,7 @@ static Property cpu_x86_properties[] = {
     DEFINE_PROP_VENDOR("vendor"),
     DEFINE_PROP_MODEL_ID("model-id"),
     DEFINE_PROP_TSC_FREQ("tsc-frequency"),
+    DEFINE_PROP_HV_SPINLOCKS("hv-spinlocks", HYPERV_SPINLOCK_NEVER_RETRY),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1632,6 +1677,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
             } else if (!strcmp(featurestr, "hv-spinlocks")) {
                 char *err;
                 const int min = 0xFFF;
+                char num[32];
                 numvalue = strtoul(val, &err, 0);
                 if (!*val || *err) {
                     error_setg(errp, "bad numerical value %s", val);
@@ -1643,7 +1689,8 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
                             min);
                     numvalue = min;
                 }
-                env->hyperv_spinlock_attempts = numvalue;
+                snprintf(num, sizeof(num), "%" PRId32, numvalue);
+                object_property_parse(OBJECT(cpu), num, featurestr, errp);
             } else {
                 error_setg(errp, "unrecognized feature %s", featurestr);
                 goto out;
@@ -1802,7 +1849,6 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
         def->features[FEAT_KVM] |= kvm_default_features;
     }
     def->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
-    env->hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY;
 
     object_property_set_str(OBJECT(cpu), def->vendor, "vendor", errp);
     object_property_set_int(OBJECT(cpu), def->level, "level", errp);
-- 
1.7.1

[Qemu-devel] [PATCH 11/15] target-i386: convert 'hv_relaxed' to static property

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 target-i386/cpu.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index c2125f4..95ffb2e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1561,6 +1561,7 @@ static Property cpu_x86_properties[] = {
     DEFINE_PROP_MODEL_ID("model-id"),
     DEFINE_PROP_TSC_FREQ("tsc-frequency"),
     DEFINE_PROP_HV_SPINLOCKS("hv-spinlocks", HYPERV_SPINLOCK_NEVER_RETRY),
+    DEFINE_PROP_BOOL("hv-relaxed", X86CPU, env.hyperv_relaxed_timing, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1700,7 +1701,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
         } else if (!strcmp(featurestr, "enforce")) {
             check_cpuid = enforce_cpuid = 1;
         } else if (!strcmp(featurestr, "hv_relaxed")) {
-            env->hyperv_relaxed_timing = true;
+            object_property_parse(OBJECT(cpu), "on", "hv-relaxed", errp);
         } else if (!strcmp(featurestr, "hv_vapic")) {
             env->hyperv_vapic = true;
         } else {
-- 
1.7.1

[Qemu-devel] [PATCH 12/15] target-i386: convert 'hv_vapic' to static property

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 target-i386/cpu.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 95ffb2e..bf56677 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1562,6 +1562,7 @@ static Property cpu_x86_properties[] = {
     DEFINE_PROP_TSC_FREQ("tsc-frequency"),
     DEFINE_PROP_HV_SPINLOCKS("hv-spinlocks", HYPERV_SPINLOCK_NEVER_RETRY),
     DEFINE_PROP_BOOL("hv-relaxed", X86CPU, env.hyperv_relaxed_timing, false),
+    DEFINE_PROP_BOOL("hv-vapic", X86CPU, env.hyperv_vapic, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1703,7 +1704,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
         } else if (!strcmp(featurestr, "hv_relaxed")) {
             object_property_parse(OBJECT(cpu), "on", "hv-relaxed", errp);
         } else if (!strcmp(featurestr, "hv_vapic")) {
-            env->hyperv_vapic = true;
+            object_property_parse(OBJECT(cpu), "on", "hv-vapic", errp);
         } else {
             error_setg(errp, "feature string `%s' not in format (+feature|"
                        "-feature|feature=xyz)", featurestr);
-- 
1.7.1

[Qemu-devel] [PATCH 13/15] target-i386: convert 'check' and 'enforce' to static properties

[Igor Mammedov]

* additionally convert check_cpuid & enforce_cpuid to bool and make them
  members of CPUX86State
* make 'enforce' feature independent from 'check'

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 target-i386/cpu.c |   13 ++++++-------
 target-i386/cpu.h |    2 ++
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index bf56677..366e3dd 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -233,9 +233,6 @@ typedef struct model_features_t {
     FeatureWord feat_word;
 } model_features_t;
 
-int check_cpuid = 0;
-int enforce_cpuid = 0;
-
 static uint32_t kvm_default_features = (1 << KVM_FEATURE_CLOCKSOURCE) |
         (1 << KVM_FEATURE_NOP_IO_DELAY) |
         (1 << KVM_FEATURE_CLOCKSOURCE2) |
@@ -1563,6 +1560,8 @@ static Property cpu_x86_properties[] = {
     DEFINE_PROP_HV_SPINLOCKS("hv-spinlocks", HYPERV_SPINLOCK_NEVER_RETRY),
     DEFINE_PROP_BOOL("hv-relaxed", X86CPU, env.hyperv_relaxed_timing, false),
     DEFINE_PROP_BOOL("hv-vapic", X86CPU, env.hyperv_vapic, false),
+    DEFINE_PROP_BOOL("check", X86CPU, env.check_cpuid, false),
+    DEFINE_PROP_BOOL("enforce", X86CPU, env.enforce_cpuid, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1698,9 +1697,9 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
                 goto out;
             }
         } else if (!strcmp(featurestr, "check")) {
-            check_cpuid = 1;
+            object_property_parse(OBJECT(cpu), "on", featurestr, errp);
         } else if (!strcmp(featurestr, "enforce")) {
-            check_cpuid = enforce_cpuid = 1;
+            object_property_parse(OBJECT(cpu), "on", featurestr, errp);
         } else if (!strcmp(featurestr, "hv_relaxed")) {
             object_property_parse(OBJECT(cpu), "on", "hv-relaxed", errp);
         } else if (!strcmp(featurestr, "hv_vapic")) {
@@ -2469,8 +2468,8 @@ 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 (check_cpuid && kvm_check_features_against_host(cpu)
-            && enforce_cpuid) {
+        if ((env->check_cpuid || env->enforce_cpuid)
+            && kvm_check_features_against_host(cpu) && env->enforce_cpuid) {
             error_setg(&local_err,
                        "Host's CPU doesn't support requested features");
             goto out;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index ea9143f..4ca7c11 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -852,6 +852,8 @@ typedef struct CPUX86State {
     bool hyperv_vapic;
     bool hyperv_relaxed_timing;
     int hyperv_spinlock_attempts;
+    bool check_cpuid;
+    bool enforce_cpuid;
 
     /* MTRRs */
     uint64_t mtrr_fixed[11];
-- 
1.7.1

[Qemu-devel] [PATCH 14/15] target-i386: cleanup 'foo' feature handling'

[Igor Mammedov]

features check, enforce, hv_relaxed and hv_vapic are treated as boolean set to 'on'
when passed from command line, so it's not neccessary to handle each of them
separetly. Collapse them to one catch-all branch which will treat
any feature in format 'foo' as boolean set to 'on'.

PS:
Any unknown feature will be rejected by CPU property setter so there is no
need to check for unknown feature in cpu_x86_parse_featurestr(), therefore
it's replaced by above mentioned catch-all handler.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
---
v2:
  * use feat2prop() to perform name convertion for hv_vapic and hv_relaxed
---
 target-i386/cpu.c |   13 ++-----------
 1 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 366e3dd..d5cc150 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1696,18 +1696,9 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
                 error_setg(errp, "unrecognized feature %s", featurestr);
                 goto out;
             }
-        } else if (!strcmp(featurestr, "check")) {
-            object_property_parse(OBJECT(cpu), "on", featurestr, errp);
-        } else if (!strcmp(featurestr, "enforce")) {
-            object_property_parse(OBJECT(cpu), "on", featurestr, errp);
-        } else if (!strcmp(featurestr, "hv_relaxed")) {
-            object_property_parse(OBJECT(cpu), "on", "hv-relaxed", errp);
-        } else if (!strcmp(featurestr, "hv_vapic")) {
-            object_property_parse(OBJECT(cpu), "on", "hv-vapic", errp);
         } else {
-            error_setg(errp, "feature string `%s' not in format (+feature|"
-                       "-feature|feature=xyz)", featurestr);
-            goto out;
+            feat2prop(featurestr);
+            object_property_parse(OBJECT(cpu), "on", featurestr, errp);
         }
         if (error_is_set(errp)) {
             goto out;
-- 
1.7.1

[Qemu-devel] [PATCH 15/15] target-i386: cleanup 'foo=val' feature handling

[Igor Mammedov]

features family, model, stepping, level, hv_spinlocks are treated similarly
when passed from command line, so it's not necessary to handle each of them
individually. Collapse them to one catch-all branch which will treat
any not explicitly handled feature in format 'foo=val'.

PS:
Any unknown feature will be rejected by property setter so there is no
need to check for unknown feature in cpu_x86_parse_featurestr(), therefore
it's replaced by above mentioned catch-all handler.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
  - style fixes
---
 target-i386/cpu.c |   17 ++---------------
 1 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index d5cc150..862ee5c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1634,15 +1634,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
         } else if ((val = strchr(featurestr, '='))) {
             *val = 0; val++;
             feat2prop(featurestr);
-            if (!strcmp(featurestr, "family")) {
-                object_property_parse(OBJECT(cpu), val, featurestr, errp);
-            } else if (!strcmp(featurestr, "model")) {
-                object_property_parse(OBJECT(cpu), val, featurestr, errp);
-            } else if (!strcmp(featurestr, "stepping")) {
-                object_property_parse(OBJECT(cpu), val, featurestr, errp);
-            } else if (!strcmp(featurestr, "level")) {
-                object_property_parse(OBJECT(cpu), val, featurestr, errp);
-            } else if (!strcmp(featurestr, "xlevel")) {
+            if (!strcmp(featurestr, "xlevel")) {
                 char *err;
                 char num[32];
 
@@ -1658,10 +1650,6 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
                 }
                 snprintf(num, sizeof(num), "%" PRIu32, numvalue);
                 object_property_parse(OBJECT(cpu), num, featurestr, errp);
-            } else if (!strcmp(featurestr, "vendor")) {
-                object_property_parse(OBJECT(cpu), val, featurestr, errp);
-            } else if (!strcmp(featurestr, "model-id")) {
-                object_property_parse(OBJECT(cpu), val, featurestr, errp);
             } else if (!strcmp(featurestr, "tsc-freq")) {
                 int64_t tsc_freq;
                 char *err;
@@ -1693,8 +1681,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
                 snprintf(num, sizeof(num), "%" PRId32, numvalue);
                 object_property_parse(OBJECT(cpu), num, featurestr, errp);
             } else {
-                error_setg(errp, "unrecognized feature %s", featurestr);
-                goto out;
+                object_property_parse(OBJECT(cpu), val, featurestr, errp);
             }
         } else {
             feat2prop(featurestr);
-- 
1.7.1

Re: [Qemu-devel] [PATCH qom-cpu 00/15 v8] target-i386: convert CPU features into properties, part 1

[Andreas Färber]

Am 05.06.2013 15:18, schrieb Igor Mammedov:
> It's a rebase of v7 on current qom-cpu tree, since then some patches from it
> were applied to master. Convertion of feature bits is left for part 2
> since it's not ready yet.
> 
> v7-v8:
> * split out dynamic properties convertion patch into per property patches
>   to simplify review
> * drop feature bits convertion

Why is conversion of dynamic properties to static properties still
needed after I applied a solution to override values of dynamic
properties with -global for 1.5?

For HyperV no doubt that the current state needs cleanups.

Andreas

> 
> v6-v7:
> * convert globals check_cpuid, enforce_cpuid and  hyperv_* to fields of
>   CPUState
> * Make PropertyInfo-s static
> * maintain legacy kvmclock semantic in cpu_x86_parse_featurestr()
> * existing properties code are not moved around, just fixed signatures where
>   it's needed and used visitors. 
> 
> v5-v6:
> * when converting feature names to property names, replace '_' with '-'
> * separate patches converting existing dynamic properties into one, were
>   squashed into one [1/9] and change tested with virt-test(next).
> * patches that were touching +-foo features are squashed into one [9/9],
>   to avoid behavior change between them(f-kvmclock property).
> * the rest of conversions were basicaly rebased on top of current qom-cpu-next
>   tree, with small corrections
> 
> git for testing: https://github.com/imammedo/qemu/tree/x86-cpu-properties.v8
> 
> Igor Mammedov (15):
>   target-i386: cpu: convert 'family' to static property
>   target-i386: cpu: convert 'model' to static property
>   target-i386: cpu: convert 'stepping' to static property
>   target-i386: cpu: convert 'level' to static property
>   target-i386: cpu: convert 'xlevel' to static property
>   target-i386: cpu: convert 'vendor' to static property
>   target-i386: cpu: convert 'model-id' to static property
>   target-i386: cpu: convert 'tsc-frequency' to static property
>   target-i386: move hyperv_* static globals to CPUState
>   target-i386: convert 'hv_spinlocks' to static property
>   target-i386: convert 'hv_relaxed' to static property
>   target-i386: convert 'hv_vapic' to static property
>   target-i386: convert 'check' and 'enforce' to static properties
>   target-i386: cleanup 'foo' feature handling'
>   target-i386: cleanup 'foo=val' feature handling
> 
>  target-i386/Makefile.objs |    2 +-
>  target-i386/cpu.c         |  250 +++++++++++++++++++++++++++------------------
>  target-i386/cpu.h         |    9 ++
>  target-i386/hyperv.c      |   64 ------------
>  target-i386/hyperv.h      |   45 --------
>  target-i386/kvm.c         |   36 +++++--
>  6 files changed, 188 insertions(+), 218 deletions(-)
>  delete mode 100644 target-i386/hyperv.c
>  delete mode 100644 target-i386/hyperv.h
> 


-- 
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 qom-cpu 00/15 v8] target-i386: convert CPU features into properties, part 1

[Igor Mammedov]

On Wed, 05 Jun 2013 15:29:08 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 05.06.2013 15:18, schrieb Igor Mammedov:
> > It's a rebase of v7 on current qom-cpu tree, since then some patches from it
> > were applied to master. Convertion of feature bits is left for part 2
> > since it's not ready yet.
> > 
> > v7-v8:
> > * split out dynamic properties convertion patch into per property patches
> >   to simplify review
> > * drop feature bits convertion
> 
> Why is conversion of dynamic properties to static properties still
> needed after I applied a solution to override values of dynamic
> properties with -global for 1.5?
Do you mean qdev_prop_set_globals_for_type() & co?
If yes, then I recall it was acceptable hack to permit more clean
approach for compat props fixes to work. And we promised Anthony to
get rid of it when possible.

Now more to the point,

1. if CPU are to be created with device_add(wich is still  a goal)
   cpu_x86_create() won't be created, so it leaves rules out compat
   properties set by qdev_prop_set_globals_for_type().
   Having properties converted by this series as static would open
   road for:
       - making cpu_x86_parse_featurestr() target specific hook that
         deals with legacy cpu_model parsing and converts provided
         features into global properties.
           as result during hot-add device_add can work without
           specifying +-foo1,foo2,level=XXX, it will be applied from
           globals.
       - above will allow to replace create_x86_cpu() with plain
         device_add when subclasses are implemented.

2. I'd like to utilize default values of static properties for defining
   subclasses like here: https://github.com/imammedo/qemu/commit/a48e252a2800bf8dd56320e68e4f9517d0a25e5c
   - that would automatically provide benefit of class introspection
     without creating CPU instance
   - replace current CPU definitions array to a set of class_init_xxx
     for each subclass.

> 
> For HyperV no doubt that the current state needs cleanups.
> 
> Andreas
> 
> > 
> > v6-v7:
> > * convert globals check_cpuid, enforce_cpuid and  hyperv_* to fields of
> >   CPUState
> > * Make PropertyInfo-s static
> > * maintain legacy kvmclock semantic in cpu_x86_parse_featurestr()
> > * existing properties code are not moved around, just fixed signatures where
> >   it's needed and used visitors. 
> > 
> > v5-v6:
> > * when converting feature names to property names, replace '_' with '-'
> > * separate patches converting existing dynamic properties into one, were
> >   squashed into one [1/9] and change tested with virt-test(next).
> > * patches that were touching +-foo features are squashed into one [9/9],
> >   to avoid behavior change between them(f-kvmclock property).
> > * the rest of conversions were basicaly rebased on top of current qom-cpu-next
> >   tree, with small corrections
> > 
> > git for testing: https://github.com/imammedo/qemu/tree/x86-cpu-properties.v8
> > 
> > Igor Mammedov (15):
> >   target-i386: cpu: convert 'family' to static property
> >   target-i386: cpu: convert 'model' to static property
> >   target-i386: cpu: convert 'stepping' to static property
> >   target-i386: cpu: convert 'level' to static property
> >   target-i386: cpu: convert 'xlevel' to static property
> >   target-i386: cpu: convert 'vendor' to static property
> >   target-i386: cpu: convert 'model-id' to static property
> >   target-i386: cpu: convert 'tsc-frequency' to static property
> >   target-i386: move hyperv_* static globals to CPUState
> >   target-i386: convert 'hv_spinlocks' to static property
> >   target-i386: convert 'hv_relaxed' to static property
> >   target-i386: convert 'hv_vapic' to static property
> >   target-i386: convert 'check' and 'enforce' to static properties
> >   target-i386: cleanup 'foo' feature handling'
> >   target-i386: cleanup 'foo=val' feature handling
> > 
> >  target-i386/Makefile.objs |    2 +-
> >  target-i386/cpu.c         |  250 +++++++++++++++++++++++++++------------------
> >  target-i386/cpu.h         |    9 ++
> >  target-i386/hyperv.c      |   64 ------------
> >  target-i386/hyperv.h      |   45 --------
> >  target-i386/kvm.c         |   36 +++++--
> >  6 files changed, 188 insertions(+), 218 deletions(-)
> >  delete mode 100644 target-i386/hyperv.c
> >  delete mode 100644 target-i386/hyperv.h
> > 
> 
> 

Re: [Qemu-devel] [PATCH qom-cpu 00/15 v8] target-i386: convert CPU features into properties, part 1

[Andreas Färber]

Am 05.06.2013 16:39, schrieb Igor Mammedov:
> On Wed, 05 Jun 2013 15:29:08 +0200
> Andreas Färber <afaerber@suse.de> wrote:
> 
>> Am 05.06.2013 15:18, schrieb Igor Mammedov:
>>> It's a rebase of v7 on current qom-cpu tree, since then some patches from it
>>> were applied to master. Convertion of feature bits is left for part 2
>>> since it's not ready yet.
>>>
>>> v7-v8:
>>> * split out dynamic properties convertion patch into per property patches
>>>   to simplify review
>>> * drop feature bits convertion
>>
>> Why is conversion of dynamic properties to static properties still
>> needed after I applied a solution to override values of dynamic
>> properties with -global for 1.5?
> Do you mean qdev_prop_set_globals_for_type() & co?

Yes.

> If yes, then I recall it was acceptable hack to permit more clean
> approach for compat props fixes to work. And we promised Anthony to
> get rid of it when possible.

Indeed, but no one talked about reverting to static properties as the
solution. :) Instead I was talking about solving this very general
problem at DeviceState / QOM level.

> Now more to the point,
> 
> 1. if CPU are to be created with device_add(wich is still  a goal)
>    cpu_x86_create() won't be created, so it leaves rules out compat
>    properties set by qdev_prop_set_globals_for_type().

It does not rule it out, but I think we all agree that we do not want
calls of it cluttered over subclasses.

Instead we have a very generic problem: instance_init is called
recursively, parents first, so a parent class cannot do any instance
initialization *after* its derived classes initialized the instance.
That's contrary to how realize and other QOM methods work but in
exchange for the flexibility put the burden of saving and calling the
parent's implementation onto subclasses.

That's what I would like to change in some way, possibly a
instance_post_init hook or the like, similar to how DeviceState got its
own base class initialization hook to handle static props.
That would not only keep the work low in this case but may also solve
the virtio-net initialization problem reported elsewhere.

I'm pretty sure if we moved instance_init into ObjectClass, we'd not
only get an insurge of *Class structs but also people forgetting to
save&call the parent type's implementation, resulting in all kinds of
weird errors, so I don't consider this a real option.

>    Having properties converted by this series as static would open
>    road for:
>        - making cpu_x86_parse_featurestr() target specific hook that
>          deals with legacy cpu_model parsing and converts provided
>          features into global properties.

Such a hook could be implemented today, no dependency on static props, I
just didn't see the need yet. sparc is the only other target where I
stumbled over this so far and haven't found time to prepare that yet.

>            as result during hot-add device_add can work without
>            specifying +-foo1,foo2,level=XXX, it will be applied from
>            globals.

Using globals for that is fine with me. That code is currently per-CPU
whereas -cpu does not allow for differently configured CPUs. Mixed CPU
configurations would be either instantiated from the board and via -device.

>        - above will allow to replace create_x86_cpu() with plain
>          device_add when subclasses are implemented.
> 
> 2. I'd like to utilize default values of static properties for defining
>    subclasses like here: https://github.com/imammedo/qemu/commit/a48e252a2800bf8dd56320e68e4f9517d0a25e5c
>    - that would automatically provide benefit of class introspection
>      without creating CPU instance
>    - replace current CPU definitions array to a set of class_init_xxx
>      for each subclass.

How and when do you actually want to inspect them? What's the use case?

If we hardcode object_property_set_*() in instance_init then surely it's
only inspectable in instances, much like the properties added for
libvirt by Eduardo which didn't seem to disturb anyone. Peter actively
chose the instance_init option for ARM CPUs, meaning they can only be
inspected once created.

However any field or pointer assignment in class_init will allow for
some form of inspection after class creation (thinking of -cpu ? and
query-cpus), including although not limited to today's x86_def_t.

The only thing that seems to require static properties today is
`qemu-system-x86_64 -device x86_64-cpu,?`, which happens to terminate
right after, so might as well create an instance of the type to list all
its properties.[*]
I expect `qemu-system-x86_64 -device Haswell-x86_64-cpu,level=4` to work
the same with dynamic and static properties.

`info qtree` by comparison works only at runtime even though operating
on static properties and can easily be replaced with qom-list / qom-get.

What I dislike about static properties is that they seem a relic from
qdev times that add yet another layer of abstraction above QOM
properties. The only practical differences are:
* QOM properties are added in instance_init and added to Object whereas
qdev properties are assigned in class_init and use external storage, and
* qdev properties specify a default value at property level whereas QOM
properties set the default value in a implementation-dependent way,
usually field assignment from either class field or a hardcoded value.

To me that poses the question of whether we may want to have a
QTAILQ_HEAD(, ObjectProperty) properties;
not only in Object but also in ObjectClass for inspection of name and
type fields. We probably don't want to rule out dynamically added
properties except for Peter's array properties in ARM devices.

[*] PowerPCCPU is known to leak memory on finalization, I have a patch
pending. But I carefully employed QOM realize in CPUState to allow for
creating an instance without creating vCPU threads so that it can easily
be destroyed again (mostly thinking of property assignment errors back
then), any problems with that should get fixed, ideally with test cases;
QOM unrealize by comparison is not yet (widely) supported by CPUs I fear.

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 qom-cpu 00/15 v8] target-i386: convert CPU features into properties, part 1

[Eduardo Habkost]

On Wed, Jun 05, 2013 at 07:04:59PM +0200, Andreas Färber wrote:
> Am 05.06.2013 16:39, schrieb Igor Mammedov:
> > On Wed, 05 Jun 2013 15:29:08 +0200
> > Andreas Färber <afaerber@suse.de> wrote:
> > 
> >> Am 05.06.2013 15:18, schrieb Igor Mammedov:
> >>> It's a rebase of v7 on current qom-cpu tree, since then some patches from it
> >>> were applied to master. Convertion of feature bits is left for part 2
> >>> since it's not ready yet.
> >>>
> >>> v7-v8:
> >>> * split out dynamic properties convertion patch into per property patches
> >>>   to simplify review
> >>> * drop feature bits convertion
> >>
> >> Why is conversion of dynamic properties to static properties still
> >> needed after I applied a solution to override values of dynamic
> >> properties with -global for 1.5?
> > Do you mean qdev_prop_set_globals_for_type() & co?
> 
> Yes.
> 
> > If yes, then I recall it was acceptable hack to permit more clean
> > approach for compat props fixes to work. And we promised Anthony to
> > get rid of it when possible.
> 
> Indeed, but no one talked about reverting to static properties as the
> solution. :) Instead I was talking about solving this very general
> problem at DeviceState / QOM level.

We have had this discussion before, and I remember Anthony saying that
anything set using global properties _must_ be static properties,
period.

That was the main motivation we even started doing the static properties
work, months ago.


> 
> > Now more to the point,
> > 
> > 1. if CPU are to be created with device_add(wich is still  a goal)
> >    cpu_x86_create() won't be created, so it leaves rules out compat
> >    properties set by qdev_prop_set_globals_for_type().
> 
> It does not rule it out, but I think we all agree that we do not want
> calls of it cluttered over subclasses.
> 
> Instead we have a very generic problem: instance_init is called
> recursively, parents first, so a parent class cannot do any instance
> initialization *after* its derived classes initialized the instance.
> That's contrary to how realize and other QOM methods work but in
> exchange for the flexibility put the burden of saving and calling the
> parent's implementation onto subclasses.
> 
> That's what I would like to change in some way, possibly a
> instance_post_init hook or the like, similar to how DeviceState got its
> own base class initialization hook to handle static props.
> That would not only keep the work low in this case but may also solve
> the virtio-net initialization problem reported elsewhere.

You mean this?
https://lists.gnu.org/archive/html/qemu-devel/2012-10/msg00434.html

> 
[...]

-- 
Eduardo

Re: [Qemu-devel] [PATCH qom-cpu 00/15 v8] target-i386: convert CPU features into properties, part 1

[Andreas Färber]

Am 05.06.2013 19:17, schrieb Eduardo Habkost:
> On Wed, Jun 05, 2013 at 07:04:59PM +0200, Andreas Färber wrote:
>> Am 05.06.2013 16:39, schrieb Igor Mammedov:
>>> On Wed, 05 Jun 2013 15:29:08 +0200
>>> Andreas Färber <afaerber@suse.de> wrote:
>>>> Why is conversion of dynamic properties to static properties still
>>>> needed after I applied a solution to override values of dynamic
>>>> properties with -global for 1.5?
>>> Do you mean qdev_prop_set_globals_for_type() & co?
>>
>> Yes.
>>
>>> If yes, then I recall it was acceptable hack to permit more clean
>>> approach for compat props fixes to work. And we promised Anthony to
>>> get rid of it when possible.
>>
>> Indeed, but no one talked about reverting to static properties as the
>> solution. :) Instead I was talking about solving this very general
>> problem at DeviceState / QOM level.
> 
> We have had this discussion before, and I remember Anthony saying that
> anything set using global properties _must_ be static properties,
> period.

Obviously I am not aware of that, might that have been an IRC discussion?!

> That was the main motivation we even started doing the static properties
> work, months ago.

Towards Paolo and me, Anthony rejected having static properties for QOM
at all! That was back when I temporarily maintained a qom-next tree
during some Hard Freeze, I had to unqueue patches to that effect.

>> Instead we have a very generic problem: instance_init is called
>> recursively, parents first, so a parent class cannot do any instance
>> initialization *after* its derived classes initialized the instance.
>> That's contrary to how realize and other QOM methods work but in
>> exchange for the flexibility put the burden of saving and calling the
>> parent's implementation onto subclasses.
>>
>> That's what I would like to change in some way, possibly a
>> instance_post_init hook or the like, similar to how DeviceState got its
>> own base class initialization hook to handle static props.
>> That would not only keep the work low in this case but may also solve
>> the virtio-net initialization problem reported elsewhere.
> 
> You mean this?
> https://lists.gnu.org/archive/html/qemu-devel/2012-10/msg00434.html

No, wasn't aware of that patchset yet, but yes, something like that I
had suggested in the qdev_set_custom_globals() context last Soft Freeze.

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 qom-cpu 00/15 v8] target-i386: convert CPU features into properties, part 1

[Eduardo Habkost]

On Wed, Jun 05, 2013 at 07:29:46PM +0200, Andreas Färber wrote:
> Am 05.06.2013 19:17, schrieb Eduardo Habkost:
> > On Wed, Jun 05, 2013 at 07:04:59PM +0200, Andreas Färber wrote:
> >> Am 05.06.2013 16:39, schrieb Igor Mammedov:
> >>> On Wed, 05 Jun 2013 15:29:08 +0200
> >>> Andreas Färber <afaerber@suse.de> wrote:
> >>>> Why is conversion of dynamic properties to static properties still
> >>>> needed after I applied a solution to override values of dynamic
> >>>> properties with -global for 1.5?
> >>> Do you mean qdev_prop_set_globals_for_type() & co?
> >>
> >> Yes.
> >>
> >>> If yes, then I recall it was acceptable hack to permit more clean
> >>> approach for compat props fixes to work. And we promised Anthony to
> >>> get rid of it when possible.
> >>
> >> Indeed, but no one talked about reverting to static properties as the
> >> solution. :) Instead I was talking about solving this very general
> >> problem at DeviceState / QOM level.
> > 
> > We have had this discussion before, and I remember Anthony saying that
> > anything set using global properties _must_ be static properties,
> > period.
> 
> Obviously I am not aware of that, might that have been an IRC discussion?!

Yeah, it was on IRC (on 2012-10-03). Quoting Anthony: "globals not
working with non-static properties is a feature, not a bug".

-- 
Eduardo

Re: [Qemu-devel] [PATCH qom-cpu 00/15 v8] target-i386: convert CPU features into properties, part 1

[Peter Maydell]

On 5 June 2013 19:31, Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Wed, Jun 05, 2013 at 07:29:46PM +0200, Andreas Färber wrote:
>> Am 05.06.2013 19:17, schrieb Eduardo Habkost:
>> > We have had this discussion before, and I remember Anthony saying that
>> > anything set using global properties _must_ be static properties,
>> > period.
>>
>> Obviously I am not aware of that, might that have been an IRC discussion?!
>
> Yeah, it was on IRC (on 2012-10-03). Quoting Anthony: "globals not
> working with non-static properties is a feature, not a bug".

Did it come with a rationale attached? Seems a bit weird...

-- PMM

Re: [Qemu-devel] [PATCH qom-cpu 00/15 v8] target-i386: convert CPU features into properties, part 1

[Eduardo Habkost]

On Wed, Jun 05, 2013 at 07:57:11PM +0100, Peter Maydell wrote:
> On 5 June 2013 19:31, Eduardo Habkost <ehabkost@redhat.com> wrote:
> > On Wed, Jun 05, 2013 at 07:29:46PM +0200, Andreas Färber wrote:
> >> Am 05.06.2013 19:17, schrieb Eduardo Habkost:
> >> > We have had this discussion before, and I remember Anthony saying that
> >> > anything set using global properties _must_ be static properties,
> >> > period.
> >>
> >> Obviously I am not aware of that, might that have been an IRC discussion?!
> >
> > Yeah, it was on IRC (on 2012-10-03). Quoting Anthony: "globals not
> > working with non-static properties is a feature, not a bug".
> 
> Did it come with a rationale attached? Seems a bit weird...

Excerpt from the chat:

 <ehabkost> imammedo2: aliguori: what about moving the current version forward, and convert that to static properties later, as part of the DeviceState work?
 <ehabkost> because we will need to support global properties only after converting to DeviceState, anyway
 <aliguori> imammedo, global properties are a qdev-only thing, i don't want to change that
 <aliguori> ehabkost, i think this has to happen as static properties from the beginnign
 <aliguori> otherwise we just dig ourselves further into the hole
 <aliguori> and it's so simple
 <aliguori> i can't understand why we'd make it harder than it needs to be :-)
 <ehabkost> aliguori: I don't understand why it has to be so harder, either. we could simply make global properties work with the dynamically-registered properties  ;-)
 <aliguori> ehabkost, problem #1: you can't introspect dynamic properties
 <aliguori> so if you do that, libvirt can't tell that they're there
 <aliguori> which means you also have to solve that problem
 <aliguori> and the approach bonzini and i agreed to on that was: only use static properties when you need to do introspection
 <ehabkost> aliguori: isn't this just a matter of ordering? we could initialize the list of properties dynamically, but at class_init time
 <aliguori> not, it has to do with how property introspection works
 <aliguori> b/c it's class based
 <aliguori> static properties are tied to classes
 <aliguori> but dynamic properties have no relationship to the class
 <aliguori> so there's no obvious way to determine whether a class has a particular property that's dynamic without instantiating an object first
 <aliguori> it gets a bit hairy
 <ehabkost> aliguori: I would be happy initializing the list of properties on class_init time
 <aliguori> that's not possible today
 <ehabkost> if DeviceClass.props were a list (still initialized by class_init) instead of an array, it would be much easier
 <aliguori> i've gotta run, but trust me, this rat holes quickly...

-- 
Eduardo

Re: [Qemu-devel] [PATCH qom-cpu 00/15 v8] target-i386: convert CPU features into properties, part 1

[Igor Mammedov]

On Wed, 05 Jun 2013 19:04:59 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 05.06.2013 16:39, schrieb Igor Mammedov:
> > On Wed, 05 Jun 2013 15:29:08 +0200
> > Andreas Färber <afaerber@suse.de> wrote:
> > 
> >> Am 05.06.2013 15:18, schrieb Igor Mammedov:
> >>> It's a rebase of v7 on current qom-cpu tree, since then some patches from it
> >>> were applied to master. Convertion of feature bits is left for part 2
> >>> since it's not ready yet.
> >>>
> >>> v7-v8:
> >>> * split out dynamic properties convertion patch into per property patches
> >>>   to simplify review
> >>> * drop feature bits convertion
> >>
> >> Why is conversion of dynamic properties to static properties still
> >> needed after I applied a solution to override values of dynamic
> >> properties with -global for 1.5?
> > Do you mean qdev_prop_set_globals_for_type() & co?
> 
> Yes.
> 
> > If yes, then I recall it was acceptable hack to permit more clean
> > approach for compat props fixes to work. And we promised Anthony to
> > get rid of it when possible.
> 
> Indeed, but no one talked about reverting to static properties as the
> solution. :) Instead I was talking about solving this very general
> problem at DeviceState / QOM level.
> 
> > Now more to the point,
> > 
> > 1. if CPU are to be created with device_add(wich is still  a goal)
> >    cpu_x86_create() won't be created, so it leaves rules out compat
> >    properties set by qdev_prop_set_globals_for_type().
> 
> It does not rule it out, but I think we all agree that we do not want
> calls of it cluttered over subclasses.
> 
> Instead we have a very generic problem: instance_init is called
> recursively, parents first, so a parent class cannot do any instance
> initialization *after* its derived classes initialized the instance.
> That's contrary to how realize and other QOM methods work but in
> exchange for the flexibility put the burden of saving and calling the
> parent's implementation onto subclasses.

So far all efforts to do it failed. But I agree that it should be solved
at this level since setting defaults/globals on properties before instance
is completely created (i.e all instance_init()s called) is just wrong.
But it's orthogonal to static vs dynamic properties question.

> That's what I would like to change in some way, possibly a
> instance_post_init hook or the like, similar to how DeviceState got its
> own base class initialization hook to handle static props.
> That would not only keep the work low in this case but may also solve
> the virtio-net initialization problem reported elsewhere.
> 
> I'm pretty sure if we moved instance_init into ObjectClass, we'd not
> only get an insurge of *Class structs but also people forgetting to
> save&call the parent type's implementation, resulting in all kinds of
> weird errors, so I don't consider this a real option.
> 
> >    Having properties converted by this series as static would open
> >    road for:
> >        - making cpu_x86_parse_featurestr() target specific hook that
> >          deals with legacy cpu_model parsing and converts provided
> >          features into global properties.
> 
> Such a hook could be implemented today, no dependency on static props, I
> just didn't see the need yet. sparc is the only other target where I
> stumbled over this so far and haven't found time to prepare that yet.
It could be done by board specific code before creating CPUs, I do not
have a preference here (but class hook looks a bit cleaner and less likely
to break).

> 
> >            as result during hot-add device_add can work without
> >            specifying +-foo1,foo2,level=XXX, it will be applied from
> >            globals.
> 
> Using globals for that is fine with me. That code is currently per-CPU
> whereas -cpu does not allow for differently configured CPUs. Mixed CPU
> configurations would be either instantiated from the board and via -device.
> 
> >        - above will allow to replace create_x86_cpu() with plain
> >          device_add when subclasses are implemented.
> > 
> > 2. I'd like to utilize default values of static properties for defining
> >    subclasses like here: https://github.com/imammedo/qemu/commit/a48e252a2800bf8dd56320e68e4f9517d0a25e5c
> >    - that would automatically provide benefit of class introspection
> >      without creating CPU instance
> >    - replace current CPU definitions array to a set of class_init_xxx
> >      for each subclass.
> 
> How and when do you actually want to inspect them? What's the use case?
> If we hardcode object_property_set_*() in instance_init then surely it's
> only inspectable in instances, much like the properties added for
> libvirt by Eduardo which didn't seem to disturb anyone. Peter actively
> chose the instance_init option for ARM CPUs, meaning they can only be
> inspected once created.
Yes, currently there is no way to do introspect property defaults at
class level, but it doesn't mean that we shouldn't try to move towards it.
Even if there was an interface to do so, it would still not be much useful
for i386 target since a lot of fixups at realize() stage, that needs to be
fixed as well for introspection at class level to work.

> However any field or pointer assignment in class_init will allow for
> some form of inspection after class creation (thinking of -cpu ? and
> query-cpus), including although not limited to today's x86_def_t.
Drawbacks in putting pointer to x86_def_t in class definition, is that
it could be beginning of bloating out class structure and would mean
implementation specific ways to inspect it.

If it's done with static properties, than needed pointer already
exists in DeviceClass and there could be a generic way to inspect it
for Device derived classes.

> 
> The only thing that seems to require static properties today is
> `qemu-system-x86_64 -device x86_64-cpu,?`, which happens to terminate
> right after, so might as well create an instance of the type to list all
> its properties.[*]
> I expect `qemu-system-x86_64 -device Haswell-x86_64-cpu,level=4` to work
> the same with dynamic and static properties.
> 
> `info qtree` by comparison works only at runtime even though operating
> on static properties and can easily be replaced with qom-list / qom-get.
> 
> What I dislike about static properties is that they seem a relic from
> qdev times that add yet another layer of abstraction above QOM
> properties. The only practical differences are:
> * QOM properties are added in instance_init and added to Object whereas
> qdev properties are assigned in class_init and use external storage, and
> * qdev properties specify a default value at property level whereas QOM
> properties set the default value in a implementation-dependent way,
> usually field assignment from either class field or a hardcoded value.
They might be relic but convenient and widely used one among "Devices".
Wouldn't it be better to have more unified property handling code vs
impl. specific?
It might be possible that if we come to having generic class properties
some day, that static properties would be much easier to convert to them.

Is there other reason not to use static properties besides "qdev relic"?
Current series simplifies x86_cpu_initfn() and makes property handling
more alike with other devices. And static properties will simplify
target-i386/cpu.c even more after feature_bits are converted to it.
And if static properties are extended to support value validation, a number
of custom property setters could be replaced by on-liners like it's done
for basic types now, reducing code base even more.


> 
> To me that poses the question of whether we may want to have a
> QTAILQ_HEAD(, ObjectProperty) properties;
> not only in Object but also in ObjectClass for inspection of name and
> type fields. We probably don't want to rule out dynamically added
> properties except for Peter's array properties in ARM devices.
> 
> [*] PowerPCCPU is known to leak memory on finalization, I have a patch
> pending. But I carefully employed QOM realize in CPUState to allow for
> creating an instance without creating vCPU threads so that it can easily
> be destroyed again (mostly thinking of property assignment errors back
> then), any problems with that should get fixed, ideally with test cases;
> QOM unrealize by comparison is not yet (widely) supported by CPUs I fear.
> 
> Regards,
> Andreas
> 

Re: [Qemu-devel] [PATCH 01/15] target-i386: cpu: convert 'family' to static property

[Eduardo Habkost]

On Wed, Jun 05, 2013 at 03:18:32PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> ---
>  target-i386/cpu.c |   17 ++++++++++++++---
>  1 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 1a501d9..c87cc9f 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1195,6 +1195,14 @@ static void x86_cpuid_version_set_family(Object *obj, Visitor *v, void *opaque,
>      }
>  }
>  
> +static PropertyInfo qdev_prop_family = {
> +    .name  = "uint32",
> +    .get   = x86_cpuid_version_get_family,
> +    .set   = x86_cpuid_version_set_family,
> +};
> +#define DEFINE_PROP_FAMILY(_n)                                                 \
> +    DEFINE_PROP(_n, X86CPU, env.cpuid_version, qdev_prop_family, uint32_t)
> +
>  static void x86_cpuid_version_get_model(Object *obj, Visitor *v, void *opaque,
>                                          const char *name, Error **errp)
>  {
> @@ -1475,6 +1483,11 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
>      error_propagate(errp, err);
>  }
>  
> +static Property cpu_x86_properties[] = {
> +    DEFINE_PROP_FAMILY("family"),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
>  {
>      x86_def_t *def;
> @@ -2451,9 +2464,6 @@ static void x86_cpu_initfn(Object *obj)
>      cs->env_ptr = env;
>      cpu_exec_init(env);
>  
> -    object_property_add(obj, "family", "int",
> -                        x86_cpuid_version_get_family,
> -                        x86_cpuid_version_set_family, NULL, NULL, NULL);
>      object_property_add(obj, "model", "int",
>                          x86_cpuid_version_get_model,
>                          x86_cpuid_version_set_model, NULL, NULL, NULL);
> @@ -2514,6 +2524,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>      xcc->parent_realize = dc->realize;
>      dc->realize = x86_cpu_realizefn;
>      dc->bus_type = TYPE_ICC_BUS;
> +    dc->props = cpu_x86_properties;
>  
>      xcc->parent_reset = cc->reset;
>      cc->reset = x86_cpu_reset;
> -- 
> 1.7.1
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 02/15] target-i386: cpu: convert 'model' to static property

[Eduardo Habkost]

On Wed, Jun 05, 2013 at 03:18:33PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> ---
>  target-i386/cpu.c |   12 +++++++++---
>  1 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index c87cc9f..5d379af 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1238,6 +1238,14 @@ static void x86_cpuid_version_set_model(Object *obj, Visitor *v, void *opaque,
>      env->cpuid_version |= ((value & 0xf) << 4) | ((value >> 4) << 16);
>  }
>  
> +static PropertyInfo qdev_prop_model = {
> +    .name  = "uint32",
> +    .get   = x86_cpuid_version_get_model,
> +    .set   = x86_cpuid_version_set_model,
> +};
> +#define DEFINE_PROP_MODEL(_n)                                                  \
> +    DEFINE_PROP(_n, X86CPU, env.cpuid_version, qdev_prop_model, uint32_t)
> +
>  static void x86_cpuid_version_get_stepping(Object *obj, Visitor *v,
>                                             void *opaque, const char *name,
>                                             Error **errp)
> @@ -1485,6 +1493,7 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
>  
>  static Property cpu_x86_properties[] = {
>      DEFINE_PROP_FAMILY("family"),
> +    DEFINE_PROP_MODEL("model"),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -2464,9 +2473,6 @@ static void x86_cpu_initfn(Object *obj)
>      cs->env_ptr = env;
>      cpu_exec_init(env);
>  
> -    object_property_add(obj, "model", "int",
> -                        x86_cpuid_version_get_model,
> -                        x86_cpuid_version_set_model, NULL, NULL, NULL);
>      object_property_add(obj, "stepping", "int",
>                          x86_cpuid_version_get_stepping,
>                          x86_cpuid_version_set_stepping, NULL, NULL, NULL);
> -- 
> 1.7.1
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 03/15] target-i386: cpu: convert 'stepping' to static property

[Eduardo Habkost]

On Wed, Jun 05, 2013 at 03:18:34PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>


> ---
>  target-i386/cpu.c |   12 +++++++++---
>  1 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 5d379af..1d997ee 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1282,6 +1282,14 @@ static void x86_cpuid_version_set_stepping(Object *obj, Visitor *v,
>      env->cpuid_version |= value & 0xf;
>  }
>  
> +static PropertyInfo qdev_prop_stepping = {
> +    .name  = "uint32",
> +    .get   = x86_cpuid_version_get_stepping,
> +    .set   = x86_cpuid_version_set_stepping,
> +};
> +#define DEFINE_PROP_STEPPING(_n)                                               \
> +    DEFINE_PROP(_n, X86CPU, env.cpuid_version, qdev_prop_stepping, uint32_t)
> +
>  static void x86_cpuid_get_level(Object *obj, Visitor *v, void *opaque,
>                                  const char *name, Error **errp)
>  {
> @@ -1494,6 +1502,7 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
>  static Property cpu_x86_properties[] = {
>      DEFINE_PROP_FAMILY("family"),
>      DEFINE_PROP_MODEL("model"),
> +    DEFINE_PROP_STEPPING("stepping"),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -2473,9 +2482,6 @@ static void x86_cpu_initfn(Object *obj)
>      cs->env_ptr = env;
>      cpu_exec_init(env);
>  
> -    object_property_add(obj, "stepping", "int",
> -                        x86_cpuid_version_get_stepping,
> -                        x86_cpuid_version_set_stepping, NULL, NULL, NULL);
>      object_property_add(obj, "level", "int",
>                          x86_cpuid_get_level,
>                          x86_cpuid_set_level, NULL, NULL, NULL);
> -- 
> 1.7.1
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 04/15] target-i386: cpu: convert 'level' to static property

[Eduardo Habkost]

On Wed, Jun 05, 2013 at 03:18:35PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> ---
>  target-i386/cpu.c |   20 +-------------------
>  1 files changed, 1 insertions(+), 19 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 1d997ee..bb86484 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1290,22 +1290,6 @@ static PropertyInfo qdev_prop_stepping = {
>  #define DEFINE_PROP_STEPPING(_n)                                               \
>      DEFINE_PROP(_n, X86CPU, env.cpuid_version, qdev_prop_stepping, uint32_t)
>  
> -static void x86_cpuid_get_level(Object *obj, Visitor *v, void *opaque,
> -                                const char *name, Error **errp)
> -{
> -    X86CPU *cpu = X86_CPU(obj);
> -
> -    visit_type_uint32(v, &cpu->env.cpuid_level, name, errp);
> -}
> -
> -static void x86_cpuid_set_level(Object *obj, Visitor *v, void *opaque,
> -                                const char *name, Error **errp)
> -{
> -    X86CPU *cpu = X86_CPU(obj);
> -
> -    visit_type_uint32(v, &cpu->env.cpuid_level, name, errp);
> -}
> -
>  static void x86_cpuid_get_xlevel(Object *obj, Visitor *v, void *opaque,
>                                   const char *name, Error **errp)
>  {
> @@ -1503,6 +1487,7 @@ static Property cpu_x86_properties[] = {
>      DEFINE_PROP_FAMILY("family"),
>      DEFINE_PROP_MODEL("model"),
>      DEFINE_PROP_STEPPING("stepping"),
> +    DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -2482,9 +2467,6 @@ static void x86_cpu_initfn(Object *obj)
>      cs->env_ptr = env;
>      cpu_exec_init(env);
>  
> -    object_property_add(obj, "level", "int",
> -                        x86_cpuid_get_level,
> -                        x86_cpuid_set_level, NULL, NULL, NULL);
>      object_property_add(obj, "xlevel", "int",
>                          x86_cpuid_get_xlevel,
>                          x86_cpuid_set_xlevel, NULL, NULL, NULL);
> -- 
> 1.7.1
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 05/15] target-i386: cpu: convert 'xlevel' to static property

[Eduardo Habkost]

On Wed, Jun 05, 2013 at 03:18:36PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> ---
>  target-i386/cpu.c |   20 +-------------------
>  1 files changed, 1 insertions(+), 19 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index bb86484..f42282e 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1290,22 +1290,6 @@ static PropertyInfo qdev_prop_stepping = {
>  #define DEFINE_PROP_STEPPING(_n)                                               \
>      DEFINE_PROP(_n, X86CPU, env.cpuid_version, qdev_prop_stepping, uint32_t)
>  
> -static void x86_cpuid_get_xlevel(Object *obj, Visitor *v, void *opaque,
> -                                 const char *name, Error **errp)
> -{
> -    X86CPU *cpu = X86_CPU(obj);
> -
> -    visit_type_uint32(v, &cpu->env.cpuid_xlevel, name, errp);
> -}
> -
> -static void x86_cpuid_set_xlevel(Object *obj, Visitor *v, void *opaque,
> -                                 const char *name, Error **errp)
> -{
> -    X86CPU *cpu = X86_CPU(obj);
> -
> -    visit_type_uint32(v, &cpu->env.cpuid_xlevel, name, errp);
> -}
> -
>  static char *x86_cpuid_get_vendor(Object *obj, Error **errp)
>  {
>      X86CPU *cpu = X86_CPU(obj);
> @@ -1488,6 +1472,7 @@ static Property cpu_x86_properties[] = {
>      DEFINE_PROP_MODEL("model"),
>      DEFINE_PROP_STEPPING("stepping"),
>      DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
> +    DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -2467,9 +2452,6 @@ static void x86_cpu_initfn(Object *obj)
>      cs->env_ptr = env;
>      cpu_exec_init(env);
>  
> -    object_property_add(obj, "xlevel", "int",
> -                        x86_cpuid_get_xlevel,
> -                        x86_cpuid_set_xlevel, NULL, NULL, NULL);
>      object_property_add_str(obj, "vendor",
>                              x86_cpuid_get_vendor,
>                              x86_cpuid_set_vendor, NULL);
> -- 
> 1.7.1
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 06/15] target-i386: cpu: convert 'vendor' to static property

[Eduardo Habkost]

On Wed, Jun 05, 2013 at 03:18:37PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> ---
>  target-i386/cpu.c |   31 ++++++++++++++++++++++++-------
>  1 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index f42282e..21e7334 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1290,7 +1290,8 @@ static PropertyInfo qdev_prop_stepping = {
>  #define DEFINE_PROP_STEPPING(_n)                                               \
>      DEFINE_PROP(_n, X86CPU, env.cpuid_version, qdev_prop_stepping, uint32_t)
>  
> -static char *x86_cpuid_get_vendor(Object *obj, Error **errp)
> +static void x86_cpuid_get_vendor(Object *obj, Visitor *v, void *opaque,
> +                                 const char *name, Error **errp)
>  {
>      X86CPU *cpu = X86_CPU(obj);
>      CPUX86State *env = &cpu->env;
> @@ -1299,16 +1300,23 @@ static char *x86_cpuid_get_vendor(Object *obj, Error **errp)
>      value = (char *)g_malloc(CPUID_VENDOR_SZ + 1);
>      x86_cpu_vendor_words2str(value, env->cpuid_vendor1, env->cpuid_vendor2,
>                               env->cpuid_vendor3);
> -    return value;
> +    visit_type_str(v, &value, name, errp);
> +    g_free(value);
>  }
>  
> -static void x86_cpuid_set_vendor(Object *obj, const char *value,
> -                                 Error **errp)
> +static void x86_cpuid_set_vendor(Object *obj, Visitor *v, void *opaque,
> +                                 const char *name, Error **errp)
>  {
>      X86CPU *cpu = X86_CPU(obj);
>      CPUX86State *env = &cpu->env;
> +    char *value;
>      int i;
>  
> +    visit_type_str(v, &value, name, errp);
> +    if (error_is_set(errp)) {
> +        return;
> +    }
> +
>      if (strlen(value) != CPUID_VENDOR_SZ) {
>          error_set(errp, QERR_PROPERTY_VALUE_BAD, "",
>                    "vendor", value);
> @@ -1323,6 +1331,17 @@ static void x86_cpuid_set_vendor(Object *obj, const char *value,
>          env->cpuid_vendor2 |= ((uint8_t)value[i + 4]) << (8 * i);
>          env->cpuid_vendor3 |= ((uint8_t)value[i + 8]) << (8 * i);
>      }
> +    g_free(value);
> +}
> +
> +static PropertyInfo qdev_prop_vendor = {
> +    .name  = "string",
> +    .get   = x86_cpuid_get_vendor,
> +    .set   = x86_cpuid_set_vendor,
> +};
> +#define DEFINE_PROP_VENDOR(_n) {                                               \
> +    .name = _n,                                                                \
> +    .info  = &qdev_prop_vendor                                                 \
>  }
>  
>  static char *x86_cpuid_get_model_id(Object *obj, Error **errp)
> @@ -1473,6 +1492,7 @@ static Property cpu_x86_properties[] = {
>      DEFINE_PROP_STEPPING("stepping"),
>      DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
>      DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
> +    DEFINE_PROP_VENDOR("vendor"),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -2452,9 +2472,6 @@ static void x86_cpu_initfn(Object *obj)
>      cs->env_ptr = env;
>      cpu_exec_init(env);
>  
> -    object_property_add_str(obj, "vendor",
> -                            x86_cpuid_get_vendor,
> -                            x86_cpuid_set_vendor, NULL);
>      object_property_add_str(obj, "model-id",
>                              x86_cpuid_get_model_id,
>                              x86_cpuid_set_model_id, NULL);
> -- 
> 1.7.1
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 07/15] target-i386: cpu: convert 'model-id' to static property

[Eduardo Habkost]

On Wed, Jun 05, 2013 at 03:18:38PM +0200, Igor Mammedov wrote:
> * check "if (model_id == NULL)" looks unnecessary now, since all
> builtin model-ids are not NULL and user shouldn't be able to set
> it NULL (cpumodel string parsing code takes care of it, if feature
> is specified as "model-id=" on command line, its parsing will
> result in an empty string as value).

Correct. Even if some code somewhere passes NULL to
object_property_parse(), StringInputVisitor's parse_type_str() checks
for NULL and sets error if that's the case.

> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> ---
>  target-i386/cpu.c |   31 ++++++++++++++++++++++---------
>  1 files changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 21e7334..9f6fe06 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1344,7 +1344,8 @@ static PropertyInfo qdev_prop_vendor = {
>      .info  = &qdev_prop_vendor                                                 \
>  }
>  
> -static char *x86_cpuid_get_model_id(Object *obj, Error **errp)
> +static void x86_cpuid_get_model_id(Object *obj, Visitor *v, void *opaque,
> +                                   const char *name, Error **errp)
>  {
>      X86CPU *cpu = X86_CPU(obj);
>      CPUX86State *env = &cpu->env;
> @@ -1356,18 +1357,21 @@ static char *x86_cpuid_get_model_id(Object *obj, Error **errp)
>          value[i] = env->cpuid_model[i >> 2] >> (8 * (i & 3));
>      }
>      value[48] = '\0';
> -    return value;
> +    visit_type_str(v, &value, name, errp);
> +    g_free(value);
>  }
>  
> -static void x86_cpuid_set_model_id(Object *obj, const char *model_id,
> -                                   Error **errp)
> +static void x86_cpuid_set_model_id(Object *obj, Visitor *v, void *opaque,
> +                                   const char *name, Error **errp)
>  {
>      X86CPU *cpu = X86_CPU(obj);
>      CPUX86State *env = &cpu->env;
>      int c, len, i;
> +    char *model_id;
>  
> -    if (model_id == NULL) {
> -        model_id = "";
> +    visit_type_str(v, &model_id, name, errp);
> +    if (error_is_set(errp)) {
> +        return;
>      }
>      len = strlen(model_id);
>      memset(env->cpuid_model, 0, 48);
> @@ -1379,6 +1383,17 @@ static void x86_cpuid_set_model_id(Object *obj, const char *model_id,
>          }
>          env->cpuid_model[i >> 2] |= c << (8 * (i & 3));
>      }
> +    g_free(model_id);
> +}
> +
> +static PropertyInfo qdev_prop_model_id = {
> +    .name  = "string",
> +    .get   = x86_cpuid_get_model_id,
> +    .set   = x86_cpuid_set_model_id,
> +};
> +#define DEFINE_PROP_MODEL_ID(_n) {                                             \
> +    .name  = _n,                                                               \
> +    .info  = &qdev_prop_model_id                                               \
>  }
>  
>  static void x86_cpuid_get_tsc_freq(Object *obj, Visitor *v, void *opaque,
> @@ -1493,6 +1508,7 @@ static Property cpu_x86_properties[] = {
>      DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
>      DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
>      DEFINE_PROP_VENDOR("vendor"),
> +    DEFINE_PROP_MODEL_ID("model-id"),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -2472,9 +2488,6 @@ static void x86_cpu_initfn(Object *obj)
>      cs->env_ptr = env;
>      cpu_exec_init(env);
>  
> -    object_property_add_str(obj, "model-id",
> -                            x86_cpuid_get_model_id,
> -                            x86_cpuid_set_model_id, NULL);
>      object_property_add(obj, "tsc-frequency", "int",
>                          x86_cpuid_get_tsc_freq,
>                          x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
> -- 
> 1.7.1
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 08/15] target-i386: cpu: convert 'tsc-frequency' to static property

[Eduardo Habkost]

On Wed, Jun 05, 2013 at 03:18:39PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  target-i386/cpu.c |   12 +++++++++---
>  1 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 9f6fe06..ec6d33f 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1427,6 +1427,14 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
>      cpu->env.tsc_khz = value / 1000;
>  }
>  
> +static PropertyInfo qdev_prop_tsc_freq = {
> +    .name  = "int64",

What about making it "uint64" instead of keeping it signed? We already
reject negative values.

As this patch just converts the current code/semantics to use static
properties, and the change to uint64 could be done in a separate patch:

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>


> +    .get   = x86_cpuid_get_tsc_freq,
> +    .set   = x86_cpuid_set_tsc_freq,
> +};
> +#define DEFINE_PROP_TSC_FREQ(_n)                                               \
> +    DEFINE_PROP(_n, X86CPU, env.tsc_khz, qdev_prop_tsc_freq, int32_t)
> +
>  static void x86_cpuid_get_apic_id(Object *obj, Visitor *v, void *opaque,
>                                    const char *name, Error **errp)
>  {
> @@ -1509,6 +1517,7 @@ static Property cpu_x86_properties[] = {
>      DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
>      DEFINE_PROP_VENDOR("vendor"),
>      DEFINE_PROP_MODEL_ID("model-id"),
> +    DEFINE_PROP_TSC_FREQ("tsc-frequency"),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -2488,9 +2497,6 @@ static void x86_cpu_initfn(Object *obj)
>      cs->env_ptr = env;
>      cpu_exec_init(env);
>  
> -    object_property_add(obj, "tsc-frequency", "int",
> -                        x86_cpuid_get_tsc_freq,
> -                        x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
>      object_property_add(obj, "apic-id", "int",
>                          x86_cpuid_get_apic_id,
>                          x86_cpuid_set_apic_id, NULL, NULL, NULL);
> -- 
> 1.7.1
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 09/15] target-i386: move hyperv_* static globals to CPUState

[Eduardo Habkost]

On Wed, Jun 05, 2013 at 03:18:40PM +0200, Igor Mammedov wrote:
> - since hyperv_* helper functions are used only in target-i386/kvm.c
>   move them there as static helpers
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Requestd-By: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>


-- 
Eduardo

Re: [Qemu-devel] [PATCH 10/15] target-i386: convert 'hv_spinlocks' to static property

[Eduardo Habkost]

On Wed, Jun 05, 2013 at 03:18:41PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
[...]
> @@ -1632,6 +1677,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
>              } else if (!strcmp(featurestr, "hv-spinlocks")) {
>                  char *err;
>                  const int min = 0xFFF;
> +                char num[32];
>                  numvalue = strtoul(val, &err, 0);
>                  if (!*val || *err) {
>                      error_setg(errp, "bad numerical value %s", val);
> @@ -1643,7 +1689,8 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
>                              min);
>                      numvalue = min;
>                  }
> -                env->hyperv_spinlock_attempts = numvalue;
> +                snprintf(num, sizeof(num), "%" PRId32, numvalue);
> +                object_property_parse(OBJECT(cpu), num, featurestr, errp);

Why not use object_property_set_int()?

-- 
Eduardo

Re: [Qemu-devel] [PATCH 10/15] target-i386: convert 'hv_spinlocks' to static property

[Eduardo Habkost]

On Tue, Jun 25, 2013 at 05:30:50PM -0300, Eduardo Habkost wrote:
> On Wed, Jun 05, 2013 at 03:18:41PM +0200, Igor Mammedov wrote:
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> [...]
> > @@ -1632,6 +1677,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
> >              } else if (!strcmp(featurestr, "hv-spinlocks")) {
> >                  char *err;
> >                  const int min = 0xFFF;
> > +                char num[32];
> >                  numvalue = strtoul(val, &err, 0);
> >                  if (!*val || *err) {
> >                      error_setg(errp, "bad numerical value %s", val);
> > @@ -1643,7 +1689,8 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
> >                              min);
> >                      numvalue = min;
> >                  }
> > -                env->hyperv_spinlock_attempts = numvalue;
> > +                snprintf(num, sizeof(num), "%" PRId32, numvalue);
> > +                object_property_parse(OBJECT(cpu), num, featurestr, errp);
> 
> Why not use object_property_set_int()?

Oh, I believe I have asked that before and you have already explained
it: you are using strings to allow the existing object_property_parse()
calls to be easily converted to qdev_prop_register_global() calls later.
Correct?

-- 
Eduardo

Re: [Qemu-devel] [PATCH 10/15] target-i386: convert 'hv_spinlocks' to static property

[Igor Mammedov]

On Tue, 25 Jun 2013 17:34:44 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Tue, Jun 25, 2013 at 05:30:50PM -0300, Eduardo Habkost wrote:
> > On Wed, Jun 05, 2013 at 03:18:41PM +0200, Igor Mammedov wrote:
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> > [...]
> > > @@ -1632,6 +1677,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
> > >              } else if (!strcmp(featurestr, "hv-spinlocks")) {
> > >                  char *err;
> > >                  const int min = 0xFFF;
> > > +                char num[32];
> > >                  numvalue = strtoul(val, &err, 0);
> > >                  if (!*val || *err) {
> > >                      error_setg(errp, "bad numerical value %s", val);
> > > @@ -1643,7 +1689,8 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
> > >                              min);
> > >                      numvalue = min;
> > >                  }
> > > -                env->hyperv_spinlock_attempts = numvalue;
> > > +                snprintf(num, sizeof(num), "%" PRId32, numvalue);
> > > +                object_property_parse(OBJECT(cpu), num, featurestr, errp);
> > 
> > Why not use object_property_set_int()?
> 
> Oh, I believe I have asked that before and you have already explained
> it: you are using strings to allow the existing object_property_parse()
> calls to be easily converted to qdev_prop_register_global() calls later.
> Correct?
> 

Yep, this will be used later to consolidate code.

Re: [Qemu-devel] [PATCH 08/15] target-i386: cpu: convert 'tsc-frequency' to static property

[Igor Mammedov]

On Mon, 24 Jun 2013 14:09:43 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Wed, Jun 05, 2013 at 03:18:39PM +0200, Igor Mammedov wrote:
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  target-i386/cpu.c |   12 +++++++++---
> >  1 files changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 9f6fe06..ec6d33f 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1427,6 +1427,14 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
> >      cpu->env.tsc_khz = value / 1000;
> >  }
> >  
> > +static PropertyInfo qdev_prop_tsc_freq = {
> > +    .name  = "int64",
> 
> What about making it "uint64" instead of keeping it signed? We already
> reject negative values.
> 
> As this patch just converts the current code/semantics to use static
> properties, and the change to uint64 could be done in a separate patch:
Yes, purpose of patch is to make conversion and nothing else, so it would be easy
to review.

> 
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> 
> > +    .get   = x86_cpuid_get_tsc_freq,
> > +    .set   = x86_cpuid_set_tsc_freq,
> > +};
> > +#define DEFINE_PROP_TSC_FREQ(_n)                                               \
> > +    DEFINE_PROP(_n, X86CPU, env.tsc_khz, qdev_prop_tsc_freq, int32_t)
> > +
> >  static void x86_cpuid_get_apic_id(Object *obj, Visitor *v, void *opaque,
> >                                    const char *name, Error **errp)
> >  {
> > @@ -1509,6 +1517,7 @@ static Property cpu_x86_properties[] = {
> >      DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
> >      DEFINE_PROP_VENDOR("vendor"),
> >      DEFINE_PROP_MODEL_ID("model-id"),
> > +    DEFINE_PROP_TSC_FREQ("tsc-frequency"),
> >      DEFINE_PROP_END_OF_LIST(),
> >  };
> >  
> > @@ -2488,9 +2497,6 @@ static void x86_cpu_initfn(Object *obj)
> >      cs->env_ptr = env;
> >      cpu_exec_init(env);
> >  
> > -    object_property_add(obj, "tsc-frequency", "int",
> > -                        x86_cpuid_get_tsc_freq,
> > -                        x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
> >      object_property_add(obj, "apic-id", "int",
> >                          x86_cpuid_get_apic_id,
> >                          x86_cpuid_set_apic_id, NULL, NULL, NULL);
> > -- 
> > 1.7.1
> > 
> > 
> 

Re: [Qemu-devel] [PATCH 01/15] target-i386: cpu: convert 'family' to static property

[chenfan]

On Mon, 2013-06-24 at 13:53 -0300, Eduardo Habkost wrote:
> On Wed, Jun 05, 2013 at 03:18:32PM +0200, Igor Mammedov wrote:
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> 
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> 
> > ---
> >  target-i386/cpu.c |   17 ++++++++++++++---
> >  1 files changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 1a501d9..c87cc9f 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1195,6 +1195,14 @@ static void x86_cpuid_version_set_family(Object *obj, Visitor *v, void *opaque,
> >      }
> >  }
> >  
> > +static PropertyInfo qdev_prop_family = {
> > +    .name  = "uint32",
> > +    .get   = x86_cpuid_version_get_family,
> > +    .set   = x86_cpuid_version_set_family,
> > +};
> > +#define DEFINE_PROP_FAMILY(_n)                                                 \
> > +    DEFINE_PROP(_n, X86CPU, env.cpuid_version, qdev_prop_family, uint32_t)
> > +
> >  static void x86_cpuid_version_get_model(Object *obj, Visitor *v, void *opaque,
> >                                          const char *name, Error **errp)
> >  {
> > @@ -1475,6 +1483,11 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
> >      error_propagate(errp, err);
> >  }
> >  
> > +static Property cpu_x86_properties[] = {
> > +    DEFINE_PROP_FAMILY("family"),
> > +    DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> >  static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
> >  {
> >      x86_def_t *def;
> > @@ -2451,9 +2464,6 @@ static void x86_cpu_initfn(Object *obj)
> >      cs->env_ptr = env;
> >      cpu_exec_init(env);
> >  
> > -    object_property_add(obj, "family", "int",
> > -                        x86_cpuid_version_get_family,
> > -                        x86_cpuid_version_set_family, NULL, NULL, NULL);
> >      object_property_add(obj, "model", "int",
> >                          x86_cpuid_version_get_model,
> >                          x86_cpuid_version_set_model, NULL, NULL, NULL);
> > @@ -2514,6 +2524,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
> >      xcc->parent_realize = dc->realize;
> >      dc->realize = x86_cpu_realizefn;
> >      dc->bus_type = TYPE_ICC_BUS;
> > +    dc->props = cpu_x86_properties;
> >  
> >      xcc->parent_reset = cc->reset;
> >      cc->reset = x86_cpu_reset;
> > -- 
> > 1.7.1
> > 
> > 
> 

Re: [Qemu-devel] [PATCH 09/15] target-i386: move hyperv_* static globals to CPUState

[Andreas Färber]

Am 05.06.2013 15:18, schrieb Igor Mammedov:
> - since hyperv_* helper functions are used only in target-i386/kvm.c
>   move them there as static helpers
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Requestd-By: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target-i386/Makefile.objs |    2 +-
>  target-i386/cpu.c         |   16 +++++++---
>  target-i386/cpu.h         |    7 +++++
>  target-i386/hyperv.c      |   64 ---------------------------------------------
>  target-i386/hyperv.h      |   45 -------------------------------
>  target-i386/kvm.c         |   36 ++++++++++++++++++-------
>  6 files changed, 45 insertions(+), 125 deletions(-)
>  delete mode 100644 target-i386/hyperv.c
>  delete mode 100644 target-i386/hyperv.h

I proposed a v9 of this that I'd like to queue if you're okay with it:
http://patchwork.ozlabs.org/patch/257454/

I do wonder if we should transfer Vadim's authorship info somewhere?
It is being deleted along with the target-i386/hyperv.[hc] files.

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 09/15] target-i386: move hyperv_* static globals to CPUState

[Igor Mammedov]

On Mon, 08 Jul 2013 13:48:55 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 05.06.2013 15:18, schrieb Igor Mammedov:
> > - since hyperv_* helper functions are used only in target-i386/kvm.c
> >   move them there as static helpers
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > Requestd-By: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  target-i386/Makefile.objs |    2 +-
> >  target-i386/cpu.c         |   16 +++++++---
> >  target-i386/cpu.h         |    7 +++++
> >  target-i386/hyperv.c      |   64 ---------------------------------------------
> >  target-i386/hyperv.h      |   45 -------------------------------
> >  target-i386/kvm.c         |   36 ++++++++++++++++++-------
> >  6 files changed, 45 insertions(+), 125 deletions(-)
> >  delete mode 100644 target-i386/hyperv.c
> >  delete mode 100644 target-i386/hyperv.h
> 
> I proposed a v9 of this that I'd like to queue if you're okay with it:
> http://patchwork.ozlabs.org/patch/257454/
I'm ok with it.

> 
> I do wonder if we should transfer Vadim's authorship info somewhere?
> It is being deleted along with the target-i386/hyperv.[hc] files.
Feel free to add it, if Vadim doesn't object to it.

> 
> Regards,
> Andreas
> 

Thanks,
   Igor