Re: [PATCH] KVM: VMX: Update instruction length on intercepted BP

Jan Kiszka
2010-02-14T12:39:48Z

[-- Attachment #1: Type: text/plain, Size: 3811 bytes --]

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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]