Inverted Frame to Pseudo Tap for Simpler Hardware to Decode DR (#373)

* Inverted Frame to Pseudo Tap for Simpler Hardware to Decode

Given the variable supported message length , a prefix  decoding approach is significantly simpler for a pseudo tap architecture with a shift reg of len =  max len of packet. This prefix coding packet also makes the message len field redundant , as that is implict in ir_len and the ir selected.

* style patch

* non-conflict with original

* style patch

* style patch

* requested changes

* style-patch

3 files changed, 150 insertions, 82 deletions

diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c
index 217bdd9..7f12779 100644
--- a/src/target/riscv/riscv-013.c
+++ b/src/target/riscv/riscv-013.c
@@ -516,22 +516,37 @@ static dmi_status_t dmi_scan(struct target *target, uint32_t *address_in,
 		   struct at the function's top-level, so its lifetime exceeds the point at which
 		   the queue is executed, and initializing with assignments here. */
 		memset(tunneled_dr, 0, sizeof(tunneled_dr));
-		tunneled_dr[0].num_bits = 1;
-		tunneled_dr[0].out_value = bscan_one;
-
-		tunneled_dr[1].num_bits = 7;
-		tunneled_dr_width = num_bits;
-		tunneled_dr[1].out_value = &tunneled_dr_width;
-
-		/* for BSCAN tunnel, there is a one-TCK skew between shift in and shift out, so
-		   scanning num_bits + 1, and then will right shift the input field after executing the queues */
-		tunneled_dr[2].num_bits = num_bits+1;
-		tunneled_dr[2].out_value = out;
-		tunneled_dr[2].in_value = in;
-
-		tunneled_dr[3].num_bits = 3;
-		tunneled_dr[3].out_value = bscan_zero;
-
+		if (bscan_tunnel_type == BSCAN_TUNNEL_DATA_REGISTER) {
+			tunneled_dr[3].num_bits = 1;
+			tunneled_dr[3].out_value = bscan_one;
+			tunneled_dr[2].num_bits = 7;
+			tunneled_dr_width = num_bits;
+			tunneled_dr[2].out_value = &tunneled_dr_width;
+			/* for BSCAN tunnel, there is a one-TCK skew between shift in and shift out, so
+			   scanning num_bits + 1, and then will right shift the input field after executing the queues */
+
+			tunneled_dr[1].num_bits = num_bits+1;
+			tunneled_dr[1].out_value = out;
+			tunneled_dr[1].in_value = in;
+
+
+			tunneled_dr[0].num_bits = 3;
+			tunneled_dr[0].out_value = bscan_zero;
+		} else {
+			/* BSCAN_TUNNEL_NESTED_TAP */
+			tunneled_dr[0].num_bits = 1;
+			tunneled_dr[0].out_value = bscan_one;
+			tunneled_dr[1].num_bits = 7;
+			tunneled_dr_width = num_bits;
+			tunneled_dr[1].out_value = &tunneled_dr_width;
+			/* for BSCAN tunnel, there is a one-TCK skew between shift in and shift out, so
+			   scanning num_bits + 1, and then will right shift the input field after executing the queues */
+			tunneled_dr[2].num_bits = num_bits+1;
+			tunneled_dr[2].out_value = out;
+			tunneled_dr[2].in_value = in;
+			tunneled_dr[3].num_bits = 3;
+			tunneled_dr[3].out_value = bscan_zero;
+		}
 		jtag_add_dr_scan(target->tap, DIM(tunneled_dr), tunneled_dr, TAP_IDLE);
 	} else {
 		/* Assume dbus is already selected. */
@@ -561,9 +576,7 @@ static dmi_status_t dmi_scan(struct target *target, uint32_t *address_in,
 
 	if (address_in)
 		*address_in = buf_get_u32(in, DTM_DMI_ADDRESS_OFFSET, info->abits);
-
 	dump_field(idle_count, &field);
-
 	return buf_get_u32(in, DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH);
 }
 
diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c
index 47c8041..1f4883c 100644
--- a/src/target/riscv/riscv.c
+++ b/src/target/riscv/riscv.c
@@ -170,8 +170,9 @@ struct scan_field select_idcode = {
 	.out_value = ir_idcode
 };
 
-
+bscan_tunnel_type_t bscan_tunnel_type;
 int bscan_tunnel_ir_width; /* if zero, then tunneling is not present/active */
+
 uint8_t bscan_zero[4] = {0};
 uint8_t bscan_one[4] = {1};
 
@@ -183,7 +184,30 @@ struct scan_field select_user4 = {
 
 
 uint8_t bscan_tunneled_ir_width[4] = {5};  /* overridden by assignment in riscv_init_target */
-struct scan_field _bscan_tunneled_select_dmi[] = {
+struct scan_field _bscan_tunnel_data_register_select_dmi[] = {
+		{
+			.num_bits = 3,
+			.out_value = bscan_zero,
+			.in_value = NULL,
+		},
+		{
+			.num_bits = 5, /* initialized in riscv_init_target to ir width of DM */
+			.out_value = ir_dbus,
+			.in_value = NULL,
+		},
+		{
+			.num_bits = 7,
+			.out_value = bscan_tunneled_ir_width,
+			.in_value = NULL,
+		},
+		{
+			.num_bits = 1,
+			.out_value = bscan_zero,
+			.in_value = NULL,
+		}
+};
+
+struct scan_field _bscan_tunnel_nested_tap_select_dmi[] = {
 		{
 			.num_bits = 1,
 			.out_value = bscan_zero,
@@ -205,8 +229,11 @@ struct scan_field _bscan_tunneled_select_dmi[] = {
 			.in_value = NULL,
 		}
 };
-struct scan_field *bscan_tunneled_select_dmi = _bscan_tunneled_select_dmi;
-uint32_t bscan_tunneled_select_dmi_num_fields = DIM(_bscan_tunneled_select_dmi);
+struct scan_field *bscan_tunnel_nested_tap_select_dmi = _bscan_tunnel_nested_tap_select_dmi;
+uint32_t bscan_tunnel_nested_tap_select_dmi_num_fields = DIM(_bscan_tunnel_nested_tap_select_dmi);
+
+struct scan_field *bscan_tunnel_data_register_select_dmi = _bscan_tunnel_data_register_select_dmi;
+uint32_t bscan_tunnel_data_register_select_dmi_num_fields = DIM(_bscan_tunnel_data_register_select_dmi);
 
 struct trigger {
 	uint64_t address;
@@ -242,10 +269,14 @@ static int riscv_resume_go_all_harts(struct target *target);
 void select_dmi_via_bscan(struct target *target)
 {
 	jtag_add_ir_scan(target->tap, &select_user4, TAP_IDLE);
-	jtag_add_dr_scan(target->tap, bscan_tunneled_select_dmi_num_fields, bscan_tunneled_select_dmi, TAP_IDLE);
+	if (bscan_tunnel_type == BSCAN_TUNNEL_DATA_REGISTER)
+		jtag_add_dr_scan(target->tap, bscan_tunnel_data_register_select_dmi_num_fields,
+										bscan_tunnel_data_register_select_dmi, TAP_IDLE);
+	else /* BSCAN_TUNNEL_NESTED_TAP */
+		jtag_add_dr_scan(target->tap, bscan_tunnel_nested_tap_select_dmi_num_fields,
+										bscan_tunnel_nested_tap_select_dmi, TAP_IDLE);
 }
 
-
 uint32_t dtmcontrol_scan_via_bscan(struct target *target, uint32_t out)
 {
 	/* On BSCAN TAP: Select IR=USER4, issue tunneled IR scan via BSCAN TAP's DR */
@@ -255,54 +286,63 @@ uint32_t dtmcontrol_scan_via_bscan(struct target *target, uint32_t out)
 	uint8_t in_value[5] = {0};
 
 	buf_set_u32(out_value, 0, 32, out);
-
-	struct scan_field tunneled_ir[] = {
-		{
-			.num_bits = 1,
-			.out_value = bscan_zero,
-			.in_value = NULL,
-		},
-		{
-			.num_bits = 7,
-			.out_value = tunneled_ir_width,
-			.in_value = NULL,
-		},
-		{
-			.num_bits = bscan_tunnel_ir_width,
-			.out_value = ir_dtmcontrol,
-			.in_value = NULL,
-		},
-		{
-			.num_bits = 3,
-			.out_value = bscan_zero,
-			.in_value = NULL,
-		}
-	};
-	struct scan_field tunneled_dr[] = {
-		{
-			.num_bits = 1,
-			.out_value = bscan_one,
-			.in_value = NULL,
-		},
-		{
-			.num_bits = 7,
-			.out_value = tunneled_dr_width,
-			.in_value = NULL,
-		},
-		/* for BSCAN tunnel, there is a one-TCK skew between shift in and shift out,
-		   so scanning 33 bits and then right shifting the in_value after the scan is completed */
-		{
-			.num_bits = 32+1,
-			.out_value = out_value,
-			.in_value = in_value,
-		},
-		{
-			.num_bits = 3,
-			.out_value = bscan_zero,
-			.in_value = NULL,
-		}
-	};
-
+	struct scan_field tunneled_ir[4] = {};
+	struct scan_field tunneled_dr[4] = {};
+
+	if (bscan_tunnel_type == BSCAN_TUNNEL_DATA_REGISTER) {
+		tunneled_ir[0].num_bits = 3;
+		tunneled_ir[0].out_value = bscan_zero;
+		tunneled_ir[0].in_value = NULL;
+		tunneled_ir[1].num_bits = bscan_tunnel_ir_width;
+		tunneled_ir[1].out_value = ir_dtmcontrol;
+		tunneled_ir[1].in_value = NULL;
+		tunneled_ir[2].num_bits = 7;
+		tunneled_ir[2].out_value = tunneled_ir_width;
+		tunneled_ir[2].in_value = NULL;
+		tunneled_ir[3].num_bits = 1;
+		tunneled_ir[3].out_value = bscan_zero;
+		tunneled_ir[3].in_value = NULL;
+
+		tunneled_dr[0].num_bits = 3;
+		tunneled_dr[0].out_value = bscan_zero;
+		tunneled_dr[0].in_value = NULL;
+		tunneled_dr[1].num_bits = 32+1;
+		tunneled_dr[1].out_value = out_value;
+		tunneled_dr[1].in_value = in_value;
+		tunneled_dr[2].num_bits = 7;
+		tunneled_dr[2].out_value = tunneled_dr_width;
+		tunneled_dr[2].in_value = NULL;
+		tunneled_dr[3].num_bits = 1;
+		tunneled_dr[3].out_value = bscan_one;
+		tunneled_dr[3].in_value = NULL;
+	} else{
+		/* BSCAN_TUNNEL_NESTED_TAP */
+		tunneled_ir[3].num_bits = 3;
+		tunneled_ir[3].out_value = bscan_zero;
+		tunneled_ir[3].in_value = NULL;
+		tunneled_ir[2].num_bits = bscan_tunnel_ir_width;
+		tunneled_ir[2].out_value = ir_dtmcontrol;
+		tunneled_ir[1].in_value = NULL;
+		tunneled_ir[1].num_bits = 7;
+		tunneled_ir[1].out_value = tunneled_ir_width;
+		tunneled_ir[2].in_value = NULL;
+		tunneled_ir[0].num_bits = 1;
+		tunneled_ir[0].out_value = bscan_zero;
+		tunneled_ir[0].in_value = NULL;
+
+		tunneled_dr[3].num_bits = 3;
+		tunneled_dr[3].out_value = bscan_zero;
+		tunneled_dr[3].in_value = NULL;
+		tunneled_dr[2].num_bits = 32+1;
+		tunneled_dr[2].out_value = out_value;
+		tunneled_dr[2].in_value = in_value;
+		tunneled_dr[1].num_bits = 7;
+		tunneled_dr[1].out_value = tunneled_dr_width;
+		tunneled_dr[1].in_value = NULL;
+		tunneled_dr[0].num_bits = 1;
+		tunneled_dr[0].out_value = bscan_one;
+		tunneled_dr[0].in_value = NULL;
+	}
 	jtag_add_ir_scan(target->tap, &select_user4, TAP_IDLE);
 	jtag_add_dr_scan(target->tap, DIM(tunneled_ir), tunneled_ir, TAP_IDLE);
 	jtag_add_dr_scan(target->tap, DIM(tunneled_dr), tunneled_dr, TAP_IDLE);
@@ -313,7 +353,6 @@ uint32_t dtmcontrol_scan_via_bscan(struct target *target, uint32_t out)
 		LOG_ERROR("failed jtag scan: %d", retval);
 		return retval;
 	}
-
 	/* Note the starting offset is bit 1, not bit 0.  In BSCAN tunnel, there is a one-bit TCK skew between
 	   output and input */
 	uint32_t in = buf_get_u32(in_value, 1, 32);
@@ -396,7 +435,10 @@ static int riscv_init_target(struct command_context *cmd_ctx,
 	if (bscan_tunnel_ir_width != 0) {
 		select_user4.num_bits = target->tap->ir_length;
 		bscan_tunneled_ir_width[0] = bscan_tunnel_ir_width;
-		bscan_tunneled_select_dmi[2].num_bits = bscan_tunnel_ir_width;
+		if (bscan_tunnel_type == BSCAN_TUNNEL_DATA_REGISTER)
+			bscan_tunnel_data_register_select_dmi[1].num_bits = bscan_tunnel_ir_width;
+		else /* BSCAN_TUNNEL_NESTED_TAP */
+			bscan_tunnel_nested_tap_select_dmi[2].num_bits = bscan_tunnel_ir_width;
 	}
 
 	riscv_semihosting_init(target);
@@ -2023,20 +2065,29 @@ COMMAND_HANDLER(riscv_set_ir)
 COMMAND_HANDLER(riscv_use_bscan_tunnel)
 {
 	int irwidth = 0;
+	int tunnel_type = BSCAN_TUNNEL_NESTED_TAP;
 
-	if (CMD_ARGC > 1) {
-		LOG_ERROR("Command takes at most one argument");
+	if (CMD_ARGC > 2) {
+		LOG_ERROR("Command takes at most two arguments");
 		return ERROR_COMMAND_SYNTAX_ERROR;
-	}
-
-	if (CMD_ARGC == 1)
+	} else if (CMD_ARGC == 1) {
 		COMMAND_PARSE_NUMBER(int, CMD_ARGV[0], irwidth);
+	} else if (CMD_ARGC == 2) {
+		COMMAND_PARSE_NUMBER(int, CMD_ARGV[0], irwidth);
+		COMMAND_PARSE_NUMBER(int, CMD_ARGV[1], tunnel_type);
+	}
+	if (tunnel_type == BSCAN_TUNNEL_NESTED_TAP)
+		LOG_INFO("Nested Tap based Bscan Tunnel Selected");
+	else if (tunnel_type == BSCAN_TUNNEL_DATA_REGISTER)
+		LOG_INFO("Simple Register based Bscan Tunnel Selected");
+	else
+		LOG_INFO("Invalid Tunnel type selected ! : selecting default Nested Tap Type");
 
+	bscan_tunnel_type = tunnel_type;
 	bscan_tunnel_ir_width = irwidth;
 	return ERROR_OK;
 }
 
-
 static const struct command_registration riscv_exec_command_handlers[] = {
 	{
 		.name = "test_compliance",
@@ -2147,10 +2198,12 @@ static const struct command_registration riscv_exec_command_handlers[] = {
 		.name = "use_bscan_tunnel",
 		.handler = riscv_use_bscan_tunnel,
 		.mode = COMMAND_ANY,
-		.usage = "riscv use_bscan_tunnel value",
+		.usage = "riscv use_bscan_tunnel value [type]",
 		.help = "Enable or disable use of a BSCAN tunnel to reach DM.  Supply "
 			"the width of the DM transport TAP's instruction register to "
-			"enable.  Supply a value of 0 to disable."
+			"enable.  Supply a value of 0 to disable. Pass A second argument "
+			"(optional) to indicate Bscan Tunnel Type {0:(default) NESTED_TAP , "
+			"1: DATA_REGISTER}"
 	},
 	COMMAND_REGISTRATION_DONE
 };
diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h
index fe1c578..53ee1f4 100644
--- a/src/target/riscv/riscv.h
+++ b/src/target/riscv/riscv.h
@@ -174,7 +174,9 @@ extern struct scan_field *bscan_tunneled_select_dmi;
 extern uint32_t bscan_tunneled_select_dmi_num_fields;
 extern uint8_t bscan_zero[4];
 extern uint8_t bscan_one[4];
+typedef enum { BSCAN_TUNNEL_NESTED_TAP, BSCAN_TUNNEL_DATA_REGISTER } bscan_tunnel_type_t;
 extern int bscan_tunnel_ir_width;
+extern bscan_tunnel_type_t bscan_tunnel_type;
 
 uint32_t dtmcontrol_scan_via_bscan(struct target *target, uint32_t out);
 void select_dmi_via_bscan(struct target *target);