All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Add cpu model configuration support.. (resend)
@ 2010-02-01 19:02 ` john cooper
  0 siblings, 0 replies; 24+ messages in thread
From: john cooper @ 2010-02-01 19:02 UTC (permalink / raw)
  To: KVM list, qemu-devel
  Cc: john.cooper, Przywara, Andre, donald.d.dugger, Anthony Liguori,
	john.cooper

[target-x86_64.conf was unintentionally omitted from the earlier patch]

This is a reimplementation of prior versions which adds
the ability to define cpu models for contemporary processors.
The added models are likewise selected via -cpu <name>,
and are intended to displace the existing convention
of "-cpu qemu64" augmented with a series of feature flags.

A primary motivation was determination of a least common
denominator within a given processor class to simplify guest
migration.  It is still possible to modify an arbitrary model
via additional feature flags however the goal here was to
make doing so unnecessary in typical usage.  The other
consideration was providing models names reflective of
current processors.  Both AMD and Intel have reviewed the
models in terms of balancing generality of migration vs.
excessive feature downgrade relative to released silicon. 

This version of the patch replaces the prior hard wired
definitions with a configuration file approach for new
models.  Existing models are thus far left as-is but may
easily be transitioned to (or may be overridden by) the
configuration file representation.

Proposed new model definitions are provided here for current
AMD and Intel processors.  Each model consists of a name
used to select it on the command line (-cpu <name>), and a
model_id which corresponds to a least common denominator
commercial instance of the processor class.

A table of names/model_ids may be queried via "-cpu ?model":

        :
    x86       Opteron_G3  AMD Opteron 23xx (Gen 3 Class Opteron)          
    x86       Opteron_G2  AMD Opteron 22xx (Gen 2 Class Opteron)          
    x86       Opteron_G1  AMD Opteron 240 (Gen 1 Class Opteron)           
    x86          Nehalem  Intel Core i7 9xx (Nehalem Class Core i7)       
    x86           Penryn  Intel Core 2 Duo P9xxx (Penryn Class Core 2)    
    x86           Conroe  Intel Celeron_4x0 (Conroe/Merom Class Core 2)
        :         

Also added is "-cpu ?dump" which exhaustively outputs all config
data for all defined models, and "-cpu ?cpuid" which enumerates
all qemu recognized CPUID feature flags.

The pseudo cpuid flag 'check' when added to the feature flag list
will warn when feature flags (either implicit in a cpu model or
explicit on the command line) would have otherwise been quietly
unavailable to a guest:

    # qemu-system-x86_64 ... -cpu Nehalem,check
    warning: host cpuid 0000_0001 lacks requested flag 'sse4.2|sse4_2' [0x00100000]
    warning: host cpuid 0000_0001 lacks requested flag 'popcnt' [0x00800000]

A similar 'enforce' pseudo flag exists which in addition
to the above causes qemu to error exit if requested flags are
unavailable.

Configuration data for a cpu model resides in the target config
file which by default will be installed as:

    /usr/local/etc/qemu/target-<arch>.conf

The format of this file should be self explanatory given the
definitions for the above six models and essentially mimics
the structure of the static x86_def_t x86_defs.

Encoding of cpuid flags names now allows aliases for both the
configuration file and the command line which reconciles some
Intel/AMD/Linux/Qemu naming differences.

This patch was tested relative to qemu.git.

Signed-off-by: john cooper <john.cooper@redhat.com>
---

diff --git a/Makefile b/Makefile
index 3848627..b7fa6ef 100644
--- a/Makefile
+++ b/Makefile
@@ -191,7 +191,11 @@ ifdef CONFIG_POSIX
 	$(INSTALL_DATA) qemu-nbd.8 "$(DESTDIR)$(mandir)/man8"
 endif
 
-install: all $(if $(BUILD_DOCS),install-doc)
+install-sysconfig:
+	$(INSTALL_DIR) "$(sysconfdir)/qemu"
+	$(INSTALL_DATA) sysconfigs/target/target-x86_64.conf "$(sysconfdir)/qemu"
+
+install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig
 	$(INSTALL_DIR) "$(DESTDIR)$(bindir)"
 ifneq ($(TOOLS),)
 	$(INSTALL_PROG) $(STRIP_OPT) $(TOOLS) "$(DESTDIR)$(bindir)"
diff --git a/qemu-config.c b/qemu-config.c
index c3203c8..246fae6 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -242,6 +242,54 @@ QemuOptsList qemu_mon_opts = {
     },
 };
 
+QemuOptsList qemu_cpudef_opts = {
+    .name = "cpudef",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_cpudef_opts.head),
+    .desc = {
+        {
+            .name = "name",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "level",
+            .type = QEMU_OPT_NUMBER,
+        },{
+            .name = "vendor",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "family",
+            .type = QEMU_OPT_NUMBER,
+        },{
+            .name = "model",
+            .type = QEMU_OPT_NUMBER,
+        },{
+            .name = "stepping",
+            .type = QEMU_OPT_NUMBER,
+        },{
+            .name = "feature_edx",      /* cpuid 0000_0001.edx */
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "feature_ecx",      /* cpuid 0000_0001.ecx */
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "extfeature_edx",   /* cpuid 8000_0001.edx */
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "extfeature_ecx",   /* cpuid 8000_0001.ecx */
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "xlevel",
+            .type = QEMU_OPT_NUMBER,
+        },{
+            .name = "model_id",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "vendor_override",
+            .type = QEMU_OPT_NUMBER,
+        },
+        { /* end of list */ }
+    },
+};
+
 static QemuOptsList *lists[] = {
     &qemu_drive_opts,
     &qemu_chardev_opts,
@@ -251,6 +299,7 @@ static QemuOptsList *lists[] = {
     &qemu_rtc_opts,
     &qemu_global_opts,
     &qemu_mon_opts,
+    &qemu_cpudef_opts,
     NULL,
 };
 
diff --git a/qemu-config.h b/qemu-config.h
index dd89ae4..b335c42 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -9,6 +9,7 @@ extern QemuOptsList qemu_net_opts;
 extern QemuOptsList qemu_rtc_opts;
 extern QemuOptsList qemu_global_opts;
 extern QemuOptsList qemu_mon_opts;
+extern QemuOptsList qemu_cpudef_opts;
 
 int qemu_set_option(const char *str);
 int qemu_global_option(const char *str);
diff --git a/sysconfigs/target/target-x86_64.conf b/sysconfigs/target/target-x86_64.conf
new file mode 100644
index 0000000..43ad282
--- /dev/null
+++ b/sysconfigs/target/target-x86_64.conf
@@ -0,0 +1,86 @@
+# x86 CPU MODELS
+
+[cpudef]
+   name = "Conroe"
+   level = "2"
+   vendor = "GenuineIntel"
+   family = "6"
+   model = "2"
+   stepping = "3"
+   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
+   feature_ecx = "sse3 ssse3"
+   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx"
+   extfeature_ecx = "lahf_lm"
+   xlevel = "0x8000000A"
+   model_id = "Intel Celeron_4x0 (Conroe/Merom Class Core 2)"
+
+[cpudef]
+   name = "Penryn"
+   level = "2"
+   vendor = "GenuineIntel"
+   family = "6"
+   model = "2"
+   stepping = "3"
+   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
+   feature_ecx = "sse3 cx16 ssse3 sse4.1"
+   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx"
+   extfeature_ecx = "lahf_lm"
+   xlevel = "0x8000000A"
+   model_id = "Intel Core 2 Duo P9xxx (Penryn Class Core 2)"
+
+[cpudef]
+   name = "Nehalem"
+   level = "2"
+   vendor = "GenuineIntel"
+   family = "6"
+   model = "2"
+   stepping = "3"
+   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
+   feature_ecx = "sse3 cx16 ssse3 sse4.1 sse4.2 popcnt"
+   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx"
+   extfeature_ecx = "lahf_lm"
+   xlevel = "0x8000000A"
+   model_id = "Intel Core i7 9xx (Nehalem Class Core i7)"
+
+[cpudef]
+   name = "Opteron_G1"
+   level = "5"
+   vendor = "AuthenticAMD"
+   family = "15"
+   model = "6"
+   stepping = "1"
+   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
+   feature_ecx = "sse3"
+   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx"
+#   extfeature_ecx = ""
+   xlevel = "0x80000008"
+   model_id = "AMD Opteron 240 (Gen 1 Class Opteron)"
+
+[cpudef]
+   name = "Opteron_G2"
+   level = "5"
+   vendor = "AuthenticAMD"
+   family = "15"
+   model = "6"
+   stepping = "1"
+   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
+   feature_ecx = "sse3 cx16"
+   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx rdtscp"
+   extfeature_ecx = "svm lahf_lm"
+   xlevel = "0x80000008"
+   model_id = "AMD Opteron 22xx (Gen 2 Class Opteron)"
+
+[cpudef]
+   name = "Opteron_G3"
+   level = "5"
+   vendor = "AuthenticAMD"
+   family = "15"
+   model = "6"
+   stepping = "1"
+   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
+   feature_ecx = "sse3 cx16 monitor popcnt"
+   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx rdtscp"
+   extfeature_ecx = "svm sse4a  abm misalignsse lahf_lm"
+   xlevel = "0x80000008"
+   model_id = "AMD Opteron 23xx (Gen 3 Class Opteron)"
+
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 216b00e..c1a5256 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -723,8 +723,10 @@ typedef struct CPUX86State {
 CPUX86State *cpu_x86_init(const char *cpu_model);
 int cpu_x86_exec(CPUX86State *s);
 void cpu_x86_close(CPUX86State *s);
-void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt,
-                                                 ...));
+void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+                   const char *optarg);
+void cpudef_setup(void);
+
 int cpu_get_pic_interrupt(CPUX86State *s);
 /* MSDOS compatibility mode FPU exception support */
 void cpu_set_ferr(CPUX86State *s);
@@ -876,7 +878,7 @@ uint64_t cpu_get_tsc(CPUX86State *env);
 #define cpu_exec cpu_x86_exec
 #define cpu_gen_code cpu_x86_gen_code
 #define cpu_signal_handler cpu_x86_signal_handler
-#define cpu_list x86_cpu_list
+#define cpu_list_id x86_cpu_list
 
 #define CPU_SAVE_VERSION 11
 
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 70762bb..37dd2c6 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -29,33 +29,52 @@
 #include "kvm.h"
 
 //#define DEBUG_MMU
+#include "qemu-option.h"
+#include "qemu-config.h"
 
 /* feature flags taken from "Intel Processor Identification and the CPUID
- * Instruction" and AMD's "CPUID Specification". In cases of disagreement
- * about feature names, the Linux name is used. */
+ * Instruction" and AMD's "CPUID Specification".  In cases of disagreement
+ * between feature naming conventions, aliases may be added.
+ */
 static const char *feature_name[] = {
-    "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
-    "cx8", "apic", NULL, "sep", "mtrr", "pge", "mca", "cmov",
-    "pat", "pse36", "pn" /* Intel psn */, "clflush" /* Intel clfsh */, NULL, "ds" /* Intel dts */, "acpi", "mmx",
-    "fxsr", "sse", "sse2", "ss", "ht" /* Intel htt */, "tm", "ia64", "pbe",
+    "fpu", "vme", "de", "pse",
+    "tsc", "msr", "pae", "mce",
+    "cx8", "apic", NULL, "sep",
+    "mtrr", "pge", "mca", "cmov",
+    "pat", "pse36", "pn" /* Intel psn */, "clflush" /* Intel clfsh */,
+    NULL, "ds" /* Intel dts */, "acpi", "mmx",
+    "fxsr", "sse", "sse2", "ss",
+    "ht" /* Intel htt */, "tm", "ia64", "pbe",
 };
 static const char *ext_feature_name[] = {
-    "pni" /* Intel,AMD sse3 */, NULL, NULL, "monitor", "ds_cpl", "vmx", NULL /* Linux smx */, "est",
-    "tm2", "ssse3", "cid", NULL, NULL, "cx16", "xtpr", NULL,
-    NULL, NULL, "dca", NULL, NULL, NULL, NULL, "popcnt",
-    NULL, NULL, NULL, NULL, NULL, NULL, NULL, "hypervisor",
+    "pni|sse3" /* Intel,AMD sse3 */, NULL, NULL, "monitor",
+    "ds_cpl", "vmx", NULL /* Linux smx */, "est",
+    "tm2", "ssse3", "cid", NULL,
+    NULL, "cx16", "xtpr", NULL,
+    NULL, NULL, "dca", "sse4.1|sse4_1",
+    "sse4.2|sse4_2", "x2apic", NULL, "popcnt",
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, "hypervisor",
 };
 static const char *ext2_feature_name[] = {
-    "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
-    "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall", "mtrr", "pge", "mca", "cmov",
-    "pat", "pse36", NULL, NULL /* Linux mp */, "nx" /* Intel xd */, NULL, "mmxext", "mmx",
-    "fxsr", "fxsr_opt" /* AMD ffxsr */, "pdpe1gb" /* AMD Page1GB */, "rdtscp", NULL, "lm" /* Intel 64 */, "3dnowext", "3dnow",
+    "fpu", "vme", "de", "pse",
+    "tsc", "msr", "pae", "mce",
+    "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall",
+    "mtrr", "pge", "mca", "cmov",
+    "pat", "pse36", NULL, NULL /* Linux mp */,
+    "nx" /* Intel xd */, NULL, "mmxext", "mmx",
+    "fxsr", "fxsr_opt" /* AMD ffxsr */, "pdpe1gb" /* AMD Page1GB */, "rdtscp",
+    NULL, "lm" /* Intel 64 */, "3dnowext", "3dnow",
 };
 static const char *ext3_feature_name[] = {
-    "lahf_lm" /* AMD LahfSahf */, "cmp_legacy", "svm", "extapic" /* AMD ExtApicSpace */, "cr8legacy" /* AMD AltMovCr8 */, "abm", "sse4a", "misalignsse",
-    "3dnowprefetch", "osvw", NULL /* Linux ibs */, NULL, "skinit", "wdt", NULL, NULL,
-    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+    "lahf_lm" /* AMD LahfSahf */, "cmp_legacy", "svm", "extapic" /* AMD ExtApicSpace */,
+    "cr8legacy" /* AMD AltMovCr8 */, "abm", "sse4a", "misalignsse",
+    "3dnowprefetch", "osvw", NULL /* Linux ibs */, NULL,
+    "skinit", "wdt", NULL, NULL,
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL,
 };
 
 static const char *kvm_feature_name[] = {
@@ -65,47 +84,99 @@ static const char *kvm_feature_name[] = {
     NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 };
 
+/* collects per-function cpuid data
+ */
+typedef struct model_features_t {
+    uint32_t *guest_feat;
+    uint32_t *host_feat;
+    uint32_t check_feat;
+    const char **flag_names;
+    uint32_t cpuid;
+    } model_features_t;
+
+int check_cpuid = 0;
+int enforce_cpuid = 0;
+
+static void host_cpuid(uint32_t function, uint32_t count, uint32_t *eax,
+                       uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
+
+#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 success, otherwise return zero
+ */
+static int lookup_feature(uint32_t *pval, const char *s, const char *e,
+    const char **featureset)
+{
+    uint32_t mask;
+    const char **ppc;
+
+    for (mask = 1, ppc = featureset; mask; mask <<= 1, ++ppc)
+        if (*ppc && !altcmp(s, e, *ppc)) {
+            *pval |= mask;
+            break;
+        }
+    return (mask ? 1 : 0);
+}
+
 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)
 {
-    int i;
-    int found = 0;
-
-    for ( i = 0 ; i < 32 ; i++ )
-        if (feature_name[i] && !strcmp (flagname, feature_name[i])) {
-            *features |= 1 << i;
-            found = 1;
-        }
-    for ( i = 0 ; i < 32 ; i++ )
-        if (ext_feature_name[i] && !strcmp (flagname, ext_feature_name[i])) {
-            *ext_features |= 1 << i;
-            found = 1;
-        }
-    for ( i = 0 ; i < 32 ; i++ )
-        if (ext2_feature_name[i] && !strcmp (flagname, ext2_feature_name[i])) {
-            *ext2_features |= 1 << i;
-            found = 1;
-        }
-    for ( i = 0 ; i < 32 ; i++ )
-        if (ext3_feature_name[i] && !strcmp (flagname, ext3_feature_name[i])) {
-            *ext3_features |= 1 << i;
-            found = 1;
-        }
-    for ( i = 0 ; i < 32 ; i++ )
-        if (kvm_feature_name[i] && !strcmp (flagname, kvm_feature_name[i])) {
-            *kvm_features |= 1 << i;
-            found = 1;
-        }
-
-    if (!found) {
-        fprintf(stderr, "CPU feature %s not found\n", flagname);
-    }
+    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))
+            fprintf(stderr, "CPU feature %s not found\n", flagname);
 }
 
 typedef struct x86_def_t {
+    struct x86_def_t *next;
     const char *name;
     uint32_t level;
     uint32_t vendor1, vendor2, vendor3;
@@ -116,6 +187,7 @@ typedef struct x86_def_t {
     uint32_t xlevel;
     char model_id[48];
     int vendor_override;
+    uint32_t flags;
 } x86_def_t;
 
 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
@@ -129,7 +201,14 @@ typedef struct x86_def_t {
           CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
           CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
           CPUID_PAE | CPUID_SEP | CPUID_APIC)
-static x86_def_t x86_defs[] = {
+
+/* maintains list of cpu model definitions
+ */
+static x86_def_t *x86_defs = {NULL};
+
+/* built-in cpu model definitions (deprecated)
+ */
+static x86_def_t builtin_x86_defs[] = {
 #ifdef TARGET_X86_64
     {
         .name = "qemu64",
@@ -334,9 +413,6 @@ static x86_def_t x86_defs[] = {
     },
 };
 
-static void host_cpuid(uint32_t function, uint32_t count, uint32_t *eax,
-                               uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
-
 static int cpu_x86_fill_model_id(char *str)
 {
     uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
@@ -382,6 +458,51 @@ static int cpu_x86_fill_host(x86_def_t *x86_cpu_def)
     return 0;
 }
 
+static int unavailable_host_feature(struct model_features_t *f, uint32_t mask)
+{
+    int i;
+
+    for (i = 0; i < 32; ++i)
+        if (1 << i & mask) {
+            fprintf(stderr, "warning: host cpuid %04x_%04x lacks requested"
+                " flag '%s' [0x%08x]\n",
+                f->cpuid >> 16, f->cpuid & 0xffff,
+                f->flag_names[i] ? f->flag_names[i] : "[reserved]", mask);
+            break;
+        }
+    return 0;
+}
+
+/* best effort attempt to inform user requested cpu flags aren't making
+ * their way to the guest.  Note: ft[].check_feat ideally should be
+ * specified via a guest_def field to suppress report of extraneous flags.
+ */
+static int check_features_against_host(x86_def_t *guest_def)
+{
+    x86_def_t host_def;
+    uint32_t mask;
+    int rv, i;
+    struct model_features_t ft[] = {
+        {&guest_def->features, &host_def.features,
+            ~0, feature_name, 0x00000000},
+        {&guest_def->ext_features, &host_def.ext_features,
+            ~CPUID_EXT_HYPERVISOR, ext_feature_name, 0x00000001},
+        {&guest_def->ext2_features, &host_def.ext2_features,
+            ~PPRO_FEATURES, ext2_feature_name, 0x80000000},
+        {&guest_def->ext3_features, &host_def.ext3_features,
+            ~CPUID_EXT3_SVM, ext3_feature_name, 0x80000001}};
+
+    cpu_x86_fill_host(&host_def);
+    for (rv = 0, i = 0; i < sizeof (ft) / sizeof (ft[0]); ++i)
+        for (mask = 1; mask; mask <<= 1)
+            if (ft[i].check_feat & mask && *ft[i].guest_feat & mask &&
+                !(*ft[i].host_feat & mask)) {
+                    unavailable_host_feature(&ft[i], mask);
+                    rv = 1;
+                }
+    return rv;
+}
+
 static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
 {
     unsigned int i;
@@ -393,13 +514,9 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
     uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 0, minus_ext3_features = 0, minus_kvm_features = 0;
     uint32_t numvalue;
 
-    def = NULL;
-    for (i = 0; i < ARRAY_SIZE(x86_defs); i++) {
-        if (strcmp(name, x86_defs[i].name) == 0) {
-            def = &x86_defs[i];
+    for (def = x86_defs; def; def = def->next)
+        if (!strcmp(name, def->name))
             break;
-        }
-    }
     if (kvm_enabled() && strcmp(name, "host") == 0) {
         cpu_x86_fill_host(x86_cpu_def);
     } else if (!def) {
@@ -488,6 +605,10 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
                 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 {
             fprintf(stderr, "feature string `%s' not in format (+feature|-feature|feature=xyz)\n", featurestr);
             goto error;
@@ -504,6 +625,10 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
     x86_cpu_def->ext2_features &= ~minus_ext2_features;
     x86_cpu_def->ext3_features &= ~minus_ext3_features;
     x86_cpu_def->kvm_features &= ~minus_kvm_features;
+    if (check_cpuid) {
+        if (check_features_against_host(x86_cpu_def) && enforce_cpuid)
+            goto error;
+    }
     free(s);
     return 0;
 
@@ -512,12 +637,97 @@ error:
     return -1;
 }
 
-void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...))
+/* generate a composite string into buf of all cpuid names in featureset
+ * selected by fbits.  indicate truncation at bufsize in the event of overflow.
+ * if flags, suppress names undefined in featureset.
+ */
+static void listflags(char *buf, int bufsize, uint32_t fbits,
+    const char **featureset, uint32_t flags)
 {
-    unsigned int i;
+    const char **p = &featureset[31];
+    char *q, *b, bit;
+    int nc;
+
+    b = 4 <= bufsize ? buf + (bufsize -= 3) - 1 : NULL;
+    *buf = '\0';
+    for (q = buf, bit = 31; fbits && bufsize; --p, fbits &= ~(1 << bit), --bit)
+        if (fbits & 1 << bit && (*p || !flags)) {
+            if (*p)
+                nc = snprintf(q, bufsize, "%s%s", q == buf ? "" : " ", *p);
+            else
+                nc = snprintf(q, bufsize, "%s[%d]", q == buf ? "" : " ", bit);
+            if (bufsize <= nc) {
+                if (b)
+                    sprintf(b, "...");
+                return;
+            }
+            q += nc;
+            bufsize -= nc;
+        }
+}
 
