Fix: BMDA ST-Link v3 crashes

[dragonmux]

Detailed description

In this PR we partially address a series of crashes and error reporting issues discovered while prodding ST-Link v3's with BMDA. There is still an issue left to figure out regarding multi-drop SWD devices which results in a fail-safe crash against multi-drop parts, detailed after the main description.

With this we have sought to improve the error reporting done by the backend, improve correctness and reliability, and sort out its JTAG support which was completely broken (complete with segfaults). These changes also better harmonise the backend with the rest of the naming conventions being established in BMDA and the Black Magic Debug project in general.

Most notably, these changes include the removal of the ap_setup() and ap_cleanup() callbacks from the ADIv5 control structures by moving this logic properly into the backend where it belongs, simplifying logic in adiv5.c and enabling proper and reliable support for multi-AP parts.

Finally, the remaining crash that is not addressed manifests against multi-drop SWD parts as:

***  1   STM32H5 M33
Unhandled exception: ST-Link error
[1]    1801873 IOT instruction (core dumped)  src/blackmagic -s 0018003A544B -tv 1

which when expanded on, is caused by this interaction:

***  1   STM32H5 M33
stlink_adiv5_clear_error (protocol recovery: false)
 request: f2 32 00 00 00 00 00 00 00 00 00 00 00 00 00 00
response: 80 00
Write TARGETSEL: 0x04740041
stlink_raw_access: Attempting access to addr 000c
 request: f2 46 ff ff 0c 00 41 00 74 04 00 00 00 00 00 00
response: 16 00
Unhandled exception: ST-Link error

This is because in the current implementation, TARGETSEL writes (and therefore multi-drop support) are broken due to invoking the wrong packet kind on the ST-Link v3. This doesn't show up on a v2, only v3. In part this is because we do not correctly implement line reset right now, or know of any specific method ST has provisioned for switching which DP we're talking to. This will be addressed in a follow-up PR at a later date once the necessary research has been completed.

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

[esden]

LGTM

[UweBonnes]

Use the ST specific AP setup

+++ b/src/platforms/hosted/stlinkv2.c
@@ -514,7 +514,7 @@ uint32_t stlink_adiv5_clear_error(adiv5_debug_port_s *const dp, const bool proto
                 */
                stlink_reset_adaptor();
                if (dp->version >= 2)
-                       adiv5_dp_write(dp, ADIV5_DP_TARGETSEL, dp->targetsel);
+                       stlink_ap_setup(dp->targetsel);
                adiv5_dp_read(dp, ADIV5_DP_DPIDR);
        }
        const uint32_t err = adiv5_dp_read(dp, ADIV5_DP_CTRLSTAT) &

[dragonmux]

That doesn't help here - TARGETSEL is about selecting which DP we're talking to, not the AP.. unless you're telling us that STLINK_DEBUG_APIV2_INIT_AP is now overloaded for this purpose somehow. Not that this is documented anywhere that we've found.

Also note that we've been unable, other than by exiting and re-entering debug mode, to generate the requisite line reset event needed to precede this for it to function correctly. How too would this interact with older ST-Link firmware?

[ALTracer]

Use the ST specific AP setup
stlink_ap_setup(uint8_t ap)

@UweBonnes I tried your change locally, and with a TARGETID 0x04800041 STM32H7B0 this line attempts to open AP 65 (lsbyte 0x41), fails that, and moves on. While this is wrong, this also skips the exception-triggering write and hence allows working with single DPv2 devices.
Please open an issue dedicated to STLink/V2,V3 vs SWD Multidrop, especially if you have some working solution (or a direct line to ST FAEs), because I am facing problems with H7B0 and MP157 over that backend driver, too.

@dragonmux Please copy/move the "remaining crash that is not addressed" detailed description of this PR1605 there as well. Even if it's actually last commit of PR1464 that breaks stuff for me. I failed to port over adiv5_swd.c->stlink_swd.c properly enough without ST-internal docs. Reverting said commit allows working with singly attached DPv2 devices, and we're not planning on stacking multiple MP15x chips (or H7, or others).

[UweBonnes]

As mentioned in the code, STLink only can access AP0..8. With

@@ -649,8 +648,8 @@ static bool stlink_ap_setup(const uint8_t ap)
 {
        uint8_t data[2];
        DEBUG_PROBE("%s: AP %u\n", __func__, ap);
-       if (ap > 7) {
-               DEBUG_WARN("Reject ap_setup %d as Stlink V3 can not handle more than 8 APs\n", ap);
+       if (ap > 8) {
+               DEBUG_WARN("Reject ap_setup %d as Stlink can only handle AP 0..8\n", ap);
                return false;
        }
        stlink_simple_request(STLINK_DEBUG_COMMAND, STLINK_DEBUG_APIV2_INIT_AP, ap, data, sizeof(data));

the failing code is not longer executed

[dragonmux]

Right, and this part of BMD has absolutely nothing to do with that.. this code is making a DP selection on a DPv2+ SWD chain, after (trying to) do a line reset.. passing a TARGETSEL value to the ST-Link AP machinery is demonstrably wrong and only hides a fundamental flaw with more bad code that does the wrong thing but silently. The AP setup code is an entirely different mechanism once a DP has been selected on the chain.. BMD will still try and attempt to select through 8 non-existent DPs for DPv2+ devices and translating those into AP selections is outright wrong.