Date: Thu, 17 Oct 2019 22:37:26 +0000 (UTC) From: Conrad Meyer <cem@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r353702 - head/sys/gdb Message-ID: <201910172237.x9HMbQ0a006493@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201910172237.x9HMbQ0a006493>