From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH v2] KVM: VMX: Conditionally reload debug register 6 Date: Tue, 01 Sep 2009 16:35:20 +0300 Message-ID: <4A9D2318.9000000@redhat.com> References: <1251810385-21050-1-git-send-email-avi@redhat.com> <4A9D1E57.4010501@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , "kvm@vger.kernel.org" To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:33294 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754541AbZIANfW (ORCPT ); Tue, 1 Sep 2009 09:35:22 -0400 In-Reply-To: <4A9D1E57.4010501@siemens.com> Sender: kvm-owner@vger.kernel.org List-ID: On 09/01/2009 04:15 PM, Jan Kiszka wrote: > Avi Kivity wrote: > >> Only reload debug register 6 if we're running with the guest's >> debug registers. Saves around 150 cycles from the guest lightweight >> exit path. >> >> dr6 contains a couple of bits that are updated on #DB, so intercept >> that unconditionally and update those bits then. >> >> Signed-off-by: Avi Kivity >> --- >> v2: trap #DB so we maintain the TF related bits of DR7. >> >> arch/x86/kvm/vmx.c | 14 +++++++++----- >> 1 files changed, 9 insertions(+), 5 deletions(-) >> >> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index 05cd554..d4be978 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -540,10 +540,12 @@ static void update_exception_bitmap(struct kvm_vcpu *vcpu) >> eb = (1u<< PF_VECTOR) | (1u<< UD_VECTOR) | (1u<< MC_VECTOR); >> if (!vcpu->fpu_active) >> eb |= 1u<< NM_VECTOR; >> + /* >> + * Unconditionally intercept #DB so we can maintain dr6 without >> + * reading it every exit. >> + */ >> + eb |= 1u<< DB_VECTOR; >> > If this is safe... > > >> if (vcpu->guest_debug& KVM_GUESTDBG_ENABLE) { >> - if (vcpu->guest_debug& >> - (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)) >> - eb |= 1u<< DB_VECTOR; >> if (vcpu->guest_debug& KVM_GUESTDBG_USE_SW_BP) >> eb |= 1u<< BP_VECTOR; >> } >> @@ -3629,7 +3631,8 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu) >> */ >> vmcs_writel(HOST_CR0, read_cr0()); >> >> - set_debugreg(vcpu->arch.dr6, 6); >> + if (vcpu->arch.switch_db_regs) >> + set_debugreg(vcpu->arch.dr6, 6); >> >> asm( >> /* Store host registers */ >> @@ -3731,7 +3734,8 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu) >> | (1<< VCPU_EXREG_PDPTR)); >> vcpu->arch.regs_dirty = 0; >> >> - get_debugreg(vcpu->arch.dr6, 6); >> + if (vcpu->arch.switch_db_regs) >> + get_debugreg(vcpu->arch.dr6, 6); >> >> vmx->idt_vectoring_info = vmcs_read32(IDT_VECTORING_INFO_FIELD); >> if (vmx->rmode.irq.pending) >> > ...I wonder why we cannot drop this save/restore? The guest is not able > to access dr6 without causing a trap, thus will never see dr6 as stored > in the hardware. > I think you're right. btw, something else I've considered was not to do any emulation for 'mov dr' but instead load the debug registers and disable the intercept. So far the only issue I've seen is that we lose support for real mode guest self-debug on intel (pre-unrestricted guest). What do you think of this? -- error compiling committee.c: too many arguments to function