From mboxrd@z Thu Jan  1 00:00:00 1970
From: Gleb Natapov <gleb@redhat.com>
Subject: Re: [PATCH] KVM: move DR register access handling into generic
 code.
Date: Mon, 12 Apr 2010 20:26:03 +0300
Message-ID: <20100412172603.GA17382@redhat.com>
References: <20100412122712.GC23554@redhat.com>
 <4BC3341E.2050108@redhat.com>
 <4BC345CE.2060301@siemens.com>
 <20100412165218.GA27579@redhat.com>
 <4BC3503E.7020106@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Jan Kiszka <jan.kiszka@siemens.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"mtosatti@redhat.com" <mtosatti@redhat.com>
To: Avi Kivity <avi@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:41764 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752761Ab0DLR0G (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 12 Apr 2010 13:26:06 -0400
Content-Disposition: inline
In-Reply-To: <4BC3503E.7020106@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Mon, Apr 12, 2010 at 07:54:22PM +0300, Avi Kivity wrote:
> On 04/12/2010 07:52 PM, Gleb Natapov wrote:
> >On Mon, Apr 12, 2010 at 06:09:50PM +0200, Jan Kiszka wrote:
> >>Avi Kivity wrote:
> >>>On 04/12/2010 03:27 PM, Gleb Natapov wrote:
> >>>>Currently both SVM and VMX have their own DR handling code. Move it to
> >>>>x86.c.
> >>>>
> >>>>
> >>>The standard process is to make them identical first and finally merge
> >>>identical code, but I guess we can skip it in this case (Jan?)
> >>Looks OK, I would just...
> >>
> >>>+int kvm_set_dr(struct kvm_vcpu *vcpu, int dr, unsigned long val)
> >>>+{
> >>>+	switch (dr) {
> >>>+	case 0 ... 3:
> >>>+		vcpu->arch.db[dr] = val;
> >>>+		if (!(vcpu->guest_debug&  KVM_GUESTDBG_USE_HW_BP))
> >>>+			vcpu->arch.eff_db[dr] = val;
> >>>+		break;
> >>>+	case 4:
> >>>+		if (kvm_read_cr4_bits(vcpu, X86_CR4_DE)) {
> >>>+			kvm_queue_exception(vcpu, UD_VECTOR);
> >>>+			return 1;
> >>>+		}
> >>>+		/* fall through */
> >>>+	case 6:
> >>>+		if (val&  0xffffffff00000000ULL) {
> >>>+			kvm_inject_gp(vcpu, 0);
> >>>+			return -1;
> >>...either return 1 or -1 on failure.
> >>
> >This is a preparation for the future :) Exception injection will be
> >moved outside the function so different return values are needed to
> >distinguish different error conditions. Moving exception injection
> >out is needed so emulator can call the function without it injecting
> >exception behind emulators back.
> >
> 
> Ugh, use symbolic names please.
All others (set_msr()/set_cr()) can cause only one type of exception
each, so returning 1 for exception, 0 for success seams logical (and this
is what set_msr() already does). I didn't want to have much different
calling convention for set_dr(). It's not like we will have one more
exception type that mov dr can generate suddenly :)

>                                   And I think that can be deferred
> for that future patch.
> 
Agree, I'll repost with 1 for error in all cases.

--
			Gleb.