Re: [Qemu-devel] [PATCH v3 1/3] target-i386: Define static "base" CPU model

[David Hildenbrand]

Am 22.02.2017 um 20:00 schrieb Eduardo Habkost:
> The query-cpu-model-expand QMP command needs at least one static
> model, to allow the "static" expansion mode to be implemented.
> Instead of defining static versions of every CPU model, define a
> "base" CPU model that has absolutely no feature flag enabled.
> 
> Despite having no CPUID data set at all, "-cpu base" is even a
> functional CPU:
> 
> * It can boot a Slackware Linux 1.01 image with a Linux 0.99.12
>   kernel[1].
> * It is even possible to boot[2] a modern Fedora x86_64 guest by
>   manually enabling the following CPU features:
>   -cpu base,+lm,+msr,+pae,+fpu,+cx8,+cmov,+sse,+sse2,+fxsr
> 
> [1] http://www.qemu-advent-calendar.org/2014/#day-1
> [2] This is what can be seen in the guest:
>     address@hidden ~]# cat /proc/cpuinfo
>     processor       : 0
>     vendor_id       : unknown
>     cpu family      : 0
>     model           : 0
>     model name      : 00/00
>     stepping        : 0
>     physical id     : 0
>     siblings        : 1
>     core id         : 0
>     cpu cores       : 1
>     apicid          : 0
>     initial apicid  : 0
>     fpu             : yes
>     fpu_exception   : yes
>     cpuid level     : 1
>     wp              : yes
>     flags           : fpu msr pae cx8 cmov fxsr sse sse2 lm nopl
>     bugs            :
>     bogomips        : 5832.70
>     clflush size    : 64
>     cache_alignment : 64
>     address sizes   : 36 bits physical, 48 bits virtual
>     power management:
> 
>     address@hidden ~]# x86info -v -a
>     x86info v1.30.  Dave Jones 2001-2011
>     Feedback to <address@hidden>.
> 
>     No TSC, MHz calculation cannot be performed.
>     Unknown vendor (0)
>     MP Table:
> 
>     Family: 0 Model: 0 Stepping: 0
>     CPU Model (x86info's best guess):
> 
>     eax in: 0x00000000, eax = 00000001 ebx = 00000000 ecx = 00000000 edx = 
> 00000000
>     eax in: 0x00000001, eax = 00000000 ebx = 00000800 ecx = 00000000 edx = 
> 07008161
> 
>     eax in: 0x80000000, eax = 80000001 ebx = 00000000 ecx = 00000000 edx = 
> 00000000
>     eax in: 0x80000001, eax = 00000000 ebx = 00000000 ecx = 00000000 edx = 
> 20000000
> 
>     Feature flags:
>      fpu            Onboard FPU
>      msr            Model-Specific Registers
>      pae            Physical Address Extensions
>      cx8            CMPXCHG8 instruction
>      cmov           CMOV instruction
>      fxsr           FXSAVE and FXRSTOR instructions
>      sse            SSE support
>      sse2           SSE2 support
> 
>     Long NOPs supported: yes
> 
>     Address sizes : 0 bits physical, 0 bits virtual
>     0MHz processor (estimate).
> 
>      running at an estimated 0MHz
>     address@hidden ~]#
> 
> Signed-off-by: Eduardo Habkost <address@hidden>
> ---
>  target/i386/cpu-qom.h |  2 ++
>  target/i386/cpu.c     | 20 ++++++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/target/i386/cpu-qom.h b/target/i386/cpu-qom.h
> index f6c704c3a9..c2205e6077 100644
> --- a/target/i386/cpu-qom.h
> +++ b/target/i386/cpu-qom.h
> @@ -50,6 +50,7 @@ typedef struct X86CPUDefinition X86CPUDefinition;
>   * @kvm_required: Whether CPU model requires KVM to be enabled.
>   * @ordering: Ordering on the "-cpu help" CPU model list.
>   * @migration_safe: See CpuDefinitionInfo::migration_safe
> + * @static_model: See CpuDefinitionInfo::static
>   * @parent_realize: The parent class' realize handler.
>   * @parent_reset: The parent class' reset handler.
>   *
> @@ -68,6 +69,7 @@ typedef struct X86CPUClass {
>      bool kvm_required;
>      int ordering;
>      bool migration_safe;
> +    bool static_model;
>  
>      /* Optional description of CPU model.
>       * If unavailable, cpu_def->model_id is used */
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index c6f62367b0..4241f56e8a 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -2230,6 +2230,7 @@ static void x86_cpu_definition_entry(gpointer data, 
> gpointer user_data)
>      info->q_typename = g_strdup(object_class_get_name(oc));
>      info->migration_safe = cc->migration_safe;
>      info->has_migration_safe = true;
> +    info->q_static = cc->static_model;
>  
>      entry = g_malloc0(sizeof(*entry));
>      entry->value = info;
> @@ -3836,6 +3837,24 @@ static const TypeInfo x86_cpu_type_info = {
>      .class_init = x86_cpu_common_class_init,
>  };
>  
> +
> +/* "base" CPU model, used by query-cpu-model-expansion */
> +static void x86_cpu_base_class_init(ObjectClass *oc, void *data)
> +{
> +    X86CPUClass *xcc = X86_CPU_CLASS(oc);
> +
> +    xcc->static_model = true;
> +    xcc->migration_safe = true;
> +    xcc->model_description = "base CPU model type with no feature enabled";

s/feature/features/ ? (no native speaker ;) )

> +    xcc->ordering = 8;
> +}
> +
> +static const TypeInfo x86_base_cpu_type_info = {
> +        .name = X86_CPU_TYPE_NAME("base"),
> +        .parent = TYPE_X86_CPU,
> +        .class_init = x86_cpu_base_class_init,
> +};
> +
>  static void x86_cpu_register_types(void)
>  {
>      int i;
> @@ -3845,6 +3864,7 @@ static void x86_cpu_register_types(void)
>          x86_register_cpudef_type(&builtin_x86_defs[i]);
>      }
>      type_register_static(&max_x86_cpu_type_info);
> +    type_register_static(&x86_base_cpu_type_info);
>  #ifdef CONFIG_KVM
>      type_register_static(&host_x86_cpu_type_info);
>  #endif
> 

Looks sane to me!

Reviewed-by: David Hildenbrand <address@hidden>

-- 
Thanks,

David