On 04/20/2018 02:09 PM, Peter Maydell wrote: > On 13 April 2018 at 08:52, Cédric Le Goater <c...@kaod.org> wrote: >> On the POWER9 processor, the XIVE interrupt controller can control >> interrupt sources using MMIO to trigger events, to EOI or to turn off >> the sources. Priority management and interrupt acknowledgment is also >> controlled by MMIO in the presenter sub-engine. >> >> These MMIO regions are exposed to guests in QEMU with a set of 'ram >> device' memory mappings, similarly to VFIO, and the VMAs are populated >> dynamically with the appropriate pages using a fault handler. >> >> But, these regions are an issue for migration. We need to discard the >> associated RAMBlocks from the RAM state on the source VM and let the >> destination VM rebuild the memory mappings on the new host in the >> post_load() operation just before resuming the system. >> >> To achieve this goal, the following introduces a new RAMBlock flag >> RAM_MIGRATABLE which is updated in the vmstate_register_ram() and >> vmstate_unregister_ram() routines. This flag is then used by the >> migration to identify RAMBlocks to discard on the source. Some checks >> are also performed on the destination to make sure nothing invalid was >> sent. >> >> Signed-off-by: Cédric Le Goater <c...@kaod.org> > > So, this is conceptually the right thing, but it is a migration > compat break for any board which has a ram block which it doesn't > call vmstate_register_ram() for. These are mostly going to be > places that call one of the _nomigrate() functions to create an > MMIO region, and then don't register the ram by hand either.> Those are bugs, > in my view, but we should consider fixing > them while we're here. > > Here's the results of a grep and eyeballing of the results: > > These places are OK, because they register the memory region > by hand one way or another: > > numa stuff, registers it when the backend is used: > backends/hostmem-ram.c: > memory_region_init_ram_shared_nomigrate(&backend->mr, OBJECT(backend), > path, > > registers it globally by hand: > numa.c: memory_region_init_ram_nomigrate(mr, owner, name, > ram_size, &error_fatal); > numa.c: memory_region_init_ram_nomigrate(mr, owner, name, > ram_size, &error_fatal); > hw/arm/aspeed_soc.c: memory_region_init_ram_nomigrate(&s->sram, > OBJECT(dev), "aspeed.sram", > hw/block/onenand.c: memory_region_init_ram_nomigrate(&s->ram, > OBJECT(s), "onenand.ram", > hw/display/cg3.c: memory_region_init_ram_nomigrate(&s->rom, obj, > "cg3.prom", FCODE_MAX_ROM_SIZE, > hw/display/tcx.c: memory_region_init_ram_nomigrate(&s->rom, obj, > "tcx.prom", FCODE_MAX_ROM_SIZE, > hw/display/tcx.c: memory_region_init_ram_nomigrate(&s->vram_mem, > OBJECT(s), "tcx.vram", > hw/display/vga.c: memory_region_init_ram_nomigrate(&s->vram, obj, > "vga.vram", s->vram_size, > hw/input/milkymist-softusb.c: > memory_region_init_ram_nomigrate(&s->pmem, OBJECT(s), > "milkymist-softusb.pmem", > hw/input/milkymist-softusb.c: > memory_region_init_ram_nomigrate(&s->dmem, OBJECT(s), > "milkymist-softusb.dmem", > hw/net/milkymist-minimac2.c: > memory_region_init_ram_nomigrate(&s->buffers, OBJECT(dev), > "milkymist-minimac2.buffers", > hw/pci-host/prep.c: memory_region_init_ram_nomigrate(&s->bios, > OBJECT(s), "bios", BIOS_SIZE, > hw/sparc/sun4m.c: memory_region_init_ram_nomigrate(&s->mem, obj, > hw/sparc/sun4m.c: memory_region_init_ram_nomigrate(&s->mem, obj, > "sun4m.afx", 4, &error_fatal); > hw/sparc/sun4m.c: memory_region_init_ram_nomigrate(&s->prom, obj, > "sun4m.prom", PROM_SIZE_MAX, > hw/sparc64/sun4u.c: memory_region_init_ram_nomigrate(&s->prom, obj, > "sun4u.prom", PROM_SIZE_MAX, > hw/sparc64/sun4u.c: memory_region_init_ram_nomigrate(&d->ram, > OBJECT(d), "sun4u.ram", d->size, > hw/xtensa/xtfpga.c: memory_region_init_ram_nomigrate(ram, > OBJECT(s), "open_eth.ram", 16384, > > registers it non-globally by hand: > hw/xen/xen_pt_load_rom.c: > memory_region_init_ram_nomigrate(&dev->rom, owner, name, st.st_size, > &error_abort); > > These callsites are not OK, because they don't register the ram: > > hw/arm/aspeed.c: memory_region_init_rom_nomigrate(boot_rom, > OBJECT(bmc), "aspeed.boot_rom", > hw/arm/highbank.c: memory_region_init_ram_nomigrate(sysram, NULL, > "highbank.sysram", 0x8000, > hw/mips/boston.c: memory_region_init_rom_nomigrate(flash, NULL, > hw/mips/mips_malta.c: memory_region_init_ram_nomigrate(bios_copy, > NULL, "bios.1fc", BIOS_SIZE, > hw/net/dp8393x.c: memory_region_init_ram_nomigrate(&s->prom, OBJECT(dev), > [device only used on mips jazz board] > hw/pci-host/xilinx-pcie.c: memory_region_init_ram_nomigrate(&s->io, > OBJECT(s), "io", 16, NULL); > [device only used on mips boston board] > > Notes: > * any device that registers a ramblock globally is a bit dodgy, because > it means you can't have more than one of it (the ramblock names would > clash). We should fix those devices for the cases where we're willing > to take the migration compat break.
There are quite a few models calling memory_region_init_ram() with a NULL owner, see below. Should we fix all these ? Thanks, C. > * the xen_pt_load_rom.c case could be cleaned up to use > memory_region_init_ram > rather than the _nomigrate function without breaking compat > > NB: I haven't actually tested whether this is a migration compat break, > I merely believe it to be so :-) > > I think we can take the compat break on aspeed, highbank, malta and jazz. > Not sure about boston, but I guess so given the board doesn't have > per-QEMU-version machine models. > > thanks > -- PMM > hw/arm/msf2-soc.c: memory_region_init_ram(sram, NULL, "MSF2.eSRAM", s->esram_size, hw/arm/vexpress.c: memory_region_init_ram(sram, NULL, "vexpress.a15sram", 0x10000, hw/arm/vexpress.c: memory_region_init_ram(sram, NULL, "vexpress.sram", sram_size, hw/arm/vexpress.c: memory_region_init_ram(vram, NULL, "vexpress.vram", vram_size, hw/arm/palm.c: memory_region_init_ram(flash, NULL, "palmte.flash", flash_size, hw/arm/omap1.c: memory_region_init_ram(&s->imif_ram, NULL, "omap1.sram", s->sram_size, hw/arm/omap2.c: memory_region_init_ram(&s->sram, NULL, "omap2.sram", s->sram_size, hw/arm/fsl-imx6.c: memory_region_init_ram(&s->ocram, NULL, "imx6.ocram", FSL_IMX6_OCRAM_SIZE, hw/arm/mps2-tz.c: memory_region_init_ram(mr, NULL, name, size, &error_fatal); hw/arm/mainstone.c: memory_region_init_ram(rom, NULL, "mainstone.rom", MAINSTONE_ROM, hw/arm/tosa.c: memory_region_init_ram(rom, NULL, "tosa.rom", TOSA_ROM, &error_fatal); hw/arm/spitz.c: memory_region_init_ram(rom, NULL, "spitz.rom", SPITZ_ROM, &error_fatal); hw/arm/exynos4210.c: memory_region_init_ram(&s->irom_mem, NULL, "exynos4210.irom", hw/arm/exynos4210.c: memory_region_init_ram(&s->iram_mem, NULL, "exynos4210.iram", hw/arm/omap_sx1.c: memory_region_init_ram(flash, NULL, "omap_sx1.flash0-0", flash_size, hw/arm/omap_sx1.c: memory_region_init_ram(flash_1, NULL, "omap_sx1.flash1-0", hw/arm/stm32f205_soc.c: memory_region_init_ram(flash, NULL, "STM32F205.flash", FLASH_SIZE, hw/arm/stm32f205_soc.c: memory_region_init_ram(sram, NULL, "STM32F205.sram", SRAM_SIZE, hw/arm/iotkit.c: memory_region_init_ram(&s->sram0, NULL, "iotkit.sram0", 0x00008000, &err); hw/arm/fsl-imx31.c: memory_region_init_ram(&s->iram, NULL, "imx31.iram", FSL_IMX31_IRAM_SIZE, hw/arm/xilinx_zynq.c: memory_region_init_ram(ocm_ram, NULL, "zynq.ocm_ram", 256 << 10, hw/arm/xlnx-zynqmp.c: memory_region_init_ram(&s->ocm_ram[i], NULL, ocm_name, hw/arm/musicpal.c: memory_region_init_ram(sram, NULL, "musicpal.sram", MP_SRAM_SIZE, hw/arm/virt.c: memory_region_init_ram(secram, NULL, "virt.secure-ram", size, hw/arm/exynos4_boards.c: memory_region_init_ram(&s->dram1_mem, NULL, "exynos4210.dram1", hw/arm/exynos4_boards.c: memory_region_init_ram(&s->dram0_mem, NULL, "exynos4210.dram0", mem_size, hw/arm/stellaris.c: memory_region_init_ram(flash, NULL, "stellaris.flash", flash_size, hw/arm/stellaris.c: memory_region_init_ram(sram, NULL, "stellaris.sram", sram_size, hw/arm/pxa2xx.c: memory_region_init_ram(&s->sdram, NULL, "pxa270.sdram", sdram_size, hw/arm/pxa2xx.c: memory_region_init_ram(&s->internal, NULL, "pxa270.internal", 0x40000, hw/arm/pxa2xx.c: memory_region_init_ram(&s->sdram, NULL, "pxa255.sdram", sdram_size, hw/arm/pxa2xx.c: memory_region_init_ram(&s->internal, NULL, "pxa255.internal", hw/arm/fsl-imx25.c: memory_region_init_ram(&s->iram, NULL, "imx25.iram", FSL_IMX25_IRAM_SIZE, hw/arm/mps2.c: memory_region_init_ram(mr, NULL, name, size, &error_fatal); hw/arm/msf2-som.c: memory_region_init_ram(ddr, NULL, "ddr-ram", DDR_SIZE, hw/arm/realview.c: memory_region_init_ram(ram_lo, NULL, "realview.lowmem", low_ram_size, hw/arm/realview.c: memory_region_init_ram(ram_hi, NULL, "realview.highmem", ram_size, hw/arm/realview.c: memory_region_init_ram(ram_hack, NULL, "realview.hack", 0x1000, hw/tricore/tricore_testboard.c: memory_region_init_ram(ext_cram, NULL, "powerlink_ext_c.ram", hw/tricore/tricore_testboard.c: memory_region_init_ram(ext_dram, NULL, "powerlink_ext_d.ram", hw/tricore/tricore_testboard.c: memory_region_init_ram(int_cram, NULL, "powerlink_int_c.ram", 48 * 1024, hw/tricore/tricore_testboard.c: memory_region_init_ram(int_dram, NULL, "powerlink_int_d.ram", 48 * 1024, hw/tricore/tricore_testboard.c: memory_region_init_ram(pcp_data, NULL, "powerlink_pcp_data.ram", hw/tricore/tricore_testboard.c: memory_region_init_ram(pcp_text, NULL, "powerlink_pcp_text.ram", hw/nios2/10m50_devboard.c: memory_region_init_ram(phys_tcm, NULL, "nios2.tcm", tcm_size, hw/nios2/10m50_devboard.c: memory_region_init_ram(phys_ram, NULL, "nios2.ram", ram_size, hw/xtensa/xtensa_memory.c: memory_region_init_ram(m, NULL, num_name->str, hw/ppc/ppc405_boards.c: memory_region_init_ram(sram, NULL, "ef405ep.sram", sram_size, hw/ppc/ppc405_boards.c: memory_region_init_ram(bios, NULL, "ef405ep.bios", BIOS_SIZE, hw/ppc/ppc405_boards.c: memory_region_init_ram(bios, NULL, "taihu_405ep.bios", BIOS_SIZE, hw/ppc/mac_newworld.c: memory_region_init_ram(bios, NULL, "ppc_core99.bios", BIOS_SIZE, hw/ppc/mac_oldworld.c: memory_region_init_ram(bios, NULL, "ppc_heathrow.bios", BIOS_SIZE, hw/ppc/ppc440_uc.c: memory_region_init_ram(&l2sram->bank[0], NULL, "ppc4xx.l2sram_bank0", hw/ppc/ppc440_uc.c: memory_region_init_ram(&l2sram->bank[1], NULL, "ppc4xx.l2sram_bank1", hw/ppc/ppc440_uc.c: memory_region_init_ram(&l2sram->bank[2], NULL, "ppc4xx.l2sram_bank2", hw/ppc/ppc440_uc.c: memory_region_init_ram(&l2sram->bank[3], NULL, "ppc4xx.l2sram_bank3", hw/ppc/ppc405_uc.c: memory_region_init_ram(&ocm->isarc_ram, NULL, "ppc405.ocm", 4096, hw/ppc/sam460ex.c: memory_region_init_ram(l2cache_ram, NULL, "ppc440.l2cache_ram", 256 << 10, hw/mips/mips_mipssim.c: memory_region_init_ram(bios, NULL, "mips_mipssim.bios", BIOS_SIZE, hw/mips/mips_fulong2e.c: memory_region_init_ram(bios, NULL, "fulong2e.bios", bios_size, hw/mips/mips_jazz.c: memory_region_init_ram(bios, NULL, "mips_jazz.bios", MAGNUM_BIOS_SIZE, hw/mips/mips_jazz.c: memory_region_init_ram(rom_mr, NULL, "g364fb.rom", 0x80000, hw/mips/mips_r4k.c: memory_region_init_ram(bios, NULL, "mips_r4k.bios", BIOS_SIZE, hw/sparc/leon3.c: memory_region_init_ram(prom, NULL, "Leon3.bios", prom_size, &error_fatal); hw/microblaze/petalogix_ml605_mmu.c: memory_region_init_ram(phys_lmb_bram, NULL, "petalogix_ml605.lmb_bram", hw/microblaze/petalogix_ml605_mmu.c: memory_region_init_ram(phys_ram, NULL, "petalogix_ml605.ram", ram_size, hw/microblaze/petalogix_s3adsp1800_mmu.c: memory_region_init_ram(phys_lmb_bram, NULL, hw/microblaze/petalogix_s3adsp1800_mmu.c: memory_region_init_ram(phys_ram, NULL, "petalogix_s3adsp1800.ram", hw/microblaze/xlnx-zynqmp-pmu.c: memory_region_init_ram(pmu_ram, NULL, "xlnx-zynqmp-pmu.ram", hw/cris/axis_dev88.c: memory_region_init_ram(phys_intmem, NULL, "axisdev88.chipram", hw/riscv/sifive_u.c: memory_region_init_ram(main_mem, NULL, "riscv.sifive.u.ram", hw/riscv/sifive_u.c: memory_region_init_ram(boot_rom, NULL, "riscv.sifive.u.mrom", hw/riscv/sifive_e.c: memory_region_init_ram(mock_mmio, NULL, name, length, &error_fatal); hw/riscv/sifive_e.c: memory_region_init_ram(main_mem, NULL, "riscv.sifive.e.ram", hw/riscv/sifive_e.c: memory_region_init_ram(mask_rom, NULL, "riscv.sifive.e.mrom", hw/riscv/sifive_e.c: memory_region_init_ram(xip_mem, NULL, "riscv.sifive.e.xip", hw/riscv/spike.c: memory_region_init_ram(main_mem, NULL, "riscv.spike.ram", hw/riscv/spike.c: memory_region_init_ram(boot_rom, NULL, "riscv.spike.bootrom", hw/riscv/spike.c: memory_region_init_ram(main_mem, NULL, "riscv.spike.ram", hw/riscv/spike.c: memory_region_init_ram(boot_rom, NULL, "riscv.spike.bootrom", hw/riscv/virt.c: memory_region_init_ram(main_mem, NULL, "riscv_virt_board.ram", hw/riscv/virt.c: memory_region_init_ram(boot_rom, NULL, "riscv_virt_board.bootrom", hw/sh4/shix.c: memory_region_init_ram(rom, NULL, "shix.rom", 0x4000, &error_fatal); hw/sh4/shix.c: memory_region_init_ram(&sdram[0], NULL, "shix.sdram1", 0x01000000, hw/sh4/shix.c: memory_region_init_ram(&sdram[1], NULL, "shix.sdram2", 0x01000000, hw/sh4/r2d.c: memory_region_init_ram(sdram, NULL, "r2d.sdram", SDRAM_SIZE, &error_fatal); hw/moxie/moxiesim.c: memory_region_init_ram(ram, NULL, "moxiesim.ram", ram_size, &error_fatal); hw/moxie/moxiesim.c: memory_region_init_ram(rom, NULL, "moxie.rom", FIRMWARE_SIZE, &error_fatal); hw/m68k/mcf5208.c: memory_region_init_ram(sram, NULL, "mcf5208.sram", 16384, &error_fatal); hw/m68k/an5206.c: memory_region_init_ram(sram, NULL, "an5206.sram", 512, &error_fatal); hw/i386/pc_sysfw.c: memory_region_init_ram(isa_bios, NULL, "isa-bios", isa_bios_size, hw/i386/pc_sysfw.c: memory_region_init_ram(bios, NULL, "pc.bios", bios_size, &error_fatal); hw/i386/xen/xen-hvm.c: memory_region_init_ram(&ram_memory, NULL, "xen.ram", block_len, hw/i386/pc.c: memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE, hw/display/tc6393xb.c: memory_region_init_ram(&s->vram, NULL, "tc6393xb.vram", 0x100000, hw/display/vmware_vga.c: memory_region_init_ram(&s->fifo_ram, NULL, "vmsvga.fifo", s->fifo_size, hw/display/cg3.c: memory_region_init_ram(&s->vram_mem, NULL, "cg3.vram", s->vram_size, hw/openrisc/openrisc_sim.c: memory_region_init_ram(ram, NULL, "openrisc.ram", ram_size, &error_fatal); hw/unicore32/puv3.c: memory_region_init_ram(ram_memory, NULL, "puv3.ram", ram_size,
