search

compuphase / Black-Magic-Probe-Book

A guide plus associated utilities for the Black Magic Probe.
Apache License 2.0
147 stars 26 forks source link

BMTrace: "setup failed" with BMP v1.9.0 and newer #38

Closed ALTracer closed 3 months ago

ALTracer commented 11 months ago

Summary

Using either binaries for Windows from 2023-03 (1.4.6918), or building bmdebug suite myself in 2023-11 (1.2.6986), I get the error on trying to use bmtrace with probes running recent firmware. https://github.com/compuphase/Black-Magic-Probe-Book/blob/531cf0488aa6cdebd7bff78d4939378d393670db/source/bmp-support.c#L1324

Description

Since PR1137 https://github.com/blackmagic-debug/blackmagic/pull/1137 the upstream has changed format of remote monitor responses for traceswo command. Existing code in bmp-support.c does not expect this and returns early, failing to detect the Trace USB endpoint number.

Initially I wanted to use the Orbuculum suite to test my experimental changes of reshuffling EP numbers for blackpill-f411ce platform, but it FTBFS on Windows 10 under MSYS2 UCRT64 (gcc throws -Werror=format etc. from libdwarf dependency code). So I remembered the other, closer to BMP ecosystem, software stack and tried the binaries with no result. Cloning and building that yielded a different error message, so I opened the logs & sources and started hacking.

Because my change remaps TRACE_ENDPOINT from 0x85 to 0x83 (anything above EP 3 does not exist in f411 dwc2 usbotg IP, and it just does not respond to IN tokens at all), I had to change the default definition both in orbtraceIf.c and in bmp-scan.h; but I noticed some code which tries to parse a live GDB RSP BMP response in an older format. Unfortunately, I also noticed that current BMP answers with a hardcoded "...EP 5\n" string (PR pending).

A related issue is that after my patching, bmtrace on Windows 10 fails harder with a yet another error message I traced down to https://github.com/compuphase/Black-Magic-Probe-Book/blob/531cf0488aa6cdebd7bff78d4939378d393670db/source/bmtrace.c#L1049 with loc_errno=8 from https://github.com/compuphase/Black-Magic-Probe-Book/blob/531cf0488aa6cdebd7bff78d4939378d393670db/source/swotrace.c#L877 ; if this is best posted separately, then I'll stop discussing it and wait for you to fix it on your machine instead. Only blackpill-f411ce does this; swlink firmware on a genuine stm32f103cbt6 bluepill works and bmtrace opens the trace port. Maybe it's the unreachable endpoints, maybe it's Windows Registry junk. You seem to walk the registry to get a DeviceInterfaceGUIDs and that key is missing from the relevant Interface (not Endpoint). I did not change interfaces in firmware.

I will post some patch hunks, not expecting to get them accepted into any sort of PR whatsoever, and gdb-rsp.log excerpts I found after launching bmtrace.exe with f411 and swlink respectively. If you don't support upstream changes which break your code as the consumer of unspoken API, then we could at least negotiate with upstream on what format to print the messages in BMP firmware. For example, I don't quite like consecutive 32 o0 packets either. If your desktop software only supports adapter firmware built from your own BMD fork, then that should be stated in program help and maybe in the book. Just FYI, both BMDebug and BMSerial work fine with v1.10.0 recent release, as far as I briefly tested.

