[Qemu-devel] [QEMU PATCH 0/3] versioned CPU models / per-machine-type aliases

[Qemu-devel] [QEMU PATCH 0/3] versioned CPU models / per-machine-type aliases

[Eduardo Habkost]

Hi,

This is the first try at a simple system to make the CPU model definitions
versioned (to allow them to get bug fixes while allowing migration from older
versions and keeping command-line compatibility), and per- machine-type aliases
for compatibility.

The lack of CPU model versioning is blocking multiple bug fixes that are
necessary on CPU model definitions, but can't be included today because they
would break migration.

Later, after this gets in (or at least gets some feedback), I plan to send a
proposal for a machine-friendly CPU feature / CPU model probing interface that
libvirt could use.

Eduardo Habkost (3):
  vl.c: extract qemu_machine_init() function
  per-machine-type CPU model alias system
  x86: pc: versioned CPU model names & compatibility aliases

 hw/boards.h                        |   13 +++++++++
 hw/pc_piix.c                       |   56 ++++++++++++++++++++++++++++++++++++
 sysconfigs/target/cpus-x86_64.conf |   18 ++++++------
 vl.c                               |   28 +++++++++++++++++-
 4 files changed, 105 insertions(+), 10 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [QEMU PATCH 1/3] vl.c: extract qemu_machine_init() function

[Eduardo Habkost]

Other code directly related to the board init function may be moved
there.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/boards.h |    7 +++++++
 vl.c        |   14 +++++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/hw/boards.h b/hw/boards.h
index 59c01d0..f20f5ab 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -34,6 +34,13 @@ typedef struct QEMUMachine {
 
 int qemu_register_machine(QEMUMachine *m);
 QEMUMachine *find_default_machine(void);
+void qemu_machine_init(QEMUMachine *machine,
+                       ram_addr_t ram_size,
+                       const char *boot_devices,
+                       const char *kernel_filename,
+                       const char *kernel_cmdline,
+                       const char *initrd_filename,
+                       const char *cpu_model);
 
 extern QEMUMachine *current_machine;
 
diff --git a/vl.c b/vl.c
index 8904db1..34cc145 100644
--- a/vl.c
+++ b/vl.c
@@ -1209,6 +1209,18 @@ QEMUMachine *find_default_machine(void)
     return NULL;
 }
 
