Skip to content

Conversation

Wren6991
Copy link
Contributor

This code forced all harts to halt when they reported a mixture of halted and non-halted status. This breaks long-running algorithms run on a single core under SMP, because the first poll will force the core executing the algorithm to halt. For example, this killed long-running flash erase operations on RP2350, which are mask ROM routines called by OpenOCD algorithms, running on a single core while the other core remains halted.

Fix by adding an exception when all running cores are running under debugger control (target->status == 4). Do not force harts to halt in this case.

This code forced all harts to halt when they reported a mixture of
halted and non-halted status. This breaks long-running algorithms run
on a single core under SMP, because the first poll will force the core
executing the algorithm to halt.

Fix by adding an exception when all running cores are running under
debugger control (target->status == 4). Do not force harts to halt in
this case.

Signed-off-by: Luke Wren <[email protected]>
Copy link
Collaborator

@MarekVCodasip MarekVCodasip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks for the fix.

I am just curious: Was this tested on another fork of OpenOCD? This fork does not support SWD for RISC-V targets, which is needed for RP2350.

Copy link
Collaborator

@en-sc en-sc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Wren6991, thank you for the patch! Sorry for taking so long to review. LGTM

@Wren6991
Copy link
Contributor Author

I am just curious: Was this tested on another fork of OpenOCD? This fork does not support SWD for RISC-V targets, which is needed for RP2350.

Yes this was tested on our downstream fork, but we've been working with an OpenOCD maintainer to get our patches upstream. Upstream now has support for accessing a Debug Module directly through an ADI DAP.

You can actually run this fork (riscv-openocd) against RP2350 with some slightly spicy probe firmware that emulates a JTAG-DTM and tunnels the writes through ADI, but that wouldn't be useful for a repro here, because you don't have the RP2350 flash driver. This could probably be repro'd on SMP spike -- there are already a few tests in riscv-tests that actually seem to be covering OpenOCD regressions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants