target/riscv: handle unavailable harts during reset halt by jerryzj · Pull Request #1324 · riscv-collab/riscv-openocd

jerryzj · 2026-05-25T06:23:01Z

Teach the reset-halt path to tolerate harts that temporarily report
unavailable instead of treating them as hard reset failures. Mark
those harts unavailable, defer examination when they cannot be fully
examined, and make the Tcl reset flow skip unavailable or deferred
targets while it waits for halted state.

Keep unavailable state transitions consistent by invalidating register
state, avoiding register flushes on unexamined targets during deinit,
and preserving enough target state for harts to be examined again when
they return.

Update DCSR setup to use progbuf plus DSCRATCH when the debug module
provides scratch storage and enough program buffer space. This avoids
extra abstract register access while still falling back to the existing
direct register path on smaller implementations.

Co-developed-by: cgsfv cgsfv@users.noreply.github.com
Signed-off-by: Jerry Zhang Jian jerry.zhangjian@sifive.com

Teach the reset-halt path to tolerate harts that temporarily report unavailable instead of treating them as hard reset failures. Mark those harts unavailable, defer examination when they cannot be fully examined, and make the Tcl reset flow skip unavailable or deferred targets while it waits for halted state. Keep unavailable state transitions consistent by invalidating register state, avoiding register flushes on unexamined targets during deinit, and preserving enough target state for harts to be examined again when they return. Update DCSR setup to use progbuf plus DSCRATCH when the debug module provides scratch storage and enough program buffer space. This avoids extra abstract register access while still falling back to the existing direct register path on smaller implementations. Co-developed-by: cgsfv <cgsfv@users.noreply.github.com> Signed-off-by: Jerry Zhang Jian <jerry.zhangjian@sifive.com>

Avoid resuming harts that were intentionally halted. Clear the prepped resume state before beginning a new resume sequence and when explicitly halting a hart, preventing stale prep state from triggering unintended resumes on the next poll cycle. When a reset halt command is issued, poll until each hart has reached the halted state before returning. Consolidate the per-hart wait loops and add inter-poll pacing to avoid busy-spinning. Restore the original base DMI delay unconditionally on return so that transient busy backoff during the wait does not affect subsequent operations. Co-developed-by: cgsfv <cgsfv@users.noreply.github.com> Signed-off-by: Jerry Zhang Jian <jerry.zhangjian@sifive.com>

Previously the reset command fired callbacks and halt management across all defined targets, regardless of which target was current. In multi-target setups this causes unintended side effects on targets that should not be part of the current reset sequence. Introduce target_in_reset_group() to determine the reset scope: if the current target is not part of an SMP group, only that target is reset; if it belongs to an SMP group, all targets in that group are reset together. Add the internal arp_reset_targets command and the ocd_get_reset_targets Tcl helper so the Tcl reset script can query the group membership without C changes to startup.tcl's control flow. Note: when SRST is used the physical reset signal still affects all hardware on the board. Only reset event handling and halt management are scoped to the selected reset group. Signed-off-by: Jerry Zhang Jian <jerry.zhangjian@sifive.com>

en-sc

Thank you for taking the time to contribute!

