[Qemu-devel] [PATCH v3 0/6] pc: Initialization and compat function cleanup

[Qemu-devel] [PATCH v3 0/6] pc: Initialization and compat function cleanup

[Eduardo Habkost]

This moves most compat data that are globals or PCMachineState
fields to PCMachineClass.

It looks like v2 (sent in August) has slipped through the cracks.
Rebased to latest qemu.git, and added 2 additional patches.

Eduardo Habkost (6):
  pc: Move compat boolean globals to PCMachineClass
  pc: Move legacy_acpi_table_size global to PCMachineClass
  pc: Move acpi_data_size global to PCMachineClass
  pc: Move enforce_aligned_dimm to PCMachineClass
  pc: Remove enforce-aligned-dimm QOM property
  pc: Move option_rom_has_mr/rom_file_has_mr globals to MachineClass

 hw/core/loader.c     |  10 ++--
 hw/core/machine.c    |   1 +
 hw/i386/pc.c         |  44 +++++++---------
 hw/i386/pc_piix.c    | 138 ++++++++++++++++++++++++---------------------------
 hw/i386/pc_q35.c     |  64 +++++++++++-------------
 include/hw/boards.h  |   2 +
 include/hw/i386/pc.h |  28 ++++++++---
 7 files changed, 142 insertions(+), 145 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH v3 1/6] pc: Move compat boolean globals to PCMachineClass

[Eduardo Habkost]

This way the compat flags can be initialized in the machine_options()
function. This will help us to eventually eliminate the pc_compat_*()
functions.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc.c         |  8 +++++
 hw/i386/pc_piix.c    | 84 +++++++++++++++++++++++++---------------------------
 hw/i386/pc_q35.c     | 54 +++++++++++++++------------------
 include/hw/i386/pc.h | 14 +++++++++
 4 files changed, 86 insertions(+), 74 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 5e20e07..129aa04 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1952,6 +1952,14 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
 
     pcmc->get_hotplug_handler = mc->get_hotplug_handler;
+    pcmc->pci_enabled = true;
+    pcmc->has_acpi_build = true;
+    pcmc->rsdp_in_ram = true;
+    pcmc->smbios_defaults = true;
+    pcmc->smbios_uuid_encoded = true;
+    pcmc->gigabyte_align = true;
+    pcmc->has_reserved_memory = true;
+    pcmc->kvmclock_enabled = true;
     mc->get_hotplug_handler = pc_get_hotpug_handler;
     mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
     mc->default_boot_order = "cad";
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 2e41efe..7a7f748 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -60,26 +60,14 @@ static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
 static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
 static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
 
-static bool pci_enabled = true;
-static bool has_acpi_build = true;
-static bool rsdp_in_ram = true;
 static int legacy_acpi_table_size;
