On 13.02.20 17:38, Shameerali Kolothum Thodi wrote: > > >> -----Original Message----- >> From: David Hildenbrand [mailto:da...@redhat.com] >> Sent: 12 February 2020 18:21 >> 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; >> dgilb...@redhat.com; Juan Jose Quintela Carreira <quint...@redhat.com> >> Subject: Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback > > [...] > >>> Hmm..it breaks x86 + seabios boot. The issue is seabios expects RSDP in FSEG >>> memory. With the above proposed change, RSDP will be aligned to PAGE_SIZE >> and >>> seabios mem allocation for RSDP fails at, >>> >>> >> https://github.com/coreboot/seabios/blob/master/src/fw/romfile_loader.c#L8 >> 5 >>> >>> To get pass the above, I changed "alloc_fseg" flag to false in >>> build_rsdp(), but >>> seabios seems to make the assumption that RSDP has to be placed in FSEG >> memory >>> here, >>> https://github.com/coreboot/seabios/blob/master/src/fw/biostables.c#L126 >>> >>> So doesn’t look like there is an easy fix for this without changing the >>> seabios >> code. >>> >>> Between, OVMF works fine with the aligned size on x86. >>> >>> One thing we can do is treat the RSDP case separately or only use the >>> aligned >>> page size for "etc/acpi/tables" as below, > > [...] > >>> >>> Thoughts? >> >> I don't think introducing memory_region_get_used_length() is a >> good idea. I also dislike, that the ram block size can differ >> to the memory region size. I wasn't aware of that condition, sorry! > > Right. They can differ in size and is the case here. > >> Making the memory region always store an aligned size might break other use >> cases. >> >> Summarizing the issue: >> 1. Memory regions contain ram blocks with a different size, if the size is >> not properly aligned. While memory regions can have an unaligned size, >> ram blocks can't. This is true when creating resizable memory region with >> an unaligned size. >> 2. When resizing a ram block/memory region, the size of the memory region >> is set to the aligned size. The callback is called with the aligned size. >> The unaligned piece is lost. >> 3. When migrating, we migrate the aligned size. >> >> >> What about something like this: (untested) > > Thanks for that. I had a go with the below patch and it indeed fixes the issue > of callback not being called on resize. But the migration fails with the below > error, > > For x86 > --------- > qemu-system-x86_64: Unknown combination of migration flags: 0x14 > qemu-system-x86_64: error while loading state for instance 0x0 of device 'ram' > qemu-system-x86_64: load of migration failed: Invalid argument > > For arm64 > -------------- > qemu-system-aarch64: Received an unexpected compressed page > qemu-system-aarch64: error while loading state for instance 0x0 of device > 'ram' > qemu-system-aarch64: load of migration failed: Invalid argument > > I haven’t debugged this further but looks like there is a corruption > happening. > Please let me know if you have any clue.
The issue is qemu_put_be64(f, ram_bytes_total_common(true) | RAM_SAVE_FLAG_MEM_SIZE) The total ram size we store must be page aligned, otherwise it will be detected as flags. Hm ... maybe we can round it up ... -- Thanks, David / dhildenb