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

2010-03-11 Thread Avi Kivity

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 Kiszkajan.kis...@siemens.com
 

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?

--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-03-11 Thread 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 Kiszkajan.kis...@siemens.com
 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.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-03-09 Thread Anthony Liguori

On 03/08/2010 02:33 PM, Marcelo Tosatti wrote:

On Fri, Mar 05, 2010 at 09:37:26PM -0500, Kevin O'Connor wrote:
   

On Thu, Mar 04, 2010 at 03:35:52PM -0300, Marcelo Tosatti wrote:
 

On Thu, Mar 04, 2010 at 12:58:58AM -0500, Kevin O'Connor wrote:
   

On Thu, Mar 04, 2010 at 01:21:12AM -0300, Marcelo Tosatti wrote:
 

The regression seems to be caused by seabios commit d7e998f. Kevin, the
failure can be seen on the attached screenshot, which happens on the
first reboot of WinXP 32 installation (after copying files etc).
   

Sorry - I also noticed a bug in that commit recently.  I pushed the
fix I had in my local tree.
 

Thanks, it does fix the issue here. Anthony can you please update
seabios?
   

Neither commit d7e998f nor the fix 8f469b96 are on the SeaBIOS stable
branch.  Is qemu ready to pull in bigger changes now?
 

Anthony pulls in seabios master into qemu.git master periodically.
   


We should be up to date now FWIW.

Regards,

Anthony Liguori

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-03-08 Thread Marcelo Tosatti
On Fri, Mar 05, 2010 at 09:37:26PM -0500, Kevin O'Connor wrote:
 On Thu, Mar 04, 2010 at 03:35:52PM -0300, Marcelo Tosatti wrote:
  On Thu, Mar 04, 2010 at 12:58:58AM -0500, Kevin O'Connor wrote:
   On Thu, Mar 04, 2010 at 01:21:12AM -0300, Marcelo Tosatti wrote:
The regression seems to be caused by seabios commit d7e998f. Kevin, the
failure can be seen on the attached screenshot, which happens on the
first reboot of WinXP 32 installation (after copying files etc).
   
   Sorry - I also noticed a bug in that commit recently.  I pushed the
   fix I had in my local tree.
  
  Thanks, it does fix the issue here. Anthony can you please update
  seabios?
 
 Neither commit d7e998f nor the fix 8f469b96 are on the SeaBIOS stable
 branch.  Is qemu ready to pull in bigger changes now?

Anthony pulls in seabios master into qemu.git master periodically.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-03-05 Thread Kevin O'Connor
On Thu, Mar 04, 2010 at 03:35:52PM -0300, Marcelo Tosatti wrote:
 On Thu, Mar 04, 2010 at 12:58:58AM -0500, Kevin O'Connor wrote:
  On Thu, Mar 04, 2010 at 01:21:12AM -0300, Marcelo Tosatti wrote:
   The regression seems to be caused by seabios commit d7e998f. Kevin, the
   failure can be seen on the attached screenshot, which happens on the
   first reboot of WinXP 32 installation (after copying files etc).
  
  Sorry - I also noticed a bug in that commit recently.  I pushed the
  fix I had in my local tree.
 
 Thanks, it does fix the issue here. Anthony can you please update
 seabios?

Neither commit d7e998f nor the fix 8f469b96 are on the SeaBIOS stable
branch.  Is qemu ready to pull in bigger changes now?

-Kevin
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-03-04 Thread Marcelo Tosatti
On Thu, Mar 04, 2010 at 12:58:58AM -0500, Kevin O'Connor wrote:
 On Thu, Mar 04, 2010 at 01:21:12AM -0300, Marcelo Tosatti wrote:
  The regression seems to be caused by seabios commit d7e998f. Kevin, the
  failure can be seen on the attached screenshot, which happens on the
  first reboot of WinXP 32 installation (after copying files etc).
 
 Sorry - I also noticed a bug in that commit recently.  I pushed the
 fix I had in my local tree.

Thanks, it does fix the issue here. Anthony can you please update
seabios?

