Patchwork [v9,3/3] pc: Support system flash memory with pflash

login
register
mail settings
Submitter jordan.l.justen@intel.com
Date Dec. 15, 2011, 8:51 p.m.
Message ID <1323982273-13623-4-git-send-email-jordan.l.justen@intel.com>
Download mbox | patch
Permalink /patch/131730/
State New
Headers show

Comments

jordan.l.justen@intel.com - Dec. 15, 2011, 8:51 p.m.
If a pflash image is found, then it is used for the system
firmware image.

If a pflash image is not initially found, then a read-only
pflash device is created using the -bios filename.

KVM cannot execute from a pflash region currently.
Therefore, when KVM is enabled, a (read-only) ram memory
region is created and filled with the contents of the
pflash drive.

Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
---
 Makefile.target                    |    1 +
 default-configs/i386-softmmu.mak   |    1 +
 default-configs/x86_64-softmmu.mak |    1 +
 hw/boards.h                        |    1 +
 hw/pc.c                            |   55 +-------
 hw/pc.h                            |    4 +
 hw/pc_sysfw.c                      |  255 ++++++++++++++++++++++++++++++++++++
 vl.c                               |    2 +-
 8 files changed, 269 insertions(+), 51 deletions(-)
 create mode 100644 hw/pc_sysfw.c
