On Thu, Mar 06, 2014 at 06:14:11PM +0200, Marcel Apfelbaum wrote:
> 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
Ouch for fixed files this was supposed to be false.
Will send v2.
> > 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)
>
>
>
