Incomplete trace (due to high-speed SIM?)

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/simtrace@lists.osmocom.org/.

Harald Welte laforge at gnumonks.org
Thu Sep 5 07:31:04 UTC 2013


Dear Min Xu,

On Wed, Sep 04, 2013 at 07:47:46PM -1000, Min Xu wrote:
> I made a bunch of changes that significantly improved my test scenario:

I _really_ appreciate your excellent technical work.  However, I would
like you to go the little extra effort it takes to 'properly' interact
with community based Free Software projects, where there are rules like

* only one feature / logical change per patch, resulting in a series
  of incremental patches, each taking us one step further, each
  rendering a compile-able and functional build after being applied.
* no re-formatting of code (whitespace changes)
* no patch for local changes like different compiler name in Makefile
* stick to coding style of the project (tab-wide indent, ...) for
  consistency
* remove old/dead code rather than comment it out (RCTX_STATE_...)
* avoid asm() statements whenever possible.  If you need them, please
  wrap them in an inline C function with descriptive name.

Some quesetions:
* why did the numberic values of RCTX_STATE_* have t be changed rather
  than amended by your new values, keeping the old ones as-is?

* interrupt nesting _should_ have been active all the time, see
  IRQ_Handler_Entry in Cstartup.S, where we first save SPSR and then
  un-set the IRQ and FIQ bits _before_ branching to the interrupt
  handler function:

	/*- Enable Interrupt and Switch in Supervisor Mode */
        msr     CPSR_c, #ARM_MODE_SVC

  Where ARM_MODE_SVC is set to 0x13, i.e. without 0x40 or 0x80.

  So if there's something wrong with the existing code, it should be
  fixed there rather than two copies of inline-asm in the USART and USB
  IRQ handler routines.

So I do want to merge this very much, but I think it needs clean-up
before it can be merged.  I don't expect to have much time for this in
the next couple of weeks, so I would be happy if either you or somebody
else on the list could work on this.

Regarding your comment about changing the USB protocol:  This would lead
to host / firmware version incompatibilities, and I'd like to avoid that
if possible in any way.  We already have the simtrace_hdr.flags
structure member, which contains things like SIMTRACE_FLAG_ATR.  I'm not
sure if I understand your request fully, but why not simply add flags
like:

* First fragment of a fragmented APDU
* More fragments to follow for this APDU

This way a new APDU still has to start at the beginning of a USB
transfer, but no changes to the simtrace_hdr are required.


Thanks again,
	Harald


-- 
- Harald Welte <laforge at gnumonks.org>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)




More information about the simtrace mailing list