[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



>>              {
>> @@ -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.

Tamas

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.