All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global.
@ 2012-05-20  9:02 Gleb Natapov
  2012-05-20  9:02 ` [Qemu-devel] [PATCH 2/2] Add PIIX4 properties to control PM system states Gleb Natapov
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Gleb Natapov @ 2012-05-20  9:02 UTC (permalink / raw)
  To: qemu-devel

There can be only one fw_cfg device, so saving global reference to it
removes the need to pass its pointer around.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 hw/fw_cfg.c       |  110 +++++++++++++++++++++++++++--------------------------
 hw/fw_cfg.h       |   15 +++----
 hw/loader.c       |   10 +----
 hw/loader.h       |    1 -
 hw/multiboot.c    |   17 ++++----
 hw/multiboot.h    |    3 +-
 hw/pc.c           |   63 ++++++++++++++----------------
 hw/ppc_newworld.c |   43 ++++++++++-----------
 hw/ppc_oldworld.c |   43 ++++++++++-----------
 hw/sun4m.c        |   93 +++++++++++++++++++++-----------------------
 hw/sun4u.c        |   35 ++++++++---------
 11 files changed, 207 insertions(+), 226 deletions(-)

diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index 7b3b576..8c50473 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -48,7 +48,7 @@ typedef struct FWCfgEntry {
     FWCfgCallback callback;
 } FWCfgEntry;
 
-struct FWCfgState {
+static struct FWCfgState {
     SysBusDevice busdev;
     MemoryRegion ctl_iomem, data_iomem, comb_iomem;
     uint32_t ctl_iobase, data_iobase;
@@ -57,7 +57,7 @@ struct FWCfgState {
     uint16_t cur_entry;
     uint32_t cur_offset;
     Notifier machine_ready;
-};
+} *fw_cfg;
 
 #define JPG_FILE 0
 #define BMP_FILE 1
@@ -113,7 +113,7 @@ error:
     return NULL;
 }
 
-static void fw_cfg_bootsplash(FWCfgState *s)
+static void fw_cfg_bootsplash(void)
 {
     int boot_splash_time = -1;
     const char *boot_splash_filename = NULL;
@@ -148,7 +148,7 @@ static void fw_cfg_bootsplash(FWCfgState *s)
         /* use little endian format */
         qemu_extra_params_fw[0] = (uint8_t)(boot_splash_time & 0xff);
         qemu_extra_params_fw[1] = (uint8_t)((boot_splash_time >> 8) & 0xff);
-        fw_cfg_add_file(s, "etc/boot-menu-wait", qemu_extra_params_fw, 2);
+        fw_cfg_add_file("etc/boot-menu-wait", qemu_extra_params_fw, 2);
     }
 
     /* insert splash file if user configurated */
@@ -173,10 +173,10 @@ static void fw_cfg_bootsplash(FWCfgState *s)
 
         /* insert data */
         if (file_type == JPG_FILE) {
-            fw_cfg_add_file(s, "bootsplash.jpg",
+            fw_cfg_add_file("bootsplash.jpg",
                     boot_splash_filedata, boot_splash_filedata_size);
         } else {
-            fw_cfg_add_file(s, "bootsplash.bmp",
+            fw_cfg_add_file("bootsplash.bmp",
                     boot_splash_filedata, boot_splash_filedata_size);
         }
         g_free(filename);
@@ -359,122 +359,126 @@ static const VMStateDescription vmstate_fw_cfg = {
     }
 };
 
-int fw_cfg_add_bytes(FWCfgState *s, uint16_t key, uint8_t *data, uint32_t len)
+int fw_cfg_add_bytes(uint16_t key, uint8_t *data, uint32_t len)
 {
     int arch = !!(key & FW_CFG_ARCH_LOCAL);
 
     key &= FW_CFG_ENTRY_MASK;
 
-    if (key >= FW_CFG_MAX_ENTRY)
+    if (key >= FW_CFG_MAX_ENTRY || !fw_cfg) {
         return 0;
+    }
 
-    s->entries[arch][key].data = data;
-    s->entries[arch][key].len = len;
+    fw_cfg->entries[arch][key].data = data;
+    fw_cfg->entries[arch][key].len = len;
 
     return 1;
 }
 
-int fw_cfg_add_i16(FWCfgState *s, uint16_t key, uint16_t value)
+int fw_cfg_add_i16(uint16_t key, uint16_t value)
 {
     uint16_t *copy;
 
     copy = g_malloc(sizeof(value));
     *copy = cpu_to_le16(value);
-    return fw_cfg_add_bytes(s, key, (uint8_t *)copy, sizeof(value));
+    return fw_cfg_add_bytes(key, (uint8_t *)copy, sizeof(value));
 }
 
-int fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t value)
+int fw_cfg_add_i32(uint16_t key, uint32_t value)
 {
     uint32_t *copy;
 
     copy = g_malloc(sizeof(value));
     *copy = cpu_to_le32(value);
-    return fw_cfg_add_bytes(s, key, (uint8_t *)copy, sizeof(value));
+    return fw_cfg_add_bytes(key, (uint8_t *)copy, sizeof(value));
 }
 
-int fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t value)
+int fw_cfg_add_i64(uint16_t key, uint64_t value)
 {
     uint64_t *copy;
 
     copy = g_malloc(sizeof(value));
     *copy = cpu_to_le64(value);
-    return fw_cfg_add_bytes(s, key, (uint8_t *)copy, sizeof(value));
+    return fw_cfg_add_bytes(key, (uint8_t *)copy, sizeof(value));
 }
 
-int fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback,
+int fw_cfg_add_callback(uint16_t key, FWCfgCallback callback,
                         void *callback_opaque, uint8_t *data, size_t len)
 {
     int arch = !!(key & FW_CFG_ARCH_LOCAL);
 
-    if (!(key & FW_CFG_WRITE_CHANNEL))
+    if (!(key & FW_CFG_WRITE_CHANNEL) || !fw_cfg) {
         return 0;
+    }
 
     key &= FW_CFG_ENTRY_MASK;
 
     if (key >= FW_CFG_MAX_ENTRY || len > 65535)
         return 0;
 
-    s->entries[arch][key].data = data;
-    s->entries[arch][key].len = len;
-    s->entries[arch][key].callback_opaque = callback_opaque;
-    s->entries[arch][key].callback = callback;
+    fw_cfg->entries[arch][key].data = data;
+    fw_cfg->entries[arch][key].len = len;
+    fw_cfg->entries[arch][key].callback_opaque = callback_opaque;
+    fw_cfg->entries[arch][key].callback = callback;
 
     return 1;
 }
 
-int fw_cfg_add_file(FWCfgState *s,  const char *filename, uint8_t *data,
-                    uint32_t len)
+int fw_cfg_add_file(const char *filename, uint8_t *data, uint32_t len)
 {
     int i, index;
 
-    if (!s->files) {
+    if (!fw_cfg) {
+        return 0;
+    }
+
+    if (!fw_cfg->files) {
         int dsize = sizeof(uint32_t) + sizeof(FWCfgFile) * FW_CFG_FILE_SLOTS;
-        s->files = g_malloc0(dsize);
-        fw_cfg_add_bytes(s, FW_CFG_FILE_DIR, (uint8_t*)s->files, dsize);
+        fw_cfg->files = g_malloc0(dsize);
+        fw_cfg_add_bytes(FW_CFG_FILE_DIR, (uint8_t *)fw_cfg->files, dsize);
     }
 
-    index = be32_to_cpu(s->files->count);
+    index = be32_to_cpu(fw_cfg->files->count);
     if (index == FW_CFG_FILE_SLOTS) {
         fprintf(stderr, "fw_cfg: out of file slots\n");
         return 0;
     }
 
-    fw_cfg_add_bytes(s, FW_CFG_FILE_FIRST + index, data, len);
+    fw_cfg_add_bytes(FW_CFG_FILE_FIRST + index, data, len);
 
-    pstrcpy(s->files->f[index].name, sizeof(s->files->f[index].name),
+    pstrcpy(fw_cfg->files->f[index].name, sizeof(fw_cfg->files->f[index].name),
             filename);
     for (i = 0; i < index; i++) {
-        if (strcmp(s->files->f[index].name, s->files->f[i].name) == 0) {
+        if (strcmp(fw_cfg->files->f[index].name, fw_cfg->files->f[i].name)
+                        == 0) {
             FW_CFG_DPRINTF("%s: skip duplicate: %s\n", __FUNCTION__,
-                           s->files->f[index].name);
+                           fw_cfg->files->f[index].name);
             return 1;
         }
     }
 
-    s->files->f[index].size   = cpu_to_be32(len);
-    s->files->f[index].select = cpu_to_be16(FW_CFG_FILE_FIRST + index);
+    fw_cfg->files->f[index].size   = cpu_to_be32(len);
+    fw_cfg->files->f[index].select = cpu_to_be16(FW_CFG_FILE_FIRST + index);
     FW_CFG_DPRINTF("%s: #%d: %s (%d bytes)\n", __FUNCTION__,
-                   index, s->files->f[index].name, len);
+                   index, fw_cfg->files->f[index].name, len);
 
-    s->files->count = cpu_to_be32(index+1);
+    fw_cfg->files->count = cpu_to_be32(index+1);
     return 1;
 }
 
 static void fw_cfg_machine_ready(struct Notifier *n, void *data)
 {
     uint32_t len;
-    FWCfgState *s = container_of(n, FWCfgState, machine_ready);
     char *bootindex = get_boot_devices_list(&len);
 
-    fw_cfg_add_file(s, "bootorder", (uint8_t*)bootindex, len);
+    fw_cfg_add_file("bootorder", (uint8_t *)bootindex, len);
 }
 
-FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
-                        target_phys_addr_t ctl_addr, target_phys_addr_t data_addr)
+void fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
+                 target_phys_addr_t ctl_addr, target_phys_addr_t data_addr)
 {
     DeviceState *dev;
     SysBusDevice *d;
-    FWCfgState *s;
 
     dev = qdev_create(NULL, "fw_cfg");
     qdev_prop_set_uint32(dev, "ctl_iobase", ctl_port);
@@ -482,7 +486,7 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
     qdev_init_nofail(dev);
     d = sysbus_from_qdev(dev);
 
-    s = DO_UPCAST(FWCfgState, busdev.qdev, dev);
+    fw_cfg = DO_UPCAST(FWCfgState, busdev.qdev, dev);
 
     if (ctl_addr) {
         sysbus_mmio_map(d, 0, ctl_addr);
@@ -490,18 +494,16 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
     if (data_addr) {
         sysbus_mmio_map(d, 1, data_addr);
     }
-    fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (uint8_t *)"QEMU", 4);
-    fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16);
-    fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)(display_type == DT_NOGRAPHIC));
-    fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
-    fw_cfg_add_i16(s, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
-    fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
-    fw_cfg_bootsplash(s);
-
-    s->machine_ready.notify = fw_cfg_machine_ready;
-    qemu_add_machine_init_done_notifier(&s->machine_ready);
-
-    return s;
+    fw_cfg_add_bytes(FW_CFG_SIGNATURE, (uint8_t *)"QEMU", 4);
+    fw_cfg_add_bytes(FW_CFG_UUID, qemu_uuid, 16);
+    fw_cfg_add_i16(FW_CFG_NOGRAPHIC, (uint16_t)(display_type == DT_NOGRAPHIC));
+    fw_cfg_add_i16(FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
+    fw_cfg_add_i16(FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
+    fw_cfg_add_i16(FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
+    fw_cfg_bootsplash();
+
+    fw_cfg->machine_ready.notify = fw_cfg_machine_ready;
+    qemu_add_machine_init_done_notifier(&fw_cfg->machine_ready);
 }
 
 static int fw_cfg_init1(SysBusDevice *dev)
diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
index 856bf91..cdb239e 100644
--- a/hw/fw_cfg.h
+++ b/hw/fw_cfg.h
@@ -54,15 +54,14 @@ typedef struct FWCfgFiles {
 typedef void (*FWCfgCallback)(void *opaque, uint8_t *data);
 
 typedef struct FWCfgState FWCfgState;
-int fw_cfg_add_bytes(FWCfgState *s, uint16_t key, uint8_t *data, uint32_t len);
-int fw_cfg_add_i16(FWCfgState *s, uint16_t key, uint16_t value);
-int fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t value);
-int fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t value);
-int fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback,
+int fw_cfg_add_bytes(uint16_t key, uint8_t *data, uint32_t len);
+int fw_cfg_add_i16(uint16_t key, uint16_t value);
+int fw_cfg_add_i32(uint16_t key, uint32_t value);
+int fw_cfg_add_i64(uint16_t key, uint64_t value);
+int fw_cfg_add_callback(uint16_t key, FWCfgCallback callback,
                         void *callback_opaque, uint8_t *data, size_t len);
-int fw_cfg_add_file(FWCfgState *s, const char *filename, uint8_t *data,
-                    uint32_t len);
-FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
+int fw_cfg_add_file(const char *filename, uint8_t *data, uint32_t len);
+void fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
                         target_phys_addr_t crl_addr, target_phys_addr_t data_addr);
 
 #endif /* NO_QEMU_PROTOS */
diff --git a/hw/loader.c b/hw/loader.c
index 415cdce..28464da 100644
--- a/hw/loader.c
+++ b/hw/loader.c
@@ -543,7 +543,6 @@ struct Rom {
     QTAILQ_ENTRY(Rom) next;
 };
 
-static FWCfgState *fw_cfg;
 static QTAILQ_HEAD(, Rom) roms = QTAILQ_HEAD_INITIALIZER(roms);
 
 static void rom_insert(Rom *rom)
@@ -601,7 +600,7 @@ int rom_add_file(const char *file, const char *fw_dir,
     }
     close(fd);
     rom_insert(rom);
-    if (rom->fw_file && fw_cfg) {
+    if (rom->fw_file) {
         const char *basename;
         char fw_file_name[56];
 
@@ -613,7 +612,7 @@ int rom_add_file(const char *file, const char *fw_dir,
         }
         snprintf(fw_file_name, sizeof(fw_file_name), "%s/%s", rom->fw_dir,
                  basename);
-        fw_cfg_add_file(fw_cfg, fw_file_name, rom->data, rom->romsize);
+        fw_cfg_add_file(fw_file_name, rom->data, rom->romsize);
         snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name);
     } else {
         snprintf(devpath, sizeof(devpath), "/rom@" TARGET_FMT_plx, addr);
@@ -704,11 +703,6 @@ int rom_load_all(void)
     return 0;
 }
 
-void rom_set_fw(void *f)
-{
-    fw_cfg = f;
-}
-
 static Rom *find_rom(target_phys_addr_t addr)
 {
     Rom *rom;
diff --git a/hw/loader.h b/hw/loader.h
index fbcaba9..c357ec4 100644
--- a/hw/loader.h
+++ b/hw/loader.h
@@ -26,7 +26,6 @@ int rom_add_file(const char *file, const char *fw_dir,
 int rom_add_blob(const char *name, const void *blob, size_t len,
                  target_phys_addr_t addr);
 int rom_load_all(void);
-void rom_set_fw(void *f);
 int rom_copy(uint8_t *dest, target_phys_addr_t addr, size_t size);
 void *rom_ptr(target_phys_addr_t addr);
 void do_info_roms(Monitor *mon);
diff --git a/hw/multiboot.c b/hw/multiboot.c
index b4484a3..fd93e13 100644
--- a/hw/multiboot.c
+++ b/hw/multiboot.c
@@ -124,8 +124,7 @@ static void mb_add_mod(MultibootState *s,
     s->mb_mods_count++;
 }
 
-int load_multiboot(void *fw_cfg,
-                   FILE *f,
+int load_multiboot(FILE *f,
                    const char *kernel_filename,
                    const char *initrd_filename,
                    const char *kernel_cmdline,
@@ -324,15 +323,15 @@ int load_multiboot(void *fw_cfg,
     memcpy(mb_bootinfo_data, bootinfo, sizeof(bootinfo));
 
     /* Pass variables to option rom */
-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, mh_entry_addr);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, mbs.mb_buf_size);
-    fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA,
+    fw_cfg_add_i32(FW_CFG_KERNEL_ENTRY, mh_entry_addr);
+    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, mh_load_addr);
+    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, mbs.mb_buf_size);
+    fw_cfg_add_bytes(FW_CFG_KERNEL_DATA,
                      mbs.mb_buf, mbs.mb_buf_size);
 
-    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, ADDR_MBI);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, sizeof(bootinfo));
-    fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, mb_bootinfo_data,
+    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, ADDR_MBI);
+    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, sizeof(bootinfo));
+    fw_cfg_add_bytes(FW_CFG_INITRD_DATA, mb_bootinfo_data,
                      sizeof(bootinfo));
 
     option_rom[nb_option_roms].name = "multiboot.bin";
diff --git a/hw/multiboot.h b/hw/multiboot.h
index 98fb1b7..1302c55 100644
--- a/hw/multiboot.h
+++ b/hw/multiboot.h
@@ -1,8 +1,7 @@
 #ifndef QEMU_MULTIBOOT_H
 #define QEMU_MULTIBOOT_H
 
-int load_multiboot(void *fw_cfg,
-                   FILE *f,
+int load_multiboot(FILE *f,
                    const char *kernel_filename,
                    const char *initrd_filename,
                    const char *kernel_cmdline,
diff --git a/hw/pc.c b/hw/pc.c
index 4d34a33..739e4ad 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -593,9 +593,8 @@ int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
     return index;
 }
 
-static void *bochs_bios_init(void)
+static void bochs_bios_init(void)
 {
-    void *fw_cfg;
     uint8_t *smbios_table;
     size_t smbios_len;
     uint64_t *numa_fw_cfg;
@@ -613,22 +612,21 @@ static void *bochs_bios_init(void)
     register_ioport_write(0x500, 1, 1, bochs_bios_write, NULL);
     register_ioport_write(0x503, 1, 1, bochs_bios_write, NULL);
 
-    fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
+    fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
 
-    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
-    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
-    fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES, (uint8_t *)acpi_tables,
+    fw_cfg_add_i32(FW_CFG_ID, 1);
+    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
+    fw_cfg_add_bytes(FW_CFG_ACPI_TABLES, (uint8_t *)acpi_tables,
                      acpi_tables_len);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
+    fw_cfg_add_i32(FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
 
     smbios_table = smbios_get_table(&smbios_len);
     if (smbios_table)
-        fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
-                         smbios_table, smbios_len);
-    fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE, (uint8_t *)&e820_table,
+        fw_cfg_add_bytes(FW_CFG_SMBIOS_ENTRIES, smbios_table, smbios_len);
+    fw_cfg_add_bytes(FW_CFG_E820_TABLE, (uint8_t *)&e820_table,
                      sizeof(struct e820_table));
 
-    fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, (uint8_t *)&hpet_cfg,
+    fw_cfg_add_bytes(FW_CFG_HPET, (uint8_t *)&hpet_cfg,
                      sizeof(struct hpet_fw_config));
     /* allocate memory for the NUMA channel: one (64bit) word for the number
      * of nodes, one word for each VCPU->node and one word for each node to
@@ -647,10 +645,8 @@ static void *bochs_bios_init(void)
     for (i = 0; i < nb_numa_nodes; i++) {
         numa_fw_cfg[max_cpus + 1 + i] = cpu_to_le64(node_mem[i]);
     }
-    fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, (uint8_t *)numa_fw_cfg,
+    fw_cfg_add_bytes(FW_CFG_NUMA, (uint8_t *)numa_fw_cfg,
                      (1 + max_cpus + nb_numa_nodes) * 8);
-
-    return fw_cfg;
 }
 
 static long get_file_size(FILE *f)
@@ -667,8 +663,7 @@ static long get_file_size(FILE *f)
     return size;
 }
 
-static void load_linux(void *fw_cfg,
-                       const char *kernel_filename,
+static void load_linux(const char *kernel_filename,
 		       const char *initrd_filename,
 		       const char *kernel_cmdline,
                        target_phys_addr_t max_ram_size)
@@ -703,9 +698,10 @@ static void load_linux(void *fw_cfg,
     else {
 	/* This looks like a multiboot kernel. If it is, let's stop
 	   treating it like a Linux kernel. */
-        if (load_multiboot(fw_cfg, f, kernel_filename, initrd_filename,
-                           kernel_cmdline, kernel_size, header))
+        if (load_multiboot(f, kernel_filename, initrd_filename,
+                           kernel_cmdline, kernel_size, header)) {
             return;
+        }
 	protocol = 0;
     }
 
@@ -745,9 +741,9 @@ static void load_linux(void *fw_cfg,
     if (initrd_max >= max_ram_size-ACPI_DATA_SIZE)
     	initrd_max = max_ram_size-ACPI_DATA_SIZE-1;
 
-    fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_ADDR, cmdline_addr);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, strlen(kernel_cmdline)+1);
-    fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
+    fw_cfg_add_i32(FW_CFG_CMDLINE_ADDR, cmdline_addr);
+    fw_cfg_add_i32(FW_CFG_CMDLINE_SIZE, strlen(kernel_cmdline)+1);
+    fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
                      (uint8_t*)strdup(kernel_cmdline),
                      strlen(kernel_cmdline)+1);
 
@@ -808,9 +804,9 @@ static void load_linux(void *fw_cfg,
         initrd_data = g_malloc(initrd_size);
         load_image(initrd_filename, initrd_data);
 
-        fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr);
-        fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
-        fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, initrd_data, initrd_size);
+        fw_cfg_add_i32(FW_CFG_INITRD_ADDR, initrd_addr);
+        fw_cfg_add_i32(FW_CFG_INITRD_SIZE, initrd_size);
+        fw_cfg_add_bytes(FW_CFG_INITRD_DATA, initrd_data, initrd_size);
 
 	stl_p(header+0x218, initrd_addr);
 	stl_p(header+0x21c, initrd_size);
@@ -837,13 +833,13 @@ static void load_linux(void *fw_cfg,
     fclose(f);
     memcpy(setup, header, MIN(sizeof(header), setup_size));
 
-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, prot_addr);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
-    fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA, kernel, kernel_size);
+    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, prot_addr);
+    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
+    fw_cfg_add_bytes(FW_CFG_KERNEL_DATA, kernel, kernel_size);
 
-    fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_ADDR, real_addr);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_SIZE, setup_size);
-    fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA, setup, setup_size);
+    fw_cfg_add_i32(FW_CFG_SETUP_ADDR, real_addr);
+    fw_cfg_add_i32(FW_CFG_SETUP_SIZE, setup_size);
+    fw_cfg_add_bytes(FW_CFG_SETUP_DATA, setup, setup_size);
 
     option_rom[nb_option_roms].name = "linuxboot.bin";
     option_rom[nb_option_roms].bootindex = 0;
@@ -987,7 +983,6 @@ void pc_memory_init(MemoryRegion *system_memory,
     int linux_boot, i;
     MemoryRegion *ram, *option_rom_mr;
     MemoryRegion *ram_below_4g, *ram_above_4g;
-    void *fw_cfg;
 
     linux_boot = (kernel_filename != NULL);
 
@@ -1024,11 +1019,11 @@ void pc_memory_init(MemoryRegion *system_memory,
                                         option_rom_mr,
                                         1);
 
-    fw_cfg = bochs_bios_init();
-    rom_set_fw(fw_cfg);
+    bochs_bios_init();
 
     if (linux_boot) {
-        load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline, below_4g_mem_size);
+        load_linux(kernel_filename, initrd_filename, kernel_cmdline,
+                   below_4g_mem_size);
     }
 
     for (i = 0; i < nb_option_roms; i++) {
diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
index 8796510..1f2a30e 100644
--- a/hw/ppc_newworld.c
+++ b/hw/ppc_newworld.c
@@ -107,7 +107,7 @@ static const MemoryRegionOps unin_ops = {
 
 static int fw_cfg_boot_set(void *opaque, const char *boot_device)
 {
-    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
     return 0;
 }
 
@@ -152,7 +152,6 @@ static void ppc_core99_init (ram_addr_t ram_size,
     MemoryRegion *ide_mem[3];
     int ppc_boot_device;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
-    void *fw_cfg;
     void *dbdma;
     int machine_arch;
 
@@ -377,42 +376,42 @@ static void ppc_core99_init (ram_addr_t ram_size,
     macio_nvram_setup_bar(nvr, get_system_memory(), 0xFFF04000);
     /* No PCI init: the BIOS will do it */
 
-    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
-    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, machine_arch);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, kernel_base);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
+    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
+    fw_cfg_add_i32(FW_CFG_ID, 1);
+    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
+    fw_cfg_add_i16(FW_CFG_MACHINE_ID, machine_arch);
+    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, kernel_base);
+    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
     if (kernel_cmdline) {
-        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, cmdline_base);
+        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, cmdline_base);
         pstrcpy_targphys("cmdline", cmdline_base, TARGET_PAGE_SIZE, kernel_cmdline);
     } else {
-        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
+        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, 0);
     }
-    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_base);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, ppc_boot_device);
+    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, initrd_base);
+    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, initrd_size);
+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, ppc_boot_device);
 
-    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_WIDTH, graphic_width);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_HEIGHT, graphic_height);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_DEPTH, graphic_depth);
+    fw_cfg_add_i16(FW_CFG_PPC_WIDTH, graphic_width);
+    fw_cfg_add_i16(FW_CFG_PPC_HEIGHT, graphic_height);
+    fw_cfg_add_i16(FW_CFG_PPC_DEPTH, graphic_depth);
 
-    fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_IS_KVM, kvm_enabled());
+    fw_cfg_add_i32(FW_CFG_PPC_IS_KVM, kvm_enabled());
     if (kvm_enabled()) {
 #ifdef CONFIG_KVM
         uint8_t *hypercall;
 
-        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, kvmppc_get_tbfreq());
+        fw_cfg_add_i32(FW_CFG_PPC_TBFREQ, kvmppc_get_tbfreq());
         hypercall = g_malloc(16);
         kvmppc_get_hypercall(env, hypercall, 16);
-        fw_cfg_add_bytes(fw_cfg, FW_CFG_PPC_KVM_HC, hypercall, 16);
-        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_KVM_PID, getpid());
+        fw_cfg_add_bytes(FW_CFG_PPC_KVM_HC, hypercall, 16);
+        fw_cfg_add_i32(FW_CFG_PPC_KVM_PID, getpid());
 #endif
     } else {
-        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, get_ticks_per_sec());
+        fw_cfg_add_i32(FW_CFG_PPC_TBFREQ, get_ticks_per_sec());
     }
 
-    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
+    qemu_register_boot_set(fw_cfg_boot_set, NULL);
 }
 
 static QEMUMachine core99_machine = {
diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c
index 7e73d37..575fe59 100644
--- a/hw/ppc_oldworld.c
+++ b/hw/ppc_oldworld.c
@@ -50,7 +50,7 @@
 
 static int fw_cfg_boot_set(void *opaque, const char *boot_device)
 {
-    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
     return 0;
 }
 
@@ -95,7 +95,6 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
     MemoryRegion *escc_mem, *escc_bar = g_new(MemoryRegion, 1), *ide_mem[2];
     uint16_t ppc_boot_device;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
-    void *fw_cfg;
     void *dbdma;
 
     linux_boot = (kernel_filename != NULL);
@@ -292,42 +291,42 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
 
     /* No PCI init: the BIOS will do it */
 
-    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
-    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, ARCH_HEATHROW);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, kernel_base);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
+    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
+    fw_cfg_add_i32(FW_CFG_ID, 1);
+    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
+    fw_cfg_add_i16(FW_CFG_MACHINE_ID, ARCH_HEATHROW);
+    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, kernel_base);
+    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
     if (kernel_cmdline) {
-        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, cmdline_base);
+        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, cmdline_base);
         pstrcpy_targphys("cmdline", cmdline_base, TARGET_PAGE_SIZE, kernel_cmdline);
     } else {
-        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
+        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, 0);
     }
-    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_base);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, ppc_boot_device);
+    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, initrd_base);
+    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, initrd_size);
+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, ppc_boot_device);
 
-    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_WIDTH, graphic_width);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_HEIGHT, graphic_height);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_DEPTH, graphic_depth);
+    fw_cfg_add_i16(FW_CFG_PPC_WIDTH, graphic_width);
+    fw_cfg_add_i16(FW_CFG_PPC_HEIGHT, graphic_height);
+    fw_cfg_add_i16(FW_CFG_PPC_DEPTH, graphic_depth);
 
-    fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_IS_KVM, kvm_enabled());
+    fw_cfg_add_i32(FW_CFG_PPC_IS_KVM, kvm_enabled());
     if (kvm_enabled()) {
 #ifdef CONFIG_KVM
         uint8_t *hypercall;
 
-        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, kvmppc_get_tbfreq());
+        fw_cfg_add_i32(FW_CFG_PPC_TBFREQ, kvmppc_get_tbfreq());
         hypercall = g_malloc(16);
         kvmppc_get_hypercall(env, hypercall, 16);
-        fw_cfg_add_bytes(fw_cfg, FW_CFG_PPC_KVM_HC, hypercall, 16);
-        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_KVM_PID, getpid());
+        fw_cfg_add_bytes(FW_CFG_PPC_KVM_HC, hypercall, 16);
+        fw_cfg_add_i32(FW_CFG_PPC_KVM_PID, getpid());
 #endif
     } else {
-        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, get_ticks_per_sec());
+        fw_cfg_add_i32(FW_CFG_PPC_TBFREQ, get_ticks_per_sec());
     }
 
-    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
+    qemu_register_boot_set(fw_cfg_boot_set, NULL);
 }
 
 static QEMUMachine heathrow_machine = {
diff --git a/hw/sun4m.c b/hw/sun4m.c
index 34088ad..9f546d6 100644
--- a/hw/sun4m.c
+++ b/hw/sun4m.c
@@ -163,7 +163,7 @@ void DMA_register_channel (int nchan,
 
 static int fw_cfg_boot_set(void *opaque, const char *boot_device)
 {
-    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
     return 0;
 }
 
@@ -843,7 +843,6 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
     qemu_irq *cpu_halt;
     unsigned long kernel_size;
     DriveInfo *fd[MAX_FD];
-    void *fw_cfg;
     unsigned int num_vsimms;
 
     /* init CPUs */
@@ -995,29 +994,29 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
         ecc_init(hwdef->ecc_base, slavio_irq[28],
                  hwdef->ecc_version);
 
-    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
-    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, hwdef->machine_id);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_SUN4M_DEPTH, graphic_depth);
-    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);
+    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
+    fw_cfg_add_i32(FW_CFG_ID, 1);
+    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
+    fw_cfg_add_i16(FW_CFG_MACHINE_ID, hwdef->machine_id);
+    fw_cfg_add_i16(FW_CFG_SUN4M_DEPTH, graphic_depth);
+    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, KERNEL_LOAD_ADDR);
+    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
     if (kernel_cmdline) {
-        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
+        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
         pstrcpy_targphys("cmdline", CMDLINE_ADDR, TARGET_PAGE_SIZE, kernel_cmdline);
-        fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
+        fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
                          (uint8_t*)strdup(kernel_cmdline),
                          strlen(kernel_cmdline) + 1);
-        fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,
+        fw_cfg_add_i32(FW_CFG_CMDLINE_SIZE,
                        strlen(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_KERNEL_CMDLINE, 0);
+        fw_cfg_add_i32(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]);
-    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
+    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, INITRD_LOAD_ADDR);
+    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, 0); /* not used */
+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
+    qemu_register_boot_set(fw_cfg_boot_set, NULL);
 }
 
 enum {
@@ -1525,7 +1524,6 @@ static void sun4d_hw_init(const struct sun4d_hwdef *hwdef, ram_addr_t RAM_size,
         espdma_irq, ledma_irq;
     qemu_irq esp_reset, dma_enable;
     unsigned long kernel_size;
-    void *fw_cfg;
     DeviceState *dev;
 
     /* init CPUs */
@@ -1606,26 +1604,26 @@ static void sun4d_hw_init(const struct sun4d_hwdef *hwdef, ram_addr_t RAM_size,
                graphic_height, graphic_depth, hwdef->nvram_machine_id,
                "Sun4d");
 
-    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
-    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, hwdef->machine_id);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_SUN4M_DEPTH, graphic_depth);
-    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);
+    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
+    fw_cfg_add_i32(FW_CFG_ID, 1);
+    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
+    fw_cfg_add_i16(FW_CFG_MACHINE_ID, hwdef->machine_id);
+    fw_cfg_add_i16(FW_CFG_SUN4M_DEPTH, graphic_depth);
+    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, KERNEL_LOAD_ADDR);
+    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
     if (kernel_cmdline) {
-        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
+        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
         pstrcpy_targphys("cmdline", CMDLINE_ADDR, TARGET_PAGE_SIZE, kernel_cmdline);
-        fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
+        fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
                          (uint8_t*)strdup(kernel_cmdline),
                          strlen(kernel_cmdline) + 1);
     } else {
-        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
+        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, 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]);
-    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
+    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, INITRD_LOAD_ADDR);
+    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, 0); /* not used */
+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
+    qemu_register_boot_set(fw_cfg_boot_set, NULL);
 }
 
 /* SPARCserver 1000 hardware initialisation */
@@ -1719,7 +1717,6 @@ static void sun4c_hw_init(const struct sun4c_hwdef *hwdef, ram_addr_t RAM_size,
     qemu_irq fdc_tc;
     unsigned long kernel_size;
     DriveInfo *fd[MAX_FD];
-    void *fw_cfg;
     DeviceState *dev;
     unsigned int i;
 
@@ -1798,26 +1795,26 @@ static void sun4c_hw_init(const struct sun4c_hwdef *hwdef, ram_addr_t RAM_size,
                graphic_height, graphic_depth, hwdef->nvram_machine_id,
                "Sun4c");
 
-    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
-    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, hwdef->machine_id);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_SUN4M_DEPTH, graphic_depth);
-    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);
+    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
+    fw_cfg_add_i32(FW_CFG_ID, 1);
+    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
+    fw_cfg_add_i16(FW_CFG_MACHINE_ID, hwdef->machine_id);
+    fw_cfg_add_i16(FW_CFG_SUN4M_DEPTH, graphic_depth);
+    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, KERNEL_LOAD_ADDR);
+    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
     if (kernel_cmdline) {
-        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
+        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
         pstrcpy_targphys("cmdline", CMDLINE_ADDR, TARGET_PAGE_SIZE, kernel_cmdline);
-        fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
+        fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
                          (uint8_t*)strdup(kernel_cmdline),
                          strlen(kernel_cmdline) + 1);
     } else {
-        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
+        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, 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]);
-    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
+    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, INITRD_LOAD_ADDR);
+    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, 0); /* not used */
+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
+    qemu_register_boot_set(fw_cfg_boot_set, NULL);
 }
 
 /* SPARCstation 2 hardware initialisation */
diff --git a/hw/sun4u.c b/hw/sun4u.c
index 517bdb8..094c07c 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -125,7 +125,7 @@ void DMA_register_channel (int nchan,
 
 static int fw_cfg_boot_set(void *opaque, const char *boot_device)
 {
-    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
     return 0;
 }
 
@@ -802,7 +802,6 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     qemu_irq *ivec_irqs, *pbm_irqs;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     DriveInfo *fd[MAX_FD];
-    void *fw_cfg;
 
     /* init CPUs */
     env = cpu_devinit(cpu_model, hwdef);
@@ -867,30 +866,30 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
                            graphic_width, graphic_height, graphic_depth,
                            (uint8_t *)&nd_table[0].macaddr);
 
-    fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
-    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
-    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);
+    fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
+    fw_cfg_add_i32(FW_CFG_ID, 1);
+    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
+    fw_cfg_add_i16(FW_CFG_MACHINE_ID, hwdef->machine_id);
+    fw_cfg_add_i64(FW_CFG_KERNEL_ADDR, kernel_entry);
+    fw_cfg_add_i64(FW_CFG_KERNEL_SIZE, kernel_size);
     if (kernel_cmdline) {
-        fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,
+        fw_cfg_add_i32(FW_CFG_CMDLINE_SIZE,
                        strlen(kernel_cmdline) + 1);
-        fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
+        fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
                          (uint8_t*)strdup(kernel_cmdline),
                          strlen(kernel_cmdline) + 1);
     } else {
-        fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, 0);
+        fw_cfg_add_i32(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_i64(FW_CFG_INITRD_ADDR, initrd_addr);
+    fw_cfg_add_i64(FW_CFG_INITRD_SIZE, initrd_size);
+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_devices[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);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_SPARC64_DEPTH, graphic_depth);
+    fw_cfg_add_i16(FW_CFG_SPARC64_WIDTH, graphic_width);
+    fw_cfg_add_i16(FW_CFG_SPARC64_HEIGHT, graphic_height);
+    fw_cfg_add_i16(FW_CFG_SPARC64_DEPTH, graphic_depth);
 
-    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
+    qemu_register_boot_set(fw_cfg_boot_set, NULL);
 }
 
 enum {
-- 
1.7.7.3

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

* [Qemu-devel] [PATCH 2/2] Add PIIX4 properties to control PM system states.
  2012-05-20  9:02 [Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global Gleb Natapov
@ 2012-05-20  9:02 ` Gleb Natapov
  2012-05-23 13:27   ` Anthony Liguori
  2012-05-23 12:37 ` [Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global Gleb Natapov
  2012-05-23 12:44 ` Peter Maydell
  2 siblings, 1 reply; 18+ messages in thread
From: Gleb Natapov @ 2012-05-20  9:02 UTC (permalink / raw)
  To: qemu-devel

This patch adds two things. First it allows QEMU to distinguish between
regular powerdown and S4 powerdown. Later separate QMP notification will
be added for S4 powerdown. Second it allows S3/S4 states to be disabled
from QEMU command line. Some guests known to be broken with regards to
power management, but allow to use it anyway. Using new properties
management will be able to disable S3/S4 for such guests.

Supported system state are passed to a firmware using new fw_cfg file.
The file contains  6 byte array. Each byte represents one system
state. If byte at offset X has its MSB set it means that system state
X is supported and to enter it guest should use the value from lowest 3
bits.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 hw/acpi.c       |    5 ++++-
 hw/acpi.h       |    2 +-
 hw/acpi_piix4.c |   16 +++++++++++++++-
 hw/vt82c686.c   |    2 +-
 4 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index 5d521e5..effc7ec 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -370,7 +370,7 @@ void acpi_pm1_cnt_init(ACPIREGS *ar)
     qemu_register_wakeup_notifier(&ar->wakeup);
 }
 
-void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val)
+void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val, char s4)
 {
     ar->pm1.cnt.cnt = val & ~(ACPI_BITMASK_SLEEP_ENABLE);
 
@@ -385,6 +385,9 @@ void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val)
             qemu_system_suspend_request();
             break;
         default:
+            if (sus_typ == s4) { /* S4 request */
+                qemu_system_shutdown_request();
+            }
             break;
         }
     }
diff --git a/hw/acpi.h b/hw/acpi.h
index fe8cdb4..7337f41 100644
--- a/hw/acpi.h
+++ b/hw/acpi.h
@@ -139,7 +139,7 @@ void acpi_pm1_evt_reset(ACPIREGS *ar);
 
 /* PM1a_CNT: piix and ich9 don't implement PM1b CNT. */
 void acpi_pm1_cnt_init(ACPIREGS *ar);
-void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val);
+void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val, char s4);
 void acpi_pm1_cnt_update(ACPIREGS *ar,
                          bool sci_enable, bool sci_disable);
 void acpi_pm1_cnt_reset(ACPIREGS *ar);
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 585da4e..883314d 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -27,6 +27,7 @@
 #include "sysemu.h"
 #include "range.h"
 #include "ioport.h"
+#include "fw_cfg.h"
 
 //#define DEBUG
 
@@ -71,6 +72,10 @@ typedef struct PIIX4PMState {
     struct pci_status pci0_status;
     uint32_t pci0_hotplug_enable;
     uint32_t pci0_slot_device_present;
+
+    uint8_t disable_s3;
+    uint8_t disable_s4;
+    uint8_t s4_val;
 } PIIX4PMState;
 
 static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s);
@@ -123,7 +128,7 @@ static void pm_ioport_write(IORange *ioport, uint64_t addr, unsigned width,
         pm_update_sci(s);
         break;
     case 0x04:
-        acpi_pm1_cnt_write(&s->ar, val);
+        acpi_pm1_cnt_write(&s->ar, val, s->s4_val);
         break;
     default:
         break;
@@ -425,6 +430,7 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
 {
     PCIDevice *dev;
     PIIX4PMState *s;
+    uint8_t suspend[6] = {128, 0, 0, 129, 128, 128};
 
     dev = pci_create(bus, devfn, "PIIX4_PM");
     qdev_prop_set_uint32(&dev->qdev, "smb_io_base", smb_io_base);
@@ -437,11 +443,19 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
 
     qdev_init_nofail(&dev->qdev);
 
+    suspend[3] = 1 | ((!s->disable_s3) << 7);
+    suspend[4] = s->s4_val | ((!s->disable_s4) << 7);
+
+    fw_cfg_add_file("etc/system-states", g_memdup(suspend, 6), 6);
+
     return s->smb.smbus;
 }
 
 static Property piix4_pm_properties[] = {
     DEFINE_PROP_UINT32("smb_io_base", PIIX4PMState, smb_io_base, 0),
+    DEFINE_PROP_UINT8("disable_s3", PIIX4PMState, disable_s3, 0),
+    DEFINE_PROP_UINT8("disable_s4", PIIX4PMState, disable_s4, 0),
+    DEFINE_PROP_UINT8("s4_val", PIIX4PMState, s4_val, 2),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/vt82c686.c b/hw/vt82c686.c
index 6fb7950..5d7c00c 100644
--- a/hw/vt82c686.c
+++ b/hw/vt82c686.c
@@ -210,7 +210,7 @@ static void pm_ioport_writew(void *opaque, uint32_t addr, uint32_t val)
         pm_update_sci(s);
         break;
     case 0x04:
-        acpi_pm1_cnt_write(&s->ar, val);
+        acpi_pm1_cnt_write(&s->ar, val, 0);
         break;
     default:
         break;
-- 
1.7.7.3

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

* Re: [Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global.
  2012-05-20  9:02 [Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global Gleb Natapov
  2012-05-20  9:02 ` [Qemu-devel] [PATCH 2/2] Add PIIX4 properties to control PM system states Gleb Natapov
@ 2012-05-23 12:37 ` Gleb Natapov
  2012-05-23 13:25   ` Anthony Liguori
  2012-05-23 12:44 ` Peter Maydell
  2 siblings, 1 reply; 18+ messages in thread
From: Gleb Natapov @ 2012-05-23 12:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: blauwirbel, anthony

Ping.

On Sun, May 20, 2012 at 12:02:40PM +0300, Gleb Natapov wrote:
> There can be only one fw_cfg device, so saving global reference to it
> removes the need to pass its pointer around.
> 
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> ---
>  hw/fw_cfg.c       |  110 +++++++++++++++++++++++++++--------------------------
>  hw/fw_cfg.h       |   15 +++----
>  hw/loader.c       |   10 +----
>  hw/loader.h       |    1 -
>  hw/multiboot.c    |   17 ++++----
>  hw/multiboot.h    |    3 +-
>  hw/pc.c           |   63 ++++++++++++++----------------
>  hw/ppc_newworld.c |   43 ++++++++++-----------
>  hw/ppc_oldworld.c |   43 ++++++++++-----------
>  hw/sun4m.c        |   93 +++++++++++++++++++++-----------------------
>  hw/sun4u.c        |   35 ++++++++---------
>  11 files changed, 207 insertions(+), 226 deletions(-)
> 
> diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
> index 7b3b576..8c50473 100644
> --- a/hw/fw_cfg.c
> +++ b/hw/fw_cfg.c
> @@ -48,7 +48,7 @@ typedef struct FWCfgEntry {
>      FWCfgCallback callback;
>  } FWCfgEntry;
>  
> -struct FWCfgState {
> +static struct FWCfgState {
>      SysBusDevice busdev;
>      MemoryRegion ctl_iomem, data_iomem, comb_iomem;
>      uint32_t ctl_iobase, data_iobase;
> @@ -57,7 +57,7 @@ struct FWCfgState {
>      uint16_t cur_entry;
>      uint32_t cur_offset;
>      Notifier machine_ready;
> -};
> +} *fw_cfg;
>  
>  #define JPG_FILE 0
>  #define BMP_FILE 1
> @@ -113,7 +113,7 @@ error:
>      return NULL;
>  }
>  
> -static void fw_cfg_bootsplash(FWCfgState *s)
> +static void fw_cfg_bootsplash(void)
>  {
>      int boot_splash_time = -1;
>      const char *boot_splash_filename = NULL;
> @@ -148,7 +148,7 @@ static void fw_cfg_bootsplash(FWCfgState *s)
>          /* use little endian format */
>          qemu_extra_params_fw[0] = (uint8_t)(boot_splash_time & 0xff);
>          qemu_extra_params_fw[1] = (uint8_t)((boot_splash_time >> 8) & 0xff);
> -        fw_cfg_add_file(s, "etc/boot-menu-wait", qemu_extra_params_fw, 2);
> +        fw_cfg_add_file("etc/boot-menu-wait", qemu_extra_params_fw, 2);
>      }
>  
>      /* insert splash file if user configurated */
> @@ -173,10 +173,10 @@ static void fw_cfg_bootsplash(FWCfgState *s)
>  
>          /* insert data */
>          if (file_type == JPG_FILE) {
> -            fw_cfg_add_file(s, "bootsplash.jpg",
> +            fw_cfg_add_file("bootsplash.jpg",
>                      boot_splash_filedata, boot_splash_filedata_size);
>          } else {
> -            fw_cfg_add_file(s, "bootsplash.bmp",
> +            fw_cfg_add_file("bootsplash.bmp",
>                      boot_splash_filedata, boot_splash_filedata_size);
>          }
>          g_free(filename);
> @@ -359,122 +359,126 @@ static const VMStateDescription vmstate_fw_cfg = {
>      }
>  };
>  
> -int fw_cfg_add_bytes(FWCfgState *s, uint16_t key, uint8_t *data, uint32_t len)
> +int fw_cfg_add_bytes(uint16_t key, uint8_t *data, uint32_t len)
>  {
>      int arch = !!(key & FW_CFG_ARCH_LOCAL);
>  
>      key &= FW_CFG_ENTRY_MASK;
>  
> -    if (key >= FW_CFG_MAX_ENTRY)
> +    if (key >= FW_CFG_MAX_ENTRY || !fw_cfg) {
>          return 0;
> +    }
>  
> -    s->entries[arch][key].data = data;
> -    s->entries[arch][key].len = len;
> +    fw_cfg->entries[arch][key].data = data;
> +    fw_cfg->entries[arch][key].len = len;
>  
>      return 1;
>  }
>  
> -int fw_cfg_add_i16(FWCfgState *s, uint16_t key, uint16_t value)
> +int fw_cfg_add_i16(uint16_t key, uint16_t value)
>  {
>      uint16_t *copy;
>  
>      copy = g_malloc(sizeof(value));
>      *copy = cpu_to_le16(value);
> -    return fw_cfg_add_bytes(s, key, (uint8_t *)copy, sizeof(value));
> +    return fw_cfg_add_bytes(key, (uint8_t *)copy, sizeof(value));
>  }
>  
> -int fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t value)
> +int fw_cfg_add_i32(uint16_t key, uint32_t value)
>  {
>      uint32_t *copy;
>  
>      copy = g_malloc(sizeof(value));
>      *copy = cpu_to_le32(value);
> -    return fw_cfg_add_bytes(s, key, (uint8_t *)copy, sizeof(value));
> +    return fw_cfg_add_bytes(key, (uint8_t *)copy, sizeof(value));
>  }
>  
> -int fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t value)
> +int fw_cfg_add_i64(uint16_t key, uint64_t value)
>  {
>      uint64_t *copy;
>  
>      copy = g_malloc(sizeof(value));
>      *copy = cpu_to_le64(value);
> -    return fw_cfg_add_bytes(s, key, (uint8_t *)copy, sizeof(value));
> +    return fw_cfg_add_bytes(key, (uint8_t *)copy, sizeof(value));
>  }
>  
> -int fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback,
> +int fw_cfg_add_callback(uint16_t key, FWCfgCallback callback,
>                          void *callback_opaque, uint8_t *data, size_t len)
>  {
>      int arch = !!(key & FW_CFG_ARCH_LOCAL);
>  
> -    if (!(key & FW_CFG_WRITE_CHANNEL))
> +    if (!(key & FW_CFG_WRITE_CHANNEL) || !fw_cfg) {
>          return 0;
> +    }
>  
>      key &= FW_CFG_ENTRY_MASK;
>  
>      if (key >= FW_CFG_MAX_ENTRY || len > 65535)
>          return 0;
>  
> -    s->entries[arch][key].data = data;
> -    s->entries[arch][key].len = len;
> -    s->entries[arch][key].callback_opaque = callback_opaque;
> -    s->entries[arch][key].callback = callback;
> +    fw_cfg->entries[arch][key].data = data;
> +    fw_cfg->entries[arch][key].len = len;
> +    fw_cfg->entries[arch][key].callback_opaque = callback_opaque;
> +    fw_cfg->entries[arch][key].callback = callback;
>  
>      return 1;
>  }
>  
> -int fw_cfg_add_file(FWCfgState *s,  const char *filename, uint8_t *data,
> -                    uint32_t len)
> +int fw_cfg_add_file(const char *filename, uint8_t *data, uint32_t len)
>  {
>      int i, index;
>  
> -    if (!s->files) {
> +    if (!fw_cfg) {
> +        return 0;
> +    }
> +
> +    if (!fw_cfg->files) {
>          int dsize = sizeof(uint32_t) + sizeof(FWCfgFile) * FW_CFG_FILE_SLOTS;
> -        s->files = g_malloc0(dsize);
> -        fw_cfg_add_bytes(s, FW_CFG_FILE_DIR, (uint8_t*)s->files, dsize);
> +        fw_cfg->files = g_malloc0(dsize);
> +        fw_cfg_add_bytes(FW_CFG_FILE_DIR, (uint8_t *)fw_cfg->files, dsize);
>      }
>  
> -    index = be32_to_cpu(s->files->count);
> +    index = be32_to_cpu(fw_cfg->files->count);
>      if (index == FW_CFG_FILE_SLOTS) {
>          fprintf(stderr, "fw_cfg: out of file slots\n");
>          return 0;
>      }
>  
> -    fw_cfg_add_bytes(s, FW_CFG_FILE_FIRST + index, data, len);
> +    fw_cfg_add_bytes(FW_CFG_FILE_FIRST + index, data, len);
>  
> -    pstrcpy(s->files->f[index].name, sizeof(s->files->f[index].name),
> +    pstrcpy(fw_cfg->files->f[index].name, sizeof(fw_cfg->files->f[index].name),
>              filename);
>      for (i = 0; i < index; i++) {
> -        if (strcmp(s->files->f[index].name, s->files->f[i].name) == 0) {
> +        if (strcmp(fw_cfg->files->f[index].name, fw_cfg->files->f[i].name)
> +                        == 0) {
>              FW_CFG_DPRINTF("%s: skip duplicate: %s\n", __FUNCTION__,
> -                           s->files->f[index].name);
> +                           fw_cfg->files->f[index].name);
>              return 1;
>          }
>      }
>  
> -    s->files->f[index].size   = cpu_to_be32(len);
> -    s->files->f[index].select = cpu_to_be16(FW_CFG_FILE_FIRST + index);
> +    fw_cfg->files->f[index].size   = cpu_to_be32(len);
> +    fw_cfg->files->f[index].select = cpu_to_be16(FW_CFG_FILE_FIRST + index);
>      FW_CFG_DPRINTF("%s: #%d: %s (%d bytes)\n", __FUNCTION__,
> -                   index, s->files->f[index].name, len);
> +                   index, fw_cfg->files->f[index].name, len);
>  
> -    s->files->count = cpu_to_be32(index+1);
> +    fw_cfg->files->count = cpu_to_be32(index+1);
>      return 1;
>  }
>  
>  static void fw_cfg_machine_ready(struct Notifier *n, void *data)
>  {
>      uint32_t len;
> -    FWCfgState *s = container_of(n, FWCfgState, machine_ready);
>      char *bootindex = get_boot_devices_list(&len);
>  
> -    fw_cfg_add_file(s, "bootorder", (uint8_t*)bootindex, len);
> +    fw_cfg_add_file("bootorder", (uint8_t *)bootindex, len);
>  }
>  
> -FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
> -                        target_phys_addr_t ctl_addr, target_phys_addr_t data_addr)
> +void fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
> +                 target_phys_addr_t ctl_addr, target_phys_addr_t data_addr)
>  {
>      DeviceState *dev;
>      SysBusDevice *d;
> -    FWCfgState *s;
>  
>      dev = qdev_create(NULL, "fw_cfg");
>      qdev_prop_set_uint32(dev, "ctl_iobase", ctl_port);
> @@ -482,7 +486,7 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
>      qdev_init_nofail(dev);
>      d = sysbus_from_qdev(dev);
>  
> -    s = DO_UPCAST(FWCfgState, busdev.qdev, dev);
> +    fw_cfg = DO_UPCAST(FWCfgState, busdev.qdev, dev);
>  
>      if (ctl_addr) {
>          sysbus_mmio_map(d, 0, ctl_addr);
> @@ -490,18 +494,16 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
>      if (data_addr) {
>          sysbus_mmio_map(d, 1, data_addr);
>      }
> -    fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (uint8_t *)"QEMU", 4);
> -    fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16);
> -    fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)(display_type == DT_NOGRAPHIC));
> -    fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
> -    fw_cfg_add_i16(s, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
> -    fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
> -    fw_cfg_bootsplash(s);
> -
> -    s->machine_ready.notify = fw_cfg_machine_ready;
> -    qemu_add_machine_init_done_notifier(&s->machine_ready);
> -
> -    return s;
> +    fw_cfg_add_bytes(FW_CFG_SIGNATURE, (uint8_t *)"QEMU", 4);
> +    fw_cfg_add_bytes(FW_CFG_UUID, qemu_uuid, 16);
> +    fw_cfg_add_i16(FW_CFG_NOGRAPHIC, (uint16_t)(display_type == DT_NOGRAPHIC));
> +    fw_cfg_add_i16(FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
> +    fw_cfg_add_i16(FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
> +    fw_cfg_add_i16(FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
> +    fw_cfg_bootsplash();
> +
> +    fw_cfg->machine_ready.notify = fw_cfg_machine_ready;
> +    qemu_add_machine_init_done_notifier(&fw_cfg->machine_ready);
>  }
>  
>  static int fw_cfg_init1(SysBusDevice *dev)
> diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
> index 856bf91..cdb239e 100644
> --- a/hw/fw_cfg.h
> +++ b/hw/fw_cfg.h
> @@ -54,15 +54,14 @@ typedef struct FWCfgFiles {
>  typedef void (*FWCfgCallback)(void *opaque, uint8_t *data);
>  
>  typedef struct FWCfgState FWCfgState;
> -int fw_cfg_add_bytes(FWCfgState *s, uint16_t key, uint8_t *data, uint32_t len);
> -int fw_cfg_add_i16(FWCfgState *s, uint16_t key, uint16_t value);
> -int fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t value);
> -int fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t value);
> -int fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback,
> +int fw_cfg_add_bytes(uint16_t key, uint8_t *data, uint32_t len);
> +int fw_cfg_add_i16(uint16_t key, uint16_t value);
> +int fw_cfg_add_i32(uint16_t key, uint32_t value);
> +int fw_cfg_add_i64(uint16_t key, uint64_t value);
> +int fw_cfg_add_callback(uint16_t key, FWCfgCallback callback,
>                          void *callback_opaque, uint8_t *data, size_t len);
> -int fw_cfg_add_file(FWCfgState *s, const char *filename, uint8_t *data,
> -                    uint32_t len);
> -FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
> +int fw_cfg_add_file(const char *filename, uint8_t *data, uint32_t len);
> +void fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
>                          target_phys_addr_t crl_addr, target_phys_addr_t data_addr);
>  
>  #endif /* NO_QEMU_PROTOS */
> diff --git a/hw/loader.c b/hw/loader.c
> index 415cdce..28464da 100644
> --- a/hw/loader.c
> +++ b/hw/loader.c
> @@ -543,7 +543,6 @@ struct Rom {
>      QTAILQ_ENTRY(Rom) next;
>  };
>  
> -static FWCfgState *fw_cfg;
>  static QTAILQ_HEAD(, Rom) roms = QTAILQ_HEAD_INITIALIZER(roms);
>  
>  static void rom_insert(Rom *rom)
> @@ -601,7 +600,7 @@ int rom_add_file(const char *file, const char *fw_dir,
>      }
>      close(fd);
>      rom_insert(rom);
> -    if (rom->fw_file && fw_cfg) {
> +    if (rom->fw_file) {
>          const char *basename;
>          char fw_file_name[56];
>  
> @@ -613,7 +612,7 @@ int rom_add_file(const char *file, const char *fw_dir,
>          }
>          snprintf(fw_file_name, sizeof(fw_file_name), "%s/%s", rom->fw_dir,
>                   basename);
> -        fw_cfg_add_file(fw_cfg, fw_file_name, rom->data, rom->romsize);
> +        fw_cfg_add_file(fw_file_name, rom->data, rom->romsize);
>          snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name);
>      } else {
>          snprintf(devpath, sizeof(devpath), "/rom@" TARGET_FMT_plx, addr);
> @@ -704,11 +703,6 @@ int rom_load_all(void)
>      return 0;
>  }
>  
> -void rom_set_fw(void *f)
> -{
> -    fw_cfg = f;
> -}
> -
>  static Rom *find_rom(target_phys_addr_t addr)
>  {
>      Rom *rom;
> diff --git a/hw/loader.h b/hw/loader.h
> index fbcaba9..c357ec4 100644
> --- a/hw/loader.h
> +++ b/hw/loader.h
> @@ -26,7 +26,6 @@ int rom_add_file(const char *file, const char *fw_dir,
>  int rom_add_blob(const char *name, const void *blob, size_t len,
>                   target_phys_addr_t addr);
>  int rom_load_all(void);
> -void rom_set_fw(void *f);
>  int rom_copy(uint8_t *dest, target_phys_addr_t addr, size_t size);
>  void *rom_ptr(target_phys_addr_t addr);
>  void do_info_roms(Monitor *mon);
> diff --git a/hw/multiboot.c b/hw/multiboot.c
> index b4484a3..fd93e13 100644
> --- a/hw/multiboot.c
> +++ b/hw/multiboot.c
> @@ -124,8 +124,7 @@ static void mb_add_mod(MultibootState *s,
>      s->mb_mods_count++;
>  }
>  
> -int load_multiboot(void *fw_cfg,
> -                   FILE *f,
> +int load_multiboot(FILE *f,
>                     const char *kernel_filename,
>                     const char *initrd_filename,
>                     const char *kernel_cmdline,
> @@ -324,15 +323,15 @@ int load_multiboot(void *fw_cfg,
>      memcpy(mb_bootinfo_data, bootinfo, sizeof(bootinfo));
>  
>      /* Pass variables to option rom */
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, mh_entry_addr);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, mbs.mb_buf_size);
> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA,
> +    fw_cfg_add_i32(FW_CFG_KERNEL_ENTRY, mh_entry_addr);
> +    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, mh_load_addr);
> +    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, mbs.mb_buf_size);
> +    fw_cfg_add_bytes(FW_CFG_KERNEL_DATA,
>                       mbs.mb_buf, mbs.mb_buf_size);
>  
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, ADDR_MBI);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, sizeof(bootinfo));
> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, mb_bootinfo_data,
> +    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, ADDR_MBI);
> +    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, sizeof(bootinfo));
> +    fw_cfg_add_bytes(FW_CFG_INITRD_DATA, mb_bootinfo_data,
>                       sizeof(bootinfo));
>  
>      option_rom[nb_option_roms].name = "multiboot.bin";
> diff --git a/hw/multiboot.h b/hw/multiboot.h
> index 98fb1b7..1302c55 100644
> --- a/hw/multiboot.h
> +++ b/hw/multiboot.h
> @@ -1,8 +1,7 @@
>  #ifndef QEMU_MULTIBOOT_H
>  #define QEMU_MULTIBOOT_H
>  
> -int load_multiboot(void *fw_cfg,
> -                   FILE *f,
> +int load_multiboot(FILE *f,
>                     const char *kernel_filename,
>                     const char *initrd_filename,
>                     const char *kernel_cmdline,
> diff --git a/hw/pc.c b/hw/pc.c
> index 4d34a33..739e4ad 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -593,9 +593,8 @@ int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
>      return index;
>  }
>  
> -static void *bochs_bios_init(void)
> +static void bochs_bios_init(void)
>  {
> -    void *fw_cfg;
>      uint8_t *smbios_table;
>      size_t smbios_len;
>      uint64_t *numa_fw_cfg;
> @@ -613,22 +612,21 @@ static void *bochs_bios_init(void)
>      register_ioport_write(0x500, 1, 1, bochs_bios_write, NULL);
>      register_ioport_write(0x503, 1, 1, bochs_bios_write, NULL);
>  
> -    fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
> +    fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
>  
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
> -    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES, (uint8_t *)acpi_tables,
> +    fw_cfg_add_i32(FW_CFG_ID, 1);
> +    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> +    fw_cfg_add_bytes(FW_CFG_ACPI_TABLES, (uint8_t *)acpi_tables,
>                       acpi_tables_len);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
> +    fw_cfg_add_i32(FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
>  
>      smbios_table = smbios_get_table(&smbios_len);
>      if (smbios_table)
> -        fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
> -                         smbios_table, smbios_len);
> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE, (uint8_t *)&e820_table,
> +        fw_cfg_add_bytes(FW_CFG_SMBIOS_ENTRIES, smbios_table, smbios_len);
> +    fw_cfg_add_bytes(FW_CFG_E820_TABLE, (uint8_t *)&e820_table,
>                       sizeof(struct e820_table));
>  
> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, (uint8_t *)&hpet_cfg,
> +    fw_cfg_add_bytes(FW_CFG_HPET, (uint8_t *)&hpet_cfg,
>                       sizeof(struct hpet_fw_config));
>      /* allocate memory for the NUMA channel: one (64bit) word for the number
>       * of nodes, one word for each VCPU->node and one word for each node to
> @@ -647,10 +645,8 @@ static void *bochs_bios_init(void)
>      for (i = 0; i < nb_numa_nodes; i++) {
>          numa_fw_cfg[max_cpus + 1 + i] = cpu_to_le64(node_mem[i]);
>      }
> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, (uint8_t *)numa_fw_cfg,
> +    fw_cfg_add_bytes(FW_CFG_NUMA, (uint8_t *)numa_fw_cfg,
>                       (1 + max_cpus + nb_numa_nodes) * 8);
> -
> -    return fw_cfg;
>  }
>  
>  static long get_file_size(FILE *f)
> @@ -667,8 +663,7 @@ static long get_file_size(FILE *f)
>      return size;
>  }
>  
> -static void load_linux(void *fw_cfg,
> -                       const char *kernel_filename,
> +static void load_linux(const char *kernel_filename,
>  		       const char *initrd_filename,
>  		       const char *kernel_cmdline,
>                         target_phys_addr_t max_ram_size)
> @@ -703,9 +698,10 @@ static void load_linux(void *fw_cfg,
>      else {
>  	/* This looks like a multiboot kernel. If it is, let's stop
>  	   treating it like a Linux kernel. */
> -        if (load_multiboot(fw_cfg, f, kernel_filename, initrd_filename,
> -                           kernel_cmdline, kernel_size, header))
> +        if (load_multiboot(f, kernel_filename, initrd_filename,
> +                           kernel_cmdline, kernel_size, header)) {
>              return;
> +        }
>  	protocol = 0;
>      }
>  
> @@ -745,9 +741,9 @@ static void load_linux(void *fw_cfg,
>      if (initrd_max >= max_ram_size-ACPI_DATA_SIZE)
>      	initrd_max = max_ram_size-ACPI_DATA_SIZE-1;
>  
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_ADDR, cmdline_addr);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, strlen(kernel_cmdline)+1);
> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
> +    fw_cfg_add_i32(FW_CFG_CMDLINE_ADDR, cmdline_addr);
> +    fw_cfg_add_i32(FW_CFG_CMDLINE_SIZE, strlen(kernel_cmdline)+1);
> +    fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
>                       (uint8_t*)strdup(kernel_cmdline),
>                       strlen(kernel_cmdline)+1);
>  
> @@ -808,9 +804,9 @@ static void load_linux(void *fw_cfg,
>          initrd_data = g_malloc(initrd_size);
>          load_image(initrd_filename, initrd_data);
>  
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr);
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
> -        fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, initrd_data, initrd_size);
> +        fw_cfg_add_i32(FW_CFG_INITRD_ADDR, initrd_addr);
> +        fw_cfg_add_i32(FW_CFG_INITRD_SIZE, initrd_size);
> +        fw_cfg_add_bytes(FW_CFG_INITRD_DATA, initrd_data, initrd_size);
>  
>  	stl_p(header+0x218, initrd_addr);
>  	stl_p(header+0x21c, initrd_size);
> @@ -837,13 +833,13 @@ static void load_linux(void *fw_cfg,
>      fclose(f);
>      memcpy(setup, header, MIN(sizeof(header), setup_size));
>  
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, prot_addr);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA, kernel, kernel_size);
> +    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, prot_addr);
> +    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
> +    fw_cfg_add_bytes(FW_CFG_KERNEL_DATA, kernel, kernel_size);
>  
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_ADDR, real_addr);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_SIZE, setup_size);
> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA, setup, setup_size);
> +    fw_cfg_add_i32(FW_CFG_SETUP_ADDR, real_addr);
> +    fw_cfg_add_i32(FW_CFG_SETUP_SIZE, setup_size);
> +    fw_cfg_add_bytes(FW_CFG_SETUP_DATA, setup, setup_size);
>  
>      option_rom[nb_option_roms].name = "linuxboot.bin";
>      option_rom[nb_option_roms].bootindex = 0;
> @@ -987,7 +983,6 @@ void pc_memory_init(MemoryRegion *system_memory,
>      int linux_boot, i;
>      MemoryRegion *ram, *option_rom_mr;
>      MemoryRegion *ram_below_4g, *ram_above_4g;
> -    void *fw_cfg;
>  
>      linux_boot = (kernel_filename != NULL);
>  
> @@ -1024,11 +1019,11 @@ void pc_memory_init(MemoryRegion *system_memory,
>                                          option_rom_mr,
>                                          1);
>  
> -    fw_cfg = bochs_bios_init();
> -    rom_set_fw(fw_cfg);
> +    bochs_bios_init();
>  
>      if (linux_boot) {
> -        load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline, below_4g_mem_size);
> +        load_linux(kernel_filename, initrd_filename, kernel_cmdline,
> +                   below_4g_mem_size);
>      }
>  
>      for (i = 0; i < nb_option_roms; i++) {
> diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
> index 8796510..1f2a30e 100644
> --- a/hw/ppc_newworld.c
> +++ b/hw/ppc_newworld.c
> @@ -107,7 +107,7 @@ static const MemoryRegionOps unin_ops = {
>  
>  static int fw_cfg_boot_set(void *opaque, const char *boot_device)
>  {
> -    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
>      return 0;
>  }
>  
> @@ -152,7 +152,6 @@ static void ppc_core99_init (ram_addr_t ram_size,
>      MemoryRegion *ide_mem[3];
>      int ppc_boot_device;
>      DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> -    void *fw_cfg;
>      void *dbdma;
>      int machine_arch;
>  
> @@ -377,42 +376,42 @@ static void ppc_core99_init (ram_addr_t ram_size,
>      macio_nvram_setup_bar(nvr, get_system_memory(), 0xFFF04000);
>      /* No PCI init: the BIOS will do it */
>  
> -    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
> -    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, machine_arch);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, kernel_base);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
> +    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> +    fw_cfg_add_i32(FW_CFG_ID, 1);
> +    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> +    fw_cfg_add_i16(FW_CFG_MACHINE_ID, machine_arch);
> +    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, kernel_base);
> +    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
>      if (kernel_cmdline) {
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, cmdline_base);
> +        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, cmdline_base);
>          pstrcpy_targphys("cmdline", cmdline_base, TARGET_PAGE_SIZE, kernel_cmdline);
>      } else {
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
> +        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, 0);
>      }
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_base);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, ppc_boot_device);
> +    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, initrd_base);
> +    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, initrd_size);
> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, ppc_boot_device);
>  
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_WIDTH, graphic_width);
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_HEIGHT, graphic_height);
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_DEPTH, graphic_depth);
> +    fw_cfg_add_i16(FW_CFG_PPC_WIDTH, graphic_width);
> +    fw_cfg_add_i16(FW_CFG_PPC_HEIGHT, graphic_height);
> +    fw_cfg_add_i16(FW_CFG_PPC_DEPTH, graphic_depth);
>  
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_IS_KVM, kvm_enabled());
> +    fw_cfg_add_i32(FW_CFG_PPC_IS_KVM, kvm_enabled());
>      if (kvm_enabled()) {
>  #ifdef CONFIG_KVM
>          uint8_t *hypercall;
>  
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, kvmppc_get_tbfreq());
> +        fw_cfg_add_i32(FW_CFG_PPC_TBFREQ, kvmppc_get_tbfreq());
>          hypercall = g_malloc(16);
>          kvmppc_get_hypercall(env, hypercall, 16);
> -        fw_cfg_add_bytes(fw_cfg, FW_CFG_PPC_KVM_HC, hypercall, 16);
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_KVM_PID, getpid());
> +        fw_cfg_add_bytes(FW_CFG_PPC_KVM_HC, hypercall, 16);
> +        fw_cfg_add_i32(FW_CFG_PPC_KVM_PID, getpid());
>  #endif
>      } else {
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, get_ticks_per_sec());
> +        fw_cfg_add_i32(FW_CFG_PPC_TBFREQ, get_ticks_per_sec());
>      }
>  
> -    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
> +    qemu_register_boot_set(fw_cfg_boot_set, NULL);
>  }
>  
>  static QEMUMachine core99_machine = {
> diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c
> index 7e73d37..575fe59 100644
> --- a/hw/ppc_oldworld.c
> +++ b/hw/ppc_oldworld.c
> @@ -50,7 +50,7 @@
>  
>  static int fw_cfg_boot_set(void *opaque, const char *boot_device)
>  {
> -    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
>      return 0;
>  }
>  
> @@ -95,7 +95,6 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
>      MemoryRegion *escc_mem, *escc_bar = g_new(MemoryRegion, 1), *ide_mem[2];
>      uint16_t ppc_boot_device;
>      DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> -    void *fw_cfg;
>      void *dbdma;
>  
>      linux_boot = (kernel_filename != NULL);
> @@ -292,42 +291,42 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
>  
>      /* No PCI init: the BIOS will do it */
>  
> -    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
> -    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, ARCH_HEATHROW);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, kernel_base);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
> +    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> +    fw_cfg_add_i32(FW_CFG_ID, 1);
> +    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> +    fw_cfg_add_i16(FW_CFG_MACHINE_ID, ARCH_HEATHROW);
> +    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, kernel_base);
> +    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
>      if (kernel_cmdline) {
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, cmdline_base);
> +        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, cmdline_base);
>          pstrcpy_targphys("cmdline", cmdline_base, TARGET_PAGE_SIZE, kernel_cmdline);
>      } else {
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
> +        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, 0);
>      }
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_base);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, ppc_boot_device);
> +    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, initrd_base);
> +    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, initrd_size);
> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, ppc_boot_device);
>  
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_WIDTH, graphic_width);
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_HEIGHT, graphic_height);
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_DEPTH, graphic_depth);
> +    fw_cfg_add_i16(FW_CFG_PPC_WIDTH, graphic_width);
> +    fw_cfg_add_i16(FW_CFG_PPC_HEIGHT, graphic_height);
> +    fw_cfg_add_i16(FW_CFG_PPC_DEPTH, graphic_depth);
>  
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_IS_KVM, kvm_enabled());
> +    fw_cfg_add_i32(FW_CFG_PPC_IS_KVM, kvm_enabled());
>      if (kvm_enabled()) {
>  #ifdef CONFIG_KVM
>          uint8_t *hypercall;
>  
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, kvmppc_get_tbfreq());
> +        fw_cfg_add_i32(FW_CFG_PPC_TBFREQ, kvmppc_get_tbfreq());
>          hypercall = g_malloc(16);
>          kvmppc_get_hypercall(env, hypercall, 16);
> -        fw_cfg_add_bytes(fw_cfg, FW_CFG_PPC_KVM_HC, hypercall, 16);
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_KVM_PID, getpid());
> +        fw_cfg_add_bytes(FW_CFG_PPC_KVM_HC, hypercall, 16);
> +        fw_cfg_add_i32(FW_CFG_PPC_KVM_PID, getpid());
>  #endif
>      } else {
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, get_ticks_per_sec());
> +        fw_cfg_add_i32(FW_CFG_PPC_TBFREQ, get_ticks_per_sec());
>      }
>  
> -    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
> +    qemu_register_boot_set(fw_cfg_boot_set, NULL);
>  }
>  
>  static QEMUMachine heathrow_machine = {
> diff --git a/hw/sun4m.c b/hw/sun4m.c
> index 34088ad..9f546d6 100644
> --- a/hw/sun4m.c
> +++ b/hw/sun4m.c
> @@ -163,7 +163,7 @@ void DMA_register_channel (int nchan,
>  
>  static int fw_cfg_boot_set(void *opaque, const char *boot_device)
>  {
> -    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
>      return 0;
>  }
>  
> @@ -843,7 +843,6 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
>      qemu_irq *cpu_halt;
>      unsigned long kernel_size;
>      DriveInfo *fd[MAX_FD];
> -    void *fw_cfg;
>      unsigned int num_vsimms;
>  
>      /* init CPUs */
> @@ -995,29 +994,29 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
>          ecc_init(hwdef->ecc_base, slavio_irq[28],
>                   hwdef->ecc_version);
>  
> -    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
> -    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, hwdef->machine_id);
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_SUN4M_DEPTH, graphic_depth);
> -    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);
> +    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> +    fw_cfg_add_i32(FW_CFG_ID, 1);
> +    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> +    fw_cfg_add_i16(FW_CFG_MACHINE_ID, hwdef->machine_id);
> +    fw_cfg_add_i16(FW_CFG_SUN4M_DEPTH, graphic_depth);
> +    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, KERNEL_LOAD_ADDR);
> +    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
>      if (kernel_cmdline) {
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
> +        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
>          pstrcpy_targphys("cmdline", CMDLINE_ADDR, TARGET_PAGE_SIZE, kernel_cmdline);
> -        fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
> +        fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
>                           (uint8_t*)strdup(kernel_cmdline),
>                           strlen(kernel_cmdline) + 1);
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,
> +        fw_cfg_add_i32(FW_CFG_CMDLINE_SIZE,
>                         strlen(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_KERNEL_CMDLINE, 0);
> +        fw_cfg_add_i32(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]);
> -    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
> +    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, INITRD_LOAD_ADDR);
> +    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, 0); /* not used */
> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
> +    qemu_register_boot_set(fw_cfg_boot_set, NULL);
>  }
>  
>  enum {
> @@ -1525,7 +1524,6 @@ static void sun4d_hw_init(const struct sun4d_hwdef *hwdef, ram_addr_t RAM_size,
>          espdma_irq, ledma_irq;
>      qemu_irq esp_reset, dma_enable;
>      unsigned long kernel_size;
> -    void *fw_cfg;
>      DeviceState *dev;
>  
>      /* init CPUs */
> @@ -1606,26 +1604,26 @@ static void sun4d_hw_init(const struct sun4d_hwdef *hwdef, ram_addr_t RAM_size,
>                 graphic_height, graphic_depth, hwdef->nvram_machine_id,
>                 "Sun4d");
>  
> -    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
> -    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, hwdef->machine_id);
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_SUN4M_DEPTH, graphic_depth);
> -    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);
> +    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> +    fw_cfg_add_i32(FW_CFG_ID, 1);
> +    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> +    fw_cfg_add_i16(FW_CFG_MACHINE_ID, hwdef->machine_id);
> +    fw_cfg_add_i16(FW_CFG_SUN4M_DEPTH, graphic_depth);
> +    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, KERNEL_LOAD_ADDR);
> +    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
>      if (kernel_cmdline) {
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
> +        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
>          pstrcpy_targphys("cmdline", CMDLINE_ADDR, TARGET_PAGE_SIZE, kernel_cmdline);
> -        fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
> +        fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
>                           (uint8_t*)strdup(kernel_cmdline),
>                           strlen(kernel_cmdline) + 1);
>      } else {
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
> +        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, 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]);
> -    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
> +    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, INITRD_LOAD_ADDR);
> +    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, 0); /* not used */
> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
> +    qemu_register_boot_set(fw_cfg_boot_set, NULL);
>  }
>  
>  /* SPARCserver 1000 hardware initialisation */
> @@ -1719,7 +1717,6 @@ static void sun4c_hw_init(const struct sun4c_hwdef *hwdef, ram_addr_t RAM_size,
>      qemu_irq fdc_tc;
>      unsigned long kernel_size;
>      DriveInfo *fd[MAX_FD];
> -    void *fw_cfg;
>      DeviceState *dev;
>      unsigned int i;
>  
> @@ -1798,26 +1795,26 @@ static void sun4c_hw_init(const struct sun4c_hwdef *hwdef, ram_addr_t RAM_size,
>                 graphic_height, graphic_depth, hwdef->nvram_machine_id,
>                 "Sun4c");
>  
> -    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
> -    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, hwdef->machine_id);
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_SUN4M_DEPTH, graphic_depth);
> -    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);
> +    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> +    fw_cfg_add_i32(FW_CFG_ID, 1);
> +    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> +    fw_cfg_add_i16(FW_CFG_MACHINE_ID, hwdef->machine_id);
> +    fw_cfg_add_i16(FW_CFG_SUN4M_DEPTH, graphic_depth);
> +    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, KERNEL_LOAD_ADDR);
> +    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
>      if (kernel_cmdline) {
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
> +        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
>          pstrcpy_targphys("cmdline", CMDLINE_ADDR, TARGET_PAGE_SIZE, kernel_cmdline);
> -        fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
> +        fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
>                           (uint8_t*)strdup(kernel_cmdline),
>                           strlen(kernel_cmdline) + 1);
>      } else {
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
> +        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, 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]);
> -    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
> +    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, INITRD_LOAD_ADDR);
> +    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, 0); /* not used */
> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
> +    qemu_register_boot_set(fw_cfg_boot_set, NULL);
>  }
>  
>  /* SPARCstation 2 hardware initialisation */
> diff --git a/hw/sun4u.c b/hw/sun4u.c
> index 517bdb8..094c07c 100644
> --- a/hw/sun4u.c
> +++ b/hw/sun4u.c
> @@ -125,7 +125,7 @@ void DMA_register_channel (int nchan,
>  
>  static int fw_cfg_boot_set(void *opaque, const char *boot_device)
>  {
> -    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
>      return 0;
>  }
>  
> @@ -802,7 +802,6 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      qemu_irq *ivec_irqs, *pbm_irqs;
>      DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>      DriveInfo *fd[MAX_FD];
> -    void *fw_cfg;
>  
>      /* init CPUs */
>      env = cpu_devinit(cpu_model, hwdef);
> @@ -867,30 +866,30 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>                             graphic_width, graphic_height, graphic_depth,
>                             (uint8_t *)&nd_table[0].macaddr);
>  
> -    fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
> -    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> -    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);
> +    fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
> +    fw_cfg_add_i32(FW_CFG_ID, 1);
> +    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> +    fw_cfg_add_i16(FW_CFG_MACHINE_ID, hwdef->machine_id);
> +    fw_cfg_add_i64(FW_CFG_KERNEL_ADDR, kernel_entry);
> +    fw_cfg_add_i64(FW_CFG_KERNEL_SIZE, kernel_size);
>      if (kernel_cmdline) {
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,
> +        fw_cfg_add_i32(FW_CFG_CMDLINE_SIZE,
>                         strlen(kernel_cmdline) + 1);
> -        fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
> +        fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
>                           (uint8_t*)strdup(kernel_cmdline),
>                           strlen(kernel_cmdline) + 1);
>      } else {
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, 0);
> +        fw_cfg_add_i32(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_i64(FW_CFG_INITRD_ADDR, initrd_addr);
> +    fw_cfg_add_i64(FW_CFG_INITRD_SIZE, initrd_size);
> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_devices[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);
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_SPARC64_DEPTH, graphic_depth);
> +    fw_cfg_add_i16(FW_CFG_SPARC64_WIDTH, graphic_width);
> +    fw_cfg_add_i16(FW_CFG_SPARC64_HEIGHT, graphic_height);
> +    fw_cfg_add_i16(FW_CFG_SPARC64_DEPTH, graphic_depth);
>  
> -    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
> +    qemu_register_boot_set(fw_cfg_boot_set, NULL);
>  }
>  
>  enum {
> -- 
> 1.7.7.3
> 

--
			Gleb.

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

* Re: [Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global.
  2012-05-20  9:02 [Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global Gleb Natapov
  2012-05-20  9:02 ` [Qemu-devel] [PATCH 2/2] Add PIIX4 properties to control PM system states Gleb Natapov
  2012-05-23 12:37 ` [Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global Gleb Natapov
@ 2012-05-23 12:44 ` Peter Maydell
  2012-05-23 12:47   ` Gleb Natapov
  2012-05-23 14:41   ` Andreas Färber
  2 siblings, 2 replies; 18+ messages in thread
From: Peter Maydell @ 2012-05-23 12:44 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: qemu-devel

On 20 May 2012 10:02, Gleb Natapov <gleb@redhat.com> wrote:
> There can be only one fw_cfg device, so saving global reference to it
> removes the need to pass its pointer around.

This seems like a backwards step to me: one of the things that prevents
us supporting "two separate machines in one emulation" is that we have
various things that assume there's only one instance of themselves and
use globals. I don't think we should be adding any more...

-- PMM

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

* Re: [Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global.
  2012-05-23 12:44 ` Peter Maydell
@ 2012-05-23 12:47   ` Gleb Natapov
  2012-05-23 14:41   ` Andreas Färber
  1 sibling, 0 replies; 18+ messages in thread
From: Gleb Natapov @ 2012-05-23 12:47 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel

On Wed, May 23, 2012 at 01:44:45PM +0100, Peter Maydell wrote:
> On 20 May 2012 10:02, Gleb Natapov <gleb@redhat.com> wrote:
> > There can be only one fw_cfg device, so saving global reference to it
> > removes the need to pass its pointer around.
> 
> This seems like a backwards step to me: one of the things that prevents
> us supporting "two separate machines in one emulation" is that we have
> various things that assume there's only one instance of themselves and
> use globals. I don't think we should be adding any more...
> 
So you do it usual way. Have "struct Machine" and put pointer to fw_cfg
there. Current code already saves global pointer to fw_cfg, the patch just
makes it official.

--
			Gleb.

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

* Re: [Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global.
  2012-05-23 12:37 ` [Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global Gleb Natapov
@ 2012-05-23 13:25   ` Anthony Liguori
  2012-05-23 13:32     ` Gleb Natapov
  0 siblings, 1 reply; 18+ messages in thread
From: Anthony Liguori @ 2012-05-23 13:25 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: blauwirbel, qemu-devel

On 05/23/2012 07:37 AM, Gleb Natapov wrote:
> Ping.

I don't understand why you're pinging..  The patch has just been on the list for 
a couple of days and is definitely not a 1.1 candidate.  Am I missing something?

However...

>
> On Sun, May 20, 2012 at 12:02:40PM +0300, Gleb Natapov wrote:
>> There can be only one fw_cfg device, so saving global reference to it
>> removes the need to pass its pointer around.
>>
>> Signed-off-by: Gleb Natapov<gleb@redhat.com>
>> ---
>>   hw/fw_cfg.c       |  110 +++++++++++++++++++++++++++--------------------------
>>   hw/fw_cfg.h       |   15 +++----
>>   hw/loader.c       |   10 +----
>>   hw/loader.h       |    1 -
>>   hw/multiboot.c    |   17 ++++----
>>   hw/multiboot.h    |    3 +-
>>   hw/pc.c           |   63 ++++++++++++++----------------
>>   hw/ppc_newworld.c |   43 ++++++++++-----------
>>   hw/ppc_oldworld.c |   43 ++++++++++-----------
>>   hw/sun4m.c        |   93 +++++++++++++++++++++-----------------------
>>   hw/sun4u.c        |   35 ++++++++---------
>>   11 files changed, 207 insertions(+), 226 deletions(-)
>>
>> diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
>> index 7b3b576..8c50473 100644
>> --- a/hw/fw_cfg.c
>> +++ b/hw/fw_cfg.c
>> @@ -48,7 +48,7 @@ typedef struct FWCfgEntry {
>>       FWCfgCallback callback;
>>   } FWCfgEntry;
>>
>> -struct FWCfgState {
>> +static struct FWCfgState {
>>       SysBusDevice busdev;
>>       MemoryRegion ctl_iomem, data_iomem, comb_iomem;
>>       uint32_t ctl_iobase, data_iobase;
>> @@ -57,7 +57,7 @@ struct FWCfgState {
>>       uint16_t cur_entry;
>>       uint32_t cur_offset;
>>       Notifier machine_ready;
>> -};
>> +} *fw_cfg;
>>
>>   #define JPG_FILE 0
>>   #define BMP_FILE 1
>> @@ -113,7 +113,7 @@ error:
>>       return NULL;
>>   }
>>
>> -static void fw_cfg_bootsplash(FWCfgState *s)

This is a step backwards IMHO.  Relying on global state is generally a bad 
thing.  Passing around pointers is a good thing because it forces you to think 
about the relationships between devices and makes it hard to do silly things 
(like have random interactions with fw_cfg in devices that have no business 
doing that).

Regards,

Anthony Liguori

>> +static void fw_cfg_bootsplash(void)
>>   {
>>       int boot_splash_time = -1;
>>       const char *boot_splash_filename = NULL;
>> @@ -148,7 +148,7 @@ static void fw_cfg_bootsplash(FWCfgState *s)
>>           /* use little endian format */
>>           qemu_extra_params_fw[0] = (uint8_t)(boot_splash_time&  0xff);
>>           qemu_extra_params_fw[1] = (uint8_t)((boot_splash_time>>  8)&  0xff);
>> -        fw_cfg_add_file(s, "etc/boot-menu-wait", qemu_extra_params_fw, 2);
>> +        fw_cfg_add_file("etc/boot-menu-wait", qemu_extra_params_fw, 2);
>>       }
>>
>>       /* insert splash file if user configurated */
>> @@ -173,10 +173,10 @@ static void fw_cfg_bootsplash(FWCfgState *s)
>>
>>           /* insert data */
>>           if (file_type == JPG_FILE) {
>> -            fw_cfg_add_file(s, "bootsplash.jpg",
>> +            fw_cfg_add_file("bootsplash.jpg",
>>                       boot_splash_filedata, boot_splash_filedata_size);
>>           } else {
>> -            fw_cfg_add_file(s, "bootsplash.bmp",
>> +            fw_cfg_add_file("bootsplash.bmp",
>>                       boot_splash_filedata, boot_splash_filedata_size);
>>           }
>>           g_free(filename);
>> @@ -359,122 +359,126 @@ static const VMStateDescription vmstate_fw_cfg = {
>>       }
>>   };
>>
>> -int fw_cfg_add_bytes(FWCfgState *s, uint16_t key, uint8_t *data, uint32_t len)
>> +int fw_cfg_add_bytes(uint16_t key, uint8_t *data, uint32_t len)
>>   {
>>       int arch = !!(key&  FW_CFG_ARCH_LOCAL);
>>
>>       key&= FW_CFG_ENTRY_MASK;
>>
>> -    if (key>= FW_CFG_MAX_ENTRY)
>> +    if (key>= FW_CFG_MAX_ENTRY || !fw_cfg) {
>>           return 0;
>> +    }
>>
>> -    s->entries[arch][key].data = data;
>> -    s->entries[arch][key].len = len;
>> +    fw_cfg->entries[arch][key].data = data;
>> +    fw_cfg->entries[arch][key].len = len;
>>
>>       return 1;
>>   }
>>
>> -int fw_cfg_add_i16(FWCfgState *s, uint16_t key, uint16_t value)
>> +int fw_cfg_add_i16(uint16_t key, uint16_t value)
>>   {
>>       uint16_t *copy;
>>
>>       copy = g_malloc(sizeof(value));
>>       *copy = cpu_to_le16(value);
>> -    return fw_cfg_add_bytes(s, key, (uint8_t *)copy, sizeof(value));
>> +    return fw_cfg_add_bytes(key, (uint8_t *)copy, sizeof(value));
>>   }
>>
>> -int fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t value)
>> +int fw_cfg_add_i32(uint16_t key, uint32_t value)
>>   {
>>       uint32_t *copy;
>>
>>       copy = g_malloc(sizeof(value));
>>       *copy = cpu_to_le32(value);
>> -    return fw_cfg_add_bytes(s, key, (uint8_t *)copy, sizeof(value));
>> +    return fw_cfg_add_bytes(key, (uint8_t *)copy, sizeof(value));
>>   }
>>
>> -int fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t value)
>> +int fw_cfg_add_i64(uint16_t key, uint64_t value)
>>   {
>>       uint64_t *copy;
>>
>>       copy = g_malloc(sizeof(value));
>>       *copy = cpu_to_le64(value);
>> -    return fw_cfg_add_bytes(s, key, (uint8_t *)copy, sizeof(value));
>> +    return fw_cfg_add_bytes(key, (uint8_t *)copy, sizeof(value));
>>   }
>>
>> -int fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback,
>> +int fw_cfg_add_callback(uint16_t key, FWCfgCallback callback,
>>                           void *callback_opaque, uint8_t *data, size_t len)
>>   {
>>       int arch = !!(key&  FW_CFG_ARCH_LOCAL);
>>
>> -    if (!(key&  FW_CFG_WRITE_CHANNEL))
>> +    if (!(key&  FW_CFG_WRITE_CHANNEL) || !fw_cfg) {
>>           return 0;
>> +    }
>>
>>       key&= FW_CFG_ENTRY_MASK;
>>
>>       if (key>= FW_CFG_MAX_ENTRY || len>  65535)
>>           return 0;
>>
>> -    s->entries[arch][key].data = data;
>> -    s->entries[arch][key].len = len;
>> -    s->entries[arch][key].callback_opaque = callback_opaque;
>> -    s->entries[arch][key].callback = callback;
>> +    fw_cfg->entries[arch][key].data = data;
>> +    fw_cfg->entries[arch][key].len = len;
>> +    fw_cfg->entries[arch][key].callback_opaque = callback_opaque;
>> +    fw_cfg->entries[arch][key].callback = callback;
>>
>>       return 1;
>>   }
>>
>> -int fw_cfg_add_file(FWCfgState *s,  const char *filename, uint8_t *data,
>> -                    uint32_t len)
>> +int fw_cfg_add_file(const char *filename, uint8_t *data, uint32_t len)
>>   {
>>       int i, index;
>>
>> -    if (!s->files) {
>> +    if (!fw_cfg) {
>> +        return 0;
>> +    }
>> +
>> +    if (!fw_cfg->files) {
>>           int dsize = sizeof(uint32_t) + sizeof(FWCfgFile) * FW_CFG_FILE_SLOTS;
>> -        s->files = g_malloc0(dsize);
>> -        fw_cfg_add_bytes(s, FW_CFG_FILE_DIR, (uint8_t*)s->files, dsize);
>> +        fw_cfg->files = g_malloc0(dsize);
>> +        fw_cfg_add_bytes(FW_CFG_FILE_DIR, (uint8_t *)fw_cfg->files, dsize);
>>       }
>>
>> -    index = be32_to_cpu(s->files->count);
>> +    index = be32_to_cpu(fw_cfg->files->count);
>>       if (index == FW_CFG_FILE_SLOTS) {
>>           fprintf(stderr, "fw_cfg: out of file slots\n");
>>           return 0;
>>       }
>>
>> -    fw_cfg_add_bytes(s, FW_CFG_FILE_FIRST + index, data, len);
>> +    fw_cfg_add_bytes(FW_CFG_FILE_FIRST + index, data, len);
>>
>> -    pstrcpy(s->files->f[index].name, sizeof(s->files->f[index].name),
>> +    pstrcpy(fw_cfg->files->f[index].name, sizeof(fw_cfg->files->f[index].name),
>>               filename);
>>       for (i = 0; i<  index; i++) {
>> -        if (strcmp(s->files->f[index].name, s->files->f[i].name) == 0) {
>> +        if (strcmp(fw_cfg->files->f[index].name, fw_cfg->files->f[i].name)
>> +                        == 0) {
>>               FW_CFG_DPRINTF("%s: skip duplicate: %s\n", __FUNCTION__,
>> -                           s->files->f[index].name);
>> +                           fw_cfg->files->f[index].name);
>>               return 1;
>>           }
>>       }
>>
>> -    s->files->f[index].size   = cpu_to_be32(len);
>> -    s->files->f[index].select = cpu_to_be16(FW_CFG_FILE_FIRST + index);
>> +    fw_cfg->files->f[index].size   = cpu_to_be32(len);
>> +    fw_cfg->files->f[index].select = cpu_to_be16(FW_CFG_FILE_FIRST + index);
>>       FW_CFG_DPRINTF("%s: #%d: %s (%d bytes)\n", __FUNCTION__,
>> -                   index, s->files->f[index].name, len);
>> +                   index, fw_cfg->files->f[index].name, len);
>>
>> -    s->files->count = cpu_to_be32(index+1);
>> +    fw_cfg->files->count = cpu_to_be32(index+1);
>>       return 1;
>>   }
>>
>>   static void fw_cfg_machine_ready(struct Notifier *n, void *data)
>>   {
>>       uint32_t len;
>> -    FWCfgState *s = container_of(n, FWCfgState, machine_ready);
>>       char *bootindex = get_boot_devices_list(&len);
>>
>> -    fw_cfg_add_file(s, "bootorder", (uint8_t*)bootindex, len);
>> +    fw_cfg_add_file("bootorder", (uint8_t *)bootindex, len);
>>   }
>>
>> -FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
>> -                        target_phys_addr_t ctl_addr, target_phys_addr_t data_addr)
>> +void fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
>> +                 target_phys_addr_t ctl_addr, target_phys_addr_t data_addr)
>>   {
>>       DeviceState *dev;
>>       SysBusDevice *d;
>> -    FWCfgState *s;
>>
>>       dev = qdev_create(NULL, "fw_cfg");
>>       qdev_prop_set_uint32(dev, "ctl_iobase", ctl_port);
>> @@ -482,7 +486,7 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
>>       qdev_init_nofail(dev);
>>       d = sysbus_from_qdev(dev);
>>
>> -    s = DO_UPCAST(FWCfgState, busdev.qdev, dev);
>> +    fw_cfg = DO_UPCAST(FWCfgState, busdev.qdev, dev);
>>
>>       if (ctl_addr) {
>>           sysbus_mmio_map(d, 0, ctl_addr);
>> @@ -490,18 +494,16 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
>>       if (data_addr) {
>>           sysbus_mmio_map(d, 1, data_addr);
>>       }
>> -    fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (uint8_t *)"QEMU", 4);
>> -    fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16);
>> -    fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)(display_type == DT_NOGRAPHIC));
>> -    fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
>> -    fw_cfg_add_i16(s, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
>> -    fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
>> -    fw_cfg_bootsplash(s);
>> -
>> -    s->machine_ready.notify = fw_cfg_machine_ready;
>> -    qemu_add_machine_init_done_notifier(&s->machine_ready);
>> -
>> -    return s;
>> +    fw_cfg_add_bytes(FW_CFG_SIGNATURE, (uint8_t *)"QEMU", 4);
>> +    fw_cfg_add_bytes(FW_CFG_UUID, qemu_uuid, 16);
>> +    fw_cfg_add_i16(FW_CFG_NOGRAPHIC, (uint16_t)(display_type == DT_NOGRAPHIC));
>> +    fw_cfg_add_i16(FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
>> +    fw_cfg_add_i16(FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
>> +    fw_cfg_add_i16(FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
>> +    fw_cfg_bootsplash();
>> +
>> +    fw_cfg->machine_ready.notify = fw_cfg_machine_ready;
>> +    qemu_add_machine_init_done_notifier(&fw_cfg->machine_ready);
>>   }
>>
>>   static int fw_cfg_init1(SysBusDevice *dev)
>> diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
>> index 856bf91..cdb239e 100644
>> --- a/hw/fw_cfg.h
>> +++ b/hw/fw_cfg.h
>> @@ -54,15 +54,14 @@ typedef struct FWCfgFiles {
>>   typedef void (*FWCfgCallback)(void *opaque, uint8_t *data);
>>
>>   typedef struct FWCfgState FWCfgState;
>> -int fw_cfg_add_bytes(FWCfgState *s, uint16_t key, uint8_t *data, uint32_t len);
>> -int fw_cfg_add_i16(FWCfgState *s, uint16_t key, uint16_t value);
>> -int fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t value);
>> -int fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t value);
>> -int fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback,
>> +int fw_cfg_add_bytes(uint16_t key, uint8_t *data, uint32_t len);
>> +int fw_cfg_add_i16(uint16_t key, uint16_t value);
>> +int fw_cfg_add_i32(uint16_t key, uint32_t value);
>> +int fw_cfg_add_i64(uint16_t key, uint64_t value);
>> +int fw_cfg_add_callback(uint16_t key, FWCfgCallback callback,
>>                           void *callback_opaque, uint8_t *data, size_t len);
>> -int fw_cfg_add_file(FWCfgState *s, const char *filename, uint8_t *data,
>> -                    uint32_t len);
>> -FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
>> +int fw_cfg_add_file(const char *filename, uint8_t *data, uint32_t len);
>> +void fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
>>                           target_phys_addr_t crl_addr, target_phys_addr_t data_addr);
>>
>>   #endif /* NO_QEMU_PROTOS */
>> diff --git a/hw/loader.c b/hw/loader.c
>> index 415cdce..28464da 100644
>> --- a/hw/loader.c
>> +++ b/hw/loader.c
>> @@ -543,7 +543,6 @@ struct Rom {
>>       QTAILQ_ENTRY(Rom) next;
>>   };
>>
>> -static FWCfgState *fw_cfg;
>>   static QTAILQ_HEAD(, Rom) roms = QTAILQ_HEAD_INITIALIZER(roms);
>>
>>   static void rom_insert(Rom *rom)
>> @@ -601,7 +600,7 @@ int rom_add_file(const char *file, const char *fw_dir,
>>       }
>>       close(fd);
>>       rom_insert(rom);
>> -    if (rom->fw_file&&  fw_cfg) {
>> +    if (rom->fw_file) {
>>           const char *basename;
>>           char fw_file_name[56];
>>
>> @@ -613,7 +612,7 @@ int rom_add_file(const char *file, const char *fw_dir,
>>           }
>>           snprintf(fw_file_name, sizeof(fw_file_name), "%s/%s", rom->fw_dir,
>>                    basename);
>> -        fw_cfg_add_file(fw_cfg, fw_file_name, rom->data, rom->romsize);
>> +        fw_cfg_add_file(fw_file_name, rom->data, rom->romsize);
>>           snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name);
>>       } else {
>>           snprintf(devpath, sizeof(devpath), "/rom@" TARGET_FMT_plx, addr);
>> @@ -704,11 +703,6 @@ int rom_load_all(void)
>>       return 0;
>>   }
>>
>> -void rom_set_fw(void *f)
>> -{
>> -    fw_cfg = f;
>> -}
>> -
>>   static Rom *find_rom(target_phys_addr_t addr)
>>   {
>>       Rom *rom;
>> diff --git a/hw/loader.h b/hw/loader.h
>> index fbcaba9..c357ec4 100644
>> --- a/hw/loader.h
>> +++ b/hw/loader.h
>> @@ -26,7 +26,6 @@ int rom_add_file(const char *file, const char *fw_dir,
>>   int rom_add_blob(const char *name, const void *blob, size_t len,
>>                    target_phys_addr_t addr);
>>   int rom_load_all(void);
>> -void rom_set_fw(void *f);
>>   int rom_copy(uint8_t *dest, target_phys_addr_t addr, size_t size);
>>   void *rom_ptr(target_phys_addr_t addr);
>>   void do_info_roms(Monitor *mon);
>> diff --git a/hw/multiboot.c b/hw/multiboot.c
>> index b4484a3..fd93e13 100644
>> --- a/hw/multiboot.c
>> +++ b/hw/multiboot.c
>> @@ -124,8 +124,7 @@ static void mb_add_mod(MultibootState *s,
>>       s->mb_mods_count++;
>>   }
>>
>> -int load_multiboot(void *fw_cfg,
>> -                   FILE *f,
>> +int load_multiboot(FILE *f,
>>                      const char *kernel_filename,
>>                      const char *initrd_filename,
>>                      const char *kernel_cmdline,
>> @@ -324,15 +323,15 @@ int load_multiboot(void *fw_cfg,
>>       memcpy(mb_bootinfo_data, bootinfo, sizeof(bootinfo));
>>
>>       /* Pass variables to option rom */
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, mh_entry_addr);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, mbs.mb_buf_size);
>> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA,
>> +    fw_cfg_add_i32(FW_CFG_KERNEL_ENTRY, mh_entry_addr);
>> +    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, mh_load_addr);
>> +    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, mbs.mb_buf_size);
>> +    fw_cfg_add_bytes(FW_CFG_KERNEL_DATA,
>>                        mbs.mb_buf, mbs.mb_buf_size);
>>
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, ADDR_MBI);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, sizeof(bootinfo));
>> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, mb_bootinfo_data,
>> +    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, ADDR_MBI);
>> +    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, sizeof(bootinfo));
>> +    fw_cfg_add_bytes(FW_CFG_INITRD_DATA, mb_bootinfo_data,
>>                        sizeof(bootinfo));
>>
>>       option_rom[nb_option_roms].name = "multiboot.bin";
>> diff --git a/hw/multiboot.h b/hw/multiboot.h
>> index 98fb1b7..1302c55 100644
>> --- a/hw/multiboot.h
>> +++ b/hw/multiboot.h
>> @@ -1,8 +1,7 @@
>>   #ifndef QEMU_MULTIBOOT_H
>>   #define QEMU_MULTIBOOT_H
>>
>> -int load_multiboot(void *fw_cfg,
>> -                   FILE *f,
>> +int load_multiboot(FILE *f,
>>                      const char *kernel_filename,
>>                      const char *initrd_filename,
>>                      const char *kernel_cmdline,
>> diff --git a/hw/pc.c b/hw/pc.c
>> index 4d34a33..739e4ad 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -593,9 +593,8 @@ int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
>>       return index;
>>   }
>>
>> -static void *bochs_bios_init(void)
>> +static void bochs_bios_init(void)
>>   {
>> -    void *fw_cfg;
>>       uint8_t *smbios_table;
>>       size_t smbios_len;
>>       uint64_t *numa_fw_cfg;
>> @@ -613,22 +612,21 @@ static void *bochs_bios_init(void)
>>       register_ioport_write(0x500, 1, 1, bochs_bios_write, NULL);
>>       register_ioport_write(0x503, 1, 1, bochs_bios_write, NULL);
>>
>> -    fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
>> +    fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
>>
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
>> -    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
>> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES, (uint8_t *)acpi_tables,
>> +    fw_cfg_add_i32(FW_CFG_ID, 1);
>> +    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
>> +    fw_cfg_add_bytes(FW_CFG_ACPI_TABLES, (uint8_t *)acpi_tables,
>>                        acpi_tables_len);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
>> +    fw_cfg_add_i32(FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
>>
>>       smbios_table = smbios_get_table(&smbios_len);
>>       if (smbios_table)
>> -        fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
>> -                         smbios_table, smbios_len);
>> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE, (uint8_t *)&e820_table,
>> +        fw_cfg_add_bytes(FW_CFG_SMBIOS_ENTRIES, smbios_table, smbios_len);
>> +    fw_cfg_add_bytes(FW_CFG_E820_TABLE, (uint8_t *)&e820_table,
>>                        sizeof(struct e820_table));
>>
>> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, (uint8_t *)&hpet_cfg,
>> +    fw_cfg_add_bytes(FW_CFG_HPET, (uint8_t *)&hpet_cfg,
>>                        sizeof(struct hpet_fw_config));
>>       /* allocate memory for the NUMA channel: one (64bit) word for the number
>>        * of nodes, one word for each VCPU->node and one word for each node to
>> @@ -647,10 +645,8 @@ static void *bochs_bios_init(void)
>>       for (i = 0; i<  nb_numa_nodes; i++) {
>>           numa_fw_cfg[max_cpus + 1 + i] = cpu_to_le64(node_mem[i]);
>>       }
>> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, (uint8_t *)numa_fw_cfg,
>> +    fw_cfg_add_bytes(FW_CFG_NUMA, (uint8_t *)numa_fw_cfg,
>>                        (1 + max_cpus + nb_numa_nodes) * 8);
>> -
>> -    return fw_cfg;
>>   }
>>
>>   static long get_file_size(FILE *f)
>> @@ -667,8 +663,7 @@ static long get_file_size(FILE *f)
>>       return size;
>>   }
>>
>> -static void load_linux(void *fw_cfg,
>> -                       const char *kernel_filename,
>> +static void load_linux(const char *kernel_filename,
>>   		       const char *initrd_filename,
>>   		       const char *kernel_cmdline,
>>                          target_phys_addr_t max_ram_size)
>> @@ -703,9 +698,10 @@ static void load_linux(void *fw_cfg,
>>       else {
>>   	/* This looks like a multiboot kernel. If it is, let's stop
>>   	   treating it like a Linux kernel. */
>> -        if (load_multiboot(fw_cfg, f, kernel_filename, initrd_filename,
>> -                           kernel_cmdline, kernel_size, header))
>> +        if (load_multiboot(f, kernel_filename, initrd_filename,
>> +                           kernel_cmdline, kernel_size, header)) {
>>               return;
>> +        }
>>   	protocol = 0;
>>       }
>>
>> @@ -745,9 +741,9 @@ static void load_linux(void *fw_cfg,
>>       if (initrd_max>= max_ram_size-ACPI_DATA_SIZE)
>>       	initrd_max = max_ram_size-ACPI_DATA_SIZE-1;
>>
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_ADDR, cmdline_addr);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, strlen(kernel_cmdline)+1);
>> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
>> +    fw_cfg_add_i32(FW_CFG_CMDLINE_ADDR, cmdline_addr);
>> +    fw_cfg_add_i32(FW_CFG_CMDLINE_SIZE, strlen(kernel_cmdline)+1);
>> +    fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
>>                        (uint8_t*)strdup(kernel_cmdline),
>>                        strlen(kernel_cmdline)+1);
>>
>> @@ -808,9 +804,9 @@ static void load_linux(void *fw_cfg,
>>           initrd_data = g_malloc(initrd_size);
>>           load_image(initrd_filename, initrd_data);
>>
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr);
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
>> -        fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, initrd_data, initrd_size);
>> +        fw_cfg_add_i32(FW_CFG_INITRD_ADDR, initrd_addr);
>> +        fw_cfg_add_i32(FW_CFG_INITRD_SIZE, initrd_size);
>> +        fw_cfg_add_bytes(FW_CFG_INITRD_DATA, initrd_data, initrd_size);
>>
>>   	stl_p(header+0x218, initrd_addr);
>>   	stl_p(header+0x21c, initrd_size);
>> @@ -837,13 +833,13 @@ static void load_linux(void *fw_cfg,
>>       fclose(f);
>>       memcpy(setup, header, MIN(sizeof(header), setup_size));
>>
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, prot_addr);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
>> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA, kernel, kernel_size);
>> +    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, prot_addr);
>> +    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
>> +    fw_cfg_add_bytes(FW_CFG_KERNEL_DATA, kernel, kernel_size);
>>
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_ADDR, real_addr);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_SIZE, setup_size);
>> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA, setup, setup_size);
>> +    fw_cfg_add_i32(FW_CFG_SETUP_ADDR, real_addr);
>> +    fw_cfg_add_i32(FW_CFG_SETUP_SIZE, setup_size);
>> +    fw_cfg_add_bytes(FW_CFG_SETUP_DATA, setup, setup_size);
>>
>>       option_rom[nb_option_roms].name = "linuxboot.bin";
>>       option_rom[nb_option_roms].bootindex = 0;
>> @@ -987,7 +983,6 @@ void pc_memory_init(MemoryRegion *system_memory,
>>       int linux_boot, i;
>>       MemoryRegion *ram, *option_rom_mr;
>>       MemoryRegion *ram_below_4g, *ram_above_4g;
>> -    void *fw_cfg;
>>
>>       linux_boot = (kernel_filename != NULL);
>>
>> @@ -1024,11 +1019,11 @@ void pc_memory_init(MemoryRegion *system_memory,
>>                                           option_rom_mr,
>>                                           1);
>>
>> -    fw_cfg = bochs_bios_init();
>> -    rom_set_fw(fw_cfg);
>> +    bochs_bios_init();
>>
>>       if (linux_boot) {
>> -        load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline, below_4g_mem_size);
>> +        load_linux(kernel_filename, initrd_filename, kernel_cmdline,
>> +                   below_4g_mem_size);
>>       }
>>
>>       for (i = 0; i<  nb_option_roms; i++) {
>> diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
>> index 8796510..1f2a30e 100644
>> --- a/hw/ppc_newworld.c
>> +++ b/hw/ppc_newworld.c
>> @@ -107,7 +107,7 @@ static const MemoryRegionOps unin_ops = {
>>
>>   static int fw_cfg_boot_set(void *opaque, const char *boot_device)
>>   {
>> -    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
>> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
>>       return 0;
>>   }
>>
>> @@ -152,7 +152,6 @@ static void ppc_core99_init (ram_addr_t ram_size,
>>       MemoryRegion *ide_mem[3];
>>       int ppc_boot_device;
>>       DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>> -    void *fw_cfg;
>>       void *dbdma;
>>       int machine_arch;
>>
>> @@ -377,42 +376,42 @@ static void ppc_core99_init (ram_addr_t ram_size,
>>       macio_nvram_setup_bar(nvr, get_system_memory(), 0xFFF04000);
>>       /* No PCI init: the BIOS will do it */
>>
>> -    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
>> -    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
>> -    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, machine_arch);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, kernel_base);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
>> +    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
>> +    fw_cfg_add_i32(FW_CFG_ID, 1);
>> +    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
>> +    fw_cfg_add_i16(FW_CFG_MACHINE_ID, machine_arch);
>> +    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, kernel_base);
>> +    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
>>       if (kernel_cmdline) {
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, cmdline_base);
>> +        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, cmdline_base);
>>           pstrcpy_targphys("cmdline", cmdline_base, TARGET_PAGE_SIZE, kernel_cmdline);
>>       } else {
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
>> +        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, 0);
>>       }
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_base);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
>> -    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, ppc_boot_device);
>> +    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, initrd_base);
>> +    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, initrd_size);
>> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, ppc_boot_device);
>>
>> -    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_WIDTH, graphic_width);
>> -    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_HEIGHT, graphic_height);
>> -    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_DEPTH, graphic_depth);
>> +    fw_cfg_add_i16(FW_CFG_PPC_WIDTH, graphic_width);
>> +    fw_cfg_add_i16(FW_CFG_PPC_HEIGHT, graphic_height);
>> +    fw_cfg_add_i16(FW_CFG_PPC_DEPTH, graphic_depth);
>>
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_IS_KVM, kvm_enabled());
>> +    fw_cfg_add_i32(FW_CFG_PPC_IS_KVM, kvm_enabled());
>>       if (kvm_enabled()) {
>>   #ifdef CONFIG_KVM
>>           uint8_t *hypercall;
>>
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, kvmppc_get_tbfreq());
>> +        fw_cfg_add_i32(FW_CFG_PPC_TBFREQ, kvmppc_get_tbfreq());
>>           hypercall = g_malloc(16);
>>           kvmppc_get_hypercall(env, hypercall, 16);
>> -        fw_cfg_add_bytes(fw_cfg, FW_CFG_PPC_KVM_HC, hypercall, 16);
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_KVM_PID, getpid());
>> +        fw_cfg_add_bytes(FW_CFG_PPC_KVM_HC, hypercall, 16);
>> +        fw_cfg_add_i32(FW_CFG_PPC_KVM_PID, getpid());
>>   #endif
>>       } else {
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, get_ticks_per_sec());
>> +        fw_cfg_add_i32(FW_CFG_PPC_TBFREQ, get_ticks_per_sec());
>>       }
>>
>> -    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
>> +    qemu_register_boot_set(fw_cfg_boot_set, NULL);
>>   }
>>
>>   static QEMUMachine core99_machine = {
>> diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c
>> index 7e73d37..575fe59 100644
>> --- a/hw/ppc_oldworld.c
>> +++ b/hw/ppc_oldworld.c
>> @@ -50,7 +50,7 @@
>>
>>   static int fw_cfg_boot_set(void *opaque, const char *boot_device)
>>   {
>> -    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
>> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
>>       return 0;
>>   }
>>
>> @@ -95,7 +95,6 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
>>       MemoryRegion *escc_mem, *escc_bar = g_new(MemoryRegion, 1), *ide_mem[2];
>>       uint16_t ppc_boot_device;
>>       DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>> -    void *fw_cfg;
>>       void *dbdma;
>>
>>       linux_boot = (kernel_filename != NULL);
>> @@ -292,42 +291,42 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
>>
>>       /* No PCI init: the BIOS will do it */
>>
>> -    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
>> -    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
>> -    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, ARCH_HEATHROW);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, kernel_base);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
>> +    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
>> +    fw_cfg_add_i32(FW_CFG_ID, 1);
>> +    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
>> +    fw_cfg_add_i16(FW_CFG_MACHINE_ID, ARCH_HEATHROW);
>> +    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, kernel_base);
>> +    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
>>       if (kernel_cmdline) {
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, cmdline_base);
>> +        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, cmdline_base);
>>           pstrcpy_targphys("cmdline", cmdline_base, TARGET_PAGE_SIZE, kernel_cmdline);
>>       } else {
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
>> +        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, 0);
>>       }
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_base);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
>> -    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, ppc_boot_device);
>> +    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, initrd_base);
>> +    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, initrd_size);
>> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, ppc_boot_device);
>>
>> -    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_WIDTH, graphic_width);
>> -    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_HEIGHT, graphic_height);
>> -    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_DEPTH, graphic_depth);
>> +    fw_cfg_add_i16(FW_CFG_PPC_WIDTH, graphic_width);
>> +    fw_cfg_add_i16(FW_CFG_PPC_HEIGHT, graphic_height);
>> +    fw_cfg_add_i16(FW_CFG_PPC_DEPTH, graphic_depth);
>>
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_IS_KVM, kvm_enabled());
>> +    fw_cfg_add_i32(FW_CFG_PPC_IS_KVM, kvm_enabled());
>>       if (kvm_enabled()) {
>>   #ifdef CONFIG_KVM
>>           uint8_t *hypercall;
>>
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, kvmppc_get_tbfreq());
>> +        fw_cfg_add_i32(FW_CFG_PPC_TBFREQ, kvmppc_get_tbfreq());
>>           hypercall = g_malloc(16);
>>           kvmppc_get_hypercall(env, hypercall, 16);
>> -        fw_cfg_add_bytes(fw_cfg, FW_CFG_PPC_KVM_HC, hypercall, 16);
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_KVM_PID, getpid());
>> +        fw_cfg_add_bytes(FW_CFG_PPC_KVM_HC, hypercall, 16);
>> +        fw_cfg_add_i32(FW_CFG_PPC_KVM_PID, getpid());
>>   #endif
>>       } else {
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, get_ticks_per_sec());
>> +        fw_cfg_add_i32(FW_CFG_PPC_TBFREQ, get_ticks_per_sec());
>>       }
>>
>> -    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
>> +    qemu_register_boot_set(fw_cfg_boot_set, NULL);
>>   }
>>
>>   static QEMUMachine heathrow_machine = {
>> diff --git a/hw/sun4m.c b/hw/sun4m.c
>> index 34088ad..9f546d6 100644
>> --- a/hw/sun4m.c
>> +++ b/hw/sun4m.c
>> @@ -163,7 +163,7 @@ void DMA_register_channel (int nchan,
>>
>>   static int fw_cfg_boot_set(void *opaque, const char *boot_device)
>>   {
>> -    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
>> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
>>       return 0;
>>   }
>>
>> @@ -843,7 +843,6 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
>>       qemu_irq *cpu_halt;
>>       unsigned long kernel_size;
>>       DriveInfo *fd[MAX_FD];
>> -    void *fw_cfg;
>>       unsigned int num_vsimms;
>>
>>       /* init CPUs */
>> @@ -995,29 +994,29 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
>>           ecc_init(hwdef->ecc_base, slavio_irq[28],
>>                    hwdef->ecc_version);
>>
>> -    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
>> -    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
>> -    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, hwdef->machine_id);
>> -    fw_cfg_add_i16(fw_cfg, FW_CFG_SUN4M_DEPTH, graphic_depth);
>> -    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);
>> +    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
>> +    fw_cfg_add_i32(FW_CFG_ID, 1);
>> +    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
>> +    fw_cfg_add_i16(FW_CFG_MACHINE_ID, hwdef->machine_id);
>> +    fw_cfg_add_i16(FW_CFG_SUN4M_DEPTH, graphic_depth);
>> +    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, KERNEL_LOAD_ADDR);
>> +    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
>>       if (kernel_cmdline) {
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
>> +        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
>>           pstrcpy_targphys("cmdline", CMDLINE_ADDR, TARGET_PAGE_SIZE, kernel_cmdline);
>> -        fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
>> +        fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
>>                            (uint8_t*)strdup(kernel_cmdline),
>>                            strlen(kernel_cmdline) + 1);
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,
>> +        fw_cfg_add_i32(FW_CFG_CMDLINE_SIZE,
>>                          strlen(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_KERNEL_CMDLINE, 0);
>> +        fw_cfg_add_i32(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]);
>> -    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
>> +    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, INITRD_LOAD_ADDR);
>> +    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, 0); /* not used */
>> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
>> +    qemu_register_boot_set(fw_cfg_boot_set, NULL);
>>   }
>>
>>   enum {
>> @@ -1525,7 +1524,6 @@ static void sun4d_hw_init(const struct sun4d_hwdef *hwdef, ram_addr_t RAM_size,
>>           espdma_irq, ledma_irq;
>>       qemu_irq esp_reset, dma_enable;
>>       unsigned long kernel_size;
>> -    void *fw_cfg;
>>       DeviceState *dev;
>>
>>       /* init CPUs */
>> @@ -1606,26 +1604,26 @@ static void sun4d_hw_init(const struct sun4d_hwdef *hwdef, ram_addr_t RAM_size,
>>                  graphic_height, graphic_depth, hwdef->nvram_machine_id,
>>                  "Sun4d");
>>
>> -    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
>> -    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
>> -    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, hwdef->machine_id);
>> -    fw_cfg_add_i16(fw_cfg, FW_CFG_SUN4M_DEPTH, graphic_depth);
>> -    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);
>> +    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
>> +    fw_cfg_add_i32(FW_CFG_ID, 1);
>> +    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
>> +    fw_cfg_add_i16(FW_CFG_MACHINE_ID, hwdef->machine_id);
>> +    fw_cfg_add_i16(FW_CFG_SUN4M_DEPTH, graphic_depth);
>> +    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, KERNEL_LOAD_ADDR);
>> +    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
>>       if (kernel_cmdline) {
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
>> +        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
>>           pstrcpy_targphys("cmdline", CMDLINE_ADDR, TARGET_PAGE_SIZE, kernel_cmdline);
>> -        fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
>> +        fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
>>                            (uint8_t*)strdup(kernel_cmdline),
>>                            strlen(kernel_cmdline) + 1);
>>       } else {
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
>> +        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, 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]);
>> -    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
>> +    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, INITRD_LOAD_ADDR);
>> +    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, 0); /* not used */
>> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
>> +    qemu_register_boot_set(fw_cfg_boot_set, NULL);
>>   }
>>
>>   /* SPARCserver 1000 hardware initialisation */
>> @@ -1719,7 +1717,6 @@ static void sun4c_hw_init(const struct sun4c_hwdef *hwdef, ram_addr_t RAM_size,
>>       qemu_irq fdc_tc;
>>       unsigned long kernel_size;
>>       DriveInfo *fd[MAX_FD];
>> -    void *fw_cfg;
>>       DeviceState *dev;
>>       unsigned int i;
>>
>> @@ -1798,26 +1795,26 @@ static void sun4c_hw_init(const struct sun4c_hwdef *hwdef, ram_addr_t RAM_size,
>>                  graphic_height, graphic_depth, hwdef->nvram_machine_id,
>>                  "Sun4c");
>>
>> -    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
>> -    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
>> -    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, hwdef->machine_id);
>> -    fw_cfg_add_i16(fw_cfg, FW_CFG_SUN4M_DEPTH, graphic_depth);
>> -    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);
>> +    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
>> +    fw_cfg_add_i32(FW_CFG_ID, 1);
>> +    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
>> +    fw_cfg_add_i16(FW_CFG_MACHINE_ID, hwdef->machine_id);
>> +    fw_cfg_add_i16(FW_CFG_SUN4M_DEPTH, graphic_depth);
>> +    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, KERNEL_LOAD_ADDR);
>> +    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
>>       if (kernel_cmdline) {
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
>> +        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
>>           pstrcpy_targphys("cmdline", CMDLINE_ADDR, TARGET_PAGE_SIZE, kernel_cmdline);
>> -        fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
>> +        fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
>>                            (uint8_t*)strdup(kernel_cmdline),
>>                            strlen(kernel_cmdline) + 1);
>>       } else {
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
>> +        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, 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]);
>> -    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
>> +    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, INITRD_LOAD_ADDR);
>> +    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, 0); /* not used */
>> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
>> +    qemu_register_boot_set(fw_cfg_boot_set, NULL);
>>   }
>>
>>   /* SPARCstation 2 hardware initialisation */
>> diff --git a/hw/sun4u.c b/hw/sun4u.c
>> index 517bdb8..094c07c 100644
>> --- a/hw/sun4u.c
>> +++ b/hw/sun4u.c
>> @@ -125,7 +125,7 @@ void DMA_register_channel (int nchan,
>>
>>   static int fw_cfg_boot_set(void *opaque, const char *boot_device)
>>   {
>> -    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
>> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
>>       return 0;
>>   }
>>
>> @@ -802,7 +802,6 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>>       qemu_irq *ivec_irqs, *pbm_irqs;
>>       DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>>       DriveInfo *fd[MAX_FD];
>> -    void *fw_cfg;
>>
>>       /* init CPUs */
>>       env = cpu_devinit(cpu_model, hwdef);
>> @@ -867,30 +866,30 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>>                              graphic_width, graphic_height, graphic_depth,
>>                              (uint8_t *)&nd_table[0].macaddr);
>>
>> -    fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
>> -    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
>> -    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
>> -    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);
>> +    fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
>> +    fw_cfg_add_i32(FW_CFG_ID, 1);
>> +    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
>> +    fw_cfg_add_i16(FW_CFG_MACHINE_ID, hwdef->machine_id);
>> +    fw_cfg_add_i64(FW_CFG_KERNEL_ADDR, kernel_entry);
>> +    fw_cfg_add_i64(FW_CFG_KERNEL_SIZE, kernel_size);
>>       if (kernel_cmdline) {
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,
>> +        fw_cfg_add_i32(FW_CFG_CMDLINE_SIZE,
>>                          strlen(kernel_cmdline) + 1);
>> -        fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
>> +        fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
>>                            (uint8_t*)strdup(kernel_cmdline),
>>                            strlen(kernel_cmdline) + 1);
>>       } else {
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, 0);
>> +        fw_cfg_add_i32(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_i64(FW_CFG_INITRD_ADDR, initrd_addr);
>> +    fw_cfg_add_i64(FW_CFG_INITRD_SIZE, initrd_size);
>> +    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_devices[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);
>> -    fw_cfg_add_i16(fw_cfg, FW_CFG_SPARC64_DEPTH, graphic_depth);
>> +    fw_cfg_add_i16(FW_CFG_SPARC64_WIDTH, graphic_width);
>> +    fw_cfg_add_i16(FW_CFG_SPARC64_HEIGHT, graphic_height);
>> +    fw_cfg_add_i16(FW_CFG_SPARC64_DEPTH, graphic_depth);
>>
>> -    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
>> +    qemu_register_boot_set(fw_cfg_boot_set, NULL);
>>   }
>>
>>   enum {
>> --
>> 1.7.7.3
>>
>
> --
> 			Gleb.

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

* Re: [Qemu-devel] [PATCH 2/2] Add PIIX4 properties to control PM system states.
  2012-05-20  9:02 ` [Qemu-devel] [PATCH 2/2] Add PIIX4 properties to control PM system states Gleb Natapov
@ 2012-05-23 13:27   ` Anthony Liguori
  2012-05-23 13:34     ` Gleb Natapov
  2012-05-23 14:34     ` Andreas Färber
  0 siblings, 2 replies; 18+ messages in thread
From: Anthony Liguori @ 2012-05-23 13:27 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: qemu-devel

On 05/20/2012 04:02 AM, Gleb Natapov wrote:
> This patch adds two things. First it allows QEMU to distinguish between
> regular powerdown and S4 powerdown. Later separate QMP notification will
> be added for S4 powerdown. Second it allows S3/S4 states to be disabled
> from QEMU command line. Some guests known to be broken with regards to
> power management, but allow to use it anyway. Using new properties
> management will be able to disable S3/S4 for such guests.
>
> Supported system state are passed to a firmware using new fw_cfg file.
> The file contains  6 byte array. Each byte represents one system
> state. If byte at offset X has its MSB set it means that system state
> X is supported and to enter it guest should use the value from lowest 3
> bits.
>
> Signed-off-by: Gleb Natapov<gleb@redhat.com>

I see nothing wrong in principle here except that you should use a PTR property 
to pass the fw_cfg object to the ACPI PM device.

Regards,

Anthony Liguori

> ---
>   hw/acpi.c       |    5 ++++-
>   hw/acpi.h       |    2 +-
>   hw/acpi_piix4.c |   16 +++++++++++++++-
>   hw/vt82c686.c   |    2 +-
>   4 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/hw/acpi.c b/hw/acpi.c
> index 5d521e5..effc7ec 100644
> --- a/hw/acpi.c
> +++ b/hw/acpi.c
> @@ -370,7 +370,7 @@ void acpi_pm1_cnt_init(ACPIREGS *ar)
>       qemu_register_wakeup_notifier(&ar->wakeup);
>   }
>
> -void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val)
> +void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val, char s4)
>   {
>       ar->pm1.cnt.cnt = val&  ~(ACPI_BITMASK_SLEEP_ENABLE);
>
> @@ -385,6 +385,9 @@ void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val)
>               qemu_system_suspend_request();
>               break;
>           default:
> +            if (sus_typ == s4) { /* S4 request */
> +                qemu_system_shutdown_request();
> +            }
>               break;
>           }
>       }
> diff --git a/hw/acpi.h b/hw/acpi.h
> index fe8cdb4..7337f41 100644
> --- a/hw/acpi.h
> +++ b/hw/acpi.h
> @@ -139,7 +139,7 @@ void acpi_pm1_evt_reset(ACPIREGS *ar);
>
>   /* PM1a_CNT: piix and ich9 don't implement PM1b CNT. */
>   void acpi_pm1_cnt_init(ACPIREGS *ar);
> -void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val);
> +void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val, char s4);
>   void acpi_pm1_cnt_update(ACPIREGS *ar,
>                            bool sci_enable, bool sci_disable);
>   void acpi_pm1_cnt_reset(ACPIREGS *ar);
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index 585da4e..883314d 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -27,6 +27,7 @@
>   #include "sysemu.h"
>   #include "range.h"
>   #include "ioport.h"
> +#include "fw_cfg.h"
>
>   //#define DEBUG
>
> @@ -71,6 +72,10 @@ typedef struct PIIX4PMState {
>       struct pci_status pci0_status;
>       uint32_t pci0_hotplug_enable;
>       uint32_t pci0_slot_device_present;
> +
> +    uint8_t disable_s3;
> +    uint8_t disable_s4;
> +    uint8_t s4_val;
>   } PIIX4PMState;
>
>   static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s);
> @@ -123,7 +128,7 @@ static void pm_ioport_write(IORange *ioport, uint64_t addr, unsigned width,
>           pm_update_sci(s);
>           break;
>       case 0x04:
> -        acpi_pm1_cnt_write(&s->ar, val);
> +        acpi_pm1_cnt_write(&s->ar, val, s->s4_val);
>           break;
>       default:
>           break;
> @@ -425,6 +430,7 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
>   {
>       PCIDevice *dev;
>       PIIX4PMState *s;
> +    uint8_t suspend[6] = {128, 0, 0, 129, 128, 128};
>
>       dev = pci_create(bus, devfn, "PIIX4_PM");
>       qdev_prop_set_uint32(&dev->qdev, "smb_io_base", smb_io_base);
> @@ -437,11 +443,19 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
>
>       qdev_init_nofail(&dev->qdev);
>
> +    suspend[3] = 1 | ((!s->disable_s3)<<  7);
> +    suspend[4] = s->s4_val | ((!s->disable_s4)<<  7);
> +
> +    fw_cfg_add_file("etc/system-states", g_memdup(suspend, 6), 6);
> +
>       return s->smb.smbus;
>   }
>
>   static Property piix4_pm_properties[] = {
>       DEFINE_PROP_UINT32("smb_io_base", PIIX4PMState, smb_io_base, 0),
> +    DEFINE_PROP_UINT8("disable_s3", PIIX4PMState, disable_s3, 0),
> +    DEFINE_PROP_UINT8("disable_s4", PIIX4PMState, disable_s4, 0),
> +    DEFINE_PROP_UINT8("s4_val", PIIX4PMState, s4_val, 2),
>       DEFINE_PROP_END_OF_LIST(),
>   };
>
> diff --git a/hw/vt82c686.c b/hw/vt82c686.c
> index 6fb7950..5d7c00c 100644
> --- a/hw/vt82c686.c
> +++ b/hw/vt82c686.c
> @@ -210,7 +210,7 @@ static void pm_ioport_writew(void *opaque, uint32_t addr, uint32_t val)
>           pm_update_sci(s);
>           break;
>       case 0x04:
> -        acpi_pm1_cnt_write(&s->ar, val);
> +        acpi_pm1_cnt_write(&s->ar, val, 0);
>           break;
>       default:
>           break;

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

* Re: [Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global.
  2012-05-23 13:25   ` Anthony Liguori
@ 2012-05-23 13:32     ` Gleb Natapov
  2012-05-23 13:46       ` Anthony Liguori
  0 siblings, 1 reply; 18+ messages in thread
From: Gleb Natapov @ 2012-05-23 13:32 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: blauwirbel, qemu-devel

On Wed, May 23, 2012 at 08:25:35AM -0500, Anthony Liguori wrote:
> On 05/23/2012 07:37 AM, Gleb Natapov wrote:
> >Ping.
> 
> I don't understand why you're pinging..  The patch has just been on
> the list for a couple of days and is definitely not a 1.1 candidate.
> Am I missing something?
> 
It is definitely not 1.1 candidate. Only 1.1 patches are accepted now? 
When master will be opened for 1.2 commits?

> However...
Well, I am pinging for review :)

> 
> >
> >On Sun, May 20, 2012 at 12:02:40PM +0300, Gleb Natapov wrote:
> >>There can be only one fw_cfg device, so saving global reference to it
> >>removes the need to pass its pointer around.
> >>
> >>Signed-off-by: Gleb Natapov<gleb@redhat.com>
> >>---
> >>  hw/fw_cfg.c       |  110 +++++++++++++++++++++++++++--------------------------
> >>  hw/fw_cfg.h       |   15 +++----
> >>  hw/loader.c       |   10 +----
> >>  hw/loader.h       |    1 -
> >>  hw/multiboot.c    |   17 ++++----
> >>  hw/multiboot.h    |    3 +-
> >>  hw/pc.c           |   63 ++++++++++++++----------------
> >>  hw/ppc_newworld.c |   43 ++++++++++-----------
> >>  hw/ppc_oldworld.c |   43 ++++++++++-----------
> >>  hw/sun4m.c        |   93 +++++++++++++++++++++-----------------------
> >>  hw/sun4u.c        |   35 ++++++++---------
> >>  11 files changed, 207 insertions(+), 226 deletions(-)
> >>
> >>diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
> >>index 7b3b576..8c50473 100644
> >>--- a/hw/fw_cfg.c
> >>+++ b/hw/fw_cfg.c
> >>@@ -48,7 +48,7 @@ typedef struct FWCfgEntry {
> >>      FWCfgCallback callback;
> >>  } FWCfgEntry;
> >>
> >>-struct FWCfgState {
> >>+static struct FWCfgState {
> >>      SysBusDevice busdev;
> >>      MemoryRegion ctl_iomem, data_iomem, comb_iomem;
> >>      uint32_t ctl_iobase, data_iobase;
> >>@@ -57,7 +57,7 @@ struct FWCfgState {
> >>      uint16_t cur_entry;
> >>      uint32_t cur_offset;
> >>      Notifier machine_ready;
> >>-};
> >>+} *fw_cfg;
> >>
> >>  #define JPG_FILE 0
> >>  #define BMP_FILE 1
> >>@@ -113,7 +113,7 @@ error:
> >>      return NULL;
> >>  }
> >>
> >>-static void fw_cfg_bootsplash(FWCfgState *s)
> 
> This is a step backwards IMHO.  Relying on global state is generally
> a bad thing.  Passing around pointers is a good thing because it
> forces you to think about the relationships between devices and
> makes it hard to do silly things (like have random interactions with
> fw_cfg in devices that have no business doing that).
> 
We are in a kind of agreement here, but fw_cfg is this rare thing that
wants to be accessed by devices which, on a first glance, have no
business doing that. We already saving fw_cfg globally for rom loaders
to use, not nice either.

> Regards,
> 
> Anthony Liguori
> 
> >>+static void fw_cfg_bootsplash(void)
> >>  {
> >>      int boot_splash_time = -1;
> >>      const char *boot_splash_filename = NULL;
> >>@@ -148,7 +148,7 @@ static void fw_cfg_bootsplash(FWCfgState *s)
> >>          /* use little endian format */
> >>          qemu_extra_params_fw[0] = (uint8_t)(boot_splash_time&  0xff);
> >>          qemu_extra_params_fw[1] = (uint8_t)((boot_splash_time>>  8)&  0xff);
> >>-        fw_cfg_add_file(s, "etc/boot-menu-wait", qemu_extra_params_fw, 2);
> >>+        fw_cfg_add_file("etc/boot-menu-wait", qemu_extra_params_fw, 2);
> >>      }
> >>
> >>      /* insert splash file if user configurated */
> >>@@ -173,10 +173,10 @@ static void fw_cfg_bootsplash(FWCfgState *s)
> >>
> >>          /* insert data */
> >>          if (file_type == JPG_FILE) {
> >>-            fw_cfg_add_file(s, "bootsplash.jpg",
> >>+            fw_cfg_add_file("bootsplash.jpg",
> >>                      boot_splash_filedata, boot_splash_filedata_size);
> >>          } else {
> >>-            fw_cfg_add_file(s, "bootsplash.bmp",
> >>+            fw_cfg_add_file("bootsplash.bmp",
> >>                      boot_splash_filedata, boot_splash_filedata_size);
> >>          }
> >>          g_free(filename);
> >>@@ -359,122 +359,126 @@ static const VMStateDescription vmstate_fw_cfg = {
> >>      }
> >>  };
> >>
> >>-int fw_cfg_add_bytes(FWCfgState *s, uint16_t key, uint8_t *data, uint32_t len)
> >>+int fw_cfg_add_bytes(uint16_t key, uint8_t *data, uint32_t len)
> >>  {
> >>      int arch = !!(key&  FW_CFG_ARCH_LOCAL);
> >>
> >>      key&= FW_CFG_ENTRY_MASK;
> >>
> >>-    if (key>= FW_CFG_MAX_ENTRY)
> >>+    if (key>= FW_CFG_MAX_ENTRY || !fw_cfg) {
> >>          return 0;
> >>+    }
> >>
> >>-    s->entries[arch][key].data = data;
> >>-    s->entries[arch][key].len = len;
> >>+    fw_cfg->entries[arch][key].data = data;
> >>+    fw_cfg->entries[arch][key].len = len;
> >>
> >>      return 1;
> >>  }
> >>
> >>-int fw_cfg_add_i16(FWCfgState *s, uint16_t key, uint16_t value)
> >>+int fw_cfg_add_i16(uint16_t key, uint16_t value)
> >>  {
> >>      uint16_t *copy;
> >>
> >>      copy = g_malloc(sizeof(value));
> >>      *copy = cpu_to_le16(value);
> >>-    return fw_cfg_add_bytes(s, key, (uint8_t *)copy, sizeof(value));
> >>+    return fw_cfg_add_bytes(key, (uint8_t *)copy, sizeof(value));
> >>  }
> >>
> >>-int fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t value)
> >>+int fw_cfg_add_i32(uint16_t key, uint32_t value)
> >>  {
> >>      uint32_t *copy;
> >>
> >>      copy = g_malloc(sizeof(value));
> >>      *copy = cpu_to_le32(value);
> >>-    return fw_cfg_add_bytes(s, key, (uint8_t *)copy, sizeof(value));
> >>+    return fw_cfg_add_bytes(key, (uint8_t *)copy, sizeof(value));
> >>  }
> >>
> >>-int fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t value)
> >>+int fw_cfg_add_i64(uint16_t key, uint64_t value)
> >>  {
> >>      uint64_t *copy;
> >>
> >>      copy = g_malloc(sizeof(value));
> >>      *copy = cpu_to_le64(value);
> >>-    return fw_cfg_add_bytes(s, key, (uint8_t *)copy, sizeof(value));
> >>+    return fw_cfg_add_bytes(key, (uint8_t *)copy, sizeof(value));
> >>  }
> >>
> >>-int fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback,
> >>+int fw_cfg_add_callback(uint16_t key, FWCfgCallback callback,
> >>                          void *callback_opaque, uint8_t *data, size_t len)
> >>  {
> >>      int arch = !!(key&  FW_CFG_ARCH_LOCAL);
> >>
> >>-    if (!(key&  FW_CFG_WRITE_CHANNEL))
> >>+    if (!(key&  FW_CFG_WRITE_CHANNEL) || !fw_cfg) {
> >>          return 0;
> >>+    }
> >>
> >>      key&= FW_CFG_ENTRY_MASK;
> >>
> >>      if (key>= FW_CFG_MAX_ENTRY || len>  65535)
> >>          return 0;
> >>
> >>-    s->entries[arch][key].data = data;
> >>-    s->entries[arch][key].len = len;
> >>-    s->entries[arch][key].callback_opaque = callback_opaque;
> >>-    s->entries[arch][key].callback = callback;
> >>+    fw_cfg->entries[arch][key].data = data;
> >>+    fw_cfg->entries[arch][key].len = len;
> >>+    fw_cfg->entries[arch][key].callback_opaque = callback_opaque;
> >>+    fw_cfg->entries[arch][key].callback = callback;
> >>
> >>      return 1;
> >>  }
> >>
> >>-int fw_cfg_add_file(FWCfgState *s,  const char *filename, uint8_t *data,
> >>-                    uint32_t len)
> >>+int fw_cfg_add_file(const char *filename, uint8_t *data, uint32_t len)
> >>  {
> >>      int i, index;
> >>
> >>-    if (!s->files) {
> >>+    if (!fw_cfg) {
> >>+        return 0;
> >>+    }
> >>+
> >>+    if (!fw_cfg->files) {
> >>          int dsize = sizeof(uint32_t) + sizeof(FWCfgFile) * FW_CFG_FILE_SLOTS;
> >>-        s->files = g_malloc0(dsize);
> >>-        fw_cfg_add_bytes(s, FW_CFG_FILE_DIR, (uint8_t*)s->files, dsize);
> >>+        fw_cfg->files = g_malloc0(dsize);
> >>+        fw_cfg_add_bytes(FW_CFG_FILE_DIR, (uint8_t *)fw_cfg->files, dsize);
> >>      }
> >>
> >>-    index = be32_to_cpu(s->files->count);
> >>+    index = be32_to_cpu(fw_cfg->files->count);
> >>      if (index == FW_CFG_FILE_SLOTS) {
> >>          fprintf(stderr, "fw_cfg: out of file slots\n");
> >>          return 0;
> >>      }
> >>
> >>-    fw_cfg_add_bytes(s, FW_CFG_FILE_FIRST + index, data, len);
> >>+    fw_cfg_add_bytes(FW_CFG_FILE_FIRST + index, data, len);
> >>
> >>-    pstrcpy(s->files->f[index].name, sizeof(s->files->f[index].name),
> >>+    pstrcpy(fw_cfg->files->f[index].name, sizeof(fw_cfg->files->f[index].name),
> >>              filename);
> >>      for (i = 0; i<  index; i++) {
> >>-        if (strcmp(s->files->f[index].name, s->files->f[i].name) == 0) {
> >>+        if (strcmp(fw_cfg->files->f[index].name, fw_cfg->files->f[i].name)
> >>+                        == 0) {
> >>              FW_CFG_DPRINTF("%s: skip duplicate: %s\n", __FUNCTION__,
> >>-                           s->files->f[index].name);
> >>+                           fw_cfg->files->f[index].name);
> >>              return 1;
> >>          }
> >>      }
> >>
> >>-    s->files->f[index].size   = cpu_to_be32(len);
> >>-    s->files->f[index].select = cpu_to_be16(FW_CFG_FILE_FIRST + index);
> >>+    fw_cfg->files->f[index].size   = cpu_to_be32(len);
> >>+    fw_cfg->files->f[index].select = cpu_to_be16(FW_CFG_FILE_FIRST + index);
> >>      FW_CFG_DPRINTF("%s: #%d: %s (%d bytes)\n", __FUNCTION__,
> >>-                   index, s->files->f[index].name, len);
> >>+                   index, fw_cfg->files->f[index].name, len);
> >>
> >>-    s->files->count = cpu_to_be32(index+1);
> >>+    fw_cfg->files->count = cpu_to_be32(index+1);
> >>      return 1;
> >>  }
> >>
> >>  static void fw_cfg_machine_ready(struct Notifier *n, void *data)
> >>  {
> >>      uint32_t len;
> >>-    FWCfgState *s = container_of(n, FWCfgState, machine_ready);
> >>      char *bootindex = get_boot_devices_list(&len);
> >>
> >>-    fw_cfg_add_file(s, "bootorder", (uint8_t*)bootindex, len);
> >>+    fw_cfg_add_file("bootorder", (uint8_t *)bootindex, len);
> >>  }
> >>
> >>-FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
> >>-                        target_phys_addr_t ctl_addr, target_phys_addr_t data_addr)
> >>+void fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
> >>+                 target_phys_addr_t ctl_addr, target_phys_addr_t data_addr)
> >>  {
> >>      DeviceState *dev;
> >>      SysBusDevice *d;
> >>-    FWCfgState *s;
> >>
> >>      dev = qdev_create(NULL, "fw_cfg");
> >>      qdev_prop_set_uint32(dev, "ctl_iobase", ctl_port);
> >>@@ -482,7 +486,7 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
> >>      qdev_init_nofail(dev);
> >>      d = sysbus_from_qdev(dev);
> >>
> >>-    s = DO_UPCAST(FWCfgState, busdev.qdev, dev);
> >>+    fw_cfg = DO_UPCAST(FWCfgState, busdev.qdev, dev);
> >>
> >>      if (ctl_addr) {
> >>          sysbus_mmio_map(d, 0, ctl_addr);
> >>@@ -490,18 +494,16 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
> >>      if (data_addr) {
> >>          sysbus_mmio_map(d, 1, data_addr);
> >>      }
> >>-    fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (uint8_t *)"QEMU", 4);
> >>-    fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16);
> >>-    fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)(display_type == DT_NOGRAPHIC));
> >>-    fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
> >>-    fw_cfg_add_i16(s, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
> >>-    fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
> >>-    fw_cfg_bootsplash(s);
> >>-
> >>-    s->machine_ready.notify = fw_cfg_machine_ready;
> >>-    qemu_add_machine_init_done_notifier(&s->machine_ready);
> >>-
> >>-    return s;
> >>+    fw_cfg_add_bytes(FW_CFG_SIGNATURE, (uint8_t *)"QEMU", 4);
> >>+    fw_cfg_add_bytes(FW_CFG_UUID, qemu_uuid, 16);
> >>+    fw_cfg_add_i16(FW_CFG_NOGRAPHIC, (uint16_t)(display_type == DT_NOGRAPHIC));
> >>+    fw_cfg_add_i16(FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
> >>+    fw_cfg_add_i16(FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
> >>+    fw_cfg_add_i16(FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
> >>+    fw_cfg_bootsplash();
> >>+
> >>+    fw_cfg->machine_ready.notify = fw_cfg_machine_ready;
> >>+    qemu_add_machine_init_done_notifier(&fw_cfg->machine_ready);
> >>  }
> >>
> >>  static int fw_cfg_init1(SysBusDevice *dev)
> >>diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
> >>index 856bf91..cdb239e 100644
> >>--- a/hw/fw_cfg.h
> >>+++ b/hw/fw_cfg.h
> >>@@ -54,15 +54,14 @@ typedef struct FWCfgFiles {
> >>  typedef void (*FWCfgCallback)(void *opaque, uint8_t *data);
> >>
> >>  typedef struct FWCfgState FWCfgState;
> >>-int fw_cfg_add_bytes(FWCfgState *s, uint16_t key, uint8_t *data, uint32_t len);
> >>-int fw_cfg_add_i16(FWCfgState *s, uint16_t key, uint16_t value);
> >>-int fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t value);
> >>-int fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t value);
> >>-int fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback,
> >>+int fw_cfg_add_bytes(uint16_t key, uint8_t *data, uint32_t len);
> >>+int fw_cfg_add_i16(uint16_t key, uint16_t value);
> >>+int fw_cfg_add_i32(uint16_t key, uint32_t value);
> >>+int fw_cfg_add_i64(uint16_t key, uint64_t value);
> >>+int fw_cfg_add_callback(uint16_t key, FWCfgCallback callback,
> >>                          void *callback_opaque, uint8_t *data, size_t len);
> >>-int fw_cfg_add_file(FWCfgState *s, const char *filename, uint8_t *data,
> >>-                    uint32_t len);
> >>-FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
> >>+int fw_cfg_add_file(const char *filename, uint8_t *data, uint32_t len);
> >>+void fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
> >>                          target_phys_addr_t crl_addr, target_phys_addr_t data_addr);
> >>
> >>  #endif /* NO_QEMU_PROTOS */
> >>diff --git a/hw/loader.c b/hw/loader.c
> >>index 415cdce..28464da 100644
> >>--- a/hw/loader.c
> >>+++ b/hw/loader.c
> >>@@ -543,7 +543,6 @@ struct Rom {
> >>      QTAILQ_ENTRY(Rom) next;
> >>  };
> >>
> >>-static FWCfgState *fw_cfg;
> >>  static QTAILQ_HEAD(, Rom) roms = QTAILQ_HEAD_INITIALIZER(roms);
> >>
> >>  static void rom_insert(Rom *rom)
> >>@@ -601,7 +600,7 @@ int rom_add_file(const char *file, const char *fw_dir,
> >>      }
> >>      close(fd);
> >>      rom_insert(rom);
> >>-    if (rom->fw_file&&  fw_cfg) {
> >>+    if (rom->fw_file) {
> >>          const char *basename;
> >>          char fw_file_name[56];
> >>
> >>@@ -613,7 +612,7 @@ int rom_add_file(const char *file, const char *fw_dir,
> >>          }
> >>          snprintf(fw_file_name, sizeof(fw_file_name), "%s/%s", rom->fw_dir,
> >>                   basename);
> >>-        fw_cfg_add_file(fw_cfg, fw_file_name, rom->data, rom->romsize);
> >>+        fw_cfg_add_file(fw_file_name, rom->data, rom->romsize);
> >>          snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name);
> >>      } else {
> >>          snprintf(devpath, sizeof(devpath), "/rom@" TARGET_FMT_plx, addr);
> >>@@ -704,11 +703,6 @@ int rom_load_all(void)
> >>      return 0;
> >>  }
> >>
> >>-void rom_set_fw(void *f)
> >>-{
> >>-    fw_cfg = f;
> >>-}
> >>-
> >>  static Rom *find_rom(target_phys_addr_t addr)
> >>  {
> >>      Rom *rom;
> >>diff --git a/hw/loader.h b/hw/loader.h
> >>index fbcaba9..c357ec4 100644
> >>--- a/hw/loader.h
> >>+++ b/hw/loader.h
> >>@@ -26,7 +26,6 @@ int rom_add_file(const char *file, const char *fw_dir,
> >>  int rom_add_blob(const char *name, const void *blob, size_t len,
> >>                   target_phys_addr_t addr);
> >>  int rom_load_all(void);
> >>-void rom_set_fw(void *f);
> >>  int rom_copy(uint8_t *dest, target_phys_addr_t addr, size_t size);
> >>  void *rom_ptr(target_phys_addr_t addr);
> >>  void do_info_roms(Monitor *mon);
> >>diff --git a/hw/multiboot.c b/hw/multiboot.c
> >>index b4484a3..fd93e13 100644
> >>--- a/hw/multiboot.c
> >>+++ b/hw/multiboot.c
> >>@@ -124,8 +124,7 @@ static void mb_add_mod(MultibootState *s,
> >>      s->mb_mods_count++;
> >>  }
> >>
> >>-int load_multiboot(void *fw_cfg,
> >>-                   FILE *f,
> >>+int load_multiboot(FILE *f,
> >>                     const char *kernel_filename,
> >>                     const char *initrd_filename,
> >>                     const char *kernel_cmdline,
> >>@@ -324,15 +323,15 @@ int load_multiboot(void *fw_cfg,
> >>      memcpy(mb_bootinfo_data, bootinfo, sizeof(bootinfo));
> >>
> >>      /* Pass variables to option rom */
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, mh_entry_addr);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, mbs.mb_buf_size);
> >>-    fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA,
> >>+    fw_cfg_add_i32(FW_CFG_KERNEL_ENTRY, mh_entry_addr);
> >>+    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, mh_load_addr);
> >>+    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, mbs.mb_buf_size);
> >>+    fw_cfg_add_bytes(FW_CFG_KERNEL_DATA,
> >>                       mbs.mb_buf, mbs.mb_buf_size);
> >>
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, ADDR_MBI);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, sizeof(bootinfo));
> >>-    fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, mb_bootinfo_data,
> >>+    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, ADDR_MBI);
> >>+    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, sizeof(bootinfo));
> >>+    fw_cfg_add_bytes(FW_CFG_INITRD_DATA, mb_bootinfo_data,
> >>                       sizeof(bootinfo));
> >>
> >>      option_rom[nb_option_roms].name = "multiboot.bin";
> >>diff --git a/hw/multiboot.h b/hw/multiboot.h
> >>index 98fb1b7..1302c55 100644
> >>--- a/hw/multiboot.h
> >>+++ b/hw/multiboot.h
> >>@@ -1,8 +1,7 @@
> >>  #ifndef QEMU_MULTIBOOT_H
> >>  #define QEMU_MULTIBOOT_H
> >>
> >>-int load_multiboot(void *fw_cfg,
> >>-                   FILE *f,
> >>+int load_multiboot(FILE *f,
> >>                     const char *kernel_filename,
> >>                     const char *initrd_filename,
> >>                     const char *kernel_cmdline,
> >>diff --git a/hw/pc.c b/hw/pc.c
> >>index 4d34a33..739e4ad 100644
> >>--- a/hw/pc.c
> >>+++ b/hw/pc.c
> >>@@ -593,9 +593,8 @@ int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
> >>      return index;
> >>  }
> >>
> >>-static void *bochs_bios_init(void)
> >>+static void bochs_bios_init(void)
> >>  {
> >>-    void *fw_cfg;
> >>      uint8_t *smbios_table;
> >>      size_t smbios_len;
> >>      uint64_t *numa_fw_cfg;
> >>@@ -613,22 +612,21 @@ static void *bochs_bios_init(void)
> >>      register_ioport_write(0x500, 1, 1, bochs_bios_write, NULL);
> >>      register_ioport_write(0x503, 1, 1, bochs_bios_write, NULL);
> >>
> >>-    fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
> >>+    fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
> >>
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
> >>-    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> >>-    fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES, (uint8_t *)acpi_tables,
> >>+    fw_cfg_add_i32(FW_CFG_ID, 1);
> >>+    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> >>+    fw_cfg_add_bytes(FW_CFG_ACPI_TABLES, (uint8_t *)acpi_tables,
> >>                       acpi_tables_len);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
> >>+    fw_cfg_add_i32(FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
> >>
> >>      smbios_table = smbios_get_table(&smbios_len);
> >>      if (smbios_table)
> >>-        fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
> >>-                         smbios_table, smbios_len);
> >>-    fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE, (uint8_t *)&e820_table,
> >>+        fw_cfg_add_bytes(FW_CFG_SMBIOS_ENTRIES, smbios_table, smbios_len);
> >>+    fw_cfg_add_bytes(FW_CFG_E820_TABLE, (uint8_t *)&e820_table,
> >>                       sizeof(struct e820_table));
> >>
> >>-    fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, (uint8_t *)&hpet_cfg,
> >>+    fw_cfg_add_bytes(FW_CFG_HPET, (uint8_t *)&hpet_cfg,
> >>                       sizeof(struct hpet_fw_config));
> >>      /* allocate memory for the NUMA channel: one (64bit) word for the number
> >>       * of nodes, one word for each VCPU->node and one word for each node to
> >>@@ -647,10 +645,8 @@ static void *bochs_bios_init(void)
> >>      for (i = 0; i<  nb_numa_nodes; i++) {
> >>          numa_fw_cfg[max_cpus + 1 + i] = cpu_to_le64(node_mem[i]);
> >>      }
> >>-    fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, (uint8_t *)numa_fw_cfg,
> >>+    fw_cfg_add_bytes(FW_CFG_NUMA, (uint8_t *)numa_fw_cfg,
> >>                       (1 + max_cpus + nb_numa_nodes) * 8);
> >>-
> >>-    return fw_cfg;
> >>  }
> >>
> >>  static long get_file_size(FILE *f)
> >>@@ -667,8 +663,7 @@ static long get_file_size(FILE *f)
> >>      return size;
> >>  }
> >>
> >>-static void load_linux(void *fw_cfg,
> >>-                       const char *kernel_filename,
> >>+static void load_linux(const char *kernel_filename,
> >>  		       const char *initrd_filename,
> >>  		       const char *kernel_cmdline,
> >>                         target_phys_addr_t max_ram_size)
> >>@@ -703,9 +698,10 @@ static void load_linux(void *fw_cfg,
> >>      else {
> >>  	/* This looks like a multiboot kernel. If it is, let's stop
> >>  	   treating it like a Linux kernel. */
> >>-        if (load_multiboot(fw_cfg, f, kernel_filename, initrd_filename,
> >>-                           kernel_cmdline, kernel_size, header))
> >>+        if (load_multiboot(f, kernel_filename, initrd_filename,
> >>+                           kernel_cmdline, kernel_size, header)) {
> >>              return;
> >>+        }
> >>  	protocol = 0;
> >>      }
> >>
> >>@@ -745,9 +741,9 @@ static void load_linux(void *fw_cfg,
> >>      if (initrd_max>= max_ram_size-ACPI_DATA_SIZE)
> >>      	initrd_max = max_ram_size-ACPI_DATA_SIZE-1;
> >>
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_ADDR, cmdline_addr);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, strlen(kernel_cmdline)+1);
> >>-    fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
> >>+    fw_cfg_add_i32(FW_CFG_CMDLINE_ADDR, cmdline_addr);
> >>+    fw_cfg_add_i32(FW_CFG_CMDLINE_SIZE, strlen(kernel_cmdline)+1);
> >>+    fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
> >>                       (uint8_t*)strdup(kernel_cmdline),
> >>                       strlen(kernel_cmdline)+1);
> >>
> >>@@ -808,9 +804,9 @@ static void load_linux(void *fw_cfg,
> >>          initrd_data = g_malloc(initrd_size);
> >>          load_image(initrd_filename, initrd_data);
> >>
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr);
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
> >>-        fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, initrd_data, initrd_size);
> >>+        fw_cfg_add_i32(FW_CFG_INITRD_ADDR, initrd_addr);
> >>+        fw_cfg_add_i32(FW_CFG_INITRD_SIZE, initrd_size);
> >>+        fw_cfg_add_bytes(FW_CFG_INITRD_DATA, initrd_data, initrd_size);
> >>
> >>  	stl_p(header+0x218, initrd_addr);
> >>  	stl_p(header+0x21c, initrd_size);
> >>@@ -837,13 +833,13 @@ static void load_linux(void *fw_cfg,
> >>      fclose(f);
> >>      memcpy(setup, header, MIN(sizeof(header), setup_size));
> >>
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, prot_addr);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
> >>-    fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA, kernel, kernel_size);
> >>+    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, prot_addr);
> >>+    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
> >>+    fw_cfg_add_bytes(FW_CFG_KERNEL_DATA, kernel, kernel_size);
> >>
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_ADDR, real_addr);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_SIZE, setup_size);
> >>-    fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA, setup, setup_size);
> >>+    fw_cfg_add_i32(FW_CFG_SETUP_ADDR, real_addr);
> >>+    fw_cfg_add_i32(FW_CFG_SETUP_SIZE, setup_size);
> >>+    fw_cfg_add_bytes(FW_CFG_SETUP_DATA, setup, setup_size);
> >>
> >>      option_rom[nb_option_roms].name = "linuxboot.bin";
> >>      option_rom[nb_option_roms].bootindex = 0;
> >>@@ -987,7 +983,6 @@ void pc_memory_init(MemoryRegion *system_memory,
> >>      int linux_boot, i;
> >>      MemoryRegion *ram, *option_rom_mr;
> >>      MemoryRegion *ram_below_4g, *ram_above_4g;
> >>-    void *fw_cfg;
> >>
> >>      linux_boot = (kernel_filename != NULL);
> >>
> >>@@ -1024,11 +1019,11 @@ void pc_memory_init(MemoryRegion *system_memory,
> >>                                          option_rom_mr,
> >>                                          1);
> >>
> >>-    fw_cfg = bochs_bios_init();
> >>-    rom_set_fw(fw_cfg);
> >>+    bochs_bios_init();
> >>
> >>      if (linux_boot) {
> >>-        load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline, below_4g_mem_size);
> >>+        load_linux(kernel_filename, initrd_filename, kernel_cmdline,
> >>+                   below_4g_mem_size);
> >>      }
> >>
> >>      for (i = 0; i<  nb_option_roms; i++) {
> >>diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
> >>index 8796510..1f2a30e 100644
> >>--- a/hw/ppc_newworld.c
> >>+++ b/hw/ppc_newworld.c
> >>@@ -107,7 +107,7 @@ static const MemoryRegionOps unin_ops = {
> >>
> >>  static int fw_cfg_boot_set(void *opaque, const char *boot_device)
> >>  {
> >>-    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
> >>+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
> >>      return 0;
> >>  }
> >>
> >>@@ -152,7 +152,6 @@ static void ppc_core99_init (ram_addr_t ram_size,
> >>      MemoryRegion *ide_mem[3];
> >>      int ppc_boot_device;
> >>      DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> >>-    void *fw_cfg;
> >>      void *dbdma;
> >>      int machine_arch;
> >>
> >>@@ -377,42 +376,42 @@ static void ppc_core99_init (ram_addr_t ram_size,
> >>      macio_nvram_setup_bar(nvr, get_system_memory(), 0xFFF04000);
> >>      /* No PCI init: the BIOS will do it */
> >>
> >>-    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
> >>-    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> >>-    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, machine_arch);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, kernel_base);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
> >>+    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> >>+    fw_cfg_add_i32(FW_CFG_ID, 1);
> >>+    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> >>+    fw_cfg_add_i16(FW_CFG_MACHINE_ID, machine_arch);
> >>+    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, kernel_base);
> >>+    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
> >>      if (kernel_cmdline) {
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, cmdline_base);
> >>+        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, cmdline_base);
> >>          pstrcpy_targphys("cmdline", cmdline_base, TARGET_PAGE_SIZE, kernel_cmdline);
> >>      } else {
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
> >>+        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, 0);
> >>      }
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_base);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
> >>-    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, ppc_boot_device);
> >>+    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, initrd_base);
> >>+    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, initrd_size);
> >>+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, ppc_boot_device);
> >>
> >>-    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_WIDTH, graphic_width);
> >>-    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_HEIGHT, graphic_height);
> >>-    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_DEPTH, graphic_depth);
> >>+    fw_cfg_add_i16(FW_CFG_PPC_WIDTH, graphic_width);
> >>+    fw_cfg_add_i16(FW_CFG_PPC_HEIGHT, graphic_height);
> >>+    fw_cfg_add_i16(FW_CFG_PPC_DEPTH, graphic_depth);
> >>
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_IS_KVM, kvm_enabled());
> >>+    fw_cfg_add_i32(FW_CFG_PPC_IS_KVM, kvm_enabled());
> >>      if (kvm_enabled()) {
> >>  #ifdef CONFIG_KVM
> >>          uint8_t *hypercall;
> >>
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, kvmppc_get_tbfreq());
> >>+        fw_cfg_add_i32(FW_CFG_PPC_TBFREQ, kvmppc_get_tbfreq());
> >>          hypercall = g_malloc(16);
> >>          kvmppc_get_hypercall(env, hypercall, 16);
> >>-        fw_cfg_add_bytes(fw_cfg, FW_CFG_PPC_KVM_HC, hypercall, 16);
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_KVM_PID, getpid());
> >>+        fw_cfg_add_bytes(FW_CFG_PPC_KVM_HC, hypercall, 16);
> >>+        fw_cfg_add_i32(FW_CFG_PPC_KVM_PID, getpid());
> >>  #endif
> >>      } else {
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, get_ticks_per_sec());
> >>+        fw_cfg_add_i32(FW_CFG_PPC_TBFREQ, get_ticks_per_sec());
> >>      }
> >>
> >>-    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
> >>+    qemu_register_boot_set(fw_cfg_boot_set, NULL);
> >>  }
> >>
> >>  static QEMUMachine core99_machine = {
> >>diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c
> >>index 7e73d37..575fe59 100644
> >>--- a/hw/ppc_oldworld.c
> >>+++ b/hw/ppc_oldworld.c
> >>@@ -50,7 +50,7 @@
> >>
> >>  static int fw_cfg_boot_set(void *opaque, const char *boot_device)
> >>  {
> >>-    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
> >>+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
> >>      return 0;
> >>  }
> >>
> >>@@ -95,7 +95,6 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
> >>      MemoryRegion *escc_mem, *escc_bar = g_new(MemoryRegion, 1), *ide_mem[2];
> >>      uint16_t ppc_boot_device;
> >>      DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> >>-    void *fw_cfg;
> >>      void *dbdma;
> >>
> >>      linux_boot = (kernel_filename != NULL);
> >>@@ -292,42 +291,42 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
> >>
> >>      /* No PCI init: the BIOS will do it */
> >>
> >>-    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
> >>-    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> >>-    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, ARCH_HEATHROW);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, kernel_base);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
> >>+    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> >>+    fw_cfg_add_i32(FW_CFG_ID, 1);
> >>+    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> >>+    fw_cfg_add_i16(FW_CFG_MACHINE_ID, ARCH_HEATHROW);
> >>+    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, kernel_base);
> >>+    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
> >>      if (kernel_cmdline) {
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, cmdline_base);
> >>+        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, cmdline_base);
> >>          pstrcpy_targphys("cmdline", cmdline_base, TARGET_PAGE_SIZE, kernel_cmdline);
> >>      } else {
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
> >>+        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, 0);
> >>      }
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_base);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
> >>-    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, ppc_boot_device);
> >>+    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, initrd_base);
> >>+    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, initrd_size);
> >>+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, ppc_boot_device);
> >>
> >>-    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_WIDTH, graphic_width);
> >>-    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_HEIGHT, graphic_height);
> >>-    fw_cfg_add_i16(fw_cfg, FW_CFG_PPC_DEPTH, graphic_depth);
> >>+    fw_cfg_add_i16(FW_CFG_PPC_WIDTH, graphic_width);
> >>+    fw_cfg_add_i16(FW_CFG_PPC_HEIGHT, graphic_height);
> >>+    fw_cfg_add_i16(FW_CFG_PPC_DEPTH, graphic_depth);
> >>
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_IS_KVM, kvm_enabled());
> >>+    fw_cfg_add_i32(FW_CFG_PPC_IS_KVM, kvm_enabled());
> >>      if (kvm_enabled()) {
> >>  #ifdef CONFIG_KVM
> >>          uint8_t *hypercall;
> >>
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, kvmppc_get_tbfreq());
> >>+        fw_cfg_add_i32(FW_CFG_PPC_TBFREQ, kvmppc_get_tbfreq());
> >>          hypercall = g_malloc(16);
> >>          kvmppc_get_hypercall(env, hypercall, 16);
> >>-        fw_cfg_add_bytes(fw_cfg, FW_CFG_PPC_KVM_HC, hypercall, 16);
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_KVM_PID, getpid());
> >>+        fw_cfg_add_bytes(FW_CFG_PPC_KVM_HC, hypercall, 16);
> >>+        fw_cfg_add_i32(FW_CFG_PPC_KVM_PID, getpid());
> >>  #endif
> >>      } else {
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, get_ticks_per_sec());
> >>+        fw_cfg_add_i32(FW_CFG_PPC_TBFREQ, get_ticks_per_sec());
> >>      }
> >>
> >>-    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
> >>+    qemu_register_boot_set(fw_cfg_boot_set, NULL);
> >>  }
> >>
> >>  static QEMUMachine heathrow_machine = {
> >>diff --git a/hw/sun4m.c b/hw/sun4m.c
> >>index 34088ad..9f546d6 100644
> >>--- a/hw/sun4m.c
> >>+++ b/hw/sun4m.c
> >>@@ -163,7 +163,7 @@ void DMA_register_channel (int nchan,
> >>
> >>  static int fw_cfg_boot_set(void *opaque, const char *boot_device)
> >>  {
> >>-    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
> >>+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
> >>      return 0;
> >>  }
> >>
> >>@@ -843,7 +843,6 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
> >>      qemu_irq *cpu_halt;
> >>      unsigned long kernel_size;
> >>      DriveInfo *fd[MAX_FD];
> >>-    void *fw_cfg;
> >>      unsigned int num_vsimms;
> >>
> >>      /* init CPUs */
> >>@@ -995,29 +994,29 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
> >>          ecc_init(hwdef->ecc_base, slavio_irq[28],
> >>                   hwdef->ecc_version);
> >>
> >>-    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
> >>-    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> >>-    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, hwdef->machine_id);
> >>-    fw_cfg_add_i16(fw_cfg, FW_CFG_SUN4M_DEPTH, graphic_depth);
> >>-    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);
> >>+    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> >>+    fw_cfg_add_i32(FW_CFG_ID, 1);
> >>+    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> >>+    fw_cfg_add_i16(FW_CFG_MACHINE_ID, hwdef->machine_id);
> >>+    fw_cfg_add_i16(FW_CFG_SUN4M_DEPTH, graphic_depth);
> >>+    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, KERNEL_LOAD_ADDR);
> >>+    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
> >>      if (kernel_cmdline) {
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
> >>+        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
> >>          pstrcpy_targphys("cmdline", CMDLINE_ADDR, TARGET_PAGE_SIZE, kernel_cmdline);
> >>-        fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
> >>+        fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
> >>                           (uint8_t*)strdup(kernel_cmdline),
> >>                           strlen(kernel_cmdline) + 1);
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,
> >>+        fw_cfg_add_i32(FW_CFG_CMDLINE_SIZE,
> >>                         strlen(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_KERNEL_CMDLINE, 0);
> >>+        fw_cfg_add_i32(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]);
> >>-    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
> >>+    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, INITRD_LOAD_ADDR);
> >>+    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, 0); /* not used */
> >>+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
> >>+    qemu_register_boot_set(fw_cfg_boot_set, NULL);
> >>  }
> >>
> >>  enum {
> >>@@ -1525,7 +1524,6 @@ static void sun4d_hw_init(const struct sun4d_hwdef *hwdef, ram_addr_t RAM_size,
> >>          espdma_irq, ledma_irq;
> >>      qemu_irq esp_reset, dma_enable;
> >>      unsigned long kernel_size;
> >>-    void *fw_cfg;
> >>      DeviceState *dev;
> >>
> >>      /* init CPUs */
> >>@@ -1606,26 +1604,26 @@ static void sun4d_hw_init(const struct sun4d_hwdef *hwdef, ram_addr_t RAM_size,
> >>                 graphic_height, graphic_depth, hwdef->nvram_machine_id,
> >>                 "Sun4d");
> >>
> >>-    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
> >>-    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> >>-    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, hwdef->machine_id);
> >>-    fw_cfg_add_i16(fw_cfg, FW_CFG_SUN4M_DEPTH, graphic_depth);
> >>-    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);
> >>+    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> >>+    fw_cfg_add_i32(FW_CFG_ID, 1);
> >>+    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> >>+    fw_cfg_add_i16(FW_CFG_MACHINE_ID, hwdef->machine_id);
> >>+    fw_cfg_add_i16(FW_CFG_SUN4M_DEPTH, graphic_depth);
> >>+    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, KERNEL_LOAD_ADDR);
> >>+    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
> >>      if (kernel_cmdline) {
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
> >>+        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
> >>          pstrcpy_targphys("cmdline", CMDLINE_ADDR, TARGET_PAGE_SIZE, kernel_cmdline);
> >>-        fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
> >>+        fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
> >>                           (uint8_t*)strdup(kernel_cmdline),
> >>                           strlen(kernel_cmdline) + 1);
> >>      } else {
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
> >>+        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, 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]);
> >>-    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
> >>+    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, INITRD_LOAD_ADDR);
> >>+    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, 0); /* not used */
> >>+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
> >>+    qemu_register_boot_set(fw_cfg_boot_set, NULL);
> >>  }
> >>
> >>  /* SPARCserver 1000 hardware initialisation */
> >>@@ -1719,7 +1717,6 @@ static void sun4c_hw_init(const struct sun4c_hwdef *hwdef, ram_addr_t RAM_size,
> >>      qemu_irq fdc_tc;
> >>      unsigned long kernel_size;
> >>      DriveInfo *fd[MAX_FD];
> >>-    void *fw_cfg;
> >>      DeviceState *dev;
> >>      unsigned int i;
> >>
> >>@@ -1798,26 +1795,26 @@ static void sun4c_hw_init(const struct sun4c_hwdef *hwdef, ram_addr_t RAM_size,
> >>                 graphic_height, graphic_depth, hwdef->nvram_machine_id,
> >>                 "Sun4c");
> >>
> >>-    fw_cfg = fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
> >>-    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> >>-    fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, hwdef->machine_id);
> >>-    fw_cfg_add_i16(fw_cfg, FW_CFG_SUN4M_DEPTH, graphic_depth);
> >>-    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);
> >>+    fw_cfg_init(0, 0, CFG_ADDR, CFG_ADDR + 2);
> >>+    fw_cfg_add_i32(FW_CFG_ID, 1);
> >>+    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> >>+    fw_cfg_add_i16(FW_CFG_MACHINE_ID, hwdef->machine_id);
> >>+    fw_cfg_add_i16(FW_CFG_SUN4M_DEPTH, graphic_depth);
> >>+    fw_cfg_add_i32(FW_CFG_KERNEL_ADDR, KERNEL_LOAD_ADDR);
> >>+    fw_cfg_add_i32(FW_CFG_KERNEL_SIZE, kernel_size);
> >>      if (kernel_cmdline) {
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
> >>+        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
> >>          pstrcpy_targphys("cmdline", CMDLINE_ADDR, TARGET_PAGE_SIZE, kernel_cmdline);
> >>-        fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
> >>+        fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
> >>                           (uint8_t*)strdup(kernel_cmdline),
> >>                           strlen(kernel_cmdline) + 1);
> >>      } else {
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
> >>+        fw_cfg_add_i32(FW_CFG_KERNEL_CMDLINE, 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]);
> >>-    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
> >>+    fw_cfg_add_i32(FW_CFG_INITRD_ADDR, INITRD_LOAD_ADDR);
> >>+    fw_cfg_add_i32(FW_CFG_INITRD_SIZE, 0); /* not used */
> >>+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
> >>+    qemu_register_boot_set(fw_cfg_boot_set, NULL);
> >>  }
> >>
> >>  /* SPARCstation 2 hardware initialisation */
> >>diff --git a/hw/sun4u.c b/hw/sun4u.c
> >>index 517bdb8..094c07c 100644
> >>--- a/hw/sun4u.c
> >>+++ b/hw/sun4u.c
> >>@@ -125,7 +125,7 @@ void DMA_register_channel (int nchan,
> >>
> >>  static int fw_cfg_boot_set(void *opaque, const char *boot_device)
> >>  {
> >>-    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
> >>+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_device[0]);
> >>      return 0;
> >>  }
> >>
> >>@@ -802,7 +802,6 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
> >>      qemu_irq *ivec_irqs, *pbm_irqs;
> >>      DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> >>      DriveInfo *fd[MAX_FD];
> >>-    void *fw_cfg;
> >>
> >>      /* init CPUs */
> >>      env = cpu_devinit(cpu_model, hwdef);
> >>@@ -867,30 +866,30 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
> >>                             graphic_width, graphic_height, graphic_depth,
> >>                             (uint8_t *)&nd_table[0].macaddr);
> >>
> >>-    fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
> >>-    fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
> >>-    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> >>-    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);
> >>+    fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
> >>+    fw_cfg_add_i32(FW_CFG_ID, 1);
> >>+    fw_cfg_add_i64(FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> >>+    fw_cfg_add_i16(FW_CFG_MACHINE_ID, hwdef->machine_id);
> >>+    fw_cfg_add_i64(FW_CFG_KERNEL_ADDR, kernel_entry);
> >>+    fw_cfg_add_i64(FW_CFG_KERNEL_SIZE, kernel_size);
> >>      if (kernel_cmdline) {
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,
> >>+        fw_cfg_add_i32(FW_CFG_CMDLINE_SIZE,
> >>                         strlen(kernel_cmdline) + 1);
> >>-        fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA,
> >>+        fw_cfg_add_bytes(FW_CFG_CMDLINE_DATA,
> >>                           (uint8_t*)strdup(kernel_cmdline),
> >>                           strlen(kernel_cmdline) + 1);
> >>      } else {
> >>-        fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, 0);
> >>+        fw_cfg_add_i32(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_i64(FW_CFG_INITRD_ADDR, initrd_addr);
> >>+    fw_cfg_add_i64(FW_CFG_INITRD_SIZE, initrd_size);
> >>+    fw_cfg_add_i16(FW_CFG_BOOT_DEVICE, boot_devices[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);
> >>-    fw_cfg_add_i16(fw_cfg, FW_CFG_SPARC64_DEPTH, graphic_depth);
> >>+    fw_cfg_add_i16(FW_CFG_SPARC64_WIDTH, graphic_width);
> >>+    fw_cfg_add_i16(FW_CFG_SPARC64_HEIGHT, graphic_height);
> >>+    fw_cfg_add_i16(FW_CFG_SPARC64_DEPTH, graphic_depth);
> >>
> >>-    qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
> >>+    qemu_register_boot_set(fw_cfg_boot_set, NULL);
> >>  }
> >>
> >>  enum {
> >>--
> >>1.7.7.3
> >>
> >
> >--
> >			Gleb.

--
			Gleb.

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

* Re: [Qemu-devel] [PATCH 2/2] Add PIIX4 properties to control PM system states.
  2012-05-23 13:27   ` Anthony Liguori
@ 2012-05-23 13:34     ` Gleb Natapov
  2012-05-23 13:48       ` Anthony Liguori
  2012-05-23 14:34     ` Andreas Färber
  1 sibling, 1 reply; 18+ messages in thread
From: Gleb Natapov @ 2012-05-23 13:34 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: qemu-devel

On Wed, May 23, 2012 at 08:27:01AM -0500, Anthony Liguori wrote:
> On 05/20/2012 04:02 AM, Gleb Natapov wrote:
> >This patch adds two things. First it allows QEMU to distinguish between
> >regular powerdown and S4 powerdown. Later separate QMP notification will
> >be added for S4 powerdown. Second it allows S3/S4 states to be disabled
> >from QEMU command line. Some guests known to be broken with regards to
> >power management, but allow to use it anyway. Using new properties
> >management will be able to disable S3/S4 for such guests.
> >
> >Supported system state are passed to a firmware using new fw_cfg file.
> >The file contains  6 byte array. Each byte represents one system
> >state. If byte at offset X has its MSB set it means that system state
> >X is supported and to enter it guest should use the value from lowest 3
> >bits.
> >
> >Signed-off-by: Gleb Natapov<gleb@redhat.com>
> 
> I see nothing wrong in principle here except that you should use a
> PTR property to pass the fw_cfg object to the ACPI PM device.
> 
Hmm, need to check that we have fw_cfg handy when the device is created.
Or can I add it through global?

> Regards,
> 
> Anthony Liguori
> 
> >---
> >  hw/acpi.c       |    5 ++++-
> >  hw/acpi.h       |    2 +-
> >  hw/acpi_piix4.c |   16 +++++++++++++++-
> >  hw/vt82c686.c   |    2 +-
> >  4 files changed, 21 insertions(+), 4 deletions(-)
> >
> >diff --git a/hw/acpi.c b/hw/acpi.c
> >index 5d521e5..effc7ec 100644
> >--- a/hw/acpi.c
> >+++ b/hw/acpi.c
> >@@ -370,7 +370,7 @@ void acpi_pm1_cnt_init(ACPIREGS *ar)
> >      qemu_register_wakeup_notifier(&ar->wakeup);
> >  }
> >
> >-void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val)
> >+void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val, char s4)
> >  {
> >      ar->pm1.cnt.cnt = val&  ~(ACPI_BITMASK_SLEEP_ENABLE);
> >
> >@@ -385,6 +385,9 @@ void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val)
> >              qemu_system_suspend_request();
> >              break;
> >          default:
> >+            if (sus_typ == s4) { /* S4 request */
> >+                qemu_system_shutdown_request();
> >+            }
> >              break;
> >          }
> >      }
> >diff --git a/hw/acpi.h b/hw/acpi.h
> >index fe8cdb4..7337f41 100644
> >--- a/hw/acpi.h
> >+++ b/hw/acpi.h
> >@@ -139,7 +139,7 @@ void acpi_pm1_evt_reset(ACPIREGS *ar);
> >
> >  /* PM1a_CNT: piix and ich9 don't implement PM1b CNT. */
> >  void acpi_pm1_cnt_init(ACPIREGS *ar);
> >-void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val);
> >+void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val, char s4);
> >  void acpi_pm1_cnt_update(ACPIREGS *ar,
> >                           bool sci_enable, bool sci_disable);
> >  void acpi_pm1_cnt_reset(ACPIREGS *ar);
> >diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> >index 585da4e..883314d 100644
> >--- a/hw/acpi_piix4.c
> >+++ b/hw/acpi_piix4.c
> >@@ -27,6 +27,7 @@
> >  #include "sysemu.h"
> >  #include "range.h"
> >  #include "ioport.h"
> >+#include "fw_cfg.h"
> >
> >  //#define DEBUG
> >
> >@@ -71,6 +72,10 @@ typedef struct PIIX4PMState {
> >      struct pci_status pci0_status;
> >      uint32_t pci0_hotplug_enable;
> >      uint32_t pci0_slot_device_present;
> >+
> >+    uint8_t disable_s3;
> >+    uint8_t disable_s4;
> >+    uint8_t s4_val;
> >  } PIIX4PMState;
> >
> >  static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s);
> >@@ -123,7 +128,7 @@ static void pm_ioport_write(IORange *ioport, uint64_t addr, unsigned width,
> >          pm_update_sci(s);
> >          break;
> >      case 0x04:
> >-        acpi_pm1_cnt_write(&s->ar, val);
> >+        acpi_pm1_cnt_write(&s->ar, val, s->s4_val);
> >          break;
> >      default:
> >          break;
> >@@ -425,6 +430,7 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
> >  {
> >      PCIDevice *dev;
> >      PIIX4PMState *s;
> >+    uint8_t suspend[6] = {128, 0, 0, 129, 128, 128};
> >
> >      dev = pci_create(bus, devfn, "PIIX4_PM");
> >      qdev_prop_set_uint32(&dev->qdev, "smb_io_base", smb_io_base);
> >@@ -437,11 +443,19 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
> >
> >      qdev_init_nofail(&dev->qdev);
> >
> >+    suspend[3] = 1 | ((!s->disable_s3)<<  7);
> >+    suspend[4] = s->s4_val | ((!s->disable_s4)<<  7);
> >+
> >+    fw_cfg_add_file("etc/system-states", g_memdup(suspend, 6), 6);
> >+
> >      return s->smb.smbus;
> >  }
> >
> >  static Property piix4_pm_properties[] = {
> >      DEFINE_PROP_UINT32("smb_io_base", PIIX4PMState, smb_io_base, 0),
> >+    DEFINE_PROP_UINT8("disable_s3", PIIX4PMState, disable_s3, 0),
> >+    DEFINE_PROP_UINT8("disable_s4", PIIX4PMState, disable_s4, 0),
> >+    DEFINE_PROP_UINT8("s4_val", PIIX4PMState, s4_val, 2),
> >      DEFINE_PROP_END_OF_LIST(),
> >  };
> >
> >diff --git a/hw/vt82c686.c b/hw/vt82c686.c
> >index 6fb7950..5d7c00c 100644
> >--- a/hw/vt82c686.c
> >+++ b/hw/vt82c686.c
> >@@ -210,7 +210,7 @@ static void pm_ioport_writew(void *opaque, uint32_t addr, uint32_t val)
> >          pm_update_sci(s);
> >          break;
> >      case 0x04:
> >-        acpi_pm1_cnt_write(&s->ar, val);
> >+        acpi_pm1_cnt_write(&s->ar, val, 0);
> >          break;
> >      default:
> >          break;

--
			Gleb.

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

* Re: [Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global.
  2012-05-23 13:32     ` Gleb Natapov
@ 2012-05-23 13:46       ` Anthony Liguori
  0 siblings, 0 replies; 18+ messages in thread
From: Anthony Liguori @ 2012-05-23 13:46 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: blauwirbel, qemu-devel

On 05/23/2012 08:32 AM, Gleb Natapov wrote:
> On Wed, May 23, 2012 at 08:25:35AM -0500, Anthony Liguori wrote:
>> On 05/23/2012 07:37 AM, Gleb Natapov wrote:
>>> Ping.
>>
>> I don't understand why you're pinging..  The patch has just been on
>> the list for a couple of days and is definitely not a 1.1 candidate.
>> Am I missing something?
>>
> It is definitely not 1.1 candidate. Only 1.1 patches are accepted now?
> When master will be opened for 1.2 commits?

After 1.1 is released (June 1st).

>
>> However...
> Well, I am pinging for review :)
>
>>
>>>
>>> On Sun, May 20, 2012 at 12:02:40PM +0300, Gleb Natapov wrote:
>>>> There can be only one fw_cfg device, so saving global reference to it
>>>> removes the need to pass its pointer around.
>>>>
>>>> Signed-off-by: Gleb Natapov<gleb@redhat.com>
>>>> ---
>>>>   hw/fw_cfg.c       |  110 +++++++++++++++++++++++++++--------------------------
>>>>   hw/fw_cfg.h       |   15 +++----
>>>>   hw/loader.c       |   10 +----
>>>>   hw/loader.h       |    1 -
>>>>   hw/multiboot.c    |   17 ++++----
>>>>   hw/multiboot.h    |    3 +-
>>>>   hw/pc.c           |   63 ++++++++++++++----------------
>>>>   hw/ppc_newworld.c |   43 ++++++++++-----------
>>>>   hw/ppc_oldworld.c |   43 ++++++++++-----------
>>>>   hw/sun4m.c        |   93 +++++++++++++++++++++-----------------------
>>>>   hw/sun4u.c        |   35 ++++++++---------
>>>>   11 files changed, 207 insertions(+), 226 deletions(-)
>>>>
>>>> diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
>>>> index 7b3b576..8c50473 100644
>>>> --- a/hw/fw_cfg.c
>>>> +++ b/hw/fw_cfg.c
>>>> @@ -48,7 +48,7 @@ typedef struct FWCfgEntry {
>>>>       FWCfgCallback callback;
>>>>   } FWCfgEntry;
>>>>
>>>> -struct FWCfgState {
>>>> +static struct FWCfgState {
>>>>       SysBusDevice busdev;
>>>>       MemoryRegion ctl_iomem, data_iomem, comb_iomem;
>>>>       uint32_t ctl_iobase, data_iobase;
>>>> @@ -57,7 +57,7 @@ struct FWCfgState {
>>>>       uint16_t cur_entry;
>>>>       uint32_t cur_offset;
>>>>       Notifier machine_ready;
>>>> -};
>>>> +} *fw_cfg;
>>>>
>>>>   #define JPG_FILE 0
>>>>   #define BMP_FILE 1
>>>> @@ -113,7 +113,7 @@ error:
>>>>       return NULL;
>>>>   }
>>>>
>>>> -static void fw_cfg_bootsplash(FWCfgState *s)
>>
>> This is a step backwards IMHO.  Relying on global state is generally
>> a bad thing.  Passing around pointers is a good thing because it
>> forces you to think about the relationships between devices and
>> makes it hard to do silly things (like have random interactions with
>> fw_cfg in devices that have no business doing that).
>>
> We are in a kind of agreement here, but fw_cfg is this rare thing that
> wants to be accessed by devices which, on a first glance, have no
> business doing that. We already saving fw_cfg globally for rom loaders
> to use, not nice either.

Rom loaders are an exception because they aren't being modeled as a device 
today.  For devices, we want the interactions to be expressed via the QOM graph 
which for now means having a PTR property (although actually on qom-next, it 
should be possible to do a proper link property).

Since you're interacting with fw_cfg in a proper device, you really should do so 
through the device interface (in this case, FWcfgState).

Regards,

Anthony Liguori

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

* Re: [Qemu-devel] [PATCH 2/2] Add PIIX4 properties to control PM system states.
  2012-05-23 13:34     ` Gleb Natapov
@ 2012-05-23 13:48       ` Anthony Liguori
  0 siblings, 0 replies; 18+ messages in thread
From: Anthony Liguori @ 2012-05-23 13:48 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: qemu-devel

On 05/23/2012 08:34 AM, Gleb Natapov wrote:
> On Wed, May 23, 2012 at 08:27:01AM -0500, Anthony Liguori wrote:
>> On 05/20/2012 04:02 AM, Gleb Natapov wrote:
>>> This patch adds two things. First it allows QEMU to distinguish between
>>> regular powerdown and S4 powerdown. Later separate QMP notification will
>>> be added for S4 powerdown. Second it allows S3/S4 states to be disabled
>> >from QEMU command line. Some guests known to be broken with regards to
>>> power management, but allow to use it anyway. Using new properties
>>> management will be able to disable S3/S4 for such guests.
>>>
>>> Supported system state are passed to a firmware using new fw_cfg file.
>>> The file contains  6 byte array. Each byte represents one system
>>> state. If byte at offset X has its MSB set it means that system state
>>> X is supported and to enter it guest should use the value from lowest 3
>>> bits.
>>>
>>> Signed-off-by: Gleb Natapov<gleb@redhat.com>
>>
>> I see nothing wrong in principle here except that you should use a
>> PTR property to pass the fw_cfg object to the ACPI PM device.
>>
> Hmm, need to check that we have fw_cfg handy when the device is created.

Yes, the pc.c/pc_piix.c split needs to die...

For now, just add another output parameter to pc_memory_init to return fw_cfg. 
Then you can pass it to piix4_pm_init().

Regards,

Anthony Liguori

> Or can I add it through global?

>
>> Regards,
>>
>> Anthony Liguori
>>
>>> ---
>>>   hw/acpi.c       |    5 ++++-
>>>   hw/acpi.h       |    2 +-
>>>   hw/acpi_piix4.c |   16 +++++++++++++++-
>>>   hw/vt82c686.c   |    2 +-
>>>   4 files changed, 21 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/acpi.c b/hw/acpi.c
>>> index 5d521e5..effc7ec 100644
>>> --- a/hw/acpi.c
>>> +++ b/hw/acpi.c
>>> @@ -370,7 +370,7 @@ void acpi_pm1_cnt_init(ACPIREGS *ar)
>>>       qemu_register_wakeup_notifier(&ar->wakeup);
>>>   }
>>>
>>> -void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val)
>>> +void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val, char s4)
>>>   {
>>>       ar->pm1.cnt.cnt = val&   ~(ACPI_BITMASK_SLEEP_ENABLE);
>>>
>>> @@ -385,6 +385,9 @@ void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val)
>>>               qemu_system_suspend_request();
>>>               break;
>>>           default:
>>> +            if (sus_typ == s4) { /* S4 request */
>>> +                qemu_system_shutdown_request();
>>> +            }
>>>               break;
>>>           }
>>>       }
>>> diff --git a/hw/acpi.h b/hw/acpi.h
>>> index fe8cdb4..7337f41 100644
>>> --- a/hw/acpi.h
>>> +++ b/hw/acpi.h
>>> @@ -139,7 +139,7 @@ void acpi_pm1_evt_reset(ACPIREGS *ar);
>>>
>>>   /* PM1a_CNT: piix and ich9 don't implement PM1b CNT. */
>>>   void acpi_pm1_cnt_init(ACPIREGS *ar);
>>> -void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val);
>>> +void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val, char s4);
>>>   void acpi_pm1_cnt_update(ACPIREGS *ar,
>>>                            bool sci_enable, bool sci_disable);
>>>   void acpi_pm1_cnt_reset(ACPIREGS *ar);
>>> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
>>> index 585da4e..883314d 100644
>>> --- a/hw/acpi_piix4.c
>>> +++ b/hw/acpi_piix4.c
>>> @@ -27,6 +27,7 @@
>>>   #include "sysemu.h"
>>>   #include "range.h"
>>>   #include "ioport.h"
>>> +#include "fw_cfg.h"
>>>
>>>   //#define DEBUG
>>>
>>> @@ -71,6 +72,10 @@ typedef struct PIIX4PMState {
>>>       struct pci_status pci0_status;
>>>       uint32_t pci0_hotplug_enable;
>>>       uint32_t pci0_slot_device_present;
>>> +
>>> +    uint8_t disable_s3;
>>> +    uint8_t disable_s4;
>>> +    uint8_t s4_val;
>>>   } PIIX4PMState;
>>>
>>>   static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s);
>>> @@ -123,7 +128,7 @@ static void pm_ioport_write(IORange *ioport, uint64_t addr, unsigned width,
>>>           pm_update_sci(s);
>>>           break;
>>>       case 0x04:
>>> -        acpi_pm1_cnt_write(&s->ar, val);
>>> +        acpi_pm1_cnt_write(&s->ar, val, s->s4_val);
>>>           break;
>>>       default:
>>>           break;
>>> @@ -425,6 +430,7 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
>>>   {
>>>       PCIDevice *dev;
>>>       PIIX4PMState *s;
>>> +    uint8_t suspend[6] = {128, 0, 0, 129, 128, 128};
>>>
>>>       dev = pci_create(bus, devfn, "PIIX4_PM");
>>>       qdev_prop_set_uint32(&dev->qdev, "smb_io_base", smb_io_base);
>>> @@ -437,11 +443,19 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
>>>
>>>       qdev_init_nofail(&dev->qdev);
>>>
>>> +    suspend[3] = 1 | ((!s->disable_s3)<<   7);
>>> +    suspend[4] = s->s4_val | ((!s->disable_s4)<<   7);
>>> +
>>> +    fw_cfg_add_file("etc/system-states", g_memdup(suspend, 6), 6);
>>> +
>>>       return s->smb.smbus;
>>>   }
>>>
>>>   static Property piix4_pm_properties[] = {
>>>       DEFINE_PROP_UINT32("smb_io_base", PIIX4PMState, smb_io_base, 0),
>>> +    DEFINE_PROP_UINT8("disable_s3", PIIX4PMState, disable_s3, 0),
>>> +    DEFINE_PROP_UINT8("disable_s4", PIIX4PMState, disable_s4, 0),
>>> +    DEFINE_PROP_UINT8("s4_val", PIIX4PMState, s4_val, 2),
>>>       DEFINE_PROP_END_OF_LIST(),
>>>   };
>>>
>>> diff --git a/hw/vt82c686.c b/hw/vt82c686.c
>>> index 6fb7950..5d7c00c 100644
>>> --- a/hw/vt82c686.c
>>> +++ b/hw/vt82c686.c
>>> @@ -210,7 +210,7 @@ static void pm_ioport_writew(void *opaque, uint32_t addr, uint32_t val)
>>>           pm_update_sci(s);
>>>           break;
>>>       case 0x04:
>>> -        acpi_pm1_cnt_write(&s->ar, val);
>>> +        acpi_pm1_cnt_write(&s->ar, val, 0);
>>>           break;
>>>       default:
>>>           break;
>
> --
> 			Gleb.

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

* Re: [Qemu-devel] [PATCH 2/2] Add PIIX4 properties to control PM system states.
  2012-05-23 13:27   ` Anthony Liguori
  2012-05-23 13:34     ` Gleb Natapov
@ 2012-05-23 14:34     ` Andreas Färber
  2012-05-23 14:42       ` Paolo Bonzini
  1 sibling, 1 reply; 18+ messages in thread
From: Andreas Färber @ 2012-05-23 14:34 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: Paolo Bonzini, qemu-devel, Gleb Natapov

Am 23.05.2012 15:27, schrieb Anthony Liguori:
> On 05/20/2012 04:02 AM, Gleb Natapov wrote:
>> This patch adds two things. First it allows QEMU to distinguish between
>> regular powerdown and S4 powerdown. Later separate QMP notification will
>> be added for S4 powerdown. Second it allows S3/S4 states to be disabled
>> from QEMU command line. Some guests known to be broken with regards to
>> power management, but allow to use it anyway. Using new properties
>> management will be able to disable S3/S4 for such guests.
>>
>> Supported system state are passed to a firmware using new fw_cfg file.
>> The file contains  6 byte array. Each byte represents one system
>> state. If byte at offset X has its MSB set it means that system state
>> X is supported and to enter it guest should use the value from lowest 3
>> bits.
>>
>> Signed-off-by: Gleb Natapov<gleb@redhat.com>
> 
> I see nothing wrong in principle here except that you should use a PTR
> property to pass the fw_cfg object to the ACPI PM device.

Paolo was on a quest to eliminate the PTR properties so I don't think we
should advocate adding any more. Without having reviewed the code, I
would suggest to rather QOM'ify the fw_cfg object and to use a link<>
property if needed.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global.
  2012-05-23 12:44 ` Peter Maydell
  2012-05-23 12:47   ` Gleb Natapov
@ 2012-05-23 14:41   ` Andreas Färber
  2012-05-23 14:54     ` Peter Maydell
  1 sibling, 1 reply; 18+ messages in thread
From: Andreas Färber @ 2012-05-23 14:41 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel, Gleb Natapov

Am 23.05.2012 14:44, schrieb Peter Maydell:
> On 20 May 2012 10:02, Gleb Natapov <gleb@redhat.com> wrote:
>> There can be only one fw_cfg device, so saving global reference to it
>> removes the need to pass its pointer around.
> 
> This seems like a backwards step to me: one of the things that prevents
> us supporting "two separate machines in one emulation" is that we have
> various things that assume there's only one instance of themselves and
> use globals. I don't think we should be adding any more...

While I concur with the design sentiment of avoiding globals where
possible, I can understand having multiple CPU types in one machine, but
multiple machines in one emulation? Do you have a specific example that
cannot easily be modeled as multiple socket-connected executables?

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [PATCH 2/2] Add PIIX4 properties to control PM system states.
  2012-05-23 14:34     ` Andreas Färber
@ 2012-05-23 14:42       ` Paolo Bonzini
  2012-05-23 15:11         ` Anthony Liguori
  0 siblings, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2012-05-23 14:42 UTC (permalink / raw)
  To: Andreas Färber; +Cc: Gleb Natapov, Anthony Liguori, qemu-devel

Il 23/05/2012 16:34, Andreas Färber ha scritto:
>> > 
>> > I see nothing wrong in principle here except that you should use a PTR
>> > property to pass the fw_cfg object to the ACPI PM device.
> Paolo was on a quest to eliminate the PTR properties so I don't think we
> should advocate adding any more. Without having reviewed the code, I
> would suggest to rather QOM'ify the fw_cfg object and to use a link<>
> property if needed.

Perhaps it should be the other way round.  fw_cfg gets links to all the
devices it has to expose information about.  In the future it could be
changed to an FWCfgProvider interface.

Paolo

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

* Re: [Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global.
  2012-05-23 14:41   ` Andreas Färber
@ 2012-05-23 14:54     ` Peter Maydell
  0 siblings, 0 replies; 18+ messages in thread
From: Peter Maydell @ 2012-05-23 14:54 UTC (permalink / raw)
  To: Andreas Färber; +Cc: qemu-devel, Gleb Natapov

On 23 May 2012 15:41, Andreas Färber <afaerber@suse.de> wrote:
> Am 23.05.2012 14:44, schrieb Peter Maydell:
>> On 20 May 2012 10:02, Gleb Natapov <gleb@redhat.com> wrote:
>>> There can be only one fw_cfg device, so saving global reference to it
>>> removes the need to pass its pointer around.
>>
>> This seems like a backwards step to me: one of the things that prevents
>> us supporting "two separate machines in one emulation" is that we have
>> various things that assume there's only one instance of themselves and
>> use globals. I don't think we should be adding any more...
>
> While I concur with the design sentiment of avoiding globals where
> possible, I can understand having multiple CPU types in one machine, but
> multiple machines in one emulation? Do you have a specific example that
> cannot easily be modeled as multiple socket-connected executables?

I mostly meant the "two different CPUs" case, but there's not
a great deal of difference between them. You're just instantiating
a set of QOM objects and wiring them up appropriately in either
case: if you can support different CPUs then you can trivially
support multiple machines, because that's just a special case of
two separate CPUs which happen to not share any memory.

We don't have any support for running QOM interface links over
sockets anyway, so that seems like a red herring.

-- PMM

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

* Re: [Qemu-devel] [PATCH 2/2] Add PIIX4 properties to control PM system states.
  2012-05-23 14:42       ` Paolo Bonzini
@ 2012-05-23 15:11         ` Anthony Liguori
  2012-05-23 15:16           ` Gleb Natapov
  0 siblings, 1 reply; 18+ messages in thread
From: Anthony Liguori @ 2012-05-23 15:11 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Andreas Färber, Gleb Natapov, qemu-devel

On 05/23/2012 09:42 AM, Paolo Bonzini wrote:
> Il 23/05/2012 16:34, Andreas Färber ha scritto:
>>>>
>>>> I see nothing wrong in principle here except that you should use a PTR
>>>> property to pass the fw_cfg object to the ACPI PM device.
>> Paolo was on a quest to eliminate the PTR properties so I don't think we
>> should advocate adding any more. Without having reviewed the code, I
>> would suggest to rather QOM'ify the fw_cfg object and to use a link<>
>> property if needed.

fw_cfg is already a QOM object.  I think there's enough in the tree already to 
do link<>s properly so that's definitely better than doing a PTR.

>
> Perhaps it should be the other way round.  fw_cfg gets links to all the
> devices it has to expose information about.  In the future it could be
> changed to an FWCfgProvider interface.

Yes, that does make more sense.

Regards,

Anthony Liguori

>
> Paolo

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

* Re: [Qemu-devel] [PATCH 2/2] Add PIIX4 properties to control PM system states.
  2012-05-23 15:11         ` Anthony Liguori
@ 2012-05-23 15:16           ` Gleb Natapov
  0 siblings, 0 replies; 18+ messages in thread
From: Gleb Natapov @ 2012-05-23 15:16 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: Paolo Bonzini, Andreas Färber, qemu-devel

On Wed, May 23, 2012 at 10:11:08AM -0500, Anthony Liguori wrote:
> On 05/23/2012 09:42 AM, Paolo Bonzini wrote:
> >Il 23/05/2012 16:34, Andreas Färber ha scritto:
> >>>>
> >>>>I see nothing wrong in principle here except that you should use a PTR
> >>>>property to pass the fw_cfg object to the ACPI PM device.
> >>Paolo was on a quest to eliminate the PTR properties so I don't think we
> >>should advocate adding any more. Without having reviewed the code, I
> >>would suggest to rather QOM'ify the fw_cfg object and to use a link<>
> >>property if needed.
> 
> fw_cfg is already a QOM object.  I think there's enough in the tree
> already to do link<>s properly so that's definitely better than
> doing a PTR.
> 
If I do what you suggested and return fw_cfg from pc_memory_init() to
pass it to piix4_pm_init() I do not need any links since piix4_pm_init()
is where it is used.

> >
> >Perhaps it should be the other way round.  fw_cfg gets links to all the
> >devices it has to expose information about.  In the future it could be
> >changed to an FWCfgProvider interface.
> 
> Yes, that does make more sense.
> 
> Regards,
> 
> Anthony Liguori
> 
> >
> >Paolo

--
			Gleb.

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

* [Qemu-devel] [PATCH 2/2] Add PIIX4 properties to control PM system states.
  2012-05-14 12:34 Gleb Natapov
@ 2012-05-14 12:34 ` Gleb Natapov
  0 siblings, 0 replies; 18+ messages in thread
From: Gleb Natapov @ 2012-05-14 12:34 UTC (permalink / raw)
  To: qemu-devel

This patch adds two things. First it allows QEMU to distinguish between
regular powerdown and S4 powerdown. Later separate QMP notification will
be added for S4 powerdown. Second it allows S3/S4 states to be disabled
from QEMU command line. Some guests known to be broken with regards to
power management, but allow to use it anyway. Using new properties
management will be able to disable S3/S4 for such guests.

Supported system state are passed to a firmware using new fw_cfg entry.
The entry contains  6 byte array. Each byte represents one system
state. If byte at offset X has its MSB set it means that system state
X is supported and to enter it guest should use the value from lowest 7
bits.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 hw/acpi.c       |    5 ++++-
 hw/acpi.h       |    2 +-
 hw/acpi_piix4.c |   21 +++++++++++++++++++--
 hw/mips_malta.c |    3 ++-
 hw/pc.c         |    6 ------
 hw/pc.h         |   10 +++++++++-
 hw/pc_piix.c    |    2 +-
 hw/vt82c686.c   |    2 +-
 8 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index 5d521e5..effc7ec 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -370,7 +370,7 @@ void acpi_pm1_cnt_init(ACPIREGS *ar)
     qemu_register_wakeup_notifier(&ar->wakeup);
 }
 
-void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val)
+void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val, char s4)
 {
     ar->pm1.cnt.cnt = val & ~(ACPI_BITMASK_SLEEP_ENABLE);
 
@@ -385,6 +385,9 @@ void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val)
             qemu_system_suspend_request();
             break;
         default:
