| Message ID | 1310153845-4373-1-git-send-email-jordan.l.justen@intel.com |
|---|---|
| State | New |
| Headers | show |
Hi all, Are there any concerns with this patch? I haven't heard much feedback, except: * Jes Sorensen - March 28 - code style * Aurelien Jarno - April 18 - Reviewed-by Thanks, -Jordan On Fri, Jul 8, 2011 at 12:37, Jordan Justen <jordan.l.justen@intel.com> wrote: > If -pflash is specified and -bios is specified then pflash will > be mapped just below the system rom using hw/pflash_cfi01.c. > > If -pflash is specified on the command line, but -bios is > not specified, then 'bios.bin' will NOT be loaded, and > instead the -pflash flash image will be mapped just below > 4GB in place of the normal rom image. > > Signed-off-by: Jordan Justen <jordan.l.justen@intel.com> > Reviewed-by: Aurelien Jarno <aurelien@aurel32.net> > --- > default-configs/i386-softmmu.mak | 1 + > default-configs/x86_64-softmmu.mak | 1 + > hw/pc.c | 161 +++++++++++++++++++++++++++--------- > 3 files changed, 125 insertions(+), 38 deletions(-) > > diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak > index 55589fa..8697cd4 100644 > --- a/default-configs/i386-softmmu.mak > +++ b/default-configs/i386-softmmu.mak > @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y > CONFIG_SOUND=y > CONFIG_HPET=y > CONFIG_APPLESMC=y > +CONFIG_PFLASH_CFI01=y > diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak > index 8895028..eca9284 100644 > --- a/default-configs/x86_64-softmmu.mak > +++ b/default-configs/x86_64-softmmu.mak > @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y > CONFIG_SOUND=y > CONFIG_HPET=y > CONFIG_APPLESMC=y > +CONFIG_PFLASH_CFI01=y > diff --git a/hw/pc.c b/hw/pc.c > index a3e8539..e25354f 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -41,6 +41,7 @@ > #include "sysemu.h" > #include "blockdev.h" > #include "ui/qemu-spice.h" > +#include "flash.h" > > /* output Bochs bios info messages */ > //#define DEBUG_BIOS > @@ -957,70 +958,154 @@ void pc_cpus_init(const char *cpu_model) > } > } > > -void pc_memory_init(const char *kernel_filename, > - const char *kernel_cmdline, > - const char *initrd_filename, > - ram_addr_t below_4g_mem_size, > - ram_addr_t above_4g_mem_size) > +static void pc_isa_bios_init(ram_addr_t ram_offset, int ram_size) > { > - char *filename; > - int ret, linux_boot, i; > - ram_addr_t ram_addr, bios_offset, option_rom_offset; > - int bios_size, isa_bios_size; > - void *fw_cfg; > - > - linux_boot = (kernel_filename != NULL); > + int isa_bios_size; > > - /* allocate RAM */ > - ram_addr = qemu_ram_alloc(NULL, "pc.ram", > - below_4g_mem_size + above_4g_mem_size); > - cpu_register_physical_memory(0, 0xa0000, ram_addr); > - cpu_register_physical_memory(0x100000, > - below_4g_mem_size - 0x100000, > - ram_addr + 0x100000); > - if (above_4g_mem_size > 0) { > - cpu_register_physical_memory(0x100000000ULL, above_4g_mem_size, > - ram_addr + below_4g_mem_size); > + /* map the last 128KB of the BIOS in ISA space */ > + isa_bios_size = ram_size; > + if (isa_bios_size > (128 * 1024)) { > + isa_bios_size = 128 * 1024; > } > + ram_offset = ram_offset + ram_size - isa_bios_size; > + cpu_register_physical_memory(0x100000 - isa_bios_size, > + isa_bios_size, > + ram_offset | IO_MEM_ROM); > +} > + > +static int pc_system_rom_init(void) > +{ > + int ret; > + int bios_size; > + ram_addr_t bios_offset; > + char *filename; > > /* BIOS load */ > - if (bios_name == NULL) > + 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; > + > + if (bios_size <= 0 || (bios_size % 65536) != 0) { > + ret = -1; > + } else { > + bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); > + ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); > } > - bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); > - 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) { > qemu_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; > - cpu_register_physical_memory(0x100000 - isa_bios_size, > - isa_bios_size, > - (bios_offset + bios_size - isa_bios_size) | IO_MEM_ROM); > > - option_rom_offset = qemu_ram_alloc(NULL, "pc.rom", PC_ROM_SIZE); > - cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE, option_rom_offset); > + pc_isa_bios_init(bios_offset, bios_size); > > /* map all the bios at the top of memory */ > cpu_register_physical_memory((uint32_t)(-bios_size), > bios_size, bios_offset | IO_MEM_ROM); > > + return bios_size; > +} > + > +static void pc_system_flash_init(DriveInfo *pflash_drv, int rom_size) > +{ > + BlockDriverState *bdrv; > + int64_t size; > + target_phys_addr_t phys_addr; > + ram_addr_t addr; > + int sector_bits, sector_size; > + > + bdrv = NULL; > + > + 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: -pflash size must be a multiple of 0x%x\n", > + sector_size); > + exit(1); > + } > + > + phys_addr = 0x100000000ULL - rom_size - size; > + addr = qemu_ram_alloc(NULL, "system.flash", size); > + DPRINTF("flash addr: 0x%lx\n", (int64_t)phys_addr); > + pflash_cfi01_register(phys_addr, addr, bdrv, > + sector_size, size >> sector_bits, > + 4, 0x0000, 0x0000, 0x0000, 0x0000, 0); > + > + if (rom_size == 0) { > + pc_isa_bios_init(addr, size); > + } > +} > + > +static void pc_system_firmware_init(void) > +{ > + int flash_present, rom_present; > + int rom_size; > + DriveInfo *pflash_drv; > + > + pflash_drv = drive_get(IF_PFLASH, 0, 0); > + flash_present = (pflash_drv != NULL); > + > + /* Load rom if -bios is used or if -pflash is not used */ > + rom_present = ((bios_name != NULL) || !flash_present); > + > + /* If rom is present, then it is mapped just below 4GB */ > + if (rom_present) { > + rom_size = pc_system_rom_init(); > + } else { > + rom_size = 0; > + } > + > + /* If flash is present, then it is mapped just below the rom, or > + * just below 4GB when rom is not present. */ > + if (flash_present) { > + pc_system_flash_init(pflash_drv, rom_size); > + } > +} > + > +void pc_memory_init(const char *kernel_filename, > + const char *kernel_cmdline, > + const char *initrd_filename, > + ram_addr_t below_4g_mem_size, > + ram_addr_t above_4g_mem_size) > +{ > + int linux_boot, i; > + ram_addr_t ram_addr, option_rom_offset; > + void *fw_cfg; > + > + linux_boot = (kernel_filename != NULL); > + > + /* allocate RAM */ > + ram_addr = qemu_ram_alloc(NULL, "pc.ram", > + below_4g_mem_size + above_4g_mem_size); > + cpu_register_physical_memory(0, 0xa0000, ram_addr); > + cpu_register_physical_memory(0x100000, > + below_4g_mem_size - 0x100000, > + ram_addr + 0x100000); > + if (above_4g_mem_size > 0) { > + cpu_register_physical_memory(0x100000000ULL, above_4g_mem_size, > + ram_addr + below_4g_mem_size); > + } > + > + /* Initialize ROM or flash ranges for PC firmware */ > + pc_system_firmware_init(); > + > + option_rom_offset = qemu_ram_alloc(NULL, "pc.rom", PC_ROM_SIZE); > + cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE, option_rom_offset); > + > fw_cfg = bochs_bios_init(); > rom_set_fw(fw_cfg); > > -- > 1.7.1 > > >
On 07/08/2011 02:37 PM, Jordan Justen wrote: > If -pflash is specified and -bios is specified then pflash will > be mapped just below the system rom using hw/pflash_cfi01.c. > > If -pflash is specified on the command line, but -bios is > not specified, then 'bios.bin' will NOT be loaded, and > instead the -pflash flash image will be mapped just below > 4GB in place of the normal rom image. This is way too tied to the pc platform to be this generic. I think a better approach would be to default to having unit=0 of IF_PFLASH default to a read-only BDS that points to bios.bin. -bios would just be a short cut to use a different file name but you should be able to override with -drive too. And to really simplify things, you could add a readonly flag to -bios such that you could do: -bios foo.img,readonly=off Which is what I think you're looking for semantically. Regards, Anthony Liguori > > Signed-off-by: Jordan Justen<jordan.l.justen@intel.com> > Reviewed-by: Aurelien Jarno<aurelien@aurel32.net> > --- > default-configs/i386-softmmu.mak | 1 + > default-configs/x86_64-softmmu.mak | 1 + > hw/pc.c | 161 +++++++++++++++++++++++++++--------- > 3 files changed, 125 insertions(+), 38 deletions(-) > > diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak > index 55589fa..8697cd4 100644 > --- a/default-configs/i386-softmmu.mak > +++ b/default-configs/i386-softmmu.mak > @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y > CONFIG_SOUND=y > CONFIG_HPET=y > CONFIG_APPLESMC=y > +CONFIG_PFLASH_CFI01=y > diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak > index 8895028..eca9284 100644 > --- a/default-configs/x86_64-softmmu.mak > +++ b/default-configs/x86_64-softmmu.mak > @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y > CONFIG_SOUND=y > CONFIG_HPET=y > CONFIG_APPLESMC=y > +CONFIG_PFLASH_CFI01=y > diff --git a/hw/pc.c b/hw/pc.c > index a3e8539..e25354f 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -41,6 +41,7 @@ > #include "sysemu.h" > #include "blockdev.h" > #include "ui/qemu-spice.h" > +#include "flash.h" > > /* output Bochs bios info messages */ > //#define DEBUG_BIOS > @@ -957,70 +958,154 @@ void pc_cpus_init(const char *cpu_model) > } > } > > -void pc_memory_init(const char *kernel_filename, > - const char *kernel_cmdline, > - const char *initrd_filename, > - ram_addr_t below_4g_mem_size, > - ram_addr_t above_4g_mem_size) > +static void pc_isa_bios_init(ram_addr_t ram_offset, int ram_size) > { > - char *filename; > - int ret, linux_boot, i; > - ram_addr_t ram_addr, bios_offset, option_rom_offset; > - int bios_size, isa_bios_size; > - void *fw_cfg; > - > - linux_boot = (kernel_filename != NULL); > + int isa_bios_size; > > - /* allocate RAM */ > - ram_addr = qemu_ram_alloc(NULL, "pc.ram", > - below_4g_mem_size + above_4g_mem_size); > - cpu_register_physical_memory(0, 0xa0000, ram_addr); > - cpu_register_physical_memory(0x100000, > - below_4g_mem_size - 0x100000, > - ram_addr + 0x100000); > - if (above_4g_mem_size> 0) { > - cpu_register_physical_memory(0x100000000ULL, above_4g_mem_size, > - ram_addr + below_4g_mem_size); > + /* map the last 128KB of the BIOS in ISA space */ > + isa_bios_size = ram_size; > + if (isa_bios_size> (128 * 1024)) { > + isa_bios_size = 128 * 1024; > } > + ram_offset = ram_offset + ram_size - isa_bios_size; > + cpu_register_physical_memory(0x100000 - isa_bios_size, > + isa_bios_size, > + ram_offset | IO_MEM_ROM); > +} > + > +static int pc_system_rom_init(void) > +{ > + int ret; > + int bios_size; > + ram_addr_t bios_offset; > + char *filename; > > /* BIOS load */ > - if (bios_name == NULL) > + 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; > + > + if (bios_size<= 0 || (bios_size % 65536) != 0) { > + ret = -1; > + } else { > + bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); > + ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); > } > - bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); > - 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) { > qemu_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; > - cpu_register_physical_memory(0x100000 - isa_bios_size, > - isa_bios_size, > - (bios_offset + bios_size - isa_bios_size) | IO_MEM_ROM); > > - option_rom_offset = qemu_ram_alloc(NULL, "pc.rom", PC_ROM_SIZE); > - cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE, option_rom_offset); > + pc_isa_bios_init(bios_offset, bios_size); > > /* map all the bios at the top of memory */ > cpu_register_physical_memory((uint32_t)(-bios_size), > bios_size, bios_offset | IO_MEM_ROM); > > + return bios_size; > +} > + > +static void pc_system_flash_init(DriveInfo *pflash_drv, int rom_size) > +{ > + BlockDriverState *bdrv; > + int64_t size; > + target_phys_addr_t phys_addr; > + ram_addr_t addr; > + int sector_bits, sector_size; > + > + bdrv = NULL; > + > + 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: -pflash size must be a multiple of 0x%x\n", > + sector_size); > + exit(1); > + } > + > + phys_addr = 0x100000000ULL - rom_size - size; > + addr = qemu_ram_alloc(NULL, "system.flash", size); > + DPRINTF("flash addr: 0x%lx\n", (int64_t)phys_addr); > + pflash_cfi01_register(phys_addr, addr, bdrv, > + sector_size, size>> sector_bits, > + 4, 0x0000, 0x0000, 0x0000, 0x0000, 0); > + > + if (rom_size == 0) { > + pc_isa_bios_init(addr, size); > + } > +} > + > +static void pc_system_firmware_init(void) > +{ > + int flash_present, rom_present; > + int rom_size; > + DriveInfo *pflash_drv; > + > + pflash_drv = drive_get(IF_PFLASH, 0, 0); > + flash_present = (pflash_drv != NULL); > + > + /* Load rom if -bios is used or if -pflash is not used */ > + rom_present = ((bios_name != NULL) || !flash_present); > + > + /* If rom is present, then it is mapped just below 4GB */ > + if (rom_present) { > + rom_size = pc_system_rom_init(); > + } else { > + rom_size = 0; > + } > + > + /* If flash is present, then it is mapped just below the rom, or > + * just below 4GB when rom is not present. */ > + if (flash_present) { > + pc_system_flash_init(pflash_drv, rom_size); > + } > +} > + > +void pc_memory_init(const char *kernel_filename, > + const char *kernel_cmdline, > + const char *initrd_filename, > + ram_addr_t below_4g_mem_size, > + ram_addr_t above_4g_mem_size) > +{ > + int linux_boot, i; > + ram_addr_t ram_addr, option_rom_offset; > + void *fw_cfg; > + > + linux_boot = (kernel_filename != NULL); > + > + /* allocate RAM */ > + ram_addr = qemu_ram_alloc(NULL, "pc.ram", > + below_4g_mem_size + above_4g_mem_size); > + cpu_register_physical_memory(0, 0xa0000, ram_addr); > + cpu_register_physical_memory(0x100000, > + below_4g_mem_size - 0x100000, > + ram_addr + 0x100000); > + if (above_4g_mem_size> 0) { > + cpu_register_physical_memory(0x100000000ULL, above_4g_mem_size, > + ram_addr + below_4g_mem_size); > + } > + > + /* Initialize ROM or flash ranges for PC firmware */ > + pc_system_firmware_init(); > + > + option_rom_offset = qemu_ram_alloc(NULL, "pc.rom", PC_ROM_SIZE); > + cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE, option_rom_offset); > + > fw_cfg = bochs_bios_init(); > rom_set_fw(fw_cfg); >
On Sat, Jul 23, 2011 at 08:51, Anthony Liguori <anthony@codemonkey.ws> wrote: > On 07/08/2011 02:37 PM, Jordan Justen wrote: >> >> If -pflash is specified and -bios is specified then pflash will >> be mapped just below the system rom using hw/pflash_cfi01.c. >> >> If -pflash is specified on the command line, but -bios is >> not specified, then 'bios.bin' will NOT be loaded, and >> instead the -pflash flash image will be mapped just below >> 4GB in place of the normal rom image. > > This is way too tied to the pc platform to be this generic. > > I think a better approach would be to default to having unit=0 of IF_PFLASH > default to a read-only BDS that points to bios.bin. -bios would just be a > short cut to use a different file name but you should be able to override > with -drive too. > > And to really simplify things, you could add a readonly flag to -bios such > that you could do: > > -bios foo.img,readonly=off > > Which is what I think you're looking for semantically. There seemed to be some feedback on the list interested in preserving a read-only firmware, and just adding a flash region. So, for example, the firmware could be read from a common system location like is generally done with bios.bin today, and VM instance specific flash region could still be added. If the entire firmware is moved to a separate VM instance specific flash, then firmware update also gets complicated. It is no longer just a matter of updating the qemu firmware package in your distro's package management system. What about taking your idea, but adding a second drive that would be mapped just below the 1st if it is specified with -drive? Thanks, -Jordan > > Regards, > > Anthony Liguori > >> >> Signed-off-by: Jordan Justen<jordan.l.justen@intel.com> >> Reviewed-by: Aurelien Jarno<aurelien@aurel32.net> > > > >> --- >> default-configs/i386-softmmu.mak | 1 + >> default-configs/x86_64-softmmu.mak | 1 + >> hw/pc.c | 161 >> +++++++++++++++++++++++++++--------- >> 3 files changed, 125 insertions(+), 38 deletions(-) >> >> diff --git a/default-configs/i386-softmmu.mak >> b/default-configs/i386-softmmu.mak >> index 55589fa..8697cd4 100644 >> --- a/default-configs/i386-softmmu.mak >> +++ b/default-configs/i386-softmmu.mak >> @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y >> CONFIG_SOUND=y >> CONFIG_HPET=y >> CONFIG_APPLESMC=y >> +CONFIG_PFLASH_CFI01=y >> diff --git a/default-configs/x86_64-softmmu.mak >> b/default-configs/x86_64-softmmu.mak >> index 8895028..eca9284 100644 >> --- a/default-configs/x86_64-softmmu.mak >> +++ b/default-configs/x86_64-softmmu.mak >> @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y >> CONFIG_SOUND=y >> CONFIG_HPET=y >> CONFIG_APPLESMC=y >> +CONFIG_PFLASH_CFI01=y >> diff --git a/hw/pc.c b/hw/pc.c >> index a3e8539..e25354f 100644 >> --- a/hw/pc.c >> +++ b/hw/pc.c >> @@ -41,6 +41,7 @@ >> #include "sysemu.h" >> #include "blockdev.h" >> #include "ui/qemu-spice.h" >> +#include "flash.h" >> >> /* output Bochs bios info messages */ >> //#define DEBUG_BIOS >> @@ -957,70 +958,154 @@ void pc_cpus_init(const char *cpu_model) >> } >> } >> >> -void pc_memory_init(const char *kernel_filename, >> - const char *kernel_cmdline, >> - const char *initrd_filename, >> - ram_addr_t below_4g_mem_size, >> - ram_addr_t above_4g_mem_size) >> +static void pc_isa_bios_init(ram_addr_t ram_offset, int ram_size) >> { >> - char *filename; >> - int ret, linux_boot, i; >> - ram_addr_t ram_addr, bios_offset, option_rom_offset; >> - int bios_size, isa_bios_size; >> - void *fw_cfg; >> - >> - linux_boot = (kernel_filename != NULL); >> + int isa_bios_size; >> >> - /* allocate RAM */ >> - ram_addr = qemu_ram_alloc(NULL, "pc.ram", >> - below_4g_mem_size + above_4g_mem_size); >> - cpu_register_physical_memory(0, 0xa0000, ram_addr); >> - cpu_register_physical_memory(0x100000, >> - below_4g_mem_size - 0x100000, >> - ram_addr + 0x100000); >> - if (above_4g_mem_size> 0) { >> - cpu_register_physical_memory(0x100000000ULL, above_4g_mem_size, >> - ram_addr + below_4g_mem_size); >> + /* map the last 128KB of the BIOS in ISA space */ >> + isa_bios_size = ram_size; >> + if (isa_bios_size> (128 * 1024)) { >> + isa_bios_size = 128 * 1024; >> } >> + ram_offset = ram_offset + ram_size - isa_bios_size; >> + cpu_register_physical_memory(0x100000 - isa_bios_size, >> + isa_bios_size, >> + ram_offset | IO_MEM_ROM); >> +} >> + >> +static int pc_system_rom_init(void) >> +{ >> + int ret; >> + int bios_size; >> + ram_addr_t bios_offset; >> + char *filename; >> >> /* BIOS load */ >> - if (bios_name == NULL) >> + 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; >> + >> + if (bios_size<= 0 || (bios_size % 65536) != 0) { >> + ret = -1; >> + } else { >> + bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); >> + ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); >> } >> - bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); >> - 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) { >> qemu_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; >> - cpu_register_physical_memory(0x100000 - isa_bios_size, >> - isa_bios_size, >> - (bios_offset + bios_size - >> isa_bios_size) | IO_MEM_ROM); >> >> - option_rom_offset = qemu_ram_alloc(NULL, "pc.rom", PC_ROM_SIZE); >> - cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE, >> option_rom_offset); >> + pc_isa_bios_init(bios_offset, bios_size); >> >> /* map all the bios at the top of memory */ >> cpu_register_physical_memory((uint32_t)(-bios_size), >> bios_size, bios_offset | IO_MEM_ROM); >> >> + return bios_size; >> +} >> + >> +static void pc_system_flash_init(DriveInfo *pflash_drv, int rom_size) >> +{ >> + BlockDriverState *bdrv; >> + int64_t size; >> + target_phys_addr_t phys_addr; >> + ram_addr_t addr; >> + int sector_bits, sector_size; >> + >> + bdrv = NULL; >> + >> + 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: -pflash size must be a multiple of 0x%x\n", >> + sector_size); >> + exit(1); >> + } >> + >> + phys_addr = 0x100000000ULL - rom_size - size; >> + addr = qemu_ram_alloc(NULL, "system.flash", size); >> + DPRINTF("flash addr: 0x%lx\n", (int64_t)phys_addr); >> + pflash_cfi01_register(phys_addr, addr, bdrv, >> + sector_size, size>> sector_bits, >> + 4, 0x0000, 0x0000, 0x0000, 0x0000, 0); >> + >> + if (rom_size == 0) { >> + pc_isa_bios_init(addr, size); >> + } >> +} >> + >> +static void pc_system_firmware_init(void) >> +{ >> + int flash_present, rom_present; >> + int rom_size; >> + DriveInfo *pflash_drv; >> + >> + pflash_drv = drive_get(IF_PFLASH, 0, 0); >> + flash_present = (pflash_drv != NULL); >> + >> + /* Load rom if -bios is used or if -pflash is not used */ >> + rom_present = ((bios_name != NULL) || !flash_present); >> + >> + /* If rom is present, then it is mapped just below 4GB */ >> + if (rom_present) { >> + rom_size = pc_system_rom_init(); >> + } else { >> + rom_size = 0; >> + } >> + >> + /* If flash is present, then it is mapped just below the rom, or >> + * just below 4GB when rom is not present. */ >> + if (flash_present) { >> + pc_system_flash_init(pflash_drv, rom_size); >> + } >> +} >> + >> +void pc_memory_init(const char *kernel_filename, >> + const char *kernel_cmdline, >> + const char *initrd_filename, >> + ram_addr_t below_4g_mem_size, >> + ram_addr_t above_4g_mem_size) >> +{ >> + int linux_boot, i; >> + ram_addr_t ram_addr, option_rom_offset; >> + void *fw_cfg; >> + >> + linux_boot = (kernel_filename != NULL); >> + >> + /* allocate RAM */ >> + ram_addr = qemu_ram_alloc(NULL, "pc.ram", >> + below_4g_mem_size + above_4g_mem_size); >> + cpu_register_physical_memory(0, 0xa0000, ram_addr); >> + cpu_register_physical_memory(0x100000, >> + below_4g_mem_size - 0x100000, >> + ram_addr + 0x100000); >> + if (above_4g_mem_size> 0) { >> + cpu_register_physical_memory(0x100000000ULL, above_4g_mem_size, >> + ram_addr + below_4g_mem_size); >> + } >> + >> + /* Initialize ROM or flash ranges for PC firmware */ >> + pc_system_firmware_init(); >> + >> + option_rom_offset = qemu_ram_alloc(NULL, "pc.rom", PC_ROM_SIZE); >> + cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE, >> option_rom_offset); >> + >> fw_cfg = bochs_bios_init(); >> rom_set_fw(fw_cfg); >> > > >
On 07/23/2011 03:19 PM, Jordan Justen wrote: > On Sat, Jul 23, 2011 at 08:51, Anthony Liguori<anthony@codemonkey.ws> wrote: >> On 07/08/2011 02:37 PM, Jordan Justen wrote: >>> >>> If -pflash is specified and -bios is specified then pflash will >>> be mapped just below the system rom using hw/pflash_cfi01.c. >>> >>> If -pflash is specified on the command line, but -bios is >>> not specified, then 'bios.bin' will NOT be loaded, and >>> instead the -pflash flash image will be mapped just below >>> 4GB in place of the normal rom image. >> >> This is way too tied to the pc platform to be this generic. >> >> I think a better approach would be to default to having unit=0 of IF_PFLASH >> default to a read-only BDS that points to bios.bin. -bios would just be a >> short cut to use a different file name but you should be able to override >> with -drive too. >> >> And to really simplify things, you could add a readonly flag to -bios such >> that you could do: >> >> -bios foo.img,readonly=off >> >> Which is what I think you're looking for semantically. > > There seemed to be some feedback on the list interested in preserving > a read-only firmware, and just adding a flash region. > > So, for example, the firmware could be read from a common system > location like is generally done with bios.bin today, and VM instance > specific flash region could still be added. You can have multiple flash regions. You're introducing two modes. In one mode, we emulate a flash device and expose it for the BIOS ROM. In the second mode, we don't emulate a device but we expose the BIOS ROM based on a file in a shared read-only location. I'm suggesting always emulating a flash device, but by default make the device read-only and have it be loaded from a file in a shared read-only location. That means we have a single code path and a consistent view from a management tooling perspective. IOW, management tools will always see that there is a BIOS block device, and they need to worry about making sure that BIOS block device is there. > > If the entire firmware is moved to a separate VM instance specific > flash, then firmware update also gets complicated. It is no longer > just a matter of updating the qemu firmware package in your distro's > package management system. I think the bit your misunderstanding is that you should default the firmware to be created from a common file as a read-only device. Regards, Anthony Liguori > > What about taking your idea, but adding a second drive that would be > mapped just below the 1st if it is specified with -drive? > > Thanks, > > -Jordan > >> >> Regards, >> >> Anthony Liguori >> >>> >>> Signed-off-by: Jordan Justen<jordan.l.justen@intel.com> >>> Reviewed-by: Aurelien Jarno<aurelien@aurel32.net> >> >> >> >>> --- >>> default-configs/i386-softmmu.mak | 1 + >>> default-configs/x86_64-softmmu.mak | 1 + >>> hw/pc.c | 161 >>> +++++++++++++++++++++++++++--------- >>> 3 files changed, 125 insertions(+), 38 deletions(-) >>> >>> diff --git a/default-configs/i386-softmmu.mak >>> b/default-configs/i386-softmmu.mak >>> index 55589fa..8697cd4 100644 >>> --- a/default-configs/i386-softmmu.mak >>> +++ b/default-configs/i386-softmmu.mak >>> @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y >>> CONFIG_SOUND=y >>> CONFIG_HPET=y >>> CONFIG_APPLESMC=y >>> +CONFIG_PFLASH_CFI01=y >>> diff --git a/default-configs/x86_64-softmmu.mak >>> b/default-configs/x86_64-softmmu.mak >>> index 8895028..eca9284 100644 >>> --- a/default-configs/x86_64-softmmu.mak >>> +++ b/default-configs/x86_64-softmmu.mak >>> @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y >>> CONFIG_SOUND=y >>> CONFIG_HPET=y >>> CONFIG_APPLESMC=y >>> +CONFIG_PFLASH_CFI01=y >>> diff --git a/hw/pc.c b/hw/pc.c >>> index a3e8539..e25354f 100644 >>> --- a/hw/pc.c >>> +++ b/hw/pc.c >>> @@ -41,6 +41,7 @@ >>> #include "sysemu.h" >>> #include "blockdev.h" >>> #include "ui/qemu-spice.h" >>> +#include "flash.h" >>> >>> /* output Bochs bios info messages */ >>> //#define DEBUG_BIOS >>> @@ -957,70 +958,154 @@ void pc_cpus_init(const char *cpu_model) >>> } >>> } >>> >>> -void pc_memory_init(const char *kernel_filename, >>> - const char *kernel_cmdline, >>> - const char *initrd_filename, >>> - ram_addr_t below_4g_mem_size, >>> - ram_addr_t above_4g_mem_size) >>> +static void pc_isa_bios_init(ram_addr_t ram_offset, int ram_size) >>> { >>> - char *filename; >>> - int ret, linux_boot, i; >>> - ram_addr_t ram_addr, bios_offset, option_rom_offset; >>> - int bios_size, isa_bios_size; >>> - void *fw_cfg; >>> - >>> - linux_boot = (kernel_filename != NULL); >>> + int isa_bios_size; >>> >>> - /* allocate RAM */ >>> - ram_addr = qemu_ram_alloc(NULL, "pc.ram", >>> - below_4g_mem_size + above_4g_mem_size); >>> - cpu_register_physical_memory(0, 0xa0000, ram_addr); >>> - cpu_register_physical_memory(0x100000, >>> - below_4g_mem_size - 0x100000, >>> - ram_addr + 0x100000); >>> - if (above_4g_mem_size> 0) { >>> - cpu_register_physical_memory(0x100000000ULL, above_4g_mem_size, >>> - ram_addr + below_4g_mem_size); >>> + /* map the last 128KB of the BIOS in ISA space */ >>> + isa_bios_size = ram_size; >>> + if (isa_bios_size> (128 * 1024)) { >>> + isa_bios_size = 128 * 1024; >>> } >>> + ram_offset = ram_offset + ram_size - isa_bios_size; >>> + cpu_register_physical_memory(0x100000 - isa_bios_size, >>> + isa_bios_size, >>> + ram_offset | IO_MEM_ROM); >>> +} >>> + >>> +static int pc_system_rom_init(void) >>> +{ >>> + int ret; >>> + int bios_size; >>> + ram_addr_t bios_offset; >>> + char *filename; >>> >>> /* BIOS load */ >>> - if (bios_name == NULL) >>> + 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; >>> + >>> + if (bios_size<= 0 || (bios_size % 65536) != 0) { >>> + ret = -1; >>> + } else { >>> + bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); >>> + ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); >>> } >>> - bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); >>> - 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) { >>> qemu_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; >>> - cpu_register_physical_memory(0x100000 - isa_bios_size, >>> - isa_bios_size, >>> - (bios_offset + bios_size - >>> isa_bios_size) | IO_MEM_ROM); >>> >>> - option_rom_offset = qemu_ram_alloc(NULL, "pc.rom", PC_ROM_SIZE); >>> - cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE, >>> option_rom_offset); >>> + pc_isa_bios_init(bios_offset, bios_size); >>> >>> /* map all the bios at the top of memory */ >>> cpu_register_physical_memory((uint32_t)(-bios_size), >>> bios_size, bios_offset | IO_MEM_ROM); >>> >>> + return bios_size; >>> +} >>> + >>> +static void pc_system_flash_init(DriveInfo *pflash_drv, int rom_size) >>> +{ >>> + BlockDriverState *bdrv; >>> + int64_t size; >>> + target_phys_addr_t phys_addr; >>> + ram_addr_t addr; >>> + int sector_bits, sector_size; >>> + >>> + bdrv = NULL; >>> + >>> + 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: -pflash size must be a multiple of 0x%x\n", >>> + sector_size); >>> + exit(1); >>> + } >>> + >>> + phys_addr = 0x100000000ULL - rom_size - size; >>> + addr = qemu_ram_alloc(NULL, "system.flash", size); >>> + DPRINTF("flash addr: 0x%lx\n", (int64_t)phys_addr); >>> + pflash_cfi01_register(phys_addr, addr, bdrv, >>> + sector_size, size>> sector_bits, >>> + 4, 0x0000, 0x0000, 0x0000, 0x0000, 0); >>> + >>> + if (rom_size == 0) { >>> + pc_isa_bios_init(addr, size); >>> + } >>> +} >>> + >>> +static void pc_system_firmware_init(void) >>> +{ >>> + int flash_present, rom_present; >>> + int rom_size; >>> + DriveInfo *pflash_drv; >>> + >>> + pflash_drv = drive_get(IF_PFLASH, 0, 0); >>> + flash_present = (pflash_drv != NULL); >>> + >>> + /* Load rom if -bios is used or if -pflash is not used */ >>> + rom_present = ((bios_name != NULL) || !flash_present); >>> + >>> + /* If rom is present, then it is mapped just below 4GB */ >>> + if (rom_present) { >>> + rom_size = pc_system_rom_init(); >>> + } else { >>> + rom_size = 0; >>> + } >>> + >>> + /* If flash is present, then it is mapped just below the rom, or >>> + * just below 4GB when rom is not present. */ >>> + if (flash_present) { >>> + pc_system_flash_init(pflash_drv, rom_size); >>> + } >>> +} >>> + >>> +void pc_memory_init(const char *kernel_filename, >>> + const char *kernel_cmdline, >>> + const char *initrd_filename, >>> + ram_addr_t below_4g_mem_size, >>> + ram_addr_t above_4g_mem_size) >>> +{ >>> + int linux_boot, i; >>> + ram_addr_t ram_addr, option_rom_offset; >>> + void *fw_cfg; >>> + >>> + linux_boot = (kernel_filename != NULL); >>> + >>> + /* allocate RAM */ >>> + ram_addr = qemu_ram_alloc(NULL, "pc.ram", >>> + below_4g_mem_size + above_4g_mem_size); >>> + cpu_register_physical_memory(0, 0xa0000, ram_addr); >>> + cpu_register_physical_memory(0x100000, >>> + below_4g_mem_size - 0x100000, >>> + ram_addr + 0x100000); >>> + if (above_4g_mem_size> 0) { >>> + cpu_register_physical_memory(0x100000000ULL, above_4g_mem_size, >>> + ram_addr + below_4g_mem_size); >>> + } >>> + >>> + /* Initialize ROM or flash ranges for PC firmware */ >>> + pc_system_firmware_init(); >>> + >>> + option_rom_offset = qemu_ram_alloc(NULL, "pc.rom", PC_ROM_SIZE); >>> + cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE, >>> option_rom_offset); >>> + >>> fw_cfg = bochs_bios_init(); >>> rom_set_fw(fw_cfg); >>> >> >> >> >
On Sat, Jul 23, 2011 at 14:25, Anthony Liguori <anthony@codemonkey.ws> wrote: > On 07/23/2011 03:19 PM, Jordan Justen wrote: >> >> On Sat, Jul 23, 2011 at 08:51, Anthony Liguori<anthony@codemonkey.ws> >> wrote: >>> >>> On 07/08/2011 02:37 PM, Jordan Justen wrote: >>>> >>>> If -pflash is specified and -bios is specified then pflash will >>>> be mapped just below the system rom using hw/pflash_cfi01.c. >>>> >>>> If -pflash is specified on the command line, but -bios is >>>> not specified, then 'bios.bin' will NOT be loaded, and >>>> instead the -pflash flash image will be mapped just below >>>> 4GB in place of the normal rom image. >>> >>> This is way too tied to the pc platform to be this generic. >>> >>> I think a better approach would be to default to having unit=0 of >>> IF_PFLASH >>> default to a read-only BDS that points to bios.bin. -bios would just be >>> a >>> short cut to use a different file name but you should be able to override >>> with -drive too. >>> >>> And to really simplify things, you could add a readonly flag to -bios >>> such >>> that you could do: >>> >>> -bios foo.img,readonly=off >>> >>> Which is what I think you're looking for semantically. >> >> There seemed to be some feedback on the list interested in preserving >> a read-only firmware, and just adding a flash region. >> >> So, for example, the firmware could be read from a common system >> location like is generally done with bios.bin today, and VM instance >> specific flash region could still be added. > > You can have multiple flash regions. So, is your recommendation that we support N pflash images in x86/x86-64? Instance/index 0 is mapped just under 4GB, and the rest follow below this? This seems like a good plan, although I can't see a usage for more than 2 instances. -Jordan > You're introducing two modes. In one mode, we emulate a flash device and > expose it for the BIOS ROM. In the second mode, we don't emulate a device > but we expose the BIOS ROM based on a file in a shared read-only location. > > I'm suggesting always emulating a flash device, but by default make the > device read-only and have it be loaded from a file in a shared read-only > location. > > That means we have a single code path and a consistent view from a > management tooling perspective. IOW, management tools will always see that > there is a BIOS block device, and they need to worry about making sure that > BIOS block device is there. > >> >> If the entire firmware is moved to a separate VM instance specific >> flash, then firmware update also gets complicated. It is no longer >> just a matter of updating the qemu firmware package in your distro's >> package management system. > > I think the bit your misunderstanding is that you should default the > firmware to be created from a common file as a read-only device. > > Regards, > > Anthony Liguori > >> >> What about taking your idea, but adding a second drive that would be >> mapped just below the 1st if it is specified with -drive? >> >> Thanks, >> >> -Jordan >> >>> >>> Regards, >>> >>> Anthony Liguori >>> >>>> >>>> Signed-off-by: Jordan Justen<jordan.l.justen@intel.com> >>>> Reviewed-by: Aurelien Jarno<aurelien@aurel32.net> >>> >>> >>> >>>> --- >>>> default-configs/i386-softmmu.mak | 1 + >>>> default-configs/x86_64-softmmu.mak | 1 + >>>> hw/pc.c | 161 >>>> +++++++++++++++++++++++++++--------- >>>> 3 files changed, 125 insertions(+), 38 deletions(-) >>>> >>>> diff --git a/default-configs/i386-softmmu.mak >>>> b/default-configs/i386-softmmu.mak >>>> index 55589fa..8697cd4 100644 >>>> --- a/default-configs/i386-softmmu.mak >>>> +++ b/default-configs/i386-softmmu.mak >>>> @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y >>>> CONFIG_SOUND=y >>>> CONFIG_HPET=y >>>> CONFIG_APPLESMC=y >>>> +CONFIG_PFLASH_CFI01=y >>>> diff --git a/default-configs/x86_64-softmmu.mak >>>> b/default-configs/x86_64-softmmu.mak >>>> index 8895028..eca9284 100644 >>>> --- a/default-configs/x86_64-softmmu.mak >>>> +++ b/default-configs/x86_64-softmmu.mak >>>> @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y >>>> CONFIG_SOUND=y >>>> CONFIG_HPET=y >>>> CONFIG_APPLESMC=y >>>> +CONFIG_PFLASH_CFI01=y >>>> diff --git a/hw/pc.c b/hw/pc.c >>>> index a3e8539..e25354f 100644 >>>> --- a/hw/pc.c >>>> +++ b/hw/pc.c >>>> @@ -41,6 +41,7 @@ >>>> #include "sysemu.h" >>>> #include "blockdev.h" >>>> #include "ui/qemu-spice.h" >>>> +#include "flash.h" >>>> >>>> /* output Bochs bios info messages */ >>>> //#define DEBUG_BIOS >>>> @@ -957,70 +958,154 @@ void pc_cpus_init(const char *cpu_model) >>>> } >>>> } >>>> >>>> -void pc_memory_init(const char *kernel_filename, >>>> - const char *kernel_cmdline, >>>> - const char *initrd_filename, >>>> - ram_addr_t below_4g_mem_size, >>>> - ram_addr_t above_4g_mem_size) >>>> +static void pc_isa_bios_init(ram_addr_t ram_offset, int ram_size) >>>> { >>>> - char *filename; >>>> - int ret, linux_boot, i; >>>> - ram_addr_t ram_addr, bios_offset, option_rom_offset; >>>> - int bios_size, isa_bios_size; >>>> - void *fw_cfg; >>>> - >>>> - linux_boot = (kernel_filename != NULL); >>>> + int isa_bios_size; >>>> >>>> - /* allocate RAM */ >>>> - ram_addr = qemu_ram_alloc(NULL, "pc.ram", >>>> - below_4g_mem_size + above_4g_mem_size); >>>> - cpu_register_physical_memory(0, 0xa0000, ram_addr); >>>> - cpu_register_physical_memory(0x100000, >>>> - below_4g_mem_size - 0x100000, >>>> - ram_addr + 0x100000); >>>> - if (above_4g_mem_size> 0) { >>>> - cpu_register_physical_memory(0x100000000ULL, above_4g_mem_size, >>>> - ram_addr + below_4g_mem_size); >>>> + /* map the last 128KB of the BIOS in ISA space */ >>>> + isa_bios_size = ram_size; >>>> + if (isa_bios_size> (128 * 1024)) { >>>> + isa_bios_size = 128 * 1024; >>>> } >>>> + ram_offset = ram_offset + ram_size - isa_bios_size; >>>> + cpu_register_physical_memory(0x100000 - isa_bios_size, >>>> + isa_bios_size, >>>> + ram_offset | IO_MEM_ROM); >>>> +} >>>> + >>>> +static int pc_system_rom_init(void) >>>> +{ >>>> + int ret; >>>> + int bios_size; >>>> + ram_addr_t bios_offset; >>>> + char *filename; >>>> >>>> /* BIOS load */ >>>> - if (bios_name == NULL) >>>> + 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; >>>> + >>>> + if (bios_size<= 0 || (bios_size % 65536) != 0) { >>>> + ret = -1; >>>> + } else { >>>> + bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); >>>> + ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), >>>> -1); >>>> } >>>> - bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); >>>> - 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) { >>>> qemu_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; >>>> - cpu_register_physical_memory(0x100000 - isa_bios_size, >>>> - isa_bios_size, >>>> - (bios_offset + bios_size - >>>> isa_bios_size) | IO_MEM_ROM); >>>> >>>> - option_rom_offset = qemu_ram_alloc(NULL, "pc.rom", PC_ROM_SIZE); >>>> - cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE, >>>> option_rom_offset); >>>> + pc_isa_bios_init(bios_offset, bios_size); >>>> >>>> /* map all the bios at the top of memory */ >>>> cpu_register_physical_memory((uint32_t)(-bios_size), >>>> bios_size, bios_offset | IO_MEM_ROM); >>>> >>>> + return bios_size; >>>> +} >>>> + >>>> +static void pc_system_flash_init(DriveInfo *pflash_drv, int rom_size) >>>> +{ >>>> + BlockDriverState *bdrv; >>>> + int64_t size; >>>> + target_phys_addr_t phys_addr; >>>> + ram_addr_t addr; >>>> + int sector_bits, sector_size; >>>> + >>>> + bdrv = NULL; >>>> + >>>> + 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: -pflash size must be a multiple of 0x%x\n", >>>> + sector_size); >>>> + exit(1); >>>> + } >>>> + >>>> + phys_addr = 0x100000000ULL - rom_size - size; >>>> + addr = qemu_ram_alloc(NULL, "system.flash", size); >>>> + DPRINTF("flash addr: 0x%lx\n", (int64_t)phys_addr); >>>> + pflash_cfi01_register(phys_addr, addr, bdrv, >>>> + sector_size, size>> sector_bits, >>>> + 4, 0x0000, 0x0000, 0x0000, 0x0000, 0); >>>> + >>>> + if (rom_size == 0) { >>>> + pc_isa_bios_init(addr, size); >>>> + } >>>> +} >>>> + >>>> +static void pc_system_firmware_init(void) >>>> +{ >>>> + int flash_present, rom_present; >>>> + int rom_size; >>>> + DriveInfo *pflash_drv; >>>> + >>>> + pflash_drv = drive_get(IF_PFLASH, 0, 0); >>>> + flash_present = (pflash_drv != NULL); >>>> + >>>> + /* Load rom if -bios is used or if -pflash is not used */ >>>> + rom_present = ((bios_name != NULL) || !flash_present); >>>> + >>>> + /* If rom is present, then it is mapped just below 4GB */ >>>> + if (rom_present) { >>>> + rom_size = pc_system_rom_init(); >>>> + } else { >>>> + rom_size = 0; >>>> + } >>>> + >>>> + /* If flash is present, then it is mapped just below the rom, or >>>> + * just below 4GB when rom is not present. */ >>>> + if (flash_present) { >>>> + pc_system_flash_init(pflash_drv, rom_size); >>>> + } >>>> +} >>>> + >>>> +void pc_memory_init(const char *kernel_filename, >>>> + const char *kernel_cmdline, >>>> + const char *initrd_filename, >>>> + ram_addr_t below_4g_mem_size, >>>> + ram_addr_t above_4g_mem_size) >>>> +{ >>>> + int linux_boot, i; >>>> + ram_addr_t ram_addr, option_rom_offset; >>>> + void *fw_cfg; >>>> + >>>> + linux_boot = (kernel_filename != NULL); >>>> + >>>> + /* allocate RAM */ >>>> + ram_addr = qemu_ram_alloc(NULL, "pc.ram", >>>> + below_4g_mem_size + above_4g_mem_size); >>>> + cpu_register_physical_memory(0, 0xa0000, ram_addr); >>>> + cpu_register_physical_memory(0x100000, >>>> + below_4g_mem_size - 0x100000, >>>> + ram_addr + 0x100000); >>>> + if (above_4g_mem_size> 0) { >>>> + cpu_register_physical_memory(0x100000000ULL, above_4g_mem_size, >>>> + ram_addr + below_4g_mem_size); >>>> + } >>>> + >>>> + /* Initialize ROM or flash ranges for PC firmware */ >>>> + pc_system_firmware_init(); >>>> + >>>> + option_rom_offset = qemu_ram_alloc(NULL, "pc.rom", PC_ROM_SIZE); >>>> + cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE, >>>> option_rom_offset); >>>> + >>>> fw_cfg = bochs_bios_init(); >>>> rom_set_fw(fw_cfg); >>>> >>> >>> >>> >> > >
On 07/23/2011 05:06 PM, Jordan Justen wrote: > On Sat, Jul 23, 2011 at 14:25, Anthony Liguori<anthony@codemonkey.ws> wrote: >> On 07/23/2011 03:19 PM, Jordan Justen wrote: >>> >>> On Sat, Jul 23, 2011 at 08:51, Anthony Liguori<anthony@codemonkey.ws> >>> wrote: >>>> >>>> On 07/08/2011 02:37 PM, Jordan Justen wrote: >>>>> >>>>> If -pflash is specified and -bios is specified then pflash will >>>>> be mapped just below the system rom using hw/pflash_cfi01.c. >>>>> >>>>> If -pflash is specified on the command line, but -bios is >>>>> not specified, then 'bios.bin' will NOT be loaded, and >>>>> instead the -pflash flash image will be mapped just below >>>>> 4GB in place of the normal rom image. >>>> >>>> This is way too tied to the pc platform to be this generic. >>>> >>>> I think a better approach would be to default to having unit=0 of >>>> IF_PFLASH >>>> default to a read-only BDS that points to bios.bin. -bios would just be >>>> a >>>> short cut to use a different file name but you should be able to override >>>> with -drive too. >>>> >>>> And to really simplify things, you could add a readonly flag to -bios >>>> such >>>> that you could do: >>>> >>>> -bios foo.img,readonly=off >>>> >>>> Which is what I think you're looking for semantically. >>> >>> There seemed to be some feedback on the list interested in preserving >>> a read-only firmware, and just adding a flash region. >>> >>> So, for example, the firmware could be read from a common system >>> location like is generally done with bios.bin today, and VM instance >>> specific flash region could still be added. >> >> You can have multiple flash regions. > > So, is your recommendation that we support N pflash images in > x86/x86-64? Instance/index 0 is mapped just under 4GB, and the rest > follow below this? No. There should be a flash device, pflash index 0 is fine, but it should be mapped under 4GB and also in the legacy BIOS space. This is the PC firmware flash. By default it should be read-only and it should be created by using ${prefix}/share/bios.bin. But it should be possible to override both the filename and the read-only flag. In terms of other flash devices, I don't think it's that simple. Flash is tied to the mobo layout so I don't think index > 0 really makes sense unless you allow a specific mapping address. I doubt that's terribly useful. Regards, Anthony Liguori > > This seems like a good plan, although I can't see a usage for more > than 2 instances. > > -Jordan > >> You're introducing two modes. In one mode, we emulate a flash device and >> expose it for the BIOS ROM. In the second mode, we don't emulate a device >> but we expose the BIOS ROM based on a file in a shared read-only location. >> >> I'm suggesting always emulating a flash device, but by default make the >> device read-only and have it be loaded from a file in a shared read-only >> location. >> >> That means we have a single code path and a consistent view from a >> management tooling perspective. IOW, management tools will always see that >> there is a BIOS block device, and they need to worry about making sure that >> BIOS block device is there. >> >>> >>> If the entire firmware is moved to a separate VM instance specific >>> flash, then firmware update also gets complicated. It is no longer >>> just a matter of updating the qemu firmware package in your distro's >>> package management system. >> >> I think the bit your misunderstanding is that you should default the >> firmware to be created from a common file as a read-only device. >> >> Regards, >> >> Anthony Liguori >> >>> >>> What about taking your idea, but adding a second drive that would be >>> mapped just below the 1st if it is specified with -drive? >>> >>> Thanks, >>> >>> -Jordan >>> >>>> >>>> Regards, >>>> >>>> Anthony Liguori >>>> >>>>> >>>>> Signed-off-by: Jordan Justen<jordan.l.justen@intel.com> >>>>> Reviewed-by: Aurelien Jarno<aurelien@aurel32.net> >>>> >>>> >>>> >>>>> --- >>>>> default-configs/i386-softmmu.mak | 1 + >>>>> default-configs/x86_64-softmmu.mak | 1 + >>>>> hw/pc.c | 161 >>>>> +++++++++++++++++++++++++++--------- >>>>> 3 files changed, 125 insertions(+), 38 deletions(-) >>>>> >>>>> diff --git a/default-configs/i386-softmmu.mak >>>>> b/default-configs/i386-softmmu.mak >>>>> index 55589fa..8697cd4 100644 >>>>> --- a/default-configs/i386-softmmu.mak >>>>> +++ b/default-configs/i386-softmmu.mak >>>>> @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y >>>>> CONFIG_SOUND=y >>>>> CONFIG_HPET=y >>>>> CONFIG_APPLESMC=y >>>>> +CONFIG_PFLASH_CFI01=y >>>>> diff --git a/default-configs/x86_64-softmmu.mak >>>>> b/default-configs/x86_64-softmmu.mak >>>>> index 8895028..eca9284 100644 >>>>> --- a/default-configs/x86_64-softmmu.mak >>>>> +++ b/default-configs/x86_64-softmmu.mak >>>>> @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y >>>>> CONFIG_SOUND=y >>>>> CONFIG_HPET=y >>>>> CONFIG_APPLESMC=y >>>>> +CONFIG_PFLASH_CFI01=y >>>>> diff --git a/hw/pc.c b/hw/pc.c >>>>> index a3e8539..e25354f 100644 >>>>> --- a/hw/pc.c >>>>> +++ b/hw/pc.c >>>>> @@ -41,6 +41,7 @@ >>>>> #include "sysemu.h" >>>>> #include "blockdev.h" >>>>> #include "ui/qemu-spice.h" >>>>> +#include "flash.h" >>>>> >>>>> /* output Bochs bios info messages */ >>>>> //#define DEBUG_BIOS >>>>> @@ -957,70 +958,154 @@ void pc_cpus_init(const char *cpu_model) >>>>> } >>>>> } >>>>> >>>>> -void pc_memory_init(const char *kernel_filename, >>>>> - const char *kernel_cmdline, >>>>> - const char *initrd_filename, >>>>> - ram_addr_t below_4g_mem_size, >>>>> - ram_addr_t above_4g_mem_size) >>>>> +static void pc_isa_bios_init(ram_addr_t ram_offset, int ram_size) >>>>> { >>>>> - char *filename; >>>>> - int ret, linux_boot, i; >>>>> - ram_addr_t ram_addr, bios_offset, option_rom_offset; >>>>> - int bios_size, isa_bios_size; >>>>> - void *fw_cfg; >>>>> - >>>>> - linux_boot = (kernel_filename != NULL); >>>>> + int isa_bios_size; >>>>> >>>>> - /* allocate RAM */ >>>>> - ram_addr = qemu_ram_alloc(NULL, "pc.ram", >>>>> - below_4g_mem_size + above_4g_mem_size); >>>>> - cpu_register_physical_memory(0, 0xa0000, ram_addr); >>>>> - cpu_register_physical_memory(0x100000, >>>>> - below_4g_mem_size - 0x100000, >>>>> - ram_addr + 0x100000); >>>>> - if (above_4g_mem_size> 0) { >>>>> - cpu_register_physical_memory(0x100000000ULL, above_4g_mem_size, >>>>> - ram_addr + below_4g_mem_size); >>>>> + /* map the last 128KB of the BIOS in ISA space */ >>>>> + isa_bios_size = ram_size; >>>>> + if (isa_bios_size> (128 * 1024)) { >>>>> + isa_bios_size = 128 * 1024; >>>>> } >>>>> + ram_offset = ram_offset + ram_size - isa_bios_size; >>>>> + cpu_register_physical_memory(0x100000 - isa_bios_size, >>>>> + isa_bios_size, >>>>> + ram_offset | IO_MEM_ROM); >>>>> +} >>>>> + >>>>> +static int pc_system_rom_init(void) >>>>> +{ >>>>> + int ret; >>>>> + int bios_size; >>>>> + ram_addr_t bios_offset; >>>>> + char *filename; >>>>> >>>>> /* BIOS load */ >>>>> - if (bios_name == NULL) >>>>> + 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; >>>>> + >>>>> + if (bios_size<= 0 || (bios_size % 65536) != 0) { >>>>> + ret = -1; >>>>> + } else { >>>>> + bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); >>>>> + ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), >>>>> -1); >>>>> } >>>>> - bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); >>>>> - 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) { >>>>> qemu_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; >>>>> - cpu_register_physical_memory(0x100000 - isa_bios_size, >>>>> - isa_bios_size, >>>>> - (bios_offset + bios_size - >>>>> isa_bios_size) | IO_MEM_ROM); >>>>> >>>>> - option_rom_offset = qemu_ram_alloc(NULL, "pc.rom", PC_ROM_SIZE); >>>>> - cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE, >>>>> option_rom_offset); >>>>> + pc_isa_bios_init(bios_offset, bios_size); >>>>> >>>>> /* map all the bios at the top of memory */ >>>>> cpu_register_physical_memory((uint32_t)(-bios_size), >>>>> bios_size, bios_offset | IO_MEM_ROM); >>>>> >>>>> + return bios_size; >>>>> +} >>>>> + >>>>> +static void pc_system_flash_init(DriveInfo *pflash_drv, int rom_size) >>>>> +{ >>>>> + BlockDriverState *bdrv; >>>>> + int64_t size; >>>>> + target_phys_addr_t phys_addr; >>>>> + ram_addr_t addr; >>>>> + int sector_bits, sector_size; >>>>> + >>>>> + bdrv = NULL; >>>>> + >>>>> + 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: -pflash size must be a multiple of 0x%x\n", >>>>> + sector_size); >>>>> + exit(1); >>>>> + } >>>>> + >>>>> + phys_addr = 0x100000000ULL - rom_size - size; >>>>> + addr = qemu_ram_alloc(NULL, "system.flash", size); >>>>> + DPRINTF("flash addr: 0x%lx\n", (int64_t)phys_addr); >>>>> + pflash_cfi01_register(phys_addr, addr, bdrv, >>>>> + sector_size, size>> sector_bits, >>>>> + 4, 0x0000, 0x0000, 0x0000, 0x0000, 0); >>>>> + >>>>> + if (rom_size == 0) { >>>>> + pc_isa_bios_init(addr, size); >>>>> + } >>>>> +} >>>>> + >>>>> +static void pc_system_firmware_init(void) >>>>> +{ >>>>> + int flash_present, rom_present; >>>>> + int rom_size; >>>>> + DriveInfo *pflash_drv; >>>>> + >>>>> + pflash_drv = drive_get(IF_PFLASH, 0, 0); >>>>> + flash_present = (pflash_drv != NULL); >>>>> + >>>>> + /* Load rom if -bios is used or if -pflash is not used */ >>>>> + rom_present = ((bios_name != NULL) || !flash_present); >>>>> + >>>>> + /* If rom is present, then it is mapped just below 4GB */ >>>>> + if (rom_present) { >>>>> + rom_size = pc_system_rom_init(); >>>>> + } else { >>>>> + rom_size = 0; >>>>> + } >>>>> + >>>>> + /* If flash is present, then it is mapped just below the rom, or >>>>> + * just below 4GB when rom is not present. */ >>>>> + if (flash_present) { >>>>> + pc_system_flash_init(pflash_drv, rom_size); >>>>> + } >>>>> +} >>>>> + >>>>> +void pc_memory_init(const char *kernel_filename, >>>>> + const char *kernel_cmdline, >>>>> + const char *initrd_filename, >>>>> + ram_addr_t below_4g_mem_size, >>>>> + ram_addr_t above_4g_mem_size) >>>>> +{ >>>>> + int linux_boot, i; >>>>> + ram_addr_t ram_addr, option_rom_offset; >>>>> + void *fw_cfg; >>>>> + >>>>> + linux_boot = (kernel_filename != NULL); >>>>> + >>>>> + /* allocate RAM */ >>>>> + ram_addr = qemu_ram_alloc(NULL, "pc.ram", >>>>> + below_4g_mem_size + above_4g_mem_size); >>>>> + cpu_register_physical_memory(0, 0xa0000, ram_addr); >>>>> + cpu_register_physical_memory(0x100000, >>>>> + below_4g_mem_size - 0x100000, >>>>> + ram_addr + 0x100000); >>>>> + if (above_4g_mem_size> 0) { >>>>> + cpu_register_physical_memory(0x100000000ULL, above_4g_mem_size, >>>>> + ram_addr + below_4g_mem_size); >>>>> + } >>>>> + >>>>> + /* Initialize ROM or flash ranges for PC firmware */ >>>>> + pc_system_firmware_init(); >>>>> + >>>>> + option_rom_offset = qemu_ram_alloc(NULL, "pc.rom", PC_ROM_SIZE); >>>>> + cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE, >>>>> option_rom_offset); >>>>> + >>>>> fw_cfg = bochs_bios_init(); >>>>> rom_set_fw(fw_cfg); >>>>> >>>> >>>> >>>> >>> >> >> >
On Sat, Jul 23, 2011 at 15:26, Anthony Liguori <anthony@codemonkey.ws> wrote: > On 07/23/2011 05:06 PM, Jordan Justen wrote: >> >> On Sat, Jul 23, 2011 at 14:25, Anthony Liguori<anthony@codemonkey.ws> >> wrote: >>> >>> On 07/23/2011 03:19 PM, Jordan Justen wrote: >>>> >>>> On Sat, Jul 23, 2011 at 08:51, Anthony Liguori<anthony@codemonkey.ws> >>>> wrote: >>>>> >>>>> On 07/08/2011 02:37 PM, Jordan Justen wrote: >>>>>> >>>>>> If -pflash is specified and -bios is specified then pflash will >>>>>> be mapped just below the system rom using hw/pflash_cfi01.c. >>>>>> >>>>>> If -pflash is specified on the command line, but -bios is >>>>>> not specified, then 'bios.bin' will NOT be loaded, and >>>>>> instead the -pflash flash image will be mapped just below >>>>>> 4GB in place of the normal rom image. >>>>> >>>>> This is way too tied to the pc platform to be this generic. >>>>> >>>>> I think a better approach would be to default to having unit=0 of >>>>> IF_PFLASH >>>>> default to a read-only BDS that points to bios.bin. -bios would just >>>>> be >>>>> a >>>>> short cut to use a different file name but you should be able to >>>>> override >>>>> with -drive too. >>>>> >>>>> And to really simplify things, you could add a readonly flag to -bios >>>>> such >>>>> that you could do: >>>>> >>>>> -bios foo.img,readonly=off >>>>> >>>>> Which is what I think you're looking for semantically. >>>> >>>> There seemed to be some feedback on the list interested in preserving >>>> a read-only firmware, and just adding a flash region. >>>> >>>> So, for example, the firmware could be read from a common system >>>> location like is generally done with bios.bin today, and VM instance >>>> specific flash region could still be added. >>> >>> You can have multiple flash regions. >> >> So, is your recommendation that we support N pflash images in >> x86/x86-64? Instance/index 0 is mapped just under 4GB, and the rest >> follow below this? > > No. There should be a flash device, pflash index 0 is fine, but it should > be mapped under 4GB and also in the legacy BIOS space. > > This is the PC firmware flash. By default it should be read-only and it > should be created by using ${prefix}/share/bios.bin. But it should be > possible to override both the filename and the read-only flag. > > In terms of other flash devices, I don't think it's that simple. Flash is > tied to the mobo layout so I don't think index > 0 really makes sense unless > you allow a specific mapping address. I doubt that's terribly useful. I think VM's have a different situation than real hardware. I'm not sure an all ROM or all flash decision will work well for qemu. In most cases it may work better to make a ROM image available just below 4GB, and add a flash image below this ROM. This allows the qemu's firmware to be updated as usual in ${prefix}/share/bios.bin, but still allows a flash memory to be available below this. (The flash below the ROM could be used only for storing UEFI variables.) Otherwise when a new qemu is released along with a new firmware image, the VM instance using writable flash will continue to use the old firmware image. -Jordan > > Regards, > > Anthony Liguori > >> >> This seems like a good plan, although I can't see a usage for more >> than 2 instances. >> >> -Jordan >> >>> You're introducing two modes. In one mode, we emulate a flash device and >>> expose it for the BIOS ROM. In the second mode, we don't emulate a >>> device >>> but we expose the BIOS ROM based on a file in a shared read-only >>> location. >>> >>> I'm suggesting always emulating a flash device, but by default make the >>> device read-only and have it be loaded from a file in a shared read-only >>> location. >>> >>> That means we have a single code path and a consistent view from a >>> management tooling perspective. IOW, management tools will always see >>> that >>> there is a BIOS block device, and they need to worry about making sure >>> that >>> BIOS block device is there. >>> >>>> >>>> If the entire firmware is moved to a separate VM instance specific >>>> flash, then firmware update also gets complicated. It is no longer >>>> just a matter of updating the qemu firmware package in your distro's >>>> package management system. >>> >>> I think the bit your misunderstanding is that you should default the >>> firmware to be created from a common file as a read-only device. >>> >>> Regards, >>> >>> Anthony Liguori >>> >>>> >>>> What about taking your idea, but adding a second drive that would be >>>> mapped just below the 1st if it is specified with -drive? >>>> >>>> Thanks, >>>> >>>> -Jordan >>>> >>>>> >>>>> Regards, >>>>> >>>>> Anthony Liguori >>>>> >>>>>> >>>>>> Signed-off-by: Jordan Justen<jordan.l.justen@intel.com> >>>>>> Reviewed-by: Aurelien Jarno<aurelien@aurel32.net> >>>>> >>>>> >>>>> >>>>>> --- >>>>>> default-configs/i386-softmmu.mak | 1 + >>>>>> default-configs/x86_64-softmmu.mak | 1 + >>>>>> hw/pc.c | 161 >>>>>> +++++++++++++++++++++++++++--------- >>>>>> 3 files changed, 125 insertions(+), 38 deletions(-) >>>>>> >>>>>> diff --git a/default-configs/i386-softmmu.mak >>>>>> b/default-configs/i386-softmmu.mak >>>>>> index 55589fa..8697cd4 100644 >>>>>> --- a/default-configs/i386-softmmu.mak >>>>>> +++ b/default-configs/i386-softmmu.mak >>>>>> @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y >>>>>> CONFIG_SOUND=y >>>>>> CONFIG_HPET=y >>>>>> CONFIG_APPLESMC=y >>>>>> +CONFIG_PFLASH_CFI01=y >>>>>> diff --git a/default-configs/x86_64-softmmu.mak >>>>>> b/default-configs/x86_64-softmmu.mak >>>>>> index 8895028..eca9284 100644 >>>>>> --- a/default-configs/x86_64-softmmu.mak >>>>>> +++ b/default-configs/x86_64-softmmu.mak >>>>>> @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y >>>>>> CONFIG_SOUND=y >>>>>> CONFIG_HPET=y >>>>>> CONFIG_APPLESMC=y >>>>>> +CONFIG_PFLASH_CFI01=y >>>>>> diff --git a/hw/pc.c b/hw/pc.c >>>>>> index a3e8539..e25354f 100644 >>>>>> --- a/hw/pc.c >>>>>> +++ b/hw/pc.c >>>>>> @@ -41,6 +41,7 @@ >>>>>> #include "sysemu.h" >>>>>> #include "blockdev.h" >>>>>> #include "ui/qemu-spice.h" >>>>>> +#include "flash.h" >>>>>> >>>>>> /* output Bochs bios info messages */ >>>>>> //#define DEBUG_BIOS >>>>>> @@ -957,70 +958,154 @@ void pc_cpus_init(const char *cpu_model) >>>>>> } >>>>>> } >>>>>> >>>>>> -void pc_memory_init(const char *kernel_filename, >>>>>> - const char *kernel_cmdline, >>>>>> - const char *initrd_filename, >>>>>> - ram_addr_t below_4g_mem_size, >>>>>> - ram_addr_t above_4g_mem_size) >>>>>> +static void pc_isa_bios_init(ram_addr_t ram_offset, int ram_size) >>>>>> { >>>>>> - char *filename; >>>>>> - int ret, linux_boot, i; >>>>>> - ram_addr_t ram_addr, bios_offset, option_rom_offset; >>>>>> - int bios_size, isa_bios_size; >>>>>> - void *fw_cfg; >>>>>> - >>>>>> - linux_boot = (kernel_filename != NULL); >>>>>> + int isa_bios_size; >>>>>> >>>>>> - /* allocate RAM */ >>>>>> - ram_addr = qemu_ram_alloc(NULL, "pc.ram", >>>>>> - below_4g_mem_size + above_4g_mem_size); >>>>>> - cpu_register_physical_memory(0, 0xa0000, ram_addr); >>>>>> - cpu_register_physical_memory(0x100000, >>>>>> - below_4g_mem_size - 0x100000, >>>>>> - ram_addr + 0x100000); >>>>>> - if (above_4g_mem_size> 0) { >>>>>> - cpu_register_physical_memory(0x100000000ULL, >>>>>> above_4g_mem_size, >>>>>> - ram_addr + below_4g_mem_size); >>>>>> + /* map the last 128KB of the BIOS in ISA space */ >>>>>> + isa_bios_size = ram_size; >>>>>> + if (isa_bios_size> (128 * 1024)) { >>>>>> + isa_bios_size = 128 * 1024; >>>>>> } >>>>>> + ram_offset = ram_offset + ram_size - isa_bios_size; >>>>>> + cpu_register_physical_memory(0x100000 - isa_bios_size, >>>>>> + isa_bios_size, >>>>>> + ram_offset | IO_MEM_ROM); >>>>>> +} >>>>>> + >>>>>> +static int pc_system_rom_init(void) >>>>>> +{ >>>>>> + int ret; >>>>>> + int bios_size; >>>>>> + ram_addr_t bios_offset; >>>>>> + char *filename; >>>>>> >>>>>> /* BIOS load */ >>>>>> - if (bios_name == NULL) >>>>>> + 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; >>>>>> + >>>>>> + if (bios_size<= 0 || (bios_size % 65536) != 0) { >>>>>> + ret = -1; >>>>>> + } else { >>>>>> + bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); >>>>>> + ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), >>>>>> -1); >>>>>> } >>>>>> - bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); >>>>>> - 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) { >>>>>> qemu_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; >>>>>> - cpu_register_physical_memory(0x100000 - isa_bios_size, >>>>>> - isa_bios_size, >>>>>> - (bios_offset + bios_size - >>>>>> isa_bios_size) | IO_MEM_ROM); >>>>>> >>>>>> - option_rom_offset = qemu_ram_alloc(NULL, "pc.rom", PC_ROM_SIZE); >>>>>> - cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE, >>>>>> option_rom_offset); >>>>>> + pc_isa_bios_init(bios_offset, bios_size); >>>>>> >>>>>> /* map all the bios at the top of memory */ >>>>>> cpu_register_physical_memory((uint32_t)(-bios_size), >>>>>> bios_size, bios_offset | >>>>>> IO_MEM_ROM); >>>>>> >>>>>> + return bios_size; >>>>>> +} >>>>>> + >>>>>> +static void pc_system_flash_init(DriveInfo *pflash_drv, int rom_size) >>>>>> +{ >>>>>> + BlockDriverState *bdrv; >>>>>> + int64_t size; >>>>>> + target_phys_addr_t phys_addr; >>>>>> + ram_addr_t addr; >>>>>> + int sector_bits, sector_size; >>>>>> + >>>>>> + bdrv = NULL; >>>>>> + >>>>>> + 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: -pflash size must be a multiple of 0x%x\n", >>>>>> + sector_size); >>>>>> + exit(1); >>>>>> + } >>>>>> + >>>>>> + phys_addr = 0x100000000ULL - rom_size - size; >>>>>> + addr = qemu_ram_alloc(NULL, "system.flash", size); >>>>>> + DPRINTF("flash addr: 0x%lx\n", (int64_t)phys_addr); >>>>>> + pflash_cfi01_register(phys_addr, addr, bdrv, >>>>>> + sector_size, size>> sector_bits, >>>>>> + 4, 0x0000, 0x0000, 0x0000, 0x0000, 0); >>>>>> + >>>>>> + if (rom_size == 0) { >>>>>> + pc_isa_bios_init(addr, size); >>>>>> + } >>>>>> +} >>>>>> + >>>>>> +static void pc_system_firmware_init(void) >>>>>> +{ >>>>>> + int flash_present, rom_present; >>>>>> + int rom_size; >>>>>> + DriveInfo *pflash_drv; >>>>>> + >>>>>> + pflash_drv = drive_get(IF_PFLASH, 0, 0); >>>>>> + flash_present = (pflash_drv != NULL); >>>>>> + >>>>>> + /* Load rom if -bios is used or if -pflash is not used */ >>>>>> + rom_present = ((bios_name != NULL) || !flash_present); >>>>>> + >>>>>> + /* If rom is present, then it is mapped just below 4GB */ >>>>>> + if (rom_present) { >>>>>> + rom_size = pc_system_rom_init(); >>>>>> + } else { >>>>>> + rom_size = 0; >>>>>> + } >>>>>> + >>>>>> + /* If flash is present, then it is mapped just below the rom, or >>>>>> + * just below 4GB when rom is not present. */ >>>>>> + if (flash_present) { >>>>>> + pc_system_flash_init(pflash_drv, rom_size); >>>>>> + } >>>>>> +} >>>>>> + >>>>>> +void pc_memory_init(const char *kernel_filename, >>>>>> + const char *kernel_cmdline, >>>>>> + const char *initrd_filename, >>>>>> + ram_addr_t below_4g_mem_size, >>>>>> + ram_addr_t above_4g_mem_size) >>>>>> +{ >>>>>> + int linux_boot, i; >>>>>> + ram_addr_t ram_addr, option_rom_offset; >>>>>> + void *fw_cfg; >>>>>> + >>>>>> + linux_boot = (kernel_filename != NULL); >>>>>> + >>>>>> + /* allocate RAM */ >>>>>> + ram_addr = qemu_ram_alloc(NULL, "pc.ram", >>>>>> + below_4g_mem_size + above_4g_mem_size); >>>>>> + cpu_register_physical_memory(0, 0xa0000, ram_addr); >>>>>> + cpu_register_physical_memory(0x100000, >>>>>> + below_4g_mem_size - 0x100000, >>>>>> + ram_addr + 0x100000); >>>>>> + if (above_4g_mem_size> 0) { >>>>>> + cpu_register_physical_memory(0x100000000ULL, >>>>>> above_4g_mem_size, >>>>>> + ram_addr + below_4g_mem_size); >>>>>> + } >>>>>> + >>>>>> + /* Initialize ROM or flash ranges for PC firmware */ >>>>>> + pc_system_firmware_init(); >>>>>> + >>>>>> + option_rom_offset = qemu_ram_alloc(NULL, "pc.rom", PC_ROM_SIZE); >>>>>> + cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE, >>>>>> option_rom_offset); >>>>>> + >>>>>> fw_cfg = bochs_bios_init(); >>>>>> rom_set_fw(fw_cfg); >>>>>> >>>>> >>>>> >>>>> >>>> >>> >>> >> > >
On 07/23/2011 07:18 PM, Jordan Justen wrote: >> In terms of other flash devices, I don't think it's that simple. Flash is >> tied to the mobo layout so I don't think index> 0 really makes sense unless >> you allow a specific mapping address. I doubt that's terribly useful. > > I think VM's have a different situation than real hardware. I'm not > sure an all ROM or all flash decision will work well for qemu. In > most cases it may work better to make a ROM image available just below > 4GB, and add a flash image below this ROM. > > This allows the qemu's firmware to be updated as usual in > ${prefix}/share/bios.bin, but still allows a flash memory to be > available below this. (The flash below the ROM could be used only for > storing UEFI variables.) > > Otherwise when a new qemu is released along with a new firmware image, > the VM instance using writable flash will continue to use the old > firmware image. Yes, that's a feature. We could have a second nvram for other purposes of course. That could be defined as index=1. Regards, Anthony Liguori
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak index 55589fa..8697cd4 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y CONFIG_SOUND=y CONFIG_HPET=y CONFIG_APPLESMC=y +CONFIG_PFLASH_CFI01=y diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak index 8895028..eca9284 100644 --- a/default-configs/x86_64-softmmu.mak +++ b/default-configs/x86_64-softmmu.mak @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y CONFIG_SOUND=y CONFIG_HPET=y CONFIG_APPLESMC=y +CONFIG_PFLASH_CFI01=y diff --git a/hw/pc.c b/hw/pc.c index a3e8539..e25354f 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -41,6 +41,7 @@ #include "sysemu.h" #include "blockdev.h" #include "ui/qemu-spice.h" +#include "flash.h" /* output Bochs bios info messages */ //#define DEBUG_BIOS @@ -957,70 +958,154 @@ void pc_cpus_init(const char *cpu_model) } } -void pc_memory_init(const char *kernel_filename, - const char *kernel_cmdline, - const char *initrd_filename, - ram_addr_t below_4g_mem_size, - ram_addr_t above_4g_mem_size) +static void pc_isa_bios_init(ram_addr_t ram_offset, int ram_size) { - char *filename; - int ret, linux_boot, i; - ram_addr_t ram_addr, bios_offset, option_rom_offset; - int bios_size, isa_bios_size; - void *fw_cfg; - - linux_boot = (kernel_filename != NULL); + int isa_bios_size; - /* allocate RAM */ - ram_addr = qemu_ram_alloc(NULL, "pc.ram", - below_4g_mem_size + above_4g_mem_size); - cpu_register_physical_memory(0, 0xa0000, ram_addr); - cpu_register_physical_memory(0x100000, - below_4g_mem_size - 0x100000, - ram_addr + 0x100000); - if (above_4g_mem_size > 0) { - cpu_register_physical_memory(0x100000000ULL, above_4g_mem_size, - ram_addr + below_4g_mem_size); + /* map the last 128KB of the BIOS in ISA space */ + isa_bios_size = ram_size; + if (isa_bios_size > (128 * 1024)) { + isa_bios_size = 128 * 1024; } + ram_offset = ram_offset + ram_size - isa_bios_size; + cpu_register_physical_memory(0x100000 - isa_bios_size, + isa_bios_size, + ram_offset | IO_MEM_ROM); +} + +static int pc_system_rom_init(void) +{ + int ret; + int bios_size; + ram_addr_t bios_offset; + char *filename; /* BIOS load */ - if (bios_name == NULL) + 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; + + if (bios_size <= 0 || (bios_size % 65536) != 0) { + ret = -1; + } else { + bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); + ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); } - bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); - 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) { qemu_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; - cpu_register_physical_memory(0x100000 - isa_bios_size, - isa_bios_size, - (bios_offset + bios_size - isa_bios_size) | IO_MEM_ROM); - option_rom_offset = qemu_ram_alloc(NULL, "pc.rom", PC_ROM_SIZE); - cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE, option_rom_offset); + pc_isa_bios_init(bios_offset, bios_size); /* map all the bios at the top of memory */ cpu_register_physical_memory((uint32_t)(-bios_size), bios_size, bios_offset | IO_MEM_ROM); + return bios_size; +} + +static void pc_system_flash_init(DriveInfo *pflash_drv, int rom_size) +{ + BlockDriverState *bdrv; + int64_t size; + target_phys_addr_t phys_addr; + ram_addr_t addr; + int sector_bits, sector_size; + + bdrv = NULL; + + 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: -pflash size must be a multiple of 0x%x\n", + sector_size); + exit(1); + } + + phys_addr = 0x100000000ULL - rom_size - size; + addr = qemu_ram_alloc(NULL, "system.flash", size); + DPRINTF("flash addr: 0x%lx\n", (int64_t)phys_addr); + pflash_cfi01_register(phys_addr, addr, bdrv, + sector_size, size >> sector_bits, + 4, 0x0000, 0x0000, 0x0000, 0x0000, 0); + + if (rom_size == 0) { + pc_isa_bios_init(addr, size); + } +} + +static void pc_system_firmware_init(void) +{ + int flash_present, rom_present; + int rom_size; + DriveInfo *pflash_drv; + + pflash_drv = drive_get(IF_PFLASH, 0, 0); + flash_present = (pflash_drv != NULL); + + /* Load rom if -bios is used or if -pflash is not used */ + rom_present = ((bios_name != NULL) || !flash_present); + + /* If rom is present, then it is mapped just below 4GB */ + if (rom_present) { + rom_size = pc_system_rom_init(); + } else { + rom_size = 0; + } + + /* If flash is present, then it is mapped just below the rom, or + * just below 4GB when rom is not present. */ + if (flash_present) { + pc_system_flash_init(pflash_drv, rom_size); + } +} + +void pc_memory_init(const char *kernel_filename, + const char *kernel_cmdline, + const char *initrd_filename, + ram_addr_t below_4g_mem_size, + ram_addr_t above_4g_mem_size) +{ + int linux_boot, i; + ram_addr_t ram_addr, option_rom_offset; + void *fw_cfg; + + linux_boot = (kernel_filename != NULL); + + /* allocate RAM */ + ram_addr = qemu_ram_alloc(NULL, "pc.ram", + below_4g_mem_size + above_4g_mem_size); + cpu_register_physical_memory(0, 0xa0000, ram_addr); + cpu_register_physical_memory(0x100000, + below_4g_mem_size - 0x100000, + ram_addr + 0x100000); + if (above_4g_mem_size > 0) { + cpu_register_physical_memory(0x100000000ULL, above_4g_mem_size, + ram_addr + below_4g_mem_size); + } + + /* Initialize ROM or flash ranges for PC firmware */ + pc_system_firmware_init(); + + option_rom_offset = qemu_ram_alloc(NULL, "pc.rom", PC_ROM_SIZE); + cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE, option_rom_offset); + fw_cfg = bochs_bios_init(); rom_set_fw(fw_cfg);