Date: Wed, 6 May 2020 22:20:37 +0000 (UTC) From: John Baldwin <jhb@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: svn commit: r360713 - in stable/12: lib/libvmmapi sys/amd64/include sys/amd64/vmm/amd sys/amd64/vmm/intel usr.sbin/bhyve Message-ID: <202005062220.046MKbPd035977@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: jhb Date: Wed May 6 22:20:37 2020 New Revision: 360713 URL: https://svnweb.freebsd.org/changeset/base/360713 Log: MFC 355724,360166: Software breakpoints on Intel CPUs. 355724: Support software breakpoints in the debug server on Intel CPUs. - Allow the userland hypervisor to intercept breakpoint exceptions (BP#) in the guest. A new capability (VM_CAP_BPT_EXIT) is used to enable this feature. These exceptions are reported to userland via a new VM_EXITCODE_BPT that includes the length of the original breakpoint instruction. If userland wishes to pass the exception through to the guest, it must be explicitly re-injected via vm_inject_exception(). - Export VMCS_ENTRY_INST_LENGTH as a VM_REG_GUEST_ENTRY_INST_LENGTH pseudo-register. Injecting a BP# on Intel requires setting this to the length of the breakpoint instruction. AMD SVM currently ignores writes to this register (but reports success) and fails to read it. - Rework the per-vCPU state tracked by the debug server. Rather than a single 'stepping_vcpu' global, add a structure for each vCPU that tracks state about that vCPU ('stepping', 'stepped', and 'hit_swbreak'). A global 'stopped_vcpu' tracks which vCPU is currently reporting an event. Event handlers for MTRAP and breakpoint exits loop until the associated event is reported to the debugger. Breakpoint events are discarded if the breakpoint is not present when a vCPU resumes in the breakpoint handler to retry submitting the breakpoint event. - Maintain a linked-list of active breakpoints in response to the GDB 'Z0' and 'z0' packets. 360166: Add description string for VM_CAP_BPT_EXIT. While here, replace the array of mapping structures with an array of string pointers where the index is the capability value. Modified: stable/12/lib/libvmmapi/vmmapi.c stable/12/sys/amd64/include/vmm.h stable/12/sys/amd64/vmm/amd/svm.c stable/12/sys/amd64/vmm/intel/vmcs.c stable/12/sys/amd64/vmm/intel/vmx.c stable/12/sys/amd64/vmm/intel/vmx.h stable/12/usr.sbin/bhyve/bhyve.8 stable/12/usr.sbin/bhyve/bhyverun.c stable/12/usr.sbin/bhyve/gdb.c stable/12/usr.sbin/bhyve/gdb.h Directory Properties: stable/12/ (props changed) Modified: stable/12/lib/libvmmapi/vmmapi.c ============================================================================== --- stable/12/lib/libvmmapi/vmmapi.c Wed May 6 22:18:24 2020 (r360712) +++ stable/12/lib/libvmmapi/vmmapi.c Wed May 6 22:20:37 2020 (r360713) @@ -812,16 +812,13 @@ vm_inject_nmi(struct vmctx *ctx, int vcpu) return (ioctl(ctx->fd, VM_INJECT_NMI, &vmnmi)); } -static struct { - const char *name; - int type; -} capstrmap[] = { - { "hlt_exit", VM_CAP_HALT_EXIT }, - { "mtrap_exit", VM_CAP_MTRAP_EXIT }, - { "pause_exit", VM_CAP_PAUSE_EXIT }, - { "unrestricted_guest", VM_CAP_UNRESTRICTED_GUEST }, - { "enable_invpcid", VM_CAP_ENABLE_INVPCID }, - { 0 } +static const char *capstrmap[] = { + [VM_CAP_HALT_EXIT] = "hlt_exit", + [VM_CAP_MTRAP_EXIT] = "mtrap_exit", + [VM_CAP_PAUSE_EXIT] = "pause_exit", + [VM_CAP_UNRESTRICTED_GUEST] = "unrestricted_guest", + [VM_CAP_ENABLE_INVPCID] = "enable_invpcid", + [VM_CAP_BPT_EXIT] = "bpt_exit", }; int @@ -829,9 +826,9 @@ vm_capability_name2type(const char *capname) { int i; - for (i = 0; capstrmap[i].name != NULL && capname != NULL; i++) { - if (strcmp(capstrmap[i].name, capname) == 0) - return (capstrmap[i].type); + for (i = 0; i < nitems(capstrmap); i++) { + if (strcmp(capstrmap[i], capname) == 0) + return (i); } return (-1); @@ -840,12 +837,8 @@ vm_capability_name2type(const char *capname) const char * vm_capability_type2name(int type) { - int i; - - for (i = 0; capstrmap[i].name != NULL; i++) { - if (capstrmap[i].type == type) - return (capstrmap[i].name); - } + if (type < nitems(capstrmap)) + return (capstrmap[type]); return (NULL); } Modified: stable/12/sys/amd64/include/vmm.h ============================================================================== --- stable/12/sys/amd64/include/vmm.h Wed May 6 22:18:24 2020 (r360712) +++ stable/12/sys/amd64/include/vmm.h Wed May 6 22:20:37 2020 (r360713) @@ -95,6 +95,7 @@ enum vm_reg_name { VM_REG_GUEST_DR2, VM_REG_GUEST_DR3, VM_REG_GUEST_DR6, + VM_REG_GUEST_ENTRY_INST_LENGTH, VM_REG_LAST }; @@ -434,6 +435,7 @@ enum vm_cap_type { VM_CAP_PAUSE_EXIT, VM_CAP_UNRESTRICTED_GUEST, VM_CAP_ENABLE_INVPCID, + VM_CAP_BPT_EXIT, VM_CAP_MAX }; @@ -559,6 +561,7 @@ enum vm_exitcode { VM_EXITCODE_REQIDLE, VM_EXITCODE_DEBUG, VM_EXITCODE_VMINSN, + VM_EXITCODE_BPT, VM_EXITCODE_MAX }; @@ -645,6 +648,9 @@ struct vm_exit { uint64_t exitinfo1; uint64_t exitinfo2; } svm; + struct { + int inst_length; + } bpt; struct { uint32_t code; /* ecx value */ uint64_t wval; Modified: stable/12/sys/amd64/vmm/amd/svm.c ============================================================================== --- stable/12/sys/amd64/vmm/amd/svm.c Wed May 6 22:18:24 2020 (r360712) +++ stable/12/sys/amd64/vmm/amd/svm.c Wed May 6 22:20:37 2020 (r360713) @@ -2187,6 +2187,11 @@ svm_setreg(void *arg, int vcpu, int ident, uint64_t va return (0); } + if (ident == VM_REG_GUEST_ENTRY_INST_LENGTH) { + /* Ignore. */ + return (0); + } + /* * XXX deal with CR3 and invalidate TLB entries tagged with the * vcpu's ASID. This needs to be treated differently depending on Modified: stable/12/sys/amd64/vmm/intel/vmcs.c ============================================================================== --- stable/12/sys/amd64/vmm/intel/vmcs.c Wed May 6 22:18:24 2020 (r360712) +++ stable/12/sys/amd64/vmm/intel/vmcs.c Wed May 6 22:20:37 2020 (r360713) @@ -120,6 +120,8 @@ vmcs_field_encoding(int ident) return (VMCS_GUEST_PDPTE2); case VM_REG_GUEST_PDPTE3: return (VMCS_GUEST_PDPTE3); + case VM_REG_GUEST_ENTRY_INST_LENGTH: + return (VMCS_ENTRY_INST_LENGTH); default: return (-1); } Modified: stable/12/sys/amd64/vmm/intel/vmx.c ============================================================================== --- stable/12/sys/amd64/vmm/intel/vmx.c Wed May 6 22:18:24 2020 (r360712) +++ stable/12/sys/amd64/vmm/intel/vmx.c Wed May 6 22:20:37 2020 (r360713) @@ -1085,6 +1085,7 @@ vmx_vminit(struct vm *vm, pmap_t pmap) vmx->cap[i].set = 0; vmx->cap[i].proc_ctls = procbased_ctls; vmx->cap[i].proc_ctls2 = procbased_ctls2; + vmx->cap[i].exc_bitmap = exc_bitmap; vmx->state[i].nextrip = ~0; vmx->state[i].lastcpu = NOCPU; @@ -2561,6 +2562,18 @@ vmx_exit_process(struct vmx *vmx, int vcpu, struct vm_ return (1); } + /* + * If the hypervisor has requested user exits for + * debug exceptions, bounce them out to userland. + */ + if (intr_type == VMCS_INTR_T_SWEXCEPTION && intr_vec == IDT_BP && + (vmx->cap[vcpu].set & (1 << VM_CAP_BPT_EXIT))) { + vmexit->exitcode = VM_EXITCODE_BPT; + vmexit->u.bpt.inst_length = vmexit->inst_length; + vmexit->inst_length = 0; + break; + } + if (intr_vec == IDT_PF) { error = vmxctx_setreg(vmxctx, VM_REG_GUEST_CR2, qual); KASSERT(error == 0, ("%s: vmxctx_setreg(cr2) error %d", @@ -3326,6 +3339,9 @@ vmx_getcap(void *arg, int vcpu, int type, int *retval) if (cap_invpcid) ret = 0; break; + case VM_CAP_BPT_EXIT: + ret = 0; + break; default: break; } @@ -3397,11 +3413,25 @@ vmx_setcap(void *arg, int vcpu, int type, int val) reg = VMCS_SEC_PROC_BASED_CTLS; } break; + case VM_CAP_BPT_EXIT: + retval = 0; + + /* Don't change the bitmap if we are tracing all exceptions. */ + if (vmx->cap[vcpu].exc_bitmap != 0xffffffff) { + pptr = &vmx->cap[vcpu].exc_bitmap; + baseval = *pptr; + flag = (1 << IDT_BP); + reg = VMCS_EXCEPTION_BITMAP; + } + break; default: break; } - if (retval == 0) { + if (retval) + return (retval); + + if (pptr != NULL) { if (val) { baseval |= flag; } else { @@ -3411,26 +3441,23 @@ vmx_setcap(void *arg, int vcpu, int type, int val) error = vmwrite(reg, baseval); VMCLEAR(vmcs); - if (error) { - retval = error; - } else { - /* - * Update optional stored flags, and record - * setting - */ - if (pptr != NULL) { - *pptr = baseval; - } + if (error) + return (error); - if (val) { - vmx->cap[vcpu].set |= (1 << type); - } else { - vmx->cap[vcpu].set &= ~(1 << type); - } - } + /* + * Update optional stored flags, and record + * setting + */ + *pptr = baseval; } - return (retval); + if (val) { + vmx->cap[vcpu].set |= (1 << type); + } else { + vmx->cap[vcpu].set &= ~(1 << type); + } + + return (0); } struct vlapic_vtx { Modified: stable/12/sys/amd64/vmm/intel/vmx.h ============================================================================== --- stable/12/sys/amd64/vmm/intel/vmx.h Wed May 6 22:18:24 2020 (r360712) +++ stable/12/sys/amd64/vmm/intel/vmx.h Wed May 6 22:20:37 2020 (r360713) @@ -87,6 +87,7 @@ struct vmxcap { int set; uint32_t proc_ctls; uint32_t proc_ctls2; + uint32_t exc_bitmap; }; struct vmxstate { Modified: stable/12/usr.sbin/bhyve/bhyve.8 ============================================================================== --- stable/12/usr.sbin/bhyve/bhyve.8 Wed May 6 22:18:24 2020 (r360712) +++ stable/12/usr.sbin/bhyve/bhyve.8 Wed May 6 22:20:37 2020 (r360713) @@ -24,7 +24,7 @@ .\" .\" $FreeBSD$ .\" -.Dd December 11, 2018 +.Dd December 13, 2019 .Dt BHYVE 8 .Os .Sh NAME @@ -519,7 +519,10 @@ The running guest can be interrupted by the debugger a .Pp Single stepping is only supported on Intel CPUs supporting the MTRAP VM exit. .Pp -Breakpoints are not supported. +Breakpoints are supported on Intel CPUs that support single stepping. +Note that continuing from a breakpoint while interrupts are enabled in the +guest may not work as expected due to timer interrupts firing while single +stepping over the breakpoint. .Sh SIGNAL HANDLING .Nm deals with the following signals: Modified: stable/12/usr.sbin/bhyve/bhyverun.c ============================================================================== --- stable/12/usr.sbin/bhyve/bhyverun.c Wed May 6 22:18:24 2020 (r360712) +++ stable/12/usr.sbin/bhyve/bhyverun.c Wed May 6 22:20:37 2020 (r360713) @@ -785,6 +785,18 @@ vmexit_debug(struct vmctx *ctx, struct vm_exit *vmexit return (VMEXIT_CONTINUE); } +static int +vmexit_breakpoint(struct vmctx *ctx, struct vm_exit *vmexit, int *pvcpu) +{ + + if (gdb_port == 0) { + fprintf(stderr, "vm_loop: unexpected VMEXIT_DEBUG\n"); + exit(4); + } + gdb_cpu_breakpoint(*pvcpu, vmexit); + return (VMEXIT_CONTINUE); +} + static vmexit_handler_t handler[VM_EXITCODE_MAX] = { [VM_EXITCODE_INOUT] = vmexit_inout, [VM_EXITCODE_INOUT_STR] = vmexit_inout, @@ -800,6 +812,7 @@ static vmexit_handler_t handler[VM_EXITCODE_MAX] = { [VM_EXITCODE_SUSPENDED] = vmexit_suspend, [VM_EXITCODE_TASK_SWITCH] = vmexit_task_switch, [VM_EXITCODE_DEBUG] = vmexit_debug, + [VM_EXITCODE_BPT] = vmexit_breakpoint, }; static void Modified: stable/12/usr.sbin/bhyve/gdb.c ============================================================================== --- stable/12/usr.sbin/bhyve/gdb.c Wed May 6 22:18:24 2020 (r360712) +++ stable/12/usr.sbin/bhyve/gdb.c Wed May 6 22:20:37 2020 (r360713) @@ -36,6 +36,7 @@ __FBSDID("$FreeBSD$"); #include <sys/endian.h> #include <sys/ioctl.h> #include <sys/mman.h> +#include <sys/queue.h> #include <sys/socket.h> #include <machine/atomic.h> #include <machine/specialreg.h> @@ -59,6 +60,7 @@ __FBSDID("$FreeBSD$"); #include <vmmapi.h> #include "bhyverun.h" +#include "gdb.h" #include "mem.h" #include "mevent.h" @@ -76,8 +78,7 @@ static struct mevent *read_event, *write_event; static cpuset_t vcpus_active, vcpus_suspended, vcpus_waiting; static pthread_mutex_t gdb_lock; static pthread_cond_t idle_vcpus; -static bool stop_pending, first_stop; -static int stepping_vcpu, stopped_vcpu; +static bool first_stop, report_next_stop, swbreak_enabled; /* * An I/O buffer contains 'capacity' bytes of room at 'data'. For a @@ -93,11 +94,44 @@ struct io_buffer { size_t len; }; +struct breakpoint { + uint64_t gpa; + uint8_t shadow_inst; + TAILQ_ENTRY(breakpoint) link; +}; + +/* + * When a vCPU stops to due to an event that should be reported to the + * debugger, information about the event is stored in this structure. + * The vCPU thread then sets 'stopped_vcpu' if it is not already set + * and stops other vCPUs so the event can be reported. The + * report_stop() function reports the event for the 'stopped_vcpu' + * vCPU. When the debugger resumes execution via continue or step, + * the event for 'stopped_vcpu' is cleared. vCPUs will loop in their + * event handlers until the associated event is reported or disabled. + * + * An idle vCPU will have all of the boolean fields set to false. + * + * When a vCPU is stepped, 'stepping' is set to true when the vCPU is + * released to execute the stepped instruction. When the vCPU reports + * the stepping trap, 'stepped' is set. + * + * When a vCPU hits a breakpoint set by the debug server, + * 'hit_swbreak' is set to true. + */ +struct vcpu_state { + bool stepping; + bool stepped; + bool hit_swbreak; +}; + static struct io_buffer cur_comm, cur_resp; static uint8_t cur_csum; -static int cur_vcpu; static struct vmctx *ctx; static int cur_fd = -1; +static TAILQ_HEAD(, breakpoint) breakpoints; +static struct vcpu_state *vcpu_state; +static int cur_vcpu, stopped_vcpu; const int gdb_regset[] = { VM_REG_GUEST_RAX, @@ -184,6 +218,8 @@ debug(const char *fmt, ...) #define debug(...) #endif +static void remove_all_sw_breakpoints(void); + static int guest_paging_info(int vcpu, struct vm_guest_paging *paging) { @@ -352,6 +388,11 @@ close_connection(void) io_buffer_reset(&cur_resp); cur_fd = -1; + remove_all_sw_breakpoints(); + + /* Clear any pending events. */ + memset(vcpu_state, 0, guest_ncpus * sizeof(*vcpu_state)); + /* Resume any stopped vCPUs. */ gdb_resume_vcpus(); pthread_mutex_unlock(&gdb_lock); @@ -557,7 +598,7 @@ append_integer(unsigned int value) if (value == 0) append_char('0'); else - append_unsigned_be(value, fls(value) + 7 / 8); + append_unsigned_be(value, (fls(value) + 7) / 8); } static void @@ -610,40 +651,81 @@ parse_threadid(const uint8_t *data, size_t len) return (parse_integer(data, len)); } +/* + * Report the current stop event to the debugger. If the stop is due + * to an event triggered on a specific vCPU such as a breakpoint or + * stepping trap, stopped_vcpu will be set to the vCPU triggering the + * stop. If 'set_cur_vcpu' is true, then cur_vcpu will be updated to + * the reporting vCPU for vCPU events. + */ static void -report_stop(void) +report_stop(bool set_cur_vcpu) { + struct vcpu_state *vs; start_packet(); - if (stopped_vcpu == -1) + if (stopped_vcpu == -1) { append_char('S'); - else + append_byte(GDB_SIGNAL_TRAP); + } else { + vs = &vcpu_state[stopped_vcpu]; + if (set_cur_vcpu) + cur_vcpu = stopped_vcpu; append_char('T'); - append_byte(GDB_SIGNAL_TRAP); - if (stopped_vcpu != -1) { + append_byte(GDB_SIGNAL_TRAP); append_string("thread:"); append_integer(stopped_vcpu + 1); append_char(';'); + if (vs->hit_swbreak) { + debug("$vCPU %d reporting swbreak\n", stopped_vcpu); + if (swbreak_enabled) + append_string("swbreak:;"); + } else if (vs->stepped) + debug("$vCPU %d reporting step\n", stopped_vcpu); + else + debug("$vCPU %d reporting ???\n", stopped_vcpu); } - stopped_vcpu = -1; finish_packet(); + report_next_stop = false; } +/* + * If this stop is due to a vCPU event, clear that event to mark it as + * acknowledged. + */ static void +discard_stop(void) +{ + struct vcpu_state *vs; + + if (stopped_vcpu != -1) { + vs = &vcpu_state[stopped_vcpu]; + vs->hit_swbreak = false; + vs->stepped = false; + stopped_vcpu = -1; + } + report_next_stop = true; +} + +static void gdb_finish_suspend_vcpus(void) { if (first_stop) { first_stop = false; stopped_vcpu = -1; - } else if (response_pending()) - stop_pending = true; - else { - report_stop(); + } else if (report_next_stop) { + assert(!response_pending()); + report_stop(true); send_pending_data(cur_fd); } } +/* + * vCPU threads invoke this function whenever the vCPU enters the + * debug server to pause or report an event. vCPU threads wait here + * as long as the debug server keeps them suspended. + */ static void _gdb_cpu_suspend(int vcpu, bool report_stop) { @@ -652,19 +734,28 @@ _gdb_cpu_suspend(int vcpu, bool report_stop) CPU_SET(vcpu, &vcpus_waiting); if (report_stop && CPU_CMP(&vcpus_waiting, &vcpus_suspended) == 0) gdb_finish_suspend_vcpus(); - while (CPU_ISSET(vcpu, &vcpus_suspended) && vcpu != stepping_vcpu) + while (CPU_ISSET(vcpu, &vcpus_suspended)) pthread_cond_wait(&idle_vcpus, &gdb_lock); CPU_CLR(vcpu, &vcpus_waiting); debug("$vCPU %d resuming\n", vcpu); } +/* + * Invoked at the start of a vCPU thread's execution to inform the + * debug server about the new thread. + */ void gdb_cpu_add(int vcpu) { debug("$vCPU %d starting\n", vcpu); pthread_mutex_lock(&gdb_lock); + assert(vcpu < guest_ncpus); CPU_SET(vcpu, &vcpus_active); + if (!TAILQ_EMPTY(&breakpoints)) { + vm_set_capability(ctx, vcpu, VM_CAP_BPT_EXIT, 1); + debug("$vCPU %d enabled breakpoint exits\n", vcpu); + } /* * If a vcpu is added while vcpus are stopped, suspend the new @@ -678,44 +769,149 @@ gdb_cpu_add(int vcpu) pthread_mutex_unlock(&gdb_lock); } +/* + * Invoked by vCPU before resuming execution. This enables stepping + * if the vCPU is marked as stepping. + */ +static void +gdb_cpu_resume(int vcpu) +{ + struct vcpu_state *vs; + int error; + + vs = &vcpu_state[vcpu]; + + /* + * Any pending event should already be reported before + * resuming. + */ + assert(vs->hit_swbreak == false); + assert(vs->stepped == false); + if (vs->stepping) { + error = vm_set_capability(ctx, vcpu, VM_CAP_MTRAP_EXIT, 1); + assert(error == 0); + } +} + +/* + * Handler for VM_EXITCODE_DEBUG used to suspend a vCPU when the guest + * has been suspended due to an event on different vCPU or in response + * to a guest-wide suspend such as Ctrl-C or the stop on attach. + */ void gdb_cpu_suspend(int vcpu) { pthread_mutex_lock(&gdb_lock); _gdb_cpu_suspend(vcpu, true); + gdb_cpu_resume(vcpu); pthread_mutex_unlock(&gdb_lock); } +static void +gdb_suspend_vcpus(void) +{ + + assert(pthread_mutex_isowned_np(&gdb_lock)); + debug("suspending all CPUs\n"); + vcpus_suspended = vcpus_active; + vm_suspend_cpu(ctx, -1); + if (CPU_CMP(&vcpus_waiting, &vcpus_suspended) == 0) + gdb_finish_suspend_vcpus(); +} + +/* + * Handler for VM_EXITCODE_MTRAP reported when a vCPU single-steps via + * the VT-x-specific MTRAP exit. + */ void gdb_cpu_mtrap(int vcpu) { + struct vcpu_state *vs; debug("$vCPU %d MTRAP\n", vcpu); pthread_mutex_lock(&gdb_lock); - if (vcpu == stepping_vcpu) { - stepping_vcpu = -1; + vs = &vcpu_state[vcpu]; + if (vs->stepping) { + vs->stepping = false; + vs->stepped = true; vm_set_capability(ctx, vcpu, VM_CAP_MTRAP_EXIT, 0); - vm_suspend_cpu(ctx, vcpu); - assert(stopped_vcpu == -1); - stopped_vcpu = vcpu; - _gdb_cpu_suspend(vcpu, true); + while (vs->stepped) { + if (stopped_vcpu == -1) { + debug("$vCPU %d reporting step\n", vcpu); + stopped_vcpu = vcpu; + gdb_suspend_vcpus(); + } + _gdb_cpu_suspend(vcpu, true); + } + gdb_cpu_resume(vcpu); } pthread_mutex_unlock(&gdb_lock); } -static void -gdb_suspend_vcpus(void) +static struct breakpoint * +find_breakpoint(uint64_t gpa) { + struct breakpoint *bp; - assert(pthread_mutex_isowned_np(&gdb_lock)); - debug("suspending all CPUs\n"); - vcpus_suspended = vcpus_active; - vm_suspend_cpu(ctx, -1); - if (CPU_CMP(&vcpus_waiting, &vcpus_suspended) == 0) - gdb_finish_suspend_vcpus(); + TAILQ_FOREACH(bp, &breakpoints, link) { + if (bp->gpa == gpa) + return (bp); + } + return (NULL); } +void +gdb_cpu_breakpoint(int vcpu, struct vm_exit *vmexit) +{ + struct breakpoint *bp; + struct vcpu_state *vs; + uint64_t gpa; + int error; + + pthread_mutex_lock(&gdb_lock); + error = guest_vaddr2paddr(vcpu, vmexit->rip, &gpa); + assert(error == 1); + bp = find_breakpoint(gpa); + if (bp != NULL) { + vs = &vcpu_state[vcpu]; + assert(vs->stepping == false); + assert(vs->stepped == false); + assert(vs->hit_swbreak == false); + vs->hit_swbreak = true; + vm_set_register(ctx, vcpu, VM_REG_GUEST_RIP, vmexit->rip); + for (;;) { + if (stopped_vcpu == -1) { + debug("$vCPU %d reporting breakpoint at rip %#lx\n", vcpu, + vmexit->rip); + stopped_vcpu = vcpu; + gdb_suspend_vcpus(); + } + _gdb_cpu_suspend(vcpu, true); + if (!vs->hit_swbreak) { + /* Breakpoint reported. */ + break; + } + bp = find_breakpoint(gpa); + if (bp == NULL) { + /* Breakpoint was removed. */ + vs->hit_swbreak = false; + break; + } + } + gdb_cpu_resume(vcpu); + } else { + debug("$vCPU %d injecting breakpoint at rip %#lx\n", vcpu, + vmexit->rip); + error = vm_set_register(ctx, vcpu, + VM_REG_GUEST_ENTRY_INST_LENGTH, vmexit->u.bpt.inst_length); + assert(error == 0); + error = vm_inject_exception(ctx, vcpu, IDT_BP, 0, 0, 0); + assert(error == 0); + } + pthread_mutex_unlock(&gdb_lock); +} + static bool gdb_step_vcpu(int vcpu) { @@ -725,9 +921,11 @@ gdb_step_vcpu(int vcpu) error = vm_get_capability(ctx, vcpu, VM_CAP_MTRAP_EXIT, &val); if (error < 0) return (false); - error = vm_set_capability(ctx, vcpu, VM_CAP_MTRAP_EXIT, 1); + + discard_stop(); + vcpu_state[vcpu].stepping = true; vm_resume_cpu(ctx, vcpu); - stepping_vcpu = vcpu; + CPU_CLR(vcpu, &vcpus_suspended); pthread_cond_broadcast(&idle_vcpus); return (true); } @@ -982,6 +1180,174 @@ gdb_write_mem(const uint8_t *data, size_t len) } static bool +set_breakpoint_caps(bool enable) +{ + cpuset_t mask; + int vcpu; + + mask = vcpus_active; + while (!CPU_EMPTY(&mask)) { + vcpu = CPU_FFS(&mask) - 1; + CPU_CLR(vcpu, &mask); + if (vm_set_capability(ctx, vcpu, VM_CAP_BPT_EXIT, + enable ? 1 : 0) < 0) + return (false); + debug("$vCPU %d %sabled breakpoint exits\n", vcpu, + enable ? "en" : "dis"); + } + return (true); +} + +static void +remove_all_sw_breakpoints(void) +{ + struct breakpoint *bp, *nbp; + uint8_t *cp; + + if (TAILQ_EMPTY(&breakpoints)) + return; + + TAILQ_FOREACH_SAFE(bp, &breakpoints, link, nbp) { + debug("remove breakpoint at %#lx\n", bp->gpa); + cp = paddr_guest2host(ctx, bp->gpa, 1); + *cp = bp->shadow_inst; + TAILQ_REMOVE(&breakpoints, bp, link); + free(bp); + } + TAILQ_INIT(&breakpoints); + set_breakpoint_caps(false); +} + +static void +update_sw_breakpoint(uint64_t gva, int kind, bool insert) +{ + struct breakpoint *bp; + uint64_t gpa; + uint8_t *cp; + int error; + + if (kind != 1) { + send_error(EINVAL); + return; + } + + error = guest_vaddr2paddr(cur_vcpu, gva, &gpa); + if (error == -1) { + send_error(errno); + return; + } + if (error == 0) { + send_error(EFAULT); + return; + } + + cp = paddr_guest2host(ctx, gpa, 1); + + /* Only permit breakpoints in guest RAM. */ + if (cp == NULL) { + send_error(EFAULT); + return; + } + + /* Find any existing breakpoint. */ + bp = find_breakpoint(gpa); + + /* + * Silently ignore duplicate commands since the protocol + * requires these packets to be idempotent. + */ + if (insert) { + if (bp == NULL) { + if (TAILQ_EMPTY(&breakpoints) && + !set_breakpoint_caps(true)) { + send_empty_response(); + return; + } + bp = malloc(sizeof(*bp)); + bp->gpa = gpa; + bp->shadow_inst = *cp; + *cp = 0xcc; /* INT 3 */ + TAILQ_INSERT_TAIL(&breakpoints, bp, link); + debug("new breakpoint at %#lx\n", gpa); + } + } else { + if (bp != NULL) { + debug("remove breakpoint at %#lx\n", gpa); + *cp = bp->shadow_inst; + TAILQ_REMOVE(&breakpoints, bp, link); + free(bp); + if (TAILQ_EMPTY(&breakpoints)) + set_breakpoint_caps(false); + } + } + send_ok(); +} + +static void +parse_breakpoint(const uint8_t *data, size_t len) +{ + uint64_t gva; + uint8_t *cp; + bool insert; + int kind, type; + + insert = data[0] == 'Z'; + + /* Skip 'Z/z' */ + data += 1; + len -= 1; + + /* Parse and consume type. */ + cp = memchr(data, ',', len); + if (cp == NULL || cp == data) { + send_error(EINVAL); + return; + } + type = parse_integer(data, cp - data); + len -= (cp - data) + 1; + data += (cp - data) + 1; + + /* Parse and consume address. */ + cp = memchr(data, ',', len); + if (cp == NULL || cp == data) { + send_error(EINVAL); + return; + } + gva = parse_integer(data, cp - data); + len -= (cp - data) + 1; + data += (cp - data) + 1; + + /* Parse and consume kind. */ + cp = memchr(data, ';', len); + if (cp == data) { + send_error(EINVAL); + return; + } + if (cp != NULL) { + /* + * We do not advertise support for either the + * ConditionalBreakpoints or BreakpointCommands + * features, so we should not be getting conditions or + * commands from the remote end. + */ + send_empty_response(); + return; + } + kind = parse_integer(data, len); + data += len; + len = 0; + + switch (type) { + case 0: + update_sw_breakpoint(gva, kind, insert); + break; + default: + send_empty_response(); + break; + } +} + +static bool command_equals(const uint8_t *data, size_t len, const char *cmd) { @@ -1039,7 +1405,8 @@ check_features(const uint8_t *data, size_t len) value = NULL; } - /* No currently supported features. */ + if (strcmp(feature, "swbreak") == 0) + swbreak_enabled = supported; } free(str); @@ -1047,6 +1414,7 @@ check_features(const uint8_t *data, size_t len) /* This is an arbitrary limit. */ append_string("PacketSize=4096"); + append_string(";swbreak+"); finish_packet(); } @@ -1140,7 +1508,7 @@ handle_command(const uint8_t *data, size_t len) break; } - /* Don't send a reply until a stop occurs. */ + discard_stop(); gdb_resume_vcpus(); break; case 'D': @@ -1212,13 +1580,12 @@ handle_command(const uint8_t *data, size_t len) break; } break; + case 'z': + case 'Z': + parse_breakpoint(data, len); + break; case '?': - /* XXX: Only if stopped? */ - /* For now, just report that we are always stopped. */ - start_packet(); - append_char('S'); - append_byte(GDB_SIGNAL_TRAP); - finish_packet(); + report_stop(false); break; case 'G': /* TODO */ case 'v': @@ -1229,8 +1596,6 @@ handle_command(const uint8_t *data, size_t len) case 'Q': /* TODO */ case 't': /* TODO */ case 'X': /* TODO */ - case 'z': /* TODO */ - case 'Z': /* TODO */ default: send_empty_response(); } @@ -1261,9 +1626,8 @@ check_command(int fd) if (response_pending()) io_buffer_reset(&cur_resp); io_buffer_consume(&cur_comm, 1); - if (stop_pending) { - stop_pending = false; - report_stop(); + if (stopped_vcpu != -1 && report_next_stop) { + report_stop(true); send_pending_data(fd); } break; @@ -1417,12 +1781,11 @@ new_connection(int fd, enum ev_type event, void *arg) cur_fd = s; cur_vcpu = 0; - stepping_vcpu = -1; stopped_vcpu = -1; - stop_pending = false; /* Break on attach. */ first_stop = true; + report_next_stop = false; gdb_suspend_vcpus(); pthread_mutex_unlock(&gdb_lock); } @@ -1474,6 +1837,9 @@ init_gdb(struct vmctx *_ctx, int sport, bool wait) if (listen(s, 1) < 0) err(1, "gdb socket listen"); + stopped_vcpu = -1; + TAILQ_INIT(&breakpoints); + vcpu_state = calloc(guest_ncpus, sizeof(*vcpu_state)); if (wait) { /* * Set vcpu 0 in vcpus_suspended. This will trigger the @@ -1481,9 +1847,8 @@ init_gdb(struct vmctx *_ctx, int sport, bool wait) * it starts execution. The vcpu will remain suspended * until a debugger connects. */ - stepping_vcpu = -1; - stopped_vcpu = -1; CPU_SET(0, &vcpus_suspended); + stopped_vcpu = 0; } flags = fcntl(s, F_GETFL); Modified: stable/12/usr.sbin/bhyve/gdb.h ============================================================================== --- stable/12/usr.sbin/bhyve/gdb.h Wed May 6 22:18:24 2020 (r360712) +++ stable/12/usr.sbin/bhyve/gdb.h Wed May 6 22:20:37 2020 (r360713) @@ -32,6 +32,7 @@ #define __GDB_H__ void gdb_cpu_add(int vcpu); +void gdb_cpu_breakpoint(int vcpu, struct vm_exit *vmexit); void gdb_cpu_mtrap(int vcpu); void gdb_cpu_suspend(int vcpu); void init_gdb(struct vmctx *ctx, int sport, bool wait);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202005062220.046MKbPd035977>