November 2022 - gerrit-log - lists.osmocom.org

Change in simtrace2[master]: firmware/sniffer: Enable interrupts for overrun/parity/frame errors

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/30200 ) Change subject: firmware/sniffer: Enable interrupts for overrun/parity/frame errors ...................................................................... firmware/sniffer: Enable interrupts for overrun/parity/frame errors We so far didn't have interrupts enabled for those, and just caught them "by accident" if a byte was received or if a timeout happened. Let's explicitly enable those interrupts so we also catch those conditions by themselves. Change-Id: Ia27f537706b9a6252dd18175545c6f27a7d17d0e --- M firmware/libcommon/source/sniffer.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: laforge: Looks good to me, approved Hoernchen: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c index 35de58f..948eee8 100644 --- a/firmware/libcommon/source/sniffer.c +++ b/firmware/libcommon/source/sniffer.c @@ -918,7 +918,7 @@ * Initialization routine *-----------------------------------------------------------------------------*/ -#define SNIFFER_IER (US_IER_RXRDY | US_IER_TIMEOUT) +#define SNIFFER_IER (US_IER_RXRDY | US_IER_TIMEOUT | US_IER_OVRE | US_IER_FRAME | US_IER_PARE) /* Called during USB enumeration after device is enumerated by host */ void Sniffer_configure(void) -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/30200 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: Ia27f537706b9a6252dd18175545c6f27a7d17d0e Gerrit-Change-Number: 30200 Gerrit-PatchSet: 2 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: tsaitgaist <kredon(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 7 months

1
0
0 0

