Re: [Qemu-devel] [PATCH v2 7/9] i386: Make unversioned CPU models be aliases

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Jun 27, 2019 at 09:28:42PM -0300, Eduardo Habkost wrote:
> This will make unversioned CPU models behavior depend on the
> machine type:
> 
> * "pc-*-4.0" and older will not report them as aliases.
>   This is done to keep compatibility with older QEMU versions
>   after management software starts translating aliases.
> 
> * "pc-*-4.1" will translate unversioned CPU models to -v1.
>   This is done to keep compatibility with existing management
>   software, that still relies on CPU model runnability promises.
> 
> * "none" will translate unversioned CPU models to their latest
>   version.  This is planned become the default in future machine
>   types (probably in pc-*-4.3).
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Patch v1 was "[PATCH 4/6] i386: Infrastructure for versioned CPU
> models", and was split into multiple patches.
> 
> Changes v1 -> v2:
> * Make version numbers simple integers, completely independent
>   from machine type versions
> * New X86CPUVersion typedef
> * New CPU_VERSION_* defines to make X86CPUModel::version
>   semantics clearer
> ---
>  include/hw/i386/pc.h                       |   3 +
>  target/i386/cpu.h                          |  12 ++
>  hw/i386/pc.c                               |   3 +
>  hw/i386/pc_piix.c                          |   4 +
>  hw/i386/pc_q35.c                           |   4 +
>  target/i386/cpu.c                          |  52 ++++++++-
>  tests/acceptance/x86_cpu_model_versions.py | 126 +++++++++++++++++++++
>  7 files changed, 203 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index ee79d71ccc..1946c1023f 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -109,6 +109,9 @@ typedef struct PCMachineClass {
>  
>      /* Compat options: */
>  
> +    /* Default CPU model version.  See x86_cpu_set_default_version(). */
> +    int default_cpu_version;
> +
>      /* ACPI compat: */
>      bool has_acpi_build;
>      bool rsdp_in_ram;
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 12bc3cd4a8..05393cf9d1 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1934,11 +1934,23 @@ void x86_cpu_change_kvm_default(const char *prop, const char *value);
>  /* Resolve to latest CPU version */
>  #define CPU_VERSION_LATEST -1
>  
> +/*
> + * Resolve to version defined by current machine type.
> + * See x86_cpu_set_default_version()
> + */
> +#define CPU_VERSION_AUTO   -2
> +
>  /* Don't resolve to any versioned CPU models, like old QEMU versions */
>  #define CPU_VERSION_LEGACY  0
>  
>  typedef int X86CPUVersion;
>  
> +/*
> + * Set default CPU model version for CPU models having
> + * version == CPU_VERSION_AUTO.
> + */
> +void x86_cpu_set_default_version(X86CPUVersion version);
> +
>  /* Return name of 32-bit register, from a R_* constant */
>  const char *get_register_name_32(unsigned int reg);
>  
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 3f0f221a5b..1df569620a 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1649,6 +1649,9 @@ void pc_cpus_init(PCMachineState *pcms)
>      const CPUArchIdList *possible_cpus;
>      MachineState *ms = MACHINE(pcms);
>      MachineClass *mc = MACHINE_GET_CLASS(pcms);
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(mc);
> +
> +    x86_cpu_set_default_version(pcmc->default_cpu_version);
>  
>      /* Calculates the limit to CPU APIC ID values
>       *
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index c07c4a5b38..0a069bff54 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -430,9 +430,11 @@ static void pc_i440fx_machine_options(MachineClass *m)
>  
>  static void pc_i440fx_4_1_machine_options(MachineClass *m)
>  {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>      pc_i440fx_machine_options(m);
>      m->alias = "pc";
>      m->is_default = 1;
> +    pcmc->default_cpu_version = 1;
>  }
>  
>  DEFINE_I440FX_MACHINE(v4_1, "pc-i440fx-4.1", NULL,
> @@ -440,9 +442,11 @@ DEFINE_I440FX_MACHINE(v4_1, "pc-i440fx-4.1", NULL,
>  
>  static void pc_i440fx_4_0_machine_options(MachineClass *m)
>  {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>      pc_i440fx_4_1_machine_options(m);
>      m->alias = NULL;
>      m->is_default = 0;
> +    pcmc->default_cpu_version = CPU_VERSION_LEGACY;
>      compat_props_add(m->compat_props, hw_compat_4_0, hw_compat_4_0_len);
>      compat_props_add(m->compat_props, pc_compat_4_0, pc_compat_4_0_len);
>  }
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 57232aed6b..397e1fdd2f 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -367,8 +367,10 @@ static void pc_q35_machine_options(MachineClass *m)
>  
>  static void pc_q35_4_1_machine_options(MachineClass *m)
>  {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>      pc_q35_machine_options(m);
>      m->alias = "q35";
> +    pcmc->default_cpu_version = 1;

How will this work in future if we need to introduce a v2 of some,
but not all CPU models, and then further a v3 of some others.

Can we select Haswell-v1,  Skylake-Server-v2, CascadeLake-v3
as defaults in some way ?


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|