All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v4 0/6] Clean up bogus default boot order
@ 2013-08-16 11:13 armbru
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 1/6] pc: Don't prematurely explode QEMUMachineInitArgs armbru
                   ` (6 more replies)
  0 siblings, 7 replies; 23+ messages in thread
From: armbru @ 2013-08-16 11:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, mjt, agraf, blauwirbel, qemu-ppc, aviksil

From: Markus Armbruster <armbru@redhat.com>

The first five patches are admittedly related to the stated purpose of
this series pretty much only by "I can't stand perpetuating this
stupid crap".  Max Filippov and Peter Maydell already cleaned up
Xtensa and ARM the same way.

v4:
* Rebase
v3:
* Rebase
* Don't screw up default CPU model for machine "isapc" in 1/6
v2:
* Straightforward rebase
* PATCH 6 improved commit message (Alex)

Markus Armbruster (6):
  pc: Don't prematurely explode QEMUMachineInitArgs
  pc: Don't explode QEMUMachineInitArgs into local variables needlessly
  sun4: Don't prematurely explode QEMUMachineInitArgs
  ppc: Don't explode QEMUMachineInitArgs into local variables needlessly
  ppc: Don't duplicate QEMUMachineInitArgs in PPCE500Params
  hw: Clean up bogus default boot order

 hw/alpha/dp264.c                         |   1 -
 hw/arm/collie.c                          |   1 -
 hw/arm/exynos4_boards.c                  |   2 -
 hw/arm/gumstix.c                         |   2 -
 hw/arm/highbank.c                        |   2 -
 hw/arm/integratorcp.c                    |   1 -
 hw/arm/kzm.c                             |   1 -
 hw/arm/mainstone.c                       |   1 -
 hw/arm/musicpal.c                        |   1 -
 hw/arm/nseries.c                         |   6 +-
 hw/arm/omap_sx1.c                        |   2 -
 hw/arm/palm.c                            |   1 -
 hw/arm/realview.c                        |   4 -
 hw/arm/spitz.c                           |   4 -
 hw/arm/stellaris.c                       |   2 -
 hw/arm/tosa.c                            |   1 -
 hw/arm/versatilepb.c                     |   2 -
 hw/arm/vexpress.c                        |   2 -
 hw/arm/xilinx_zynq.c                     |   1 -
 hw/arm/z2.c                              |   1 -
 hw/core/null-machine.c                   |   1 -
 hw/cris/axis_dev88.c                     |   1 -
 hw/i386/pc_piix.c                        |  95 ++++++++--------------
 hw/i386/pc_q35.c                         |  28 +++----
 hw/i386/xen_machine_pv.c                 |   1 -
 hw/lm32/lm32_boards.c                    |   2 -
 hw/lm32/milkymist.c                      |   1 -
 hw/m68k/an5206.c                         |   1 -
 hw/m68k/dummy_m68k.c                     |   1 -
 hw/m68k/mcf5208.c                        |   1 -
 hw/microblaze/petalogix_ml605_mmu.c      |   1 -
 hw/microblaze/petalogix_s3adsp1800_mmu.c |   1 -
 hw/mips/mips_fulong2e.c                  |   1 -
 hw/mips/mips_jazz.c                      |   2 -
 hw/mips/mips_malta.c                     |   1 -
 hw/mips/mips_mipssim.c                   |   1 -
 hw/mips/mips_r4k.c                       |   1 -
 hw/openrisc/openrisc_sim.c               |   1 -
 hw/ppc/e500.c                            |  35 +++++----
 hw/ppc/e500.h                            |  13 +--
 hw/ppc/e500plat.c                        |  15 +---
 hw/ppc/mac_newworld.c                    |   4 +-
 hw/ppc/mac_oldworld.c                    |   4 +-
 hw/ppc/mpc8544ds.c                       |  15 +---
 hw/ppc/ppc405_boards.c                   |   2 -
 hw/ppc/ppc440_bamboo.c                   |   1 -
 hw/ppc/prep.c                            |   4 +-
 hw/ppc/spapr.c                           |   4 +-
 hw/ppc/virtex_ml507.c                    |   1 -
 hw/s390x/s390-virtio-ccw.c               |   1 -
 hw/s390x/s390-virtio.c                   |   1 -
 hw/sh4/r2d.c                             |   1 -
 hw/sh4/shix.c                            |   1 -
 hw/sparc/leon3.c                         |   1 -
 hw/sparc/sun4m.c                         | 131 ++++++++-----------------------
 hw/sparc64/sun4u.c                       |  58 +++++---------
 hw/unicore32/puv3.c                      |   1 -
 hw/xtensa/xtensa_lx60.c                  |   2 -
 hw/xtensa/xtensa_sim.c                   |   1 -
 include/hw/boards.h                      |   7 +-
 vl.c                                     |   4 +-
 61 files changed, 133 insertions(+), 353 deletions(-)

-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH v4 1/6] pc: Don't prematurely explode QEMUMachineInitArgs
  2013-08-16 11:13 [Qemu-devel] [PATCH v4 0/6] Clean up bogus default boot order armbru
@ 2013-08-16 11:13 ` armbru
  2013-08-17 10:07   ` Laszlo Ersek
  2013-08-21 18:05   ` Eduardo Habkost
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 2/6] pc: Don't explode QEMUMachineInitArgs into local variables needlessly armbru
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 23+ messages in thread
From: armbru @ 2013-08-16 11:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, mjt, agraf, blauwirbel, qemu-ppc, aviksil

From: Markus Armbruster <armbru@redhat.com>

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

Signed-off-by: Markus Armbruster <armbru@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 6e1e654..2ae7f95 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -60,14 +60,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)
 {
@@ -102,18 +97,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) {
@@ -132,7 +127,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);
     }
@@ -148,7 +144,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,
@@ -207,7 +203,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)) {
@@ -236,17 +232,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)
@@ -291,40 +277,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.1.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH v4 2/6] pc: Don't explode QEMUMachineInitArgs into local variables needlessly
  2013-08-16 11:13 [Qemu-devel] [PATCH v4 0/6] Clean up bogus default boot order armbru
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 1/6] pc: Don't prematurely explode QEMUMachineInitArgs armbru
@ 2013-08-16 11:13 ` armbru
  2013-08-17 10:12   ` Laszlo Ersek
  2013-08-21 18:06   ` Eduardo Habkost
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 3/6] sun4: Don't prematurely explode QEMUMachineInitArgs armbru
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 23+ messages in thread
From: armbru @ 2013-08-16 11:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, mjt, agraf, blauwirbel, qemu-ppc, aviksil

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>
---
 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 10e770e..f636c65 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -52,12 +52,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;
@@ -85,17 +79,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 */
@@ -114,8 +108,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);
     }
 
@@ -203,7 +199,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.1.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH v4 3/6] sun4: Don't prematurely explode QEMUMachineInitArgs
  2013-08-16 11:13 [Qemu-devel] [PATCH v4 0/6] Clean up bogus default boot order armbru
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 1/6] pc: Don't prematurely explode QEMUMachineInitArgs armbru
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 2/6] pc: Don't explode QEMUMachineInitArgs into local variables needlessly armbru
@ 2013-08-16 11:13 ` armbru
  2013-08-17 10:29   ` Laszlo Ersek
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 4/6] ppc: Don't explode QEMUMachineInitArgs into local variables needlessly armbru
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: armbru @ 2013-08-16 11:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, mjt, agraf, blauwirbel, qemu-ppc, aviksil

From: Markus Armbruster <armbru@redhat.com>

Don't explode QEMUMachineInitArgs before passing it to
sun4m_hw_init(), sun4uv_init().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/sparc/sun4m.c   | 113 ++++++++++++-----------------------------------------
 hw/sparc64/sun4u.c |  52 +++++++-----------------
 2 files changed, 40 insertions(+), 125 deletions(-)

diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 942ca37..36ef36f 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -836,12 +836,10 @@ static void dummy_fdc_tc(void *opaque, int irq, int level)
 {
 }
 
-static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, 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)
+static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
+                          QEMUMachineInitArgs *args)
 {
+    const char *cpu_model = args->cpu_model;
     unsigned int i;
     void *iommu, *espdma, *ledma, *nvram;
     qemu_irq *cpu_irqs[MAX_CPUS], slavio_irq[32], slavio_cpu_irq[MAX_CPUS],
@@ -867,10 +865,10 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
 
 
     /* set up devices */
-    ram_init(0, RAM_size, hwdef->max_mem);
+    ram_init(0, args->ram_size, hwdef->max_mem);
     /* models without ECC don't trap when missing ram is accessed */
     if (!hwdef->ecc_base) {
-        empty_slot_init(RAM_size, hwdef->max_mem - RAM_size);
+        empty_slot_init(args->ram_size, hwdef->max_mem - args->ram_size);
     }
 
     prom_init(hwdef->slavio_base, bios_name);
@@ -993,11 +991,12 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
         empty_slot_init(hwdef->bpp_base, 0x20);
     }
 
-    kernel_size = sun4m_load_kernel(kernel_filename, initrd_filename,
-                                    RAM_size);
+    kernel_size = sun4m_load_kernel(args->kernel_filename,
+                                    args->initrd_filename,
+                                    args->ram_size);
 
-    nvram_init(nvram, (uint8_t *)&nd_table[0].macaddr, kernel_cmdline,
-               boot_device, RAM_size, kernel_size, graphic_width,
+    nvram_init(nvram, (uint8_t *)&nd_table[0].macaddr, args->kernel_cmdline,
+               args->boot_device, args->ram_size, kernel_size, graphic_width,
                graphic_height, graphic_depth, hwdef->nvram_machine_id,
                "Sun4m");
 
@@ -1015,19 +1014,20 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
     fw_cfg_add_i16(fw_cfg, FW_CFG_SUN4M_HEIGHT, graphic_height);
     fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, KERNEL_LOAD_ADDR);
     fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
-    if (kernel_cmdline) {
+    if (args->kernel_cmdline) {
         fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
-        pstrcpy_targphys("cmdline", CMDLINE_ADDR, TARGET_PAGE_SIZE, kernel_cmdline);
-        fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, kernel_cmdline);
+        pstrcpy_targphys("cmdline", CMDLINE_ADDR, TARGET_PAGE_SIZE,
+                         args->kernel_cmdline);
+        fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, args->kernel_cmdline);
         fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,
-                       strlen(kernel_cmdline) + 1);
+                       strlen(args->kernel_cmdline) + 1);
     } else {
         fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
         fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, 0);
     }
     fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, INITRD_LOAD_ADDR);
     fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, 0); // not used
-    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, boot_device[0]);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, args->boot_device[0]);
     qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
 }
 
@@ -1291,118 +1291,55 @@ static const struct sun4m_hwdef sun4m_hwdefs[] = {
 /* SPARCstation 5 hardware initialisation */
 static void ss5_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;
-    sun4m_hw_init(&sun4m_hwdefs[0], RAM_size, boot_device, kernel_filename,
-                  kernel_cmdline, initrd_filename, cpu_model);
+    sun4m_hw_init(&sun4m_hwdefs[0], args);
 }
 
 /* SPARCstation 10 hardware initialisation */
 static void ss10_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;
-    sun4m_hw_init(&sun4m_hwdefs[1], RAM_size, boot_device, kernel_filename,
-                  kernel_cmdline, initrd_filename, cpu_model);
+    sun4m_hw_init(&sun4m_hwdefs[1], args);
 }
 
 /* SPARCserver 600MP hardware initialisation */
 static void ss600mp_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;
-    sun4m_hw_init(&sun4m_hwdefs[2], RAM_size, boot_device, kernel_filename,
-                  kernel_cmdline, initrd_filename, cpu_model);
+    sun4m_hw_init(&sun4m_hwdefs[2], args);
 }
 
 /* SPARCstation 20 hardware initialisation */
 static void ss20_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;
-    sun4m_hw_init(&sun4m_hwdefs[3], RAM_size, boot_device, kernel_filename,
-                  kernel_cmdline, initrd_filename, cpu_model);
+    sun4m_hw_init(&sun4m_hwdefs[3], args);
 }
 
 /* SPARCstation Voyager hardware initialisation */
 static void vger_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;
-    sun4m_hw_init(&sun4m_hwdefs[4], RAM_size, boot_device, kernel_filename,
-                  kernel_cmdline, initrd_filename, cpu_model);
+    sun4m_hw_init(&sun4m_hwdefs[4], args);
 }
 
 /* SPARCstation LX hardware initialisation */
 static void ss_lx_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;
-    sun4m_hw_init(&sun4m_hwdefs[5], RAM_size, boot_device, kernel_filename,
-                  kernel_cmdline, initrd_filename, cpu_model);
+    sun4m_hw_init(&sun4m_hwdefs[5], args);
 }
 
 /* SPARCstation 4 hardware initialisation */
 static void ss4_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;
-    sun4m_hw_init(&sun4m_hwdefs[6], RAM_size, boot_device, kernel_filename,
-                  kernel_cmdline, initrd_filename, cpu_model);
+    sun4m_hw_init(&sun4m_hwdefs[6], args);
 }
 
 /* SPARCClassic hardware initialisation */
 static void scls_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;
-    sun4m_hw_init(&sun4m_hwdefs[7], RAM_size, boot_device, kernel_filename,
-                  kernel_cmdline, initrd_filename, cpu_model);
+    sun4m_hw_init(&sun4m_hwdefs[7], args);
 }
 
 /* SPARCbook hardware initialisation */
 static void sbook_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;
-    sun4m_hw_init(&sun4m_hwdefs[8], RAM_size, boot_device, kernel_filename,
-                  kernel_cmdline, initrd_filename, cpu_model);
+    sun4m_hw_init(&sun4m_hwdefs[8], args);
 }
 
 static QEMUMachine ss5_machine = {
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index a7214a3..8bd7fb9 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -811,10 +811,7 @@ static SPARCCPU *cpu_devinit(const char *cpu_model, const struct hwdef *hwdef)
 }
 
 static void sun4uv_init(MemoryRegion *address_space_mem,
-                        ram_addr_t RAM_size,
-                        const char *boot_devices,
-                        const char *kernel_filename, const char *kernel_cmdline,
-                        const char *initrd_filename, const char *cpu_model,
+                        QEMUMachineInitArgs *args,
                         const struct hwdef *hwdef)
 {
     SPARCCPU *cpu;
@@ -829,10 +826,10 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     FWCfgState *fw_cfg;
 
     /* init CPUs */
-    cpu = cpu_devinit(cpu_model, hwdef);
+    cpu = cpu_devinit(args->cpu_model, hwdef);
 
     /* set up devices */
-    ram_init(0, RAM_size);
+    ram_init(0, args->ram_size);
 
     prom_init(hwdef->prom_addr, bios_name);
 
@@ -878,13 +875,15 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
 
     initrd_size = 0;
     initrd_addr = 0;
-    kernel_size = sun4u_load_kernel(kernel_filename, initrd_filename,
+    kernel_size = sun4u_load_kernel(args->kernel_filename,
+                                    args->initrd_filename,
                                     ram_size, &initrd_size, &initrd_addr,
                                     &kernel_addr, &kernel_entry);
 
-    sun4u_NVRAM_set_params(nvram, NVRAM_SIZE, "Sun4u", RAM_size, boot_devices,
+    sun4u_NVRAM_set_params(nvram, NVRAM_SIZE, "Sun4u", args->ram_size,
+                           args->boot_device,
                            kernel_addr, kernel_size,
-                           kernel_cmdline,
+                           args->kernel_cmdline,
                            initrd_addr, initrd_size,
                            /* XXX: need an option to load a NVRAM image */
                            0,
@@ -898,16 +897,16 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, hwdef->machine_id);
     fw_cfg_add_i64(fw_cfg, FW_CFG_KERNEL_ADDR, kernel_entry);
     fw_cfg_add_i64(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
-    if (kernel_cmdline) {
+    if (args->kernel_cmdline) {
         fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,
-                       strlen(kernel_cmdline) + 1);
-        fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, kernel_cmdline);
+                       strlen(args->kernel_cmdline) + 1);
+        fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, args->kernel_cmdline);
     } else {
         fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, 0);
     }
     fw_cfg_add_i64(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr);
     fw_cfg_add_i64(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, boot_devices[0]);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, args->boot_device[0]);
 
     fw_cfg_add_i16(fw_cfg, FW_CFG_SPARC64_WIDTH, graphic_width);
     fw_cfg_add_i16(fw_cfg, FW_CFG_SPARC64_HEIGHT, graphic_height);
@@ -949,40 +948,19 @@ static const struct hwdef hwdefs[] = {
 /* Sun4u hardware initialisation */
 static void sun4u_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_devices = args->boot_device;
-    sun4uv_init(get_system_memory(), RAM_size, boot_devices, kernel_filename,
-                kernel_cmdline, initrd_filename, cpu_model, &hwdefs[0]);
+    sun4uv_init(get_system_memory(), args, &hwdefs[0]);
 }
 
 /* Sun4v hardware initialisation */
 static void sun4v_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_devices = args->boot_device;
-    sun4uv_init(get_system_memory(), RAM_size, boot_devices, kernel_filename,
-                kernel_cmdline, initrd_filename, cpu_model, &hwdefs[1]);
+    sun4uv_init(get_system_memory(), args, &hwdefs[1]);
 }
 
 /* Niagara hardware initialisation */
 static void niagara_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_devices = args->boot_device;
-    sun4uv_init(get_system_memory(), RAM_size, boot_devices, kernel_filename,
-                kernel_cmdline, initrd_filename, cpu_model, &hwdefs[2]);
+    sun4uv_init(get_system_memory(), args, &hwdefs[2]);
 }
 
 static QEMUMachine sun4u_machine = {
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH v4 4/6] ppc: Don't explode QEMUMachineInitArgs into local variables needlessly
  2013-08-16 11:13 [Qemu-devel] [PATCH v4 0/6] Clean up bogus default boot order armbru
                   ` (2 preceding siblings ...)
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 3/6] sun4: Don't prematurely explode QEMUMachineInitArgs armbru
@ 2013-08-16 11:13 ` armbru
  2013-08-17 10:33   ` Laszlo Ersek
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 5/6] ppc: Don't duplicate QEMUMachineInitArgs in PPCE500Params armbru
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: armbru @ 2013-08-16 11:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, mjt, agraf, blauwirbel, qemu-ppc, aviksil

