From 92c44c572e407d7cf0f572a2b46fee49da7a0c98 Mon Sep 17 00:00:00 2001 From: Harald Welte Date: Wed, 16 Nov 2022 10:48:54 +0100 Subject: [PATCH] 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 --- firmware/libcommon/include/ringbuffer.h | 18 +++++++ firmware/libcommon/source/ringbuffer.c | 64 +++++++++++++++++++++++++ firmware/libcommon/source/sniffer.c | 12 ++--- 3 files changed, 88 insertions(+), 6 deletions(-) diff --git a/firmware/libcommon/include/ringbuffer.h b/firmware/libcommon/include/ringbuffer.h index 51af3b2c..99fe96b3 100644 --- a/firmware/libcommon/include/ringbuffer.h +++ b/firmware/libcommon/include/ringbuffer.h @@ -32,4 +32,22 @@ int rbuf_write(volatile ringbuf * rb, uint8_t item); 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 591d31f3..d3690be5 100644 --- a/firmware/libcommon/source/ringbuffer.c +++ b/firmware/libcommon/source/ringbuffer.c @@ -28,6 +28,16 @@ void rbuf_reset(volatile ringbuf * rb) 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 @@ uint8_t rbuf_read(volatile ringbuf * rb) 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 @@ bool rbuf_is_full(volatile ringbuf * rb) 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 rbuf_write(volatile ringbuf * rb, uint8_t item) } } +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 153c4839..bea4d95a 100644 --- a/firmware/libcommon/source/sniffer.c +++ b/firmware/libcommon/source/sniffer.c @@ -138,7 +138,7 @@ static struct Usart_info sniff_usart = { .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 @@ static void change_state(enum iso7816_3_sniff_state iso_state_new) 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 @@ void Sniffer_usart_isr(void) /* 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 @@ void Sniffer_init(void) 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 @@ void Sniffer_run(void) * 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];