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.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
