qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 0/6] save/restore on Xen


From: Stefano Stabellini
Subject: Re: [Qemu-devel] [PATCH v4 0/6] save/restore on Xen
Date: Mon, 23 Jan 2012 16:46:41 +0000
User-agent: Alpine 2.00 (DEB 1167 2008-08-23)

On Mon, 23 Jan 2012, Anthony Liguori wrote:
> On 01/23/2012 04:47 AM, Stefano Stabellini wrote:
> > On Fri, 20 Jan 2012, Jan Kiszka wrote:
> >> On 2012-01-20 18:20, Stefano Stabellini wrote:
> >>> Hi all,
> >>> this is the fourth version of the Xen save/restore patch series.
> >>> We have been discussing this issue for quite a while on #qemu and
> >>> qemu-devel:
> >>>
> >>>
> >>> http://marc.info/?l=qemu-devel&m=132346828427314&w=2
> >>> http://marc.info/?l=qemu-devel&m=132377734605464&w=2
> >>>
> >>>
> >>> A few different approaches were proposed to achieve the goal
> >>> of a working save/restore with upstream Qemu on Xen, however after
> >>> prototyping some of them I came up with yet another solution, that I
> >>> think leads to the best results with the less amount of code
> >>> duplications and ugliness.
> >>> Far from saying that this patch series is an example of elegance and
> >>> simplicity, but it is closer to acceptable anything else I have seen so
> >>> far.
> >>>
> >>> What's new is that Qemu is going to keep track of its own physmap on
> >>> xenstore, so that Xen can be fully aware of the changes Qemu makes to
> >>> the guest's memory map at any time.
> >>> This is all handled by Xen or Xen support in Qemu internally and can be
> >>> used to solve our save/restore framebuffer problem.
> >>>
> >>> > From the Qemu common code POV, we still need to avoid saving the guest's
> >>> ram when running on Xen, and we need to avoid resetting the videoram on
> >>> restore (that is a benefit to the generic Qemu case too, because it
> >>> saves few cpu cycles).
> >>
> >> For my understanding: Refraining from the memset is required as the
> >> already restored vram would then be overwritten?
> >
> > Yep
> >
> >> Or what is the ordering
> >> of init, RAM restore, and initial device reset now?
> >
> > RAM restore (done by Xen)
> >
> > physmap rebuild (done by xen_hvm_init in qemu)
> > pc_init()
> > qemu_system_reset()
> > load_vmstate()
> 
> That's your problem.  You don't want to do load_vmstate().  You just want to 
> load the device model, not RAM.

True


> Why not introduce new Xen specific commands like I suggested on IRC?

Introducing a Xen specific command is not an issue, but I didn't want to
duplicate all the functionalities currently in savevm.c.


> You should have a separate load_device_state() function and mark anything 
> that 
> is RAM as RAM when doing savevm registration.  Better yet, mark devices as 
> devices since that's what you really care about.

I dropped this approach because I thought it causes too much code
duplication.
However, following your suggestion, if I add a generic "device" flag in
SaveStateEntry and implement a generic qemu_save_device_state in
savevm.c, I believe that the duplication of code would be small.
And patch #1 could go away.


However the issue of patch #4, "do not reset videoram on resume", still
remains: no matter what parameter I pass to Qemu, if qemu_system_reset
is called on resume the videoram is going to be overwritten by 0xff.
In this regard, don't you think it would be advantageous to Qemu in
general not to reset the videram in resume? It can be pretty large, so
it is a significant waste of a memset.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]