blackmagic-debug / blackmagic

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

Fix: CMSIS-DAP block I/O #1999

Closed dragonmux closed 5 days ago

dragonmux commented 1 week ago

Detailed description

This PR addresses a series of bugs found by j4cbo and Xobs on Discord concerning CMSIS-DAP adaptors. They concern: how the backend handles FAULT responses; how the backend handles WAIT responses due to AP hangs during block transfers (and subsequently, short responses); how the backend handles full responses over Bulk (ZLP handling); how the backend chunks reads; how the backend chunks writes.

This fixes the behaviour of the backend so that when a FAULT response is encountered, the backend cleanly returns up the stack as in a normal transport request - the main code base knows how to handle this, so don't exit() out when it can usually be recovered, and otherwise reported.

This fixes the behaviour of the backend not aborting a transfer on encountering a WAIT response, which is roughly equivalent to the WAIT repsonse handling in adiv5_swd.c.

This fixes mischunking causing over-long read requests to be sent to the adaptor, and over-short write requests, both resulting in nasty data corruption issues. It also makes the buffer behaviour more consistent in initialisation and usage so when a fault like a WAIT response occurs, the data passed back to GDB is in an initialised state, and not just whatever garbage was on the stack.

Tested against the STM32H7 and STM32F1 - the F1's Flash routines trigger the read chunking issue, resulting in Flash data corruption; the H7's tracing components trigger the other conditions as when TRACECLKEN is not set in the DBGMCU, the tracing components are entirely unclocked and accessing their registers results in a permanent WAIT condition on the AP and a bus hang; and then the next request after that results in a FAULT response. Reading 512 byte blocks using GDB's Python interface triggers the over-long read requests. The following was the testing protocol for this:

python
target = gdb.selected_inferior()
gdb.write(target.read_memory(0x5c000000, 512).hex()) # Read the ROM table for the tracing block
gdb.write(target.read_memory(0x5c003000, 512).hex()) # Read the SWO registers - if TRACECLKEN is not set, this will fail but "succeed", WAIT response will be reported on BMDA console
gdb.write(target.read_memory(0x5c015000, 512).hex()) # Read the TPIU registers - if TRACECLKEN is not set, this will outright fail, FAULT response will be reported on BMDA console

Power cycling the target is required after doing this testing protocol in its failing state configuration, and writing DBGMCU's CR register to 0x00700007 with set required for a re-run to test the success state.

Your checklist for this pull request

Closing issues