On Thu, 2014-03-06 at 17:48 +0200, Michael S. Tsirkin wrote:
> Since
> commit 04920fc0faa4760f9c4fc0e73b992b768099be70
> loader: store FW CFG ROM files in RAM
> RAM MRs including ROM files in FW CFGs are created
> and named using the file basename.
>
> This becomes problematic if these names are
> supplied by user, since the basename might not
> be unique.
>
> There are two cases we care about:
> - option-rom flag.
> - option ROM for devices. This triggers e.g. when
> using rombar=0.
>
> At the moment we get an assert. E.g
> qemu -option-rom /usr/share/ipxe/8086100e.rom -option-rom
> /usr/share/ipxe.efi/8086100e.rom
> RAMBlock "/rom@genroms/8086100e.rom" already registered, abort!
>
> This is a regression from 1.7.
>
> For now let's keep it simple and just avoid creating the
> MRs in this case.
>
> Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> ---
> include/hw/loader.h | 6 ++++--
> hw/core/loader.c | 10 ++++++----
> 2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index 91b0122..edca1ed 100644
> --- a/include/hw/loader.h
> +++ b/include/hw/loader.h
> @@ -44,9 +44,11 @@ void pstrcpy_targphys(const char *name,
> const char *source);
>
> extern bool rom_file_in_ram;
> +extern bool option_rom_in_ram;
>
> int rom_add_file(const char *file, const char *fw_dir,
> - hwaddr addr, int32_t bootindex);
> + hwaddr addr, int32_t bootindex,
> + bool option);
Hi Michael,
I don't understand the usage of the new "option" parameter.
It is always set to true by the caller
Thanks,
Marcel
> void *rom_add_blob(const char *name, const void *blob, size_t len,
> hwaddr addr, const char *fw_file_name,
> FWCfgReadCallback fw_callback, void *callback_opaque);
> @@ -60,7 +62,7 @@ void *rom_ptr(hwaddr addr);
> void do_info_roms(Monitor *mon, const QDict *qdict);
>
> #define rom_add_file_fixed(_f, _a, _i) \
> - rom_add_file(_f, NULL, _a, _i)
> + rom_add_file(_f, NULL, _a, _i, true)
> #define rom_add_blob_fixed(_f, _b, _l, _a) \
> rom_add_blob(_f, _b, _l, _a, NULL, NULL, NULL)
>
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 0634bee..a19b158 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -54,6 +54,7 @@
>
> #include <zlib.h>
>
> +bool option_rom_in_ram = false;
> bool rom_file_in_ram = true;
>
> static int roms_loaded;
> @@ -624,7 +625,8 @@ static void *rom_set_mr(Rom *rom, Object *owner, const
> char *name)
> }
>
> int rom_add_file(const char *file, const char *fw_dir,
> - hwaddr addr, int32_t bootindex)
> + hwaddr addr, int32_t bootindex,
> + bool option)
> {
> Rom *rom;
> int rc, fd = -1;
> @@ -676,7 +678,7 @@ int rom_add_file(const char *file, const char *fw_dir,
> basename);
> snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name);
>
> - if (rom_file_in_ram) {
> + if ((!option || option_rom_in_ram) && rom_file_in_ram) {
> data = rom_set_mr(rom, OBJECT(fw_cfg), devpath);
> } else {
> data = rom->data;
> @@ -755,12 +757,12 @@ int rom_add_elf_program(const char *name, void *data,
> size_t datasize,
>
> int rom_add_vga(const char *file)
> {
> - return rom_add_file(file, "vgaroms", 0, -1);
> + return rom_add_file(file, "vgaroms", 0, -1, true);
> }
>
> int rom_add_option(const char *file, int32_t bootindex)
> {
> - return rom_add_file(file, "genroms", 0, bootindex);
> + return rom_add_file(file, "genroms", 0, bootindex, true);
> }
>
> static void rom_reset(void *unused)
