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

[Anthony Liguori]

On 10/18/2010 03:22 AM, Roedel, Joerg wrote:
(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.
   

I would think that having different default machines for KVM and TCG 
would be a better solution.

Further the QEMUMachine data structure is used for all architectures in
QEMU and the model-names only make sense for x86.

SPARC uses cpu_model too FWIW.  I believe Blue Swirl has even discussed 
using a feature-format similar to how x86 does it for SPARC CPUs.

Regards,

Anthony Liguori

  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<aligu...@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<aligu...@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