Re: [PATCH] KVM: move DR register access handling into generic code.

[Gleb Natapov <gleb@redhat.com>]

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.