[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: |
Tue, 4 Feb 2020 20:05:04 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.3.1 |
On 04.02.20 17:44, David Hildenbrand wrote:
> On 04.02.20 16:23, Igor Mammedov wrote:
>> On Fri, 17 Jan 2020 17:45:16 +0000
>> Shameer Kolothum <address@hidden> wrote:
>>
>>> If ACPI blob length modifications happens after the initial
>>> virt_acpi_build() call, and the changed blob length is within
>>> the PAGE size boundary, then the revised size is not seen by
>>> the firmware on Guest reboot. The is because in the
>>> virt_acpi_build_update() -> acpi_ram_update() -> qemu_ram_resize()
>>> path, qemu_ram_resize() uses used_length (ram_block size which is
>>> aligned to PAGE size) and the "resize callback" to update the size
>>> seen by firmware is not getting invoked.
>>>
>>> Hence make sure callback is called if the new size is different
>>> from original requested size.
>>>
>>> Signed-off-by: Shameer Kolothum <address@hidden>
>>> ---
>>> Please find the previous discussions on this issue here,
>>> https://patchwork.kernel.org/patch/11174947/
>>>
>>> But this one attempts a different solution to fix it by introducing
>>> req_length var to RAMBlock struct.
>>>
>>
>> looks fine to me, so
>> Acked-by: Igor Mammedov <address@hidden>
>
> Thanks for CCing.
>
> This in fact collides with my changes ... but not severely :)
>
>>
>> CCing David who touches this area in his latest series for and
>> might have an opinion on how it should be handled.
>>
>
> So we are talking about sub-page size changes? I somewhat dislike
> storing "req_length" in ram blocks. Looks like sub-pages stuff does not
> belong there.
>
> Ram blocks only operate on page granularity. Ram block notifiers only
> operate on page granularity. Memory regions only operate on page
> granularity. Dirty bitmaps operate on page granularity. Especially,
> memory_region_size(mr) will always return aligned values.
>
> I think users/owner should deal with anything smaller manually if
> they really need it.
>
> What about always calling the resized() callback and letting the
> actual owner figure out if the size changed on sub-page granularity
> or not? (by looking up the size manually using some mechanism not glued to
> memory regions/ram blocks/whatever)
>
> diff --git a/exec.c b/exec.c
> index 67e520d18e..59d46cc388 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2130,6 +2130,13 @@ int qemu_ram_resize(RAMBlock *block, ram_addr_t
> newsize, Error **errp)
> newsize = HOST_PAGE_ALIGN(newsize);
>
> if (block->used_length == newsize) {
> + /*
> + * The owner might want to handle sub-page resizes. We only provide
> + * the aligned size - because ram blocks are always page aligned.
> + */
> + if (block->resized) {
> + block->resized(block->idstr, newsize, block->host);
> + }
> return 0;
> }
>
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.
--
Thanks,
David / dhildenb
- Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, Igor Mammedov, 2020/02/04
- Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, David Hildenbrand, 2020/02/04
- 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, Shameerali Kolothum Thodi, 2020/02/05
- Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, David Hildenbrand, 2020/02/05
- 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/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