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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.05.16 at 22:13, <tamas@xxxxxxxxxxxxx> wrote:
> On Mon, May 30, 2016 at 8:16 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>> On 30.05.16 at 00:37, <tamas@xxxxxxxxxxxxx> wrote:
>>> @@ -3393,8 +3409,9 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>>>              }
>>>              else {
>>>                  int handled =
>>> -                    hvm_monitor_breakpoint(regs->eip,
>>> -                                           
>>> HVM_MONITOR_SOFTWARE_BREAKPOINT);
>>> +                        hvm_monitor_debug(regs->eip,
>>> +                                          HVM_MONITOR_SOFTWARE_BREAKPOINT,
>>> +                                          X86_EVENTTYPE_SW_EXCEPTION, 1);
>>
>> Please let's not add further mistakes like this, assuming INT3 can't
>> have any prefixes. It can, even if they're useless.
> 
> You mean the instruction length is not necessarily 1? Ultimately it
> doesn't seem to matter because reinjecting it with xc_hvm_inject_trap
> ignores this field. Instruction length is only required to be properly
> set AFAICT for a subset of debug exceptions during reinjection.

As you suggest later in your reply, if the insn length really doesn't
matter, this should be made recognizable here. Either by a suitably
named manifest constant (which could then even evaluate to zero),
or by a comment (personally I'd prefer the former, but I'm not
maintainer of this code).

Jan


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