<p>laforge has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/simtrace2/+/23639">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">card_emu: Clarify and differentiate F/Fi/F_index/Fi_index<br><br>The ISO7816 spec terms are well-defined, let's not abuse them. We used<br>to consider "Fi" as the "index into the table of F values", while the<br>spec actually considers Fi as the initial value for F.<br><br>Let's make sure we use the terms quite clearly:<br>* Fi and Di are the initial values for F and D<br>* F*_index and D*_index are the indexes into the ISO7816-3 Tables<br><br>Furthermore, let's track Fi separately from F, as e.g. the waiting<br>time definition only considers Fi as indicated in the ATR, despite<br>an actually different F value might have been negotiated via PTS<br>meanwhile.<br><br>Change-Id: Ieb2425e8380a81b79df7b2bd072902994e9c3ee7<br>---<br>M firmware/libcommon/include/simtrace_prot.h<br>M firmware/libcommon/source/card_emu.c<br>2 files changed, 52 insertions(+), 26 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/simtrace2 refs/changes/39/23639/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/firmware/libcommon/include/simtrace_prot.h b/firmware/libcommon/include/simtrace_prot.h</span><br><span>index eca844a..e550616 100644</span><br><span>--- a/firmware/libcommon/include/simtrace_prot.h</span><br><span>+++ b/firmware/libcommon/include/simtrace_prot.h</span><br><span>@@ -230,11 +230,17 @@</span><br><span>    uint32_t flags;</span><br><span>      /* phone-applied target voltage in mV */</span><br><span>     uint16_t voltage_mv;</span><br><span style="color: hsl(0, 100%, 40%);">-    /* Fi/Di related information */</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t fi;</span><br><span style="color: hsl(0, 100%, 40%);">-     uint8_t di;</span><br><span style="color: hsl(0, 100%, 40%);">-     uint8_t wi;</span><br><span style="color: hsl(0, 100%, 40%);">-     uint32_t waiting_time;</span><br><span style="color: hsl(120, 100%, 40%);">+        /* F/D related information. Not actual Fn/Dn values but indexes into tables! */</span><br><span style="color: hsl(120, 100%, 40%);">+       union {</span><br><span style="color: hsl(120, 100%, 40%);">+               uint8_t F_index;        /* <! Index to ISO7816-3 Table 7 (F and f_max values) */</span><br><span style="color: hsl(120, 100%, 40%);">+           uint8_t fi;             /* <! old, wrong name for API compatibility */</span><br><span style="color: hsl(120, 100%, 40%);">+     };</span><br><span style="color: hsl(120, 100%, 40%);">+    union {</span><br><span style="color: hsl(120, 100%, 40%);">+               uint8_t D_index;        /* <! Index to ISO7816-3 Table 8 (D value) */</span><br><span style="color: hsl(120, 100%, 40%);">+              uint8_t di;             /* <! old, wrong name for API compatibility */</span><br><span style="color: hsl(120, 100%, 40%);">+     };</span><br><span style="color: hsl(120, 100%, 40%);">+    uint8_t wi;             /* <! Waiting Integer as defined in ISO7816-3 Section 10.2 */</span><br><span style="color: hsl(120, 100%, 40%);">+      uint32_t waiting_time;  /* <! Waiting Time in etu as defined in ISO7816-3 Section 8.1 */</span><br><span> } __attribute__ ((packed));</span><br><span> </span><br><span> /* CEMU_USB_MSGT_DO_PTS */</span><br><span>diff --git a/firmware/libcommon/source/card_emu.c b/firmware/libcommon/source/card_emu.c</span><br><span>index 5feb157..1193121 100644</span><br><span>--- a/firmware/libcommon/source/card_emu.c</span><br><span>+++ b/firmware/libcommon/source/card_emu.c</span><br><span>@@ -1,6 +1,6 @@</span><br><span> /* ISO7816-3 state machine for the card side</span><br><span>  *</span><br><span style="color: hsl(0, 100%, 40%);">- * (C) 2010-2019 by Harald Welte <laforge@gnumonks.org></span><br><span style="color: hsl(120, 100%, 40%);">+ * (C) 2010-2021 by Harald Welte <laforge@gnumonks.org></span><br><span>  * (C) 2018 by sysmocom -s.f.m.c. GmbH, Author: Kevin Redon <kredon@sysmocom.de></span><br><span>  *</span><br><span>  * This program is free software; you can redistribute it and/or modify</span><br><span>@@ -154,19 +154,35 @@</span><br><span>     bool in_reset;  /*< if card is in reset (true = RST low/asserted, false = RST high/ released) */</span><br><span>  bool clocked;   /*< if clock is active ( true = active, false = inactive) */</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-     /* timing parameters, from PTS */</span><br><span style="color: hsl(0, 100%, 40%);">-       uint8_t Fi;</span><br><span style="color: hsl(0, 100%, 40%);">-     uint8_t Di;</span><br><span style="color: hsl(120, 100%, 40%);">+   /* All below variables with _index suffix are indexes from 0..15 into Tables 7 + 8</span><br><span style="color: hsl(120, 100%, 40%);">+     * of ISO7816-3. */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /*! Index to clock rate conversion integer Fi (ISO7816-3 Table 7).</span><br><span style="color: hsl(120, 100%, 40%);">+     *  \note this represents the maximum value supported by the card, and can be indicated in TA1 */</span><br><span style="color: hsl(120, 100%, 40%);">+     uint8_t Fi_index;</span><br><span style="color: hsl(120, 100%, 40%);">+     /*! Current value of index to clock rate conversion integer F (ISO 7816-3 Section 7.1). */</span><br><span style="color: hsl(120, 100%, 40%);">+    uint8_t F_index;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+    /*! Index to baud rate adjustment factor Di (ISO7816-3 Table 8).</span><br><span style="color: hsl(120, 100%, 40%);">+       *  \note this represents the maximum value supported by the card, and can be indicated in TA1 */</span><br><span style="color: hsl(120, 100%, 40%);">+     uint8_t Di_index;</span><br><span style="color: hsl(120, 100%, 40%);">+     /*! Current value of index to baud rate adjustment factor D (ISO 7816-3 Section 7.1). */</span><br><span style="color: hsl(120, 100%, 40%);">+      uint8_t D_index;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+    /*! Waiting Integer (ISO7816-3 Section 10.2).</span><br><span style="color: hsl(120, 100%, 40%);">+  *  \note this value can be set in TA2 */</span><br><span>    uint8_t wi;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+       /*! Waiting Time, in ETU (ISO7816-3 Section 8.1).</span><br><span style="color: hsl(120, 100%, 40%);">+      *  \note this depends on Fi, Di, and WI if T=0 is used */</span><br><span style="color: hsl(120, 100%, 40%);">+    uint32_t waiting_time;  /* in etu */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>       uint8_t tc_chan;        /* TC channel number */</span><br><span>      uint8_t uart_chan;      /* UART channel */</span><br><span> </span><br><span>       uint8_t in_ep;          /* USB IN EP */</span><br><span>      uint8_t irq_ep;         /* USB IN EP */</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-     uint32_t waiting_time;  /* in etu */</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span>         /* ATR state machine */</span><br><span>      struct {</span><br><span>             uint8_t idx;</span><br><span>@@ -361,16 +377,16 @@</span><br><span> {</span><br><span>    int rc;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-     rc = compute_fidi_ratio(ch->Fi, ch->Di);</span><br><span style="color: hsl(120, 100%, 40%);">+        rc = compute_fidi_ratio(ch->F_index, ch->D_index);</span><br><span>     if (rc > 0 && rc < 0x400) {</span><br><span style="color: hsl(0, 100%, 40%);">-               TRACE_INFO("%u: computed Fi(%u) Di(%u) ratio: %d\r\n",</span><br><span style="color: hsl(0, 100%, 40%);">-                            ch->num, ch->Fi, ch->Di, rc);</span><br><span style="color: hsl(120, 100%, 40%);">+            TRACE_INFO("%u: computed F(%u)/D(%u) ratio: %d\r\n", ch->num,</span><br><span style="color: hsl(120, 100%, 40%);">+                       ch->F_index, ch->D_index, rc);</span><br><span>              /* make sure UART uses new F/D ratio */</span><br><span>              card_emu_uart_update_fidi(ch->uart_chan, rc);</span><br><span>             /* notify ETU timer about this */</span><br><span>            tc_etu_set_etu(ch->tc_chan, rc);</span><br><span>  } else</span><br><span style="color: hsl(0, 100%, 40%);">-          TRACE_INFO("%u: computed FiDi ration %d unsupported\r\n",</span><br><span style="color: hsl(120, 100%, 40%);">+           TRACE_INFO("%u: computed F/D ratio %d unsupported\r\n",</span><br><span>                       ch->num, rc);</span><br><span> }</span><br><span> </span><br><span>@@ -395,8 +411,10 @@</span><br><span>          break;</span><br><span>       case ISO_S_WAIT_ATR:</span><br><span>                 /* Reset to initial Fi / Di ratio */</span><br><span style="color: hsl(0, 100%, 40%);">-            ch->Fi = 1;</span><br><span style="color: hsl(0, 100%, 40%);">-          ch->Di = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+                ch->Fi_index = ch->F_index = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+         ch->Di_index = ch->D_index = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+         ch->wi = ISO7816_3_DEFAULT_WI;</span><br><span style="color: hsl(120, 100%, 40%);">+             ch->waiting_time = ISO7816_3_INIT_WTIME;</span><br><span>          emu_update_fidi(ch);</span><br><span>                 /* the ATR should only be sent 400 to 40k clock cycles after the RESET.</span><br><span>               * we use the tc_etu mechanism to wait this time.</span><br><span>@@ -490,7 +508,7 @@</span><br><span>                      }</span><br><span>            }</span><br><span>            /* update waiting time (see ISO 7816-3 10.2) */</span><br><span style="color: hsl(0, 100%, 40%);">-         ch->waiting_time = ch->wi * 960 * ch->Fi;</span><br><span style="color: hsl(120, 100%, 40%);">+            ch->waiting_time = ch->wi * 960 * fi_table[ch->F_index];</span><br><span>            tc_etu_set_wtime(ch->tc_chan, ch->waiting_time);</span><br><span>               /* go to next state */</span><br><span>               card_set_state(ch, ISO_S_WAIT_TPDU);</span><br><span>@@ -626,9 +644,11 @@</span><br><span>  case PTS_S_WAIT_RESP_PTS1:</span><br><span>           byte = ch->pts.resp[_PTS1];</span><br><span>               /* This must be TA1 */</span><br><span style="color: hsl(0, 100%, 40%);">-          ch->Fi = byte >> 4;</span><br><span style="color: hsl(0, 100%, 40%);">-            ch->Di = byte & 0xf;</span><br><span style="color: hsl(0, 100%, 40%);">-             TRACE_DEBUG("%u: found Fi=%u Di=%u\r\n", ch->num, ch->Fi, ch->Di);</span><br><span style="color: hsl(120, 100%, 40%);">+         ch->F_index = byte >> 4;</span><br><span style="color: hsl(120, 100%, 40%);">+             ch->D_index = byte & 0xf;</span><br><span style="color: hsl(120, 100%, 40%);">+              TRACE_DEBUG("%u: found F=%u D=%u\r\n", ch->num,</span><br><span style="color: hsl(120, 100%, 40%);">+                      fi_table[ch->F_index], di_table[ch->D_index]);</span><br><span style="color: hsl(120, 100%, 40%);">+              /* FIXME: if F or D are 0, become unresponsive to signal error condition */</span><br><span>          break;</span><br><span>       case PTS_S_WAIT_RESP_PTS2:</span><br><span>           byte = ch->pts.resp[_PTS2];</span><br><span>@@ -653,7 +673,7 @@</span><br><span>         switch (ch->pts.state) {</span><br><span>  case PTS_S_WAIT_RESP_PCK:</span><br><span>            card_emu_uart_wait_tx_idle(ch->uart_chan);</span><br><span style="color: hsl(0, 100%, 40%);">-           /* update baud rate generator with Fi/Di */</span><br><span style="color: hsl(120, 100%, 40%);">+           /* update baud rate generator with F/D */</span><br><span>            emu_update_fidi(ch);</span><br><span>                 /* Wait for the next TPDU */</span><br><span>                 card_set_state(ch, ISO_S_WAIT_TPDU);</span><br><span>@@ -1024,8 +1044,8 @@</span><br><span>         if (ch->in_reset)</span><br><span>                 sts->flags |= CEMU_STATUS_F_RESET_ACTIVE;</span><br><span>         /* FIXME: voltage + card insert */</span><br><span style="color: hsl(0, 100%, 40%);">-      sts->fi = ch->Fi;</span><br><span style="color: hsl(0, 100%, 40%);">- sts->di = ch->Di;</span><br><span style="color: hsl(120, 100%, 40%);">+       sts->F_index = ch->F_index;</span><br><span style="color: hsl(120, 100%, 40%);">+     sts->D_index = ch->D_index;</span><br><span>    sts->wi = ch->wi;</span><br><span>      sts->waiting_time = ch->waiting_time;</span><br><span> </span><br><span>@@ -1231,8 +1251,8 @@</span><br><span>      ch->in_reset = in_reset;</span><br><span>  ch->clocked = clocked;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   ch->Fi = 0;</span><br><span style="color: hsl(0, 100%, 40%);">-  ch->Di = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+        ch->Fi_index = ch->F_index = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ ch->Di_index = ch->D_index = 1;</span><br><span>        ch->wi = ISO7816_3_DEFAULT_WI;</span><br><span> </span><br><span>        ch->tc_chan = tc_chan;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/simtrace2/+/23639">change 23639</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/simtrace2/+/23639"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: simtrace2 </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ieb2425e8380a81b79df7b2bd072902994e9c3ee7 </div>
<div style="display:none"> Gerrit-Change-Number: 23639 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-CC: Jenkins Builder </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>