From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60670) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btZ6L-000170-W2 for qemu-devel@nongnu.org; Mon, 10 Oct 2016 07:50:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1btZ6H-0004NC-Jm for qemu-devel@nongnu.org; Mon, 10 Oct 2016 07:50:36 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:57963) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btZ6H-0004Mh-4y for qemu-devel@nongnu.org; Mon, 10 Oct 2016 07:50:33 -0400 Received: from pps.filterd (m0098396.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id u9ABhhYe113919 for ; Mon, 10 Oct 2016 07:50:32 -0400 Received: from e06smtp15.uk.ibm.com (e06smtp15.uk.ibm.com [195.75.94.111]) by mx0a-001b2d01.pphosted.com with ESMTP id 26058exgen-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Mon, 10 Oct 2016 07:50:32 -0400 Received: from localhost by e06smtp15.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 10 Oct 2016 12:50:29 +0100 Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id 1C52F219004D for ; Mon, 10 Oct 2016 12:49:43 +0100 (BST) Received: from d06av01.portsmouth.uk.ibm.com (d06av01.portsmouth.uk.ibm.com [9.149.37.212]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u9ABoPW723855332 for ; Mon, 10 Oct 2016 11:50:25 GMT Received: from d06av01.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av01.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u9ABoPHg006464 for ; Mon, 10 Oct 2016 05:50:25 -0600 Received: from p-imbrenda.boeblingen.de.ibm.com (dyn-9-152-224-35.boeblingen.de.ibm.com [9.152.224.35]) by d06av01.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id u9ABoOCh006371 (version=TLSv1/SSLv3 cipher=AES256-SHA256 bits=256 verify=NO) for ; Mon, 10 Oct 2016 05:50:25 -0600 From: Claudio Imbrenda Date: Mon, 10 Oct 2016 13:50:24 +0200 In-Reply-To: <1476100224-19760-1-git-send-email-imbrenda@linux.vnet.ibm.com> References: <1476100224-19760-1-git-send-email-imbrenda@linux.vnet.ibm.com> Message-Id: <1476100224-19760-3-git-send-email-imbrenda@linux.vnet.ibm.com> Subject: [Qemu-devel] [PATCH v1 2/2] gdbstub: Fix vCont behaviour List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org When GDB issues a "vCont", QEMU was not handling it correctly when multiple VCPUs are active. For vCont, for each thread (VCPU), it can be specified whether to single step, continue or stop that thread. The default is to stop a thread. However, when (for example) "vCont;s:2" is issued, all VCPUs continue to run, although all but VCPU nr 2 are to be stopped. This patch: * adds an additional helper function to selectively restart only some CPUs * completely rewrites the vCont parsing code Signed-off-by: Claudio Imbrenda --- gdbstub.c | 189 +++++++++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 150 insertions(+), 39 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index ecea8c4..0ce9c83 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -386,6 +386,65 @@ static inline void gdb_continue(GDBState *s) #endif } +/* Resume execution, per CPU actions. */ +static int gdb_continue_partial(GDBState *s, char def, CPUState **scpus, + CPUState **ccpus) +{ + int res = 0; +#ifdef CONFIG_USER_ONLY + s->running_state = 1; +#else + CPUState *cpu; + int cx; + + if (!runstate_needs_reset()) { + /* + * XXX vm_start also calls qemu_vmstop_requested(&requested); here, is + * it actually important? it's static in vl.c + */ + cpu_enable_ticks(); + runstate_set(RUN_STATE_RUNNING); + vm_state_notify(1, RUN_STATE_RUNNING); + qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true); + if (def == 0) { + for (cx = 0; scpus && scpus[cx]; cx++) { + cpu_single_step(scpus[cx], sstep_flags); + cpu_resume(scpus[cx]); + } + for (cx = 0; ccpus && ccpus[cx]; cx++) { + cpu_resume(ccpus[cx]); + } + } else if (def == 'c' || def == 'C') { + for (cx = 0; scpus && scpus[cx]; cx++) { + cpu_single_step(scpus[cx], sstep_flags); + } + CPU_FOREACH(cpu) { + cpu_resume(cpu); + } + } else if (def == 's' || def == 'S') { + CPU_FOREACH(cpu) { + cpu_single_step(cpu, sstep_flags); + } + for (cx = 0; ccpus && ccpus[cx]; cx++) { + cpu_single_step(cpu, 0); + } + CPU_FOREACH(cpu) { + cpu_resume(cpu); + } + } else { + res = -1; + } + /* + * XXX vm_start also calls qapi_event_send_resume(&error_abort); here, + * is it actually important? moreover I can't find where it's defined, + * and calling it here yields a compiler error. + */ + /* qapi_event_send_resume(&error_abort); */ + } +#endif + return res; +} + static void put_buffer(GDBState *s, const uint8_t *buf, int len) { #ifdef CONFIG_USER_ONLY @@ -829,62 +888,114 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) return RS_IDLE; case 'v': if (strncmp(p, "Cont", 4) == 0) { - int res_signal, res_thread; + CPUState **s_cpus, **c_cpus; + CPUState *curcpu; + int idx, signal, broadcast_action = 0; + int scnt, ccnt; + char default_action = 0; + char cur_action = 0; + unsigned long tmp; p += 4; if (*p == '?') { put_packet(s, "vCont;c;C;s;S"); break; } + if (*p != ';') { + goto unknown_command; + } + + for (idx = scnt = 0; p[idx]; idx++) { + if (p[idx] == ';') { + scnt++; + } + } + s_cpus = g_malloc((scnt + 1) * sizeof(*s_cpus)); + c_cpus = g_malloc((scnt + 1) * sizeof(*c_cpus)); + scnt = ccnt = 0; + signal = 0; + + /* + * For us: vCont[;action[:thread-id]]... + * where action can be one of c s C S + */ res = 0; - res_signal = 0; - res_thread = 0; while (*p) { - int action, signal; - - if (*p++ != ';') { - res = 0; + if (*p != ';') { + res = -1; break; } - action = *p++; - signal = 0; - if (action == 'C' || action == 'S') { - signal = gdb_signal_to_target(strtoul(p, (char **)&p, 16)); - if (signal == -1) { - signal = 0; + p++; /* skip the ; */ + + if (*p == 'C' || *p == 'S' || *p == 'c' || *p == 's') { + cur_action = *p; + if (*p == 'C' || *p == 'S') { + (void) qemu_strtoul(p + 1, &p, 16, &tmp); + signal = gdb_signal_to_target(tmp); + } else { + p++; + } + /* thread specified. special values: -1 = all, 0 = any */ + if (*p == ':') { + if (broadcast_action) { + res = -22; + break; + } + p++; + if ((p[0] == '-') && (p[1] == '1')) { + if (broadcast_action || scnt || ccnt) { + res = -22; + break; + } + broadcast_action = cur_action; + p += 2; + continue; + } + (void) qemu_strtoul(p, &p, 16, &tmp); + idx = tmp; + /* 0 means any thread, so we pick the first */ + idx = idx ? idx : 1; + curcpu = find_cpu(idx); + if (!curcpu) { + res = -22; + break; + } + if (cur_action == 's' || cur_action == 'S') { + s_cpus[scnt++] = curcpu; + } else { + c_cpus[ccnt++] = curcpu; + } + } else { /* no thread specified */ + if (default_action != 0) { + res = -22; /* error */ + break; + } + default_action = cur_action; } - } else if (action != 'c' && action != 's') { - res = 0; + } else { /* unknown/invalid/unsupported command */ + res = -1; break; } - thread = 0; - if (*p == ':') { - thread = strtoull(p+1, (char **)&p, 16); - } - action = tolower(action); - if (res == 0 || (res == 'c' && action == 's')) { - res = action; - res_signal = signal; - res_thread = thread; - } } if (res) { - if (res_thread != -1 && res_thread != 0) { - cpu = find_cpu(res_thread); - if (cpu == NULL) { - put_packet(s, "E22"); - break; - } - s->c_cpu = cpu; + g_free(s_cpus); + g_free(c_cpus); + if (res == -1) { + goto unknown_command; } - if (res == 's') { - cpu_single_step(s->c_cpu, sstep_flags); - } - s->signal = res_signal; - gdb_continue(s); - return RS_IDLE; + put_packet(s, "E22"); + break; } - break; + s->signal = signal; + s_cpus[scnt] = c_cpus[ccnt] = NULL; + if (broadcast_action) { + gdb_continue_partial(s, broadcast_action, NULL, NULL); + } else { + gdb_continue_partial(s, default_action, s_cpus, c_cpus); + } + g_free(s_cpus); + g_free(c_cpus); + return RS_IDLE; } else { goto unknown_command; } -- 1.9.1