[Qemu-devel] [PATCH 2/2] x86: extract legacy cpu features format parser

[Igor Mammedov <imammedo@redhat.com>]

Move cpu_model +-feat parsing into a separate file so that it
could be reused later for parsing similar format of sparc target

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
CC: Riku Voipio <riku.voipio@iki.fi>
CC: Laurent Vivier <laurent@vivier.eu>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Richard Henderson <rth@twiddle.net>
CC: Eduardo Habkost <ehabkost@redhat.com>
---
 include/qom/cpu.h                     |   2 +
 default-configs/i386-bsd-user.mak     |   1 +
 default-configs/i386-linux-user.mak   |   1 +
 default-configs/i386-softmmu.mak      |   1 +
 default-configs/x86_64-bsd-user.mak   |   1 +
 default-configs/x86_64-linux-user.mak |   1 +
 default-configs/x86_64-softmmu.mak    |   1 +
 target/i386/cpu.c                     | 125 +-------------------------
 util/Makefile.objs                    |   1 +
 util/legacy_cpu_features_parser.c     | 161 ++++++++++++++++++++++++++++++++++
 10 files changed, 171 insertions(+), 124 deletions(-)
 create mode 100644 util/legacy_cpu_features_parser.c

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 25eefea..30247dc 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -1038,4 +1038,6 @@ extern const struct VMStateDescription vmstate_cpu_common;
 
 #define UNASSIGNED_CPU_INDEX -1
 
+void cpu_legacy_parse_featurestr(const char *typename, char *features,
+                                 Error **errp);
 #endif
diff --git a/default-configs/i386-bsd-user.mak b/default-configs/i386-bsd-user.mak
index af1b31a..b28a05f 100644
--- a/default-configs/i386-bsd-user.mak
+++ b/default-configs/i386-bsd-user.mak
@@ -1 +1,2 @@
 # Default configuration for i386-bsd-user
+CONFIG_LEGACY_CPU_FEATURES=y
diff --git a/default-configs/i386-linux-user.mak b/default-configs/i386-linux-user.mak
index 8657e68..c136967 100644
--- a/default-configs/i386-linux-user.mak
+++ b/default-configs/i386-linux-user.mak
@@ -1 +1,2 @@
 # Default configuration for i386-linux-user
+CONFIG_LEGACY_CPU_FEATURES=y
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index d2ab2f6..e3e7c0e 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -59,3 +59,4 @@ CONFIG_SMBIOS=y
 CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
 CONFIG_PXB=y
 CONFIG_ACPI_VMGENID=y
+CONFIG_LEGACY_CPU_FEATURES=y
diff --git a/default-configs/x86_64-bsd-user.mak b/default-configs/x86_64-bsd-user.mak
index 73e5d34..952323c 100644
--- a/default-configs/x86_64-bsd-user.mak
+++ b/default-configs/x86_64-bsd-user.mak
@@ -1 +1,2 @@
 # Default configuration for x86_64-bsd-user
+CONFIG_LEGACY_CPU_FEATURES=y
diff --git a/default-configs/x86_64-linux-user.mak b/default-configs/x86_64-linux-user.mak
index bec1d9e..b513ef2 100644
--- a/default-configs/x86_64-linux-user.mak
+++ b/default-configs/x86_64-linux-user.mak
@@ -1 +1,2 @@
 # Default configuration for x86_64-linux-user
+CONFIG_LEGACY_CPU_FEATURES=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 9bde2f1..6594ddf 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -59,3 +59,4 @@ CONFIG_SMBIOS=y
 CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
 CONFIG_PXB=y
 CONFIG_ACPI_VMGENID=y
+CONFIG_LEGACY_CPU_FEATURES=y
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 84f552d..ac60c1a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -17,7 +17,6 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 #include "qemu/osdep.h"
-#include "qemu/cutils.h"
 
 #include "cpu.h"
 #include "exec/exec-all.h"