-    for (i = 0; i < ARRAY_SIZE(x86_defs); i++)
-        (*cpu_fprintf)(f, "x86 %16s\n", x86_defs[i].name);
+/* generate CPU information:
+ * -?        list model names
+ * -?model   list model names/IDs
+ * -?dump    output all model (x86_def_t) data
+ * -?cpuid   list all recognized cpuid flag names
+ */ 
+void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+                  const char *optarg)
+{
+    unsigned char model = !strcmp("?model", optarg);
+    unsigned char dump = !strcmp("?dump", optarg);
+    unsigned char cpuid = !strcmp("?cpuid", optarg);
+    x86_def_t *def;
+    char buf[256];
+
+    if (cpuid) {
+        (*cpu_fprintf)(f, "Recognized CPUID flags:\n");
+        listflags(buf, sizeof (buf), (uint32_t)~0, feature_name, 1);
+        (*cpu_fprintf)(f, "  f_edx: %s\n", buf);
+        listflags(buf, sizeof (buf), (uint32_t)~0, ext_feature_name, 1);
+        (*cpu_fprintf)(f, "  f_ecx: %s\n", buf);
+        listflags(buf, sizeof (buf), (uint32_t)~0, ext2_feature_name, 1);
+        (*cpu_fprintf)(f, "  extf_edx: %s\n", buf);
+        listflags(buf, sizeof (buf), (uint32_t)~0, ext3_feature_name, 1);
+        (*cpu_fprintf)(f, "  extf_ecx: %s\n", buf);
+        return;
+    }
+    for (def = x86_defs; def; def = def->next) {
+        snprintf(buf, sizeof (buf), def->flags ? "[%s]": "%s", def->name);
+        if (model || dump) {
+            (*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf, def->model_id);
+        } else {
+            (*cpu_fprintf)(f, "x86 %16s\n", buf);
+        }
+        if (dump) {
+            memcpy(buf, &def->vendor1, sizeof (def->vendor1));
+            memcpy(buf + 4, &def->vendor2, sizeof (def->vendor2));
+            memcpy(buf + 8, &def->vendor3, sizeof (def->vendor3));
+            buf[12] = '\0';
+            (*cpu_fprintf)(f,
+                "  family %d model %d stepping %d level %d xlevel 0x%x"
+                " vendor \"%s\"\n",
+                def->family, def->model, def->stepping, def->level,
+                def->xlevel, buf);
+            listflags(buf, sizeof (buf), def->features, feature_name, 0);
+            (*cpu_fprintf)(f, "  feature_edx %08x (%s)\n", def->features,
+                buf);
+            listflags(buf, sizeof (buf), def->ext_features, ext_feature_name,
+                0);
+            (*cpu_fprintf)(f, "  feature_ecx %08x (%s)\n", def->ext_features,
+                buf);
+            listflags(buf, sizeof (buf), def->ext2_features, ext2_feature_name,
+                0);
+            (*cpu_fprintf)(f, "  extfeature_edx %08x (%s)\n",
+                def->ext2_features, buf);
+            listflags(buf, sizeof (buf), def->ext3_features, ext3_feature_name,
+                0);
+            (*cpu_fprintf)(f, "  extfeature_ecx %08x (%s)\n",
+                def->ext3_features, buf);
+            (*cpu_fprintf)(f, "\n");
+        }
+    }
 }
 
 static int cpu_x86_register (CPUX86State *env, const char *cpu_model)
@@ -566,6 +776,124 @@ static int cpu_x86_register (CPUX86State *env, const char *cpu_model)
     return 0;
 }
 
+/* copy vendor id string to 32 bit register, nul pad as needed
+ */
+static void cpyid(const char *s, uint32_t *id)
+{
+    char *d = (char *)id;
+    char i;
+
+    for (i = sizeof (*id); i--; )
+        *d++ = *s ? *s++ : '\0';
+}
+
+/* interpret radix and convert from string to arbitrary scalar,
+ * otherwise flag failure
+ */
+#define setscalar(pval, str, perr)                      \
+{                                                       \
+    char *pend;                                         \
+    unsigned long ul;                                   \
+                                                        \
+    ul = strtoul(str, &pend, 0);                        \
+    *str && !*pend ? (*pval = ul) : (*perr = 1);        \
+}
+
+/* map cpuid options to feature bits, otherwise return failure
+ * (option tags in *str are delimited by whitespace)
+ */
+static void setfeatures(uint32_t *pval, const char *str,
+    const char **featureset, int *perr)
+{
+    const char *p, *q;
+
+    for (q = p = str; *p || *q; q = p) {
+        while (iswhite(*p))
+            q = ++p; 
+        while (*p && !iswhite(*p))
+            ++p;
+        if (!*q && !*p)
+            return;
+        if (!lookup_feature(pval, q, p, featureset)) {
+            fprintf(stderr, "error: feature \"%.*s\" not available in set\n",
+                (int)(p - q), q);
+            *perr = 1;
+            return;
+        }
+    }
+}
+
+/* map config file options to x86_def_t form
+ */
+static int cpudef_setfield(const char *name, const char *str, void *opaque)
+{
+    x86_def_t *def = opaque;
+    int err = 0;
+
+    if (!strcmp(name, "name")) {
+        def->name = strdup(str);
+    } else if (!strcmp(name, "model_id")) {
+        strncpy(def->model_id, str, sizeof (def->model_id));
+    } else if (!strcmp(name, "level")) {
+        setscalar(&def->level, str, &err)
+    } else if (!strcmp(name, "vendor")) {
+        cpyid(&str[0], &def->vendor1);
+        cpyid(&str[4], &def->vendor2);
+        cpyid(&str[8], &def->vendor3);
+    } else if (!strcmp(name, "family")) {
+        setscalar(&def->family, str, &err)
+    } else if (!strcmp(name, "model")) {
+        setscalar(&def->model, str, &err)
+    } else if (!strcmp(name, "stepping")) {
+        setscalar(&def->stepping, str, &err)
+    } else if (!strcmp(name, "feature_edx")) {
+        setfeatures(&def->features, str, feature_name, &err);
+    } else if (!strcmp(name, "feature_ecx")) {
+        setfeatures(&def->ext_features, str, ext_feature_name, &err);
+    } else if (!strcmp(name, "extfeature_edx")) {
+        setfeatures(&def->ext2_features, str, ext2_feature_name, &err);
+    } else if (!strcmp(name, "extfeature_ecx")) {
+        setfeatures(&def->ext3_features, str, ext3_feature_name, &err);
+    } else if (!strcmp(name, "xlevel")) {
+        setscalar(&def->xlevel, str, &err)
+    } else {
+        fprintf(stderr, "error: unknown option [%s = %s]\n", name, str);
+        return (1);
+    }
+    if (err) {
+        fprintf(stderr, "error: bad option value [%s = %s]\n", name, str);
+        return (1);
+    }
+    return (0);
+}
+
+/* register config file entry as x86_def_t
+ */
+static int cpudef_register(QemuOpts *opts, void *opaque)
+{
+    x86_def_t *def = qemu_mallocz(sizeof (x86_def_t));
+
+    qemu_opt_foreach(opts, cpudef_setfield, def, 1);
+    def->next = x86_defs;
+    x86_defs = def;
+    return (0);
+}
+
+/* register "cpudef" models defined in configuration file after preloading
+ * built-in definitions
+ */
+void cpudef_setup(void)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); ++i) {
+        builtin_x86_defs[i].next = x86_defs;
+        builtin_x86_defs[i].flags = 1;
+        x86_defs = &builtin_x86_defs[i];
+    }
+    qemu_opts_foreach(&qemu_cpudef_opts, cpudef_register, NULL, 0);
+}
+
 /* NOTE: must be called outside the CPU execute loop */
 void cpu_reset(CPUX86State *env)
 {
diff --git a/vl.c b/vl.c
index 6f1e1ab..d6cd62c 100644
--- a/vl.c
+++ b/vl.c
@@ -4851,6 +4851,7 @@ int main(int argc, char **argv, char **envp)
             fclose(fp);
         }
     }
+    cpudef_setup();
 
     /* second pass of option parsing */
     optind = 1;
@@ -4884,8 +4885,10 @@ int main(int argc, char **argv, char **envp)
                 /* hw initialization will check this */
                 if (*optarg == '?') {
 /* XXX: implement xxx_cpu_list for targets that still miss it */
-#if defined(cpu_list)
-                    cpu_list(stdout, &fprintf);
+#if defined(cpu_list_id)
+                    cpu_list_id(stdout, &fprintf, optarg);
+#elif defined(cpu_list)
+                    cpu_list(stdout, &fprintf);	        /* deprecated */
 #endif
                     exit(0);
                 } else {
-- 
john.cooper@redhat.com

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH] Add cpu model configuration support.. (resend)
@ 2010-02-01 19:02 ` john cooper
  0 siblings, 0 replies; 24+ messages in thread
From: john cooper @ 2010-02-01 19:02 UTC (permalink / raw)
  To: KVM list, qemu-devel; +Cc: john.cooper, Przywara, Andre

[target-x86_64.conf was unintentionally omitted from the earlier patch]

This is a reimplementation of prior versions which adds
the ability to define cpu models for contemporary processors.
The added models are likewise selected via -cpu <name>,
and are intended to displace the existing convention
of "-cpu qemu64" augmented with a series of feature flags.

A primary motivation was determination of a least common
denominator within a given processor class to simplify guest
migration.  It is still possible to modify an arbitrary model
via additional feature flags however the goal here was to
make doing so unnecessary in typical usage.  The other
consideration was providing models names reflective of
current processors.  Both AMD and Intel have reviewed the
models in terms of balancing generality of migration vs.
excessive feature downgrade relative to released silicon. 

This version of the patch replaces the prior hard wired
definitions with a configuration file approach for new
models.  Existing models are thus far left as-is but may
easily be transitioned to (or may be overridden by) the
configuration file representation.

Proposed new model definitions are provided here for current
AMD and Intel processors.  Each model consists of a name
used to select it on the command line (-cpu <name>), and a
model_id which corresponds to a least common denominator
commercial instance of the processor class.

A table of names/model_ids may be queried via "-cpu ?model":

        :
    x86       Opteron_G3  AMD Opteron 23xx (Gen 3 Class Opteron)          
    x86       Opteron_G2  AMD Opteron 22xx (Gen 2 Class Opteron)          
    x86       Opteron_G1  AMD Opteron 240 (Gen 1 Class Opteron)           
    x86          Nehalem  Intel Core i7 9xx (Nehalem Class Core i7)       
    x86           Penryn  Intel Core 2 Duo P9xxx (Penryn Class Core 2)    
    x86           Conroe  Intel Celeron_4x0 (Conroe/Merom Class Core 2)
        :         

Also added is "-cpu ?dump" which exhaustively outputs all config
data for all defined models, and "-cpu ?cpuid" which enumerates
all qemu recognized CPUID feature flags.

The pseudo cpuid flag 'check' when added to the feature flag list
will warn when feature flags (either implicit in a cpu model or
explicit on the command line) would have otherwise been quietly
unavailable to a guest:

    # qemu-system-x86_64 ... -cpu Nehalem,check
    warning: host cpuid 0000_0001 lacks requested flag 'sse4.2|sse4_2' [0x00100000]
    warning: host cpuid 0000_0001 lacks requested flag 'popcnt' [0x00800000]

A similar 'enforce' pseudo flag exists which in addition
to the above causes qemu to error exit if requested flags are
unavailable.

Configuration data for a cpu model resides in the target config
file which by default will be installed as:

    /usr/local/etc/qemu/target-<arch>.conf

The format of this file should be self explanatory given the
definitions for the above six models and essentially mimics
the structure of the static x86_def_t x86_defs.

Encoding of cpuid flags names now allows aliases for both the
configuration file and the command line which reconciles some
Intel/AMD/Linux/Qemu naming differences.

This patch was tested relative to qemu.git.

Signed-off-by: john cooper <john.cooper@redhat.com>
---

diff --git a/Makefile b/Makefile
index 3848627..b7fa6ef 100644
--- a/Makefile
+++ b/Makefile
@@ -191,7 +191,11 @@ ifdef CONFIG_POSIX
 	$(INSTALL_DATA) qemu-nbd.8 "$(DESTDIR)$(mandir)/man8"
 endif
 
-install: all $(if $(BUILD_DOCS),install-doc)
+install-sysconfig:
+	$(INSTALL_DIR) "$(sysconfdir)/qemu"
+	$(INSTALL_DATA) sysconfigs/target/target-x86_64.conf "$(sysconfdir)/qemu"
+
+install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig
 	$(INSTALL_DIR) "$(DESTDIR)$(bindir)"
 ifneq ($(TOOLS),)
 	$(INSTALL_PROG) $(STRIP_OPT) $(TOOLS) "$(DESTDIR)$(bindir)"
diff --git a/qemu-config.c b/qemu-config.c
index c3203c8..246fae6 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -242,6 +242,54 @@ QemuOptsList qemu_mon_opts = {
     },
 };
 
+QemuOptsList qemu_cpudef_opts = {
+    .name = "cpudef",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_cpudef_opts.head),
+    .desc = {
+        {
+            .name = "name",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "level",
+            .type = QEMU_OPT_NUMBER,
+        },{
+            .name = "vendor",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "family",
+            .type = QEMU_OPT_NUMBER,
+        },{
+            .name = "model",
+            .type = QEMU_OPT_NUMBER,
+        },{
+            .name = "stepping",
+            .type = QEMU_OPT_NUMBER,
+        },{
+            .name = "feature_edx",      /* cpuid 0000_0001.edx */
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "feature_ecx",      /* cpuid 0000_0001.ecx */
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "extfeature_edx",   /* cpuid 8000_0001.edx */
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "extfeature_ecx",   /* cpuid 8000_0001.ecx */
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "xlevel",
+            .type = QEMU_OPT_NUMBER,
+        },{
+            .name = "model_id",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "vendor_override",
+            .type = QEMU_OPT_NUMBER,
+        },
+        { /* end of list */ }
+    },
+};
+
 static QemuOptsList *lists[] = {
     &qemu_drive_opts,
     &qemu_chardev_opts,
@@ -251,6 +299,7 @@ static QemuOptsList *lists[] = {
     &qemu_rtc_opts,
     &qemu_global_opts,
     &qemu_mon_opts,
+    &qemu_cpudef_opts,
     NULL,
 };
 
diff --git a/qemu-config.h b/qemu-config.h
index dd89ae4..b335c42 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -9,6 +9,7 @@ extern QemuOptsList qemu_net_opts;
 extern QemuOptsList qemu_rtc_opts;
 extern QemuOptsList qemu_global_opts;
 extern QemuOptsList qemu_mon_opts;
+extern QemuOptsList qemu_cpudef_opts;
 
 int qemu_set_option(const char *str);
 int qemu_global_option(const char *str);
diff --git a/sysconfigs/target/target-x86_64.conf b/sysconfigs/target/target-x86_64.conf
new file mode 100644
index 0000000..43ad282
--- /dev/null
+++ b/sysconfigs/target/target-x86_64.conf
@@ -0,0 +1,86 @@
+# x86 CPU MODELS
+
+[cpudef]
+   name = "Conroe"
+   level = "2"
+   vendor = "GenuineIntel"
+   family = "6"
+   model = "2"
+   stepping = "3"
+   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
+   feature_ecx = "sse3 ssse3"
+   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx"
+   extfeature_ecx = "lahf_lm"
+   xlevel = "0x8000000A"
+   model_id = "Intel Celeron_4x0 (Conroe/Merom Class Core 2)"
+
+[cpudef]
+   name = "Penryn"
+   level = "2"
+   vendor = "GenuineIntel"
+   family = "6"
+   model = "2"
+   stepping = "3"
+   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
+   feature_ecx = "sse3 cx16 ssse3 sse4.1"
+   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx"
+   extfeature_ecx = "lahf_lm"
+   xlevel = "0x8000000A"
+   model_id = "Intel Core 2 Duo P9xxx (Penryn Class Core 2)"
+
+[cpudef]
+   name = "Nehalem"
+   level = "2"
+   vendor = "GenuineIntel"
+   family = "6"
+   model = "2"
+   stepping = "3"
+   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
+   feature_ecx = "sse3 cx16 ssse3 sse4.1 sse4.2 popcnt"
+   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx"
+   extfeature_ecx = "lahf_lm"
+   xlevel = "0x8000000A"
+   model_id = "Intel Core i7 9xx (Nehalem Class Core i7)"
+
+[cpudef]
+   name = "Opteron_G1"
+   level = "5"
+   vendor = "AuthenticAMD"
+   family = "15"
+   model = "6"
+   stepping = "1"
+   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
+   feature_ecx = "sse3"
+   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx"
+#   extfeature_ecx = ""
+   xlevel = "0x80000008"
+   model_id = "AMD Opteron 240 (Gen 1 Class Opteron)"
+
+[cpudef]
+   name = "Opteron_G2"
+   level = "5"
+   vendor = "AuthenticAMD"
+   family = "15"
+   model = "6"
+   stepping = "1"
+   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
+   feature_ecx = "sse3 cx16"
+   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx rdtscp"
+   extfeature_ecx = "svm lahf_lm"
+   xlevel = "0x80000008"
+   model_id = "AMD Opteron 22xx (Gen 2 Class Opteron)"
+
+[cpudef]
+   name = "Opteron_G3"
+   level = "5"
+   vendor = "AuthenticAMD"
+   family = "15"
+   model = "6"
+   stepping = "1"
+   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
+   feature_ecx = "sse3 cx16 monitor popcnt"
+   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx rdtscp"
+   extfeature_ecx = "svm sse4a  abm misalignsse lahf_lm"
+   xlevel = "0x80000008"
+   model_id = "AMD Opteron 23xx (Gen 3 Class Opteron)"
+
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 216b00e..c1a5256 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -723,8 +723,10 @@ typedef struct CPUX86State {
 CPUX86State *cpu_x86_init(const char *cpu_model);
 int cpu_x86_exec(CPUX86State *s);
 void cpu_x86_close(CPUX86State *s);
-void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt,
-                                                 ...));
+void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+                   const char *optarg);
+void cpudef_setup(void);
+
 int cpu_get_pic_interrupt(CPUX86State *s);
 /* MSDOS compatibility mode FPU exception support */
 void cpu_set_ferr(CPUX86State *s);
@@ -876,7 +878,7 @@ uint64_t cpu_get_tsc(CPUX86State *env);
 #define cpu_exec cpu_x86_exec
 #define cpu_gen_code cpu_x86_gen_code
 #define cpu_signal_handler cpu_x86_signal_handler
-#define cpu_list x86_cpu_list
+#define cpu_list_id x86_cpu_list
 
 #define CPU_SAVE_VERSION 11
 
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 70762bb..37dd2c6 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -29,33 +29,52 @@
 #include "kvm.h"
 
 //#define DEBUG_MMU
+#include "qemu-option.h"
+#include "qemu-config.h"
 
 /* feature flags taken from "Intel Processor Identification and the CPUID
- * Instruction" and AMD's "CPUID Specification". In cases of disagreement
- * about feature names, the Linux name is used. */
+ * Instruction" and AMD's "CPUID Specification".  In cases of disagreement
+ * between feature naming conventions, aliases may be added.
+ */
 static const char *feature_name[] = {
-    "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
-    "cx8", "apic", NULL, "sep", "mtrr", "pge", "mca", "cmov",
-    "pat", "pse36", "pn" /* Intel psn */, "clflush" /* Intel clfsh */, NULL, "ds" /* Intel dts */, "acpi", "mmx",
-    "fxsr", "sse", "sse2", "ss", "ht" /* Intel htt */, "tm", "ia64", "pbe",
+    "fpu", "vme", "de", "pse",
+    "tsc", "msr", "pae", "mce",
+    "cx8", "apic", NULL, "sep",
+    "mtrr", "pge", "mca", "cmov",
+    "pat", "pse36", "pn" /* Intel psn */, "clflush" /* Intel clfsh */,
+    NULL, "ds" /* Intel dts */, "acpi", "mmx",
+    "fxsr", "sse", "sse2", "ss",
+    "ht" /* Intel htt */, "tm", "ia64", "pbe",
 };
 static const char *ext_feature_name[] = {
-    "pni" /* Intel,AMD sse3 */, NULL, NULL, "monitor", "ds_cpl", "vmx", NULL /* Linux smx */, "est",
-    "tm2", "ssse3", "cid", NULL, NULL, "cx16", "xtpr", NULL,
-    NULL, NULL, "dca", NULL, NULL, NULL, NULL, "popcnt",
-    NULL, NULL, NULL, NULL, NULL, NULL, NULL, "hypervisor",
+    "pni|sse3" /* Intel,AMD sse3 */, NULL, NULL, "monitor",
+    "ds_cpl", "vmx", NULL /* Linux smx */, "est",
+    "tm2", "ssse3", "cid", NULL,
+    NULL, "cx16", "xtpr", NULL,
+    NULL, NULL, "dca", "sse4.1|sse4_1",
+    "sse4.2|sse4_2", "x2apic", NULL, "popcnt",
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, "hypervisor",
 };
 static const char *ext2_feature_name[] = {
-    "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
-    "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall", "mtrr", "pge", "mca", "cmov",
-    "pat", "pse36", NULL, NULL /* Linux mp */, "nx" /* Intel xd */, NULL, "mmxext", "mmx",
-    "fxsr", "fxsr_opt" /* AMD ffxsr */, "pdpe1gb" /* AMD Page1GB */, "rdtscp", NULL, "lm" /* Intel 64 */, "3dnowext", "3dnow",
+    "fpu", "vme", "de", "pse",
+    "tsc", "msr", "pae", "mce",
+    "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall",
+    "mtrr", "pge", "mca", "cmov",
+    "pat", "pse36", NULL, NULL /* Linux mp */,
+    "nx" /* Intel xd */, NULL, "mmxext", "mmx",
+    "fxsr", "fxsr_opt" /* AMD ffxsr */, "pdpe1gb" /* AMD Page1GB */, "rdtscp",
+    NULL, "lm" /* Intel 64 */, "3dnowext", "3dnow",
 };
 static const char *ext3_feature_name[] = {
-    "lahf_lm" /* AMD LahfSahf */, "cmp_legacy", "svm", "extapic" /* AMD ExtApicSpace */, "cr8legacy" /* AMD AltMovCr8 */, "abm", "sse4a", "misalignsse",
-    "3dnowprefetch", "osvw", NULL /* Linux ibs */, NULL, "skinit", "wdt", NULL, NULL,
-    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+    "lahf_lm" /* AMD LahfSahf */, "cmp_legacy", "svm", "extapic" /* AMD ExtApicSpace */,
+    "cr8legacy" /* AMD AltMovCr8 */, "abm", "sse4a", "misalignsse",
+    "3dnowprefetch", "osvw", NULL /* Linux ibs */, NULL,
+    "skinit", "wdt", NULL, NULL,
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL,
 };
 
 static const char *kvm_feature_name[] = {
@@ -65,47 +84,99 @@ static const char *kvm_feature_name[] = {
     NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 };
 
+/* collects per-function cpuid data
+ */
+typedef struct model_features_t {
+    uint32_t *guest_feat;
+    uint32_t *host_feat;
+    uint32_t check_feat;
+    const char **flag_names;
+    uint32_t cpuid;
+    } model_features_t;
+
+int check_cpuid = 0;
+int enforce_cpuid = 0;
+
+static void host_cpuid(uint32_t function, uint32_t count, uint32_t *eax,
+                       uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
+
+#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 success, otherwise return zero
+ */
+static int lookup_feature(uint32_t *pval, const char *s, const char *e,
+    const char **featureset)
+{
+    uint32_t mask;
+    const char **ppc;
+
+    for (mask = 1, ppc = featureset; mask; mask <<= 1, ++ppc)
+        if (*ppc && !altcmp(s, e, *ppc)) {
+            *pval |= mask;
+            break;
+        }
+    return (mask ? 1 : 0);
+}
+
 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)
 {
-    int i;
-    int found = 0;
-
-    for ( i = 0 ; i < 32 ; i++ )
-        if (feature_name[i] && !strcmp (flagname, feature_name[i])) {
-            *features |= 1 << i;
-            found = 1;
-        }
-    for ( i = 0 ; i < 32 ; i++ )
-        if (ext_feature_name[i] && !strcmp (flagname, ext_feature_name[i])) {
-            *ext_features |= 1 << i;
-            found = 1;
-        }
-    for ( i = 0 ; i < 32 ; i++ )
-        if (ext2_feature_name[i] && !strcmp (flagname, ext2_feature_name[i])) {
-            *ext2_features |= 1 << i;
-            found = 1;
-        }
-    for ( i = 0 ; i < 32 ; i++ )
-        if (ext3_feature_name[i] && !strcmp (flagname, ext3_feature_name[i])) {
-            *ext3_features |= 1 << i;
-            found = 1;
-        }
-    for ( i = 0 ; i < 32 ; i++ )
-        if (kvm_feature_name[i] && !strcmp (flagname, kvm_feature_name[i])) {
-            *kvm_features |= 1 << i;
-            found = 1;
-        }
-
-    if (!found) {
-        fprintf(stderr, "CPU feature %s not found\n", flagname);
-    }
+    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))
+            fprintf(stderr, "CPU feature %s not found\n", flagname);
 }
 
 typedef struct x86_def_t {
+    struct x86_def_t *next;
     const char *name;
     uint32_t level;
     uint32_t vendor1, vendor2, vendor3;
@@ -116,6 +187,7 @@ typedef struct x86_def_t {
     uint32_t xlevel;
     char model_id[48];
     int vendor_override;
+    uint32_t flags;
 } x86_def_t;
 
 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
@@ -129,7 +201,14 @@ typedef struct x86_def_t {
           CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
           CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
           CPUID_PAE | CPUID_SEP | CPUID_APIC)
-static x86_def_t x86_defs[] = {
+
+/* maintains list of cpu model definitions
+ */
+static x86_def_t *x86_defs = {NULL};
+
+/* built-in cpu model definitions (deprecated)
+ */
+static x86_def_t builtin_x86_defs[] = {
 #ifdef TARGET_X86_64
     {
         .name = "qemu64",
@@ -334,9 +413,6 @@ static x86_def_t x86_defs[] = {
     },
 };
 
-static void host_cpuid(uint32_t function, uint32_t count, uint32_t *eax,
-                               uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
-
 static int cpu_x86_fill_model_id(char *str)
 {
     uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
@@ -382,6 +458,51 @@ static int cpu_x86_fill_host(x86_def_t *x86_cpu_def)
     return 0;
 }
 
+static int unavailable_host_feature(struct model_features_t *f, uint32_t mask)
+{
+    int i;
+
+    for (i = 0; i < 32; ++i)
+        if (1 << i & mask) {
+            fprintf(stderr, "warning: host cpuid %04x_%04x lacks requested"
+                " flag '%s' [0x%08x]\n",
+                f->cpuid >> 16, f->cpuid & 0xffff,
+                f->flag_names[i] ? f->flag_names[i] : "[reserved]", mask);
+            break;
+        }
+    return 0;
+}
+
+/* best effort attempt to inform user requested cpu flags aren't making
+ * their way to the guest.  Note: ft[].check_feat ideally should be
+ * specified via a guest_def field to suppress report of extraneous flags.
+ */
+static int check_features_against_host(x86_def_t *guest_def)
+{
+    x86_def_t host_def;
+    uint32_t mask;
+    int rv, i;
+    struct model_features_t ft[] = {
+        {&guest_def->features, &host_def.features,
+            ~0, feature_name, 0x00000000},
+        {&guest_def->ext_features, &host_def.ext_features,
+            ~CPUID_EXT_HYPERVISOR, ext_feature_name, 0x00000001},
+        {&guest_def->ext2_features, &host_def.ext2_features,
+            ~PPRO_FEATURES, ext2_feature_name, 0x80000000},
+        {&guest_def->ext3_features, &host_def.ext3_features,
+            ~CPUID_EXT3_SVM, ext3_feature_name, 0x80000001}};
+
+    cpu_x86_fill_host(&host_def);
+    for (rv = 0, i = 0; i < sizeof (ft) / sizeof (ft[0]); ++i)
+        for (mask = 1; mask; mask <<= 1)
+            if (ft[i].check_feat & mask && *ft[i].guest_feat & mask &&
+                !(*ft[i].host_feat & mask)) {
+                    unavailable_host_feature(&ft[i], mask);
+                    rv = 1;
+                }
+    return rv;
+}
+
 static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
 {
     unsigned int i;
@@ -393,13 +514,9 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
     uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 0, minus_ext3_features = 0, minus_kvm_features = 0;
     uint32_t numvalue;
 
-    def = NULL;
-    for (i = 0; i < ARRAY_SIZE(x86_defs); i++) {
-        if (strcmp(name, x86_defs[i].name) == 0) {
-            def = &x86_defs[i];
+    for (def = x86_defs; def; def = def->next)
+        if (!strcmp(name, def->name))
             break;
-        }
-    }
     if (kvm_enabled() && strcmp(name, "host") == 0) {
         cpu_x86_fill_host(x86_cpu_def);
     } else if (!def) {
@@ -488,6 +605,10 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
                 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 {
             fprintf(stderr, "feature string `%s' not in format (+feature|-feature|feature=xyz)\n", featurestr);
             goto error;
