Hoernchen has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16284 )


Change subject: fsm completion event handling from main loop
......................................................................

fsm completion event handling from main loop

The main loop will now poll for finished/failed transactions and handle
them, this was previously handled during the last rx interrupt of a
transaction, which was bad for timing. This does also fix malloc/free
while handling interrupts.

Change-Id: I055110720089e20e65db592eccc3ce4d618e8c63
---
M ccid_common/ccid_device.h
M ccid_common/ccid_slot_fsm.c
M ccid_common/iso7816_fsm.c
M sysmoOCTSIM/main.c
4 files changed, 68 insertions(+), 21 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-ccid-firmware 
refs/changes/84/16284/1

diff --git a/ccid_common/ccid_device.h b/ccid_common/ccid_device.h
index 38a71db..f364f72 100644
--- a/ccid_common/ccid_device.h
+++ b/ccid_common/ccid_device.h
@@ -57,6 +57,8 @@
        struct ccid_pars_decoded proposed_pars;
        /* default parameters; applied on ResetParameters */
        const struct ccid_pars_decoded *default_pars;
+       volatile uint32_t event;
+       volatile void* event_data;
 };

 /* CCID operations provided by USB transport layer */
@@ -86,6 +88,7 @@
                          const struct ccid_pars_decoded *pars_dec);
        int (*set_rate_and_clock)(struct ccid_slot *cs, uint32_t freq_hz, 
uint32_t rate_bps);
        void (*icc_set_insertion_status)(struct ccid_slot *cs, bool present);
+       int (*handle_fsm_events)(struct ccid_slot *cs, bool enable);
 };

 /* An instance of CCID (i.e. a card reader device) */
diff --git a/ccid_common/ccid_slot_fsm.c b/ccid_common/ccid_slot_fsm.c
index d2aec26..9be31d6 100644
--- a/ccid_common/ccid_slot_fsm.c
+++ b/ccid_common/ccid_slot_fsm.c
@@ -110,34 +110,76 @@
 {
        struct iso_fsm_slot *ss = iso7816_fsm_get_user_priv(fi);
        struct ccid_slot *cs = ss->cs;
+
+       switch (event) {
+       case ISO7816_E_ATR_DONE_IND:
+       case ISO7816_E_ATR_ERR_IND:
+       case ISO7816_E_TPDU_DONE_IND:
+       case ISO7816_E_TPDU_FAILED_IND:
+       case ISO7816_E_PPS_DONE_IND:
+       case ISO7816_E_PPS_FAILED_IND:
+               cs->event_data = data;
+               asm volatile("dmb st": : :"memory");
+               cs->event = event;
+               break;
+       default:
+               LOGPCS(cs, LOGL_NOTICE, "%s(event=%d, cause=%d, data=%p) 
unhandled\n",
+                       __func__, event, cause, data);
+               break;
+       }
+}
+
+static int iso_handle_fsm_events(struct ccid_slot *cs, bool enable){
+       struct iso_fsm_slot *ss = ccid_slot2iso_fsm_slot(cs);
        struct msgb *tpdu, *resp;
+       volatile uint32_t event = cs->event;
+       volatile void * volatile data = cs->event_data;
+
+       if(!event)
+               return 0;
+       if(event && !data)
+               return 0;

        switch (event) {
        case ISO7816_E_ATR_DONE_IND:
                tpdu = data;
-               LOGPCS(cs, LOGL_DEBUG, "%s(event=%d, cause=%d, data=%s)\n", 
__func__, event, cause,
+               LOGPCS(cs, LOGL_DEBUG, "%s(event=%d, data=%s)\n", __func__, 
event,
                        msgb_hexdump(tpdu));
                resp = ccid_gen_data_block(cs, ss->seq, CCID_CMD_STATUS_OK, 0,
                                           msgb_data(tpdu), msgb_length(tpdu));
                ccid_slot_send_unbusy(cs, resp);
                /* Don't free "TPDU" here, as the ATR should survive */
+               cs->event = 0;
+               break;
+       case ISO7816_E_ATR_ERR_IND:
+               tpdu = data;
+               LOGPCS(cs, LOGL_DEBUG, "%s(event=%d, data=%s)\n", __func__, 
event,
+                       msgb_hexdump(tpdu));
+               resp = ccid_gen_data_block(cs, ss->seq, CCID_CMD_STATUS_FAILED, 
CCID_ERR_ICC_MUTE,
+                                          msgb_data(tpdu), msgb_length(tpdu));
+               ccid_slot_send_unbusy(cs, resp);
+               /* Don't free "TPDU" here, as the ATR should survive */
+               cs->event = 0;
+               break;
                break;
        case ISO7816_E_TPDU_DONE_IND:
                tpdu = data;
-               LOGPCS(cs, LOGL_DEBUG, "%s(event=%d, cause=%d, data=%s)\n", 
__func__, event, cause,
+               LOGPCS(cs, LOGL_DEBUG, "%s(event=%d, data=%s)\n", __func__, 
event,
                        msgb_hexdump(tpdu));
                resp = ccid_gen_data_block(cs, ss->seq, CCID_CMD_STATUS_OK, 0, 
msgb_l2(tpdu), msgb_l2len(tpdu));
                ccid_slot_send_unbusy(cs, resp);
                msgb_free(tpdu);
+               cs->event = 0;
                break;
        case ISO7816_E_TPDU_FAILED_IND:
                tpdu = data;
-               LOGPCS(cs, LOGL_DEBUG, "%s(event=%d, cause=%d, data=%s)\n", 
__func__, event, cause,
+               LOGPCS(cs, LOGL_DEBUG, "%s(event=%d, data=%s)\n", __func__, 
event,
                        msgb_hexdump(tpdu));
                /* FIXME: other error causes than card removal?*/
                resp = ccid_gen_data_block(cs, ss->seq, CCID_CMD_STATUS_FAILED, 
CCID_ERR_ICC_MUTE, msgb_l2(tpdu), 0);
                ccid_slot_send_unbusy(cs, resp);
                msgb_free(tpdu);
+               cs->event = 0;
                break;
        case ISO7816_E_PPS_DONE_IND:
                tpdu = data;
@@ -157,6 +199,7 @@

                /* this frees the pps req from the host, pps resp buffer stays 
with the pps fsm */
                msgb_free(tpdu);
+               cs->event = 0;
                break;
        case ISO7816_E_PPS_FAILED_IND:
                tpdu = data;
@@ -165,10 +208,13 @@
                ccid_slot_send_unbusy(cs, resp);
                /* this frees the pps req from the host, pps resp buffer stays 
with the pps fsm */
                msgb_free(tpdu);
+               cs->event = 0;
+               break;
+       case 0:
                break;
        default:
-               LOGPCS(cs, LOGL_NOTICE, "%s(event=%d, cause=%d, data=%p) 
unhandled\n",
-                       __func__, event, cause, data);
+               LOGPCS(cs, LOGL_NOTICE, "%s(event=%d, data=%p) unhandled\n",
+                       __func__, event, data);
                break;
        }
 }
@@ -177,7 +223,7 @@
                                const struct ccid_pc_to_rdr_xfr_block *xfb)
 {
        struct iso_fsm_slot *ss = ccid_slot2iso_fsm_slot(cs);
-       struct msgb *tpdu;
+

        ss->seq = xfb->hdr.bSeq;

@@ -185,17 +231,10 @@
        OSMO_ASSERT(xfb->wLevelParameter == 0x0000);
        OSMO_ASSERT(msgb_length(msg) > xfb->hdr.dwLength);

-       /* 'msg' contains the raw CCID message as received from USB. We could 
create
-        * a new message buffer for the ISO7816 side here or we could 'strip 
the CCID
-        * header off the start of the message. Let's KISS and do a copy here */
-       tpdu = msgb_alloc(512, "TPDU");
-       OSMO_ASSERT(tpdu);
-       memcpy(msgb_data(tpdu), xfb->abData, xfb->hdr.dwLength);
-       msgb_put(tpdu, xfb->hdr.dwLength);
-       msgb_free(msg);
+       msgb_pull(msg, 10);

-       LOGPCS(cs, LOGL_DEBUG, "scheduling TPDU transfer: %s\n", 
msgb_hexdump(tpdu));
-       osmo_fsm_inst_dispatch(ss->fi, ISO7816_E_XCEIVE_TPDU_CMD, tpdu);
+       LOGPCS(cs, LOGL_DEBUG, "scheduling TPDU transfer: %s\n", 
msgb_hexdump(msg));
+       osmo_fsm_inst_dispatch(ss->fi, ISO7816_E_XCEIVE_TPDU_CMD, msg);
        /* continues in iso_fsm_clot_user_cb once response/error/timeout is 
received */
        return 0;
 }
@@ -329,4 +368,5 @@
        .set_clock = iso_fsm_slot_set_clock,
        .set_params = iso_fsm_slot_set_params,
        .set_rate_and_clock = iso_fsm_slot_set_rate_and_clock,
+       .handle_fsm_events = iso_handle_fsm_events,
 };
diff --git a/ccid_common/iso7816_fsm.c b/ccid_common/iso7816_fsm.c
index 3778dcc..44c8191 100644
--- a/ccid_common/iso7816_fsm.c
+++ b/ccid_common/iso7816_fsm.c
@@ -238,6 +238,8 @@
                osmo_fsm_inst_state_chg_ms(fi, ISO7816_S_WAIT_ATR,
                                           fi_cycles2ms(fi, 40000), T_WAIT_ATR);
                break;
+       case ISO7816_E_POWER_UP_IND:
+               break;
        case ISO7816_E_PPS_FAILED_IND:
                msg = data;
                /* notify user about PPS result */
@@ -876,7 +878,7 @@
                msgb_reset(atp->rx_cmd);

        /* notify in case card got pulled out */
-       if (atp->tx_cmd){
+       if (atp->tx_cmd && old_state != PPS_S_DONE){
                osmo_fsm_inst_dispatch(fi->proc.parent,
                                ISO7816_E_PPS_FAILED_IND, atp->tx_cmd);
                atp->tx_cmd = 0;
@@ -1099,7 +1101,7 @@
        struct tpdu_fsm_priv *tfp = get_tpdu_fsm_priv(fi);
 
        /* notify in case card got pulled out */
-       if (tfp->tpdu){
+       if (tfp->tpdu && old_state != TPDU_S_DONE){
                osmo_fsm_inst_dispatch(fi->proc.parent, 
ISO7816_E_TPDU_FAILED_IND, tfp->tpdu);
                tfp->tpdu = 0;
        }
diff --git a/sysmoOCTSIM/main.c b/sysmoOCTSIM/main.c
index 02b7b09..806a9f6 100644
--- a/sysmoOCTSIM/main.c
+++ b/sysmoOCTSIM/main.c
@@ -267,7 +267,7 @@
 {
        uint8_t statusbytes[2] = {0};
        //struct msgb *msg = ccid_msgb_alloc();
-       struct msgb *msg = msgb_alloc(64, "IRQ");
+       struct msgb *msg = msgb_alloc(300,"IRQ");
        struct ccid_rdr_to_pc_notify_slot_change *nsc = msgb_put(msg, 
sizeof(*nsc) + sizeof(statusbytes));
        nsc->bMessageType = RDR_to_PC_NotifySlotChange;

@@ -523,6 +523,9 @@
                command_try_recv();
                poll_card_detect();
                submit_next_irq();
+               for (int i = 0; i < usb_fs_descs.ccid.class.bMaxSlotIndex; i++){
+                       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);
@@ -533,13 +536,12 @@
                                msgb_free(msg);
                        }
                if(qs < NUM_OUT_BUF)
-                       for (int i= 0; i < qs-NUM_OUT_BUF; i++){
+                       for (int i= 0; i < NUM_OUT_BUF-qs; i++){
                                struct msgb *msg = msgb_alloc(300,"ccid");
                                OSMO_ASSERT(msg);
                                /* return the message back to the queue of free 
message buffers */
                                llist_add_tail_at(&msg->list, &g_ccid_s.free_q);
                        }

-
        }
 }

--
To view, visit https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16284
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-ccid-firmware
Gerrit-Branch: master
Gerrit-Change-Id: I055110720089e20e65db592eccc3ce4d618e8c63
Gerrit-Change-Number: 16284
Gerrit-PatchSet: 1
Gerrit-Owner: Hoernchen <ew...@sysmocom.de>
Gerrit-MessageType: newchange

Reply via email to