[OpenOCD-commit] Main OpenOCD repository branch, master, updated. v0.12.0-549-g35e4c4616

[openocd-gerrit <ope...@us...>]

This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "Main OpenOCD repository".

The branch, master has been updated
       via  35e4c4616f5a924313e85290f56f84f7a8534801 (commit)
       via  0c0243228cb958b7c12c2e5a81c84d35734fce25 (commit)
      from  01a797af143398c6f3aa1eb6f8949deff4ccf044 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 35e4c4616f5a924313e85290f56f84f7a8534801
Author: Antonio Borneo <bor...@gm...>
Date:   Sun Feb 25 23:03:27 2024 +0100

    gdb_server: drop useless check in gdb_keep_client_alive()
    
    OpenOCD can send it's log to gdb, and gdb replies with 'OK'.
    Calls to LOG_XXX() are also present in the code that communicates
    with gdb. This can cause infinite nested calls.
    
    OpenOCD uses the flag 'gdb_con->busy' to protect the communication
    with gdb and prevent nested calls.
    
    There is no reason to check for 'gdb_con->busy' in the code for
    keep-alive, as keep_alive() is never called in this gdb server;
    the flag would eventually be set if the current keep_alive() will
    send something to gdb.
    
    Drop the flag 'gdb_con->busy' in gdb_keep_client_alive().
    While there, document the use of 'gdb_con->busy'.
    
    Change-Id: I1ea20bf96abb5d2f1fcdba1e3861df257c396bb6
    Signed-off-by: Antonio Borneo <bor...@gm...>
    Reviewed-on: https://review.openocd.org/c/openocd/+/8166
    Tested-by: jenkins

diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index 00e43cfe9..c1ee4aa2a 100644
--- a/src/server/gdb_server.c
+++ b/src/server/gdb_server.c
@@ -73,6 +73,8 @@ struct gdb_connection {
 	enum target_state frontend_state;
 	struct image *vflash_image;
 	bool closed;
+	/* set to prevent re-entrance from log messages during gdb_get_packet()
+	 * and gdb_put_packet(). */
 	bool busy;
 	int noack_mode;
 	/* set flag to true if you want the next stepi to return immediately.
@@ -3794,11 +3796,6 @@ static void gdb_keep_client_alive(struct connection *connection)
 {
 	struct gdb_connection *gdb_con = connection->priv;
 
-	if (gdb_con->busy) {
-		/* do not send packets, retry asap */
-		return;
-	}
-
 	switch (gdb_con->output_flag) {
 	case GDB_OUTPUT_NO:
 		/* no need for keep-alive */

commit 0c0243228cb958b7c12c2e5a81c84d35734fce25
Author: Antonio Borneo <bor...@gm...>
Date:   Sun Feb 25 22:55:35 2024 +0100

    gdb_server: add async-notif keep-alive during memory read/write
    
    To avoid gdb to timeout, OpenOCD implements a keep-alive mechanism
    that consists in sending periodically to gdb empty strings embedded
    in the "O" remote reply packet.
    
    The main purpose of "O" packets is to forward in the gdb console
    the output of the remote execution; the gdb-remote puts in the "O"
    packet the string that gdb will print. It's use is restricted to
    few "running/execution" contexts listed in
    http://sourceware.org/gdb/onlinedocs/gdb/Stop-Reply-Packets.html
    and this currently limits the keep-alive capabilities of OpenOCD.
    
    Long data transfer (memory R/W) can also cause gdb to timeout if
    the interface is too slow. In this case the usual keep-alive based
    on "O" packet cannot be used and, if used, would trigger a protocol
    error that causes the transfer to be dropped.
    The slow transfer rate can be simulated by adding some delay in the
    main loop of mem_ap_write() and mem_ap_read(), then using the gdb
    commands "dump" and "restore".
    
    In the wait loop during a memory R/W, gdb drops any extra character
    received from the gdb-remote that is not recognized as a valid
    reply to the memory command. Every dropped character re-initializes
    the timeout counter and could be used as keep-alive.
    
    From gdb 7.0 (released 2009-10-06), an asynchronous notification
    can also be received from gdb-remote during a memory R/W and has
    the effect to reset the timeout counter, thus can be used as
    keep-alive.
    The notification would be treated as "junk" extra characters by any
    gdb older than 7.0, being still valid as keep-alive.
    Check putpkt_binary() and getpkt_sane() in gdb commit
    74531fed1f2d662debc2c209b8b3faddceb55960
    
    Currently, only one notification packet ("Stop") is recognized by
    gdb, and gdb documentation reports that notification packets that
    are not recognized should be silently dropped.
    Use 'set debug remote 1' in gdb to dump the received notifications
    and the junk extra characters.
    
    Add a new level in enum gdb_output_flag for using the asynchronous
    notifications.
    Activate this new level during memory transfers.
    Send a custom "oocd_keepalive" notification packet as keep_alive.
    While there, drop a useless return in the switch/case, already
    managed in case of break.
    
    After this commit, the proper calls to keep_alive() have to be
    added in the loops that code the memory transfers. Of course, the
    keep_alive() should be placed during the wait for JTAG flush, not
    while locally queuing the JTAG elementary transfers.
    
    Change-Id: I9ca8e78630611597d15984bd0e8634c8fc3c32b9
    Signed-off-by: Antonio Borneo <bor...@gm...>
    Reviewed-on: https://review.openocd.org/c/openocd/+/8165
    Tested-by: jenkins

diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index ae288de1c..00e43cfe9 100644
--- a/src/server/gdb_server.c
+++ b/src/server/gdb_server.c
@@ -53,6 +53,8 @@
 enum gdb_output_flag {
 	/* GDB doesn't accept 'O' packets */
 	GDB_OUTPUT_NO,
+	/* GDB doesn't accept 'O' packets but accepts notifications */
+	GDB_OUTPUT_NOTIF,
 	/* GDB accepts 'O' packets */
 	GDB_OUTPUT_ALL,
 };
@@ -1486,9 +1488,6 @@ static int gdb_error(struct connection *connection, int retval)
 	return ERROR_OK;
 }
 
