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 | 6 ++
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 | 124 ++-------------------------
util/Makefile.objs | 1 +
util/legacy_cpu_features_parser.c | 153 ++++++++++++++++++++++++++++++++++
10 files changed, 172 insertions(+), 118 deletions(-)
create mode 100644 util/legacy_cpu_features_parser.c
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 7bfd50c..60aea03 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -1039,4 +1039,10 @@ extern const struct VMStateDescription vmstate_cpu_common;
#define UNASSIGNED_CPU_INDEX -1
+int cpu_legacy_apply_features(Object *obj, GList *features, bool enable,
+ Error **errp);
+
+void cpu_legacy_parse_featurestr(const char *typename, char *features,
+ GList **plus_features, GList **minus_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 c571772..91d3684 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"
@@ -1970,13 +1969,6 @@ static 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)
{
@@ -2005,100 +1997,11 @@ static const char *x86_cpu_feature_name(FeatureWord w, int bitnr)
*/
static GList *plus_features, *minus_features;
-static gint compare_string(gconstpointer a, gconstpointer b)
-{
- return g_strcmp0(a, b);
-}
-
-/* Parse "+feature,-feature,feature=foo" CPU feature string
- */
static void x86_cpu_parse_featurestr(const char *typename, char *features,
Error **errp)
{
- 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];
- GlobalProperty *prop;
-
- /* 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)) {
- error_report("warning: 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)) {
- error_report("warning: 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";
- }
-
- 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);
- }
-
- if (ambiguous) {
- error_report("warning: Compatibility of ambiguous CPU model "
- "strings won't be kept on future QEMU versions");
- }
+ cpu_legacy_parse_featurestr(typename, features,
+ &plus_features, &minus_features, errp);
}
static void x86_cpu_expand_features(X86CPU *cpu, Error **errp);
@@ -3370,8 +3273,6 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
{
CPUX86State *env = &cpu->env;
FeatureWord w;
- GList *l;
- Error *local_err = NULL;
/*TODO: Now cpu->max_features doesn't overwrite features
* set using QOM properties, and we can convert
@@ -3389,20 +3290,12 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
}
}
- for (l = plus_features; l; l = l->next) {
- const char *prop = l->data;
- object_property_set_bool(OBJECT(cpu), true, prop, &local_err);
- if (local_err) {
- goto out;
- }
+ if (cpu_legacy_apply_features(OBJECT(cpu), plus_features, true, errp)) {
+ return;
}
- for (l = minus_features; l; l = l->next) {
- const char *prop = l->data;
- object_property_set_bool(OBJECT(cpu), false, prop, &local_err);
- if (local_err) {
- goto out;
- }
+ if (cpu_legacy_apply_features(OBJECT(cpu), minus_features, false, errp)) {
+ return;
}
if (!kvm_enabled() || !cpu->expose_kvm) {
@@ -3440,11 +3333,6 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
if (env->cpuid_xlevel2 == UINT32_MAX) {
env->cpuid_xlevel2 = env->cpuid_min_xlevel2;
}
-
-out:
- if (local_err != NULL) {
- error_propagate(errp, local_err);
- }
}
/*
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..f2e3b81
--- /dev/null
+++ b/util/legacy_cpu_features_parser.c
@@ -0,0 +1,153 @@
+/* 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"
+
+/* DO NOT USE WITH NEW CODE */
+int cpu_legacy_apply_features(Object *obj, GList *features, bool enable,
+ Error **errp)
+{
+ GList *l;
+ Error *local_err = NULL;
+
+ for (l = features; l; l = l->next) {
+ const char *prop = l->data;
+ object_property_set_bool(obj, enable, prop, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return 1;
+ }
+ }
+ return 0;
+}
+
+static inline void feat2prop(char *s)
+{
+ while ((s = strchr(s, '_'))) {
+ *s = '-';
+ }
+}
+
+static gint compare_string(gconstpointer a, gconstpointer b)
+{
+ return g_strcmp0(a, b);
+}
+
+/* DO NOT USE WITH NEW CODE
+ * Parse "+feature,-feature,feature=foo" CPU feature string
+ */
+void cpu_legacy_parse_featurestr(const char *typename, char *features,
+ GList **plus_features, GList **minus_features,
+ Error **errp)
+{
+ 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];
+ GlobalProperty *prop;
+
+ /* 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)) {
+ error_report("warning: 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)) {
+ error_report("warning: 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";
+ }
+
+ 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);
+ }
+
+ if (ambiguous) {
+ error_report("warning: Compatibility of ambiguous CPU model "
+ "strings won't be kept on future QEMU versions");
+ }
+}
--
2.7.4
On Fri, Jul 14, 2017 at 03:51:59PM +0200, Igor Mammedov wrote:
> 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 | 6 ++
> 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 | 124 ++-------------------------
> util/Makefile.objs | 1 +
> util/legacy_cpu_features_parser.c | 153 ++++++++++++++++++++++++++++++++++
> 10 files changed, 172 insertions(+), 118 deletions(-)
> create mode 100644 util/legacy_cpu_features_parser.c
>
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 7bfd50c..60aea03 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -1039,4 +1039,10 @@ extern const struct VMStateDescription vmstate_cpu_common;
>
> #define UNASSIGNED_CPU_INDEX -1
>
> +int cpu_legacy_apply_features(Object *obj, GList *features, bool enable,
> + Error **errp);
> +
> +void cpu_legacy_parse_featurestr(const char *typename, char *features,
> + GList **plus_features, GList **minus_features,
> + Error **errp);
plus_features and minus_features only exist because of a (now
fixed) bug in the handling of cpu->max_features, and we don't
need them anymore, see the comment at x86_cpu_expand_features():
/*TODO: Now cpu->max_features doesn't overwrite features
* set using QOM properties, and we can convert
* plus_features & minus_features to global properties
* inside x86_cpu_parse_featurestr() too.
*/
Let's remove them instead of exposing this unnecessary misfeature
on the generic API.
--
Eduardo
Since
(commit d4a606b3 i386: Don't override -cpu options on -cpu host/max)
it became possible to delete hack where it was necessary to
postpone applying plus/minus features to realize time
after max_features were applied to keep legacy +-feat
override semantics.
With above commit it's possible to convert +-feat to a set
of GlobalProperty items and keep +-feat override semantics,
these properties should be added to global list at the end
to override properties that were set with feat=on|off syntax.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
CC: Eduardo Habkost <ehabkost@redhat.com>
target/i386/cpu.c | 108 ++++++++++++++++++++----------------------------------
1 file changed, 40 insertions(+), 68 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ddc45ab..84f552d 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2058,23 +2058,32 @@ static const char *x86_cpu_feature_name(FeatureWord w, int bitnr)
return feature_word_info[w].feat_names[bitnr];
}
-/* 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)
- */
-static GList *plus_features, *minus_features;
-
static gint compare_string(gconstpointer a, gconstpointer b)
{
return g_strcmp0(a, b);
}
-/* Parse "+feature,-feature,feature=foo" CPU feature string
- */
+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;
@@ -2095,7 +2104,6 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
const char *val = NULL;
char *eq = NULL;
char num[32];
- GlobalProperty *prop;
/* Compatibility syntax: */
if (featurestr[0] == '+') {
@@ -2147,21 +2155,32 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
name = "tsc-frequency";
}
- 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);
+ 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);
+ }
}
-static void x86_cpu_expand_features(X86CPU *cpu, Error **errp);
+static void x86_cpu_expand_features(X86CPU *cpu);
static int x86_cpu_filter_features(X86CPU *cpu);
/* Check for missing features that may prevent the CPU class from
@@ -2172,7 +2191,6 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
{
X86CPU *xc;
FeatureWord w;
- Error *err = NULL;
strList **next = missing_feats;
if (xcc->kvm_required && !kvm_enabled()) {
@@ -2184,18 +2202,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc))));
- x86_cpu_expand_features(xc, &err);
- if (err) {
- /* Errors at x86_cpu_expand_features should never happen,
- * but in case it does, just report the model as not
- * runnable at all using the "type" property.
- */
- strList *new = g_new0(strList, 1);
- new->value = g_strdup("type");
- *next = new;
- next = &new->next;
- }
-
+ x86_cpu_expand_features(xc);
x86_cpu_filter_features(xc);
for (w = 0; w < FEATURE_WORDS; w++) {
@@ -2559,11 +2566,7 @@ static X86CPU *x86_cpu_from_model(const char *model, QDict *props, Error **errp)
}
}
- x86_cpu_expand_features(xc, &err);
- if (err) {
- goto out;
- }
-
+ x86_cpu_expand_features(xc);
out:
if (err) {
error_propagate(errp, err);
@@ -3453,18 +3456,11 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu)
/* Expand CPU configuration data, based on configured features
* and host/accelerator capabilities when appropriate.
*/
-static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
+static void x86_cpu_expand_features(X86CPU *cpu)
{
CPUX86State *env = &cpu->env;
FeatureWord w;
- GList *l;
- Error *local_err = NULL;
- /*TODO: Now cpu->max_features doesn't overwrite features
- * set using QOM properties, and we can convert
- * plus_features & minus_features to global properties
- * inside x86_cpu_parse_featurestr() too.
- */
if (cpu->max_features) {
for (w = 0; w < FEATURE_WORDS; w++) {
/* Override only features that weren't set explicitly
@@ -3476,22 +3472,6 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
}
}
- for (l = plus_features; l; l = l->next) {
- const char *prop = l->data;
- object_property_set_bool(OBJECT(cpu), true, prop, &local_err);
- if (local_err) {
- goto out;
- }
- }
-
- for (l = minus_features; l; l = l->next) {
- const char *prop = l->data;
- object_property_set_bool(OBJECT(cpu), false, prop, &local_err);
- if (local_err) {
- goto out;
- }
- }
-
if (!kvm_enabled() || !cpu->expose_kvm) {
env->features[FEAT_KVM] = 0;
}
@@ -3527,11 +3507,6 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
if (env->cpuid_xlevel2 == UINT32_MAX) {
env->cpuid_xlevel2 = env->cpuid_min_xlevel2;
}
-
-out:
- if (local_err != NULL) {
- error_propagate(errp, local_err);
- }
}
/*
@@ -3587,10 +3562,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
return;
}
- x86_cpu_expand_features(cpu, &local_err);
- if (local_err) {
- goto out;
- }
+ x86_cpu_expand_features(cpu);
if (x86_cpu_filter_features(cpu) &&
(cpu->check_cpuid || cpu->enforce_cpuid)) {
--
2.7.4
On Thu, Aug 17, 2017 at 04:07:56PM +0200, Igor Mammedov wrote:
> Since
> (commit d4a606b3 i386: Don't override -cpu options on -cpu host/max)
> it became possible to delete hack where it was necessary to
> postpone applying plus/minus features to realize time
> after max_features were applied to keep legacy +-feat
> override semantics.
>
> With above commit it's possible to convert +-feat to a set
> of GlobalProperty items and keep +-feat override semantics,
> these properties should be added to global list at the end
> to override properties that were set with feat=on|off syntax.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
[...]
> +/* 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;
The warning about ambiguous CPU options exists since 2.8, I think
this is a good opportunity to get rid of the "[+-]feat overrides
feat=on|off" rule and simplify the parsing code. Do you want to
do this in the same patch?
> char *featurestr; /* Single 'key=value" string being parsed */
> static bool cpu_globals_initialized;
> bool ambiguous = false;
> @@ -2095,7 +2104,6 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
> const char *val = NULL;
> char *eq = NULL;
> char num[32];
> - GlobalProperty *prop;
>
> /* Compatibility syntax: */
> if (featurestr[0] == '+') {
> @@ -2147,21 +2155,32 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
> name = "tsc-frequency";
> }
>
> - 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);
> + 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);
> + }
> }
>
> -static void x86_cpu_expand_features(X86CPU *cpu, Error **errp);
> +static void x86_cpu_expand_features(X86CPU *cpu);
> static int x86_cpu_filter_features(X86CPU *cpu);
>
> /* Check for missing features that may prevent the CPU class from
> @@ -2172,7 +2191,6 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
> {
> X86CPU *xc;
> FeatureWord w;
> - Error *err = NULL;
> strList **next = missing_feats;
>
> if (xcc->kvm_required && !kvm_enabled()) {
> @@ -2184,18 +2202,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
>
> xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc))));
>
> - x86_cpu_expand_features(xc, &err);
> - if (err) {
> - /* Errors at x86_cpu_expand_features should never happen,
> - * but in case it does, just report the model as not
> - * runnable at all using the "type" property.
> - */
> - strList *new = g_new0(strList, 1);
> - new->value = g_strdup("type");
> - *next = new;
> - next = &new->next;
> - }
> -
> + x86_cpu_expand_features(xc);
> x86_cpu_filter_features(xc);
>
> for (w = 0; w < FEATURE_WORDS; w++) {
> @@ -2559,11 +2566,7 @@ static X86CPU *x86_cpu_from_model(const char *model, QDict *props, Error **errp)
> }
> }
>
> - x86_cpu_expand_features(xc, &err);
> - if (err) {
> - goto out;
> - }
> -
> + x86_cpu_expand_features(xc);
> out:
> if (err) {
> error_propagate(errp, err);
> @@ -3453,18 +3456,11 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu)
> /* Expand CPU configuration data, based on configured features
> * and host/accelerator capabilities when appropriate.
> */
> -static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
> +static void x86_cpu_expand_features(X86CPU *cpu)
> {
> CPUX86State *env = &cpu->env;
> FeatureWord w;
> - GList *l;
> - Error *local_err = NULL;
>
> - /*TODO: Now cpu->max_features doesn't overwrite features
> - * set using QOM properties, and we can convert
> - * plus_features & minus_features to global properties
> - * inside x86_cpu_parse_featurestr() too.
> - */
> if (cpu->max_features) {
> for (w = 0; w < FEATURE_WORDS; w++) {
> /* Override only features that weren't set explicitly
> @@ -3476,22 +3472,6 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
> }
> }
>
> - for (l = plus_features; l; l = l->next) {
> - const char *prop = l->data;
> - object_property_set_bool(OBJECT(cpu), true, prop, &local_err);
> - if (local_err) {
> - goto out;
> - }
> - }
> -
> - for (l = minus_features; l; l = l->next) {
> - const char *prop = l->data;
> - object_property_set_bool(OBJECT(cpu), false, prop, &local_err);
> - if (local_err) {
> - goto out;
> - }
> - }
> -
> if (!kvm_enabled() || !cpu->expose_kvm) {
> env->features[FEAT_KVM] = 0;
> }
> @@ -3527,11 +3507,6 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
> if (env->cpuid_xlevel2 == UINT32_MAX) {
> env->cpuid_xlevel2 = env->cpuid_min_xlevel2;
> }
> -
> -out:
> - if (local_err != NULL) {
> - error_propagate(errp, local_err);
> - }
> }
>
> /*
> @@ -3587,10 +3562,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> return;
> }
>
> - x86_cpu_expand_features(cpu, &local_err);
> - if (local_err) {
> - goto out;
> - }
> + x86_cpu_expand_features(cpu);
>
> if (x86_cpu_filter_features(cpu) &&
> (cpu->check_cpuid || cpu->enforce_cpuid)) {
> --
> 2.7.4
>
>
--
Eduardo
On Fri, 18 Aug 2017 14:40:29 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Thu, Aug 17, 2017 at 04:07:56PM +0200, Igor Mammedov wrote:
> > Since
> > (commit d4a606b3 i386: Don't override -cpu options on -cpu host/max)
> > it became possible to delete hack where it was necessary to
> > postpone applying plus/minus features to realize time
> > after max_features were applied to keep legacy +-feat
> > override semantics.
> >
> > With above commit it's possible to convert +-feat to a set
> > of GlobalProperty items and keep +-feat override semantics,
> > these properties should be added to global list at the end
> > to override properties that were set with feat=on|off syntax.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> [...]
> > +/* 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;
>
> The warning about ambiguous CPU options exists since 2.8, I think
> this is a good opportunity to get rid of the "[+-]feat overrides
> feat=on|off" rule and simplify the parsing code. Do you want to
> do this in the same patch?
I'd prefer not to do it in this patch/series, as it's not related.
WE can do cleanups later on top.
>
>
> > char *featurestr; /* Single 'key=value" string being parsed */
> > static bool cpu_globals_initialized;
> > bool ambiguous = false;
> > @@ -2095,7 +2104,6 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
> > const char *val = NULL;
> > char *eq = NULL;
> > char num[32];
> > - GlobalProperty *prop;
> >
> > /* Compatibility syntax: */
> > if (featurestr[0] == '+') {
> > @@ -2147,21 +2155,32 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
> > name = "tsc-frequency";
> > }
> >
> > - 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);
> > + 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);
> > + }
> > }
> >
> > -static void x86_cpu_expand_features(X86CPU *cpu, Error **errp);
> > +static void x86_cpu_expand_features(X86CPU *cpu);
> > static int x86_cpu_filter_features(X86CPU *cpu);
> >
> > /* Check for missing features that may prevent the CPU class from
> > @@ -2172,7 +2191,6 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
> > {
> > X86CPU *xc;
> > FeatureWord w;
> > - Error *err = NULL;
> > strList **next = missing_feats;
> >
> > if (xcc->kvm_required && !kvm_enabled()) {
> > @@ -2184,18 +2202,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
> >
> > xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc))));
> >
> > - x86_cpu_expand_features(xc, &err);
> > - if (err) {
> > - /* Errors at x86_cpu_expand_features should never happen,
> > - * but in case it does, just report the model as not
> > - * runnable at all using the "type" property.
> > - */
> > - strList *new = g_new0(strList, 1);
> > - new->value = g_strdup("type");
> > - *next = new;
> > - next = &new->next;
> > - }
> > -
> > + x86_cpu_expand_features(xc);
> > x86_cpu_filter_features(xc);
> >
> > for (w = 0; w < FEATURE_WORDS; w++) {
> > @@ -2559,11 +2566,7 @@ static X86CPU *x86_cpu_from_model(const char *model, QDict *props, Error **errp)
> > }
> > }
> >
> > - x86_cpu_expand_features(xc, &err);
> > - if (err) {
> > - goto out;
> > - }
> > -
> > + x86_cpu_expand_features(xc);
> > out:
> > if (err) {
> > error_propagate(errp, err);
> > @@ -3453,18 +3456,11 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu)
> > /* Expand CPU configuration data, based on configured features
> > * and host/accelerator capabilities when appropriate.
> > */
> > -static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
> > +static void x86_cpu_expand_features(X86CPU *cpu)
> > {
> > CPUX86State *env = &cpu->env;
> > FeatureWord w;
> > - GList *l;
> > - Error *local_err = NULL;
> >
> > - /*TODO: Now cpu->max_features doesn't overwrite features
> > - * set using QOM properties, and we can convert
> > - * plus_features & minus_features to global properties
> > - * inside x86_cpu_parse_featurestr() too.
> > - */
> > if (cpu->max_features) {
> > for (w = 0; w < FEATURE_WORDS; w++) {
> > /* Override only features that weren't set explicitly
> > @@ -3476,22 +3472,6 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
> > }
> > }
> >
> > - for (l = plus_features; l; l = l->next) {
> > - const char *prop = l->data;
> > - object_property_set_bool(OBJECT(cpu), true, prop, &local_err);
> > - if (local_err) {
> > - goto out;
> > - }
> > - }
> > -
> > - for (l = minus_features; l; l = l->next) {
> > - const char *prop = l->data;
> > - object_property_set_bool(OBJECT(cpu), false, prop, &local_err);
> > - if (local_err) {
> > - goto out;
> > - }
> > - }
> > -
> > if (!kvm_enabled() || !cpu->expose_kvm) {
> > env->features[FEAT_KVM] = 0;
> > }
> > @@ -3527,11 +3507,6 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
> > if (env->cpuid_xlevel2 == UINT32_MAX) {
> > env->cpuid_xlevel2 = env->cpuid_min_xlevel2;
> > }
> > -
> > -out:
> > - if (local_err != NULL) {
> > - error_propagate(errp, local_err);
> > - }
> > }
> >
> > /*
> > @@ -3587,10 +3562,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> > return;
> > }
> >
> > - x86_cpu_expand_features(cpu, &local_err);
> > - if (local_err) {
> > - goto out;
> > - }
> > + x86_cpu_expand_features(cpu);
> >
> > if (x86_cpu_filter_features(cpu) &&
> > (cpu->check_cpuid || cpu->enforce_cpuid)) {
> > --
> > 2.7.4
> >
> >
>
On Mon, Aug 21, 2017 at 10:32:41AM +0200, Igor Mammedov wrote:
> On Fri, 18 Aug 2017 14:40:29 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Thu, Aug 17, 2017 at 04:07:56PM +0200, Igor Mammedov wrote:
> > > Since
> > > (commit d4a606b3 i386: Don't override -cpu options on -cpu host/max)
> > > it became possible to delete hack where it was necessary to
> > > postpone applying plus/minus features to realize time
> > > after max_features were applied to keep legacy +-feat
> > > override semantics.
> > >
> > > With above commit it's possible to convert +-feat to a set
> > > of GlobalProperty items and keep +-feat override semantics,
> > > these properties should be added to global list at the end
> > > to override properties that were set with feat=on|off syntax.
> > >
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > [...]
> > > +/* 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;
> >
> > The warning about ambiguous CPU options exists since 2.8, I think
> > this is a good opportunity to get rid of the "[+-]feat overrides
> > feat=on|off" rule and simplify the parsing code. Do you want to
> > do this in the same patch?
> I'd prefer not to do it in this patch/series, as it's not related.
>
> WE can do cleanups later on top.
This is not a problem in this patch, but it is a problem when you
create a generic cpu_legacy_parse_featurestr() function in
another series. We should remove that useless feature from the
function before making it generic. It has the additional benefit
of making the resulting patch and code easier to review.
--
Eduardo
On Wed, 23 Aug 2017 11:24:27 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Mon, Aug 21, 2017 at 10:32:41AM +0200, Igor Mammedov wrote:
> > On Fri, 18 Aug 2017 14:40:29 -0300
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > > On Thu, Aug 17, 2017 at 04:07:56PM +0200, Igor Mammedov wrote:
> > > > Since
> > > > (commit d4a606b3 i386: Don't override -cpu options on -cpu host/max)
> > > > it became possible to delete hack where it was necessary to
> > > > postpone applying plus/minus features to realize time
> > > > after max_features were applied to keep legacy +-feat
> > > > override semantics.
> > > >
> > > > With above commit it's possible to convert +-feat to a set
> > > > of GlobalProperty items and keep +-feat override semantics,
> > > > these properties should be added to global list at the end
> > > > to override properties that were set with feat=on|off syntax.
> > > >
> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > [...]
> > > > +/* 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;
> > >
> > > The warning about ambiguous CPU options exists since 2.8, I think
> > > this is a good opportunity to get rid of the "[+-]feat overrides
> > > feat=on|off" rule and simplify the parsing code. Do you want to
> > > do this in the same patch?
> > I'd prefer not to do it in this patch/series, as it's not related.
> >
> > WE can do cleanups later on top.
>
> This is not a problem in this patch, but it is a problem when you
> create a generic cpu_legacy_parse_featurestr() function in
> another series. We should remove that useless feature from the
> function before making it generic. It has the additional benefit
> of making the resulting patch and code easier to review.
Removing means replacing warning with hard error, so that setups
that happen to use this combination would fail instead of silently
changing behavior.
So it won't actually simplify function but will cause side-effects
that weren't intended by this series.
As is in this series, I'm being rather conservative,
it's just replacing code duplication that exists in SPARC with x86 impl.
without behavioral change
(and even though it's in public header it's not really public API
that's why it's called legacy_foo() - to try preventing new usage).
If we decide to make it hard error, it would be better to do it separately.
I can post a patch on top if you insist, that would do it
so we could discuss there if it's ok to break users in 2 releases
or not but it should not hold this series as it's totally
orthogonal matter. (it also would be easier to revert patch
if we apply it but later decide to keep old ways).
On Wed, Aug 23, 2017 at 05:54:12PM +0200, Igor Mammedov wrote:
> On Wed, 23 Aug 2017 11:24:27 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Mon, Aug 21, 2017 at 10:32:41AM +0200, Igor Mammedov wrote:
> > > On Fri, 18 Aug 2017 14:40:29 -0300
> > > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > >
> > > > On Thu, Aug 17, 2017 at 04:07:56PM +0200, Igor Mammedov wrote:
> > > > > Since
> > > > > (commit d4a606b3 i386: Don't override -cpu options on -cpu host/max)
> > > > > it became possible to delete hack where it was necessary to
> > > > > postpone applying plus/minus features to realize time
> > > > > after max_features were applied to keep legacy +-feat
> > > > > override semantics.
> > > > >
> > > > > With above commit it's possible to convert +-feat to a set
> > > > > of GlobalProperty items and keep +-feat override semantics,
> > > > > these properties should be added to global list at the end
> > > > > to override properties that were set with feat=on|off syntax.
> > > > >
> > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > [...]
> > > > > +/* 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;
> > > >
> > > > The warning about ambiguous CPU options exists since 2.8, I think
> > > > this is a good opportunity to get rid of the "[+-]feat overrides
> > > > feat=on|off" rule and simplify the parsing code. Do you want to
> > > > do this in the same patch?
> > > I'd prefer not to do it in this patch/series, as it's not related.
> > >
> > > WE can do cleanups later on top.
> >
> > This is not a problem in this patch, but it is a problem when you
> > create a generic cpu_legacy_parse_featurestr() function in
> > another series. We should remove that useless feature from the
> > function before making it generic. It has the additional benefit
> > of making the resulting patch and code easier to review.
> Removing means replacing warning with hard error, so that setups
> that happen to use this combination would fail instead of silently
> changing behavior.
> So it won't actually simplify function but will cause side-effects
> that weren't intended by this series.
I don't think it should be a hard error, it will be just a
semantics change (that we're already warning about for 3
releases). But I understand that you don't intend to introduce
this behavior change in x86 now.
However:
>
> As is in this series, I'm being rather conservative,
> it's just replacing code duplication that exists in SPARC with x86 impl.
> without behavioral change
You are not introducing behavior change on x86, that's right.
But you are introducing new behavior on sparc, and the new
behavior includes the ordering misfeature we have in x86. Let's
keep the misfeature x86-only.
> (and even though it's in public header it's not really public API
> that's why it's called legacy_foo() - to try preventing new usage).
>
> If we decide to make it hard error, it would be better to do it separately.
> I can post a patch on top if you insist, that would do it
> so we could discuss there if it's ok to break users in 2 releases
> or not but it should not hold this series as it's totally
> orthogonal matter. (it also would be easier to revert patch
> if we apply it but later decide to keep old ways).
I agree to implement x86 behavior change separately, but I insist
we don't introduce the ordering misfeature in sparc too. Fixing
x86 shouldn't hold the series, I agree. But making sparc parser
as bad as x86 is a blocker to me.
--
Eduardo
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
© 2016 - 2026 Red Hat, Inc.