[Qemu-devel] Executing from a rom device - Re: [PATCH 2/4] pc: Support system flash memory with pflash

[Jordan Justen]

Avi,

Alex pointed out that my patch below should not work for kvm, because
kvm currently does not support executing from a rom region.  This
surprised me, because I thought I had been testing with kvm enabled.
But, it turns out I wasn't, and in fact this patch does not work with
kvm enabled.  (Sorry all for this big mistake in my testing.)

Alex also suggested that you might be able to answer whether it would
be possible to execute from a flash device (ie, qemu 'rom_device'
device).  Is this something that would be possible with kvm?

If so, would it require changes to kvm on the kernel side?  Or, the
qemu side?  Perhaps just within the pflash_cfi01 device?

Would implementing it require the flash based execution to be very
slow under kvm?

Thanks for your time,

-Jordan

On Mon, Oct 17, 2011 at 12:27, Jordan Justen <address@hidden> wrote:
> On Mon, Oct 17, 2011 at 12:16, Jordan Justen <address@hidden> 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.
>>
>> Signed-off-by: Jordan Justen <address@hidden>
>> Cc: Anthony Liguori <address@hidden>
>> ---
>>  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                            |    3 +
>>  hw/pcflash.c                       |  145 
>> ++++++++++++++++++++++++++++++++++++
>>  vl.c                               |    2 +-
>>  8 files changed, 158 insertions(+), 51 deletions(-)
>>  create mode 100644 hw/pcflash.c
>>
>> diff --git a/Makefile.target b/Makefile.target
>> index 417f23e..37a5b56 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -225,6 +225,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 += pcflash.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 f0802b7..0c9b7ba 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 *rom_memory,
>>                     MemoryRegion **ram_memory)
>>  {
>> -    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);
>>
>>     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 b8ad9a3..333bd70 100644
>> --- a/hw/pc.h
>> +++ b/hw/pc.h
>> @@ -243,6 +243,9 @@ static inline bool isa_ne2000_init(int base, int irq, 
>> NICInfo *nd)
>>     return true;
>>  }
>>
>> +/* pcflash.c */
>> +void pc_system_firmware_init(MemoryRegion *rom_memory);
>> +
>>  /* e820 types */
>>  #define E820_RAM        1
>>  #define E820_RESERVED   2
>> diff --git a/hw/pcflash.c b/hw/pcflash.c
>> new file mode 100644
>> index 0000000..eece7ec
>> --- /dev/null
>> +++ b/hw/pcflash.c
>> @@ -0,0 +1,145 @@
>> +/*
>> + * QEMU PC System Flash
>> + *
>> + * 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"
>> +
>> +#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);
>
> Note: This does not 'alias' the pflash memory region, and therefore
> this does not emulate a real 440 chipset well.  I could not get this
> to work.  Is there a limitation with aliasing rom memory regions like
> pflash?
>
> Instead I create a new ram memory region, copy the initial pflash
> contents, and then present it as a ROM in the 0xe0000-0xfffff range.
> This worked for seabios.
>
> Thanks,
>
> -Jordan
>
>> +}
>> +
>> +static void pc_default_system_flash_init(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);
>> +}
>> +
>> +void pc_system_firmware_init(MemoryRegion *rom_memory)
>> +{
>> +    int flash_present;
>> +    DriveInfo *pflash_drv;
>> +
>> +    pflash_drv = drive_get(IF_PFLASH, 0, 0);
>> +    flash_present = (pflash_drv != NULL);
>> +
>> +    if (!flash_present) {
>> +        pc_default_system_flash_init();
>> +        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);
>> +    }
>> +
>> +    pc_system_flash_init(rom_memory, pflash_drv);
>> +}
>> +
>> +
>> diff --git a/vl.c b/vl.c
>> index 2dce3ae..0deae10 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1173,7 +1173,7 @@ static QEMUMachine *find_machine(const char *name)
>>     return NULL;
>>  }
>>
>> -static QEMUMachine *find_default_machine(void)
>> +QEMUMachine *find_default_machine(void)
>>  {
>>     QEMUMachine *m;
>>
>> --
>> 1.7.1
>>
>>
>>
>