From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH] KVM: VMX: Update instruction length on intercepted BP Date: Sun, 14 Feb 2010 13:39:41 +0100 Message-ID: <4B77EF0D.4050303@web.de> References: <4B767160.4070609@web.de> <20100214075303.GF2511@redhat.com> <4B77CFD7.9080504@web.de> <20100214103445.GH2511@redhat.com> <4B77EC14.4000302@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig3E16B87E6A17348CF577B71B" Cc: Gleb Natapov , Marcelo Tosatti , kvm To: Avi Kivity Return-path: Received: from fmmailgate03.web.de ([217.72.192.234]:43480 "EHLO fmmailgate03.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753872Ab0BNMjs (ORCPT ); Sun, 14 Feb 2010 07:39:48 -0500 In-Reply-To: <4B77EC14.4000302@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig3E16B87E6A17348CF577B71B Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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? >>>> =20 >>> In guest debugging mode a #BP exception is always reported to user sp= ace >>> 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 w= ill >>> 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 som= e >>> other exit. >>> >>> =20 >> 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 caus= ed >> #BP after event reinjection? >> >> =20 >=20 > Note, newer AMDs do have such a field (nRIP, 0xC8). We need to support= > older machines, though. >=20 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; =20 bool nmi_singlestep; + bool bp_singlestep; }; =20 /* enable NPT for AMD64 and X86 with PAE */ @@ -244,6 +245,19 @@ static void svm_queue_exception(struct kvm_vcpu *vcp= u, unsigned nr, if (nested_svm_check_exception(svm, nr, has_error_code, error_code)) return; =20 + if (nr =3D=3D 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 &=3D ~(1 << BP_VECTOR); + svm->bp_singlestep =3D true; + svm->vmcb->save.rflags |=3D (X86_EFLAGS_TF | X86_EFLAGS_RF); + } + svm->vmcb->control.event_inj =3D nr | SVM_EVTINJ_VALID | (has_error_code ? SVM_EVTINJ_VALID_ERR : 0) @@ -1083,7 +1097,8 @@ static void update_db_intercept(struct kvm_vcpu *vc= pu) (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)) svm->vmcb->control.intercept_exceptions |=3D 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 |=3D 1 << BP_VECTOR; } else @@ -1214,7 +1229,7 @@ static int db_interception(struct vcpu_svm *svm) =20 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); } =20 + if (svm->bp_singlestep) { + svm->bp_singlestep =3D false; + if (!(svm->vcpu.guest_debug & KVM_GUESTDBG_SINGLESTEP)) + svm->vmcb->save.rflags &=3D + ~(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 =3D 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 --------------enig3E16B87E6A17348CF577B71B 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 iEYEARECAAYFAkt37xEACgkQitSsb3rl5xTBxwCg2yRAgtguJLw9EMXl4KNd+AEI o1cAoL3RKvE7WiizWb6IcEllfRNNFz44 =HmkR -----END PGP SIGNATURE----- --------------enig3E16B87E6A17348CF577B71B--