Please, consider submitting it to the mainline OpenOCD (https://review.openocd.org/). This fork is stale and is not actively maintained. Moreover the RISC-V support is more up-to-date in the mainline.
Please, update the commit description (at least for the second commit -- you are talking about the change in poll interaction with reset, but it seems to be mostly cosmetic changes like state -> riscv_state rename).
Please make an effort to split the patches as small as possible (cosmetics/functional/optimization). From my experience, review in the mainline is very long, and posting simpler smaller patches will help you go through the process quicker.

en-sc · 2026-06-01T08:26:53Z

+
+		wait_done = riscv013_reset_wait_done(condition, *dmstatus);
+		if (!wait_done)
+			usleep(10);


Could you please explain the point of this usleep()?

You have the call to dmstatus_read(). Internally, it fills the queues a bunch of operations, sends them to execution and then waits for completion. It can either busy-wait or block on a syscall, but anyway it waits.
If there is a point where yielding control from this thread via a usleep() could be useful, this point is in the inner loop, internal to dmstatus_read() (that is probably absent, since the call is blocking).

So please, remove this usleep(). It just confuses the reader without any positive impact on performance.

en-sc · 2026-06-01T08:40:07Z

-	if (riscv_reg_flush_all(target) != ERROR_OK)
+	/* Unexamined targets may still own per-target allocations, but they won't
+	 * have dirty register state to flush. */
+	if (target_was_examined(target) && riscv_reg_flush_all(target) != ERROR_OK)


I don't quite get what are you trying to fix here. If there are no dirty register in the cache the riscv_reg_flush_all() will not do anything.

If the register cache state is broken, the issue is in the code that breaks the state, not in the code that operates on this broken state.

en-sc · 2026-06-01T08:53:13Z

+		struct riscv_program program;
+		riscv_program_init(&program, target);
+		riscv_program_insert(&program, csrw(S0, CSR_DSCRATCH0));
+		riscv_program_insert(&program, lui(S0, set_ebreak_bits));
+		riscv_program_insert(&program, csrrs(ZERO, S0, CSR_DCSR));
+		riscv_program_insert(&program, lui(S0, clear_ebreak_bits));
+		riscv_program_insert(&program, csrrc(ZERO, S0, CSR_DCSR));
+		if (step)
+			riscv_program_insert(&program, csrsi(CSR_DCSR, 0x4));
+		else
+			riscv_program_insert(&program, csrci(CSR_DCSR, 0x4));
+		riscv_program_insert(&program, csrr(S0, CSR_DSCRATCH0));
+		if (riscv_program_exec(&program, target) != ERROR_OK)
+			return ERROR_FAIL;


Please consider making this a separate patch where we can discuss whether avoiding the extra read does speed up any real-world scenario (the register is cached on reads).

In general, I'd suggest to separate functional changes from optimizations and style-changes to make the review faster.

jerryzj · 2026-06-02T01:29:36Z

Thank you for taking the time to contribute!

Please, consider submitting it to the mainline OpenOCD (review.openocd.org). This fork is stale and is not actively maintained. Moreover the RISC-V support is more up-to-date in the mainline.

Please, update the commit description (at least for the second commit -- you are talking about the change in poll interaction with reset, but it seems to be mostly cosmetic changes like state -> riscv_state rename).

Please make an effort to split the patches as small as possible (cosmetics/functional/optimization). From my experience, review in the mainline is very long, and posting simpler smaller patches will help you go through the process quicker.

Noted. Thanks for the reply & review.

JanMatCodasip · 2026-06-08T12:57:28Z

+		struct riscv_program program;
+		riscv_program_init(&program, target);
+		riscv_program_insert(&program, csrw(S0, CSR_DSCRATCH0));
+		riscv_program_insert(&program, lui(S0, set_ebreak_bits));
+		riscv_program_insert(&program, csrrs(ZERO, S0, CSR_DCSR));
+		riscv_program_insert(&program, lui(S0, clear_ebreak_bits));
+		riscv_program_insert(&program, csrrc(ZERO, S0, CSR_DCSR));
+		if (step)
+			riscv_program_insert(&program, csrsi(CSR_DCSR, 0x4));
+		else
+			riscv_program_insert(&program, csrci(CSR_DCSR, 0x4));
+		riscv_program_insert(&program, csrr(S0, CSR_DSCRATCH0));
+		if (riscv_program_exec(&program, target) != ERROR_OK)
+			return ERROR_FAIL;


@jerryzj Thanks for your patch.

Why do you please introduce a second code path (program buffer access) here?

Instead, I would just suggest to:

Read the current value using riscv_reg_get().

Make the modifications - set and clear bits as needed.

Write the value back using riscv_reg_write().

In any case, as @en-sc already mentioned, please submit your changes to the vanilla OpenOCD repository instead: https://review.openocd.org.

jerryzj added 3 commits May 25, 2026 14:22

en-sc reviewed Jun 1, 2026

View reviewed changes

JanMatCodasip reviewed Jun 8, 2026

View reviewed changes

Navigation Menu

Uh oh!

target/riscv: handle unavailable harts during reset halt - #1324

target/riscv: handle unavailable harts during reset halt#1324
jerryzj wants to merge 3 commits into
riscv-collab:riscvfrom
jerryzj:dev/reset-halt

jerryzj commented May 25, 2026

Uh oh!

en-sc left a comment

Uh oh!

en-sc Jun 1, 2026

Uh oh!

en-sc Jun 1, 2026

Uh oh!

en-sc Jun 1, 2026

Uh oh!

jerryzj commented Jun 2, 2026

Uh oh!

JanMatCodasip Jun 8, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

jerryzj commented May 25, 2026

Uh oh!

en-sc left a comment

Choose a reason for hiding this comment

Uh oh!

en-sc Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

en-sc Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

en-sc Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

jerryzj commented Jun 2, 2026

Uh oh!

JanMatCodasip Jun 8, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JanMatCodasip Jun 8, 2026 •

edited

Loading