Fix: ARM JTAG regressions

src/hex_utils.c

@@ -3,6 +3,8 @@
  *
  * Copyright (C) 2011  Black Sphere Technologies Ltd.
  * Written by Gareth McMullin <gareth@blacksphere.co.nz>
+ * Copyright (C) 2023 1BitSquared <info@1bitsquared.com>
+ * Modified by Rachel Mant <git@dragonmux.network>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -23,23 +25,30 @@
 #include "general.h"
 #include "hex_utils.h"
 
-static const char hexdigits[] = "0123456789abcdef";
+char hex_digit(const uint8_t value)
+{
+	char digit = (char)value;
+	if (value > 9U)
+		digit += 'A' - '0' - 10U;
+	digit += '0';
+	return digit;
+}
 
 char *hexify(char *const hex, const void *const buf, const size_t size)
 {
 	char *dst = hex;
 	const uint8_t *const src = buf;
 
 	for (size_t idx = 0; idx < size; ++idx) {
-		*dst++ = hexdigits[src[idx] >> 4U];
-		*dst++ = hexdigits[src[idx] & 0xfU];
+		*dst++ = hex_digit(src[idx] >> 4U);
+		*dst++ = hex_digit(src[idx] & 0xfU);
 	}
 	*dst = 0;
 
 	return hex;
 }
 
