laforge has submitted this change. ( 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.
Additionally ignore checkpatch jiffies complaints because we're trying to do firmware here.
Closes: SYS#7877 Change-Id: Id2bd67b3946bb451008965f0b68b4a919f4d10bd --- M .checkpatch.conf M ccid_common/ccid_slot_fsm.c M ccid_common/cuart.c M ccid_common/cuart.h M ccid_common/iso7816_fsm.c M ccid_host/Makefile A ccid_host/libosmo_emb.c A ccid_host/libosmo_emb.h M sysmoOCTSIM/main.c 9 files changed, 112 insertions(+), 46 deletions(-)
Approvals: laforge: Looks good to me, approved Jenkins Builder: Verified
diff --git a/.checkpatch.conf b/.checkpatch.conf index 76a4cc5..0421be5 100644 --- a/.checkpatch.conf +++ b/.checkpatch.conf @@ -16,3 +16,4 @@ --ignore LEADING_SPACE --ignore SUSPECT_CODE_INDENT --ignore SPACE_BEFORE_TAB +--ignore JIFFIES_COMPARISON 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..bb17c4b 100644 --- a/ccid_common/cuart.c +++ b/ccid_common/cuart.c @@ -21,7 +21,6 @@ #include <string.h> #include <osmocom/core/linuxlist.h> #include <osmocom/core/utils.h> -#include <osmocom/core/timer.h>
#include "cuart.h"
@@ -56,36 +55,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_set_deadline(cuart, get_jiffies() + ms); +} + +static inline void card_uart_wtime_stop(struct card_uart *cuart) +{ + cuart_set_deadline(cuart, 0); +} + +void card_uart_wtime_poll(struct card_uart *cuart) +{ + uint64_t deadline = cuart_get_deadline(cuart); + if (deadline && (int64_t)(get_jiffies() - deadline) >= 0) { + cuart_set_deadline(cuart, 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 +99,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_set_deadline(cuart, 0);
rc = drv->ops->open(cuart, device_name); if (rc < 0) @@ -137,16 +137,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 +152,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..a9afc1d 100644 --- a/ccid_common/cuart.h +++ b/ccid_common/cuart.h @@ -21,10 +21,9 @@ #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" +#include "libosmo_emb.h"
struct usart_async_descriptor;
@@ -101,7 +100,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 __attribute__((aligned(8))); /* expected number of bytes, for timeout */ uint32_t current_wtime_byte;
@@ -133,6 +134,16 @@ } u; };
+static inline uint64_t cuart_get_deadline(struct card_uart *cuart) +{ + return ldrd_u64(&cuart->wtime_deadline); +} + +static inline void cuart_set_deadline(struct card_uart *cuart, uint64_t deadline) +{ + strd_u64(&cuart->wtime_deadline, deadline); +} + /*! Open the Card UART */ int card_uart_open(struct card_uart *cuart, const char *driver_name, const char *device_name);
@@ -153,6 +164,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..4f01bd7 100644 --- a/ccid_common/iso7816_fsm.c +++ b/ccid_common/iso7816_fsm.c @@ -137,12 +137,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 +258,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); + cuart_set_deadline(ip->uart, timeout_ms ? get_jiffies() + timeout_ms : 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 +746,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 +788,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 +819,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/ccid_host/Makefile b/ccid_host/Makefile index 718377a..f72a4ad 100644 --- a/ccid_host/Makefile +++ b/ccid_host/Makefile @@ -18,6 +18,7 @@ cuart_driver_tty.o \ utils_ringbuffer.o \ logging.o \ + libosmo_emb.o \ ../ccid_common/cuart.o \ ../ccid_common/ccid_proto.o \ ../ccid_common/ccid_device.o \ @@ -32,6 +33,7 @@ cuart_test: cuart_test.o \ cuart_driver_tty.o \ utils_ringbuffer.o \ + libosmo_emb.o \ ../ccid_common/cuart.o $(CC) $(CFLAGS) -o $@ $^ $(LIBS)
@@ -39,6 +41,7 @@ logging.o \ cuart_driver_tty.o \ utils_ringbuffer.o \ + libosmo_emb.o \ ../ccid_common/iso7816_fsm.o \ ../ccid_common/iso7816_3.o \ ../ccid_common/cuart.o diff --git a/ccid_host/libosmo_emb.c b/ccid_host/libosmo_emb.c new file mode 100644 index 0000000..c144b86 --- /dev/null +++ b/ccid_host/libosmo_emb.c @@ -0,0 +1,29 @@ +/* Host-side jiffies: milliseconds from gettimeofday(). + * + * Copyright (C) 2026 sysmocom -s.f.m.c. GmbH, Author: Eric Wild ewild@sysmocom.de + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307, USA + */ + +#include <stddef.h> +#include <osmocom/core/timer.h> +#include "libosmo_emb.h" + +uint64_t get_jiffies(void) +{ + struct timespec ts; + osmo_clock_gettime(CLOCK_MONOTONIC, &ts); + return (uint64_t)ts.tv_sec * 1000 + ts.tv_nsec / 1000000; +} diff --git a/ccid_host/libosmo_emb.h b/ccid_host/libosmo_emb.h new file mode 100644 index 0000000..ffc0324 --- /dev/null +++ b/ccid_host/libosmo_emb.h @@ -0,0 +1,15 @@ +#pragma once +/* Host-side jiffies emulation */ + +#include <stdint.h> + +static inline uint64_t ldrd_u64(const volatile uint64_t *p) +{ + return *p; +} +static inline void strd_u64(volatile uint64_t *p, uint64_t v) +{ + *p = v; +} + +uint64_t get_jiffies(void); 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++) {
-
laforge