| Message ID | 1357582848-16575-5-git-send-email-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Jan 07, 2013 at 04:20:45PM -0200, Eduardo Habkost wrote: > This introduces a FeatureWord enum, FeatureWordInfo struct (with > generation information about a feature word), and a FeatureWordArray > typedef, and changes add_flagname_to_bitmaps() code and > cpu_x86_parse_featurestr() to use the new typedefs instead of separate > variables for each feature word. > > This will help us keep the code at kvm_check_features_against_host(), > cpu_x86_parse_featurestr() and add_flagname_to_bitmaps() sane while > adding new feature name arrays. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > target-i386/cpu.c | 97 +++++++++++++++++++++++++++---------------------------- > target-i386/cpu.h | 15 +++++++++ > 2 files changed, 63 insertions(+), 49 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index b09b625..7d62d48 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -124,6 +124,20 @@ static const char *cpuid_7_0_ebx_feature_name[] = { > NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, > }; > > +typedef struct FeatureWordInfo { > + const char **feat_names; > +} FeatureWordInfo; > + > +static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { > + [FEAT_1_EDX] = { .feat_names = feature_name }, > + [FEAT_1_ECX] = { .feat_names = ext_feature_name }, > + [FEAT_8000_0001_EDX] = { .feat_names = ext2_feature_name }, > + [FEAT_8000_0001_ECX] = { .feat_names = ext3_feature_name }, > + [FEAT_KVM] = { .feat_names = kvm_feature_name }, > + [FEAT_SVM] = { .feat_names = svm_feature_name }, > + [FEAT_7_0_EBX] = { .feat_names = cpuid_7_0_ebx_feature_name }, > +}; > + > const char *get_register_name_32(unsigned int reg) > { > static const char *reg_names[CPU_NB_REGS32] = { > @@ -271,23 +285,20 @@ 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, > - uint32_t *cpuid_7_0_ebx_features) > +static void add_flagname_to_bitmaps(const char *flagname, > + FeatureWordArray words) > { > - 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); > + FeatureWord w; > + for (w = 0; w < FEATURE_WORDS; w++) { > + FeatureWordInfo *wi = &feature_word_info[w]; > + if (wi->feat_names && I would move patch 6 before that one and drop this check here, but if you disagree it is your call. > + lookup_feature(&words[w], flagname, NULL, wi->feat_names)) { > + break; > + } > + } > + if (w == FEATURE_WORDS) { > + fprintf(stderr, "CPU feature %s not found\n", flagname); > + } > } > > typedef struct x86_def_t { > @@ -1256,35 +1267,23 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features) > unsigned int i; > char *featurestr; /* Single 'key=value" string being parsed */ > /* 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 = kvm_default_features, plus_svm_features = 0; > - uint32_t plus_7_0_ebx_features = 0; > + FeatureWordArray plus_features = { > + [FEAT_KVM] = kvm_default_features, > + }; > /* 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; > + FeatureWordArray minus_features = { 0 }; > uint32_t numvalue; > > - add_flagname_to_bitmaps("hypervisor", &plus_features, > - &plus_ext_features, &plus_ext2_features, &plus_ext3_features, > - &plus_kvm_features, &plus_svm_features, &plus_7_0_ebx_features); > + add_flagname_to_bitmaps("hypervisor", plus_features); > > featurestr = features ? strtok(features, ",") : 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); > + add_flagname_to_bitmaps(featurestr + 1, plus_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); > + add_flagname_to_bitmaps(featurestr + 1, minus_features); > } else if ((val = strchr(featurestr, '='))) { > *val = 0; val++; > if (!strcmp(featurestr, "family")) { > @@ -1384,20 +1383,20 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features) > } > 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->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; > + x86_cpu_def->features |= plus_features[FEAT_1_EDX]; > + x86_cpu_def->ext_features |= plus_features[FEAT_1_ECX]; > + x86_cpu_def->ext2_features |= plus_features[FEAT_8000_0001_EDX]; > + x86_cpu_def->ext3_features |= plus_features[FEAT_8000_0001_ECX]; > + x86_cpu_def->kvm_features |= plus_features[FEAT_KVM]; > + x86_cpu_def->svm_features |= plus_features[FEAT_SVM]; > + x86_cpu_def->cpuid_7_0_ebx_features |= plus_features[FEAT_7_0_EBX]; > + x86_cpu_def->features &= ~minus_features[FEAT_1_EDX]; > + x86_cpu_def->ext_features &= ~minus_features[FEAT_1_ECX]; > + x86_cpu_def->ext2_features &= ~minus_features[FEAT_8000_0001_EDX]; > + x86_cpu_def->ext3_features &= ~minus_features[FEAT_8000_0001_ECX]; > + x86_cpu_def->kvm_features &= ~minus_features[FEAT_KVM]; > + x86_cpu_def->svm_features &= ~minus_features[FEAT_SVM]; > + x86_cpu_def->cpuid_7_0_ebx_features &= ~minus_features[FEAT_7_0_EBX]; > if (check_cpuid && kvm_enabled()) { > if (kvm_check_features_against_host(x86_cpu_def) && enforce_cpuid) > goto error; > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index e56921b..e4a7c50 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -361,6 +361,21 @@ > > #define MSR_VM_HSAVE_PA 0xc0010117 > > +/* CPUID feature words */ > +typedef enum FeatureWord { > + FEAT_1_EDX, /* CPUID[1].EDX */ > + FEAT_1_ECX, /* CPUID[1].ECX */ > + FEAT_7_0_EBX, /* CPUID[EAX=7,ECX=0].EBX */ > + FEAT_8000_0001_EDX, /* CPUID[8000_0001].EDX */ > + FEAT_8000_0001_ECX, /* CPUID[8000_0001].ECX */ > + FEAT_C000_0001_EDX, /* CPUID[C000_0001].EDX */ > + FEAT_KVM, /* CPUID[4000_0001].EAX (KVM_CPUID_FEATURES) */ > + FEAT_SVM, /* CPUID[8000_000A].EDX */ > + FEATURE_WORDS, > +} FeatureWord; > + > +typedef uint32_t FeatureWordArray[FEATURE_WORDS]; > + > /* cpuid_features bits */ > #define CPUID_FP87 (1 << 0) > #define CPUID_VME (1 << 1) > -- > 1.7.11.7 -- Gleb.
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index b09b625..7d62d48 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -124,6 +124,20 @@ static const char *cpuid_7_0_ebx_feature_name[] = { NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, }; +typedef struct FeatureWordInfo { + const char **feat_names; +} FeatureWordInfo; + +static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { + [FEAT_1_EDX] = { .feat_names = feature_name }, + [FEAT_1_ECX] = { .feat_names = ext_feature_name }, + [FEAT_8000_0001_EDX] = { .feat_names = ext2_feature_name }, + [FEAT_8000_0001_ECX] = { .feat_names = ext3_feature_name }, + [FEAT_KVM] = { .feat_names = kvm_feature_name }, + [FEAT_SVM] = { .feat_names = svm_feature_name }, + [FEAT_7_0_EBX] = { .feat_names = cpuid_7_0_ebx_feature_name }, +}; + const char *get_register_name_32(unsigned int reg) { static const char *reg_names[CPU_NB_REGS32] = { @@ -271,23 +285,20 @@ 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, - uint32_t *cpuid_7_0_ebx_features) +static void add_flagname_to_bitmaps(const char *flagname, + FeatureWordArray words) { - 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); + FeatureWord w; + for (w = 0; w < FEATURE_WORDS; w++) { + FeatureWordInfo *wi = &feature_word_info[w]; + if (wi->feat_names && + lookup_feature(&words[w], flagname, NULL, wi->feat_names)) { + break; + } + } + if (w == FEATURE_WORDS) { + fprintf(stderr, "CPU feature %s not found\n", flagname); + } } typedef struct x86_def_t { @@ -1256,35 +1267,23 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features) unsigned int i; char *featurestr; /* Single 'key=value" string being parsed */ /* 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 = kvm_default_features, plus_svm_features = 0; - uint32_t plus_7_0_ebx_features = 0; + FeatureWordArray plus_features = { + [FEAT_KVM] = kvm_default_features, + }; /* 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; + FeatureWordArray minus_features = { 0 }; uint32_t numvalue; - add_flagname_to_bitmaps("hypervisor", &plus_features, - &plus_ext_features, &plus_ext2_features, &plus_ext3_features, - &plus_kvm_features, &plus_svm_features, &plus_7_0_ebx_features); + add_flagname_to_bitmaps("hypervisor", plus_features); featurestr = features ? strtok(features, ",") : 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); + add_flagname_to_bitmaps(featurestr + 1, plus_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); + add_flagname_to_bitmaps(featurestr + 1, minus_features); } else if ((val = strchr(featurestr, '='))) { *val = 0; val++; if (!strcmp(featurestr, "family")) { @@ -1384,20 +1383,20 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features) } 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->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; + x86_cpu_def->features |= plus_features[FEAT_1_EDX]; + x86_cpu_def->ext_features |= plus_features[FEAT_1_ECX]; + x86_cpu_def->ext2_features |= plus_features[FEAT_8000_0001_EDX]; + x86_cpu_def->ext3_features |= plus_features[FEAT_8000_0001_ECX]; + x86_cpu_def->kvm_features |= plus_features[FEAT_KVM]; + x86_cpu_def->svm_features |= plus_features[FEAT_SVM]; + x86_cpu_def->cpuid_7_0_ebx_features |= plus_features[FEAT_7_0_EBX]; + x86_cpu_def->features &= ~minus_features[FEAT_1_EDX]; + x86_cpu_def->ext_features &= ~minus_features[FEAT_1_ECX]; + x86_cpu_def->ext2_features &= ~minus_features[FEAT_8000_0001_EDX]; + x86_cpu_def->ext3_features &= ~minus_features[FEAT_8000_0001_ECX]; + x86_cpu_def->kvm_features &= ~minus_features[FEAT_KVM]; + x86_cpu_def->svm_features &= ~minus_features[FEAT_SVM]; + x86_cpu_def->cpuid_7_0_ebx_features &= ~minus_features[FEAT_7_0_EBX]; if (check_cpuid && kvm_enabled()) { if (kvm_check_features_against_host(x86_cpu_def) && enforce_cpuid) goto error; diff --git a/target-i386/cpu.h b/target-i386/cpu.h index e56921b..e4a7c50 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -361,6 +361,21 @@ #define MSR_VM_HSAVE_PA 0xc0010117 +/* CPUID feature words */ +typedef enum FeatureWord { + FEAT_1_EDX, /* CPUID[1].EDX */ + FEAT_1_ECX, /* CPUID[1].ECX */ + FEAT_7_0_EBX, /* CPUID[EAX=7,ECX=0].EBX */ + FEAT_8000_0001_EDX, /* CPUID[8000_0001].EDX */ + FEAT_8000_0001_ECX, /* CPUID[8000_0001].ECX */ + FEAT_C000_0001_EDX, /* CPUID[C000_0001].EDX */ + FEAT_KVM, /* CPUID[4000_0001].EAX (KVM_CPUID_FEATURES) */ + FEAT_SVM, /* CPUID[8000_000A].EDX */ + FEATURE_WORDS, +} FeatureWord; + +typedef uint32_t FeatureWordArray[FEATURE_WORDS]; + /* cpuid_features bits */ #define CPUID_FP87 (1 << 0) #define CPUID_VME (1 << 1)
This introduces a FeatureWord enum, FeatureWordInfo struct (with generation information about a feature word), and a FeatureWordArray typedef, and changes add_flagname_to_bitmaps() code and cpu_x86_parse_featurestr() to use the new typedefs instead of separate variables for each feature word. This will help us keep the code at kvm_check_features_against_host(), cpu_x86_parse_featurestr() and add_flagname_to_bitmaps() sane while adding new feature name arrays. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target-i386/cpu.c | 97 +++++++++++++++++++++++++++---------------------------- target-i386/cpu.h | 15 +++++++++ 2 files changed, 63 insertions(+), 49 deletions(-)