[Qemu-devel] [pci PATCH v2 0/6] pc init & compat function cleanup

[Qemu-devel] [pci PATCH v2 0/6] pc init & compat function cleanup

[Eduardo Habkost]

Now sending the same series based on Michael's pci tree, to avoid conflicts,
and including 2 "don't prematurely explode QEMUMachineInitArgs" patches
from Markus.

Eduardo Habkost (4):
  pc: Kill pc_init_pci_1_0()
  pc: Create pc_compat_*() functions
  pc: pc_compat_1_4() now can call pc_compat_1_5()
  pc_piix: Kill pc_init1() memory region args

Markus Armbruster (2):
  pc: Don't prematurely explode QEMUMachineInitArgs
  pc: Don't explode QEMUMachineInitArgs into local variables needlessly

 hw/i386/pc_piix.c | 127 +++++++++++++++++++++++++-----------------------------
 hw/i386/pc_q35.c  |  53 ++++++++++++++---------
 2 files changed, 92 insertions(+), 88 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [pci PATCH v2 1/6] pc: Don't prematurely explode QEMUMachineInitArgs

[Eduardo Habkost]

From: Markus Armbruster <armbru@redhat.com>

Don't explode QEMUMachineInitArgs before passing it to pc_init1().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c | 65 ++++++++++++++-----------------------------------------
 1 file changed, 16 insertions(+), 49 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4591027..bbefea6 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -61,14 +61,9 @@ static bool has_pvpanic;
 static bool has_pci_info = true;
 
 /* PC hardware initialisation */