@@ -504,6 +625,10 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
     x86_cpu_def->ext2_features &= ~minus_ext2_features;
     x86_cpu_def->ext3_features &= ~minus_ext3_features;
     x86_cpu_def->kvm_features &= ~minus_kvm_features;
+    if (check_cpuid) {
+        if (check_features_against_host(x86_cpu_def) && enforce_cpuid)
+            goto error;
+    }
     free(s);
     return 0;
 
@@ -512,12 +637,97 @@ error:
     return -1;
 }
 
-void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...))
+/* generate a composite string into buf of all cpuid names in featureset
+ * selected by fbits.  indicate truncation at bufsize in the event of overflow.
+ * if flags, suppress names undefined in featureset.
+ */
+static void listflags(char *buf, int bufsize, uint32_t fbits,
+    const char **featureset, uint32_t flags)
 {
-    unsigned int i;
+    const char **p = &featureset[31];
+    char *q, *b, bit;
+    int nc;
+
+    b = 4 <= bufsize ? buf + (bufsize -= 3) - 1 : NULL;
+    *buf = '\0';
+    for (q = buf, bit = 31; fbits && bufsize; --p, fbits &= ~(1 << bit), --bit)
+        if (fbits & 1 << bit && (*p || !flags)) {
+            if (*p)
+                nc = snprintf(q, bufsize, "%s%s", q == buf ? "" : " ", *p);
+            else
+                nc = snprintf(q, bufsize, "%s[%d]", q == buf ? "" : " ", bit);
+            if (bufsize <= nc) {
+                if (b)
+                    sprintf(b, "...");
+                return;
+            }
+            q += nc;
+            bufsize -= nc;
+        }
+}
 
-    for (i = 0; i < ARRAY_SIZE(x86_defs); i++)
-        (*cpu_fprintf)(f, "x86 %16s\n", x86_defs[i].name);
+/* generate CPU information:
+ * -?        list model names
+ * -?model   list model names/IDs
+ * -?dump    output all model (x86_def_t) data
+ * -?cpuid   list all recognized cpuid flag names
+ */ 
+void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+                  const char *optarg)
+{
+    unsigned char model = !strcmp("?model", optarg);
+    unsigned char dump = !strcmp("?dump", optarg);
+    unsigned char cpuid = !strcmp("?cpuid", optarg);
+    x86_def_t *def;
+    char buf[256];
+
+    if (cpuid) {
+        (*cpu_fprintf)(f, "Recognized CPUID flags:\n");
+        listflags(buf, sizeof (buf), (uint32_t)~0, feature_name, 1);
+        (*cpu_fprintf)(f, "  f_edx: %s\n", buf);
+        listflags(buf, sizeof (buf), (uint32_t)~0, ext_feature_name, 1);
+        (*cpu_fprintf)(f, "  f_ecx: %s\n", buf);
+        listflags(buf, sizeof (buf), (uint32_t)~0, ext2_feature_name, 1);
+        (*cpu_fprintf)(f, "  extf_edx: %s\n", buf);
+        listflags(buf, sizeof (buf), (uint32_t)~0, ext3_feature_name, 1);
+        (*cpu_fprintf)(f, "  extf_ecx: %s\n", buf);
+        return;
+    }
+    for (def = x86_defs; def; def = def->next) {
+        snprintf(buf, sizeof (buf), def->flags ? "[%s]": "%s", def->name);
+        if (model || dump) {
+            (*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf, def->model_id);
+        } else {
+            (*cpu_fprintf)(f, "x86 %16s\n", buf);
+        }
+        if (dump) {
+            memcpy(buf, &def->vendor1, sizeof (def->vendor1));
+            memcpy(buf + 4, &def->vendor2, sizeof (def->vendor2));
+            memcpy(buf + 8, &def->vendor3, sizeof (def->vendor3));
+            buf[12] = '\0';
+            (*cpu_fprintf)(f,
+                "  family %d model %d stepping %d level %d xlevel 0x%x"
+                " vendor \"%s\"\n",
+                def->family, def->model, def->stepping, def->level,
+                def->xlevel, buf);
+            listflags(buf, sizeof (buf), def->features, feature_name, 0);
+            (*cpu_fprintf)(f, "  feature_edx %08x (%s)\n", def->features,
+                buf);
+            listflags(buf, sizeof (buf), def->ext_features, ext_feature_name,
+                0);
+            (*cpu_fprintf)(f, "  feature_ecx %08x (%s)\n", def->ext_features,
+                buf);
+            listflags(buf, sizeof (buf), def->ext2_features, ext2_feature_name,
+                0);
+            (*cpu_fprintf)(f, "  extfeature_edx %08x (%s)\n",
+                def->ext2_features, buf);
+            listflags(buf, sizeof (buf), def->ext3_features, ext3_feature_name,
+                0);
+            (*cpu_fprintf)(f, "  extfeature_ecx %08x (%s)\n",
+                def->ext3_features, buf);
+            (*cpu_fprintf)(f, "\n");
+        }
+    }
 }
 
 static int cpu_x86_register (CPUX86State *env, const char *cpu_model)
@@ -566,6 +776,124 @@ static int cpu_x86_register (CPUX86State *env, const char *cpu_model)
     return 0;
 }
 
+/* copy vendor id string to 32 bit register, nul pad as needed
+ */
+static void cpyid(const char *s, uint32_t *id)
+{
+    char *d = (char *)id;
+    char i;
+
+    for (i = sizeof (*id); i--; )
+        *d++ = *s ? *s++ : '\0';
+}
+
+/* interpret radix and convert from string to arbitrary scalar,
+ * otherwise flag failure
+ */
+#define setscalar(pval, str, perr)                      \
+{                                                       \
+    char *pend;                                         \
+    unsigned long ul;                                   \
+                                                        \
+    ul = strtoul(str, &pend, 0);                        \
+    *str && !*pend ? (*pval = ul) : (*perr = 1);        \
+}
+
+/* map cpuid options to feature bits, otherwise return failure
+ * (option tags in *str are delimited by whitespace)
+ */
+static void setfeatures(uint32_t *pval, const char *str,
+    const char **featureset, int *perr)
+{
+    const char *p, *q;
+
+    for (q = p = str; *p || *q; q = p) {
+        while (iswhite(*p))
+            q = ++p; 
+        while (*p && !iswhite(*p))
+            ++p;
+        if (!*q && !*p)
+            return;
+        if (!lookup_feature(pval, q, p, featureset)) {
+            fprintf(stderr, "error: feature \"%.*s\" not available in set\n",
+                (int)(p - q), q);
+            *perr = 1;
+            return;
+        }
+    }
+}
+
+/* map config file options to x86_def_t form
+ */
+static int cpudef_setfield(const char *name, const char *str, void *opaque)
+{
+    x86_def_t *def = opaque;
+    int err = 0;
+
+    if (!strcmp(name, "name")) {
+        def->name = strdup(str);
+    } else if (!strcmp(name, "model_id")) {
+        strncpy(def->model_id, str, sizeof (def->model_id));
+    } else if (!strcmp(name, "level")) {
+        setscalar(&def->level, str, &err)
+    } else if (!strcmp(name, "vendor")) {
+        cpyid(&str[0], &def->vendor1);
+        cpyid(&str[4], &def->vendor2);
+        cpyid(&str[8], &def->vendor3);
+    } else if (!strcmp(name, "family")) {
+        setscalar(&def->family, str, &err)
+    } else if (!strcmp(name, "model")) {
+        setscalar(&def->model, str, &err)
+    } else if (!strcmp(name, "stepping")) {
+        setscalar(&def->stepping, str, &err)
+    } else if (!strcmp(name, "feature_edx")) {
+        setfeatures(&def->features, str, feature_name, &err);
+    } else if (!strcmp(name, "feature_ecx")) {
+        setfeatures(&def->ext_features, str, ext_feature_name, &err);
+    } else if (!strcmp(name, "extfeature_edx")) {
+        setfeatures(&def->ext2_features, str, ext2_feature_name, &err);
+    } else if (!strcmp(name, "extfeature_ecx")) {
+        setfeatures(&def->ext3_features, str, ext3_feature_name, &err);
+    } else if (!strcmp(name, "xlevel")) {
+        setscalar(&def->xlevel, str, &err)
+    } else {
+        fprintf(stderr, "error: unknown option [%s = %s]\n", name, str);
+        return (1);
+    }
+    if (err) {
+        fprintf(stderr, "error: bad option value [%s = %s]\n", name, str);
+        return (1);
+    }
+    return (0);
+}
+
+/* register config file entry as x86_def_t
+ */
+static int cpudef_register(QemuOpts *opts, void *opaque)
+{
+    x86_def_t *def = qemu_mallocz(sizeof (x86_def_t));
+
+    qemu_opt_foreach(opts, cpudef_setfield, def, 1);
+    def->next = x86_defs;
+    x86_defs = def;
+    return (0);
+}
+
+/* register "cpudef" models defined in configuration file after preloading
+ * built-in definitions
+ */
+void cpudef_setup(void)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); ++i) {
+        builtin_x86_defs[i].next = x86_defs;
+        builtin_x86_defs[i].flags = 1;
+        x86_defs = &builtin_x86_defs[i];
+    }
+    qemu_opts_foreach(&qemu_cpudef_opts, cpudef_register, NULL, 0);
+}
+
 /* NOTE: must be called outside the CPU execute loop */
 void cpu_reset(CPUX86State *env)
 {
diff --git a/vl.c b/vl.c
index 6f1e1ab..d6cd62c 100644
--- a/vl.c
+++ b/vl.c
@@ -4851,6 +4851,7 @@ int main(int argc, char **argv, char **envp)
             fclose(fp);
         }
     }
+    cpudef_setup();
 
     /* second pass of option parsing */
     optind = 1;
@@ -4884,8 +4885,10 @@ int main(int argc, char **argv, char **envp)
                 /* hw initialization will check this */
                 if (*optarg == '?') {
 /* XXX: implement xxx_cpu_list for targets that still miss it */
-#if defined(cpu_list)
-                    cpu_list(stdout, &fprintf);
+#if defined(cpu_list_id)
+                    cpu_list_id(stdout, &fprintf, optarg);
+#elif defined(cpu_list)
+                    cpu_list(stdout, &fprintf);	        /* deprecated */
 #endif
                     exit(0);
                 } else {
-- 
john.cooper@redhat.com

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH] Add cpu model configuration support.. (resend)
  2010-02-01 19:02 ` [Qemu-devel] " john cooper
@ 2010-02-02 10:17   ` Andre Przywara
  -1 siblings, 0 replies; 24+ messages in thread
From: Andre Przywara @ 2010-02-02 10:17 UTC (permalink / raw)
  To: john cooper; +Cc: KVM list, qemu-devel, donald.d.dugger, Anthony Liguori

john cooper wrote:

> [target-x86_64.conf was unintentionally omitted from the earlier patch]
> 
> This is a reimplementation of prior versions which adds
> the ability to define cpu models for contemporary processors.
> The added models are likewise selected via -cpu <name>,
> and are intended to displace the existing convention
> of "-cpu qemu64" augmented with a series of feature flags.
 > ...
John,

first I would like to apologize for sending out my patch series although 
I know that it heavily conflicts with yours. Actually you beat me just 
by hours with yours, I had mine ready on Friday evening and just delayed 
the sending until Monday ;-)

Can you split up the patch into a series of smaller ones (maybe git add 
-i can help you here?). This version is a bit large for proper review 
and mixes fixes and feature additions. Additionally this would help to 
merge our both versions.

Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448 3567 12
----to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd; Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Qemu-devel] Re: [PATCH] Add cpu model configuration support.. (resend)
@ 2010-02-02 10:17   ` Andre Przywara
  0 siblings, 0 replies; 24+ messages in thread
From: Andre Przywara @ 2010-02-02 10:17 UTC (permalink / raw)
  To: john cooper; +Cc: qemu-devel, KVM list

john cooper wrote:

> [target-x86_64.conf was unintentionally omitted from the earlier patch]
> 
> This is a reimplementation of prior versions which adds
> the ability to define cpu models for contemporary processors.
> The added models are likewise selected via -cpu <name>,
> and are intended to displace the existing convention
> of "-cpu qemu64" augmented with a series of feature flags.
 > ...
John,

first I would like to apologize for sending out my patch series although 
I know that it heavily conflicts with yours. Actually you beat me just 
by hours with yours, I had mine ready on Friday evening and just delayed 
the sending until Monday ;-)

Can you split up the patch into a series of smaller ones (maybe git add 
-i can help you here?). This version is a bit large for proper review 
and mixes fixes and feature additions. Additionally this would help to 
merge our both versions.

Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448 3567 12
----to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd; Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Add cpu model configuration support.. (resend)
  2010-02-01 19:02 ` [Qemu-devel] " john cooper
@ 2010-02-02 11:07   ` Andre Przywara
  -1 siblings, 0 replies; 24+ messages in thread
From: Andre Przywara @ 2010-02-02 11:07 UTC (permalink / raw)
  To: john cooper; +Cc: KVM list, qemu-devel, donald.d.dugger, Anthony Liguori

John,

just two comments from skimming through the patch:

> diff --git a/sysconfigs/target/target-x86_64.conf b/sysconfigs/target/target-x86_64.conf
> new file mode 100644
> index 0000000..43ad282
> --- /dev/null
> +++ b/sysconfigs/target/target-x86_64.conf
> @@ -0,0 +1,86 @@
> +# x86 CPU MODELS
> +
> +[cpudef]
> +   name = "Conroe"
> +   level = "2"
> +   vendor = "GenuineIntel"
> +   family = "6"
> +   model = "2"
> +   stepping = "3"
> +   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
> +   feature_ecx = "sse3 ssse3"
> +   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx"
> +   extfeature_ecx = "lahf_lm"
Wouldn't it be much more user friendly to merge them all into one 
string? Just from the feature names it is quite obscure to guess which 
flag belongs into which string (especially since we lack the EXTn_ 
prefix we had in helper.c). I haven't tried it, but the parsing code 
looks like this shouldn't be too hard.
To avoid overlong lines one could think about a += operator.

> @@ -129,7 +201,14 @@ typedef struct x86_def_t {
>            CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
>            CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
>            CPUID_PAE | CPUID_SEP | CPUID_APIC)
> -static x86_def_t x86_defs[] = {
> +
> +/* maintains list of cpu model definitions
> + */
> +static x86_def_t *x86_defs = {NULL};
> +
> +/* built-in cpu model definitions (deprecated)
> + */
> +static x86_def_t builtin_x86_defs[] = {
>  #ifdef TARGET_X86_64
>      {
>          .name = "qemu64",

I would just drop all definitions here except qemu{32,64} and 
kvm{32,64}. The other models should be described in the config file.

Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448 3567 12
----to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd; Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Qemu-devel] Re: [PATCH] Add cpu model configuration support.. (resend)
@ 2010-02-02 11:07   ` Andre Przywara
  0 siblings, 0 replies; 24+ messages in thread
From: Andre Przywara @ 2010-02-02 11:07 UTC (permalink / raw)
  To: john cooper; +Cc: qemu-devel, KVM list

John,

just two comments from skimming through the patch:

> diff --git a/sysconfigs/target/target-x86_64.conf b/sysconfigs/target/target-x86_64.conf
> new file mode 100644
> index 0000000..43ad282
> --- /dev/null
> +++ b/sysconfigs/target/target-x86_64.conf
> @@ -0,0 +1,86 @@
> +# x86 CPU MODELS
> +
> +[cpudef]
> +   name = "Conroe"
> +   level = "2"
> +   vendor = "GenuineIntel"
> +   family = "6"
> +   model = "2"
> +   stepping = "3"
> +   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
> +   feature_ecx = "sse3 ssse3"
> +   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx"
> +   extfeature_ecx = "lahf_lm"
Wouldn't it be much more user friendly to merge them all into one 
string? Just from the feature names it is quite obscure to guess which 
flag belongs into which string (especially since we lack the EXTn_ 
prefix we had in helper.c). I haven't tried it, but the parsing code 
looks like this shouldn't be too hard.
To avoid overlong lines one could think about a += operator.

