| Message ID | 1350918203-25198-33-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 10/22/12 11:03, Igor Mammedov wrote: > - def->kvm_features was used only for setting hadcoded defaults, > with defaults set by static props, we do not need it anymore, > ignore it in cpudef_2_x86_cpu(). > - take in account CPUID_EXT_HYPERVISOR feature set by static > property default value; > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > * fix double free of features in cpu_x86_find_by_name() > --- > target-i386/cpu.c | 271 +++++++----------------------------------------------- > 1 file changed, 31 insertions(+), 240 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index c804965..59809f8 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -87,31 +87,6 @@ static const char *ext3_feature_name[] = { > NULL, NULL, NULL, NULL, > }; > > -static const char *kvm_feature_name[] = { > - "kvmclock", "kvm_nopiodelay", "kvm_mmu", "kvmclock", "kvm_asyncpf", NULL, "kvm_pv_eoi", NULL, > - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, > - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, > - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, > -}; > - > -static const char *svm_feature_name[] = { > - "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, > - NULL, NULL, NULL, NULL, > - NULL, NULL, NULL, NULL, > -}; > - > -static const char *cpuid_7_0_ebx_feature_name[] = { > - NULL, NULL, NULL, NULL, NULL, NULL, NULL, "smep", > - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, > - NULL, NULL, NULL, NULL, "smap", NULL, NULL, NULL, > - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, > -}; > - > #if defined(CONFIG_KVM) > static void x86_cpu_get_kvmclock(Object *obj, Visitor *v, void *opaque, > const char *name, Error **errp) > @@ -752,88 +727,6 @@ void host_cpuid(uint32_t function, uint32_t count, > #endif > } > > -#define iswhite(c) ((c) && ((c) <= ' ' || '~' < (c))) > - > -/* general substring compare of *[s1..e1) and *[s2..e2). sx is start of > - * a substring. ex if !NULL points to the first char after a substring, > - * otherwise the string is assumed to sized by a terminating nul. > - * Return lexical ordering of *s1:*s2. > - */ > -static int sstrcmp(const char *s1, const char *e1, const char *s2, > - const char *e2) > -{ > - for (;;) { > - if (!*s1 || !*s2 || *s1 != *s2) > - return (*s1 - *s2); > - ++s1, ++s2; > - if (s1 == e1 && s2 == e2) > - return (0); > - else if (s1 == e1) > - return (*s2); > - else if (s2 == e2) > - return (*s1); > - } > -} > - > -/* compare *[s..e) to *altstr. *altstr may be a simple string or multiple > - * '|' delimited (possibly empty) strings in which case search for a match > - * within the alternatives proceeds left to right. Return 0 for success, > - * non-zero otherwise. > - */ > -static int altcmp(const char *s, const char *e, const char *altstr) > -{ > - const char *p, *q; > - > - for (q = p = altstr; ; ) { > - while (*p && *p != '|') > - ++p; > - if ((q == p && !*s) || (q != p && !sstrcmp(s, e, q, p))) > - return (0); > - if (!*p) > - return (1); > - else > - q = ++p; > - } > -} > - > -/* search featureset for flag *[s..e), if found set corresponding bit in > - * *pval and return true, otherwise return false > - */ > -static bool lookup_feature(uint32_t *pval, const char *s, const char *e, > - const char **featureset) > -{ > - uint32_t mask; > - const char **ppc; > - bool found = false; > - > - for (mask = 1, ppc = featureset; mask; mask <<= 1, ++ppc) { > - if (*ppc && !altcmp(s, e, *ppc)) { > - *pval |= mask; > - found = true; > - } > - } > - return found; > -} > - > -static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features, > - uint32_t *ext_features, > - uint32_t *ext2_features, > - uint32_t *ext3_features, > - uint32_t *kvm_features, > - uint32_t *svm_features, > - uint32_t *cpuid_7_0_ebx_features) > -{ > - if (!lookup_feature(features, flagname, NULL, feature_name) && > - !lookup_feature(ext_features, flagname, NULL, ext_feature_name) && > - !lookup_feature(ext2_features, flagname, NULL, ext2_feature_name) && > - !lookup_feature(ext3_features, flagname, NULL, ext3_feature_name) && > - !lookup_feature(kvm_features, flagname, NULL, kvm_feature_name) && > - !lookup_feature(svm_features, flagname, NULL, svm_feature_name) && > - !lookup_feature(cpuid_7_0_ebx_features, flagname, NULL, > - cpuid_7_0_ebx_feature_name)) > - fprintf(stderr, "CPU feature %s not found\n", flagname); > -} > - > typedef struct x86_def_t { > struct x86_def_t *next; > const char *name; > @@ -1432,10 +1325,10 @@ static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t *def, Error **errp) > object_property_set_int(OBJECT(cpu), (int64_t)def->tsc_khz * 1000, > "tsc-frequency", errp); > env->cpuid_features = def->features; > - env->cpuid_ext_features = def->ext_features; > + env->cpuid_ext_features = (CPUID_EXT_HYPERVISOR & env->cpuid_ext_features) | > + def->ext_features; > env->cpuid_ext2_features = def->ext2_features; > env->cpuid_ext3_features = def->ext3_features; > - env->cpuid_kvm_features = def->kvm_features; > env->cpuid_svm_features = def->svm_features; > env->cpuid_ext4_features = def->ext4_features; > env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features; > @@ -1532,25 +1425,32 @@ out: > return; > } > > +/* Set features on X86CPU object based on a QDict */ > +static void cpu_x86_set_props(X86CPU *cpu, QDict *features, Error **errp) > +{ > + const QDictEntry *ent; > + > + 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)) { > + 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; > - CPUX86State *env = &cpu->env; > + QDict *features = NULL; > + char *name = NULL; > > - char *s = g_strdup(cpu_model); > - char *featurestr, *name = strtok(s, ","); > - /* Features to be added*/ > - uint32_t plus_features = 0, plus_ext_features = env->cpuid_ext_features; > - uint32_t plus_ext2_features = 0, plus_ext3_features = 0; > - uint32_t plus_kvm_features = env->cpuid_kvm_features, plus_svm_features = 0; > - uint32_t plus_7_0_ebx_features = 0; > - /* Features to be removed */ > - uint32_t minus_features = 0, minus_ext_features = 0; > - uint32_t minus_ext2_features = 0, minus_ext3_features = 0; > - uint32_t minus_kvm_features = 0, minus_svm_features = 0; > - uint32_t minus_7_0_ebx_features = 0; > - uint32_t numvalue; > + 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)) > @@ -1563,127 +1463,18 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, > memcpy(x86_cpu_def, def, sizeof(*def)); > } > > - featurestr = strtok(NULL, ","); > - > - while (featurestr) { > - char *val; > - if (featurestr[0] == '+') { > - add_flagname_to_bitmaps(featurestr + 1, &plus_features, > - &plus_ext_features, &plus_ext2_features, > - &plus_ext3_features, &plus_kvm_features, > - &plus_svm_features, &plus_7_0_ebx_features); > - } else if (featurestr[0] == '-') { > - add_flagname_to_bitmaps(featurestr + 1, &minus_features, > - &minus_ext_features, &minus_ext2_features, > - &minus_ext3_features, &minus_kvm_features, > - &minus_svm_features, &minus_7_0_ebx_features); > - } else if ((val = strchr(featurestr, '='))) { > - *val = 0; val++; > - if (!strcmp(featurestr, "family")) { > - char *err; > - numvalue = strtoul(val, &err, 0); > - if (!*val || *err || numvalue > 0xff + 0xf) { > - fprintf(stderr, "bad numerical value %s\n", val); > - goto error; > - } > - x86_cpu_def->family = numvalue; > - } else if (!strcmp(featurestr, "model")) { > - char *err; > - numvalue = strtoul(val, &err, 0); > - if (!*val || *err || numvalue > 0xff) { > - fprintf(stderr, "bad numerical value %s\n", val); > - goto error; > - } > - x86_cpu_def->model = numvalue; > - } else if (!strcmp(featurestr, "stepping")) { > - char *err; > - numvalue = strtoul(val, &err, 0); > - if (!*val || *err || numvalue > 0xf) { > - fprintf(stderr, "bad numerical value %s\n", val); > - goto error; > - } > - x86_cpu_def->stepping = numvalue ; > - } else if (!strcmp(featurestr, "level")) { > - char *err; > - numvalue = strtoul(val, &err, 0); > - if (!*val || *err) { > - fprintf(stderr, "bad numerical value %s\n", val); > - goto error; > - } > - x86_cpu_def->level = numvalue; > - } else if (!strcmp(featurestr, "xlevel")) { > - char *err; > - numvalue = strtoul(val, &err, 0); > - if (!*val || *err) { > - fprintf(stderr, "bad numerical value %s\n", val); > - goto error; > - } > - if (numvalue < 0x80000000) { > - numvalue += 0x80000000; > - } > - x86_cpu_def->xlevel = numvalue; > - } else if (!strcmp(featurestr, "vendor")) { > - pstrcpy(x86_cpu_def->vendor, sizeof(x86_cpu_def->vendor), val); > - x86_cpu_def->vendor_override = 1; > - } else if (!strcmp(featurestr, "model_id")) { This makes model_id= as a feature stop working. It needs to be added to this patch, or maybe #10 where "tsc_freq" was handled. > - pstrcpy(x86_cpu_def->model_id, sizeof(x86_cpu_def->model_id), > - val); > - } else if (!strcmp(featurestr, "tsc_freq")) { > - int64_t tsc_freq; > - char *err; > - > - tsc_freq = strtosz_suffix_unit(val, &err, > - STRTOSZ_DEFSUFFIX_B, 1000); > - if (tsc_freq < 0 || *err) { > - fprintf(stderr, "bad numerical value %s\n", val); > - goto error; > - } > - x86_cpu_def->tsc_khz = tsc_freq / 1000; > - } else if (!strcmp(featurestr, "hv_spinlocks")) { > - char *err; > - numvalue = strtoul(val, &err, 0); > - if (!*val || *err) { > - fprintf(stderr, "bad numerical value %s\n", val); > - goto error; > - } > - hyperv_set_spinlock_retries(numvalue); > - } else { > - fprintf(stderr, "unrecognized feature %s\n", featurestr); > - goto error; > - } > - } else if (!strcmp(featurestr, "check")) { > - check_cpuid = 1; > - } else if (!strcmp(featurestr, "enforce")) { > - check_cpuid = enforce_cpuid = 1; > - } else if (!strcmp(featurestr, "hv_relaxed")) { > - hyperv_enable_relaxed_timing(true); > - } else if (!strcmp(featurestr, "hv_vapic")) { > - hyperv_enable_vapic_recommended(true); > - } else { > - fprintf(stderr, "feature string `%s' not in format (+feature|-feature|feature=xyz)\n", featurestr); > - goto error; > - } > - featurestr = strtok(NULL, ","); > + cpu_x86_set_props(cpu, features, errp); > + if (error_is_set(errp)) { > + goto error; > } > - x86_cpu_def->features |= plus_features; > - x86_cpu_def->ext_features |= plus_ext_features; > - x86_cpu_def->ext2_features |= plus_ext2_features; > - x86_cpu_def->ext3_features |= plus_ext3_features; > - x86_cpu_def->kvm_features |= plus_kvm_features; > - x86_cpu_def->svm_features |= plus_svm_features; > - x86_cpu_def->cpuid_7_0_ebx_features |= plus_7_0_ebx_features; > - x86_cpu_def->features &= ~minus_features; > - x86_cpu_def->ext_features &= ~minus_ext_features; > - x86_cpu_def->ext2_features &= ~minus_ext2_features; > - x86_cpu_def->ext3_features &= ~minus_ext3_features; > - x86_cpu_def->kvm_features &= ~minus_kvm_features; > - x86_cpu_def->svm_features &= ~minus_svm_features; > - x86_cpu_def->cpuid_7_0_ebx_features &= ~minus_7_0_ebx_features; > - g_free(s); > + > + g_free(name); > + QDECREF(features); > return 0; > > error: > - g_free(s); > + g_free(name); > + QDECREF(features); > if (!error_is_set(errp)) { > error_set(errp, QERR_INVALID_PARAMETER_COMBINATION); > } -Don Slutz
On Tue, 23 Oct 2012 11:29:46 -0400 Don Slutz <Don@cloudswitch.com> wrote: > On 10/22/12 11:03, Igor Mammedov wrote: > > - def->kvm_features was used only for setting hadcoded defaults, > > with defaults set by static props, we do not need it anymore, > > ignore it in cpudef_2_x86_cpu(). > > - take in account CPUID_EXT_HYPERVISOR feature set by static > > property default value; > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > * fix double free of features in cpu_x86_find_by_name() > > --- > > target-i386/cpu.c | 271 +++++++----------------------------------------------- > > 1 file changed, 31 insertions(+), 240 deletions(-) > > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index c804965..59809f8 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -87,31 +87,6 @@ static const char *ext3_feature_name[] = { > > NULL, NULL, NULL, NULL, > > }; > > > > -static const char *kvm_feature_name[] = { > > - "kvmclock", "kvm_nopiodelay", "kvm_mmu", "kvmclock", "kvm_asyncpf", NULL, "kvm_pv_eoi", NULL, > > - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, > > - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, > > - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, > > -}; > > - > > -static const char *svm_feature_name[] = { > > - "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, > > - NULL, NULL, NULL, NULL, > > - NULL, NULL, NULL, NULL, > > -}; > > - > > -static const char *cpuid_7_0_ebx_feature_name[] = { > > - NULL, NULL, NULL, NULL, NULL, NULL, NULL, "smep", > > - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, > > - NULL, NULL, NULL, NULL, "smap", NULL, NULL, NULL, > > - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, > > -}; > > - > > #if defined(CONFIG_KVM) > > static void x86_cpu_get_kvmclock(Object *obj, Visitor *v, void *opaque, > > const char *name, Error **errp) > > @@ -752,88 +727,6 @@ void host_cpuid(uint32_t function, uint32_t count, > > #endif > > } > > > > -#define iswhite(c) ((c) && ((c) <= ' ' || '~' < (c))) > > - > > -/* general substring compare of *[s1..e1) and *[s2..e2). sx is start of > > - * a substring. ex if !NULL points to the first char after a substring, > > - * otherwise the string is assumed to sized by a terminating nul. > > - * Return lexical ordering of *s1:*s2. > > - */ > > -static int sstrcmp(const char *s1, const char *e1, const char *s2, > > - const char *e2) > > -{ > > - for (;;) { > > - if (!*s1 || !*s2 || *s1 != *s2) > > - return (*s1 - *s2); > > - ++s1, ++s2; > > - if (s1 == e1 && s2 == e2) > > - return (0); > > - else if (s1 == e1) > > - return (*s2); > > - else if (s2 == e2) > > - return (*s1); > > - } > > -} > > - > > -/* compare *[s..e) to *altstr. *altstr may be a simple string or multiple > > - * '|' delimited (possibly empty) strings in which case search for a match > > - * within the alternatives proceeds left to right. Return 0 for success, > > - * non-zero otherwise. > > - */ > > -static int altcmp(const char *s, const char *e, const char *altstr) > > -{ > > - const char *p, *q; > > - > > - for (q = p = altstr; ; ) { > > - while (*p && *p != '|') > > - ++p; > > - if ((q == p && !*s) || (q != p && !sstrcmp(s, e, q, p))) > > - return (0); > > - if (!*p) > > - return (1); > > - else > > - q = ++p; > > - } > > -} > > - > > -/* search featureset for flag *[s..e), if found set corresponding bit in > > - * *pval and return true, otherwise return false > > - */ > > -static bool lookup_feature(uint32_t *pval, const char *s, const char *e, > > - const char **featureset) > > -{ > > - uint32_t mask; > > - const char **ppc; > > - bool found = false; > > - > > - for (mask = 1, ppc = featureset; mask; mask <<= 1, ++ppc) { > > - if (*ppc && !altcmp(s, e, *ppc)) { > > - *pval |= mask; > > - found = true; > > - } > > - } > > - return found; > > -} > > - > > -static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features, > > - uint32_t *ext_features, > > - uint32_t *ext2_features, > > - uint32_t *ext3_features, > > - uint32_t *kvm_features, > > - uint32_t *svm_features, > > - uint32_t *cpuid_7_0_ebx_features) > > -{ > > - if (!lookup_feature(features, flagname, NULL, feature_name) && > > - !lookup_feature(ext_features, flagname, NULL, ext_feature_name) && > > - !lookup_feature(ext2_features, flagname, NULL, ext2_feature_name) && > > - !lookup_feature(ext3_features, flagname, NULL, ext3_feature_name) && > > - !lookup_feature(kvm_features, flagname, NULL, kvm_feature_name) && > > - !lookup_feature(svm_features, flagname, NULL, svm_feature_name) && > > - !lookup_feature(cpuid_7_0_ebx_features, flagname, NULL, > > - cpuid_7_0_ebx_feature_name)) > > - fprintf(stderr, "CPU feature %s not found\n", flagname); > > -} > > - > > typedef struct x86_def_t { > > struct x86_def_t *next; > > const char *name; > > @@ -1432,10 +1325,10 @@ static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t *def, Error **errp) > > object_property_set_int(OBJECT(cpu), (int64_t)def->tsc_khz * 1000, > > "tsc-frequency", errp); > > env->cpuid_features = def->features; > > - env->cpuid_ext_features = def->ext_features; > > + env->cpuid_ext_features = (CPUID_EXT_HYPERVISOR & env->cpuid_ext_features) | > > + def->ext_features; > > env->cpuid_ext2_features = def->ext2_features; > > env->cpuid_ext3_features = def->ext3_features; > > - env->cpuid_kvm_features = def->kvm_features; > > env->cpuid_svm_features = def->svm_features; > > env->cpuid_ext4_features = def->ext4_features; > > env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features; > > @@ -1532,25 +1425,32 @@ out: > > return; > > } > > > > +/* Set features on X86CPU object based on a QDict */ > > +static void cpu_x86_set_props(X86CPU *cpu, QDict *features, Error **errp) > > +{ > > + const QDictEntry *ent; > > + > > + 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)) { > > + 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; > > - CPUX86State *env = &cpu->env; > > + QDict *features = NULL; > > + char *name = NULL; > > > > - char *s = g_strdup(cpu_model); > > - char *featurestr, *name = strtok(s, ","); > > - /* Features to be added*/ > > - uint32_t plus_features = 0, plus_ext_features = env->cpuid_ext_features; > > - uint32_t plus_ext2_features = 0, plus_ext3_features = 0; > > - uint32_t plus_kvm_features = env->cpuid_kvm_features, plus_svm_features = 0; > > - uint32_t plus_7_0_ebx_features = 0; > > - /* Features to be removed */ > > - uint32_t minus_features = 0, minus_ext_features = 0; > > - uint32_t minus_ext2_features = 0, minus_ext3_features = 0; > > - uint32_t minus_kvm_features = 0, minus_svm_features = 0; > > - uint32_t minus_7_0_ebx_features = 0; > > - uint32_t numvalue; > > + 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)) > > @@ -1563,127 +1463,18 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, > > memcpy(x86_cpu_def, def, sizeof(*def)); > > } > > > > - featurestr = strtok(NULL, ","); > > - > > - while (featurestr) { > > - char *val; > > - if (featurestr[0] == '+') { > > - add_flagname_to_bitmaps(featurestr + 1, &plus_features, > > - &plus_ext_features, &plus_ext2_features, > > - &plus_ext3_features, &plus_kvm_features, > > - &plus_svm_features, &plus_7_0_ebx_features); > > - } else if (featurestr[0] == '-') { > > - add_flagname_to_bitmaps(featurestr + 1, &minus_features, > > - &minus_ext_features, &minus_ext2_features, > > - &minus_ext3_features, &minus_kvm_features, > > - &minus_svm_features, &minus_7_0_ebx_features); > > - } else if ((val = strchr(featurestr, '='))) { > > - *val = 0; val++; > > - if (!strcmp(featurestr, "family")) { > > - char *err; > > - numvalue = strtoul(val, &err, 0); > > - if (!*val || *err || numvalue > 0xff + 0xf) { > > - fprintf(stderr, "bad numerical value %s\n", val); > > - goto error; > > - } > > - x86_cpu_def->family = numvalue; > > - } else if (!strcmp(featurestr, "model")) { > > - char *err; > > - numvalue = strtoul(val, &err, 0); > > - if (!*val || *err || numvalue > 0xff) { > > - fprintf(stderr, "bad numerical value %s\n", val); > > - goto error; > > - } > > - x86_cpu_def->model = numvalue; > > - } else if (!strcmp(featurestr, "stepping")) { > > - char *err; > > - numvalue = strtoul(val, &err, 0); > > - if (!*val || *err || numvalue > 0xf) { > > - fprintf(stderr, "bad numerical value %s\n", val); > > - goto error; > > - } > > - x86_cpu_def->stepping = numvalue ; > > - } else if (!strcmp(featurestr, "level")) { > > - char *err; > > - numvalue = strtoul(val, &err, 0); > > - if (!*val || *err) { > > - fprintf(stderr, "bad numerical value %s\n", val); > > - goto error; > > - } > > - x86_cpu_def->level = numvalue; > > - } else if (!strcmp(featurestr, "xlevel")) { > > - char *err; > > - numvalue = strtoul(val, &err, 0); > > - if (!*val || *err) { > > - fprintf(stderr, "bad numerical value %s\n", val); > > - goto error; > > - } > > - if (numvalue < 0x80000000) { > > - numvalue += 0x80000000; > > - } > > - x86_cpu_def->xlevel = numvalue; > > - } else if (!strcmp(featurestr, "vendor")) { > > - pstrcpy(x86_cpu_def->vendor, sizeof(x86_cpu_def->vendor), val); > > - x86_cpu_def->vendor_override = 1; > > - } else if (!strcmp(featurestr, "model_id")) { > This makes model_id= as a feature stop working. It needs to be added to > this patch, or maybe #10 where "tsc_freq" was handled. Thanks for finding this one, I'll put it in compat_normalize_cpu_model() alongside with "tsc_freq" legacy name handling. > > - pstrcpy(x86_cpu_def->model_id, sizeof(x86_cpu_def->model_id), > > - val); > > - } else if (!strcmp(featurestr, "tsc_freq")) { > > - int64_t tsc_freq; > > - char *err; > > - > > - tsc_freq = strtosz_suffix_unit(val, &err, > > - STRTOSZ_DEFSUFFIX_B, 1000); > > - if (tsc_freq < 0 || *err) { > > - fprintf(stderr, "bad numerical value %s\n", val); > > - goto error; > > - } > > - x86_cpu_def->tsc_khz = tsc_freq / 1000; > > - } else if (!strcmp(featurestr, "hv_spinlocks")) { > > - char *err; > > - numvalue = strtoul(val, &err, 0); > > - if (!*val || *err) { > > - fprintf(stderr, "bad numerical value %s\n", val); > > - goto error; > > - } > > - hyperv_set_spinlock_retries(numvalue); > > - } else { > > - fprintf(stderr, "unrecognized feature %s\n", featurestr); > > - goto error; > > - } > > - } else if (!strcmp(featurestr, "check")) { > > - check_cpuid = 1; > > - } else if (!strcmp(featurestr, "enforce")) { > > - check_cpuid = enforce_cpuid = 1; > > - } else if (!strcmp(featurestr, "hv_relaxed")) { > > - hyperv_enable_relaxed_timing(true); > > - } else if (!strcmp(featurestr, "hv_vapic")) { > > - hyperv_enable_vapic_recommended(true); > > - } else { > > - fprintf(stderr, "feature string `%s' not in format (+feature|-feature|feature=xyz)\n", featurestr); > > - goto error; > > - } > > - featurestr = strtok(NULL, ","); > > + cpu_x86_set_props(cpu, features, errp); > > + if (error_is_set(errp)) { > > + goto error; > > } > > - x86_cpu_def->features |= plus_features; > > - x86_cpu_def->ext_features |= plus_ext_features; > > - x86_cpu_def->ext2_features |= plus_ext2_features; > > - x86_cpu_def->ext3_features |= plus_ext3_features; > > - x86_cpu_def->kvm_features |= plus_kvm_features; > > - x86_cpu_def->svm_features |= plus_svm_features; > > - x86_cpu_def->cpuid_7_0_ebx_features |= plus_7_0_ebx_features; > > - x86_cpu_def->features &= ~minus_features; > > - x86_cpu_def->ext_features &= ~minus_ext_features; > > - x86_cpu_def->ext2_features &= ~minus_ext2_features; > > - x86_cpu_def->ext3_features &= ~minus_ext3_features; > > - x86_cpu_def->kvm_features &= ~minus_kvm_features; > > - x86_cpu_def->svm_features &= ~minus_svm_features; > > - x86_cpu_def->cpuid_7_0_ebx_features &= ~minus_7_0_ebx_features; > > - g_free(s); > > + > > + g_free(name); > > + QDECREF(features); > > return 0; > > > > error: > > - g_free(s); > > + g_free(name); > > + QDECREF(features); > > if (!error_is_set(errp)) { > > error_set(errp, QERR_INVALID_PARAMETER_COMBINATION); > > } > -Don Slutz
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index c804965..59809f8 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -87,31 +87,6 @@ static const char *ext3_feature_name[] = { NULL, NULL, NULL, NULL, }; -static const char *kvm_feature_name[] = { - "kvmclock", "kvm_nopiodelay", "kvm_mmu", "kvmclock", "kvm_asyncpf", NULL, "kvm_pv_eoi", NULL, - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, -}; - -static const char *svm_feature_name[] = { - "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, - NULL, NULL, NULL, NULL, - NULL, NULL, NULL, NULL, -}; - -static const char *cpuid_7_0_ebx_feature_name[] = { - NULL, NULL, NULL, NULL, NULL, NULL, NULL, "smep", - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, - NULL, NULL, NULL, NULL, "smap", NULL, NULL, NULL, - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, -}; - #if defined(CONFIG_KVM) static void x86_cpu_get_kvmclock(Object *obj, Visitor *v, void *opaque, const char *name, Error **errp) @@ -752,88 +727,6 @@ void host_cpuid(uint32_t function, uint32_t count, #endif } -#define iswhite(c) ((c) && ((c) <= ' ' || '~' < (c))) - -/* general substring compare of *[s1..e1) and *[s2..e2). sx is start of - * a substring. ex if !NULL points to the first char after a substring, - * otherwise the string is assumed to sized by a terminating nul. - * Return lexical ordering of *s1:*s2. - */ -static int sstrcmp(const char *s1, const char *e1, const char *s2, - const char *e2) -{ - for (;;) { - if (!*s1 || !*s2 || *s1 != *s2) - return (*s1 - *s2); - ++s1, ++s2; - if (s1 == e1 && s2 == e2) - return (0); - else if (s1 == e1) - return (*s2); - else if (s2 == e2) - return (*s1); - } -} - -/* compare *[s..e) to *altstr. *altstr may be a simple string or multiple - * '|' delimited (possibly empty) strings in which case search for a match - * within the alternatives proceeds left to right. Return 0 for success, - * non-zero otherwise. - */ -static int altcmp(const char *s, const char *e, const char *altstr) -{ - const char *p, *q; - - for (q = p = altstr; ; ) { - while (*p && *p != '|') - ++p; - if ((q == p && !*s) || (q != p && !sstrcmp(s, e, q, p))) - return (0); - if (!*p) - return (1); - else - q = ++p; - } -} - -/* search featureset for flag *[s..e), if found set corresponding bit in - * *pval and return true, otherwise return false - */ -static bool lookup_feature(uint32_t *pval, const char *s, const char *e, - const char **featureset) -{ - uint32_t mask; - const char **ppc; - bool found = false; - - for (mask = 1, ppc = featureset; mask; mask <<= 1, ++ppc) { - if (*ppc && !altcmp(s, e, *ppc)) { - *pval |= mask; - found = true; - } - } - return found; -} - -static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features, - uint32_t *ext_features, - uint32_t *ext2_features, - uint32_t *ext3_features, - uint32_t *kvm_features, - uint32_t *svm_features, - uint32_t *cpuid_7_0_ebx_features) -{ - if (!lookup_feature(features, flagname, NULL, feature_name) && - !lookup_feature(ext_features, flagname, NULL, ext_feature_name) && - !lookup_feature(ext2_features, flagname, NULL, ext2_feature_name) && - !lookup_feature(ext3_features, flagname, NULL, ext3_feature_name) && - !lookup_feature(kvm_features, flagname, NULL, kvm_feature_name) && - !lookup_feature(svm_features, flagname, NULL, svm_feature_name) && - !lookup_feature(cpuid_7_0_ebx_features, flagname, NULL, - cpuid_7_0_ebx_feature_name)) - fprintf(stderr, "CPU feature %s not found\n", flagname); -} - typedef struct x86_def_t { struct x86_def_t *next; const char *name; @@ -1432,10 +1325,10 @@ static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t *def, Error **errp) object_property_set_int(OBJECT(cpu), (int64_t)def->tsc_khz * 1000, "tsc-frequency", errp); env->cpuid_features = def->features; - env->cpuid_ext_features = def->ext_features; + env->cpuid_ext_features = (CPUID_EXT_HYPERVISOR & env->cpuid_ext_features) | + def->ext_features; env->cpuid_ext2_features = def->ext2_features; env->cpuid_ext3_features = def->ext3_features; - env->cpuid_kvm_features = def->kvm_features; env->cpuid_svm_features = def->svm_features; env->cpuid_ext4_features = def->ext4_features; env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features; @@ -1532,25 +1425,32 @@ out: return; } +/* Set features on X86CPU object based on a QDict */ +static void cpu_x86_set_props(X86CPU *cpu, QDict *features, Error **errp) +{ + const QDictEntry *ent; + + 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)) { + 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; - CPUX86State *env = &cpu->env; + QDict *features = NULL; + char *name = NULL; - char *s = g_strdup(cpu_model); - char *featurestr, *name = strtok(s, ","); - /* Features to be added*/ - uint32_t plus_features = 0, plus_ext_features = env->cpuid_ext_features; - uint32_t plus_ext2_features = 0, plus_ext3_features = 0; - uint32_t plus_kvm_features = env->cpuid_kvm_features, plus_svm_features = 0; - uint32_t plus_7_0_ebx_features = 0; - /* Features to be removed */ - uint32_t minus_features = 0, minus_ext_features = 0; - uint32_t minus_ext2_features = 0, minus_ext3_features = 0; - uint32_t minus_kvm_features = 0, minus_svm_features = 0; - uint32_t minus_7_0_ebx_features = 0; - uint32_t numvalue; + 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)) @@ -1563,127 +1463,18 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, memcpy(x86_cpu_def, def, sizeof(*def)); } - featurestr = strtok(NULL, ","); - - while (featurestr) { - char *val; - if (featurestr[0] == '+') { - add_flagname_to_bitmaps(featurestr + 1, &plus_features, - &plus_ext_features, &plus_ext2_features, - &plus_ext3_features, &plus_kvm_features, - &plus_svm_features, &plus_7_0_ebx_features); - } else if (featurestr[0] == '-') { - add_flagname_to_bitmaps(featurestr + 1, &minus_features, - &minus_ext_features, &minus_ext2_features, - &minus_ext3_features, &minus_kvm_features, - &minus_svm_features, &minus_7_0_ebx_features); - } else if ((val = strchr(featurestr, '='))) { - *val = 0; val++; - if (!strcmp(featurestr, "family")) { - char *err; - numvalue = strtoul(val, &err, 0); - if (!*val || *err || numvalue > 0xff + 0xf) { - fprintf(stderr, "bad numerical value %s\n", val); - goto error; - } - x86_cpu_def->family = numvalue; - } else if (!strcmp(featurestr, "model")) { - char *err; - numvalue = strtoul(val, &err, 0); - if (!*val || *err || numvalue > 0xff) { - fprintf(stderr, "bad numerical value %s\n", val); - goto error; - } - x86_cpu_def->model = numvalue; - } else if (!strcmp(featurestr, "stepping")) { - char *err; - numvalue = strtoul(val, &err, 0); - if (!*val || *err || numvalue > 0xf) { - fprintf(stderr, "bad numerical value %s\n", val); - goto error; - } - x86_cpu_def->stepping = numvalue ; - } else if (!strcmp(featurestr, "level")) { - char *err; - numvalue = strtoul(val, &err, 0); - if (!*val || *err) { - fprintf(stderr, "bad numerical value %s\n", val); - goto error; - } - x86_cpu_def->level = numvalue; - } else if (!strcmp(featurestr, "xlevel")) { - char *err; - numvalue = strtoul(val, &err, 0); - if (!*val || *err) { - fprintf(stderr, "bad numerical value %s\n", val); - goto error; - } - if (numvalue < 0x80000000) { - numvalue += 0x80000000; - } - x86_cpu_def->xlevel = numvalue; - } else if (!strcmp(featurestr, "vendor")) { - pstrcpy(x86_cpu_def->vendor, sizeof(x86_cpu_def->vendor), val); - x86_cpu_def->vendor_override = 1; - } else if (!strcmp(featurestr, "model_id")) { - pstrcpy(x86_cpu_def->model_id, sizeof(x86_cpu_def->model_id), - val); - } else if (!strcmp(featurestr, "tsc_freq")) { - int64_t tsc_freq; - char *err; - - tsc_freq = strtosz_suffix_unit(val, &err, - STRTOSZ_DEFSUFFIX_B, 1000); - if (tsc_freq < 0 || *err) { - fprintf(stderr, "bad numerical value %s\n", val); - goto error; - } - x86_cpu_def->tsc_khz = tsc_freq / 1000; - } else if (!strcmp(featurestr, "hv_spinlocks")) { - char *err; - numvalue = strtoul(val, &err, 0); - if (!*val || *err) { - fprintf(stderr, "bad numerical value %s\n", val); - goto error; - } - hyperv_set_spinlock_retries(numvalue); - } else { - fprintf(stderr, "unrecognized feature %s\n", featurestr); - goto error; - } - } else if (!strcmp(featurestr, "check")) { - check_cpuid = 1; - } else if (!strcmp(featurestr, "enforce")) { - check_cpuid = enforce_cpuid = 1; - } else if (!strcmp(featurestr, "hv_relaxed")) { - hyperv_enable_relaxed_timing(true); - } else if (!strcmp(featurestr, "hv_vapic")) { - hyperv_enable_vapic_recommended(true); - } else { - fprintf(stderr, "feature string `%s' not in format (+feature|-feature|feature=xyz)\n", featurestr); - goto error; - } - featurestr = strtok(NULL, ","); + cpu_x86_set_props(cpu, features, errp); + if (error_is_set(errp)) { + goto error; } - x86_cpu_def->features |= plus_features; - x86_cpu_def->ext_features |= plus_ext_features; - x86_cpu_def->ext2_features |= plus_ext2_features; - x86_cpu_def->ext3_features |= plus_ext3_features; - x86_cpu_def->kvm_features |= plus_kvm_features; - x86_cpu_def->svm_features |= plus_svm_features; - x86_cpu_def->cpuid_7_0_ebx_features |= plus_7_0_ebx_features; - x86_cpu_def->features &= ~minus_features; - x86_cpu_def->ext_features &= ~minus_ext_features; - x86_cpu_def->ext2_features &= ~minus_ext2_features; - x86_cpu_def->ext3_features &= ~minus_ext3_features; - x86_cpu_def->kvm_features &= ~minus_kvm_features; - x86_cpu_def->svm_features &= ~minus_svm_features; - x86_cpu_def->cpuid_7_0_ebx_features &= ~minus_7_0_ebx_features; - g_free(s); + + g_free(name); + QDECREF(features); return 0; error: - g_free(s); + g_free(name); + QDECREF(features); if (!error_is_set(errp)) { error_set(errp, QERR_INVALID_PARAMETER_COMBINATION); }
- def->kvm_features was used only for setting hadcoded defaults, with defaults set by static props, we do not need it anymore, ignore it in cpudef_2_x86_cpu(). - take in account CPUID_EXT_HYPERVISOR feature set by static property default value; Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- * fix double free of features in cpu_x86_find_by_name() --- target-i386/cpu.c | 271 +++++++----------------------------------------------- 1 file changed, 31 insertions(+), 240 deletions(-)