[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback
From: |
David Hildenbrand |
Subject: |
Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback |
Date: |
Thu, 13 Feb 2020 17:59:28 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 |
On 13.02.20 17:38, Shameerali Kolothum Thodi wrote:
>
>
>> -----Original Message-----
>> From: David Hildenbrand [mailto:address@hidden]
>> Sent: 12 February 2020 18:21
>> To: Shameerali Kolothum Thodi <address@hidden>;
>> Igor Mammedov <address@hidden>
>> Cc: address@hidden; address@hidden;
>> address@hidden; address@hidden; address@hidden;
>> xuwei (O) <address@hidden>; Linuxarm <address@hidden>;
>> address@hidden; address@hidden; address@hidden;
>> address@hidden; Juan Jose Quintela Carreira <address@hidden>
>> 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
- Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, (continued)
- Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, David Hildenbrand, 2020/02/06
- RE: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, Shameerali Kolothum Thodi, 2020/02/06
- Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, David Hildenbrand, 2020/02/06
- RE: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, Shameerali Kolothum Thodi, 2020/02/07
- Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, David Hildenbrand, 2020/02/10
- RE: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, Shameerali Kolothum Thodi, 2020/02/10
- Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, David Hildenbrand, 2020/02/10
- RE: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, Shameerali Kolothum Thodi, 2020/02/12
- Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, David Hildenbrand, 2020/02/12
- RE: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, Shameerali Kolothum Thodi, 2020/02/13
- Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback,
David Hildenbrand <=
- Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, David Hildenbrand, 2020/02/13
- RE: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, Shameerali Kolothum Thodi, 2020/02/28
- Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, David Hildenbrand, 2020/02/28