> @@ -129,7 +201,14 @@ typedef struct x86_def_t {
>            CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
>            CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
>            CPUID_PAE | CPUID_SEP | CPUID_APIC)
> -static x86_def_t x86_defs[] = {
> +
> +/* maintains list of cpu model definitions
> + */
> +static x86_def_t *x86_defs = {NULL};
> +
> +/* built-in cpu model definitions (deprecated)
> + */
> +static x86_def_t builtin_x86_defs[] = {
>  #ifdef TARGET_X86_64
>      {
>          .name = "qemu64",

I would just drop all definitions here except qemu{32,64} and 
kvm{32,64}. The other models should be described in the config file.

Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448 3567 12
----to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd; Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Add cpu model configuration support.. (resend)
  2010-02-02 11:07   ` [Qemu-devel] " Andre Przywara
@ 2010-02-02 19:34     ` john cooper
  -1 siblings, 0 replies; 24+ messages in thread
From: john cooper @ 2010-02-02 19:34 UTC (permalink / raw)
  To: Andre Przywara
  Cc: KVM list, qemu-devel, donald.d.dugger, Anthony Liguori, john.cooper

Andre Przywara wrote:

>> +[cpudef]
>> +   name = "Conroe"
>> +   level = "2"
>> +   vendor = "GenuineIntel"
>> +   family = "6"
>> +   model = "2"
>> +   stepping = "3"
>> +   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae
>> msr tsc pse de fpu    mtrr clflush mca pse36"
>> +   feature_ecx = "sse3 ssse3"
>> +   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc
>> pse de fpu    lm syscall nx"
>> +   extfeature_ecx = "lahf_lm"
> Wouldn't it be much more user friendly to merge them all into one
> string? Just from the feature names it is quite obscure to guess which
> flag belongs into which string (especially since we lack the EXTn_
> prefix we had in helper.c). I haven't tried it, but the parsing code
> looks like this shouldn't be too hard.
> To avoid overlong lines one could think about a += operator.

That's true.  Although I expect setup of a cpu model to
be a rather infrequent occurrence by the expert (+/-)
user so the above didn't strike me as a significant issue.
Also "-cpu ?cpuid" dumps out the entire motley crew of
flags relative to each grouping for reference.

That said the current config file syntax seems rather
rigid and I think your suggestion makes sense.  I avoided
modifying the parser at this point just in the interest of
minimizing the sprawl of this patch.

> I would just drop all definitions here except qemu{32,64} and
> kvm{32,64}. The other models should be described in the config file.

That's the goal but I wanted to leave an interim firewall
of sorts.  If the target-x86_64.conf isn't installed for
whatever reason, qemu still can fall back to the internal
definitions.  Even here it isn't strictly necessary to
remove an internal def as it can be redefined in the
config file which will override the internal version.
In general -cpu "?model" will indicate internal vs.
externally defined models by enclosing internal model names
in brackets:

    :
x86       Opteron_G3  AMD Opteron 23xx (Gen 3 Class Opteron)
    :
x86         [athlon]  QEMU Virtual CPU version 0.12.50
    :

It also seems worth dropping a hint to the user in the case qemu
fails to find a target config file rather than leaving them to
puzzle out why an external model has gone missing.

Thanks for the feedback.

-john

-- 
john.cooper@redhat.com

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Qemu-devel] Re: [PATCH] Add cpu model configuration support.. (resend)
@ 2010-02-02 19:34     ` john cooper
  0 siblings, 0 replies; 24+ messages in thread
From: john cooper @ 2010-02-02 19:34 UTC (permalink / raw)
  To: Andre Przywara; +Cc: john.cooper, qemu-devel, KVM list

Andre Przywara wrote:

>> +[cpudef]
>> +   name = "Conroe"
>> +   level = "2"
>> +   vendor = "GenuineIntel"
>> +   family = "6"
>> +   model = "2"
>> +   stepping = "3"
>> +   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae
>> msr tsc pse de fpu    mtrr clflush mca pse36"
>> +   feature_ecx = "sse3 ssse3"
>> +   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc
>> pse de fpu    lm syscall nx"
>> +   extfeature_ecx = "lahf_lm"
> Wouldn't it be much more user friendly to merge them all into one
> string? Just from the feature names it is quite obscure to guess which
> flag belongs into which string (especially since we lack the EXTn_
> prefix we had in helper.c). I haven't tried it, but the parsing code
> looks like this shouldn't be too hard.
> To avoid overlong lines one could think about a += operator.

That's true.  Although I expect setup of a cpu model to
be a rather infrequent occurrence by the expert (+/-)
user so the above didn't strike me as a significant issue.
Also "-cpu ?cpuid" dumps out the entire motley crew of
flags relative to each grouping for reference.

That said the current config file syntax seems rather
rigid and I think your suggestion makes sense.  I avoided
modifying the parser at this point just in the interest of
minimizing the sprawl of this patch.

> I would just drop all definitions here except qemu{32,64} and
> kvm{32,64}. The other models should be described in the config file.

That's the goal but I wanted to leave an interim firewall
of sorts.  If the target-x86_64.conf isn't installed for
whatever reason, qemu still can fall back to the internal
definitions.  Even here it isn't strictly necessary to
remove an internal def as it can be redefined in the
config file which will override the internal version.
In general -cpu "?model" will indicate internal vs.
externally defined models by enclosing internal model names
in brackets:

    :
x86       Opteron_G3  AMD Opteron 23xx (Gen 3 Class Opteron)
    :
x86         [athlon]  QEMU Virtual CPU version 0.12.50
    :

It also seems worth dropping a hint to the user in the case qemu
fails to find a target config file rather than leaving them to
puzzle out why an external model has gone missing.

Thanks for the feedback.

-john

-- 
john.cooper@redhat.com

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH] Add assignment operation to config file parser..
  2010-02-01 19:02 ` [Qemu-devel] " john cooper
@ 2010-02-06 18:59   ` john cooper
  -1 siblings, 0 replies; 24+ messages in thread
From: john cooper @ 2010-02-06 18:59 UTC (permalink / raw)
  To: KVM list, qemu-devel
  Cc: Przywara, Andre, Anthony Liguori, Mark McLoughlin, john.cooper

This patch reworks support for both assignment and
append in the config file parser.  It was motivated
by comments received on the cpu model config file
format.

Commit dc9ca4ba27be4fe6a0284061b8f056c4364fb0d9
changed the behavior of "=" from assign to append.
This patch preserves the ability to append to an
option (however now via "+="), reverts "=" to its
previous behavior, and allows both to interoperate.

Signed-off-by: john cooper <john.cooper@redhat.com>
---

diff --git a/qemu-config.c b/qemu-config.c
index 246fae6..4e53250 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -429,6 +429,7 @@ int qemu_config_parse(FILE *fp)
     char line[1024], group[64], id[64], arg[64], value[1024];
     QemuOptsList *list = NULL;
     QemuOpts *opts = NULL;
+    char append;
 
     while (fgets(line, sizeof(line), fp) != NULL) {
         if (line[0] == '\n') {
@@ -455,13 +456,16 @@ int qemu_config_parse(FILE *fp)
             opts = qemu_opts_create(list, NULL, 0);
             continue;
         }
-        if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2) {
-            /* arg = value */
+        append = 0;
+        if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2 ||
+            (sscanf(line, " %63s += \"%1023[^\"]\"", arg, value) == 2 ?
+                append = 1 : 0)) {
+            /* arg = value, arg += value */
             if (opts == NULL) {
                 fprintf(stderr, "no group defined\n");
                 return -1;
             }
-            if (qemu_opt_set(opts, arg, value) != 0) {
+            if (_qemu_opt_set(opts, arg, value, append) != 0) {
                 fprintf(stderr, "failed to set \"%s\" for %s\n",
                         arg, group);
                 return -1;
diff --git a/qemu-option.c b/qemu-option.c
index a52a4c4..7c0faed 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -562,7 +562,11 @@ static void qemu_opt_del(QemuOpt *opt)
     qemu_free(opt);
 }
 
-int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
+/* add option *name,*value to group *opts.  if append add to tail of option
+ * list, else set as sole element (overwrite any existing entries of *name).
+ */
+int _qemu_opt_set(QemuOpts *opts, const char *name, const char *value,
+                  char append)
 {
     QemuOpt *opt;
     QemuOptDesc *desc = opts->list->desc;
@@ -582,13 +586,27 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
             return -1;
         }
     }
-
-    opt = qemu_mallocz(sizeof(*opt));
-    opt->name = qemu_strdup(name);
-    opt->opts = opts;
-    QTAILQ_INSERT_TAIL(&opts->head, opt, next);
-    if (desc[i].name != NULL) {
-        opt->desc = desc+i;
+    if (append || !(opt = qemu_opt_find(opts, name))) {
+        opt = qemu_mallocz(sizeof(*opt));
+        opt->name = qemu_strdup(name);
+        opt->opts = opts;
+        QTAILQ_INSERT_TAIL(&opts->head, opt, next);
+        if (desc[i].name != NULL) {
+            opt->desc = desc+i;
+        }
+    } else if (!append) {
+        QemuOpt *p, *next;
+
+        /* assign to reclaimed *opt, remove all other *name defs */
+        QTAILQ_FOREACH_SAFE(p, &opts->head, next, next) {
+            if (p != opt && !strcmp(name, p->name)) {
+                qemu_free((char *)p->str);
+                QTAILQ_REMOVE(&opts->head, p, next);
+                qemu_free((char *)p);
+            }
+        }
+        qemu_free((char *)opt->str);
+        opt->str = NULL;
     }
     if (value) {
         opt->str = qemu_strdup(value);
diff --git a/qemu-option.h b/qemu-option.h
index 666b666..2385b1a 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -104,7 +104,14 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name);
 int qemu_opt_get_bool(QemuOpts *opts, const char *name, int defval);
 uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval);
 uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval);
-int qemu_opt_set(QemuOpts *opts, const char *name, const char *value);
+int _qemu_opt_set(QemuOpts *opts, const char *name, const char *value,
+                  char append);
+static inline int qemu_opt_set(QemuOpts *opts, const char *name,
+                               const char *value)
+{
+    return (_qemu_opt_set(opts, name, value, 0));
+}
+
 typedef int (*qemu_opt_loopfunc)(const char *name, const char *value, void *opaque);
 int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
                      int abort_on_failure);
-- 
john.cooper@redhat.com

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH] Add assignment operation to config file parser..
@ 2010-02-06 18:59   ` john cooper
  0 siblings, 0 replies; 24+ messages in thread
From: john cooper @ 2010-02-06 18:59 UTC (permalink / raw)
  To: KVM list, qemu-devel; +Cc: john.cooper, Przywara, Andre, Mark McLoughlin

This patch reworks support for both assignment and
append in the config file parser.  It was motivated
by comments received on the cpu model config file
format.

Commit dc9ca4ba27be4fe6a0284061b8f056c4364fb0d9
changed the behavior of "=" from assign to append.
This patch preserves the ability to append to an
option (however now via "+="), reverts "=" to its
previous behavior, and allows both to interoperate.

Signed-off-by: john cooper <john.cooper@redhat.com>
---

diff --git a/qemu-config.c b/qemu-config.c
index 246fae6..4e53250 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -429,6 +429,7 @@ int qemu_config_parse(FILE *fp)
     char line[1024], group[64], id[64], arg[64], value[1024];
     QemuOptsList *list = NULL;
     QemuOpts *opts = NULL;
+    char append;
 
     while (fgets(line, sizeof(line), fp) != NULL) {
         if (line[0] == '\n') {
@@ -455,13 +456,16 @@ int qemu_config_parse(FILE *fp)
             opts = qemu_opts_create(list, NULL, 0);
             continue;
         }
-        if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2) {
-            /* arg = value */
+        append = 0;
+        if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2 ||
+            (sscanf(line, " %63s += \"%1023[^\"]\"", arg, value) == 2 ?
+                append = 1 : 0)) {
+            /* arg = value, arg += value */
             if (opts == NULL) {
                 fprintf(stderr, "no group defined\n");
                 return -1;
             }
-            if (qemu_opt_set(opts, arg, value) != 0) {
+            if (_qemu_opt_set(opts, arg, value, append) != 0) {
                 fprintf(stderr, "failed to set \"%s\" for %s\n",
                         arg, group);
                 return -1;
diff --git a/qemu-option.c b/qemu-option.c
index a52a4c4..7c0faed 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -562,7 +562,11 @@ static void qemu_opt_del(QemuOpt *opt)
     qemu_free(opt);
 }
 
-int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
+/* add option *name,*value to group *opts.  if append add to tail of option
+ * list, else set as sole element (overwrite any existing entries of *name).
+ */
+int _qemu_opt_set(QemuOpts *opts, const char *name, const char *value,
+                  char append)
 {
     QemuOpt *opt;
     QemuOptDesc *desc = opts->list->desc;
@@ -582,13 +586,27 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
             return -1;
         }
     }
-
-    opt = qemu_mallocz(sizeof(*opt));
-    opt->name = qemu_strdup(name);
-    opt->opts = opts;
-    QTAILQ_INSERT_TAIL(&opts->head, opt, next);
-    if (desc[i].name != NULL) {
-        opt->desc = desc+i;
+    if (append || !(opt = qemu_opt_find(opts, name))) {
+        opt = qemu_mallocz(sizeof(*opt));
+        opt->name = qemu_strdup(name);
+        opt->opts = opts;
+        QTAILQ_INSERT_TAIL(&opts->head, opt, next);
+        if (desc[i].name != NULL) {
+            opt->desc = desc+i;
+        }
+    } else if (!append) {
+        QemuOpt *p, *next;
+
+        /* assign to reclaimed *opt, remove all other *name defs */
+        QTAILQ_FOREACH_SAFE(p, &opts->head, next, next) {
+            if (p != opt && !strcmp(name, p->name)) {
+                qemu_free((char *)p->str);
+                QTAILQ_REMOVE(&opts->head, p, next);
+                qemu_free((char *)p);
+            }
+        }
+        qemu_free((char *)opt->str);
+        opt->str = NULL;
     }
     if (value) {
         opt->str = qemu_strdup(value);
diff --git a/qemu-option.h b/qemu-option.h
index 666b666..2385b1a 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -104,7 +104,14 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name);
 int qemu_opt_get_bool(QemuOpts *opts, const char *name, int defval);
 uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval);
 uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval);
-int qemu_opt_set(QemuOpts *opts, const char *name, const char *value);
+int _qemu_opt_set(QemuOpts *opts, const char *name, const char *value,
+                  char append);
+static inline int qemu_opt_set(QemuOpts *opts, const char *name,
+                               const char *value)
+{
+    return (_qemu_opt_set(opts, name, value, 0));
+}
+
 typedef int (*qemu_opt_loopfunc)(const char *name, const char *value, void *opaque);
 int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
                      int abort_on_failure);
-- 
john.cooper@redhat.com

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH] Add assignment operation to config file parser..
  2010-02-06 18:59   ` [Qemu-devel] " john cooper
@ 2010-02-07 16:24     ` Anthony Liguori
  -1 siblings, 0 replies; 24+ messages in thread
From: Anthony Liguori @ 2010-02-07 16:24 UTC (permalink / raw)
  To: john cooper
  Cc: KVM list, qemu-devel, Przywara, Andre, Mark McLoughlin, Gerd Hoffmann

On 02/06/2010 12:59 PM, john cooper wrote:
> This patch reworks support for both assignment and
> append in the config file parser.  It was motivated
> by comments received on the cpu model config file
> format.
>
> Commit dc9ca4ba27be4fe6a0284061b8f056c4364fb0d9
> changed the behavior of "=" from assign to append.
> This patch preserves the ability to append to an
> option (however now via "+="), reverts "=" to its
> previous behavior, and allows both to interoperate.
>
> Signed-off-by: john cooper<john.cooper@redhat.com>
>    

This deviates from standard ini syntax which makes me a big 
uncomfortable with it.  Gerd, do you have an opinion?

Regards,

Anthony Liguori


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH] Add assignment operation to config file parser..
@ 2010-02-07 16:24     ` Anthony Liguori
  0 siblings, 0 replies; 24+ messages in thread
From: Anthony Liguori @ 2010-02-07 16:24 UTC (permalink / raw)
  To: john cooper
  Cc: Przywara, Andre, Gerd Hoffmann, qemu-devel, KVM list, Mark McLoughlin

On 02/06/2010 12:59 PM, john cooper wrote:
> This patch reworks support for both assignment and
> append in the config file parser.  It was motivated
> by comments received on the cpu model config file
> format.
>
> Commit dc9ca4ba27be4fe6a0284061b8f056c4364fb0d9
> changed the behavior of "=" from assign to append.
> This patch preserves the ability to append to an
> option (however now via "+="), reverts "=" to its
> previous behavior, and allows both to interoperate.
>
> Signed-off-by: john cooper<john.cooper@redhat.com>
>    

This deviates from standard ini syntax which makes me a big 
uncomfortable with it.  Gerd, do you have an opinion?

Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH] Add assignment operation to config file parser..
  2010-02-07 16:24     ` Anthony Liguori
  (?)
@ 2010-02-08 13:21     ` Gerd Hoffmann
  2010-02-08 16:00         ` john cooper
  -1 siblings, 1 reply; 24+ messages in thread
From: Gerd Hoffmann @ 2010-02-08 13:21 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: john cooper, Przywara, Andre, qemu-devel, KVM list, Mark McLoughlin

On 02/07/10 17:24, Anthony Liguori wrote:
> On 02/06/2010 12:59 PM, john cooper wrote:
>> This patch reworks support for both assignment and
>> append in the config file parser. It was motivated
>> by comments received on the cpu model config file
>> format.
>>
>> Commit dc9ca4ba27be4fe6a0284061b8f056c4364fb0d9
>> changed the behavior of "=" from assign to append.
>> This patch preserves the ability to append to an
>> option (however now via "+="), reverts "=" to its
>> previous behavior, and allows both to interoperate.
>>
>> Signed-off-by: john cooper<john.cooper@redhat.com>
>
> This deviates from standard ini syntax which makes me a big
> uncomfortable with it. Gerd, do you have an opinion?

Also it the syntax change will break existing users of the append 
feature (host/guestfwd for slirp networking).

Any reason why you can't use the current append to avoid the overlong 
feature flag lines?

Another idea:  One could reference other processors "base", then you can 
define a -- say -- Opteron_G3 as "Opteron_G2 features plus these".

cheers,
   Gerd

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH] Add assignment operation to config file parser..
  2010-02-08 13:21     ` Gerd Hoffmann
@ 2010-02-08 16:00         ` john cooper
  0 siblings, 0 replies; 24+ messages in thread
From: john cooper @ 2010-02-08 16:00 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: Anthony Liguori, Przywara, Andre, qemu-devel, KVM list,
	Mark McLoughlin, john.cooper

Gerd Hoffmann wrote:
> On 02/07/10 17:24, Anthony Liguori wrote:
>> On 02/06/2010 12:59 PM, john cooper wrote:
>>> This patch reworks support for both assignment and
>>> append in the config file parser. It was motivated
>>> by comments received on the cpu model config file
>>> format.
>>>
>>> Commit dc9ca4ba27be4fe6a0284061b8f056c4364fb0d9
>>> changed the behavior of "=" from assign to append.
>>> This patch preserves the ability to append to an
>>> option (however now via "+="), reverts "=" to its
>>> previous behavior, and allows both to interoperate.
>>>
>>> Signed-off-by: john cooper<john.cooper@redhat.com>
>>
>> This deviates from standard ini syntax which makes me a big
>> uncomfortable with it. Gerd, do you have an opinion?
> 
> Also it the syntax change will break existing users of the append
> feature (host/guestfwd for slirp networking).
> 
> Any reason why you can't use the current append to avoid the overlong
> feature flag lines?

Impacting existing usage was my primary concern as well.
I'd run this by Mark who created the patch changing the
disposition of "=" to append and I didn't find any alarms
there.

The proposed scheme supports both semantics and arguably
seems more intuitive.  If that is the general consensus
and it won't unduly perturb existing usage the above
would be opportune before the current behavior becomes
more entrenched (e.g. by further dependencies such as the
proposed cpu model configuration).

But to answer the question, my prior patch can work with
either.

Thanks,

-john

-- 
john.cooper@redhat.com

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH] Add assignment operation to config file parser..
@ 2010-02-08 16:00         ` john cooper
  0 siblings, 0 replies; 24+ messages in thread
From: john cooper @ 2010-02-08 16:00 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: Mark McLoughlin, KVM list, Przywara, Andre, john.cooper, qemu-devel

Gerd Hoffmann wrote:
> On 02/07/10 17:24, Anthony Liguori wrote:
>> On 02/06/2010 12:59 PM, john cooper wrote:
>>> This patch reworks support for both assignment and
>>> append in the config file parser. It was motivated
>>> by comments received on the cpu model config file
>>> format.
>>>
>>> Commit dc9ca4ba27be4fe6a0284061b8f056c4364fb0d9
>>> changed the behavior of "=" from assign to append.
>>> This patch preserves the ability to append to an
>>> option (however now via "+="), reverts "=" to its
>>> previous behavior, and allows both to interoperate.
>>>
>>> Signed-off-by: john cooper<john.cooper@redhat.com>
>>
>> This deviates from standard ini syntax which makes me a big
>> uncomfortable with it. Gerd, do you have an opinion?
> 
> Also it the syntax change will break existing users of the append
> feature (host/guestfwd for slirp networking).
> 
> Any reason why you can't use the current append to avoid the overlong
> feature flag lines?

Impacting existing usage was my primary concern as well.
I'd run this by Mark who created the patch changing the
disposition of "=" to append and I didn't find any alarms
there.

The proposed scheme supports both semantics and arguably
seems more intuitive.  If that is the general consensus
and it won't unduly perturb existing usage the above
would be opportune before the current behavior becomes
more entrenched (e.g. by further dependencies such as the
proposed cpu model configuration).

But to answer the question, my prior patch can work with
either.

Thanks,

-john

-- 
john.cooper@redhat.com

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH] Add cpu model configuration support.. (resend)
  2010-02-01 19:02 ` [Qemu-devel] " john cooper
