)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"8756f54663b3a04343091e24849c4f464cf848bb","unresolved":true,"context_lines":[{"line_number":9,"context_line":"Currently, instruction cache is being invalidated in"},{"line_number":10,"context_line":"arc_{un,}set_breakpoint() regardless of whether the breakpoint\u0027s type is"},{"line_number":11,"context_line":"HW or SW. For SW breakpoints, this has no net effect as the caches are"},{"line_number":12,"context_line":"flushed prior to resuming execution anyway and is thus merely"},{"line_number":13,"context_line":"unnecessary; but for HW breakpoints this invalidation is not preceded by"},{"line_number":14,"context_line":"a flush and might lead to loss of data. This patch removes the"},{"line_number":15,"context_line":"invalidate() call altogether to correct this undesired behavior for HW"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"f50ffb8e_f836c778","line":12,"updated":"2023-07-23 14:53:06.000000000","message":"Can you please point to the code that flushes/invalidate the cache on resume? Maybe it is hidden in some other operation and a comment there could help.","commit_id":"df9f5863fd7ea3b81f9da3c5de3fadd55fcc64c0"},{"author":{"_account_id":1002160,"name":"Artemiy Volkov","display_name":"Artemiy Volkov","email":"artemiy@synopsys.com","username":"artemiy-volkov","status":"Synopsys"},"change_message_id":"1968ee23bcb503c73ab8a45a3bae7fae3ee1d641","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Currently, instruction cache is being invalidated in"},{"line_number":10,"context_line":"arc_{un,}set_breakpoint() regardless of whether the breakpoint\u0027s type is"},{"line_number":11,"context_line":"HW or SW. For SW breakpoints, this has no net effect as the caches are"},{"line_number":12,"context_line":"flushed prior to resuming execution anyway and is thus merely"},{"line_number":13,"context_line":"unnecessary; but for HW breakpoints this invalidation is not preceded by"},{"line_number":14,"context_line":"a flush and might lead to loss of data. This patch removes the"},{"line_number":15,"context_line":"invalidate() call altogether to correct this undesired behavior for HW"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"bdedd76b_bec7b876","line":12,"in_reply_to":"f50ffb8e_f836c778","updated":"2023-07-24 20:13:22.000000000","message":"The cache isn\u0027t flushed in the -\u003eresume() method itself, but rather directly after (un)setting a software breakpoint, as a by-product of overwriting instructions in memory. Call chain example: arc_set_breakpoint() -\u003e target_write_u16() -\u003e target_write_memory() -\u003e arc_mem_write() -\u003e arc_mem_write_block16() -\u003e arc_cache_flush().","commit_id":"df9f5863fd7ea3b81f9da3c5de3fadd55fcc64c0"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"8756f54663b3a04343091e24849c4f464cf848bb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b3c7f1db_126676dd","updated":"2023-07-23 14:53:06.000000000","message":"I have no experience on arc cores, nor I have any arc device for test, but looking at this patch I\u0027m not fully convinced it is correct","commit_id":"df9f5863fd7ea3b81f9da3c5de3fadd55fcc64c0"},{"author":{"_account_id":1001674,"name":"Evgeniy Didin","email":"didin@synopsys.com","username":"EvgeniiDidin"},"change_message_id":"c3039b80ed498ad945b4a671c9379a0030b830ee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e9afbf95_acf45e29","updated":"2023-07-10 06:49:45.000000000","message":"LGTM!\n\nThank you!","commit_id":"df9f5863fd7ea3b81f9da3c5de3fadd55fcc64c0"},{"author":{"_account_id":1002160,"name":"Artemiy Volkov","display_name":"Artemiy Volkov","email":"artemiy@synopsys.com","username":"artemiy-volkov","status":"Synopsys"},"change_message_id":"53102c785d1786954fa00c250ce01aef7d342df7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7a953b2f_87f8edd9","in_reply_to":"b3c7f1db_126676dd","updated":"2023-08-28 07:11:51.000000000","message":"Hi Antonio, I hope my changes to the commit message have clarified the intent of this patch, could you please give this another look?","commit_id":"df9f5863fd7ea3b81f9da3c5de3fadd55fcc64c0"},{"author":{"_account_id":1001674,"name":"Evgeniy Didin","email":"didin@synopsys.com","username":"EvgeniiDidin"},"change_message_id":"68bbc786405cf032cc0a51b9fe633fc226db1345","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e11ae2ee_2345dc9e","updated":"2023-09-18 05:00:15.000000000","message":"LGTM!","commit_id":"a9e7ccbe43b6a3b08c79debbb244e07a1270193d"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"4c064642a423d60c49ec4fabb00b7d4163cb1026","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"afaf6c75_9591f917","updated":"2023-09-17 15:25:35.000000000","message":"thanks","commit_id":"a9e7ccbe43b6a3b08c79debbb244e07a1270193d"}],"src/target/arc.c":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"8756f54663b3a04343091e24849c4f464cf848bb","unresolved":true,"context_lines":[{"line_number":1577,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":1578,"context_line":"\t}"},{"line_number":1579,"context_line":""},{"line_number":1580,"context_line":"\t/* core instruction cache is now invalid. */"},{"line_number":1581,"context_line":"\tCHECK_RETVAL(arc_cache_invalidate(target));"},{"line_number":1582,"context_line":""},{"line_number":1583,"context_line":"\treturn ERROR_OK;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"5dea5a6b_9f24eef3","side":"PARENT","line":1580,"updated":"2023-07-23 14:53:06.000000000","message":"for future memory, what about keeping a comment here that say that there is no need to flush and invalidate icache for SW BKP because icache is flushed by resume()?\n\nFor me here the code should instead be something like:\nif (sw_bkp)\narc_icache_invalidate(target);\nto explicitly invalidate I-cache only (not D-cache). The flush of D-Cache should be already done by the memory write, if I have checked correctly.","commit_id":"56fd04832abc0ebadc21ee6127be4be9c7b46e15"},{"author":{"_account_id":1002160,"name":"Artemiy Volkov","display_name":"Artemiy Volkov","email":"artemiy@synopsys.com","username":"artemiy-volkov","status":"Synopsys"},"change_message_id":"1968ee23bcb503c73ab8a45a3bae7fae3ee1d641","unresolved":false,"context_lines":[{"line_number":1577,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":1578,"context_line":"\t}"},{"line_number":1579,"context_line":""},{"line_number":1580,"context_line":"\t/* core instruction cache is now invalid. */"},{"line_number":1581,"context_line":"\tCHECK_RETVAL(arc_cache_invalidate(target));"},{"line_number":1582,"context_line":""},{"line_number":1583,"context_line":"\treturn ERROR_OK;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"d91f7196_652bf56f","side":"PARENT","line":1580,"in_reply_to":"5dea5a6b_9f24eef3","updated":"2023-07-24 20:13:22.000000000","message":"See previous comment. In the case of SW breakpoints, the arc_cache_flush() call takes care of everything, and this call to arc_cache_invalidate() has no effect since the cache is already in sync with the contents of the main memory.\n\nIt does appear that the phrase \"prior to resuming execution\" can be worded better. I have slightly amended the commit message and re-uploaded the patch, please see if the new wording makes more sense.","commit_id":"56fd04832abc0ebadc21ee6127be4be9c7b46e15"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"8756f54663b3a04343091e24849c4f464cf848bb","unresolved":true,"context_lines":[{"line_number":1662,"context_line":"\t\t\treturn ERROR_FAIL;"},{"line_number":1663,"context_line":"\t}"},{"line_number":1664,"context_line":""},{"line_number":1665,"context_line":"\t/* core instruction cache is now invalid. */"},{"line_number":1666,"context_line":"\tCHECK_RETVAL(arc_cache_invalidate(target));"},{"line_number":1667,"context_line":""},{"line_number":1668,"context_line":"\treturn retval;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"93b03651_2b3b469d","side":"PARENT","line":1665,"updated":"2023-07-23 14:53:06.000000000","message":"here too!","commit_id":"56fd04832abc0ebadc21ee6127be4be9c7b46e15"},{"author":{"_account_id":1002160,"name":"Artemiy Volkov","display_name":"Artemiy Volkov","email":"artemiy@synopsys.com","username":"artemiy-volkov","status":"Synopsys"},"change_message_id":"1968ee23bcb503c73ab8a45a3bae7fae3ee1d641","unresolved":false,"context_lines":[{"line_number":1662,"context_line":"\t\t\treturn ERROR_FAIL;"},{"line_number":1663,"context_line":"\t}"},{"line_number":1664,"context_line":""},{"line_number":1665,"context_line":"\t/* core instruction cache is now invalid. */"},{"line_number":1666,"context_line":"\tCHECK_RETVAL(arc_cache_invalidate(target));"},{"line_number":1667,"context_line":""},{"line_number":1668,"context_line":"\treturn retval;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"afcd7dd4_c5174ebe","side":"PARENT","line":1665,"in_reply_to":"93b03651_2b3b469d","updated":"2023-07-24 20:13:22.000000000","message":"See above.","commit_id":"56fd04832abc0ebadc21ee6127be4be9c7b46e15"}]}