Re: [Qemu-devel] [Xen-devel] [PATCH v5] xen: don't save/restore the physmap on VM save/restore

[Igor Druzhinin]

On 16/03/17 12:54, Igor Druzhinin wrote:
> On 16/03/17 12:26, Anthony PERARD wrote:
>> On Wed, Mar 15, 2017 at 04:01:19PM +0000, Igor Druzhinin wrote:
>>> Saving/restoring the physmap to/from xenstore was introduced to
>>> QEMU majorly in order to cover up the VRAM region restore issue.
>>> The sequence of restore operations implies that we should know
>>> the effective guest VRAM address *before* we have the VRAM region
>>> restored (which happens later). Unfortunately, in Xen environment
>>> VRAM memory does actually belong to a guest - not QEMU itself -
>>> which means the position of this region is unknown beforehand and
>>> can't be mapped into QEMU address space immediately.
>>>
>>> Previously, recreating xenstore keys, holding the physmap, by the
>>> toolstack helped to get this information in place at the right
>>> moment ready to be consumed by QEMU to map the region properly.
>>>
>>> The extraneous complexity of having those keys transferred by the
>>> toolstack and unnecessary redundancy prompted us to propose a
>>> solution which doesn't require any extra data in xenstore. The idea
>>> is to defer the VRAM region mapping till the point we actually know
>>> the effective address and able to map it. To that end, we initially
>>> just skip the mapping request for the framebuffer if we unable to
>>> map it now. Then, after the memory region restore phase, we perform
>>> the mapping again, this time successfully, and update the VRAM region
>>> metadata accordingly.
>>>
>>> Signed-off-by: Igor Druzhinin <address@hidden>
>>
>> I've tried to migrate a guest with this patch, but once migrated, the
>> screen is black (via VNC, keyboard is working fine).
>>
>> I haven't try to migrate a guest from QEMU without this patch to a QEMU
>> with it.
>>
> 
> Hmm. It works for me - I've tried to migrate between identical QEMUs
> with this patch on localhost. Save/restore also works fine.
> 
> What do you mean 'the screen is black'? Could you describe your actions
> so I could try to reproduce it?

Ok. I could track down the issue - starting from v4 the patch doesn't
work for cirrus. The reason is that post_load handler is different for
cirrus and doesn't call the parent handler from common vga code.

I manage to fix it by updating the corresponding handler by duplicating
the code. But is it a good solution? Would it be better to have the
common handler called in this function instead?

> 
> Igor
> 
> _______________________________________________
> Xen-devel mailing list
> address@hidden
> https://lists.xen.org/xen-devel
>