Avi Kivity wrote:
> 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<avi@xxxxxxxxxx>
>>> ---
>>> 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?
I think you can't have both: optimized dr save/restore on vmentry/exit
and optimized dr access. If you drop on-demand dr register
readout/update, you need to deal with this on every vmentry/exit. My
feeling is that this would be awfully slower, even slower than what we
currently have without your patches.
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
