contrib/flash.py: fix typos

Python has no versoin nor printf.

Change-Id: I4fd4d2592b44b3ae827e00ae73d8c451aa2ec18f

README.md

@@ -40,7 +40,7 @@ The host software includes
 * libosmo-simtrace2 - a shared library to talk to devices running the simtrace2 firmware
 * simtrace2-list - list any USB-attached devices running simtrace2 firmware
 * simtrace2-sniff - interface the 'trace' firmware to obtain card protocol traces
-* simtrace2-cardem-pcsc - interface the 'cardem' fimrware to use a SIM in a PC/SC reader
+* simtrace2-cardem-pcsc - interface the 'cardem' firmware to use a SIM in a PC/SC reader
 
 
 Do not expect SIMtrace2 to work in VMs
@@ -52,3 +52,11 @@ with USB pass-through for things with critical timing like SIMtrace2 is
 calling for trouble** and we will not accept related bug reports or
 support you if you do.  If you still want to use VMs: Feel free to do
 so, but understand that it's unsupported and you are on your own.
+
+Debug UART
+----------
+
+The debug UART configuration is 921600 8N1, TTL 3.3V.
+
+On the simtrace 2 use either a 2.5 mm stereo headphone connector (tip = TX, ring = RX, sleeve = GND)
+or the nearby DEBUG port (pin 1 = GND, pin 4 = TX, pin 5 = RX).

contrib/flash.py

@@ -110,12 +110,12 @@ for usb_device in usb_devices:
 			if serial:
 				version = "< 0.5.1.45-ac7e"
 			else:
-				versoin = "< 0.5.1.45-ac7e"
+				version = "< 0.5.1.45-ac7e"
 		else:
 			if serial:
 				version = "< 0.5.1.37-ede8"
 			else:
-				versoin = "< 0.5.1.34-e026"
+				version = "< 0.5.1.34-e026"
 	print("device firmware version: " + version)
 	# flash latest firmware
 	if to_flash == "list": # we just want to list the devices, not flash them
@@ -156,7 +156,7 @@ for usb_device in usb_devices:
 	dfu_result = subprocess.run(["dfu-util", "--device", hex(definition.usb_vendor_id) + ":" + hex(definition.usb_product_id), "--path", usb_path, "--cfg", "1", "--alt", "1", "--reset", "--download", dl_path])
 	os.remove(dl_path)
 	if 0 != dfu_result.returncode:
-		printf("flashing firmware using dfu-util failed. ensure dfu-util is installed and you have the permissions to access this USB device")
+		print("flashing firmware using dfu-util failed. ensure dfu-util is installed and you have the permissions to access this USB device")
 		continue
 	updated_nb += 1
 

firmware/libboard/qmod/source/board_qmod.c

@@ -287,7 +287,7 @@ void board_exec_dbg_cmd(int ch)
 		sim_switch_use_physical(0, 0);
 		break;
 	case '@':
-		sim_switch_use_physical(0, 0);
+		sim_switch_use_physical(1, 0);
 		break;
 	case 't':
 		talloc_report(NULL, stdout);

firmware/libcommon/include/card_emu.h

@@ -15,9 +15,11 @@
  */
 #pragma once
 
+#include <stdbool.h>
 #include <stdint.h>
 
 struct card_handle;
+struct msgb;
 
 enum card_io {
 	CARD_IO_VCC,
@@ -74,3 +76,5 @@ int card_emu_get_vcc(uint8_t uart_chan);
 struct cardemu_usb_msg_config;
 int card_emu_set_config(struct card_handle *ch, const struct cardemu_usb_msg_config *scfg,
 			unsigned int scfg_len);
+
+void usb_buf_upd_len_and_submit(struct msgb *msg);

firmware/libcommon/include/llist_irqsafe.h

@@ -34,7 +34,7 @@ static inline void llist_add_tail_irqsafe(struct llist_head *_new,
 
 	local_irq_save(x);
 	llist_add_tail(_new, head);
-	__enable_irq();
+	local_irq_restore(x);
 }
 
 static inline struct llist_head *llist_head_dequeue_irqsafe(struct llist_head *head)

firmware/libcommon/source/card_emu.c

@@ -219,19 +219,31 @@ struct card_handle {
 static void card_handle_reset(struct card_handle *ch)
 {
 	struct msgb *msg;
+	unsigned long x;
 
 	card_emu_uart_update_wt(ch->uart_chan, 0);
 
-	/* release any buffers we may still own */
-	if (ch->uart_tx_msg) {
-		usb_buf_free(ch->uart_tx_msg);
-		ch->uart_tx_msg = NULL;
-	}
+	/* Release any buffers we may still own.
+	 * uart_tx_msg + uart_tx_queue are shared with the UART IRQ handler,
+	 * that preempts us here -> needs atomic detach and free */
+	local_irq_save(x);
+	msg = ch->uart_tx_msg;
+	ch->uart_tx_msg = NULL;
+	local_irq_restore(x);
+	if (msg)
+		usb_buf_free(msg);
+
 	if (ch->uart_rx_msg) {
 		usb_buf_free(ch->uart_rx_msg);
 		ch->uart_rx_msg = NULL;
 	}
-	while ((msg = msgb_dequeue(&ch->uart_tx_queue))) {
+
+	while (1) {
+		local_irq_save(x);
+		msg = msgb_dequeue(&ch->uart_tx_queue);
+		local_irq_restore(x);
+		if (!msg)
+			break;
 		usb_buf_free(msg);
 	}
 }
@@ -376,7 +388,7 @@ static void emu_update_fidi(struct card_handle *ch)
 	int rc;
 
 	rc = iso7816_3_compute_fd_ratio(ch->F_index, ch->D_index);
-	if (rc > 0 && rc < 0x400) {
+	if (rc > 0 && rc <= (US_FIDI_FI_DI_RATIO_Msk >> US_FIDI_FI_DI_RATIO_Pos)) {
 		TRACE_INFO("%u: computed F(%u)/D(%u) ratio: %d\r\n", ch->num,
 			   ch->F_index, ch->D_index, rc);
 		/* make sure UART uses new F/D ratio */
@@ -386,6 +398,26 @@ static void emu_update_fidi(struct card_handle *ch)
 			   ch->num, rc);
 }
 
+/*! Calculate the WT from current WI and D.
+ *
+ * ISO 7816-3 10.2 defines WT = WI x 960 x Fi / f [seconds].
+ * Our waiting time is stored in units of etu = Fi / (D x f) seconds
+ * -> the Fi cancels out, but D does not.
+ *
+ *	WT [etu] = WI x 960 x D
+ *
+ * D is the value from ISO 7816-3 Table 8. Only 1..9 are defined,
+ * 0 and RFU range 10..15 have no D -> use D = 1 */
+static void emu_update_wt(struct card_handle *ch)
+{
+	uint8_t d = 1;
+
+	if (ch->D_index >= 1 && ch->D_index <= 9)
+		d = iso7816_3_di_table[ch->D_index];
+
+	ch->waiting_time = ch->wi * 960 * d;
+}
+
 /* Update the ISO 7816-3 TPDU receiver state */
 static void card_set_state(struct card_handle *ch,
 			   enum iso7816_3_card_state new_state)
@@ -505,11 +537,8 @@ static int tx_byte_atr(struct card_handle *ch)
 				}
 			}
 		}
-		/* update waiting time (see ISO 7816-3 10.2). We can drop the Fi
-		 * multiplier as we store the waiting time in units of 'etu', and
-		 * don't really care what the number of clock cycles or the absolute
-		 * wall clock time is */
-		ch->waiting_time = ch->wi * 960;
+		/* update the waiting time now that WI is known (see emu_update_wt) */
+		emu_update_wt(ch);
 		/* go to next state */
 		card_set_state(ch, ISO_S_WAIT_TPDU);
 		return 0;
@@ -675,6 +704,9 @@ static int tx_byte_pts(struct card_handle *ch)
 		card_emu_uart_wait_tx_idle(ch->uart_chan);
 		/* update baud rate generator with F/D */
 		emu_update_fidi(ch);
+		/* the waiting time is expressed in etu and scales with D, so it has
+		 * to be recomputed whenever D changes */
+		emu_update_wt(ch);
 		/* Wait for the next TPDU */
 		card_set_state(ch, ISO_S_WAIT_TPDU);
 		set_pts_state(ch, PTS_S_WAIT_REQ_PTSS);
@@ -713,7 +745,8 @@ static void add_tpdu_byte(struct card_handle *ch, uint8_t byte)
 {
 	struct msgb *msg;
 	struct cardemu_usb_msg_rx_data *rd;
-	unsigned int num_data_bytes = t0_num_data_bytes(ch->tpdu.hdr[_P3], 0);
+	/* these are bytes the reader sends to us, so P3 is a literal count */
+	unsigned int num_data_bytes = t0_num_data_bytes(ch->tpdu.hdr[_P3], 1);
 
 	/* ensure we have a buffer */
 	if (!ch->uart_rx_msg) {

firmware/libcommon/source/iso7816_fidi.c

@@ -48,9 +48,13 @@ int iso7816_3_compute_fd_ratio(uint8_t f_index, uint8_t d_index)
 	if (d == 0)
 		return -EINVAL;
 
-	/* See table 7 of ISO 7816-3: From 1000 on we divide by 1/d,
-	 * which equals a multiplication by d */
-	if (d_index < 8)
+	/* DI defined in Table 8 of ISO/IEC 7816-3:2006
+	 * has values 0001..1001 as div 1, 2, 4, 8, 16, 32, 64, 12, 20
+	 * so indices 1..9 are all divisors and the ratio is F/D.
+	 * But Indices 1010..1111 are RFU in the 2006 edition!
+	 * 1997 used those for 1/2 .. 1/64, where dividing by 1/d equals multiplying by d.
+	 * Keep that legacy interpretation for the RFU range only. */
+	if (d_index < 10)
 		ret = f / d;
 	else
 		ret = f * d;

firmware/libcommon/source/mode_cardemu.c

@@ -387,7 +387,7 @@ int card_emu_uart_update_fidi(uint8_t uart_chan, unsigned int fidi)
 	Usart *usart = get_usart_by_chan(uart_chan);
 
 	usart->US_CR |= US_CR_RXDIS | US_CR_RSTRX;
-	usart->US_FIDI = fidi & 0x3ff;
+	usart->US_FIDI = US_FIDI_FI_DI_RATIO(fidi);
 	usart->US_CR |= US_CR_RXEN | US_CR_STTTO;
 	return 0;
 }
@@ -737,7 +737,8 @@ static void dispatch_usb_command_cardem(struct msgb *msg, struct cardem_inst *ci
 	switch (hdr->msg_type) {
 	case SIMTRACE_MSGT_DT_CEMU_TX_DATA:
 		queue = card_emu_get_uart_tx_queue(ci->ch);
-		llist_add_tail(&msg->list, queue);
+		/* drained from the USART IRQ handler at highest NVIC prio */
+		llist_add_tail_irqsafe(&msg->list, queue);
 		card_emu_have_new_uart_tx(ci->ch);
 		break;
 	case SIMTRACE_MSGT_DT_CEMU_SET_ATR:

firmware/libcommon/source/simtrace_iso7816.c

@@ -127,14 +127,14 @@ void update_fidi(Usart_info *usart, uint8_t fidi)
 	uint8_t di = fidi & 0xf;
 	int ratio = iso7816_3_compute_fd_ratio(fi, di);
 
-	if (ratio > 0 && ratio < 0x8000) {
+	if (ratio > 0 && ratio <= (US_FIDI_FI_DI_RATIO_Msk >> US_FIDI_FI_DI_RATIO_Pos)) {
 		/* make sure USART uses new F/D ratio */
 		usart->base->US_CR |= US_CR_RXDIS | US_CR_RSTRX;
 		/* disable write protection */
 		if (usart->base->US_WPMR) {
 			usart->base->US_WPMR = US_WPMR_WPKEY(0x555341);
 		}
-		usart->base->US_FIDI = (ratio & 0x7ff);
+		usart->base->US_FIDI = US_FIDI_FI_DI_RATIO(ratio);
 		usart->base->US_CR |= US_CR_RXEN | US_CR_STTTO;
 		//TRACE_INFO("updated USART(%u) Fi(%u)/Di(%u) ratio(%d): %u\n\r", usart->id, fi, di, ratio, usart->base->US_FIDI);
 	} else {

firmware/libcommon/source/sniffer.c

@@ -239,14 +239,14 @@ static void update_wt(uint8_t wi, uint8_t d, const char *cause)
 static struct msgb *usb_msg_alloc_hdr(uint8_t ep, uint8_t msg_class, uint8_t msg_type)
 {
 	/* Only allocate message if not too many are already in the queue */
-	struct llist_head *head = usb_get_queue(SIMTRACE_USB_EP_CARD_DATAIN);
+	struct llist_head *head = usb_get_queue(ep);
 	if (!head) {
 		return NULL;
 	}
 	if (llist_count(head) > 5) {
 		return NULL;
 	}
-	struct msgb *usb_msg = usb_buf_alloc(SIMTRACE_USB_EP_CARD_DATAIN);
+	struct msgb *usb_msg = usb_buf_alloc(ep);
 	if (!usb_msg) {
 		return NULL;
 	}
@@ -406,7 +406,7 @@ static void usb_send_atr(uint32_t flags)
 		TRACE_WARNING("Can't print ATR in ISO 7816-3 state %u\n\r", iso_state);
 		return;
 	}
-	if (g_atr.atr_i >= ARRAY_SIZE(g_atr.atr)) {
+	if (g_atr.atr_i > ARRAY_SIZE(g_atr.atr)) {
 		TRACE_ERROR("ATR buffer overflow\n\r");
 		return;
 	}
@@ -775,7 +775,7 @@ static void process_byte_tpdu(uint8_t byte)
 		} else if (g_tpdu.packet[1] == byte) { /* get all remaining data bytes */
 			change_tpdu_state(TPDU_S_DATA_REMAINING);
 			break;
-		} else if ((~g_tpdu.packet[1]) == byte) { /* get single data byte */
+		} else if ((uint8_t)(~g_tpdu.packet[1]) == byte) { /* get single data byte */
 			change_tpdu_state(TPDU_S_DATA_SINGLE);
 			break;
 		}

firmware/test/card_emu_tests.c

@@ -396,12 +396,73 @@ const uint8_t pps[] = {
 	0xFF ^ 0b00010000// PCK
 };
 
-static void
-test_ppss(struct card_handle *ch)
+/* Fi/Di that is actually valid: Fi idx 9 (Fi=512) and Di idx 4 (Di=8)
+ * This tests calculating F/D ratio (512/8 = 64) and the calculation of the
+ * waiting time which scales with Di. */
+const uint8_t pps_fidi[] = {
+	0xFF,			// PPSS
+	0b00010000,		// PPS0: PPS1 present
+	0x94,			// PPS1: Fi index 9, Di index 4
+	0xFF ^ 0b00010000 ^ 0x94// PCK
+};
+
+/* Di 8 idx Di=12 (ISO 7816-3:2006 Table 8),:
+ * the ratio be 372/12 = 31, not 372*12. */
+const uint8_t pps_di12[] = {
+	0xFF,			// PPSS
+	0b00010000,		// PPS0: PPS1 present
+	0x18,			// PPS1: Fi index 1, Di index 8
+	0xFF ^ 0b00010000 ^ 0x18// PCK
+};
+
+/* Fi idx 5 Fi=1488 andDi=1 -> ratio 1488, needs all 11 bits of
+ * the US_FIDI.FI_DI_RATIO field. */
+const uint8_t pps_hi_ratio[] = {
+	0xFF,			// PPSS
+	0b00010000,		// PPS0: PPS1 present
+	0x51,			// PPS1: Fi index 5, Di index 1
+	0xFF ^ 0b00010000 ^ 0x51// PCK
+};
+
+/* Get a cemu status report to check the negotiated parameters
+ * This is the only way to get the waiting time from struct card_handle. */
+static void verify_status(struct card_handle *ch, uint8_t exp_f_index, uint8_t exp_d_index,
+			  uint32_t exp_waiting_time)
 {
-	reader_send_bytes(ch, pps, sizeof(pps));
-	get_and_verify_rctx_pps(pps, sizeof(pps));
-	card_tx_verify_chars(ch, pps, sizeof(pps));
+	struct usb_buffered_ep *bep = usb_get_buf_ep(PHONE_DATAIN);
+	struct cardemu_usb_msg_status *sts;
+	struct simtrace_msg_hdr *mh;
+	struct msgb *msg;
+
+	card_emu_report_status(ch, false);
+
+	assert(bep);
+	msg = msgb_dequeue_count(&bep->queue, &bep->queue_len);
+	assert(msg);
+	mh = (struct simtrace_msg_hdr *) msg->l1h;
+	assert(mh->msg_type == SIMTRACE_MSGT_BD_CEMU_STATUS);
+	sts = (struct cardemu_usb_msg_status *) msg->l2h;
+
+	printf("status: F_index=%u D_index=%u wi=%u waiting_time=%u\n",
+		sts->F_index, sts->D_index, sts->wi, sts->waiting_time);
+
+	assert(sts->F_index == exp_f_index);
+	assert(sts->D_index == exp_d_index);
+	/* WT = WI x 960 x D in etu, see ISO 7816-3 Section 10.2 */
+	assert(sts->waiting_time == exp_waiting_time);
+
+	usb_buf_free(msg);
+}
+
+static void
+test_ppss(struct card_handle *ch, const uint8_t *req, unsigned int req_len,
+	  uint8_t exp_f_index, uint8_t exp_d_index, uint32_t exp_waiting_time)
+{
+	printf("\n==> PPS exchange\n");
+	reader_send_bytes(ch, req, req_len);
+	get_and_verify_rctx_pps(req, req_len);
+	card_tx_verify_chars(ch, req, req_len);
+	verify_status(ch, exp_f_index, exp_d_index, exp_waiting_time);
 }
 
 /* READ RECORD (offset 0, 10 bytes) */
@@ -426,7 +487,12 @@ int main(int argc, char **argv)
 	io_start_card(ch);
 	card_tx_verify_chars(ch, NULL, 0);
 
-	test_ppss(ch);
+	/* WI is 10 so WT = 10 x 960 x D */
+	/* Fi/Di index 0/0 is invalid: the F/D ratio is rejected, D falls back to 1 */
+	test_ppss(ch, pps, sizeof(pps), 0, 0, 10 * 960 * 1);
+	test_ppss(ch, pps_fidi, sizeof(pps_fidi), 9, 4, 10 * 960 * 8);
+	test_ppss(ch, pps_di12, sizeof(pps_di12), 1, 8, 10 * 960 * 12);
+	test_ppss(ch, pps_hi_ratio, sizeof(pps_hi_ratio), 5, 1, 10 * 960 * 1);
 
 	for (i = 0; i < 2; i++) {
 		test_tpdu_reader2card(ch, tpdu_hdr_write_rec, tpdu_body_write_rec, sizeof(tpdu_body_write_rec));

host/lib/apdu_dispatch.c

@@ -63,7 +63,7 @@ const char *osmo_apdu_dump_context_buf(char *buf, unsigned int buf_len,
 }
 
 /*! \brief input function for APDU segmentation
- *  \param ac APDU context accross successive calls
+ *  \param ac APDU context across successive calls
  *  \param[in] apdu_buf APDU inpud data buffer
  *  \param[in] apdu_len Length of apdu_buf
  *  \param[in] new_apdu Is this the beginning of a new APDU?

host/lib/simtrace2_api.c

@@ -280,7 +280,7 @@ int osmo_st2_cardem_request_config2(struct osmo_st2_cardem_inst *ci, const struc
 
 	tx_cfg = (struct cardemu_usb_msg_config *) msgb_put(msg, sizeof(*tx_cfg));
 
-	LOGSLOT(ci->slot, LOGL_NOTICE, "<= %s(features=%08x)\n", __func__, tx_cfg->features);
+	LOGSLOT(ci->slot, LOGL_NOTICE, "<= %s(features=%08x)\n", __func__, user_cfg->features);
 	memcpy(tx_cfg, user_cfg, sizeof(*tx_cfg));
 	osmo_store32le(user_cfg->features, &tx_cfg->features);
 

host/src/simtrace2-sniff.c

@@ -328,7 +328,7 @@ static void signal_handler(int signal)
 
 int main(int argc, char **argv)
 {
-	int i, rc, ret;
+	int i, rc, ret = 1;
 	print_welcome();
 
 	/* Parse arguments */