Fix: CMSIS-DAP block I/O

src/platforms/hosted/cmsis_dap.c

@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2013-2019, Alex Taradov <alex@taradov.com>
- * Copyright (C) 2023 1BitSquared <info@1bitsquared.com>
+ * Copyright (C) 2023-2024 1BitSquared <info@1bitsquared.com>
  * Modified by Rachel Mant <git@dragonmux.network>
  * All rights reserved.
  *
@@ -64,6 +64,7 @@
 #include <hidapi.h>
 #include <wchar.h>
 #include <sys/stat.h>
+#include <assert.h>
 
 #include "bmp_hosted.h"
 #include "dap.h"
@@ -103,9 +104,9 @@ static hid_device *handle = NULL;
 static uint8_t buffer[1025U];
 /*
  * Start by defaulting this to the typical size of `DAP_PACKET_SIZE` for FS USB. This value is pulled from here:
- * https://www.keil.com/pack/doc/CMSIS/DAP/html/group__DAP__Config__Debug__gr.html#gaa28bb1da2661291634c4a8fb3e227404
+ * https://arm-software.github.io/CMSIS-DAP/latest/group__DAP__Config__Debug__gr.html#gaa28bb1da2661291634c4a8fb3e227404
  */
-static size_t packet_size = 64U;
+static size_t dap_packet_size = 64U;
 bool dap_has_swd_sequence = false;
 
 dap_version_s dap_adaptor_version(dap_info_e version_kind);
@@ -157,7 +158,7 @@ static inline bool dap_version_compare_le(const dap_version_s lhs, const dap_ver
  */
 static inline size_t dap_max_transfer_data(size_t command_header_len)
 {
-	const size_t result = packet_size - command_header_len;
+	const size_t result = dap_packet_size - command_header_len;
 
 	/* Allow for an additional byte of payload overhead when sending data in HID Report payloads */
 	if (type == CMSIS_TYPE_HID)
@@ -221,13 +222,13 @@ static bool dap_init_hid(void)
 
 	/*
 	 * Base the report length information for the device on the max packet length from its descriptors.
-	 * Add 1 to account for HIDAPI's need to prefix with a report type byte.
+	 * Add 1 to account for HIDAPI's need to prefix with a report type byte. Limit to at most 512 bytes.
 	 */
-	packet_size = bmda_probe_info.max_packet_length + 1U;
+	dap_packet_size = MIN(bmda_probe_info.max_packet_length + 1U, 512U);
 
 	/* Handle the NXP LPC11U3x CMSIS-DAP v1.0.7 implementation needing a 64 byte report length */
 	if (bmda_probe_info.vid == 0x1fc9U && bmda_probe_info.pid == 0x0132U)
-		packet_size = 64U + 1U;
+		dap_packet_size = 64U + 1U;
 
 	/* Now open the device with HIDAPI so we can talk with it */
 	handle = hid_open(bmda_probe_info.vid, bmda_probe_info.pid, serial[0] ? serial : NULL);
@@ -255,7 +256,7 @@ static bool dap_init_bulk(void)
 		return false;
 	}
 	/* Base the packet size on the one retrieved from the device descriptors */
-	packet_size = bmda_probe_info.max_packet_length;
+	dap_packet_size = bmda_probe_info.max_packet_length;
 	in_ep = bmda_probe_info.in_ep;
 	out_ep = bmda_probe_info.out_ep;
 	return true;
@@ -309,7 +310,7 @@ bool dap_init(bool allow_fallback)
 		/* Report the failure */
 		DEBUG_WARN("Failed to get adaptor packet size, assuming descriptor provided size\n");
 	else
-		packet_size = dap_packet_size + (type == CMSIS_TYPE_HID ? 1U : 0U);
+		dap_packet_size = dap_packet_size + (type == CMSIS_TYPE_HID ? 1U : 0U);
 
 	/* Try to get the device's capabilities */
 	const size_t size = dap_info(DAP_INFO_CAPABILITIES, &dap_caps, sizeof(dap_caps));
@@ -350,6 +351,10 @@ bool dap_init(bool allow_fallback)
 		dap_version_compare_le(adaptor_version, (dap_version_s){1, 3, 1}))
 		dap_quirks |= DAP_QUIRK_BAD_SWD_NO_RESP_DATA_PHASE;
 
