From patchwork Fri Oct 14 11:53:03 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Claudio Imbrenda X-Patchwork-Id: 9376471 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 606EF60779 for ; Fri, 14 Oct 2016 11:53:49 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 50C162A2C4 for ; Fri, 14 Oct 2016 11:53:49 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 454B82A65E; Fri, 14 Oct 2016 11:53:49 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 9DD752A2C4 for ; Fri, 14 Oct 2016 11:53:48 +0000 (UTC) Received: from localhost ([::1]:46693 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bv13b-0005H9-Pb for patchwork-qemu-devel@patchwork.kernel.org; Fri, 14 Oct 2016 07:53:47 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55552) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bv136-0005FH-2X for qemu-devel@nongnu.org; Fri, 14 Oct 2016 07:53:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bv131-0003S1-KE for qemu-devel@nongnu.org; Fri, 14 Oct 2016 07:53:15 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:32925) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bv131-0003Rh-B5 for qemu-devel@nongnu.org; Fri, 14 Oct 2016 07:53:11 -0400 Received: from pps.filterd (m0098409.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id u9EBnNpo125903 for ; Fri, 14 Oct 2016 07:53:10 -0400 Received: from e06smtp07.uk.ibm.com (e06smtp07.uk.ibm.com [195.75.94.103]) by mx0a-001b2d01.pphosted.com with ESMTP id 262w6258x5-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Fri, 14 Oct 2016 07:53:09 -0400 Received: from localhost by e06smtp07.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 14 Oct 2016 12:53:07 +0100 Received: from d06dlp03.portsmouth.uk.ibm.com (9.149.20.15) by e06smtp07.uk.ibm.com (192.168.101.137) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Fri, 14 Oct 2016 12:53:05 +0100 Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id E2AD61B08061 for ; Fri, 14 Oct 2016 12:55:05 +0100 (BST) Received: from d06av10.portsmouth.uk.ibm.com (d06av10.portsmouth.uk.ibm.com [9.149.37.251]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u9EBr44H23527632 for ; Fri, 14 Oct 2016 11:53:04 GMT Received: from d06av10.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av10.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u9EAr6hi004827 for ; Fri, 14 Oct 2016 04:53:06 -0600 Received: from p-imbrenda.boeblingen.de.ibm.com (dyn-9-152-224-35.boeblingen.de.ibm.com [9.152.224.35]) by d06av10.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id u9EAr5m1004781 (version=TLSv1/SSLv3 cipher=AES256-SHA256 bits=256 verify=NO) for ; Fri, 14 Oct 2016 04:53:05 -0600 From: Claudio Imbrenda To: qemu-devel@nongnu.org Date: Fri, 14 Oct 2016 13:53:03 +0200 X-Mailer: git-send-email 1.9.1 In-Reply-To: <1476445983-16661-1-git-send-email-imbrenda@linux.vnet.ibm.com> References: <1476445983-16661-1-git-send-email-imbrenda@linux.vnet.ibm.com> X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16101411-0028-0000-0000-000002306F23 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16101411-0029-0000-0000-000020CE4B9D Message-Id: <1476445983-16661-3-git-send-email-imbrenda@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2016-10-14_06:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=3 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1609300000 definitions=main-1610140211 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [generic] X-Received-From: 148.163.156.1 Subject: [Qemu-devel] [PATCH v2 2/2] gdbstub: Fix vCont behaviour X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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 some additional helper functions to selectively restart only some CPUs * completely rewrites the vCont parsing code Please note that this improvement only works in system emulation mode, when in userspace emulation mode the old behaviour is preserved. Signed-off-by: Claudio Imbrenda --- gdbstub.c | 226 +++++++++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 179 insertions(+), 47 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index ecea8c4..ea42afa 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -386,6 +386,61 @@ static inline void gdb_continue(GDBState *s) #endif } +static void gdb_single_step_cpus(CPUState **scpus, int flags) +{ + CPUState *cpu; + int cx; + + if (!scpus) { + CPU_FOREACH(cpu) { + cpu_single_step(cpu, flags); + } + } else { + for (cx = 0; scpus[cx]; cx++) { + cpu_single_step(scpus[cx], flags); + } + } +} + +/* + * Resume execution, per CPU actions. For user-more emulation it's + * equivalent to gdb_continue . + */ +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 + if (!runstate_needs_reset()) { + if (vm_prepare_start()) { + return 0; + } + + if (def == 0) { + gdb_single_step_cpus(scpus, sstep_flags); + /* CPUs not in scpus or ccpus stay stopped */ + qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true); + resume_some_vcpus(scpus); + resume_some_vcpus(ccpus); + } else if (def == 'c' || def == 'C') { + gdb_single_step_cpus(scpus, sstep_flags); + qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true); + resume_all_vcpus(); + } else if (def == 's' || def == 'S') { + gdb_single_step_cpus(NULL, sstep_flags); + gdb_single_step_cpus(ccpus, 0); + qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true); + resume_all_vcpus(); + } else { + res = -1; + } + } +#endif + return res; +} + static void put_buffer(GDBState *s, const uint8_t *buf, int len) { #ifdef CONFIG_USER_ONLY @@ -784,6 +839,123 @@ static int is_query_packet(const char *p, const char *query, char separator) (p[query_len] == '\0' || p[query_len] == separator); } +/** + * gdb_handle_vcont - Parses and handles a vCont packet. + * returns -1 if a command is unsupported, -22 if there is a format error, + * 0 on success. + */ +static int gdb_handle_vcont(GDBState *s, const char *p) +{ + CPUState **s_cpus, **c_cpus; + CPUState *curcpu; + int res, idx, signal, scnt, ccnt; + char cur_action, default_action, broadcast_action; + unsigned long tmp; + + /* + * For us: vCont[;action[:thread-id]]... + * where action can be one of c s C S + */ + for (idx = scnt = 0; p[idx]; idx++) { + if (p[idx] == ';') { + scnt++; + } + } + s_cpus = g_new(CPUState *, scnt + 1); + c_cpus = g_new(CPUState *, scnt + 1); + scnt = ccnt = signal = 0; + default_action = broadcast_action = 0; + + /* + * res keeps track of what error we are returning, with -1 meaning + * that the command is unknown or unsupported, and thus returning + * an empty packet, while -22 returns an E22 packet due to + * invalid or incorrect parameters passed. + */ + res = 0; + while (*p) { + if (*p != ';') { + res = -1; + break; + } + p++; /* skip the ; */ + + /* unknown/invalid/unsupported command */ + if (*p != 'C' && *p != 'S' && *p != 'c' && *p != 's') { + res = -1; + break; + } + cur_action = *p; + if (*p == 'C' || *p == 'S') { + if (qemu_strtoul(p + 1, &p, 16, &tmp)) { + res = -22; + break; + } + signal = gdb_signal_to_target(tmp); + } else { + p++; + } + /* thread specified. special values: -1 = all, 0 = any */ + if (*p == ':') { + /* + * cannot specify an action for a single thread when an action + * was already specified for all threads + */ + if (broadcast_action) { + res = -22; + break; + } + p++; + if ((p[0] == '-') && (p[1] == '1')) { + /* + * specifying an action for all threads when some threads + * already have actions. + */ + if (scnt || ccnt) { + res = -22; + break; + } + broadcast_action = cur_action; + p += 2; + continue; + } + if (qemu_strtoul(p, &p, 16, &tmp)) { + res = -22; + break; + } + /* 0 means any thread, so we pick the first */ + idx = tmp ? tmp : 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, can't specify multiple defaults! */ + break; + } + default_action = cur_action; + } + } + if (!res) { + s->signal = signal; + s_cpus[scnt] = c_cpus[ccnt] = NULL; + default_action = broadcast_action ? broadcast_action : default_action; + gdb_continue_partial(s, default_action, s_cpus, c_cpus); + } + + g_free(s_cpus); + g_free(c_cpus); + + return res; +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *cpu; @@ -829,60 +1001,20 @@ 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; - p += 4; if (*p == '?') { put_packet(s, "vCont;c;C;s;S"); break; } - res = 0; - res_signal = 0; - res_thread = 0; - while (*p) { - int action, signal; - - if (*p++ != ';') { - res = 0; - 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; - } - } else if (action != 'c' && action != 's') { - res = 0; - 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; - } - } + + res = gdb_handle_vcont(s, p); + 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; - } - if (res == 's') { - cpu_single_step(s->c_cpu, sstep_flags); + if (res == -22) { + put_packet(s, "E22"); + break; } - s->signal = res_signal; - gdb_continue(s); - return RS_IDLE; + goto unknown_command; } break; } else {