| Message ID | 1385596372-12167-1-git-send-email-lersek@redhat.com |
|---|---|
| State | New |
| Headers | show |
Laszlo Ersek <lersek@redhat.com> writes: > This patch allows the user to usefully specify > > -drive file=img_1,if=pflash,format=raw,readonly \ > -drive file=img_2,if=pflash,format=raw > > on the command line. The flash images will be mapped under 4G in their > reverse unit order -- that is, with their base addresses progressing > downwards, in increasing unit order. > > (The unit number increases with command line order if not explicitly > specified.) > > This accommodates the following use case: suppose that OVMF is split in > two parts, a writeable host file for non-volatile variable storage, and a > read-only part for bootstrap and decompressible executable code. > > The binary code part would be read-only, centrally managed on the host > system, and passed in as unit 0. The variable store would be writeable, > VM-specific, and passed in as unit 1. > > 00000000ffe00000-00000000ffe1ffff (prio 0, R-): system.flash1 > 00000000ffe20000-00000000ffffffff (prio 0, R-): system.flash0 > > (If the guest tries to write to the flash range that is backed by the > read-only drive, pflash_update() is never called; various flash > programming/erase errors are returned to the guest instead. See the > callers of pflash_update(), and the initialization of "pfl->ro", in > "hw/block/pflash_cfi01.c".) > > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > v2: > - don't map flash devices beyond unit#1 [Markus] > - explicit low bound on cumulative base address [Markus] > - updated comment block on pc_system_flash_init() [Laszlo] > - dropped (DriveInfo*) param of pc_system_flash_init(), use drive_get() > internally for unit#0 too [Markus] > - turn do-loop into for-loop [Markus] > - check bdrv_getlength() for errors [Laszlo] > - reject zero-sized flash [Laszlo] > - use Location of -pflash / -drive if=pflash,... option in error > reporting [Markus] > - describe the real spots where write attempts to r/o flash are caught > [Laszlo] > > hw/i386/pc_sysfw.c | 105 +++++++++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 86 insertions(+), 19 deletions(-) > > diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c > index e917c83..75a7ebb 100644 > --- a/hw/i386/pc_sysfw.c > +++ b/hw/i386/pc_sysfw.c > @@ -72,35 +72,102 @@ static void pc_isa_bios_init(MemoryRegion *rom_memory, > memory_region_set_readonly(isa_bios, true); > } > > -static void pc_system_flash_init(MemoryRegion *rom_memory, > - DriveInfo *pflash_drv) > +#define FLASH_MAP_UNIT_MAX 2 > + > +/* We don't have a theoretically justifiable exact lower bound on the base > + * address of any flash mapping. In practice, the IO-APIC MMIO range is > + * [0xFEE00000..0xFEE01000[ -- see IO_APIC_DEFAULT_ADDRESS --, leaving free > + * only 18MB-4KB below 4G. For now, restrict the cumulative mapping to 8MB in > + * size. > + */ > +#define FLASH_MAP_BASE_MIN ((hwaddr)(0x100000000ULL - 8*1024*1024)) > + > +/* This function maps flash drives from 4G downward, in order of their unit > + * numbers. The mapping starts at unit#0, with unit number increments of 1, and > + * stops before the first missing flash drive, or before > + * unit#FLASH_MAP_UNIT_MAX, whichever is reached first. > + * > + * Addressing within one flash drive is of course not reversed. > + * > + * An error message is printed and the process exits if: > + * - the size of the backing file for a flash drive is non-positive, or not a > + * multiple of the required sector size, or > + * - the current mapping's base address would fall below FLASH_MAP_BASE_MIN. > + * > + * The drive with unit#0 (if available) is mapped at the highest address, and > + * it is passed to pc_isa_bios_init(). Merging several drives for isa-bios is > + * not supported. > + */ > +static void pc_system_flash_init(MemoryRegion *rom_memory) > { > + int unit; > + DriveInfo *pflash_drv; > BlockDriverState *bdrv; > int64_t size; > - hwaddr phys_addr; > + char *fatal_errmsg = NULL; > + hwaddr phys_addr = 0x100000000ULL; > int sector_bits, sector_size; > pflash_t *system_flash; > MemoryRegion *flash_mem; > + char name[64]; > > - 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); > + for (unit = 0; > + (unit < FLASH_MAP_UNIT_MAX && > + (pflash_drv = drive_get(IF_PFLASH, 0, unit)) != NULL); > + ++unit) { > + bdrv = pflash_drv->bdrv; Used just twice, sure it's worth a variable? Your choice, of course. > + size = bdrv_getlength(bdrv); > + if (size < 0) { > + fatal_errmsg = g_strdup_printf("failed to get backing file size"); > + } else if (size == 0) { > + fatal_errmsg = g_strdup_printf("PC system firmware (pflash) " > + "cannot have zero size"); > + } else if ((size % sector_size) != 0) { > + fatal_errmsg = g_strdup_printf("PC system firmware (pflash) " > + "must be a multiple of 0x%x", sector_size); > + } else if (phys_addr < size || phys_addr - size < FLASH_MAP_BASE_MIN) { > + fatal_errmsg = g_strdup_printf("oversized backing file, pflash " > + "segments cannot be mapped under " "mapped below "? > + TARGET_FMT_plx, FLASH_MAP_BASE_MIN); > + } > + if (fatal_errmsg != NULL) { > + Location loc; > + > + /* push a new, "none" location on the location stack; overwrite its > + * contents with the location saved in the option; print the error > + * (includes location); pop the top > + */ Sure this comment is worthwhile? Your decision. I suspect we should have error_report_loc(). > + loc_push_none(&loc); > + if (pflash_drv->opts != NULL) { > + qemu_opts_loc_restore(pflash_drv->opts); > + } > + error_report("%s", fatal_errmsg); > + loc_pop(&loc); > + g_free(fatal_errmsg); > + exit(1); You hoist the reporting of a fatal error out of the conditionals. If we had error_report_loc(), we could leave it there. But we don't. > + } > + > + phys_addr -= size; > + > + /* pflash_cfi01_register() creates a deep copy of the name */ > + snprintf(name, sizeof name, "system.flash%d", unit); > + system_flash = pflash_cfi01_register(phys_addr, NULL /* qdev */, name, pflash_cfi01_register() doesn't use its second parameter, and all callers pass NULL, ugh. I looked because I don't like nor trust /* name of parameter */ comments. Matter of taste. > + size, bdrv, sector_size, > + size >> sector_bits, > + 1 /* width */, > + 0x0000 /* id0 */, > + 0x0000 /* id1 */, > + 0x0000 /* id2 */, > + 0x0000 /* id3 */, > + 0 /* be */); > + if (unit == 0) { > + flash_mem = pflash_cfi01_get_memory(system_flash); > + pc_isa_bios_init(rom_memory, flash_mem, size); > + } > } > - > - 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 old_pc_system_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw) > @@ -181,5 +248,5 @@ void pc_system_firmware_init(MemoryRegion *rom_memory, bool isapc_ram_fw) > exit(1); > } > > - pc_system_flash_init(rom_memory, pflash_drv); > + pc_system_flash_init(rom_memory); > } Nothing but nits, therefore Reviewed-by: Markus Armbruster <armbru@redhat.com> Still pining for that star destroyer, though... ;)
Il 03/12/2013 18:23, Markus Armbruster ha scritto: >> > + /* push a new, "none" location on the location stack; overwrite its >> > + * contents with the location saved in the option; print the error >> > + * (includes location); pop the top >> > + */ > Sure this comment is worthwhile? Your decision. > > I suspect we should have error_report_loc(). Or qemu_opts_loc_push. In any case, the comment is necessary but shouldn't be. :) Paolo >> > + loc_push_none(&loc); >> > + if (pflash_drv->opts != NULL) { >> > + qemu_opts_loc_restore(pflash_drv->opts); >> > + }
On 12/03/13 18:23, Markus Armbruster wrote: > Laszlo Ersek <lersek@redhat.com> writes: > >> This patch allows the user to usefully specify >> >> -drive file=img_1,if=pflash,format=raw,readonly \ >> -drive file=img_2,if=pflash,format=raw >> >> on the command line. The flash images will be mapped under 4G in their >> reverse unit order -- that is, with their base addresses progressing >> downwards, in increasing unit order. >> >> (The unit number increases with command line order if not explicitly >> specified.) >> >> This accommodates the following use case: suppose that OVMF is split in >> two parts, a writeable host file for non-volatile variable storage, and a >> read-only part for bootstrap and decompressible executable code. >> >> The binary code part would be read-only, centrally managed on the host >> system, and passed in as unit 0. The variable store would be writeable, >> VM-specific, and passed in as unit 1. >> >> 00000000ffe00000-00000000ffe1ffff (prio 0, R-): system.flash1 >> 00000000ffe20000-00000000ffffffff (prio 0, R-): system.flash0 >> >> (If the guest tries to write to the flash range that is backed by the >> read-only drive, pflash_update() is never called; various flash >> programming/erase errors are returned to the guest instead. See the >> callers of pflash_update(), and the initialization of "pfl->ro", in >> "hw/block/pflash_cfi01.c".) >> >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> --- >> v2: >> - don't map flash devices beyond unit#1 [Markus] >> - explicit low bound on cumulative base address [Markus] >> - updated comment block on pc_system_flash_init() [Laszlo] >> - dropped (DriveInfo*) param of pc_system_flash_init(), use drive_get() >> internally for unit#0 too [Markus] >> - turn do-loop into for-loop [Markus] >> - check bdrv_getlength() for errors [Laszlo] >> - reject zero-sized flash [Laszlo] >> - use Location of -pflash / -drive if=pflash,... option in error >> reporting [Markus] >> - describe the real spots where write attempts to r/o flash are caught >> [Laszlo] >> >> hw/i386/pc_sysfw.c | 105 +++++++++++++++++++++++++++++++++++++++++++---------- >> 1 file changed, 86 insertions(+), 19 deletions(-) >> >> diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c >> index e917c83..75a7ebb 100644 >> --- a/hw/i386/pc_sysfw.c >> +++ b/hw/i386/pc_sysfw.c >> @@ -72,35 +72,102 @@ static void pc_isa_bios_init(MemoryRegion *rom_memory, >> memory_region_set_readonly(isa_bios, true); >> } >> >> -static void pc_system_flash_init(MemoryRegion *rom_memory, >> - DriveInfo *pflash_drv) >> +#define FLASH_MAP_UNIT_MAX 2 >> + >> +/* We don't have a theoretically justifiable exact lower bound on the base >> + * address of any flash mapping. In practice, the IO-APIC MMIO range is >> + * [0xFEE00000..0xFEE01000[ -- see IO_APIC_DEFAULT_ADDRESS --, leaving free >> + * only 18MB-4KB below 4G. For now, restrict the cumulative mapping to 8MB in >> + * size. >> + */ >> +#define FLASH_MAP_BASE_MIN ((hwaddr)(0x100000000ULL - 8*1024*1024)) >> + >> +/* This function maps flash drives from 4G downward, in order of their unit >> + * numbers. The mapping starts at unit#0, with unit number increments of 1, and >> + * stops before the first missing flash drive, or before >> + * unit#FLASH_MAP_UNIT_MAX, whichever is reached first. >> + * >> + * Addressing within one flash drive is of course not reversed. >> + * >> + * An error message is printed and the process exits if: >> + * - the size of the backing file for a flash drive is non-positive, or not a >> + * multiple of the required sector size, or >> + * - the current mapping's base address would fall below FLASH_MAP_BASE_MIN. >> + * >> + * The drive with unit#0 (if available) is mapped at the highest address, and >> + * it is passed to pc_isa_bios_init(). Merging several drives for isa-bios is >> + * not supported. >> + */ >> +static void pc_system_flash_init(MemoryRegion *rom_memory) >> { >> + int unit; >> + DriveInfo *pflash_drv; >> BlockDriverState *bdrv; >> int64_t size; >> - hwaddr phys_addr; >> + char *fatal_errmsg = NULL; >> + hwaddr phys_addr = 0x100000000ULL; >> int sector_bits, sector_size; >> pflash_t *system_flash; >> MemoryRegion *flash_mem; >> + char name[64]; >> >> - 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); >> + for (unit = 0; >> + (unit < FLASH_MAP_UNIT_MAX && >> + (pflash_drv = drive_get(IF_PFLASH, 0, unit)) != NULL); >> + ++unit) { >> + bdrv = pflash_drv->bdrv; > > Used just twice, sure it's worth a variable? Your choice, of course. I sought to keep the initial block of definitions intact as much as I could. Personally I would have wanted to kill off most of those, and (re)introduce anything I'd need in the narrowest scope possible. But, I remembered that you hate narrow scope declarations and like to keep everything at the top of the function, so I didn't mess with that block precisely to keep you happy. :) > >> + size = bdrv_getlength(bdrv); >> + if (size < 0) { >> + fatal_errmsg = g_strdup_printf("failed to get backing file size"); >> + } else if (size == 0) { >> + fatal_errmsg = g_strdup_printf("PC system firmware (pflash) " >> + "cannot have zero size"); >> + } else if ((size % sector_size) != 0) { >> + fatal_errmsg = g_strdup_printf("PC system firmware (pflash) " >> + "must be a multiple of 0x%x", sector_size); >> + } else if (phys_addr < size || phys_addr - size < FLASH_MAP_BASE_MIN) { >> + fatal_errmsg = g_strdup_printf("oversized backing file, pflash " >> + "segments cannot be mapped under " > > "mapped below "? Too much finesse, can't care about everything! :) > >> + TARGET_FMT_plx, FLASH_MAP_BASE_MIN); >> + } >> + if (fatal_errmsg != NULL) { >> + Location loc; >> + >> + /* push a new, "none" location on the location stack; overwrite its >> + * contents with the location saved in the option; print the error >> + * (includes location); pop the top >> + */ > > Sure this comment is worthwhile? Your decision. It shows you that I did my homework on the Location thing. More importantly, it shows the casual reader what's going on. Considering that I found exactly one other such pattern in the entire source, I think we can qualify all developers "casual readers" in this regard. I myself would have been greatly helped by such a comment (anywhere), so I added it. > > I suspect we should have error_report_loc(). I was happy that I managed to use the existing functions correctly. Please feel free to introduce the new function and refactor this call site :) > >> + loc_push_none(&loc); >> + if (pflash_drv->opts != NULL) { >> + qemu_opts_loc_restore(pflash_drv->opts); >> + } >> + error_report("%s", fatal_errmsg); >> + loc_pop(&loc); >> + g_free(fatal_errmsg); >> + exit(1); > > You hoist the reporting of a fatal error out of the conditionals. If we > had error_report_loc(), we could leave it there. But we don't. Correct, I do that, with a smile on my face. :) > >> + } >> + >> + phys_addr -= size; >> + >> + /* pflash_cfi01_register() creates a deep copy of the name */ >> + snprintf(name, sizeof name, "system.flash%d", unit); >> + system_flash = pflash_cfi01_register(phys_addr, NULL /* qdev */, name, > > pflash_cfi01_register() doesn't use its second parameter, and all > callers pass NULL, ugh. I looked too. > > I looked because I don't like nor trust /* name of parameter */ > comments. Matter of taste. I don't care for such in-call comments, but I saw MST use them, so I thought what the heck. > >> + size, bdrv, sector_size, >> + size >> sector_bits, >> + 1 /* width */, >> + 0x0000 /* id0 */, >> + 0x0000 /* id1 */, >> + 0x0000 /* id2 */, >> + 0x0000 /* id3 */, >> + 0 /* be */); >> + if (unit == 0) { >> + flash_mem = pflash_cfi01_get_memory(system_flash); >> + pc_isa_bios_init(rom_memory, flash_mem, size); >> + } >> } >> - >> - 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 old_pc_system_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw) >> @@ -181,5 +248,5 @@ void pc_system_firmware_init(MemoryRegion *rom_memory, bool isapc_ram_fw) >> exit(1); >> } >> >> - pc_system_flash_init(rom_memory, pflash_drv); >> + pc_system_flash_init(rom_memory); >> } > > Nothing but nits, therefore > > Reviewed-by: Markus Armbruster <armbru@redhat.com> Why thank you! :) > > Still pining for that star destroyer, though... ;) > I'm pining for 36-hour days! :) Thanks! Laszlo
On 12/03/13 18:23, Markus Armbruster wrote:
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
My horse for a commit, please?
Thanks
Laszlo
Laszlo Ersek <lersek@redhat.com> writes: > On 12/03/13 18:23, Markus Armbruster wrote: > >> Reviewed-by: Markus Armbruster <armbru@redhat.com> > > My horse for a commit, please? Your horse, two months, and four more rebases.
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c index e917c83..75a7ebb 100644 --- a/hw/i386/pc_sysfw.c +++ b/hw/i386/pc_sysfw.c @@ -72,35 +72,102 @@ static void pc_isa_bios_init(MemoryRegion *rom_memory, memory_region_set_readonly(isa_bios, true); } -static void pc_system_flash_init(MemoryRegion *rom_memory, - DriveInfo *pflash_drv) +#define FLASH_MAP_UNIT_MAX 2 + +/* We don't have a theoretically justifiable exact lower bound on the base + * address of any flash mapping. In practice, the IO-APIC MMIO range is + * [0xFEE00000..0xFEE01000[ -- see IO_APIC_DEFAULT_ADDRESS --, leaving free + * only 18MB-4KB below 4G. For now, restrict the cumulative mapping to 8MB in + * size. + */ +#define FLASH_MAP_BASE_MIN ((hwaddr)(0x100000000ULL - 8*1024*1024)) + +/* This function maps flash drives from 4G downward, in order of their unit + * numbers. The mapping starts at unit#0, with unit number increments of 1, and + * stops before the first missing flash drive, or before + * unit#FLASH_MAP_UNIT_MAX, whichever is reached first. + * + * Addressing within one flash drive is of course not reversed. + * + * An error message is printed and the process exits if: + * - the size of the backing file for a flash drive is non-positive, or not a + * multiple of the required sector size, or + * - the current mapping's base address would fall below FLASH_MAP_BASE_MIN. + * + * The drive with unit#0 (if available) is mapped at the highest address, and + * it is passed to pc_isa_bios_init(). Merging several drives for isa-bios is + * not supported. + */ +static void pc_system_flash_init(MemoryRegion *rom_memory) { + int unit; + DriveInfo *pflash_drv; BlockDriverState *bdrv; int64_t size; - hwaddr phys_addr; + char *fatal_errmsg = NULL; + hwaddr phys_addr = 0x100000000ULL; int sector_bits, sector_size; pflash_t *system_flash; MemoryRegion *flash_mem; + char name[64]; - 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); + for (unit = 0; + (unit < FLASH_MAP_UNIT_MAX && + (pflash_drv = drive_get(IF_PFLASH, 0, unit)) != NULL); + ++unit) { + bdrv = pflash_drv->bdrv; + size = bdrv_getlength(bdrv); + if (size < 0) { + fatal_errmsg = g_strdup_printf("failed to get backing file size"); + } else if (size == 0) { + fatal_errmsg = g_strdup_printf("PC system firmware (pflash) " + "cannot have zero size"); + } else if ((size % sector_size) != 0) { + fatal_errmsg = g_strdup_printf("PC system firmware (pflash) " + "must be a multiple of 0x%x", sector_size); + } else if (phys_addr < size || phys_addr - size < FLASH_MAP_BASE_MIN) { + fatal_errmsg = g_strdup_printf("oversized backing file, pflash " + "segments cannot be mapped under " + TARGET_FMT_plx, FLASH_MAP_BASE_MIN); + } + if (fatal_errmsg != NULL) { + Location loc; + + /* push a new, "none" location on the location stack; overwrite its + * contents with the location saved in the option; print the error + * (includes location); pop the top + */ + loc_push_none(&loc); + if (pflash_drv->opts != NULL) { + qemu_opts_loc_restore(pflash_drv->opts); + } + error_report("%s", fatal_errmsg); + loc_pop(&loc); + g_free(fatal_errmsg); + exit(1); + } + + phys_addr -= size; + + /* pflash_cfi01_register() creates a deep copy of the name */ + snprintf(name, sizeof name, "system.flash%d", unit); + system_flash = pflash_cfi01_register(phys_addr, NULL /* qdev */, name, + size, bdrv, sector_size, + size >> sector_bits, + 1 /* width */, + 0x0000 /* id0 */, + 0x0000 /* id1 */, + 0x0000 /* id2 */, + 0x0000 /* id3 */, + 0 /* be */); + if (unit == 0) { + flash_mem = pflash_cfi01_get_memory(system_flash); + pc_isa_bios_init(rom_memory, flash_mem, size); + } } - - 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 old_pc_system_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw) @@ -181,5 +248,5 @@ void pc_system_firmware_init(MemoryRegion *rom_memory, bool isapc_ram_fw) exit(1); } - pc_system_flash_init(rom_memory, pflash_drv); + pc_system_flash_init(rom_memory); }
This patch allows the user to usefully specify -drive file=img_1,if=pflash,format=raw,readonly \ -drive file=img_2,if=pflash,format=raw on the command line. The flash images will be mapped under 4G in their reverse unit order -- that is, with their base addresses progressing downwards, in increasing unit order. (The unit number increases with command line order if not explicitly specified.) This accommodates the following use case: suppose that OVMF is split in two parts, a writeable host file for non-volatile variable storage, and a read-only part for bootstrap and decompressible executable code. The binary code part would be read-only, centrally managed on the host system, and passed in as unit 0. The variable store would be writeable, VM-specific, and passed in as unit 1. 00000000ffe00000-00000000ffe1ffff (prio 0, R-): system.flash1 00000000ffe20000-00000000ffffffff (prio 0, R-): system.flash0 (If the guest tries to write to the flash range that is backed by the read-only drive, pflash_update() is never called; various flash programming/erase errors are returned to the guest instead. See the callers of pflash_update(), and the initialization of "pfl->ro", in "hw/block/pflash_cfi01.c".) Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- v2: - don't map flash devices beyond unit#1 [Markus] - explicit low bound on cumulative base address [Markus] - updated comment block on pc_system_flash_init() [Laszlo] - dropped (DriveInfo*) param of pc_system_flash_init(), use drive_get() internally for unit#0 too [Markus] - turn do-loop into for-loop [Markus] - check bdrv_getlength() for errors [Laszlo] - reject zero-sized flash [Laszlo] - use Location of -pflash / -drive if=pflash,... option in error reporting [Markus] - describe the real spots where write attempts to r/o flash are caught [Laszlo] hw/i386/pc_sysfw.c | 105 +++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 86 insertions(+), 19 deletions(-)