8:22 a.m.
On Tue, 24 May 2016 09:34:05 -0300
Eduardo Habkost <ehabkost(a)redhat.com> wrote:
On Tue, May 24, 2016 at 02:17:03PM +0200, Igor Mammedov wrote:
> On Fri, 6 May 2016 15:11:31 -0300
> Eduardo Habkost <ehabkost(a)redhat.com> wrote:
>
> > This makes the feature name tables in feature_word_info all match
> > the actual QOM property names we use.
> >
> > This will make the command-line interface more consistent,
> > allowing the QOM property names to be used as "-cpu" arguments
> > directly.
> >
> > Add extra feat2prop() calls to x86_cpu_parse_featurestr() to keep
> > compatibility with the old that had underscores.
> >
> > Cc: Jiri Denemark <jdenemar(a)redhat.com>
> > Cc: libvir-list(a)redhat.com
> > Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
> > ---
> > target-i386/cpu.c | 32 ++++++++++++++++----------------
> > 1 file changed, 16 insertions(+), 16 deletions(-)
> >
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 309ef55..e7365d1 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -187,7 +187,7 @@ static const char *feature_name[] = {
> > };
> > static const char *ext_feature_name[] = {
> > "pni|sse3" /* Intel,AMD sse3 */, "pclmulqdq|pclmuldq",
"dtes64", "monitor",
> > - "ds_cpl", "vmx", "smx", "est",
> > + "ds-cpl", "vmx", "smx", "est",
> > "tm2", "ssse3", "cid", NULL,
> > "fma", "cx16", "xtpr", "pdcm",
> > NULL, "pcid", "dca", "sse4.1|sse4_1",
> > @@ -207,17 +207,17 @@ static const char *ext2_feature_name[] = {
> > NULL /* mtrr */, NULL /* pge */, NULL /* mca */, NULL /* cmov */,
> > NULL /* pat */, NULL /* pse36 */, NULL, NULL /* Linux mp */,
> > "nx|xd", NULL, "mmxext", NULL /* mmx */,
> > - NULL /* fxsr */, "fxsr_opt|ffxsr", "pdpe1gb" /* AMD
Page1GB */, "rdtscp",
> > + NULL /* fxsr */, "fxsr-opt|ffxsr", "pdpe1gb" /* AMD
Page1GB */, "rdtscp",
> > NULL, "lm|i64", "3dnowext", "3dnow",
> > };
> > static const char *ext3_feature_name[] = {
> > - "lahf_lm" /* AMD LahfSahf */, "cmp_legacy",
"svm", "extapic" /* AMD ExtApicSpace */,
> > + "lahf-lm" /* AMD LahfSahf */, "cmp-legacy",
"svm", "extapic" /* AMD ExtApicSpace */,
> > "cr8legacy" /* AMD AltMovCr8 */, "abm",
"sse4a", "misalignsse",
> > "3dnowprefetch", "osvw", "ibs",
"xop",
> > "skinit", "wdt", NULL, "lwp",
> > - "fma4", "tce", NULL, "nodeid_msr",
> > - NULL, "tbm", "topoext", "perfctr_core",
> > - "perfctr_nb", NULL, NULL, NULL,
> > + "fma4", "tce", NULL, "nodeid-msr",
> > + NULL, "tbm", "topoext", "perfctr-core",
> > + "perfctr-nb", NULL, NULL, NULL,
> > NULL, NULL, NULL, NULL,
> > };
> >
> > @@ -233,8 +233,8 @@ static const char *ext4_feature_name[] = {
> > };
> >
> > static const char *kvm_feature_name[] = {
> > - "kvmclock", "kvm_nopiodelay", "kvm_mmu",
"kvmclock",
> > - "kvm_asyncpf", "kvm_steal_time",
"kvm_pv_eoi", "kvm_pv_unhalt",
> > + "kvmclock", "kvm-nopiodelay", "kvm-mmu",
"kvmclock",
> > + "kvm-asyncpf", "kvm-steal-time",
"kvm-pv-eoi", "kvm-pv-unhalt",
> > NULL, NULL, NULL, NULL,
> > NULL, NULL, NULL, NULL,
> > NULL, NULL, NULL, NULL,
> > @@ -244,9 +244,9 @@ static const char *kvm_feature_name[] = {
> > };
> >
> > static const char *svm_feature_name[] = {
> > - "npt", "lbrv", "svm_lock",
"nrip_save",
> > - "tsc_scale", "vmcb_clean", "flushbyasid",
"decodeassists",
> > - NULL, NULL, "pause_filter", NULL,
> > + "npt", "lbrv", "svm-lock",
"nrip-save",
> > + "tsc-scale", "vmcb-clean", "flushbyasid",
"decodeassists",
> > + NULL, NULL, "pause-filter", NULL,
> > "pfthreshold", NULL, NULL, NULL,
> > NULL, NULL, NULL, NULL,
> > NULL, NULL, NULL, NULL,
> > @@ -255,7 +255,7 @@ static const char *svm_feature_name[] = {
> > };
> >
> > static const char *cpuid_7_0_ebx_feature_name[] = {
> > - "fsgsbase", "tsc_adjust", NULL, "bmi1",
"hle", "avx2", NULL, "smep",
> > + "fsgsbase", "tsc-adjust", NULL, "bmi1",
"hle", "avx2", NULL, "smep",
> > "bmi2", "erms", "invpcid", "rtm",
NULL, NULL, "mpx", NULL,
> > "avx512f", NULL, "rdseed", "adx",
"smap", NULL, "pcommit", "clflushopt",
> > "clwb", NULL, "avx512pf", "avx512er",
"avx512cd", NULL, NULL, NULL,
> > @@ -1894,8 +1894,8 @@ static PropertyInfo qdev_prop_spinlocks = {
> > .set = x86_set_hv_spinlocks,
> > };
> >
> > -/* Convert all '_' in a feature string option name to '-', to
make feature
> > - * name conform to QOM property naming rule, which uses '-' instead of
'_'.
> > +/* Convert all '_' in a feature string option name to '-', to
keep compatibility
> > + * with old feature names that used "_" instead of "-".
> > */
> > static inline void feat2prop(char *s)
> > {
> > @@ -1925,8 +1925,10 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char
*features,
> > while (featurestr) {
> > char *val;
> I'd place a single feat2prop() here
> and delete it from other call sites in this function.
A previous version of this patch had it. But it would change the
property value too, not just the property name (breaking stuff
like "model-id=some_string").
it's bug in feat2prop(), which probably should be fixed there,
so it would do what comment above it says. Or as alternative:
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ca2a893..e46e4c3 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1941,14 +1941,16 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features
featurestr = features ? strtok(features, ",") : NULL;
while (featurestr) {
- char *val;
+ char *val = strchr(featurestr, '=');
+ if (val) {
+ *val = 0; val++;
+ }
+ feat2prop(featurestr);
if (featurestr[0] == '+') {
add_flagname_to_bitmaps(featurestr + 1, plus_features, &local_err);
} else if (featurestr[0] == '-') {
add_flagname_to_bitmaps(featurestr + 1, minus_features, &local_err);
- } else if ((val = strchr(featurestr, '='))) {
- *val = 0; val++;
- feat2prop(featurestr);
+ } else if (val) {
if (!strcmp(featurestr, "xlevel")) {
char *err;
char num[32];
@@ -2000,7 +2002,6 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features,
object_property_parse(OBJECT(cpu), val, featurestr, &local_err);
}
} else {
- feat2prop(featurestr);
object_property_parse(OBJECT(cpu), "on", featurestr,
&local_err);
}
if (local_err) {