@@ -2030,13 +2029,6 @@ static const PropertyInfo qdev_prop_spinlocks = {
 /* Convert all '_' in a feature string option name to '-', to make feature
  * name conform to QOM property naming rule, which uses '-' instead of '_'.
  */
-static inline void feat2prop(char *s)
-{
-    while ((s = strchr(s, '_'))) {
-        *s = '-';
-    }
-}
-
 /* Return the feature property name for a feature flag bit */
 static const char *x86_cpu_feature_name(FeatureWord w, int bitnr)
 {
@@ -2058,126 +2050,11 @@ static const char *x86_cpu_feature_name(FeatureWord w, int bitnr)
     return feature_word_info[w].feat_names[bitnr];
 }
 
-static gint compare_string(gconstpointer a, gconstpointer b)
-{
-    return g_strcmp0(a, b);
-}
-
-static void
-cpu_add_feat_as_prop(const char *typename, const char *name, const char *val)
-{
-    GlobalProperty *prop = g_new0(typeof(*prop), 1);
-    prop->driver = typename;
-    prop->property = g_strdup(name);
-    prop->value = g_strdup(val);
-    prop->errp = &error_fatal;
-    qdev_prop_register_global(prop);
-}
-
 /* Parse "+feature,-feature,feature=foo" CPU feature string */
 static void x86_cpu_parse_featurestr(const char *typename, char *features,
                                      Error **errp)
 {
-    /* Compatibily hack to maintain legacy +-feat semantic,
-     * where +-feat overwrites any feature set by
-     * feat=on|feat even if the later is parsed after +-feat
-     * (i.e. "-x2apic,x2apic=on" will result in x2apic disabled)
-     */
-    GList *l, *plus_features = NULL, *minus_features = NULL;
-    char *featurestr; /* Single 'key=value" string being parsed */
-    static bool cpu_globals_initialized;
-    bool ambiguous = false;
-
-    if (cpu_globals_initialized) {
-        return;
-    }
-    cpu_globals_initialized = true;
-
-    if (!features) {
-        return;
-    }
-
-    for (featurestr = strtok(features, ",");
-         featurestr;
-         featurestr = strtok(NULL, ",")) {
-        const char *name;
-        const char *val = NULL;
-        char *eq = NULL;
-        char num[32];
-
-        /* Compatibility syntax: */
-        if (featurestr[0] == '+') {
-            plus_features = g_list_append(plus_features,
-                                          g_strdup(featurestr + 1));
-            continue;
-        } else if (featurestr[0] == '-') {
-            minus_features = g_list_append(minus_features,
-                                           g_strdup(featurestr + 1));
-            continue;
-        }
-
-        eq = strchr(featurestr, '=');
-        if (eq) {
-            *eq++ = 0;
-            val = eq;
-        } else {
-            val = "on";
-        }
-
-        feat2prop(featurestr);
-        name = featurestr;
-
-        if (g_list_find_custom(plus_features, name, compare_string)) {
-            warn_report("Ambiguous CPU model string. "
-                        "Don't mix both \"+%s\" and \"%s=%s\"",
-                        name, name, val);
-            ambiguous = true;
-        }
-        if (g_list_find_custom(minus_features, name, compare_string)) {
-            warn_report("Ambiguous CPU model string. "
-                        "Don't mix both \"-%s\" and \"%s=%s\"",
-                        name, name, val);
-            ambiguous = true;
-        }
-
-        /* Special case: */
-        if (!strcmp(name, "tsc-freq")) {
-            int ret;
-            uint64_t tsc_freq;
-
-            ret = qemu_strtosz_metric(val, NULL, &tsc_freq);
-            if (ret < 0 || tsc_freq > INT64_MAX) {
-                error_setg(errp, "bad numerical value %s", val);
-                return;
-            }
-            snprintf(num, sizeof(num), "%" PRId64, tsc_freq);
-            val = num;
-            name = "tsc-frequency";
-        }
-
-        cpu_add_feat_as_prop(typename, name, val);
-    }
-
-    if (ambiguous) {
-        warn_report("Compatibility of ambiguous CPU model "
-                    "strings won't be kept on future QEMU versions");
-    }
-
-    for (l = plus_features; l; l = l->next) {
-        const char *name = l->data;
-        cpu_add_feat_as_prop(typename, name, "on");
-    }
-    if (plus_features) {
-        g_list_free_full(plus_features, g_free);
-    }
-
-    for (l = minus_features; l; l = l->next) {
-        const char *name = l->data;
-        cpu_add_feat_as_prop(typename, name, "off");
-    }
-    if (minus_features) {
-        g_list_free_full(minus_features, g_free);
-    }
+    cpu_legacy_parse_featurestr(typename, features, errp);
 }
 
 static void x86_cpu_expand_features(X86CPU *cpu);
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 50a55ec..14e28f7 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -45,3 +45,4 @@ util-obj-y += qht.o
 util-obj-y += range.o
 util-obj-y += stats64.o
 util-obj-y += systemd.o
+util-obj-$(CONFIG_LEGACY_CPU_FEATURES) += legacy_cpu_features_parser.o
diff --git a/util/legacy_cpu_features_parser.c b/util/legacy_cpu_features_parser.c
new file mode 100644
index 0000000..6b352a3
--- /dev/null
+++ b/util/legacy_cpu_features_parser.c
@@ -0,0 +1,161 @@
+/* Support for legacy -cpu cpu,features CLI option with +-feat syntax,
+ * used by x86/sparc targets
+ *
+ * Author: Andreas Färber <afaerber@suse.de>
+ * Author: Andre Przywara <andre.przywara@amd.com>
+ * Author: Eduardo Habkost <ehabkost@redhat.com>
+ * Author: Igor Mammedov <imammedo@redhat.com>
+ * Author: Paolo Bonzini <pbonzini@redhat.com>
+ * Author: Markus Armbruster <armbru@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/cutils.h"
+#include "qom/cpu.h"
+#include "qemu/error-report.h"
+#include "hw/qdev-properties.h"
+
+static inline void feat2prop(char *s)
+{
+    while ((s = strchr(s, '_'))) {
+        *s = '-';
+    }
+}
+
+static gint compare_string(gconstpointer a, gconstpointer b)
+{
+    return g_strcmp0(a, b);
+}
+
+static void
+cpu_add_feat_as_prop(const char *typename, const char *name, const char *val)
+{
+    GlobalProperty *prop = g_new0(typeof(*prop), 1);
+    prop->driver = typename;
+    prop->property = g_strdup(name);
+    prop->value = g_strdup(val);
+    prop->errp = &error_fatal;
+    qdev_prop_register_global(prop);
+}
+
+/* DO NOT USE WITH NEW CODE
+ * Parse "+feature,-feature,feature=foo" CPU feature string
+ */
+void cpu_legacy_parse_featurestr(const char *typename, char *features,
+                                 Error **errp)
+{
+    /* Compatibily hack to maintain legacy +-feat semantic,
+     * where +-feat overwrites any feature set by
+     * feat=on|feat even if the later is parsed after +-feat
+     * (i.e. "-x2apic,x2apic=on" will result in x2apic disabled)
+     */
+    GList *l, *plus_features = NULL, *minus_features = NULL;
+    char *featurestr; /* Single 'key=value" string being parsed */
+    static bool cpu_globals_initialized;
+    bool ambiguous = false;
+
+    if (cpu_globals_initialized) {
+        return;
+    }
+    cpu_globals_initialized = true;
+
+    if (!features) {
+        return;
+    }
+
+    for (featurestr = strtok(features, ",");
+         featurestr;
+         featurestr = strtok(NULL, ",")) {
+        const char *name;
+        const char *val = NULL;
+        char *eq = NULL;
+        char num[32];
+
+        /* Compatibility syntax: */
+        if (featurestr[0] == '+') {
+            plus_features = g_list_append(plus_features,
+                                          g_strdup(featurestr + 1));
+            continue;
+        } else if (featurestr[0] == '-') {
+            minus_features = g_list_append(minus_features,
+                                           g_strdup(featurestr + 1));
+            continue;
+        }
+
+        eq = strchr(featurestr, '=');
+        if (eq) {
+            *eq++ = 0;
+            val = eq;
+        } else {
+            val = "on";
+        }
+
+        feat2prop(featurestr);
+        name = featurestr;
+
+        if (g_list_find_custom(plus_features, name, compare_string)) {
+            warn_report("Ambiguous CPU model string. "
+                        "Don't mix both \"+%s\" and \"%s=%s\"",
+                        name, name, val);
+            ambiguous = true;
+        }
+        if (g_list_find_custom(minus_features, name, compare_string)) {
+            warn_report("Ambiguous CPU model string. "
+                        "Don't mix both \"-%s\" and \"%s=%s\"",
+                        name, name, val);
+            ambiguous = true;
+        }
+
+        /* Special case: */
+        if (!strcmp(name, "tsc-freq")) {
+            int ret;
+            uint64_t tsc_freq;
+
+            ret = qemu_strtosz_metric(val, NULL, &tsc_freq);
+            if (ret < 0 || tsc_freq > INT64_MAX) {
+                error_setg(errp, "bad numerical value %s", val);
+                return;
+            }
+            snprintf(num, sizeof(num), "%" PRId64, tsc_freq);
+            val = num;
+            name = "tsc-frequency";
+        }
+
+        cpu_add_feat_as_prop(typename, name, val);
+    }
+
+    if (ambiguous) {
+        warn_report("Compatibility of ambiguous CPU model "
+                    "strings won't be kept on future QEMU versions");
+    }
+
+    for (l = plus_features; l; l = l->next) {
+        const char *name = l->data;
+        cpu_add_feat_as_prop(typename, name, "on");
+    }
+    if (plus_features) {
+        g_list_free_full(plus_features, g_free);
+    }
+
+    for (l = minus_features; l; l = l->next) {
+        const char *name = l->data;
+        cpu_add_feat_as_prop(typename, name, "off");
+    }
+    if (minus_features) {
+        g_list_free_full(minus_features, g_free);
+    }
+}
-- 
2.7.4