Anthony Liguori - Dec. 19, 2011, 7:41 p.m.
On 12/15/2011 02:51 PM, Jordan Justen wrote:
> If a pflash image is found, then it is used for the system
> firmware image.
>
> If a pflash image is not initially found, then a read-only
> pflash device is created using the -bios filename.
>
> KVM cannot execute from a pflash region currently.
> Therefore, when KVM is enabled, a (read-only) ram memory
> region is created and filled with the contents of the
> pflash drive.
>
> Signed-off-by: Jordan Justen<jordan.l.justen@intel.com>
> Cc: Anthony Liguori<aliguori@us.ibm.com>
> ---
>   Makefile.target                    |    1 +
>   default-configs/i386-softmmu.mak   |    1 +
>   default-configs/x86_64-softmmu.mak |    1 +
>   hw/boards.h                        |    1 +
>   hw/pc.c                            |   55 +-------
>   hw/pc.h                            |    4 +
>   hw/pc_sysfw.c                      |  255 ++++++++++++++++++++++++++++++++++++
>   vl.c                               |    2 +-
>   8 files changed, 269 insertions(+), 51 deletions(-)
>   create mode 100644 hw/pc_sysfw.c
>
> diff --git a/Makefile.target b/Makefile.target
> index a111521..b1dc882 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -236,6 +236,7 @@ obj-i386-y += vmport.o
>   obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
>   obj-i386-y += debugcon.o multiboot.o
>   obj-i386-y += pc_piix.o
> +obj-i386-y += pc_sysfw.o
>   obj-i386-$(CONFIG_KVM) += kvmclock.o
>   obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
>
> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> index e67ebb3..cd407a9 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -22,3 +22,4 @@ CONFIG_SOUND=y
>   CONFIG_HPET=y
>   CONFIG_APPLESMC=y
>   CONFIG_I8259=y
> +CONFIG_PFLASH_CFI01=y
> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
> index b75757e..47734ea 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -22,3 +22,4 @@ CONFIG_SOUND=y
>   CONFIG_HPET=y
>   CONFIG_APPLESMC=y
>   CONFIG_I8259=y
> +CONFIG_PFLASH_CFI01=y
> diff --git a/hw/boards.h b/hw/boards.h
> index 716fd7b..45a31a1 100644
> --- a/hw/boards.h
> +++ b/hw/boards.h
> @@ -33,6 +33,7 @@ typedef struct QEMUMachine {
>   } QEMUMachine;
>
>   int qemu_register_machine(QEMUMachine *m);
> +QEMUMachine *find_default_machine(void);
>
>   extern QEMUMachine *current_machine;
>
> diff --git a/hw/pc.c b/hw/pc.c
> index cc6cfad..e5550ca 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -57,10 +57,6 @@
>   #define DPRINTF(fmt, ...)
>   #endif
>
> -#define BIOS_FILENAME "bios.bin"
> -
> -#define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
> -
>   /* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables.  */
>   #define ACPI_DATA_SIZE       0x10000
>   #define BIOS_CFG_IOPORT 0x510
> @@ -974,11 +970,9 @@ void pc_memory_init(MemoryRegion *system_memory,
>                       MemoryRegion **ram_memory,
>                       int system_firmware_enabled)
>   {
> -    char *filename;
> -    int ret, linux_boot, i;
> -    MemoryRegion *ram, *bios, *isa_bios, *option_rom_mr;
> +    int linux_boot, i;
> +    MemoryRegion *ram, *option_rom_mr;
>       MemoryRegion *ram_below_4g, *ram_above_4g;
> -    int bios_size, isa_bios_size;
>       void *fw_cfg;
>
>       linux_boot = (kernel_filename != NULL);
> @@ -1003,43 +997,9 @@ void pc_memory_init(MemoryRegion *system_memory,
>                                       ram_above_4g);
>       }
>
> -    /* BIOS load */
> -    if (bios_name == NULL)
> -        bios_name = BIOS_FILENAME;
> -    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> -    if (filename) {
> -        bios_size = get_image_size(filename);
> -    } else {
> -        bios_size = -1;
> -    }
> -    if (bios_size<= 0 ||
> -        (bios_size % 65536) != 0) {
> -        goto bios_error;
> -    }
> -    bios = g_malloc(sizeof(*bios));
> -    memory_region_init_ram(bios, NULL, "pc.bios", bios_size);
> -    memory_region_set_readonly(bios, true);
> -    ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1);
> -    if (ret != 0) {
> -    bios_error:
> -        fprintf(stderr, "qemu: could not load PC BIOS '%s'\n", bios_name);
> -        exit(1);
> -    }
> -    if (filename) {
> -        g_free(filename);
> -    }
> -    /* map the last 128KB of the BIOS in ISA space */
> -    isa_bios_size = bios_size;
> -    if (isa_bios_size>  (128 * 1024))
> -        isa_bios_size = 128 * 1024;
> -    isa_bios = g_malloc(sizeof(*isa_bios));
> -    memory_region_init_alias(isa_bios, "isa-bios", bios,
> -                             bios_size - isa_bios_size, isa_bios_size);
> -    memory_region_add_subregion_overlap(rom_memory,
> -                                        0x100000 - isa_bios_size,
> -                                        isa_bios,
> -                                        1);
> -    memory_region_set_readonly(isa_bios, true);
> +
> +    /* Initialize ROM or flash ranges for PC firmware */
> +    pc_system_firmware_init(rom_memory, system_firmware_enabled);
>
>       option_rom_mr = g_malloc(sizeof(*option_rom_mr));
>       memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE);
> @@ -1048,11 +1008,6 @@ void pc_memory_init(MemoryRegion *system_memory,
>                                           option_rom_mr,
>                                           1);
>
> -    /* map all the bios at the top of memory */
> -    memory_region_add_subregion(rom_memory,
> -                                (uint32_t)(-bios_size),
> -                                bios);
> -
>       fw_cfg = bochs_bios_init();
>       rom_set_fw(fw_cfg);
>
> diff --git a/hw/pc.h b/hw/pc.h
> index 49471cb..727e231 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -246,6 +246,10 @@ static inline bool isa_ne2000_init(int base, int irq, NICInfo *nd)
>       return true;
>   }
>
> +/* pc_sysfw.c */
> +void pc_system_firmware_init(MemoryRegion *rom_memory,
> +                             int system_firmware_enabled);
> +
>   /* e820 types */
>   #define E820_RAM        1
>   #define E820_RESERVED   2
> diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
> new file mode 100644
> index 0000000..20027b2
> --- /dev/null
> +++ b/hw/pc_sysfw.c
> @@ -0,0 +1,255 @@
> +/*
> + * QEMU PC System Firmware
> + *
> + * Copyright (c) 2003-2004 Fabrice Bellard
> + * Copyright (c) 2011 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "hw.h"
> +#include "pc.h"
> +#include "hw/boards.h"
> +#include "loader.h"
> +#include "sysemu.h"
> +#include "flash.h"
> +#include "kvm.h"
> +
> +#define BIOS_FILENAME "bios.bin"
> +
> +static void pc_isa_bios_init(MemoryRegion *rom_memory,
> +                             MemoryRegion *flash_mem,
> +                             int ram_size)
> +{
> +    int isa_bios_size;
> +    MemoryRegion *isa_bios;
> +    uint64_t flash_size;
> +    void *flash_ptr, *isa_bios_ptr;
> +
> +    flash_size = memory_region_size(flash_mem);
> +
> +    /* map the last 128KB of the BIOS in ISA space */
> +    isa_bios_size = flash_size;
> +    if (isa_bios_size>  (128 * 1024)) {
> +        isa_bios_size = 128 * 1024;
> +    }
> +    isa_bios = g_malloc(sizeof(*isa_bios));
> +    memory_region_init_ram(isa_bios, NULL, "isa-bios", isa_bios_size);
> +    memory_region_add_subregion_overlap(rom_memory,
> +                                        0x100000 - isa_bios_size,
> +                                        isa_bios,
> +                                        1);
> +
> +    /* copy ISA rom image from top of flash memory */
> +    flash_ptr = memory_region_get_ram_ptr(flash_mem);
> +    isa_bios_ptr = memory_region_get_ram_ptr(isa_bios);
> +    memcpy(isa_bios_ptr,
> +           ((uint8_t*)flash_ptr) + (flash_size - isa_bios_size),
> +           isa_bios_size);
> +
> +    memory_region_set_readonly(isa_bios, true);
> +}
> +
> +static void pc_fw_add_pflash_drv(void)
> +{
> +    QemuOpts *opts;
> +    QEMUMachine *machine;
> +    char *filename;
> +
> +    if (bios_name == NULL) {
> +        bios_name = BIOS_FILENAME;
> +    }
> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> +
> +    opts = drive_add(IF_PFLASH, -1, filename, "readonly=on");
> +    if (opts == NULL) {
> +      return;
> +    }
> +
> +    machine = find_default_machine();
> +    if (machine == NULL) {
> +      return;
> +    }
> +
> +    drive_init(opts, machine->use_scsi);
> +}
> +
> +static void pc_system_flash_init(MemoryRegion *rom_memory,
> +                                 DriveInfo *pflash_drv)
> +{
> +    BlockDriverState *bdrv;
> +    int64_t size;
> +    target_phys_addr_t phys_addr;
> +    int sector_bits, sector_size;
> +    pflash_t *system_flash;
> +    MemoryRegion *flash_mem;
> +
> +    bdrv = pflash_drv->bdrv;
> +    size = bdrv_getlength(pflash_drv->bdrv);
> +    sector_bits = 12;
> +    sector_size = 1<<  sector_bits;
> +
> +    if ((size % sector_size) != 0) {
> +        fprintf(stderr,
> +                "qemu: PC system firmware (pflash) must be a multiple of 0x%x\n",
> +                sector_size);
> +        exit(1);
> +    }
> +
> +    phys_addr = 0x100000000ULL - size;
> +    system_flash = pflash_cfi01_register(phys_addr, NULL, "system.flash", size,
> +                                         bdrv, sector_size, size>>  sector_bits,
> +                                         1, 0x0000, 0x0000, 0x0000, 0x0000, 0);
> +    flash_mem = pflash_cfi01_get_memory(system_flash);
> +
> +    pc_isa_bios_init(rom_memory, flash_mem, size);
> +}
> +
> +static void pc_system_rom_init(MemoryRegion *rom_memory,
> +                               DriveInfo *pflash_drv)
> +{
> +    BlockDriverState *bdrv;
> +    int64_t size;
> +    target_phys_addr_t phys_addr;
> +    int sector_bits, sector_size;
> +    MemoryRegion *sys_rom;
> +    void *buffer;
> +    int ret;
> +
> +    bdrv = pflash_drv->bdrv;
> +    size = bdrv_getlength(pflash_drv->bdrv);
> +    sector_bits = 9;
> +    sector_size = 1<<  sector_bits;
> +
> +    if ((size % sector_size) != 0) {
> +        fprintf(stderr,
> +                "qemu: PC system rom (pflash) must be a multiple of 0x%x\n",
> +                sector_size);
> +        exit(1);
> +    }
> +
> +    phys_addr = 0x100000000ULL - size;
> +    sys_rom = g_malloc(sizeof(*sys_rom));
> +    memory_region_init_ram(sys_rom, NULL, "system.rom", size);
> +    buffer = memory_region_get_ram_ptr(sys_rom);
> +    memory_region_add_subregion(rom_memory, phys_addr, sys_rom);
> +
> +    /* read the rom content */
> +    ret = bdrv_read(bdrv, 0, buffer, size>>  sector_bits);

I think we're trying to get rid of synchronous block I/O in machine 
initialization for a number of reasons.

Kevin/Stefan, care to comment?  Will this be problematic in the future?

Regards,

Anthony Liguori
Jordan Justen - Dec. 19, 2011, 9:25 p.m.
On Mon, Dec 19, 2011 at 11:41, Anthony Liguori <aliguori@us.ibm.com> wrote:
> On 12/15/2011 02:51 PM, Jordan Justen wrote:
>>
>> If a pflash image is found, then it is used for the system
>> firmware image.
>>
>> If a pflash image is not initially found, then a read-only
>> pflash device is created using the -bios filename.
>>
>> KVM cannot execute from a pflash region currently.
>> Therefore, when KVM is enabled, a (read-only) ram memory
>> region is created and filled with the contents of the
>> pflash drive.
>>
>> Signed-off-by: Jordan Justen<jordan.l.justen@intel.com>
>> Cc: Anthony Liguori<aliguori@us.ibm.com>
>> ---
>>  Makefile.target                    |    1 +
>>  default-configs/i386-softmmu.mak   |    1 +
>>  default-configs/x86_64-softmmu.mak |    1 +
>>  hw/boards.h                        |    1 +
>>  hw/pc.c                            |   55 +-------
>>  hw/pc.h                            |    4 +
>>  hw/pc_sysfw.c                      |  255
>> ++++++++++++++++++++++++++++++++++++
>>  vl.c                               |    2 +-
>>  8 files changed, 269 insertions(+), 51 deletions(-)
>>  create mode 100644 hw/pc_sysfw.c
>>
>> diff --git a/Makefile.target b/Makefile.target
>> index a111521..b1dc882 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -236,6 +236,7 @@ obj-i386-y += vmport.o
>>  obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
>>  obj-i386-y += debugcon.o multiboot.o
>>  obj-i386-y += pc_piix.o
>> +obj-i386-y += pc_sysfw.o
>>  obj-i386-$(CONFIG_KVM) += kvmclock.o
>>  obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
>>
>> diff --git a/default-configs/i386-softmmu.mak
>> b/default-configs/i386-softmmu.mak
>> index e67ebb3..cd407a9 100644
>> --- a/default-configs/i386-softmmu.mak
>> +++ b/default-configs/i386-softmmu.mak
>> @@ -22,3 +22,4 @@ CONFIG_SOUND=y
>>  CONFIG_HPET=y
>>  CONFIG_APPLESMC=y
>>  CONFIG_I8259=y
>> +CONFIG_PFLASH_CFI01=y
>> diff --git a/default-configs/x86_64-softmmu.mak
>> b/default-configs/x86_64-softmmu.mak
>> index b75757e..47734ea 100644
>> --- a/default-configs/x86_64-softmmu.mak
>> +++ b/default-configs/x86_64-softmmu.mak
>> @@ -22,3 +22,4 @@ CONFIG_SOUND=y
>>  CONFIG_HPET=y
>>  CONFIG_APPLESMC=y
>>  CONFIG_I8259=y
>> +CONFIG_PFLASH_CFI01=y
>> diff --git a/hw/boards.h b/hw/boards.h
>> index 716fd7b..45a31a1 100644
>> --- a/hw/boards.h
>> +++ b/hw/boards.h
>> @@ -33,6 +33,7 @@ typedef struct QEMUMachine {
>>  } QEMUMachine;
>>
>>  int qemu_register_machine(QEMUMachine *m);
>> +QEMUMachine *find_default_machine(void);
>>
>>  extern QEMUMachine *current_machine;
>>
>> diff --git a/hw/pc.c b/hw/pc.c
>> index cc6cfad..e5550ca 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -57,10 +57,6 @@
>>  #define DPRINTF(fmt, ...)
>>  #endif
>>
>> -#define BIOS_FILENAME "bios.bin"
>> -
>> -#define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
>> -
>>  /* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables.
>>  */
>>  #define ACPI_DATA_SIZE       0x10000
>>  #define BIOS_CFG_IOPORT 0x510
>> @@ -974,11 +970,9 @@ void pc_memory_init(MemoryRegion *system_memory,
>>                      MemoryRegion **ram_memory,
>>                      int system_firmware_enabled)
>>  {
>> -    char *filename;
>> -    int ret, linux_boot, i;
>> -    MemoryRegion *ram, *bios, *isa_bios, *option_rom_mr;
>> +    int linux_boot, i;
>> +    MemoryRegion *ram, *option_rom_mr;
>>      MemoryRegion *ram_below_4g, *ram_above_4g;
>> -    int bios_size, isa_bios_size;
>>      void *fw_cfg;
>>
>>      linux_boot = (kernel_filename != NULL);
>> @@ -1003,43 +997,9 @@ void pc_memory_init(MemoryRegion *system_memory,
>>                                      ram_above_4g);
>>      }
>>
>> -    /* BIOS load */
>> -    if (bios_name == NULL)
>> -        bios_name = BIOS_FILENAME;
>> -    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>> -    if (filename) {
>> -        bios_size = get_image_size(filename);
>> -    } else {
>> -        bios_size = -1;
>> -    }
>> -    if (bios_size<= 0 ||
>> -        (bios_size % 65536) != 0) {
>> -        goto bios_error;
>> -    }
>> -    bios = g_malloc(sizeof(*bios));
>> -    memory_region_init_ram(bios, NULL, "pc.bios", bios_size);
>> -    memory_region_set_readonly(bios, true);
>> -    ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1);
>> -    if (ret != 0) {
>> -    bios_error:
>> -        fprintf(stderr, "qemu: could not load PC BIOS '%s'\n",
>> bios_name);
>> -        exit(1);
>> -    }
>> -    if (filename) {
>> -        g_free(filename);
>> -    }
>> -    /* map the last 128KB of the BIOS in ISA space */
>> -    isa_bios_size = bios_size;
>> -    if (isa_bios_size>  (128 * 1024))
>> -        isa_bios_size = 128 * 1024;
>> -    isa_bios = g_malloc(sizeof(*isa_bios));
>> -    memory_region_init_alias(isa_bios, "isa-bios", bios,
>> -                             bios_size - isa_bios_size, isa_bios_size);
>> -    memory_region_add_subregion_overlap(rom_memory,
>> -                                        0x100000 - isa_bios_size,
>> -                                        isa_bios,
>> -                                        1);
>> -    memory_region_set_readonly(isa_bios, true);
>> +
>> +    /* Initialize ROM or flash ranges for PC firmware */
>> +    pc_system_firmware_init(rom_memory, system_firmware_enabled);
>>
>>      option_rom_mr = g_malloc(sizeof(*option_rom_mr));
>>      memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE);
>> @@ -1048,11 +1008,6 @@ void pc_memory_init(MemoryRegion *system_memory,
>>                                          option_rom_mr,
>>                                          1);
>>
>> -    /* map all the bios at the top of memory */
>> -    memory_region_add_subregion(rom_memory,
>> -                                (uint32_t)(-bios_size),
>> -                                bios);
>> -
>>      fw_cfg = bochs_bios_init();
>>      rom_set_fw(fw_cfg);
>>
>> diff --git a/hw/pc.h b/hw/pc.h
>> index 49471cb..727e231 100644
>> --- a/hw/pc.h
>> +++ b/hw/pc.h
>> @@ -246,6 +246,10 @@ static inline bool isa_ne2000_init(int base, int irq,
>> NICInfo *nd)
>>      return true;
>>  }
>>
>> +/* pc_sysfw.c */
>> +void pc_system_firmware_init(MemoryRegion *rom_memory,
>> +                             int system_firmware_enabled);
>> +
>>  /* e820 types */
>>  #define E820_RAM        1
>>  #define E820_RESERVED   2
>> diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
>> new file mode 100644
>> index 0000000..20027b2
>> --- /dev/null
>> +++ b/hw/pc_sysfw.c
>> @@ -0,0 +1,255 @@
>> +/*
>> + * QEMU PC System Firmware
>> + *
>> + * Copyright (c) 2003-2004 Fabrice Bellard
>> + * Copyright (c) 2011 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining
>> a copy
>> + * of this software and associated documentation files (the "Software"),
>> to deal
>> + * in the Software without restriction, including without limitation the
>> rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or
>> sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be
>> included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>> SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>> IN
>> + * THE SOFTWARE.
>> + */
>> +
>> +#include "hw.h"
>> +#include "pc.h"
>> +#include "hw/boards.h"
>> +#include "loader.h"
>> +#include "sysemu.h"
>> +#include "flash.h"
>> +#include "kvm.h"
>> +
>> +#define BIOS_FILENAME "bios.bin"
>> +
>> +static void pc_isa_bios_init(MemoryRegion *rom_memory,
>> +                             MemoryRegion *flash_mem,
>> +                             int ram_size)
>> +{
>> +    int isa_bios_size;
>> +    MemoryRegion *isa_bios;
>> +    uint64_t flash_size;
>> +    void *flash_ptr, *isa_bios_ptr;
>> +
>> +    flash_size = memory_region_size(flash_mem);
>> +
>> +    /* map the last 128KB of the BIOS in ISA space */
>> +    isa_bios_size = flash_size;
>> +    if (isa_bios_size>  (128 * 1024)) {
>> +        isa_bios_size = 128 * 1024;
>> +    }
>> +    isa_bios = g_malloc(sizeof(*isa_bios));
>> +    memory_region_init_ram(isa_bios, NULL, "isa-bios", isa_bios_size);
>> +    memory_region_add_subregion_overlap(rom_memory,
>> +                                        0x100000 - isa_bios_size,
>> +                                        isa_bios,
>> +                                        1);
>> +
>> +    /* copy ISA rom image from top of flash memory */
>> +    flash_ptr = memory_region_get_ram_ptr(flash_mem);
>> +    isa_bios_ptr = memory_region_get_ram_ptr(isa_bios);
>> +    memcpy(isa_bios_ptr,
>> +           ((uint8_t*)flash_ptr) + (flash_size - isa_bios_size),
>> +           isa_bios_size);
>> +
>> +    memory_region_set_readonly(isa_bios, true);
>> +}
>> +
>> +static void pc_fw_add_pflash_drv(void)
>> +{
>> +    QemuOpts *opts;
>> +    QEMUMachine *machine;
>> +    char *filename;
>> +
>> +    if (bios_name == NULL) {
>> +        bios_name = BIOS_FILENAME;
>> +    }
>> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>> +
>> +    opts = drive_add(IF_PFLASH, -1, filename, "readonly=on");
>> +    if (opts == NULL) {
>> +      return;
>> +    }
>> +
>> +    machine = find_default_machine();
>> +    if (machine == NULL) {
>> +      return;
>> +    }
>> +
>> +    drive_init(opts, machine->use_scsi);
>> +}
>> +
>> +static void pc_system_flash_init(MemoryRegion *rom_memory,
>> +                                 DriveInfo *pflash_drv)
>> +{
>> +    BlockDriverState *bdrv;
>> +    int64_t size;
>> +    target_phys_addr_t phys_addr;
>> +    int sector_bits, sector_size;
>> +    pflash_t *system_flash;
>> +    MemoryRegion *flash_mem;
>> +
>> +    bdrv = pflash_drv->bdrv;
>> +    size = bdrv_getlength(pflash_drv->bdrv);
>> +    sector_bits = 12;
>> +    sector_size = 1<<  sector_bits;
>> +
>> +    if ((size % sector_size) != 0) {
>> +        fprintf(stderr,
>> +                "qemu: PC system firmware (pflash) must be a multiple of
>> 0x%x\n",
>> +                sector_size);
>> +        exit(1);
>> +    }
>> +
>> +    phys_addr = 0x100000000ULL - size;
>> +    system_flash = pflash_cfi01_register(phys_addr, NULL, "system.flash",
>> size,
>> +                                         bdrv, sector_size, size>>
>>  sector_bits,
>> +                                         1, 0x0000, 0x0000, 0x0000,
>> 0x0000, 0);
>> +    flash_mem = pflash_cfi01_get_memory(system_flash);
>> +
>> +    pc_isa_bios_init(rom_memory, flash_mem, size);
>> +}
>> +
>> +static void pc_system_rom_init(MemoryRegion *rom_memory,
>> +                               DriveInfo *pflash_drv)
>> +{
>> +    BlockDriverState *bdrv;
>> +    int64_t size;
>> +    target_phys_addr_t phys_addr;
>> +    int sector_bits, sector_size;
>> +    MemoryRegion *sys_rom;
>> +    void *buffer;
>> +    int ret;
>> +
>> +    bdrv = pflash_drv->bdrv;
>> +    size = bdrv_getlength(pflash_drv->bdrv);
>> +    sector_bits = 9;
>> +    sector_size = 1<<  sector_bits;
>> +
>> +    if ((size % sector_size) != 0) {
>> +        fprintf(stderr,
>> +                "qemu: PC system rom (pflash) must be a multiple of
>> 0x%x\n",
>> +                sector_size);
>> +        exit(1);
>> +    }
>> +
>> +    phys_addr = 0x100000000ULL - size;
>> +    sys_rom = g_malloc(sizeof(*sys_rom));
>> +    memory_region_init_ram(sys_rom, NULL, "system.rom", size);
>> +    buffer = memory_region_get_ram_ptr(sys_rom);
>> +    memory_region_add_subregion(rom_memory, phys_addr, sys_rom);
>> +
>> +    /* read the rom content */
>> +    ret = bdrv_read(bdrv, 0, buffer, size>>  sector_bits);
>
>
> I think we're trying to get rid of synchronous block I/O in machine
> initialization for a number of reasons.
>
> Kevin/Stefan, care to comment?  Will this be problematic in the future?

I was hoping pc-1.1 with and without kvm could be as close as
possible, but I guess I can make pc-1.1 with kvm behave the same as
pc-1.0.  Then I can delete pc_system_rom_init.

-Jordan
Anthony Liguori - Dec. 19, 2011, 10:19 p.m.
On 12/19/2011 03:25 PM, Jordan Justen wrote:
> On Mon, Dec 19, 2011 at 11:41, Anthony Liguori<aliguori@us.ibm.com>  wrote:
>> On 12/15/2011 02:51 PM, Jordan Justen wrote:
>>>
>>> If a pflash image is found, then it is used for the system
>>> firmware image.
>>>
>>> If a pflash image is not initially found, then a read-only
>>> pflash device is created using the -bios filename.
>>>
>>> KVM cannot execute from a pflash region currently.
>>> Therefore, when KVM is enabled, a (read-only) ram memory
>>> region is created and filled with the contents of the
>>> pflash drive.
>>>
>>> Signed-off-by: Jordan Justen<jordan.l.justen@intel.com>
>>> Cc: Anthony Liguori<aliguori@us.ibm.com>
>>> ---
>>>   Makefile.target                    |    1 +
>>>   default-configs/i386-softmmu.mak   |    1 +
>>>   default-configs/x86_64-softmmu.mak |    1 +
>>>   hw/boards.h                        |    1 +
>>>   hw/pc.c                            |   55 +-------
>>>   hw/pc.h                            |    4 +
>>>   hw/pc_sysfw.c                      |  255
>>> ++++++++++++++++++++++++++++++++++++
>>>   vl.c                               |    2 +-
>>>   8 files changed, 269 insertions(+), 51 deletions(-)
>>>   create mode 100644 hw/pc_sysfw.c
>>>
>>> diff --git a/Makefile.target b/Makefile.target
>>> index a111521..b1dc882 100644
>>> --- a/Makefile.target
>>> +++ b/Makefile.target
>>> @@ -236,6 +236,7 @@ obj-i386-y += vmport.o
>>>   obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
>>>   obj-i386-y += debugcon.o multiboot.o
>>>   obj-i386-y += pc_piix.o
>>> +obj-i386-y += pc_sysfw.o
>>>   obj-i386-$(CONFIG_KVM) += kvmclock.o
>>>   obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
>>>
>>> diff --git a/default-configs/i386-softmmu.mak
>>> b/default-configs/i386-softmmu.mak
>>> index e67ebb3..cd407a9 100644
>>> --- a/default-configs/i386-softmmu.mak
>>> +++ b/default-configs/i386-softmmu.mak
>>> @@ -22,3 +22,4 @@ CONFIG_SOUND=y
>>>   CONFIG_HPET=y
>>>   CONFIG_APPLESMC=y
>>>   CONFIG_I8259=y
>>> +CONFIG_PFLASH_CFI01=y
>>> diff --git a/default-configs/x86_64-softmmu.mak
>>> b/default-configs/x86_64-softmmu.mak
>>> index b75757e..47734ea 100644
>>> --- a/default-configs/x86_64-softmmu.mak
>>> +++ b/default-configs/x86_64-softmmu.mak
>>> @@ -22,3 +22,4 @@ CONFIG_SOUND=y
>>>   CONFIG_HPET=y
>>>   CONFIG_APPLESMC=y
>>>   CONFIG_I8259=y
>>> +CONFIG_PFLASH_CFI01=y
>>> diff --git a/hw/boards.h b/hw/boards.h
>>> index 716fd7b..45a31a1 100644
>>> --- a/hw/boards.h
>>> +++ b/hw/boards.h
>>> @@ -33,6 +33,7 @@ typedef struct QEMUMachine {
>>>   } QEMUMachine;
>>>
>>>   int qemu_register_machine(QEMUMachine *m);
>>> +QEMUMachine *find_default_machine(void);
>>>
>>>   extern QEMUMachine *current_machine;
>>>
>>> diff --git a/hw/pc.c b/hw/pc.c
>>> index cc6cfad..e5550ca 100644
>>> --- a/hw/pc.c
>>> +++ b/hw/pc.c
>>> @@ -57,10 +57,6 @@
>>>   #define DPRINTF(fmt, ...)
>>>   #endif
>>>
>>> -#define BIOS_FILENAME "bios.bin"
>>> -
>>> -#define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
>>> -
>>>   /* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables.
>>>   */
>>>   #define ACPI_DATA_SIZE       0x10000
>>>   #define BIOS_CFG_IOPORT 0x510
>>> @@ -974,11 +970,9 @@ void pc_memory_init(MemoryRegion *system_memory,
>>>                       MemoryRegion **ram_memory,
>>>                       int system_firmware_enabled)
>>>   {
>>> -    char *filename;
>>> -    int ret, linux_boot, i;
>>> -    MemoryRegion *ram, *bios, *isa_bios, *option_rom_mr;
>>> +    int linux_boot, i;
>>> +    MemoryRegion *ram, *option_rom_mr;
>>>       MemoryRegion *ram_below_4g, *ram_above_4g;
>>> -    int bios_size, isa_bios_size;
>>>       void *fw_cfg;
>>>
>>>       linux_boot = (kernel_filename != NULL);
>>> @@ -1003,43 +997,9 @@ void pc_memory_init(MemoryRegion *system_memory,
>>>                                       ram_above_4g);
>>>       }
>>>
>>> -    /* BIOS load */
>>> -    if (bios_name == NULL)
>>> -        bios_name = BIOS_FILENAME;
>>> -    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>>> -    if (filename) {
>>> -        bios_size = get_image_size(filename);
>>> -    } else {
>>> -        bios_size = -1;
>>> -    }
>>> -    if (bios_size<= 0 ||
>>> -        (bios_size % 65536) != 0) {
>>> -        goto bios_error;
>>> -    }
>>> -    bios = g_malloc(sizeof(*bios));
>>> -    memory_region_init_ram(bios, NULL, "pc.bios", bios_size);
>>> -    memory_region_set_readonly(bios, true);
>>> -    ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1);
>>> -    if (ret != 0) {
>>> -    bios_error:
>>> -        fprintf(stderr, "qemu: could not load PC BIOS '%s'\n",
>>> bios_name);
>>> -        exit(1);
>>> -    }
>>> -    if (filename) {
>>> -        g_free(filename);
>>> -    }
>>> -    /* map the last 128KB of the BIOS in ISA space */
>>> -    isa_bios_size = bios_size;
>>> -    if (isa_bios_size>    (128 * 1024))
>>> -        isa_bios_size = 128 * 1024;
>>> -    isa_bios = g_malloc(sizeof(*isa_bios));
>>> -    memory_region_init_alias(isa_bios, "isa-bios", bios,
>>> -                             bios_size - isa_bios_size, isa_bios_size);
>>> -    memory_region_add_subregion_overlap(rom_memory,
>>> -                                        0x100000 - isa_bios_size,
>>> -                                        isa_bios,
>>> -                                        1);
>>> -    memory_region_set_readonly(isa_bios, true);
>>> +
>>> +    /* Initialize ROM or flash ranges for PC firmware */
>>> +    pc_system_firmware_init(rom_memory, system_firmware_enabled);
>>>
>>>       option_rom_mr = g_malloc(sizeof(*option_rom_mr));
>>>       memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE);
>>> @@ -1048,11 +1008,6 @@ void pc_memory_init(MemoryRegion *system_memory,
>>>                                           option_rom_mr,
>>>                                           1);
>>>
>>> -    /* map all the bios at the top of memory */
>>> -    memory_region_add_subregion(rom_memory,
>>> -                                (uint32_t)(-bios_size),
>>> -                                bios);
>>> -
>>>       fw_cfg = bochs_bios_init();
>>>       rom_set_fw(fw_cfg);
>>>
>>> diff --git a/hw/pc.h b/hw/pc.h
>>> index 49471cb..727e231 100644
>>> --- a/hw/pc.h
>>> +++ b/hw/pc.h
>>> @@ -246,6 +246,10 @@ static inline bool isa_ne2000_init(int base, int irq,
>>> NICInfo *nd)
>>>       return true;
>>>   }
>>>
>>> +/* pc_sysfw.c */
>>> +void pc_system_firmware_init(MemoryRegion *rom_memory,
>>> +                             int system_firmware_enabled);
>>> +
>>>   /* e820 types */
>>>   #define E820_RAM        1
>>>   #define E820_RESERVED   2
>>> diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
>>> new file mode 100644
>>> index 0000000..20027b2
>>> --- /dev/null
>>> +++ b/hw/pc_sysfw.c
>>> @@ -0,0 +1,255 @@
>>> +/*
>>> + * QEMU PC System Firmware
>>> + *
>>> + * Copyright (c) 2003-2004 Fabrice Bellard
>>> + * Copyright (c) 2011 Intel Corporation
>>> + *
>>> + * Permission is hereby granted, free of charge, to any person obtaining
>>> a copy
>>> + * of this software and associated documentation files (the "Software"),
>>> to deal
>>> + * in the Software without restriction, including without limitation the
>>> rights
>>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or
>>> sell
>>> + * copies of the Software, and to permit persons to whom the Software is
>>> + * furnished to do so, subject to the following conditions:
>>> + *
>>> + * The above copyright notice and this permission notice shall be
>>> included in
>>> + * all copies or substantial portions of the Software.
>>> + *
>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>>> EXPRESS OR
>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>>> MERCHANTABILITY,
>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>>> SHALL
>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>>> OTHER
>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>>> ARISING FROM,
>>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>>> IN
>>> + * THE SOFTWARE.
>>> + */
>>> +
>>> +#include "hw.h"
>>> +#include "pc.h"
>>> +#include "hw/boards.h"
>>> +#include "loader.h"
>>> +#include "sysemu.h"
>>> +#include "flash.h"
>>> +#include "kvm.h"
>>> +
>>> +#define BIOS_FILENAME "bios.bin"
>>> +
>>> +static void pc_isa_bios_init(MemoryRegion *rom_memory,
>>> +                             MemoryRegion *flash_mem,
>>> +                             int ram_size)
>>> +{
>>> +    int isa_bios_size;
>>> +    MemoryRegion *isa_bios;
>>> +    uint64_t flash_size;
>>> +    void *flash_ptr, *isa_bios_ptr;
>>> +
>>> +    flash_size = memory_region_size(flash_mem);
>>> +
>>> +    /* map the last 128KB of the BIOS in ISA space */
>>> +    isa_bios_size = flash_size;
>>> +    if (isa_bios_size>    (128 * 1024)) {
>>> +        isa_bios_size = 128 * 1024;
>>> +    }
>>> +    isa_bios = g_malloc(sizeof(*isa_bios));
>>> +    memory_region_init_ram(isa_bios, NULL, "isa-bios", isa_bios_size);
>>> +    memory_region_add_subregion_overlap(rom_memory,
>>> +                                        0x100000 - isa_bios_size,
>>> +                                        isa_bios,
>>> +                                        1);
>>> +
>>> +    /* copy ISA rom image from top of flash memory */
>>> +    flash_ptr = memory_region_get_ram_ptr(flash_mem);
>>> +    isa_bios_ptr = memory_region_get_ram_ptr(isa_bios);
>>> +    memcpy(isa_bios_ptr,
>>> +           ((uint8_t*)flash_ptr) + (flash_size - isa_bios_size),
>>> +           isa_bios_size);
>>> +
>>> +    memory_region_set_readonly(isa_bios, true);
>>> +}
>>> +
>>> +static void pc_fw_add_pflash_drv(void)
>>> +{
>>> +    QemuOpts *opts;
>>> +    QEMUMachine *machine;
>>> +    char *filename;
>>> +
>>> +    if (bios_name == NULL) {
>>> +        bios_name = BIOS_FILENAME;
>>> +    }
>>> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>>> +
>>> +    opts = drive_add(IF_PFLASH, -1, filename, "readonly=on");
>>> +    if (opts == NULL) {
>>> +      return;
>>> +    }
>>> +
>>> +    machine = find_default_machine();
>>> +    if (machine == NULL) {
>>> +      return;
>>> +    }
>>> +
>>> +    drive_init(opts, machine->use_scsi);
>>> +}
>>> +
>>> +static void pc_system_flash_init(MemoryRegion *rom_memory,
>>> +                                 DriveInfo *pflash_drv)
>>> +{
>>> +    BlockDriverState *bdrv;
>>> +    int64_t size;
>>> +    target_phys_addr_t phys_addr;
>>> +    int sector_bits, sector_size;
>>> +    pflash_t *system_flash;
>>> +    MemoryRegion *flash_mem;
>>> +
>>> +    bdrv = pflash_drv->bdrv;
>>> +    size = bdrv_getlength(pflash_drv->bdrv);
>>> +    sector_bits = 12;
>>> +    sector_size = 1<<    sector_bits;
>>> +
>>> +    if ((size % sector_size) != 0) {
>>> +        fprintf(stderr,
>>> +                "qemu: PC system firmware (pflash) must be a multiple of
>>> 0x%x\n",
>>> +                sector_size);
>>> +        exit(1);
>>> +    }
>>> +
>>> +    phys_addr = 0x100000000ULL - size;
>>> +    system_flash = pflash_cfi01_register(phys_addr, NULL, "system.flash",
>>> size,
>>> +                                         bdrv, sector_size, size>>
>>>   sector_bits,
>>> +                                         1, 0x0000, 0x0000, 0x0000,
>>> 0x0000, 0);
>>> +    flash_mem = pflash_cfi01_get_memory(system_flash);
>>> +
>>> +    pc_isa_bios_init(rom_memory, flash_mem, size);
>>> +}
>>> +
>>> +static void pc_system_rom_init(MemoryRegion *rom_memory,
>>> +                               DriveInfo *pflash_drv)
>>> +{
>>> +    BlockDriverState *bdrv;
>>> +    int64_t size;
>>> +    target_phys_addr_t phys_addr;
>>> +    int sector_bits, sector_size;
>>> +    MemoryRegion *sys_rom;
>>> +    void *buffer;
>>> +    int ret;
>>> +
>>> +    bdrv = pflash_drv->bdrv;
>>> +    size = bdrv_getlength(pflash_drv->bdrv);
>>> +    sector_bits = 9;
>>> +    sector_size = 1<<    sector_bits;
>>> +
>>> +    if ((size % sector_size) != 0) {
>>> +        fprintf(stderr,
>>> +                "qemu: PC system rom (pflash) must be a multiple of
>>> 0x%x\n",
>>> +                sector_size);
>>> +        exit(1);
>>> +    }
>>> +
>>> +    phys_addr = 0x100000000ULL - size;
>>> +    sys_rom = g_malloc(sizeof(*sys_rom));
>>> +    memory_region_init_ram(sys_rom, NULL, "system.rom", size);
>>> +    buffer = memory_region_get_ram_ptr(sys_rom);
>>> +    memory_region_add_subregion(rom_memory, phys_addr, sys_rom);
>>> +
>>> +    /* read the rom content */
>>> +    ret = bdrv_read(bdrv, 0, buffer, size>>    sector_bits);
>>
>>
>> I think we're trying to get rid of synchronous block I/O in machine
>> initialization for a number of reasons.
>>
>> Kevin/Stefan, care to comment?  Will this be problematic in the future?
>
> I was hoping pc-1.1 with and without kvm could be as close as
> possible, but I guess I can make pc-1.1 with kvm behave the same as
> pc-1.0.  Then I can delete pc_system_rom_init.

I think your general approach is right, I'm just not sure what we're going to do 
short term about synchronous I/O in the machine init routines.  It may just be a 
matter of structuring this in such a way that you can use an async interface.

Regards,

Anthony Liguori

>
> -Jordan
>
Stefan Hajnoczi - Dec. 20, 2011, 8:11 a.m.
On Mon, Dec 19, 2011 at 01:41:03PM -0600, Anthony Liguori wrote:
> On 12/15/2011 02:51 PM, Jordan Justen wrote:
> >If a pflash image is found, then it is used for the system
> >firmware image.
> >
> >If a pflash image is not initially found, then a read-only
> >pflash device is created using the -bios filename.
> >
> >KVM cannot execute from a pflash region currently.
> >Therefore, when KVM is enabled, a (read-only) ram memory
> >region is created and filled with the contents of the
> >pflash drive.
> >
> >Signed-off-by: Jordan Justen<jordan.l.justen@intel.com>
> >Cc: Anthony Liguori<aliguori@us.ibm.com>
> >---
> >  Makefile.target                    |    1 +
> >  default-configs/i386-softmmu.mak   |    1 +
> >  default-configs/x86_64-softmmu.mak |    1 +
> >  hw/boards.h                        |    1 +
> >  hw/pc.c                            |   55 +-------
> >  hw/pc.h                            |    4 +
> >  hw/pc_sysfw.c                      |  255 ++++++++++++++++++++++++++++++++++++
> >  vl.c                               |    2 +-
> >  8 files changed, 269 insertions(+), 51 deletions(-)
> >  create mode 100644 hw/pc_sysfw.c
> >
> >diff --git a/Makefile.target b/Makefile.target
> >index a111521..b1dc882 100644
> >--- a/Makefile.target
> >+++ b/Makefile.target
> >@@ -236,6 +236,7 @@ obj-i386-y += vmport.o
> >  obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
> >  obj-i386-y += debugcon.o multiboot.o
> >  obj-i386-y += pc_piix.o
> >+obj-i386-y += pc_sysfw.o
> >  obj-i386-$(CONFIG_KVM) += kvmclock.o
> >  obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
> >
> >diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> >index e67ebb3..cd407a9 100644
> >--- a/default-configs/i386-softmmu.mak
> >+++ b/default-configs/i386-softmmu.mak
> >@@ -22,3 +22,4 @@ CONFIG_SOUND=y
> >  CONFIG_HPET=y
> >  CONFIG_APPLESMC=y
> >  CONFIG_I8259=y
> >+CONFIG_PFLASH_CFI01=y
> >diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
> >index b75757e..47734ea 100644
> >--- a/default-configs/x86_64-softmmu.mak
> >+++ b/default-configs/x86_64-softmmu.mak
> >@@ -22,3 +22,4 @@ CONFIG_SOUND=y
> >  CONFIG_HPET=y
> >  CONFIG_APPLESMC=y
> >  CONFIG_I8259=y
> >+CONFIG_PFLASH_CFI01=y
> >diff --git a/hw/boards.h b/hw/boards.h
> >index 716fd7b..45a31a1 100644
> >--- a/hw/boards.h
> >+++ b/hw/boards.h
> >@@ -33,6 +33,7 @@ typedef struct QEMUMachine {
> >  } QEMUMachine;
> >
> >  int qemu_register_machine(QEMUMachine *m);
> >+QEMUMachine *find_default_machine(void);
> >
> >  extern QEMUMachine *current_machine;
> >
> >diff --git a/hw/pc.c b/hw/pc.c
> >index cc6cfad..e5550ca 100644
> >--- a/hw/pc.c
> >+++ b/hw/pc.c
> >@@ -57,10 +57,6 @@
> >  #define DPRINTF(fmt, ...)
> >  #endif
> >
> >-#define BIOS_FILENAME "bios.bin"
> >-
> >-#define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
> >-
> >  /* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables.  */
> >  #define ACPI_DATA_SIZE       0x10000
> >  #define BIOS_CFG_IOPORT 0x510
> >@@ -974,11 +970,9 @@ void pc_memory_init(MemoryRegion *system_memory,
> >                      MemoryRegion **ram_memory,
> >                      int system_firmware_enabled)
> >  {
> >-    char *filename;
> >-    int ret, linux_boot, i;
> >-    MemoryRegion *ram, *bios, *isa_bios, *option_rom_mr;
> >+    int linux_boot, i;
> >+    MemoryRegion *ram, *option_rom_mr;
> >      MemoryRegion *ram_below_4g, *ram_above_4g;
> >-    int bios_size, isa_bios_size;
> >      void *fw_cfg;
> >
> >      linux_boot = (kernel_filename != NULL);
> >@@ -1003,43 +997,9 @@ void pc_memory_init(MemoryRegion *system_memory,
> >                                      ram_above_4g);
> >      }
> >
> >-    /* BIOS load */
> >-    if (bios_name == NULL)
> >-        bios_name = BIOS_FILENAME;
> >-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> >-    if (filename) {
> >-        bios_size = get_image_size(filename);
> >-    } else {
> >-        bios_size = -1;
> >-    }
> >-    if (bios_size<= 0 ||
> >-        (bios_size % 65536) != 0) {
> >-        goto bios_error;
> >-    }
> >-    bios = g_malloc(sizeof(*bios));
> >-    memory_region_init_ram(bios, NULL, "pc.bios", bios_size);
> >-    memory_region_set_readonly(bios, true);
> >-    ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1);
> >-    if (ret != 0) {
> >-    bios_error:
> >-        fprintf(stderr, "qemu: could not load PC BIOS '%s'\n", bios_name);
> >-        exit(1);
> >-    }
> >-    if (filename) {
> >-        g_free(filename);
> >-    }
> >-    /* map the last 128KB of the BIOS in ISA space */
> >-    isa_bios_size = bios_size;
> >-    if (isa_bios_size>  (128 * 1024))
> >-        isa_bios_size = 128 * 1024;
> >-    isa_bios = g_malloc(sizeof(*isa_bios));
> >-    memory_region_init_alias(isa_bios, "isa-bios", bios,
> >-                             bios_size - isa_bios_size, isa_bios_size);
> >-    memory_region_add_subregion_overlap(rom_memory,
> >-                                        0x100000 - isa_bios_size,
> >-                                        isa_bios,
> >-                                        1);
> >-    memory_region_set_readonly(isa_bios, true);
> >+
> >+    /* Initialize ROM or flash ranges for PC firmware */
> >+    pc_system_firmware_init(rom_memory, system_firmware_enabled);
> >
> >      option_rom_mr = g_malloc(sizeof(*option_rom_mr));
> >      memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE);
> >@@ -1048,11 +1008,6 @@ void pc_memory_init(MemoryRegion *system_memory,
> >                                          option_rom_mr,
> >                                          1);
> >
> >-    /* map all the bios at the top of memory */
> >-    memory_region_add_subregion(rom_memory,
> >-                                (uint32_t)(-bios_size),
> >-                                bios);
> >-
> >      fw_cfg = bochs_bios_init();
> >      rom_set_fw(fw_cfg);
> >
> >diff --git a/hw/pc.h b/hw/pc.h
> >index 49471cb..727e231 100644
> >--- a/hw/pc.h
> >+++ b/hw/pc.h
> >@@ -246,6 +246,10 @@ static inline bool isa_ne2000_init(int base, int irq, NICInfo *nd)
> >      return true;
> >  }
> >
> >+/* pc_sysfw.c */
> >+void pc_system_firmware_init(MemoryRegion *rom_memory,
> >+                             int system_firmware_enabled);
> >+
> >  /* e820 types */
> >  #define E820_RAM        1
> >  #define E820_RESERVED   2
> >diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
> >new file mode 100644
> >index 0000000..20027b2
> >--- /dev/null
> >+++ b/hw/pc_sysfw.c
> >@@ -0,0 +1,255 @@
> >+/*
> >+ * QEMU PC System Firmware
> >+ *
> >+ * Copyright (c) 2003-2004 Fabrice Bellard
> >+ * Copyright (c) 2011 Intel Corporation
> >+ *
> >+ * Permission is hereby granted, free of charge, to any person obtaining a copy
> >+ * of this software and associated documentation files (the "Software"), to deal
> >+ * in the Software without restriction, including without limitation the rights
> >+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> >+ * copies of the Software, and to permit persons to whom the Software is
> >+ * furnished to do so, subject to the following conditions:
> >+ *
> >+ * The above copyright notice and this permission notice shall be included in
> >+ * all copies or substantial portions of the Software.
> >+ *
> >+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> >+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> >+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> >+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> >+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> >+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> >+ * THE SOFTWARE.
> >+ */
> >+
> >+#include "hw.h"
> >+#include "pc.h"
> >+#include "hw/boards.h"
> >+#include "loader.h"
> >+#include "sysemu.h"
> >+#include "flash.h"
> >+#include "kvm.h"
> >+
> >+#define BIOS_FILENAME "bios.bin"
> >+
> >+static void pc_isa_bios_init(MemoryRegion *rom_memory,
> >+                             MemoryRegion *flash_mem,
> >+                             int ram_size)
> >+{
> >+    int isa_bios_size;
> >+    MemoryRegion *isa_bios;
> >+    uint64_t flash_size;
> >+    void *flash_ptr, *isa_bios_ptr;
> >+
> >+    flash_size = memory_region_size(flash_mem);
> >+
> >+    /* map the last 128KB of the BIOS in ISA space */
> >+    isa_bios_size = flash_size;
> >+    if (isa_bios_size>  (128 * 1024)) {
> >+        isa_bios_size = 128 * 1024;
> >+    }
> >+    isa_bios = g_malloc(sizeof(*isa_bios));
> >+    memory_region_init_ram(isa_bios, NULL, "isa-bios", isa_bios_size);
> >+    memory_region_add_subregion_overlap(rom_memory,
> >+                                        0x100000 - isa_bios_size,
> >+                                        isa_bios,
> >+                                        1);
> >+
> >+    /* copy ISA rom image from top of flash memory */
> >+    flash_ptr = memory_region_get_ram_ptr(flash_mem);
> >+    isa_bios_ptr = memory_region_get_ram_ptr(isa_bios);
> >+    memcpy(isa_bios_ptr,
> >+           ((uint8_t*)flash_ptr) + (flash_size - isa_bios_size),
> >+           isa_bios_size);
> >+
> >+    memory_region_set_readonly(isa_bios, true);
> >+}
> >+
> >+static void pc_fw_add_pflash_drv(void)
> >+{
> >+    QemuOpts *opts;
> >+    QEMUMachine *machine;
> >+    char *filename;
> >+
> >+    if (bios_name == NULL) {
> >+        bios_name = BIOS_FILENAME;
> >+    }
> >+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> >+
> >+    opts = drive_add(IF_PFLASH, -1, filename, "readonly=on");
> >+    if (opts == NULL) {
> >+      return;
> >+    }
> >+
> >+    machine = find_default_machine();
> >+    if (machine == NULL) {
> >+      return;
> >+    }
> >+
> >+    drive_init(opts, machine->use_scsi);
> >+}
> >+
> >+static void pc_system_flash_init(MemoryRegion *rom_memory,
> >+                                 DriveInfo *pflash_drv)
> >+{
> >+    BlockDriverState *bdrv;
> >+    int64_t size;
> >+    target_phys_addr_t phys_addr;
> >+    int sector_bits, sector_size;
> >+    pflash_t *system_flash;
> >+    MemoryRegion *flash_mem;
> >+
> >+    bdrv = pflash_drv->bdrv;
> >+    size = bdrv_getlength(pflash_drv->bdrv);
> >+    sector_bits = 12;
> >+    sector_size = 1<<  sector_bits;
> >+
> >+    if ((size % sector_size) != 0) {
> >+        fprintf(stderr,
> >+                "qemu: PC system firmware (pflash) must be a multiple of 0x%x\n",
> >+                sector_size);
> >+        exit(1);
> >+    }
> >+
> >+    phys_addr = 0x100000000ULL - size;
> >+    system_flash = pflash_cfi01_register(phys_addr, NULL, "system.flash", size,
> >+                                         bdrv, sector_size, size>>  sector_bits,
> >+                                         1, 0x0000, 0x0000, 0x0000, 0x0000, 0);
> >+    flash_mem = pflash_cfi01_get_memory(system_flash);
> >+
> >+    pc_isa_bios_init(rom_memory, flash_mem, size);
> >+}
> >+
> >+static void pc_system_rom_init(MemoryRegion *rom_memory,
> >+                               DriveInfo *pflash_drv)
> >+{
> >+    BlockDriverState *bdrv;
> >+    int64_t size;
> >+    target_phys_addr_t phys_addr;
> >+    int sector_bits, sector_size;
> >+    MemoryRegion *sys_rom;
> >+    void *buffer;
> >+    int ret;
> >+
> >+    bdrv = pflash_drv->bdrv;
> >+    size = bdrv_getlength(pflash_drv->bdrv);
> >+    sector_bits = 9;
> >+    sector_size = 1<<  sector_bits;
> >+
> >+    if ((size % sector_size) != 0) {
> >+        fprintf(stderr,
> >+                "qemu: PC system rom (pflash) must be a multiple of 0x%x\n",
> >+                sector_size);
> >+        exit(1);
> >+    }
> >+
> >+    phys_addr = 0x100000000ULL - size;
> >+    sys_rom = g_malloc(sizeof(*sys_rom));
> >+    memory_region_init_ram(sys_rom, NULL, "system.rom", size);
> >+    buffer = memory_region_get_ram_ptr(sys_rom);
> >+    memory_region_add_subregion(rom_memory, phys_addr, sys_rom);
> >+
> >+    /* read the rom content */
> >+    ret = bdrv_read(bdrv, 0, buffer, size>>  sector_bits);
> 
> I think we're trying to get rid of synchronous block I/O in machine
> initialization for a number of reasons.
> 
> Kevin/Stefan, care to comment?  Will this be problematic in the future?

This looks okay to me because it is not called from vcpu context.  We
need to avoid device emulation that uses synchronous I/O for pio/mmio
because that blocks the guest from executing code.  In this case the
code runs before the VM starts and should be easy to update if we ever
drop the bdrv_read() interface.

Stefan
Kevin Wolf - Jan. 9, 2012, 9:28 a.m.
Am 19.12.2011 23:19, schrieb Anthony Liguori:
> On 12/19/2011 03:25 PM, Jordan Justen wrote:
>> On Mon, Dec 19, 2011 at 11:41, Anthony Liguori<aliguori@us.ibm.com>  wrote:
>>> On 12/15/2011 02:51 PM, Jordan Justen wrote:
>>>>
>>>> If a pflash image is found, then it is used for the system
>>>> firmware image.
>>>>
>>>> If a pflash image is not initially found, then a read-only
>>>> pflash device is created using the -bios filename.
>>>>
>>>> KVM cannot execute from a pflash region currently.
>>>> Therefore, when KVM is enabled, a (read-only) ram memory
>>>> region is created and filled with the contents of the
>>>> pflash drive.
>>>>
>>>> Signed-off-by: Jordan Justen<jordan.l.justen@intel.com>
>>>> Cc: Anthony Liguori<aliguori@us.ibm.com>
>>>> ---
>>>>   Makefile.target                    |    1 +
>>>>   default-configs/i386-softmmu.mak   |    1 +
>>>>   default-configs/x86_64-softmmu.mak |    1 +
>>>>   hw/boards.h                        |    1 +
>>>>   hw/pc.c                            |   55 +-------
>>>>   hw/pc.h                            |    4 +
>>>>   hw/pc_sysfw.c                      |  255
>>>> ++++++++++++++++++++++++++++++++++++
>>>>   vl.c                               |    2 +-
>>>>   8 files changed, 269 insertions(+), 51 deletions(-)
>>>>   create mode 100644 hw/pc_sysfw.c
>>>>
>>>> diff --git a/Makefile.target b/Makefile.target
>>>> index a111521..b1dc882 100644
>>>> --- a/Makefile.target
>>>> +++ b/Makefile.target
>>>> @@ -236,6 +236,7 @@ obj-i386-y += vmport.o
>>>>   obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
>>>>   obj-i386-y += debugcon.o multiboot.o
>>>>   obj-i386-y += pc_piix.o
>>>> +obj-i386-y += pc_sysfw.o
>>>>   obj-i386-$(CONFIG_KVM) += kvmclock.o
>>>>   obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
>>>>
>>>> diff --git a/default-configs/i386-softmmu.mak
>>>> b/default-configs/i386-softmmu.mak
>>>> index e67ebb3..cd407a9 100644
>>>> --- a/default-configs/i386-softmmu.mak
>>>> +++ b/default-configs/i386-softmmu.mak
>>>> @@ -22,3 +22,4 @@ CONFIG_SOUND=y
>>>>   CONFIG_HPET=y
>>>>   CONFIG_APPLESMC=y
>>>>   CONFIG_I8259=y
>>>> +CONFIG_PFLASH_CFI01=y
>>>> diff --git a/default-configs/x86_64-softmmu.mak
>>>> b/default-configs/x86_64-softmmu.mak
>>>> index b75757e..47734ea 100644
>>>> --- a/default-configs/x86_64-softmmu.mak
>>>> +++ b/default-configs/x86_64-softmmu.mak
>>>> @@ -22,3 +22,4 @@ CONFIG_SOUND=y
>>>>   CONFIG_HPET=y
>>>>   CONFIG_APPLESMC=y
>>>>   CONFIG_I8259=y
>>>> +CONFIG_PFLASH_CFI01=y
>>>> diff --git a/hw/boards.h b/hw/boards.h
>>>> index 716fd7b..45a31a1 100644
>>>> --- a/hw/boards.h
>>>> +++ b/hw/boards.h
>>>> @@ -33,6 +33,7 @@ typedef struct QEMUMachine {
>>>>   } QEMUMachine;
>>>>
>>>>   int qemu_register_machine(QEMUMachine *m);
>>>> +QEMUMachine *find_default_machine(void);
>>>>
>>>>   extern QEMUMachine *current_machine;
>>>>
>>>> diff --git a/hw/pc.c b/hw/pc.c
>>>> index cc6cfad..e5550ca 100644
>>>> --- a/hw/pc.c
>>>> +++ b/hw/pc.c
>>>> @@ -57,10 +57,6 @@
>>>>   #define DPRINTF(fmt, ...)
>>>>   #endif
>>>>
>>>> -#define BIOS_FILENAME "bios.bin"
>>>> -
>>>> -#define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
>>>> -
>>>>   /* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables.
>>>>   */
>>>>   #define ACPI_DATA_SIZE       0x10000
>>>>   #define BIOS_CFG_IOPORT 0x510
>>>> @@ -974,11 +970,9 @@ void pc_memory_init(MemoryRegion *system_memory,
>>>>                       MemoryRegion **ram_memory,
>>>>                       int system_firmware_enabled)
>>>>   {
>>>> -    char *filename;
>>>> -    int ret, linux_boot, i;
>>>> -    MemoryRegion *ram, *bios, *isa_bios, *option_rom_mr;
>>>> +    int linux_boot, i;
>>>> +    MemoryRegion *ram, *option_rom_mr;
>>>>       MemoryRegion *ram_below_4g, *ram_above_4g;
>>>> -    int bios_size, isa_bios_size;
>>>>       void *fw_cfg;
>>>>
>>>>       linux_boot = (kernel_filename != NULL);
>>>> @@ -1003,43 +997,9 @@ void pc_memory_init(MemoryRegion *system_memory,
>>>>                                       ram_above_4g);
>>>>       }
>>>>
>>>> -    /* BIOS load */
>>>> -    if (bios_name == NULL)
>>>> -        bios_name = BIOS_FILENAME;
>>>> -    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>>>> -    if (filename) {
>>>> -        bios_size = get_image_size(filename);
>>>> -    } else {
>>>> -        bios_size = -1;
>>>> -    }
>>>> -    if (bios_size<= 0 ||
>>>> -        (bios_size % 65536) != 0) {
>>>> -        goto bios_error;
>>>> -    }
>>>> -    bios = g_malloc(sizeof(*bios));
>>>> -    memory_region_init_ram(bios, NULL, "pc.bios", bios_size);
>>>> -    memory_region_set_readonly(bios, true);
>>>> -    ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1);
>>>> -    if (ret != 0) {
>>>> -    bios_error:
>>>> -        fprintf(stderr, "qemu: could not load PC BIOS '%s'\n",
>>>> bios_name);
>>>> -        exit(1);
>>>> -    }
>>>> -    if (filename) {
>>>> -        g_free(filename);
>>>> -    }
>>>> -    /* map the last 128KB of the BIOS in ISA space */
>>>> -    isa_bios_size = bios_size;
>>>> -    if (isa_bios_size>    (128 * 1024))
>>>> -        isa_bios_size = 128 * 1024;
>>>> -    isa_bios = g_malloc(sizeof(*isa_bios));
>>>> -    memory_region_init_alias(isa_bios, "isa-bios", bios,
>>>> -                             bios_size - isa_bios_size, isa_bios_size);
>>>> -    memory_region_add_subregion_overlap(rom_memory,
>>>> -                                        0x100000 - isa_bios_size,
>>>> -                                        isa_bios,
>>>> -                                        1);
>>>> -    memory_region_set_readonly(isa_bios, true);
>>>> +
>>>> +    /* Initialize ROM or flash ranges for PC firmware */
>>>> +    pc_system_firmware_init(rom_memory, system_firmware_enabled);
>>>>
>>>>       option_rom_mr = g_malloc(sizeof(*option_rom_mr));
>>>>       memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE);
>>>> @@ -1048,11 +1008,6 @@ void pc_memory_init(MemoryRegion *system_memory,
>>>>                                           option_rom_mr,
>>>>                                           1);
>>>>
>>>> -    /* map all the bios at the top of memory */
>>>> -    memory_region_add_subregion(rom_memory,
>>>> -                                (uint32_t)(-bios_size),
>>>> -                                bios);
>>>> -
>>>>       fw_cfg = bochs_bios_init();
>>>>       rom_set_fw(fw_cfg);
>>>>
>>>> diff --git a/hw/pc.h b/hw/pc.h
>>>> index 49471cb..727e231 100644
>>>> --- a/hw/pc.h
>>>> +++ b/hw/pc.h
>>>> @@ -246,6 +246,10 @@ static inline bool isa_ne2000_init(int base, int irq,
>>>> NICInfo *nd)
>>>>       return true;
>>>>   }
>>>>
>>>> +/* pc_sysfw.c */
>>>> +void pc_system_firmware_init(MemoryRegion *rom_memory,
>>>> +                             int system_firmware_enabled);
>>>> +
>>>>   /* e820 types */
>>>>   #define E820_RAM        1
>>>>   #define E820_RESERVED   2
>>>> diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
>>>> new file mode 100644
>>>> index 0000000..20027b2
>>>> --- /dev/null
>>>> +++ b/hw/pc_sysfw.c
>>>> @@ -0,0 +1,255 @@
>>>> +/*
>>>> + * QEMU PC System Firmware
>>>> + *
>>>> + * Copyright (c) 2003-2004 Fabrice Bellard
>>>> + * Copyright (c) 2011 Intel Corporation
>>>> + *
>>>> + * Permission is hereby granted, free of charge, to any person obtaining
>>>> a copy
>>>> + * of this software and associated documentation files (the "Software"),
>>>> to deal
>>>> + * in the Software without restriction, including without limitation the
>>>> rights
>>>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or
>>>> sell
>>>> + * copies of the Software, and to permit persons to whom the Software is
>>>> + * furnished to do so, subject to the following conditions:
>>>> + *
>>>> + * The above copyright notice and this permission notice shall be
>>>> included in
>>>> + * all copies or substantial portions of the Software.
>>>> + *
>>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>>>> EXPRESS OR
>>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>>>> MERCHANTABILITY,
>>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>>>> SHALL
>>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>>>> OTHER
>>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>>>> ARISING FROM,
>>>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>>>> IN
>>>> + * THE SOFTWARE.
>>>> + */
>>>> +
>>>> +#include "hw.h"
>>>> +#include "pc.h"
>>>> +#include "hw/boards.h"
>>>> +#include "loader.h"
>>>> +#include "sysemu.h"
>>>> +#include "flash.h"
>>>> +#include "kvm.h"
>>>> +
>>>> +#define BIOS_FILENAME "bios.bin"
>>>> +
>>>> +static void pc_isa_bios_init(MemoryRegion *rom_memory,
>>>> +                             MemoryRegion *flash_mem,
>>>> +                             int ram_size)
>>>> +{
>>>> +    int isa_bios_size;
>>>> +    MemoryRegion *isa_bios;
>>>> +    uint64_t flash_size;
>>>> +    void *flash_ptr, *isa_bios_ptr;
>>>> +
>>>> +    flash_size = memory_region_size(flash_mem);
>>>> +
>>>> +    /* map the last 128KB of the BIOS in ISA space */
>>>> +    isa_bios_size = flash_size;
>>>> +    if (isa_bios_size>    (128 * 1024)) {
>>>> +        isa_bios_size = 128 * 1024;
>>>> +    }
>>>> +    isa_bios = g_malloc(sizeof(*isa_bios));
>>>> +    memory_region_init_ram(isa_bios, NULL, "isa-bios", isa_bios_size);
>>>> +    memory_region_add_subregion_overlap(rom_memory,
>>>> +                                        0x100000 - isa_bios_size,
>>>> +                                        isa_bios,
>>>> +                                        1);
>>>> +
>>>> +    /* copy ISA rom image from top of flash memory */
>>>> +    flash_ptr = memory_region_get_ram_ptr(flash_mem);
>>>> +    isa_bios_ptr = memory_region_get_ram_ptr(isa_bios);
>>>> +    memcpy(isa_bios_ptr,
>>>> +           ((uint8_t*)flash_ptr) + (flash_size - isa_bios_size),
>>>> +           isa_bios_size);
>>>> +
>>>> +    memory_region_set_readonly(isa_bios, true);
>>>> +}
>>>> +
>>>> +static void pc_fw_add_pflash_drv(void)
>>>> +{
>>>> +    QemuOpts *opts;
>>>> +    QEMUMachine *machine;
>>>> +    char *filename;
>>>> +
>>>> +    if (bios_name == NULL) {
>>>> +        bios_name = BIOS_FILENAME;
>>>> +    }
>>>> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>>>> +
>>>> +    opts = drive_add(IF_PFLASH, -1, filename, "readonly=on");
>>>> +    if (opts == NULL) {
>>>> +      return;
>>>> +    }
>>>> +
>>>> +    machine = find_default_machine();
>>>> +    if (machine == NULL) {
>>>> +      return;
>>>> +    }
>>>> +
>>>> +    drive_init(opts, machine->use_scsi);
>>>> +}
>>>> +
>>>> +static void pc_system_flash_init(MemoryRegion *rom_memory,
>>>> +                                 DriveInfo *pflash_drv)
>>>> +{
>>>> +    BlockDriverState *bdrv;
>>>> +    int64_t size;
>>>> +    target_phys_addr_t phys_addr;
>>>> +    int sector_bits, sector_size;
>>>> +    pflash_t *system_flash;
>>>> +    MemoryRegion *flash_mem;
>>>> +
>>>> +    bdrv = pflash_drv->bdrv;
>>>> +    size = bdrv_getlength(pflash_drv->bdrv);
>>>> +    sector_bits = 12;
>>>> +    sector_size = 1<<    sector_bits;
>>>> +
>>>> +    if ((size % sector_size) != 0) {
>>>> +        fprintf(stderr,
>>>> +                "qemu: PC system firmware (pflash) must be a multiple of
>>>> 0x%x\n",
>>>> +                sector_size);
>>>> +        exit(1);
>>>> +    }
>>>> +
>>>> +    phys_addr = 0x100000000ULL - size;
>>>> +    system_flash = pflash_cfi01_register(phys_addr, NULL, "system.flash",
>>>> size,
>>>> +                                         bdrv, sector_size, size>>
>>>>   sector_bits,
>>>> +                                         1, 0x0000, 0x0000, 0x0000,
>>>> 0x0000, 0);
>>>> +    flash_mem = pflash_cfi01_get_memory(system_flash);
>>>> +
>>>> +    pc_isa_bios_init(rom_memory, flash_mem, size);
>>>> +}
>>>> +
>>>> +static void pc_system_rom_init(MemoryRegion *rom_memory,
>>>> +                               DriveInfo *pflash_drv)
>>>> +{
>>>> +    BlockDriverState *bdrv;
>>>> +    int64_t size;
>>>> +    target_phys_addr_t phys_addr;
>>>> +    int sector_bits, sector_size;
>>>> +    MemoryRegion *sys_rom;
>>>> +    void *buffer;
>>>> +    int ret;
>>>> +
>>>> +    bdrv = pflash_drv->bdrv;
>>>> +    size = bdrv_getlength(pflash_drv->bdrv);
>>>> +    sector_bits = 9;
>>>> +    sector_size = 1<<    sector_bits;
>>>> +
>>>> +    if ((size % sector_size) != 0) {
>>>> +        fprintf(stderr,
>>>> +                "qemu: PC system rom (pflash) must be a multiple of
>>>> 0x%x\n",
>>>> +                sector_size);
>>>> +        exit(1);
>>>> +    }
>>>> +
>>>> +    phys_addr = 0x100000000ULL - size;
>>>> +    sys_rom = g_malloc(sizeof(*sys_rom));
>>>> +    memory_region_init_ram(sys_rom, NULL, "system.rom", size);
>>>> +    buffer = memory_region_get_ram_ptr(sys_rom);
>>>> +    memory_region_add_subregion(rom_memory, phys_addr, sys_rom);
>>>> +
>>>> +    /* read the rom content */
>>>> +    ret = bdrv_read(bdrv, 0, buffer, size>>    sector_bits);
>>>
>>>
>>> I think we're trying to get rid of synchronous block I/O in machine
>>> initialization for a number of reasons.
>>>
>>> Kevin/Stefan, care to comment?  Will this be problematic in the future?
>>
>> I was hoping pc-1.1 with and without kvm could be as close as
>> possible, but I guess I can make pc-1.1 with kvm behave the same as
>> pc-1.0.  Then I can delete pc_system_rom_init.
> 
> I think your general approach is right, I'm just not sure what we're going to do 
> short term about synchronous I/O in the machine init routines.  It may just be a 
> matter of structuring this in such a way that you can use an async interface.

I don't think there is a problem with using the synchronous interface
here. But I'm not sure if having any I/O in the machine init is a good
idea with respect to migration. Didn't we already have to move some code
there to a later stage to make sure that the destination doesn't use
outdated data?

Kevin
Jordan Justen - Jan. 28, 2012, 11:13 p.m.
On Mon, Jan 9, 2012 at 01:28, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 19.12.2011 23:19, schrieb Anthony Liguori:
>> On 12/19/2011 03:25 PM, Jordan Justen wrote:
>>> On Mon, Dec 19, 2011 at 11:41, Anthony Liguori<aliguori@us.ibm.com>  wrote:
>>>> On 12/15/2011 02:51 PM, Jordan Justen wrote:
>>>>> +static void pc_system_rom_init(MemoryRegion *rom_memory,
>>>>> +                               DriveInfo *pflash_drv)
>>>>> +{
>>>>> +    BlockDriverState *bdrv;
>>>>> +    int64_t size;
>>>>> +    target_phys_addr_t phys_addr;
>>>>> +    int sector_bits, sector_size;
>>>>> +    MemoryRegion *sys_rom;
>>>>> +    void *buffer;
>>>>> +    int ret;
>>>>> +
>>>>> +    bdrv = pflash_drv->bdrv;
>>>>> +    size = bdrv_getlength(pflash_drv->bdrv);
>>>>> +    sector_bits = 9;
>>>>> +    sector_size = 1<<    sector_bits;
>>>>> +
>>>>> +    if ((size % sector_size) != 0) {
>>>>> +        fprintf(stderr,
>>>>> +                "qemu: PC system rom (pflash) must be a multiple of
>>>>> 0x%x\n",
>>>>> +                sector_size);
>>>>> +        exit(1);
>>>>> +    }
>>>>> +
>>>>> +    phys_addr = 0x100000000ULL - size;
>>>>> +    sys_rom = g_malloc(sizeof(*sys_rom));
>>>>> +    memory_region_init_ram(sys_rom, NULL, "system.rom", size);
>>>>> +    buffer = memory_region_get_ram_ptr(sys_rom);
>>>>> +    memory_region_add_subregion(rom_memory, phys_addr, sys_rom);
>>>>> +
>>>>> +    /* read the rom content */
>>>>> +    ret = bdrv_read(bdrv, 0, buffer, size>>    sector_bits);
>>>>
>>>>
>>>> I think we're trying to get rid of synchronous block I/O in machine
>>>> initialization for a number of reasons.
>>>>
>>>> Kevin/Stefan, care to comment?  Will this be problematic in the future?
>>>
>>> I was hoping pc-1.1 with and without kvm could be as close as
>>> possible, but I guess I can make pc-1.1 with kvm behave the same as
>>> pc-1.0.  Then I can delete pc_system_rom_init.
>>
>> I think your general approach is right, I'm just not sure what we're going to do
>> short term about synchronous I/O in the machine init routines.  It may just be a
>> matter of structuring this in such a way that you can use an async interface.
>
> I don't think there is a problem with using the synchronous interface
> here. But I'm not sure if having any I/O in the machine init is a good
> idea with respect to migration. Didn't we already have to move some code
> there to a later stage to make sure that the destination doesn't use
> outdated data?

The pc_system_rom_init function is only used when kvm is enabled.
Since kvm cannot currently execute code from device memory, we have to
create a ram region, and transfer the pflash contents into the ram
region.

Instead of bdrv_read, should I get the pflash memory region buffer,
and use memcpy?  Or, would still suffer from the same issue of getting
outdated data?

Thanks,

-Jordan
Jordan Justen - Feb. 20, 2012, 12:39 a.m.
On Mon, Jan 9, 2012 at 01:28, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 19.12.2011 23:19, schrieb Anthony Liguori:
>> On 12/19/2011 03:25 PM, Jordan Justen wrote:
>>> On Mon, Dec 19, 2011 at 11:41, Anthony Liguori<aliguori@us.ibm.com>  wrote:
>>>> On 12/15/2011 02:51 PM, Jordan Justen wrote:
>>>>>
>>>>> If a pflash image is found, then it is used for the system
>>>>> firmware image.
>>>>>
>>>>> If a pflash image is not initially found, then a read-only
>>>>> pflash device is created using the -bios filename.
>>>>>
>>>>> KVM cannot execute from a pflash region currently.
>>>>> Therefore, when KVM is enabled, a (read-only) ram memory
>>>>> region is created and filled with the contents of the
>>>>> pflash drive.
>>>>>
>>>>> Signed-off-by: Jordan Justen<jordan.l.justen@intel.com>
>>>>> Cc: Anthony Liguori<aliguori@us.ibm.com>
>>>>> ---
>>>>>   Makefile.target                    |    1 +
>>>>>   default-configs/i386-softmmu.mak   |    1 +
>>>>>   default-configs/x86_64-softmmu.mak |    1 +
>>>>>   hw/boards.h                        |    1 +
>>>>>   hw/pc.c                            |   55 +-------
>>>>>   hw/pc.h                            |    4 +
>>>>>   hw/pc_sysfw.c                      |  255
>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>   vl.c                               |    2 +-
>>>>>   8 files changed, 269 insertions(+), 51 deletions(-)
>>>>>   create mode 100644 hw/pc_sysfw.c
>>>>>
>>>>> diff --git a/Makefile.target b/Makefile.target
>>>>> index a111521..b1dc882 100644
>>>>> --- a/Makefile.target
>>>>> +++ b/Makefile.target
>>>>> @@ -236,6 +236,7 @@ obj-i386-y += vmport.o
>>>>>   obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
>>>>>   obj-i386-y += debugcon.o multiboot.o
>>>>>   obj-i386-y += pc_piix.o
>>>>> +obj-i386-y += pc_sysfw.o
>>>>>   obj-i386-$(CONFIG_KVM) += kvmclock.o
>>>>>   obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
>>>>>
>>>>> diff --git a/default-configs/i386-softmmu.mak
>>>>> b/default-configs/i386-softmmu.mak
>>>>> index e67ebb3..cd407a9 100644
>>>>> --- a/default-configs/i386-softmmu.mak
>>>>> +++ b/default-configs/i386-softmmu.mak
>>>>> @@ -22,3 +22,4 @@ CONFIG_SOUND=y
>>>>>   CONFIG_HPET=y
>>>>>   CONFIG_APPLESMC=y
>>>>>   CONFIG_I8259=y
>>>>> +CONFIG_PFLASH_CFI01=y
>>>>> diff --git a/default-configs/x86_64-softmmu.mak
>>>>> b/default-configs/x86_64-softmmu.mak
>>>>> index b75757e..47734ea 100644
>>>>> --- a/default-configs/x86_64-softmmu.mak
>>>>> +++ b/default-configs/x86_64-softmmu.mak
>>>>> @@ -22,3 +22,4 @@ CONFIG_SOUND=y
>>>>>   CONFIG_HPET=y
>>>>>   CONFIG_APPLESMC=y
>>>>>   CONFIG_I8259=y
>>>>> +CONFIG_PFLASH_CFI01=y
>>>>> diff --git a/hw/boards.h b/hw/boards.h
>>>>> index 716fd7b..45a31a1 100644
>>>>> --- a/hw/boards.h
>>>>> +++ b/hw/boards.h
>>>>> @@ -33,6 +33,7 @@ typedef struct QEMUMachine {
>>>>>   } QEMUMachine;
>>>>>
>>>>>   int qemu_register_machine(QEMUMachine *m);
>>>>> +QEMUMachine *find_default_machine(void);
>>>>>
>>>>>   extern QEMUMachine *current_machine;
>>>>>
>>>>> diff --git a/hw/pc.c b/hw/pc.c
>>>>> index cc6cfad..e5550ca 100644
>>>>> --- a/hw/pc.c
>>>>> +++ b/hw/pc.c
>>>>> @@ -57,10 +57,6 @@
>>>>>   #define DPRINTF(fmt, ...)
>>>>>   #endif
>>>>>
>>>>> -#define BIOS_FILENAME "bios.bin"
>>>>> -
>>>>> -#define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
>>>>> -
>>>>>   /* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables.
>>>>>   */
>>>>>   #define ACPI_DATA_SIZE       0x10000
>>>>>   #define BIOS_CFG_IOPORT 0x510
>>>>> @@ -974,11 +970,9 @@ void pc_memory_init(MemoryRegion *system_memory,
>>>>>                       MemoryRegion **ram_memory,
>>>>>                       int system_firmware_enabled)
>>>>>   {
>>>>> -    char *filename;
>>>>> -    int ret, linux_boot, i;
>>>>> -    MemoryRegion *ram, *bios, *isa_bios, *option_rom_mr;
>>>>> +    int linux_boot, i;
>>>>> +    MemoryRegion *ram, *option_rom_mr;
>>>>>       MemoryRegion *ram_below_4g, *ram_above_4g;
>>>>> -    int bios_size, isa_bios_size;
>>>>>       void *fw_cfg;
>>>>>
>>>>>       linux_boot = (kernel_filename != NULL);
>>>>> @@ -1003,43 +997,9 @@ void pc_memory_init(MemoryRegion *system_memory,
>>>>>                                       ram_above_4g);
>>>>>       }
>>>>>
>>>>> -    /* BIOS load */
>>>>> -    if (bios_name == NULL)
>>>>> -        bios_name = BIOS_FILENAME;
>>>>> -    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>>>>> -    if (filename) {
>>>>> -        bios_size = get_image_size(filename);
>>>>> -    } else {
>>>>> -        bios_size = -1;
>>>>> -    }
>>>>> -    if (bios_size<= 0 ||
>>>>> -        (bios_size % 65536) != 0) {
>>>>> -        goto bios_error;
>>>>> -    }
>>>>> -    bios = g_malloc(sizeof(*bios));
>>>>> -    memory_region_init_ram(bios, NULL, "pc.bios", bios_size);
>>>>> -    memory_region_set_readonly(bios, true);
>>>>> -    ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1);
>>>>> -    if (ret != 0) {
>>>>> -    bios_error:
>>>>> -        fprintf(stderr, "qemu: could not load PC BIOS '%s'\n",
>>>>> bios_name);
>>>>> -        exit(1);
>>>>> -    }
>>>>> -    if (filename) {
>>>>> -        g_free(filename);
>>>>> -    }
>>>>> -    /* map the last 128KB of the BIOS in ISA space */
>>>>> -    isa_bios_size = bios_size;
>>>>> -    if (isa_bios_size>    (128 * 1024))
>>>>> -        isa_bios_size = 128 * 1024;
>>>>> -    isa_bios = g_malloc(sizeof(*isa_bios));
>>>>> -    memory_region_init_alias(isa_bios, "isa-bios", bios,
>>>>> -                             bios_size - isa_bios_size, isa_bios_size);
>>>>> -    memory_region_add_subregion_overlap(rom_memory,
>>>>> -                                        0x100000 - isa_bios_size,
>>>>> -                                        isa_bios,
>>>>> -                                        1);
>>>>> -    memory_region_set_readonly(isa_bios, true);
>>>>> +
>>>>> +    /* Initialize ROM or flash ranges for PC firmware */
>>>>> +    pc_system_firmware_init(rom_memory, system_firmware_enabled);
>>>>>
>>>>>       option_rom_mr = g_malloc(sizeof(*option_rom_mr));
>>>>>       memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE);
>>>>> @@ -1048,11 +1008,6 @@ void pc_memory_init(MemoryRegion *system_memory,
>>>>>                                           option_rom_mr,
>>>>>                                           1);
>>>>>
>>>>> -    /* map all the bios at the top of memory */
>>>>> -    memory_region_add_subregion(rom_memory,
>>>>> -                                (uint32_t)(-bios_size),
>>>>> -                                bios);
>>>>> -
>>>>>       fw_cfg = bochs_bios_init();
>>>>>       rom_set_fw(fw_cfg);
>>>>>
>>>>> diff --git a/hw/pc.h b/hw/pc.h
>>>>> index 49471cb..727e231 100644
>>>>> --- a/hw/pc.h
>>>>> +++ b/hw/pc.h
>>>>> @@ -246,6 +246,10 @@ static inline bool isa_ne2000_init(int base, int irq,
>>>>> NICInfo *nd)
>>>>>       return true;
>>>>>   }
>>>>>
>>>>> +/* pc_sysfw.c */
>>>>> +void pc_system_firmware_init(MemoryRegion *rom_memory,
>>>>> +                             int system_firmware_enabled);
>>>>> +
>>>>>   /* e820 types */
>>>>>   #define E820_RAM        1
>>>>>   #define E820_RESERVED   2
>>>>> diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
>>>>> new file mode 100644
>>>>> index 0000000..20027b2
>>>>> --- /dev/null
>>>>> +++ b/hw/pc_sysfw.c
>>>>> @@ -0,0 +1,255 @@
>>>>> +/*
>>>>> + * QEMU PC System Firmware
>>>>> + *
>>>>> + * Copyright (c) 2003-2004 Fabrice Bellard
>>>>> + * Copyright (c) 2011 Intel Corporation
>>>>> + *
>>>>> + * Permission is hereby granted, free of charge, to any person obtaining
>>>>> a copy
>>>>> + * of this software and associated documentation files (the "Software"),
>>>>> to deal
>>>>> + * in the Software without restriction, including without limitation the
>>>>> rights
>>>>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or
>>>>> sell
>>>>> + * copies of the Software, and to permit persons to whom the Software is
>>>>> + * furnished to do so, subject to the following conditions:
>>>>> + *
>>>>> + * The above copyright notice and this permission notice shall be
>>>>> included in
>>>>> + * all copies or substantial portions of the Software.
>>>>> + *
>>>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>>>>> EXPRESS OR
>>>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>>>>> MERCHANTABILITY,
>>>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>>>>> SHALL
>>>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>>>>> OTHER
>>>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>>>>> ARISING FROM,
>>>>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>>>>> IN
>>>>> + * THE SOFTWARE.
>>>>> + */
>>>>> +
>>>>> +#include "hw.h"
>>>>> +#include "pc.h"
>>>>> +#include "hw/boards.h"
>>>>> +#include "loader.h"
>>>>> +#include "sysemu.h"
>>>>> +#include "flash.h"
>>>>> +#include "kvm.h"
>>>>> +
>>>>> +#define BIOS_FILENAME "bios.bin"
>>>>> +
>>>>> +static void pc_isa_bios_init(MemoryRegion *rom_memory,
>>>>> +                             MemoryRegion *flash_mem,
>>>>> +                             int ram_size)
>>>>> +{
>>>>> +    int isa_bios_size;
>>>>> +    MemoryRegion *isa_bios;
>>>>> +    uint64_t flash_size;
>>>>> +    void *flash_ptr, *isa_bios_ptr;
>>>>> +
>>>>> +    flash_size = memory_region_size(flash_mem);
>>>>> +
>>>>> +    /* map the last 128KB of the BIOS in ISA space */
>>>>> +    isa_bios_size = flash_size;
>>>>> +    if (isa_bios_size>    (128 * 1024)) {
>>>>> +        isa_bios_size = 128 * 1024;
>>>>> +    }
>>>>> +    isa_bios = g_malloc(sizeof(*isa_bios));
>>>>> +    memory_region_init_ram(isa_bios, NULL, "isa-bios", isa_bios_size);
>>>>> +    memory_region_add_subregion_overlap(rom_memory,
>>>>> +                                        0x100000 - isa_bios_size,
>>>>> +                                        isa_bios,
>>>>> +                                        1);
>>>>> +
>>>>> +    /* copy ISA rom image from top of flash memory */
>>>>> +    flash_ptr = memory_region_get_ram_ptr(flash_mem);
>>>>> +    isa_bios_ptr = memory_region_get_ram_ptr(isa_bios);
>>>>> +    memcpy(isa_bios_ptr,
>>>>> +           ((uint8_t*)flash_ptr) + (flash_size - isa_bios_size),
>>>>> +           isa_bios_size);
>>>>> +
>>>>> +    memory_region_set_readonly(isa_bios, true);
>>>>> +}
>>>>> +
>>>>> +static void pc_fw_add_pflash_drv(void)
>>>>> +{
>>>>> +    QemuOpts *opts;
>>>>> +    QEMUMachine *machine;
>>>>> +    char *filename;
>>>>> +
>>>>> +    if (bios_name == NULL) {
>>>>> +        bios_name = BIOS_FILENAME;
>>>>> +    }
>>>>> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>>>>> +
>>>>> +    opts = drive_add(IF_PFLASH, -1, filename, "readonly=on");
>>>>> +    if (opts == NULL) {
>>>>> +      return;
>>>>> +    }
>>>>> +
>>>>> +    machine = find_default_machine();
>>>>> +    if (machine == NULL) {
>>>>> +      return;
>>>>> +    }
>>>>> +
>>>>> +    drive_init(opts, machine->use_scsi);
>>>>> +}
>>>>> +
>>>>> +static void pc_system_flash_init(MemoryRegion *rom_memory,
>>>>> +                                 DriveInfo *pflash_drv)
>>>>> +{
>>>>> +    BlockDriverState *bdrv;
>>>>> +    int64_t size;
>>>>> +    target_phys_addr_t phys_addr;
>>>>> +    int sector_bits, sector_size;
>>>>> +    pflash_t *system_flash;
>>>>> +    MemoryRegion *flash_mem;
>>>>> +
>>>>> +    bdrv = pflash_drv->bdrv;
>>>>> +    size = bdrv_getlength(pflash_drv->bdrv);
>>>>> +    sector_bits = 12;
>>>>> +    sector_size = 1<<    sector_bits;
>>>>> +
>>>>> +    if ((size % sector_size) != 0) {
>>>>> +        fprintf(stderr,
>>>>> +                "qemu: PC system firmware (pflash) must be a multiple of
>>>>> 0x%x\n",
>>>>> +                sector_size);
>>>>> +        exit(1);
>>>>> +    }
>>>>> +
>>>>> +    phys_addr = 0x100000000ULL - size;
>>>>> +    system_flash = pflash_cfi01_register(phys_addr, NULL, "system.flash",
>>>>> size,
>>>>> +                                         bdrv, sector_size, size>>
>>>>>   sector_bits,
>>>>> +                                         1, 0x0000, 0x0000, 0x0000,
>>>>> 0x0000, 0);
>>>>> +    flash_mem = pflash_cfi01_get_memory(system_flash);
>>>>> +
>>>>> +    pc_isa_bios_init(rom_memory, flash_mem, size);
>>>>> +}
>>>>> +
>>>>> +static void pc_system_rom_init(MemoryRegion *rom_memory,
>>>>> +                               DriveInfo *pflash_drv)
>>>>> +{
>>>>> +    BlockDriverState *bdrv;
>>>>> +    int64_t size;
>>>>> +    target_phys_addr_t phys_addr;
>>>>> +    int sector_bits, sector_size;
>>>>> +    MemoryRegion *sys_rom;
>>>>> +    void *buffer;
>>>>> +    int ret;
>>>>> +
>>>>> +    bdrv = pflash_drv->bdrv;
>>>>> +    size = bdrv_getlength(pflash_drv->bdrv);
>>>>> +    sector_bits = 9;
>>>>> +    sector_size = 1<<    sector_bits;
>>>>> +
>>>>> +    if ((size % sector_size) != 0) {
>>>>> +        fprintf(stderr,
>>>>> +                "qemu: PC system rom (pflash) must be a multiple of
>>>>> 0x%x\n",
>>>>> +                sector_size);
>>>>> +        exit(1);
>>>>> +    }
>>>>> +
>>>>> +    phys_addr = 0x100000000ULL - size;
>>>>> +    sys_rom = g_malloc(sizeof(*sys_rom));
>>>>> +    memory_region_init_ram(sys_rom, NULL, "system.rom", size);
>>>>> +    buffer = memory_region_get_ram_ptr(sys_rom);
>>>>> +    memory_region_add_subregion(rom_memory, phys_addr, sys_rom);
>>>>> +
>>>>> +    /* read the rom content */
>>>>> +    ret = bdrv_read(bdrv, 0, buffer, size>>    sector_bits);
>>>>
>>>>
>>>> I think we're trying to get rid of synchronous block I/O in machine
>>>> initialization for a number of reasons.
>>>>
>>>> Kevin/Stefan, care to comment?  Will this be problematic in the future?
>>>
>>> I was hoping pc-1.1 with and without kvm could be as close as
>>> possible, but I guess I can make pc-1.1 with kvm behave the same as
>>> pc-1.0.  Then I can delete pc_system_rom_init.
>>
>> I think your general approach is right, I'm just not sure what we're going to do
>> short term about synchronous I/O in the machine init routines.  It may just be a
>> matter of structuring this in such a way that you can use an async interface.
>
> I don't think there is a problem with using the synchronous interface
> here. But I'm not sure if having any I/O in the machine init is a good
> idea with respect to migration. Didn't we already have to move some code
> there to a later stage to make sure that the destination doesn't use
> outdated data?

Since the bdrv_read call seems to be the concern here, I noticed that
in hw/pflash_cfi01.c pflash_cfi01_register there is a call to
bdrv_read.  So, it seems like it is not possible to initialize a
pflash/cfi device without a bdrv_read call being made.

So, what should pflash/cfi be doing instead?  Should these devices
somehow hold off on initializing their contents from the drive until a
later stage?  If so, is there a later call into the devices before the
firmware is launched that could be used for this later initialization
step?

-Jordan

Patch

diff --git a/Makefile.target b/Makefile.target
index a111521..b1dc882 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -236,6 +236,7 @@  obj-i386-y += vmport.o
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += debugcon.o multiboot.o
 obj-i386-y += pc_piix.o
+obj-i386-y += pc_sysfw.o
 obj-i386-$(CONFIG_KVM) += kvmclock.o
 obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
 
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index e67ebb3..cd407a9 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -22,3 +22,4 @@  CONFIG_SOUND=y
 CONFIG_HPET=y
 CONFIG_APPLESMC=y
 CONFIG_I8259=y
+CONFIG_PFLASH_CFI01=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index b75757e..47734ea 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -22,3 +22,4 @@  CONFIG_SOUND=y
 CONFIG_HPET=y
 CONFIG_APPLESMC=y
 CONFIG_I8259=y
+CONFIG_PFLASH_CFI01=y
diff --git a/hw/boards.h b/hw/boards.h
index 716fd7b..45a31a1 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -33,6 +33,7 @@  typedef struct QEMUMachine {
 } QEMUMachine;
 
 int qemu_register_machine(QEMUMachine *m);
+QEMUMachine *find_default_machine(void);
 
 extern QEMUMachine *current_machine;
 
diff --git a/hw/pc.c b/hw/pc.c
index cc6cfad..e5550ca 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -57,10 +57,6 @@ 
 #define DPRINTF(fmt, ...)
 #endif
 
-#define BIOS_FILENAME "bios.bin"
-
-#define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
-
 /* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables.  */
 #define ACPI_DATA_SIZE       0x10000
 #define BIOS_CFG_IOPORT 0x510
@@ -974,11 +970,9 @@  void pc_memory_init(MemoryRegion *system_memory,
                     MemoryRegion **ram_memory,
                     int system_firmware_enabled)
 {
-    char *filename;
-    int ret, linux_boot, i;
-    MemoryRegion *ram, *bios, *isa_bios, *option_rom_mr;
+    int linux_boot, i;
+    MemoryRegion *ram, *option_rom_mr;
     MemoryRegion *ram_below_4g, *ram_above_4g;
-    int bios_size, isa_bios_size;
     void *fw_cfg;
 
     linux_boot = (kernel_filename != NULL);
@@ -1003,43 +997,9 @@  void pc_memory_init(MemoryRegion *system_memory,
                                     ram_above_4g);
     }
 
-    /* BIOS load */
-    if (bios_name == NULL)
-        bios_name = BIOS_FILENAME;
-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
-    if (filename) {
-        bios_size = get_image_size(filename);
-    } else {
-        bios_size = -1;
-    }
-    if (bios_size <= 0 ||
-        (bios_size % 65536) != 0) {
-        goto bios_error;
-    }
-    bios = g_malloc(sizeof(*bios));
-    memory_region_init_ram(bios, NULL, "pc.bios", bios_size);
-    memory_region_set_readonly(bios, true);
-    ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1);
-    if (ret != 0) {
-    bios_error:
-        fprintf(stderr, "qemu: could not load PC BIOS '%s'\n", bios_name);
-        exit(1);
-    }
-    if (filename) {
-        g_free(filename);
-    }
-    /* map the last 128KB of the BIOS in ISA space */
-    isa_bios_size = bios_size;
-    if (isa_bios_size > (128 * 1024))
-        isa_bios_size = 128 * 1024;
-    isa_bios = g_malloc(sizeof(*isa_bios));
-    memory_region_init_alias(isa_bios, "isa-bios", bios,
-                             bios_size - isa_bios_size, isa_bios_size);
-    memory_region_add_subregion_overlap(rom_memory,
-                                        0x100000 - isa_bios_size,
-                                        isa_bios,
-                                        1);
-    memory_region_set_readonly(isa_bios, true);
+
+    /* Initialize ROM or flash ranges for PC firmware */
+    pc_system_firmware_init(rom_memory, system_firmware_enabled);
 
     option_rom_mr = g_malloc(sizeof(*option_rom_mr));
     memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE);
