blackmagic-debug / blackmagic

In application debugger for ARM Cortex microcontrollers.
GNU General Public License v3.0
3.29k stars 774 forks source link

Fix: Memory access halting heuristic #1962

Closed dragonmux closed 1 month ago

dragonmux commented 1 month ago

Detailed description

In this PR we address the incredibly broken target halting for memory access heuristic, replacing it fully with a TOPT which can be set on targets that are confirmed to work without bad reads, CPU halts or perturbed timings (such as stolen bus cycles that upset hard real-time applications).

For now this is only enabled on RISC-V targets that use the System Bus mechanism for memory access as that's guaranteed to be okay by the RISC-V Debug specification, however it can be easily enabled for targets where it's confirmed okay by simply adding target->target_options |= TOPT_NON_HALTING_MEM_IO; in the probe routine for that target.

This method has been chosen on account of how difficult it is to find out if a target can do this correctly, and the numerous counter-examples shown for, eg, saying "all Cortex-M are okay" as this is not true on the Tiva-C series, instantly striking down this statement and requiring a narrower enablement. Before a target can be enabled as allowing non-halting memory I/O it must be shown to: be able to read memory correctly without halting (this is the problem for the Tiva-C, you will get back garbled reads if you try this); be able to read without halting the CPU (eg, checking SysTick timings); be able to show that the reads to not perturb the CPU using the bus (indication of stolen cycles) in a way that throws off timings (eg, by wiggling a GPIO in a loop).

With this change done, some sort of memory monitoring command becomes possible as was being requested/talked about in #1532 and Koen's PRs for "memwatch". These were not taken before as it introduced obvious unsafety into the code on account it did not block reading on Cortex-A/R targets where, short of the vendor providing a secondary AP for memory I/O, it is not possible to do a non-halting read, and this heuristic did not stop attempts on RISC-V parts that use the Abstract Command mechanism or progbuf mechanism for access.

Your checklist for this pull request

Closing issues

dragonmux commented 1 month ago

💯 we're glad to have got this heuristic situation fixed as well as it'd been bothering us as we do actually want to see more features for memory debugging merged, just.. safely!