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

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 06/24/16 21:48, Tamas K Lengyel wrote:
>>>              {
>>> @@ -635,22 +661,22 @@ int main(int argc, char *argv[])
>>>                  rsp.u.mem_access = req.u.mem_access;
>>>                  break;
>>>              case VM_EVENT_REASON_SOFTWARE_BREAKPOINT:
>>> -                printf("Breakpoint: rip=%016"PRIx64", gfn=%"PRIx64" (vcpu 
>>> %d)\n",
>>> +                printf("Breakpoint: rip=%016"PRIx64" gfn=%"PRIx64" (vcpu 
>>> %d)\n",
>>
>> You seem to have removed the comma here (',') after the first "PRIx64",
>> but ...
>>
>>>                         req.data.regs.x86.rip,
>>>                         req.u.software_breakpoint.gfn,
>>>                         req.vcpu_id);
>>>
>>>                  /* Reinject */
>>> -                rc = xc_hvm_inject_trap(
>>> -                    xch, domain_id, req.vcpu_id, 3,
>>> -                    HVMOP_TRAP_sw_exc, -1, 0, 0);
>>> +                rc = xc_hvm_inject_trap(xch, domain_id, req.vcpu_id,
>>> +                                        X86_TRAP_INT3,
>>> +                                        req.u.software_breakpoint.type, -1,
>>> +                                        
>>> req.u.software_breakpoint.insn_length, 0);
>>>                  if (rc < 0)
>>>                  {
>>>                      ERROR("Error %d injecting breakpoint\n", rc);
>>>                      interrupted = -1;
>>>                      continue;
>>>                  }
>>> -
>>>                  break;
>>>              case VM_EVENT_REASON_SINGLESTEP:
>>>                  printf("Singlestep: rip=%016"PRIx64", vcpu %d, altp2m 
>>> %u\n",
>>> @@ -669,6 +695,27 @@ int main(int argc, char *argv[])
>>>                  rsp.flags |= VM_EVENT_FLAG_TOGGLE_SINGLESTEP;
>>>
>>>                  break;
>>> +            case VM_EVENT_REASON_DEBUG_EXCEPTION:
>>> +                printf("Debug exception: rip=%016"PRIx64", vcpu %d. Type: 
>>> %u. Length: %u\n",
>>
>> ... this newly added line uses the old style (with a comma after the
>> first "PRIx64"). Minor issue maybe, I just don't understand why the
>> first modification was made.
> 
> Yea, for no reason. Will add the comma back above.
> 
>>
>>> +                       req.data.regs.x86.rip,
>>> +                       req.vcpu_id,
>>> +                       req.u.debug_exception.type,
>>> +                       req.u.debug_exception.insn_length);
>>> +
>>> +                /* Reinject */
>>> +                rc = xc_hvm_inject_trap(xch, domain_id, req.vcpu_id,
>>> +                                        X86_TRAP_DEBUG,
>>> +                                        req.u.debug_exception.type, -1,
>>> +                                        req.u.debug_exception.insn_length,
>>> +                                        req.data.regs.x86.cr2);
>>> +                if (rc < 0)
>>> +                {
>>> +                    ERROR("Error %d injecting breakpoint\n", rc);
>>> +                    interrupted = -1;
>>> +                    continue;
>>> +                }
>>> +
>>> +                break;
>>>              default:
>>>                  fprintf(stderr, "UNKNOWN REASON CODE %d\n", req.reason);
>>>              }
>>> diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
>>> index 472926c..bbe5952 100644
>>> --- a/xen/arch/x86/hvm/monitor.c
>>> +++ b/xen/arch/x86/hvm/monitor.c
>>> @@ -87,12 +87,13 @@ static inline unsigned long gfn_of_rip(unsigned long 
>>> rip)
>>>      return paging_gva_to_gfn(curr, sreg.base + rip, &pfec);
>>>  }
>>>
>>> -int hvm_monitor_breakpoint(unsigned long rip,
>>> -                           enum hvm_monitor_breakpoint_type type)
>>> +int hvm_monitor_debug(unsigned long rip, enum hvm_monitor_debug_type type,
>>> +                      unsigned long trap_type, unsigned long insn_length)
>>>  {
>>>      struct vcpu *curr = current;
>>>      struct arch_domain *ad = &curr->domain->arch;
>>>      vm_event_request_t req = {};
>>> +    bool_t sync;
>>>
>>>      switch ( type )
>>>      {
>>> @@ -101,6 +102,9 @@ int hvm_monitor_breakpoint(unsigned long rip,
>>>              return 0;
>>>          req.reason = VM_EVENT_REASON_SOFTWARE_BREAKPOINT;
>>>          req.u.software_breakpoint.gfn = gfn_of_rip(rip);
>>> +        req.u.software_breakpoint.type = trap_type;
>>> +        req.u.software_breakpoint.insn_length = insn_length;
>>> +        sync = 1;
>>>          break;
>>>
>>>      case HVM_MONITOR_SINGLESTEP_BREAKPOINT:
>>> @@ -108,6 +112,17 @@ int hvm_monitor_breakpoint(unsigned long rip,
>>>              return 0;
>>>          req.reason = VM_EVENT_REASON_SINGLESTEP;
>>>          req.u.singlestep.gfn = gfn_of_rip(rip);
>>> +        sync = 1;
>>> +        break;
>>> +
>>> +    case HVM_MONITOR_DEBUG_EXCEPTION:
>>> +        if ( !ad->monitor.debug_exception_enabled )
>>> +            return 0;
>>> +        req.reason = VM_EVENT_REASON_DEBUG_EXCEPTION;
>>> +        req.u.debug_exception.gfn = gfn_of_rip(rip);
>>> +        req.u.debug_exception.type = trap_type;
>>> +        req.u.debug_exception.insn_length = insn_length;
>>> +        sync = !!ad->monitor.debug_exception_sync;
>>>          break;
>>>
>>>      default:
>>> @@ -116,7 +131,7 @@ int hvm_monitor_breakpoint(unsigned long rip,
>>>
>>>      req.vcpu_id = curr->vcpu_id;
>>>
>>> -    return vm_event_monitor_traps(curr, 1, &req);
>>> +    return vm_event_monitor_traps(curr, sync, &req);
>>
>>
>> What would be a basic use-case for this event to be sent without pausing
>> the VCPU (i.e. with sync != 1)?
> 
> If you wish to passively monitor the occurrences of debug events in
> the guest you can use the !sync option. That will make Xen
> automatically reinject the event as it would normally but will send
> you a notification. I don't have a particular use for this but since
> Xen already had all the necessary wiring ready for it so we might as
> well make it available as an option, similar to the other !sync
> events.

Fair enough, thanks for the explanation.


Thanks,
Razvan

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