+void qemu_machine_init(QEMUMachine *machine,
+                       ram_addr_t ram_size,
+                       const char *boot_devices,
+                       const char *kernel_filename,
+                       const char *kernel_cmdline,
+                       const char *initrd_filename,
+                       const char *cpu_model)
+{
+    machine->init(ram_size, boot_devices,
+                  kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
+}
+
 /***********************************************************/
 /* main execution loop */
 
@@ -3517,7 +3529,7 @@ int main(int argc, char **argv, char **envp)
 
     qdev_machine_init();
 
-    machine->init(ram_size, boot_devices,
+    qemu_machine_init(machine, ram_size, boot_devices,
                   kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
 
     cpu_synchronize_all_post_init();
-- 
1.7.10.4

[Qemu-devel] [QEMU PATCH 2/3] per-machine-type CPU model alias system

[Eduardo Habkost]

This allow QEMUMachine structs to contain a list of CPU model aliases,
used to keep command-line compatibility with older machine types, while
making CPU model fixes available on newer machine types.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/boards.h |    6 ++++++
 vl.c        |   14 ++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/hw/boards.h b/hw/boards.h
index f20f5ab..ad48399 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -12,6 +12,11 @@ typedef void QEMUMachineInitFunc(ram_addr_t ram_size,
                                  const char *initrd_filename,
                                  const char *cpu_model);
 
+
+typedef struct CPUModelAlias {
+    const char *alias, *cpu_model;
+} CPUModelAlias;
+
 typedef struct QEMUMachine {
     const char *name;
     const char *alias;
@@ -27,6 +32,7 @@ typedef struct QEMUMachine {
         no_sdcard:1;
     int is_default;
     const char *default_machine_opts;
+    struct CPUModelAlias *cpu_aliases;
     GlobalProperty *compat_props;
     struct QEMUMachine *next;
     const char *hw_version;
diff --git a/vl.c b/vl.c
index 34cc145..cd87e06 100644
--- a/vl.c
+++ b/vl.c
@@ -1209,6 +1209,19 @@ QEMUMachine *find_default_machine(void)
     return NULL;
 }
 
+static const char *qemu_machine_resolve_cpu_model(QEMUMachine *m,
+                                                  const char *cpu_model)
+{
+    if (cpu_model && m->cpu_aliases) {
+        CPUModelAlias *a;
+        for (a = m->cpu_aliases; a->alias; a++) {
+            if (!strcmp(cpu_model, a->alias))
+                return a->cpu_model;
+        }
+    }
+    return cpu_model;
+}
+
 void qemu_machine_init(QEMUMachine *machine,
                        ram_addr_t ram_size,
                        const char *boot_devices,
@@ -1217,6 +1230,7 @@ void qemu_machine_init(QEMUMachine *machine,
                        const char *initrd_filename,
                        const char *cpu_model)
 {
+    cpu_model = qemu_machine_resolve_cpu_model(machine, cpu_model);
     machine->init(ram_size, boot_devices,
                   kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
 }
-- 
1.7.10.4

[Qemu-devel] [QEMU PATCH 3/3] x86: pc: versioned CPU model names & compatibility aliases

[Eduardo Habkost]

This adds version number to CPU model names on the "pc-<version>"
machine-types, so we can create new models with bug fixes while keeping
compatibility when using older machine-types.

When naming the existing models, I used the last QEMU version where the
model was changed (see summary below), but by coincidence every single
one was changed on QEMU-1.1.

- Conroe, Penryn, Nehalem, Opteron_G1, Opteron_G2, Opteron_G3:
  added on 0.13, changed on 1.1
- Westmere, SandyBridge, Opteron_G4: added on 1.1

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/pc_piix.c                       |   56 ++++++++++++++++++++++++++++++++++++
 sysconfigs/target/cpus-x86_64.conf |   18 ++++++------
 2 files changed, 65 insertions(+), 9 deletions(-)

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 0c0096f..ef3840f 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -349,6 +349,18 @@ static void pc_xen_hvm_init(ram_addr_t ram_size,
 }
 #endif
 
+/* CPU aliases for pre-1.2 CPU models */
+#define V1_1_CPU_ALIASES  \
+    { "Conroe",      "Conroe-1.1" }, \
+    { "Penryn",      "Penryn-1.1" }, \
+    { "Nehalem",     "Nehalem-1.1" }, \
+    { "Westmere",    "Westmere-1.1" }, \
+    { "SandyBridge", "SandyBridge-1.1" }, \
+    { "Opteron_G1",  "Opteron_G1-1.1" }, \
+    { "Opteron_G2",  "Opteron_G2-1.1" }, \
+    { "Opteron_G3",  "Opteron_G3-1.1" }, \
+    { "Opteron_G4",  "Opteron_G4-1.1" },
+
 static QEMUMachine pc_machine_v1_2 = {
     .name = "pc-1.2",
     .alias = "pc",
@@ -356,6 +368,10 @@ static QEMUMachine pc_machine_v1_2 = {
     .init = pc_init_pci,
     .max_cpus = 255,
     .is_default = 1,
+    .cpu_aliases = (CPUModelAlias[]) {
+        V1_1_CPU_ALIASES
+        {NULL, NULL},
+    },
 };
 
 #define PC_COMPAT_1_1 \
@@ -386,6 +402,10 @@ static QEMUMachine pc_machine_v1_1 = {
         PC_COMPAT_1_1,
         { /* end of list */ }
     },
+    .cpu_aliases = (CPUModelAlias[]) {
+        V1_1_CPU_ALIASES
+        {NULL, NULL},
+    },
 };
 
 #define PC_COMPAT_1_0 \
@@ -422,6 +442,10 @@ static QEMUMachine pc_machine_v1_0 = {
         { /* end of list */ }
     },
     .hw_version = "1.0",
+    .cpu_aliases = (CPUModelAlias[]) {
+        V1_1_CPU_ALIASES
+        {NULL, NULL},
+    },
 };
 
 #define PC_COMPAT_0_15 \
@@ -437,6 +461,10 @@ static QEMUMachine pc_machine_v0_15 = {
         { /* end of list */ }
     },
     .hw_version = "0.15",
+    .cpu_aliases = (CPUModelAlias[]) {
+        V1_1_CPU_ALIASES
+        {NULL, NULL},
+    },
 };
 
 #define PC_COMPAT_0_14 \
@@ -478,6 +506,10 @@ static QEMUMachine pc_machine_v0_14 = {
         { /* end of list */ }
     },
     .hw_version = "0.14",
+    .cpu_aliases = (CPUModelAlias[]) {
+        V1_1_CPU_ALIASES
+        {NULL, NULL},
+    },
 };
 
 #define PC_COMPAT_0_13 \
@@ -515,6 +547,10 @@ static QEMUMachine pc_machine_v0_13 = {
         { /* end of list */ }
     },
     .hw_version = "0.13",
+    .cpu_aliases = (CPUModelAlias[]) {
+        V1_1_CPU_ALIASES
+        {NULL, NULL},
+    },
 };
 
 #define PC_COMPAT_0_12 \
@@ -548,6 +584,10 @@ static QEMUMachine pc_machine_v0_12 = {
         { /* end of list */ }
     },
     .hw_version = "0.12",
+    .cpu_aliases = (CPUModelAlias[]) {
+        V1_1_CPU_ALIASES
+        {NULL, NULL},
+    },
 };
 
 #define PC_COMPAT_0_11 \
@@ -581,6 +621,10 @@ static QEMUMachine pc_machine_v0_11 = {
         { /* end of list */ }
     },
     .hw_version = "0.11",
+    .cpu_aliases = (CPUModelAlias[]) {
+        V1_1_CPU_ALIASES
+        {NULL, NULL},
+    },
 };
 
 static QEMUMachine pc_machine_v0_10 = {
@@ -614,6 +658,10 @@ static QEMUMachine pc_machine_v0_10 = {
         { /* end of list */ }
     },
     .hw_version = "0.10",
+    .cpu_aliases = (CPUModelAlias[]) {
+        V1_1_CPU_ALIASES
+        {NULL, NULL},
+    },
 };
 
 static QEMUMachine isapc_machine = {
@@ -629,6 +677,10 @@ static QEMUMachine isapc_machine = {
         },
         { /* end of list */ }
     },
+    .cpu_aliases = (CPUModelAlias[]) {
+        V1_1_CPU_ALIASES
+        {NULL, NULL},
+    },
 };
 
 #ifdef CONFIG_XEN
@@ -638,6 +690,10 @@ static QEMUMachine xenfv_machine = {
     .init = pc_xen_hvm_init,
     .max_cpus = HVM_MAX_VCPUS,
     .default_machine_opts = "accel=xen",
+    .cpu_aliases = (CPUModelAlias[]) {
+        V1_1_CPU_ALIASES
+        {NULL, NULL},
+    },
 };
 #endif
 
diff --git a/sysconfigs/target/cpus-x86_64.conf b/sysconfigs/target/cpus-x86_64.conf
index cee0ea9..14c7891 100644
--- a/sysconfigs/target/cpus-x86_64.conf
+++ b/sysconfigs/target/cpus-x86_64.conf
@@ -1,7 +1,7 @@
 # x86 CPU MODELS
 
 [cpudef]
-   name = "Conroe"
+   name = "Conroe-1.1"
    level = "2"
    vendor = "GenuineIntel"
    family = "6"
@@ -15,7 +15,7 @@
    model_id = "Intel Celeron_4x0 (Conroe/Merom Class Core 2)"
 
 [cpudef]
-   name = "Penryn"
+   name = "Penryn-1.1"
    level = "2"
    vendor = "GenuineIntel"
    family = "6"
@@ -29,7 +29,7 @@
    model_id = "Intel Core 2 Duo P9xxx (Penryn Class Core 2)"
 
 [cpudef]
-   name = "Nehalem"
+   name = "Nehalem-1.1"
    level = "2"
    vendor = "GenuineIntel"
    family = "6"
@@ -43,7 +43,7 @@
    model_id = "Intel Core i7 9xx (Nehalem Class Core i7)"
 
 [cpudef]
