From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 2/2] KVM: x86: Preserve guest single-stepping on register Date: Sun, 04 Oct 2009 21:02:29 +0200 Message-ID: <4AC8F145.4000204@web.de> References: <4AC67D94.6090406@web.de> <4AC67F45.3010808@web.de> <4AC8B63B.8060602@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigCA5D1342ADE5ADEE76CB3B2A" Cc: Marcelo Tosatti , kvm-devel To: Avi Kivity Return-path: Received: from fmmailgate02.web.de ([217.72.192.227]:41628 "EHLO fmmailgate02.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757784AbZJDTPY (ORCPT ); Sun, 4 Oct 2009 15:15:24 -0400 In-Reply-To: <4AC8B63B.8060602@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigCA5D1342ADE5ADEE76CB3B2A Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Avi Kivity wrote: > On 10/03/2009 12:31 AM, Jan Kiszka wrote: >> Give user space more flexibility /wrt its IOCTL order. So far updating= >> the rflags via KVM_SET_REGS ignored potentially set single-step flags.= >> Now they will be kept. >> =20 >=20 >> >> kvm_rip_write(vcpu, regs->rip); >> - kvm_x86_ops->set_rflags(vcpu, regs->rflags); >> >> + rflags =3D regs->rflags; >> + if (vcpu->guest_debug& KVM_GUESTDBG_SINGLESTEP) >> + rflags |=3D X86_EFLAGS_TF | X86_EFLAGS_RF; >> + kvm_x86_ops->set_rflags(vcpu, rflags); >> >> =20 >=20 > I think we need same on popf instruction emulation. >=20 Hmmmm, good point. Mind reverting 2/2 and applying this one instead? Jan ---------> KVM: x86: Rework guest single-step flag injection and filtering Push TF and RF injection and filtering on guest single-stepping into the vender get/set_rflags callbacks. This makes the whole mechanism more robust /wrt user space IOTCTL order and instruction emulations. Signed-off-by: Jan Kiszka --- arch/x86/kvm/svm.c | 8 +++++++- arch/x86/kvm/vmx.c | 4 ++++ arch/x86/kvm/x86.c | 24 +++++++++--------------- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 279a2ae..407e1a7 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -797,11 +797,17 @@ static void svm_vcpu_put(struct kvm_vcpu *vcpu) =20 static unsigned long svm_get_rflags(struct kvm_vcpu *vcpu) { - return to_svm(vcpu)->vmcb->save.rflags; + unsigned long rflags =3D to_svm(vcpu)->vmcb->save.rflags; + + if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) + rflags &=3D ~(unsigned long)(X86_EFLAGS_TF | X86_EFLAGS_RF); + return rflags; } =20 static void svm_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags) { + if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) + rflags |=3D X86_EFLAGS_TF | X86_EFLAGS_RF; to_svm(vcpu)->vmcb->save.rflags =3D rflags; } =20 diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 70020e5..8e678ef 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -787,6 +787,8 @@ static unsigned long vmx_get_rflags(struct kvm_vcpu *= vcpu) rflags =3D vmcs_readl(GUEST_RFLAGS); if (to_vmx(vcpu)->rmode.vm86_active) rflags &=3D ~(unsigned long)(X86_EFLAGS_IOPL | X86_EFLAGS_VM); + if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) + rflags &=3D ~(unsigned long)(X86_EFLAGS_TF | X86_EFLAGS_RF); return rflags; } =20 @@ -794,6 +796,8 @@ static void vmx_set_rflags(struct kvm_vcpu *vcpu, uns= igned long rflags) { if (to_vmx(vcpu)->rmode.vm86_active) rflags |=3D X86_EFLAGS_IOPL | X86_EFLAGS_VM; + if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) + rflags |=3D X86_EFLAGS_TF | X86_EFLAGS_RF; vmcs_writel(GUEST_RFLAGS, rflags); } =20 diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index aa5d574..5b562dd 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3840,12 +3840,6 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *= vcpu, struct kvm_regs *regs) regs->rip =3D kvm_rip_read(vcpu); regs->rflags =3D kvm_x86_ops->get_rflags(vcpu); =20 - /* - * Don't leak debug flags in case they were set for guest debugging - */ - if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) - regs->rflags &=3D ~(X86_EFLAGS_TF | X86_EFLAGS_RF); - vcpu_put(vcpu); =20 return 0; @@ -3872,13 +3866,11 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu = *vcpu, struct kvm_regs *regs) kvm_register_write(vcpu, VCPU_REGS_R13, regs->r13); kvm_register_write(vcpu, VCPU_REGS_R14, regs->r14); kvm_register_write(vcpu, VCPU_REGS_R15, regs->r15); - #endif =20 kvm_rip_write(vcpu, regs->rip); kvm_x86_ops->set_rflags(vcpu, regs->rflags); =20 - vcpu->arch.exception.pending =3D false; =20 vcpu_put(vcpu); @@ -4471,12 +4463,15 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kv= m_vcpu *vcpu, struct kvm_guest_debug *dbg) { unsigned long rflags; - int old_debug; int i; =20 vcpu_load(vcpu); =20 - old_debug =3D vcpu->guest_debug; + /* + * Read rflags as long as potentially injected trace flags are still + * filtered out. + */ + rflags =3D kvm_x86_ops->get_rflags(vcpu); =20 vcpu->guest_debug =3D dbg->control; if (!(vcpu->guest_debug & KVM_GUESTDBG_ENABLE)) @@ -4493,11 +4488,10 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kv= m_vcpu *vcpu, vcpu->arch.switch_db_regs =3D (vcpu->arch.dr7 & DR7_BP_EN_MASK); } =20 - rflags =3D kvm_x86_ops->get_rflags(vcpu); - if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) - rflags |=3D X86_EFLAGS_TF | X86_EFLAGS_RF; - else if (old_debug & KVM_GUESTDBG_SINGLESTEP) - rflags &=3D ~(X86_EFLAGS_TF | X86_EFLAGS_RF); + /* + * Trigger an rflags update that will inject or remove the trace + * flags. + */ kvm_x86_ops->set_rflags(vcpu, rflags); =20 kvm_x86_ops->set_guest_debug(vcpu, dbg); --------------enigCA5D1342ADE5ADEE76CB3B2A 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 iEYEARECAAYFAkrI8UsACgkQitSsb3rl5xQ99ACg3EgB7VUw2ansfZWoDqt4ME3C 89MAoLgADpK1chz8MBPw8K0Tvj1JnZ80 =OZ/k -----END PGP SIGNATURE----- --------------enigCA5D1342ADE5ADEE76CB3B2A--