From fef28891aa401e8f9d048c65f32067f51d695f4e Mon Sep 17 00:00:00 2001 From: Stefano Garzarella Date: Wed, 24 Jul 2019 16:31:03 +0200 Subject: loader: Handle memory-mapped ELFs This patch allows handling an ELF memory-mapped, taking care the reference count of the GMappedFile* passed through rom_add_elf_program(). In this case, the 'data' pointer is not heap-allocated, so we cannot free it. Suggested-by: Paolo Bonzini Signed-off-by: Stefano Garzarella Message-Id: <20190724143105.307042-2-sgarzare@redhat.com> Signed-off-by: Paolo Bonzini --- hw/core/loader.c | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) (limited to 'hw/core') diff --git a/hw/core/loader.c b/hw/core/loader.c index 84e4f3e..de00f56 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -838,6 +838,7 @@ struct Rom { int isrom; char *fw_dir; char *fw_file; + GMappedFile *mapped_file; bool committed; @@ -848,10 +849,25 @@ struct Rom { static FWCfgState *fw_cfg; static QTAILQ_HEAD(, Rom) roms = QTAILQ_HEAD_INITIALIZER(roms); -/* rom->data must be heap-allocated (do not use with rom_add_elf_program()) */ +/* + * rom->data can be heap-allocated or memory-mapped (e.g. when added with + * rom_add_elf_program()) + */ +static void rom_free_data(Rom *rom) +{ + if (rom->mapped_file) { + g_mapped_file_unref(rom->mapped_file); + rom->mapped_file = NULL; + } else { + g_free(rom->data); + } + + rom->data = NULL; +} + static void rom_free(Rom *rom) { - g_free(rom->data); + rom_free_data(rom); g_free(rom->path); g_free(rom->name); g_free(rom->fw_dir); @@ -1058,11 +1074,12 @@ MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len, /* This function is specific for elf program because we don't need to allocate * all the rom. We just allocate the first part and the rest is just zeros. This - * is why romsize and datasize are different. Also, this function seize the - * memory ownership of "data", so we don't have to allocate and copy the buffer. + * is why romsize and datasize are different. Also, this function takes its own + * reference to "mapped_file", 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, AddressSpace *as) +int rom_add_elf_program(const char *name, GMappedFile *mapped_file, void *data, + size_t datasize, size_t romsize, hwaddr addr, + AddressSpace *as) { Rom *rom; @@ -1073,6 +1090,12 @@ int rom_add_elf_program(const char *name, void *data, size_t datasize, rom->romsize = romsize; rom->data = data; rom->as = as; + + if (mapped_file && data) { + g_mapped_file_ref(mapped_file); + rom->mapped_file = mapped_file; + } + rom_insert(rom); return 0; } @@ -1107,8 +1130,7 @@ static void rom_reset(void *unused) } if (rom->isrom) { /* rom needs to be written only once */ - g_free(rom->data); - rom->data = NULL; + rom_free_data(rom); } /* * The rom loader is really on the same level as firmware in the guest -- cgit v1.1 From 355477f8c73e9c6b60704c57472c71393ff39bca Mon Sep 17 00:00:00 2001 From: Catherine Ho Date: Mon, 8 Apr 2019 04:42:13 -0400 Subject: migration: do not rom_reset() during incoming migration Commit 18269069c310 ("migration: Introduce ignore-shared capability") addes ignore-shared capability to bypass the shared ramblock (e,g, membackend + numa node). It does good to live migration. As told by Yury,this commit expectes that QEMU doesn't write to guest RAM until VM starts, but it does on aarch64 qemu: Backtrace: 1 0x000055f4a296dd84 in address_space_write_rom_internal () at exec.c:3458 2 0x000055f4a296de3a in address_space_write_rom () at exec.c:3479 3 0x000055f4a2d519ff in rom_reset () at hw/core/loader.c:1101 4 0x000055f4a2d475ec in qemu_devices_reset () at hw/core/reset.c:69 5 0x000055f4a2c90a28 in qemu_system_reset () at vl.c:1675 6 0x000055f4a2c9851d in main () at vl.c:4552 Actually, on arm64 virt marchine, ramblock "dtb" will be filled into ram druing rom_reset. In ignore-shared incoming case, this rom filling is not required since all the data has been stored in memory backend file. Further more, as suggested by Peter Xu, if we do rom_reset() now with these ROMs then the RAM data should be re-filled again too with the migration stream coming in. Fixes: commit 18269069c310 ("migration: Introduce ignore-shared capability") Suggested-by: Yury Kotov Suggested-by: Peter Xu Signed-off-by: Catherine Ho Acked-by: Peter Xu Signed-off-by: Paolo Bonzini --- hw/core/loader.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'hw/core') diff --git a/hw/core/loader.c b/hw/core/loader.c index de00f56..32f7cc7 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -58,6 +58,7 @@ #include "exec/address-spaces.h" #include "hw/boards.h" #include "qemu/cutils.h" +#include "sysemu/runstate.h" #include @@ -1114,6 +1115,15 @@ static void rom_reset(void *unused) { Rom *rom; + /* + * We don't need to fill in the RAM with ROM data because we'll fill + * the data in during the next incoming migration in all cases. Note + * that some of those RAMs can actually be modified by the guest on ARM + * so this is probably the only right thing to do here. + */ + if (runstate_check(RUN_STATE_INMIGRATE)) + return; + QTAILQ_FOREACH(rom, &roms, next) { if (rom->fw_file) { continue; -- cgit v1.1