-   name = "Westmere"
+   name = "Westmere-1.1"
    level = "11"
    vendor = "GenuineIntel"
    family = "6"
@@ -57,7 +57,7 @@
    model_id = "Westmere E56xx/L56xx/X56xx (Nehalem-C)"
 
 [cpudef]
-   name = "SandyBridge"
+   name = "SandyBridge-1.1"
    level = "0xd"
    vendor = "GenuineIntel"
    family = "6"
@@ -71,7 +71,7 @@
    model_id = "Intel Xeon E312xx (Sandy Bridge)"
 
 [cpudef]
-   name = "Opteron_G1"
+   name = "Opteron_G1-1.1"
    level = "5"
    vendor = "AuthenticAMD"
    family = "15"
@@ -85,7 +85,7 @@
    model_id = "AMD Opteron 240 (Gen 1 Class Opteron)"
 
 [cpudef]
-   name = "Opteron_G2"
+   name = "Opteron_G2-1.1"
    level = "5"
    vendor = "AuthenticAMD"
    family = "15"
@@ -99,7 +99,7 @@
    model_id = "AMD Opteron 22xx (Gen 2 Class Opteron)"
 
 [cpudef]
-   name = "Opteron_G3"
+   name = "Opteron_G3-1.1"
    level = "5"
    vendor = "AuthenticAMD"
    family = "15"
@@ -113,7 +113,7 @@
    model_id = "AMD Opteron 23xx (Gen 3 Class Opteron)"
 
 [cpudef]
-   name = "Opteron_G4"
+   name = "Opteron_G4-1.1"
    level = "0xd"
    vendor = "AuthenticAMD"
    family = "21"
-- 
1.7.10.4

Re: [Qemu-devel] [libvirt] [QEMU PATCH 1/3] vl.c: extract qemu_machine_init() function

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 692 bytes --]

On 07/25/2012 12:18 PM, Eduardo Habkost wrote:
> Other code directly related to the board init function may be moved
> there.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  hw/boards.h |    7 +++++++
>  vl.c        |   14 +++++++++++++-
>  2 files changed, 20 insertions(+), 1 deletion(-)
> 

