)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000410,"name":"Oleksij Rempel","email":"linux@rempel-privat.de","username":"olerem"},"change_message_id":"8851bb827b2d82922a4095056078ddd90955cf59","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1079e3a3_d1e0e93f","updated":"2021-09-06 04:20:46.000000000","message":"@Antonio, means, we have way to stop all cores on AMP-like system?","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"},{"author":{"_account_id":1000410,"name":"Oleksij Rempel","email":"linux@rempel-privat.de","username":"olerem"},"change_message_id":"290bdfe1164ae59f23f863510219779b5fb88ebf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"543fc65d_bad07891","updated":"2021-09-02 17:30:31.000000000","message":"Hm.. I think, it is bad idea, because SMP chip can be partitioned to run separate systems on each core. Since, it is application specific, it should be done on higher level. Or it should be configurable and not depend only on SMP flag.","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"1a130c5ab868f84152a7b1c14aa2279c01ff24e4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e1aa0214_ec67d824","updated":"2021-09-04 20:18:59.000000000","message":"I have not checked the patch line-by-line, but the concept is correct.\nThis mechanism is already implemented for breakpoints in the same file; watchpoints should follow the same way.\n\nSMP means that all the cores are executing threads from the same application, and one GDB controls all of them loading the symbols from the single application\u0027s ELF.\nWhen you set a bp or a wp you don\u0027t know which core will hit it, so you need to set it on all the cores.\n\nIf you have two cores running different applications, then you need to run two GDB, one for each application. And you need two GDB ports from openocd. So you need to remove the SMP flag from openocd. An then bp and wp have to be independent.","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"},{"author":{"_account_id":1000410,"name":"Oleksij Rempel","email":"linux@rempel-privat.de","username":"olerem"},"change_message_id":"06e5954428c0bf82cb1c98b4d4daecaa138463b3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"96e86c5a_afd4e795","updated":"2021-09-06 10:25:16.000000000","message":"Some chips have internal support for multicore halt. For example the TAP for the  DMA CPU of iMX6, can be configured to halt all cores if DMA breakpoint is asserted. I assume, currently we have no abstraction for this.","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"},{"author":{"_account_id":1001242,"name":"Tim Newsome","email":"tim@sifive.com","username":"timsifive"},"change_message_id":"a3ae19772aee7ddb9def2163c04a114ae1bde2f7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"577d4017_9329e41e","updated":"2021-09-07 18:23:19.000000000","message":"Thanks for taking the time review.","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"dba53bfe698ac0eadfebae5c0b3c035f34f5eec7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2f658ecb_210a39e8","updated":"2021-09-04 20:35:34.000000000","message":"minor fixes here and there","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"f58881b0b487d3d15bc72c87301a85c2ce4aae84","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2594e970_923c19c6","in_reply_to":"1079e3a3_d1e0e93f","updated":"2021-09-06 10:17:42.000000000","message":"openocd only halts/resumes the other cores in the SMP group. So without \"smp\" command you loose this automatic support.\nBut you can still setup a CTI to halt the other cores of the same SoC. I used it to halt Cortex-M and Cortex-A together in STM32MP157.\nIf there is no CTI or you have a multi-chip system, you can use the event handler \"halted\" or \"debug-halted\" to halt the other cores, and the event \"resumed\" or \"debug-resumed\" to restart them. I never tried this but would be good to document it as an example.","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"},{"author":{"_account_id":1001242,"name":"Tim Newsome","email":"tim@sifive.com","username":"timsifive"},"change_message_id":"be661b4548ef7fd95afe52e94c07b7fdbd618585","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e800650f_035fd450","in_reply_to":"543fc65d_bad07891","updated":"2021-09-02 18:03:10.000000000","message":"In that case, would you use `target smp`? I thought the whole point of `target smp` was to expose multiple targets as threads to gdb. For sure gdb cannot debug more than one binary at once, and it also assumes that all \"threads\" have the same view of memory.\n\nI can definitely make this behavior optional if people think that\u0027s better.","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"},{"author":{"_account_id":1000410,"name":"Oleksij Rempel","email":"linux@rempel-privat.de","username":"olerem"},"change_message_id":"97836c9f493d64a081c05cfc6e3e27406335afbc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"62b0f06c_0579c97e","in_reply_to":"5cf69f19_37aae171","updated":"2021-09-11 12:08:33.000000000","message":"Done","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"d7a668d9869797539e3155eac861a944e122644a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"382e6e4d_6fbc256f","in_reply_to":"96e86c5a_afd4e795","updated":"2021-09-06 10:59:47.000000000","message":"DMA breakpoint! Interesting feature!\nI think would be nice to extend the current SMP model of OpenOCD to handle AMP and to dynamically add/remove core in a MP node.","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"},{"author":{"_account_id":1000410,"name":"Oleksij Rempel","email":"linux@rempel-privat.de","username":"olerem"},"change_message_id":"abdb97dc5731285b8696a0158e2fcd18a5b2cb1f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5cf69f19_37aae171","in_reply_to":"e800650f_035fd450","updated":"2021-09-03 07:01:06.000000000","message":"Sure, SMP can be used to stop both CPU cores at time. Debugging AMP system by stopping only one core can get other kind of pain. So, i personally prefer to have it optional.","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"}],"src/target/breakpoints.c":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"dba53bfe698ac0eadfebae5c0b3c035f34f5eec7","unresolved":true,"context_lines":[{"line_number":496,"context_line":"\t\t\thead \u003d head-\u003enext;"},{"line_number":497,"context_line":"\t\t}"},{"line_number":498,"context_line":"\t\treturn retval;"},{"line_number":499,"context_line":"\t} else"},{"line_number":500,"context_line":"\t\treturn watchpoint_add_internal(target, address, length, rw, value,"},{"line_number":501,"context_line":"\t\t\t\tmask);"},{"line_number":502,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"d020af70_e972bc6a","line":499,"updated":"2021-09-04 20:35:34.000000000","message":"per coding style, use the {} on both side of the if/then/else.","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"},{"author":{"_account_id":1001242,"name":"Tim Newsome","email":"tim@sifive.com","username":"timsifive"},"change_message_id":"a3ae19772aee7ddb9def2163c04a114ae1bde2f7","unresolved":false,"context_lines":[{"line_number":496,"context_line":"\t\t\thead \u003d head-\u003enext;"},{"line_number":497,"context_line":"\t\t}"},{"line_number":498,"context_line":"\t\treturn retval;"},{"line_number":499,"context_line":"\t} else"},{"line_number":500,"context_line":"\t\treturn watchpoint_add_internal(target, address, length, rw, value,"},{"line_number":501,"context_line":"\t\t\t\tmask);"},{"line_number":502,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"40339774_a3930c6f","line":499,"in_reply_to":"d020af70_e972bc6a","updated":"2021-09-07 18:23:19.000000000","message":"Done","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"dba53bfe698ac0eadfebae5c0b3c035f34f5eec7","unresolved":true,"context_lines":[{"line_number":544,"context_line":""},{"line_number":545,"context_line":"void watchpoint_remove(struct target *target, target_addr_t address)"},{"line_number":546,"context_line":"{"},{"line_number":547,"context_line":"\tint found \u003d 0;"},{"line_number":548,"context_line":"\tif (target-\u003esmp) {"},{"line_number":549,"context_line":"\t\tstruct target_list *head;"},{"line_number":550,"context_line":"\t\tstruct target *curr;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"9c2a9a05_4090d4a7","line":547,"updated":"2021-09-04 20:35:34.000000000","message":"this variable is visible only in the if block below. Move the definition in the block itself. Please also rename it \"unsigned int num_watchpoints\", to match the code in breakpoint_remove().","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"},{"author":{"_account_id":1001242,"name":"Tim Newsome","email":"tim@sifive.com","username":"timsifive"},"change_message_id":"a3ae19772aee7ddb9def2163c04a114ae1bde2f7","unresolved":false,"context_lines":[{"line_number":544,"context_line":""},{"line_number":545,"context_line":"void watchpoint_remove(struct target *target, target_addr_t address)"},{"line_number":546,"context_line":"{"},{"line_number":547,"context_line":"\tint found \u003d 0;"},{"line_number":548,"context_line":"\tif (target-\u003esmp) {"},{"line_number":549,"context_line":"\t\tstruct target_list *head;"},{"line_number":550,"context_line":"\t\tstruct target *curr;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"db5d4a94_0ad1e3d0","line":547,"in_reply_to":"9c2a9a05_4090d4a7","updated":"2021-09-07 18:23:19.000000000","message":"Done","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"dba53bfe698ac0eadfebae5c0b3c035f34f5eec7","unresolved":true,"context_lines":[{"line_number":549,"context_line":"\t\tstruct target_list *head;"},{"line_number":550,"context_line":"\t\tstruct target *curr;"},{"line_number":551,"context_line":"\t\thead \u003d target-\u003ehead;"},{"line_number":552,"context_line":"\t\twhile (head !\u003d (struct target_list *)NULL) {"},{"line_number":553,"context_line":"\t\t\tcurr \u003d head-\u003etarget;"},{"line_number":554,"context_line":"\t\t\tfound +\u003d watchpoint_remove_internal(curr, address);"},{"line_number":555,"context_line":"\t\t\thead \u003d head-\u003enext;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"18cf9e0f_04050938","line":552,"updated":"2021-09-04 20:35:34.000000000","message":"Per coding style, no comparisons to NULL. Can you rewrite as:\nwhile (head) {\n\nNote for myself.\nThe comparison to NULL were supposed to be already removed with review.openocd.org/6350/ and following, but I have missed the one in breakpoint_remove() similar to this.","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"},{"author":{"_account_id":1001242,"name":"Tim Newsome","email":"tim@sifive.com","username":"timsifive"},"change_message_id":"a3ae19772aee7ddb9def2163c04a114ae1bde2f7","unresolved":false,"context_lines":[{"line_number":549,"context_line":"\t\tstruct target_list *head;"},{"line_number":550,"context_line":"\t\tstruct target *curr;"},{"line_number":551,"context_line":"\t\thead \u003d target-\u003ehead;"},{"line_number":552,"context_line":"\t\twhile (head !\u003d (struct target_list *)NULL) {"},{"line_number":553,"context_line":"\t\t\tcurr \u003d head-\u003etarget;"},{"line_number":554,"context_line":"\t\t\tfound +\u003d watchpoint_remove_internal(curr, address);"},{"line_number":555,"context_line":"\t\t\thead \u003d head-\u003enext;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"5fc23e1f_c903816a","line":552,"in_reply_to":"18cf9e0f_04050938","updated":"2021-09-07 18:23:19.000000000","message":"Done","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"dba53bfe698ac0eadfebae5c0b3c035f34f5eec7","unresolved":true,"context_lines":[{"line_number":556,"context_line":"\t\t}"},{"line_number":557,"context_line":"\t\tif (found \u003d\u003d 0)"},{"line_number":558,"context_line":"\t\t\tLOG_ERROR(\"no watchpoint at address \" TARGET_ADDR_FMT \" found\", address);"},{"line_number":559,"context_line":"\t} else"},{"line_number":560,"context_line":"\t\twatchpoint_remove_internal(target, address);"},{"line_number":561,"context_line":"}"},{"line_number":562,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"9f7fc6ac_be52c9e6","line":559,"updated":"2021-09-04 20:35:34.000000000","message":"use {} also for the \"else\" block","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"},{"author":{"_account_id":1001242,"name":"Tim Newsome","email":"tim@sifive.com","username":"timsifive"},"change_message_id":"a3ae19772aee7ddb9def2163c04a114ae1bde2f7","unresolved":false,"context_lines":[{"line_number":556,"context_line":"\t\t}"},{"line_number":557,"context_line":"\t\tif (found \u003d\u003d 0)"},{"line_number":558,"context_line":"\t\t\tLOG_ERROR(\"no watchpoint at address \" TARGET_ADDR_FMT \" found\", address);"},{"line_number":559,"context_line":"\t} else"},{"line_number":560,"context_line":"\t\twatchpoint_remove_internal(target, address);"},{"line_number":561,"context_line":"}"},{"line_number":562,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"68e1be3a_79c7ffe0","line":559,"in_reply_to":"9f7fc6ac_be52c9e6","updated":"2021-09-07 18:23:19.000000000","message":"Done","commit_id":"4e3ecfe264a34ea2bb6e3a9d302ab33861e984ab"},{"author":{"_account_id":1000410,"name":"Oleksij Rempel","email":"linux@rempel-privat.de","username":"olerem"},"change_message_id":"fc3327b9b931f7f9ed84f5e57773a27f3d60a058","unresolved":true,"context_lines":[{"line_number":95,"context_line":"\t\t\treturn retval;"},{"line_number":96,"context_line":"\t}"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"\tLOG_DEBUG(\"[%d] added %s breakpoint at \" TARGET_ADDR_FMT"},{"line_number":99,"context_line":"\t\t\t\" of length 0x%8.8x, (BPID: %\" PRIu32 \")\","},{"line_number":100,"context_line":"\t\ttarget-\u003ecoreid,"},{"line_number":101,"context_line":"\t\tbreakpoint_type_strings[(*breakpoint_p)-\u003etype],"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"0be9469b_bb1d2f78","line":98,"updated":"2021-09-09 04:28:50.000000000","message":"The same should be done for the watchpoint debug print too.","commit_id":"db2e5d2522e0098ea04089e0505f89fde75c268c"},{"author":{"_account_id":1001242,"name":"Tim Newsome","email":"tim@sifive.com","username":"timsifive"},"change_message_id":"6919627dc6bd05c01e2f79c09cbb6fb325a5af0c","unresolved":false,"context_lines":[{"line_number":95,"context_line":"\t\t\treturn retval;"},{"line_number":96,"context_line":"\t}"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"\tLOG_DEBUG(\"[%d] added %s breakpoint at \" TARGET_ADDR_FMT"},{"line_number":99,"context_line":"\t\t\t\" of length 0x%8.8x, (BPID: %\" PRIu32 \")\","},{"line_number":100,"context_line":"\t\ttarget-\u003ecoreid,"},{"line_number":101,"context_line":"\t\tbreakpoint_type_strings[(*breakpoint_p)-\u003etype],"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"cb38534b_1b530237","line":98,"in_reply_to":"0be9469b_bb1d2f78","updated":"2021-09-09 18:03:10.000000000","message":"Done","commit_id":"db2e5d2522e0098ea04089e0505f89fde75c268c"}]}