+            if (sus_typ == s4) { /* S4 request */
+                qemu_system_shutdown_request();
+            }
             break;
         }
     }
diff --git a/hw/acpi.h b/hw/acpi.h
index fe8cdb4..7337f41 100644
--- a/hw/acpi.h
+++ b/hw/acpi.h
@@ -139,7 +139,7 @@ void acpi_pm1_evt_reset(ACPIREGS *ar);
 
 /* PM1a_CNT: piix and ich9 don't implement PM1b CNT. */
 void acpi_pm1_cnt_init(ACPIREGS *ar);
-void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val);
+void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val, char s4);
 void acpi_pm1_cnt_update(ACPIREGS *ar,
                          bool sci_enable, bool sci_disable);
 void acpi_pm1_cnt_reset(ACPIREGS *ar);
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 585da4e..ce1f5af 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -27,6 +27,7 @@
 #include "sysemu.h"
 #include "range.h"
 #include "ioport.h"
+#include "fw_cfg.h"
 
 //#define DEBUG
 
@@ -71,6 +72,10 @@ typedef struct PIIX4PMState {
     struct pci_status pci0_status;
     uint32_t pci0_hotplug_enable;
     uint32_t pci0_slot_device_present;
+
+    uint8_t disable_s3;
+    uint8_t disable_s4;
+    uint8_t s4_val;
 } PIIX4PMState;
 
 static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s);
@@ -123,7 +128,7 @@ static void pm_ioport_write(IORange *ioport, uint64_t addr, unsigned width,
         pm_update_sci(s);
         break;
     case 0x04:
-        acpi_pm1_cnt_write(&s->ar, val);
+        acpi_pm1_cnt_write(&s->ar, val, s->s4_val);
         break;
     default:
         break;
@@ -421,7 +426,7 @@ static int piix4_pm_initfn(PCIDevice *dev)
 
 i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
                        qemu_irq sci_irq, qemu_irq smi_irq,
-                       int kvm_enabled)
+                       int kvm_enabled, uint16_t fw_cfg_key)
 {
     PCIDevice *dev;
     PIIX4PMState *s;
@@ -437,11 +442,23 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
 
     qdev_init_nofail(&dev->qdev);
 
+    if (fw_cfg_key != FW_CFG_INVALID) {
+        uint8_t suspend[6] = {128, 0, 0, 129, 128, 128};
+
+	suspend[3] = 1 | ((!s->disable_s3) << 7);
+	suspend[4] = s->s4_val | ((!s->disable_s4) << 7);
+	       
+        fw_cfg_add_bytes(fw_cfg_key, g_memdup(suspend, 6), 6);
+    }
+
     return s->smb.smbus;
 }
 
 static Property piix4_pm_properties[] = {
     DEFINE_PROP_UINT32("smb_io_base", PIIX4PMState, smb_io_base, 0),
+    DEFINE_PROP_UINT8("disable_s3", PIIX4PMState, disable_s3, 0),
+    DEFINE_PROP_UINT8("disable_s4", PIIX4PMState, disable_s4, 0),
+    DEFINE_PROP_UINT8("s4_val", PIIX4PMState, s4_val, 2),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/mips_malta.c b/hw/mips_malta.c
index 4752bb2..5f0f5b6 100644
--- a/hw/mips_malta.c
+++ b/hw/mips_malta.c
@@ -48,6 +48,7 @@
 #include "blockdev.h"
 #include "exec-memory.h"
 #include "sysbus.h"             /* SysBusDevice */
+#include "fw_cfg.h"
 
 //#define DEBUG_BOARD_INIT
 
@@ -954,7 +955,7 @@ void mips_malta_init (ram_addr_t ram_size,
     pci_piix4_ide_init(pci_bus, hd, piix4_devfn + 1);
     pci_create_simple(pci_bus, piix4_devfn + 2, "piix4-usb-uhci");
     smbus = piix4_pm_init(pci_bus, piix4_devfn + 3, 0x1100,
-                          isa_get_irq(NULL, 9), NULL, 0);
+                          isa_get_irq(NULL, 9), NULL, 0, FW_CFG_INVALID);
     /* TODO: Populate SPD eeprom data.  */
     smbus_eeprom_init(smbus, 8, NULL, 0);
     pit = pit_init(isa_bus, 0x40, 0, NULL);
diff --git a/hw/pc.c b/hw/pc.c
index 5d28d3b..3a2c2d2 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -29,7 +29,6 @@
 #include "pci.h"
 #include "vmware_vga.h"
 #include "monitor.h"
-#include "fw_cfg.h"
 #include "hpet_emul.h"
 #include "smbios.h"
 #include "loader.h"
@@ -64,11 +63,6 @@
 /* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables.  */
 #define ACPI_DATA_SIZE       0x10000
 #define BIOS_CFG_IOPORT 0x510
-#define FW_CFG_ACPI_TABLES (FW_CFG_ARCH_LOCAL + 0)
-#define FW_CFG_SMBIOS_ENTRIES (FW_CFG_ARCH_LOCAL + 1)
-#define FW_CFG_IRQ0_OVERRIDE (FW_CFG_ARCH_LOCAL + 2)
-#define FW_CFG_E820_TABLE (FW_CFG_ARCH_LOCAL + 3)
-#define FW_CFG_HPET (FW_CFG_ARCH_LOCAL + 4)
 
 #define MSI_ADDR_BASE 0xfee00000
 
diff --git a/hw/pc.h b/hw/pc.h
index 74d3369..4d23712 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -9,6 +9,7 @@
 #include "net.h"
 #include "memory.h"
 #include "ioapic.h"
+#include "fw_cfg.h"
 
 /* PC-style peripherals (also used by other machines).  */
 
@@ -142,7 +143,7 @@ int acpi_table_add(const char *table_desc);
 
 i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
                        qemu_irq sci_irq, qemu_irq smi_irq,
-                       int kvm_enabled);
+                       int kvm_enabled, uint16_t fw_cfg_key);
 void piix4_smbus_register_device(SMBusDevice *dev, uint8_t addr);
 
 /* hpet.c */