@ 2010-02-10 20:00   ` Anthony Liguori
  -1 siblings, 0 replies; 24+ messages in thread
From: Anthony Liguori @ 2010-02-10 20:00 UTC (permalink / raw)
  To: john cooper; +Cc: KVM list, qemu-devel, Przywara, Andre

On 02/01/2010 01:02 PM, john cooper wrote:
> [target-x86_64.conf was unintentionally omitted from the earlier patch]
>
> This is a reimplementation of prior versions which adds
> the ability to define cpu models for contemporary processors.
> The added models are likewise selected via -cpu<name>,
> and are intended to displace the existing convention
> of "-cpu qemu64" augmented with a series of feature flags
>    

This breaks the arm-softmmu build.

Regards,

Anthony Liguori

> A primary motivation was determination of a least common
> denominator within a given processor class to simplify guest
> migration.  It is still possible to modify an arbitrary model
> via additional feature flags however the goal here was to
> make doing so unnecessary in typical usage.  The other
> consideration was providing models names reflective of
> current processors.  Both AMD and Intel have reviewed the
> models in terms of balancing generality of migration vs.
> excessive feature downgrade relative to released silicon.
>
> This version of the patch replaces the prior hard wired
> definitions with a configuration file approach for new
> models.  Existing models are thus far left as-is but may
> easily be transitioned to (or may be overridden by) the
> configuration file representation.
>
> Proposed new model definitions are provided here for current
> AMD and Intel processors.  Each model consists of a name
> used to select it on the command line (-cpu<name>), and a
> model_id which corresponds to a least common denominator
> commercial instance of the processor class.
>
> A table of names/model_ids may be queried via "-cpu ?model":
>
>          :
>      x86       Opteron_G3  AMD Opteron 23xx (Gen 3 Class Opteron)
>      x86       Opteron_G2  AMD Opteron 22xx (Gen 2 Class Opteron)
>      x86       Opteron_G1  AMD Opteron 240 (Gen 1 Class Opteron)
>      x86          Nehalem  Intel Core i7 9xx (Nehalem Class Core i7)
>      x86           Penryn  Intel Core 2 Duo P9xxx (Penryn Class Core 2)
>      x86           Conroe  Intel Celeron_4x0 (Conroe/Merom Class Core 2)
>          :
>
> Also added is "-cpu ?dump" which exhaustively outputs all config
> data for all defined models, and "-cpu ?cpuid" which enumerates
> all qemu recognized CPUID feature flags.
>
> The pseudo cpuid flag 'check' when added to the feature flag list
> will warn when feature flags (either implicit in a cpu model or
> explicit on the command line) would have otherwise been quietly
> unavailable to a guest:
>
>      # qemu-system-x86_64 ... -cpu Nehalem,check
>      warning: host cpuid 0000_0001 lacks requested flag 'sse4.2|sse4_2' [0x00100000]
>      warning: host cpuid 0000_0001 lacks requested flag 'popcnt' [0x00800000]
>
> A similar 'enforce' pseudo flag exists which in addition
> to the above causes qemu to error exit if requested flags are
> unavailable.
>
> Configuration data for a cpu model resides in the target config
> file which by default will be installed as:
>
>      /usr/local/etc/qemu/target-<arch>.conf
>
> The format of this file should be self explanatory given the
> definitions for the above six models and essentially mimics
> the structure of the static x86_def_t x86_defs.
>
> Encoding of cpuid flags names now allows aliases for both the
> configuration file and the command line which reconciles some
> Intel/AMD/Linux/Qemu naming differences.
>
> This patch was tested relative to qemu.git.
>
> Signed-off-by: john cooper<john.cooper@redhat.com>
> ---
>
> diff --git a/Makefile b/Makefile
> index 3848627..b7fa6ef 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -191,7 +191,11 @@ ifdef CONFIG_POSIX
>   	$(INSTALL_DATA) qemu-nbd.8 "$(DESTDIR)$(mandir)/man8"
>   endif
>
> -install: all $(if $(BUILD_DOCS),install-doc)
> +install-sysconfig:
> +	$(INSTALL_DIR) "$(sysconfdir)/qemu"
> +	$(INSTALL_DATA) sysconfigs/target/target-x86_64.conf "$(sysconfdir)/qemu"
> +
> +install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig
>   	$(INSTALL_DIR) "$(DESTDIR)$(bindir)"
>   ifneq ($(TOOLS),)
>   	$(INSTALL_PROG) $(STRIP_OPT) $(TOOLS) "$(DESTDIR)$(bindir)"
> diff --git a/qemu-config.c b/qemu-config.c
> index c3203c8..246fae6 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -242,6 +242,54 @@ QemuOptsList qemu_mon_opts = {
>       },
>   };
>
> +QemuOptsList qemu_cpudef_opts = {
> +    .name = "cpudef",
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_cpudef_opts.head),
> +    .desc = {
> +        {
> +            .name = "name",
> +            .type = QEMU_OPT_STRING,
> +        },{
> +            .name = "level",
> +            .type = QEMU_OPT_NUMBER,
> +        },{
> +            .name = "vendor",
> +            .type = QEMU_OPT_STRING,
> +        },{
> +            .name = "family",
> +            .type = QEMU_OPT_NUMBER,
> +        },{
> +            .name = "model",
> +            .type = QEMU_OPT_NUMBER,
> +        },{
> +            .name = "stepping",
> +            .type = QEMU_OPT_NUMBER,
> +        },{
> +            .name = "feature_edx",      /* cpuid 0000_0001.edx */
> +            .type = QEMU_OPT_STRING,
> +        },{
> +            .name = "feature_ecx",      /* cpuid 0000_0001.ecx */
> +            .type = QEMU_OPT_STRING,
> +        },{
> +            .name = "extfeature_edx",   /* cpuid 8000_0001.edx */
> +            .type = QEMU_OPT_STRING,
> +        },{
> +            .name = "extfeature_ecx",   /* cpuid 8000_0001.ecx */
> +            .type = QEMU_OPT_STRING,
> +        },{
> +            .name = "xlevel",
> +            .type = QEMU_OPT_NUMBER,
> +        },{
> +            .name = "model_id",
> +            .type = QEMU_OPT_STRING,
> +        },{
> +            .name = "vendor_override",
> +            .type = QEMU_OPT_NUMBER,
> +        },
> +        { /* end of list */ }
> +    },
> +};
> +
>   static QemuOptsList *lists[] = {
>       &qemu_drive_opts,
>       &qemu_chardev_opts,
> @@ -251,6 +299,7 @@ static QemuOptsList *lists[] = {
>       &qemu_rtc_opts,
>       &qemu_global_opts,
>       &qemu_mon_opts,
> +&qemu_cpudef_opts,
>       NULL,
>   };
>
> diff --git a/qemu-config.h b/qemu-config.h
> index dd89ae4..b335c42 100644
> --- a/qemu-config.h
> +++ b/qemu-config.h
> @@ -9,6 +9,7 @@ extern QemuOptsList qemu_net_opts;
>   extern QemuOptsList qemu_rtc_opts;
>   extern QemuOptsList qemu_global_opts;
>   extern QemuOptsList qemu_mon_opts;
> +extern QemuOptsList qemu_cpudef_opts;
>
>   int qemu_set_option(const char *str);
>   int qemu_global_option(const char *str);
> diff --git a/sysconfigs/target/target-x86_64.conf b/sysconfigs/target/target-x86_64.conf
> new file mode 100644
> index 0000000..43ad282
> --- /dev/null
> +++ b/sysconfigs/target/target-x86_64.conf
> @@ -0,0 +1,86 @@
> +# x86 CPU MODELS
> +
> +[cpudef]
> +   name = "Conroe"
> +   level = "2"
> +   vendor = "GenuineIntel"
> +   family = "6"
> +   model = "2"
> +   stepping = "3"
> +   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
> +   feature_ecx = "sse3 ssse3"
> +   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx"
> +   extfeature_ecx = "lahf_lm"
> +   xlevel = "0x8000000A"
> +   model_id = "Intel Celeron_4x0 (Conroe/Merom Class Core 2)"
> +
> +[cpudef]
> +   name = "Penryn"
> +   level = "2"
> +   vendor = "GenuineIntel"
> +   family = "6"
> +   model = "2"
> +   stepping = "3"
> +   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
> +   feature_ecx = "sse3 cx16 ssse3 sse4.1"
> +   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx"
> +   extfeature_ecx = "lahf_lm"
> +   xlevel = "0x8000000A"
> +   model_id = "Intel Core 2 Duo P9xxx (Penryn Class Core 2)"
> +
> +[cpudef]
> +   name = "Nehalem"
> +   level = "2"
> +   vendor = "GenuineIntel"
> +   family = "6"
> +   model = "2"
> +   stepping = "3"
> +   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
> +   feature_ecx = "sse3 cx16 ssse3 sse4.1 sse4.2 popcnt"
> +   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx"
> +   extfeature_ecx = "lahf_lm"
> +   xlevel = "0x8000000A"
> +   model_id = "Intel Core i7 9xx (Nehalem Class Core i7)"
> +
> +[cpudef]
> +   name = "Opteron_G1"
> +   level = "5"
> +   vendor = "AuthenticAMD"
> +   family = "15"
> +   model = "6"
> +   stepping = "1"
> +   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
> +   feature_ecx = "sse3"
> +   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx"
> +#   extfeature_ecx = ""
> +   xlevel = "0x80000008"
> +   model_id = "AMD Opteron 240 (Gen 1 Class Opteron)"
> +
> +[cpudef]
> +   name = "Opteron_G2"
> +   level = "5"
> +   vendor = "AuthenticAMD"
> +   family = "15"
> +   model = "6"
> +   stepping = "1"
> +   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
> +   feature_ecx = "sse3 cx16"
> +   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx rdtscp"
> +   extfeature_ecx = "svm lahf_lm"
> +   xlevel = "0x80000008"
> +   model_id = "AMD Opteron 22xx (Gen 2 Class Opteron)"
> +
> +[cpudef]
> +   name = "Opteron_G3"
> +   level = "5"
> +   vendor = "AuthenticAMD"
> +   family = "15"
> +   model = "6"
> +   stepping = "1"
> +   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
> +   feature_ecx = "sse3 cx16 monitor popcnt"
> +   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx rdtscp"
> +   extfeature_ecx = "svm sse4a  abm misalignsse lahf_lm"
> +   xlevel = "0x80000008"
> +   model_id = "AMD Opteron 23xx (Gen 3 Class Opteron)"
> +
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 216b00e..c1a5256 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -723,8 +723,10 @@ typedef struct CPUX86State {
>   CPUX86State *cpu_x86_init(const char *cpu_model);
>   int cpu_x86_exec(CPUX86State *s);
>   void cpu_x86_close(CPUX86State *s);
> -void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt,
> -                                                 ...));
> +void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
> +                   const char *optarg);
> +void cpudef_setup(void);
> +
>   int cpu_get_pic_interrupt(CPUX86State *s);
>   /* MSDOS compatibility mode FPU exception support */
>   void cpu_set_ferr(CPUX86State *s);
> @@ -876,7 +878,7 @@ uint64_t cpu_get_tsc(CPUX86State *env);
>   #define cpu_exec cpu_x86_exec
>   #define cpu_gen_code cpu_x86_gen_code
>   #define cpu_signal_handler cpu_x86_signal_handler
> -#define cpu_list x86_cpu_list
> +#define cpu_list_id x86_cpu_list
>
>   #define CPU_SAVE_VERSION 11
>
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 70762bb..37dd2c6 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -29,33 +29,52 @@
>   #include "kvm.h"
>
>   //#define DEBUG_MMU
> +#include "qemu-option.h"
> +#include "qemu-config.h"
>
>   /* feature flags taken from "Intel Processor Identification and the CPUID
> - * Instruction" and AMD's "CPUID Specification". In cases of disagreement
> - * about feature names, the Linux name is used. */
> + * Instruction" and AMD's "CPUID Specification".  In cases of disagreement
> + * between feature naming conventions, aliases may be added.
> + */
>   static const char *feature_name[] = {
> -    "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
> -    "cx8", "apic", NULL, "sep", "mtrr", "pge", "mca", "cmov",
> -    "pat", "pse36", "pn" /* Intel psn */, "clflush" /* Intel clfsh */, NULL, "ds" /* Intel dts */, "acpi", "mmx",
> -    "fxsr", "sse", "sse2", "ss", "ht" /* Intel htt */, "tm", "ia64", "pbe",
> +    "fpu", "vme", "de", "pse",
> +    "tsc", "msr", "pae", "mce",
> +    "cx8", "apic", NULL, "sep",
> +    "mtrr", "pge", "mca", "cmov",
> +    "pat", "pse36", "pn" /* Intel psn */, "clflush" /* Intel clfsh */,
> +    NULL, "ds" /* Intel dts */, "acpi", "mmx",
> +    "fxsr", "sse", "sse2", "ss",
> +    "ht" /* Intel htt */, "tm", "ia64", "pbe",
>   };
>   static const char *ext_feature_name[] = {
> -    "pni" /* Intel,AMD sse3 */, NULL, NULL, "monitor", "ds_cpl", "vmx", NULL /* Linux smx */, "est",
> -    "tm2", "ssse3", "cid", NULL, NULL, "cx16", "xtpr", NULL,
> -    NULL, NULL, "dca", NULL, NULL, NULL, NULL, "popcnt",
> -    NULL, NULL, NULL, NULL, NULL, NULL, NULL, "hypervisor",
> +    "pni|sse3" /* Intel,AMD sse3 */, NULL, NULL, "monitor",
> +    "ds_cpl", "vmx", NULL /* Linux smx */, "est",
> +    "tm2", "ssse3", "cid", NULL,
> +    NULL, "cx16", "xtpr", NULL,
> +    NULL, NULL, "dca", "sse4.1|sse4_1",
> +    "sse4.2|sse4_2", "x2apic", NULL, "popcnt",
> +    NULL, NULL, NULL, NULL,
> +    NULL, NULL, NULL, "hypervisor",
>   };
>   static const char *ext2_feature_name[] = {
> -    "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
> -    "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall", "mtrr", "pge", "mca", "cmov",
> -    "pat", "pse36", NULL, NULL /* Linux mp */, "nx" /* Intel xd */, NULL, "mmxext", "mmx",
> -    "fxsr", "fxsr_opt" /* AMD ffxsr */, "pdpe1gb" /* AMD Page1GB */, "rdtscp", NULL, "lm" /* Intel 64 */, "3dnowext", "3dnow",
> +    "fpu", "vme", "de", "pse",
> +    "tsc", "msr", "pae", "mce",
> +    "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall",
> +    "mtrr", "pge", "mca", "cmov",
> +    "pat", "pse36", NULL, NULL /* Linux mp */,
> +    "nx" /* Intel xd */, NULL, "mmxext", "mmx",
> +    "fxsr", "fxsr_opt" /* AMD ffxsr */, "pdpe1gb" /* AMD Page1GB */, "rdtscp",
> +    NULL, "lm" /* Intel 64 */, "3dnowext", "3dnow",
>   };
>   static const char *ext3_feature_name[] = {
> -    "lahf_lm" /* AMD LahfSahf */, "cmp_legacy", "svm", "extapic" /* AMD ExtApicSpace */, "cr8legacy" /* AMD AltMovCr8 */, "abm", "sse4a", "misalignsse",
> -    "3dnowprefetch", "osvw", NULL /* Linux ibs */, NULL, "skinit", "wdt", NULL, NULL,
> -    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> -    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> +    "lahf_lm" /* AMD LahfSahf */, "cmp_legacy", "svm", "extapic" /* AMD ExtApicSpace */,
> +    "cr8legacy" /* AMD AltMovCr8 */, "abm", "sse4a", "misalignsse",
> +    "3dnowprefetch", "osvw", NULL /* Linux ibs */, NULL,
> +    "skinit", "wdt", NULL, NULL,
> +    NULL, NULL, NULL, NULL,
> +    NULL, NULL, NULL, NULL,
> +    NULL, NULL, NULL, NULL,
> +    NULL, NULL, NULL, NULL,
>   };
>
>   static const char *kvm_feature_name[] = {
> @@ -65,47 +84,99 @@ static const char *kvm_feature_name[] = {
>       NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
>   };
>
> +/* collects per-function cpuid data
> + */
> +typedef struct model_features_t {
> +    uint32_t *guest_feat;
> +    uint32_t *host_feat;
> +    uint32_t check_feat;
> +    const char **flag_names;
> +    uint32_t cpuid;
> +    } model_features_t;
> +
> +int check_cpuid = 0;
> +int enforce_cpuid = 0;
> +
> +static void host_cpuid(uint32_t function, uint32_t count, uint32_t *eax,
> +                       uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
> +
> +#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 success, otherwise return zero
> + */
> +static int lookup_feature(uint32_t *pval, const char *s, const char *e,
> +    const char **featureset)
> +{
> +    uint32_t mask;
> +    const char **ppc;
> +
> +    for (mask = 1, ppc = featureset; mask; mask<<= 1, ++ppc)
> +        if (*ppc&&  !altcmp(s, e, *ppc)) {
> +            *pval |= mask;
> +            break;
> +        }
> +    return (mask ? 1 : 0);
> +}
> +
>   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)
>   {
> -    int i;
> -    int found = 0;
> -
> -    for ( i = 0 ; i<  32 ; i++ )
> -        if (feature_name[i]&&  !strcmp (flagname, feature_name[i])) {
> -            *features |= 1<<  i;
> -            found = 1;
> -        }
> -    for ( i = 0 ; i<  32 ; i++ )
> -        if (ext_feature_name[i]&&  !strcmp (flagname, ext_feature_name[i])) {
> -            *ext_features |= 1<<  i;
> -            found = 1;
> -        }
> -    for ( i = 0 ; i<  32 ; i++ )
> -        if (ext2_feature_name[i]&&  !strcmp (flagname, ext2_feature_name[i])) {
> -            *ext2_features |= 1<<  i;
> -            found = 1;
> -        }
> -    for ( i = 0 ; i<  32 ; i++ )
> -        if (ext3_feature_name[i]&&  !strcmp (flagname, ext3_feature_name[i])) {
> -            *ext3_features |= 1<<  i;
> -            found = 1;
> -        }
> -    for ( i = 0 ; i<  32 ; i++ )
> -        if (kvm_feature_name[i]&&  !strcmp (flagname, kvm_feature_name[i])) {
> -            *kvm_features |= 1<<  i;
> -            found = 1;
> -        }
> -
> -    if (!found) {
> -        fprintf(stderr, "CPU feature %s not found\n", flagname);
> -    }
> +    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))
> +            fprintf(stderr, "CPU feature %s not found\n", flagname);
>   }
>
>   typedef struct x86_def_t {
> +    struct x86_def_t *next;
>       const char *name;
>       uint32_t level;
>       uint32_t vendor1, vendor2, vendor3;
> @@ -116,6 +187,7 @@ typedef struct x86_def_t {
>       uint32_t xlevel;
>       char model_id[48];
>       int vendor_override;
> +    uint32_t flags;
>   } x86_def_t;
>
>   #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
> @@ -129,7 +201,14 @@ typedef struct x86_def_t {
>             CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
>             CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
>             CPUID_PAE | CPUID_SEP | CPUID_APIC)
> -static x86_def_t x86_defs[] = {
> +
> +/* maintains list of cpu model definitions
> + */
> +static x86_def_t *x86_defs = {NULL};
> +
> +/* built-in cpu model definitions (deprecated)
> + */
> +static x86_def_t builtin_x86_defs[] = {
>   #ifdef TARGET_X86_64
>       {
>           .name = "qemu64",
> @@ -334,9 +413,6 @@ static x86_def_t x86_defs[] = {
>       },
>   };
>
> -static void host_cpuid(uint32_t function, uint32_t count, uint32_t *eax,
> -                               uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
> -
>   static int cpu_x86_fill_model_id(char *str)
>   {
>       uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
> @@ -382,6 +458,51 @@ static int cpu_x86_fill_host(x86_def_t *x86_cpu_def)
>       return 0;
>   }
>
> +static int unavailable_host_feature(struct model_features_t *f, uint32_t mask)
> +{
> +    int i;
> +
> +    for (i = 0; i<  32; ++i)
> +        if (1<<  i&  mask) {
> +            fprintf(stderr, "warning: host cpuid %04x_%04x lacks requested"
> +                " flag '%s' [0x%08x]\n",
> +                f->cpuid>>  16, f->cpuid&  0xffff,
> +                f->flag_names[i] ? f->flag_names[i] : "[reserved]", mask);
> +            break;
> +        }
> +    return 0;
> +}
> +
> +/* best effort attempt to inform user requested cpu flags aren't making
> + * their way to the guest.  Note: ft[].check_feat ideally should be
> + * specified via a guest_def field to suppress report of extraneous flags.
> + */
> +static int check_features_against_host(x86_def_t *guest_def)
> +{
> +    x86_def_t host_def;
> +    uint32_t mask;
> +    int rv, i;
> +    struct model_features_t ft[] = {
> +        {&guest_def->features,&host_def.features,
> +            ~0, feature_name, 0x00000000},
> +        {&guest_def->ext_features,&host_def.ext_features,
> +            ~CPUID_EXT_HYPERVISOR, ext_feature_name, 0x00000001},
> +        {&guest_def->ext2_features,&host_def.ext2_features,
> +            ~PPRO_FEATURES, ext2_feature_name, 0x80000000},
> +        {&guest_def->ext3_features,&host_def.ext3_features,
> +            ~CPUID_EXT3_SVM, ext3_feature_name, 0x80000001}};
> +
> +    cpu_x86_fill_host(&host_def);
> +    for (rv = 0, i = 0; i<  sizeof (ft) / sizeof (ft[0]); ++i)
> +        for (mask = 1; mask; mask<<= 1)
> +            if (ft[i].check_feat&  mask&&  *ft[i].guest_feat&  mask&&
> +                !(*ft[i].host_feat&  mask)) {
> +                    unavailable_host_feature(&ft[i], mask);
> +                    rv = 1;
> +                }
> +    return rv;
> +}
> +
>   static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
>   {
>       unsigned int i;
> @@ -393,13 +514,9 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
>       uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 0, minus_ext3_features = 0, minus_kvm_features = 0;
>       uint32_t numvalue;
>
> -    def = NULL;
> -    for (i = 0; i<  ARRAY_SIZE(x86_defs); i++) {
> -        if (strcmp(name, x86_defs[i].name) == 0) {
> -            def =&x86_defs[i];
> +    for (def = x86_defs; def; def = def->next)
> +        if (!strcmp(name, def->name))
>               break;
> -        }
> -    }
>       if (kvm_enabled()&&  strcmp(name, "host") == 0) {
>           cpu_x86_fill_host(x86_cpu_def);
>       } else if (!def) {
> @@ -488,6 +605,10 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
>                   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 {
>               fprintf(stderr, "feature string `%s' not in format (+feature|-feature|feature=xyz)\n", featurestr);
>               goto error;
> @@ -504,6 +625,10 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
>       x86_cpu_def->ext2_features&= ~minus_ext2_features;
>       x86_cpu_def->ext3_features&= ~minus_ext3_features;
>       x86_cpu_def->kvm_features&= ~minus_kvm_features;
> +    if (check_cpuid) {
> +        if (check_features_against_host(x86_cpu_def)&&  enforce_cpuid)
> +            goto error;
> +    }
>       free(s);
>       return 0;
>
> @@ -512,12 +637,97 @@ error:
>       return -1;
>   }
>
> -void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...))
> +/* generate a composite string into buf of all cpuid names in featureset
> + * selected by fbits.  indicate truncation at bufsize in the event of overflow.
> + * if flags, suppress names undefined in featureset.
> + */
> +static void listflags(char *buf, int bufsize, uint32_t fbits,
> +    const char **featureset, uint32_t flags)
>   {
> -    unsigned int i;
> +    const char **p =&featureset[31];
> +    char *q, *b, bit;
> +    int nc;
> +
> +    b = 4<= bufsize ? buf + (bufsize -= 3) - 1 : NULL;
> +    *buf = '\0';
> +    for (q = buf, bit = 31; fbits&&  bufsize; --p, fbits&= ~(1<<  bit), --bit)
> +        if (fbits&  1<<  bit&&  (*p || !flags)) {
> +            if (*p)
> +                nc = snprintf(q, bufsize, "%s%s", q == buf ? "" : " ", *p);
> +            else
> +                nc = snprintf(q, bufsize, "%s[%d]", q == buf ? "" : " ", bit);
> +            if (bufsize<= nc) {
> +                if (b)
> +                    sprintf(b, "...");
> +                return;
> +            }
> +            q += nc;
> +            bufsize -= nc;
> +        }
> +}
>
> -    for (i = 0; i<  ARRAY_SIZE(x86_defs); i++)
> -        (*cpu_fprintf)(f, "x86 %16s\n", x86_defs[i].name);
> +/* generate CPU information:
> + * -?        list model names
> + * -?model   list model names/IDs
> + * -?dump    output all model (x86_def_t) data
> + * -?cpuid   list all recognized cpuid flag names
> + */
> +void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
> +                  const char *optarg)
> +{
> +    unsigned char model = !strcmp("?model", optarg);
> +    unsigned char dump = !strcmp("?dump", optarg);
> +    unsigned char cpuid = !strcmp("?cpuid", optarg);
> +    x86_def_t *def;
> +    char buf[256];
> +
> +    if (cpuid) {
> +        (*cpu_fprintf)(f, "Recognized CPUID flags:\n");
> +        listflags(buf, sizeof (buf), (uint32_t)~0, feature_name, 1);
> +        (*cpu_fprintf)(f, "  f_edx: %s\n", buf);
> +        listflags(buf, sizeof (buf), (uint32_t)~0, ext_feature_name, 1);
> +        (*cpu_fprintf)(f, "  f_ecx: %s\n", buf);
> +        listflags(buf, sizeof (buf), (uint32_t)~0, ext2_feature_name, 1);
> +        (*cpu_fprintf)(f, "  extf_edx: %s\n", buf);
> +        listflags(buf, sizeof (buf), (uint32_t)~0, ext3_feature_name, 1);
> +        (*cpu_fprintf)(f, "  extf_ecx: %s\n", buf);
> +        return;
> +    }
> +    for (def = x86_defs; def; def = def->next) {
> +        snprintf(buf, sizeof (buf), def->flags ? "[%s]": "%s", def->name);
> +        if (model || dump) {
> +            (*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf, def->model_id);
> +        } else {
> +            (*cpu_fprintf)(f, "x86 %16s\n", buf);
> +        }
> +        if (dump) {
> +            memcpy(buf,&def->vendor1, sizeof (def->vendor1));
> +            memcpy(buf + 4,&def->vendor2, sizeof (def->vendor2));
> +            memcpy(buf + 8,&def->vendor3, sizeof (def->vendor3));
> +            buf[12] = '\0';
> +            (*cpu_fprintf)(f,
> +                "  family %d model %d stepping %d level %d xlevel 0x%x"
> +                " vendor \"%s\"\n",
> +                def->family, def->model, def->stepping, def->level,
> +                def->xlevel, buf);
> +            listflags(buf, sizeof (buf), def->features, feature_name, 0);
> +            (*cpu_fprintf)(f, "  feature_edx %08x (%s)\n", def->features,
> +                buf);
> +            listflags(buf, sizeof (buf), def->ext_features, ext_feature_name,
> +                0);
> +            (*cpu_fprintf)(f, "  feature_ecx %08x (%s)\n", def->ext_features,
> +                buf);
> +            listflags(buf, sizeof (buf), def->ext2_features, ext2_feature_name,
> +                0);
> +            (*cpu_fprintf)(f, "  extfeature_edx %08x (%s)\n",
> +                def->ext2_features, buf);
> +            listflags(buf, sizeof (buf), def->ext3_features, ext3_feature_name,
> +                0);
> +            (*cpu_fprintf)(f, "  extfeature_ecx %08x (%s)\n",
> +                def->ext3_features, buf);
> +            (*cpu_fprintf)(f, "\n");
> +        }
> +    }
>   }
>
>   static int cpu_x86_register (CPUX86State *env, const char *cpu_model)
> @@ -566,6 +776,124 @@ static int cpu_x86_register (CPUX86State *env, const char *cpu_model)
>       return 0;
>   }
>
> +/* copy vendor id string to 32 bit register, nul pad as needed
> + */
> +static void cpyid(const char *s, uint32_t *id)
> +{
> +    char *d = (char *)id;
> +    char i;
> +
> +    for (i = sizeof (*id); i--; )
> +        *d++ = *s ? *s++ : '\0';
> +}
> +
> +/* interpret radix and convert from string to arbitrary scalar,
> + * otherwise flag failure
> + */
> +#define setscalar(pval, str, perr)                      \
> +{                                                       \
> +    char *pend;                                         \
> +    unsigned long ul;                                   \
> +                                                        \
> +    ul = strtoul(str,&pend, 0);                        \
> +    *str&&  !*pend ? (*pval = ul) : (*perr = 1);        \
> +}
> +
> +/* map cpuid options to feature bits, otherwise return failure
> + * (option tags in *str are delimited by whitespace)
> + */
> +static void setfeatures(uint32_t *pval, const char *str,
> +    const char **featureset, int *perr)
> +{
> +    const char *p, *q;
> +
> +    for (q = p = str; *p || *q; q = p) {
> +        while (iswhite(*p))
> +            q = ++p;
> +        while (*p&&  !iswhite(*p))
> +            ++p;
> +        if (!*q&&  !*p)
> +            return;
> +        if (!lookup_feature(pval, q, p, featureset)) {
> +            fprintf(stderr, "error: feature \"%.*s\" not available in set\n",
> +                (int)(p - q), q);
> +            *perr = 1;
> +            return;
> +        }
> +    }
> +}
> +
> +/* map config file options to x86_def_t form
> + */
> +static int cpudef_setfield(const char *name, const char *str, void *opaque)
> +{
> +    x86_def_t *def = opaque;
> +    int err = 0;
> +
> +    if (!strcmp(name, "name")) {
> +        def->name = strdup(str);
> +    } else if (!strcmp(name, "model_id")) {
> +        strncpy(def->model_id, str, sizeof (def->model_id));
> +    } else if (!strcmp(name, "level")) {
> +        setscalar(&def->level, str,&err)
> +    } else if (!strcmp(name, "vendor")) {
> +        cpyid(&str[0],&def->vendor1);
> +        cpyid(&str[4],&def->vendor2);
> +        cpyid(&str[8],&def->vendor3);
> +    } else if (!strcmp(name, "family")) {
> +        setscalar(&def->family, str,&err)
> +    } else if (!strcmp(name, "model")) {
> +        setscalar(&def->model, str,&err)
> +    } else if (!strcmp(name, "stepping")) {
> +        setscalar(&def->stepping, str,&err)
> +    } else if (!strcmp(name, "feature_edx")) {
> +        setfeatures(&def->features, str, feature_name,&err);
> +    } else if (!strcmp(name, "feature_ecx")) {
> +        setfeatures(&def->ext_features, str, ext_feature_name,&err);
> +    } else if (!strcmp(name, "extfeature_edx")) {
> +        setfeatures(&def->ext2_features, str, ext2_feature_name,&err);
> +    } else if (!strcmp(name, "extfeature_ecx")) {
> +        setfeatures(&def->ext3_features, str, ext3_feature_name,&err);
> +    } else if (!strcmp(name, "xlevel")) {
> +        setscalar(&def->xlevel, str,&err)
> +    } else {
> +        fprintf(stderr, "error: unknown option [%s = %s]\n", name, str);
> +        return (1);
> +    }
> +    if (err) {
> +        fprintf(stderr, "error: bad option value [%s = %s]\n", name, str);
> +        return (1);
> +    }
> +    return (0);
> +}
> +
> +/* register config file entry as x86_def_t
> + */
> +static int cpudef_register(QemuOpts *opts, void *opaque)
> +{
> +    x86_def_t *def = qemu_mallocz(sizeof (x86_def_t));
> +
> +    qemu_opt_foreach(opts, cpudef_setfield, def, 1);
> +    def->next = x86_defs;
> +    x86_defs = def;
> +    return (0);
> +}
> +
> +/* register "cpudef" models defined in configuration file after preloading
> + * built-in definitions
> + */
> +void cpudef_setup(void)
> +{
> +    int i;
> +
> +    for (i = 0; i<  ARRAY_SIZE(builtin_x86_defs); ++i) {
> +        builtin_x86_defs[i].next = x86_defs;
> +        builtin_x86_defs[i].flags = 1;
> +        x86_defs =&builtin_x86_defs[i];
> +    }
> +    qemu_opts_foreach(&qemu_cpudef_opts, cpudef_register, NULL, 0);
> +}
> +
>   /* NOTE: must be called outside the CPU execute loop */
>   void cpu_reset(CPUX86State *env)
>   {
> diff --git a/vl.c b/vl.c
> index 6f1e1ab..d6cd62c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4851,6 +4851,7 @@ int main(int argc, char **argv, char **envp)
>               fclose(fp);
>           }
>       }
> +    cpudef_setup();
>
>       /* second pass of option parsing */
>       optind = 1;
> @@ -4884,8 +4885,10 @@ int main(int argc, char **argv, char **envp)
>                   /* hw initialization will check this */
>                   if (*optarg == '?') {
>   /* XXX: implement xxx_cpu_list for targets that still miss it */
> -#if defined(cpu_list)
> -                    cpu_list(stdout,&fprintf);
> +#if defined(cpu_list_id)
> +                    cpu_list_id(stdout,&fprintf, optarg);
> +#elif defined(cpu_list)
> +                    cpu_list(stdout,&fprintf);	        /* deprecated */
>   #endif
>                       exit(0);
>                   } else {
>    


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH] Add cpu model configuration support.. (resend)
@ 2010-02-10 20:00   ` Anthony Liguori
  0 siblings, 0 replies; 24+ messages in thread
From: Anthony Liguori @ 2010-02-10 20:00 UTC (permalink / raw)
  To: john cooper; +Cc: Przywara, Andre, qemu-devel, KVM list

On 02/01/2010 01:02 PM, john cooper wrote:
> [target-x86_64.conf was unintentionally omitted from the earlier patch]
>
> This is a reimplementation of prior versions which adds
> the ability to define cpu models for contemporary processors.
> The added models are likewise selected via -cpu<name>,
> and are intended to displace the existing convention
> of "-cpu qemu64" augmented with a series of feature flags
>    

This breaks the arm-softmmu build.

Regards,

Anthony Liguori

> A primary motivation was determination of a least common
> denominator within a given processor class to simplify guest
> migration.  It is still possible to modify an arbitrary model
> via additional feature flags however the goal here was to
> make doing so unnecessary in typical usage.  The other
> consideration was providing models names reflective of
> current processors.  Both AMD and Intel have reviewed the
> models in terms of balancing generality of migration vs.
> excessive feature downgrade relative to released silicon.
>
> This version of the patch replaces the prior hard wired
> definitions with a configuration file approach for new
> models.  Existing models are thus far left as-is but may
> easily be transitioned to (or may be overridden by) the
> configuration file representation.
>
> Proposed new model definitions are provided here for current
> AMD and Intel processors.  Each model consists of a name
> used to select it on the command line (-cpu<name>), and a
> model_id which corresponds to a least common denominator
> commercial instance of the processor class.
>
> A table of names/model_ids may be queried via "-cpu ?model":
>
>          :
>      x86       Opteron_G3  AMD Opteron 23xx (Gen 3 Class Opteron)
>      x86       Opteron_G2  AMD Opteron 22xx (Gen 2 Class Opteron)
>      x86       Opteron_G1  AMD Opteron 240 (Gen 1 Class Opteron)
>      x86          Nehalem  Intel Core i7 9xx (Nehalem Class Core i7)
>      x86           Penryn  Intel Core 2 Duo P9xxx (Penryn Class Core 2)
>      x86           Conroe  Intel Celeron_4x0 (Conroe/Merom Class Core 2)
>          :
>
> Also added is "-cpu ?dump" which exhaustively outputs all config
> data for all defined models, and "-cpu ?cpuid" which enumerates
> all qemu recognized CPUID feature flags.
>
> The pseudo cpuid flag 'check' when added to the feature flag list
> will warn when feature flags (either implicit in a cpu model or
> explicit on the command line) would have otherwise been quietly
> unavailable to a guest:
>
>      # qemu-system-x86_64 ... -cpu Nehalem,check
>      warning: host cpuid 0000_0001 lacks requested flag 'sse4.2|sse4_2' [0x00100000]
>      warning: host cpuid 0000_0001 lacks requested flag 'popcnt' [0x00800000]
>
> A similar 'enforce' pseudo flag exists which in addition
> to the above causes qemu to error exit if requested flags are
> unavailable.
>
> Configuration data for a cpu model resides in the target config
> file which by default will be installed as:
>
>      /usr/local/etc/qemu/target-<arch>.conf
>
> The format of this file should be self explanatory given the
> definitions for the above six models and essentially mimics
> the structure of the static x86_def_t x86_defs.
>
> Encoding of cpuid flags names now allows aliases for both the
> configuration file and the command line which reconciles some
> Intel/AMD/Linux/Qemu naming differences.
>
> This patch was tested relative to qemu.git.
>
> Signed-off-by: john cooper<john.cooper@redhat.com>
> ---
>
> diff --git a/Makefile b/Makefile
> index 3848627..b7fa6ef 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -191,7 +191,11 @@ ifdef CONFIG_POSIX
>   	$(INSTALL_DATA) qemu-nbd.8 "$(DESTDIR)$(mandir)/man8"
>   endif
>
> -install: all $(if $(BUILD_DOCS),install-doc)
> +install-sysconfig:
> +	$(INSTALL_DIR) "$(sysconfdir)/qemu"
> +	$(INSTALL_DATA) sysconfigs/target/target-x86_64.conf "$(sysconfdir)/qemu"
> +
> +install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig
>   	$(INSTALL_DIR) "$(DESTDIR)$(bindir)"
>   ifneq ($(TOOLS),)
>   	$(INSTALL_PROG) $(STRIP_OPT) $(TOOLS) "$(DESTDIR)$(bindir)"
> diff --git a/qemu-config.c b/qemu-config.c
> index c3203c8..246fae6 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -242,6 +242,54 @@ QemuOptsList qemu_mon_opts = {
>       },
>   };
>
> +QemuOptsList qemu_cpudef_opts = {
> +    .name = "cpudef",
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_cpudef_opts.head),
> +    .desc = {
> +        {
> +            .name = "name",
> +            .type = QEMU_OPT_STRING,
> +        },{
> +            .name = "level",
> +            .type = QEMU_OPT_NUMBER,
> +        },{
> +            .name = "vendor",
> +            .type = QEMU_OPT_STRING,
> +        },{
> +            .name = "family",
> +            .type = QEMU_OPT_NUMBER,
> +        },{
> +            .name = "model",
> +            .type = QEMU_OPT_NUMBER,
> +        },{
> +            .name = "stepping",
> +            .type = QEMU_OPT_NUMBER,
> +        },{
> +            .name = "feature_edx",      /* cpuid 0000_0001.edx */
> +            .type = QEMU_OPT_STRING,
> +        },{
> +            .name = "feature_ecx",      /* cpuid 0000_0001.ecx */
> +            .type = QEMU_OPT_STRING,
> +        },{
> +            .name = "extfeature_edx",   /* cpuid 8000_0001.edx */
> +            .type = QEMU_OPT_STRING,
> +        },{
> +            .name = "extfeature_ecx",   /* cpuid 8000_0001.ecx */
> +            .type = QEMU_OPT_STRING,
> +        },{
> +            .name = "xlevel",
> +            .type = QEMU_OPT_NUMBER,
> +        },{
> +            .name = "model_id",
> +            .type = QEMU_OPT_STRING,
> +        },{
> +            .name = "vendor_override",
> +            .type = QEMU_OPT_NUMBER,
> +        },
> +        { /* end of list */ }
> +    },
> +};
> +
>   static QemuOptsList *lists[] = {
>       &qemu_drive_opts,
>       &qemu_chardev_opts,
> @@ -251,6 +299,7 @@ static QemuOptsList *lists[] = {
>       &qemu_rtc_opts,
>       &qemu_global_opts,
>       &qemu_mon_opts,
> +&qemu_cpudef_opts,
>       NULL,
>   };
>
> diff --git a/qemu-config.h b/qemu-config.h
> index dd89ae4..b335c42 100644
> --- a/qemu-config.h
> +++ b/qemu-config.h
> @@ -9,6 +9,7 @@ extern QemuOptsList qemu_net_opts;
>   extern QemuOptsList qemu_rtc_opts;
>   extern QemuOptsList qemu_global_opts;
>   extern QemuOptsList qemu_mon_opts;
> +extern QemuOptsList qemu_cpudef_opts;
>
>   int qemu_set_option(const char *str);
>   int qemu_global_option(const char *str);
> diff --git a/sysconfigs/target/target-x86_64.conf b/sysconfigs/target/target-x86_64.conf
> new file mode 100644
> index 0000000..43ad282
> --- /dev/null
> +++ b/sysconfigs/target/target-x86_64.conf
> @@ -0,0 +1,86 @@
> +# x86 CPU MODELS
> +
> +[cpudef]
> +   name = "Conroe"
> +   level = "2"
> +   vendor = "GenuineIntel"
> +   family = "6"
> +   model = "2"
> +   stepping = "3"
> +   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
> +   feature_ecx = "sse3 ssse3"
> +   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx"
> +   extfeature_ecx = "lahf_lm"
> +   xlevel = "0x8000000A"
> +   model_id = "Intel Celeron_4x0 (Conroe/Merom Class Core 2)"
> +
> +[cpudef]
> +   name = "Penryn"
> +   level = "2"
> +   vendor = "GenuineIntel"
> +   family = "6"
> +   model = "2"
> +   stepping = "3"
> +   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
> +   feature_ecx = "sse3 cx16 ssse3 sse4.1"
> +   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx"
> +   extfeature_ecx = "lahf_lm"
> +   xlevel = "0x8000000A"
> +   model_id = "Intel Core 2 Duo P9xxx (Penryn Class Core 2)"
> +
> +[cpudef]
> +   name = "Nehalem"
> +   level = "2"
> +   vendor = "GenuineIntel"
> +   family = "6"
> +   model = "2"
> +   stepping = "3"
> +   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
> +   feature_ecx = "sse3 cx16 ssse3 sse4.1 sse4.2 popcnt"
> +   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx"
> +   extfeature_ecx = "lahf_lm"
> +   xlevel = "0x8000000A"
> +   model_id = "Intel Core i7 9xx (Nehalem Class Core i7)"
> +
> +[cpudef]
> +   name = "Opteron_G1"
> +   level = "5"
> +   vendor = "AuthenticAMD"
> +   family = "15"
> +   model = "6"
> +   stepping = "1"
> +   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
> +   feature_ecx = "sse3"
> +   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx"
> +#   extfeature_ecx = ""
> +   xlevel = "0x80000008"
> +   model_id = "AMD Opteron 240 (Gen 1 Class Opteron)"
> +
> +[cpudef]
> +   name = "Opteron_G2"
> +   level = "5"
> +   vendor = "AuthenticAMD"
> +   family = "15"
> +   model = "6"
> +   stepping = "1"
> +   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
> +   feature_ecx = "sse3 cx16"
> +   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx rdtscp"
> +   extfeature_ecx = "svm lahf_lm"
> +   xlevel = "0x80000008"
> +   model_id = "AMD Opteron 22xx (Gen 2 Class Opteron)"
> +
> +[cpudef]
> +   name = "Opteron_G3"
> +   level = "5"
> +   vendor = "AuthenticAMD"
> +   family = "15"
> +   model = "6"
> +   stepping = "1"
> +   feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae msr tsc pse de fpu    mtrr clflush mca pse36"
> +   feature_ecx = "sse3 cx16 monitor popcnt"
> +   extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc pse de fpu    lm syscall nx rdtscp"
> +   extfeature_ecx = "svm sse4a  abm misalignsse lahf_lm"
> +   xlevel = "0x80000008"
> +   model_id = "AMD Opteron 23xx (Gen 3 Class Opteron)"
> +
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 216b00e..c1a5256 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -723,8 +723,10 @@ typedef struct CPUX86State {
>   CPUX86State *cpu_x86_init(const char *cpu_model);
>   int cpu_x86_exec(CPUX86State *s);
>   void cpu_x86_close(CPUX86State *s);
> -void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt,
> -                                                 ...));
> +void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
> +                   const char *optarg);
> +void cpudef_setup(void);
> +
>   int cpu_get_pic_interrupt(CPUX86State *s);
>   /* MSDOS compatibility mode FPU exception support */
>   void cpu_set_ferr(CPUX86State *s);
> @@ -876,7 +878,7 @@ uint64_t cpu_get_tsc(CPUX86State *env);
>   #define cpu_exec cpu_x86_exec
>   #define cpu_gen_code cpu_x86_gen_code
>   #define cpu_signal_handler cpu_x86_signal_handler
> -#define cpu_list x86_cpu_list
> +#define cpu_list_id x86_cpu_list
>
>   #define CPU_SAVE_VERSION 11
>
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 70762bb..37dd2c6 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -29,33 +29,52 @@
>   #include "kvm.h"
>
>   //#define DEBUG_MMU
> +#include "qemu-option.h"
> +#include "qemu-config.h"
>
>   /* feature flags taken from "Intel Processor Identification and the CPUID
> - * Instruction" and AMD's "CPUID Specification". In cases of disagreement
> - * about feature names, the Linux name is used. */
> + * Instruction" and AMD's "CPUID Specification".  In cases of disagreement
> + * between feature naming conventions, aliases may be added.
> + */
>   static const char *feature_name[] = {
> -    "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
> -    "cx8", "apic", NULL, "sep", "mtrr", "pge", "mca", "cmov",
> -    "pat", "pse36", "pn" /* Intel psn */, "clflush" /* Intel clfsh */, NULL, "ds" /* Intel dts */, "acpi", "mmx",
> -    "fxsr", "sse", "sse2", "ss", "ht" /* Intel htt */, "tm", "ia64", "pbe",
> +    "fpu", "vme", "de", "pse",
> +    "tsc", "msr", "pae", "mce",
> +    "cx8", "apic", NULL, "sep",
> +    "mtrr", "pge", "mca", "cmov",
> +    "pat", "pse36", "pn" /* Intel psn */, "clflush" /* Intel clfsh */,
> +    NULL, "ds" /* Intel dts */, "acpi", "mmx",
> +    "fxsr", "sse", "sse2", "ss",
> +    "ht" /* Intel htt */, "tm", "ia64", "pbe",
>   };
>   static const char *ext_feature_name[] = {
> -    "pni" /* Intel,AMD sse3 */, NULL, NULL, "monitor", "ds_cpl", "vmx", NULL /* Linux smx */, "est",
> -    "tm2", "ssse3", "cid", NULL, NULL, "cx16", "xtpr", NULL,
> -    NULL, NULL, "dca", NULL, NULL, NULL, NULL, "popcnt",
> -    NULL, NULL, NULL, NULL, NULL, NULL, NULL, "hypervisor",
> +    "pni|sse3" /* Intel,AMD sse3 */, NULL, NULL, "monitor",
> +    "ds_cpl", "vmx", NULL /* Linux smx */, "est",
> +    "tm2", "ssse3", "cid", NULL,
> +    NULL, "cx16", "xtpr", NULL,
> +    NULL, NULL, "dca", "sse4.1|sse4_1",
> +    "sse4.2|sse4_2", "x2apic", NULL, "popcnt",
> +    NULL, NULL, NULL, NULL,
> +    NULL, NULL, NULL, "hypervisor",
>   };
>   static const char *ext2_feature_name[] = {
> -    "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
> -    "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall", "mtrr", "pge", "mca", "cmov",
> -    "pat", "pse36", NULL, NULL /* Linux mp */, "nx" /* Intel xd */, NULL, "mmxext", "mmx",
> -    "fxsr", "fxsr_opt" /* AMD ffxsr */, "pdpe1gb" /* AMD Page1GB */, "rdtscp", NULL, "lm" /* Intel 64 */, "3dnowext", "3dnow",
> +    "fpu", "vme", "de", "pse",
> +    "tsc", "msr", "pae", "mce",
> +    "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall",
> +    "mtrr", "pge", "mca", "cmov",
> +    "pat", "pse36", NULL, NULL /* Linux mp */,
> +    "nx" /* Intel xd */, NULL, "mmxext", "mmx",
> +    "fxsr", "fxsr_opt" /* AMD ffxsr */, "pdpe1gb" /* AMD Page1GB */, "rdtscp",
> +    NULL, "lm" /* Intel 64 */, "3dnowext", "3dnow",
>   };
>   static const char *ext3_feature_name[] = {
> -    "lahf_lm" /* AMD LahfSahf */, "cmp_legacy", "svm", "extapic" /* AMD ExtApicSpace */, "cr8legacy" /* AMD AltMovCr8 */, "abm", "sse4a", "misalignsse",
> -    "3dnowprefetch", "osvw", NULL /* Linux ibs */, NULL, "skinit", "wdt", NULL, NULL,
> -    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> -    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> +    "lahf_lm" /* AMD LahfSahf */, "cmp_legacy", "svm", "extapic" /* AMD ExtApicSpace */,
> +    "cr8legacy" /* AMD AltMovCr8 */, "abm", "sse4a", "misalignsse",
> +    "3dnowprefetch", "osvw", NULL /* Linux ibs */, NULL,
> +    "skinit", "wdt", NULL, NULL,
> +    NULL, NULL, NULL, NULL,
> +    NULL, NULL, NULL, NULL,
> +    NULL, NULL, NULL, NULL,
> +    NULL, NULL, NULL, NULL,
>   };
>
>   static const char *kvm_feature_name[] = {
> @@ -65,47 +84,99 @@ static const char *kvm_feature_name[] = {
>       NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
>   };
>
> +/* collects per-function cpuid data
> + */
> +typedef struct model_features_t {
> +    uint32_t *guest_feat;
> +    uint32_t *host_feat;
> +    uint32_t check_feat;
> +    const char **flag_names;
> +    uint32_t cpuid;
> +    } model_features_t;
> +
> +int check_cpuid = 0;
> +int enforce_cpuid = 0;
> +
> +static void host_cpuid(uint32_t function, uint32_t count, uint32_t *eax,
> +                       uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
> +
> +#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 success, otherwise return zero
> + */
> +static int lookup_feature(uint32_t *pval, const char *s, const char *e,
> +    const char **featureset)
> +{
> +    uint32_t mask;
> +    const char **ppc;
> +
> +    for (mask = 1, ppc = featureset; mask; mask<<= 1, ++ppc)
> +        if (*ppc&&  !altcmp(s, e, *ppc)) {
> +            *pval |= mask;
> +            break;
> +        }
> +    return (mask ? 1 : 0);
> +}
> +
>   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)
>   {
> -    int i;
> -    int found = 0;
> -
> -    for ( i = 0 ; i<  32 ; i++ )
> -        if (feature_name[i]&&  !strcmp (flagname, feature_name[i])) {
> -            *features |= 1<<  i;
> -            found = 1;
> -        }
> -    for ( i = 0 ; i<  32 ; i++ )
> -        if (ext_feature_name[i]&&  !strcmp (flagname, ext_feature_name[i])) {
> -            *ext_features |= 1<<  i;
> -            found = 1;
> -        }
> -    for ( i = 0 ; i<  32 ; i++ )
> -        if (ext2_feature_name[i]&&  !strcmp (flagname, ext2_feature_name[i])) {
> -            *ext2_features |= 1<<  i;
> -            found = 1;
> -        }
> -    for ( i = 0 ; i<  32 ; i++ )
> -        if (ext3_feature_name[i]&&  !strcmp (flagname, ext3_feature_name[i])) {
> -            *ext3_features |= 1<<  i;
> -            found = 1;
> -        }
> -    for ( i = 0 ; i<  32 ; i++ )
> -        if (kvm_feature_name[i]&&  !strcmp (flagname, kvm_feature_name[i])) {
> -            *kvm_features |= 1<<  i;
> -            found = 1;
> -        }
> -
> -    if (!found) {
> -        fprintf(stderr, "CPU feature %s not found\n", flagname);
> -    }
> +    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))
> +            fprintf(stderr, "CPU feature %s not found\n", flagname);
>   }
>
>   typedef struct x86_def_t {
> +    struct x86_def_t *next;
>       const char *name;
>       uint32_t level;
>       uint32_t vendor1, vendor2, vendor3;
> @@ -116,6 +187,7 @@ typedef struct x86_def_t {
>       uint32_t xlevel;
>       char model_id[48];
>       int vendor_override;
> +    uint32_t flags;
>   } x86_def_t;
>
>   #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
> @@ -129,7 +201,14 @@ typedef struct x86_def_t {
>             CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
>             CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
>             CPUID_PAE | CPUID_SEP | CPUID_APIC)
> -static x86_def_t x86_defs[] = {
> +
> +/* maintains list of cpu model definitions
> + */
> +static x86_def_t *x86_defs = {NULL};
> +
> +/* built-in cpu model definitions (deprecated)
> + */
> +static x86_def_t builtin_x86_defs[] = {
>   #ifdef TARGET_X86_64
>       {
>           .name = "qemu64",
> @@ -334,9 +413,6 @@ static x86_def_t x86_defs[] = {
>       },
>   };
>
> -static void host_cpuid(uint32_t function, uint32_t count, uint32_t *eax,
> -                               uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
> -
>   static int cpu_x86_fill_model_id(char *str)
>   {
>       uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
> @@ -382,6 +458,51 @@ static int cpu_x86_fill_host(x86_def_t *x86_cpu_def)
>       return 0;
>   }
>
> +static int unavailable_host_feature(struct model_features_t *f, uint32_t mask)
> +{
> +    int i;
> +
> +    for (i = 0; i<  32; ++i)
> +        if (1<<  i&  mask) {
> +            fprintf(stderr, "warning: host cpuid %04x_%04x lacks requested"
> +                " flag '%s' [0x%08x]\n",
> +                f->cpuid>>  16, f->cpuid&  0xffff,
> +                f->flag_names[i] ? f->flag_names[i] : "[reserved]", mask);
> +            break;
> +        }
> +    return 0;
> +}
> +
> +/* best effort attempt to inform user requested cpu flags aren't making
> + * their way to the guest.  Note: ft[].check_feat ideally should be
> + * specified via a guest_def field to suppress report of extraneous flags.
> + */
> +static int check_features_against_host(x86_def_t *guest_def)
> +{
> +    x86_def_t host_def;
> +    uint32_t mask;
> +    int rv, i;
> +    struct model_features_t ft[] = {
> +        {&guest_def->features,&host_def.features,
> +            ~0, feature_name, 0x00000000},
> +        {&guest_def->ext_features,&host_def.ext_features,
> +            ~CPUID_EXT_HYPERVISOR, ext_feature_name, 0x00000001},
> +        {&guest_def->ext2_features,&host_def.ext2_features,
> +            ~PPRO_FEATURES, ext2_feature_name, 0x80000000},
> +        {&guest_def->ext3_features,&host_def.ext3_features,
> +            ~CPUID_EXT3_SVM, ext3_feature_name, 0x80000001}};
> +
> +    cpu_x86_fill_host(&host_def);
> +    for (rv = 0, i = 0; i<  sizeof (ft) / sizeof (ft[0]); ++i)
> +        for (mask = 1; mask; mask<<= 1)
> +            if (ft[i].check_feat&  mask&&  *ft[i].guest_feat&  mask&&
> +                !(*ft[i].host_feat&  mask)) {
> +                    unavailable_host_feature(&ft[i], mask);
> +                    rv = 1;
> +                }
> +    return rv;
> +}
> +
>   static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
>   {
>       unsigned int i;
> @@ -393,13 +514,9 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
>       uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 0, minus_ext3_features = 0, minus_kvm_features = 0;
>       uint32_t numvalue;
>
> -    def = NULL;
> -    for (i = 0; i<  ARRAY_SIZE(x86_defs); i++) {
> -        if (strcmp(name, x86_defs[i].name) == 0) {
> -            def =&x86_defs[i];
> +    for (def = x86_defs; def; def = def->next)
> +        if (!strcmp(name, def->name))
>               break;
> -        }
> -    }
>       if (kvm_enabled()&&  strcmp(name, "host") == 0) {
>           cpu_x86_fill_host(x86_cpu_def);
>       } else if (!def) {
> @@ -488,6 +605,10 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
>                   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 {
>               fprintf(stderr, "feature string `%s' not in format (+feature|-feature|feature=xyz)\n", featurestr);
>               goto error;
> @@ -504,6 +625,10 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
>       x86_cpu_def->ext2_features&= ~minus_ext2_features;
>       x86_cpu_def->ext3_features&= ~minus_ext3_features;
>       x86_cpu_def->kvm_features&= ~minus_kvm_features;
> +    if (check_cpuid) {
> +        if (check_features_against_host(x86_cpu_def)&&  enforce_cpuid)
> +            goto error;
> +    }
>       free(s);
>       return 0;
>
> @@ -512,12 +637,97 @@ error:
>       return -1;
>   }
>
> -void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...))
> +/* generate a composite string into buf of all cpuid names in featureset
> + * selected by fbits.  indicate truncation at bufsize in the event of overflow.
> + * if flags, suppress names undefined in featureset.
> + */
> +static void listflags(char *buf, int bufsize, uint32_t fbits,
> +    const char **featureset, uint32_t flags)
>   {
> -    unsigned int i;
> +    const char **p =&featureset[31];
> +    char *q, *b, bit;
> +    int nc;
> +
> +    b = 4<= bufsize ? buf + (bufsize -= 3) - 1 : NULL;
> +    *buf = '\0';
> +    for (q = buf, bit = 31; fbits&&  bufsize; --p, fbits&= ~(1<<  bit), --bit)
> +        if (fbits&  1<<  bit&&  (*p || !flags)) {
> +            if (*p)
> +                nc = snprintf(q, bufsize, "%s%s", q == buf ? "" : " ", *p);
> +            else
> +                nc = snprintf(q, bufsize, "%s[%d]", q == buf ? "" : " ", bit);
> +            if (bufsize<= nc) {
> +                if (b)
> +                    sprintf(b, "...");
> +                return;
> +            }
> +            q += nc;
> +            bufsize -= nc;
> +        }
> +}
>
> -    for (i = 0; i<  ARRAY_SIZE(x86_defs); i++)
> -        (*cpu_fprintf)(f, "x86 %16s\n", x86_defs[i].name);
> +/* generate CPU information:
> + * -?        list model names
> + * -?model   list model names/IDs
> + * -?dump    output all model (x86_def_t) data
> + * -?cpuid   list all recognized cpuid flag names
> + */
> +void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
> +                  const char *optarg)
> +{
> +    unsigned char model = !strcmp("?model", optarg);
> +    unsigned char dump = !strcmp("?dump", optarg);
> +    unsigned char cpuid = !strcmp("?cpuid", optarg);
> +    x86_def_t *def;
> +    char buf[256];
> +
> +    if (cpuid) {
> +        (*cpu_fprintf)(f, "Recognized CPUID flags:\n");
> +        listflags(buf, sizeof (buf), (uint32_t)~0, feature_name, 1);
> +        (*cpu_fprintf)(f, "  f_edx: %s\n", buf);
> +        listflags(buf, sizeof (buf), (uint32_t)~0, ext_feature_name, 1);
> +        (*cpu_fprintf)(f, "  f_ecx: %s\n", buf);
> +        listflags(buf, sizeof (buf), (uint32_t)~0, ext2_feature_name, 1);
> +        (*cpu_fprintf)(f, "  extf_edx: %s\n", buf);
> +        listflags(buf, sizeof (buf), (uint32_t)~0, ext3_feature_name, 1);
> +        (*cpu_fprintf)(f, "  extf_ecx: %s\n", buf);
> +        return;
> +    }
> +    for (def = x86_defs; def; def = def->next) {
> +        snprintf(buf, sizeof (buf), def->flags ? "[%s]": "%s", def->name);
> +        if (model || dump) {
> +            (*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf, def->model_id);
> +        } else {
> +            (*cpu_fprintf)(f, "x86 %16s\n", buf);
> +        }
> +        if (dump) {
> +            memcpy(buf,&def->vendor1, sizeof (def->vendor1));
> +            memcpy(buf + 4,&def->vendor2, sizeof (def->vendor2));
> +            memcpy(buf + 8,&def->vendor3, sizeof (def->vendor3));
> +            buf[12] = '\0';
> +            (*cpu_fprintf)(f,
> +                "  family %d model %d stepping %d level %d xlevel 0x%x"
> +                " vendor \"%s\"\n",
> +                def->family, def->model, def->stepping, def->level,
> +                def->xlevel, buf);
> +            listflags(buf, sizeof (buf), def->features, feature_name, 0);
> +            (*cpu_fprintf)(f, "  feature_edx %08x (%s)\n", def->features,
> +                buf);
> +            listflags(buf, sizeof (buf), def->ext_features, ext_feature_name,
> +                0);
> +            (*cpu_fprintf)(f, "  feature_ecx %08x (%s)\n", def->ext_features,
> +                buf);
> +            listflags(buf, sizeof (buf), def->ext2_features, ext2_feature_name,
> +                0);
> +            (*cpu_fprintf)(f, "  extfeature_edx %08x (%s)\n",
> +                def->ext2_features, buf);
> +            listflags(buf, sizeof (buf), def->ext3_features, ext3_feature_name,
> +                0);
> +            (*cpu_fprintf)(f, "  extfeature_ecx %08x (%s)\n",
> +                def->ext3_features, buf);
> +            (*cpu_fprintf)(f, "\n");
> +        }
> +    }
>   }
>
>   static int cpu_x86_register (CPUX86State *env, const char *cpu_model)
> @@ -566,6 +776,124 @@ static int cpu_x86_register (CPUX86State *env, const char *cpu_model)
>       return 0;
>   }
>
> +/* copy vendor id string to 32 bit register, nul pad as needed
> + */
> +static void cpyid(const char *s, uint32_t *id)
> +{
> +    char *d = (char *)id;
> +    char i;
> +
> +    for (i = sizeof (*id); i--; )
> +        *d++ = *s ? *s++ : '\0';
> +}
> +
> +/* interpret radix and convert from string to arbitrary scalar,
> + * otherwise flag failure
> + */
> +#define setscalar(pval, str, perr)                      \
> +{                                                       \
> +    char *pend;                                         \
> +    unsigned long ul;                                   \
> +                                                        \
> +    ul = strtoul(str,&pend, 0);                        \
> +    *str&&  !*pend ? (*pval = ul) : (*perr = 1);        \
> +}
> +
> +/* map cpuid options to feature bits, otherwise return failure
> + * (option tags in *str are delimited by whitespace)
> + */
> +static void setfeatures(uint32_t *pval, const char *str,
> +    const char **featureset, int *perr)
> +{
> +    const char *p, *q;
> +
> +    for (q = p = str; *p || *q; q = p) {
> +        while (iswhite(*p))
> +            q = ++p;
> +        while (*p&&  !iswhite(*p))
> +            ++p;
> +        if (!*q&&  !*p)
> +            return;
> +        if (!lookup_feature(pval, q, p, featureset)) {
> +            fprintf(stderr, "error: feature \"%.*s\" not available in set\n",
> +                (int)(p - q), q);
> +            *perr = 1;
> +            return;
> +        }
> +    }
> +}
> +
> +/* map config file options to x86_def_t form
> + */
> +static int cpudef_setfield(const char *name, const char *str, void *opaque)
> +{
> +    x86_def_t *def = opaque;
> +    int err = 0;
> +
> +    if (!strcmp(name, "name")) {
> +        def->name = strdup(str);
> +    } else if (!strcmp(name, "model_id")) {
> +        strncpy(def->model_id, str, sizeof (def->model_id));
> +    } else if (!strcmp(name, "level")) {
> +        setscalar(&def->level, str,&err)
> +    } else if (!strcmp(name, "vendor")) {
> +        cpyid(&str[0],&def->vendor1);
> +        cpyid(&str[4],&def->vendor2);
> +        cpyid(&str[8],&def->vendor3);
> +    } else if (!strcmp(name, "family")) {
> +        setscalar(&def->family, str,&err)
> +    } else if (!strcmp(name, "model")) {
> +        setscalar(&def->model, str,&err)
> +    } else if (!strcmp(name, "stepping")) {
> +        setscalar(&def->stepping, str,&err)
> +    } else if (!strcmp(name, "feature_edx")) {
> +        setfeatures(&def->features, str, feature_name,&err);
> +    } else if (!strcmp(name, "feature_ecx")) {
> +        setfeatures(&def->ext_features, str, ext_feature_name,&err);
> +    } else if (!strcmp(name, "extfeature_edx")) {
> +        setfeatures(&def->ext2_features, str, ext2_feature_name,&err);
> +    } else if (!strcmp(name, "extfeature_ecx")) {
> +        setfeatures(&def->ext3_features, str, ext3_feature_name,&err);
> +    } else if (!strcmp(name, "xlevel")) {
> +        setscalar(&def->xlevel, str,&err)
> +    } else {
> +        fprintf(stderr, "error: unknown option [%s = %s]\n", name, str);
> +        return (1);
> +    }
> +    if (err) {
> +        fprintf(stderr, "error: bad option value [%s = %s]\n", name, str);
> +        return (1);
> +    }
> +    return (0);
> +}
> +
> +/* register config file entry as x86_def_t
> + */
> +static int cpudef_register(QemuOpts *opts, void *opaque)
> +{
> +    x86_def_t *def = qemu_mallocz(sizeof (x86_def_t));
> +
> +    qemu_opt_foreach(opts, cpudef_setfield, def, 1);
> +    def->next = x86_defs;
> +    x86_defs = def;
> +    return (0);
> +}
> +
> +/* register "cpudef" models defined in configuration file after preloading
> + * built-in definitions
> + */
> +void cpudef_setup(void)
> +{
> +    int i;
> +
> +    for (i = 0; i<  ARRAY_SIZE(builtin_x86_defs); ++i) {
> +        builtin_x86_defs[i].next = x86_defs;
> +        builtin_x86_defs[i].flags = 1;
> +        x86_defs =&builtin_x86_defs[i];
> +    }
> +    qemu_opts_foreach(&qemu_cpudef_opts, cpudef_register, NULL, 0);
> +}
> +
>   /* NOTE: must be called outside the CPU execute loop */
>   void cpu_reset(CPUX86State *env)
>   {
> diff --git a/vl.c b/vl.c
> index 6f1e1ab..d6cd62c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4851,6 +4851,7 @@ int main(int argc, char **argv, char **envp)
>               fclose(fp);
>           }
>       }
> +    cpudef_setup();
>
>       /* second pass of option parsing */
>       optind = 1;
> @@ -4884,8 +4885,10 @@ int main(int argc, char **argv, char **envp)
>                   /* hw initialization will check this */
>                   if (*optarg == '?') {
>   /* XXX: implement xxx_cpu_list for targets that still miss it */
> -#if defined(cpu_list)
> -                    cpu_list(stdout,&fprintf);
> +#if defined(cpu_list_id)
> +                    cpu_list_id(stdout,&fprintf, optarg);
> +#elif defined(cpu_list)
> +                    cpu_list(stdout,&fprintf);	        /* deprecated */
>   #endif
>                       exit(0);
>                   } else {
>    

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH] Add cpu model configuration support.. (resend)
  2010-02-10 20:00   ` Anthony Liguori
@ 2010-02-14  6:52     ` john cooper
  -1 siblings, 0 replies; 24+ messages in thread
