)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"56691d690b7dd38e2697209fa0de36af7d70d131","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":7,"id":"db088129_f38f71ca","updated":"2026-06-02 09:27:06.000000000","message":"@liangzhen, I\u0027d like to suggest an alternative approach that is less reliant on external factors (polling order, reset of `target-\u003eexamined` on failure).\nThe idea is as follows:\n* Add `RPH_UNKNOWN` into `enum riscv_next_action`.\n* In `riscv_poll_hart()` `next_action` is set to `RPH_UNKNOWN` by default and is changed to `RPH_NONE` in `case SEMIHOSTING_NONE:` block.\n* Introduce `enum riscv_next_action riscv_info::next_action` to store the output of the last poll of a target.\n* Upon entry to `riscv_openocd_poll()`, `riscv_poll_hart()` is called on the current `target` only, updating `i-\u003enext_action`.\n* If `riscv_poll_hart()` fails, exit early (note that we should take care to reset `next_action` to `RPH_UNKNOWN` on failure inside `riscv_poll_hart()`).\n* Check other harts in the SMP group, counting `should_remain_halted`, etc. If a target in the SMP group has `info-\u003enext_action` set to `RPH_UNKNOWN` exit returning `ERROR_OK`.\n* Once `should_remain_halted` etc., are counted the processing is left unchanged.\n\nPlease let me know what you think. If needed I can implement this in a different patch, or you can implement it yourself here, whatever is more convenient to you.","commit_id":"ccaf26e9570d72cf573e48f3b6fda070508a04a6"},{"author":{"_account_id":1002315,"name":"liangzhen","email":"zhen.liang@spacemit.com","username":"liangzhen"},"change_message_id":"232de22da8f0b6d129134632ca1a9db25dd0b10c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":7,"id":"fa0d8f56_97ce9255","in_reply_to":"db088129_f38f71ca","updated":"2026-06-02 10:13:50.000000000","message":"Thank you for the suggestion! I think it is a great idea and I can\nimplement it in here.","commit_id":"ccaf26e9570d72cf573e48f3b6fda070508a04a6"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"32e39ded52eba75a15041cfb378f23e93c58bf4c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"bd04eecc_da9eb11b","in_reply_to":"fa0d8f56_97ce9255","updated":"2026-06-02 15:10:39.000000000","message":"Ack","commit_id":"ccaf26e9570d72cf573e48f3b6fda070508a04a6"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"5cafd28250768e707d1d4bfbf796693bad97aea5","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":11,"id":"17964a7f_ff79e05d","updated":"2026-06-24 06:28:46.000000000","message":"Hi liangzhen,\n\nthank you for your contribution.\n\nSince the logic around target state and target polling is already complex, I would recommend a different approach:\n\n- Don\u0027t store the next action into \"struct riscv_info\" (which is already huge and error-prone).\n- Instead, avoid the redundant polling by updating the riscv_openocd_poll() this way (pseudo-code):\n\n```\nint riscv_openocd_poll(struct target *target)\n{\n    if (target is part of SMP group) {\n        if (target is the first item in the SMP grou) {\n            poll_all_harts_in_the_group();\n        } else {\n            // Don\u0027t do anything - the harts were already polled\n            return ERROR_OK;\n        }\n    } else {\n        // Not a member of any SMP group\n        poll_the_single_hart();\n    }\n}\n\n```\n\nDoes that approach look OK to you?","commit_id":"5e27bf2852d10a428518647c0321fdbdff54b94e"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"e935cb6451b38f645302955de9e304d608afa81f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"40704b6d_7cdc150b","updated":"2026-06-03 09:22:15.000000000","message":"LGTM","commit_id":"5e27bf2852d10a428518647c0321fdbdff54b94e"},{"author":{"_account_id":1002315,"name":"liangzhen","email":"zhen.liang@spacemit.com","username":"liangzhen"},"change_message_id":"a8a3d41ee5d441aa96fa0750957afb6a6c2af993","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":11,"id":"91285ba0_fac36395","in_reply_to":"17964a7f_ff79e05d","updated":"2026-06-24 07:39:05.000000000","message":"Hi, Jan\n\nI believe this code structure is quite clear for readers. I think patchset 7 have used a similar method as described by you (target is the first examine successful item in the SMP grou), but Evgeniy suggest using the current method.","commit_id":"5e27bf2852d10a428518647c0321fdbdff54b94e"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"38d96d0b311583d2f0ef2ec582cc2033b143450e","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":11,"id":"0bfc998b_c8652464","in_reply_to":"91285ba0_fac36395","updated":"2026-06-24 08:06:43.000000000","message":"Thanks for the explanation. Let me re-read the previous discussion, then.\n\nStoring the action into struct riscv_info still looks to me like a hack and I am concerned that it makes the code around target states and polling even more complicated and bug-prone (and honestly, it is hard to understand and maintain already as is).\n\nBut maybe after reading your past discussion I\u0027ll change my mind.","commit_id":"5e27bf2852d10a428518647c0321fdbdff54b94e"}],"src/target/riscv/riscv.c":[{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"349293ad0e5cea73a041e1bd4b3be257cbca4233","unresolved":true,"context_lines":[{"line_number":4034,"context_line":""},{"line_number":4035,"context_line":"\tif (target-\u003esmp) {"},{"line_number":4036,"context_line":"\t\ttargets \u003d target-\u003esmp_targets;"},{"line_number":4037,"context_line":"\t\tentry \u003d list_first_entry(targets, struct target_list, lh);"},{"line_number":4038,"context_line":"\t\tstruct target *first_target \u003d entry-\u003etarget;"},{"line_number":4039,"context_line":"\t\tif (first_target !\u003d target) {"},{"line_number":4040,"context_line":"\t\t\tLOG_TARGET_DEBUG(target, \"target already polled in smp group %d\", target-\u003esmp);"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"499e334f_7401f9be","line":4037,"updated":"2026-05-26 12:29:05.000000000","message":"There seems to be an issue with this approach regarding the interaction between `target-\u003eexamined` and `poll()`.\n\nConsider the following examples of 2 targets in an SMP group (`rv0` and `rv1`) (`rv0` is the first in the SMP group and in target list):\n\n1. Target `examine()` fails on `rv0` and continues to fail. `rv1` is never polled.\n\n2. All accesses to `rv1` fails. This causes `poll()` to fail on `rv0`. Now `rv0` is needlessly re-examined again and again (succeeds every time). Before the change both targets were re-examined.\n\n3. Examine succeeds on `rv0`, fails on `rv1`. During `poll()` of `rv0` `rv1` is accessed, **though it is not examined**. Note, that this is not specific to this patch. @Tomas, please comment on this when you have the time.\n\nTBH I don\u0027t see an easy fix for these issues.","commit_id":"3ab4f860debbad765d71fd3ba2c5add1059470aa"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"56691d690b7dd38e2697209fa0de36af7d70d131","unresolved":true,"context_lines":[{"line_number":4034,"context_line":""},{"line_number":4035,"context_line":"\tif (target-\u003esmp) {"},{"line_number":4036,"context_line":"\t\ttargets \u003d target-\u003esmp_targets;"},{"line_number":4037,"context_line":"\t\tentry \u003d list_first_entry(targets, struct target_list, lh);"},{"line_number":4038,"context_line":"\t\tstruct target *first_target \u003d entry-\u003etarget;"},{"line_number":4039,"context_line":"\t\tif (first_target !\u003d target) {"},{"line_number":4040,"context_line":"\t\t\tLOG_TARGET_DEBUG(target, \"target already polled in smp group %d\", target-\u003esmp);"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"5cce388f_ccc108e1","line":4037,"in_reply_to":"381daf2a_f23222be","updated":"2026-06-02 09:27:06.000000000","message":"Actually, (3) is not an issue, I\u0027ve missed a check in the loop.","commit_id":"3ab4f860debbad765d71fd3ba2c5add1059470aa"},{"author":{"_account_id":1002315,"name":"liangzhen","email":"zhen.liang@spacemit.com","username":"liangzhen"},"change_message_id":"71764a5015f9989489bf596a78fca46eab7feca4","unresolved":true,"context_lines":[{"line_number":4034,"context_line":""},{"line_number":4035,"context_line":"\tif (target-\u003esmp) {"},{"line_number":4036,"context_line":"\t\ttargets \u003d target-\u003esmp_targets;"},{"line_number":4037,"context_line":"\t\tentry \u003d list_first_entry(targets, struct target_list, lh);"},{"line_number":4038,"context_line":"\t\tstruct target *first_target \u003d entry-\u003etarget;"},{"line_number":4039,"context_line":"\t\tif (first_target !\u003d target) {"},{"line_number":4040,"context_line":"\t\t\tLOG_TARGET_DEBUG(target, \"target already polled in smp group %d\", target-\u003esmp);"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"381daf2a_f23222be","line":4037,"in_reply_to":"499e334f_7401f9be","updated":"2026-06-01 10:54:40.000000000","message":"Thanks for the detailed description. I made some simple modifications, which seem to fix some of the issues mentioned above.","commit_id":"3ab4f860debbad765d71fd3ba2c5add1059470aa"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"32e39ded52eba75a15041cfb378f23e93c58bf4c","unresolved":false,"context_lines":[{"line_number":4034,"context_line":""},{"line_number":4035,"context_line":"\tif (target-\u003esmp) {"},{"line_number":4036,"context_line":"\t\ttargets \u003d target-\u003esmp_targets;"},{"line_number":4037,"context_line":"\t\tentry \u003d list_first_entry(targets, struct target_list, lh);"},{"line_number":4038,"context_line":"\t\tstruct target *first_target \u003d entry-\u003etarget;"},{"line_number":4039,"context_line":"\t\tif (first_target !\u003d target) {"},{"line_number":4040,"context_line":"\t\t\tLOG_TARGET_DEBUG(target, \"target already polled in smp group %d\", target-\u003esmp);"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"eaa79858_f7e7914d","line":4037,"in_reply_to":"5cce388f_ccc108e1","updated":"2026-06-02 15:10:39.000000000","message":"Ack","commit_id":"3ab4f860debbad765d71fd3ba2c5add1059470aa"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"4bf6bc55e7802ed8c53fa579023513f60e736407","unresolved":true,"context_lines":[{"line_number":4042,"context_line":"\t\t\tif (t \u003d\u003d target)"},{"line_number":4043,"context_line":"\t\t\t\tbreak;"},{"line_number":4044,"context_line":"\t\t\tif (target_was_examined(t)) {"},{"line_number":4045,"context_line":"\t\t\t\tLOG_TARGET_DEBUG(target, \"target already polled in smp group %d\", target-\u003esmp);"},{"line_number":4046,"context_line":"\t\t\t\treturn i-\u003esmp_polled;"},{"line_number":4047,"context_line":"\t\t\t}"},{"line_number":4048,"context_line":"\t\t}"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"3ffab51e_2576ea92","line":4045,"updated":"2026-06-01 12:56:46.000000000","message":"This seems strange.\n\nImagine two targets. Both are examined. The order when polling is `tgt0`, `tgt1`. The order in SMP group is `tgt1`, `tgt0`.\n`riscv_openocd_poll()` is entered, `target` is `tgt0`.\nFirst iteration of the `foreach_smp_target`, `tgt1` is not `target`, `tgt1` is examined, this log message is printed and `riscv_openocd_poll()` is exited. Seems like an error.\n\nMoreover, the check `target_was_examined(t)` is a property of `t`, but reporting is done on `target`, which is not equal to `t`.","commit_id":"2513d81c72e7c405d4701594660bad122d1941c0"},{"author":{"_account_id":1002315,"name":"liangzhen","email":"zhen.liang@spacemit.com","username":"liangzhen"},"change_message_id":"e052bba4ab07149e81aa35c2f1dc09003cf67a81","unresolved":true,"context_lines":[{"line_number":4042,"context_line":"\t\t\tif (t \u003d\u003d target)"},{"line_number":4043,"context_line":"\t\t\t\tbreak;"},{"line_number":4044,"context_line":"\t\t\tif (target_was_examined(t)) {"},{"line_number":4045,"context_line":"\t\t\t\tLOG_TARGET_DEBUG(target, \"target already polled in smp group %d\", target-\u003esmp);"},{"line_number":4046,"context_line":"\t\t\t\treturn i-\u003esmp_polled;"},{"line_number":4047,"context_line":"\t\t\t}"},{"line_number":4048,"context_line":"\t\t}"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"f458bbdf_7162888a","line":4045,"in_reply_to":"3ffab51e_2576ea92","updated":"2026-06-01 14:43:06.000000000","message":"\u003eImagine two targets. Both are examined. The order when polling is tgt0, tgt1. \u003eThe order in SMP group is tgt1, tgt0.\n\nI didn\u0027t consider the possibility of inconsistency between the order of the polling targets(based on `target create`) and the order of the smp goup(based on `target smp`) here.\n\n\u003eMoreover, the check target_was_examined(t) is a property of t, but reporting \u003eis done on target, which is not equal to t.\n\nBased on the above assumption, under `target_was_examined(t) \u003d\u003d true`, all harts have already been polled.","commit_id":"2513d81c72e7c405d4701594660bad122d1941c0"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"32e39ded52eba75a15041cfb378f23e93c58bf4c","unresolved":false,"context_lines":[{"line_number":4042,"context_line":"\t\t\tif (t \u003d\u003d target)"},{"line_number":4043,"context_line":"\t\t\t\tbreak;"},{"line_number":4044,"context_line":"\t\t\tif (target_was_examined(t)) {"},{"line_number":4045,"context_line":"\t\t\t\tLOG_TARGET_DEBUG(target, \"target already polled in smp group %d\", target-\u003esmp);"},{"line_number":4046,"context_line":"\t\t\t\treturn i-\u003esmp_polled;"},{"line_number":4047,"context_line":"\t\t\t}"},{"line_number":4048,"context_line":"\t\t}"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"8f3ccdc7_f0e28ac3","line":4045,"in_reply_to":"f458bbdf_7162888a","updated":"2026-06-02 15:10:39.000000000","message":"Ack","commit_id":"2513d81c72e7c405d4701594660bad122d1941c0"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"4bf6bc55e7802ed8c53fa579023513f60e736407","unresolved":true,"context_lines":[{"line_number":4043,"context_line":"\t\t\t\tbreak;"},{"line_number":4044,"context_line":"\t\t\tif (target_was_examined(t)) {"},{"line_number":4045,"context_line":"\t\t\t\tLOG_TARGET_DEBUG(target, \"target already polled in smp group %d\", target-\u003esmp);"},{"line_number":4046,"context_line":"\t\t\t\treturn i-\u003esmp_polled;"},{"line_number":4047,"context_line":"\t\t\t}"},{"line_number":4048,"context_line":"\t\t}"},{"line_number":4049,"context_line":"\t} else {"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"ea056d34_cdc64dfe","line":4046,"updated":"2026-06-01 12:56:46.000000000","message":"Returning a `bool` seems like an error.\n\nMoreover, AFAIU `i` corresponds to `target`, which is not equal to `t` here. I don\u0027t get what is the intention here.","commit_id":"2513d81c72e7c405d4701594660bad122d1941c0"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"32e39ded52eba75a15041cfb378f23e93c58bf4c","unresolved":false,"context_lines":[{"line_number":4043,"context_line":"\t\t\t\tbreak;"},{"line_number":4044,"context_line":"\t\t\tif (target_was_examined(t)) {"},{"line_number":4045,"context_line":"\t\t\t\tLOG_TARGET_DEBUG(target, \"target already polled in smp group %d\", target-\u003esmp);"},{"line_number":4046,"context_line":"\t\t\t\treturn i-\u003esmp_polled;"},{"line_number":4047,"context_line":"\t\t\t}"},{"line_number":4048,"context_line":"\t\t}"},{"line_number":4049,"context_line":"\t} else {"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"5eef4ca6_f08be693","line":4046,"in_reply_to":"ea056d34_cdc64dfe","updated":"2026-06-02 15:10:39.000000000","message":"Ack","commit_id":"2513d81c72e7c405d4701594660bad122d1941c0"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"4bf6bc55e7802ed8c53fa579023513f60e736407","unresolved":true,"context_lines":[{"line_number":4100,"context_line":""},{"line_number":4101,"context_line":"\tif (result !\u003d ERROR_OK) {"},{"line_number":4102,"context_line":"\t\tLOG_TARGET_WARNING(target, \"failed to poll all harts\u0027 state\");"},{"line_number":4103,"context_line":"\t\treturn i-\u003esmp_polled;"},{"line_number":4104,"context_line":"\t}"},{"line_number":4105,"context_line":""},{"line_number":4106,"context_line":"\tLOG_TARGET_DEBUG(target, \"should_remain_halted\u003d%d, should_resume\u003d%d\","}],"source_content_type":"text/x-csrc","patch_set":5,"id":"a2ae6fca_e3bed866","line":4103,"updated":"2026-06-01 12:56:46.000000000","message":"Same as above. Returning a `bool` instead of an error code.","commit_id":"2513d81c72e7c405d4701594660bad122d1941c0"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"32e39ded52eba75a15041cfb378f23e93c58bf4c","unresolved":false,"context_lines":[{"line_number":4100,"context_line":""},{"line_number":4101,"context_line":"\tif (result !\u003d ERROR_OK) {"},{"line_number":4102,"context_line":"\t\tLOG_TARGET_WARNING(target, \"failed to poll all harts\u0027 state\");"},{"line_number":4103,"context_line":"\t\treturn i-\u003esmp_polled;"},{"line_number":4104,"context_line":"\t}"},{"line_number":4105,"context_line":""},{"line_number":4106,"context_line":"\tLOG_TARGET_DEBUG(target, \"should_remain_halted\u003d%d, should_resume\u003d%d\","}],"source_content_type":"text/x-csrc","patch_set":5,"id":"cc1509e6_039e65c4","line":4103,"in_reply_to":"a2ae6fca_e3bed866","updated":"2026-06-02 15:10:39.000000000","message":"Ack","commit_id":"2513d81c72e7c405d4701594660bad122d1941c0"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"32e39ded52eba75a15041cfb378f23e93c58bf4c","unresolved":true,"context_lines":[{"line_number":4140,"context_line":"\t\t\t\ti-\u003ehalt_group_repoll_count++;"},{"line_number":4141,"context_line":"\t\t\t\talive_sleep(10);"},{"line_number":4142,"context_line":"\t\t\t\tLOG_TARGET_DEBUG(target, \"Re-polling the state of the SMP group.\");"},{"line_number":4143,"context_line":"\t\t\t\treturn riscv_openocd_poll(target);"},{"line_number":4144,"context_line":"\t\t\t}"},{"line_number":4145,"context_line":"\t\t\t/* We have already re-polled multiple times but the halt group is still inconsistent. */"},{"line_number":4146,"context_line":"\t\t\tLOG_TARGET_DEBUG(target, \"Re-polled the SMP group %d times it is still not in a consistent state.\","}],"source_content_type":"text/x-csrc","patch_set":8,"id":"8e40d245_492cf345","line":4143,"updated":"2026-06-02 15:10:39.000000000","message":"I believe, to preserve the behavior we should do something like:\n```\nint result \u003d ERROR_OK;\nforeach_smp_target(entry, targets) {\n\tstruct target *t \u003d entry-\u003etarget;\n\tresult \u003d riscv_openocd_poll(t) \u003d\u003d ERROR_OK ? result : ERROR_FAIL;\n}\nreturn result;\n```\nI\u0027m not sure the current behavior is better then the alternative, I just think it should be preserved by this patch.","commit_id":"0345b8d89bd98baea508f5c3265b1348b4e48fed"},{"author":{"_account_id":1002315,"name":"liangzhen","email":"zhen.liang@spacemit.com","username":"liangzhen"},"change_message_id":"6277a9382e3d13910fe07926f4d0364e4f3a8f6c","unresolved":false,"context_lines":[{"line_number":4140,"context_line":"\t\t\t\ti-\u003ehalt_group_repoll_count++;"},{"line_number":4141,"context_line":"\t\t\t\talive_sleep(10);"},{"line_number":4142,"context_line":"\t\t\t\tLOG_TARGET_DEBUG(target, \"Re-polling the state of the SMP group.\");"},{"line_number":4143,"context_line":"\t\t\t\treturn riscv_openocd_poll(target);"},{"line_number":4144,"context_line":"\t\t\t}"},{"line_number":4145,"context_line":"\t\t\t/* We have already re-polled multiple times but the halt group is still inconsistent. */"},{"line_number":4146,"context_line":"\t\t\tLOG_TARGET_DEBUG(target, \"Re-polled the SMP group %d times it is still not in a consistent state.\","}],"source_content_type":"text/x-csrc","patch_set":8,"id":"db8b54db_fac3f81d","line":4143,"in_reply_to":"8e40d245_492cf345","updated":"2026-06-02 15:54:53.000000000","message":"Done","commit_id":"0345b8d89bd98baea508f5c3265b1348b4e48fed"}]}