@@ -1048,11 +1008,6 @@  void pc_memory_init(MemoryRegion *system_memory,
                                         option_rom_mr,
                                         1);
 
-    /* map all the bios at the top of memory */
-    memory_region_add_subregion(rom_memory,
-                                (uint32_t)(-bios_size),
-                                bios);
-
     fw_cfg = bochs_bios_init();
     rom_set_fw(fw_cfg);
 
diff --git a/hw/pc.h b/hw/pc.h
index 49471cb..727e231 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -246,6 +246,10 @@  static inline bool isa_ne2000_init(int base, int irq, NICInfo *nd)
     return true;
 }
 
+/* pc_sysfw.c */
+void pc_system_firmware_init(MemoryRegion *rom_memory,
+                             int system_firmware_enabled);
+
 /* e820 types */
 #define E820_RAM        1
 #define E820_RESERVED   2
diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
new file mode 100644
index 0000000..20027b2
--- /dev/null
+++ b/hw/pc_sysfw.c
@@ -0,0 +1,255 @@ 
+/*
+ * QEMU PC System Firmware
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2011 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw.h"
+#include "pc.h"
+#include "hw/boards.h"
+#include "loader.h"
+#include "sysemu.h"
+#include "flash.h"
+#include "kvm.h"
+
+#define BIOS_FILENAME "bios.bin"
+
+static void pc_isa_bios_init(MemoryRegion *rom_memory,
+                             MemoryRegion *flash_mem,
+                             int ram_size)
+{
+    int isa_bios_size;
+    MemoryRegion *isa_bios;
+    uint64_t flash_size;
+    void *flash_ptr, *isa_bios_ptr;
+
+    flash_size = memory_region_size(flash_mem);
+
+    /* map the last 128KB of the BIOS in ISA space */
+    isa_bios_size = flash_size;
+    if (isa_bios_size > (128 * 1024)) {
+        isa_bios_size = 128 * 1024;
+    }
+    isa_bios = g_malloc(sizeof(*isa_bios));
+    memory_region_init_ram(isa_bios, NULL, "isa-bios", isa_bios_size);
+    memory_region_add_subregion_overlap(rom_memory,
+                                        0x100000 - isa_bios_size,
+                                        isa_bios,
+                                        1);
+
+    /* copy ISA rom image from top of flash memory */
+    flash_ptr = memory_region_get_ram_ptr(flash_mem);
+    isa_bios_ptr = memory_region_get_ram_ptr(isa_bios);
+    memcpy(isa_bios_ptr,
+           ((uint8_t*)flash_ptr) + (flash_size - isa_bios_size),
+           isa_bios_size);
+
+    memory_region_set_readonly(isa_bios, true);
+}
+
+static void pc_fw_add_pflash_drv(void)
+{
+    QemuOpts *opts;
+    QEMUMachine *machine;
+    char *filename;
+
+    if (bios_name == NULL) {
+        bios_name = BIOS_FILENAME;
+    }
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+
+    opts = drive_add(IF_PFLASH, -1, filename, "readonly=on");
+    if (opts == NULL) {
+      return;
+    }
+
+    machine = find_default_machine();
+    if (machine == NULL) {
+      return;
+    }
+
+    drive_init(opts, machine->use_scsi);
+}
+
+static void pc_system_flash_init(MemoryRegion *rom_memory,
+                                 DriveInfo *pflash_drv)
+{
+    BlockDriverState *bdrv;
+    int64_t size;
+    target_phys_addr_t phys_addr;
+    int sector_bits, sector_size;
+    pflash_t *system_flash;
+    MemoryRegion *flash_mem;
+
+    bdrv = pflash_drv->bdrv;
+    size = bdrv_getlength(pflash_drv->bdrv);
+    sector_bits = 12;
+    sector_size = 1 << sector_bits;
+
+    if ((size % sector_size) != 0) {
+        fprintf(stderr,
+                "qemu: PC system firmware (pflash) must be a multiple of 0x%x\n",
+                sector_size);
+        exit(1);
+    }
+
+    phys_addr = 0x100000000ULL - size;
+    system_flash = pflash_cfi01_register(phys_addr, NULL, "system.flash", size,
+                                         bdrv, sector_size, size >> sector_bits,
+                                         1, 0x0000, 0x0000, 0x0000, 0x0000, 0);
+    flash_mem = pflash_cfi01_get_memory(system_flash);
+
+    pc_isa_bios_init(rom_memory, flash_mem, size);
+}
+
+static void pc_system_rom_init(MemoryRegion *rom_memory,
+                               DriveInfo *pflash_drv)
+{
+    BlockDriverState *bdrv;
+    int64_t size;
+    target_phys_addr_t phys_addr;
+    int sector_bits, sector_size;
+    MemoryRegion *sys_rom;
+    void *buffer;
+    int ret;
+
+    bdrv = pflash_drv->bdrv;
+    size = bdrv_getlength(pflash_drv->bdrv);
+    sector_bits = 9;
+    sector_size = 1 << sector_bits;
+
+    if ((size % sector_size) != 0) {
+        fprintf(stderr,
+                "qemu: PC system rom (pflash) must be a multiple of 0x%x\n",
+                sector_size);
+        exit(1);
+    }
+
+    phys_addr = 0x100000000ULL - size;
+    sys_rom = g_malloc(sizeof(*sys_rom));
+    memory_region_init_ram(sys_rom, NULL, "system.rom", size);
+    buffer = memory_region_get_ram_ptr(sys_rom);
+    memory_region_add_subregion(rom_memory, phys_addr, sys_rom);
+
+    /* read the rom content */
+    ret = bdrv_read(bdrv, 0, buffer, size >> sector_bits);
+    if (ret < 0) {
+        memory_region_destroy(sys_rom);
+        g_free(sys_rom);
+        fprintf(stderr,
+                "qemu: Failed to read rom image from pflash drive\n");
+        exit(1);
+    }
+
+    memory_region_set_readonly(sys_rom, true);
+
+    pc_isa_bios_init(rom_memory, sys_rom, size);
+}
+
+static void old_pc_system_rom_init(MemoryRegion *rom_memory)
+{
+    char *filename;
+    MemoryRegion *bios, *isa_bios;
+    int bios_size, isa_bios_size;
+    int ret;
+
+    /* BIOS load */
+    if (bios_name == NULL) {
+        bios_name = BIOS_FILENAME;
+    }
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+    if (filename) {
+        bios_size = get_image_size(filename);
+    } else {
+        bios_size = -1;
+    }
+    if (bios_size <= 0 ||
+        (bios_size % 65536) != 0) {
+        goto bios_error;
+    }
+    bios = g_malloc(sizeof(*bios));
+    memory_region_init_ram(bios, NULL, "pc.bios", bios_size);
+    memory_region_set_readonly(bios, true);
+    ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1);
+    if (ret != 0) {
+    bios_error:
+        fprintf(stderr, "qemu: could not load PC BIOS '%s'\n", bios_name);
+        exit(1);
+    }
+    if (filename) {
+        g_free(filename);
+    }
+
+    /* map the last 128KB of the BIOS in ISA space */
+    isa_bios_size = bios_size;
+    if (isa_bios_size > (128 * 1024)) {
+        isa_bios_size = 128 * 1024;
+    }
+    isa_bios = g_malloc(sizeof(*isa_bios));
+    memory_region_init_alias(isa_bios, "isa-bios", bios,
+                             bios_size - isa_bios_size, isa_bios_size);
+    memory_region_add_subregion_overlap(rom_memory,
+                                        0x100000 - isa_bios_size,
+                                        isa_bios,
+                                        1);
+    memory_region_set_readonly(isa_bios, true);
+
+    /* map all the bios at the top of memory */
+    memory_region_add_subregion(rom_memory,
+                                (uint32_t)(-bios_size),
+                                bios);
+
+}
+
+void pc_system_firmware_init(MemoryRegion *rom_memory,
+                             int system_firmware_enabled)
+{
+    int flash_present;
+    DriveInfo *pflash_drv;
+
+    if (!system_firmware_enabled) {
+        old_pc_system_rom_init(rom_memory);
+        return;
+    }
+
+    pflash_drv = drive_get(IF_PFLASH, 0, 0);
+    flash_present = (pflash_drv != NULL);
+
+    if (!flash_present) {
+        pc_fw_add_pflash_drv();
+        pflash_drv = drive_get(IF_PFLASH, 0, 0);
+        flash_present = (pflash_drv != NULL);
+    }
+
+    if (!flash_present) {
+        fprintf(stderr, "qemu: PC system firmware (pflash) not available\n");
+        exit(1);
+    }
+
+    if (!kvm_enabled()) {
+        pc_system_flash_init(rom_memory, pflash_drv);
+    } else {
+        pc_system_rom_init(rom_memory, pflash_drv);
+    }
+}
+
+
diff --git a/vl.c b/vl.c
index 5372a96..9f77742 100644
--- a/vl.c
+++ b/vl.c
@@ -1186,7 +1186,7 @@  static QEMUMachine *find_machine(const char *name)
     return NULL;
 }
 
-static QEMUMachine *find_default_machine(void)
+QEMUMachine *find_default_machine(void)
 {
     QEMUMachine *m;