Fix: libusb overflow with JLink #1673
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Log from latest main: Log with this patchset: This is Nucleo-G071RB. Hosted RTT performs at only 6700 chars/s. |
|
Verbose log from latest main. Each timeout error takes 5 seconds. Could also crank LIBUSB_DEBUG to 4. |
ALTracer
force-pushed
the
jlink-overflow
branch
from
ee8cb42
to
f1a4fc4
Compare
dragonmux
added
Bug
perigoso
suggested changes
Nov 6, 2023
ALTracer
force-pushed
the
jlink-overflow
branch
2 times, most recently
from
bbc6e72
to
4ee6908
Compare
|
Thank you very much @perigoso for coming up and reviewing this. Applied the review items and rebased on main. |
ALTracer
force-pushed
the
jlink-overflow
branch
from
4ee6908
to
fe848f2
Compare
dragonmux
reviewed
Nov 7, 2023
There was a problem hiding this comment.
There are two small items from us, see the review below, and then we're happy to merge this.
Tested against our LPC Link-2 running the J-Link "OB" firmware, and it works great.
❯ src/blackmagic -s 000611000000 -tjv 5
Black Magic Debug App v1.10.0-186-gfe848f23
for Black Magic Probe, ST-Link v2 and v3, CMSIS-DAP, J-Link and FTDI (MPSSE)
Using 1366:0101 000611000000 SEGGER
J-Link ---
Extended capabilities: 0xb9ff7bbf 0x00003c0d 0x00000000 0x00000000
Firmware version: J-Link LPC-Link 2 compiled May 31 2013 17:31:46
Copyright 2003-2013 SEGGER: www.segger.com
Hardware version: LPC-Link2 v1.0.0
Available interfaces:
0: JTAG*
1: SWD
Running in Test Mode
Target voltage: 3.300V Volt
JTAG interface frequency:
Base frequency: 48000000Hz
Minimum divisor: 8
Speed set to 4.000MHz for JTAG
* Try to print extended capabilities. If the adapter doesn't advertise support for them, then just print standard capabilities, like it was before.
…l byte * According to Wireshark usbmon dumps, the firmware returns 112 bytes of version, including some copyright information after an effectively 0-terminator. * Use strchr() instead of index() for Windows reasons. * Make sure the version buffer contains at least one NULL * And avoid replacing the only NULL with LF, losing null-termination in the process
ALTracer
force-pushed
the
jlink-overflow
branch
from
fe848f2
to
c5db5f8
Compare
* Asking for 1 byte less raises a libusb OVERFLOW error when using JLink V8 and newer with BMDA on Linux hosts after the proprietary software stack touches the adapter (but not before). * libusb docs recommend receiving into a bigger buffer (1028=512*2+4) and checking whether all of expected data got indeed received. * For firmwares which send that transaction-error byte in a separate packet, keep the second read call (V5 ones do this regardless).
ALTracer
force-pushed
the
jlink-overflow
branch
from
c5db5f8
to
fab71d6
Compare
dragonmux
approved these changes
Nov 11, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Detailed description
RM08001 states that the IO TRANSACTION (JTAG3) command should return TDO (or SWDIO) data according to requested nBytes length, like JTAG2, and one more byte for OK status from adapter (or nonzero error). Requesting that in two libusb calls may not work on [my] setup and render BMDA inoperable with this adapter type.
Reading some libusb docs I figured that we should optimistically request a read for length of entire possible packet, then check whether we got handed that 1 byte less (apparently some firmwares give it in a separate bulk packet), and only then do a second read for it. The 1028-byte stack buffer of hosted (4+2*512 for header, tms & tdi) allows for receiving (512+1), but a dedicated smaller buffer could also suffice.
I consider this an intermittent bug because I cannot reproduce it after rebooting the machine. Detected against a STLinkReflashed adapter after a week of uptime on an Intel Pentium N6000 host, that is with Jasper Lake PCH USB3 HCI.Once you let J-Link Commander software interact with their adapter firmware, it changes the responses' framing. I suppose anything else using
libjlinkarm.soalso will, like RTTViewer and GDBServer. I reproduced the bug on two different machines running Linux, and on three different adapters (V8 from 2015 and Reflash from 2017). V5 from 2008 are not affected and always split the status byte. I could NOT reproduce the bug on Windows 10, and it required swapping drivers between proprietary and WinUSB a lot. Still,I believe fixing BMDA for developers toggling between JLink gdbserver and BMDA on Linux machines (this is possible in Eclipse CDT and derived IDEs) is useful, and might even reduce the number of libusb calls. I tried V7.88k, V6.98, V6.34, V5.12h.
Below that change I also tacked two other commits. One avoids discarding the copyright string returned by adapter in firmware version behind a nul byte (which is treated like a terminator by printf), another queries any extended capabilities of adapters, when supported. These are not strictly required to fix the problem I faced.
Your checklist for this pull request
make PROBE_HOST=native) -- not applicablemake PROBE_HOST=hosted)Closing issues
None reported yet.