Re: [Xen-devel] Single step in HVM domU on Intel machine may see wrong DB6

["Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>]

Jan Beulich wrote on 2014-02-27:
>>>> On 26.02.14 at 06:15, "Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx> wrote:
>> @@ -2690,9 +2688,13 @@ void vmx_vmexit_handler(struct cpu_user_regs
> *regs)
>>              __vmread(EXIT_QUALIFICATION, &exit_qualification);
>>              HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
>>              write_debugreg(6, exit_qualification | 0xffff0ff0);
>> -            if ( !v->domain->debugger_attached ||
>> cpu_has_monitor_trap_flag ) -                goto exit_and_crash; -    
>>        domain_pause_for_debugger(); +            if (
>> v->domain->debugger_attached ) +               
>> domain_pause_for_debugger(); +            else +            { +        
>>        __restore_debug_registers(v); +               
>> hvm_inject_hw_exception(TRAP_debug, HVM_DELIVER_NO_ERROR_CODE); +      
>>      }
> 
> I suppose you need to set DR6.BS after restoring the reigsters?

Right but is not enough. If flag_dr_dirty is set, we need to restore register 
from hardware. Conversely, restore is from debugreg and set DR6 to 
exit_qualification.

> 
> Also, the change looks rather simple - is that really correct for both
> cpu_has_monitor_trap_flag and !cpu_has_monitor_trap_flag cases?
> 

Until now, MTF is used for single step by gdb. And with MTF enabled, it should 
never goto here. So those changes should not impact the MTF. Also, I used gdb 
to debug guest with using MTF approach and applied this patch. It seems 
everything is working well.
But still need more testing from Juergen.

>> BTW: I also think we should clear the CPU_BASED_MOV_DR_EXITING bit
>> in __restore_debug_registers(). After restore the debug register, we
>> should not trap any DR access unless the VCPU is scheduled out again.
>> Not sure whether I am wrong.
>> 
>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>> index b128e81..56a3140 100644
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -394,6 +394,8 @@ static void __restore_debug_registers(struct
>> vcpu
> *v)
>>      write_debugreg(3, v->arch.debugreg[3]);
>>      write_debugreg(6, v->arch.debugreg[6]);
>>      /* DR7 is loaded from the VMCS. */
>> +    v->arch.hvm_vmx.exec_control &= ~CPU_BASED_MOV_DR_EXITING;
>> +    vmx_update_cpu_exec_control(v);
>>  }
>>  
>>  /*
> 
> That's being done by at least one of its callers (vmx_dr_access())
> already, and I think it was purposefully not done in other cases.

Yes, the question is why only one caller does this. Per my understanding, after 
restoring debug register, hypervisor should allow the guest to access them 
unless the VCPU is scheduled out again and vmx_save_dr is called.
Take the injecting debug_trap to guest as example, hypervisor will restore the 
debug register to hardware. After returning to guest, we should allow guest to 
read DR without trap again. But now, it still causes DR access vmexit and 
hypervisor restores it again. Then it clear the DR exiting bit in this vmexit 
handler. Are this behavior we expecting?

> 
> Jan


Best regards,
Yang


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel