Hoernchen has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/42481?usp=email )
Change subject: firmware: replace libosmocore fsm timers ......................................................................
firmware: replace libosmocore fsm timers
Concurrent access from main loop and the different uart irqs was not going well and caused crashes. The libosmocore fsm does still allocate one internal timer, but it is not used by the firmware, so libosmocore will not touch the rbtree.
Closes: SYS#7877 Change-Id: Id2bd67b3946bb451008965f0b68b4a919f4d10bd --- M ccid_common/ccid_slot_fsm.c M ccid_common/cuart.c M ccid_common/cuart.h M ccid_common/iso7816_fsm.c M sysmoOCTSIM/main.c 5 files changed, 57 insertions(+), 46 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-ccid-firmware refs/changes/81/42481/1
diff --git a/ccid_common/ccid_slot_fsm.c b/ccid_common/ccid_slot_fsm.c index 05024c9..22d3f70 100644 --- a/ccid_common/ccid_slot_fsm.c +++ b/ccid_common/ccid_slot_fsm.c @@ -207,7 +207,7 @@ volatile void * volatile data = cs->event_data;
if (!event) - return 0; + goto out; // if(event && !data) // return 0;
@@ -340,6 +340,8 @@ break; }
+out: + card_uart_wtime_poll(ss->cuart); return 0; }
diff --git a/ccid_common/cuart.c b/ccid_common/cuart.c index d228c13..593265e 100644 --- a/ccid_common/cuart.c +++ b/ccid_common/cuart.c @@ -21,10 +21,11 @@ #include <string.h> #include <osmocom/core/linuxlist.h> #include <osmocom/core/utils.h> -#include <osmocom/core/timer.h>
#include "cuart.h"
+extern volatile uint64_t jiffies; + static LLIST_HEAD(g_cuart_drivers);
const struct value_string card_uart_event_vals[] = { @@ -56,36 +57,37 @@ return 1e6 / cuart->driver->ops->ctrl(cuart, CUART_CTL_GET_BAUDRATE, 0); }
-/* software waiting-time timer has expired */ -static void card_uart_wtime_cb(void *data) -{ - struct card_uart *cuart = (struct card_uart *) data; - card_uart_notification(cuart, CUART_E_RX_TIMEOUT, NULL); - /* should we automatically disable the receiver? */ -} - void card_uart_wtime_restart(struct card_uart *cuart) { - int secs, usecs; - if (!cuart->current_wtime_byte) return;
int etu_in_us = get_etu_in_us(cuart) + 1; cuart->wtime_etu = cuart->wtime_etu ? cuart->wtime_etu : 1;
- /* timemout is wtime * ETU * expected number of bytes */ - usecs = etu_in_us * cuart->wtime_etu * cuart->current_wtime_byte; + /* timeout is wtime * ETU * expected number of bytes */ + uint32_t usecs = etu_in_us * cuart->wtime_etu * cuart->current_wtime_byte;
/* limit lower wait time to reasonable value */ - usecs = usecs < 300000 ? 300000 : usecs; + if (usecs < 300000) + usecs = 300000;
- if (usecs > 1000000) { - secs = usecs / 1000000; - usecs = usecs % 1000000; - } else - secs = 0; - osmo_timer_schedule(&cuart->wtime_tmr, secs, usecs); + uint32_t ms = (usecs + 999) / 1000; + cuart->wtime_deadline = jiffies + ms; +} + +static inline void card_uart_wtime_stop(struct card_uart *cuart) +{ + cuart->wtime_deadline = 0; +} + +void card_uart_wtime_poll(struct card_uart *cuart) +{ + uint64_t deadline = cuart->wtime_deadline; + if (deadline && jiffies >= deadline) { + cuart->wtime_deadline = 0; + card_uart_notification(cuart, CUART_E_RX_TIMEOUT, NULL); + } }
int card_uart_open(struct card_uart *cuart, const char *driver_name, const char *device_name) @@ -99,7 +101,7 @@ cuart->wtime_etu = 9600; /* ISO 7816-3 Section 8.1 */ cuart->rx_enabled = true; cuart->rx_threshold = 1; - osmo_timer_setup(&cuart->wtime_tmr, card_uart_wtime_cb, cuart); + cuart->wtime_deadline = 0;
rc = drv->ops->open(cuart, device_name); if (rc < 0) @@ -137,16 +139,14 @@ case CUART_CTL_RX: cuart->rx_enabled = arg ? true : false; if (!cuart->rx_enabled) - osmo_timer_del(&cuart->wtime_tmr); -// else -// card_uart_wtime_restart(cuart); + card_uart_wtime_stop(cuart); break; case CUART_CTL_RX_TIMER_HINT: cuart->current_wtime_byte = arg; if(arg) card_uart_wtime_restart(cuart); else - osmo_timer_del(&cuart->wtime_tmr); + card_uart_wtime_stop(cuart); break; case CUART_CTL_POWER_5V0: case CUART_CTL_POWER_3V0: @@ -154,7 +154,7 @@ /* we have to reset this somewhere, and powering down loses all state * this is not hw specific so it belongs here, after handling the hw specific part */ if (!arg) { - osmo_timer_del(&cuart->wtime_tmr); + card_uart_wtime_stop(cuart); cuart->tx_busy = false; cuart->rx_threshold = 1; cuart->wtime_etu = 9600; /* ISO 7816-3 Section 8.1 */ diff --git a/ccid_common/cuart.h b/ccid_common/cuart.h index c8573c5..bb1c18b 100644 --- a/ccid_common/cuart.h +++ b/ccid_common/cuart.h @@ -21,8 +21,6 @@ #include <stdint.h> #include <stdbool.h> #include <osmocom/core/linuxlist.h> -#include <osmocom/core/timer.h> - #include <osmocom/core/select.h> #include "utils_ringbuffer.h"
@@ -101,7 +99,9 @@ uint32_t rx_threshold;
uint32_t wtime_etu; - struct osmo_timer_list wtime_tmr; + /* deadline in jiffies (ms) for card response timeout, 0 = inactive. + * Set from IRQ context, checked from main loop */ + volatile uint64_t wtime_deadline; /* expected number of bytes, for timeout */ uint32_t current_wtime_byte;
@@ -153,6 +153,11 @@ /* (re)start the software WTIME timer */ void card_uart_wtime_restart(struct card_uart *cuart);
+/* poll wtime deadline from main loop, fires CUART_E_RX_TIMEOUT if expired */ +void card_uart_wtime_poll(struct card_uart *cuart); + void card_uart_notification(struct card_uart *cuart, enum card_uart_event evt, void *data);
int card_uart_driver_register(struct card_uart_driver *drv); + +struct card_uart *cuart4slot_nr(uint8_t slot_nr); diff --git a/ccid_common/iso7816_fsm.c b/ccid_common/iso7816_fsm.c index a035148..778528a 100644 --- a/ccid_common/iso7816_fsm.c +++ b/ccid_common/iso7816_fsm.c @@ -33,6 +33,8 @@ #include "cuart.h" #include "iso7816_fsm.h"
+extern volatile uint64_t jiffies; + /* unionize to ensure at least properly aligned msgb struct */ #define DECLARE_STATIC_MSGB(name, size) \ struct msgb* name; \ @@ -137,12 +139,6 @@ TPDU_S_DONE, };
-/* FSM timer enumeration */ -enum iso7816_3_timer { - T_WAIT_ATR = 1, - T_GUARD, -}; - /* forward declarations */ static struct osmo_fsm iso7816_3_fsm; static struct osmo_fsm atr_fsm; @@ -264,6 +260,17 @@ return (struct iso7816_3_priv *) fi->priv; }
+/* libosmocore fsm timers can't be used to to concurrency issues. + * Expiry fires CUART_E_RX_TIMEOUT from main loop -> ISO7816_E_WTIME_EXP + */ +static void atr_state_chg_guard(struct osmo_fsm_inst *atr_fi, uint32_t new_state, uint32_t timeout_ms) +{ + struct osmo_fsm_inst *parent_fi = atr_fi->proc.parent; + struct iso7816_3_priv *ip = get_iso7816_3_priv(parent_fi); + osmo_fsm_inst_state_chg(atr_fi, new_state, 0, 0); + ip->uart->wtime_deadline = timeout_ms ? (jiffies + timeout_ms) + 1 : 0; +} + /* convert from clock cycles of the CLK line to milli-seconds */ uint32_t fi_cycles2ms(struct osmo_fsm_inst *fi, uint32_t cyclces) { @@ -741,7 +748,7 @@ /* fall-through */ case 0x3f: /* inverse convention used and correctly decoded */ atr_append_byte(fi, byte); - osmo_fsm_inst_state_chg_ms(fi, ATR_S_WAIT_T0, atr_fi_gt_ms(fi), T_GUARD); + atr_state_chg_guard(fi, ATR_S_WAIT_T0, atr_fi_gt_ms(fi)); break; default: LOGPFSML(fi, LOGL_ERROR, "Invalid TS received: 0x%02X\n", byte); @@ -783,28 +790,28 @@ atp->y = (byte & 0xf0); /* remember incoming interface bytes */ atp->i++; if (atp->y & 0x10) { - osmo_fsm_inst_state_chg_ms(fi, ATR_S_WAIT_TA, guard_time_ms, T_GUARD); + atr_state_chg_guard(fi, ATR_S_WAIT_TA, guard_time_ms); break; } /* fall-through */ case ATR_S_WAIT_TA: /* see ISO/IEC 7816-3:2006 section 8.2.3 */ if (atp->y & 0x20) { - osmo_fsm_inst_state_chg_ms(fi, ATR_S_WAIT_TB, guard_time_ms, T_GUARD); + atr_state_chg_guard(fi, ATR_S_WAIT_TB, guard_time_ms); break; } /* fall-through */ case ATR_S_WAIT_TB: /* see ISO/IEC 7816-3:2006 section 8.2.3 */ if (atp->y & 0x40) { - osmo_fsm_inst_state_chg_ms(fi, ATR_S_WAIT_TC, guard_time_ms, T_GUARD); + atr_state_chg_guard(fi, ATR_S_WAIT_TC, guard_time_ms); break; } /* fall-through */ case ATR_S_WAIT_TC: /* see ISO/IEC 7816-3:2006 section 8.2.3 */ if (atp->y & 0x80) { - osmo_fsm_inst_state_chg_ms(fi, ATR_S_WAIT_TD, guard_time_ms, T_GUARD); + atr_state_chg_guard(fi, ATR_S_WAIT_TD, guard_time_ms); break; } else if (atp->hist_len) { - osmo_fsm_inst_state_chg_ms(fi, ATR_S_WAIT_HIST, guard_time_ms, T_GUARD); + atr_state_chg_guard(fi, ATR_S_WAIT_HIST, guard_time_ms); break; } /* fall-through */ @@ -814,8 +821,7 @@ if (atp->hist_len == 0) { if (atp->protocol_support > 1) { /* wait for check byte */ - osmo_fsm_inst_state_chg_ms(fi, ATR_S_WAIT_TCK, - guard_time_ms, T_GUARD); + atr_state_chg_guard(fi, ATR_S_WAIT_TCK, guard_time_ms); break; } else { /* no TCK present, ATR complete; notify parent */ diff --git a/sysmoOCTSIM/main.c b/sysmoOCTSIM/main.c index e1e83c7..7873c86 100644 --- a/sysmoOCTSIM/main.c +++ b/sysmoOCTSIM/main.c @@ -22,7 +22,6 @@ #include <errno.h>
#include <osmocom/core/utils.h> -#include <osmocom/core/timer.h>
#include <hal_cache.h> #include <hri_port_e54.h> @@ -714,7 +713,6 @@ g_ci.slot_ops->handle_fsm_events(&g_ci.slot[i], true); } feed_ccid(); - osmo_timers_update(); int qs = llist_count_at(&g_ccid_s.free_q); if (qs > NUM_OUT_BUF) for (int i = 0; i < qs - NUM_OUT_BUF; i++) {
-
Hoernchen