From owner-svn-src-all@freebsd.org Thu Oct 17 22:37:27 2019 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 59E5515F3BA; Thu, 17 Oct 2019 22:37:27 +0000 (UTC) (envelope-from cem@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 46vPDl1fp8z3Kgy; Thu, 17 Oct 2019 22:37:27 +0000 (UTC) (envelope-from cem@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 15FEF27675; Thu, 17 Oct 2019 22:37:27 +0000 (UTC) (envelope-from cem@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x9HMbQaT006497; Thu, 17 Oct 2019 22:37:26 GMT (envelope-from cem@FreeBSD.org) Received: (from cem@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x9HMbQ0a006493; Thu, 17 Oct 2019 22:37:26 GMT (envelope-from cem@FreeBSD.org) Message-Id: <201910172237.x9HMbQ0a006493@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: cem set sender to cem@FreeBSD.org using -f From: Conrad Meyer Date: Thu, 17 Oct 2019 22:37:26 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r353702 - head/sys/gdb X-SVN-Group: head X-SVN-Commit-Author: cem X-SVN-Commit-Paths: head/sys/gdb X-SVN-Commit-Revision: 353702 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Oct 2019 22:37:27 -0000 Author: cem Date: Thu Oct 17 22:37:25 2019 New Revision: 353702 URL: https://svnweb.freebsd.org/changeset/base/353702 Log: gdb(4): Implement support for NoAckMode When the underlying debugport transport is reliable, GDB's additional checksums and acknowledgements are redundant. NoAckMode eliminates the the acks and allows us to skip checking RX checksums. The GDB packet framing does not change, so unfortunately (valid) checksums are still included as message trailers. The gdb(4) stub in FreeBSD advertises support for the feature in response to the client's 'qSupported' request IFF the current debugport has the gdb_dbfeatures flag GDB_DBGP_FEAT_RELIABLE set. Currently, only netgdb(4) supports this feature. If the remote GDB client supports the feature and does not have it disabled via a GDB configuration knob, it may instruct our gdb(4) stub to enter NoAckMode. Unless and until it issues that command, we must continue to transmit acks as usual (and for now, we continue to wait until we receive them as well, even if we know the debugport is on a reliable transport). In the kernel sources, the sense of the flag representing the state of the feature is reversed from that of the GDB command. (I.e., it is 'gdb_ackmode', not 'gdb_noackmode.') This is to avoid confusing double- negative conditions. For reference, see: * https://sourceware.org/gdb/onlinedocs/gdb/Packet-Acknowledgment.html * https://sourceware.org/gdb/onlinedocs/gdb/General-Query-Packets.html#QStartNoAckMode Reviewed by: jhb, markj (both earlier version) Differential Revision: https://reviews.freebsd.org/D21761 Modified: head/sys/gdb/gdb.h head/sys/gdb/gdb_int.h head/sys/gdb/gdb_main.c head/sys/gdb/gdb_packet.c head/sys/gdb/netgdb.c Modified: head/sys/gdb/gdb.h ============================================================================== --- head/sys/gdb/gdb.h Thu Oct 17 21:39:23 2019 (r353701) +++ head/sys/gdb/gdb.h Thu Oct 17 22:37:25 2019 (r353702) @@ -55,6 +55,9 @@ struct gdb_dbgport { deadcode and never invoked for so long I don't want to just blindly start invoking it without opt-in. */ +#define GDB_DBGP_FEAT_RELIABLE 0x2 /* The debugport promises it is a + reliable transport, which allows GDB + acks to be turned off. */ #define GDB_DBGPORT(name, probe, init, term, getc, putc) \ static struct gdb_dbgport name##_gdb_dbgport = { \ Modified: head/sys/gdb/gdb_int.h ============================================================================== --- head/sys/gdb/gdb_int.h Thu Oct 17 21:39:23 2019 (r353701) +++ head/sys/gdb/gdb_int.h Thu Oct 17 22:37:25 2019 (r353702) @@ -54,6 +54,8 @@ extern char *gdb_rxp; extern size_t gdb_rxsz; extern char *gdb_txp; +extern bool gdb_ackmode; + #ifdef DDB /* If set, return to DDB when controlling GDB detaches. */ extern bool gdb_return_to_ddb; @@ -132,6 +134,20 @@ static __inline void gdb_tx_varhex(uintmax_t n) { gdb_txp += sprintf(gdb_txp, "%jx", n); +} + +static __inline void +gdb_nack(void) +{ + if (gdb_ackmode) + gdb_cur->gdb_putc('-'); +} + +static __inline void +gdb_ack(void) +{ + if (gdb_ackmode) + gdb_cur->gdb_putc('+'); } #endif /* !_GDB_GDB_INT_H_ */ Modified: head/sys/gdb/gdb_main.c ============================================================================== --- head/sys/gdb/gdb_main.c Thu Oct 17 21:39:23 2019 (r353701) +++ head/sys/gdb/gdb_main.c Thu Oct 17 22:37:25 2019 (r353702) @@ -57,6 +57,7 @@ SET_DECLARE(gdb_dbgport_set, struct gdb_dbgport); struct gdb_dbgport *gdb_cur = NULL; int gdb_listening = 0; +bool gdb_ackmode = true; static unsigned char gdb_bindata[64]; @@ -262,6 +263,14 @@ gdb_do_qsupported(uint32_t *feat) gdb_tx_str(";qXfer:threads:read+"); /* + * If the debugport is a reliable transport, request No Ack mode from + * the server. The server may or may not choose to enter No Ack mode. + * https://sourceware.org/gdb/onlinedocs/gdb/Packet-Acknowledgment.html + */ + if (gdb_cur->gdb_dbfeatures & GDB_DBGP_FEAT_RELIABLE) + gdb_tx_str(";QStartNoAckMode+"); + + /* * Future consideration: * - vCont * - multiprocess @@ -610,6 +619,8 @@ gdb_trap(int type, int code) } gdb_listening = 0; + gdb_ackmode = true; + /* * Send a T packet. We currently do not support watchpoints (the * awatch, rwatch or watch elements). @@ -759,6 +770,22 @@ gdb_trap(int type, int code) } else if (gdb_rx_equal("Search:memory:")) { gdb_do_mem_search(); } else if (!gdb_cpu_query()) + gdb_tx_empty(); + break; + case 'Q': + if (gdb_rx_equal("StartNoAckMode")) { + if ((gdb_cur->gdb_dbfeatures & + GDB_DBGP_FEAT_RELIABLE) == 0) { + /* + * Shouldn't happen if we didn't + * advertise support. Reject. + */ + gdb_tx_empty(); + break; + } + gdb_ackmode = false; + gdb_tx_ok(); + } else gdb_tx_empty(); break; case 's': { /* Step. */ Modified: head/sys/gdb/gdb_packet.c ============================================================================== --- head/sys/gdb/gdb_packet.c Thu Oct 17 21:39:23 2019 (r353701) +++ head/sys/gdb/gdb_packet.c Thu Oct 17 22:37:25 2019 (r353702) @@ -134,18 +134,28 @@ gdb_rx_begin(void) /* Bail out on a buffer overflow. */ if (c != '#') { - gdb_cur->gdb_putc('-'); + gdb_nack(); return (ENOSPC); } + /* + * In Not-AckMode, we can assume reliable transport and neither + * need to verify checksums nor send Ack/Nack. + */ + if (!gdb_ackmode) + break; + c = gdb_getc(); cksum -= (C2N(c) << 4) & 0xf0; c = gdb_getc(); cksum -= C2N(c) & 0x0f; - gdb_cur->gdb_putc((cksum == 0) ? '+' : '-'); - if (cksum != 0) + if (cksum == 0) { + gdb_ack(); + } else { + gdb_nack(); printf("GDB: packet `%s' has invalid checksum\n", gdb_rxbuf); + } } while (cksum != 0); gdb_rxp = gdb_rxbuf; @@ -336,6 +346,14 @@ gdb_tx_end(void) gdb_cur->gdb_putc(N2C(c)); getack: + /* + * In NoAckMode, it is assumed that the underlying transport is + * reliable and thus neither conservant sends acknowledgements; + * there is nothing to wait for here. + */ + if (!gdb_ackmode) + break; + c = gdb_getc(); } while (c != '+'); Modified: head/sys/gdb/netgdb.c ============================================================================== --- head/sys/gdb/netgdb.c Thu Oct 17 21:39:23 2019 (r353701) +++ head/sys/gdb/netgdb.c Thu Oct 17 22:37:25 2019 (r353702) @@ -272,7 +272,7 @@ static struct gdb_dbgport netgdb_gdb_dbgport = { .gdb_putc = netgdb_dbg_putc, .gdb_term = netgdb_fini, .gdb_sendpacket = netgdb_dbg_sendpacket, - .gdb_dbfeatures = GDB_DBGP_FEAT_WANTTERM, + .gdb_dbfeatures = GDB_DBGP_FEAT_WANTTERM | GDB_DBGP_FEAT_RELIABLE, }; static void