From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH] KVM: Use thread debug register storage instead of kvm specific data Date: Tue, 01 Sep 2009 12:42:24 +0200 Message-ID: <4A9CFA90.50202@siemens.com> References: <1251798248-13164-1-git-send-email-avi@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , "kvm@vger.kernel.org" To: Avi Kivity Return-path: Received: from goliath.siemens.de ([192.35.17.28]:24883 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753770AbZIAKmh (ORCPT ); Tue, 1 Sep 2009 06:42:37 -0400 In-Reply-To: <1251798248-13164-1-git-send-email-avi@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Avi Kivity wrote: > Instead of saving the debug registers from the processor to a kvm data > structure, rely in the debug registers stored in the thread structure. > This allows us not to save dr6 and dr7. > > Reduces lightweight vmexit cost by 350 cycles, or 11 percent. > > Signed-off-by: Avi Kivity > --- > arch/x86/include/asm/kvm_host.h | 3 --- > arch/x86/kvm/x86.c | 22 +++++++--------------- > 2 files changed, 7 insertions(+), 18 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 6046e6f..45226f0 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -362,9 +362,6 @@ struct kvm_vcpu_arch { > u32 pat; > > int switch_db_regs; > - unsigned long host_db[KVM_NR_DB_REGS]; > - unsigned long host_dr6; > - unsigned long host_dr7; > unsigned long db[KVM_NR_DB_REGS]; > unsigned long dr6; > unsigned long dr7; > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 891234b..9e3acbd 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3627,14 +3627,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) > > kvm_guest_enter(); > > - get_debugreg(vcpu->arch.host_dr6, 6); > - get_debugreg(vcpu->arch.host_dr7, 7); > if (unlikely(vcpu->arch.switch_db_regs)) { > - get_debugreg(vcpu->arch.host_db[0], 0); > - get_debugreg(vcpu->arch.host_db[1], 1); > - get_debugreg(vcpu->arch.host_db[2], 2); > - get_debugreg(vcpu->arch.host_db[3], 3); > - I'm fine with this for the common case, but we should switch to the safe pattern on test_thread_flag(TIF_DEBUG), ie. if someone debugs qemu using hardware break/watchpoints. > set_debugreg(0, 7); > set_debugreg(vcpu->arch.eff_db[0], 0); > set_debugreg(vcpu->arch.eff_db[1], 1); > @@ -3645,15 +3638,14 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) > trace_kvm_entry(vcpu->vcpu_id); > kvm_x86_ops->run(vcpu); > > - if (unlikely(vcpu->arch.switch_db_regs)) { > - set_debugreg(0, 7); > - set_debugreg(vcpu->arch.host_db[0], 0); > - set_debugreg(vcpu->arch.host_db[1], 1); > - set_debugreg(vcpu->arch.host_db[2], 2); > - set_debugreg(vcpu->arch.host_db[3], 3); > + if (unlikely(vcpu->arch.switch_db_regs || test_thread_flag(TIF_DEBUG))) { IIRC, you must clear dr7 before loading dr0..3 to avoid spurious effects. > + set_debugreg(current->thread.debugreg0, 0); > + set_debugreg(current->thread.debugreg1, 1); > + set_debugreg(current->thread.debugreg2, 2); > + set_debugreg(current->thread.debugreg3, 3); > + set_debugreg(current->thread.debugreg6, 6); > + set_debugreg(current->thread.debugreg7, 7); > } > - set_debugreg(vcpu->arch.host_dr6, 6); > - set_debugreg(vcpu->arch.host_dr7, 7); Unless I miss something ATM, moving this into the if-clause should cause troubles if the guest leaves dr7 armed behind. But I need to refresh my memory on this. > > set_bit(KVM_REQ_KICK, &vcpu->requests); > local_irq_enable(); Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux