[Qemu-devel] [NOT RFC] target-i386: prepare for convertion to subclasses

[Igor Mammedov <imammedo@redhat.com>]

crude attempt to show how to move out cpu_model string handling out of cpu.c
and make CPU more suitable to converting into subclasses.

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

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e266792..b5dcf56 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1099,76 +1099,15 @@ static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t *def, Error **errp)
     object_property_set_bool(OBJECT(cpu), true, "hypervisor", errp);
 }
 
-/* convert legacy cpumodel string to string cpu_name and
- * a uniforms set of custom features that will be applied to CPU
- * using object_property_parse()
- */
-static void compat_normalize_cpu_model(const char *cpu_model, char **cpu_name,
-                                        QDict **features, Error **errp)
-{
-
-    char *s = g_strdup(cpu_model);
-    char *featurestr, *sptr;
-
-    *cpu_name = strtok_r(s, ",", &sptr);
-    *features = qdict_new();
-
-    featurestr = strtok_r(NULL, ",", &sptr);
-    while (featurestr) {
-        char *val;
-        if (featurestr[0] == '+') {
-            /*
-             * preseve legacy behaviour, if feature was disabled once
-             * do not allow to enable it again
-             */
-            if (!qdict_haskey(*features, featurestr + 1)) {
-                qdict_put(*features, featurestr + 1, qstring_from_str("on"));
-            }
-        } else if (featurestr[0] == '-') {
-            qdict_put(*features, featurestr + 1, qstring_from_str("off"));
-        } else {
-            val = strchr(featurestr, '=');
-            if (val) {
-                *val = 0; val++;
-                if (!strcmp(featurestr, "vendor")) {
-                    qdict_put(*features, "vendor-override",
-                              qstring_from_str("on"));
-                    qdict_put(*features, featurestr, qstring_from_str(val));
-                } else if (!strcmp(featurestr, "tsc_freq")) {
-                    qdict_put(*features, "tsc-frequency",
-                              qstring_from_str(val));
-                } else {
-                    qdict_put(*features, featurestr, qstring_from_str(val));
-                }
-            } else {
-                qdict_put(*features, featurestr, qstring_from_str("on"));
-            }
-        }
-
-        featurestr = strtok_r(NULL, ",", &sptr);
-    }
-
-    return;
-}
-
 static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
                                 const char *cpu_model, Error **errp)
 {
     x86_def_t *def;
 
-    QDict *features;
-    const QDictEntry *ent;
-    char *name;
-
-    compat_normalize_cpu_model(cpu_model, &name, &features, errp);
-    if (error_is_set(errp)) {
-        goto error;
-    }
-
     for (def = x86_defs; def; def = def->next)
-        if (name && !strcmp(name, def->name))
+        if (cpu_model && !strcmp(cpu_model, def->name))
             break;
-    if (kvm_enabled() && name && strcmp(name, "host") == 0) {
+    if (kvm_enabled() && cpu_model && strcmp(cpu_model, "host") == 0) {
         cpu_x86_fill_host(x86_cpu_def);
     } else if (!def) {
         goto error;
@@ -1176,23 +1115,9 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
         memcpy(x86_cpu_def, def, sizeof(*def));
     }
 
-    cpudef_2_x86_cpu(cpu, def, errp);
-
-    for (ent = qdict_first(features); ent; ent = qdict_next(features, ent)) {
-        const QString *qval = qobject_to_qstring(qdict_entry_value(ent));
-        object_property_parse(OBJECT(cpu), qstring_get_str(qval),
-                              qdict_entry_key(ent), errp);
-        if (error_is_set(errp)) {
-            goto error;
-        }
-    }
-    QDECREF(features);
-
-    g_free(name);
     return 0;
 
 error:
-    g_free(name);
     if (!error_is_set(errp)) {
         error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
     }
@@ -1302,6 +1227,10 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
     if (cpu_x86_find_by_name(cpu, def, cpu_model, &error) < 0)
         goto out;
 
+    /* ==> should go into initfn */
+    cpudef_2_x86_cpu(cpu, def, &error);
+    /* <== */
+
 out:
     if (error_is_set(&error)) {
         fprintf(stderr, "%s\n", error_get_pretty(error));
diff --git a/target-i386/helper.c b/target-i386/helper.c
index a0e4c89..17acb4e 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -23,6 +23,8 @@
 #include "sysemu.h"
 #include "monitor.h"
 #endif
+#include "qdict.h"
+#include "qstring.h"
 
 //#define DEBUG_MMU
 
@@ -1147,20 +1149,105 @@ int cpu_x86_get_descr_debug(CPUX86State *env, unsigned int selector,
     return 1;
 }
 
+/* convert legacy cpumodel string to string cpu_name and
+ * a uniforms set of custom features that will be applied to CPU
+ * using object_property_parse()
+ */
+static void compat_normalize_cpu_model(const char *cpu_model, char **cpu_name,
+                                        QDict **features, Error **errp)
+{
+
+    char *s = g_strdup(cpu_model);
+    char *featurestr, *sptr;
+
+    *cpu_name = strtok_r(s, ",", &sptr);
+    *features = qdict_new();
+
+    featurestr = strtok_r(NULL, ",", &sptr);
+    while (featurestr) {
+        char *val;
+        if (featurestr[0] == '+') {
+            /*
+             * preseve legacy behaviour, if feature was disabled once
+             * do not allow to enable it again
+             */
+            if (!qdict_haskey(*features, featurestr + 1)) {
+                qdict_put(*features, featurestr + 1, qstring_from_str("on"));
+            }
+        } else if (featurestr[0] == '-') {
+            qdict_put(*features, featurestr + 1, qstring_from_str("off"));
+        } else {
+            val = strchr(featurestr, '=');
+            if (val) {
+                *val = 0; val++;
+                if (!strcmp(featurestr, "vendor")) {
+                    qdict_put(*features, "vendor-override",
+                              qstring_from_str("on"));
+                    qdict_put(*features, featurestr, qstring_from_str(val));
+                } else if (!strcmp(featurestr, "tsc_freq")) {
+                    qdict_put(*features, "tsc-frequency",
+                              qstring_from_str(val));
+                } else {
+                    qdict_put(*features, featurestr, qstring_from_str(val));
+                }
+            } else {
+                qdict_put(*features, featurestr, qstring_from_str("on"));
+            }
+        }
+
+        featurestr = strtok_r(NULL, ",", &sptr);
+    }
+
+    return;
+}
+
 X86CPU *cpu_x86_init(const char *cpu_model)
 {
     X86CPU *cpu;
     CPUX86State *env;
     Error *error = NULL;
+    QDict *features;
+    const QDictEntry *ent;
+    char *name;
+
+    /* extract/remap legacy cpu name from cpu_model string and
+     * covert rest of it into set of properties in (feat=key, value=keyval) format */ 
+    compat_normalize_cpu_model(cpu_model, &name, &features, &error);
+    if (error_is_set(&error)) {
+        return NULL;
+    }
 
+    /* with subclasses it should take name argument */
     cpu = X86_CPU(object_new(TYPE_X86_CPU));
+
+    /* ==> should go away with subclsses */
     env = &cpu->env;
     env->cpu_model_str = cpu_model;
 
-    if (cpu_x86_register(cpu, cpu_model) < 0) {
+    if (cpu_x86_register(cpu, name) < 0) {
+        QDECREF(features);
+        g_free(name);
         object_delete(OBJECT(cpu));
         return NULL;
     }
+    /* <== */
+
+    g_free(name);
+
+    /* ==> just plain setting of properties */
+    for (ent = qdict_first(features); ent; ent = qdict_next(features, ent)) {
+        const QString *qval = qobject_to_qstring(qdict_entry_value(ent));
+        object_property_parse(OBJECT(cpu), qstring_get_str(qval),
+                              qdict_entry_key(ent), &error);
+        if (error_is_set(&error)) {
+            QDECREF(features);
+            error_free(error);
+            object_delete(OBJECT(cpu));
+            return NULL;
+        }
+    }
+    QDECREF(features);
+    /* <== */
 
     x86_cpu_realize(OBJECT(cpu), &error);
     if (error_is_set(&error)) {
-- 
1.7.11.2