Re: [Qemu-devel] [PATCH] migration: avoid copying ignore-shared ramblock when in incoming migration

[Catherine Ho]

Thanks, Peter
See my comments below, please

On Thu, 21 Mar 2019 at 14:10, Peter Xu <address@hidden> wrote:
>
> On Wed, Mar 20, 2019 at 04:05:58PM +0800, chenxi He wrote:
> > On Wed, 20 Mar 2019 at 13:07, Peter Xu <address@hidden> wrote:
> > >
> > > On Tue, Mar 19, 2019 at 11:49:22AM -0400, Catherine Ho wrote:
> > > > Commit 18269069c310 ("migration: Introduce ignore-shared capability")
> > > > addes a ignore-shared capability to bypass the shared ramblock (e,g,
> > > > membackend + numa node). It does good to live migration.
> > > >
> > > > This commit expected 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
> > > >
> > > > In address_space_write_rom_internal, if the ramblock is RAM of
> > > > ramblock_is_ignored(), memcpy this ramblock will cause the 
> > > > memory-backend
> > > > file corruption.
> > > > But in normal case (!in_incoming_migration), this logic should be 
> > > > reserved.
> > >
> > > (I am sorry if these questions are silly...)
> > >
> > > Could I ask when a ROM will be changed during execution of the guest,
> > > and why?
> > Sure :),  as told by Peter
> > "as part of reset, we write the contents of ROM blobs, ELF files loaded 
> > through
> > -kernel, etc, to the guest memory."
>
> I see...
>
> I tried to dig on how x86 implemented the "-kernel" parameter and I
> noticed that it's dumping the kernel image into fw_cfg and probably
> that's also the reason why this should not be a problem for x86
> because rom reset on x86 won't overwrite the image.  Meanwhile, it
> seems totally different comparing to what has been done by ARM, which
> should probably be using rom_add_elf_program() to load the image if my
> reading is correct, so the kernel image is a ROM on ARM even if it can
> be written... Is my understanding correct?
>
> With that, I still feel that hacking into the memory write functions
> are tricky and I feel like it could bring hard to debug issues.  Would
> it be possible that we identify somehow that this ROM is a fake one
> (since it can actually be written) and we ignore the reset of it when
> proper (e.g., the initial reset on destination)?
I found that special rom block is "dtb"
See
commit 4c4bf654746eae5a042bde6c150d534d8849b762
Author: Ard Biesheuvel <address@hidden>
Date:   Fri Sep 12 14:06:50 2014 +0100

    hw/arm/boot: load DTB as a ROM image

    In order to make the device tree blob (DTB) available in memory not only at
    first boot, but also after system reset, use rom_blob_add_fixed() to install
    it into memory.

So in igore-shared case, dtb is not required here ?
Maybe I could add a flag in struct Rom to set it when the rom is created by
rom_add_blob_fixed_as()? And in rom_reset, just bypass this rom when
in_incoming_migration && ignore_shared

[+ Ard Biesheuvel]
Best Regard,
Catherine

>
> Thanks,

>
> --
> Peter Xu