On Fri, Aug 18, 2017 at 12:08:38PM +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 <imamm...@redhat.com> > --- > CC: Richard Henderson <r...@twiddle.net> > CC: Eduardo Habkost <ehabk...@redhat.com> > CC: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> > CC: Artyom Tarasenko <atar4q...@gmail.com> > CC: Philippe Mathieu-Daudé <f4...@amsat.org> > --- > 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/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 <afaer...@suse.de> > + * Author: Andre Przywara <andre.przyw...@amd.com> > + * Author: Eduardo Habkost <ehabk...@redhat.com> > + * Author: Igor Mammedov <imamm...@redhat.com> > + * Author: Paolo Bonzini <pbonz...@redhat.com> > + * Author: Markus Armbruster <arm...@redhat.com>
IANAL, but I believe a Copyright (c) <YEAR> <COPYRIGHT HOLDER> line is needed here. > + * > + * 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; > + } These 6 lines of code (or something equivalent to them) are supposed to be the only difference to the generic parsing function. I would simply make this feature (support for [+-]feature) enabled by a CPUClass::plus_minus_features flag handled by cpu_common_parse_features(). (But this can be done as a follow-up patch.) > + > + 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"; > + } This is x86-specific and should stay in x86-specific code. It can probably be handled by the tsc-freq setter. > + > + 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"); > + } As noted in the review of the x86 patch that removes the plus_features/minus_features static variables, this obsolete (and confusing) property ordering misfeature should be removed before we make this code generic and reuse it on other architectures. > + > + 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 > > -- Eduardo