From: john cooper @ 2010-02-14  6:52 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: KVM list, qemu-devel, Przywara, Andre, john.cooper

Anthony Liguori wrote:
> On 02/01/2010 01:02 PM, john cooper wrote:
>> [target-x86_64.conf was unintentionally omitted from the earlier patch]
>>
>> This is a reimplementation of prior versions which adds
>> the ability to define cpu models for contemporary processors.
>> The added models are likewise selected via -cpu<name>,
>> and are intended to displace the existing convention
>> of "-cpu qemu64" augmented with a series of feature flags
>>    
> 
> This breaks the arm-softmmu build.

Ugh, does indeed as well as a few other builds.
Updated patch follows.

-john

-- 
john.cooper@redhat.com

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH] Add cpu model configuration support.. (resend)
@ 2010-02-14  6:52     ` john cooper
  0 siblings, 0 replies; 24+ messages in thread
From: john cooper @ 2010-02-14  6:52 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: john.cooper, Przywara, Andre, qemu-devel, KVM list

Anthony Liguori wrote:
> On 02/01/2010 01:02 PM, john cooper wrote:
>> [target-x86_64.conf was unintentionally omitted from the earlier patch]
>>
>> This is a reimplementation of prior versions which adds
>> the ability to define cpu models for contemporary processors.
>> The added models are likewise selected via -cpu<name>,
>> and are intended to displace the existing convention
>> of "-cpu qemu64" augmented with a series of feature flags
>>    
> 
> This breaks the arm-softmmu build.

Ugh, does indeed as well as a few other builds.
Updated patch follows.

-john

-- 
john.cooper@redhat.com

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH] Add optional dump of default config file paths..
  2010-02-06 18:59   ` [Qemu-devel] " john cooper
  (?)
  (?)
