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

[Eduardo Habkost <ehabkost@redhat.com>]

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