|
[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
On 06/23/2016 08:07 PM, Tamas K Lengyel wrote:
> Since in-guest debug exceptions are now unconditionally trapped to Xen, adding
> a hook for vm_event subscribers to tap into this new always-on guest event. We
> rename along the way hvm_event_breakpoint_type to hvm_event_type to better
> match the various events that can be passed with it. We also introduce the
> necessary monitor_op domctl's to enable subscribing to the events.
>
> This patch also provides monitor subscribers to int3 events proper access
> to the instruction length necessary for accurate event-reinjection. Without
> this subscribers manually have to evaluate if the int3 instruction has any
> prefix attached which would change the instruction length.
>
> Signed-off-by: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> ---
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> Cc: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: Jun Nakajima <jun.nakajima@xxxxxxxxx>
> Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
>
> v6: Add comment describing rc condition values for the monitor call
> v5: Transmit the proper insn_length for int3 events as well to fix
> the current bug with prefixed int3 instructions.
> ---
> tools/libxc/include/xenctrl.h | 3 +-
> tools/libxc/xc_monitor.c | 25 +++++++++++++++
> tools/tests/xen-access/xen-access.c | 63
> ++++++++++++++++++++++++++++++++-----
> xen/arch/x86/hvm/monitor.c | 21 +++++++++++--
> xen/arch/x86/hvm/vmx/vmx.c | 55 +++++++++++++++++++++++++-------
> xen/arch/x86/monitor.c | 16 ++++++++++
> xen/include/asm-x86/domain.h | 2 ++
> xen/include/asm-x86/hvm/monitor.h | 7 +++--
> xen/include/asm-x86/monitor.h | 3 +-
> xen/include/public/domctl.h | 6 ++++
> xen/include/public/vm_event.h | 14 +++++++--
> 11 files changed, 186 insertions(+), 29 deletions(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 4f5d954..4a85b4a 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2165,7 +2165,8 @@ int xc_monitor_software_breakpoint(xc_interface *xch,
> domid_t domain_id,
> bool enable);
> int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
> bool enable, bool sync);
> -
> +int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
> + bool enable, bool sync);
> /**
> * This function enables / disables emulation for each REP for a
> * REP-compatible instruction.
> diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
> index 78131b2..264992c 100644
> --- a/tools/libxc/xc_monitor.c
> +++ b/tools/libxc/xc_monitor.c
> @@ -156,3 +156,28 @@ int xc_monitor_emulate_each_rep(xc_interface *xch,
> domid_t domain_id,
>
> return do_domctl(xch, &domctl);
> }
> +
> +int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
> + bool enable, bool sync)
> +{
> + DECLARE_DOMCTL;
> +
> + domctl.cmd = XEN_DOMCTL_monitor_op;
> + domctl.domain = domain_id;
> + domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE
> + : XEN_DOMCTL_MONITOR_OP_DISABLE;
> + domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION;
> + domctl.u.monitor_op.u.debug_exception.sync = sync;
> +
> + return do_domctl(xch, &domctl);
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/tools/tests/xen-access/xen-access.c
> b/tools/tests/xen-access/xen-access.c
> index f26e723..e615476 100644
> --- a/tools/tests/xen-access/xen-access.c
> +++ b/tools/tests/xen-access/xen-access.c
> @@ -53,6 +53,10 @@
> #define ERROR(a, b...) fprintf(stderr, a "\n", ## b)
> #define PERROR(a, b...) fprintf(stderr, a ": %s\n", ## b, strerror(errno))
>
> +/* From xen/include/asm-x86/processor.h */
> +#define X86_TRAP_DEBUG 1
> +#define X86_TRAP_INT3 3
> +
> typedef struct vm_event {
> domid_t domain_id;
> xenevtchn_handle *xce_handle;
> @@ -333,7 +337,7 @@ void usage(char* progname)
> {
> fprintf(stderr, "Usage: %s [-m] <domain_id> write|exec", progname);
> #if defined(__i386__) || defined(__x86_64__)
> - fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec");
> + fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec|debug");
> #endif
> fprintf(stderr,
> "\n"
> @@ -354,10 +358,12 @@ int main(int argc, char *argv[])
> xc_interface *xch;
> xenmem_access_t default_access = XENMEM_access_rwx;
> xenmem_access_t after_first_access = XENMEM_access_rwx;
> + int memaccess = 0;
> int required = 0;
> int breakpoint = 0;
> int shutting_down = 0;
> int altp2m = 0;
> + int debug = 0;
> uint16_t altp2m_view_id = 0;
>
> char* progname = argv[0];
> @@ -391,11 +397,13 @@ int main(int argc, char *argv[])
> {
> default_access = XENMEM_access_rx;
> after_first_access = XENMEM_access_rwx;
> + memaccess = 1;
> }
> else if ( !strcmp(argv[0], "exec") )
> {
> default_access = XENMEM_access_rw;
> after_first_access = XENMEM_access_rwx;
> + memaccess = 1;
> }
> #if defined(__i386__) || defined(__x86_64__)
> else if ( !strcmp(argv[0], "breakpoint") )
> @@ -406,11 +414,17 @@ int main(int argc, char *argv[])
> {
> default_access = XENMEM_access_rx;
> altp2m = 1;
> + memaccess = 1;
> }
> else if ( !strcmp(argv[0], "altp2m_exec") )
> {
> default_access = XENMEM_access_rw;
> altp2m = 1;
> + memaccess = 1;
> + }
> + else if ( !strcmp(argv[0], "debug") )
> + {
> + debug = 1;
> }
> #endif
> else
> @@ -446,7 +460,7 @@ int main(int argc, char *argv[])
> }
>
> /* With altp2m we just create a new, restricted view of the memory */
> - if ( altp2m )
> + if ( memaccess && altp2m )
> {
> xen_pfn_t gfn = 0;
> unsigned long perm_set = 0;
> @@ -493,7 +507,7 @@ int main(int argc, char *argv[])
> }
> }
>
> - if ( !altp2m )
> + if ( memaccess && !altp2m )
> {
> /* Set the default access type and convert all pages to it */
> rc = xc_set_mem_access(xch, domain_id, default_access, ~0ull, 0);
> @@ -524,6 +538,16 @@ int main(int argc, char *argv[])
> }
> }
>
> + if ( debug )
> + {
> + rc = xc_monitor_debug_exceptions(xch, domain_id, 1, 1);
> + if ( rc < 0 )
> + {
> + ERROR("Error %d setting debug exception listener with
> vm_event\n", rc);
> + goto exit;
> + }
> + }
> +
> /* Wait for access */
> for (;;)
> {
> @@ -534,6 +558,8 @@ int main(int argc, char *argv[])
>
> if ( breakpoint )
> rc = xc_monitor_software_breakpoint(xch, domain_id, 0);
> + if ( debug )
> + rc = xc_monitor_debug_exceptions(xch, domain_id, 0, 0);
>
> if ( altp2m )
> {
> @@ -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.
> + 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)?
Thanks,
Razvan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |