Inconsistent jtagtap_cycle() implementations #2180

ALTracer commented 2026年01月28日 18:38:39 +01:00 (Migrated from github.com)

A method to generate N cycles of TCK with fixed values of TMS and TDI, jtagtap_cycles(), was added somewhat later than the other base methods to jtag_proc structure of function pointers. I have identified four places where this is used.

1. jtagtap_init() and equivalents, for emitting SWD LINERESET (50 cycles with TMS high), sometimes coded as a literal for-loop around jtagtap_next() or a prepared byte buffer.
   github.com/blackmagic-debug/blackmagic@54ddc3505c/src/platforms/common/jtagtap.c (L54-L59)
2. jtag_scan.c:jtag_sanity_check(), called as early as jtag_scan() => jtag_discover() => jtag_sanity_check(). I have touched this recently in #2178.github.com/blackmagic-debug/blackmagic@54ddc3505c/src/target/jtag_scan.c (L306-L307)
3. adiv5_jtag.c:adiv5_jtag_raw_access(), for 8 idle cycles but only for ADI v6 JTAG-DP targets. Never encountered with ADI v5.github.com/blackmagic-debug/blackmagic@54ddc3505c/src/target/adiv5_jtag.c (L166-L170)
4. icepick.c:icepick_router_handler(), for 10 idle cycles after reconfiguring type-D controller. Very target-specific.
   github.com/blackmagic-debug/blackmagic@54ddc3505c/src/target/icepick.c (L135-L140)

None of these places are particularly hot code (intensively called during debug). The problem is that BMDA backends for FTDI MPSSE, J-Link, CMSIS-DAP never set the pointer, so it stays NULL (in a calloc()'ed struct), allowing one to inadvertently segfault BMDA mid-scan.

The obvious solutions are a) null-check the pointer at all callsites and fall back to jtagtap_next(); b) code the wrappers which invoke corresponding methods bound to jtagtap_next(). Which can then evolve into proper functions optimized for bit-packing and buffering characteristic for the protocols of these low-level adapters.

I've started at it in #2179 just to fix it until other usable or optimized implementations arrive. The CMSIS-DAP protocol is too convoluted for me to do it properly. J-Link HW_JTAG3 and FTDI MPSSE are easier.

A method to generate N cycles of TCK with fixed values of TMS and TDI, `jtagtap_cycles()`, was added somewhat later than the other base methods to `jtag_proc` structure of function pointers. I have identified four places where this is used. 1. `jtagtap_init()` and equivalents, for emitting SWD LINERESET (50 cycles with TMS high), sometimes coded as a literal for-loop around `jtagtap_next()` or a prepared byte buffer. https://github.com/blackmagic-debug/blackmagic/blob/54ddc3505c5ac3baaa0253ebc08a75444eabf976/src/platforms/common/jtagtap.c#L54-L59 2. jtag_scan.c:`jtag_sanity_check()`, called as early as `jtag_scan()` => `jtag_discover()` => `jtag_sanity_check()`. I have touched this recently in #2178.https://github.com/blackmagic-debug/blackmagic/blob/54ddc3505c5ac3baaa0253ebc08a75444eabf976/src/target/jtag_scan.c#L306-L307 3. adiv5_jtag.c:`adiv5_jtag_raw_access()`, for 8 idle cycles but only for ADI v6 JTAG-DP targets. Never encountered with ADI v5.https://github.com/blackmagic-debug/blackmagic/blob/54ddc3505c5ac3baaa0253ebc08a75444eabf976/src/target/adiv5_jtag.c#L166-L170 4. icepick.c:`icepick_router_handler()`, for 10 idle cycles after reconfiguring type-D controller. Very target-specific. https://github.com/blackmagic-debug/blackmagic/blob/54ddc3505c5ac3baaa0253ebc08a75444eabf976/src/target/icepick.c#L135-L140 None of these places are particularly hot code (intensively called during debug). The problem is that BMDA backends for FTDI MPSSE, J-Link, CMSIS-DAP never set the pointer, so it stays NULL (in a `calloc()`'ed struct), allowing one to inadvertently segfault BMDA mid-scan. The obvious solutions are a) null-check the pointer at all callsites and fall back to `jtagtap_next()`; b) code the wrappers which invoke corresponding methods bound to `jtagtap_next()`. Which can then evolve into proper functions optimized for bit-packing and buffering characteristic for the protocols of these low-level adapters. I've started at it in #2179 just to fix it until other usable or optimized implementations arrive. The CMSIS-DAP protocol is too convoluted for me to do it properly. J-Link HW_JTAG3 and FTDI MPSSE are easier.

dragonmux commented 2026年01月29日 15:05:19 +01:00

Our preference, and what has now been done in #2179 is to provide polyfills over the backend's jtagtap_next() impl to provide the cycles function, and then when a more proper way to implement it is identified for a given backend, replace the polyfill with that specific way, such as building a buffer for FTDI that has the MPSSE run the correct number of cycles in one command with TDI and TMS held in the requested state for all of those cycles.

Because of that PR, we're going to consider this fixed by that now all backends have an implementation.

Our preference, and what has now been done in #2179 is to provide polyfills over the backend's jtagtap_next() impl to provide the cycles function, and then when a more proper way to implement it is identified for a given backend, replace the polyfill with that specific way, such as building a buffer for FTDI that has the MPSSE run the correct number of cycles in one command with TDI and TMS held in the requested state for all of those cycles. Because of that PR, we're going to consider this fixed by that now all backends have an implementation.