From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 1/2] KVM: SVM: Emulate nRIP feature when reinjecting INT3 Date: Tue, 16 Feb 2010 09:02:59 +0100 Message-ID: <4B7A5133.7000009@web.de> References: <79170b70424440dfbede5b6eac85f734aa8785e8.1266257833.git.jan.kiszka@siemens.com> <20100216075259.GW2995@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig575F97C63C6CF93AB3F027BE" Cc: Avi Kivity , Marcelo Tosatti , kvm To: Gleb Natapov Return-path: Received: from fmmailgate03.web.de ([217.72.192.234]:44569 "EHLO fmmailgate03.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754493Ab0BPIDJ (ORCPT ); Tue, 16 Feb 2010 03:03:09 -0500 In-Reply-To: <20100216075259.GW2995@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig575F97C63C6CF93AB3F027BE Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Gleb Natapov wrote: > On Mon, Feb 15, 2010 at 07:17:17PM +0100, Jan Kiszka wrote: >> When in guest debugging mode, we have to reinject those #BP software >> exceptions that are caused by guest-injected INT3. As older AMD >> processors to not support the required nRIP VMCB field, try to emulate= >> it by moving RIP by one on injection. Fix it up again in case the >> injection failed and we were able to catch this. This does not work fo= r >> unintercepted faults, but it is better than doing nothing. >> >> Signed-off-by: Jan Kiszka >> --- >> arch/x86/kvm/svm.c | 54 +++++++++++++++++++++++++++++++++++--------= -------- >> 1 files changed, 37 insertions(+), 17 deletions(-) >> >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> index 52f78dd..f63f1db 100644 >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -46,6 +46,7 @@ MODULE_LICENSE("GPL"); >> #define SVM_FEATURE_NPT (1 << 0) >> #define SVM_FEATURE_LBRV (1 << 1) >> #define SVM_FEATURE_SVML (1 << 2) >> +#define SVM_FEATURE_NRIP (1 << 3) >> #define SVM_FEATURE_PAUSE_FILTER (1 << 10) >> =20 >> #define NESTED_EXIT_HOST 0 /* Exit handled on host level */ >> @@ -109,6 +110,8 @@ struct vcpu_svm { >> struct nested_state nested; >> =20 >> bool nmi_singlestep; >> + >> + bool int3_injected; >> }; >> =20 >> /* enable NPT for AMD64 and X86 with PAE */ >> @@ -234,23 +237,6 @@ static void svm_set_efer(struct kvm_vcpu *vcpu, u= 64 efer) >> vcpu->arch.efer =3D efer; >> } >> =20 >> -static void svm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr, >> - bool has_error_code, u32 error_code) >> -{ >> - struct vcpu_svm *svm =3D to_svm(vcpu); >> - >> - /* If we are within a nested VM we'd better #VMEXIT and let the >> - guest handle the exception */ >> - if (nested_svm_check_exception(svm, nr, has_error_code, error_code))= >> - return; >> - >> - svm->vmcb->control.event_inj =3D nr >> - | SVM_EVTINJ_VALID >> - | (has_error_code ? SVM_EVTINJ_VALID_ERR : 0) >> - | SVM_EVTINJ_TYPE_EXEPT; >> - svm->vmcb->control.event_inj_err =3D error_code; >> -} >> - >> static int is_external_interrupt(u32 info) >> { >> info &=3D SVM_EVTINJ_TYPE_MASK | SVM_EVTINJ_VALID; >> @@ -296,6 +282,36 @@ static void skip_emulated_instruction(struct kvm_= vcpu *vcpu) >> svm_set_interrupt_shadow(vcpu, 0); >> } >> =20 >> +static void svm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr, >> + bool has_error_code, u32 error_code) >> +{ >> + struct vcpu_svm *svm =3D to_svm(vcpu); >> + >> + /* If we are within a nested VM we'd better #VMEXIT and let the >> + guest handle the exception */ >> + if (nested_svm_check_exception(svm, nr, has_error_code, error_code))= >> + return; >> + >> + if (nr =3D=3D BP_VECTOR && !svm_has(SVM_FEATURE_NRIP)) { >> + /* >> + * For guest debugging where we have to reinject #BP if some >> + * INT3 is guest-owned: >> + * Emulate nRIP by moving RIP one forward. Will fail if >> + * injection raises a fault that is not intercepted. Still >> + * better than failing in all cases. >> + */ >> + svm->next_rip =3D kvm_rip_read(&svm->vcpu) + 1; >> + skip_emulated_instruction(&svm->vcpu); > if next_rip is zero skip_emulated_instruction() decodes instruction by > itself to properly calculate next rip so no need to guess instruction > length. Just copied what all the instruction emulations do here. Can change, thou= gh. >=20 >> + svm->int3_injected =3D true; >> + } >> + >> + svm->vmcb->control.event_inj =3D nr >> + | SVM_EVTINJ_VALID >> + | (has_error_code ? SVM_EVTINJ_VALID_ERR : 0) >> + | SVM_EVTINJ_TYPE_EXEPT; >> + svm->vmcb->control.event_inj_err =3D error_code; >> +} >> + >> static int has_svm(void) >> { >> const char *msg; >> @@ -2653,6 +2669,9 @@ static void svm_complete_interrupts(struct vcpu_= svm *svm) >> if (is_nested(svm)) >> break; >> if (kvm_exception_is_soft(vector)) >> + if (vector =3D=3D BP_VECTOR && svm->int3_injected) >> + kvm_rip_write(&svm->vcpu, >> + kvm_rip_read(&svm->vcpu) - 1); > You don't even check current rip? So if fault happens during unrelated = #BP you move > rip backwards and restart. Yep, true. Will fix. >=20 >> break; >> if (exitintinfo & SVM_EXITINTINFO_VALID_ERR) { >> u32 err =3D svm->vmcb->control.exit_int_info_err; >> @@ -2667,6 +2686,7 @@ static void svm_complete_interrupts(struct vcpu_= svm *svm) >> default: >> break; >> } >> + svm->int3_injected =3Dfalse; > Looks like the wrong place to clear this. It should be cleared on every= > exit, not only with valid vectoring info. Right. >=20 >> } >> =20 >> #ifdef CONFIG_X86_64 >> --=20 >> 1.6.0.2 >=20 > -- > Gleb. Thanks, Jan --------------enig575F97C63C6CF93AB3F027BE Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAkt6UToACgkQitSsb3rl5xRX4gCgiCJbkyCPw5msFP5e2UICw1zl NTQAniXAsD07W/ANxWTsDVnPqj42mEs0 =evso -----END PGP SIGNATURE----- --------------enig575F97C63C6CF93AB3F027BE--