)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1002143,"name":"Marek Vrbka","email":"marek.vrbka@codasip.com","username":"MarekVCodasip"},"change_message_id":"53c72effb3395a074cfeda058bf914301109336d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c0aeb92f_12b3474a","updated":"2023-08-01 11:01:55.000000000","message":"Thanks for the patch.\n\nI recommend changing the commit title. Something like: \"target: check if target is not examined on reg command\"","commit_id":"df46b4a18d213262f272a68d47ef3cf617a993c1"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"659bba8336d22df3084b064764501bcc4bb5e569","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"163b5400_5bcf6505","in_reply_to":"c0aeb92f_12b3474a","updated":"2023-08-01 11:27:21.000000000","message":"addressed","commit_id":"df46b4a18d213262f272a68d47ef3cf617a993c1"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"659bba8336d22df3084b064764501bcc4bb5e569","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"282f6459_455a7c7a","in_reply_to":"c0aeb92f_12b3474a","updated":"2023-08-01 11:27:21.000000000","message":"changed the title. Thanks for suggestion.","commit_id":"df46b4a18d213262f272a68d47ef3cf617a993c1"},{"author":{"_account_id":1001242,"name":"Tim Newsome","email":"tim@sifive.com","username":"timsifive"},"change_message_id":"49b4d0c6733653b46482fed5354c68c6bc9d5723","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ddb79cba_1e5a93b8","updated":"2023-10-09 19:53:24.000000000","message":"\u003e Jan, please correct me, but If I\u0027m not mistaken, there is no way to figure that \u003e out during the examine procedure (in the sense that there is no control register \u003e that contains this information). The only way to figure out if such access is supported is to try.","commit_id":"61a2bd65e9b4ce742115bbe614c3b4c84df5e247"},{"author":{"_account_id":1002072,"name":"Anatoly Parshintsev","username":"aap-sc"},"change_message_id":"074d3c6c36f0f74ba8be7e3de4fa525ff4d97cce","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a9e075b6_4008aafb","updated":"2023-10-09 11:28:34.000000000","message":"Antonio, Jan\n\nFrom Antonio:\n\n\u003e The target should also be HALTED, otherwise no way to read the registers\n\nFrom Jan:\n\n\u003e Actually, RISC-V hardware (optionally) supports access to registers while the target is running.\n\nYeah, in case RISCV-V we have the capability to read some registers even if the target is not halted. So adding the check may be problematic (as mentioned by Jan).\n\nI could try to modify the code for all existing targets to add appropriate checks, but I\u0027m worried that this may require lot\u0027s of people in review (since I\u0027m not familiar with how debug subsystem works for most targets and there is a chance that I may misinterpret the codebase). Could you please provide an advise if we should attempt to add such checks?","commit_id":"61a2bd65e9b4ce742115bbe614c3b4c84df5e247"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"9cecd329407357c53acd86eed8aee9f52594d980","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"315bcd07_68e3930c","updated":"2023-08-18 11:48:26.000000000","message":"Antonio, would you kindly recommend someone who could review this further?","commit_id":"61a2bd65e9b4ce742115bbe614c3b4c84df5e247"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"5787c79d9f450d4797beae8da495078a0fcf9e37","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"964cd74c_463e52ba","updated":"2023-10-05 14:12:16.000000000","message":"LGTM","commit_id":"61a2bd65e9b4ce742115bbe614c3b4c84df5e247"},{"author":{"_account_id":1002143,"name":"Marek Vrbka","email":"marek.vrbka@codasip.com","username":"MarekVCodasip"},"change_message_id":"2b6cfef40c1c8e6941694a8d82a090618c13c150","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"da44dd50_9629f0d2","updated":"2023-08-01 11:53:59.000000000","message":"Thanks for the quick resolve.","commit_id":"61a2bd65e9b4ce742115bbe614c3b4c84df5e247"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"1d27924ea9473b69e527de585a00f31c47de86ab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9b459d69_9d883f05","updated":"2023-10-08 21:31:33.000000000","message":"Yes, thanks!\nThe target should also be HALTED, otherwise no way to read the registers.\nWould you mind sending a new patch to check for HALTED too?\nThe new patch would be in conflict with this one, so maybe better sending it after this one is already merged.","commit_id":"61a2bd65e9b4ce742115bbe614c3b4c84df5e247"},{"author":{"_account_id":1002072,"name":"Anatoly Parshintsev","username":"aap-sc"},"change_message_id":"2b92ce84007653165c104a4b727eb3fd6e945ae4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7167ffc7_740d5dad","updated":"2023-10-10 19:19:51.000000000","message":"just resolving to avoid the thread to avoid confusion,","commit_id":"61a2bd65e9b4ce742115bbe614c3b4c84df5e247"},{"author":{"_account_id":1001242,"name":"Tim Newsome","email":"tim@sifive.com","username":"timsifive"},"change_message_id":"49b4d0c6733653b46482fed5354c68c6bc9d5723","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d5de65ab_b429e1c6","in_reply_to":"2e3e0136_a8984d00","updated":"2023-10-09 19:53:24.000000000","message":"\u003e Jan, please correct me, but If I\u0027m not mistaken, there is no way to figure that out during the examine procedure (in the sense that there is no control register that contains this information). The only way to figure out if such access is supported is to try.\n\nThat is right. To really be sure, we\u0027d have to try for each register, which would take too long during examine(). (Some day there may be a configuration structure that can tell the debugger stuff like this.)","commit_id":"61a2bd65e9b4ce742115bbe614c3b4c84df5e247"},{"author":{"_account_id":1002072,"name":"Anatoly Parshintsev","username":"aap-sc"},"change_message_id":"2b92ce84007653165c104a4b727eb3fd6e945ae4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"251b3f3f_35151e5f","in_reply_to":"909e8b3a_46f6af09","updated":"2023-10-10 19:19:51.000000000","message":"Ack","commit_id":"61a2bd65e9b4ce742115bbe614c3b4c84df5e247"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"92f0a30ae6bb7f49ed54eb3947aa4d1bfc2c8c7c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"96da7c01_e676a831","in_reply_to":"9b459d69_9d883f05","updated":"2023-10-09 05:19:14.000000000","message":"\u003e The target should also be HALTED, otherwise no way to read the registers.\n\nActually, RISC-V hardware (optionally) supports access to registers while the target is running. So I believe it is best to leave this decision up to the target itself.","commit_id":"61a2bd65e9b4ce742115bbe614c3b4c84df5e247"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"6b82529d4dcf46829c309e3c0e90bbe52b292e09","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"cb000cd4_3343faf7","in_reply_to":"a9e075b6_4008aafb","updated":"2023-10-09 15:44:27.000000000","message":"While this behavior of RISC-V is quite unusual, let\u0027s keep this possibility in the code. Let\u0027s drop the idea of adding the check for CPU halted.\nBut, maybe we should just add a comment to explain the RISC-V use case, otherwise someone else will re-propose the check on HALTED.\n\nJan, is there a way to detect, maybe during examine, if the registers are readable with core not halted?\nAnd, do you confirm that it\u0027s only for read? Write should be blocked until core is halted!","commit_id":"61a2bd65e9b4ce742115bbe614c3b4c84df5e247"},{"author":{"_account_id":1002072,"name":"Anatoly Parshintsev","username":"aap-sc"},"change_message_id":"eec1e90bb2679ad4953027e351d8e09fd57650f9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"2e3e0136_a8984d00","in_reply_to":"cb000cd4_3343faf7","updated":"2023-10-09 16:07:54.000000000","message":"I\u0027ve added Tim to CC...\n\n\u003e Jan, is there a way to detect, maybe during examine, if the registers are readable with core not halted?\n\nJan, please correct me, but If I\u0027m not mistaken, there is no way to figure that out during the examine procedure (in the sense that there is no control register that contains this information). The only way to figure out if such access is supported is to try.\n\n\n\u003e And, do you confirm that it\u0027s only for read? Write should be blocked until core is halted!\n\nAgain, it looks like writes are not explicitly prohibited. IMHO, this feature (write while running) is questionable, but here is what the spec says:\n\nhttps://github.com/riscv/riscv-debug-spec/blob/master/riscv-debug-stable.pdf\n\n\"Section 3.8.1.1\".\n\nNOTE: This is about abstract command.\n\n\u003e Debug Modules must implement this command and must support read and write access to all GPRs when the selected hart is halted. Debug Modules may optionally support accessing other registers, or accessing registers when the hart is running.","commit_id":"61a2bd65e9b4ce742115bbe614c3b4c84df5e247"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"c8573b8f0f84982baa6dc5eaa2cf644c16cc361f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"909e8b3a_46f6af09","in_reply_to":"d5de65ab_b429e1c6","updated":"2023-10-10 05:55:05.000000000","message":"\u003e And, do you confirm that it\u0027s only for read? Write should be blocked until core is halted\n\nAs indicated by Anatoly above, RISC-V Debug Spec leaves the possibility to even write registers on a running target (if the target is designed to support it). \n\nI understand that it may sound awkward, and the support for this operation will be very rare in real-world HW. But I can envision specialized use cases where writing a register of non-halted target would be beneficial (for example to enable some sort of extended logging by writing to a CSR).\n\nAnd yes, as Tim pointed out, we cannot detect beforehand if the non-halted read/write feature is supported. That\u0027s because it can differ on per-register basis. Some registers may support it whereas others not. We simply have to try the operation and then check the outcome.","commit_id":"61a2bd65e9b4ce742115bbe614c3b4c84df5e247"}],"src/target/target.c":[{"author":{"_account_id":1002143,"name":"Marek Vrbka","email":"marek.vrbka@codasip.com","username":"MarekVCodasip"},"change_message_id":"091ca063d8d17d1dee3690aee515b60f00969165","unresolved":true,"context_lines":[{"line_number":3054,"context_line":"\tstruct target *target \u003d get_current_target(CMD_CTX);"},{"line_number":3055,"context_line":"\tif (!target_was_examined(target)) {"},{"line_number":3056,"context_line":"\t\tLOG_ERROR(\"Target not examined yet\");"},{"line_number":3057,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":3058,"context_line":"\t}"},{"line_number":3059,"context_line":"\tstruct reg *reg \u003d NULL;"},{"line_number":3060,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"3ce4cbc9_3cf762eb","line":3057,"range":{"start_line":3057,"start_character":9,"end_line":3057,"end_character":19},"updated":"2023-08-01 11:07:24.000000000","message":"I recommend returning ERROR_TARGET_NOT_EXAMINED","commit_id":"df46b4a18d213262f272a68d47ef3cf617a993c1"},{"author":{"_account_id":1002143,"name":"Marek Vrbka","email":"marek.vrbka@codasip.com","username":"MarekVCodasip"},"change_message_id":"fec52ba453fc47ed4d2ff923c4f935df91761818","unresolved":true,"context_lines":[{"line_number":3054,"context_line":"\tstruct target *target \u003d get_current_target(CMD_CTX);"},{"line_number":3055,"context_line":"\tif (!target_was_examined(target)) {"},{"line_number":3056,"context_line":"\t\tLOG_ERROR(\"Target not examined yet\");"},{"line_number":3057,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":3058,"context_line":"\t}"},{"line_number":3059,"context_line":"\tstruct reg *reg \u003d NULL;"},{"line_number":3060,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"c04e958e_97f93a8f","line":3057,"range":{"start_line":3057,"start_character":9,"end_line":3057,"end_character":19},"in_reply_to":"3ce4cbc9_3cf762eb","updated":"2023-08-01 11:12:40.000000000","message":"I do see that other cases return ERROR_FAIL. So unless others have strong opinion on it, you can probably leave as you have it now.","commit_id":"df46b4a18d213262f272a68d47ef3cf617a993c1"},{"author":{"_account_id":1002143,"name":"Marek Vrbka","email":"marek.vrbka@codasip.com","username":"MarekVCodasip"},"change_message_id":"2b6cfef40c1c8e6941694a8d82a090618c13c150","unresolved":false,"context_lines":[{"line_number":3054,"context_line":"\tstruct target *target \u003d get_current_target(CMD_CTX);"},{"line_number":3055,"context_line":"\tif (!target_was_examined(target)) {"},{"line_number":3056,"context_line":"\t\tLOG_ERROR(\"Target not examined yet\");"},{"line_number":3057,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":3058,"context_line":"\t}"},{"line_number":3059,"context_line":"\tstruct reg *reg \u003d NULL;"},{"line_number":3060,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"d7f72cdd_5031e487","line":3057,"range":{"start_line":3057,"start_character":9,"end_line":3057,"end_character":19},"in_reply_to":"54211e0d_b45da35d","updated":"2023-08-01 11:53:59.000000000","message":"Done","commit_id":"df46b4a18d213262f272a68d47ef3cf617a993c1"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"4d231f792fb8f5dbbf9c05a4b294aa11a116c98f","unresolved":true,"context_lines":[{"line_number":3054,"context_line":"\tstruct target *target \u003d get_current_target(CMD_CTX);"},{"line_number":3055,"context_line":"\tif (!target_was_examined(target)) {"},{"line_number":3056,"context_line":"\t\tLOG_ERROR(\"Target not examined yet\");"},{"line_number":3057,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":3058,"context_line":"\t}"},{"line_number":3059,"context_line":"\tstruct reg *reg \u003d NULL;"},{"line_number":3060,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"d704eea4_b456b9ae","line":3057,"range":{"start_line":3057,"start_character":9,"end_line":3057,"end_character":19},"in_reply_to":"c04e958e_97f93a8f","updated":"2023-08-01 11:20:30.000000000","message":"I\u0027ll adjust the MR to return `ERROR_TARGET_NOT_EXAMINED` - it seems reasonable. If this MR will go through I can create a separate cosmetic MR to adjust return codes in other places","commit_id":"df46b4a18d213262f272a68d47ef3cf617a993c1"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"659bba8336d22df3084b064764501bcc4bb5e569","unresolved":true,"context_lines":[{"line_number":3054,"context_line":"\tstruct target *target \u003d get_current_target(CMD_CTX);"},{"line_number":3055,"context_line":"\tif (!target_was_examined(target)) {"},{"line_number":3056,"context_line":"\t\tLOG_ERROR(\"Target not examined yet\");"},{"line_number":3057,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":3058,"context_line":"\t}"},{"line_number":3059,"context_line":"\tstruct reg *reg \u003d NULL;"},{"line_number":3060,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"54211e0d_b45da35d","line":3057,"range":{"start_line":3057,"start_character":9,"end_line":3057,"end_character":19},"in_reply_to":"d704eea4_b456b9ae","updated":"2023-08-01 11:27:21.000000000","message":"addressed","commit_id":"df46b4a18d213262f272a68d47ef3cf617a993c1"}]}