| Message ID | 1344597756-2890-19-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Aug 10, 2012 at 01:22:34PM +0200, Igor Mammedov wrote: > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > target-i386/cpu.c | 167 +++++++----------------------------------------------- > 1 file changed, 20 insertions(+), 147 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 7555b08..0174c4d 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -206,22 +206,6 @@ static bool lookup_feature(uint32_t *pval, const char *s, const char *e, > 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) > -{ > - 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)) > - fprintf(stderr, "CPU feature %s not found\n", flagname); > -} > - > typedef struct x86_def_t { > struct x86_def_t *next; > const char *name; > @@ -1104,7 +1088,6 @@ static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t *def, Error **errp) > 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 = def->cpuid_7_0_ebx_features; > @@ -1168,17 +1151,14 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, > { > x86_def_t *def; > > - char *s = g_strdup(cpu_model); > - char *featurestr, *name = strtok(s, ","); > - /* Features to be added*/ > - uint32_t plus_features = 0, plus_ext_features = 0; > - uint32_t plus_ext2_features = 0, plus_ext3_features = 0; > - uint32_t plus_kvm_features = 0, plus_svm_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 numvalue; > + QDict *features; > + const QDictEntry *ent; > + char *name; > + > + 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)) > @@ -1191,133 +1171,28 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, > memcpy(x86_cpu_def, def, sizeof(*def)); > } > > - plus_kvm_features = ~0; /* not supported bits will be filtered out later */ > - > - add_flagname_to_bitmaps("hypervisor", &plus_features, > - &plus_ext_features, &plus_ext2_features, &plus_ext3_features, > - &plus_kvm_features, &plus_svm_features); > + cpudef_2_x86_cpu(cpu, def, errp); It looks like you have to use 'x86_cpu_def' instead of 'def', here, as 'def' is the variable for the x86_defs name-lookup loop, only, 'x86_cpu_def' is the actual CPU definition. > > - featurestr = strtok(NULL, ","); > + /* not supported bits will be filtered out later */ > + env->cpuid_kvm_features = ~0; > > - 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); > - } 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); > - } 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; > - } > - 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; > - } > + object_property_set_bool(OBJECT(cpu), true, "hypervisor", errp); > > + 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)) { > goto error; > } > - > - featurestr = strtok(NULL, ","); > } > - 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->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; > - g_free(s); > + QDECREF(features); > + > + g_free(name); > return 0; > > error: > - g_free(s); > + g_free(name); > if (!error_is_set(errp)) { > error_set(errp, QERR_INVALID_PARAMETER_COMBINATION); > } > @@ -1427,8 +1302,6 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model) > if (cpu_x86_find_by_name(cpu, def, cpu_model, &error) < 0) > goto out; > > - cpudef_2_x86_cpu(cpu, def, &error); > - You could move the 'def' and 'def1' variables inside cpu_x86_find_by_name(), now, as they are now only used inside that function. > out: > if (error_is_set(&error)) { > fprintf(stderr, "%s\n", error_get_pretty(error)); > -- > 1.7.11.2 > >
On Mon, 13 Aug 2012 17:48:24 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Fri, Aug 10, 2012 at 01:22:34PM +0200, Igor Mammedov wrote: > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > target-i386/cpu.c | 167 +++++++----------------------------------------------- > > 1 file changed, 20 insertions(+), 147 deletions(-) > > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index 7555b08..0174c4d 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -206,22 +206,6 @@ static bool lookup_feature(uint32_t *pval, const char *s, const char *e, > > 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) > > -{ > > - 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)) > > - fprintf(stderr, "CPU feature %s not found\n", flagname); > > -} > > - > > typedef struct x86_def_t { > > struct x86_def_t *next; > > const char *name; > > @@ -1104,7 +1088,6 @@ static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t *def, Error **errp) > > 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 = def->cpuid_7_0_ebx_features; > > @@ -1168,17 +1151,14 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, > > { > > x86_def_t *def; > > > > - char *s = g_strdup(cpu_model); > > - char *featurestr, *name = strtok(s, ","); > > - /* Features to be added*/ > > - uint32_t plus_features = 0, plus_ext_features = 0; > > - uint32_t plus_ext2_features = 0, plus_ext3_features = 0; > > - uint32_t plus_kvm_features = 0, plus_svm_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 numvalue; > > + QDict *features; > > + const QDictEntry *ent; > > + char *name; > > + > > + 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)) > > @@ -1191,133 +1171,28 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, > > memcpy(x86_cpu_def, def, sizeof(*def)); > > } > > > > - plus_kvm_features = ~0; /* not supported bits will be filtered out later */ > > - > > - add_flagname_to_bitmaps("hypervisor", &plus_features, > > - &plus_ext_features, &plus_ext2_features, &plus_ext3_features, > > - &plus_kvm_features, &plus_svm_features); > > + cpudef_2_x86_cpu(cpu, def, errp); > > It looks like you have to use 'x86_cpu_def' instead of 'def', here, as > 'def' is the variable for the x86_defs name-lookup loop, only, > 'x86_cpu_def' is the actual CPU definition. yep, I'll fix it. > > > > > > - featurestr = strtok(NULL, ","); > > + /* not supported bits will be filtered out later */ > > + env->cpuid_kvm_features = ~0; > > > > - 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); > > - } 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); > > - } 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; > > - } > > - 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; > > - } > > + object_property_set_bool(OBJECT(cpu), true, "hypervisor", errp); > > > > + 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)) { > > goto error; > > } > > - > > - featurestr = strtok(NULL, ","); > > } > > - 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->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; > > - g_free(s); > > + QDECREF(features); > > + > > + g_free(name); > > return 0; > > > > error: > > - g_free(s); > > + g_free(name); > > if (!error_is_set(errp)) { > > error_set(errp, QERR_INVALID_PARAMETER_COMBINATION); > > } > > @@ -1427,8 +1302,6 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model) > > if (cpu_x86_find_by_name(cpu, def, cpu_model, &error) < 0) > > goto out; > > > > - cpudef_2_x86_cpu(cpu, def, &error); > > - > > You could move the 'def' and 'def1' variables inside > cpu_x86_find_by_name(), now, as they are now only used inside that > function. it's better stay here, I'll make another patch that will leave only def lookup in cpu_x86_find_by_name() and move the rest in cpu_x86_register(). with subclasses cpu_x86_find_by_name() should go away and cpu_x86_register() could be disbanded into pieces as it was shown in [NOT RFC] patch I've posted recently. and if you do not object I'd take your "create cpu_x86_set_props() function" patch from https://github.com/ehabkost/qemu-hacks/commit/4d734b316625d0b689031460328ce6e531be5b90 > > > out: > > if (error_is_set(&error)) { > > fprintf(stderr, "%s\n", error_get_pretty(error)); > > -- > > 1.7.11.2 > > > > > > -- > Eduardo
Another issue: On Fri, Aug 10, 2012 at 01:22:34PM +0200, Igor Mammedov wrote: > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > target-i386/cpu.c | 167 +++++++----------------------------------------------- > 1 file changed, 20 insertions(+), 147 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 7555b08..0174c4d 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c [...] > @@ -1191,133 +1171,28 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, > memcpy(x86_cpu_def, def, sizeof(*def)); > } > > - plus_kvm_features = ~0; /* not supported bits will be filtered out later */ > - > - add_flagname_to_bitmaps("hypervisor", &plus_features, > - &plus_ext_features, &plus_ext2_features, &plus_ext3_features, > - &plus_kvm_features, &plus_svm_features); > + cpudef_2_x86_cpu(cpu, def, errp); > > - featurestr = strtok(NULL, ","); > + /* not supported bits will be filtered out later */ > + env->cpuid_kvm_features = ~0; This doesn't compile. The 'env' variable doesn't exist inside cpu_x86_find_by_name(). > > - 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); > - } 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); > - } 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; > - } > - 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; > - } > + object_property_set_bool(OBJECT(cpu), true, "hypervisor", errp); > > + 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)) { > goto error; > } > - > - featurestr = strtok(NULL, ","); > } > - 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->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; > - g_free(s); > + QDECREF(features); > + > + g_free(name); > return 0; > > error: > - g_free(s); > + g_free(name); > if (!error_is_set(errp)) { > error_set(errp, QERR_INVALID_PARAMETER_COMBINATION); > } > @@ -1427,8 +1302,6 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model) > if (cpu_x86_find_by_name(cpu, def, cpu_model, &error) < 0) > goto out; > > - cpudef_2_x86_cpu(cpu, def, &error); > - > out: > if (error_is_set(&error)) { > fprintf(stderr, "%s\n", error_get_pretty(error)); > -- > 1.7.11.2 > >
On Wed, 15 Aug 2012 09:37:22 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > Another issue: > > On Fri, Aug 10, 2012 at 01:22:34PM +0200, Igor Mammedov wrote: > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > target-i386/cpu.c | 167 +++++++----------------------------------------------- > > 1 file changed, 20 insertions(+), 147 deletions(-) > > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index 7555b08..0174c4d 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > [...] > > @@ -1191,133 +1171,28 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, > > memcpy(x86_cpu_def, def, sizeof(*def)); > > } > > > > - plus_kvm_features = ~0; /* not supported bits will be filtered out later */ > > - > > - add_flagname_to_bitmaps("hypervisor", &plus_features, > > - &plus_ext_features, &plus_ext2_features, &plus_ext3_features, > > - &plus_kvm_features, &plus_svm_features); > > + cpudef_2_x86_cpu(cpu, def, errp); > > > > - featurestr = strtok(NULL, ","); > > + /* not supported bits will be filtered out later */ > > + env->cpuid_kvm_features = ~0; > > This doesn't compile. The 'env' variable doesn't exist inside > cpu_x86_find_by_name(). Thanks, it's fixed now. > > > > > - 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); > > - } 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); > > - } 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; > > - } > > - 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; > > - } > > + object_property_set_bool(OBJECT(cpu), true, "hypervisor", errp); > > > > + 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)) { > > goto error; > > } > > - > > - featurestr = strtok(NULL, ","); > > } > > - 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->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; > > - g_free(s); > > + QDECREF(features); > > + > > + g_free(name); > > return 0; > > > > error: > > - g_free(s); > > + g_free(name); > > if (!error_is_set(errp)) { > > error_set(errp, QERR_INVALID_PARAMETER_COMBINATION); > > } > > @@ -1427,8 +1302,6 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model) > > if (cpu_x86_find_by_name(cpu, def, cpu_model, &error) < 0) > > goto out; > > > > - cpudef_2_x86_cpu(cpu, def, &error); > > - > > out: > > if (error_is_set(&error)) { > > fprintf(stderr, "%s\n", error_get_pretty(error)); > > -- > > 1.7.11.2 > > > > > > -- > Eduardo
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 7555b08..0174c4d 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -206,22 +206,6 @@ static bool lookup_feature(uint32_t *pval, const char *s, const char *e, 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) -{ - 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)) - fprintf(stderr, "CPU feature %s not found\n", flagname); -} - typedef struct x86_def_t { struct x86_def_t *next; const char *name; @@ -1104,7 +1088,6 @@ static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t *def, Error **errp) 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 = def->cpuid_7_0_ebx_features; @@ -1168,17 +1151,14 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, { x86_def_t *def; - char *s = g_strdup(cpu_model); - char *featurestr, *name = strtok(s, ","); - /* Features to be added*/ - uint32_t plus_features = 0, plus_ext_features = 0; - uint32_t plus_ext2_features = 0, plus_ext3_features = 0; - uint32_t plus_kvm_features = 0, plus_svm_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 numvalue; + QDict *features; + const QDictEntry *ent; + char *name; + + 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)) @@ -1191,133 +1171,28 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, memcpy(x86_cpu_def, def, sizeof(*def)); } - plus_kvm_features = ~0; /* not supported bits will be filtered out later */ - - add_flagname_to_bitmaps("hypervisor", &plus_features, - &plus_ext_features, &plus_ext2_features, &plus_ext3_features, - &plus_kvm_features, &plus_svm_features); + cpudef_2_x86_cpu(cpu, def, errp); - featurestr = strtok(NULL, ","); + /* not supported bits will be filtered out later */ + env->cpuid_kvm_features = ~0; - 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); - } 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); - } 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; - } - 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; - } + object_property_set_bool(OBJECT(cpu), true, "hypervisor", errp); + 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)) { goto error; } - - featurestr = strtok(NULL, ","); } - 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->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; - g_free(s); + QDECREF(features); + + g_free(name); return 0; error: - g_free(s); + g_free(name); if (!error_is_set(errp)) { error_set(errp, QERR_INVALID_PARAMETER_COMBINATION); } @@ -1427,8 +1302,6 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model) if (cpu_x86_find_by_name(cpu, def, cpu_model, &error) < 0) goto out; - cpudef_2_x86_cpu(cpu, def, &error); - out: if (error_is_set(&error)) { fprintf(stderr, "%s\n", error_get_pretty(error));
Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- target-i386/cpu.c | 167 +++++++----------------------------------------------- 1 file changed, 20 insertions(+), 147 deletions(-)