@ 2010-06-09  8:05   ` john cooper
  2010-06-14 17:01     ` Anthony Liguori
  -1 siblings, 1 reply; 24+ messages in thread
From: john cooper @ 2010-06-09  8:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: john.cooper

This patch adds the ability to determine the build-configured
runtime "config file" paths from the command line.  After
support for cpu model definitions were added to the default
runtime "target-" config file, testing of this feature has
tripped over an unintentionally mis-installed config file
enough to indicate some help is needed resolving such issues.

As no general "verbose" flag is currently available, specifying
"-readconfig ?" on the command line will maintain the default
(config file) disposition but additionally emit diagnostic info.
This mode is optional, otherwise the existing startup behavior
is identical.

Signed-off-by: john cooper <john.cooper@redhat.com>
---

diff --git a/qemu-config.c b/qemu-config.c
index 5a4e61b..a490603 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -518,21 +518,29 @@ out:
     return res;
 }
 
-int qemu_read_config_file(const char *filename)
+/* attempt to open and parse config file, report problems if vflag
+ */
+int qemu_read_config_file(const char *filename, int vflag)
 {
     FILE *f = fopen(filename, "r");
-    int ret;
+    int rv = 0;
 
     if (f == NULL) {
-        return -errno;
+        rv = -errno;
     }
-
-    ret = qemu_config_parse(f, vm_config_groups, filename);
-    fclose(f);
-
-    if (ret == 0) {
-        return 0;
-    } else {
-        return -EINVAL;
+    else if (qemu_config_parse(f, vm_config_groups, filename) != 0) {
+        rv = -EINVAL;
+    }
+    else if (vflag) {
+        fprintf(stderr, "read config file %s\n", filename);
     }
+    if (f) {
+        fclose(f);
+    }
+    if (rv && vflag) {
+        fprintf(stderr, "can't read config file %s: %s\n",
+                filename, strerror(-rv));
+    }
+    return rv;
 }
+
diff --git a/qemu-config.h b/qemu-config.h
index dca69d4..2e15556 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -23,6 +23,6 @@ void qemu_add_globals(void);
 void qemu_config_write(FILE *fp);
 int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname);
 
