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 13:28:46 +0200 Message-ID: <4A9D056E.4030906@siemens.com> References: <1251798248-13164-1-git-send-email-avi@redhat.com> <20090901112220.GA18271@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Avi Kivity , "kvm@vger.kernel.org" To: Marcelo Tosatti Return-path: Received: from goliath.siemens.de ([192.35.17.28]:19795 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753885AbZIAL26 (ORCPT ); Tue, 1 Sep 2009 07:28:58 -0400 In-Reply-To: <20090901112220.GA18271@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: 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. 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