@@ -227,4 +228,11 @@ void pc_system_firmware_init(MemoryRegion *rom_memory);
 
 int e820_add_entry(uint64_t, uint64_t, uint32_t);
 
+#define FW_CFG_ACPI_TABLES (FW_CFG_ARCH_LOCAL + 0)
+#define FW_CFG_SMBIOS_ENTRIES (FW_CFG_ARCH_LOCAL + 1)
+#define FW_CFG_IRQ0_OVERRIDE (FW_CFG_ARCH_LOCAL + 2)
+#define FW_CFG_E820_TABLE (FW_CFG_ARCH_LOCAL + 3)
+#define FW_CFG_HPET (FW_CFG_ARCH_LOCAL + 4)
+#define FW_CFG_SYSTEM_STATES (FW_CFG_ARCH_LOCAL + 5)
+
 #endif
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 6a75718..6bd2791 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -280,7 +280,7 @@ static void pc_init1(MemoryRegion *system_memory,
         /* TODO: Populate SPD eeprom data.  */
         smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
                               gsi[9], *smi_irq,
-                              kvm_enabled());
+                              kvm_enabled(), FW_CFG_SYSTEM_STATES);
         smbus_eeprom_init(smbus, 8, NULL, 0);
     }
 
diff --git a/hw/vt82c686.c b/hw/vt82c686.c
index 6fb7950..5d7c00c 100644
--- a/hw/vt82c686.c
+++ b/hw/vt82c686.c
@@ -210,7 +210,7 @@ static void pm_ioport_writew(void *opaque, uint32_t addr, uint32_t val)
         pm_update_sci(s);
         break;
     case 0x04:
-        acpi_pm1_cnt_write(&s->ar, val);
+        acpi_pm1_cnt_write(&s->ar, val, 0);
         break;
     default:
         break;
-- 
1.7.7.3

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

end of thread, other threads:[~2012-05-23 15:16 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-20  9:02 [Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global Gleb Natapov
2012-05-20  9:02 ` [Qemu-devel] [PATCH 2/2] Add PIIX4 properties to control PM system states Gleb Natapov
2012-05-23 13:27   ` Anthony Liguori
2012-05-23 13:34     ` Gleb Natapov
2012-05-23 13:48       ` Anthony Liguori
2012-05-23 14:34     ` Andreas Färber
2012-05-23 14:42       ` Paolo Bonzini
2012-05-23 15:11         ` Anthony Liguori
2012-05-23 15:16           ` Gleb Natapov
2012-05-23 12:37 ` [Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global Gleb Natapov
2012-05-23 13:25   ` Anthony Liguori
2012-05-23 13:32     ` Gleb Natapov
2012-05-23 13:46       ` Anthony Liguori
2012-05-23 12:44 ` Peter Maydell
2012-05-23 12:47   ` Gleb Natapov
2012-05-23 14:41   ` Andreas Färber
2012-05-23 14:54     ` Peter Maydell
  -- strict thread matches above, loose matches on Subject: below --
2012-05-14 12:34 Gleb Natapov
2012-05-14 12:34 ` [Qemu-devel] [PATCH 2/2] Add PIIX4 properties to control PM system states Gleb Natapov

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.