dm_mem: bind abstract commands to admission hartsel by devtyagi3909 · Pull Request #204 · pulp-platform/riscv-dbg

devtyagi3909 · 2026-09-02T16:59:07Z

Fixes #203.

As described in the issue, dm_mem previously keyed go/resume flags and abstract command completion off the live hartsel value instead of the hart selected at command admission. If dmcs.hartsel changed mid-command, the execution would be incorrectly relocated to the new hart.

This PR introduces cmd_hartsel_q, which latches hartsel at command admission (during the Idle state) and uses the latched value throughout the Go, Resume, and CmdExecuting states, as well as for the FlagsBaseAddr polling logic. This ensures that the command is safely bound to the originally selected hart regardless of subsequent hartsel changes while busy=1.

Keep accepted resume actions pending per hart until acknowledged, clear only the acknowledgement of the request target, and preserve same-cycle request and acknowledgement priority. Co-authored-by: Luis Donatien <luis.waucquez.jimenez@alumnos.upm.es>

Exercise retained per-hart requests and flag routing, stale and simultaneous acknowledgements, WARL hart selection, selectable harts, and reset precedence through the integrated Debug Module. Share the Verilator runner and Bender checkout across focused simulator targets.

Mask the write-only haltreq field from DMControl reads while retaining the stored request that drives the selected hart. Extend the multi-hart regression to verify both behaviors.

Expose per-hart pending resume actions directly through the flag page so they cannot block or discard unrelated abstract commands. Admit pending actions only for available halted harts, preserve acknowledgements across non-DM reset, reset them through dmactive, and reject invalid debug-memory hart IDs.

phsauter · 2026-09-02T20:48:38Z

Thanks for the PR, this looks like the correct general approach.

I think it is best to merge #194 first. That PR removes the Resume state from dm_mem and handles resume requests per-art independent of abstract commands.
Could you rebase this on top of #194 and restrict cmd_hartsel_q to abstract commands?
I will give #194 another look in the coming days and merge if I cannot find anything wrong with it.

Some other thoughts:

Please latch cmd_hartsel_q only when an abstract command is accepted, rather than always updating it while in Idle. That should makes it more explicit which hartsel belongs to the command.
We will also need to add a multi-hart regression test which starts an abstract command on one hart, changes hartsel while it is busy, and verifies that the go flag and completion are associated with the original hart, you can do that or I will do it in a separate follow-up PR.

Optionally, since the current debug spec explicitly recommends the hardware to enforce the busy-time restriction, it would be nice to just ignore writes to hartsel and the listed dmcontrol action bits while abstractcs.busy is asserted The hart latch is still useful for robustness but ignoring those writes would implement the specs recommendation precisely.

Fixes pulp-platform#203. Previously, dm_mem used the live dmcs.hartsel value to route the 'go' and 'resume' flags during command execution, and to wait for completion. If hartsel was changed by the debugger mid-command (while busy=1), the in-flight command would erroneously be relocated to the newly selected hart, causing cross-hart register injection or unrecoverable wedges. This introduces a cmd_hartsel_q register that latches the target hart at command admission (in the Idle state) and uses the latched value for go/resume routing and completion tracking, ensuring execution is bound to the hart selected when the command was admitted. It also enforces the debug spec recommendation by having dm_csrs ignore writes to dmcontrol (except dmactive and ndmreset) while cmdbusy is high. Rebased on top of pulp-platform#194.

devtyagi3909 · 2026-09-02T22:26:20Z

Thanks @phsauter! I've force-pushed an update that incorporates all your feedback:

Rebased on top of dm: Fix multi-hart resume acknowledgement handling #194.
cmd_hartsel_d is now exclusively latched when an abstract command is accepted, rather than continuously in Idle.
In dm_csrs.sv, writes to dmcontrol (except ndmreset and dmactive) are now ignored while cmdbusy_i is asserted, strictly implementing the busy-time restriction recommended by the debug spec.

I will leave the multi-hart regression test to the follow-up PR, but everything on the RTL side should now be fully covered and spec-compliant!

phsauter · 2026-09-03T16:11:38Z

+  assign dmcontrol_write_accepted = dmi_req_ready_o && dmi_req_valid_i &&
+                                    dtm_op == dm::DTM_WRITE &&
+                                    dm_csr_addr == dm::DMControl;
+  assign dmcontrol_write_resumereq = dmcontrol_write_accepted && dmi_req_i.data[30];


I think this got lost in the rebase because its now handled immediately here:

assign dmcontrol_write_resumereq = dmcontrol_write_accepted && !cmdbusy_i && dmi_req_i.data[30];

phsauter

Just one thing probably caused by the rebase, the rest is good.

phsauter · 2026-09-03T22:13:56Z

I had to add one more commit to #194 to fix an edge-case but now it is merged, so yours is next.

devtyagi3909 · 2026-09-04T02:01:01Z

Done! Added the !cmdbusy_i check to dmcontrol_write_resumereq as requested. Good catch on the rebase conflict.

phsauter and others added 4 commits August 11, 2026 15:37

dm: Fix multi-hart resume handling

2dbd76c

Keep accepted resume actions pending per hart until acknowledged, clear only the acknowledgement of the request target, and preserve same-cycle request and acknowledgement priority. Co-authored-by: Luis Donatien <luis.waucquez.jimenez@alumnos.upm.es>

dm: Make haltreq read as zero

c4fc8a2

Mask the write-only haltreq field from DMControl reads while retaining the stored request that drives the selected hart. Extend the multi-hart regression to verify both behaviors.

devtyagi3909 force-pushed the fix-abstract-cmd-hartsel branch from 3a6ddc5 to 2a1801d Compare September 2, 2026 22:25

phsauter reviewed Sep 3, 2026

View reviewed changes

phsauter requested changes Sep 3, 2026

View reviewed changes

dm_csrs: ignore resumereq when cmdbusy is asserted

a9bcc93

phsauter merged commit 5d8d735 into pulp-platform:master Sep 4, 2026
6 checks passed

devtyagi3909 deleted the fix-abstract-cmd-hartsel branch September 4, 2026 18:07

devtyagi3909 mentioned this pull request Sep 5, 2026

[rv_dm] Fix dmcontrol read to return 0 for haltreq and resumereq lowRISC/opentitan#31209

Closed

john-kearney mentioned this pull request Sep 11, 2026

Abstract commands are not bound to the hart selected at admission — hartsel change mid-command relocates execution to a different hart #203

Closed

Navigation Menu

Repository navigation

dm_mem: bind abstract commands to admission hartsel - #204

dm_mem: bind abstract commands to admission hartsel#204
phsauter merged 6 commits into
pulp-platform:masterfrom
devtyagi3909:fix-abstract-cmd-hartsel

devtyagi3909 commented Sep 2, 2026

Uh oh!

phsauter commented Sep 2, 2026

Uh oh!

devtyagi3909 commented Sep 2, 2026

Uh oh!

phsauter Sep 3, 2026 •

edited

Loading

Uh oh!

phsauter left a comment

Uh oh!

phsauter commented Sep 3, 2026

Uh oh!

devtyagi3909 commented Sep 4, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

devtyagi3909 commented Sep 2, 2026

Uh oh!

phsauter commented Sep 2, 2026

Uh oh!

devtyagi3909 commented Sep 2, 2026

Uh oh!

phsauter Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phsauter left a comment

Choose a reason for hiding this comment

Uh oh!

phsauter commented Sep 3, 2026

Uh oh!

devtyagi3909 commented Sep 4, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

phsauter Sep 3, 2026 •

edited

Loading