+	/* ORBTrace needs an extra ZLP read done on full packet reception */
+	if (strcmp(bmda_probe_info.product, "Orbtrace") == 0)
+		dap_quirks |= DAP_QUIRK_NEEDS_EXTRA_ZLP_READ;
+
 	return true;
 }
 
@@ -459,17 +464,17 @@ ssize_t dbg_dap_cmd_hid(const uint8_t *const request_data, const size_t request_
 	const size_t response_length)
 {
 	// Need room to prepend HID Report ID byte
-	if (request_length + 1U > packet_size) {
-		DEBUG_ERROR(
-			"Attempted to make over-long request of %zu bytes, max length is %zu\n", request_length + 1U, packet_size);
+	if (request_length + 1U > dap_packet_size) {
+		DEBUG_ERROR("Attempted to make over-long request of %zu bytes, max length is %zu\n", request_length + 1U,
+			dap_packet_size);
 		exit(-1);
 	}
 
-	memset(buffer + request_length + 1U, 0xff, packet_size - (request_length + 1U));
+	memset(buffer + request_length + 1U, 0xff, dap_packet_size - (request_length + 1U));
 	buffer[0] = 0x00; // Report ID
 	memcpy(buffer + 1, request_data, request_length);
 
-	const int result = hid_write(handle, buffer, packet_size);
+	const int result = hid_write(handle, buffer, dap_packet_size);
 	if (result < 0) {
 		DEBUG_ERROR("CMSIS-DAP write error: %ls\n", hid_error(handle));
 		exit(-1);
@@ -512,6 +517,14 @@ ssize_t dbg_dap_cmd_bulk(const uint8_t *const request_data, const size_t request
 			return response_result;
 		}
 	} while (response_data[0] != request_data[0]);
+
+	/* If the response requested is the size of the packet size for the adaptor, generate a ZLP read to clean state */
+	if ((dap_quirks & DAP_QUIRK_NEEDS_EXTRA_ZLP_READ) && transferred == (int)dap_packet_size) {
+		uint8_t zlp;
+		int zlp_read = 0;
+		libusb_bulk_transfer(usb_handle, in_ep, &zlp, sizeof(zlp), &zlp_read, TRANSFER_TIMEOUT_MS);
+		assert(zlp_read == 0);
+	}
 	return transferred;
 }
 
@@ -526,16 +539,16 @@ static ssize_t dap_run_cmd_raw(const uint8_t *const request_data, const size_t r
 	/* Provide enough space for up to a HS USB HID payload */
 	uint8_t data[1024];
 	/* Make sure that we're not about to blow this buffer when we request data back */
-	if (sizeof(data) < packet_size) {
+	if (sizeof(data) < dap_packet_size) {
 		DEBUG_ERROR("CMSIS-DAP request would exceed response buffer\n");
 		return -1;
 	}
 
 	ssize_t response = -1;
 	if (type == CMSIS_TYPE_HID)
-		response = dbg_dap_cmd_hid(request_data, request_length, data, packet_size);
+		response = dbg_dap_cmd_hid(request_data, request_length, data, dap_packet_size);
 	else if (type == CMSIS_TYPE_BULK)
-		response = dbg_dap_cmd_bulk(request_data, request_length, data, packet_size);
+		response = dbg_dap_cmd_bulk(request_data, request_length, data, dap_packet_size);
 	if (response < 0)
 		return response;
 	const size_t result = (size_t)response;
@@ -561,6 +574,23 @@ bool dap_run_cmd(const void *const request_data, const size_t request_length, vo
 	return (size_t)result >= response_length;
 }
 
+bool dap_run_transfer(const void *const request_data, const size_t request_length, void *const response_data,
+	const size_t response_length, size_t *const actual_length)
+{
+	/*
+	 * This function works almost exactly the same as dap_run_cmd(), but captures and preserves the resulting
+	 * response length if the result is not an outright failure. It sets the actual response length to 0 when it is.
+	 */
+	const ssize_t result =
+		dap_run_cmd_raw((const uint8_t *)request_data, request_length, (uint8_t *)response_data, response_length) - 1U;
+	if (result < 0) {
+		*actual_length = 0U;
+		return false;
+	}
+	*actual_length = (size_t)result;
+	return *actual_length >= response_length;
+}
+
 static void dap_adiv5_mem_read(adiv5_access_port_s *ap, void *dest, target_addr64_t src, size_t len)
 {
 	if (len == 0U)
@@ -573,11 +603,12 @@ static void dap_adiv5_mem_read(adiv5_access_port_s *ap, void *dest, target_addr6
 		return;
 	}
 	/* Otherwise proceed blockwise */
-	const size_t blocks_per_transfer = dap_max_transfer_data(DAP_CMD_BLOCK_READ_HDR_LEN) >> 2U;
+	const size_t blocks_per_transfer = dap_max_transfer_data(DAP_CMD_BLOCK_READ_HDR_LEN + 1U) >> 2U;
 	uint8_t *const data = (uint8_t *)dest;
 	for (size_t offset = 0; offset < len;) {
 		/* Setup AP_TAR every loop as failing to do so results in it wrapping */
-		dap_adiv5_mem_access_setup(ap, src + offset, align);
+		if (!dap_adiv5_mem_access_setup(ap, src + offset, align))
+			return;
 		/*
 		 * src can start out unaligned to a 1024 byte chunk size,
 		 * so we have to calculate how much is left of the chunk.
@@ -615,7 +646,8 @@ static void dap_adiv5_mem_write(
 	const uint8_t *const data = (const uint8_t *)src;
 	for (size_t offset = 0; offset < len;) {
 		/* Setup AP_TAR every loop as failing to do so results in it wrapping */
-		dap_adiv5_mem_access_setup(ap, dest + offset, align);
+		if (!dap_adiv5_mem_access_setup(ap, dest + offset, align))
+			return;
 		/*
 		 * dest can start out unaligned to a 1024 byte chunk size,
 		 * so we have to calculate how much is left of the chunk.
@@ -654,11 +686,12 @@ static void dap_adiv6_mem_read(
 		return;
 	}
 	/* Otherwise proceed blockwise */
-	const size_t blocks_per_transfer = dap_max_transfer_data(DAP_CMD_BLOCK_READ_HDR_LEN) >> 2U;
+	const size_t blocks_per_transfer = dap_max_transfer_data(DAP_CMD_BLOCK_READ_HDR_LEN + 1U) >> 2U;
 	uint8_t *const data = (uint8_t *)dest;
 	for (size_t offset = 0; offset < len;) {
 		/* Setup AP_TAR every loop as failing to do so results in it wrapping */
-		dap_adiv6_mem_access_setup(ap, src + offset, align);
+		if (!dap_adiv6_mem_access_setup(ap, src + offset, align))
+			return;
 		/*
 		 * src can start out unaligned to a 1024 byte chunk size,
 		 * so we have to calculate how much is left of the chunk.
@@ -697,7 +730,8 @@ static void dap_adiv6_mem_write(
 	const uint8_t *const data = (const uint8_t *)src;
 	for (size_t offset = 0; offset < len;) {
 		/* Setup AP_TAR every loop as failing to do so results in it wrapping */
-		dap_adiv6_mem_access_setup(ap, dest + offset, align);
+		if (!dap_adiv6_mem_access_setup(ap, dest + offset, align))
+			return;
 		/*
 		 * dest can start out unaligned to a 1024 byte chunk size,
 		 * so we have to calculate how much is left of the chunk.

src/platforms/hosted/cmsis_dap.h

@@ -2,6 +2,8 @@
  * This file is part of the Black Magic Debug project.
  *
  * Copyright (C) 2019-2021 Uwe Bonnes <bon@elektron.ikp.physik.tu-darmstadt.de>
+ * Copyright (C) 2023-2024 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

src/platforms/hosted/dap.c

@@ -1,6 +1,7 @@
 /*
- * Copyright (c) 2013-2015, Alex Taradov <alex@taradov.com>
- * Copyright (C) 2023 1BitSquared <info@1bitsquared.com>
+ * Copyright (C) 2013-2015 Alex Taradov <alex@taradov.com>
+ * Copyright (C) 2020-2021 Uwe Bonnes <bon@elektron.ikp.physik.tu-darmstadt.de>
+ * Copyright (C) 2023-2024 1BitSquared <info@1bitsquared.com>
  * Modified by Rachel Mant <git@dragonmux.network>
  * All rights reserved.
  *
@@ -28,10 +29,6 @@
  * POSSIBILITY OF SUCH DAMAGE.
  */
 
-/* Modified for Blackmagic Probe
- * Copyright (c) 2020-21 Uwe Bonnes bon@elektron.ikp.physik.tu-darmstadt.de
- */
-
 #include <string.h>
 #include "general.h"
 #include "exception.h"
@@ -265,13 +262,12 @@ void dap_write_reg(adiv5_debug_port_s *target_dp, const uint8_t reg, const uint3
 bool dap_mem_read_block(
 	adiv5_access_port_s *const target_ap, void *dest, target_addr64_t src, const size_t len, const align_e align)
 {
+	/* Try to read the 32-bit blocks requested */
 	const size_t blocks = len >> MIN(align, 2U);
-	uint32_t data[256];
-	if (!perform_dap_transfer_block_read(target_ap->dp, SWD_AP_DRW, blocks, data)) {
-		DEBUG_ERROR("dap_read_block failed\n");
-		return false;
-	}
+	uint32_t data[127U] = {0U};
+	const bool result = perform_dap_transfer_block_read(target_ap->dp, SWD_AP_DRW, blocks, data);
 
+	/* Unpack the data from those blocks */
 	if (align > ALIGN_16BIT)
 		memcpy(dest, data, len);
 	else {
@@ -280,15 +276,20 @@ bool dap_mem_read_block(
 			src += 1U << align;
 		}
 	}
-	return true;
+
+	/* Report if it actually failed and then propagate the failure up accordingly */
+	if (!result)
+		DEBUG_ERROR("dap_read_block failed\n");
+	return result;
 }
 
 bool dap_mem_write_block(
 	adiv5_access_port_s *const target_ap, target_addr64_t dest, const void *src, const size_t len, const align_e align)
 {
-	const size_t blocks = len >> MAX(align, 2U);
-	uint32_t data[256];
+	const size_t blocks = len >> MIN(align, 2U);
+	uint32_t data[126U];
 
+	/* Pack the data to send into 32-bit blocks */
 	if (align > ALIGN_16BIT)
 		memcpy(data, src, len);
 	else {
@@ -298,7 +299,9 @@ bool dap_mem_write_block(
 		}
 	}
 
+	/* Try to write the blocks to the target's memory */
 	const bool result = perform_dap_transfer_block_write(target_ap->dp, SWD_AP_DRW, blocks, data);
+	/* Report if it actually failed and then propagate the failure up accordingly */
 	if (!result)
 		DEBUG_ERROR("dap_write_block failed\n");
 	return result;
@@ -340,21 +343,14 @@ static size_t dap_adiv5_mem_access_build(const adiv5_access_port_s *const target
 	}
 }
 
-void dap_adiv5_mem_access_setup(adiv5_access_port_s *const target_ap, const target_addr64_t addr, const align_e align)
+bool dap_adiv5_mem_access_setup(adiv5_access_port_s *const target_ap, const target_addr64_t addr, const align_e align)
 {
 	/* Start by setting up the transfer and attempting it */
 	dap_transfer_request_s requests[4];
 	const size_t requests_count = dap_adiv5_mem_access_build(target_ap, requests, addr, align);
 	adiv5_debug_port_s *const target_dp = target_ap->dp;
-	const bool result = perform_dap_transfer_recoverable(target_dp, requests, requests_count, NULL, 0U);
-	/* If it didn't go well, say something and abort */
-	if (!result) {
-		if (target_dp->fault != DAP_TRANSFER_NO_RESPONSE)
-			DEBUG_ERROR("Transport error (%u), aborting\n", target_dp->fault);
-		else
-			DEBUG_ERROR("Transaction unrecoverably failed\n");
-		exit(-1);
-	}
+	/* The result of this call is then fed up the stack for proper handling */
+	return perform_dap_transfer_recoverable(target_dp, requests, requests_count, NULL, 0U);
 }
 
 static size_t dap_adiv6_mem_access_build(const adiv6_access_port_s *const target_ap,
@@ -398,21 +394,14 @@ static size_t dap_adiv6_mem_access_build(const adiv6_access_port_s *const target
 	}
 }
 
-void dap_adiv6_mem_access_setup(adiv6_access_port_s *const target_ap, const target_addr64_t addr, const align_e align)
+bool dap_adiv6_mem_access_setup(adiv6_access_port_s *const target_ap, const target_addr64_t addr, const align_e align)
 {
 	/* Start by setting up the transfer and attempting it */
 	dap_transfer_request_s requests[6];
 	const size_t requests_count = dap_adiv6_mem_access_build(target_ap, requests, addr, align);
 	adiv5_debug_port_s *const target_dp = target_ap->base.dp;
-	const bool result = perform_dap_transfer_recoverable(target_dp, requests, requests_count, NULL, 0U);
-	/* If it didn't go well, say something and abort */
-	if (!result) {
-		if (target_dp->fault != DAP_TRANSFER_NO_RESPONSE)
-			DEBUG_ERROR("Transport error (%u), aborting\n", target_dp->fault);
-		else
-			DEBUG_ERROR("Transaction unrecoverably failed\n");
-		exit(-1);
-	}
+	/* The result of this call is then fed up the stack for proper handling */
+	return perform_dap_transfer_recoverable(target_dp, requests, requests_count, NULL, 0U);
 }
 
 uint32_t dap_adiv5_ap_read(adiv5_access_port_s *const target_ap, const uint16_t addr)

src/platforms/hosted/dap.h

@@ -1,5 +1,9 @@
 /*
+ * This file is part of the Black Magic Debug project.
+ *
  * Copyright (c) 2013-2015, Alex Taradov <alex@taradov.com>
+ * Copyright (C) 2023-2024 1BitSquared <info@1bitsquared.com>
+ * Modified by Rachel Mant <git@dragonmux.network>
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -68,6 +72,7 @@ typedef enum dap_led_type {
 #define DAP_QUIRK_NO_JTAG_MUTLI_TAP          (1U << 0U)
 #define DAP_QUIRK_BAD_SWD_NO_RESP_DATA_PHASE (1U << 1U)
 #define DAP_QUIRK_BROKEN_SWD_SEQUENCE        (1U << 2U)
+#define DAP_QUIRK_NEEDS_EXTRA_ZLP_READ       (1U << 3U)
 
 extern uint8_t dap_caps;
 extern dap_cap_e dap_mode;
@@ -87,14 +92,16 @@ uint32_t dap_adiv6_ap_read(adiv5_access_port_s *base_ap, uint16_t addr);
 void dap_adiv6_ap_write(adiv5_access_port_s *base_ap, uint16_t addr, uint32_t value);
 void dap_adiv5_mem_read_single(adiv5_access_port_s *target_ap, void *dest, target_addr64_t src, align_e align);
 void dap_adiv5_mem_write_single(adiv5_access_port_s *target_ap, target_addr64_t dest, const void *src, align_e align);
-void dap_adiv5_mem_access_setup(adiv5_access_port_s *target_ap, target_addr64_t addr, align_e align);
+bool dap_adiv5_mem_access_setup(adiv5_access_port_s *target_ap, target_addr64_t addr, align_e align);
 void dap_adiv6_mem_read_single(adiv6_access_port_s *target_ap, void *dest, target_addr64_t src, align_e align);
 void dap_adiv6_mem_write_single(adiv6_access_port_s *target_ap, target_addr64_t dest, const void *src, align_e align);
-void dap_adiv6_mem_access_setup(adiv6_access_port_s *target_ap, target_addr64_t addr, align_e align);
+bool dap_adiv6_mem_access_setup(adiv6_access_port_s *target_ap, target_addr64_t addr, align_e align);
 bool dap_mem_read_block(adiv5_access_port_s *target_ap, void *dest, target_addr64_t src, size_t len, align_e align);
 bool dap_mem_write_block(
 	adiv5_access_port_s *target_ap, target_addr64_t dest, const void *src, size_t len, align_e align);
 bool dap_run_cmd(const void *request_data, size_t request_length, void *response_data, size_t response_length);
+bool dap_run_transfer(const void *request_data, size_t request_length, void *response_data, size_t response_length,
+	size_t *actual_length);
 bool dap_jtag_configure(void);
 
 void dap_dp_abort(adiv5_debug_port_s *target_dp, uint32_t abort);