Min Xu patches testing (Lukas Kuzmiak)

[Min Xu]

Hi Lukas

I saw your email to my @sanjole.com address but since this address is
on the mailing list I'll just reply here so everyone can see.

--- your email below ---
Hi Min Xu,

first of all thanks for all the work you put into simtrace.

I’ve been testing your patches last couple days, they have not yet
been in any release which I think is a shame, so I’d like to push the
community to fix that so it can be even further tested (by testing,
reporting back, fixing building the firmware with latest
arm-none-eabi, etc.)

I have found one bug for which I fail to identify the source .. The
whole trace is fine except sometimes there are 4 bytes inside the data
which seems to be the simtrace_hdr (first line is my debug):

USB MSG: sh->cmd: 1, sh->flags: 0, sh->res[9, 5], payload: 00 a4 00 04
02 a4 6f 07 61 2a 00 c0 00 00 2a c0 62 28 82 02 41 21 83 02 6f 07 a5
0f 80 01 71 c0 01 00 91 04 7f 20 6f 07 92 01 00 8a 01 05 8b 03 6f 06
03 80 02 00 09 88 01 38 90 00 01 00 09 05 00 b0 00 00 09 b0 08 29 03
30 10 66 03 91 12 90 00
APDU: 00 a4 00 04 02 6f 07 61 2a
APDU: 00 c0 00 00 2a 62 28 82 02 41 21 83 02 6f 07 a5 0f 80 01 71 c0
01 00 91 04 7f 20 6f 07 92 01 00 8a 01 05 8b 03 6f 06 03 80 02 00 09
88 01 38 90 00
APDU: 01 00 09 05 00 b0 00
APDU: 00 09 b0 08 29 03 30
APDU: 10 66 03 91 12 90 00

The 01 00 09 05 (which seems to be sh->cmd, sh->flags and Fi/Di (9/5)
just randomly appear in APDU data every now and then. If those 4 bytes
were not there apdu_split would split it fine .. like this it breaks
this into nonsense pieces and breaks the trace.

I’ve gone through your patches in the firmware, fiddled around with
some of them but didn’t manage to find the root cause yet - seems like
the simtrace_hdr is inserted in the middle (but that does not seem
possible), so perhaps 2 USB messages somehow get merged into one?
So I figured I’ll try to write you, maybe it will ring a bell - I have
not fully verified the merge of your patches went correctly but from a
fast compare it seems like it.

If you’ll find a minute to give me a few tips I’ll appreciate that, in
the meantime I’ll keep digging.

PS: the version of FW I’m using is latest master in the git
(https://git.osmocom.org/openpcd).

Thanks!
Lukas
--- END ---

I believe the reason for this is actually in an email I sent to the
list on Sep 10, 2013.

Basically, the ATMEL chip can break up the req_ctx ( the usb response
) and combine as it see fit (if there's a large burst etc).  So you
cannot rely on the "natural break" between the calls to transmit.
Therefore, since it's a stream you'll get on receiving side, then a
natural packet header that accounts of subsequent bytes must be added.

So I added extra bytes into the simtrace_hdr header so that the actual
data payload can be correctly accounted for.

The changes are:

 struct simtrace_hdr {
        u_int8_t cmd;
        u_int8_t flags;
        u_int8_t res[2];
+       u_int16_t seq_num;
+       u_int16_t offset;
+       u_int16_t tot_len;
        u_int8_t data[0];
 } __attribute__ ((packed));


So the desktop client will have to have equivalent changes to account
for these extra fields.

Let me know if this answers your question.    If not I can try send
you the full code I have for the firmware and a sample of the desktop
receiving / parsing code so you'll have a baseline.