From: Markus Armbruster <armbru@redhat.com>

Don't explode when the variable is used just once, and never changed.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Alexander Graf <agraf@suse.de>
---
 hw/ppc/e500plat.c  | 18 ++++++------------
 hw/ppc/mpc8544ds.c | 18 ++++++------------
 2 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
index c852995..a78de07 100644
--- a/hw/ppc/e500plat.c
+++ b/hw/ppc/e500plat.c
@@ -30,19 +30,13 @@ static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt)
 
 static void e500plat_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t ram_size = args->ram_size;
-    const char *boot_device = args->boot_device;
-    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;
     PPCE500Params params = {
-        .ram_size = ram_size,
-        .boot_device = boot_device,
-        .kernel_filename = kernel_filename,
-        .kernel_cmdline = kernel_cmdline,
-        .initrd_filename = initrd_filename,
-        .cpu_model = cpu_model,
+        .ram_size = args->ram_size,
+        .boot_device = args->boot_device,
+        .kernel_filename = args->kernel_filename,
+        .kernel_cmdline = args->kernel_cmdline,
+        .initrd_filename = args->initrd_filename,
+        .cpu_model = args->cpu_model,
         .pci_first_slot = 0x1,
         .pci_nr_slots = PCI_SLOT_MAX - 1,
         .fixup_devtree = e500plat_fixup_devtree,
diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
index 444da02..4e551af 100644
--- a/hw/ppc/mpc8544ds.c
+++ b/hw/ppc/mpc8544ds.c
@@ -28,19 +28,13 @@ static void mpc8544ds_fixup_devtree(PPCE500Params *params, void *fdt)
 
 static void mpc8544ds_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t ram_size = args->ram_size;
-    const char *boot_device = args->boot_device;
-    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;
     PPCE500Params params = {
-        .ram_size = ram_size,
-        .boot_device = boot_device,
-        .kernel_filename = kernel_filename,
-        .kernel_cmdline = kernel_cmdline,
-        .initrd_filename = initrd_filename,
-        .cpu_model = cpu_model,
+        .ram_size = args->ram_size,
+        .boot_device = args->boot_device,
+        .kernel_filename = args->kernel_filename,
+        .kernel_cmdline = args->kernel_cmdline,
+        .initrd_filename = args->initrd_filename,
+        .cpu_model = args->cpu_model,
         .pci_first_slot = 0x11,
         .pci_nr_slots = 2,
         .fixup_devtree = mpc8544ds_fixup_devtree,
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH v4 5/6] ppc: Don't duplicate QEMUMachineInitArgs in PPCE500Params
  2013-08-16 11:13 [Qemu-devel] [PATCH v4 0/6] Clean up bogus default boot order armbru
                   ` (3 preceding siblings ...)
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 4/6] ppc: Don't explode QEMUMachineInitArgs into local variables needlessly armbru
@ 2013-08-16 11:13 ` armbru
  2013-08-17 10:46   ` Laszlo Ersek
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 6/6] hw: Clean up bogus default boot order armbru
  2013-08-21 20:28 ` [Qemu-devel] [PATCH v4 0/6] " Michael S. Tsirkin
  6 siblings, 1 reply; 23+ messages in thread
From: armbru @ 2013-08-16 11:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, mjt, agraf, blauwirbel, qemu-ppc, aviksil

From: Markus Armbruster <armbru@redhat.com>

Pass on the generic arguments unadulterated, and the machine-specific
ones as separate argument.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Alexander Graf <agraf@suse.de>
---
 hw/ppc/e500.c      | 35 ++++++++++++++++++-----------------
 hw/ppc/e500.h      | 13 +++----------
 hw/ppc/e500plat.c  |  8 +-------
 hw/ppc/mpc8544ds.c |  8 +-------
 4 files changed, 23 insertions(+), 41 deletions(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index f00a62a..e79612b 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -124,13 +124,14 @@ static void dt_serial_create(void *fdt, unsigned long long offset,
 }
 
 static int ppce500_load_device_tree(CPUPPCState *env,
+                                    QEMUMachineInitArgs *args,
                                     PPCE500Params *params,
                                     hwaddr addr,
                                     hwaddr initrd_base,
                                     hwaddr initrd_size)
 {
     int ret = -1;
-    uint64_t mem_reg_property[] = { 0, cpu_to_be64(params->ram_size) };
+    uint64_t mem_reg_property[] = { 0, cpu_to_be64(args->ram_size) };
     int fdt_size;
     void *fdt;
     uint8_t hypercall[16];
@@ -205,7 +206,7 @@ static int ppce500_load_device_tree(CPUPPCState *env,
     }
 
     ret = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
-                                      params->kernel_cmdline);
+                                      args->kernel_cmdline);
     if (ret < 0)
         fprintf(stderr, "couldn't set /chosen/bootargs\n");
 
@@ -559,7 +560,7 @@ static qemu_irq *ppce500_init_mpic(PPCE500Params *params, MemoryRegion *ccsr,
     return mpic;
 }
 
-void ppce500_init(PPCE500Params *params)
+void ppce500_init(QEMUMachineInitArgs *args, PPCE500Params *params)
 {
     MemoryRegion *address_space_mem = get_system_memory();
     MemoryRegion *ram = g_new(MemoryRegion, 1);
@@ -584,8 +585,8 @@ void ppce500_init(PPCE500Params *params)
     PPCE500CCSRState *ccsr;
 
     /* Setup CPUs */
-    if (params->cpu_model == NULL) {
-        params->cpu_model = "e500v2_v30";
+    if (args->cpu_model == NULL) {
+        args->cpu_model = "e500v2_v30";
     }
 
     irqs = g_malloc0(smp_cpus * sizeof(qemu_irq *));
@@ -595,7 +596,7 @@ void ppce500_init(PPCE500Params *params)
         CPUState *cs;
         qemu_irq *input;
 
-        cpu = cpu_ppc_init(params->cpu_model);
+        cpu = cpu_ppc_init(args->cpu_model);
         if (cpu == NULL) {
             fprintf(stderr, "Unable to initialize CPU!\n");
             exit(1);
@@ -634,7 +635,7 @@ void ppce500_init(PPCE500Params *params)
 
     /* Fixup Memory size on a alignment boundary */
     ram_size &= ~(RAM_SIZES_ALIGN - 1);
-    params->ram_size = ram_size;
+    args->ram_size = ram_size;
 
     /* Register Memory */
     memory_region_init_ram(ram, NULL, "mpc8544ds.ram", ram_size);
@@ -701,11 +702,11 @@ void ppce500_init(PPCE500Params *params)
     sysbus_create_simple("e500-spin", MPC8544_SPIN_BASE, NULL);
 
     /* Load kernel. */
-    if (params->kernel_filename) {
-        kernel_size = load_uimage(params->kernel_filename, &entry,
+    if (args->kernel_filename) {
+        kernel_size = load_uimage(args->kernel_filename, &entry,
                                   &loadaddr, NULL);
         if (kernel_size < 0) {
-            kernel_size = load_elf(params->kernel_filename, NULL, NULL,
+            kernel_size = load_elf(args->kernel_filename, NULL, NULL,
                                    &elf_entry, &elf_lowaddr, NULL, 1,
                                    ELF_MACHINE, 0);
             entry = elf_entry;
@@ -714,7 +715,7 @@ void ppce500_init(PPCE500Params *params)
         /* XXX try again as binary */
         if (kernel_size < 0) {
             fprintf(stderr, "qemu: could not load kernel '%s'\n",
-                    params->kernel_filename);
+                    args->kernel_filename);
             exit(1);
         }
 
@@ -726,14 +727,14 @@ void ppce500_init(PPCE500Params *params)
     }
 
     /* Load initrd. */
-    if (params->initrd_filename) {
+    if (args->initrd_filename) {
         initrd_base = (cur_base + INITRD_LOAD_PAD) & ~INITRD_PAD_MASK;
-        initrd_size = load_image_targphys(params->initrd_filename, initrd_base,
+        initrd_size = load_image_targphys(args->initrd_filename, initrd_base,
                                           ram_size - initrd_base);
 
         if (initrd_size < 0) {
             fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
-                    params->initrd_filename);
+                    args->initrd_filename);
             exit(1);
         }
 
@@ -741,12 +742,12 @@ void ppce500_init(PPCE500Params *params)
     }
 
     /* If we're loading a kernel directly, we must load the device tree too. */
-    if (params->kernel_filename) {
+    if (args->kernel_filename) {
         struct boot_info *boot_info;
         int dt_size;
 
-        dt_size = ppce500_load_device_tree(env, params, dt_base, initrd_base,
-                                           initrd_size);
+        dt_size = ppce500_load_device_tree(env, args, params, dt_base,
+                                           initrd_base, initrd_size);
         if (dt_size < 0) {
             fprintf(stderr, "couldn't load device tree\n");
             exit(1);
diff --git a/hw/ppc/e500.h b/hw/ppc/e500.h
index 226c93d..52726a2 100644
--- a/hw/ppc/e500.h
+++ b/hw/ppc/e500.h
@@ -1,25 +1,18 @@
 #ifndef PPCE500_H
 #define PPCE500_H
 
+#include "hw/boards.h"
+
 typedef struct PPCE500Params {
-    /* Standard QEMU machine init params */
-    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_first_slot;
     int pci_nr_slots;
 
-    /* e500-specific params */
-
     /* required -- must at least add toplevel board compatible */
     void (*fixup_devtree)(struct PPCE500Params *params, void *fdt);
 
     int mpic_version;
 } PPCE500Params;
 
-void ppce500_init(PPCE500Params *params);
+void ppce500_init(QEMUMachineInitArgs *args, PPCE500Params *params);
 
 #endif
diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
index a78de07..bf65b69 100644
--- a/hw/ppc/e500plat.c
+++ b/hw/ppc/e500plat.c
@@ -31,12 +31,6 @@ static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt)
 static void e500plat_init(QEMUMachineInitArgs *args)
 {
     PPCE500Params params = {
-        .ram_size = args->ram_size,
-        .boot_device = args->boot_device,
-        .kernel_filename = args->kernel_filename,
-        .kernel_cmdline = args->kernel_cmdline,
-        .initrd_filename = args->initrd_filename,
-        .cpu_model = args->cpu_model,
         .pci_first_slot = 0x1,
         .pci_nr_slots = PCI_SLOT_MAX - 1,
         .fixup_devtree = e500plat_fixup_devtree,
@@ -49,7 +43,7 @@ static void e500plat_init(QEMUMachineInitArgs *args)
         params.mpic_version = OPENPIC_MODEL_FSL_MPIC_20;
     }
 
-    ppce500_init(&params);
+    ppce500_init(args, &params);
 }
 
 static QEMUMachine e500plat_machine = {
diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
index 4e551af..1888e75 100644
--- a/hw/ppc/mpc8544ds.c
+++ b/hw/ppc/mpc8544ds.c
@@ -29,19 +29,13 @@ static void mpc8544ds_fixup_devtree(PPCE500Params *params, void *fdt)
 static void mpc8544ds_init(QEMUMachineInitArgs *args)
 {
     PPCE500Params params = {
-        .ram_size = args->ram_size,
-        .boot_device = args->boot_device,
-        .kernel_filename = args->kernel_filename,
-        .kernel_cmdline = args->kernel_cmdline,
-        .initrd_filename = args->initrd_filename,
-        .cpu_model = args->cpu_model,
         .pci_first_slot = 0x11,
         .pci_nr_slots = 2,
         .fixup_devtree = mpc8544ds_fixup_devtree,
         .mpic_version = OPENPIC_MODEL_FSL_MPIC_20,
     };
 
-    ppce500_init(&params);
+    ppce500_init(args, &params);
 }
 
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH v4 6/6] hw: Clean up bogus default boot order
  2013-08-16 11:13 [Qemu-devel] [PATCH v4 0/6] Clean up bogus default boot order armbru
                   ` (4 preceding siblings ...)
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 5/6] ppc: Don't duplicate QEMUMachineInitArgs in PPCE500Params armbru
@ 2013-08-16 11:13 ` armbru
  2013-08-17 11:59   ` Laszlo Ersek
  2013-08-21 20:28 ` [Qemu-devel] [PATCH v4 0/6] " Michael S. Tsirkin
  6 siblings, 1 reply; 23+ messages in thread
From: armbru @ 2013-08-16 11:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, mjt, agraf, blauwirbel, qemu-ppc, aviksil

From: Markus Armbruster <armbru@redhat.com>

We set default boot order "cad" in every single machine definition
except "pseries" and "moxiesim", even though very few boards actually
care for boot order, and "cad" makes sense for even fewer.

Machines that care:

* pc and its variants

  Accept up to three letters 'a', 'b' (undocumented alias for 'a'),
  'c', 'd' and 'n'.  Reject all others (fatal with -boot).

* nseries (n800, n810)

  Check whether order starts with 'n'.  Silently ignored otherwise.

* prep, g3beige, mac99

  Extract the first character the machine understands (subset of
  'a'..'f').  Silently ignored otherwise.

* spapr

  Accept an arbitrary string (vl.c restricts it to contain only
  'a'..'p', no duplicates).

* sun4[mdc]

  Use the first character.  Silently ignored otherwise.

Strip characters these machines ignore from their default boot order.

For all other machines, remove the unused default boot order
alltogether.

Note that my rename of QEMUMachine member boot_order to
default_boot_order and QEMUMachineInitArgs member boot_device to
boot_order has a welcome side effect: it makes every use of boot
orders visible in this patch, for easy review.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/alpha/dp264.c                         |  1 -
 hw/arm/collie.c                          |  1 -
 hw/arm/exynos4_boards.c                  |  2 --
 hw/arm/gumstix.c                         |  2 --
 hw/arm/highbank.c                        |  2 --
 hw/arm/integratorcp.c                    |  1 -
 hw/arm/kzm.c                             |  1 -
 hw/arm/mainstone.c                       |  1 -
 hw/arm/musicpal.c                        |  1 -
 hw/arm/nseries.c                         |  6 +++---
 hw/arm/omap_sx1.c                        |  2 --
 hw/arm/palm.c                            |  1 -
 hw/arm/realview.c                        |  4 ----
 hw/arm/spitz.c                           |  4 ----
 hw/arm/stellaris.c                       |  2 --
 hw/arm/tosa.c                            |  1 -
 hw/arm/versatilepb.c                     |  2 --
 hw/arm/vexpress.c                        |  2 --
 hw/arm/xilinx_zynq.c                     |  1 -
 hw/arm/z2.c                              |  1 -
 hw/core/null-machine.c                   |  1 -
 hw/cris/axis_dev88.c                     |  1 -
 hw/i386/pc_piix.c                        | 32 ++++++++++++++++----------------
 hw/i386/pc_q35.c                         |  8 ++++----
 hw/i386/xen_machine_pv.c                 |  1 -
 hw/lm32/lm32_boards.c                    |  2 --
 hw/lm32/milkymist.c                      |  1 -
 hw/m68k/an5206.c                         |  1 -
 hw/m68k/dummy_m68k.c                     |  1 -
 hw/m68k/mcf5208.c                        |  1 -
 hw/microblaze/petalogix_ml605_mmu.c      |  1 -
 hw/microblaze/petalogix_s3adsp1800_mmu.c |  1 -
 hw/mips/mips_fulong2e.c                  |  1 -
 hw/mips/mips_jazz.c                      |  2 --
 hw/mips/mips_malta.c                     |  1 -
 hw/mips/mips_mipssim.c                   |  1 -
 hw/mips/mips_r4k.c                       |  1 -
 hw/openrisc/openrisc_sim.c               |  1 -
 hw/ppc/e500plat.c                        |  1 -
 hw/ppc/mac_newworld.c                    |  4 ++--
 hw/ppc/mac_oldworld.c                    |  4 ++--
 hw/ppc/mpc8544ds.c                       |  1 -
 hw/ppc/ppc405_boards.c                   |  2 --
 hw/ppc/ppc440_bamboo.c                   |  1 -
 hw/ppc/prep.c                            |  4 ++--
 hw/ppc/spapr.c                           |  4 ++--
 hw/ppc/virtex_ml507.c                    |  1 -
 hw/s390x/s390-virtio-ccw.c               |  1 -
 hw/s390x/s390-virtio.c                   |  1 -
 hw/sh4/r2d.c                             |  1 -
 hw/sh4/shix.c                            |  1 -
 hw/sparc/leon3.c                         |  1 -
 hw/sparc/sun4m.c                         | 22 +++++++++++-----------
 hw/sparc64/sun4u.c                       | 10 +++++-----
 hw/unicore32/puv3.c                      |  1 -
 hw/xtensa/xtensa_lx60.c                  |  2 --
 hw/xtensa/xtensa_sim.c                   |  1 -
 include/hw/boards.h                      |  7 ++-----
 vl.c                                     |  4 ++--
 59 files changed, 51 insertions(+), 119 deletions(-)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 95fde61..20795ac 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -173,7 +173,6 @@ static QEMUMachine clipper_machine = {
     .init = clipper_init,
     .max_cpus = 4,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void clipper_machine_init(void)
diff --git a/hw/arm/collie.c b/hw/arm/collie.c
index a19857a..8878b0e 100644
--- a/hw/arm/collie.c
+++ b/hw/arm/collie.c
@@ -62,7 +62,6 @@ static QEMUMachine collie_machine = {
     .name = "collie",
     .desc = "Collie PDA (SA-1110)",
     .init = collie_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void collie_machine_init(void)
diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
index 7c90b2d..2929f9f 100644
--- a/hw/arm/exynos4_boards.c
+++ b/hw/arm/exynos4_boards.c
@@ -150,14 +150,12 @@ static QEMUMachine exynos4_machines[EXYNOS4_NUM_OF_BOARDS] = {
         .desc = "Samsung NURI board (Exynos4210)",
         .init = nuri_init,
         .max_cpus = EXYNOS4210_NCPUS,
-        DEFAULT_MACHINE_OPTIONS,
     },
     [EXYNOS4_BOARD_SMDKC210] = {
         .name = "smdkc210",
         .desc = "Samsung SMDKC210 board (Exynos4210)",
         .init = smdkc210_init,
         .max_cpus = EXYNOS4210_NCPUS,
-        DEFAULT_MACHINE_OPTIONS,
     },
 };
 
diff --git a/hw/arm/gumstix.c b/hw/arm/gumstix.c
index b8cab10..e97fbbd 100644
--- a/hw/arm/gumstix.c
+++ b/hw/arm/gumstix.c
@@ -122,14 +122,12 @@ static QEMUMachine connex_machine = {
     .name = "connex",
     .desc = "Gumstix Connex (PXA255)",
     .init = connex_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine verdex_machine = {
     .name = "verdex",
     .desc = "Gumstix Verdex (PXA270)",
     .init = verdex_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void gumstix_machine_init(void)
diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index 35d5511..0e1b90f 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -365,7 +365,6 @@ static QEMUMachine highbank_machine = {
     .init = highbank_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine midway_machine = {
@@ -374,7 +373,6 @@ static QEMUMachine midway_machine = {
     .init = midway_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void calxeda_machines_init(void)
diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index d518188..90c499d 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -527,7 +527,6 @@ static QEMUMachine integratorcp_machine = {
     .desc = "ARM Integrator/CP (ARM926EJ-S)",
     .init = integratorcp_init,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void integratorcp_machine_init(void)
diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
index bd6c05c..0007923 100644
--- a/hw/arm/kzm.c
+++ b/hw/arm/kzm.c
@@ -146,7 +146,6 @@ static QEMUMachine kzm_machine = {
     .name = "kzm",
     .desc = "ARM KZM Emulation Baseboard (ARM1136)",
     .init = kzm_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void kzm_machine_init(void)
diff --git a/hw/arm/mainstone.c b/hw/arm/mainstone.c
index 8e5fc26..b244f7e 100644
--- a/hw/arm/mainstone.c
+++ b/hw/arm/mainstone.c
@@ -179,7 +179,6 @@ static QEMUMachine mainstone2_machine = {
     .name = "mainstone",
     .desc = "Mainstone II (PXA27x)",
     .init = mainstone_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void mainstone_machine_init(void)
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index d715143..786d82c 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -1731,7 +1731,6 @@ static QEMUMachine musicpal_machine = {
     .name = "musicpal",
     .desc = "Marvell 88w8618 / MusicPal (ARM926EJ-S)",
     .init = musicpal_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void musicpal_machine_init(void)
diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index f6c9dc0..9ef31ca 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -1340,7 +1340,7 @@ static void n8x0_init(QEMUMachineInitArgs *args,
     }
 
     if (option_rom[0].name &&
-        (args->boot_device[0] == 'n' || !args->kernel_filename)) {
+        (args->boot_order[0] == 'n' || !args->kernel_filename)) {
         uint8_t nolo_tags[0x10000];
         /* No, wait, better start at the ROM.  */
         s->mpu->cpu->env.regs[15] = OMAP2_Q2_BASE + 0x400000;
@@ -1396,14 +1396,14 @@ static QEMUMachine n800_machine = {
     .name = "n800",
     .desc = "Nokia N800 tablet aka. RX-34 (OMAP2420)",
     .init = n800_init,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "",
 };
 
 static QEMUMachine n810_machine = {
     .name = "n810",
     .desc = "Nokia N810 tablet aka. RX-44 (OMAP2420)",
     .init = n810_init,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "",
 };
 
 static void nseries_machine_init(void)
diff --git a/hw/arm/omap_sx1.c b/hw/arm/omap_sx1.c
index 05b0353..b0f8664 100644
--- a/hw/arm/omap_sx1.c
+++ b/hw/arm/omap_sx1.c
@@ -219,14 +219,12 @@ static QEMUMachine sx1_machine_v2 = {
     .name = "sx1",
     .desc = "Siemens SX1 (OMAP310) V2",
     .init = sx1_init_v2,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine sx1_machine_v1 = {
     .name = "sx1-v1",
     .desc = "Siemens SX1 (OMAP310) V1",
     .init = sx1_init_v1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void sx1_machine_init(void)
diff --git a/hw/arm/palm.c b/hw/arm/palm.c
index cdc3c3a..3e39044 100644
--- a/hw/arm/palm.c
+++ b/hw/arm/palm.c
@@ -273,7 +273,6 @@ static QEMUMachine palmte_machine = {
     .name = "cheetah",
     .desc = "Palm Tungsten|E aka. Cheetah PDA (OMAP310)",
     .init = palmte_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void palmte_machine_init(void)
diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index 3060f48..1f4e5b0 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -371,7 +371,6 @@ static QEMUMachine realview_eb_machine = {
     .desc = "ARM RealView Emulation Baseboard (ARM926EJ-S)",
     .init = realview_eb_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine realview_eb_mpcore_machine = {
@@ -380,14 +379,12 @@ static QEMUMachine realview_eb_mpcore_machine = {
     .init = realview_eb_mpcore_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine realview_pb_a8_machine = {
     .name = "realview-pb-a8",
     .desc = "ARM RealView Platform Baseboard for Cortex-A8",
     .init = realview_pb_a8_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine realview_pbx_a9_machine = {
@@ -396,7 +393,6 @@ static QEMUMachine realview_pbx_a9_machine = {
     .init = realview_pbx_a9_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void realview_machine_init(void)
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 34f9582..a532cf6 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -966,28 +966,24 @@ static QEMUMachine akitapda_machine = {
     .name = "akita",
     .desc = "Akita PDA (PXA270)",
     .init = akita_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine spitzpda_machine = {
     .name = "spitz",
     .desc = "Spitz PDA (PXA270)",
     .init = spitz_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine borzoipda_machine = {
     .name = "borzoi",
     .desc = "Borzoi PDA (PXA270)",
     .init = borzoi_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine terrierpda_machine = {
     .name = "terrier",
     .desc = "Terrier PDA (PXA270)",
     .init = terrier_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void spitz_machine_init(void)
diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index 79f6b4e..49be8fd 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -1348,14 +1348,12 @@ static QEMUMachine lm3s811evb_machine = {
     .name = "lm3s811evb",
     .desc = "Stellaris LM3S811EVB",
     .init = lm3s811evb_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine lm3s6965evb_machine = {
     .name = "lm3s6965evb",
     .desc = "Stellaris LM3S6965EVB",
     .init = lm3s6965evb_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void stellaris_machine_init(void)
diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c
index 47d1f4f..c00d8c2 100644
--- a/hw/arm/tosa.c
+++ b/hw/arm/tosa.c
@@ -251,7 +251,6 @@ static QEMUMachine tosapda_machine = {
     .name = "tosa",
     .desc = "Tosa PDA (PXA255)",
     .init = tosa_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void tosapda_machine_init(void)
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index b48d84c..3c0eb85 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -368,7 +368,6 @@ static QEMUMachine versatilepb_machine = {
     .desc = "ARM Versatile/PB (ARM926EJ-S)",
     .init = vpb_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine versatileab_machine = {
@@ -376,7 +375,6 @@ static QEMUMachine versatileab_machine = {
     .desc = "ARM Versatile/AB (ARM926EJ-S)",
     .init = vab_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void versatile_machine_init(void)
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 9586e38..71cfe1f 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -651,7 +651,6 @@ static QEMUMachine vexpress_a9_machine = {
     .init = vexpress_a9_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine vexpress_a15_machine = {
@@ -660,7 +659,6 @@ static QEMUMachine vexpress_a15_machine = {
     .init = vexpress_a15_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void vexpress_machine_init(void)
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 3444823..608686a 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -236,7 +236,6 @@ static QEMUMachine zynq_machine = {
     .block_default_type = IF_SCSI,
     .max_cpus = 1,
     .no_sdcard = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void zynq_machine_init(void)
diff --git a/hw/arm/z2.c b/hw/arm/z2.c
index 07a127b..2e0d5d4 100644
--- a/hw/arm/z2.c
+++ b/hw/arm/z2.c
@@ -373,7 +373,6 @@ static QEMUMachine z2_machine = {
     .name = "z2",
     .desc = "Zipit Z2 (PXA27x)",
     .init = z2_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void z2_machine_init(void)
diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c
index bdf109f..d813c08 100644
--- a/hw/core/null-machine.c
+++ b/hw/core/null-machine.c
@@ -24,7 +24,6 @@ static QEMUMachine machine_none = {
     .desc = "empty machine",
     .init = machine_none_init,
     .max_cpus = 0,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void register_machines(void)
diff --git a/hw/cris/axis_dev88.c b/hw/cris/axis_dev88.c
index 9104d61..03058d3 100644
--- a/hw/cris/axis_dev88.c
+++ b/hw/cris/axis_dev88.c
@@ -355,7 +355,6 @@ static QEMUMachine axisdev88_machine = {
     .desc = "AXIS devboard 88",
     .init = axisdev88_init,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void axisdev88_machine_init(void)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 2ae7f95..09e5d38 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -203,7 +203,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
         }
     }
 
-    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
+    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_order,
                  floppy, idebus[0], idebus[1], rtc_state);
 
     if (pci_enabled && usb_enabled(false)) {
@@ -316,7 +316,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
     .hot_add_cpu = pc_hot_add_cpu,
     .max_cpus = 255,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "cad",
 };
 
 static QEMUMachine pc_i440fx_machine_v1_5 = {
@@ -329,7 +329,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
         PC_COMPAT_1_5,
         { /* end of list */ }
     },
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "cad",
 };
 
 static QEMUMachine pc_i440fx_machine_v1_4 = {
@@ -337,11 +337,11 @@ static QEMUMachine pc_i440fx_machine_v1_4 = {
     .desc = "Standard PC (i440FX + PIIX, 1996)",
     .init = pc_init_pci_1_4,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_4,
         { /* end of list */ }
     },
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #define PC_COMPAT_1_3 \
@@ -369,11 +369,11 @@ static QEMUMachine pc_machine_v1_3 = {
     .desc = "Standard PC",
     .init = pc_init_pci_1_3,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_3,
         { /* end of list */ }
     },
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #define PC_COMPAT_1_2 \
@@ -409,11 +409,11 @@ static QEMUMachine pc_machine_v1_2 = {
     .desc = "Standard PC",
     .init = pc_init_pci_1_2,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_2,
         { /* end of list */ }
     },
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #define PC_COMPAT_1_1 \
@@ -453,11 +453,11 @@ static QEMUMachine pc_machine_v1_1 = {
     .desc = "Standard PC",
     .init = pc_init_pci_1_2,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_1,
         { /* end of list */ }
     },
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #define PC_COMPAT_1_0 \
@@ -485,12 +485,12 @@ static QEMUMachine pc_machine_v1_0 = {
     .desc = "Standard PC",
     .init = pc_init_pci_1_0,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_0,
         { /* end of list */ }
     },
     .hw_version = "1.0",
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #define PC_COMPAT_0_15 \
@@ -501,12 +501,12 @@ static QEMUMachine pc_machine_v0_15 = {
     .desc = "Standard PC",
     .init = pc_init_pci_1_0,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_15,
         { /* end of list */ }
     },
     .hw_version = "0.15",
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #define PC_COMPAT_0_14 \
@@ -534,6 +534,7 @@ static QEMUMachine pc_machine_v0_14 = {
     .desc = "Standard PC",
     .init = pc_init_pci_1_0,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_14, 
         {
@@ -548,7 +549,6 @@ static QEMUMachine pc_machine_v0_14 = {
         { /* end of list */ }
     },
     .hw_version = "0.14",
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #define PC_COMPAT_0_13 \
@@ -568,6 +568,7 @@ static QEMUMachine pc_machine_v0_13 = {
     .desc = "Standard PC",
     .init = pc_init_pci_no_kvmclock,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_13,
         {
@@ -586,7 +587,6 @@ static QEMUMachine pc_machine_v0_13 = {
         { /* end of list */ }
     },
     .hw_version = "0.13",
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #define PC_COMPAT_0_12 \
@@ -618,6 +618,7 @@ static QEMUMachine pc_machine_v0_12 = {
     .desc = "Standard PC",
     .init = pc_init_pci_no_kvmclock,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_12,
         {
@@ -632,7 +633,6 @@ static QEMUMachine pc_machine_v0_12 = {
         { /* end of list */ }
     },
     .hw_version = "0.12",
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #define PC_COMPAT_0_11 \
@@ -652,6 +652,7 @@ static QEMUMachine pc_machine_v0_11 = {
     .desc = "Standard PC, qemu 0.11",
     .init = pc_init_pci_no_kvmclock,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_11,
         {
@@ -666,7 +667,6 @@ static QEMUMachine pc_machine_v0_11 = {
         { /* end of list */ }
     },
     .hw_version = "0.11",
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine pc_machine_v0_10 = {
@@ -674,6 +674,7 @@ static QEMUMachine pc_machine_v0_10 = {
     .desc = "Standard PC, qemu 0.10",
     .init = pc_init_pci_no_kvmclock,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_11,
         {
@@ -700,7 +701,6 @@ static QEMUMachine pc_machine_v0_10 = {
         { /* end of list */ }
     },
     .hw_version = "0.10",
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine isapc_machine = {
@@ -708,10 +708,10 @@ static QEMUMachine isapc_machine = {
     .desc = "ISA-only PC",
     .init = pc_init_isa,
     .max_cpus = 1,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         { /* end of list */ }
     },
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #ifdef CONFIG_XEN
@@ -721,7 +721,7 @@ static QEMUMachine xenfv_machine = {
     .init = pc_xen_hvm_init,
     .max_cpus = HVM_MAX_VCPUS,
     .default_machine_opts = "accel=xen",
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "cad",
 };
 #endif
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index f636c65..39423e0 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -199,7 +199,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
                                     0xb100),
                       8, NULL, 0);
 
-    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
+    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_order,
                  floppy, idebus[0], idebus[1], rtc_state);
 
     /* the rest devices to which pci devfn is automatically assigned */
@@ -241,7 +241,7 @@ static QEMUMachine pc_q35_machine_v1_6 = {
     .init = pc_q35_init_1_6,
     .hot_add_cpu = pc_hot_add_cpu,
     .max_cpus = 255,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "cad",
 };
 
 static QEMUMachine pc_q35_machine_v1_5 = {
@@ -250,11 +250,11 @@ static QEMUMachine pc_q35_machine_v1_5 = {
     .init = pc_q35_init_1_5,
     .hot_add_cpu = pc_hot_add_cpu,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_5,
         { /* end of list */ }
     },
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine pc_q35_machine_v1_4 = {
@@ -262,11 +262,11 @@ static QEMUMachine pc_q35_machine_v1_4 = {
     .desc = "Standard PC (Q35 + ICH9, 2009)",
     .init = pc_q35_init_1_4,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_4,
         { /* end of list */ }
     },
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void pc_q35_machine_init(void)
diff --git a/hw/i386/xen_machine_pv.c b/hw/i386/xen_machine_pv.c
index 9f2e291..9adb57f 100644
--- a/hw/i386/xen_machine_pv.c
+++ b/hw/i386/xen_machine_pv.c
@@ -99,7 +99,6 @@ static QEMUMachine xenpv_machine = {
     .init = xen_init_pv,
     .max_cpus = 1,
     .default_machine_opts = "accel=xen",
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void xenpv_machine_init(void)
diff --git a/hw/lm32/lm32_boards.c b/hw/lm32/lm32_boards.c
index 62003b8..c032bb8 100644
--- a/hw/lm32/lm32_boards.c
+++ b/hw/lm32/lm32_boards.c
@@ -289,7 +289,6 @@ static QEMUMachine lm32_evr_machine = {
     .desc = "LatticeMico32 EVR32 eval system",
     .init = lm32_evr_init,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine lm32_uclinux_machine = {
@@ -297,7 +296,6 @@ static QEMUMachine lm32_uclinux_machine = {
     .desc = "lm32 platform for uClinux and u-boot by Theobroma Systems",
     .init = lm32_uclinux_init,
     .is_default = 0,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void lm32_machine_init(void)
diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
index 7ceedb8..f1744ec 100644
--- a/hw/lm32/milkymist.c
+++ b/hw/lm32/milkymist.c
@@ -208,7 +208,6 @@ static QEMUMachine milkymist_machine = {
     .desc = "Milkymist One",
     .init = milkymist_init,
     .is_default = 0,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void milkymist_machine_init(void)
diff --git a/hw/m68k/an5206.c b/hw/m68k/an5206.c
index 0c03a87..a8eee44 100644
--- a/hw/m68k/an5206.c
+++ b/hw/m68k/an5206.c
@@ -89,7 +89,6 @@ static QEMUMachine an5206_machine = {
     .name = "an5206",
     .desc = "Arnewsh 5206",
     .init = an5206_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void an5206_machine_init(void)
diff --git a/hw/m68k/dummy_m68k.c b/hw/m68k/dummy_m68k.c
index f4ed7c6..86e2e6e 100644
--- a/hw/m68k/dummy_m68k.c
+++ b/hw/m68k/dummy_m68k.c
@@ -73,7 +73,6 @@ static QEMUMachine dummy_m68k_machine = {
     .name = "dummy",
     .desc = "Dummy board",
     .init = dummy_m68k_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void dummy_m68k_machine_init(void)
diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
index 9cf000f..fb96fe8 100644
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -295,7 +295,6 @@ static QEMUMachine mcf5208evb_machine = {
     .desc = "MCF5206EVB",
     .init = mcf5208evb_init,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void mcf5208evb_machine_init(void)
diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
index 989da25..e003c7c 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -186,7 +186,6 @@ static QEMUMachine petalogix_ml605_machine = {
     .desc = "PetaLogix linux refdesign for xilinx ml605 little endian",
     .init = petalogix_ml605_init,
     .is_default = 0,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void petalogix_ml605_machine_init(void)
diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index a461494..00af2b5 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -116,7 +116,6 @@ static QEMUMachine petalogix_s3adsp1800_machine = {
     .desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800",
     .init = petalogix_s3adsp1800_init,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void petalogix_s3adsp1800_machine_init(void)
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index e8d5dd0..9ef3a97 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -403,7 +403,6 @@ static QEMUMachine mips_fulong2e_machine = {
     .name = "fulong2e",
     .desc = "Fulong 2e mini pc",
     .init = mips_fulong2e_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void mips_fulong2e_machine_init(void)
diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index d748ded..49bdd02 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -327,7 +327,6 @@ static QEMUMachine mips_magnum_machine = {
     .desc = "MIPS Magnum",
     .init = mips_magnum_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine mips_pica61_machine = {
@@ -335,7 +334,6 @@ static QEMUMachine mips_pica61_machine = {
     .desc = "Acer Pica 61",
     .init = mips_pica61_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void mips_jazz_machine_init(void)
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index f8d064c..ae0921c 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1136,7 +1136,6 @@ static QEMUMachine mips_malta_machine = {
     .init = mips_malta_init,
     .max_cpus = 16,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void mips_malta_register_types(void)
diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
index 297f01e..242bab9 100644
--- a/hw/mips/mips_mipssim.c
+++ b/hw/mips/mips_mipssim.c
@@ -232,7 +232,6 @@ static QEMUMachine mips_mipssim_machine = {
     .name = "mipssim",
     .desc = "MIPS MIPSsim platform",
     .init = mips_mipssim_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void mips_mipssim_machine_init(void)
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index 044f232..e94b543 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -306,7 +306,6 @@ static QEMUMachine mips_machine = {
     .name = "mips",
     .desc = "mips r4k platform",
     .init = mips_r4k_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void mips_machine_init(void)
diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index a08f27c..2a2d077 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -139,7 +139,6 @@ static QEMUMachine openrisc_sim_machine = {
     .init = openrisc_sim_init,
     .max_cpus = 1,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void openrisc_sim_machine_init(void)
diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
index bf65b69..2e964b2 100644
--- a/hw/ppc/e500plat.c
+++ b/hw/ppc/e500plat.c
@@ -51,7 +51,6 @@ static QEMUMachine e500plat_machine = {
     .desc = "generic paravirt e500 platform",
     .init = e500plat_init,
     .max_cpus = 32,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void e500plat_machine_init(void)
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 7ef806e..5e79575 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -147,7 +147,7 @@ static void ppc_core99_init(QEMUMachineInitArgs *args)
     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;
+    const char *boot_device = args->boot_order;
     PowerPCCPU *cpu = NULL;
     CPUPPCState *env = NULL;
     char *filename;
@@ -477,7 +477,7 @@ static QEMUMachine core99_machine = {
     .desc = "Mac99 based PowerMAC",
     .init = ppc_core99_init,
     .max_cpus = MAX_CPUS,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "cd",
 };
 
 static void core99_machine_init(void)
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 42bb9d5..2f27754 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -78,7 +78,7 @@ static void ppc_heathrow_init(QEMUMachineInitArgs *args)
     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;
+    const char *boot_device = args->boot_order;
     MemoryRegion *sysmem = get_system_memory();
     PowerPCCPU *cpu = NULL;
     CPUPPCState *env = NULL;
@@ -350,7 +350,7 @@ static QEMUMachine heathrow_machine = {
 #ifndef TARGET_PPC64
     .is_default = 1,
 #endif
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "cd", /* TOFIX "cad" when Mac floppy is implemented */
 };
 
 static void heathrow_machine_init(void)
diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
index 1888e75..edcc0be 100644
--- a/hw/ppc/mpc8544ds.c
+++ b/hw/ppc/mpc8544ds.c
@@ -44,7 +44,6 @@ static QEMUMachine ppce500_machine = {
     .desc = "mpc8544ds",
     .init = mpc8544ds_init,
     .max_cpus = 15,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void ppce500_machine_init(void)
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index f74e5e5..43a83ca 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -362,7 +362,6 @@ static QEMUMachine ref405ep_machine = {
     .name = "ref405ep",
     .desc = "ref405ep",
     .init = ref405ep_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 /*****************************************************************************/
@@ -650,7 +649,6 @@ static QEMUMachine taihu_machine = {
     .name = "taihu",
     .desc = "taihu",
     .init = taihu_405ep_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void ppc405_machine_init(void)
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index 369ab9e..655e499 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -296,7 +296,6 @@ static QEMUMachine bamboo_machine = {
     .name = "bamboo",
     .desc = "bamboo",
     .init = bamboo_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void bamboo_machine_init(void)
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 7e04b1a..aad0f69 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -452,7 +452,7 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
     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;
+    const char *boot_device = args->boot_order;
     MemoryRegion *sysmem = get_system_memory();
     PowerPCCPU *cpu = NULL;
     CPUPPCState *env = NULL;
@@ -691,7 +691,7 @@ static QEMUMachine prep_machine = {
     .desc = "PowerPC PREP platform",
     .init = ppc_prep_init,
     .max_cpus = MAX_CPUS,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "cad",
 };
 
 static void prep_machine_init(void)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 16bfab9..3a2b381 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1071,7 +1071,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
     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;
+    const char *boot_device = args->boot_order;
     PowerPCCPU *cpu;
     CPUPPCState *env;
     PCIHostState *phb;
@@ -1325,7 +1325,7 @@ static QEMUMachine spapr_machine = {
     .block_default_type = IF_SCSI,
     .max_cpus = MAX_CPUS,
     .no_parallel = 1,
-    .boot_order = NULL,
+    .default_boot_order = NULL,
 };
 
 static void spapr_machine_init(void)
diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index 08e77fb..7250f51 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -245,7 +245,6 @@ static QEMUMachine virtex_machine = {
     .name = "virtex-ml507",
     .desc = "Xilinx Virtex ML507 reference design",
     .init = virtex_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void virtex_machine_init(void)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index aebbbf1..e2681a6 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -126,7 +126,6 @@ static QEMUMachine ccw_machine = {
     .no_sdcard = 1,
     .use_sclp = 1,
     .max_cpus = 255,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void ccw_machine_init(void)
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 439d732..7adf92a 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -293,7 +293,6 @@ static QEMUMachine s390_machine = {
     .use_virtcon = 1,
     .max_cpus = 255,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void s390_machine_init(void)
diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
index 98b3408..7b1de85 100644
--- a/hw/sh4/r2d.c
+++ b/hw/sh4/r2d.c
@@ -356,7 +356,6 @@ static QEMUMachine r2d_machine = {
     .name = "r2d",
     .desc = "r2d-plus board",
     .init = r2d_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void r2d_machine_init(void)
diff --git a/hw/sh4/shix.c b/hw/sh4/shix.c
index 84dd666..1ff37f5 100644
--- a/hw/sh4/shix.c
+++ b/hw/sh4/shix.c
@@ -96,7 +96,6 @@ static QEMUMachine shix_machine = {
     .desc = "shix card",
     .init = shix_init,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void shix_machine_init(void)
diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index 5ef282f..390f3e4 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -216,7 +216,6 @@ static QEMUMachine leon3_generic_machine = {
     .name     = "leon3_generic",
     .desc     = "Leon-3 generic",
     .init     = leon3_generic_hw_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void leon3_machine_init(void)
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 36ef36f..a0d366c 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -996,7 +996,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
                                     args->ram_size);
 
     nvram_init(nvram, (uint8_t *)&nd_table[0].macaddr, args->kernel_cmdline,
-               args->boot_device, args->ram_size, kernel_size, graphic_width,
+               args->boot_order, args->ram_size, kernel_size, graphic_width,
                graphic_height, graphic_depth, hwdef->nvram_machine_id,
                "Sun4m");
 
@@ -1027,7 +1027,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     }
     fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, INITRD_LOAD_ADDR);
     fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, 0); // not used
-    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, args->boot_device[0]);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, args->boot_order[0]);
     qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
 }
 
@@ -1348,7 +1348,7 @@ static QEMUMachine ss5_machine = {
     .init = ss5_init,
     .block_default_type = IF_SCSI,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine ss10_machine = {
@@ -1357,7 +1357,7 @@ static QEMUMachine ss10_machine = {
     .init = ss10_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine ss600mp_machine = {
@@ -1366,7 +1366,7 @@ static QEMUMachine ss600mp_machine = {
     .init = ss600mp_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine ss20_machine = {
@@ -1375,7 +1375,7 @@ static QEMUMachine ss20_machine = {
     .init = ss20_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine voyager_machine = {
@@ -1383,7 +1383,7 @@ static QEMUMachine voyager_machine = {
     .desc = "Sun4m platform, SPARCstation Voyager",
     .init = vger_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine ss_lx_machine = {
@@ -1391,7 +1391,7 @@ static QEMUMachine ss_lx_machine = {
     .desc = "Sun4m platform, SPARCstation LX",
     .init = ss_lx_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine ss4_machine = {
@@ -1399,7 +1399,7 @@ static QEMUMachine ss4_machine = {
     .desc = "Sun4m platform, SPARCstation 4",
     .init = ss4_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine scls_machine = {
@@ -1407,7 +1407,7 @@ static QEMUMachine scls_machine = {
     .desc = "Sun4m platform, SPARCClassic",
     .init = scls_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine sbook_machine = {
@@ -1415,7 +1415,7 @@ static QEMUMachine sbook_machine = {
     .desc = "Sun4m platform, SPARCbook",
     .init = sbook_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static void sun4m_register_types(void)
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 8bd7fb9..e7a4893 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -881,7 +881,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
                                     &kernel_addr, &kernel_entry);
 
     sun4u_NVRAM_set_params(nvram, NVRAM_SIZE, "Sun4u", args->ram_size,
-                           args->boot_device,
+                           args->boot_order,
                            kernel_addr, kernel_size,
                            args->kernel_cmdline,
                            initrd_addr, initrd_size,
@@ -906,7 +906,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     }
     fw_cfg_add_i64(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr);
     fw_cfg_add_i64(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, args->boot_device[0]);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, args->boot_order[0]);
 
     fw_cfg_add_i16(fw_cfg, FW_CFG_SPARC64_WIDTH, graphic_width);
     fw_cfg_add_i16(fw_cfg, FW_CFG_SPARC64_HEIGHT, graphic_height);
@@ -969,7 +969,7 @@ static QEMUMachine sun4u_machine = {
     .init = sun4u_init,
     .max_cpus = 1, // XXX for now
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine sun4v_machine = {
@@ -977,7 +977,7 @@ static QEMUMachine sun4v_machine = {
     .desc = "Sun4v platform",
     .init = sun4v_init,
     .max_cpus = 1, // XXX for now
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine niagara_machine = {
@@ -985,7 +985,7 @@ static QEMUMachine niagara_machine = {
     .desc = "Sun4v platform, Niagara",
     .init = niagara_init,
     .max_cpus = 1, // XXX for now
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static void sun4u_register_types(void)
diff --git a/hw/unicore32/puv3.c b/hw/unicore32/puv3.c
index 5ff0dc9..a900061 100644
--- a/hw/unicore32/puv3.c
+++ b/hw/unicore32/puv3.c
@@ -128,7 +128,6 @@ static QEMUMachine puv3_machine = {
     .desc = "PKUnity Version-3 based on UniCore32",
     .init = puv3_init,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void puv3_machine_init(void)
diff --git a/hw/xtensa/xtensa_lx60.c b/hw/xtensa/xtensa_lx60.c
index 1138666..22e124d 100644
--- a/hw/xtensa/xtensa_lx60.c
+++ b/hw/xtensa/xtensa_lx60.c
@@ -297,7 +297,6 @@ static QEMUMachine xtensa_lx60_machine = {
     .desc = "lx60 EVB (" XTENSA_DEFAULT_CPU_MODEL ")",
     .init = xtensa_lx60_init,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine xtensa_lx200_machine = {
@@ -305,7 +304,6 @@ static QEMUMachine xtensa_lx200_machine = {
     .desc = "lx200 EVB (" XTENSA_DEFAULT_CPU_MODEL ")",
     .init = xtensa_lx200_init,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void xtensa_lx_machines_init(void)
diff --git a/hw/xtensa/xtensa_sim.c b/hw/xtensa/xtensa_sim.c
index ea91162..1192ce7 100644
--- a/hw/xtensa/xtensa_sim.c
+++ b/hw/xtensa/xtensa_sim.c
@@ -108,7 +108,6 @@ static QEMUMachine xtensa_sim_machine = {
     .is_default = true,
     .init = xtensa_sim_init,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void xtensa_sim_machine_init(void)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index fb7c6f1..5a7ae9f 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -6,12 +6,9 @@
 #include "sysemu/blockdev.h"
 #include "hw/qdev.h"
 
-#define DEFAULT_MACHINE_OPTIONS \
-    .boot_order = "cad"
-
 typedef struct QEMUMachineInitArgs {
     ram_addr_t ram_size;
-    const char *boot_device;
+    const char *boot_order;
     const char *kernel_filename;
     const char *kernel_cmdline;
     const char *initrd_filename;
@@ -42,7 +39,7 @@ typedef struct QEMUMachine {
         no_sdcard:1;
     int is_default;
     const char *default_machine_opts;
-    const char *boot_order;
+    const char *default_boot_order;
     GlobalProperty *compat_props;
     struct QEMUMachine *next;
     const char *hw_version;
diff --git a/vl.c b/vl.c
index f422a1c..2624c0f 100644
--- a/vl.c
+++ b/vl.c
@@ -4121,7 +4121,7 @@ int main(int argc, char **argv, char **envp)
     kernel_cmdline = qemu_opt_get(machine_opts, "append");
 
     if (!boot_order) {
-        boot_order = machine->boot_order;
+        boot_order = machine->default_boot_order;
     }
     opts = qemu_opts_find(qemu_find_opts("boot-opts"), NULL);
     if (opts) {
@@ -4309,7 +4309,7 @@ int main(int argc, char **argv, char **envp)
     qdev_machine_init();
 
     QEMUMachineInitArgs args = { .ram_size = ram_size,
-                                 .boot_device = boot_order,
+                                 .boot_order = boot_order,
                                  .kernel_filename = kernel_filename,
                                  .kernel_cmdline = kernel_cmdline,
                                  .initrd_filename = initrd_filename,
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v4 1/6] pc: Don't prematurely explode QEMUMachineInitArgs
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 1/6] pc: Don't prematurely explode QEMUMachineInitArgs armbru
@ 2013-08-17 10:07   ` Laszlo Ersek
  2013-08-19  9:09     ` Markus Armbruster
  2013-08-21 18:05   ` Eduardo Habkost
  1 sibling, 1 reply; 23+ messages in thread
From: Laszlo Ersek @ 2013-08-17 10:07 UTC (permalink / raw)
  To: armbru; +Cc: aliguori, mjt, qemu-devel, agraf, blauwirbel, qemu-ppc, aviksil

On 08/16/13 13:13, armbru@redhat.com wrote:

>  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";
> +    }

This modifies *args.

IIUC, args here points to the "args" auto variable in main().
"args.cpu_model" is initialized from the standalone "cpu_model"
variable. That one in turn is either NULL, or points to a command line
argument. Ie. "args.cpu_model" is never expected to be freed, and the
underlying char array is not expected to be modified. OK.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/6] pc: Don't explode QEMUMachineInitArgs into local variables needlessly
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 2/6] pc: Don't explode QEMUMachineInitArgs into local variables needlessly armbru
@ 2013-08-17 10:12   ` Laszlo Ersek
  2013-08-21 18:06   ` Eduardo Habkost
  1 sibling, 0 replies; 23+ messages in thread
From: Laszlo Ersek @ 2013-08-17 10:12 UTC (permalink / raw)
  To: armbru; +Cc: aliguori, mjt, qemu-devel, agraf, blauwirbel, qemu-ppc, aviksil

On 08/16/13 13:13, armbru@redhat.com wrote:
> 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>
> ---
>  hw/i386/pc_q35.c | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v4 3/6] sun4: Don't prematurely explode QEMUMachineInitArgs
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 3/6] sun4: Don't prematurely explode QEMUMachineInitArgs armbru
@ 2013-08-17 10:29   ` Laszlo Ersek
  2013-08-19  9:15     ` Markus Armbruster
  0 siblings, 1 reply; 23+ messages in thread
From: Laszlo Ersek @ 2013-08-17 10:29 UTC (permalink / raw)
  To: armbru; +Cc: aliguori, mjt, qemu-devel, agraf, blauwirbel, qemu-ppc, aviksil

comments below

On 08/16/13 13:13, armbru@redhat.com wrote:
> From: Markus Armbruster <armbru@redhat.com>
> 
> Don't explode QEMUMachineInitArgs before passing it to
> sun4m_hw_init(), sun4uv_init().
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/sparc/sun4m.c   | 113 ++++++++++++-----------------------------------------
>  hw/sparc64/sun4u.c |  52 +++++++-----------------
>  2 files changed, 40 insertions(+), 125 deletions(-)
> 
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 942ca37..36ef36f 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -836,12 +836,10 @@ static void dummy_fdc_tc(void *opaque, int irq, int level)
>  {
>  }
>  
> -static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, 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)
> +static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
> +                          QEMUMachineInitArgs *args)
>  {
> +    const char *cpu_model = args->cpu_model;

This may not be strictly necessary; in the first patch you modify
args->cpu_model too.

In any case the above is not wrong.

> @@ -878,13 +875,15 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>  
>      initrd_size = 0;
>      initrd_addr = 0;
> -    kernel_size = sun4u_load_kernel(kernel_filename, initrd_filename,
> +    kernel_size = sun4u_load_kernel(args->kernel_filename,
> +                                    args->initrd_filename,
>                                      ram_size, &initrd_size, &initrd_addr,
>                                      &kernel_addr, &kernel_entry);

The patch is correct / faithful here, but I smell some fubar in the code
that the patch keeps intact.

"ram_size" is apparently the global variable from "vl.c". So this
function gets the RAM size twice, once via RAM_size / args->ram_size,
then via the "ram_size" global. (They should have identical values,
"args.ram_size" in main() is initialized with "ram_size" the global.)

The rest of the code below this hunk (in the full source file, not just
in the patch) alternates between RAM_size / args->ram_size and
"ram_size" quite schizophrenically too; see eg. FW_CFG_RAM_SIZE.

Anyway the patch only improves things.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v4 4/6] ppc: Don't explode QEMUMachineInitArgs into local variables needlessly
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 4/6] ppc: Don't explode QEMUMachineInitArgs into local variables needlessly armbru
@ 2013-08-17 10:33   ` Laszlo Ersek
  0 siblings, 0 replies; 23+ messages in thread
From: Laszlo Ersek @ 2013-08-17 10:33 UTC (permalink / raw)
  To: armbru; +Cc: aliguori, mjt, qemu-devel, agraf, blauwirbel, qemu-ppc, aviksil

On 08/16/13 13:13, armbru@redhat.com wrote:
> From: Markus Armbruster <armbru@redhat.com>
> 
> Don't explode when the variable is used just once, and never changed.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Acked-by: Alexander Graf <agraf@suse.de>
> ---
>  hw/ppc/e500plat.c  | 18 ++++++------------
>  hw/ppc/mpc8544ds.c | 18 ++++++------------
>  2 files changed, 12 insertions(+), 24 deletions(-)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v4 5/6] ppc: Don't duplicate QEMUMachineInitArgs in PPCE500Params
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 5/6] ppc: Don't duplicate QEMUMachineInitArgs in PPCE500Params armbru
@ 2013-08-17 10:46   ` Laszlo Ersek
  2013-08-19  9:24     ` Markus Armbruster
  0 siblings, 1 reply; 23+ messages in thread
From: Laszlo Ersek @ 2013-08-17 10:46 UTC (permalink / raw)
  To: armbru; +Cc: aliguori, mjt, qemu-devel, agraf, blauwirbel, qemu-ppc, aviksil

comments below

On 08/16/13 13:13, armbru@redhat.com wrote:
> From: Markus Armbruster <armbru@redhat.com>
> 
> Pass on the generic arguments unadulterated, and the machine-specific
> ones as separate argument.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Acked-by: Alexander Graf <agraf@suse.de>
> ---
>  hw/ppc/e500.c      | 35 ++++++++++++++++++-----------------
>  hw/ppc/e500.h      | 13 +++----------
>  hw/ppc/e500plat.c  |  8 +-------
>  hw/ppc/mpc8544ds.c |  8 +-------
>  4 files changed, 23 insertions(+), 41 deletions(-)

Please always use

  -O/path/to/order_file

when invoking git-format-patch.

The contents of "order_file" should be minimally

  configure
  Makefile*
  *.json
  *.h
  *.c

It's much easier to review a patch when "declarative changes" are shown
first (ie. in approximate logical dependency order).

Then,

> -void ppce500_init(PPCE500Params *params)
> +void ppce500_init(QEMUMachineInitArgs *args, PPCE500Params *params)
>  {
>      MemoryRegion *address_space_mem = get_system_memory();
>      MemoryRegion *ram = g_new(MemoryRegion, 1);
> @@ -584,8 +585,8 @@ void ppce500_init(PPCE500Params *params)
>      PPCE500CCSRState *ccsr;
>  
>      /* Setup CPUs */
> -    if (params->cpu_model == NULL) {
> -        params->cpu_model = "e500v2_v30";
> +    if (args->cpu_model == NULL) {
> +        args->cpu_model = "e500v2_v30";
>      }

As discussed before, this change will modify the "args.cpu_model" member
in main(), but that's OK.


> @@ -634,7 +635,7 @@ void ppce500_init(PPCE500Params *params)
>  
>      /* Fixup Memory size on a alignment boundary */
>      ram_size &= ~(RAM_SIZES_ALIGN - 1);
> -    params->ram_size = ram_size;
> +    args->ram_size = ram_size;

This hackery (commendably left intact by the patch) is convincing me
that QEMUMachineInitArgs should not have a "ram_size" member at all. If
"ram_size" is a well-founded global, then let's treat it as such. Whatever.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v4 6/6] hw: Clean up bogus default boot order
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 6/6] hw: Clean up bogus default boot order armbru
@ 2013-08-17 11:59   ` Laszlo Ersek
  0 siblings, 0 replies; 23+ messages in thread
From: Laszlo Ersek @ 2013-08-17 11:59 UTC (permalink / raw)
  To: armbru; +Cc: aliguori, mjt, qemu-devel, agraf, blauwirbel, qemu-ppc, aviksil

On 08/16/13 13:13, armbru@redhat.com wrote:
> From: Markus Armbruster <armbru@redhat.com>
> 
> We set default boot order "cad" in every single machine definition
> except "pseries" and "moxiesim", even though very few boards actually
> care for boot order, and "cad" makes sense for even fewer.
> 
> Machines that care:
> 
> * pc and its variants
> 
>   Accept up to three letters 'a', 'b' (undocumented alias for 'a'),
>   'c', 'd' and 'n'.  Reject all others (fatal with -boot).
> 
> * nseries (n800, n810)
> 
>   Check whether order starts with 'n'.  Silently ignored otherwise.
> 
> * prep, g3beige, mac99
> 
>   Extract the first character the machine understands (subset of
>   'a'..'f').  Silently ignored otherwise.
> 
> * spapr
> 
>   Accept an arbitrary string (vl.c restricts it to contain only
>   'a'..'p', no duplicates).
> 
> * sun4[mdc]
> 
>   Use the first character.  Silently ignored otherwise.
> 
> Strip characters these machines ignore from their default boot order.
> 
> For all other machines, remove the unused default boot order
> alltogether.
> 
> Note that my rename of QEMUMachine member boot_order to
> default_boot_order and QEMUMachineInitArgs member boot_device to
> boot_order has a welcome side effect: it makes every use of boot
> orders visible in this patch, for easy review.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/alpha/dp264.c                         |  1 -
>  hw/arm/collie.c                          |  1 -
>  hw/arm/exynos4_boards.c                  |  2 --
>  hw/arm/gumstix.c                         |  2 --
>  hw/arm/highbank.c                        |  2 --
>  hw/arm/integratorcp.c                    |  1 -
>  hw/arm/kzm.c                             |  1 -
>  hw/arm/mainstone.c                       |  1 -
>  hw/arm/musicpal.c                        |  1 -
>  hw/arm/nseries.c                         |  6 +++---
>  hw/arm/omap_sx1.c                        |  2 --
>  hw/arm/palm.c                            |  1 -
>  hw/arm/realview.c                        |  4 ----
>  hw/arm/spitz.c                           |  4 ----
>  hw/arm/stellaris.c                       |  2 --
>  hw/arm/tosa.c                            |  1 -
>  hw/arm/versatilepb.c                     |  2 --
>  hw/arm/vexpress.c                        |  2 --
>  hw/arm/xilinx_zynq.c                     |  1 -
>  hw/arm/z2.c                              |  1 -
>  hw/core/null-machine.c                   |  1 -
>  hw/cris/axis_dev88.c                     |  1 -
>  hw/i386/pc_piix.c                        | 32 ++++++++++++++++----------------
>  hw/i386/pc_q35.c                         |  8 ++++----
>  hw/i386/xen_machine_pv.c                 |  1 -
>  hw/lm32/lm32_boards.c                    |  2 --
>  hw/lm32/milkymist.c                      |  1 -
>  hw/m68k/an5206.c                         |  1 -
>  hw/m68k/dummy_m68k.c                     |  1 -
>  hw/m68k/mcf5208.c                        |  1 -
>  hw/microblaze/petalogix_ml605_mmu.c      |  1 -
>  hw/microblaze/petalogix_s3adsp1800_mmu.c |  1 -
>  hw/mips/mips_fulong2e.c                  |  1 -
>  hw/mips/mips_jazz.c                      |  2 --
>  hw/mips/mips_malta.c                     |  1 -
>  hw/mips/mips_mipssim.c                   |  1 -
>  hw/mips/mips_r4k.c                       |  1 -
>  hw/openrisc/openrisc_sim.c               |  1 -
>  hw/ppc/e500plat.c                        |  1 -
>  hw/ppc/mac_newworld.c                    |  4 ++--
>  hw/ppc/mac_oldworld.c                    |  4 ++--
>  hw/ppc/mpc8544ds.c                       |  1 -
>  hw/ppc/ppc405_boards.c                   |  2 --
>  hw/ppc/ppc440_bamboo.c                   |  1 -
>  hw/ppc/prep.c                            |  4 ++--
>  hw/ppc/spapr.c                           |  4 ++--
>  hw/ppc/virtex_ml507.c                    |  1 -
>  hw/s390x/s390-virtio-ccw.c               |  1 -
>  hw/s390x/s390-virtio.c                   |  1 -
>  hw/sh4/r2d.c                             |  1 -
>  hw/sh4/shix.c                            |  1 -
>  hw/sparc/leon3.c                         |  1 -
>  hw/sparc/sun4m.c                         | 22 +++++++++++-----------
>  hw/sparc64/sun4u.c                       | 10 +++++-----
>  hw/unicore32/puv3.c                      |  1 -
>  hw/xtensa/xtensa_lx60.c                  |  2 --
>  hw/xtensa/xtensa_sim.c                   |  1 -
>  include/hw/boards.h                      |  7 ++-----
>  vl.c                                     |  4 ++--
>  59 files changed, 51 insertions(+), 119 deletions(-)

This patch wasn't easy to verify. I hope I haven't missed anything.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v4 1/6] pc: Don't prematurely explode QEMUMachineInitArgs
  2013-08-17 10:07   ` Laszlo Ersek
@ 2013-08-19  9:09     ` Markus Armbruster
  0 siblings, 0 replies; 23+ messages in thread
From: Markus Armbruster @ 2013-08-19  9:09 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: aliguori, mjt, qemu-devel, agraf, blauwirbel, qemu-ppc, aviksil

Laszlo Ersek <lersek@redhat.com> writes:

> On 08/16/13 13:13, armbru@redhat.com wrote:
>
>>  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";
>> +    }
>
> This modifies *args.

Precedence:

db4ff6f hw/realview.c: Don't prematurely explode QEMUMachineInitArgs
1b523b5 hw/versatilepb: Don't prematurely explode QEMUMachineInitArgs

> IIUC, args here points to the "args" auto variable in main().
> "args.cpu_model" is initialized from the standalone "cpu_model"
> variable. That one in turn is either NULL, or points to a command line
> argument. Ie. "args.cpu_model" is never expected to be freed, and the
> underlying char array is not expected to be modified. OK.
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v4 3/6] sun4: Don't prematurely explode QEMUMachineInitArgs
  2013-08-17 10:29   ` Laszlo Ersek
@ 2013-08-19  9:15     ` Markus Armbruster
  0 siblings, 0 replies; 23+ messages in thread
From: Markus Armbruster @ 2013-08-19  9:15 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: aliguori, mjt, qemu-devel, agraf, blauwirbel, qemu-ppc, aviksil

Laszlo Ersek <lersek@redhat.com> writes:

> comments below
>
> On 08/16/13 13:13, armbru@redhat.com wrote:
>> From: Markus Armbruster <armbru@redhat.com>
>> 
>> Don't explode QEMUMachineInitArgs before passing it to
>> sun4m_hw_init(), sun4uv_init().
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  hw/sparc/sun4m.c | 113
>> ++++++++++++-----------------------------------------
>>  hw/sparc64/sun4u.c |  52 +++++++-----------------
>>  2 files changed, 40 insertions(+), 125 deletions(-)
>> 
>> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
>> index 942ca37..36ef36f 100644
>> --- a/hw/sparc/sun4m.c
>> +++ b/hw/sparc/sun4m.c
>> @@ -836,12 +836,10 @@ static void dummy_fdc_tc(void *opaque, int
>> irq, int level)
>>  {
>>  }
>>  
>> -static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>> 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)
>> +static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>> +                          QEMUMachineInitArgs *args)
>>  {
>> +    const char *cpu_model = args->cpu_model;
>
> This may not be strictly necessary; in the first patch you modify
> args->cpu_model too.

Yes.

> In any case the above is not wrong.
>
>> @@ -878,13 +875,15 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>>  
>>      initrd_size = 0;
>>      initrd_addr = 0;
>> -    kernel_size = sun4u_load_kernel(kernel_filename, initrd_filename,
>> +    kernel_size = sun4u_load_kernel(args->kernel_filename,
>> +                                    args->initrd_filename,
>>                                      ram_size, &initrd_size, &initrd_addr,
>>                                      &kernel_addr, &kernel_entry);
>
> The patch is correct / faithful here, but I smell some fubar in the code
> that the patch keeps intact.
>
> "ram_size" is apparently the global variable from "vl.c". So this
> function gets the RAM size twice, once via RAM_size / args->ram_size,
> then via the "ram_size" global. (They should have identical values,
> "args.ram_size" in main() is initialized with "ram_size" the global.)
>
> The rest of the code below this hunk (in the full source file, not just
> in the patch) alternates between RAM_size / args->ram_size and
> "ram_size" quite schizophrenically too; see eg. FW_CFG_RAM_SIZE.

Correct.  Could be cleaned up on top.

> Anyway the patch only improves things.
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v4 5/6] ppc: Don't duplicate QEMUMachineInitArgs in PPCE500Params
  2013-08-17 10:46   ` Laszlo Ersek
@ 2013-08-19  9:24     ` Markus Armbruster
  2013-08-19  9:36       ` Laszlo Ersek
  0 siblings, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2013-08-19  9:24 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: aliguori, mjt, qemu-devel, agraf, blauwirbel, qemu-ppc, aviksil

Laszlo Ersek <lersek@redhat.com> writes:

> comments below
>
> On 08/16/13 13:13, armbru@redhat.com wrote:
>> From: Markus Armbruster <armbru@redhat.com>
>> 
>> Pass on the generic arguments unadulterated, and the machine-specific
>> ones as separate argument.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> Acked-by: Alexander Graf <agraf@suse.de>
>> ---
>>  hw/ppc/e500.c      | 35 ++++++++++++++++++-----------------
>>  hw/ppc/e500.h      | 13 +++----------
>>  hw/ppc/e500plat.c  |  8 +-------
>>  hw/ppc/mpc8544ds.c |  8 +-------
>>  4 files changed, 23 insertions(+), 41 deletions(-)
>
> Please always use
>
>   -O/path/to/order_file
>
> when invoking git-format-patch.
>
> The contents of "order_file" should be minimally
>
>   configure
>   Makefile*
>   *.json
>   *.h
>   *.c
>
> It's much easier to review a patch when "declarative changes" are shown
> first (ie. in approximate logical dependency order).

Is there a way to put this in .git/config?

Should http://wiki.qemu.org/Contribute/SubmitAPatch ask for this?

> Then,
>
>> -void ppce500_init(PPCE500Params *params)
>> +void ppce500_init(QEMUMachineInitArgs *args, PPCE500Params *params)
>>  {
>>      MemoryRegion *address_space_mem = get_system_memory();
>>      MemoryRegion *ram = g_new(MemoryRegion, 1);
>> @@ -584,8 +585,8 @@ void ppce500_init(PPCE500Params *params)
>>      PPCE500CCSRState *ccsr;
>>  
>>      /* Setup CPUs */
>> -    if (params->cpu_model == NULL) {
>> -        params->cpu_model = "e500v2_v30";
>> +    if (args->cpu_model == NULL) {
>> +        args->cpu_model = "e500v2_v30";
>>      }
>
> As discussed before, this change will modify the "args.cpu_model" member
> in main(), but that's OK.
>
>
>> @@ -634,7 +635,7 @@ void ppce500_init(PPCE500Params *params)
>>  
>>      /* Fixup Memory size on a alignment boundary */
>>      ram_size &= ~(RAM_SIZES_ALIGN - 1);
>> -    params->ram_size = ram_size;
>> +    args->ram_size = ram_size;
>
> This hackery (commendably left intact by the patch) is convincing me
> that QEMUMachineInitArgs should not have a "ram_size" member at all. If
> "ram_size" is a well-founded global, then let's treat it as such. Whatever.

Global variables are often bad style (there are exceptions).  Even worse
is passing what is essentially global state down call chains while
keeping the global variables around for random poking.  And that's what
we tend to do %-/

> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v4 5/6] ppc: Don't duplicate QEMUMachineInitArgs in PPCE500Params
  2013-08-19  9:24     ` Markus Armbruster
@ 2013-08-19  9:36       ` Laszlo Ersek
  0 siblings, 0 replies; 23+ messages in thread
From: Laszlo Ersek @ 2013-08-19  9:36 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: aliguori, mjt, qemu-devel, agraf, blauwirbel, qemu-ppc, aviksil

On 08/19/13 11:24, Markus Armbruster wrote:
> Laszlo Ersek <lersek@redhat.com> writes:

>> Please always use
>>
>>   -O/path/to/order_file
>>
>> when invoking git-format-patch.
>>
>> The contents of "order_file" should be minimally
>>
>>   configure
>>   Makefile*
>>   *.json
>>   *.h
>>   *.c
>>
>> It's much easier to review a patch when "declarative changes" are shown
>> first (ie. in approximate logical dependency order).
> 
> Is there a way to put this in .git/config?

The only way we've found thus far is to introduce a new alias for
git-format-patch that hardwires it.

> Should http://wiki.qemu.org/Contribute/SubmitAPatch ask for this?

Not a bad idea, but I'm afraid new contributors don't read it because
they don't know about it, and veteran contributors don't read it because
they don't need it.

The wiki would need a usable table of contents anyway, I have great
trouble every time I want to find anything. Wikipedia probably should
not have a flat sitemap for its millions of articles. The qemu wiki
should, for its handful.

Laszlo

> 
>> Then,
>>
>>> -void ppce500_init(PPCE500Params *params)
>>> +void ppce500_init(QEMUMachineInitArgs *args, PPCE500Params *params)
>>>  {
>>>      MemoryRegion *address_space_mem = get_system_memory();
>>>      MemoryRegion *ram = g_new(MemoryRegion, 1);
>>> @@ -584,8 +585,8 @@ void ppce500_init(PPCE500Params *params)
>>>      PPCE500CCSRState *ccsr;
>>>  
>>>      /* Setup CPUs */
>>> -    if (params->cpu_model == NULL) {
>>> -        params->cpu_model = "e500v2_v30";
>>> +    if (args->cpu_model == NULL) {
>>> +        args->cpu_model = "e500v2_v30";
>>>      }
>>
>> As discussed before, this change will modify the "args.cpu_model" member
>> in main(), but that's OK.
>>
>>
>>> @@ -634,7 +635,7 @@ void ppce500_init(PPCE500Params *params)
>>>  
>>>      /* Fixup Memory size on a alignment boundary */
>>>      ram_size &= ~(RAM_SIZES_ALIGN - 1);
>>> -    params->ram_size = ram_size;
>>> +    args->ram_size = ram_size;
>>
>> This hackery (commendably left intact by the patch) is convincing me
>> that QEMUMachineInitArgs should not have a "ram_size" member at all. If
>> "ram_size" is a well-founded global, then let's treat it as such. Whatever.
> 
> Global variables are often bad style (there are exceptions).  Even worse
> is passing what is essentially global state down call chains while
> keeping the global variables around for random poking.  And that's what
> we tend to do %-/
> 
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 
> Thanks!
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v4 1/6] pc: Don't prematurely explode QEMUMachineInitArgs
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 1/6] pc: Don't prematurely explode QEMUMachineInitArgs armbru
  2013-08-17 10:07   ` Laszlo Ersek
@ 2013-08-21 18:05   ` Eduardo Habkost
  1 sibling, 0 replies; 23+ messages in thread
From: Eduardo Habkost @ 2013-08-21 18:05 UTC (permalink / raw)
  To: armbru; +Cc: aliguori, mjt, qemu-devel, agraf, blauwirbel, qemu-ppc, aviksil

On Fri, Aug 16, 2013 at 01:13:45PM +0200, armbru@redhat.com wrote:
> 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: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/6] pc: Don't explode QEMUMachineInitArgs into local variables needlessly
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 2/6] pc: Don't explode QEMUMachineInitArgs into local variables needlessly armbru
  2013-08-17 10:12   ` Laszlo Ersek
@ 2013-08-21 18:06   ` Eduardo Habkost
  1 sibling, 0 replies; 23+ messages in thread
From: Eduardo Habkost @ 2013-08-21 18:06 UTC (permalink / raw)
  To: armbru; +Cc: aliguori, mjt, qemu-devel, agraf, blauwirbel, qemu-ppc, aviksil

On Fri, Aug 16, 2013 at 01:13:46PM +0200, armbru@redhat.com wrote:
> 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: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v4 0/6] Clean up bogus default boot order
  2013-08-16 11:13 [Qemu-devel] [PATCH v4 0/6] Clean up bogus default boot order armbru
                   ` (5 preceding siblings ...)
  2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 6/6] hw: Clean up bogus default boot order armbru
@ 2013-08-21 20:28 ` Michael S. Tsirkin
  2013-08-21 20:29   ` Michael S. Tsirkin
  2013-08-23 12:31   ` Markus Armbruster
  6 siblings, 2 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2013-08-21 20:28 UTC (permalink / raw)
  To: armbru; +Cc: aliguori, mjt, qemu-devel, agraf, blauwirbel, qemu-ppc, aviksil

On Fri, Aug 16, 2013 at 01:13:44PM +0200, armbru@redhat.com wrote:
> From: Markus Armbruster <armbru@redhat.com>
> 
> The first five patches are admittedly related to the stated purpose of
> this series pretty much only by "I can't stand perpetuating this
> stupid crap".  Max Filippov and Peter Maydell already cleaned up
> Xtensa and ARM the same way.

I picked up patches 3,4 and 5 on my tree.
1 and 2 were rebased by Eduardo, I'm taking them
from his patchset.
6 needs to be rebased and comments addressed.

> v4:
> * Rebase
> v3:
> * Rebase
> * Don't screw up default CPU model for machine "isapc" in 1/6
> v2:
> * Straightforward rebase
> * PATCH 6 improved commit message (Alex)
> 
> Markus Armbruster (6):
>   pc: Don't prematurely explode QEMUMachineInitArgs
>   pc: Don't explode QEMUMachineInitArgs into local variables needlessly
>   sun4: Don't prematurely explode QEMUMachineInitArgs
>   ppc: Don't explode QEMUMachineInitArgs into local variables needlessly
>   ppc: Don't duplicate QEMUMachineInitArgs in PPCE500Params
>   hw: Clean up bogus default boot order
> 
>  hw/alpha/dp264.c                         |   1 -
>  hw/arm/collie.c                          |   1 -
>  hw/arm/exynos4_boards.c                  |   2 -
>  hw/arm/gumstix.c                         |   2 -
>  hw/arm/highbank.c                        |   2 -
>  hw/arm/integratorcp.c                    |   1 -
>  hw/arm/kzm.c                             |   1 -
>  hw/arm/mainstone.c                       |   1 -
>  hw/arm/musicpal.c                        |   1 -
>  hw/arm/nseries.c                         |   6 +-
>  hw/arm/omap_sx1.c                        |   2 -
>  hw/arm/palm.c                            |   1 -
>  hw/arm/realview.c                        |   4 -
>  hw/arm/spitz.c                           |   4 -
>  hw/arm/stellaris.c                       |   2 -
>  hw/arm/tosa.c                            |   1 -
>  hw/arm/versatilepb.c                     |   2 -
>  hw/arm/vexpress.c                        |   2 -
>  hw/arm/xilinx_zynq.c                     |   1 -
>  hw/arm/z2.c                              |   1 -
>  hw/core/null-machine.c                   |   1 -
>  hw/cris/axis_dev88.c                     |   1 -
>  hw/i386/pc_piix.c                        |  95 ++++++++--------------
>  hw/i386/pc_q35.c                         |  28 +++----
>  hw/i386/xen_machine_pv.c                 |   1 -
>  hw/lm32/lm32_boards.c                    |   2 -
>  hw/lm32/milkymist.c                      |   1 -
>  hw/m68k/an5206.c                         |   1 -
>  hw/m68k/dummy_m68k.c                     |   1 -
>  hw/m68k/mcf5208.c                        |   1 -
>  hw/microblaze/petalogix_ml605_mmu.c      |   1 -
>  hw/microblaze/petalogix_s3adsp1800_mmu.c |   1 -
>  hw/mips/mips_fulong2e.c                  |   1 -
>  hw/mips/mips_jazz.c                      |   2 -
>  hw/mips/mips_malta.c                     |   1 -
>  hw/mips/mips_mipssim.c                   |   1 -
>  hw/mips/mips_r4k.c                       |   1 -
>  hw/openrisc/openrisc_sim.c               |   1 -
>  hw/ppc/e500.c                            |  35 +++++----
>  hw/ppc/e500.h                            |  13 +--
>  hw/ppc/e500plat.c                        |  15 +---
>  hw/ppc/mac_newworld.c                    |   4 +-
>  hw/ppc/mac_oldworld.c                    |   4 +-
>  hw/ppc/mpc8544ds.c                       |  15 +---
>  hw/ppc/ppc405_boards.c                   |   2 -
>  hw/ppc/ppc440_bamboo.c                   |   1 -
>  hw/ppc/prep.c                            |   4 +-
>  hw/ppc/spapr.c                           |   4 +-
>  hw/ppc/virtex_ml507.c                    |   1 -
>  hw/s390x/s390-virtio-ccw.c               |   1 -
>  hw/s390x/s390-virtio.c                   |   1 -
>  hw/sh4/r2d.c                             |   1 -
>  hw/sh4/shix.c                            |   1 -
>  hw/sparc/leon3.c                         |   1 -
>  hw/sparc/sun4m.c                         | 131 ++++++++-----------------------
>  hw/sparc64/sun4u.c                       |  58 +++++---------
>  hw/unicore32/puv3.c                      |   1 -
>  hw/xtensa/xtensa_lx60.c                  |   2 -
>  hw/xtensa/xtensa_sim.c                   |   1 -
>  include/hw/boards.h                      |   7 +-
>  vl.c                                     |   4 +-
>  61 files changed, 133 insertions(+), 353 deletions(-)
> 
> -- 
> 1.8.1.4
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v4 0/6] Clean up bogus default boot order
  2013-08-21 20:28 ` [Qemu-devel] [PATCH v4 0/6] " Michael S. Tsirkin
@ 2013-08-21 20:29   ` Michael S. Tsirkin
  2013-08-23 12:31   ` Markus Armbruster
  1 sibling, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2013-08-21 20:29 UTC (permalink / raw)
  To: armbru; +Cc: aliguori, mjt, qemu-devel, agraf, blauwirbel, qemu-ppc, aviksil

On Wed, Aug 21, 2013 at 11:28:33PM +0300, Michael S. Tsirkin wrote:
> On Fri, Aug 16, 2013 at 01:13:44PM +0200, armbru@redhat.com wrote:
> > From: Markus Armbruster <armbru@redhat.com>
> > 
> > The first five patches are admittedly related to the stated purpose of
> > this series pretty much only by "I can't stand perpetuating this
> > stupid crap".  Max Filippov and Peter Maydell already cleaned up
> > Xtensa and ARM the same way.
> 
> I picked up patches 3,4 and 5 on my tree.
> 1 and 2 were rebased by Eduardo, I'm taking them
> from his patchset.
> 6 needs to be rebased and comments addressed.

Sent a bit too fast.

Most importantly -

Thanks everyone.

> > v4:
> > * Rebase
> > v3:
> > * Rebase
> > * Don't screw up default CPU model for machine "isapc" in 1/6
> > v2:
> > * Straightforward rebase
> > * PATCH 6 improved commit message (Alex)
> > 
> > Markus Armbruster (6):
> >   pc: Don't prematurely explode QEMUMachineInitArgs
> >   pc: Don't explode QEMUMachineInitArgs into local variables needlessly
> >   sun4: Don't prematurely explode QEMUMachineInitArgs
> >   ppc: Don't explode QEMUMachineInitArgs into local variables needlessly
> >   ppc: Don't duplicate QEMUMachineInitArgs in PPCE500Params
> >   hw: Clean up bogus default boot order
> > 
> >  hw/alpha/dp264.c                         |   1 -
> >  hw/arm/collie.c                          |   1 -
> >  hw/arm/exynos4_boards.c                  |   2 -
> >  hw/arm/gumstix.c                         |   2 -
> >  hw/arm/highbank.c                        |   2 -
> >  hw/arm/integratorcp.c                    |   1 -
> >  hw/arm/kzm.c                             |   1 -
> >  hw/arm/mainstone.c                       |   1 -
> >  hw/arm/musicpal.c                        |   1 -
> >  hw/arm/nseries.c                         |   6 +-
> >  hw/arm/omap_sx1.c                        |   2 -
> >  hw/arm/palm.c                            |   1 -
> >  hw/arm/realview.c                        |   4 -
> >  hw/arm/spitz.c                           |   4 -
> >  hw/arm/stellaris.c                       |   2 -
> >  hw/arm/tosa.c                            |   1 -
> >  hw/arm/versatilepb.c                     |   2 -
> >  hw/arm/vexpress.c                        |   2 -
> >  hw/arm/xilinx_zynq.c                     |   1 -
> >  hw/arm/z2.c                              |   1 -
> >  hw/core/null-machine.c                   |   1 -
> >  hw/cris/axis_dev88.c                     |   1 -
> >  hw/i386/pc_piix.c                        |  95 ++++++++--------------
> >  hw/i386/pc_q35.c                         |  28 +++----
> >  hw/i386/xen_machine_pv.c                 |   1 -
> >  hw/lm32/lm32_boards.c                    |   2 -
> >  hw/lm32/milkymist.c                      |   1 -
> >  hw/m68k/an5206.c                         |   1 -
> >  hw/m68k/dummy_m68k.c                     |   1 -
> >  hw/m68k/mcf5208.c                        |   1 -
> >  hw/microblaze/petalogix_ml605_mmu.c      |   1 -
> >  hw/microblaze/petalogix_s3adsp1800_mmu.c |   1 -
> >  hw/mips/mips_fulong2e.c                  |   1 -
> >  hw/mips/mips_jazz.c                      |   2 -
> >  hw/mips/mips_malta.c                     |   1 -
> >  hw/mips/mips_mipssim.c                   |   1 -
> >  hw/mips/mips_r4k.c                       |   1 -
> >  hw/openrisc/openrisc_sim.c               |   1 -
> >  hw/ppc/e500.c                            |  35 +++++----
> >  hw/ppc/e500.h                            |  13 +--
> >  hw/ppc/e500plat.c                        |  15 +---
> >  hw/ppc/mac_newworld.c                    |   4 +-
> >  hw/ppc/mac_oldworld.c                    |   4 +-
> >  hw/ppc/mpc8544ds.c                       |  15 +---
> >  hw/ppc/ppc405_boards.c                   |   2 -
> >  hw/ppc/ppc440_bamboo.c                   |   1 -
> >  hw/ppc/prep.c                            |   4 +-
> >  hw/ppc/spapr.c                           |   4 +-
> >  hw/ppc/virtex_ml507.c                    |   1 -
> >  hw/s390x/s390-virtio-ccw.c               |   1 -
> >  hw/s390x/s390-virtio.c                   |   1 -
> >  hw/sh4/r2d.c                             |   1 -
> >  hw/sh4/shix.c                            |   1 -
> >  hw/sparc/leon3.c                         |   1 -
> >  hw/sparc/sun4m.c                         | 131 ++++++++-----------------------
> >  hw/sparc64/sun4u.c                       |  58 +++++---------
> >  hw/unicore32/puv3.c                      |   1 -
> >  hw/xtensa/xtensa_lx60.c                  |   2 -
> >  hw/xtensa/xtensa_sim.c                   |   1 -
> >  include/hw/boards.h                      |   7 +-
> >  vl.c                                     |   4 +-
> >  61 files changed, 133 insertions(+), 353 deletions(-)
> > 
> > -- 
> > 1.8.1.4
> > 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v4 0/6] Clean up bogus default boot order
  2013-08-21 20:28 ` [Qemu-devel] [PATCH v4 0/6] " Michael S. Tsirkin
  2013-08-21 20:29   ` Michael S. Tsirkin
@ 2013-08-23 12:31   ` Markus Armbruster
  2013-08-25 11:10     ` Michael S. Tsirkin
  1 sibling, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2013-08-23 12:31 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: aliguori, mjt, qemu-devel, agraf, blauwirbel, qemu-ppc, aviksil

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Fri, Aug 16, 2013 at 01:13:44PM +0200, armbru@redhat.com wrote:
>> From: Markus Armbruster <armbru@redhat.com>
>> 
>> The first five patches are admittedly related to the stated purpose of
>> this series pretty much only by "I can't stand perpetuating this
>> stupid crap".  Max Filippov and Peter Maydell already cleaned up
>> Xtensa and ARM the same way.
>
> I picked up patches 3,4 and 5 on my tree.
> 1 and 2 were rebased by Eduardo, I'm taking them
> from his patchset.
> 6 needs to be rebased and comments addressed.

Applies fine with "git-am -3".  Pushed to
http://repo.or.cz/w/qemu/armbru.git/shortlog/refs/heads/boot-order
for your convenience.

We discussed the patch at some length, but it's not 100% clear to me
what exactly you'd like me to address and how.  So let's recap briefly.

I think your main point was that PC machine type declarations are a bit
repetitive.  They all share two lines:

    .max_cpus = 255,
    DEFAULT_MACHINE_OPTIONS,

where DEFAULT_MACHINE_OPTIONS is defined as

    #define DEFAULT_MACHINE_OPTIONS \
        .boot_order = "cad"

Many of them also share one of these lines:

    .desc = "Standard PC (Q35 + ICH9, 2009)",
    .desc = "Standard PC (i440FX + PIIX, 1996)",
    .desc = "Standard PC",

My patch touches only the shared DEFAULT_MACHINE_OPTIONS line.  It
becomes

   .boot_order = "cad"

Commit message explains why:

    We set default boot order "cad" in every single machine definition
    except "pseries" and "moxiesim", even though very few boards actually
    care for boot order, and "cad" makes sense for even fewer.

    Machines that care:
    
    * pc and its variants
    
      Accept up to three letters 'a', 'b' (undocumented alias for 'a'),
      'c', 'd' and 'n'.  Reject all others (fatal with -boot).

    [...]

    Strip characters these machines ignore from their default boot order.
    
    For all other machines, remove the unused default boot order
    alltogether.

The change is systematic: if the machine uses .boot_order, strip the
characters it ignores from its initial value, else drop the initializer,
so .boot_order remains null.

I don't want to squash further cleanup into this one, because it's hard
enough to review as it is (and it already got competent review).  I
could be persuaded to do further cleanup on top, but you need to tell me
what cleanup you want.  Probably faster if you do it yourself :)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v4 0/6] Clean up bogus default boot order
  2013-08-23 12:31   ` Markus Armbruster
@ 2013-08-25 11:10     ` Michael S. Tsirkin
  0 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2013-08-25 11:10 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: aliguori, mjt, qemu-devel, agraf, blauwirbel, qemu-ppc, aviksil

On Fri, Aug 23, 2013 at 02:31:49PM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Fri, Aug 16, 2013 at 01:13:44PM +0200, armbru@redhat.com wrote:
> >> From: Markus Armbruster <armbru@redhat.com>
> >> 
> >> The first five patches are admittedly related to the stated purpose of
> >> this series pretty much only by "I can't stand perpetuating this
> >> stupid crap".  Max Filippov and Peter Maydell already cleaned up
> >> Xtensa and ARM the same way.
> >
> > I picked up patches 3,4 and 5 on my tree.
> > 1 and 2 were rebased by Eduardo, I'm taking them
> > from his patchset.
> > 6 needs to be rebased and comments addressed.
> 
> Applies fine with "git-am -3".  Pushed to
> http://repo.or.cz/w/qemu/armbru.git/shortlog/refs/heads/boot-order
> for your convenience.
> 
> We discussed the patch at some length, but it's not 100% clear to me
> what exactly you'd like me to address and how.  So let's recap briefly.
> 
> I think your main point was that PC machine type declarations are a bit
> repetitive.  They all share two lines:
> 
>     .max_cpus = 255,
>     DEFAULT_MACHINE_OPTIONS,
> 
> where DEFAULT_MACHINE_OPTIONS is defined as
> 
>     #define DEFAULT_MACHINE_OPTIONS \
>         .boot_order = "cad"
> 
> Many of them also share one of these lines:
> 
>     .desc = "Standard PC (Q35 + ICH9, 2009)",
>     .desc = "Standard PC (i440FX + PIIX, 1996)",
>     .desc = "Standard PC",
> 
> My patch touches only the shared DEFAULT_MACHINE_OPTIONS line.  It
> becomes
> 
>    .boot_order = "cad"
> 
> Commit message explains why:
> 
>     We set default boot order "cad" in every single machine definition
>     except "pseries" and "moxiesim", even though very few boards actually
>     care for boot order, and "cad" makes sense for even fewer.
> 
>     Machines that care:
>     
>     * pc and its variants
>     
>       Accept up to three letters 'a', 'b' (undocumented alias for 'a'),
>       'c', 'd' and 'n'.  Reject all others (fatal with -boot).
> 
>     [...]
> 
>     Strip characters these machines ignore from their default boot order.
>     
>     For all other machines, remove the unused default boot order
>     alltogether.
> 
> The change is systematic: if the machine uses .boot_order, strip the
> characters it ignores from its initial value, else drop the initializer,
> so .boot_order remains null.
> 
> I don't want to squash further cleanup into this one, because it's hard
> enough to review as it is (and it already got competent review).  I
> could be persuaded to do further cleanup on top, but you need to tell me
> what cleanup you want.  Probably faster if you do it yourself :)

Responded in the relevant thread.
Hope this helps.

-- 
MST

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2013-08-25 11:08 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-16 11:13 [Qemu-devel] [PATCH v4 0/6] Clean up bogus default boot order armbru
2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 1/6] pc: Don't prematurely explode QEMUMachineInitArgs armbru
2013-08-17 10:07   ` Laszlo Ersek
2013-08-19  9:09     ` Markus Armbruster
2013-08-21 18:05   ` Eduardo Habkost
2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 2/6] pc: Don't explode QEMUMachineInitArgs into local variables needlessly armbru
2013-08-17 10:12   ` Laszlo Ersek
2013-08-21 18:06   ` Eduardo Habkost
2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 3/6] sun4: Don't prematurely explode QEMUMachineInitArgs armbru
2013-08-17 10:29   ` Laszlo Ersek
2013-08-19  9:15     ` Markus Armbruster
2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 4/6] ppc: Don't explode QEMUMachineInitArgs into local variables needlessly armbru
2013-08-17 10:33   ` Laszlo Ersek
2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 5/6] ppc: Don't duplicate QEMUMachineInitArgs in PPCE500Params armbru
2013-08-17 10:46   ` Laszlo Ersek
2013-08-19  9:24     ` Markus Armbruster
2013-08-19  9:36       ` Laszlo Ersek
2013-08-16 11:13 ` [Qemu-devel] [PATCH v4 6/6] hw: Clean up bogus default boot order armbru
2013-08-17 11:59   ` Laszlo Ersek
2013-08-21 20:28 ` [Qemu-devel] [PATCH v4 0/6] " Michael S. Tsirkin
2013-08-21 20:29   ` Michael S. Tsirkin
2013-08-23 12:31   ` Markus Armbruster
2013-08-25 11:10     ` Michael S. Tsirkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.