-static void pc_init1(MemoryRegion *system_memory,
+static void pc_init1(QEMUMachineInitArgs *args,
+                     MemoryRegion *system_memory,
                      MemoryRegion *system_io,
-                     ram_addr_t ram_size,
-                     const char *boot_device,
-                     const char *kernel_filename,
-                     const char *kernel_cmdline,
-                     const char *initrd_filename,
-                     const char *cpu_model,
                      int pci_enabled,
                      int kvmclock_enabled)
 {
@@ -103,18 +98,18 @@ static void pc_init1(MemoryRegion *system_memory,
     object_property_add_child(qdev_get_machine(), "icc-bridge",
                               OBJECT(icc_bridge), NULL);
 
-    pc_cpus_init(cpu_model, icc_bridge);
+    pc_cpus_init(args->cpu_model, icc_bridge);
 
     if (kvm_enabled() && kvmclock_enabled) {
         kvmclock_create();
     }
 
-    if (ram_size >= 0xe0000000 ) {
-        above_4g_mem_size = ram_size - 0xe0000000;
+    if (args->ram_size >= 0xe0000000) {
+        above_4g_mem_size = args->ram_size - 0xe0000000;
         below_4g_mem_size = 0xe0000000;
     } else {
         above_4g_mem_size = 0;
-        below_4g_mem_size = ram_size;
+        below_4g_mem_size = args->ram_size;
     }
 
     if (pci_enabled) {
@@ -133,7 +128,8 @@ static void pc_init1(MemoryRegion *system_memory,
     /* allocate ram and load rom/bios */
     if (!xen_enabled()) {
         fw_cfg = pc_memory_init(system_memory,
-                       kernel_filename, kernel_cmdline, initrd_filename,
+                       args->kernel_filename, args->kernel_cmdline,
+                       args->initrd_filename,
                        below_4g_mem_size, above_4g_mem_size,
                        rom_memory, &ram_memory, guest_info);
     }
@@ -149,7 +145,7 @@ static void pc_init1(MemoryRegion *system_memory,
 
     if (pci_enabled) {
         pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
-                              system_memory, system_io, ram_size,
+                              system_memory, system_io, args->ram_size,
                               below_4g_mem_size,
                               0x100000000ULL - below_4g_mem_size,
                               above_4g_mem_size,
@@ -208,7 +204,7 @@ static void pc_init1(MemoryRegion *system_memory,
         }
     }
 
-    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, boot_device,
+    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
                  floppy, idebus[0], idebus[1], rtc_state);
 
     if (pci_enabled && usb_enabled(false)) {
@@ -237,17 +233,7 @@ static void pc_init1(MemoryRegion *system_memory,
 
 static void pc_init_pci(QEMUMachineInitArgs *args)
 {
-    ram_addr_t ram_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
-    pc_init1(get_system_memory(),
-             get_system_io(),
-             ram_size, boot_device,
-             kernel_filename, kernel_cmdline,
-             initrd_filename, cpu_model, 1, 1);
+    pc_init1(args, get_system_memory(), get_system_io(), 1, 1);
 }
 
 static void pc_init_pci_1_6(QEMUMachineInitArgs *args)
@@ -293,40 +279,21 @@ static void pc_init_pci_1_0(QEMUMachineInitArgs *args)
 /* PC init function for pc-0.10 to pc-0.13, and reused by xenfv */
 static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
 {
-    ram_addr_t ram_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
     has_pci_info = false;
     disable_kvm_pv_eoi();
     enable_compat_apic_id_mode();
-    pc_init1(get_system_memory(),
-             get_system_io(),
-             ram_size, boot_device,
-             kernel_filename, kernel_cmdline,
-             initrd_filename, cpu_model, 1, 0);
+    pc_init1(args, get_system_memory(), get_system_io(), 1, 0);
 }
 
 static void pc_init_isa(QEMUMachineInitArgs *args)
 {
-    ram_addr_t ram_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
     has_pci_info = false;
-    if (cpu_model == NULL)
-        cpu_model = "486";
+    if (!args->cpu_model) {
+        args->cpu_model = "486";
+    }
     disable_kvm_pv_eoi();
     enable_compat_apic_id_mode();
-    pc_init1(get_system_memory(),
-             get_system_io(),
-             ram_size, boot_device,
-             kernel_filename, kernel_cmdline,
-             initrd_filename, cpu_model, 0, 1);
+    pc_init1(args, get_system_memory(), get_system_io(), 0, 1);
 }
 
 #ifdef CONFIG_XEN
-- 
1.8.3.1

[Qemu-devel] [pci PATCH v2 2/6] pc: Don't explode QEMUMachineInitArgs into local variables needlessly

[Eduardo Habkost]

From: Markus Armbruster <armbru@redhat.com>

Don't explode when the variable is used just a few times, and never
changed.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_q35.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 533a2d6..4847f46 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -53,12 +53,6 @@ static bool has_pci_info = true;
 /* PC hardware initialisation */
 static void pc_q35_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t ram_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
     ram_addr_t below_4g_mem_size, above_4g_mem_size;
     Q35PCIHost *q35_host;
     PCIHostState *phb;
@@ -86,17 +80,17 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     object_property_add_child(qdev_get_machine(), "icc-bridge",
                               OBJECT(icc_bridge), NULL);
 
-    pc_cpus_init(cpu_model, icc_bridge);
+    pc_cpus_init(args->cpu_model, icc_bridge);
     pc_acpi_init("q35-acpi-dsdt.aml");
 
     kvmclock_create();
 
-    if (ram_size >= 0xb0000000) {
-        above_4g_mem_size = ram_size - 0xb0000000;
+    if (args->ram_size >= 0xb0000000) {
+        above_4g_mem_size = args->ram_size - 0xb0000000;
         below_4g_mem_size = 0xb0000000;
     } else {
         above_4g_mem_size = 0;
-        below_4g_mem_size = ram_size;
+        below_4g_mem_size = args->ram_size;
     }
 
     /* pci enabled */
@@ -115,8 +109,10 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
 
     /* allocate ram and load rom/bios */
     if (!xen_enabled()) {
-        pc_memory_init(get_system_memory(), kernel_filename, kernel_cmdline,
-                       initrd_filename, below_4g_mem_size, above_4g_mem_size,
+        pc_memory_init(get_system_memory(),
+                       args->kernel_filename, args->kernel_cmdline,
+                       args->initrd_filename,
+                       below_4g_mem_size, above_4g_mem_size,
                        rom_memory, &ram_memory, guest_info);
     }
 
@@ -204,7 +200,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
                                     0xb100),
                       8, NULL, 0);
 
-    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, boot_device,
+    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
                  floppy, idebus[0], idebus[1], rtc_state);
 
     /* the rest devices to which pci devfn is automatically assigned */
-- 
1.8.3.1

[Qemu-devel] [pci PATCH v2 3/6] pc: Kill pc_init_pci_1_0()

[Eduardo Habkost]

The pc_init_pci_1_2()/pc_init_pci_1_0() split was made on commit
6fd028f64f662c801fd5a54d0e3a1d2baeee93ea, in preparation for commit
9953f8822cc316eec9962f0a2858c3439a80adec. The latter was reverted, so there's
no reason to keep two separate functions that do exactly the same, anymore.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
---
 hw/i386/pc_piix.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index bbefea6..dd730b1 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -263,19 +263,13 @@ static void pc_init_pci_1_3(QEMUMachineInitArgs *args)
     pc_init_pci_1_4(args);
 }
 
-/* PC machine init function for pc-1.1 to pc-1.2 */
+/* PC machine init function for pc-0.14 to pc-1.2 */
 static void pc_init_pci_1_2(QEMUMachineInitArgs *args)
 {
     disable_kvm_pv_eoi();
     pc_init_pci_1_3(args);
 }
 
-/* PC machine init function for pc-0.14 to pc-1.0 */
-static void pc_init_pci_1_0(QEMUMachineInitArgs *args)
-{
-    pc_init_pci_1_2(args);
-}
-
 /* PC init function for pc-0.10 to pc-0.13, and reused by xenfv */
 static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
 {
@@ -485,7 +479,7 @@ static QEMUMachine pc_machine_v1_1 = {
 static QEMUMachine pc_machine_v1_0 = {
     .name = "pc-1.0",
     .desc = "Standard PC",
-    .init = pc_init_pci_1_0,
+    .init = pc_init_pci_1_2,
     .max_cpus = 255,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_0,
@@ -501,7 +495,7 @@ static QEMUMachine pc_machine_v1_0 = {
 static QEMUMachine pc_machine_v0_15 = {
     .name = "pc-0.15",
     .desc = "Standard PC",
-    .init = pc_init_pci_1_0,
+    .init = pc_init_pci_1_2,
     .max_cpus = 255,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_15,
@@ -534,7 +528,7 @@ static QEMUMachine pc_machine_v0_15 = {
 static QEMUMachine pc_machine_v0_14 = {
     .name = "pc-0.14",
     .desc = "Standard PC",
-    .init = pc_init_pci_1_0,
+    .init = pc_init_pci_1_2,
     .max_cpus = 255,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_14, 
-- 
1.8.3.1

[Qemu-devel] [pci PATCH v2 4/6] pc: Create pc_compat_*() functions

[Eduardo Habkost]

Making the older compat functions call the newer compat functions at the
beginning allows the older functions undo what's done by newer compat
functions. e.g.: pc_compat_1_4() will be able to call pc_compat_1_5()
and then set has_pvpanic=false.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c | 52 +++++++++++++++++++++++++++++++++++++++++-----------
 hw/i386/pc_q35.c  | 31 ++++++++++++++++++++++++-------
 2 files changed, 65 insertions(+), 18 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index dd730b1..a5e5bec 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -236,38 +236,68 @@ static void pc_init_pci(QEMUMachineInitArgs *args)
     pc_init1(args, get_system_memory(), get_system_io(), 1, 1);
 }
 
-static void pc_init_pci_1_6(QEMUMachineInitArgs *args)
+static void pc_compat_1_6(QEMUMachineInitArgs *args)
 {
     has_pci_info = false;
     rom_file_in_ram = false;
-    pc_init_pci(args);
 }
 
-static void pc_init_pci_1_5(QEMUMachineInitArgs *args)
+static void pc_compat_1_5(QEMUMachineInitArgs *args)
 {
+    pc_compat_1_6(args);
     has_pvpanic = true;
-    pc_init_pci_1_6(args);
 }
 
-static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
+static void pc_compat_1_4(QEMUMachineInitArgs *args)
 {
+    /* 1.5 was special - it enabled pvpanic in builtin machine */
+    pc_compat_1_6(args);
     x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
     x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, CPUID_EXT_PCLMULQDQ);
-    /* 1.5 was special - it enabled pvpanic in builtin machine */
-    pc_init_pci_1_6(args);
 }
 
-static void pc_init_pci_1_3(QEMUMachineInitArgs *args)
+static void pc_compat_1_3(QEMUMachineInitArgs *args)
 {
+    pc_compat_1_4(args);
     enable_compat_apic_id_mode();
-    pc_init_pci_1_4(args);
+}
+
+/* PC compat function for pc-0.14 to pc-1.2 */
+static void pc_compat_1_2(QEMUMachineInitArgs *args)
+{
+    pc_compat_1_3(args);
+    disable_kvm_pv_eoi();
+}
+
+static void pc_init_pci_1_6(QEMUMachineInitArgs *args)
+{
+    pc_compat_1_6(args);
+    pc_init_pci(args);
+}
+
+static void pc_init_pci_1_5(QEMUMachineInitArgs *args)
+{
+    pc_compat_1_5(args);
+    pc_init_pci(args);
+}
+
+static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
+{
+    pc_compat_1_4(args);
+    pc_init_pci(args);
+}
+
+static void pc_init_pci_1_3(QEMUMachineInitArgs *args)
+{
+    pc_compat_1_3(args);
+    pc_init_pci(args);
 }
 
 /* PC machine init function for pc-0.14 to pc-1.2 */
 static void pc_init_pci_1_2(QEMUMachineInitArgs *args)
 {
-    disable_kvm_pv_eoi();
-    pc_init_pci_1_3(args);
+    pc_compat_1_2(args);
+    pc_init_pci(args);
 }
 
 /* PC init function for pc-0.10 to pc-0.13, and reused by xenfv */
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 4847f46..f5e0b94 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -215,25 +215,42 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     }
 }
 
-static void pc_q35_init_1_6(QEMUMachineInitArgs *args)
+static void pc_compat_1_6(QEMUMachineInitArgs *args)
 {
     has_pci_info = false;
     rom_file_in_ram = false;
-    pc_q35_init(args);
 }
 
-static void pc_q35_init_1_5(QEMUMachineInitArgs *args)
+static void pc_compat_1_5(QEMUMachineInitArgs *args)
 {
+    pc_compat_1_6(args);
     has_pvpanic = true;
-    pc_q35_init_1_6(args);
 }
 
-static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
+static void pc_compat_1_4(QEMUMachineInitArgs *args)
 {
+    /* 1.5 was special - it enabled pvpanic in builtin machine */
+    pc_compat_1_6(args);
     x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
     x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, CPUID_EXT_PCLMULQDQ);
-    /* 1.5 was special - it enabled pvpanic in builtin machine */
-    pc_q35_init_1_6(args);
+}
+
+static void pc_q35_init_1_6(QEMUMachineInitArgs *args)
+{
+    pc_compat_1_6(args);
+    pc_q35_init(args);
+}
+
+static void pc_q35_init_1_5(QEMUMachineInitArgs *args)
+{
+    pc_compat_1_5(args);
+    pc_q35_init(args);
+}
+
+static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
+{
+    pc_compat_1_4(args);
+    pc_q35_init(args);
 }
 
 static QEMUMachine pc_q35_machine_v1_6 = {
-- 
1.8.3.1

[Qemu-devel] [pci PATCH v2 5/6] pc: pc_compat_1_4() now can call pc_compat_1_5()

[Eduardo Habkost]

It just needs to set has_pvpanic=false after calling it. This way, it
won't be a special case anymore.

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

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index a5e5bec..d3d4893 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -250,8 +250,8 @@ static void pc_compat_1_5(QEMUMachineInitArgs *args)
 
 static void pc_compat_1_4(QEMUMachineInitArgs *args)
 {
-    /* 1.5 was special - it enabled pvpanic in builtin machine */
-    pc_compat_1_6(args);
+    pc_compat_1_5(args);
+    has_pvpanic = false;
     x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
     x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, CPUID_EXT_PCLMULQDQ);
 }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index f5e0b94..198c785 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -229,8 +229,8 @@ static void pc_compat_1_5(QEMUMachineInitArgs *args)
 
 static void pc_compat_1_4(QEMUMachineInitArgs *args)
 {
-    /* 1.5 was special - it enabled pvpanic in builtin machine */
-    pc_compat_1_6(args);
+    pc_compat_1_5(args);
+    has_pvpanic = false;
     x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
     x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, CPUID_EXT_PCLMULQDQ);
 }
-- 
1.8.3.1

[Qemu-devel] [pci PATCH v2 6/6] pc_piix: Kill pc_init1() memory region args

[Eduardo Habkost]

All callers always use the same values (get_system_memory(),
get_system_io()), so the parameters are pointless.

If one day we decide to eliminate get_system_memory() and
get_system_io(), we will be able to do that more easily by adding the
values to struct QEMUMachineInitArgs.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index d3d4893..3c36a2a 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -62,11 +62,11 @@ static bool has_pci_info = true;
 
 /* PC hardware initialisation */
 static void pc_init1(QEMUMachineInitArgs *args,
-                     MemoryRegion *system_memory,
-                     MemoryRegion *system_io,
                      int pci_enabled,
                      int kvmclock_enabled)
 {
+    MemoryRegion *system_memory = get_system_memory();
+    MemoryRegion *system_io = get_system_io();
     int i;
     ram_addr_t below_4g_mem_size, above_4g_mem_size;
     PCIBus *pci_bus;
@@ -233,7 +233,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
 
 static void pc_init_pci(QEMUMachineInitArgs *args)
 {
-    pc_init1(args, get_system_memory(), get_system_io(), 1, 1);
+    pc_init1(args, 1, 1);
 }
 
 static void pc_compat_1_6(QEMUMachineInitArgs *args)
@@ -306,7 +306,7 @@ static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
     has_pci_info = false;
     disable_kvm_pv_eoi();
     enable_compat_apic_id_mode();
-    pc_init1(args, get_system_memory(), get_system_io(), 1, 0);
+    pc_init1(args, 1, 0);
 }
 
 static void pc_init_isa(QEMUMachineInitArgs *args)
@@ -317,7 +317,7 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
     }
     disable_kvm_pv_eoi();
     enable_compat_apic_id_mode();
-    pc_init1(args, get_system_memory(), get_system_io(), 0, 1);
+    pc_init1(args, 0, 1);
 }
 
 #ifdef CONFIG_XEN
-- 
1.8.3.1

Re: [Qemu-devel] [pci PATCH v2 6/6] pc_piix: Kill pc_init1() memory region args

[Michael S. Tsirkin]

On Wed, Aug 21, 2013 at 03:14:45PM -0300, Eduardo Habkost wrote:
> All callers always use the same values (get_system_memory(),
> get_system_io()), so the parameters are pointless.
> 
> If one day we decide to eliminate get_system_memory() and
> get_system_io(), we will be able to do that more easily by adding the
> values to struct QEMUMachineInitArgs.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Paolo, could you ack this patch please?
It's on my tree but I can always revert ... :)

> ---
>  hw/i386/pc_piix.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index d3d4893..3c36a2a 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -62,11 +62,11 @@ static bool has_pci_info = true;
>  
>  /* PC hardware initialisation */
>  static void pc_init1(QEMUMachineInitArgs *args,
> -                     MemoryRegion *system_memory,
> -                     MemoryRegion *system_io,
>                       int pci_enabled,
>                       int kvmclock_enabled)
>  {
> +    MemoryRegion *system_memory = get_system_memory();
> +    MemoryRegion *system_io = get_system_io();
>      int i;
>      ram_addr_t below_4g_mem_size, above_4g_mem_size;
>      PCIBus *pci_bus;
> @@ -233,7 +233,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
>  
>  static void pc_init_pci(QEMUMachineInitArgs *args)
>  {
> -    pc_init1(args, get_system_memory(), get_system_io(), 1, 1);
> +    pc_init1(args, 1, 1);
>  }
>  
>  static void pc_compat_1_6(QEMUMachineInitArgs *args)
> @@ -306,7 +306,7 @@ static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
>      has_pci_info = false;
>      disable_kvm_pv_eoi();
>      enable_compat_apic_id_mode();
> -    pc_init1(args, get_system_memory(), get_system_io(), 1, 0);
> +    pc_init1(args, 1, 0);
>  }
>  
>  static void pc_init_isa(QEMUMachineInitArgs *args)
> @@ -317,7 +317,7 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
>      }
>      disable_kvm_pv_eoi();
>      enable_compat_apic_id_mode();
> -    pc_init1(args, get_system_memory(), get_system_io(), 0, 1);
> +    pc_init1(args, 0, 1);
>  }
>  
>  #ifdef CONFIG_XEN
> -- 
> 1.8.3.1

Re: [Qemu-devel] [pci PATCH v2 0/6] pc init & compat function cleanup

[Michael S. Tsirkin]

On Wed, Aug 21, 2013 at 03:14:39PM -0300, Eduardo Habkost wrote:
> Now sending the same series based on Michael's pci tree, to avoid conflicts,
> and including 2 "don't prematurely explode QEMUMachineInitArgs" patches
> from Markus.

Looks good to me.
Applied, thanks.

> Eduardo Habkost (4):
>   pc: Kill pc_init_pci_1_0()
>   pc: Create pc_compat_*() functions
>   pc: pc_compat_1_4() now can call pc_compat_1_5()
>   pc_piix: Kill pc_init1() memory region args
> 
> Markus Armbruster (2):
>   pc: Don't prematurely explode QEMUMachineInitArgs
>   pc: Don't explode QEMUMachineInitArgs into local variables needlessly
> 
>  hw/i386/pc_piix.c | 127 +++++++++++++++++++++++++-----------------------------
>  hw/i386/pc_q35.c  |  53 ++++++++++++++---------
>  2 files changed, 92 insertions(+), 88 deletions(-)
> 
> -- 
> 1.8.3.1