On Fri, Jun 24, 2016 at 5:24 AM, Jan Beulich <[email protected]> wrote:
>>>> On 24.06.16 at 13:20, <[email protected]> wrote:
>>> From: Tamas K Lengyel [mailto:[email protected]]
>>> 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.
Yes, sure.
>>
>>> + * 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.
>
I'm OK with adding the rc < 0 case, it's just that the fall-through
style is already used for handling the int3 events for example. Should
I fix that too while I'm at it so the code is consistent?
Tamas
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
