From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH] KVM: Use thread debug register storage instead of kvm specific data Date: Tue, 1 Sep 2009 08:32:25 -0300 Message-ID: <20090901113225.GA18690@amt.cnet> References: <1251798248-13164-1-git-send-email-avi@redhat.com> <20090901112220.GA18271@amt.cnet> <4A9D056E.4030906@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Avi Kivity , "kvm@vger.kernel.org" To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:29689 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754083AbZIALcq (ORCPT ); Tue, 1 Sep 2009 07:32:46 -0400 Content-Disposition: inline In-Reply-To: <4A9D056E.4030906@siemens.com> Sender: kvm-owner@vger.kernel.org List-ID: On Tue, Sep 01, 2009 at 01:28:46PM +0200, Jan Kiszka wrote: > Marcelo Tosatti wrote: > > On Tue, Sep 01, 2009 at 12:44:08PM +0300, 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. > > > > Is this kgdb safe? > > Nope, kgdb writes directly to the debug registers. > > I vaguely recall someone trying to push a debug register management > framework. Did it hit mainline in the meantime? I do not find any trace > on quick glance, at least not in kgdb. A simple kgdb_enabled sort of flag, in addition to TIF_DEBUG would do it? > Jan > > > > >> 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); > >> - > >> 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))) { > >> + 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); > >> > >> set_bit(KVM_REQ_KICK, &vcpu->requests); > >> local_irq_enable(); > >> -- > >> 1.6.4.1 > > -- > Siemens AG, Corporate Technology, CT SE 2 > Corporate Competence Center Embedded Linux