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

[Anthony Liguori <aliguori@linux.vnet.ibm.com>]

On 10/06/2010 01:53 PM, Marcelo Tosatti wrote:
> On Mon, Sep 27, 2010 at 03:16:15PM +0200, Joerg Roedel wrote:
>    
>> As requested by Alex this patch makes kvm64 the default CPU
>> model when qemu is started with -enable-kvm. This takes only
>> effect for qemu-versions newer or equal to 0.14.0.
>>
>> Signed-off-by: Joerg Roedel<joerg.roedel@amd.com>
>> ---
>>   hw/boards.h    |    1 +
>>   hw/pc.c        |   21 ++++++++++++++++-----
>>   hw/pc_piix.c   |    6 ++++++
>>   qemu-version.h |   35 +++++++++++++++++++++++++++++++++++
>>   vl.c           |    4 ++++
>>   5 files changed, 62 insertions(+), 5 deletions(-)
>>   create mode 100644 qemu-version.h
>>
>> diff --git a/hw/boards.h b/hw/boards.h
>> index 6f0f0d7..2d41b2d 100644
>> --- a/hw/boards.h
>> +++ b/hw/boards.h
>> @@ -19,6 +19,7 @@ typedef struct QEMUMachine {
>>       QEMUMachineInitFunc *init;
>>       int use_scsi;
>>       int max_cpus;
>> +    unsigned int compat_version;
>>       unsigned int no_serial:1,
>>           no_parallel:1,
>>           use_virtcon:1,
>> diff --git a/hw/pc.c b/hw/pc.c
>> index 69b13bf..372ec4c 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -40,6 +40,16 @@
>>   #include "sysbus.h"
>>   #include "sysemu.h"
>>   #include "blockdev.h"
>> +#include "kvm.h"
>> +#include "qemu-version.h"
>> +
>> +#ifdef TARGET_X86_64
>> +#define DEFAULT_KVM_CPU_MODEL "kvm64"
>> +#define DEFAULT_QEMU_CPU_MODEL "qemu64"
>> +#else
>> +#define DEFAULT_KVM_CPU_MODEL "kvm32"
>> +#define DEFAULT_QEMU_CPU_MODEL "qemu32"
>> +#endif
>>
>>   /* output Bochs bios info messages */
>>   //#define DEBUG_BIOS
>> @@ -867,11 +877,12 @@ void pc_cpus_init(const char *cpu_model)
>>
>>       /* init CPUs */
>>       if (cpu_model == NULL) {
>> -#ifdef TARGET_X86_64
>> -        cpu_model = "qemu64";
>> -#else
>> -        cpu_model = "qemu32";
>> -#endif
>> +        if (kvm_enabled()&&
>> +            qemu_compat_version>= QEMU_COMPAT_VERSION(0, 14, 0)) {
>> +            cpu_model = DEFAULT_KVM_CPU_MODEL;
>> +        } else {
>> +            cpu_model = DEFAULT_QEMU_CPU_MODEL;
>> +        }
>>       }
>>
>>       for(i = 0; i<  smp_cpus; i++) {
>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
>> index 12359a7..9e46b71 100644
>> --- a/hw/pc_piix.c
>> +++ b/hw/pc_piix.c
>> @@ -35,6 +35,7 @@
>>   #include "sysemu.h"
>>   #include "sysbus.h"
>>   #include "blockdev.h"
>> +#include "qemu-version.h"
>>
>>   #define MAX_IDE_BUS 2
>>
>> @@ -217,6 +218,7 @@ static QEMUMachine pc_machine = {
>>       .desc = "Standard PC",
>>       .init = pc_init_pci,
>>       .max_cpus = 255,
>> +    .compat_version = QEMU_COMPAT_VERSION(0, 13, 0),
>>       .is_default = 1,
>>   };
>>
>> @@ -225,6 +227,7 @@ static QEMUMachine pc_machine_v0_12 = {
>>       .desc = "Standard PC",
>>       .init = pc_init_pci,
>>       .max_cpus = 255,
>> +    .compat_version = QEMU_COMPAT_VERSION(0, 12, 0),
>>       .compat_props = (GlobalProperty[]) {
>>           {
>>               .driver   = "virtio-serial-pci",
>> @@ -244,6 +247,7 @@ static QEMUMachine pc_machine_v0_11 = {
>>       .desc = "Standard PC, qemu 0.11",
>>       .init = pc_init_pci,
>>       .max_cpus = 255,
>> +    .compat_version = QEMU_COMPAT_VERSION(0, 11, 0),
>>       .compat_props = (GlobalProperty[]) {
>>           {
>>               .driver   = "virtio-blk-pci",
>> @@ -279,6 +283,7 @@ static QEMUMachine pc_machine_v0_10 = {
>>       .desc = "Standard PC, qemu 0.10",
>>       .init = pc_init_pci,
>>       .max_cpus = 255,
>> +    .compat_version = QEMU_COMPAT_VERSION(0, 10, 0),
>>       .compat_props = (GlobalProperty[]) {
>>           {
>>               .driver   = "virtio-blk-pci",
>> @@ -325,6 +330,7 @@ static QEMUMachine isapc_machine = {
>>       .name = "isapc",
>>       .desc = "ISA-only PC",
>>       .init = pc_init_isa,
>> +    .compat_version = QEMU_COMPAT_VERSION(0, 10, 0),
>>       .max_cpus = 1,
>>   };
>>
>> diff --git a/qemu-version.h b/qemu-version.h
>> new file mode 100644
>> index 0000000..b4bfe48
>> --- /dev/null
>> +++ b/qemu-version.h
>> @@ -0,0 +1,35 @@
>> +/*
>> + * qemu-version.h
>> + *
>> + * Defines needed for handling QEMU version compatibility
>> + *
>> + * Copyright (c) 2010 Joerg Roedel<joerg.roedel@amd.com>
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this software and associated documentation files (the "Software"), to deal
>> + * in the Software without restriction, including without limitation the rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +
>> +#ifndef _QEMU_VERSION_H_
>> +#define _QEMU_VERSION_H_
>> +
>> +extern unsigned int qemu_compat_version;
>> +
>> +#define QEMU_COMPAT_VERSION(major, minor, patchlevel) \
>> +        ((unsigned int)(major<<  16) | (minor<<  8) | (patchlevel))
>> +
>> +#endif
>> diff --git a/vl.c b/vl.c
>> index d352d18..37727f3 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -161,6 +161,7 @@ int main(int argc, char **argv)
>>   #include "qemu-queue.h"
>>   #include "cpus.h"
>>   #include "arch_init.h"
>> +#include "qemu-version.h"
>>
>>   //#define DEBUG_NET
>>   //#define DEBUG_SLIRP
>> @@ -169,6 +170,7 @@ int main(int argc, char **argv)
>>
>>   #define MAX_VIRTIO_CONSOLES 1
>>
>> +unsigned int qemu_compat_version;
>>   static const char *data_dir;
>>   const char *bios_name = NULL;
>>   enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
>> @@ -2696,6 +2698,8 @@ int main(int argc, char **argv, char **envp)
>>           default_sdcard = 0;
>>       }
>>
>> +    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?

Regards,

Anthony Liguori