[Qemu-devel] Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled()

["Roedel, Joerg" <Joerg.Roedel@amd.com>]

(Sorry for the late reply)

On Thu, Oct 07, 2010 at 08:48:06AM -0400, Anthony Liguori wrote:
> On 10/07/2010 03:42 AM, Roedel, Joerg wrote:
> > On Wed, Oct 06, 2010 at 03:24:59PM -0400, Anthony Liguori wrote:
> >    
> >>>> +    qemu_compat_version = machine->compat_version;
> >>>> +
> >>>>        if (display_type == DT_NOGRAPHIC) {
> >>>>            if (default_parallel)
> >>>>                add_device_config(DEV_PARALLEL, "null");
> >>>> -- 
> >>>> 1.7.0.4
> >>>>
> >>>>          
> >>> Looks fine to me, given CPUs are not in qdev. Anthony?
> >>>
> >>>        
> >> The idea is fine, but why not just add the default CPU to the machine
> >> description?
> >>      
> > If I remember correctly the reason was that the machine description was
> > not accessible in the cpuid initialization path because it is a function
> > local variable.
> 
> Not tested at all but I think the attached patch addresses it in a 
> pretty nice way.
> 
> There's a couple ways you could support your patch on top of this.  You 
> could add a kvm_cpu_model to the machine structure that gets defaulted 
> too if kvm_enabled().  You could also introduce a new KVM machine type 
> that gets defaulted to if no explicit machine is specified.

I had something similar in mind but then I realized that we need at
least a cpu_model and a cpu_model_kvm to distinguish between the TCG and
the KVM case.
Further the QEMUMachine data structure is used for all architectures in
QEMU and the model-names only make sense for x86. So I decided for the
comapt-version way (which doesn't mean I object against this one ;-) )

	Joerg

> From d2370c88cef4b07d48ba3c4804e35ae2db8db7c0 Mon Sep 17 00:00:00 2001
> From: Anthony Liguori <aliguori@us.ibm.com>
> Date: Thu, 7 Oct 2010 07:43:42 -0500
> Subject: [PATCH] machine: make default cpu model part of machine structure
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> diff --git a/hw/boards.h b/hw/boards.h
> index 6f0f0d7..8c6ef27 100644
> --- a/hw/boards.h
> +++ b/hw/boards.h
> @@ -16,6 +16,7 @@ typedef struct QEMUMachine {
>      const char *name;
>      const char *alias;
>      const char *desc;
> +    const char *cpu_model;
>      QEMUMachineInitFunc *init;
>      int use_scsi;
>      int max_cpus;
> diff --git a/hw/pc.c b/hw/pc.c
> index 69b13bf..0826107 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -866,14 +866,6 @@ void pc_cpus_init(const char *cpu_model)
>      int i;
>  
>      /* init CPUs */
> -    if (cpu_model == NULL) {
> -#ifdef TARGET_X86_64
> -        cpu_model = "qemu64";
> -#else
> -        cpu_model = "qemu32";
> -#endif
> -    }
> -
>      for(i = 0; i < smp_cpus; i++) {
>          pc_new_cpu(cpu_model);
>      }
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 12359a7..919b4d6 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -204,17 +204,22 @@ static void pc_init_isa(ram_addr_t ram_size,
>                          const char *initrd_filename,
>                          const char *cpu_model)
>  {
> -    if (cpu_model == NULL)
> -        cpu_model = "486";
>      pc_init1(ram_size, boot_device,
>               kernel_filename, kernel_cmdline,
>               initrd_filename, cpu_model, 0);
>  }
>  
> +#ifdef TARGET_X86_64
> +#define DEF_CPU_MODEL "qemu64"
> +#else
> +#define DEF_CPU_MODEL "qemu32"
> +#endif
> +
>  static QEMUMachine pc_machine = {
>      .name = "pc-0.13",
>      .alias = "pc",
>      .desc = "Standard PC",
> +    .cpu_model = DEF_CPU_MODEL,
>      .init = pc_init_pci,
>      .max_cpus = 255,
>      .is_default = 1,
> @@ -223,6 +228,7 @@ static QEMUMachine pc_machine = {
>  static QEMUMachine pc_machine_v0_12 = {
>      .name = "pc-0.12",
>      .desc = "Standard PC",
> +    .cpu_model = DEF_CPU_MODEL,
>      .init = pc_init_pci,
>      .max_cpus = 255,
>      .compat_props = (GlobalProperty[]) {
> @@ -242,6 +248,7 @@ static QEMUMachine pc_machine_v0_12 = {
>  static QEMUMachine pc_machine_v0_11 = {
>      .name = "pc-0.11",
>      .desc = "Standard PC, qemu 0.11",
> +    .cpu_model = DEF_CPU_MODEL,
>      .init = pc_init_pci,
>      .max_cpus = 255,
>      .compat_props = (GlobalProperty[]) {
> @@ -277,6 +284,7 @@ static QEMUMachine pc_machine_v0_11 = {
>  static QEMUMachine pc_machine_v0_10 = {
>      .name = "pc-0.10",
>      .desc = "Standard PC, qemu 0.10",
> +    .cpu_model = DEF_CPU_MODEL,
>      .init = pc_init_pci,
>      .max_cpus = 255,
>      .compat_props = (GlobalProperty[]) {
> @@ -324,6 +332,7 @@ static QEMUMachine pc_machine_v0_10 = {
>  static QEMUMachine isapc_machine = {
>      .name = "isapc",
>      .desc = "ISA-only PC",
> +    .cpu_model = "486",
>      .init = pc_init_isa,
>      .max_cpus = 1,
>  };
> diff --git a/vl.c b/vl.c
> index df414ef..3a55cc8 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2904,6 +2904,10 @@ int main(int argc, char **argv, char **envp)
>      }
>      qemu_add_globals();
>  
> +    if (cpu_model == NULL) {
> +        cpu_model = machine->cpu_model;
> +    }
> +
>      machine->init(ram_size, boot_devices,
>                    kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
>  
> -- 
> 1.7.0.4
> 


-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632