Avi Kivity wrote: > On 02/14/2010 12:34 PM, Gleb Natapov wrote: >>>> >>>> event_exit_inst_len is only used for event reinjection. Since event >>>> intercepted here will not be reinjected why updating >>>> event_exit_inst_len >>>> is needed here? >>>> >>> In guest debugging mode a #BP exception is always reported to user space >>> to find out what caused it. If it was the guest itself, the exception is >>> reinjected, on older kernels via KVM_SET_GUEST_DEBUG and since 2.6.33 >>> via KVM_SET_VCPU_EVENTS (the latter requires some qemu patch that I will >>> post later). >>> >>> As we currently do not update event_exit_inst_len on #BP exits, >>> reinjecting fails unless event_exit_inst_len happens to be 1 from some >>> other exit. >>> >>> >> Hmm, how does it work on SVM then where we do not have >> event_exit_inst_len so execution will resume on the same rip that caused >> #BP after event reinjection? >> >> > > Note, newer AMDs do have such a field (nRIP, 0xC8). We need to support > older machines, though. > Nice. [ /me goes updating his manual - September 07... ] Actually, emulation via single-stepping should work as only #DB clears the TF on entry. So this draft may fly (nothing to test on at hand): diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 52f78dd..450c1e7 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -109,6 +109,7 @@ struct vcpu_svm { struct nested_state nested; bool nmi_singlestep; + bool bp_singlestep; }; /* enable NPT for AMD64 and X86 with PAE */ @@ -244,6 +245,19 @@ static void svm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr, if (nested_svm_check_exception(svm, nr, has_error_code, error_code)) return; + if (nr == BP_VECTOR && vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) { + /* + * Temporarily disable BP interception so that the trap is + * properly delivered to the guest (if we left it on, SVM + * would push the wrong IP on the stack). + * We single-step into theexception handler in order to + * reenble interception ASAP. + */ + svm->vmcb->control.intercept_exceptions &= ~(1 << BP_VECTOR); + svm->bp_singlestep = true; + svm->vmcb->save.rflags |= (X86_EFLAGS_TF | X86_EFLAGS_RF); + } + svm->vmcb->control.event_inj = nr | SVM_EVTINJ_VALID | (has_error_code ? SVM_EVTINJ_VALID_ERR : 0) @@ -1083,7 +1097,8 @@ static void update_db_intercept(struct kvm_vcpu *vcpu) (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)) svm->vmcb->control.intercept_exceptions |= 1 << DB_VECTOR; - if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) + if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP && + !svm->bp_singlestep) svm->vmcb->control.intercept_exceptions |= 1 << BP_VECTOR; } else @@ -1214,7 +1229,7 @@ static int db_interception(struct vcpu_svm *svm) if (!(svm->vcpu.guest_debug & (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)) && - !svm->nmi_singlestep) { + !svm->nmi_singlestep && !svm->bp_singlestep) { kvm_queue_exception(&svm->vcpu, DB_VECTOR); return 1; } @@ -1227,6 +1242,14 @@ static int db_interception(struct vcpu_svm *svm) update_db_intercept(&svm->vcpu); } + if (svm->bp_singlestep) { + svm->bp_singlestep = false; + if (!(svm->vcpu.guest_debug & KVM_GUESTDBG_SINGLESTEP)) + svm->vmcb->save.rflags &= + ~(X86_EFLAGS_TF | X86_EFLAGS_RF); + update_db_intercept(&svm->vcpu); + } + if (svm->vcpu.guest_debug & (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)){ kvm_run->exit_reason = KVM_EXIT_DEBUG; But it requires a bit more work as I just realized that already nmi_singlestep could cause spurious #DB reports to user space that may propagate to the guest in the end. Jan
Attachment:
signature.asc
Description: OpenPGP digital signature