-int qemu_read_config_file(const char *filename);
+int qemu_read_config_file(const char *filename, int vflag);
 
 #endif /* QEMU_CONFIG_H */
diff --git a/vl.c b/vl.c
index 7121cd0..23c7276 100644
--- a/vl.c
+++ b/vl.c
@@ -2582,6 +2582,7 @@ int main(int argc, char **argv, char **envp)
 #endif
     int show_vnc_port = 0;
     int defconfig = 1;
+    int defconfig_verbose = 0;	
 
     error_set_progname(argv[0]);
 
@@ -2657,6 +2658,10 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_nodefconfig:
                 defconfig=0;
                 break;
+            case QEMU_OPTION_readconfig:
+                if (!strcmp(optarg, "?"))
+                    defconfig_verbose = 1;
+                break;
             }
         }
     }
@@ -2664,12 +2669,14 @@ int main(int argc, char **argv, char **envp)
     if (defconfig) {
         int ret;
 
-        ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf");
+        ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf",
+                                    defconfig_verbose);
         if (ret < 0 && ret != -ENOENT) {
             exit(1);
         }
 
-        ret = qemu_read_config_file(arch_config_name);
+        ret = qemu_read_config_file(arch_config_name,
+                                    defconfig_verbose);
         if (ret < 0 && ret != -ENOENT) {
             exit(1);
         }
@@ -3386,15 +3393,9 @@ int main(int argc, char **argv, char **envp)
                 xen_mode = XEN_ATTACH;
                 break;
             case QEMU_OPTION_readconfig:
-                {
-                    int ret = qemu_read_config_file(optarg);
-                    if (ret < 0) {
-                        fprintf(stderr, "read config %s: %s\n", optarg,
-                            strerror(-ret));
-                        exit(1);
-                    }
-                    break;
-                }
+                if (!defconfig_verbose && qemu_read_config_file(optarg, 1) < 0)
+                    exit(1);
+                break;
             case QEMU_OPTION_writeconfig:
                 {
                     FILE *fp;

-- 
john.cooper@redhat.com

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH] Add optional dump of default config file paths..
  2010-06-09  8:05   ` [Qemu-devel] [PATCH] Add optional dump of default config file paths john cooper
@ 2010-06-14 17:01     ` Anthony Liguori
  2010-06-14 17:07       ` Daniel P. Berrange
  2010-06-14 17:59       ` john cooper
  0 siblings, 2 replies; 24+ messages in thread
From: Anthony Liguori @ 2010-06-14 17:01 UTC (permalink / raw)
  To: john cooper; +Cc: qemu-devel

On 06/09/2010 03:05 AM, john cooper wrote:
> This patch adds the ability to determine the build-configured
> runtime "config file" paths from the command line.  After
> support for cpu model definitions were added to the default
> runtime "target-" config file, testing of this feature has
> tripped over an unintentionally mis-installed config file
> enough to indicate some help is needed resolving such issues.
>
> As no general "verbose" flag is currently available, specifying
> "-readconfig ?" on the command line will maintain the default
> (config file) disposition but additionally emit diagnostic info.
> This mode is optional, otherwise the existing startup behavior
> is identical.
>    

I assume this is something requested by libvirt?  I'd prefer we support 
this via Daniel's capabilities patchset instead of adding yet another 
hidden help output and having libvirt parse that output.

Regards,

Anthony Liguori

> Signed-off-by: john cooper<john.cooper@redhat.com>
> ---
>
> diff --git a/qemu-config.c b/qemu-config.c
> index 5a4e61b..a490603 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -518,21 +518,29 @@ out:
>       return res;
>   }
>
> -int qemu_read_config_file(const char *filename)
> +/* attempt to open and parse config file, report problems if vflag
> + */
> +int qemu_read_config_file(const char *filename, int vflag)
>   {
>       FILE *f = fopen(filename, "r");
> -    int ret;
> +    int rv = 0;
>
>       if (f == NULL) {
> -        return -errno;
> +        rv = -errno;
>       }
> -
> -    ret = qemu_config_parse(f, vm_config_groups, filename);
> -    fclose(f);
> -
> -    if (ret == 0) {
> -        return 0;
> -    } else {
> -        return -EINVAL;
> +    else if (qemu_config_parse(f, vm_config_groups, filename) != 0) {
> +        rv = -EINVAL;
> +    }
> +    else if (vflag) {
> +        fprintf(stderr, "read config file %s\n", filename);
>       }
> +    if (f) {
> +        fclose(f);
> +    }
> +    if (rv&&  vflag) {
> +        fprintf(stderr, "can't read config file %s: %s\n",
> +                filename, strerror(-rv));
> +    }
> +    return rv;
>   }
> +
> diff --git a/qemu-config.h b/qemu-config.h
> index dca69d4..2e15556 100644
> --- a/qemu-config.h
> +++ b/qemu-config.h
> @@ -23,6 +23,6 @@ void qemu_add_globals(void);
>   void qemu_config_write(FILE *fp);
>   int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname);
>
> -int qemu_read_config_file(const char *filename);
> +int qemu_read_config_file(const char *filename, int vflag);
>
>   #endif /* QEMU_CONFIG_H */
> diff --git a/vl.c b/vl.c
> index 7121cd0..23c7276 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2582,6 +2582,7 @@ int main(int argc, char **argv, char **envp)
>   #endif
>       int show_vnc_port = 0;
>       int defconfig = 1;
> +    int defconfig_verbose = 0;	
>
>       error_set_progname(argv[0]);
>
> @@ -2657,6 +2658,10 @@ int main(int argc, char **argv, char **envp)
>               case QEMU_OPTION_nodefconfig:
>                   defconfig=0;
>                   break;
> +            case QEMU_OPTION_readconfig:
> +                if (!strcmp(optarg, "?"))
> +                    defconfig_verbose = 1;
> +                break;
>               }
>           }
>       }
> @@ -2664,12 +2669,14 @@ int main(int argc, char **argv, char **envp)
>       if (defconfig) {
>           int ret;
>
> -        ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf");
> +        ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf",
> +                                    defconfig_verbose);
>           if (ret<  0&&  ret != -ENOENT) {
>               exit(1);
>           }
>
> -        ret = qemu_read_config_file(arch_config_name);
> +        ret = qemu_read_config_file(arch_config_name,
> +                                    defconfig_verbose);
>           if (ret<  0&&  ret != -ENOENT) {
>               exit(1);
>           }
> @@ -3386,15 +3393,9 @@ int main(int argc, char **argv, char **envp)
>                   xen_mode = XEN_ATTACH;
>                   break;
>               case QEMU_OPTION_readconfig:
> -                {
> -                    int ret = qemu_read_config_file(optarg);
> -                    if (ret<  0) {
> -                        fprintf(stderr, "read config %s: %s\n", optarg,
> -                            strerror(-ret));
> -                        exit(1);
> -                    }
> -                    break;
> -                }
> +                if (!defconfig_verbose&&  qemu_read_config_file(optarg, 1)<  0)
> +                    exit(1);
> +                break;
>               case QEMU_OPTION_writeconfig:
>                   {
>                       FILE *fp;
>
>    

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH] Add optional dump of default config file paths..
  2010-06-14 17:01     ` Anthony Liguori
@ 2010-06-14 17:07       ` Daniel P. Berrange
  2010-06-14 17:59       ` john cooper
  1 sibling, 0 replies; 24+ messages in thread
From: Daniel P. Berrange @ 2010-06-14 17:07 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: john cooper, qemu-devel

On Mon, Jun 14, 2010 at 12:01:42PM -0500, Anthony Liguori wrote:
> On 06/09/2010 03:05 AM, john cooper wrote:
> >This patch adds the ability to determine the build-configured
> >runtime "config file" paths from the command line.  After
> >support for cpu model definitions were added to the default
> >runtime "target-" config file, testing of this feature has
> >tripped over an unintentionally mis-installed config file
> >enough to indicate some help is needed resolving such issues.
> >
> >As no general "verbose" flag is currently available, specifying
> >"-readconfig ?" on the command line will maintain the default
> >(config file) disposition but additionally emit diagnostic info.
> >This mode is optional, otherwise the existing startup behavior
> >is identical.
> >   
> 
> I assume this is something requested by libvirt?  I'd prefer we support 
> this via Daniel's capabilities patchset instead of adding yet another 
> hidden help output and having libvirt parse that output.

No, libvirt has no need for this patch. We now forcably disable
all default configs with -nodefconfig.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH] Add optional dump of default config file paths..
  2010-06-14 17:01     ` Anthony Liguori
  2010-06-14 17:07       ` Daniel P. Berrange
@ 2010-06-14 17:59       ` john cooper
  2010-06-14 19:13         ` Anthony Liguori
  1 sibling, 1 reply; 24+ messages in thread
From: john cooper @ 2010-06-14 17:59 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: john.cooper, qemu-devel

Anthony Liguori wrote:
> On 06/09/2010 03:05 AM, john cooper wrote:
>> This patch adds the ability to determine the build-configured
>> runtime "config file" paths from the command line.  After
>> support for cpu model definitions were added to the default
>> runtime "target-" config file, testing of this feature has
>> tripped over an unintentionally mis-installed config file
>> enough to indicate some help is needed resolving such issues.
>>
>> As no general "verbose" flag is currently available, specifying
>> "-readconfig ?" on the command line will maintain the default
>> (config file) disposition but additionally emit diagnostic info.
>> This mode is optional, otherwise the existing startup behavior
>> is identical.
>>    
> 
> I assume this is something requested by libvirt?

Not requested by libvirt but rather intended for the qemu
CLI facing user.  The alternatives of trying to puzzle
out built-in config file paths via strace or strings when
config problems surface is rather awkward.  This also
seems a general need for test of config file related
functionality.

Thanks,

-john

> I'd prefer we support
> this via Daniel's capabilities patchset instead of adding yet another
> hidden help output and having libvirt parse that output.
> 
> Regards,
> 
> Anthony Liguori
> 
>> Signed-off-by: john cooper<john.cooper@redhat.com>
>> ---
>>
>> diff --git a/qemu-config.c b/qemu-config.c
>> index 5a4e61b..a490603 100644
>> --- a/qemu-config.c
>> +++ b/qemu-config.c
>> @@ -518,21 +518,29 @@ out:
>>       return res;
>>   }
>>
>> -int qemu_read_config_file(const char *filename)
>> +/* attempt to open and parse config file, report problems if vflag
>> + */
>> +int qemu_read_config_file(const char *filename, int vflag)
>>   {
>>       FILE *f = fopen(filename, "r");
>> -    int ret;
>> +    int rv = 0;
>>
>>       if (f == NULL) {
>> -        return -errno;
>> +        rv = -errno;
>>       }
>> -
>> -    ret = qemu_config_parse(f, vm_config_groups, filename);
>> -    fclose(f);
>> -
>> -    if (ret == 0) {
>> -        return 0;
>> -    } else {
>> -        return -EINVAL;
>> +    else if (qemu_config_parse(f, vm_config_groups, filename) != 0) {
>> +        rv = -EINVAL;
>> +    }
>> +    else if (vflag) {
>> +        fprintf(stderr, "read config file %s\n", filename);
>>       }
>> +    if (f) {
>> +        fclose(f);
>> +    }
>> +    if (rv&&  vflag) {
>> +        fprintf(stderr, "can't read config file %s: %s\n",
>> +                filename, strerror(-rv));
>> +    }
>> +    return rv;
>>   }
>> +
>> diff --git a/qemu-config.h b/qemu-config.h
>> index dca69d4..2e15556 100644
>> --- a/qemu-config.h
>> +++ b/qemu-config.h
>> @@ -23,6 +23,6 @@ void qemu_add_globals(void);
>>   void qemu_config_write(FILE *fp);
>>   int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char
>> *fname);
>>
>> -int qemu_read_config_file(const char *filename);
>> +int qemu_read_config_file(const char *filename, int vflag);
>>
>>   #endif /* QEMU_CONFIG_H */
>> diff --git a/vl.c b/vl.c
>> index 7121cd0..23c7276 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2582,6 +2582,7 @@ int main(int argc, char **argv, char **envp)
>>   #endif
>>       int show_vnc_port = 0;
>>       int defconfig = 1;
>> +    int defconfig_verbose = 0;   
>>
>>       error_set_progname(argv[0]);
>>
>> @@ -2657,6 +2658,10 @@ int main(int argc, char **argv, char **envp)
>>               case QEMU_OPTION_nodefconfig:
>>                   defconfig=0;
>>                   break;
>> +            case QEMU_OPTION_readconfig:
>> +                if (!strcmp(optarg, "?"))
>> +                    defconfig_verbose = 1;
>> +                break;
>>               }
>>           }
>>       }
>> @@ -2664,12 +2669,14 @@ int main(int argc, char **argv, char **envp)
>>       if (defconfig) {
>>           int ret;
>>
>> -        ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf");
>> +        ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf",
>> +                                    defconfig_verbose);
>>           if (ret<  0&&  ret != -ENOENT) {
>>               exit(1);
>>           }
>>
>> -        ret = qemu_read_config_file(arch_config_name);
>> +        ret = qemu_read_config_file(arch_config_name,
>> +                                    defconfig_verbose);
>>           if (ret<  0&&  ret != -ENOENT) {
>>               exit(1);
>>           }
>> @@ -3386,15 +3393,9 @@ int main(int argc, char **argv, char **envp)
>>                   xen_mode = XEN_ATTACH;
>>                   break;
>>               case QEMU_OPTION_readconfig:
>> -                {
>> -                    int ret = qemu_read_config_file(optarg);
>> -                    if (ret<  0) {
>> -                        fprintf(stderr, "read config %s: %s\n", optarg,
>> -                            strerror(-ret));
>> -                        exit(1);
>> -                    }
>> -                    break;
>> -                }
>> +                if (!defconfig_verbose&& 
>> qemu_read_config_file(optarg, 1)<  0)
>> +                    exit(1);
>> +                break;
>>               case QEMU_OPTION_writeconfig:
>>                   {
>>                       FILE *fp;
>>
>>    
> 


-- 
john.cooper@redhat.com

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH] Add optional dump of default config file paths..
  2010-06-14 17:59       ` john cooper
@ 2010-06-14 19:13         ` Anthony Liguori
  0 siblings, 0 replies; 24+ messages in thread
From: Anthony Liguori @ 2010-06-14 19:13 UTC (permalink / raw)
  To: john cooper; +Cc: qemu-devel

On 06/14/2010 12:59 PM, john cooper wrote:
> Anthony Liguori wrote:
>    
>> On 06/09/2010 03:05 AM, john cooper wrote:
>>      
>>> This patch adds the ability to determine the build-configured
>>> runtime "config file" paths from the command line.  After
>>> support for cpu model definitions were added to the default
>>> runtime "target-" config file, testing of this feature has
>>> tripped over an unintentionally mis-installed config file
>>> enough to indicate some help is needed resolving such issues.
>>>
>>> As no general "verbose" flag is currently available, specifying
>>> "-readconfig ?" on the command line will maintain the default
>>> (config file) disposition but additionally emit diagnostic info.
>>> This mode is optional, otherwise the existing startup behavior
>>> is identical.
>>>
>>>        
>> I assume this is something requested by libvirt?
>>      
> Not requested by libvirt but rather intended for the qemu
> CLI facing user.  The alternatives of trying to puzzle
> out built-in config file paths via strace or strings when
> config problems surface is rather awkward.  This also
> seems a general need for test of config file related
> functionality.
>    

I wouldn't mind spitting out the ./configure line in qemu -version like 
gcc does.  That has the benefit of being a bit more obviously discoverable.

Regards,

Anthony Liguori

> Thanks,
>
> -john
>
>    
>> I'd prefer we support
>> this via Daniel's capabilities patchset instead of adding yet another
>> hidden help output and having libvirt parse that output.
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>      
>>> Signed-off-by: john cooper<john.cooper@redhat.com>
>>> ---
>>>
>>> diff --git a/qemu-config.c b/qemu-config.c
>>> index 5a4e61b..a490603 100644
>>> --- a/qemu-config.c
>>> +++ b/qemu-config.c
>>> @@ -518,21 +518,29 @@ out:
>>>        return res;
>>>    }
>>>
>>> -int qemu_read_config_file(const char *filename)
>>> +/* attempt to open and parse config file, report problems if vflag
>>> + */
>>> +int qemu_read_config_file(const char *filename, int vflag)
>>>    {
>>>        FILE *f = fopen(filename, "r");
>>> -    int ret;
>>> +    int rv = 0;
>>>
>>>        if (f == NULL) {
>>> -        return -errno;
>>> +        rv = -errno;
>>>        }
>>> -
>>> -    ret = qemu_config_parse(f, vm_config_groups, filename);
>>> -    fclose(f);
>>> -
>>> -    if (ret == 0) {
>>> -        return 0;
>>> -    } else {
>>> -        return -EINVAL;
>>> +    else if (qemu_config_parse(f, vm_config_groups, filename) != 0) {
>>> +        rv = -EINVAL;
>>> +    }
>>> +    else if (vflag) {
>>> +        fprintf(stderr, "read config file %s\n", filename);
>>>        }
>>> +    if (f) {
>>> +        fclose(f);
>>> +    }
>>> +    if (rv&&   vflag) {
>>> +        fprintf(stderr, "can't read config file %s: %s\n",
>>> +                filename, strerror(-rv));
>>> +    }
>>> +    return rv;
>>>    }
>>> +
>>> diff --git a/qemu-config.h b/qemu-config.h
>>> index dca69d4..2e15556 100644
>>> --- a/qemu-config.h
>>> +++ b/qemu-config.h
>>> @@ -23,6 +23,6 @@ void qemu_add_globals(void);
>>>    void qemu_config_write(FILE *fp);
>>>    int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char
>>> *fname);
>>>
>>> -int qemu_read_config_file(const char *filename);
>>> +int qemu_read_config_file(const char *filename, int vflag);
>>>
>>>    #endif /* QEMU_CONFIG_H */
>>> diff --git a/vl.c b/vl.c
>>> index 7121cd0..23c7276 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -2582,6 +2582,7 @@ int main(int argc, char **argv, char **envp)
>>>    #endif
>>>        int show_vnc_port = 0;
>>>        int defconfig = 1;
>>> +    int defconfig_verbose = 0;
>>>
>>>        error_set_progname(argv[0]);
>>>
>>> @@ -2657,6 +2658,10 @@ int main(int argc, char **argv, char **envp)
>>>                case QEMU_OPTION_nodefconfig:
>>>                    defconfig=0;
>>>                    break;
>>> +            case QEMU_OPTION_readconfig:
>>> +                if (!strcmp(optarg, "?"))
>>> +                    defconfig_verbose = 1;
>>> +                break;
>>>                }
>>>            }
>>>        }
>>> @@ -2664,12 +2669,14 @@ int main(int argc, char **argv, char **envp)
>>>        if (defconfig) {
>>>            int ret;
>>>
>>> -        ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf");
>>> +        ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf",
>>> +                                    defconfig_verbose);
>>>            if (ret<   0&&   ret != -ENOENT) {
>>>                exit(1);
>>>            }
>>>
>>> -        ret = qemu_read_config_file(arch_config_name);
>>> +        ret = qemu_read_config_file(arch_config_name,
>>> +                                    defconfig_verbose);
>>>            if (ret<   0&&   ret != -ENOENT) {
>>>                exit(1);
>>>            }
>>> @@ -3386,15 +3393,9 @@ int main(int argc, char **argv, char **envp)
>>>                    xen_mode = XEN_ATTACH;
>>>                    break;
>>>                case QEMU_OPTION_readconfig:
>>> -                {
>>> -                    int ret = qemu_read_config_file(optarg);
>>> -                    if (ret<   0) {
>>> -                        fprintf(stderr, "read config %s: %s\n", optarg,
>>> -                            strerror(-ret));
>>> -                        exit(1);
>>> -                    }
>>> -                    break;
>>> -                }
>>> +                if (!defconfig_verbose&&
>>> qemu_read_config_file(optarg, 1)<   0)
>>> +                    exit(1);
>>> +                break;
>>>                case QEMU_OPTION_writeconfig:
>>>                    {
>>>                        FILE *fp;
>>>
>>>
>>>        
>>      
>
>    

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2010-06-14 19:13 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-02-01 19:02 [PATCH] Add cpu model configuration support.. (resend) john cooper
2010-02-01 19:02 ` [Qemu-devel] " john cooper
2010-02-02 10:17 ` Andre Przywara
2010-02-02 10:17   ` [Qemu-devel] " Andre Przywara
2010-02-02 11:07 ` Andre Przywara
2010-02-02 11:07   ` [Qemu-devel] " Andre Przywara
2010-02-02 19:34   ` john cooper
2010-02-02 19:34     ` [Qemu-devel] " john cooper
2010-02-06 18:59 ` [PATCH] Add assignment operation to config file parser john cooper
2010-02-06 18:59   ` [Qemu-devel] " john cooper
2010-02-07 16:24   ` Anthony Liguori
2010-02-07 16:24     ` Anthony Liguori
2010-02-08 13:21     ` Gerd Hoffmann
2010-02-08 16:00       ` john cooper
2010-02-08 16:00         ` john cooper
2010-06-09  8:05   ` [Qemu-devel] [PATCH] Add optional dump of default config file paths john cooper
2010-06-14 17:01     ` Anthony Liguori
2010-06-14 17:07       ` Daniel P. Berrange
2010-06-14 17:59       ` john cooper
2010-06-14 19:13         ` Anthony Liguori
2010-02-10 20:00 ` [Qemu-devel] [PATCH] Add cpu model configuration support.. (resend) Anthony Liguori
2010-02-10 20:00   ` Anthony Liguori
2010-02-14  6:52   ` john cooper
2010-02-14  6:52     ` john cooper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.