Change in simtrace2[master]: firmware/sniffer: Handle WT timeouts via ring-buffer

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/30198 ) Change subject: firmware/sniffer: Handle WT timeouts via ring-buffer ...................................................................... firmware/sniffer: Handle WT timeouts via ring-buffer Before this patch, all UART characters went through a fifo/ringbuffer of depth 512, while events like timeout were delivered directly via a global flags variable from ISR to main code. This means that one or more correct/complete TPDUs could theoretically still be in the FIFO, but the "Fast path" of the timeout handling is pre-empting that and messing with the state machines. All events from the UART should be delivered via the ring-buffer to make sure they arrive in order at the main function. The old "report timeout via change flags in separate USB message" code is left in place. On the USB protocol we should keep it for compatibility. Internally we should probably also migrate that over to the new ring-buffer method in a second step. Change-Id: I4434c6fcd59d1a425e9ded734bbc8b0411a0a0d8 --- M firmware/libcommon/source/sniffer.c 1 file changed, 73 insertions(+), 66 deletions(-) Approvals: laforge: Looks good to me, approved Hoernchen: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c index e87e9b8..75c140c 100644 --- a/firmware/libcommon/source/sniffer.c +++ b/firmware/libcommon/source/sniffer.c @@ -1,6 +1,6 @@ /* SIMtrace 2 sniffer mode * - * (C) 2016-2017 by Harald Welte <hwelte(a)hmw-consulting.de> + * (C) 2016-2022 by Harald Welte <hwelte(a)hmw-consulting.de> * (C) 2018 by sysmocom -s.f.m.c. GmbH, Author: Kevin Redon <kredon(a)sysmocom.de> * * This program is free software; you can redistribute it and/or modify @@ -115,6 +115,8 @@ #define RBUF16_F_OVERRUN 0x0100 #define RBUF16_F_FRAMING 0x0200 #define RBUF16_F_PARITY 0x0400 +#define RBUF16_F_TIMEOUT_WT 0x0800 +#define RBUF16_F_DATA_BYTE 0x8000 /*------------------------------------------------------------------------------ * Internal variables @@ -825,13 +827,11 @@ uint32_t csr = sniff_usart.base->US_CSR; uint16_t byte = 0; - bool byte_received = false; /* Verify if character has been received */ if (csr & US_CSR_RXRDY) { - byte_received = true; /* Read communication data byte between phone and SIM */ - byte = sniff_usart.base->US_RHR; + byte = RBUF16_F_DATA_BYTE | (sniff_usart.base->US_RHR & 0xff); /* Reset WT timer */ wt_remaining = g_wt; } @@ -847,15 +847,11 @@ if (csr & (US_CSR_OVRE|US_CSR_FRAME|US_CSR_PARE)) sniff_usart.base->US_CR |= US_CR_RSTSTA; - /* Store sniffed data (or error flags, or both) into buffer */ - if (byte_received || byte) { - if (rbuf16_write(&sniff_buffer, byte) != 0) - TRACE_ERROR("USART buffer full\n\r"); - } - /* Verify it WT timeout occurred, to detect unresponsive card */ if (csr & US_CSR_TIMEOUT) { if (wt_remaining <= (sniff_usart.base->US_RTOR & 0xffff)) { + /* ensure the timeout is enqueued in the ring-buffer */ + byte |= RBUF16_F_TIMEOUT_WT; /* Just set the flag and let the main loop handle it */ change_flags |= SNIFF_CHANGE_FLAG_TIMEOUT_WT; /* Reset timeout value */ @@ -875,6 +871,12 @@ sniff_usart.base->US_CR |= US_CR_RETTO; } } + + /* Store sniffed data (or error flags, or both) into buffer */ + if (byte) { + if (rbuf16_write(&sniff_buffer, byte) != 0) + TRACE_ERROR("USART buffer full\n\r"); + } } /** PIO interrupt service routine to checks if the card reset line has changed @@ -1023,43 +1025,72 @@ /* Handle sniffed data */ if (!rbuf16_is_empty(&sniff_buffer)) { /* use if instead of while to let the main loop restart the watchdog */ uint16_t entry = rbuf16_read(&sniff_buffer); - uint8_t byte = entry & 0xff; - /* Convert convention if required */ - if (convention_convert) { - byte = convention_convert_lut[byte]; - } - //TRACE_ERROR_WP(">%02x", byte); - switch (iso_state) { /* Handle byte depending on state */ - case ISO7816_S_RESET: /* During reset we shouldn't receive any data */ - break; - case ISO7816_S_WAIT_ATR: /* After a reset we expect the ATR */ - change_state(ISO7816_S_IN_ATR); /* go to next state */ - case ISO7816_S_IN_ATR: /* More ATR data incoming */ - process_byte_atr(byte); - break; - case ISO7816_S_WAIT_TPDU: /* After the ATR we expect TPDU or PPS data */ - case ISO7816_S_WAIT_PPS_RSP: - if (0xff == byte) { - if (ISO7816_S_WAIT_PPS_RSP == iso_state) { - change_state(ISO7816_S_IN_PPS_RSP); /* Go to PPS state */ - } else { - change_state(ISO7816_S_IN_PPS_REQ); /* Go to PPS state */ + + if (entry & RBUF16_F_DATA_BYTE) { + uint8_t byte = entry & 0xff; + /* Convert convention if required */ + if (convention_convert) { + byte = convention_convert_lut[byte]; + } + + //TRACE_ERROR_WP(">%02x", byte); + switch (iso_state) { /* Handle byte depending on state */ + case ISO7816_S_RESET: /* During reset we shouldn't receive any data */ + break; + case ISO7816_S_WAIT_ATR: /* After a reset we expect the ATR */ + change_state(ISO7816_S_IN_ATR); /* go to next state */ + case ISO7816_S_IN_ATR: /* More ATR data incoming */ + process_byte_atr(byte); + break; + case ISO7816_S_WAIT_TPDU: /* After the ATR we expect TPDU or PPS data */ + case ISO7816_S_WAIT_PPS_RSP: + if (0xff == byte) { + if (ISO7816_S_WAIT_PPS_RSP == iso_state) { + change_state(ISO7816_S_IN_PPS_RSP); /* Go to PPS state */ + } else { + change_state(ISO7816_S_IN_PPS_REQ); /* Go to PPS state */ + } + process_byte_pps(byte); + break; } + case ISO7816_S_IN_TPDU: /* More TPDU data incoming */ + if (ISO7816_S_WAIT_TPDU == iso_state) { + change_state(ISO7816_S_IN_TPDU); + } + process_byte_tpdu(byte); + break; + case ISO7816_S_IN_PPS_REQ: + case ISO7816_S_IN_PPS_RSP: process_byte_pps(byte); break; + default: + TRACE_ERROR("Data received in unknown state %u\n\r", iso_state); } - case ISO7816_S_IN_TPDU: /* More TPDU data incoming */ - if (ISO7816_S_WAIT_TPDU == iso_state) { - change_state(ISO7816_S_IN_TPDU); + } + + /* Use timeout to detect interrupted data transmission */ + if (entry & RBUF16_F_TIMEOUT_WT) { + TRACE_ERROR("USART TIMEOUT Error\n\r"); + switch (iso_state) { + case ISO7816_S_IN_ATR: + led_blink(LED_RED, BLINK_2F_O); /* indicate error to user */ + usb_send_atr(SNIFF_DATA_FLAG_ERROR_INCOMPLETE); /* send incomplete ATR to host software using USB */ + change_state(ISO7816_S_WAIT_ATR); + break; + case ISO7816_S_IN_TPDU: + led_blink(LED_RED, BLINK_2F_O); /* indicate error to user */ + usb_send_tpdu(SNIFF_DATA_FLAG_ERROR_INCOMPLETE); /* send incomplete PPS to host software using USB */ + change_state(ISO7816_S_WAIT_TPDU); + break; + case ISO7816_S_IN_PPS_REQ: + case ISO7816_S_IN_PPS_RSP: + led_blink(LED_RED, BLINK_2F_O); /* indicate error to user */ + usb_send_pps(SNIFF_DATA_FLAG_ERROR_INCOMPLETE); /* send incomplete TPDU to host software using USB */ + change_state(ISO7816_S_WAIT_TPDU); + break; + default: + break; } - process_byte_tpdu(byte); - break; - case ISO7816_S_IN_PPS_REQ: - case ISO7816_S_IN_PPS_RSP: - process_byte_pps(byte); - break; - default: - TRACE_ERROR("Data received in unknown state %u\n\r", iso_state); } if (entry & RBUF16_F_PARITY) @@ -1101,30 +1132,6 @@ printf("reset de-asserted\n\r"); } } - if (change_flags & SNIFF_CHANGE_FLAG_TIMEOUT_WT) { - /* Use timeout to detect interrupted data transmission */ - switch (iso_state) { - case ISO7816_S_IN_ATR: - led_blink(LED_RED, BLINK_2F_O); /* indicate error to user */ - usb_send_atr(SNIFF_DATA_FLAG_ERROR_INCOMPLETE); /* send incomplete ATR to host software using USB */ - change_state(ISO7816_S_WAIT_ATR); - break; - case ISO7816_S_IN_TPDU: - led_blink(LED_RED, BLINK_2F_O); /* indicate error to user */ - usb_send_tpdu(SNIFF_DATA_FLAG_ERROR_INCOMPLETE); /* send incomplete PPS to host software using USB */ - change_state(ISO7816_S_WAIT_TPDU); - break; - case ISO7816_S_IN_PPS_REQ: - case ISO7816_S_IN_PPS_RSP: - led_blink(LED_RED, BLINK_2F_O); /* indicate error to user */ - usb_send_pps(SNIFF_DATA_FLAG_ERROR_INCOMPLETE); /* send incomplete TPDU to host software using USB */ - change_state(ISO7816_S_WAIT_TPDU); - break; - default: - change_flags &= ~SNIFF_CHANGE_FLAG_TIMEOUT_WT; /* We don't care about the timeout is all other cases */ - break; - } - } if (change_flags) { usb_send_change(change_flags); /* send timeout to host software over USB */ change_flags = 0; /* Reset flags */ -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/30198 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: I4434c6fcd59d1a425e9ded734bbc8b0411a0a0d8 Gerrit-Change-Number: 30198 Gerrit-PatchSet: 2 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: tsaitgaist <kredon(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 7 months

1
0
0 0

Change in simtrace2[master]: firmware/sniffer: Pass PARITY/OVERRUN/FRAMING error via ringbuffer

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/30193 ) Change subject: firmware/sniffer: Pass PARITY/OVERRUN/FRAMING error via ringbuffer ...................................................................... firmware/sniffer: Pass PARITY/OVERRUN/FRAMING error via ringbuffer those kind of errors should be passed to the main loop for further processing, in-order together with the byte stream received from the USART. Change-Id: Iebd9bbc97c2a5a0c402e7a2711520299a1ade568 --- M firmware/libcommon/include/simtrace_prot.h M firmware/libcommon/source/sniffer.c 2 files changed, 37 insertions(+), 12 deletions(-) Approvals: laforge: Looks good to me, approved Hoernchen: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/firmware/libcommon/include/simtrace_prot.h b/firmware/libcommon/include/simtrace_prot.h index 5c43b06..d1eb89b 100644 --- a/firmware/libcommon/include/simtrace_prot.h +++ b/firmware/libcommon/include/simtrace_prot.h @@ -313,6 +313,9 @@ #define SNIFF_DATA_FLAG_ERROR_INCOMPLETE (1<<5) #define SNIFF_DATA_FLAG_ERROR_MALFORMED (1<<6) #define SNIFF_DATA_FLAG_ERROR_CHECKSUM (1<<7) +#define SNIFF_DATA_FLAG_ERROR_OVERRUN (1<<8) +#define SNIFF_DATA_FLAG_ERROR_FRAMING (1<<9) +#define SNIFF_DATA_FLAG_ERROR_PARITY (1<<10) /* SIMTRACE_MSGT_SNIFF_CHANGE */ struct sniff_change { diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c index bea4d95..e87e9b8 100644 --- a/firmware/libcommon/source/sniffer.c +++ b/firmware/libcommon/source/sniffer.c @@ -111,6 +111,11 @@ TPDU_S_SW2, /*!< second status word */ }; +/*! Error flags we use to report USART errors via the ringbuffer */ +#define RBUF16_F_OVERRUN 0x0100 +#define RBUF16_F_FRAMING 0x0200 +#define RBUF16_F_PARITY 0x0400 + /*------------------------------------------------------------------------------ * Internal variables *------------------------------------------------------------------------------*/ @@ -818,23 +823,32 @@ /* Read channel status register */ uint32_t csr = sniff_usart.base->US_CSR; - /* Verify if there was an error */ - if (csr & US_CSR_OVRE) - TRACE_ERROR("USART overrun error\n\r"); - if (csr & US_CSR_FRAME) - TRACE_ERROR("USART framing error\n\r"); - if (csr & US_CSR_PARE) - TRACE_ERROR("USART parity error\n\r"); - if (csr & (US_CSR_OVRE|US_CSR_FRAME|US_CSR_PARE)) - sniff_usart.base->US_CR |= US_CR_RSTSTA; + + uint16_t byte = 0; + bool byte_received = false; /* Verify if character has been received */ if (csr & US_CSR_RXRDY) { + byte_received = true; /* Read communication data byte between phone and SIM */ - uint8_t byte = sniff_usart.base->US_RHR; + byte = sniff_usart.base->US_RHR; /* Reset WT timer */ wt_remaining = g_wt; - /* Store sniffed data into buffer (also clear interrupt */ + } + + /* Verify if there was an error */ + if (csr & US_CSR_OVRE) + byte |= RBUF16_F_OVERRUN; + if (csr & US_CSR_FRAME) + byte |= RBUF16_F_FRAMING; + if (csr & US_CSR_PARE) + byte |= RBUF16_F_PARITY; + + if (csr & (US_CSR_OVRE|US_CSR_FRAME|US_CSR_PARE)) + sniff_usart.base->US_CR |= US_CR_RSTSTA; + + /* Store sniffed data (or error flags, or both) into buffer */ + if (byte_received || byte) { if (rbuf16_write(&sniff_buffer, byte) != 0) TRACE_ERROR("USART buffer full\n\r"); } @@ -1008,7 +1022,8 @@ */ /* Handle sniffed data */ if (!rbuf16_is_empty(&sniff_buffer)) { /* use if instead of while to let the main loop restart the watchdog */ - uint8_t byte = rbuf16_read(&sniff_buffer); + uint16_t entry = rbuf16_read(&sniff_buffer); + uint8_t byte = entry & 0xff; /* Convert convention if required */ if (convention_convert) { byte = convention_convert_lut[byte]; @@ -1046,6 +1061,13 @@ default: TRACE_ERROR("Data received in unknown state %u\n\r", iso_state); } + + if (entry & RBUF16_F_PARITY) + TRACE_ERROR("USART PARITY Error\r\n"); + if (entry & RBUF16_F_FRAMING) + TRACE_ERROR("USART FRAMING Error\r\n"); + if (entry & RBUF16_F_OVERRUN) + TRACE_ERROR("USART OVERRUN Error\r\n"); } /* Handle flags */ -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/30193 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: Iebd9bbc97c2a5a0c402e7a2711520299a1ade568 Gerrit-Change-Number: 30193 Gerrit-PatchSet: 2 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: tsaitgaist <kredon(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 7 months

