Re: [Xen-devel] [PATCH v7 2/3] x86/vm_event: Add HVM debug exception vm_events

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Tue, Jun 28, 2016 at 1:37 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> On 27.06.16 at 20:08, <tamas@xxxxxxxxxxxxx> wrote:
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -3376,7 +3376,29 @@ 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 > 0 paused waiting for response, work here is done
>> +                 * rc < 0 error in monitor/vm_event, crash
>> +                 */
>> +                if ( !rc )
>> +                    vmx_propagate_intr(intr_info);
>> +                if ( rc < 0 )
>> +                    goto exit_and_crash;
>> +            }
>
> As opposed to earlier versions, here omitting the "else" seems
> undesirable. Or, perhaps better, simply re-order the two if()-s.
> This is to make clear that what is now the second if() does in no
> way depend on what the body of the current first if() does.
>
> The same would then apply to patch 3, and I'd be fine doing the
> adjustment while committing (provided all necessary acks trickle
> in). Feel free to add my ack here for the few changes for which
> that's actually relevant.
>
> Jan

That sounds fine to me. I think this patch only needs Wei's or Ian's
ack now for the libxc changes.

Tamas

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