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.kiszka@xxxxxxxxxxx> > >>> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html