1
0
0 0

Change in simtrace2[master]: firmware/sniffer: Add + use 16bit ringbuffer

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/30192 ) Change subject: firmware/sniffer: Add + use 16bit ringbuffer ...................................................................... firmware/sniffer: Add + use 16bit ringbuffer So far, we use a uint8_t ring buffer as "FIFO" between USART Rx interrupt and main context. That's fine for expressing the bytes we receive. However, if we also want to report USART errors synchronously in that stream, we actually need more bits to express those. Reporting USART errors via the ring buffer is the only way how the sniffer code can know in which TPDU the error occurred. Reporting them any other way (global variable, ...) would loose the timing relationship where in the received stream the error occurred. This change just changes the ringbuffer from 1024-entry 8bit to 512-entry 16bit and doesn't add any error reporting. Change-Id: Ifde054fbfe7f753b61e7d3409c56eca6e0faeb4b --- M firmware/libcommon/include/ringbuffer.h M firmware/libcommon/source/ringbuffer.c M firmware/libcommon/source/sniffer.c 3 files changed, 88 insertions(+), 6 deletions(-) Approvals: laforge: Looks good to me, approved; Verified Hoernchen: Looks good to me, but someone else must approve diff --git a/firmware/libcommon/include/ringbuffer.h b/firmware/libcommon/include/ringbuffer.h index 51af3b2..99fe96b 100644 --- a/firmware/libcommon/include/ringbuffer.h +++ b/firmware/libcommon/include/ringbuffer.h @@ -32,4 +32,22 @@ bool rbuf_is_empty(volatile ringbuf * rb); bool rbuf_is_full(volatile ringbuf * rb); + +/* same as above but with 16bit values instead of 8bit */ + +#define RING16_BUFLEN 512 + +typedef struct ringbuf16 { + uint16_t buf[RING16_BUFLEN]; + size_t ird; + size_t iwr; +} ringbuf16; + +void rbuf16_reset(volatile ringbuf16 * rb); +uint16_t rbuf16_read(volatile ringbuf16 * rb); +uint16_t rbuf16_peek(volatile ringbuf16 * rb); +int rbuf16_write(volatile ringbuf16 * rb, uint16_t item); +bool rbuf16_is_empty(volatile ringbuf16 * rb); +bool rbuf16_is_full(volatile ringbuf16 * rb); + #endif /* end of include guard: SIMTRACE_RINGBUF_H */ diff --git a/firmware/libcommon/source/ringbuffer.c b/firmware/libcommon/source/ringbuffer.c index 591d31f..d3690be 100644 --- a/firmware/libcommon/source/ringbuffer.c +++ b/firmware/libcommon/source/ringbuffer.c @@ -28,6 +28,16 @@ local_irq_restore(state); } +void rbuf16_reset(volatile ringbuf16 * rb) +{ + unsigned long state; + + local_irq_save(state); + rb->ird = 0; + rb->iwr = 0; + local_irq_restore(state); +} + uint8_t rbuf_read(volatile ringbuf * rb) { unsigned long state; @@ -41,21 +51,49 @@ return val; } +uint16_t rbuf16_read(volatile ringbuf16 * rb) +{ + unsigned long state; + uint16_t val; + + local_irq_save(state); + val = rb->buf[rb->ird]; + rb->ird = (rb->ird + 1) % RING16_BUFLEN; + local_irq_restore(state); + + return val; +} + uint8_t rbuf_peek(volatile ringbuf * rb) { return rb->buf[rb->ird]; } +uint16_t rbuf16_peek(volatile ringbuf16 * rb) +{ + return rb->buf[rb->ird]; +} + bool rbuf_is_empty(volatile ringbuf * rb) { return rb->ird == rb->iwr; } +bool rbuf16_is_empty(volatile ringbuf16 * rb) +{ + return rb->ird == rb->iwr; +} + static bool __rbuf_is_full(volatile ringbuf * rb) { return rb->ird == (rb->iwr + 1) % RING_BUFLEN; } +static bool __rbuf16_is_full(volatile ringbuf16 * rb) +{ + return rb->ird == (rb->iwr + 1) % RING16_BUFLEN; +} + bool rbuf_is_full(volatile ringbuf * rb) { unsigned long state; @@ -68,6 +106,18 @@ return rc; } +bool rbuf16_is_full(volatile ringbuf16 * rb) +{ + unsigned long state; + bool rc; + + local_irq_save(state); + rc = rb->ird == (rb->iwr + 1) % RING16_BUFLEN; + local_irq_restore(state); + + return rc; +} + int rbuf_write(volatile ringbuf * rb, uint8_t item) { unsigned long state; @@ -84,4 +134,18 @@ } } +int rbuf16_write(volatile ringbuf16 * rb, uint16_t item) +{ + unsigned long state; + local_irq_save(state); + if (!__rbuf16_is_full(rb)) { + rb->buf[rb->iwr] = item; + rb->iwr = (rb->iwr + 1) % RING16_BUFLEN; + local_irq_restore(state); + return 0; + } else { + local_irq_restore(state); + return -1; + } +} diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c index 153c483..bea4d95 100644 --- a/firmware/libcommon/source/sniffer.c +++ b/firmware/libcommon/source/sniffer.c @@ -138,7 +138,7 @@ .state = USART_RCV, }; /*! Ring buffer to store sniffer communication data */ -static struct ringbuf sniff_buffer; +static struct ringbuf16 sniff_buffer; /* Flags to know is the card status changed (see SIMTRACE_MSGT_DT_SNIFF_CHANGE flags) */ static volatile uint32_t change_flags = 0; @@ -291,7 +291,7 @@ update_wt(10, 1, "RESET"); /* reset WT time-out */ break; case ISO7816_S_WAIT_ATR: - rbuf_reset(&sniff_buffer); /* reset buffer for new communication */ + rbuf16_reset(&sniff_buffer); /* reset buffer for new communication */ break; case ISO7816_S_IN_ATR: g_atr.atr_i = 0; @@ -835,7 +835,7 @@ /* Reset WT timer */ wt_remaining = g_wt; /* Store sniffed data into buffer (also clear interrupt */ - if (rbuf_write(&sniff_buffer, byte) != 0) + if (rbuf16_write(&sniff_buffer, byte) != 0) TRACE_ERROR("USART buffer full\n\r"); } @@ -941,7 +941,7 @@ PIO_EnableIt(&pin_rst); /* Clear ring buffer containing the sniffed data */ - rbuf_reset(&sniff_buffer); + rbuf16_reset(&sniff_buffer); /* Configure USART to as ISO-7816 slave communication to sniff communication */ ISO7816_Init(&sniff_usart, CLK_SLAVE); /* Only receive data when sniffing */ @@ -1007,8 +1007,8 @@ * is remaining */ /* Handle sniffed data */ - if (!rbuf_is_empty(&sniff_buffer)) { /* use if instead of while to let the main loop restart the watchdog */ - uint8_t byte = rbuf_read(&sniff_buffer); + if (!rbuf16_is_empty(&sniff_buffer)) { /* use if instead of while to let the main loop restart the watchdog */ + uint8_t byte = rbuf16_read(&sniff_buffer); /* Convert convention if required */ if (convention_convert) { byte = convention_convert_lut[byte]; -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/30192 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: Ifde054fbfe7f753b61e7d3409c56eca6e0faeb4b Gerrit-Change-Number: 30192 Gerrit-PatchSet: 2 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: tsaitgaist <kredon(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 7 months

