Re: [PATCH] KVM: Use thread debug register storage instead of kvm specific data

[Marcelo Tosatti <mtosatti@redhat.com>]

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 <avi@redhat.com>
> >> ---
> >>  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