)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1002557,"name":"Saket Sinha","email":"saket.sinha89@gmail.com"},"change_message_id":"3de1efad1498dd0302436c6449d12405b0210221","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"9052c7cc_df924ba5","updated":"2026-08-12 05:01:12.000000000","message":"Thanks for this change — the DTM abstraction + Mem-AP backend is the right shape for\nRP2350 Hazard3 (and other AP-backed RISC-V DMs).\n\nI cherry-picked patchset 1 onto current master and tried the in-tree sample config\non real hardware:\n\n  Pico 2 (RP2350, RISC-V / Hazard3 arch selected)\n  Raspberry Pi Debug Probe (CMSIS-DAP v2, SWD)\n  openocd -f interface/cmsis-dap.cfg -f target/rp2350-riscv.cfg\n\nWith patchset 1 alone, that path does not complete init. Two independent bugs showed\nup; both are in the shared DTM/AP plumbing and are not specific to any third-party\ntarget.\n\n----------------------------------------------------------------\nBug 1 — NULL dereference in get_dm() for non-JTAG DTMs\n----------------------------------------------------------------\n\nSymptom: segfault during \"init\" / target examine when the target is created with\n  riscv -dap … -ap-num …\n(as rp2350-riscv.cfg does).\n\nCause: dm013_info_t is still keyed by target-\u003etap-\u003eabs_chain_position. An AP-backed\nDTM has no TAP, so target-\u003etap is NULL.\n\nFix (small): key the DM on the DTM (r-\u003edtm) instead of the TAP. A JTAG DTM owns one\nTAP and an AP-backed DTM owns one AP, so the DTM is at least as discriminating as\nthe old TAP key and is defined for every backend. Guard if r-\u003edtm is missing.\n\n----------------------------------------------------------------\nBug 2 — use-after-free / AP refcount BUG at shutdown\n----------------------------------------------------------------\n\nSymptom on exit / \"shutdown\":\n  Error: BUG: refcount AP#0 still 1 at exit\nand (under ASan / careful inspection) dap_put_ap() after the DAP is freed.\n\nCause: openocd_main() calls dap_cleanup_all() before riscv_dtm_cleanup_all().\nAn AP-backed DTM holds an AP via dap_get_ap() and releases it with dap_put_ap()\nduring DTM cleanup. Freeing the DAP first leaves a dangling AP pointer and\nleaves the refcount audit unhappy.\n\nFix (small): call riscv_dtm_cleanup_all() *before* arm_cti_cleanup_all() /\ndap_cleanup_all() so APs are put while the DAP is still alive.\n\n----------------------------------------------------------------\nHardware verification\n----------------------------------------------------------------\n\nHost: OpenOCD master + this change + Combined fix below as diff\nInterface: CMSIS-DAP v2 SWD @ 4000 kHz\nTarget Tcl: tcl/target/rp2350-riscv.cfg (from this change)\nFirmware: pico-examples RISC-V blink.elf\n\nResults:\n  - SWD DPIDR 0x4c013477\n  - [target] datacount\u003d1 progbufsize\u003d2  (DMI via Mem-AP reaches the DM)\n  - Examined RISC-V core / GDB attach\n  - monitor reset halt\n  - load blink.elf\n  - break main ; continue → hit main\n  - clean shutdown with Fix 2 (no \"refcount AP#0 still 1\" BUG)\n\n\nPlease find cobined diff for Fix 1 and Fix 2 below : \n\ndiff --git a/src/openocd.c b/src/openocd.c\nindex 06a37980e..8484cddb7 100644\n--- a/src/openocd.c\n+++ b/src/openocd.c\n@@ -362,10 +362,15 @@ int openocd_main(int argc, char *argv[])\n \tunregister_all_commands(cmd_ctx, NULL);\n \thelp_del_all_commands(cmd_ctx);\n \n+\t/* Release AP references held by RISC-V DTMs before the DAP objects they\n+\t * point into are freed: dap_cleanup_all() both audits AP refcounts and\n+\t * frees the DAP, so running it first turns the DTM\u0027s dap_put_ap() into a\n+\t * use-after-free and reports the still-held reference as a BUG. */\n+\triscv_dtm_cleanup_all();\n+\n \t/* free all DAP and CTI objects */\n \tarm_cti_cleanup_all();\n \tdap_cleanup_all();\n-\triscv_dtm_cleanup_all();\n \n \tadapter_quit();\n \ndiff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c\nindex f2c9f1b78..6a93c8e78 100644\n--- a/src/target/riscv/riscv-013.c\n+++ b/src/target/riscv/riscv-013.c\n@@ -112,7 +112,11 @@ typedef enum {\n \n typedef struct {\n \tstruct list_head list;\n-\tunsigned int abs_chain_position;\n+\t/* The DTM this DM is reached through. Used to tell DMs apart: a JTAG DTM\n+\t * owns one TAP, an AP-backed DTM owns one AP, and either way a DM sits\n+\t * behind exactly one of them. Keying on the TAP instead would dereference\n+\t * NULL for every DTM that is not JTAG. */\n+\tconst struct riscv_dtm *dtm;\n \t/* The base address to access this DM on DMI */\n \tuint32_t base;\n \t/* The number of harts connected to this DM. */\n@@ -275,13 +279,16 @@ static dm013_info_t *get_dm(struct target *target)\n \tif (info-\u003edm)\n \t\treturn info-\u003edm;\n \n-\tunsigned int abs_chain_position \u003d target-\u003etap-\u003eabs_chain_position;\n+\tRISCV_INFO(r);\n+\tif (!r-\u003edtm) {\n+\t\tLOG_TARGET_ERROR(target, \"BUG: no DTM assigned to this target.\");\n+\t\treturn NULL;\n+\t}\n \n \tdm013_info_t *entry;\n \tdm013_info_t *dm \u003d NULL;\n \tlist_for_each_entry(entry, \u0026dm_list, list) {\n-\t\tif (entry-\u003eabs_chain_position \u003d\u003d abs_chain_position\n-\t\t\t\t\u0026\u0026 entry-\u003ebase \u003d\u003d target-\u003edbgbase) {\n+\t\tif (entry-\u003edtm \u003d\u003d r-\u003edtm \u0026\u0026 entry-\u003ebase \u003d\u003d target-\u003edbgbase) {\n \t\t\tdm \u003d entry;\n \t\t\tbreak;\n \t\t}\n@@ -292,7 +299,7 @@ static dm013_info_t *get_dm(struct target *target)\n \t\tdm \u003d calloc(1, sizeof(dm013_info_t));\n \t\tif (!dm)\n \t\t\treturn NULL;\n-\t\tdm-\u003eabs_chain_position \u003d abs_chain_position;\n+\t\tdm-\u003edtm \u003d r-\u003edtm;\n \n \t\t/* Safety check for dbgbase */\n \t\tassert(target-\u003edbgbase_set || target-\u003edbgbase \u003d\u003d 0);","commit_id":"faedee0edf4bb469dfbe56dd8f362ae9e2fee117"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"tomas.vanek.oocd@gmail.com","username":"vanekt"},"change_message_id":"78a37decbdda6a290016478c6ec5d43f3101a51f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6e9db5a1_8fbef98e","updated":"2026-08-17 09:20:51.000000000","message":"Thanks for working on this topic.\nI\u0027m the author of 9590: target/riscv: DM access on a DAP | https://review.openocd.org/c/openocd/+/9590\nwhich is also used in RPi OpenOCD fork.\n\nI started the work in 2023 when RISC-V code was even more\nJTAG centric so making riscv target aware of alternate DM interface was the only option. Also the change was made as minimal to ease keeping it with RISC-V code progress.\n\nOf course it would be nice to introduce new API for plugging DMI to more master types than JTAG DTM. On the other hand your patch is way too large for easy review and some regressions are highly possible. For comparison #9590 has \nonly 266 new lines, this one has almost 10 times more.\n\nI\u0027d propose to focus first to review the series around\n9695: target/riscv: add dtm support | https://review.openocd.org/c/openocd/+/9695\nPlease feel free to put comments what would ease future integration of your code.\nAs soon as #9695 or its replacements are ready we can move to this topic.\n\nBTW: You introduced the term DTM backend which is not covered in RISC-V debug spec. Honestly I don\u0027t like such naming, mainly dtm/adi_ap.c. Although in terms of RISC-V debug spec ADI MEM AP is used in the role of \"DTM\", in the particular chip description/block schematics there are only ADI blocks like SWJDP, MEM-AP connected to DM, no DTM at all. So for a new contributor the \"virtual DTM\" may be quite misleading.\nI\u0027d prefer to use DTM name for real JTAG DTM implementation only and reflect the DMI is the layer responsible for selecting the master JTAG DTM, ADI MEM-AP or DMI direct.","commit_id":"faedee0edf4bb469dfbe56dd8f362ae9e2fee117"}]}
