> -----Original Message----- > From: David Hildenbrand [mailto:da...@redhat.com] > Sent: 05 February 2020 16:41 > To: Shameerali Kolothum Thodi <shameerali.kolothum.th...@huawei.com>; > Igor Mammedov <imamm...@redhat.com> > Cc: peter.mayd...@linaro.org; xiaoguangrong.e...@gmail.com; > m...@redhat.com; shannon.zha...@gmail.com; qemu-devel@nongnu.org; > xuwei (O) <xuw...@huawei.com>; Linuxarm <linux...@huawei.com>; > eric.au...@redhat.com; qemu-...@nongnu.org; ler...@redhat.com > Subject: Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback > > >> Oh, and one more reason why the proposal in this patch is inconsistent: > >> > >> When migrating resizable memory regions (RAM_SAVE_FLAG_MEM_SIZE) > we > >> store the block->used_length (ram_save_setup()) and use that value to > >> resize the region on the target (ram_load_precopy() -> qemu_ram_resize()). > >> > >> This will be the value the callback will be called with. Page aligned. > >> > > > > Sorry, I didn’t quite get that point and not sure how "req_length" approach > > will affect the migration. > > The issue is that on migration, you will lose the sub-page size either > way. So your callback will be called > - on the migration source with a sub-page size (via > memory_region_ram_resize() from e.g., hw/i386/acpi-build.c) > - on the migration target with a page-aligned size (via > qemu_ram_resize() from migration/ram.c) > > So this is inconsistent, especially when migrating.
Thanks for explaining. I tried to add some debug prints to further understand what actually happens during migration case. Guest-source with initial one nvdimm ---------------------------------------------------- -object memory-backend-ram,id=mem1,size=1G \ -device nvdimm,id=dimm1,memdev=mem1 \ fw_cfg_add_file_callback: filename etc/boot-fail-wait size 0x4 fw_cfg_add_file_callback: filename etc/acpi/nvdimm-mem size 0x1000 fw_cfg_add_file_callback: filename etc/acpi/tables size 0x55f4 fw_cfg_add_file_callback: filename etc/table-loader size 0xd00 fw_cfg_add_file_callback: filename etc/tpm/log size 0x0 fw_cfg_add_file_callback: filename etc/acpi/rsdp size 0x24 fw_cfg_add_file_callback: filename etc/smbios/smbios-tables size 0x104 fw_cfg_add_file_callback: filename etc/smbios/smbios-anchor size 0x18 fw_cfg_modify_file: filename bootorder len 0x0 fw_cfg_add_file_callback: filename bootorder size 0x0 fw_cfg_modify_file: filename bios-geometry len 0x0 fw_cfg_add_file_callback: filename bios-geometry size 0x0 fw_cfg_modify_file: filename etc/acpi/tables len 0x55f4 fw_cfg_modify_file: filename etc/acpi/rsdp len 0x24 fw_cfg_modify_file: filename etc/table-loader len 0xd00 .... hot add another nvdimm device, (qemu) object_add memory-backend-ram,id=mem2,size=1G (qemu) device_add nvdimm,id=dimm2,memdev=mem2 root@ubuntu:/# cat /dev/pmem pmem0 pmem1 Guest-target with two nvdimms ----------------------------------------------- -object memory-backend-ram,id=mem1,size=1G \ -device nvdimm,id=dimm1,memdev=mem1 \ -object memory-backend-ram,id=mem2,size=1G \ -device nvdimm,id=dimm2,memdev=mem2 \ fw_cfg_add_file_callback: filename etc/boot-fail-wait size 0x4 fw_cfg_add_file_callback: filename etc/acpi/nvdimm-mem size 0x1000 fw_cfg_add_file_callback: filename etc/acpi/tables size 0x56ac fw_cfg_add_file_callback: filename etc/table-loader size 0xd00 fw_cfg_add_file_callback: filename etc/tpm/log size 0x0 fw_cfg_add_file_callback: filename etc/acpi/rsdp size 0x24 fw_cfg_add_file_callback: filename etc/smbios/smbios-tables size 0x104 fw_cfg_add_file_callback: filename etc/smbios/smbios-anchor size 0x18 fw_cfg_modify_file: filename bootorder len 0x0 fw_cfg_add_file_callback: filename bootorder size 0x0 fw_cfg_modify_file: filename bios-geometry len 0x0 fw_cfg_add_file_callback: filename bios-geometry size 0x0 Initiate migration Source --> Target, ram_load_precopy: Ram blk mach-virt.ram length 0x100000000 used_length 0x100000000 ram_load_precopy: Ram blk mem1 length 0x40000000 used_length 0x40000000 ram_load_precopy: Ram blk mem2 length 0x40000000 used_length 0x40000000 ram_load_precopy: Ram blk virt.flash0 length 0x4000000 used_length 0x4000000 ram_load_precopy: Ram blk virt.flash1 length 0x4000000 used_length 0x4000000 ram_load_precopy: Ram blk /rom@etc/acpi/tables length 0x6000 used_length 0x6000 ram_load_precopy: Ram blk 0000:00:01.0/virtio-net-pci.rom length 0x40000 used_length 0x40000 ram_load_precopy: Ram blk /rom@etc/table-loader length 0x1000 used_length 0x1000 ram_load_precopy: Ram blk /rom@etc/acpi/rsdp length 0x1000 used_length 0x1000 root@ubuntu:/# cat /dev/pmem pmem0 pmem1 From the logs, it looks like the ram_load_precopy() --> qemu_ram_resize() is not called as length == used_length and both seems to be page aligned values. And from https://github.com/qemu/qemu/blob/master/migration/ram.c#L3421 qemu_ram_resize() is called with length if length != used_length. Of course my knowledge on Qemu migration is very limited and maybe I am missing something here. But I am trying to see under what circumstances qemu_ram_resize() will get invoked with a page aligned size that will be different to the size used in the FWCfgEntry . Please let me know, Much appreciated, Shameer > Is there a way to get access to the sub-page size without passing it > through the callback? > > Like in fw_cfg_modify_file() do some fancy lookup and use the sub-page > size instead of the passed size? (might have to be stored somewhere and > refetched - and migrated) > > -- > Thanks, > > David / dhildenb