TIA

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-03-03 Thread Marcelo Tosatti
On Tue, Mar 02, 2010 at 11:29:10PM -0300, Marcelo Tosatti wrote:
 On Tue, Mar 02, 2010 at 05:31:09PM +0100, Jan Kiszka wrote:
  Marcelo Tosatti wrote:
   On Tue, Mar 02, 2010 at 09:00:04AM +0100, Jan Kiszka wrote:
   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 jan.kis...@siemens.com
   Jan,
  
   This patch breaks system reset of WinXP.32 install (more easily
   reproducible without iothread enabled).
  
   Screenshot attached.
  
   Strange - no issues with qemu-kvm? Any special command line switch? /me
   goes scrounging for some installation XP32 CD in the meantime...
   
   No issues with qemu-kvm. Could not spot anything obvious.
   
  
  And, of course, my WinXP installation did not trigger any reset issue,
  even in non-iothreaded mode. :(

The regression seems to be caused by seabios commit d7e998f. Kevin, the
failure can be seen on the attached screenshot, which happens on the
first reboot of WinXP 32 installation (after copying files etc).

attachment: uqmaster-failure.png

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

2010-03-03 Thread Kevin O'Connor
On Thu, Mar 04, 2010 at 01:21:12AM -0300, Marcelo Tosatti wrote:
 The regression seems to be caused by seabios commit d7e998f. Kevin, the
 failure can be seen on the attached screenshot, which happens on the
 first reboot of WinXP 32 installation (after copying files etc).

Sorry - I also noticed a bug in that commit recently.  I pushed the
fix I had in my local tree.

-Kevin
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-03-02 Thread Jan Kiszka
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 jan.kis...@siemens.com
 
 Jan,
 
 This patch breaks system reset of WinXP.32 install (more easily
 reproducible without iothread enabled).
 
 Screenshot attached.
 

Strange - no issues with qemu-kvm? Any special command line switch? /me
goes scrounging for some installation XP32 CD in the meantime...

Jan



signature.asc
Description: OpenPGP digital signature


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

2010-03-02 Thread Marcelo Tosatti
On Tue, Mar 02, 2010 at 09:00:04AM +0100, Jan Kiszka wrote:
 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 jan.kis...@siemens.com
  
  Jan,
  
  This patch breaks system reset of WinXP.32 install (more easily
  reproducible without iothread enabled).
  
  Screenshot attached.
  
 
 Strange - no issues with qemu-kvm? Any special command line switch? /me
 goes scrounging for some installation XP32 CD in the meantime...

No issues with qemu-kvm. Could not spot anything obvious.

 
 Jan
 


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-03-02 Thread Jan Kiszka
Marcelo Tosatti wrote:
 On Tue, Mar 02, 2010 at 09:00:04AM +0100, Jan Kiszka wrote:
 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 jan.kis...@siemens.com
 Jan,

 This patch breaks system reset of WinXP.32 install (more easily
 reproducible without iothread enabled).

 Screenshot attached.

 Strange - no issues with qemu-kvm? Any special command line switch? /me
 goes scrounging for some installation XP32 CD in the meantime...
 
 No issues with qemu-kvm. Could not spot anything obvious.
 

And, of course, my WinXP installation did not trigger any reset issue,
even in non-iothreaded mode. :(

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-03-02 Thread Marcelo Tosatti
On Tue, Mar 02, 2010 at 05:31:09PM +0100, Jan Kiszka wrote:
 Marcelo Tosatti wrote:
  On Tue, Mar 02, 2010 at 09:00:04AM +0100, Jan Kiszka wrote:
  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 jan.kis...@siemens.com
  Jan,
 
  This patch breaks system reset of WinXP.32 install (more easily
  reproducible without iothread enabled).
 
  Screenshot attached.
 
  Strange - no issues with qemu-kvm? Any special command line switch? /me
  goes scrounging for some installation XP32 CD in the meantime...
  
  No issues with qemu-kvm. Could not spot anything obvious.
  
 
 And, of course, my WinXP installation did not trigger any reset issue,
 even in non-iothreaded mode. :(

Try kvm-autotest. I could not reproduce easily in hand run either.

This is not it, but still looks wrong:

In real mode emulation, kvm_get_sregs returns TR segment values of VMX
fields, while KVM caches them in vmx_vcpu-rmode.

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 14873b9..898173a 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1822,13 +1822,23 @@ static u64 vmx_get_segment_base(struct kvm_vcpu *vcpu, 
int seg)
 static void vmx_get_segment(struct kvm_vcpu *vcpu,
struct kvm_segment *var, int seg)
 {
+   struct vcpu_vmx *vmx = to_vmx(vcpu);
struct kvm_vmx_segment_field *sf = kvm_vmx_segment_fields[seg];
u32 ar;
 
+   if (vmx-rmode.vm86_active  seg == VCPU_SREG_TR) {
+   var-base = vmx-rmode.tr.base;
+   var-limit = vmx-rmode.tr.limit;
+   var-selector = vmx-rmode.tr.selector;
+   ar = vmx-rmode.tr.ar;
+   goto ar;
+   }
+
var-base = vmcs_readl(sf-base);
var-limit = vmcs_read32(sf-limit);
var-selector = vmcs_read16(sf-selector);
ar = vmcs_read32(sf-ar_bytes);
+ar:
if ((ar  AR_UNUSABLE_MASK)  !emulate_invalid_guest_state)
ar = 0;
var-type = ar  15;
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/4] KVM: Rework VCPU state writeback API

2010-03-01 Thread Jan Kiszka
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 jan.kis...@siemens.com
---
 exec.c|   17 -
 hw/apic.c |2 --
 hw/ppc_newworld.c |3 ---
 hw/ppc_oldworld.c |3 ---
 hw/s390-virtio.c  |1 -
 kvm-all.c |   19 +--
 kvm.h |   25 -
 savevm.c  |4 
 sysemu.h  |4 
 target-i386/kvm.c |2 +-
 target-i386/machine.c |   11 ---
 target-ppc/kvm.c  |2 +-
 target-ppc/machine.c  |4 
 target-s390x/kvm.c|3 +--
 vl.c  |   29 +
 15 files changed, 77 insertions(+), 52 deletions(-)

diff --git a/exec.c b/exec.c
index 8616ff9..50a2e46 100644
--- a/exec.c
+++ b/exec.c
@@ -518,21 +518,6 @@ void cpu_exec_init_all(unsigned long tb_size)
 
 #if defined(CPU_SAVE_VERSION)  !defined(CONFIG_USER_ONLY)
 
-static void cpu_common_pre_save(void *opaque)
-{
-CPUState *env = opaque;
-
-cpu_synchronize_state(env);
-}
-
-static int cpu_common_pre_load(void *opaque)
-{
-CPUState *env = opaque;
-
-cpu_synchronize_state(env);
-return 0;
-}
-
 static int cpu_common_post_load(void *opaque, int version_id)
 {
 CPUState *env = opaque;
@@ -550,8 +535,6 @@ static const VMStateDescription vmstate_cpu_common = {
 .version_id = 1,
 .minimum_version_id = 1,
 .minimum_version_id_old = 1,
-.pre_save = cpu_common_pre_save,
-.pre_load = cpu_common_pre_load,
 .post_load = cpu_common_post_load,
 .fields  = (VMStateField []) {
 VMSTATE_UINT32(halted, CPUState),
diff --git a/hw/apic.c b/hw/apic.c
index 87e7dc0..3c90f4c 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -938,8 +938,6 @@ static void apic_reset(void *opaque)
 APICState *s = opaque;
 int bsp;
 
-cpu_synchronize_state(s-cpu_env);
-
 bsp = cpu_is_bsp(s-cpu_env);
 s-apicbase = 0xfee0 |
 (bsp ? MSR_IA32_APICBASE_BSP : 0) | MSR_IA32_APICBASE_ENABLE;
diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
index bc86c85..d4f9013 100644
--- a/hw/ppc_newworld.c
+++ b/hw/ppc_newworld.c
@@ -167,9 +167,6 @@ static void ppc_core99_init (ram_addr_t ram_size,
 envs[i] = env;
 }
 
-/* Make sure all register sets take effect */
-cpu_synchronize_state(env);
-
 /* allocate RAM */
 ram_offset = qemu_ram_alloc(ram_size);
 cpu_register_physical_memory(0, ram_size, ram_offset);
diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c
index 04a7835..93c95ba 100644
--- a/hw/ppc_oldworld.c
+++ b/hw/ppc_oldworld.c
@@ -165,9 +165,6 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
 envs[i] = env;
 }
 
-/* Make sure all register sets take effect */
-cpu_synchronize_state(env);
-
 /* allocate RAM */
 if (ram_size  (2047  20)) {
 fprintf(stderr,
diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index 3582728..ad3386f 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -185,7 +185,6 @@ static void s390_init(ram_addr_t ram_size,
 exit(1);
 }
 
-cpu_synchronize_state(env);
 env-psw.addr = KERN_IMAGE_START;
 env-psw.mask = 0x00018000ULL;
 }
diff --git a/kvm-all.c b/kvm-all.c
index 2f7e33a..534ead0 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -156,10 +156,6 @@ static void kvm_reset_vcpu(void *opaque)
 CPUState *env = opaque;
 
 kvm_arch_reset_vcpu(env);
-if (kvm_arch_put_registers(env)) {
-fprintf(stderr, Fatal: kvm vcpu reset failed\n);
-

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

2010-03-01 Thread Marcelo Tosatti
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 jan.kis...@siemens.com

Jan,

This patch breaks system reset of WinXP.32 install (more easily
reproducible without iothread enabled).

Screenshot attached.

attachment: uqmaster-failure.png