-static uint8_t unhex_digit(const char hex)
+uint8_t unhex_digit(const char hex)
 {
 	uint8_t tmp = hex - '0';
 	if (tmp > 9U)

src/include/gdb_main.h

@@ -22,9 +22,12 @@
 #define INCLUDE_GDB_MAIN_H
 
 #include "target.h"
+
 extern bool gdb_target_running;
 extern target_s *cur_target;
+
 void gdb_poll_target(void);
 void gdb_main(char *pbuf, size_t pbuf_size, size_t size);
+char *gdb_packet_buffer();
 
 #endif /* INCLUDE_GDB_MAIN_H */

src/include/general.h

@@ -103,80 +103,73 @@ extern int cl_debuglevel;
 
 static inline void DEBUG_WARN(const char *format, ...)
 {
-	va_list ap;
-	va_start(ap, format);
-	vfprintf(stderr, format, ap);
-	va_end(ap);
-	return;
+	va_list args;
+	va_start(args, format);
+	vfprintf(stderr, format, args);
+	va_end(args);
 }
 
 static inline void DEBUG_INFO(const char *format, ...)
 {
 	if (~cl_debuglevel & BMP_DEBUG_INFO)
 		return;
-	va_list ap;
-	va_start(ap, format);
+	va_list args;
+	va_start(args, format);
 	if (cl_debuglevel & BMP_DEBUG_STDOUT)
-		vfprintf(stdout, format, ap);
+		vfprintf(stdout, format, args);
 	else
-		vfprintf(stderr, format, ap);
-	va_end(ap);
-	return;
+		vfprintf(stderr, format, args);
+	va_end(args);
 }
 
 static inline void DEBUG_GDB(const char *format, ...)
 {
 	if (~cl_debuglevel & BMP_DEBUG_GDB)
 		return;
-	va_list ap;
-	va_start(ap, format);
-	vfprintf(stderr, format, ap);
-	va_end(ap);
-	return;
+	va_list args;
+	va_start(args, format);
+	vfprintf(stderr, format, args);
+	va_end(args);
 }
 
 static inline void DEBUG_GDB_WIRE(const char *format, ...)
 {
 	if ((cl_debuglevel & (BMP_DEBUG_GDB | BMP_DEBUG_WIRE)) != (BMP_DEBUG_GDB | BMP_DEBUG_WIRE))
 		return;
-	va_list ap;
-	va_start(ap, format);
-	vfprintf(stderr, format, ap);
-	va_end(ap);
-	return;
+	va_list args;
+	va_start(args, format);
+	vfprintf(stderr, format, args);
+	va_end(args);
 }
 
 static inline void DEBUG_TARGET(const char *format, ...)
 {
 	if (~cl_debuglevel & BMP_DEBUG_TARGET)
 		return;
-	va_list ap;
-	va_start(ap, format);
-	vfprintf(stderr, format, ap);
-	va_end(ap);
-	return;
+	va_list args;
+	va_start(args, format);
+	vfprintf(stderr, format, args);
+	va_end(args);
 }
 
 static inline void DEBUG_PROBE(const char *format, ...)
 {
 	if (~cl_debuglevel & BMP_DEBUG_PROBE)
 		return;
-	va_list ap;
-	va_start(ap, format);
-	vfprintf(stderr, format, ap);
-	va_end(ap);
-	return;
+	va_list args;
+	va_start(args, format);
+	vfprintf(stderr, format, args);
+	va_end(args);
 }
 
 static inline void DEBUG_WIRE(const char *format, ...)
 {
 	if (~cl_debuglevel & BMP_DEBUG_WIRE)
 		return;
-	va_list ap;
-	va_start(ap, format);
-	vfprintf(stderr, format, ap);
-	va_end(ap);
-	return;
+	va_list args;
+	va_start(args, format);
+	vfprintf(stderr, format, args);
+	va_end(args);
 }
 #endif
 

src/include/hex_utils.h

@@ -3,6 +3,8 @@
  *
  * Copyright (C) 2011  Black Sphere Technologies Ltd.
  * Written by Gareth McMullin <gareth@blacksphere.co.nz>
+ * Copyright (C) 2023 1BitSquared <info@1bitsquared.com>
+ * Modified by Rachel Mant <git@dragonmux.network>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -21,9 +23,13 @@
 #ifndef INCLUDE_HEX_UTILS_H
 #define INCLUDE_HEX_UTILS_H
 
+#include <stdint.h>
 #include <stddef.h>
 
 char *hexify(char *hex, const void *buf, size_t size);
 char *unhexify(void *buf, const char *hex, size_t size);
 
+char hex_digit(uint8_t value);
+uint8_t unhex_digit(char hex);
+
 #endif /* INCLUDE_HEX_UTILS_H */

src/main.c

@@ -33,7 +33,13 @@
 #endif
 
 #define BUF_SIZE 1024U
-static char pbuf[BUF_SIZE + 1U];
+/* This has to be aligned so the remote protocol can re-use it without causing Problems */
+static char pbuf[BUF_SIZE + 1U] __attribute__((aligned(8)));
+
+char *gdb_packet_buffer()
+{
+	return pbuf;
+}
 
 static void bmp_poll_loop(void)
 {

src/platforms/common/jtagtap.c

@@ -3,6 +3,8 @@
  *
  * Copyright (C) 2011  Black Sphere Technologies Ltd.
  * Written by Gareth McMullin <gareth@blacksphere.co.nz>
+ * Copyright (C) 2022-2023 1BitSquared <info@1bitsquared.com>
+ * Modified by Rachel Mant <git@dragonmux.network>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -168,15 +170,6 @@ static void jtagtap_tdi_tdo_seq_swd_delay(
 		data_out[byte] = value;
 }
 
-static inline void jtag_next_tms_tdi(const bool tms, const bool tdi)
-{
-	/* Initiate the falling edge on the bus */
-	gpio_clear(TCK_PORT, TCK_PIN);
-	/* It is now safe to change TMS and TDI */
-	gpio_set_val(TMS_PORT, TMS_PIN, tms);
-	gpio_set_val(TDI_PORT, TDI_PIN, tdi);
-}
-
 static void jtagtap_tdi_tdo_seq_no_delay(
 	const uint8_t *const data_in, uint8_t *const data_out, const bool final_tms, const size_t clock_cycles)
 {
@@ -185,16 +178,28 @@ static void jtagtap_tdi_tdo_seq_no_delay(
 		/* Calculate the next bit and byte to consume data from */
 		const uint8_t bit = cycle & 7U;
 		const size_t byte = cycle >> 3U;
+		const bool tms = cycle + 1U >= clock_cycles && final_tms;
+		const bool tdi = data_in[byte] & (1U << bit);
+		/* Block the compiler from re-ordering the calculations to preserve timings */
+		__asm__ volatile("" ::: "memory");
+		gpio_clear(TCK_PORT, TCK_PIN);
+		/* Block the compiler from re-ordering the calculations to preserve timings */
+		__asm__ volatile("" ::: "memory");
 		/* Configure the bus for the next cycle */
-		jtag_next_tms_tdi(cycle + 1U >= clock_cycles && final_tms, data_in[byte] & (1U << bit));
+		gpio_set_val(TDI_PORT, TDI_PIN, tdi);
+		gpio_set_val(TMS_PORT, TMS_PIN, tms);
+		/* Block the compiler from re-ordering the calculations to preserve timings */
+		__asm__ volatile("" ::: "memory");
 		/* Increment the cycle counter */
 		++cycle;
+		__asm__("nop");
+		__asm__("nop");
 		/* Block the compiler from re-ordering the calculations to preserve timings */
-		__asm__ volatile("" ::: "memory");
+		__asm__ volatile("nop" ::: "memory");
 		/* Start the clock cycle */
 		gpio_set(TCK_PORT, TCK_PIN);
 		/* If TDO is high, store a 1 in the appropriate position in the value being accumulated */
-		if (gpio_get(TDO_PORT, TDO_PIN))
+		if (gpio_get(TDO_PORT, TDO_PIN)) /* XXX: Try to remove the need for the if here */
 			value |= 1U << bit;
 		/* If we've got the next whole byte, store the accumulated value and reset state */
 		if (bit == 7U) {
@@ -203,6 +208,11 @@ static void jtagtap_tdi_tdo_seq_no_delay(
 		}
 		/* Finish the clock cycle */
 	}
+	/* If clock_cycles is not divisable by 8, we have some extra data to write back here. */
+	if (clock_cycles & 7U) {
+		const size_t byte = (clock_cycles - 1U) >> 3U;
+		data_out[byte] = value;
+	}
 	gpio_clear(TCK_PORT, TCK_PIN);
 }
 
@@ -238,14 +248,27 @@ static void jtagtap_tdi_seq_swd_delay(const uint8_t *const data_in, const bool f
 
 static void jtagtap_tdi_seq_no_delay(const uint8_t *const data_in, const bool final_tms, size_t clock_cycles)
 {
-	for (size_t cycle = 0; cycle < clock_cycles; ++cycle) {
-		gpio_clear(TCK_PORT, TCK_PIN);
+	for (size_t cycle = 0; cycle < clock_cycles;) {
 		const size_t bit = cycle & 7U;
 		const size_t byte = cycle >> 3U;
+		const bool tms = cycle + 1U >= clock_cycles && final_tms;
+		const bool tdi = data_in[byte] & (1U << bit);
+		/* Block the compiler from re-ordering the calculations to preserve timings */
+		__asm__ volatile("" ::: "memory");
+		gpio_clear(TCK_PORT, TCK_PIN);
 		/* On the last tick, assert final_tms to TMS_PIN */
-		gpio_set_val(TMS_PORT, TMS_PIN, cycle + 1U >= clock_cycles && final_tms);
+		gpio_set_val(TMS_PORT, TMS_PIN, tms);
 		/* Set up the TDI pin and start the clock cycle */
-		gpio_set_val(TDI_PORT, TDI_PIN, data_in[byte] & (1U << bit));
+		gpio_set_val(TDI_PORT, TDI_PIN, tdi);
+		/* Block the compiler from re-ordering the calculations to preserve timings */
+		__asm__ volatile("" ::: "memory");
+		/* Increment the cycle counter */
+		++cycle;
+		__asm__("nop");
+		__asm__("nop");
+		/* Block the compiler from re-ordering the calculations to preserve timings */
+		__asm__ volatile("nop" ::: "memory");
+		/* Start the clock cycle */
 		gpio_set(TCK_PORT, TCK_PIN);
 		/* Finish the clock cycle */
 	}
@@ -279,7 +302,7 @@ static void jtagtap_cycle_no_delay(const size_t clock_cycles)
 {
 	for (size_t cycle = 0; cycle < clock_cycles; ++cycle) {
 		gpio_set(TCK_PORT, TCK_PIN);
-		gpio_set(TCK_PORT, TCK_PIN);
+		__asm__ volatile("nop" ::: "memory");
 		gpio_clear(TCK_PORT, TCK_PIN);
 	}
 }