ALTracer commented 11 months ago
gdbrsp.log after patching bmp-support.c when talking to an `swlink` ``` << - << - >> qRcmd,version << oBlack Magic Probe (SWLINK) v1.10.0-232-ga06e9e5c-dirty << o, Hardware Version 1\x0a << oCopyright (C) 2010-2023 Black Magic Debug Project\x0a << oLicense GPLv3+: GNU GPL version 3 or later \x0a\x0a << OK << - >> qSupported:multiprocess+ << PacketSize=400;qXfer:memory-map:read+;qXfer:features:read+;vContSupported+ >> ! << OK >> qRcmd,help << oGeneral commands:\x0a << o\x09version -- Display firmware version info\x0a << o\x09help -- Display help for monitor commands\x0a << o\x09jtag_scan -- Scan JTAG chain for devices\x0a << o\x09swd_scan -- Scan SWD interface for devices: [TARGET_ID]\x0a << o\x09swdp_scan -- Deprecated: use swd_scan instead\x0a << o\x09auto_scan -- Automatically scan all chain types for devices\x0a << o\x09frequency -- set minimum high and low times: [FREQ]\x0a << o\x09targets -- Display list of available targets\x0a << o\x09morse -- Display morse error message\x0a << o\x09halt_timeout -- Timeout to wait until Cortex-M is halted: [TIMEOUT, default 2000ms]\x0a << o\x09connect_rst -- Configure connect under reset: [enable|disable]\x0a << o\x09reset -- Pulse the nRST line - disconnects target: [PULSE_LEN, default 0ms]\x0a << o\x09tdi_low_reset -- Pulse nRST with TDI set low to attempt to wake certain targets up (eg LPC82x)\x0a << o\x09traceswo -- Start trace capture, NRZ mode: [BAUDRATE] [decode [CHANNEL_NR ...]]\x0a << o\x09heapinfo -- Set semihosting heapinfo: HEAP_BASE HEAP_LIMIT STACK_BASE STACK_LIMIT\x0a << o\x09debug_bmp -- Output BMP "debug" strings to the second vcom: [enable|disable]\x0a << OK >> qRcmd,connect_rst disable << oAssert nRST during connect: disabled\x0a << OK >> qRcmd,swdp_scan << oAvailable Targets:\x0a << oNo. Att Driver\x0a << o 1 AT32F403A/407 M4\x0a << OK >> vAttach;1 << T05thread:1; >> qXfer:memory-map:read::0,3fc << m0x800 >> qRcmd,help << oGeneral commands:\x0a << o\x09version -- Display firmware version info\x0a << o\x09help -- Display help for monitor commands\x0a << o\x09jtag_scan -- Scan JTAG chain for devices\x0a << o\x09swd_scan -- Scan SWD interface for devices: [TARGET_ID]\x0a << o\x09swdp_scan -- Deprecated: use swd_scan instead\x0a << o\x09auto_scan -- Automatically scan all chain types for devices\x0a << o\x09frequency -- set minimum high and low times: [FREQ]\x0a << o\x09targets -- Display list of available targets\x0a << o\x09morse -- Display morse error message\x0a << o\x09halt_timeout -- Timeout to wait until Cortex-M is halted: [TIMEOUT, default 2000ms]\x0a << o\x09connect_rst -- Configure connect under reset: [enable|disable]\x0a << o\x09reset -- Pulse the nRST line - disconnects target: [PULSE_LEN, default 0ms]\x0a << o\x09tdi_low_reset -- Pulse nRST with TDI set low to attempt to wake certain targets up (eg LPC82x)\x0a << o\x09traceswo -- Start trace capture, NRZ mode: [BAUDRATE] [decode [CHANNEL_NR ...]]\x0a << o\x09heapinfo -- Set semihosting heapinfo: HEAP_BASE HEAP_LIMIT STACK_BASE STACK_LIMIT\x0a << o\x09debug_bmp -- Output BMP "debug" strings to the second vcom: [enable|disable]\x0a << oTarget specific commands:\x0a << o\x09erase_mass -- Erase whole device Flash\x0a << o\x09erase_range -- Erase a range of memory on a device\x0a << oARM Cortex-M specific commands:\x0a << o\x09vector_catch -- Catch exception vectors\x0a << o\x09redirect_stdout -- Redirect semihosting stdout to USB UART\x0a << OK >> qRcmd,version << oBlack Magic Probe (SWLINK) v1.10.0-232-ga06e9e5c-dirty << o, Hardware Version 1\x0a << oCopyright (C) 2010-2023 Black Magic Debug Project\x0a << oLicense GPLv3+: GNU GPL version 3 or later \x0a\x0a << OK >> qRcmd,traceswo 100000 << oBaudrate: 100000 << oChannel mask: << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o\x0a << oTrace enabled for BMP serial 766768CC, USB EP 5\x0a >> vRun; << T05 >> c ```

(it still uses traceswoasync.c which you touched in at least PR555)

