[Qemu-devel] Re: [PATCH 2/4] KVM: Rework VCPU state writeback API

[Marcelo Tosatti]

On Thu, Mar 11, 2010 at 10:32:50AM +0200, Avi Kivity wrote:
> On 03/02/2010 02:14 AM, Marcelo Tosatti wrote:
> >On Mon, Mar 01, 2010 at 07:10:30PM +0100, Jan Kiszka wrote:
> >>This grand cleanup drops all reset and vmsave/load related
> >>synchronization points in favor of four(!) generic hooks:
> >>
> >>- cpu_synchronize_all_states in qemu_savevm_state_complete
> >>   (initial sync from kernel before vmsave)
> >>- cpu_synchronize_all_post_init in qemu_loadvm_state
> >>   (writeback after vmload)
> >>- cpu_synchronize_all_post_init in main after machine init
> >>- cpu_synchronize_all_post_reset in qemu_system_reset
> >>   (writeback after system reset)
> >>
> >>These writeback points + the existing one of VCPU exec after
> >>cpu_synchronize_state map on three levels of writeback:
> >>
> >>- KVM_PUT_RUNTIME_STATE (during runtime, other VCPUs continue to run)
> >>- KVM_PUT_RESET_STATE   (on synchronous system reset, all VCPUs stopped)
> >>- KVM_PUT_FULL_STATE    (on init or vmload, all VCPUs stopped as well)
> >>
> >>This level is passed to the arch-specific VCPU state writing function
> >>that will decide which concrete substates need to be written. That way,
> >>no writer of load, save or reset functions that interact with in-kernel
> >>KVM states will ever have to worry about synchronization again. That
> >>also means that a lot of reasons for races, segfaults and deadlocks are
> >>eliminated.
> >>
> >>cpu_synchronize_state remains untouched, just as Anthony suggested. We
> >>continue to need it before reading or writing of VCPU states that are
> >>also tracked by in-kernel KVM subsystems.
> >>
> >>Consequently, this patch removes many cpu_synchronize_state calls that
> >>are now redundant, just like remaining explicit register syncs.
> >>
> >>Signed-off-by: Jan Kiszka<address@hidden>
> >Jan,
> >
> >This patch breaks system reset of WinXP.32 install (more easily
> >reproducible without iothread enabled).
> >
> 
> What's the conclusion here?  The patch is innocent of the regression?

Yes, it is. The problem was caused by a recent seabios change, now
fixed.