)]}'
{"id":"openocd~master~I7681bdd80e86c79130f62ed85c8f6a5ee1478232","project":"openocd","branch":"master","hashtags":[],"change_id":"I7681bdd80e86c79130f62ed85c8f6a5ee1478232","subject":"gdb_server: add keep-alive during memory read/write","status":"NEW","created":"2018-12-26 15:16:04.000000000","updated":"2019-01-08 16:57:04.000000000","submit_type":"CHERRY_PICK","mergeable":false,"submittable":false,"total_comment_count":0,"unresolved_comment_count":0,"has_review_started":true,"meta_rev_id":"e9fafadd062b038385bb0f5d2eabe7c862c364d1","_number":4828,"owner":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"actions":{},"labels":{"Verified":{"approved":{"_account_id":1000014,"name":"jenkins","username":"jenkins","tags":["SERVICE_USER"]},"all":[{"date":"2019-01-08 16:57:04.000000000","_account_id":1001127,"name":"Steven Stallion","email":"sstallion@gmail.com","username":"sstallion"},{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},{"value":1,"date":"2018-12-26 17:12:54.000000000","permitted_voting_range":{"min":-1,"max":1},"_account_id":1000014,"name":"jenkins","username":"jenkins","tags":["SERVICE_USER"]}],"values":{"-1":"Fails"," 0":"No score","+1":"Verified"},"description":"","default_value":0},"Code-Review":{"recommended":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"all":[{"value":0,"permitted_voting_range":{"min":-1,"max":1},"_account_id":1001127,"name":"Steven Stallion","email":"sstallion@gmail.com","username":"sstallion"},{"value":1,"date":"2018-12-27 00:05:47.000000000","permitted_voting_range":{"min":-2,"max":2},"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},{"value":0,"permitted_voting_range":{"min":-1,"max":1},"_account_id":1000014,"name":"jenkins","username":"jenkins","tags":["SERVICE_USER"]}],"values":{"-2":"This shall not be merged","-1":"I would prefer this is not merged as is"," 0":"No score","+1":"Looks good to me, but someone else must approve","+2":"Looks good to me, approved"},"description":"","value":1,"default_value":0}},"removable_reviewers":[],"reviewers":{"REVIEWER":[{"_account_id":1000014,"name":"jenkins","username":"jenkins","tags":["SERVICE_USER"]},{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},{"_account_id":1001127,"name":"Steven Stallion","email":"sstallion@gmail.com","username":"sstallion"}]},"pending_reviewers":{},"reviewer_updates":[{"updated":"2018-12-26 17:12:54.000000000","updated_by":{"_account_id":1000014,"name":"jenkins","username":"jenkins","tags":["SERVICE_USER"]},"reviewer":{"_account_id":1000014,"name":"jenkins","username":"jenkins","tags":["SERVICE_USER"]},"state":"REVIEWER"},{"updated":"2019-01-08 16:57:04.000000000","updated_by":{"_account_id":1001127,"name":"Steven Stallion","email":"sstallion@gmail.com","username":"sstallion"},"reviewer":{"_account_id":1001127,"name":"Steven Stallion","email":"sstallion@gmail.com","username":"sstallion"},"state":"REVIEWER"}],"messages":[{"id":"88f10c5f6d589b1591931f7787b625e8b8e8d23f","author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"real_author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"date":"2018-12-26 15:16:04.000000000","message":"Uploaded patch set 1.","accounts_in_message":[],"_revision_number":1},{"id":"31695450537a40e1f9c13041f3e3d298f4fa76ad","author":{"_account_id":1000014,"name":"jenkins","username":"jenkins","tags":["SERVICE_USER"]},"real_author":{"_account_id":1000014,"name":"jenkins","username":"jenkins","tags":["SERVICE_USER"]},"date":"2018-12-26 17:12:54.000000000","message":"Patch Set 1: Verified+1\n\nBuild Successful \n\nhttp://build.openocd.org/job/openocd-gerrit/10971/ : SUCCESS\n\nhttp://build.openocd.org/job/openocd-gerrit-build/10319/ : SUCCESS","accounts_in_message":[],"_revision_number":1},{"id":"473bd006e368e4b0609f191d9d528b445326bfda","author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"real_author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"date":"2018-12-27 00:05:47.000000000","message":"Patch Set 1: Code-Review+1","accounts_in_message":[],"_revision_number":1},{"id":"03de4f889d3b92755b737f580fd097de5179b06c","author":{"_account_id":1001127,"name":"Steven Stallion","email":"sstallion@gmail.com","username":"sstallion"},"real_author":{"_account_id":1001127,"name":"Steven Stallion","email":"sstallion@gmail.com","username":"sstallion"},"date":"2019-01-07 17:38:22.000000000","message":"Patch Set 1:\n\nAntonio, out of curiosity how would this impact some of the custom commands that perform long transfers or flash programming?","accounts_in_message":[],"_revision_number":1},{"id":"09d46bda3bd8a4d0a6cf62a131202998faa1f1f7","author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"real_author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"date":"2019-01-07 21:02:59.000000000","message":"Patch Set 1:\n\n\u003e Antonio, out of curiosity how would this impact some of the custom\n \u003e commands that perform long transfers or flash programming?\n\nSteven,\nwhat you mean for long transfer?\nRegarding flash progamming, the documentation of gdb for the packets vFlashErase / vFlashWrite / vFlashDone, in\nhttps://sourceware.org/gdb/onlinedocs/gdb/Packets.html\ndoes not mention any possible reply from the remote target that can be reused as keep-alive, only \"OK\" or an error.\nBut checking latest gdb source code, also the three commands above use the same low level procedure putpkt_binary() used for the memory R/W that I target in this patch. GDB relevant code is in\nhttps://sourceware.org/git/gitweb.cgi?p\u003dbinutils-gdb.git;a\u003dblob;f\u003dgdb/remote.c;hb\u003dHEAD\nThus, I feel confident that the same code in this patch can be reused also for flash programming. On top of this patch it should be required to wrap \"part of\" gdb_v_packet() in src/server/gdb_server.c within\nlog_{add,remove}_callback(gdb_keepalive_callback, connection);\nbut excluding the management of \"vCont\" that should not require keep-alive.","accounts_in_message":[],"_revision_number":1},{"id":"e9fafadd062b038385bb0f5d2eabe7c862c364d1","author":{"_account_id":1001127,"name":"Steven Stallion","email":"sstallion@gmail.com","username":"sstallion"},"real_author":{"_account_id":1001127,"name":"Steven Stallion","email":"sstallion@gmail.com","username":"sstallion"},"date":"2019-01-08 16:57:04.000000000","message":"Patch Set 1:\n\n\u003e \u003e Antonio, out of curiosity how would this impact some of the\n \u003e custom\n \u003e \u003e commands that perform long transfers or flash programming?\n \u003e \n \u003e Steven,\n \u003e what you mean for long transfer?\n \u003e Regarding flash progamming, the documentation of gdb for the\n \u003e packets vFlashErase / vFlashWrite / vFlashDone, in\n \u003e https://sourceware.org/gdb/onlinedocs/gdb/Packets.html\n \u003e does not mention any possible reply from the remote target that can\n \u003e be reused as keep-alive, only \"OK\" or an error.\n \u003e But checking latest gdb source code, also the three commands above\n \u003e use the same low level procedure putpkt_binary() used for the\n \u003e memory R/W that I target in this patch. GDB relevant code is in\n \u003e https://sourceware.org/git/gitweb.cgi?p\u003dbinutils-gdb.git;a\u003dblob;f\u003dgdb/remote.c;hb\u003dHEAD\n \u003e Thus, I feel confident that the same code in this patch can be\n \u003e reused also for flash programming. On top of this patch it should\n \u003e be required to wrap \"part of\" gdb_v_packet() in src/server/gdb_server.c\n \u003e within\n \u003e log_{add,remove}_callback(gdb_keepalive_callback, connection);\n \u003e but excluding the management of \"vCont\" that should not require\n \u003e keep-alive.\n\nGotcha. I was thinking of some of the more esoteric commands like ETM or eSi-Trace that perform largish dumps of memory, but that may not matter since these are executed as monitor commands from GDB\u0027s perspective.","accounts_in_message":[],"_revision_number":1}],"current_revision":"1fde262047cc0df36218bef9fe8f9fe84e959e2d","revisions":{"1fde262047cc0df36218bef9fe8f9fe84e959e2d":{"kind":"REWORK","_number":1,"created":"2018-12-26 15:16:04.000000000","uploader":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"ref":"refs/changes/28/4828/1","fetch":{"anonymous http":{"url":"https://review.openocd.org/openocd","ref":"refs/changes/28/4828/1","commands":{"Branch":"git fetch https://review.openocd.org/openocd refs/changes/28/4828/1 \u0026\u0026 git checkout -b change-4828 FETCH_HEAD","Checkout":"git fetch https://review.openocd.org/openocd refs/changes/28/4828/1 \u0026\u0026 git checkout FETCH_HEAD","Cherry Pick":"git fetch https://review.openocd.org/openocd refs/changes/28/4828/1 \u0026\u0026 git cherry-pick FETCH_HEAD","Format Patch":"git fetch https://review.openocd.org/openocd refs/changes/28/4828/1 \u0026\u0026 git format-patch -1 --stdout FETCH_HEAD","Pull":"git pull https://review.openocd.org/openocd refs/changes/28/4828/1","Reset To":"git fetch https://review.openocd.org/openocd refs/changes/28/4828/1 \u0026\u0026 git reset --hard FETCH_HEAD"}}},"commit":{"parents":[{"commit":"f5bb6ed1ff84d9de45bfd31b5f631e46c291cb98","subject":"target/cortex_a: fix test for target halted"}],"author":{"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","date":"2018-06-07 17:13:09.000000000","tz":120},"committer":{"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","date":"2018-12-26 14:57:56.000000000","tz":60},"subject":"gdb_server: add keep-alive during memory read/write","message":"gdb_server: add keep-alive during memory read/write\n\nTo avoid gdb to timeout, OpenOCD implements a keep-alive mechanism\nthat consists in sending periodically to gdb empty strings embedded\nin the \"O\" remote reply packet.\n\nThe main purpose of \"O\" packets is to forward in the gdb console\nthe output of the remote execution; the gdb-remote puts in the \"O\"\npacket the string that gdb will print. It\u0027s use is restricted to\nfew \"running/execution\" contexts listed in\nhttp://sourceware.org/gdb/onlinedocs/gdb/Stop-Reply-Packets.html\nand this currently limits the keep-alive capabilities of OpenOCD.\n\nLong data transfer (memory R/W) can also cause gdb to timeout if\nthe interface is too slow. In this case the usual keep-alive based\non \"O\" packet cannot be used and, if used, would trigger a protocol\nerror that causes the transfer to be dropped.\nThe slow transfer rate can be simulated by adding some delay in the\nmain loop of mem_ap_write() and mem_ap_read(), then using the gdb\ncommands \"dump\" and \"restore\".\n\nIn the wait loop during a memory R/W, gdb drops any extra character\nreceived from the gdb-remote that is not recognized as a valid\nreply to the memory command. Every dropped character re-initializes\nthe timeout counter and could be used as keep-alive.\n\nFrom gdb 7.0 (released 2009-10-06), an asynchronous notification\ncan also be received from gdb-remote during a memory R/W and has\nthe effect to reset the timeout counter, thus can be used as\nkeep-alive.\nThe notification would be treated as \"junk\" extra characters by any\ngdb older than 7.0, being still valid as keep-alive.\nCheck putpkt_binary() and getpkt_sane() in gdb commit\n74531fed1f2d662debc2c209b8b3faddceb55960\n\nCurrently, only one notification packet (\"Stop\") is recognized by\ngdb, and gdb documentation reports that notification packets that\nare not recognized should be silently dropped.\nUse \u0027set debug remote 1\u0027 in gdb to dump the received notifications\nand the junk extra characters.\n\nImplement a new log callback that sends a custom \"oocd_keepalive\"\nnotification packet when keep_alive() prints an empty string.\nActivate this new callback only during memory transfers.\n\nAfter this commit, the proper calls to keep_alive() have to be\nadded in the loops that code the memory transfers.\n\nChange-Id: I7681bdd80e86c79130f62ed85c8f6a5ee1478232\nSigned-off-by: Antonio Borneo \u003cborneo.antonio@gmail.com\u003e\n"}}},"requirements":[],"submit_records":[{"rule_name":"gerrit~DefaultSubmitRule","status":"NOT_READY","labels":[{"label":"Verified","status":"OK","applied_by":{"_account_id":1000014,"name":"jenkins","username":"jenkins","tags":["SERVICE_USER"]}},{"label":"Code-Review","status":"NEED"}]}],"submit_requirements":[{"name":"Verified","status":"SATISFIED","is_legacy":true,"submittability_expression_result":{"expression":"label:Verified\u003dMAX -label:Verified\u003dMIN","fulfilled":true,"status":"PASS","passing_atoms":["label:Verified\u003dMAX","-label:Verified\u003dMIN"],"failing_atoms":[]}},{"name":"Code-Review","status":"UNSATISFIED","is_legacy":true,"submittability_expression_result":{"expression":"label:Code-Review\u003dMAX -label:Code-Review\u003dMIN","fulfilled":false,"status":"FAIL","passing_atoms":[],"failing_atoms":["label:Code-Review\u003dMAX","-label:Code-Review\u003dMIN"]}}]}