>  
> -    machine->init(ram_size, boot_devices,
> +    qemu_machine_init(machine, ram_size, boot_devices,
>                    kernel_filename, kernel_cmdline, initrd_filename, cpu_model);

Doesn't this next line also need an indentation adjustment?

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [QEMU PATCH 2/3] per-machine-type CPU model alias system

[Andreas Färber]

Am 25.07.2012 20:18, schrieb Eduardo Habkost:
> This allow QEMUMachine structs to contain a list of CPU model aliases,
> used to keep command-line compatibility with older machine types, while
> making CPU model fixes available on newer machine types.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  hw/boards.h |    6 ++++++
>  vl.c        |   14 ++++++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/hw/boards.h b/hw/boards.h
> index f20f5ab..ad48399 100644
> --- a/hw/boards.h
> +++ b/hw/boards.h
> @@ -12,6 +12,11 @@ typedef void QEMUMachineInitFunc(ram_addr_t ram_size,
>                                   const char *initrd_filename,
>                                   const char *cpu_model);
>  
> +
> +typedef struct CPUModelAlias {
> +    const char *alias, *cpu_model;
> +} CPUModelAlias;
> +
>  typedef struct QEMUMachine {
>      const char *name;
>      const char *alias;
> @@ -27,6 +32,7 @@ typedef struct QEMUMachine {
>          no_sdcard:1;
>      int is_default;
>      const char *default_machine_opts;
> +    struct CPUModelAlias *cpu_aliases;
>      GlobalProperty *compat_props;
>      struct QEMUMachine *next;
>      const char *hw_version;
> diff --git a/vl.c b/vl.c
> index 34cc145..cd87e06 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1209,6 +1209,19 @@ QEMUMachine *find_default_machine(void)
>      return NULL;
>  }
>  
> +static const char *qemu_machine_resolve_cpu_model(QEMUMachine *m,
> +                                                  const char *cpu_model)
> +{
> +    if (cpu_model && m->cpu_aliases) {
> +        CPUModelAlias *a;
> +        for (a = m->cpu_aliases; a->alias; a++) {
> +            if (!strcmp(cpu_model, a->alias))
> +                return a->cpu_model;
> +        }
> +    }
> +    return cpu_model;
> +}
> +
>  void qemu_machine_init(QEMUMachine *machine,
>                         ram_addr_t ram_size,
>                         const char *boot_devices,
> @@ -1217,6 +1230,7 @@ void qemu_machine_init(QEMUMachine *machine,
>                         const char *initrd_filename,
>                         const char *cpu_model)
>  {
> +    cpu_model = qemu_machine_resolve_cpu_model(machine, cpu_model);
>      machine->init(ram_size, boot_devices,
>                    kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
>  }
> 

If the model is specified with -M by user or libvirt, then all seems
fine, but renaming default CPUs like qemu64 et al. (as done for
non-default ones in the next patch) would break machines' cpu_init()
call. That's a fairly common use case for non-x86 machines.

The alternative would be to incept closer to object_new(), i.e. call an
alias resolution helper from the various cpu_*_init() functions.

Please keep me cc'ed.

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [QEMU PATCH 3/3] x86: pc: versioned CPU model names & compatibility aliases

[Andreas Färber]

Am 25.07.2012 20:18, schrieb Eduardo Habkost:
> This adds version number to CPU model names on the "pc-<version>"
> machine-types, so we can create new models with bug fixes while keeping
> compatibility when using older machine-types.
> 
> When naming the existing models, I used the last QEMU version where the
> model was changed (see summary below), but by coincidence every single
> one was changed on QEMU-1.1.
> 
> - Conroe, Penryn, Nehalem, Opteron_G1, Opteron_G2, Opteron_G3:
>   added on 0.13, changed on 1.1
> - Westmere, SandyBridge, Opteron_G4: added on 1.1
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  hw/pc_piix.c                       |   56 ++++++++++++++++++++++++++++++++++++
>  sysconfigs/target/cpus-x86_64.conf |   18 ++++++------
>  2 files changed, 65 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 0c0096f..ef3840f 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -349,6 +349,18 @@ static void pc_xen_hvm_init(ram_addr_t ram_size,
>  }
>  #endif
>  
> +/* CPU aliases for pre-1.2 CPU models */
> +#define V1_1_CPU_ALIASES  \
> +    { "Conroe",      "Conroe-1.1" }, \
> +    { "Penryn",      "Penryn-1.1" }, \
> +    { "Nehalem",     "Nehalem-1.1" }, \
> +    { "Westmere",    "Westmere-1.1" }, \
> +    { "SandyBridge", "SandyBridge-1.1" }, \
> +    { "Opteron_G1",  "Opteron_G1-1.1" }, \
> +    { "Opteron_G2",  "Opteron_G2-1.1" }, \
> +    { "Opteron_G3",  "Opteron_G3-1.1" }, \
> +    { "Opteron_G4",  "Opteron_G4-1.1" },
> +
>  static QEMUMachine pc_machine_v1_2 = {
>      .name = "pc-1.2",
>      .alias = "pc",
> @@ -356,6 +368,10 @@ static QEMUMachine pc_machine_v1_2 = {
>      .init = pc_init_pci,
>      .max_cpus = 255,
>      .is_default = 1,
> +    .cpu_aliases = (CPUModelAlias[]) {
> +        V1_1_CPU_ALIASES
> +        {NULL, NULL},
> +    },
>  };
>  
>  #define PC_COMPAT_1_1 \
[...]
> diff --git a/sysconfigs/target/cpus-x86_64.conf b/sysconfigs/target/cpus-x86_64.conf
> index cee0ea9..14c7891 100644
> --- a/sysconfigs/target/cpus-x86_64.conf
> +++ b/sysconfigs/target/cpus-x86_64.conf
> @@ -1,7 +1,7 @@
>  # x86 CPU MODELS
>  
>  [cpudef]
> -   name = "Conroe"
> +   name = "Conroe-1.1"
>     level = "2"
>     vendor = "GenuineIntel"
>     family = "6"
[snip]

So where are the actual differences between, e.g., Conroe-1.1 and
Conroe? I'd expect we need either an additional string applying
parameter presets such as maybe "x2apic=off" or a nested list of
(property, value) pairs.

As long as there's no concept for actually modelling versioned CPUs, I
consider this RFC stage and not worth merging yet...

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [QEMU PATCH 0/3] versioned CPU models / per-machine-type aliases

[Anthony Liguori]

Eduardo Habkost <ehabkost@redhat.com> writes:

> Hi,
>
> This is the first try at a simple system to make the CPU model definitions
> versioned (to allow them to get bug fixes while allowing migration from older
> versions and keeping command-line compatibility), and per- machine-type aliases
> for compatibility.
>
> The lack of CPU model versioning is blocking multiple bug fixes that are
> necessary on CPU model definitions, but can't be included today because they
> would break migration.
>
> Later, after this gets in (or at least gets some feedback), I plan to send a
> proposal for a machine-friendly CPU feature / CPU model probing interface that
> libvirt could use.

This isn't the right approach.  The CPU properties should be exposed as
QOM properties which then allows the machine type globals to be used to
control stuff like this.

Is there a specific set of properties you want to control?  As long as
it's a small number, we can start with that and get something in shape
for 1.2.

Regards,

Anthony Liguori

>
> Eduardo Habkost (3):
>   vl.c: extract qemu_machine_init() function
>   per-machine-type CPU model alias system
>   x86: pc: versioned CPU model names & compatibility aliases
>
>  hw/boards.h                        |   13 +++++++++
>  hw/pc_piix.c                       |   56 ++++++++++++++++++++++++++++++++++++
>  sysconfigs/target/cpus-x86_64.conf |   18 ++++++------
>  vl.c                               |   28 +++++++++++++++++-
>  4 files changed, 105 insertions(+), 10 deletions(-)
>
> -- 
> 1.7.10.4

Re: [Qemu-devel] [QEMU PATCH 0/3] versioned CPU models / per-machine-type aliases

[Eduardo Habkost]

On Wed, Jul 25, 2012 at 06:43:25PM -0500, Anthony Liguori wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > Hi,
> >
> > This is the first try at a simple system to make the CPU model definitions
> > versioned (to allow them to get bug fixes while allowing migration from older
> > versions and keeping command-line compatibility), and per- machine-type aliases
> > for compatibility.
> >
> > The lack of CPU model versioning is blocking multiple bug fixes that are
> > necessary on CPU model definitions, but can't be included today because they
> > would break migration.
> >
> > Later, after this gets in (or at least gets some feedback), I plan to send a
> > proposal for a machine-friendly CPU feature / CPU model probing interface that
> > libvirt could use.
> 
> This isn't the right approach.  The CPU properties should be exposed as
> QOM properties which then allows the machine type globals to be used to
> control stuff like this.

I would like to use global properties for this, but the obstacles I have
found were:

- As far as I can see in the code, global properties are usable only by
  qdev objects, and CPUs were not qdevfied yet
- The per-machine-type properties I need to set are for CPU models, not
  CPUs.
  - For example: if we fix the Nehalem CPU model by changing the "level"
    field, we need to make the pc-1.1 and lower machine-types to keep
    the old "level" value, but only on the Nehalem CPU model

> 
> Is there a specific set of properties you want to control?  As long as
> it's a small number, we can start with that and get something in shape
> for 1.2.

The main bugs that need this to allow us to fix it are:

- Some CPU models have a too low "level" field and need it to be
  increased on newer machine-types (and kept the same on older
  machine-types).
- Some subtle bugs need the "model" field to be changed, too. See:
  http://lists.gnu.org/archive/html/qemu-devel/2011-05/msg02545.html
- Some features may need to be added or removed from some CPU models.
  - Example: SandyBridge has tsc-deadline enabled on the config file,
    but it simply does not enable the feature on qemu-1.1. The fix will
    require disabling tsc-deadline on the pc-1.1 machine type.

Anyway, I think the number of properties is not necessarily a problem:
the problem is to have the proper infra-structure so the CPU
model/machine-type compatibility can be implemented using global
properties. If we have something in place to allow that, supporting 3 or
20 properties seems to be equally feasible.

> 
> Regards,
> 
> Anthony Liguori
> 
> >
> > Eduardo Habkost (3):
> >   vl.c: extract qemu_machine_init() function
> >   per-machine-type CPU model alias system
> >   x86: pc: versioned CPU model names & compatibility aliases
> >
> >  hw/boards.h                        |   13 +++++++++
> >  hw/pc_piix.c                       |   56 ++++++++++++++++++++++++++++++++++++
> >  sysconfigs/target/cpus-x86_64.conf |   18 ++++++------
> >  vl.c                               |   28 +++++++++++++++++-
> >  4 files changed, 105 insertions(+), 10 deletions(-)
> >
> > -- 
> > 1.7.10.4

Re: [Qemu-devel] [QEMU PATCH 0/3] versioned CPU models / per-machine-type aliases

[Andreas Färber]

Am 26.07.2012 15:53, schrieb Eduardo Habkost:
> On Wed, Jul 25, 2012 at 06:43:25PM -0500, Anthony Liguori wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>>
>>> Hi,
>>>
>>> This is the first try at a simple system to make the CPU model definitions
>>> versioned (to allow them to get bug fixes while allowing migration from older
>>> versions and keeping command-line compatibility), and per- machine-type aliases
>>> for compatibility.
>>>
>>> The lack of CPU model versioning is blocking multiple bug fixes that are
>>> necessary on CPU model definitions, but can't be included today because they
>>> would break migration.
>>>
>>> Later, after this gets in (or at least gets some feedback), I plan to send a
>>> proposal for a machine-friendly CPU feature / CPU model probing interface that
>>> libvirt could use.
>>
>> This isn't the right approach.  The CPU properties should be exposed as
>> QOM properties which then allows the machine type globals to be used to
>> control stuff like this.
> 
> I would like to use global properties for this, but the obstacles I have
> found were:
> 
> - As far as I can see in the code, global properties are usable only by
>   qdev objects, and CPUs were not qdevfied yet

After Hackweek I plan to put together some compromise or even multiple
alternatives. We definitely need this for multiple open issues.

> - The per-machine-type properties I need to set are for CPU models, not
>   CPUs.
>   - For example: if we fix the Nehalem CPU model by changing the "level"
>     field, we need to make the pc-1.1 and lower machine-types to keep
>     the old "level" value, but only on the Nehalem CPU model

Is that part crying for CPU subclasses? Or what is the problem there?
(Still have a mail about -cpudef in my drafts folder, need to post RFC.)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [QEMU PATCH 3/3] x86: pc: versioned CPU model names & compatibility aliases

[Eduardo Habkost]

On Thu, Jul 26, 2012 at 12:52:33AM +0200, Andreas Färber wrote:
> Am 25.07.2012 20:18, schrieb Eduardo Habkost:
> > This adds version number to CPU model names on the "pc-<version>"
> > machine-types, so we can create new models with bug fixes while keeping
> > compatibility when using older machine-types.
> > 
> > When naming the existing models, I used the last QEMU version where the
> > model was changed (see summary below), but by coincidence every single
> > one was changed on QEMU-1.1.
> > 
> > - Conroe, Penryn, Nehalem, Opteron_G1, Opteron_G2, Opteron_G3:
> >   added on 0.13, changed on 1.1
> > - Westmere, SandyBridge, Opteron_G4: added on 1.1
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  hw/pc_piix.c                       |   56 ++++++++++++++++++++++++++++++++++++
> >  sysconfigs/target/cpus-x86_64.conf |   18 ++++++------
> >  2 files changed, 65 insertions(+), 9 deletions(-)
> > 
> > diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> > index 0c0096f..ef3840f 100644
> > --- a/hw/pc_piix.c
> > +++ b/hw/pc_piix.c
> > @@ -349,6 +349,18 @@ static void pc_xen_hvm_init(ram_addr_t ram_size,
> >  }
> >  #endif
> >  
> > +/* CPU aliases for pre-1.2 CPU models */
> > +#define V1_1_CPU_ALIASES  \
> > +    { "Conroe",      "Conroe-1.1" }, \
> > +    { "Penryn",      "Penryn-1.1" }, \
> > +    { "Nehalem",     "Nehalem-1.1" }, \
> > +    { "Westmere",    "Westmere-1.1" }, \
> > +    { "SandyBridge", "SandyBridge-1.1" }, \
> > +    { "Opteron_G1",  "Opteron_G1-1.1" }, \
> > +    { "Opteron_G2",  "Opteron_G2-1.1" }, \
> > +    { "Opteron_G3",  "Opteron_G3-1.1" }, \
> > +    { "Opteron_G4",  "Opteron_G4-1.1" },
> > +
> >  static QEMUMachine pc_machine_v1_2 = {
> >      .name = "pc-1.2",
> >      .alias = "pc",
> > @@ -356,6 +368,10 @@ static QEMUMachine pc_machine_v1_2 = {
> >      .init = pc_init_pci,
> >      .max_cpus = 255,
> >      .is_default = 1,
> > +    .cpu_aliases = (CPUModelAlias[]) {
> > +        V1_1_CPU_ALIASES
> > +        {NULL, NULL},
> > +    },
> >  };
> >  
> >  #define PC_COMPAT_1_1 \
> [...]
> > diff --git a/sysconfigs/target/cpus-x86_64.conf b/sysconfigs/target/cpus-x86_64.conf
> > index cee0ea9..14c7891 100644
> > --- a/sysconfigs/target/cpus-x86_64.conf
> > +++ b/sysconfigs/target/cpus-x86_64.conf
> > @@ -1,7 +1,7 @@
> >  # x86 CPU MODELS
> >  
> >  [cpudef]
> > -   name = "Conroe"
> > +   name = "Conroe-1.1"
> >     level = "2"
> >     vendor = "GenuineIntel"
> >     family = "6"
> [snip]
> 
> So where are the actual differences between, e.g., Conroe-1.1 and
> Conroe? I'd expect we need either an additional string applying
> parameter presets such as maybe "x2apic=off" or a nested list of
> (property, value) pairs.

There are no differences yet, until we make updates in the Conroe model.
If we have to make any change (to fix a bug, for example), we would
create a "Conroe-1.2" CPU model, and make the "pc-1.2" machine-type
alias "Conroe" to "Conroe-1.2" while keeping the older machine-types
using "Conroe-1.1".

> 
> As long as there's no concept for actually modelling versioned CPUs, I
> consider this RFC stage and not worth merging yet...

What do you mean by "no concept for actually modelling versioned CPUs"?
You mean there's no use-case or reason for versioning them, or that the
series don't model the versioning properly?

-- 
Eduardo

Re: [Qemu-devel] [QEMU PATCH 0/3] versioned CPU models / per-machine-type aliases

[Eduardo Habkost]

On Thu, Jul 26, 2012 at 04:06:03PM +0200, Andreas Färber wrote:
> Am 26.07.2012 15:53, schrieb Eduardo Habkost:
> > On Wed, Jul 25, 2012 at 06:43:25PM -0500, Anthony Liguori wrote:
> >> Eduardo Habkost <ehabkost@redhat.com> writes:
> >>
> >>> Hi,
> >>>
> >>> This is the first try at a simple system to make the CPU model definitions
> >>> versioned (to allow them to get bug fixes while allowing migration from older
> >>> versions and keeping command-line compatibility), and per- machine-type aliases
> >>> for compatibility.
> >>>
> >>> The lack of CPU model versioning is blocking multiple bug fixes that are
> >>> necessary on CPU model definitions, but can't be included today because they
> >>> would break migration.
> >>>
> >>> Later, after this gets in (or at least gets some feedback), I plan to send a
> >>> proposal for a machine-friendly CPU feature / CPU model probing interface that
> >>> libvirt could use.
> >>
> >> This isn't the right approach.  The CPU properties should be exposed as
> >> QOM properties which then allows the machine type globals to be used to
> >> control stuff like this.
> > 
> > I would like to use global properties for this, but the obstacles I have
> > found were:
> > 
> > - As far as I can see in the code, global properties are usable only by
> >   qdev objects, and CPUs were not qdevfied yet
> 
> After Hackweek I plan to put together some compromise or even multiple
> alternatives. We definitely need this for multiple open issues.
> 
> > - The per-machine-type properties I need to set are for CPU models, not
> >   CPUs.
> >   - For example: if we fix the Nehalem CPU model by changing the "level"
> >     field, we need to make the pc-1.1 and lower machine-types to keep
> >     the old "level" value, but only on the Nehalem CPU model
> 
> Is that part crying for CPU subclasses? Or what is the problem there?
> (Still have a mail about -cpudef in my drafts folder, need to post RFC.)

Maybe, yes.

If we have the subclasses, then the only problem I see is that global
properties currently require qdev. Maybe having a simple interface
non-qdev objects can use to query for global properties would solve
that?

-- 
Eduardo

Re: [Qemu-devel] [QEMU PATCH 3/3] x86: pc: versioned CPU model names & compatibility aliases

[Andreas Färber]

Am 26.07.2012 16:24, schrieb Eduardo Habkost:
> On Thu, Jul 26, 2012 at 12:52:33AM +0200, Andreas Färber wrote:
>> Am 25.07.2012 20:18, schrieb Eduardo Habkost:
>>> This adds version number to CPU model names on the "pc-<version>"
>>> machine-types, so we can create new models with bug fixes while keeping
>>> compatibility when using older machine-types.
>>>
>>> When naming the existing models, I used the last QEMU version where the
>>> model was changed (see summary below), but by coincidence every single
>>> one was changed on QEMU-1.1.
>>>
>>> - Conroe, Penryn, Nehalem, Opteron_G1, Opteron_G2, Opteron_G3:
>>>   added on 0.13, changed on 1.1
>>> - Westmere, SandyBridge, Opteron_G4: added on 1.1
>>>
>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>>> ---
>>>  hw/pc_piix.c                       |   56 ++++++++++++++++++++++++++++++++++++
>>>  sysconfigs/target/cpus-x86_64.conf |   18 ++++++------
>>>  2 files changed, 65 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
>>> index 0c0096f..ef3840f 100644
>>> --- a/hw/pc_piix.c
>>> +++ b/hw/pc_piix.c
>>> @@ -349,6 +349,18 @@ static void pc_xen_hvm_init(ram_addr_t ram_size,
>>>  }
>>>  #endif
>>>  
>>> +/* CPU aliases for pre-1.2 CPU models */
>>> +#define V1_1_CPU_ALIASES  \
>>> +    { "Conroe",      "Conroe-1.1" }, \
>>> +    { "Penryn",      "Penryn-1.1" }, \
>>> +    { "Nehalem",     "Nehalem-1.1" }, \
>>> +    { "Westmere",    "Westmere-1.1" }, \
>>> +    { "SandyBridge", "SandyBridge-1.1" }, \
>>> +    { "Opteron_G1",  "Opteron_G1-1.1" }, \
>>> +    { "Opteron_G2",  "Opteron_G2-1.1" }, \
>>> +    { "Opteron_G3",  "Opteron_G3-1.1" }, \
>>> +    { "Opteron_G4",  "Opteron_G4-1.1" },
>>> +
>>>  static QEMUMachine pc_machine_v1_2 = {
>>>      .name = "pc-1.2",
>>>      .alias = "pc",
>>> @@ -356,6 +368,10 @@ static QEMUMachine pc_machine_v1_2 = {
>>>      .init = pc_init_pci,
>>>      .max_cpus = 255,
>>>      .is_default = 1,
>>> +    .cpu_aliases = (CPUModelAlias[]) {
>>> +        V1_1_CPU_ALIASES
>>> +        {NULL, NULL},
>>> +    },
>>>  };
>>>  
>>>  #define PC_COMPAT_1_1 \
>> [...]
>>> diff --git a/sysconfigs/target/cpus-x86_64.conf b/sysconfigs/target/cpus-x86_64.conf
>>> index cee0ea9..14c7891 100644
>>> --- a/sysconfigs/target/cpus-x86_64.conf
>>> +++ b/sysconfigs/target/cpus-x86_64.conf
>>> @@ -1,7 +1,7 @@
>>>  # x86 CPU MODELS
>>>  
>>>  [cpudef]
>>> -   name = "Conroe"
>>> +   name = "Conroe-1.1"
>>>     level = "2"
>>>     vendor = "GenuineIntel"
>>>     family = "6"
>> [snip]
>>
>> So where are the actual differences between, e.g., Conroe-1.1 and
>> Conroe? I'd expect we need either an additional string applying
>> parameter presets such as maybe "x2apic=off" or a nested list of
>> (property, value) pairs.
> 
> There are no differences yet, until we make updates in the Conroe model.
> If we have to make any change (to fix a bug, for example), we would
> create a "Conroe-1.2" CPU model, and make the "pc-1.2" machine-type
> alias "Conroe" to "Conroe-1.2" while keeping the older machine-types
> using "Conroe-1.1".
> 
>>
>> As long as there's no concept for actually modelling versioned CPUs, I
>> consider this RFC stage and not worth merging yet...
> 
> What do you mean by "no concept for actually modelling versioned CPUs"?
> You mean there's no use-case or reason for versioning them, or that the
> series don't model the versioning properly?

I mean, you add infrastructure for remapping Conroe to Conroe-1.1 or
Conroe-x.y, but I am missing something that lets us declare "Conroe-1.1
is Conroe-1.2 with this difference", like we do for machines. We surely
don't want to duplicate everything that stays the same for each new CPU
version.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [QEMU PATCH 3/3] x86: pc: versioned CPU model names & compatibility aliases

[Jiri Denemark]

On Wed, Jul 25, 2012 at 15:18:43 -0300, Eduardo Habkost wrote:
> This adds version number to CPU model names on the "pc-<version>"
> machine-types, so we can create new models with bug fixes while keeping
> compatibility when using older machine-types.
> 
> When naming the existing models, I used the last QEMU version where the
> model was changed (see summary below), but by coincidence every single
> one was changed on QEMU-1.1.
> 
> - Conroe, Penryn, Nehalem, Opteron_G1, Opteron_G2, Opteron_G3:
>   added on 0.13, changed on 1.1
> - Westmere, SandyBridge, Opteron_G4: added on 1.1

Hi Eduardo,

What models would be listed by "qemu-system-x86_64 -cpu ?"? Only non-versioned
model aliases or full names with versions or both?

Jirka

Re: [Qemu-devel] [QEMU PATCH 3/3] x86: pc: versioned CPU model names & compatibility aliases

[Eduardo Habkost]

On Thu, Jul 26, 2012 at 04:31:45PM +0200, Andreas Färber wrote:
> Am 26.07.2012 16:24, schrieb Eduardo Habkost:
> > On Thu, Jul 26, 2012 at 12:52:33AM +0200, Andreas Färber wrote:
> >> Am 25.07.2012 20:18, schrieb Eduardo Habkost:
> >>> This adds version number to CPU model names on the "pc-<version>"
> >>> machine-types, so we can create new models with bug fixes while keeping
> >>> compatibility when using older machine-types.
> >>>
> >>> When naming the existing models, I used the last QEMU version where the
> >>> model was changed (see summary below), but by coincidence every single
> >>> one was changed on QEMU-1.1.
> >>>
> >>> - Conroe, Penryn, Nehalem, Opteron_G1, Opteron_G2, Opteron_G3:
> >>>   added on 0.13, changed on 1.1
> >>> - Westmere, SandyBridge, Opteron_G4: added on 1.1
> >>>
> >>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> >>> ---
> >>>  hw/pc_piix.c                       |   56 ++++++++++++++++++++++++++++++++++++
> >>>  sysconfigs/target/cpus-x86_64.conf |   18 ++++++------
> >>>  2 files changed, 65 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> >>> index 0c0096f..ef3840f 100644
> >>> --- a/hw/pc_piix.c
> >>> +++ b/hw/pc_piix.c
> >>> @@ -349,6 +349,18 @@ static void pc_xen_hvm_init(ram_addr_t ram_size,
> >>>  }
> >>>  #endif
> >>>  
> >>> +/* CPU aliases for pre-1.2 CPU models */
> >>> +#define V1_1_CPU_ALIASES  \
> >>> +    { "Conroe",      "Conroe-1.1" }, \
> >>> +    { "Penryn",      "Penryn-1.1" }, \
> >>> +    { "Nehalem",     "Nehalem-1.1" }, \
> >>> +    { "Westmere",    "Westmere-1.1" }, \
> >>> +    { "SandyBridge", "SandyBridge-1.1" }, \
> >>> +    { "Opteron_G1",  "Opteron_G1-1.1" }, \
> >>> +    { "Opteron_G2",  "Opteron_G2-1.1" }, \
> >>> +    { "Opteron_G3",  "Opteron_G3-1.1" }, \
> >>> +    { "Opteron_G4",  "Opteron_G4-1.1" },
> >>> +
> >>>  static QEMUMachine pc_machine_v1_2 = {
> >>>      .name = "pc-1.2",
> >>>      .alias = "pc",
> >>> @@ -356,6 +368,10 @@ static QEMUMachine pc_machine_v1_2 = {
> >>>      .init = pc_init_pci,
> >>>      .max_cpus = 255,
> >>>      .is_default = 1,
> >>> +    .cpu_aliases = (CPUModelAlias[]) {
> >>> +        V1_1_CPU_ALIASES
> >>> +        {NULL, NULL},
> >>> +    },
> >>>  };
> >>>  
> >>>  #define PC_COMPAT_1_1 \
> >> [...]
> >>> diff --git a/sysconfigs/target/cpus-x86_64.conf b/sysconfigs/target/cpus-x86_64.conf
> >>> index cee0ea9..14c7891 100644
> >>> --- a/sysconfigs/target/cpus-x86_64.conf
> >>> +++ b/sysconfigs/target/cpus-x86_64.conf
> >>> @@ -1,7 +1,7 @@
> >>>  # x86 CPU MODELS
> >>>  
> >>>  [cpudef]
> >>> -   name = "Conroe"
> >>> +   name = "Conroe-1.1"
> >>>     level = "2"
> >>>     vendor = "GenuineIntel"
> >>>     family = "6"
> >> [snip]
> >>
> >> So where are the actual differences between, e.g., Conroe-1.1 and
> >> Conroe? I'd expect we need either an additional string applying
> >> parameter presets such as maybe "x2apic=off" or a nested list of
> >> (property, value) pairs.
> > 
> > There are no differences yet, until we make updates in the Conroe model.
> > If we have to make any change (to fix a bug, for example), we would
> > create a "Conroe-1.2" CPU model, and make the "pc-1.2" machine-type
> > alias "Conroe" to "Conroe-1.2" while keeping the older machine-types
> > using "Conroe-1.1".
> > 
> >>
> >> As long as there's no concept for actually modelling versioned CPUs, I
> >> consider this RFC stage and not worth merging yet...
> > 
> > What do you mean by "no concept for actually modelling versioned CPUs"?
> > You mean there's no use-case or reason for versioning them, or that the
> > series don't model the versioning properly?
> 
> I mean, you add infrastructure for remapping Conroe to Conroe-1.1 or
> Conroe-x.y, but I am missing something that lets us declare "Conroe-1.1
> is Conroe-1.2 with this difference", like we do for machines. We surely
> don't want to duplicate everything that stays the same for each new CPU
> version.

Oh, that I want too[1], but IMO it's orthogonal to the problem of
actually having the per-machine-type aliases. The per-machine-type
aliases (or properties) are a requirement to allow us to fix bugs while
keeping compatibility an "inheritance" system is something to make the
CPU config files look better and be more maintainable.


[1] There are multiple changes I want to make the cpudef config format:

- Make it based on boolean per-feature flags, not low-level
  feature_<register> bits
- Make it easy to say "model FOO is like model BAR, but with these
  differences"
  - This is useful for versioning but may be useful for cases like
    "SandyBridge has all the features from Westmere, plus these
    additional ones"

-- 
Eduardo

Re: [Qemu-devel] [QEMU PATCH 3/3] x86: pc: versioned CPU model names & compatibility aliases

[Eduardo Habkost]

On Thu, Jul 26, 2012 at 04:33:36PM +0200, Jiri Denemark wrote:
> On Wed, Jul 25, 2012 at 15:18:43 -0300, Eduardo Habkost wrote:
> > This adds version number to CPU model names on the "pc-<version>"
> > machine-types, so we can create new models with bug fixes while keeping
> > compatibility when using older machine-types.
> > 
> > When naming the existing models, I used the last QEMU version where the
> > model was changed (see summary below), but by coincidence every single
> > one was changed on QEMU-1.1.
> > 
> > - Conroe, Penryn, Nehalem, Opteron_G1, Opteron_G2, Opteron_G3:
> >   added on 0.13, changed on 1.1
> > - Westmere, SandyBridge, Opteron_G4: added on 1.1
> 
> Hi Eduardo,
> 
> What models would be listed by "qemu-system-x86_64 -cpu ?"? Only non-versioned
> model aliases or full names with versions or both?

Just the full names. The aliases are machine-type properties, but I
didn't see a good place to list them on the help text. (suggestions are
welcome)

Anyway, "-cpu ?" and "-cpu ?dump" are interfaces for humans. libvirt
will require a new interface that allows it to query the CPU model
versions and the aliases. But defining this interface will be possible
only after we settle on the design of the system inside QEMU. (e.g. if
we implement it based on global properties, there may be no concept of
"versions" and "aliases" at all, and we may have to rethink the way the
QEMU<->libvirt interface will look like).

-- 
Eduardo

Re: [Qemu-devel] [libvirt] [QEMU PATCH 1/3] vl.c: extract qemu_machine_init() function

[Eduardo Habkost]

On Wed, Jul 25, 2012 at 04:18:01PM -0600, Eric Blake wrote:
> On 07/25/2012 12:18 PM, Eduardo Habkost wrote:
> > Other code directly related to the board init function may be moved
> > there.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  hw/boards.h |    7 +++++++
> >  vl.c        |   14 +++++++++++++-
> >  2 files changed, 20 insertions(+), 1 deletion(-)
> > 
> 
> >  
> > -    machine->init(ram_size, boot_devices,
> > +    qemu_machine_init(machine, ram_size, boot_devices,
> >                    kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
> 
> Doesn't this next line also need an indentation adjustment?

Yes. I will fix it in the next version.

-- 
Eduardo

Re: [Qemu-devel] [QEMU PATCH 3/3] x86: pc: versioned CPU model names & compatibility aliases

[Igor Mammedov]

On Thu, Jul 26, 2012 at 10:48:45AM -0400, Eduardo Habkost wrote:
... 
> 
> [1] There are multiple changes I want to make the cpudef config format:
> 
> - Make it based on boolean per-feature flags, not low-level
>   feature_<register> bits
I'm trying to convert features to properties before looking at global props.
current state is at https://github.com/imammedo/qemu/tree/x86-cpu-properties.WIP
Does it represent something you have in mind?

>
... 
> -- 
> Eduardo
> 

Re: [Qemu-devel] [QEMU PATCH 3/3] x86: pc: versioned CPU model names & compatibility aliases

[Eduardo Habkost]

On Tue, Jul 31, 2012 at 03:22:59PM +0200, Igor Mammedov wrote:
> On Thu, Jul 26, 2012 at 10:48:45AM -0400, Eduardo Habkost wrote:
> ... 
> > 
> > [1] There are multiple changes I want to make the cpudef config format:
> > 
> > - Make it based on boolean per-feature flags, not low-level
> >   feature_<register> bits
> I'm trying to convert features to properties before looking at global props.
> current state is at https://github.com/imammedo/qemu/tree/x86-cpu-properties.WIP
> Does it represent something you have in mind?

Yes, it's like what I have in mind.

But note that cpudefs are not objects, yet. We have lots of people
focusing on the CPU objects themselves, but the cpudefs read from the
config are still plain old structs, with no properties, and there's no
clear plan on how to model them in the code.

Even if we convert the cpudefs to become classes, we still don't have a
simple way to make the cpudef read from the config file set CPU
properties only for specific classes.

Some of the approaches I see:
- Make classes have properties, too;
- Make cpudefs be full-featured objects, that are used as argument when
  creating CPU objects;
- Do it manually: have a feature bitmap (or other data structure) for
  each CPU class, translate the cpudef config options to that bitmap
  manually, and translate it to CPU features manually when creating the
  CPU object;
- Translate all the cpudef sections to global properties, that would
  in turn set CPU properties once the CPU objects are created.

The last option seems to work (once we make the CPU objects really able
to use global properties). But I have the feeling that setting lots of
global properties by default is an abuse of the global property system.

-- 
Eduardo