On 13.02.20 17:59, David Hildenbrand wrote: > 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 ... >
I'm afraid we can't otherwise we will run into issues in ram_load_precopy(). Hm ... -- Thanks, David / dhildenb
