From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH v3 4/4] KVM: x86 emulator: Allow PM/VM86 switch during task switch Date: Mon, 06 Feb 2012 12:32:10 +0200 Message-ID: <4F2FAC2A.8000101@redhat.com> References: <1328293765-13663-1-git-send-email-kwolf@redhat.com> <1328293765-13663-5-git-send-email-kwolf@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: Kevin Wolf Return-path: Received: from mx1.redhat.com ([209.132.183.28]:53321 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754161Ab2BFKcQ (ORCPT ); Mon, 6 Feb 2012 05:32:16 -0500 In-Reply-To: <1328293765-13663-5-git-send-email-kwolf@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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...). 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. -- error compiling committee.c: too many arguments to function