On 2 July 2016 at 02:07, Alistair Francis <alistair.fran...@xilinx.com> wrote: > When loading ROMs allow the caller to specify an AddressSpace to use for > the load. > > Signed-off-by: Alistair Francis <alistair.fran...@xilinx.com> > --- > V8: > - Introduce an RFC version of AddressSpace loading support > > hw/core/loader.c | 18 ++++++++++++------ > include/hw/elf_ops.h | 2 +- > include/hw/loader.h | 10 ++++++---- > 3 files changed, 19 insertions(+), 11 deletions(-) > > diff --git a/hw/core/loader.c b/hw/core/loader.c > index 53e0e41..fcbcfbf 100644 > --- a/hw/core/loader.c > +++ b/hw/core/loader.c > @@ -777,6 +777,7 @@ struct Rom { > > uint8_t *data; > MemoryRegion *mr; > + AddressSpace *as; > int isrom; > char *fw_dir; > char *fw_file; > @@ -833,7 +834,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, > - bool option_rom, MemoryRegion *mr) > + bool option_rom, MemoryRegion *mr, > + AddressSpace *as)
We seem to add this argument to the function but never use it? I think specifying both mr and as should be an error. > { > MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); > Rom *rom; > @@ -969,7 +971,7 @@ MemoryRegion *rom_add_blob(const char *name, const void > *blob, size_t len, > * memory ownership of "data", so we don't have to allocate and copy the > buffer. > */ > int rom_add_elf_program(const char *name, void *data, size_t datasize, > - size_t romsize, hwaddr addr) > + size_t romsize, hwaddr addr, AddressSpace *as) > { > Rom *rom; > > @@ -979,18 +981,19 @@ int rom_add_elf_program(const char *name, void *data, > size_t datasize, > rom->datasize = datasize; > rom->romsize = romsize; > rom->data = data; > + rom->as = as; > rom_insert(rom); > return 0; > } > > int rom_add_vga(const char *file) > { > - return rom_add_file(file, "vgaroms", 0, -1, true, NULL); > + return rom_add_file(file, "vgaroms", 0, -1, true, NULL, NULL); > } > > int rom_add_option(const char *file, int32_t bootindex) > { > - return rom_add_file(file, "genroms", 0, bootindex, true, NULL); > + return rom_add_file(file, "genroms", 0, bootindex, true, NULL, NULL); > } > > static void rom_reset(void *unused) > @@ -1008,7 +1011,8 @@ static void rom_reset(void *unused) > void *host = memory_region_get_ram_ptr(rom->mr); > memcpy(host, rom->data, rom->datasize); > } else { > - cpu_physical_memory_write_rom(&address_space_memory, > + cpu_physical_memory_write_rom(rom->as ? rom->as : > + &address_space_memory, > rom->addr, rom->data, > rom->datasize); > } > if (rom->isrom) { > @@ -1031,12 +1035,13 @@ int rom_check_and_register_reset(void) > hwaddr addr = 0; > MemoryRegionSection section; > Rom *rom; > + AddressSpace *as = NULL; > > QTAILQ_FOREACH(rom, &roms, next) { > if (rom->fw_file) { > continue; > } > - if (addr > rom->addr) { > + if ((addr > rom->addr) && (as == rom->as)) { > fprintf(stderr, "rom: requested regions overlap " > "(rom %s. free=0x" TARGET_FMT_plx > ", addr=0x" TARGET_FMT_plx ")\n", > @@ -1048,6 +1053,7 @@ int rom_check_and_register_reset(void) > section = memory_region_find(get_system_memory(), rom->addr, 1); (An unrelated bug but I've just noticed that this code doesn't make sense for ROMs which go into a memory region rather than at an address in the system address space.) > rom->isrom = int128_nz(section.size) && > memory_region_is_rom(section.mr); > memory_region_unref(section.mr); > + as = rom->as; > } I don't think this attempt at checking is going to actually catch all the overlap cases if you allow multiple address spaces. The rom_insert() code orders roms in this list purely by load address, so you could get cases like [AS A, 0..0x1000], [AS B, 0..0x1000], [AS A, 0x500..0x1000] where entries 1 and 3 overlap but won't get caught. You probably need to order the list by AS first and then by address, and then adjust this code accordingly. > qemu_register_reset(rom_reset, NULL); > roms_loaded = 1; > diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h > index db70c11..1339677 100644 > --- a/include/hw/elf_ops.h > +++ b/include/hw/elf_ops.h > @@ -405,7 +405,7 @@ static int glue(load_elf, SZ)(const char *name, int fd, > snprintf(label, sizeof(label), "phdr #%d: %s", i, name); > > /* rom_add_elf_program() seize the ownership of 'data' */ > - rom_add_elf_program(label, data, file_size, mem_size, addr); > + rom_add_elf_program(label, data, file_size, mem_size, addr, > NULL); > > total_size += mem_size; > if (addr < low) > diff --git a/include/hw/loader.h b/include/hw/loader.h > index 4879b63..18eb0f2 100644 > --- a/include/hw/loader.h > +++ b/include/hw/loader.h > @@ -118,14 +118,14 @@ extern bool rom_file_has_mr; > > int rom_add_file(const char *file, const char *fw_dir, > hwaddr addr, int32_t bootindex, > - bool option_rom, MemoryRegion *mr); > + bool option_rom, MemoryRegion *mr, AddressSpace *as); > MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len, > size_t max_len, hwaddr addr, > const char *fw_file_name, > FWCfgReadCallback fw_callback, > void *callback_opaque); > int rom_add_elf_program(const char *name, void *data, size_t datasize, > - size_t romsize, hwaddr addr); > + size_t romsize, hwaddr addr, AddressSpace *as); > int rom_check_and_register_reset(void); > void rom_set_fw(FWCfgState *f); > void rom_set_order_override(int order); > @@ -135,11 +135,13 @@ void *rom_ptr(hwaddr addr); > void hmp_info_roms(Monitor *mon, const QDict *qdict); > > #define rom_add_file_fixed(_f, _a, _i) \ > - rom_add_file(_f, NULL, _a, _i, false, NULL) > + rom_add_file(_f, NULL, _a, _i, false, NULL, NULL) > #define rom_add_blob_fixed(_f, _b, _l, _a) \ > rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL) > #define rom_add_file_mr(_f, _mr, _i) \ > - rom_add_file(_f, NULL, 0, _i, false, _mr) > + rom_add_file(_f, NULL, 0, _i, false, _mr, NULL) > +#define rom_add_file_as(_f, _as, _i) \ > + rom_add_file(_f, NULL, 0, _i, false, NULL, _as) > > #define PC_ROM_MIN_VGA 0xc0000 > #define PC_ROM_MIN_OPTION 0xc8000 > -- > 2.7.4 > thanks -- PMM