From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Roedel, Joerg" Subject: Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled() Date: Mon, 18 Oct 2010 10:22:30 +0200 Message-ID: <20101018082230.GB1890@amd.com> 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> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Cc: Marcelo Tosatti , Anthony Liguori , Avi Kivity , Alexander Graf , "kvm@vger.kernel.org" , "qemu-devel@nongnu.org" To: Anthony Liguori Return-path: Received: from va3ehsobe006.messaging.microsoft.com ([216.32.180.16]:2356 "EHLO VA3EHSOBE006.bigfish.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751343Ab0JRIW0 (ORCPT ); Mon, 18 Oct 2010 04:22:26 -0400 Content-Disposition: inline In-Reply-To: <4CADC186.3050309@linux.vnet.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: (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 > 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 > -- 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