1
0
0 0

Change in simtrace2[master]: firmware/sniffer: Rename global variable 'wt' to 'g_wt'

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/30188 ) Change subject: firmware/sniffer: Rename global variable 'wt' to 'g_wt' ...................................................................... firmware/sniffer: Rename global variable 'wt' to 'g_wt' It's a bad idea to have a two-character global variable which might easily clash with local variable names. Change-Id: Ic2fac64129d2772a1923f35e48582be3b130a0f2 --- M firmware/libcommon/source/sniffer.c 1 file changed, 6 insertions(+), 6 deletions(-) Approvals: laforge: Looks good to me, approved Hoernchen: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c index 1368f0b..cedf634 100644 --- a/firmware/libcommon/source/sniffer.c +++ b/firmware/libcommon/source/sniffer.c @@ -189,7 +189,7 @@ /*! Waiting Time (WT) * @note defined in ISO/IEC 7816-3:2006(E) section 8.1 and 10.2 */ -static uint32_t wt = 9600; +static uint32_t g_wt = 9600; /*------------------------------------------------------------------------------ * Internal functions @@ -219,8 +219,8 @@ if (0 != d) { wt_d = d; } - wt = wt_wi * 960UL * wt_d; - TRACE_INFO("WT updated (wi=%u, d=%u, cause=%s) to %lu ETU\n\r", wi, d, cause, wt); + g_wt = wt_wi * 960UL * wt_d; + TRACE_INFO("WT updated (wi=%u, d=%u, cause=%s) to %lu ETU\n\r", wi, d, cause, g_wt); } /*! Allocate USB buffer and push + initialize simtrace_msg_hdr @@ -833,7 +833,7 @@ /* Read communication data byte between phone and SIM */ uint8_t byte = sniff_usart.base->US_RHR; /* Reset WT timer */ - wt_remaining = wt; + wt_remaining = g_wt; /* Store sniffed data into buffer (also clear interrupt */ if (rbuf_write(&sniff_buffer, byte) != 0) TRACE_ERROR("USART buffer full\n\r"); @@ -845,7 +845,7 @@ /* Just set the flag and let the main loop handle it */ change_flags |= SNIFF_CHANGE_FLAG_TIMEOUT_WT; /* Reset timeout value */ - wt_remaining = wt; + wt_remaining = g_wt; } else { wt_remaining -= (sniff_usart.base->US_RTOR & 0xffff); /* be sure to subtract the actual timeout since the new might not have been set and reloaded yet */ } @@ -947,7 +947,7 @@ /* Only receive data when sniffing */ USART_SetReceiverEnabled(sniff_usart.base, 1); /* Enable Receiver time-out to detect waiting time (WT) time-out (e.g. unresponsive cards) */ - sniff_usart.base->US_RTOR = wt; + sniff_usart.base->US_RTOR = g_wt; /* Enable interrupt to indicate when data has been received or timeout occurred */ USART_EnableIt(sniff_usart.base, US_IER_RXRDY | US_IER_TIMEOUT); /* Set USB priority lower than USART to not miss sniffing data (both at 0 per default) */ -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/30188 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: Ic2fac64129d2772a1923f35e48582be3b130a0f2 Gerrit-Change-Number: 30188 Gerrit-PatchSet: 3 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: tsaitgaist <kredon(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 7 months

1
0
0 0

