qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 1/2] x86: Add 2.4 pc machine versions


From: Eduardo Habkost
Subject: Re: [Qemu-devel] [PATCH 1/2] x86: Add 2.4 pc machine versions
Date: Thu, 23 Apr 2015 16:05:56 -0300
User-agent: Mutt/1.5.23 (2014-03-12)

(CCing the PC maintainer, Michael)

On Wed, Apr 22, 2015 at 12:38:10AM +0200, Alexander Graf wrote:
> This patch adds all the boilerplate required to handle a 2.4 machine type
> and compatibility options required on older machines.
> 
> Signed-off-by: Alexander Graf <address@hidden>
> ---
>  hw/i386/pc_piix.c    | 37 +++++++++++++++++++++++++++++++++----
>  hw/i386/pc_q35.c     | 34 +++++++++++++++++++++++++++++++---
>  include/hw/i386/pc.h |  3 +++
>  3 files changed, 67 insertions(+), 7 deletions(-)
> 

This doesn't build:

  CC    x86_64-softmmu/hw/i386/pc_piix.o
/home/ehabkost/rh/proj/virt/qemu/hw/i386/pc_piix.c:546:9: error: expected 
expression before ‘,’ token
         PC_COMPAT_2_3,
         ^
/home/ehabkost/rh/proj/virt/qemu/hw/i386/pc_piix.c:558:9: error: expected 
expression before ‘,’ token
         PC_COMPAT_2_3,
         ^

> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 1fe7bfb..cf14b1f 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -310,8 +310,13 @@ static void pc_init_pci(MachineState *machine)
>      pc_init1(machine, 1, 1);
>  }
>  
> +static void pc_compat_2_3(MachineState *machine)
> +{
> +}
> +
>  static void pc_compat_2_2(MachineState *machine)
>  {
> +    pc_compat_2_3(machine);
>      rsdp_in_ram = false;
>      x86_cpu_compat_set_features("kvm64", FEAT_1_EDX, 0, CPUID_VME);
>      x86_cpu_compat_set_features("kvm32", FEAT_1_EDX, 0, CPUID_VME);
> @@ -413,6 +418,12 @@ static void pc_compat_1_2(MachineState *machine)
>      x86_cpu_compat_kvm_no_autoenable(FEAT_KVM, 1 << KVM_FEATURE_PV_EOI);
>  }
>  
> +static void pc_init_pci_2_3(MachineState *machine)
> +{
> +    pc_compat_2_3(machine);
> +    pc_init_pci(machine);
> +}
> +
>  static void pc_init_pci_2_2(MachineState *machine)
>  {
>      pc_compat_2_2(machine);
> @@ -512,25 +523,41 @@ static void pc_xen_hvm_init(MachineState *machine)
>      .desc = "Standard PC (i440FX + PIIX, 1996)", \
>      .hot_add_cpu = pc_hot_add_cpu
>  
> -#define PC_I440FX_2_3_MACHINE_OPTIONS                           \
> +#define PC_I440FX_2_4_MACHINE_OPTIONS                           \
>      PC_I440FX_MACHINE_OPTIONS,                                  \
>      .default_machine_opts = "firmware=bios-256k.bin",           \
>      .default_display = "std"
>  
> -static QEMUMachine pc_i440fx_machine_v2_3 = {
> -    PC_I440FX_2_3_MACHINE_OPTIONS,
> -    .name = "pc-i440fx-2.3",
> +static QEMUMachine pc_i440fx_machine_v2_4 = {
> +    PC_I440FX_2_4_MACHINE_OPTIONS,
> +    .name = "pc-i440fx-2.4",
>      .alias = "pc",
>      .init = pc_init_pci,
>      .is_default = 1,
>  };
>  
> +#define PC_I440FX_2_3_MACHINE_OPTIONS PC_I440FX_2_4_MACHINE_OPTIONS
> +
> +static QEMUMachine pc_i440fx_machine_v2_3 = {
> +    PC_I440FX_2_3_MACHINE_OPTIONS,
> +    .name = "pc-i440fx-2.3",
> +    .init = pc_init_pci_2_3,
> +    .compat_props = (GlobalProperty[]) {
> +        PC_COMPAT_2_3,
> +        { /* end of list */ }
> +    },
> +};
> +
>  #define PC_I440FX_2_2_MACHINE_OPTIONS PC_I440FX_2_3_MACHINE_OPTIONS
>  
>  static QEMUMachine pc_i440fx_machine_v2_2 = {
>      PC_I440FX_2_2_MACHINE_OPTIONS,
>      .name = "pc-i440fx-2.2",
>      .init = pc_init_pci_2_2,
> +    .compat_props = (GlobalProperty[]) {
> +        PC_COMPAT_2_3,
> +        { /* end of list */ }
> +    },
>  };
>  
>  #define PC_I440FX_2_1_MACHINE_OPTIONS                           \
> @@ -543,6 +570,7 @@ static QEMUMachine pc_i440fx_machine_v2_1 = {
>      .init = pc_init_pci_2_1,
>      .compat_props = (GlobalProperty[]) {
>          HW_COMPAT_2_1,
> +        PC_COMPAT_2_3,

PC_COMPAT_2_3 must be in the beginning of the list, not in the end. To make
QEMU 2.4 behave like QEMU 2.1, first we apply the properties that make QEMU 2.4
behave as QEMU 2.3, then we apply the properties that made QEMU 2.3 behave as
QEMU 2.2, then we apply the properties that made QEMU 2.2 behave as QEMU 2.1.

Also, why did you decide to define a PC_COMPAT_2_3 macro and not define
a PC_COMPAT_2_2 macro?

We could make it more consistent and always define the PC_* macros even if they
are empty (like you already tried to do on PC_COMPAT_2_3), like this:

    #define PC_COMPAT_2_3 \
            /* empty */

    #define PC_COMPAT_2_2 \
            PC_COMPAT_2_3
            /* no extra elements */

    #define PC_COMPAT_2_1 \
            PC_COMPAT_2_2
            HW_COMPAT_2_1
            { /* item 1 */ },
            { /* item 2 */ },

    #define PC_COMPAT_2_0 \
            PC_COMPAT_2_1
            { /* item 1 */ },
            { /* item 2 */ },

Note that I moved the final commas inside the macros, to allow us to define
empty compat macros without compilation errors like the one introduced by this
patch.


>          { /* end of list */ }
>      },
>  };
> @@ -970,6 +998,7 @@ static QEMUMachine xenfv_machine = {
>  
>  static void pc_machine_init(void)
>  {
> +    qemu_register_pc_machine(&pc_i440fx_machine_v2_4);
>      qemu_register_pc_machine(&pc_i440fx_machine_v2_3);
>      qemu_register_pc_machine(&pc_i440fx_machine_v2_2);
>      qemu_register_pc_machine(&pc_i440fx_machine_v2_1);
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index dcc17c0..8ef9e27 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -289,6 +289,10 @@ static void pc_q35_init(MachineState *machine)
>      }
>  }
>  
> +static void pc_compat_2_3(MachineState *machine)
> +{
> +}
> +
>  static void pc_compat_2_2(MachineState *machine)
>  {
>      rsdp_in_ram = false;
> @@ -361,6 +365,12 @@ static void pc_compat_1_4(MachineState *machine)
>      x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, 
> CPUID_EXT_PCLMULQDQ);
>  }
>  
> +static void pc_q35_init_2_3(MachineState *machine)
> +{
> +    pc_compat_2_3(machine);
> +    pc_q35_init(machine);
> +}
> +
>  static void pc_q35_init_2_2(MachineState *machine)
>  {
>      pc_compat_2_2(machine);
> @@ -410,16 +420,28 @@ static void pc_q35_init_1_4(MachineState *machine)
>      .hot_add_cpu = pc_hot_add_cpu, \
>      .units_per_default_bus = 1
>  
> -#define PC_Q35_2_3_MACHINE_OPTIONS                      \
> +#define PC_Q35_2_4_MACHINE_OPTIONS                      \
>      PC_Q35_MACHINE_OPTIONS,                             \
>      .default_machine_opts = "firmware=bios-256k.bin",   \
>      .default_display = "std"
>  
> +static QEMUMachine pc_q35_machine_v2_4 = {
> +    PC_Q35_2_4_MACHINE_OPTIONS,
> +    .name = "pc-q35-2.4",
> +    .alias = "q35",
> +    .init = pc_q35_init,
> +};
> +
> +#define PC_Q35_2_3_MACHINE_OPTIONS PC_Q35_2_4_MACHINE_OPTIONS
> +
>  static QEMUMachine pc_q35_machine_v2_3 = {
>      PC_Q35_2_3_MACHINE_OPTIONS,
>      .name = "pc-q35-2.3",
> -    .alias = "q35",
> -    .init = pc_q35_init,
> +    .init = pc_q35_init_2_3,
> +    .compat_props = (GlobalProperty[]) {
> +        PC_COMPAT_2_3,
> +        { /* end of list */ }
> +    },
>  };
>  
>  #define PC_Q35_2_2_MACHINE_OPTIONS PC_Q35_2_3_MACHINE_OPTIONS
> @@ -428,6 +450,10 @@ static QEMUMachine pc_q35_machine_v2_2 = {
>      PC_Q35_2_2_MACHINE_OPTIONS,
>      .name = "pc-q35-2.2",
>      .init = pc_q35_init_2_2,
> +    .compat_props = (GlobalProperty[]) {
> +        PC_COMPAT_2_3,
> +        { /* end of list */ }
> +    },
>  };
>  
>  #define PC_Q35_2_1_MACHINE_OPTIONS                      \
> @@ -440,6 +466,7 @@ static QEMUMachine pc_q35_machine_v2_1 = {
>      .init = pc_q35_init_2_1,
>      .compat_props = (GlobalProperty[]) {
>          HW_COMPAT_2_1,
> +        PC_COMPAT_2_3,
>          { /* end of list */ }
>      },
>  };
> @@ -506,6 +533,7 @@ static QEMUMachine pc_q35_machine_v1_4 = {
>  
>  static void pc_q35_machine_init(void)
>  {
> +    qemu_register_pc_machine(&pc_q35_machine_v2_4);
>      qemu_register_pc_machine(&pc_q35_machine_v2_3);
>      qemu_register_pc_machine(&pc_q35_machine_v2_2);
>      qemu_register_pc_machine(&pc_q35_machine_v2_1);
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 1b35168..05ac5f9 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -295,7 +295,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
>  int e820_get_num_entries(void);
>  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>  
> +#define PC_COMPAT_2_3 \
> +
>  #define PC_COMPAT_2_0 \
> +        PC_COMPAT_2_3, \
>          HW_COMPAT_2_1, \
>          {\
>              .driver   = "virtio-scsi-pci",\
> -- 
> 1.7.12.4
> 
> 

-- 
Eduardo



reply via email to

[Prev in Thread] Current Thread [Next in Thread]