Re: [Xen-devel] [PATCH v6 5/5] x86/vm_event: Add HVM debug exception vm_events

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.06.16 at 13:20, <kevin.tian@xxxxxxxxx> wrote:
>>  From: Tamas K Lengyel [mailto:tamas@xxxxxxxxxxxxx]
>> Sent: Friday, June 24, 2016 1:07 AM
>> 
>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>> index 03fcba7..4b69ca6 100644
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -3373,10 +3373,39 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>>              HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
>>              write_debugreg(6, exit_qualification | DR_STATUS_RESERVED_ONE);
>>              if ( !v->domain->debugger_attached )
>> -                vmx_propagate_intr(intr_info);
>> +            {
>> +                unsigned long insn_len = 0;
>> +                int rc;
>> +                unsigned long trap_type = MASK_EXTR(intr_info,
>> +                                                    
>> INTR_INFO_INTR_TYPE_MASK);
>> +
>> +                if ( trap_type >= X86_EVENTTYPE_SW_INTERRUPT )
>> +                    __vmread(VM_EXIT_INSTRUCTION_LEN, &insn_len);
>> +
>> +                rc = hvm_monitor_debug(regs->eip,
>> +                                       HVM_MONITOR_DEBUG_EXCEPTION,
>> +                                       trap_type, insn_len);
>> +
>> +                /*
>> +                 * !rc  continue normally
>> +                 * rc > paused waiting for response, work here is done
> 
> Did you mean "rc > 0, vcpu is paused and waiting for response..."? Please
> make the comment clear to understand.
> 
>> +                 * rc < error, fall-through to exit_and_crash
>> +                 */
>> +                if ( !rc )
>> +                {
>> +                    vmx_propagate_intr(intr_info);
>> +                    break;
>> +                }
>> +                if ( rc > 0 )
>> +                    break;
>> +            }
>>              else
>> +            {
>>                  domain_pause_for_debugger();
>> -            break;
>> +                break;
>> +            }
>> +
>> +            goto exit_and_crash;
> 
> Putting goto as the last line within a 'case' looks a bit strange. What
> about putting goto directly under a "if ( rc < 0 )" check earlier?
> 
>               if ( !rc )
>                       ...
>               if ( rc < 0 )
>                       goto exit_and_crash;
>       }
>       else
>               domain_pause_for_debugger();
>       break;

Thanks, Kevin - indeed that's exactly what I had asked for already
on the previous iteration.

Jan


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