[1/2] Make pointer to fw_cfg device global.

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(-)

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.

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

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.

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.

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.

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

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

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