Re: [Qemu-devel] [PATCH v3 6/6] pc: Move option_rom_has_mr/rom_file_has_mr globals to MachineClass

[Marcel Apfelbaum <marcel@redhat.com>]

On 12/02/2015 12:58 AM, Eduardo Habkost wrote:
> This way, these settings can be simply set on the corresponding
> machine_options() function, instead of requiring code in
> pc_compat_*() functions.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>   hw/core/loader.c    | 10 +++++-----
>   hw/core/machine.c   |  1 +
>   hw/i386/pc_piix.c   |  8 ++++----
>   hw/i386/pc_q35.c    |  4 ++--
>   include/hw/boards.h |  2 ++
>   5 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index eb67f05..6b69852 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -51,12 +51,10 @@
>   #include "hw/nvram/fw_cfg.h"
>   #include "exec/memory.h"
>   #include "exec/address-spaces.h"
> +#include "hw/boards.h"
>
>   #include <zlib.h>
>
> -bool option_rom_has_mr = false;
> -bool rom_file_has_mr = true;
> -
>   static int roms_loaded;
>
>   /* return the size or -1 if error */
> @@ -754,6 +752,7 @@ int rom_add_file(const char *file, const char *fw_dir,
>                    hwaddr addr, int32_t bootindex,
>                    bool option_rom)
>   {
> +    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
>       Rom *rom;
>       int rc, fd = -1;
>       char devpath[100];
> @@ -810,7 +809,7 @@ int rom_add_file(const char *file, const char *fw_dir,
>                    basename);
>           snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name);
>
> -        if ((!option_rom || option_rom_has_mr) && rom_file_has_mr) {
> +        if ((!option_rom || mc->option_rom_has_mr) && mc->rom_file_has_mr) {
>               data = rom_set_mr(rom, OBJECT(fw_cfg), devpath);
>           } else {
>               data = rom->data;
> @@ -838,6 +837,7 @@ MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len,
>                      size_t max_len, hwaddr addr, const char *fw_file_name,
>                      FWCfgReadCallback fw_callback, void *callback_opaque)
>   {
> +    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
>       Rom *rom;
>       MemoryRegion *mr = NULL;
>
> @@ -855,7 +855,7 @@ MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len,
>
>           snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name);
>
> -        if (rom_file_has_mr) {
> +        if (mc->rom_file_has_mr) {
>               data = rom_set_mr(rom, OBJECT(fw_cfg), devpath);
>               mr = rom->mr;
>           } else {
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index acca00d..1702397 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -314,6 +314,7 @@ static void machine_class_init(ObjectClass *oc, void *data)
>
>       /* Default 128 MB as guest ram size */
>       mc->default_ram_size = 128 * M_BYTE;
> +    mc->rom_file_has_mr = true;
>   }
>
>   static void machine_class_base_init(ObjectClass *oc, void *data)
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 869c64b..0e9a9d8 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -319,14 +319,12 @@ static void pc_compat_2_0(MachineState *machine)
>   static void pc_compat_1_7(MachineState *machine)
>   {
>       pc_compat_2_0(machine);
> -    option_rom_has_mr = true;
>       x86_cpu_change_kvm_default("x2apic", NULL);
>   }
>
>   static void pc_compat_1_6(MachineState *machine)
>   {
>       pc_compat_1_7(machine);
> -    rom_file_has_mr = false;
>   }
>
>   static void pc_compat_1_5(MachineState *machine)
> @@ -360,8 +358,6 @@ static void pc_compat_0_13(MachineState *machine)
>
>   static void pc_init_isa(MachineState *machine)
>   {
> -    option_rom_has_mr = true;
> -    rom_file_has_mr = false;
>       if (!machine->cpu_model) {
>           machine->cpu_model = "486";
>       }
> @@ -526,6 +522,7 @@ static void pc_i440fx_1_7_machine_options(MachineClass *m)
>       pc_i440fx_2_0_machine_options(m);
>       m->hw_version = "1.7.0";
>       m->default_machine_opts = NULL;
> +    m->option_rom_has_mr = true;
>       SET_MACHINE_COMPAT(m, PC_COMPAT_1_7);
>       pcmc->smbios_defaults = false;
>       pcmc->gigabyte_align = false;
> @@ -541,6 +538,7 @@ static void pc_i440fx_1_6_machine_options(MachineClass *m)
>       PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>       pc_i440fx_1_7_machine_options(m);
>       m->hw_version = "1.6.0";
> +    m->rom_file_has_mr = false;
>       SET_MACHINE_COMPAT(m, PC_COMPAT_1_6);
>       pcmc->has_acpi_build = false;
>   }
> @@ -1025,6 +1023,8 @@ static void isapc_machine_options(MachineClass *m)
>       PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>       m->desc = "ISA-only PC";
>       m->max_cpus = 1;
> +    m->option_rom_has_mr = true;
> +    m->rom_file_has_mr = false;
>       pcmc->pci_enabled = false;
>       pcmc->has_acpi_build = false;
>       pcmc->smbios_defaults = false;
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index e16dec1..f004134 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -304,14 +304,12 @@ static void pc_compat_2_0(MachineState *machine)
>   static void pc_compat_1_7(MachineState *machine)
>   {
>       pc_compat_2_0(machine);
> -    option_rom_has_mr = true;
>       x86_cpu_change_kvm_default("x2apic", NULL);
>   }
>
>   static void pc_compat_1_6(MachineState *machine)
>   {
>       pc_compat_1_7(machine);
> -    rom_file_has_mr = false;
>   }
>
>   static void pc_compat_1_5(MachineState *machine)
> @@ -434,6 +432,7 @@ static void pc_q35_1_7_machine_options(MachineClass *m)
>       pc_q35_2_0_machine_options(m);
>       m->hw_version = "1.7.0";
>       m->default_machine_opts = NULL;
> +    m->option_rom_has_mr = true;
>       SET_MACHINE_COMPAT(m, PC_COMPAT_1_7);
>       pcmc->smbios_defaults = false;
>       pcmc->gigabyte_align = false;
> @@ -448,6 +447,7 @@ static void pc_q35_1_6_machine_options(MachineClass *m)
>       PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>       pc_q35_machine_options(m);
>       m->hw_version = "1.6.0";
> +    m->rom_file_has_mr = false;
>       SET_MACHINE_COMPAT(m, PC_COMPAT_1_6);
>       pcmc->has_acpi_build = false;
>   }
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 5da4fb0..ba5628a 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -92,6 +92,8 @@ struct MachineClass {
>       GlobalProperty *compat_props;
>       const char *hw_version;
>       ram_addr_t default_ram_size;
> +    bool option_rom_has_mr;
> +    bool rom_file_has_mr;
>
>       HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
>                                              DeviceState *dev);
>

Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>

Thanks,
Marcel