-static bool smbios_defaults = true;
-static bool smbios_legacy_mode;
-static bool smbios_uuid_encoded = true;
-/* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
- * host addresses aligned at 1Gbyte boundaries.  This way we can use 1GByte
- * pages in the host.
- */
-static bool gigabyte_align = true;
-static bool has_reserved_memory = true;
-static bool kvmclock_enabled = true;
 
 /* PC hardware initialisation */
 static void pc_init1(MachineState *machine,
                      const char *host_type, const char *pci_type)
 {
     PCMachineState *pcms = PC_MACHINE(machine);
+    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
     MemoryRegion *system_memory = get_system_memory();
     MemoryRegion *system_io = get_system_io();
     int i;
@@ -108,7 +96,7 @@ static void pc_init1(MachineState *machine,
      * breaking migration.
      */
     if (machine->ram_size >= 0xe0000000) {
-        lowmem = gigabyte_align ? 0xc0000000 : 0xe0000000;
+        lowmem = pcmc->gigabyte_align ? 0xc0000000 : 0xe0000000;
     } else {
         lowmem = 0xe0000000;
     }
@@ -141,11 +129,11 @@ static void pc_init1(MachineState *machine,
 
     pc_cpus_init(pcms);
 
-    if (kvm_enabled() && kvmclock_enabled) {
+    if (kvm_enabled() && pcmc->kvmclock_enabled) {
         kvmclock_create();
     }
 
-    if (pci_enabled) {
+    if (pcmc->pci_enabled) {
         pci_memory = g_new(MemoryRegion, 1);
         memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
         rom_memory = pci_memory;
@@ -156,18 +144,19 @@ static void pc_init1(MachineState *machine,
 
     guest_info = pc_guest_info_init(pcms);
 
-    guest_info->has_acpi_build = has_acpi_build;
+    guest_info->has_acpi_build = pcmc->has_acpi_build;
     guest_info->legacy_acpi_table_size = legacy_acpi_table_size;
 
-    guest_info->isapc_ram_fw = !pci_enabled;
-    guest_info->has_reserved_memory = has_reserved_memory;
-    guest_info->rsdp_in_ram = rsdp_in_ram;
+    guest_info->isapc_ram_fw = !pcmc->pci_enabled;
+    guest_info->has_reserved_memory = pcmc->has_reserved_memory;
+    guest_info->rsdp_in_ram = pcmc->rsdp_in_ram;
 
-    if (smbios_defaults) {
+    if (pcmc->smbios_defaults) {
         MachineClass *mc = MACHINE_GET_CLASS(machine);
         /* These values are guest ABI, do not change */
         smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
-                            mc->name, smbios_legacy_mode, smbios_uuid_encoded,
+                            mc->name, pcmc->smbios_legacy_mode,
+                            pcmc->smbios_uuid_encoded,
                             SMBIOS_ENTRY_POINT_21);
     }
 
@@ -182,14 +171,14 @@ static void pc_init1(MachineState *machine,
 
     gsi_state = g_malloc0(sizeof(*gsi_state));
     if (kvm_irqchip_in_kernel()) {
-        kvm_pc_setup_irq_routing(pci_enabled);
+        kvm_pc_setup_irq_routing(pcmc->pci_enabled);
         gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
                                  GSI_NUM_PINS);
     } else {
         gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
     }
 
-    if (pci_enabled) {
+    if (pcmc->pci_enabled) {
         pci_bus = i440fx_init(host_type,
                               pci_type,
                               &i440fx_state, &piix3_devfn, &isa_bus, gsi,
@@ -217,13 +206,13 @@ static void pc_init1(MachineState *machine,
         gsi_state->i8259_irq[i] = i8259[i];
     }
     g_free(i8259);
-    if (pci_enabled) {
+    if (pcmc->pci_enabled) {
         ioapic_init_gsi(gsi_state, "i440fx");
     }
 
     pc_register_ferr_irq(gsi[13]);
 
-    pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
+    pc_vga_init(isa_bus, pcmc->pci_enabled ? pci_bus : NULL);
 
     assert(pcms->vmport != ON_OFF_AUTO_MAX);
     if (pcms->vmport == ON_OFF_AUTO_AUTO) {
@@ -237,7 +226,7 @@ static void pc_init1(MachineState *machine,
     pc_nic_init(isa_bus, pci_bus);
 
     ide_drive_get(hd, ARRAY_SIZE(hd));
-    if (pci_enabled) {
+    if (pcmc->pci_enabled) {
         PCIDevice *dev;
         if (xen_enabled()) {
             dev = pci_piix3_xen_ide_init(pci_bus, hd, piix3_devfn + 1);
@@ -264,11 +253,11 @@ static void pc_init1(MachineState *machine,
 
     pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state);
 
-    if (pci_enabled && usb_enabled()) {
+    if (pcmc->pci_enabled && usb_enabled()) {
         pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
     }
 
-    if (pci_enabled && acpi_enabled) {
+    if (pcmc->pci_enabled && acpi_enabled) {
         DeviceState *piix4_pm;
         I2CBus *smbus;
 
@@ -289,7 +278,7 @@ static void pc_init1(MachineState *machine,
                                  PC_MACHINE_ACPI_DEVICE_PROP, &error_abort);
     }
 
-    if (pci_enabled) {
+    if (pcmc->pci_enabled) {
         pc_pci_device_init(pci_bus);
     }
 }
@@ -315,7 +304,6 @@ 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;
     machine->suppress_vmdesc = true;
 }
 
@@ -324,7 +312,6 @@ static void pc_compat_2_1(MachineState *machine)
     PCMachineState *pcms = PC_MACHINE(machine);
 
     pc_compat_2_2(machine);
-    smbios_uuid_encoded = false;
     x86_cpu_change_kvm_default("svm", NULL);
     pcms->enforce_aligned_dimm = false;
 }
@@ -349,16 +336,12 @@ static void pc_compat_2_0(MachineState *machine)
      * QEMU 1.7 it is 6414.  For RHEL/CentOS 7.0 it is 6418.
      */
     legacy_acpi_table_size = 6652;
-    smbios_legacy_mode = true;
-    has_reserved_memory = false;
     pc_set_legacy_acpi_data_size();
 }
 
 static void pc_compat_1_7(MachineState *machine)
 {
     pc_compat_2_0(machine);
-    smbios_defaults = false;
-    gigabyte_align = false;
     option_rom_has_mr = true;
     legacy_acpi_table_size = 6414;
     x86_cpu_change_kvm_default("x2apic", NULL);
@@ -368,7 +351,6 @@ static void pc_compat_1_6(MachineState *machine)
 {
     pc_compat_1_7(machine);
     rom_file_has_mr = false;
-    has_acpi_build = false;
 }
 
 static void pc_compat_1_5(MachineState *machine)
@@ -398,17 +380,10 @@ static void pc_compat_1_2(MachineState *machine)
 static void pc_compat_0_13(MachineState *machine)
 {
     pc_compat_1_2(machine);
-    kvmclock_enabled = false;
 }
 
 static void pc_init_isa(MachineState *machine)
 {
-    pci_enabled = false;
-    has_acpi_build = false;
-    smbios_defaults = false;
-    gigabyte_align = false;
-    smbios_legacy_mode = true;
-    has_reserved_memory = false;
     option_rom_has_mr = true;
     rom_file_has_mr = false;
     if (!machine->cpu_model) {
@@ -510,9 +485,11 @@ DEFINE_I440FX_MACHINE(v2_3, "pc-i440fx-2.3", pc_compat_2_3,
 
 static void pc_i440fx_2_2_machine_options(MachineClass *m)
 {
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_i440fx_2_3_machine_options(m);
     m->hw_version = "2.2.0";
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_2);
+    pcmc->rsdp_in_ram = false;
 }
 
 DEFINE_I440FX_MACHINE(v2_2, "pc-i440fx-2.2", pc_compat_2_2,
@@ -521,10 +498,12 @@ DEFINE_I440FX_MACHINE(v2_2, "pc-i440fx-2.2", pc_compat_2_2,
 
 static void pc_i440fx_2_1_machine_options(MachineClass *m)
 {
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_i440fx_2_2_machine_options(m);
     m->hw_version = "2.1.0";
     m->default_display = NULL;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_1);
+    pcmc->smbios_uuid_encoded = false;
 }
 
 DEFINE_I440FX_MACHINE(v2_1, "pc-i440fx-2.1", pc_compat_2_1,
@@ -534,9 +513,12 @@ DEFINE_I440FX_MACHINE(v2_1, "pc-i440fx-2.1", pc_compat_2_1,
 
 static void pc_i440fx_2_0_machine_options(MachineClass *m)
 {
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_i440fx_2_1_machine_options(m);
     m->hw_version = "2.0.0";
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_0);
+    pcmc->smbios_legacy_mode = true;
+    pcmc->has_reserved_memory = false;
 }
 
 DEFINE_I440FX_MACHINE(v2_0, "pc-i440fx-2.0", pc_compat_2_0,
@@ -545,10 +527,13 @@ DEFINE_I440FX_MACHINE(v2_0, "pc-i440fx-2.0", pc_compat_2_0,
 
 static void pc_i440fx_1_7_machine_options(MachineClass *m)
 {
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_i440fx_2_0_machine_options(m);
     m->hw_version = "1.7.0";
     m->default_machine_opts = NULL;
     SET_MACHINE_COMPAT(m, PC_COMPAT_1_7);
+    pcmc->smbios_defaults = false;
+    pcmc->gigabyte_align = false;
 }
 
 DEFINE_I440FX_MACHINE(v1_7, "pc-i440fx-1.7", pc_compat_1_7,
@@ -557,9 +542,11 @@ DEFINE_I440FX_MACHINE(v1_7, "pc-i440fx-1.7", pc_compat_1_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";
     SET_MACHINE_COMPAT(m, PC_COMPAT_1_6);
+    pcmc->has_acpi_build = false;
 }
 
 DEFINE_I440FX_MACHINE(v1_6, "pc-i440fx-1.6", pc_compat_1_6,
@@ -813,9 +800,11 @@ DEFINE_I440FX_MACHINE(v0_14, "pc-0.14", pc_compat_1_2,
 
 static void pc_i440fx_0_13_machine_options(MachineClass *m)
 {
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_i440fx_0_14_machine_options(m);
     m->hw_version = "0.13";
     SET_MACHINE_COMPAT(m, PC_COMPAT_0_13);
+    pcmc->kvmclock_enabled = false;
 }
 
 DEFINE_I440FX_MACHINE(v0_13, "pc-0.13", pc_compat_0_13,
@@ -1037,8 +1026,15 @@ void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id)
 
 static void isapc_machine_options(MachineClass *m)
 {
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     m->desc = "ISA-only PC";
     m->max_cpus = 1;
+    pcmc->pci_enabled = false;
+    pcmc->has_acpi_build = false;
+    pcmc->smbios_defaults = false;
+    pcmc->gigabyte_align = false;
+    pcmc->smbios_legacy_mode = true;
+    pcmc->has_reserved_memory = false;
 }
 
 DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 133bc68..f9afaa2 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -49,22 +49,11 @@
 /* ICH9 AHCI has 6 ports */
 #define MAX_SATA_PORTS     6
 
-static bool has_acpi_build = true;
-static bool rsdp_in_ram = true;
-static bool smbios_defaults = true;
-static bool smbios_legacy_mode;
-static bool smbios_uuid_encoded = true;
-/* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
- * host addresses aligned at 1Gbyte boundaries.  This way we can use 1GByte
- * pages in the host.
- */
-static bool gigabyte_align = true;
-static bool has_reserved_memory = true;
-
 /* PC hardware initialisation */
 static void pc_q35_init(MachineState *machine)
 {
     PCMachineState *pcms = PC_MACHINE(machine);
+    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
     Q35PCIHost *q35_host;
     PCIHostState *phb;
     PCIBus *host_bus;
@@ -76,7 +65,6 @@ static void pc_q35_init(MachineState *machine)
     MemoryRegion *ram_memory;
     GSIState *gsi_state;
     ISABus *isa_bus;
-    int pci_enabled = 1;
     qemu_irq *gsi;
     qemu_irq *i8259;
     int i;
@@ -97,7 +85,7 @@ static void pc_q35_init(MachineState *machine)
      * breaking migration.
      */
     if (machine->ram_size >= 0xb0000000) {
-        lowmem = gigabyte_align ? 0x80000000 : 0xb0000000;
+        lowmem = pcmc->gigabyte_align ? 0x80000000 : 0xb0000000;
     } else {
         lowmem = 0xb0000000;
     }
@@ -134,7 +122,7 @@ static void pc_q35_init(MachineState *machine)
     kvmclock_create();
 
     /* pci enabled */
-    if (pci_enabled) {
+    if (pcmc->pci_enabled) {
         pci_memory = g_new(MemoryRegion, 1);
         memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
         rom_memory = pci_memory;
@@ -145,19 +133,20 @@ static void pc_q35_init(MachineState *machine)
 
     guest_info = pc_guest_info_init(pcms);
     guest_info->isapc_ram_fw = false;
-    guest_info->has_acpi_build = has_acpi_build;
-    guest_info->has_reserved_memory = has_reserved_memory;
-    guest_info->rsdp_in_ram = rsdp_in_ram;
+    guest_info->has_acpi_build = pcmc->has_acpi_build;
+    guest_info->has_reserved_memory = pcmc->has_reserved_memory;
+    guest_info->rsdp_in_ram = pcmc->rsdp_in_ram;
 
     /* Migration was not supported in 2.0 for Q35, so do not bother
      * with this hack (see hw/i386/acpi-build.c).
      */
     guest_info->legacy_acpi_table_size = 0;
 
-    if (smbios_defaults) {
+    if (pcmc->smbios_defaults) {
         /* These values are guest ABI, do not change */
         smbios_set_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
-                            mc->name, smbios_legacy_mode, smbios_uuid_encoded,
+                            mc->name, pcmc->smbios_legacy_mode,
+                            pcmc->smbios_uuid_encoded,
                             SMBIOS_ENTRY_POINT_21);
     }
 
@@ -170,7 +159,7 @@ static void pc_q35_init(MachineState *machine)
     /* irq lines */
     gsi_state = g_malloc0(sizeof(*gsi_state));
     if (kvm_irqchip_in_kernel()) {
-        kvm_pc_setup_irq_routing(pci_enabled);
+        kvm_pc_setup_irq_routing(pcmc->pci_enabled);
         gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
                                  GSI_NUM_PINS);
     } else {
@@ -227,7 +216,7 @@ static void pc_q35_init(MachineState *machine)
     for (i = 0; i < ISA_NUM_IRQS; i++) {
         gsi_state->i8259_irq[i] = i8259[i];
     }
-    if (pci_enabled) {
+    if (pcmc->pci_enabled) {
         ioapic_init_gsi(gsi_state, "q35");
     }
 
@@ -272,7 +261,7 @@ static void pc_q35_init(MachineState *machine)
     /* the rest devices to which pci devfn is automatically assigned */
     pc_vga_init(isa_bus, host_bus);
     pc_nic_init(isa_bus, host_bus);
-    if (pci_enabled) {
+    if (pcmc->pci_enabled) {
         pc_pci_device_init(host_bus);
     }
 }
@@ -298,7 +287,6 @@ 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;
     machine->suppress_vmdesc = true;
 }
 
@@ -308,23 +296,18 @@ static void pc_compat_2_1(MachineState *machine)
 
     pc_compat_2_2(machine);
     pcms->enforce_aligned_dimm = false;
-    smbios_uuid_encoded = false;
     x86_cpu_change_kvm_default("svm", NULL);
 }
 
 static void pc_compat_2_0(MachineState *machine)
 {
     pc_compat_2_1(machine);
-    smbios_legacy_mode = true;
-    has_reserved_memory = false;
     pc_set_legacy_acpi_data_size();
 }
 
 static void pc_compat_1_7(MachineState *machine)
 {
     pc_compat_2_0(machine);
-    smbios_defaults = false;
-    gigabyte_align = false;
     option_rom_has_mr = true;
     x86_cpu_change_kvm_default("x2apic", NULL);
 }
@@ -333,7 +316,6 @@ static void pc_compat_1_6(MachineState *machine)
 {
     pc_compat_1_7(machine);
     rom_file_has_mr = false;
-    has_acpi_build = false;
 }
 
 static void pc_compat_1_5(MachineState *machine)
@@ -409,9 +391,11 @@ DEFINE_Q35_MACHINE(v2_3, "pc-q35-2.3", pc_compat_2_3,
 
 static void pc_q35_2_2_machine_options(MachineClass *m)
 {
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_q35_2_3_machine_options(m);
     m->hw_version = "2.2.0";
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_2);
+    pcmc->rsdp_in_ram = false;
 }
 
 DEFINE_Q35_MACHINE(v2_2, "pc-q35-2.2", pc_compat_2_2,
@@ -420,10 +404,12 @@ DEFINE_Q35_MACHINE(v2_2, "pc-q35-2.2", pc_compat_2_2,
 
 static void pc_q35_2_1_machine_options(MachineClass *m)
 {
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_q35_2_2_machine_options(m);
     m->hw_version = "2.1.0";
     m->default_display = NULL;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_1);
+    pcmc->smbios_uuid_encoded = false;
 }
 
 DEFINE_Q35_MACHINE(v2_1, "pc-q35-2.1", pc_compat_2_1,
@@ -432,9 +418,12 @@ DEFINE_Q35_MACHINE(v2_1, "pc-q35-2.1", pc_compat_2_1,
 
 static void pc_q35_2_0_machine_options(MachineClass *m)
 {
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_q35_2_1_machine_options(m);
     m->hw_version = "2.0.0";
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_0);
+    pcmc->has_reserved_memory = false;
+    pcmc->smbios_legacy_mode = true;
 }
 
 DEFINE_Q35_MACHINE(v2_0, "pc-q35-2.0", pc_compat_2_0,
@@ -443,10 +432,13 @@ DEFINE_Q35_MACHINE(v2_0, "pc-q35-2.0", pc_compat_2_0,
 
 static void pc_q35_1_7_machine_options(MachineClass *m)
 {
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_q35_2_0_machine_options(m);
     m->hw_version = "1.7.0";
     m->default_machine_opts = NULL;
     SET_MACHINE_COMPAT(m, PC_COMPAT_1_7);
+    pcmc->smbios_defaults = false;
+    pcmc->gigabyte_align = false;
 }
 
 DEFINE_Q35_MACHINE(v1_7, "pc-q35-1.7", pc_compat_1_7,
@@ -455,9 +447,11 @@ DEFINE_Q35_MACHINE(v1_7, "pc-q35-1.7", pc_compat_1_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";
     SET_MACHINE_COMPAT(m, PC_COMPAT_1_6);
+    pcmc->has_acpi_build = false;
 }
 
 DEFINE_Q35_MACHINE(v1_6, "pc-q35-1.6", pc_compat_1_6,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 854c330..15171e9 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -62,6 +62,20 @@ struct PCMachineClass {
     bool broken_reserved_end;
     HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
                                            DeviceState *dev);
+
+    bool pci_enabled;
+    bool has_acpi_build;
+    bool rsdp_in_ram;
+    bool smbios_defaults;
+    bool smbios_legacy_mode;
+    bool smbios_uuid_encoded;
+    /* Make sure that guest addresses aligned at 1Gbyte boundaries get
+     * mapped to host addresses aligned at 1Gbyte boundaries.  This way
+     * we can use 1GByte pages in the host.
+     */
+    bool gigabyte_align;
+    bool has_reserved_memory;
+    bool kvmclock_enabled;
 };
 
 #define TYPE_PC_MACHINE "generic-pc-machine"
-- 
2.1.0

Re: [Qemu-devel] [PATCH v3 1/6] pc: Move compat boolean globals to PCMachineClass

[Marcel Apfelbaum]

On 12/02/2015 12:58 AM, Eduardo Habkost wrote:
> This way the compat flags can be initialized in the machine_options()
> function. This will help us to eventually eliminate the pc_compat_*()
> functions.

Hi, I have only a minor comment here,

>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>   hw/i386/pc.c         |  8 +++++
>   hw/i386/pc_piix.c    | 84 +++++++++++++++++++++++++---------------------------
>   hw/i386/pc_q35.c     | 54 +++++++++++++++------------------
>   include/hw/i386/pc.h | 14 +++++++++
>   4 files changed, 86 insertions(+), 74 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 5e20e07..129aa04 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1952,6 +1952,14 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
>       HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>
>       pcmc->get_hotplug_handler = mc->get_hotplug_handler;
> +    pcmc->pci_enabled = true;
> +    pcmc->has_acpi_build = true;
> +    pcmc->rsdp_in_ram = true;
> +    pcmc->smbios_defaults = true;
> +    pcmc->smbios_uuid_encoded = true;
> +    pcmc->gigabyte_align = true;
> +    pcmc->has_reserved_memory = true;
> +    pcmc->kvmclock_enabled = true;
>       mc->get_hotplug_handler = pc_get_hotpug_handler;
>       mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
>       mc->default_boot_order = "cad";
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 2e41efe..7a7f748 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -60,26 +60,14 @@ static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
>   static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
>   static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
>
> -static bool pci_enabled = true;
> -static bool has_acpi_build = true;
> -static bool rsdp_in_ram = true;
>   static int legacy_acpi_table_size;
> -static bool smbios_defaults = true;
> -static bool smbios_legacy_mode;
> -static bool smbios_uuid_encoded = true;
> -/* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
> - * host addresses aligned at 1Gbyte boundaries.  This way we can use 1GByte
> - * pages in the host.
> - */
> -static bool gigabyte_align = true;
> -static bool has_reserved_memory = true;
> -static bool kvmclock_enabled = true;
>
>   /* PC hardware initialisation */
>   static void pc_init1(MachineState *machine,
>                        const char *host_type, const char *pci_type)
>   {
>       PCMachineState *pcms = PC_MACHINE(machine);
> +    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>       MemoryRegion *system_memory = get_system_memory();
>       MemoryRegion *system_io = get_system_io();
>       int i;
> @@ -108,7 +96,7 @@ static void pc_init1(MachineState *machine,
>        * breaking migration.
>        */
>       if (machine->ram_size >= 0xe0000000) {
> -        lowmem = gigabyte_align ? 0xc0000000 : 0xe0000000;
> +        lowmem = pcmc->gigabyte_align ? 0xc0000000 : 0xe0000000;
>       } else {
>           lowmem = 0xe0000000;
>       }
> @@ -141,11 +129,11 @@ static void pc_init1(MachineState *machine,
>
>       pc_cpus_init(pcms);
>
> -    if (kvm_enabled() && kvmclock_enabled) {
> +    if (kvm_enabled() && pcmc->kvmclock_enabled) {
>           kvmclock_create();
>       }
>
> -    if (pci_enabled) {
> +    if (pcmc->pci_enabled) {
>           pci_memory = g_new(MemoryRegion, 1);
>           memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
>           rom_memory = pci_memory;
> @@ -156,18 +144,19 @@ static void pc_init1(MachineState *machine,
>
>       guest_info = pc_guest_info_init(pcms);
>
> -    guest_info->has_acpi_build = has_acpi_build;
> +    guest_info->has_acpi_build = pcmc->has_acpi_build;
>       guest_info->legacy_acpi_table_size = legacy_acpi_table_size;

Why is legacy_acpi_table_size left behind? Maybe it is a new field.

Thanks,
Marcel

>
> -    guest_info->isapc_ram_fw = !pci_enabled;
> -    guest_info->has_reserved_memory = has_reserved_memory;
> -    guest_info->rsdp_in_ram = rsdp_in_ram;
> +    guest_info->isapc_ram_fw = !pcmc->pci_enabled;
> +    guest_info->has_reserved_memory = pcmc->has_reserved_memory;
> +    guest_info->rsdp_in_ram = pcmc->rsdp_in_ram;
>
> -    if (smbios_defaults) {
> +    if (pcmc->smbios_defaults) {
>           MachineClass *mc = MACHINE_GET_CLASS(machine);
>           /* These values are guest ABI, do not change */
>           smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
> -                            mc->name, smbios_legacy_mode, smbios_uuid_encoded,
> +                            mc->name, pcmc->smbios_legacy_mode,
> +                            pcmc->smbios_uuid_encoded,
>                               SMBIOS_ENTRY_POINT_21);
>       }
>
> @@ -182,14 +171,14 @@ static void pc_init1(MachineState *machine,
>
>       gsi_state = g_malloc0(sizeof(*gsi_state));
>       if (kvm_irqchip_in_kernel()) {
> -        kvm_pc_setup_irq_routing(pci_enabled);
> +        kvm_pc_setup_irq_routing(pcmc->pci_enabled);
>           gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
>                                    GSI_NUM_PINS);
>       } else {
>           gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
>       }
>
> -    if (pci_enabled) {
> +    if (pcmc->pci_enabled) {
>           pci_bus = i440fx_init(host_type,
>                                 pci_type,
>                                 &i440fx_state, &piix3_devfn, &isa_bus, gsi,
> @@ -217,13 +206,13 @@ static void pc_init1(MachineState *machine,
>           gsi_state->i8259_irq[i] = i8259[i];
>       }
>       g_free(i8259);
> -    if (pci_enabled) {
> +    if (pcmc->pci_enabled) {
>           ioapic_init_gsi(gsi_state, "i440fx");
>       }
>
>       pc_register_ferr_irq(gsi[13]);
>
> -    pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
> +    pc_vga_init(isa_bus, pcmc->pci_enabled ? pci_bus : NULL);
>
>       assert(pcms->vmport != ON_OFF_AUTO_MAX);
>       if (pcms->vmport == ON_OFF_AUTO_AUTO) {
> @@ -237,7 +226,7 @@ static void pc_init1(MachineState *machine,
>       pc_nic_init(isa_bus, pci_bus);
>
>       ide_drive_get(hd, ARRAY_SIZE(hd));
> -    if (pci_enabled) {
> +    if (pcmc->pci_enabled) {
>           PCIDevice *dev;
>           if (xen_enabled()) {
>               dev = pci_piix3_xen_ide_init(pci_bus, hd, piix3_devfn + 1);
> @@ -264,11 +253,11 @@ static void pc_init1(MachineState *machine,
>
>       pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state);
>
> -    if (pci_enabled && usb_enabled()) {
> +    if (pcmc->pci_enabled && usb_enabled()) {
>           pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
>       }
>
> -    if (pci_enabled && acpi_enabled) {
> +    if (pcmc->pci_enabled && acpi_enabled) {
>           DeviceState *piix4_pm;
>           I2CBus *smbus;
>
> @@ -289,7 +278,7 @@ static void pc_init1(MachineState *machine,
>                                    PC_MACHINE_ACPI_DEVICE_PROP, &error_abort);
>       }
>
> -    if (pci_enabled) {
> +    if (pcmc->pci_enabled) {
>           pc_pci_device_init(pci_bus);
>       }
>   }
> @@ -315,7 +304,6 @@ 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;
>       machine->suppress_vmdesc = true;
>   }
>
> @@ -324,7 +312,6 @@ static void pc_compat_2_1(MachineState *machine)
>       PCMachineState *pcms = PC_MACHINE(machine);
>
>       pc_compat_2_2(machine);
> -    smbios_uuid_encoded = false;
>       x86_cpu_change_kvm_default("svm", NULL);
>       pcms->enforce_aligned_dimm = false;
>   }
> @@ -349,16 +336,12 @@ static void pc_compat_2_0(MachineState *machine)
>        * QEMU 1.7 it is 6414.  For RHEL/CentOS 7.0 it is 6418.
>        */
>       legacy_acpi_table_size = 6652;
> -    smbios_legacy_mode = true;
> -    has_reserved_memory = false;
>       pc_set_legacy_acpi_data_size();
>   }
>
>   static void pc_compat_1_7(MachineState *machine)
>   {
>       pc_compat_2_0(machine);
> -    smbios_defaults = false;
> -    gigabyte_align = false;
>       option_rom_has_mr = true;
>       legacy_acpi_table_size = 6414;
>       x86_cpu_change_kvm_default("x2apic", NULL);
> @@ -368,7 +351,6 @@ static void pc_compat_1_6(MachineState *machine)
>   {
>       pc_compat_1_7(machine);
>       rom_file_has_mr = false;
> -    has_acpi_build = false;
>   }
>
>   static void pc_compat_1_5(MachineState *machine)
> @@ -398,17 +380,10 @@ static void pc_compat_1_2(MachineState *machine)
>   static void pc_compat_0_13(MachineState *machine)
>   {
>       pc_compat_1_2(machine);
> -    kvmclock_enabled = false;
>   }
>
>   static void pc_init_isa(MachineState *machine)
>   {
> -    pci_enabled = false;
> -    has_acpi_build = false;
> -    smbios_defaults = false;
> -    gigabyte_align = false;
> -    smbios_legacy_mode = true;
> -    has_reserved_memory = false;
>       option_rom_has_mr = true;
>       rom_file_has_mr = false;
>       if (!machine->cpu_model) {
> @@ -510,9 +485,11 @@ DEFINE_I440FX_MACHINE(v2_3, "pc-i440fx-2.3", pc_compat_2_3,
>
>   static void pc_i440fx_2_2_machine_options(MachineClass *m)
>   {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>       pc_i440fx_2_3_machine_options(m);
>       m->hw_version = "2.2.0";
>       SET_MACHINE_COMPAT(m, PC_COMPAT_2_2);
> +    pcmc->rsdp_in_ram = false;
>   }
>
>   DEFINE_I440FX_MACHINE(v2_2, "pc-i440fx-2.2", pc_compat_2_2,
> @@ -521,10 +498,12 @@ DEFINE_I440FX_MACHINE(v2_2, "pc-i440fx-2.2", pc_compat_2_2,
>
>   static void pc_i440fx_2_1_machine_options(MachineClass *m)
>   {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>       pc_i440fx_2_2_machine_options(m);
>       m->hw_version = "2.1.0";
>       m->default_display = NULL;
>       SET_MACHINE_COMPAT(m, PC_COMPAT_2_1);
> +    pcmc->smbios_uuid_encoded = false;
>   }
>
>   DEFINE_I440FX_MACHINE(v2_1, "pc-i440fx-2.1", pc_compat_2_1,
> @@ -534,9 +513,12 @@ DEFINE_I440FX_MACHINE(v2_1, "pc-i440fx-2.1", pc_compat_2_1,
>
>   static void pc_i440fx_2_0_machine_options(MachineClass *m)
>   {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>       pc_i440fx_2_1_machine_options(m);
>       m->hw_version = "2.0.0";
>       SET_MACHINE_COMPAT(m, PC_COMPAT_2_0);
> +    pcmc->smbios_legacy_mode = true;
> +    pcmc->has_reserved_memory = false;
>   }
>
>   DEFINE_I440FX_MACHINE(v2_0, "pc-i440fx-2.0", pc_compat_2_0,
> @@ -545,10 +527,13 @@ DEFINE_I440FX_MACHINE(v2_0, "pc-i440fx-2.0", pc_compat_2_0,
>
>   static void pc_i440fx_1_7_machine_options(MachineClass *m)
>   {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>       pc_i440fx_2_0_machine_options(m);
>       m->hw_version = "1.7.0";
>       m->default_machine_opts = NULL;
>       SET_MACHINE_COMPAT(m, PC_COMPAT_1_7);
> +    pcmc->smbios_defaults = false;
> +    pcmc->gigabyte_align = false;
>   }
>
>   DEFINE_I440FX_MACHINE(v1_7, "pc-i440fx-1.7", pc_compat_1_7,
> @@ -557,9 +542,11 @@ DEFINE_I440FX_MACHINE(v1_7, "pc-i440fx-1.7", pc_compat_1_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";
>       SET_MACHINE_COMPAT(m, PC_COMPAT_1_6);
> +    pcmc->has_acpi_build = false;
>   }
>
>   DEFINE_I440FX_MACHINE(v1_6, "pc-i440fx-1.6", pc_compat_1_6,
> @@ -813,9 +800,11 @@ DEFINE_I440FX_MACHINE(v0_14, "pc-0.14", pc_compat_1_2,
>
>   static void pc_i440fx_0_13_machine_options(MachineClass *m)
>   {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>       pc_i440fx_0_14_machine_options(m);
>       m->hw_version = "0.13";
>       SET_MACHINE_COMPAT(m, PC_COMPAT_0_13);
> +    pcmc->kvmclock_enabled = false;
>   }
>
>   DEFINE_I440FX_MACHINE(v0_13, "pc-0.13", pc_compat_0_13,
> @@ -1037,8 +1026,15 @@ void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id)
>
>   static void isapc_machine_options(MachineClass *m)
>   {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>       m->desc = "ISA-only PC";
>       m->max_cpus = 1;
> +    pcmc->pci_enabled = false;
> +    pcmc->has_acpi_build = false;
> +    pcmc->smbios_defaults = false;
> +    pcmc->gigabyte_align = false;
> +    pcmc->smbios_legacy_mode = true;
> +    pcmc->has_reserved_memory = false;
>   }
>
>   DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 133bc68..f9afaa2 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -49,22 +49,11 @@
>   /* ICH9 AHCI has 6 ports */
>   #define MAX_SATA_PORTS     6
>
> -static bool has_acpi_build = true;
> -static bool rsdp_in_ram = true;
> -static bool smbios_defaults = true;
> -static bool smbios_legacy_mode;
> -static bool smbios_uuid_encoded = true;
> -/* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
> - * host addresses aligned at 1Gbyte boundaries.  This way we can use 1GByte
> - * pages in the host.
> - */
> -static bool gigabyte_align = true;
> -static bool has_reserved_memory = true;
> -
>   /* PC hardware initialisation */
>   static void pc_q35_init(MachineState *machine)
>   {
>       PCMachineState *pcms = PC_MACHINE(machine);
> +    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>       Q35PCIHost *q35_host;
>       PCIHostState *phb;
>       PCIBus *host_bus;
> @@ -76,7 +65,6 @@ static void pc_q35_init(MachineState *machine)
>       MemoryRegion *ram_memory;
>       GSIState *gsi_state;
>       ISABus *isa_bus;
> -    int pci_enabled = 1;
>       qemu_irq *gsi;
>       qemu_irq *i8259;
>       int i;
> @@ -97,7 +85,7 @@ static void pc_q35_init(MachineState *machine)
>        * breaking migration.
>        */
>       if (machine->ram_size >= 0xb0000000) {
> -        lowmem = gigabyte_align ? 0x80000000 : 0xb0000000;
> +        lowmem = pcmc->gigabyte_align ? 0x80000000 : 0xb0000000;
>       } else {
>           lowmem = 0xb0000000;
>       }
> @@ -134,7 +122,7 @@ static void pc_q35_init(MachineState *machine)
>       kvmclock_create();
>
>       /* pci enabled */
> -    if (pci_enabled) {
> +    if (pcmc->pci_enabled) {
>           pci_memory = g_new(MemoryRegion, 1);
>           memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
>           rom_memory = pci_memory;
> @@ -145,19 +133,20 @@ static void pc_q35_init(MachineState *machine)
>
>       guest_info = pc_guest_info_init(pcms);
>       guest_info->isapc_ram_fw = false;
> -    guest_info->has_acpi_build = has_acpi_build;
> -    guest_info->has_reserved_memory = has_reserved_memory;
> -    guest_info->rsdp_in_ram = rsdp_in_ram;
> +    guest_info->has_acpi_build = pcmc->has_acpi_build;
> +    guest_info->has_reserved_memory = pcmc->has_reserved_memory;
> +    guest_info->rsdp_in_ram = pcmc->rsdp_in_ram;
>
>       /* Migration was not supported in 2.0 for Q35, so do not bother
>        * with this hack (see hw/i386/acpi-build.c).
>        */
>       guest_info->legacy_acpi_table_size = 0;
>
> -    if (smbios_defaults) {
> +    if (pcmc->smbios_defaults) {
>           /* These values are guest ABI, do not change */
>           smbios_set_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
> -                            mc->name, smbios_legacy_mode, smbios_uuid_encoded,
> +                            mc->name, pcmc->smbios_legacy_mode,
> +                            pcmc->smbios_uuid_encoded,
>                               SMBIOS_ENTRY_POINT_21);
>       }
>
> @@ -170,7 +159,7 @@ static void pc_q35_init(MachineState *machine)
>       /* irq lines */
>       gsi_state = g_malloc0(sizeof(*gsi_state));
>       if (kvm_irqchip_in_kernel()) {
> -        kvm_pc_setup_irq_routing(pci_enabled);
> +        kvm_pc_setup_irq_routing(pcmc->pci_enabled);
>           gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
>                                    GSI_NUM_PINS);
>       } else {
> @@ -227,7 +216,7 @@ static void pc_q35_init(MachineState *machine)
>       for (i = 0; i < ISA_NUM_IRQS; i++) {
>           gsi_state->i8259_irq[i] = i8259[i];
>       }
> -    if (pci_enabled) {
> +    if (pcmc->pci_enabled) {
>           ioapic_init_gsi(gsi_state, "q35");
>       }
>
> @@ -272,7 +261,7 @@ static void pc_q35_init(MachineState *machine)
>       /* the rest devices to which pci devfn is automatically assigned */
>       pc_vga_init(isa_bus, host_bus);
>       pc_nic_init(isa_bus, host_bus);
> -    if (pci_enabled) {
> +    if (pcmc->pci_enabled) {
>           pc_pci_device_init(host_bus);
>       }
>   }
> @@ -298,7 +287,6 @@ 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;
>       machine->suppress_vmdesc = true;
>   }
>
> @@ -308,23 +296,18 @@ static void pc_compat_2_1(MachineState *machine)
>
>       pc_compat_2_2(machine);
>       pcms->enforce_aligned_dimm = false;
> -    smbios_uuid_encoded = false;
>       x86_cpu_change_kvm_default("svm", NULL);
>   }
>
>   static void pc_compat_2_0(MachineState *machine)
>   {
>       pc_compat_2_1(machine);
> -    smbios_legacy_mode = true;
> -    has_reserved_memory = false;
>       pc_set_legacy_acpi_data_size();
>   }
>
>   static void pc_compat_1_7(MachineState *machine)
>   {
>       pc_compat_2_0(machine);
> -    smbios_defaults = false;
> -    gigabyte_align = false;
>       option_rom_has_mr = true;
>       x86_cpu_change_kvm_default("x2apic", NULL);
>   }
> @@ -333,7 +316,6 @@ static void pc_compat_1_6(MachineState *machine)
>   {
>       pc_compat_1_7(machine);
>       rom_file_has_mr = false;
> -    has_acpi_build = false;
>   }
>
>   static void pc_compat_1_5(MachineState *machine)
> @@ -409,9 +391,11 @@ DEFINE_Q35_MACHINE(v2_3, "pc-q35-2.3", pc_compat_2_3,
>
>   static void pc_q35_2_2_machine_options(MachineClass *m)
>   {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>       pc_q35_2_3_machine_options(m);
>       m->hw_version = "2.2.0";
>       SET_MACHINE_COMPAT(m, PC_COMPAT_2_2);
> +    pcmc->rsdp_in_ram = false;
>   }
>
>   DEFINE_Q35_MACHINE(v2_2, "pc-q35-2.2", pc_compat_2_2,
> @@ -420,10 +404,12 @@ DEFINE_Q35_MACHINE(v2_2, "pc-q35-2.2", pc_compat_2_2,
>
>   static void pc_q35_2_1_machine_options(MachineClass *m)
>   {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>       pc_q35_2_2_machine_options(m);
>       m->hw_version = "2.1.0";
>       m->default_display = NULL;
>       SET_MACHINE_COMPAT(m, PC_COMPAT_2_1);
> +    pcmc->smbios_uuid_encoded = false;
>   }
>
>   DEFINE_Q35_MACHINE(v2_1, "pc-q35-2.1", pc_compat_2_1,
> @@ -432,9 +418,12 @@ DEFINE_Q35_MACHINE(v2_1, "pc-q35-2.1", pc_compat_2_1,
>
>   static void pc_q35_2_0_machine_options(MachineClass *m)
>   {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>       pc_q35_2_1_machine_options(m);
>       m->hw_version = "2.0.0";
>       SET_MACHINE_COMPAT(m, PC_COMPAT_2_0);
> +    pcmc->has_reserved_memory = false;
> +    pcmc->smbios_legacy_mode = true;
>   }
>
>   DEFINE_Q35_MACHINE(v2_0, "pc-q35-2.0", pc_compat_2_0,
> @@ -443,10 +432,13 @@ DEFINE_Q35_MACHINE(v2_0, "pc-q35-2.0", pc_compat_2_0,
>
>   static void pc_q35_1_7_machine_options(MachineClass *m)
>   {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>       pc_q35_2_0_machine_options(m);
>       m->hw_version = "1.7.0";
>       m->default_machine_opts = NULL;
>       SET_MACHINE_COMPAT(m, PC_COMPAT_1_7);
> +    pcmc->smbios_defaults = false;
> +    pcmc->gigabyte_align = false;
>   }
>
>   DEFINE_Q35_MACHINE(v1_7, "pc-q35-1.7", pc_compat_1_7,
> @@ -455,9 +447,11 @@ DEFINE_Q35_MACHINE(v1_7, "pc-q35-1.7", pc_compat_1_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";
>       SET_MACHINE_COMPAT(m, PC_COMPAT_1_6);
> +    pcmc->has_acpi_build = false;
>   }
>
>   DEFINE_Q35_MACHINE(v1_6, "pc-q35-1.6", pc_compat_1_6,
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 854c330..15171e9 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -62,6 +62,20 @@ struct PCMachineClass {
>       bool broken_reserved_end;
>       HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
>                                              DeviceState *dev);
> +
> +    bool pci_enabled;
> +    bool has_acpi_build;
> +    bool rsdp_in_ram;
> +    bool smbios_defaults;
> +    bool smbios_legacy_mode;
> +    bool smbios_uuid_encoded;
> +    /* Make sure that guest addresses aligned at 1Gbyte boundaries get
> +     * mapped to host addresses aligned at 1Gbyte boundaries.  This way
> +     * we can use 1GByte pages in the host.
> +     */
> +    bool gigabyte_align;
> +    bool has_reserved_memory;
> +    bool kvmclock_enabled;
>   };
>
>   #define TYPE_PC_MACHINE "generic-pc-machine"
>

Re: [Qemu-devel] [PATCH v3 1/6] pc: Move compat boolean globals to PCMachineClass

[Marcel Apfelbaum]

On 12/02/2015 11:57 AM, Marcel Apfelbaum wrote:
> On 12/02/2015 12:58 AM, Eduardo Habkost wrote:
>> This way the compat flags can be initialized in the machine_options()
>> function. This will help us to eventually eliminate the pc_compat_*()
>> functions.
>
> Hi, I have only a minor comment here,
>
>>
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> ---
>>   hw/i386/pc.c         |  8 +++++
>>   hw/i386/pc_piix.c    | 84 +++++++++++++++++++++++++---------------------------
>>   hw/i386/pc_q35.c     | 54 +++++++++++++++------------------
>>   include/hw/i386/pc.h | 14 +++++++++
>>   4 files changed, 86 insertions(+), 74 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 5e20e07..129aa04 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1952,6 +1952,14 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
>>       HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>>
>>       pcmc->get_hotplug_handler = mc->get_hotplug_handler;
>> +    pcmc->pci_enabled = true;
>> +    pcmc->has_acpi_build = true;
>> +    pcmc->rsdp_in_ram = true;
>> +    pcmc->smbios_defaults = true;
>> +    pcmc->smbios_uuid_encoded = true;
>> +    pcmc->gigabyte_align = true;
>> +    pcmc->has_reserved_memory = true;
>> +    pcmc->kvmclock_enabled = true;
>>       mc->get_hotplug_handler = pc_get_hotpug_handler;
>>       mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
>>       mc->default_boot_order = "cad";
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 2e41efe..7a7f748 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -60,26 +60,14 @@ static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
>>   static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
>>   static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
>>
>> -static bool pci_enabled = true;
>> -static bool has_acpi_build = true;
>> -static bool rsdp_in_ram = true;
>>   static int legacy_acpi_table_size;
>> -static bool smbios_defaults = true;
>> -static bool smbios_legacy_mode;
>> -static bool smbios_uuid_encoded = true;
>> -/* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
>> - * host addresses aligned at 1Gbyte boundaries.  This way we can use 1GByte
>> - * pages in the host.
>> - */
>> -static bool gigabyte_align = true;
>> -static bool has_reserved_memory = true;
>> -static bool kvmclock_enabled = true;
>>
>>   /* PC hardware initialisation */
>>   static void pc_init1(MachineState *machine,
>>                        const char *host_type, const char *pci_type)
>>   {
>>       PCMachineState *pcms = PC_MACHINE(machine);
>> +    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>>       MemoryRegion *system_memory = get_system_memory();
>>       MemoryRegion *system_io = get_system_io();
>>       int i;
>> @@ -108,7 +96,7 @@ static void pc_init1(MachineState *machine,
>>        * breaking migration.
>>        */
>>       if (machine->ram_size >= 0xe0000000) {
>> -        lowmem = gigabyte_align ? 0xc0000000 : 0xe0000000;
>> +        lowmem = pcmc->gigabyte_align ? 0xc0000000 : 0xe0000000;
>>       } else {
>>           lowmem = 0xe0000000;
>>       }
>> @@ -141,11 +129,11 @@ static void pc_init1(MachineState *machine,
>>
>>       pc_cpus_init(pcms);
>>
>> -    if (kvm_enabled() && kvmclock_enabled) {
>> +    if (kvm_enabled() && pcmc->kvmclock_enabled) {
>>           kvmclock_create();
>>       }
>>
>> -    if (pci_enabled) {
>> +    if (pcmc->pci_enabled) {
>>           pci_memory = g_new(MemoryRegion, 1);
>>           memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
>>           rom_memory = pci_memory;
>> @@ -156,18 +144,19 @@ static void pc_init1(MachineState *machine,
>>
>>       guest_info = pc_guest_info_init(pcms);
>>
>> -    guest_info->has_acpi_build = has_acpi_build;
>> +    guest_info->has_acpi_build = pcmc->has_acpi_build;
>>       guest_info->legacy_acpi_table_size = legacy_acpi_table_size;
>
> Why is legacy_acpi_table_size left behind? Maybe it is a new field.

Forget about it, the answer is the next patch :)

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

Thanks,
Marcel

[Qemu-devel] [PATCH v3 2/6] pc: Move legacy_acpi_table_size global to PCMachineClass

[Eduardo Habkost]

This way we can set legacy_acpi_table_size on the machine_options()
functions, instead of requirng code in pc_compat_*() functions.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c    | 40 +++++++++++++++++++---------------------
 include/hw/i386/pc.h |  1 +
 2 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7a7f748..5f23e9c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -60,8 +60,6 @@ static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
 static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
 static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
 
-static int legacy_acpi_table_size;
-
 /* PC hardware initialisation */
 static void pc_init1(MachineState *machine,
                      const char *host_type, const char *pci_type)
@@ -145,7 +143,7 @@ static void pc_init1(MachineState *machine,
     guest_info = pc_guest_info_init(pcms);
 
     guest_info->has_acpi_build = pcmc->has_acpi_build;
-    guest_info->legacy_acpi_table_size = legacy_acpi_table_size;
+    guest_info->legacy_acpi_table_size = pcmc->legacy_acpi_table_size;
 
     guest_info->isapc_ram_fw = !pcmc->pci_enabled;
     guest_info->has_reserved_memory = pcmc->has_reserved_memory;
@@ -319,23 +317,6 @@ static void pc_compat_2_1(MachineState *machine)
 static void pc_compat_2_0(MachineState *machine)
 {
     pc_compat_2_1(machine);
-    /* This value depends on the actual DSDT and SSDT compiled into
-     * the source QEMU; unfortunately it depends on the binary and
-     * not on the machine type, so we cannot make pc-i440fx-1.7 work on
-     * both QEMU 1.7 and QEMU 2.0.
-     *
-     * Large variations cause migration to fail for more than one
-     * consecutive value of the "-smp" maxcpus option.
-     *
-     * For small variations of the kind caused by different iasl versions,
-     * the 4k rounding usually leaves slack.  However, there could be still
-     * one or two values that break.  For QEMU 1.7 and QEMU 2.0 the
-     * slack is only ~10 bytes before one "-smp maxcpus" value breaks!
-     *
-     * 6652 is valid for QEMU 2.0, the right value for pc-i440fx-1.7 on
-     * QEMU 1.7 it is 6414.  For RHEL/CentOS 7.0 it is 6418.
-     */
-    legacy_acpi_table_size = 6652;
     pc_set_legacy_acpi_data_size();
 }
 
@@ -343,7 +324,6 @@ static void pc_compat_1_7(MachineState *machine)
 {
     pc_compat_2_0(machine);
     option_rom_has_mr = true;
-    legacy_acpi_table_size = 6414;
     x86_cpu_change_kvm_default("x2apic", NULL);
 }
 
@@ -519,6 +499,23 @@ static void pc_i440fx_2_0_machine_options(MachineClass *m)
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_0);
     pcmc->smbios_legacy_mode = true;
     pcmc->has_reserved_memory = false;
+    /* This value depends on the actual DSDT and SSDT compiled into
+     * the source QEMU; unfortunately it depends on the binary and
+     * not on the machine type, so we cannot make pc-i440fx-1.7 work on
+     * both QEMU 1.7 and QEMU 2.0.
+     *
+     * Large variations cause migration to fail for more than one
+     * consecutive value of the "-smp" maxcpus option.
+     *
+     * For small variations of the kind caused by different iasl versions,
+     * the 4k rounding usually leaves slack.  However, there could be still
+     * one or two values that break.  For QEMU 1.7 and QEMU 2.0 the
+     * slack is only ~10 bytes before one "-smp maxcpus" value breaks!
+     *
+     * 6652 is valid for QEMU 2.0, the right value for pc-i440fx-1.7 on
+     * QEMU 1.7 it is 6414.  For RHEL/CentOS 7.0 it is 6418.
+     */
+    pcmc->legacy_acpi_table_size = 6652;
 }
 
 DEFINE_I440FX_MACHINE(v2_0, "pc-i440fx-2.0", pc_compat_2_0,
@@ -534,6 +531,7 @@ static void pc_i440fx_1_7_machine_options(MachineClass *m)
     SET_MACHINE_COMPAT(m, PC_COMPAT_1_7);
     pcmc->smbios_defaults = false;
     pcmc->gigabyte_align = false;
+    pcmc->legacy_acpi_table_size = 6414;
 }
 
 DEFINE_I440FX_MACHINE(v1_7, "pc-i440fx-1.7", pc_compat_1_7,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 15171e9..7c2cc9f 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -76,6 +76,7 @@ struct PCMachineClass {
     bool gigabyte_align;
     bool has_reserved_memory;
     bool kvmclock_enabled;
+    int legacy_acpi_table_size;
 };
 
 #define TYPE_PC_MACHINE "generic-pc-machine"
-- 
2.1.0

Re: [Qemu-devel] [PATCH v3 2/6] pc: Move legacy_acpi_table_size global to PCMachineClass

[Marcel Apfelbaum]

On 12/02/2015 12:58 AM, Eduardo Habkost wrote:
> This way we can set legacy_acpi_table_size on the machine_options()
> functions, instead of requirng code in pc_compat_*() functions.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>   hw/i386/pc_piix.c    | 40 +++++++++++++++++++---------------------
>   include/hw/i386/pc.h |  1 +
>   2 files changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 7a7f748..5f23e9c 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -60,8 +60,6 @@ static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
>   static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
>   static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
>
> -static int legacy_acpi_table_size;
> -
>   /* PC hardware initialisation */
>   static void pc_init1(MachineState *machine,
>                        const char *host_type, const char *pci_type)
> @@ -145,7 +143,7 @@ static void pc_init1(MachineState *machine,
>       guest_info = pc_guest_info_init(pcms);
>
>       guest_info->has_acpi_build = pcmc->has_acpi_build;
> -    guest_info->legacy_acpi_table_size = legacy_acpi_table_size;
> +    guest_info->legacy_acpi_table_size = pcmc->legacy_acpi_table_size;
>
>       guest_info->isapc_ram_fw = !pcmc->pci_enabled;
>       guest_info->has_reserved_memory = pcmc->has_reserved_memory;
> @@ -319,23 +317,6 @@ static void pc_compat_2_1(MachineState *machine)
>   static void pc_compat_2_0(MachineState *machine)
>   {
>       pc_compat_2_1(machine);
> -    /* This value depends on the actual DSDT and SSDT compiled into
> -     * the source QEMU; unfortunately it depends on the binary and
> -     * not on the machine type, so we cannot make pc-i440fx-1.7 work on
> -     * both QEMU 1.7 and QEMU 2.0.
> -     *
> -     * Large variations cause migration to fail for more than one
> -     * consecutive value of the "-smp" maxcpus option.
> -     *
> -     * For small variations of the kind caused by different iasl versions,
> -     * the 4k rounding usually leaves slack.  However, there could be still
> -     * one or two values that break.  For QEMU 1.7 and QEMU 2.0 the
> -     * slack is only ~10 bytes before one "-smp maxcpus" value breaks!
> -     *
> -     * 6652 is valid for QEMU 2.0, the right value for pc-i440fx-1.7 on
> -     * QEMU 1.7 it is 6414.  For RHEL/CentOS 7.0 it is 6418.
> -     */
> -    legacy_acpi_table_size = 6652;
>       pc_set_legacy_acpi_data_size();
>   }
>
> @@ -343,7 +324,6 @@ static void pc_compat_1_7(MachineState *machine)
>   {
>       pc_compat_2_0(machine);
>       option_rom_has_mr = true;
> -    legacy_acpi_table_size = 6414;
>       x86_cpu_change_kvm_default("x2apic", NULL);
>   }
>
> @@ -519,6 +499,23 @@ static void pc_i440fx_2_0_machine_options(MachineClass *m)
>       SET_MACHINE_COMPAT(m, PC_COMPAT_2_0);
>       pcmc->smbios_legacy_mode = true;
>       pcmc->has_reserved_memory = false;
> +    /* This value depends on the actual DSDT and SSDT compiled into
> +     * the source QEMU; unfortunately it depends on the binary and
> +     * not on the machine type, so we cannot make pc-i440fx-1.7 work on
> +     * both QEMU 1.7 and QEMU 2.0.
> +     *
> +     * Large variations cause migration to fail for more than one
> +     * consecutive value of the "-smp" maxcpus option.
> +     *
> +     * For small variations of the kind caused by different iasl versions,
> +     * the 4k rounding usually leaves slack.  However, there could be still
> +     * one or two values that break.  For QEMU 1.7 and QEMU 2.0 the
> +     * slack is only ~10 bytes before one "-smp maxcpus" value breaks!
> +     *
> +     * 6652 is valid for QEMU 2.0, the right value for pc-i440fx-1.7 on
> +     * QEMU 1.7 it is 6414.  For RHEL/CentOS 7.0 it is 6418.
> +     */
> +    pcmc->legacy_acpi_table_size = 6652;
>   }
>
>   DEFINE_I440FX_MACHINE(v2_0, "pc-i440fx-2.0", pc_compat_2_0,
> @@ -534,6 +531,7 @@ static void pc_i440fx_1_7_machine_options(MachineClass *m)
>       SET_MACHINE_COMPAT(m, PC_COMPAT_1_7);
>       pcmc->smbios_defaults = false;
>       pcmc->gigabyte_align = false;
> +    pcmc->legacy_acpi_table_size = 6414;
>   }
>
>   DEFINE_I440FX_MACHINE(v1_7, "pc-i440fx-1.7", pc_compat_1_7,
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 15171e9..7c2cc9f 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -76,6 +76,7 @@ struct PCMachineClass {
>       bool gigabyte_align;
>       bool has_reserved_memory;
>       bool kvmclock_enabled;
> +    int legacy_acpi_table_size;
>   };
>
>   #define TYPE_PC_MACHINE "generic-pc-machine"
>

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

Thanks,
Marcel

[Qemu-devel] [PATCH v3 3/6] pc: Move acpi_data_size global to PCMachineClass

[Eduardo Habkost]

This way we don't need code in pc_compat_*() functions to set the legacy
acpi_data_size value.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc.c         | 17 ++++++-----------
 hw/i386/pc_piix.c    |  2 +-
 hw/i386/pc_q35.c     |  2 +-
 include/hw/i386/pc.h |  6 ++++--
 4 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 129aa04..a79b8af 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -76,15 +76,6 @@
 #define DPRINTF(fmt, ...)
 #endif
 
-/* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables
- * (128K) and other BIOS datastructures (less than 4K reported to be used at
- * the moment, 32K should be enough for a while).  */
-static unsigned acpi_data_size = 0x20000 + 0x8000;
-void pc_set_legacy_acpi_data_size(void)
-{
-    acpi_data_size = 0x10000;
-}
-
 #define BIOS_CFG_IOPORT 0x510
 #define FW_CFG_ACPI_TABLES (FW_CFG_ARCH_LOCAL + 0)
 #define FW_CFG_SMBIOS_ENTRIES (FW_CFG_ARCH_LOCAL + 1)
@@ -840,6 +831,7 @@ static void load_linux(PCMachineState *pcms,
     FILE *f;
     char *vmode;
     MachineState *machine = MACHINE(pcms);
+    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
     const char *kernel_filename = machine->kernel_filename;
     const char *initrd_filename = machine->initrd_filename;
     const char *kernel_cmdline = machine->kernel_cmdline;
@@ -907,8 +899,8 @@ static void load_linux(PCMachineState *pcms,
         initrd_max = 0x37ffffff;
     }
 
-    if (initrd_max >= pcms->below_4g_mem_size - acpi_data_size) {
-        initrd_max = pcms->below_4g_mem_size - acpi_data_size - 1;
+    if (initrd_max >= pcms->below_4g_mem_size - pcmc->acpi_data_size) {
+        initrd_max = pcms->below_4g_mem_size - pcmc->acpi_data_size - 1;
     }
 
     fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_ADDR, cmdline_addr);
@@ -1960,6 +1952,9 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     pcmc->gigabyte_align = true;
     pcmc->has_reserved_memory = true;
     pcmc->kvmclock_enabled = true;
+    /* BIOS ACPI tables: 128K. Other BIOS datastructures: less than 4K reported
+     * to be used at the moment, 32K should be enough for a while.  */
+    pcmc->acpi_data_size = 0x20000 + 0x8000;
     mc->get_hotplug_handler = pc_get_hotpug_handler;
     mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
     mc->default_boot_order = "cad";
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 5f23e9c..3562888 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -317,7 +317,6 @@ static void pc_compat_2_1(MachineState *machine)
 static void pc_compat_2_0(MachineState *machine)
 {
     pc_compat_2_1(machine);
-    pc_set_legacy_acpi_data_size();
 }
 
 static void pc_compat_1_7(MachineState *machine)
@@ -516,6 +515,7 @@ static void pc_i440fx_2_0_machine_options(MachineClass *m)
      * QEMU 1.7 it is 6414.  For RHEL/CentOS 7.0 it is 6418.
      */
     pcmc->legacy_acpi_table_size = 6652;
+    pcmc->acpi_data_size = 0x10000;
 }
 
 DEFINE_I440FX_MACHINE(v2_0, "pc-i440fx-2.0", pc_compat_2_0,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index f9afaa2..43e77f4 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -302,7 +302,6 @@ static void pc_compat_2_1(MachineState *machine)
 static void pc_compat_2_0(MachineState *machine)
 {
     pc_compat_2_1(machine);
-    pc_set_legacy_acpi_data_size();
 }
 
 static void pc_compat_1_7(MachineState *machine)
@@ -424,6 +423,7 @@ static void pc_q35_2_0_machine_options(MachineClass *m)
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_0);
     pcmc->has_reserved_memory = false;
     pcmc->smbios_legacy_mode = true;
+    pcmc->acpi_data_size = 0x10000;
 }
 
 DEFINE_Q35_MACHINE(v2_0, "pc-q35-2.0", pc_compat_2_0,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 7c2cc9f..e604ca5 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -77,6 +77,10 @@ struct PCMachineClass {
     bool has_reserved_memory;
     bool kvmclock_enabled;
     int legacy_acpi_table_size;
+    /* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables
+     * and other BIOS datastructures.
+     */
+    unsigned acpi_data_size;
 };
 
 #define TYPE_PC_MACHINE "generic-pc-machine"
@@ -188,8 +192,6 @@ void pc_acpi_init(const char *default_dsdt);
 
 PcGuestInfo *pc_guest_info_init(PCMachineState *pcms);
 
-void pc_set_legacy_acpi_data_size(void);
-
 #define PCI_HOST_PROP_PCI_HOLE_START   "pci-hole-start"
 #define PCI_HOST_PROP_PCI_HOLE_END     "pci-hole-end"
 #define PCI_HOST_PROP_PCI_HOLE64_START "pci-hole64-start"
-- 
2.1.0

Re: [Qemu-devel] [PATCH v3 3/6] pc: Move acpi_data_size global to PCMachineClass

[Marcel Apfelbaum]

On 12/02/2015 12:58 AM, Eduardo Habkost wrote:
> This way we don't need code in pc_compat_*() functions to set the legacy
> acpi_data_size value.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>   hw/i386/pc.c         | 17 ++++++-----------
>   hw/i386/pc_piix.c    |  2 +-
>   hw/i386/pc_q35.c     |  2 +-
>   include/hw/i386/pc.h |  6 ++++--
>   4 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 129aa04..a79b8af 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -76,15 +76,6 @@
>   #define DPRINTF(fmt, ...)
>   #endif
>
> -/* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables
> - * (128K) and other BIOS datastructures (less than 4K reported to be used at
> - * the moment, 32K should be enough for a while).  */
> -static unsigned acpi_data_size = 0x20000 + 0x8000;
> -void pc_set_legacy_acpi_data_size(void)
> -{
> -    acpi_data_size = 0x10000;
> -}
> -
>   #define BIOS_CFG_IOPORT 0x510
>   #define FW_CFG_ACPI_TABLES (FW_CFG_ARCH_LOCAL + 0)
>   #define FW_CFG_SMBIOS_ENTRIES (FW_CFG_ARCH_LOCAL + 1)
> @@ -840,6 +831,7 @@ static void load_linux(PCMachineState *pcms,
>       FILE *f;
>       char *vmode;
>       MachineState *machine = MACHINE(pcms);
> +    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>       const char *kernel_filename = machine->kernel_filename;
>       const char *initrd_filename = machine->initrd_filename;
>       const char *kernel_cmdline = machine->kernel_cmdline;
> @@ -907,8 +899,8 @@ static void load_linux(PCMachineState *pcms,
>           initrd_max = 0x37ffffff;
>       }
>
> -    if (initrd_max >= pcms->below_4g_mem_size - acpi_data_size) {
> -        initrd_max = pcms->below_4g_mem_size - acpi_data_size - 1;
> +    if (initrd_max >= pcms->below_4g_mem_size - pcmc->acpi_data_size) {
> +        initrd_max = pcms->below_4g_mem_size - pcmc->acpi_data_size - 1;
>       }
>
>       fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_ADDR, cmdline_addr);
> @@ -1960,6 +1952,9 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
>       pcmc->gigabyte_align = true;
>       pcmc->has_reserved_memory = true;
>       pcmc->kvmclock_enabled = true;
> +    /* BIOS ACPI tables: 128K. Other BIOS datastructures: less than 4K reported
> +     * to be used at the moment, 32K should be enough for a while.  */
> +    pcmc->acpi_data_size = 0x20000 + 0x8000;
>       mc->get_hotplug_handler = pc_get_hotpug_handler;
>       mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
>       mc->default_boot_order = "cad";
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 5f23e9c..3562888 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -317,7 +317,6 @@ static void pc_compat_2_1(MachineState *machine)
>   static void pc_compat_2_0(MachineState *machine)
>   {
>       pc_compat_2_1(machine);
> -    pc_set_legacy_acpi_data_size();
>   }
>
>   static void pc_compat_1_7(MachineState *machine)
> @@ -516,6 +515,7 @@ static void pc_i440fx_2_0_machine_options(MachineClass *m)
>        * QEMU 1.7 it is 6414.  For RHEL/CentOS 7.0 it is 6418.
>        */
>       pcmc->legacy_acpi_table_size = 6652;
> +    pcmc->acpi_data_size = 0x10000;
>   }
>
>   DEFINE_I440FX_MACHINE(v2_0, "pc-i440fx-2.0", pc_compat_2_0,
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index f9afaa2..43e77f4 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -302,7 +302,6 @@ static void pc_compat_2_1(MachineState *machine)
>   static void pc_compat_2_0(MachineState *machine)
>   {
>       pc_compat_2_1(machine);
> -    pc_set_legacy_acpi_data_size();
>   }
>
>   static void pc_compat_1_7(MachineState *machine)
> @@ -424,6 +423,7 @@ static void pc_q35_2_0_machine_options(MachineClass *m)
>       SET_MACHINE_COMPAT(m, PC_COMPAT_2_0);
>       pcmc->has_reserved_memory = false;
>       pcmc->smbios_legacy_mode = true;
> +    pcmc->acpi_data_size = 0x10000;
>   }
>
>   DEFINE_Q35_MACHINE(v2_0, "pc-q35-2.0", pc_compat_2_0,
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 7c2cc9f..e604ca5 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -77,6 +77,10 @@ struct PCMachineClass {
>       bool has_reserved_memory;
>       bool kvmclock_enabled;
>       int legacy_acpi_table_size;
> +    /* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables
> +     * and other BIOS datastructures.
> +     */
> +    unsigned acpi_data_size;
>   };
>
>   #define TYPE_PC_MACHINE "generic-pc-machine"
> @@ -188,8 +192,6 @@ void pc_acpi_init(const char *default_dsdt);
>
>   PcGuestInfo *pc_guest_info_init(PCMachineState *pcms);
>
> -void pc_set_legacy_acpi_data_size(void);
> -
>   #define PCI_HOST_PROP_PCI_HOLE_START   "pci-hole-start"
>   #define PCI_HOST_PROP_PCI_HOLE_END     "pci-hole-end"
>   #define PCI_HOST_PROP_PCI_HOLE64_START "pci-hole64-start"
>

Looks OK to me.

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

Thanks,
Marcel

[Qemu-devel] [PATCH v3 4/6] pc: Move enforce_aligned_dimm to PCMachineClass

[Eduardo Habkost]

enforce_aligned_dimm never changes after the machine is
initialized, so it can be simply set in PCMachineClass like all
the other compat fields.

Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc.c         | 12 +++++++-----
 hw/i386/pc_piix.c    |  4 +---
 hw/i386/pc_q35.c     |  4 +---
 include/hw/i386/pc.h |  6 +++---
 4 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index a79b8af..72c25ec 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1294,6 +1294,7 @@ FWCfgState *pc_memory_init(PCMachineState *pcms,
     MemoryRegion *ram_below_4g, *ram_above_4g;
     FWCfgState *fw_cfg;
     MachineState *machine = MACHINE(pcms);
+    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
 
     assert(machine->ram_size == pcms->below_4g_mem_size +
                                 pcms->above_4g_mem_size);
@@ -1355,7 +1356,7 @@ FWCfgState *pc_memory_init(PCMachineState *pcms,
         pcms->hotplug_memory.base =
             ROUND_UP(0x100000000ULL + pcms->above_4g_mem_size, 1ULL << 30);
 
-        if (pcms->enforce_aligned_dimm) {
+        if (pcmc->enforce_aligned_dimm) {
             /* size hotplug region assuming 1G page max alignment per slot */
             hotplug_mem_size += (1ULL << 30) * machine->ram_slots;
         }
@@ -1608,12 +1609,13 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
     HotplugHandlerClass *hhc;
     Error *local_err = NULL;
     PCMachineState *pcms = PC_MACHINE(hotplug_dev);
+    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
     PCDIMMDevice *dimm = PC_DIMM(dev);
     PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
     MemoryRegion *mr = ddc->get_memory_region(dimm);
     uint64_t align = TARGET_PAGE_SIZE;
 
-    if (memory_region_get_alignment(mr) && pcms->enforce_aligned_dimm) {
+    if (memory_region_get_alignment(mr) && pcmc->enforce_aligned_dimm) {
         align = memory_region_get_alignment(mr);
     }
 
@@ -1864,9 +1866,9 @@ static void pc_machine_set_smm(Object *obj, Visitor *v, void *opaque,
 
 static bool pc_machine_get_aligned_dimm(Object *obj, Error **errp)
 {
-    PCMachineState *pcms = PC_MACHINE(obj);
+    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(obj);
 
-    return pcms->enforce_aligned_dimm;
+    return pcmc->enforce_aligned_dimm;
 }
 
 static void pc_machine_initfn(Object *obj)
@@ -1904,7 +1906,6 @@ static void pc_machine_initfn(Object *obj)
                                     "Enable vmport (pc & q35)",
                                     &error_abort);
 
-    pcms->enforce_aligned_dimm = true;
     object_property_add_bool(obj, PC_MACHINE_ENFORCE_ALIGNED_DIMM,
                              pc_machine_get_aligned_dimm,
                              NULL, &error_abort);
@@ -1952,6 +1953,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     pcmc->gigabyte_align = true;
     pcmc->has_reserved_memory = true;
     pcmc->kvmclock_enabled = true;
+    pcmc->enforce_aligned_dimm = true;
     /* BIOS ACPI tables: 128K. Other BIOS datastructures: less than 4K reported
      * to be used at the moment, 32K should be enough for a while.  */
     pcmc->acpi_data_size = 0x20000 + 0x8000;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 3562888..869c64b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -307,11 +307,8 @@ static void pc_compat_2_2(MachineState *machine)
 
 static void pc_compat_2_1(MachineState *machine)
 {
-    PCMachineState *pcms = PC_MACHINE(machine);
-
     pc_compat_2_2(machine);
     x86_cpu_change_kvm_default("svm", NULL);
-    pcms->enforce_aligned_dimm = false;
 }
 
 static void pc_compat_2_0(MachineState *machine)
@@ -483,6 +480,7 @@ static void pc_i440fx_2_1_machine_options(MachineClass *m)
     m->default_display = NULL;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_1);
     pcmc->smbios_uuid_encoded = false;
+    pcmc->enforce_aligned_dimm = false;
 }
 
 DEFINE_I440FX_MACHINE(v2_1, "pc-i440fx-2.1", pc_compat_2_1,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 43e77f4..e16dec1 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -292,10 +292,7 @@ static void pc_compat_2_2(MachineState *machine)
 
 static void pc_compat_2_1(MachineState *machine)
 {
-    PCMachineState *pcms = PC_MACHINE(machine);
-
     pc_compat_2_2(machine);
-    pcms->enforce_aligned_dimm = false;
     x86_cpu_change_kvm_default("svm", NULL);
 }
 
@@ -409,6 +406,7 @@ static void pc_q35_2_1_machine_options(MachineClass *m)
     m->default_display = NULL;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_1);
     pcmc->smbios_uuid_encoded = false;
+    pcmc->enforce_aligned_dimm = false;
 }
 
 DEFINE_Q35_MACHINE(v2_1, "pc-q35-2.1", pc_compat_2_1,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index e604ca5..6b86deb 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -23,8 +23,6 @@
 /**
  * PCMachineState:
  * @acpi_dev: link to ACPI PM device that performs ACPI hotplug handling
- * @enforce_aligned_dimm: check that DIMM's address/size is aligned by
- *                        backend's alignment value if provided
  */
 struct PCMachineState {
     /*< private >*/
@@ -39,7 +37,6 @@ struct PCMachineState {
     uint64_t max_ram_below_4g;
     OnOffAuto vmport;
     OnOffAuto smm;
-    bool enforce_aligned_dimm;
     ram_addr_t below_4g_mem_size, above_4g_mem_size;
 };
 
@@ -53,6 +50,8 @@ struct PCMachineState {
 /**
  * PCMachineClass:
  * @get_hotplug_handler: pointer to parent class callback @get_hotplug_handler
+ * @enforce_aligned_dimm: check that DIMM's address/size is aligned by
+ *                        backend's alignment value if provided
  */
 struct PCMachineClass {
     /*< private >*/
@@ -81,6 +80,7 @@ struct PCMachineClass {
      * and other BIOS datastructures.
      */
     unsigned acpi_data_size;
+    bool enforce_aligned_dimm;
 };
 
 #define TYPE_PC_MACHINE "generic-pc-machine"
-- 
2.1.0

[Qemu-devel] [PATCH v3 5/6] pc: Remove enforce-aligned-dimm QOM property

[Eduardo Habkost]

The property is read-only and not used for anything.

Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc.c         | 11 -----------
 include/hw/i386/pc.h |  1 -
 2 files changed, 12 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 72c25ec..1b22ad2 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1864,13 +1864,6 @@ static void pc_machine_set_smm(Object *obj, Visitor *v, void *opaque,
     visit_type_OnOffAuto(v, &pcms->smm, name, errp);
 }
 
-static bool pc_machine_get_aligned_dimm(Object *obj, Error **errp)
-{
-    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(obj);
-
-    return pcmc->enforce_aligned_dimm;
-}
-
 static void pc_machine_initfn(Object *obj)
 {
     PCMachineState *pcms = PC_MACHINE(obj);
@@ -1905,10 +1898,6 @@ static void pc_machine_initfn(Object *obj)
     object_property_set_description(obj, PC_MACHINE_VMPORT,
                                     "Enable vmport (pc & q35)",
                                     &error_abort);
-
-    object_property_add_bool(obj, PC_MACHINE_ENFORCE_ALIGNED_DIMM,
-                             pc_machine_get_aligned_dimm,
-                             NULL, &error_abort);
 }
 
 static void pc_machine_reset(void)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 6b86deb..12f0ab0 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -45,7 +45,6 @@ struct PCMachineState {
 #define PC_MACHINE_MAX_RAM_BELOW_4G "max-ram-below-4g"
 #define PC_MACHINE_VMPORT           "vmport"
 #define PC_MACHINE_SMM              "smm"
-#define PC_MACHINE_ENFORCE_ALIGNED_DIMM "enforce-aligned-dimm"
 
 /**
  * PCMachineClass:
-- 
2.1.0

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

[Eduardo Habkost]

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);
-- 
2.1.0

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

[Marcel Apfelbaum]

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