Re: [PATCH qom-cpu 4/7] target-i386/cpu: Introduce FeatureWord typedefs

2013-01-09 Thread Gleb Natapov 
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 ehabk...@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,
 -

[PATCH qom-cpu 4/7] target-i386/cpu: Introduce FeatureWord typedefs

2013-01-07 Thread Eduardo Habkost 
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 ehabk...@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 
+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