Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Avoid a hang in scanning Cortex-A of STM32MP15x #1628

Merged
merged 9 commits into from
Sep 23, 2023

Conversation

ALTracer
Copy link
Contributor

Detailed description

BMD at current main busy-loops (hangs, effective DoS) if user attempts to scan a STM32MP15x device with any of supported adapters, requiring BMDA restart or probe reboot. This behaviour was enabled recently by Cortex-A related PRs. Previously BMD did not attempt to attach, halt, or otherwise touch CA7 during scan/probe.

  • This is more a fix than a feature, but introduces registers not touched previously.
  • The pull request solves an existing problem of BMD hang upon trying to even scan STM32MP15x SoC.
  • To solve this, the pull request implements checking and clearing OSLock in ARMv7-A Debug registers.

It took three consecutive days of log analysis from different adapters and gdbserver software with help and suggestions from Dragonmux to come up with some working fix.
Our working hypothesis is that even in Engineering Boot, the primary bootloader (BootROM) enables OSLock, which per Arch.ref.man. prohibits external debugger access to some registers. OpenOCD clears it, so when working through a kind of adapter both servers speak, I could observe BMDA working fine with this target after running OOCD once and before power-cycling DUT (there is no good SRST/Reset button solution without dropping Vdd+Vddcore, see errata).

I tried coding a few different things across cortexa.c and most of them ended up here. Namely, the OSLock check, the HALT_DBG_ENABLE + ITR_ENABLE typo fix, the unused LAR/DSCCR/DSMCR clears, macro definitons for these, and last but not least, my helper routine for printing human-readable names for set bits of a register to save developers from the hassle of using a decoder ring keeping A.R.M. and calculator open for understanding some hex dumps.

Note that because the problem happens in cortexa_probe, earlier than cortexa_attach, I had to move HDBGen+ITRen writes up there. This is less clean because the user is not guaranteed to attach (so BMD then runs detach cleanup), but I submit just the bug fix for now.

This patchset as it stands was successfully tested in operation by me against STM32MP157D-DK1 with its onboard STLink/V2-1 SWD-only, but also against custom hardware connected to {STLink/V2 standalone; JLink V8; CMSIS-DAP v1/v2 free-dap on rp2040; finally a BMP(-compatible) blackpill-f411ce}. On all debuggers the hang is not seen anymore, in GDB attach and run/continue works, proper general register values are shown. On BMP there's no hang anymore in *_scan which required pressing the probe's reset button.

I understand this patchset may be not as clean and subject to renames and nitpicks, but a) I needed to fix in VCS a known good working solution; b) this problem blocks my PR1546, in which CM4 target support code would not be reachable anymore. I may agree to rebase away all changes except OSLock which is the core change, and/or reformat the DSCR-touching stuff.

Users of Cortex-A targets are welcome to test this patchset on their setups in case I introduce a regression for them by fixing the regression I face.

Your checklist for this pull request

  • I've read the Code of Conduct
  • I've read the guidelines for contributing to this repository
  • It builds for hardware native (make PROBE_HOST=native)
  • It builds as BMDA (make PROBE_HOST=hosted)
  • I've tested it to the best of my ability
  • My commit messages provide a useful short description of what the commits do

Closing issues

Not quite fixes but helps #1546.

Example output:

cortexa_attach: DBGOSLSR = 0x00000008
cortexa_attach: DBGDSCR = 0x02000002 (1)
Bits set in reg DBGDSCR: RESTARTED PipeAdv
cortexa_attach: DBGDSCR = 0x02006002 (2)
Bits set in reg DBGDSCR: RESTARTED ITRen HDBGen PipeAdv
cortexa_halt_poll: DBGDSCR = 0x03086003
* This fixes the BMD hang on scanning STM32MP15x SoCs
…robe()

* Unblock the target_halt in cortexa_probe
* Move code setting up debug access from cortexa_detach() up to cortexa_probe()
* Scans of STM32MP15x still work, gdb shows $pc and registers
This reverts commit fc93f6fa0df5d618c4c2027f52a003cba86ac386.
@dragonmux dragonmux added this to the v1.10 milestone Sep 23, 2023
@dragonmux dragonmux added Bug Confirmed bug HwIssue Mitigation Solving or mitigating a Hardware issue in Software New Target New debug target labels Sep 23, 2023
Copy link
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

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

There's a couple of items to address, but this is looking good. Nice work figuring out how to fix what was going so wrong!

src/target/cortexa.c Outdated Show resolved Hide resolved
src/target/cortexa.c Outdated Show resolved Hide resolved
src/target/cortexa.c Outdated Show resolved Hide resolved
src/target/cortexa.c Show resolved Hide resolved
src/target/cortexa.c Outdated Show resolved Hide resolved
* Drop extraneous  parentheses
* Hide the helper behind ENABLE_DEBUG and a dummy implementation
* Replace one introduced %08X with %08PRIx32 for *firmware* builds
Copy link
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

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

LGTM at least for v1.10 - as said in one of the review threads, we can refine this further as part of bringing the Cortex-A and Cortex-R support together in v2.0, so "good enough that it works" is fine for now. Merging!

@dragonmux dragonmux merged commit ead1657 into blackmagic-debug:main Sep 23, 2023
6 checks passed
@ALTracer ALTracer deleted the fix/cortexa-oslock branch November 22, 2023 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Confirmed bug HwIssue Mitigation Solving or mitigating a Hardware issue in Software New Target New debug target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants