From mboxrd@z Thu Jan  1 00:00:00 1970
From: Gleb Natapov <gleb@redhat.com>
Subject: Re: [PATCH v3 3/4] KVM: SVM: Fix CPL updates
Date: Mon, 6 Feb 2012 11:57:46 +0200
Message-ID: <20120206095746.GW23536@redhat.com>
References: <1328293765-13663-1-git-send-email-kwolf@redhat.com>
 <1328293765-13663-4-git-send-email-kwolf@redhat.com>
 <20120205111617.GT23536@redhat.com>
 <4F2F9AEB.9080406@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org, joerg.roedel@amd.com,
	yoshikawa.takuya@oss.ntt.co.jp, avi@redhat.com, mtosatti@redhat.com
To: Kevin Wolf <kwolf@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:10769 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753368Ab2BFJ5u (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 6 Feb 2012 04:57:50 -0500
Content-Disposition: inline
In-Reply-To: <4F2F9AEB.9080406@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Mon, Feb 06, 2012 at 10:18:35AM +0100, Kevin Wolf wrote:
> Am 05.02.2012 12:16, schrieb Gleb Natapov:
> > On Fri, Feb 03, 2012 at 07:29:24PM +0100, Kevin Wolf wrote:
> >> Keep CPL at 0 in real mode and at 3 in VM86. In protected/long mode, use
> >> RPL rather than DPL of the code segment.
> >>
> >> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> >> ---
> >>  arch/x86/kvm/svm.c |   19 ++++++++++++++++---
> >>  1 files changed, 16 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> >> index 6a977c1..4124a7e 100644
> >> --- a/arch/x86/kvm/svm.c
> >> +++ b/arch/x86/kvm/svm.c
> >> @@ -1263,6 +1263,21 @@ static void svm_vcpu_put(struct kvm_vcpu *vcpu)
> >>  		wrmsrl(host_save_user_msrs[i], svm->host_user_msrs[i]);
> >>  }
> >>  
> >> +static void svm_update_cpl(struct kvm_vcpu *vcpu)
> >> +{
> >> +	struct vcpu_svm *svm = to_svm(vcpu);
> >> +	int cpl;
> >> +
> >> +	if (!is_protmode(vcpu))
> >> +		cpl = 0;
> >> +	else if (svm->vmcb->save.rflags & X86_EFLAGS_VM)
> >> +		cpl = 3;
> >> +	else
> >> +		cpl = svm->vmcb->save.cs.selector & 0x3;
> >> +
> >> +	svm->vmcb->save.cpl = cpl;
> >> +}
> >> +
> > As you probably know already I think cpl should be updated in
> > svm_get_rflags() too. With current patch restoring CS segment
> > register before rflags register after migration may cause cpl
> > to get wrong value for instance.
> 
> I think you mean set_rflags rather than get_rflags?
> 
Uh, yes.

> Patch 4 adds this with the set_rflags emulator callback.
> 
Sorry, missed that, I expected it to be in this patch for some reason.

I think calling svm_update_cpl() from set_rflags() is incorrect though.
svm_update_cpl() checks cr0 so if guest does "mov 1, %cr0; popf" and
popf happens to be emulated it will change cpl to cs&3 which is
incorrect.

--
			Gleb.