[PATCH] KVM: VMX: Conditionally reload debug register 6

[PATCH] KVM: VMX: Conditionally reload debug register 6

[Avi Kivity]

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.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/vmx.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 05cd554..70b0c54 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3629,7 +3629,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 +3732,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)
-- 
1.6.4.1

Re: [PATCH] KVM: VMX: Conditionally reload debug register 6

[Jan Kiszka]

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.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  arch/x86/kvm/vmx.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 05cd554..70b0c54 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3629,7 +3629,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 +3732,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)

That reduces the emulation quality as vcpu->arch.switch_db_regs is only
set if some breakpoint is active while dr6 has its use also when that is
not the case).

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [PATCH] KVM: VMX: Conditionally reload debug register 6

[Avi Kivity]

On 09/01/2009 02:43 PM, Jan Kiszka wrote:
> @@ -3731,7 +3732,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)
>>      
> That reduces the emulation quality as vcpu->arch.switch_db_regs is only
> set if some breakpoint is active while dr6 has its use also when that is
> not the case).
>    

True - there's the TF reason reporting bits.

How about this then:

- if !switch_db_regs, trap #DB
- on #DB trap, copy DR6.BS and DR6.BT to vcpu->arch.dr6, and reinject 
the #DB

?

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] KVM: VMX: Conditionally reload debug register 6

[Jan Kiszka]

Avi Kivity wrote:
> On 09/01/2009 02:43 PM, Jan Kiszka wrote:
>> @@ -3731,7 +3732,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)
>>>      
>> That reduces the emulation quality as vcpu->arch.switch_db_regs is only
>> set if some breakpoint is active while dr6 has its use also when that is
>> not the case).
>>    
> 
> True - there's the TF reason reporting bits.
> 
> How about this then:
> 
> - if !switch_db_regs, trap #DB
> - on #DB trap, copy DR6.BS and DR6.BT to vcpu->arch.dr6, and reinject 
> the #DB
> 
> ?

I'm worried about vm-exits that may take precedence over the #db trap.
If we skip to save/restore dr6 for them, the value that the interception
handler sees later on will be bogus. Or is this architecturally impossible?

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [PATCH] KVM: VMX: Conditionally reload debug register 6

[Avi Kivity]

On 09/01/2009 03:32 PM, Jan Kiszka wrote:
> I'm worried about vm-exits that may take precedence over the #db trap.
> If we skip to save/restore dr6 for them, the value that the interception
> handler sees later on will be bogus. Or is this architecturally impossible?
>    

I think it's architectually impossible.  Debug traps take effect after 
the instruction executed, so if we made it to that point, nothing else 
can cause the vmexit.

-- 
error compiling committee.c: too many arguments to function