From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Wolf Subject: Re: [PATCH v3 4/4] KVM: x86 emulator: Allow PM/VM86 switch during task switch Date: Mon, 06 Feb 2012 11:54:19 +0100 Message-ID: <4F2FB15B.8000508@redhat.com> References: <1328293765-13663-1-git-send-email-kwolf@redhat.com> <1328293765-13663-5-git-send-email-kwolf@redhat.com> <4F2FAC2A.8000101@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, gleb@redhat.com, joerg.roedel@amd.com, yoshikawa.takuya@oss.ntt.co.jp, mtosatti@redhat.com To: Avi Kivity Return-path: Received: from mx1.redhat.com ([209.132.183.28]:56743 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753718Ab2BFKu4 (ORCPT ); Mon, 6 Feb 2012 05:50:56 -0500 In-Reply-To: <4F2FAC2A.8000101@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Am 06.02.2012 11:32, schrieb Avi Kivity: > On 02/03/2012 08:29 PM, Kevin Wolf wrote: >> Task switches can switch between Protected Mode and VM86. The current >> mode must be updated during the task switch emulation so that the new >> segment selectors are interpreted correctly. >> >> In order to let privilege checks succeed, rflags needs to be updated in >> the vcpu struct as this causes a CPL update. >> >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> index 4124a7e..38d5ef3 100644 >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -1286,6 +1286,7 @@ static unsigned long svm_get_rflags(struct kvm_vcpu *vcpu) >> static void svm_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags) >> { >> to_svm(vcpu)->vmcb->save.rflags = rflags; >> + svm_update_cpl(vcpu); >> } > > This can trigger the (cs&3)!=0 && (cr0&1) != 0 problem, right after we > enter protected mode. > > What this code does is slave the cpl to other x86 state (copying vmx), > whereas in reality it is separate state (with independent existence from > the mov cr0 instruction until the next far jump...). Yes, as discussed in v2, eventually we'll want to call set_cpl() rather than set_rflags() during the task switch (I left a TODO comment there). Then the not quite correct update in svm_set_rflags can be removed again. > We can fix this with > > if ((save.rflags ^ rflags) & EFLAGS_VM) > svm_update_cpl(vcpu) > > but that leaves us with an unupdated cpl after live migration (well, it > will update after loading segment registers). Both are bad, but I think > my version is less bad - live migration is broken anyway in this window, > since we don't save the cpl. Right, it just leaves broken what is already broken. If the rest of the series is okay now, I'll resend with a check if rflags.VM has changed. Kevin