From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=51620 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P7uWl-0007qj-16 for qemu-devel@nongnu.org; Mon, 18 Oct 2010 14:33:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1P7uWj-00024v-EG for qemu-devel@nongnu.org; Mon, 18 Oct 2010 14:33:42 -0400 Received: from e5.ny.us.ibm.com ([32.97.182.145]:58929) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1P7uWj-00024r-Bi for qemu-devel@nongnu.org; Mon, 18 Oct 2010 14:33:41 -0400 Received: from d01relay05.pok.ibm.com (d01relay05.pok.ibm.com [9.56.227.237]) by e5.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id o9IDuGOs009594 for ; Mon, 18 Oct 2010 09:56:17 -0400 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay05.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o9IEGuOg160670 for ; Mon, 18 Oct 2010 10:16:56 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id o9IEGtDB019789 for ; Mon, 18 Oct 2010 12:16:56 -0200 Message-ID: <4CBC56A7.7020305@linux.vnet.ibm.com> Date: Mon, 18 Oct 2010 09:16:07 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1285593377-1754-1-git-send-email-joerg.roedel@amd.com> <1285593377-1754-2-git-send-email-joerg.roedel@amd.com> <20101006185306.GA8237@amt.cnet> <4CACCD0B.5090302@linux.vnet.ibm.com> <20101007084215.GB1983@amd.com> <4CADC186.3050309@linux.vnet.ibm.com> <20101018082230.GB1890@amd.com> In-Reply-To: <20101018082230.GB1890@amd.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Roedel, Joerg" Cc: Anthony Liguori , "kvm@vger.kernel.org" , Marcelo Tosatti , "qemu-devel@nongnu.org" , Alexander Graf , Avi Kivity 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 >> 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 >> >> 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 >> >> > >