ALTracer commented 11 months ago
gdbrsp.log after patching bmp-support.c when talking to a `blackpill-f411ce` ``` << - << - >> qRcmd,version << oBlack Magic Probe (BlackPill-F411CE) v1.10.0-207-g34e7a8fc-dirty << o, Hardware Version 0\x0a << oCopyright (C) 2010-2023 Black Magic Debug Project\x0a << oLicense GPLv3+: GNU GPL version 3 or later \x0a\x0a << OK << - >> qSupported:multiprocess+ << PacketSize=400;qXfer:memory-map:read+;qXfer:features:read+;vContSupported+ >> ! << OK >> qRcmd,help << oGeneral commands:\x0a << o\x09version -- Display firmware version info\x0a << o\x09help -- Display help for monitor commands\x0a << o\x09jtag_scan -- Scan JTAG chain for devices\x0a << o\x09swd_scan -- Scan SWD interface for devices: [TARGET_ID]\x0a << o\x09swdp_scan -- Deprecated: use swd_scan instead\x0a << o\x09auto_scan -- Automatically scan all chain types for devices\x0a << o\x09frequency -- set minimum high and low times: [FREQ]\x0a << o\x09targets -- Display list of available targets\x0a << o\x09morse -- Display morse error message\x0a << o\x09halt_timeout -- Timeout to wait until Cortex-M is halted: [TIMEOUT, default 2000ms]\x0a << o\x09connect_rst -- Configure connect under reset: [enable|disable]\x0a << o\x09reset -- Pulse the nRST line - disconnects target: [PULSE_LEN, default 0ms]\x0a << o\x09tdi_low_reset -- Pulse nRST with TDI set low to attempt to wake certain targets up (eg LPC82x)\x0a << o\x09rtt -- [enable|disable|status|channel [0..15 ...]|ident [STR]|cblock|ram [RAM_START RAM_END]|poll [MAXMS MINMS MAXERR]]\x0a << o\x09traceswo -- Start trace capture, Manchester mode: [decode [CHANNEL_NR ...]]\x0a << o\x09heapinfo -- Set semihosting heapinfo: HEAPINFO HEAP_BASE HEAP_LIMIT STACK_BASE STACK_LIMIT\x0a << OK >> qRcmd,connect_rst disable << oAssert nRST during connect: disabled\x0a << OK >> qRcmd,swdp_scan << oAvailable Targets:\x0a << oNo. Att Driver\x0a << o 1 GD32E5 M33\x0a << OK >> vAttach;1 << T05thread:1; >> qXfer:memory-map:read::0,3fc << m0x2000 >> qRcmd,help << oGeneral commands:\x0a << o\x09version -- Display firmware version info\x0a << o\x09help -- Display help for monitor commands\x0a << o\x09jtag_scan -- Scan JTAG chain for devices\x0a << o\x09swd_scan -- Scan SWD interface for devices: [TARGET_ID]\x0a << o\x09swdp_scan -- Deprecated: use swd_scan instead\x0a << o\x09auto_scan -- Automatically scan all chain types for devices\x0a << o\x09frequency -- set minimum high and low times: [FREQ]\x0a << o\x09targets -- Display list of available targets\x0a << o\x09morse -- Display morse error message\x0a << o\x09halt_timeout -- Timeout to wait until Cortex-M is halted: [TIMEOUT, default 2000ms]\x0a << o\x09connect_rst -- Configure connect under reset: [enable|disable]\x0a << o\x09reset -- Pulse the nRST line - disconnects target: [PULSE_LEN, default 0ms]\x0a << o\x09tdi_low_reset -- Pulse nRST with TDI set low to attempt to wake certain targets up (eg LPC82x)\x0a << o\x09rtt -- [enable|disable|status|channel [0..15 ...]|ident [STR]|cblock|ram [RAM_START RAM_END]|poll [MAXMS MINMS MAXERR]]\x0a << o\x09traceswo -- Start trace capture, Manchester mode: [decode [CHANNEL_NR ...]]\x0a << o\x09heapinfo -- Set semihosting heapinfo: HEAPINFO HEAP_BASE HEAP_LIMIT STACK_BASE STACK_LIMIT\x0a << oTarget specific commands:\x0a << o\x09erase_mass -- Erase whole device Flash\x0a << o\x09erase_range -- Erase a range of memory on a device\x0a << oARM Cortex-M specific commands:\x0a << o\x09vector_catch -- Catch exception vectors\x0a << o\x09redirect_stdout -- Redirect semihosting stdout to USB UART\x0a << oGD32E5 specific commands:\x0a << o\x09option -- Manipulate option bytes\x0a << OK >> qRcmd,version << oBlack Magic Probe (BlackPill-F411CE) v1.10.0-207-g34e7a8fc-dirty << o, Hardware Version 0\x0a << oCopyright (C) 2010-2023 Black Magic Debug Project\x0a << oLicense GPLv3+: GNU GPL version 3 or later \x0a\x0a << OK >> qRcmd,traceswo << oChannel mask: << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o0 << o\x0a << oTrace enabled for BMP serial 67918462, USB EP 5\x0a >> vRun; << T05 >> c ```
ALTracer commented 11 months ago
Working copy changes, not all of them ended up being important, but one hunk implements consuming the new remote responses for `qRcmd,traceswo` ```diff diff --git a/source/bmp-scan.c b/source/bmp-scan.c index 2f6af79..f33d1d0 100644 --- a/source/bmp-scan.c +++ b/source/bmp-scan.c @@ -260,7 +260,7 @@ int find_bmp(int seqnr, int iface, TCHAR *name, size_t namelen) ptr = portname; } else { /* read GUID */ - LSTATUS stat = RegQueryValueEx(hkeyItem, _T("DeviceInterfaceGUIDs"), NULL, NULL, (LPBYTE)portname, &maxlen); + LSTATUS stat = RegQueryValueEx(hkeyItem, _T("DeviceInterfaceGUID"), NULL, NULL, (LPBYTE)portname, &maxlen); /* ERROR_MORE_DATA is returned because there may technically be more GUIDs assigned to the device; we only care about the first one ERROR_FILE_NOT_FOUND is returned when the key is not found, which may diff --git a/source/bmp-scan.h b/source/bmp-scan.h index 94ff31d..7c8baa2 100644 --- a/source/bmp-scan.h +++ b/source/bmp-scan.h @@ -32,7 +32,7 @@ #define BMP_IF_UART 2 /* interface 2 -> 3.3V TTL UART */ #define BMP_IF_DFU 4 #define BMP_IF_TRACE 5 -#define BMP_EP_TRACE 0x85 /* endpoint 5 is bulk data endpoint for trace interface */ +#define BMP_EP_TRACE 0x83 /* endpoint 5 is bulk data endpoint for trace interface */ #define BMP_IF_SERIAL 9 /* pseudo-interface for getting the serial number */ diff --git a/source/bmp-support.c b/source/bmp-support.c index 362375f..dfa4585 100644 --- a/source/bmp-support.c +++ b/source/bmp-support.c @@ -158,7 +158,7 @@ static bool testreply(const char *reply, size_t reply_length, const char *match) if (reply_length == match_length + 1 && reply[reply_length - 1] == '\0') reply_length -= 1; - return (reply_length == match_length && memcmp(reply, match, reply_length) == 0); + return (reply_length >= match_length && memcmp(reply, match, match_length) == 0); } /** bmp_connect() scans for the USB port of the Black Magic Probe and connects @@ -231,7 +231,7 @@ bool bmp_connect(int probe, const char *ipaddress) gdbrsp_xmit("qRcmd,version", -1); do { size = gdbrsp_recv(buffer, sizearray(buffer)-1, 250); - } while (size > 0 && size < 2); + } while (size > 4); if (!testreply(buffer, size, "OK")) { notice(BMPERR_NORESPONSE, "No response on %s", devname); hCom = rs232_close(hCom); @@ -1278,6 +1278,13 @@ static bool bmp_parsetracereply(const char *reply, unsigned char *endpoint) ptr = strstr(reply, "USB EP"); if (ptr != NULL) { long ep = strtol(ptr + 6, NULL, 16); +#if 0 + char buf[80]; + snprintf(buf, 80, "Swapping endpoint number %ld", ep); + notice(BMPERR_MONITORCMD, buf); + if (endpoint != NULL && *endpoint == 5) + *endpoint = 3; +#endif if (endpoint != NULL) *endpoint = (unsigned char)(ep | 0x80); /* direction flag is not set in the reply */ ok = true; @@ -1319,6 +1326,29 @@ bool bmp_enabletrace(int async_bitrate, unsigned char *endpoint) interface for trace capture (0x05) and the endpoint (0x85, on the original Black Magic Probe) */ buffer[rcvd] = '\0'; + bool tryagain = false; + /* Probes implementing Async will post their baudrate first */ + if (strncmp(buffer, "oBaudrate:", 10) == 0) { + tryagain = true; + rcvd = gdbrsp_recv(buffer, sizearray(buffer), 1000); + } + /* + * Probes implementing Manchester will post their channel decode mask first + * `Channel mask: 00000000000000000000000000000000` + */ + if (strncmp(buffer, "oChannel mask:", 14) == 0) { + tryagain = false; + /* Drain the bits of mask coming in separate putchar-like packets */ + do { + rcvd = gdbrsp_recv(buffer, sizearray(buffer), 1000); + } while (rcvd > 0 && rcvd < 3); + buffer[rcvd] = '\0'; + } + /* Get another packet which should contain the actual EP number */ + if (tryagain) { + rcvd = gdbrsp_recv(buffer, sizearray(buffer), 1000); + buffer[rcvd] = '\0'; + } bool ok = (buffer[0] == 'o') && bmp_parsetracereply(buffer + 1, endpoint); if (!ok) notice(BMPERR_MONITORCMD, "Trace setup failed"); diff --git a/source/bmtrace.c b/source/bmtrace.c index d3d05ed..61b5ef2 100644 --- a/source/bmtrace.c +++ b/source/bmtrace.c @@ -1047,10 +1047,17 @@ static void handle_stateaction(APPSTATE *state) case TRACESTAT_NO_INTERFACE: case TRACESTAT_NO_DEVPATH: case TRACESTAT_NO_PIPE: - strlcpy(msg, "Trace interface not available", sizearray(msg)); + { +// strlcpy(msg, "Trace interface not available", sizearray(msg)); + snprintf(msg, sizearray(msg), "Trace interface not available (%d)", state->trace_status); if (state->probe == state->netprobe && state->swomode != MODE_ASYNC) strlcat(msg, "; try NRZ/Async mode", sizearray(msg)); tracelog_statusmsg(TRACESTATMSG_BMP, msg, BMPERR_GENERAL); + int loc; + unsigned long error = trace_errno(&loc); + sprintf(msg, "Trace setup problem (error %d:%lu)", loc, error); + tracelog_statusmsg(TRACESTATMSG_BMP, msg, BMPERR_GENERAL); + } break; case TRACESTAT_NO_ACCESS: { int loc; ```