-/* We don't have to worry about the default 2 second timeout for GDB packets,
- * because GDB breaks up large memory reads into smaller reads.
- */
 static int gdb_read_memory_packet(struct connection *connection,
 		char const *packet, int packet_size)
 {
@@ -3477,7 +3476,7 @@ static void gdb_log_callback(void *priv, const char *file, unsigned line,
 	struct connection *connection = priv;
 	struct gdb_connection *gdb_con = connection->priv;
 
-	if (gdb_con->output_flag == GDB_OUTPUT_NO)
+	if (gdb_con->output_flag != GDB_OUTPUT_ALL)
 		/* No out allowed */
 		return;
 
@@ -3562,10 +3561,14 @@ static int gdb_input_inner(struct connection *connection)
 					retval = gdb_set_register_packet(connection, packet, packet_size);
 					break;
 				case 'm':
+					gdb_con->output_flag = GDB_OUTPUT_NOTIF;
 					retval = gdb_read_memory_packet(connection, packet, packet_size);
+					gdb_con->output_flag = GDB_OUTPUT_NO;
 					break;
 				case 'M':
+					gdb_con->output_flag = GDB_OUTPUT_NOTIF;
 					retval = gdb_write_memory_packet(connection, packet, packet_size);
+					gdb_con->output_flag = GDB_OUTPUT_NO;
 					break;
 				case 'z':
 				case 'Z':
@@ -3656,9 +3659,9 @@ static int gdb_input_inner(struct connection *connection)
 					retval = gdb_detach(connection);
 					break;
 				case 'X':
+					gdb_con->output_flag = GDB_OUTPUT_NOTIF;
 					retval = gdb_write_memory_binary_packet(connection, packet, packet_size);
-					if (retval != ERROR_OK)
-						return retval;
+					gdb_con->output_flag = GDB_OUTPUT_NO;
 					break;
 				case 'k':
 					if (gdb_con->extended_protocol) {
@@ -3754,6 +3757,39 @@ static int gdb_input(struct connection *connection)
 	return ERROR_OK;
 }
 
+/*
+ * Send custom notification packet as keep-alive during memory read/write.
+ *
+ * From gdb 7.0 (released 2009-10-06) an unknown notification received during
+ * memory read/write would be silently dropped.
+ * Before gdb 7.0 any character, with exclusion of "+-$", would be considered
+ * as junk and ignored.
+ * In both cases the reception will reset the timeout counter in gdb, thus
+ * working as a keep-alive.
+ * Check putpkt_binary() and getpkt_sane() in gdb commit
+ * 74531fed1f2d662debc2c209b8b3faddceb55960
+ *
+ * Enable remote debug in gdb with 'set debug remote 1' to either dump the junk
+ * characters in gdb pre-7.0 and the notification from gdb 7.0.
+ */
+static void gdb_async_notif(struct connection *connection)
+{
+	static unsigned char count;
+	unsigned char checksum = 0;
+	char buf[22];
+
+	int len = sprintf(buf, "%%oocd_keepalive:%2.2x", count++);
+	for (int i = 1; i < len; i++)
+		checksum += buf[i];
+	len += sprintf(buf + len, "#%2.2x", checksum);
+
+#ifdef _DEBUG_GDB_IO_
+	LOG_DEBUG("sending packet '%s'", buf);
+#endif
+
+	gdb_write(connection, buf, len);
+}
+
 static void gdb_keep_client_alive(struct connection *connection)
 {
 	struct gdb_connection *gdb_con = connection->priv;
@@ -3767,6 +3803,10 @@ static void gdb_keep_client_alive(struct connection *connection)
 	case GDB_OUTPUT_NO:
 		/* no need for keep-alive */
 		break;
+	case GDB_OUTPUT_NOTIF:
+		/* send asynchronous notification */
+		gdb_async_notif(connection);
+		break;
 	case GDB_OUTPUT_ALL:
 		/* send an empty O packet */
 		gdb_output_con(connection, "");

-----------------------------------------------------------------------

Summary of changes:
 src/server/gdb_server.c | 59 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 48 insertions(+), 11 deletions(-)


hooks/post-receive
-- 
Main OpenOCD repository