Change in simtrace2[master]: firmware/sniffer: Disable TIMEOUT interrupts in USART IER on exit

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/30189 ) Change subject: firmware/sniffer: Disable TIMEOUT interrupts in USART IER on exit ...................................................................... firmware/sniffer: Disable TIMEOUT interrupts in USART IER on exit Not critical (we disable the USART interrupts in NVIC anyway), but if Sniffer_init() enables this flag, it's good style for Sniffer_exit() to disable it. Change-Id: I92e16a160d60fcab33c81e0cf074088b9f20b9ae --- M firmware/libcommon/source/sniffer.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: laforge: Looks good to me, approved Hoernchen: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c index cedf634..153c483 100644 --- a/firmware/libcommon/source/sniffer.c +++ b/firmware/libcommon/source/sniffer.c @@ -913,7 +913,7 @@ { TRACE_INFO("Sniffer exit\n\r"); /* Disable USART */ - USART_DisableIt(sniff_usart.base, US_IER_RXRDY); + USART_DisableIt(sniff_usart.base, US_IER_RXRDY | US_IER_TIMEOUT); /* NOTE: don't forget to set the IRQ according to the USART peripheral used */ NVIC_DisableIRQ(IRQ_USART_SIM); USART_SetReceiverEnabled(sniff_usart.base, 0); -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/30189 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: I92e16a160d60fcab33c81e0cf074088b9f20b9ae Gerrit-Change-Number: 30189 Gerrit-PatchSet: 3 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: tsaitgaist <kredon(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 7 months

1
0
0 0

Change in simtrace2[master]: firmware/sniffer: Log cause of WT change

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/30187 ) Change subject: firmware/sniffer: Log cause of WT change ...................................................................... firmware/sniffer: Log cause of WT change Change-Id: I14245c0ca96a258146e48bb9909efd9f8150f5ac --- M firmware/libcommon/source/sniffer.c 1 file changed, 7 insertions(+), 6 deletions(-) Approvals: laforge: Looks good to me, approved Hoernchen: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c index f86d1f7..1368f0b 100644 --- a/firmware/libcommon/source/sniffer.c +++ b/firmware/libcommon/source/sniffer.c @@ -204,10 +204,11 @@ /*! Update Waiting Time (WT) * @param[in] wi Waiting Integer (0 if unchanged) * @param[in] d Baud Rate divider (0 if unchanged) + * @param[in] cause String describing the source of the change * @note set wt to be used by the receiver timeout * @note defined in ISO/IEC 7816-3:2006(E) section 8.1 and 10.2 */ -static void update_wt(uint8_t wi, uint8_t d) +static void update_wt(uint8_t wi, uint8_t d, const char *cause) { static uint8_t wt_wi = 10; /* Waiting time Integer (WI), used to calculate the Waiting Time (WT) */ static uint8_t wt_d = 1; /* baud rate adjustment integer (the actual value, not the table index) */ @@ -219,7 +220,7 @@ wt_d = d; } wt = wt_wi * 960UL * wt_d; - TRACE_INFO("WT updated to %lu ETU\n\r", wt); + TRACE_INFO("WT updated (wi=%u, d=%u, cause=%s) to %lu ETU\n\r", wi, d, cause, wt); } /*! Allocate USB buffer and push + initialize simtrace_msg_hdr @@ -287,7 +288,7 @@ switch (iso_state_new) { case ISO7816_S_RESET: update_fidi(&sniff_usart, 0x11); /* reset baud rate to default Di/Fi values */ - update_wt(10, 1); /* reset WT time-out */ + update_wt(10, 1, "RESET"); /* reset WT time-out */ break; case ISO7816_S_WAIT_ATR: rbuf_reset(&sniff_buffer); /* reset buffer for new communication */ @@ -478,9 +479,9 @@ /* retrieve WI encoded in TC2*/ if (ATR_S_WAIT_TC == g_atr.state && 2 == i) { if (0 == byte) { - update_wt(10, 0); + update_wt(10, 0, "TC2=0"); } else { - update_wt(byte, 0); + update_wt(byte, 0, "TC2"); } } if (y & 0x80) { @@ -675,7 +676,7 @@ TRACE_INFO("PPS negotiation successful: Fn=%u Dn=%u\n\r", iso7816_3_fi_table[fn], iso7816_3_di_table[dn]); update_fidi(&sniff_usart, pps_cur[2]); - update_wt(0, iso7816_3_di_table[dn]); + update_wt(0, iso7816_3_di_table[dn], "PPS"); usb_send_fidi(pps_cur[2]); /* send Fi/Di change notification to host software over USB */ } else { /* checksum is invalid */ TRACE_INFO("PPS negotiation failed\n\r"); -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/30187 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: I14245c0ca96a258146e48bb9909efd9f8150f5ac Gerrit-Change-Number: 30187 Gerrit-PatchSet: 3 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: tsaitgaist <kredon(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 7 months

1
0
0 0

Change in simtrace2[master]: firmware/sniffer: Make all global variables 'static'

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/30182 ) Change subject: firmware/sniffer: Make all global variables 'static' ...................................................................... firmware/sniffer: Make all global variables 'static' None of those variables are used outside sniffer.c, so they can all be static. Change-Id: I8946acb6189d5ade57214295f0ba87f0608bad92 --- M firmware/libcommon/source/sniffer.c 1 file changed, 14 insertions(+), 14 deletions(-) Approvals: laforge: Looks good to me, approved Hoernchen: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c index da5d743..8deda9c 100644 --- a/firmware/libcommon/source/sniffer.c +++ b/firmware/libcommon/source/sniffer.c @@ -141,47 +141,47 @@ static struct ringbuf sniff_buffer; /* Flags to know is the card status changed (see SIMTRACE_MSGT_DT_SNIFF_CHANGE flags) */ -volatile uint32_t change_flags = 0; +static volatile uint32_t change_flags = 0; /* ISO 7816 variables */ /*! ISO 7816-3 state */ -enum iso7816_3_sniff_state iso_state = ISO7816_S_RESET; +static enum iso7816_3_sniff_state iso_state = ISO7816_S_RESET; /*! ATR state */ -enum atr_sniff_state atr_state; +static enum atr_sniff_state atr_state; /*! ATR data * @remark can be used to check later protocol changes */ -uint8_t atr[MAX_ATR_SIZE]; +static uint8_t atr[MAX_ATR_SIZE]; /*! Current index in the ATR data */ -uint8_t atr_i = 0; +static uint8_t atr_i = 0; /*! If convention conversion is needed */ -bool convention_convert = false; +static bool convention_convert = false; /*! The supported T protocols */ -uint16_t t_protocol_support = 0; +static uint16_t t_protocol_support = 0; /*! PPS state * @remark it is shared between request and response since they aren't simultaneous but follow the same procedure */ -enum pps_sniff_state pps_state; +static enum pps_sniff_state pps_state; /*! PPS request data * @remark can be used to check PPS response */ -uint8_t pps_req[MAX_PPS_SIZE]; +static uint8_t pps_req[MAX_PPS_SIZE]; /*! PPS response data */ -uint8_t pps_rsp[MAX_PPS_SIZE]; +static uint8_t pps_rsp[MAX_PPS_SIZE]; /*! TPDU state */ -enum tpdu_sniff_state tpdu_state; +static enum tpdu_sniff_state tpdu_state; /*! Final TPDU packet * @note this is the complete command+response TPDU, including header, data, and status words * @remark this does not include the procedure bytes */ -uint8_t tpdu_packet[5+256+2]; +static uint8_t tpdu_packet[5+256+2]; /*! Current index in TPDU packet */ -uint16_t tpdu_packet_i = 0; +static uint16_t tpdu_packet_i = 0; /*! Waiting Time (WT) * @note defined in ISO/IEC 7816-3:2006(E) section 8.1 and 10.2 */ -uint32_t wt = 9600; +static uint32_t wt = 9600; /*------------------------------------------------------------------------------ * Internal functions -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/30182 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: I8946acb6189d5ade57214295f0ba87f0608bad92 Gerrit-Change-Number: 30182 Gerrit-PatchSet: 2 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: tsaitgaist <kredon(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 7 months

1
0
0 0

Change in simtrace2[master]: firmware/sniffer: Group global variables in structs

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/30183 ) Change subject: firmware/sniffer: Group global variables in structs ...................................................................... firmware/sniffer: Group global variables in structs This is a purely cosmetic change that groups PPS, TPDU and ATR related global variables into structs. The structs get g_ prefixes to indicate a global variable. This avoids confusion between very short/generic variable names that might clash with local variables. Change-Id: I3e02f6c6b063ebc860b7a2a54dfc6051f1ea584f --- M firmware/libcommon/source/sniffer.c 1 file changed, 102 insertions(+), 94 deletions(-) Approvals: laforge: Looks good to me, approved Hoernchen: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c index 8deda9c..f86d1f7 100644 --- a/firmware/libcommon/source/sniffer.c +++ b/firmware/libcommon/source/sniffer.c @@ -146,37 +146,45 @@ /* ISO 7816 variables */ /*! ISO 7816-3 state */ static enum iso7816_3_sniff_state iso_state = ISO7816_S_RESET; -/*! ATR state */ -static enum atr_sniff_state atr_state; -/*! ATR data - * @remark can be used to check later protocol changes - */ -static uint8_t atr[MAX_ATR_SIZE]; -/*! Current index in the ATR data */ -static uint8_t atr_i = 0; + +static struct { + /*! ATR state */ + enum atr_sniff_state state; + /*! ATR data + * @remark can be used to check later protocol changes + */ + uint8_t atr[MAX_ATR_SIZE]; + /*! Current index in the ATR data */ + uint8_t atr_i; +} g_atr; + /*! If convention conversion is needed */ static bool convention_convert = false; /*! The supported T protocols */ static uint16_t t_protocol_support = 0; -/*! PPS state - * @remark it is shared between request and response since they aren't simultaneous but follow the same procedure - */ -static enum pps_sniff_state pps_state; -/*! PPS request data - * @remark can be used to check PPS response - */ -static uint8_t pps_req[MAX_PPS_SIZE]; -/*! PPS response data */ -static uint8_t pps_rsp[MAX_PPS_SIZE]; -/*! TPDU state */ -static enum tpdu_sniff_state tpdu_state; -/*! Final TPDU packet - * @note this is the complete command+response TPDU, including header, data, and status words - * @remark this does not include the procedure bytes - */ -static uint8_t tpdu_packet[5+256+2]; -/*! Current index in TPDU packet */ -static uint16_t tpdu_packet_i = 0; + +static struct { + /*! PPS state + * @remark it is shared between request and response since they aren't simultaneous but + * follow the same procedure */ + enum pps_sniff_state state; + /*! PPS request data + * @remark can be used to check PPS response */ + uint8_t req[MAX_PPS_SIZE]; + /*! PPS response data */ + uint8_t rsp[MAX_PPS_SIZE]; +} g_pps; + +static struct { + /*! TPDU state */ + enum tpdu_sniff_state state; + /*! Final TPDU packet + * @note this is the complete command+response TPDU, including header, data, and status words + * @remark this does not include the procedure bytes */ + uint8_t packet[5+256+2]; + /*! Current index in TPDU packet */ + uint16_t packet_i; +} g_tpdu; /*! Waiting Time (WT) * @note defined in ISO/IEC 7816-3:2006(E) section 8.1 and 10.2 @@ -260,8 +268,8 @@ */ static void change_tpdu_state(enum tpdu_sniff_state tpdu_state_new) { - //TRACE_ERROR("TPDU state %u->%u\n\r", tpdu_state, tpdu_state_new); - tpdu_state = tpdu_state_new; + //TRACE_ERROR("TPDU state %u->%u\n\r", g_tpdu.state, tpdu_state_new); + g_tpdu.state = tpdu_state_new; } /*! Update the ISO 7816-3 state @@ -285,18 +293,18 @@ rbuf_reset(&sniff_buffer); /* reset buffer for new communication */ break; case ISO7816_S_IN_ATR: - atr_i = 0; + g_atr.atr_i = 0; convention_convert = false; t_protocol_support = 0; - atr_state = ATR_S_WAIT_TS; + g_atr.state = ATR_S_WAIT_TS; break; case ISO7816_S_IN_PPS_REQ: case ISO7816_S_IN_PPS_RSP: - pps_state = PPS_S_WAIT_PPSS; + g_pps.state = PPS_S_WAIT_PPSS; break; case ISO7816_S_WAIT_TPDU: change_tpdu_state(TPDU_S_CLA); - tpdu_packet_i = 0; + g_tpdu.packet_i = 0; break; default: break; @@ -390,13 +398,13 @@ TRACE_WARNING("Can't print ATR in ISO 7816-3 state %u\n\r", iso_state); return; } - if (atr_i >= ARRAY_SIZE(atr)) { + if (g_atr.atr_i >= ARRAY_SIZE(g_atr.atr)) { TRACE_ERROR("ATR buffer overflow\n\r"); return; } /* Send ATR over USB */ - usb_send_data(SIMTRACE_MSGT_SNIFF_ATR, atr, atr_i, flags); + usb_send_data(SIMTRACE_MSGT_SNIFF_ATR, g_atr.atr, g_atr.atr_i, flags); } /*! Process ATR byte @@ -414,16 +422,16 @@ TRACE_ERROR("Processing ATR data in wrong ISO 7816-3 state %u\n\r", iso_state); return; } - if (atr_i >= ARRAY_SIZE(atr)) { + if (g_atr.atr_i >= ARRAY_SIZE(g_atr.atr)) { TRACE_ERROR("ATR data overflow\n\r"); return; } /* save data for use by other functions */ - atr[atr_i++] = byte; + g_atr.atr[g_atr.atr_i++] = byte; /* handle ATR byte depending on current state */ - switch (atr_state) { + switch (g_atr.state) { case ATR_S_WAIT_TS: /* see ISO/IEC 7816-3:2006 section 8.1 */ flags = 0; switch (byte) { @@ -432,7 +440,7 @@ convention_convert = !convention_convert; case 0x3b: /* direct convention used and correctly decoded */ case 0x3f: /* inverse convention used and correctly decoded */ - atr_state = ATR_S_WAIT_T0; /* wait for format byte */ + g_atr.state = ATR_S_WAIT_T0; /* wait for format byte */ break; default: TRACE_WARNING("Invalid TS received\n\r"); @@ -445,30 +453,30 @@ break; case ATR_S_WAIT_T0: /* see ISO/IEC 7816-3:2006 section 8.2.2 */ case ATR_S_WAIT_TD: /* see ISO/IEC 7816-3:2006 section 8.2.3 */ - if (ATR_S_WAIT_T0 == atr_state) { + if (ATR_S_WAIT_T0 == g_atr.state) { atr_hist_len = (byte & 0x0f); /* save the number of historical bytes */ - } else if (ATR_S_WAIT_TD == atr_state) { + } else if (ATR_S_WAIT_TD == g_atr.state) { t_protocol_support |= (1<<(byte & 0x0f)); /* remember supported protocol to know if TCK will be present */ } y = (byte & 0xf0); /* remember upcoming interface bytes */ i++; /* next interface byte sub-group is coming */ if (y & 0x10) { - atr_state = ATR_S_WAIT_TA; /* wait for interface byte TA */ + g_atr.state = ATR_S_WAIT_TA; /* wait for interface byte TA */ break; } case ATR_S_WAIT_TA: /* see ISO/IEC 7816-3:2006 section 8.2.3 */ if (y & 0x20) { - atr_state = ATR_S_WAIT_TB; /* wait for interface byte TB */ + g_atr.state = ATR_S_WAIT_TB; /* wait for interface byte TB */ break; } case ATR_S_WAIT_TB: /* see ISO/IEC 7816-3:2006 section 8.2.3 */ if (y & 0x40) { - atr_state = ATR_S_WAIT_TC; /* wait for interface byte TC */ + g_atr.state = ATR_S_WAIT_TC; /* wait for interface byte TC */ break; } case ATR_S_WAIT_TC: /* see ISO/IEC 7816-3:2006 section 8.2.3 */ /* retrieve WI encoded in TC2*/ - if (ATR_S_WAIT_TC==atr_state && 2==i) { + if (ATR_S_WAIT_TC == g_atr.state && 2 == i) { if (0 == byte) { update_wt(10, 0); } else { @@ -476,10 +484,10 @@ } } if (y & 0x80) { - atr_state = ATR_S_WAIT_TD; /* wait for interface byte TD */ + g_atr.state = ATR_S_WAIT_TD; /* wait for interface byte TD */ break; } else if (atr_hist_len) { - atr_state = ATR_S_WAIT_HIST; /* wait for historical bytes */ + g_atr.state = ATR_S_WAIT_HIST; /* wait for historical bytes */ break; } case ATR_S_WAIT_HIST: /* see ISO/IEC 7816-3:2006 section 8.2.4 */ @@ -488,7 +496,7 @@ } if (0 == atr_hist_len) { if (t_protocol_support > 1) { - atr_state = ATR_S_WAIT_TCK; /* wait for check bytes */ + g_atr.state = ATR_S_WAIT_TCK; /* wait for check bytes */ break; } } else { @@ -496,11 +504,11 @@ } case ATR_S_WAIT_TCK: /* see ISO/IEC 7816-3:2006 section 8.2.5 */ /* verify checksum if present */ - if (ATR_S_WAIT_TCK == atr_state) { + if (ATR_S_WAIT_TCK == g_atr.state) { uint8_t ui; uint8_t checksum = 0; - for (ui = 1; ui < atr_i; ui++) { - checksum ^= atr[ui]; + for (ui = 1; ui < g_atr.atr_i; ui++) { + checksum ^= g_atr.atr[ui]; } if (checksum) { flags |= SNIFF_DATA_FLAG_ERROR_CHECKSUM; @@ -513,7 +521,7 @@ change_state(ISO7816_S_WAIT_TPDU); /* go to next state */ break; default: - TRACE_INFO("Unknown ATR state %u\n\r", atr_state); + TRACE_INFO("Unknown ATR state %u\n\r", g_atr.state); } } @@ -527,9 +535,9 @@ /* Sanity check */ if (ISO7816_S_IN_PPS_REQ == iso_state) { - pps_cur = pps_req; + pps_cur = g_pps.req; } else if (ISO7816_S_IN_PPS_RSP == iso_state) { - pps_cur = pps_rsp; + pps_cur = g_pps.rsp; } else { TRACE_ERROR("Can't print PPS in ISO 7816-3 state %u\n\r", iso_state); return; @@ -538,22 +546,22 @@ /* Get only relevant data */ uint8_t pps[6]; uint8_t pps_i = 0; - if (pps_state > PPS_S_WAIT_PPSS) { + if (g_pps.state > PPS_S_WAIT_PPSS) { pps[pps_i++] = pps_cur[0]; } - if (pps_state > PPS_S_WAIT_PPS0) { + if (g_pps.state > PPS_S_WAIT_PPS0) { pps[pps_i++] = pps_cur[1]; } - if (pps_state > PPS_S_WAIT_PPS1 && pps_cur[1] & 0x10) { + if (g_pps.state > PPS_S_WAIT_PPS1 && pps_cur[1] & 0x10) { pps[pps_i++] = pps_cur[2]; } - if (pps_state > PPS_S_WAIT_PPS2 && pps_cur[1] & 0x20) { + if (g_pps.state > PPS_S_WAIT_PPS2 && pps_cur[1] & 0x20) { pps[pps_i++] = pps_cur[3]; } - if (pps_state > PPS_S_WAIT_PPS3 && pps_cur[1] & 0x40) { + if (g_pps.state > PPS_S_WAIT_PPS3 && pps_cur[1] & 0x40) { pps[pps_i++] = pps_cur[4]; } - if (pps_state > PPS_S_WAIT_PCK) { + if (g_pps.state > PPS_S_WAIT_PCK) { pps[pps_i++] = pps_cur[5]; } @@ -583,21 +591,21 @@ /* sanity check */ if (ISO7816_S_IN_PPS_REQ == iso_state) { - pps_cur = pps_req; + pps_cur = g_pps.req; } else if (ISO7816_S_IN_PPS_RSP == iso_state) { - pps_cur = pps_rsp; + pps_cur = g_pps.rsp; } else { TRACE_ERROR("Processing PPS data in wrong ISO 7816-3 state %u\n\r", iso_state); return; } /* handle PPS byte depending on current state */ - switch (pps_state) { /* see ISO/IEC 7816-3:2006 section 9.2 */ + switch (g_pps.state) { /* see ISO/IEC 7816-3:2006 section 9.2 */ case PPS_S_WAIT_PPSS: /*!< initial byte */ flags = 0; if (byte == 0xff) { pps_cur[0] = byte; - pps_state = PPS_S_WAIT_PPS0; /* go to next state */ + g_pps.state = PPS_S_WAIT_PPS0; /* go to next state */ } else { TRACE_INFO("Invalid PPSS received\n\r"); led_blink(LED_RED, BLINK_2F_O); /* indicate error to user */ @@ -608,24 +616,24 @@ case PPS_S_WAIT_PPS0: /*!< format byte */ pps_cur[1] = byte; if (pps_cur[1] & 0x10) { - pps_state = PPS_S_WAIT_PPS1; /* go to next state */ + g_pps.state = PPS_S_WAIT_PPS1; /* go to next state */ break; } case PPS_S_WAIT_PPS1: /*!< first parameter byte */ pps_cur[2] = byte; /* not always right but doesn't affect the process */ if (pps_cur[1] & 0x20) { - pps_state = PPS_S_WAIT_PPS2; /* go to next state */ + g_pps.state = PPS_S_WAIT_PPS2; /* go to next state */ break; } case PPS_S_WAIT_PPS2: /*!< second parameter byte */ pps_cur[3] = byte; /* not always right but doesn't affect the process */ if (pps_cur[1] & 0x40) { - pps_state = PPS_S_WAIT_PPS3; /* go to next state */ + g_pps.state = PPS_S_WAIT_PPS3; /* go to next state */ break; } case PPS_S_WAIT_PPS3: /*!< third parameter byte */ pps_cur[4] = byte; /* not always right but doesn't affect the process */ - pps_state = PPS_S_WAIT_PCK; /* go to next state */ + g_pps.state = PPS_S_WAIT_PCK; /* go to next state */ break; case PPS_S_WAIT_PCK: /*!< check byte */ pps_cur[5] = byte; /* not always right but doesn't affect the process */ @@ -646,7 +654,7 @@ if (check) { flags |= SNIFF_DATA_FLAG_ERROR_CHECKSUM; } - pps_state = PPS_S_WAIT_END; + g_pps.state = PPS_S_WAIT_END; usb_send_pps(flags); /* send PPS to host software using USB */ if (ISO7816_S_IN_PPS_REQ == iso_state) { if (0 == check) { /* checksum is valid */ @@ -676,10 +684,10 @@ } break; case PPS_S_WAIT_END: - TRACE_WARNING("Unexpected PPS received %u\n\r", pps_state); + TRACE_WARNING("Unexpected PPS received %u\n\r", g_pps.state); break; default: - TRACE_WARNING("Unknown PPS state %u\n\r", pps_state); + TRACE_WARNING("Unknown PPS state %u\n\r", g_pps.state); break; } } @@ -697,7 +705,7 @@ } /* Send ATR over USB */ - usb_send_data(SIMTRACE_MSGT_SNIFF_TPDU, tpdu_packet, tpdu_packet_i, flags); + usb_send_data(SIMTRACE_MSGT_SNIFF_TPDU, g_tpdu.packet, g_tpdu.packet_i, flags); } static void process_byte_tpdu(uint8_t byte) @@ -707,13 +715,13 @@ TRACE_ERROR("Processing TPDU data in wrong ISO 7816-3 state %u\n\r", iso_state); return; } - if (tpdu_packet_i >= ARRAY_SIZE(tpdu_packet)) { + if (g_tpdu.packet_i >= ARRAY_SIZE(g_tpdu.packet)) { TRACE_ERROR("TPDU data overflow\n\r"); return; } /* handle TPDU byte depending on current state */ - switch (tpdu_state) { + switch (g_tpdu.state) { case TPDU_S_CLA: if (0xff == byte) { TRACE_WARNING("0xff is not a valid class byte\n\r"); @@ -722,8 +730,8 @@ change_state(ISO7816_S_WAIT_TPDU); /* go back to TPDU state */ return; } - tpdu_packet_i = 0; - tpdu_packet[tpdu_packet_i++] = byte; + g_tpdu.packet_i = 0; + g_tpdu.packet[g_tpdu.packet_i++] = byte; change_tpdu_state(TPDU_S_INS); break; case TPDU_S_INS: @@ -734,38 +742,38 @@ change_state(ISO7816_S_WAIT_TPDU); /* go back to TPDU state */ return; } - tpdu_packet_i = 1; - tpdu_packet[tpdu_packet_i++] = byte; + g_tpdu.packet_i = 1; + g_tpdu.packet[g_tpdu.packet_i++] = byte; change_tpdu_state(TPDU_S_P1); break; case TPDU_S_P1: - tpdu_packet_i = 2; - tpdu_packet[tpdu_packet_i++] = byte; + g_tpdu.packet_i = 2; + g_tpdu.packet[g_tpdu.packet_i++] = byte; change_tpdu_state(TPDU_S_P2); break; case TPDU_S_P2: - tpdu_packet_i = 3; - tpdu_packet[tpdu_packet_i++] = byte; + g_tpdu.packet_i = 3; + g_tpdu.packet[g_tpdu.packet_i++] = byte; change_tpdu_state(TPDU_S_P3); break; case TPDU_S_P3: - tpdu_packet_i = 4; - tpdu_packet[tpdu_packet_i++] = byte; + g_tpdu.packet_i = 4; + g_tpdu.packet[g_tpdu.packet_i++] = byte; change_tpdu_state(TPDU_S_PROCEDURE); break; case TPDU_S_PROCEDURE: if (0x60 == byte) { /* wait for next procedure byte */ break; - } else if (tpdu_packet[1] == byte) { /* get all remaining data bytes */ + } else if (g_tpdu.packet[1] == byte) { /* get all remaining data bytes */ change_tpdu_state(TPDU_S_DATA_REMAINING); break; - } else if ((~tpdu_packet[1]) == byte) { /* get single data byte */ + } else if ((~g_tpdu.packet[1]) == byte) { /* get single data byte */ change_tpdu_state(TPDU_S_DATA_SINGLE); break; } case TPDU_S_SW1: if ((0x60 == (byte & 0xf0)) || (0x90 == (byte & 0xf0))) { /* this procedure byte is SW1 */ - tpdu_packet[tpdu_packet_i++] = byte; + g_tpdu.packet[g_tpdu.packet_i++] = byte; change_tpdu_state(TPDU_S_SW2); } else { TRACE_WARNING("invalid SW1 0x%02x\n\r", byte); @@ -776,28 +784,28 @@ } break; case TPDU_S_SW2: - tpdu_packet[tpdu_packet_i++] = byte; + g_tpdu.packet[g_tpdu.packet_i++] = byte; usb_send_tpdu(0); /* send TPDU to host software using USB */ change_state(ISO7816_S_WAIT_TPDU); /* this is the end of the TPDU */ break; case TPDU_S_DATA_SINGLE: case TPDU_S_DATA_REMAINING: - tpdu_packet[tpdu_packet_i++] = byte; - if (0 == tpdu_packet[4]) { - if (5+256 <= tpdu_packet_i) { + g_tpdu.packet[g_tpdu.packet_i++] = byte; + if (0 == g_tpdu.packet[4]) { + if (5+256 <= g_tpdu.packet_i) { change_tpdu_state(TPDU_S_PROCEDURE); } } else { - if (5+tpdu_packet[4] <= tpdu_packet_i) { + if (5+g_tpdu.packet[4] <= g_tpdu.packet_i) { change_tpdu_state(TPDU_S_PROCEDURE); } } - if (TPDU_S_DATA_SINGLE == tpdu_state) { + if (TPDU_S_DATA_SINGLE == g_tpdu.state) { change_tpdu_state(TPDU_S_PROCEDURE); } break; default: - TRACE_ERROR("unhandled TPDU state %u\n\r", tpdu_state); + TRACE_ERROR("unhandled TPDU state %u\n\r", g_tpdu.state); } } -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/30183 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: I3e02f6c6b063ebc860b7a2a54dfc6051f1ea584f Gerrit-Change-Number: 30183 Gerrit-PatchSet: 3 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: tsaitgaist <kredon(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 7 months

1
0
0 0

Change in simtrace2[master]: firmware/sniffer: Avoid extra call for rbuf_is_full

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/30180 ) Change subject: firmware/sniffer: Avoid extra call for rbuf_is_full ...................................................................... firmware/sniffer: Avoid extra call for rbuf_is_full rbuf_write() will tell us in the return value if the buffer was full (error) or not (success). Let's use that return value rather than a theoretically race-y call to rbuf_is_full() before. It's theoretical as the write happens from IRQ context and the read from normal process context, so the fill-level cannot really change while we're in the USART interrupt. So it doesn't fix a bug, just improves coding style and also avoids an extra function call + irq-disable/re-enable. Change-Id: Icf570d0aa48d67a19e63c6e2b6caa14438fe88e3 --- M firmware/libcommon/source/sniffer.c 1 file changed, 1 insertion(+), 4 deletions(-) Approvals: laforge: Looks good to me, approved Hoernchen: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c index e3b7fbb..cbfa7c9 100644 --- a/firmware/libcommon/source/sniffer.c +++ b/firmware/libcommon/source/sniffer.c @@ -826,11 +826,8 @@ /* Reset WT timer */ wt_remaining = wt; /* Store sniffed data into buffer (also clear interrupt */ - if (rbuf_is_full(&sniff_buffer)) { + if (rbuf_write(&sniff_buffer, byte) != 0) TRACE_ERROR("USART buffer full\n\r"); - } else { - rbuf_write(&sniff_buffer, byte); - } } /* Verify it WT timeout occurred, to detect unresponsive card */ -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/30180 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: Icf570d0aa48d67a19e63c6e2b6caa14438fe88e3 Gerrit-Change-Number: 30180 Gerrit-PatchSet: 2 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: tsaitgaist <kredon(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 7 months

1
0
0 0

2025

2024

2023

2022

gerrit-log November 2022