qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Gleb Natapov <gleb@redhat.com>
Cc: blauwirbel@gmail.com, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global.
Date: Wed, 23 May 2012 08:25:35 -0500	[thread overview]
Message-ID: <4FBCE54F.9070603@codemonkey.ws> (raw)
In-Reply-To: <20120523123731.GF10209@redhat.com>

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.

  reply	other threads:[~2012-05-23 13:25 UTC|newest]

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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FBCE54F.9070603@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=blauwirbel@gmail.com \
    --cc=gleb@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).