Actually any probes implementing either Manchester or UART will post the decode mask, this was introduced in https://github.com/blackmagic-debug/blackmagic/pull/664 , so before v1.7.0; now that I think about it, I often build firmware with ENABLE_DEBUG=1 to diagnose issues within it, and I successfully fixed a couple of them that way. You can see a debug_en in logs in mon help response. But in current BMD codebase these lines seem to not be gated on ENABLE_DEBUG anymore, and I don't know how to git annotate for that without total git grep.

I don't have screenshots, but I got a "Trace setup problem (error 8:0)" triggering in my message. Drivers may or may not have been installed, I use a slightly patched upstream blackmagic.inf with Rev_0109 to correspond with new descriptors (which pack MS BOS anyway). I could unplug tear down everything hidden/unplugged and Zadig them again, if needed; but the error message didn't lead me to think about it then.

ALTracer commented 3 months ago

Thanks. I'll consider this closed (and reopen in case I'll have a proper bugreport with logs etc.) There is a misphrasing in code comments from my side, but it doesn't affect operation.

Current state of things (v1.10 and 2024 main): BMP native and BMP-compatibles support Manchester XOR UART, not both. Manchester doesn't need baudrate (and doesn't parse it / doesn't consume extraneous parameters), UART needs it (or uses #define SWO_DEFAULT_BAUD 2250000). All BMP-compatible platforms have ITM decode capability, and will indicate which of 32 channels they'll parse and print over ttyBmpTarg -- this is not coupled to capture engine flavour (and present even in your v1.7-fork).

In the near future it might be refactored to parse parameters differently, and to include both engines, but no ETAs on that.