From: Tim Newsome <tim@sifive.com>
Date: Wed, 5 Oct 2016 14:57:38 +0000 (-0700)
Subject: Perform small writes before sending OK.
X-Git-Tag: v0.10.0-rc1~76
X-Git-Url: https://review.openocd.org/gitweb?p=openocd.git;a=commitdiff_plain;h=19df456a79c912b9e2162ef05c1fd0c02ec7b14a

Perform small writes before sending OK.

To speed up downloads, OpenOCD sends gdb OK when a write is received,
even before the write has actually occurred. The failure is then
returned for the next write. That leads to the following confusing
behavior:
```
(gdb) p/x *((int*)0xdeadbeef)=8675309
$2 = 0x845fed
(gdb) p/x *((int*)0x80000000)=6874742
Cannot access memory at address 0x80000000
```
While it's actually the first write that failed.

This change hacks around this problem by not sending OK for small writes
(len<8) until the write has actually occurred. This does not impact
download speed, since during downloads (almost) all writes will have
much larger length.

Change-Id: I1f8b9bb19b0707487f840df6871e372e4ba228dd
Signed-off-by: Tim Newsome <tim@sifive.com>
Reviewed-on: http://openocd.zylin.com/3803
Tested-by: jenkins
Reviewed-by: Tomas Vanek <vanekt@fbl.cz>
---

diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index f85fa1bde9..7fd579b465 100644
--- a/src/server/gdb_server.c
+++ b/src/server/gdb_server.c
@@ -1473,6 +1473,10 @@ static int gdb_write_memory_binary_packet(struct connection *connection,
 	uint32_t len = 0;
 
 	int retval = ERROR_OK;
+	/* Packets larger than fast_limit bytes will be acknowledged instantly on
+	 * the assumption that we're in a download and it's important to go as fast
+	 * as possible. */
+	uint32_t fast_limit = 8;
 
 	/* skip command character */
 	packet++;
@@ -1493,19 +1497,23 @@ static int gdb_write_memory_binary_packet(struct connection *connection,
 
 	struct gdb_connection *gdb_connection = connection->priv;
 
-	if (gdb_connection->mem_write_error) {
+	if (gdb_connection->mem_write_error)
 		retval = ERROR_FAIL;
-		/* now that we have reported the memory write error, we can clear the condition */
-		gdb_connection->mem_write_error = false;
-	}
 
-	/* By replying the packet *immediately* GDB will send us a new packet
-	 * while we write the last one to the target.
-	 */
-	if (retval == ERROR_OK)
-		gdb_put_packet(connection, "OK", 2);
-	else {
+	if (retval == ERROR_OK) {
+		if (len >= fast_limit) {
+			/* By replying the packet *immediately* GDB will send us a new packet
+			 * while we write the last one to the target.
+			 * We only do this for larger writes, so that users who do something like:
+			 * p *((int*)0xdeadbeef)=8675309
+			 * will get immediate feedback that that write failed.
+			 */
+			gdb_put_packet(connection, "OK", 2);
+		}
+	} else {
 		retval = gdb_error(connection, retval);
+		/* now that we have reported the memory write error, we can clear the condition */
+		gdb_connection->mem_write_error = false;
 		if (retval != ERROR_OK)
 			return retval;
 	}
@@ -1518,6 +1526,15 @@ static int gdb_write_memory_binary_packet(struct connection *connection,
 			gdb_connection->mem_write_error = true;
 	}
 
+	if (len < fast_limit) {
+		if (retval != ERROR_OK) {
+			gdb_error(connection, retval);
+			gdb_connection->mem_write_error = false;
+		} else {
+			gdb_put_packet(connection, "OK", 2);
+		}
+	}
+
 	return ERROR_OK;
 }