955946 - B2G RIL: Distinguish SIM/USIM getResponse p1/p2/p3 parameters.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[shawn ku [:sku]]

By TS 151 011 clause 9.2.1 SELECT, p1/p2 should be set to '00', and p3 (length) is fixed to 15 bytes.

And,

By TS 102 221 clause 11.1.1 SELECT, p1 should be '00', p2 should be '04' (Return FCP template), and p3 should be '00' (In order to retrieve the entire FCP template, Le should be set to '00'.)

We need to distinguish between SIM and USIM parameters to get correct response.

Comment 1

[:gerard-majax]

Do we know the behavior of RIL's partner ?

Comment 2

[:gerard-majax]

Does this behavior also applies to other commands? I hacked the getResponse handler, forcing p1 to 0, p2 to 4 and p3 to 0. As far as I could say, it did not worked and I ended up having my SIM card not recognized.

Comment 3

[shawn ku [:sku]]

(In reply to Alexandre LISSY :gerard-majax from comment #2)
> Does this behavior also applies to other commands? I hacked the getResponse
> handler, forcing p1 to 0, p2 to 4 and p3 to 0. As far as I could say, it did
> not worked and I ended up having my SIM card not recognized.

Hi Alexandre:
 I will find partner on Monday to have more discussion first.
And, I will keep you posted.

Thanks!!
sku

Comment 4

[shawn ku [:sku]]

Attachment: p1_0_p2_0_p3_0.log

Comment 5

[shawn ku [:sku]]

Attachment: p1_0_p2_4_p3_0.log

Comment 6

[shawn ku [:sku]]

I tried different parameters on Unagi, it seems the results are both the same.
ril_worker.js both get 51.011 response format (with 15-byte-long result).

p1=0, p2=0, p3=0 case:
01-06 03:02:17.064   111   187 D RILC    : RIL_SIM_IO_Response: sw1=144 sw2=0 data=0000000a2fe2040000000005020000

p1=0, p2=0x04, p3=0 case:
01-06 16:25:31.249   111   187 D RILC    : RIL_SIM_IO_Response: sw1=144 sw2=0 data=0000000a2fe2040000000005020000

Comment 7

[shawn ku [:sku]]

Hi Anshul:
 Could you please help check Comment 6 to see if Unagi can support FCP template for ICC_IO getResponse?
It looks to me there are some hack in RILD/modem to always return 51.011 response format to Gecko.

Thanks!!
sku

Comment 8

[Anshul]

Shawn, I am sorry but we don't support Unagi device.

Comment 9

[shawn ku [:sku]]

Hi Anshul:
 I guess I did not express ni? clearly.
I tried to ask if codeaurora code base can report FCP template to GECKO?

It seems to me that codeaurora hack some stuffs in RILD.

Thanks!!
sku

Comment 10

[:gerard-majax]

(In reply to shawn ku [:sku] from comment #6)
> I tried different parameters on Unagi, it seems the results are both the
> same.
> ril_worker.js both get 51.011 response format (with 15-byte-long result).
> 
> p1=0, p2=0, p3=0 case:
> 01-06 03:02:17.064   111   187 D RILC    : RIL_SIM_IO_Response: sw1=144
> sw2=0 data=0000000a2fe2040000000005020000
> 
> p1=0, p2=0x04, p3=0 case:
> 01-06 16:25:31.249   111   187 D RILC    : RIL_SIM_IO_Response: sw1=144
> sw2=0 data=0000000a2fe2040000000005020000

I'll check this today.

Comment 11

[:gerard-majax]

p1=0, p2=0, p3=GET_RESPONSE_EF_SIZE_BYTES case:
 - SIM card is not detected
 - logcat -b radio shows truncated output:
   - "05-11 15:19:26.256  1697  1701 D HTC_RIL : (t=1399814366)<< +CRSM: 144,0,621C8202412183022FE2A503920100\r\n0\r"

p1=0, p2=0x04, p3=0 case:
 - SIM card is detected
 - Some ICCIO values behave strangely, namely:
   - SPN is read as "oste Mobile" instead of "La Poste Mobile"
 - logcat -b radio shows non truncated output:
   - "05-11 15:22:33.318  1697  1701 D HTC_RIL : (t=1399814553)<< +CRSM: 144,0,621C8202412183022FE2A5039201008A01058B032F061B8002000A880110\r\n0\r"

p1=0, p2=0, p3=0 case:
 - SIM card is detected
 - I see no bad SPN
 - logcat -b radio shows non truncated output:
   - "05-11 15:28:27.534  1697  1701 D HTC_RIL : (t=1399814907)<< +CRSM: 144,0,621C8202412183022FE2A5039201008A01058B032F061B8002000A880110\r\n0\r"

Comment 12

[shawn ku [:sku]]

Hi Alexandre:
 GET RESPONSE w/ (p1=0, p2=0x04, p3=0) and (p1=0, p2=0, p3=0) are the same - 621C8202412183022FE2A5039201008A01058B032F061B8002000A880110, need to check the READ BINARY result.

Could you please provide me whole radio/device log to check?

// Command to get radio/device log
adb logcat -b radio -b main -v threadtime



62 1C 
82 02 41 21       // File Descriptor            , Transparent structure
83 02 2F E2       // File Identifier            , 0x2FE2
A5 03 92 01 00    // Proprietary information 
8A 01 05          // Life Cycle Status Integer
8B 03 2F 06 1B    // Security attributes
80 02 00 0A       // File size                  , 0x0A
88 01 10          // Short File Identifier (SFI)

Comment 13

[shawn ku [:sku]]

Hi Alexandre:
 the loadTransparentEF (see [1]) in ril_wroker.js also need some revise to make transparent EF works properly.

Per 3GPP spec., the p2 for read binary is dedicated to "Offset low", and it should be 0.
However, p2 is 0x04 while requesting getResponse, and that value did not reset to 0 that cause we got 4 bytes offset then.


ETSI TS 102 221
11.1.3 READ BINARY
Code                       Value
CLA                        As specified in clause 10.1.1
INS                        As specified in clause 10.1.2
P1                         See table 11.10
P2                         Offset low
Lc                         Not present
Data                       Not present
Le                         Number of bytes to be read


[1] - http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js?from=ril_worker.js&case=true#11840

Comment 14

[:gerard-majax]

(In reply to shawn ku [:sku] from comment #13)
> Hi Alexandre:
>  the loadTransparentEF (see [1]) in ril_wroker.js also need some revise to
> make transparent EF works properly.
> 
> Per 3GPP spec., the p2 for read binary is dedicated to "Offset low", and it
> should be 0.
> However, p2 is 0x04 while requesting getResponse, and that value did not
> reset to 0 that cause we got 4 bytes offset then.
> 
> 
> ETSI TS 102 221
> 11.1.3 READ BINARY
> Code                       Value
> CLA                        As specified in clause 10.1.1
> INS                        As specified in clause 10.1.2
> P1                         See table 11.10
> P2                         Offset low
> Lc                         Not present
> Data                       Not present
> Le                         Number of bytes to be read
> 
> 
> [1] -
> http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.
> js?from=ril_worker.js&case=true#11840

Nice catch. Switching back p2 to 0x04 when performing GET_RESPONSE and forcing p2 to 0 when performing READ_BINARY, I now get correct behavior.

Comment 15

[:gerard-majax]

Attachment: 0001-Bug-955946-HTC-Vision-Request-FCP-Template.patch

Shawn, here is my latest try that works as expected.

Comment 16

[:gerard-majax]

Shawn, what's the rationale for p2=0x04 ? According to TS 102 221, section 12.1.1.2, the table 12.1 gives the parameters for GET_RESPONSE:
 - CLA : as specified in clause 10.1.1
 - INS : as specified in clause 10.1.2
 - P1 : '00'
 - P2 : '00'
 - Lc : Not present
 - Data : Not present
 - Le : '00' or the value of sw2 of the previous command.

Comment 17

[shawn ku [:sku]]

For get response, we might need to check below table.
Table 11.2: Coding of P2
---
The section 12.1.1.2 describe the command from card to terminal. (ie: device to SIM).

Please correct me if anything I replied is wrong. :)

Comment 18

[shawn ku [:sku]]

typo.....

The section 12.1.1.2 describe the command from card to terminal. (ie: SIM to device).

Comment 19

[shawn ku [:sku]]

Comment on attachment 8420772 [details] [diff] [review]
0001-Bug-955946-HTC-Vision-Request-FCP-Template.patch

Review of attachment 8420772 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/ril_worker.js
@@ +11860,4 @@
>        throw new Error("Unknown pathId for " + options.fileId.toString(16));
>      }
>      options.p1 = 0; // For GET_RESPONSE, p1 = 0
> +    options.p2 = 0x04; // For GET_RESPONSE, p2 = 0x04

We should consider the SIM/USIM case, and setup p2 then.

SIM: p2 = 0 (TS 51.011 clause 9.2.1 SELECT)
USIM: p2 = 0x04 (TS 102 221 Table 11.1: Coding of P1)

@@ +11860,5 @@
>        throw new Error("Unknown pathId for " + options.fileId.toString(16));
>      }
>      options.p1 = 0; // For GET_RESPONSE, p1 = 0
> +    options.p2 = 0x04; // For GET_RESPONSE, p2 = 0x04
> +    options.p3 = 0; //

Same as above,
SIM: p3 need to be 15 (GET_RESPONSE_EF_SIZE_BYTES).
USIM: p3 need to be 0 (TS 102 221 clause 11.1.1.3 Response Data)

Comment 20

[:gerard-majax]

(In reply to shawn ku [:sku] from comment #17)
> For get response, we might need to check below table.
> Table 11.2: Coding of P2
> ---
> The section 12.1.1.2 describe the command from card to terminal. (ie: device
> to SIM).
> 
> Please correct me if anything I replied is wrong. :)

Ah yes, you're right. Coding of P2 in the device to SIM case is described in table 11.35:

b8 b7 b6 b5 b4 b3 b2 b1 -- Meaning
 1  0  -  -  -  -  -  -  | First block
 0  0  -  0  0  0  0  0  | Next block
 0  1  -  0  0  0  0  0  | Retransmit previous block
[...]
 -  -  -  0  0  0  0  0  | Current EF

Comment 21

[:gerard-majax]

Attachment: 0001-Bug-955946-HTC-Vision-Request-FCP-Template.patch

Setting review? although I suspect we would want tests for this, but I'm unsure how we can perform those.

Comment 22

[shawn ku [:sku]]

Comment on attachment 8420829 [details] [diff] [review]
0001-Bug-955946-HTC-Vision-Request-FCP-Template.patch

Review of attachment 8420829 [details] [diff] [review]:
-----------------------------------------------------------------

I don't have permission to review the patch. Please ask owner/peer to review it:)

I am still happy with feedback?

Comment 23

[shawn ku [:sku]]

[e-mail] will be the proper reviewer for this topic.

Comment 24

[:gerard-majax]

Comment on attachment 8420829 [details] [diff] [review]
0001-Bug-955946-HTC-Vision-Request-FCP-Template.patch

Make it a feedback? :)

I'm not sure what we should be doing for other SIM cards types.

Comment 25

[shawn ku [:sku]]

Comment on attachment 8420829 [details] [diff] [review]
0001-Bug-955946-HTC-Vision-Request-FCP-Template.patch

Review of attachment 8420829 [details] [diff] [review]:
-----------------------------------------------------------------

RUIM record also need to be considered, or CDMA projects will be affected.
Since I am not familiar with RUIM, please ask Yoshi to have more detail comment. :)

http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js?from=ril_worker.js&case=true#13393

Comment 26

[:gerard-majax]

Yoshi, what should be done for the case of RUIM SIM cards ? And do we need to handle others also ? (I see ISIM and CSIM in ril_consts.js).

Comment 27

[:gerard-majax]

Shawn, no news on this?

Comment 28

[shawn ku [:sku]]

Hi Alexandre:
 My opinion is to add an extra handling for USIM case, and keep original logic for rest of cases.

ex:
case USIM
  // USIM case
  break;
default:
  // Original case
  break;

Besides,
We still need Yoshi's opinion/suggesion here.

Thanks!!
Shawn

Comment 29

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8420829 [details] [diff] [review]
0001-Bug-955946-HTC-Vision-Request-FCP-Template.patch

Review of attachment 8420829 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the late response,
8 lines of context, please.

For SIM_IO, CDMA should also behave the same way,
however I don't have CDMA phone to test this.

::: dom/system/gonk/ril_worker.js
@@ +11860,4 @@
>        throw new Error("Unknown pathId for " + options.fileId.toString(16));
>      }
>      options.p1 = 0; // For GET_RESPONSE, p1 = 0
> +    switch(GECKO_CARD_TYPE.indexOf(this.context.RIL.iccInfo.iccType)) {

space after switch, and use 'RIL.appType'

@@ +11865,5 @@
> +        options.p2 = 0x00;
> +        options.p3 = GET_RESPONSE_EF_SIZE_BYTES;
> +        break;
> +      case CARD_APPTYPE_USIM:
> +        options.p2 = 0x04;

const this.

@@ +11869,5 @@
> +        options.p2 = 0x04;
> +        options.p3 = 0x00;
> +        break;
> +      default:
> +        // Nothing to do.

This patch will definitely fail on CSIM/RUIM.

Comment 30

[:gerard-majax]

(In reply to Yoshi Huang[:allstars.chh] from comment #29)
> @@ +11869,5 @@
> > +        options.p2 = 0x04;
> > +        options.p3 = 0x00;
> > +        break;
> > +      default:
> > +        // Nothing to do.
> 
> This patch will definitely fail on CSIM/RUIM.

I was expecting this, but I have no idea what should be done in this case.

Comment 31

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Sorry I don't know CDMA supports FCP or not.
And I don't know anyone is working on CDMA now.
So handling USIM only, and leave other types as they are right now.

Comment 32

[:gerard-majax]

(In reply to Yoshi Huang[:allstars.chh] from comment #31)
> Sorry I don't know CDMA supports FCP or not.
> And I don't know anyone is working on CDMA now.
> So handling USIM only, and leave other types as they are right now.

Thanks. I'll keep the old code path for RUIM/CSIM/ISIM then.

Comment 33

[:gerard-majax]

Attachment: [HTC Vision] Request FCP Template

Comment 34

[:gerard-majax]

Comment on attachment 8466916 [details] [diff] [review]
[HTC Vision] Request FCP Template

Yoshi, this last version should keep the old behavior for RUIM/CSIM/ISIM.

Comment 35

[Edgar Chen [:edgar]]

Comment on attachment 8466916 [details] [diff] [review]
[HTC Vision] Request FCP Template

Review of attachment 8466916 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for being late.

I am going to test this change in flame and dolphin to make sure both of them can work well with this change.

::: dom/system/gonk/ril_worker.js
@@ +12316,5 @@
>        throw new Error("Unknown pathId for " + options.fileId.toString(16));
>      }
>      options.p1 = 0; // For GET_RESPONSE, p1 = 0
> +    switch (GECKO_CARD_TYPE.indexOf(this.context.RIL.appType)) {
> +      case CARD_APPTYPE_USIM:

Hmm, the way you check appType is wrong. :(

Please use,

switch (this.context.RIL.appType) {
 case CARD_APPTYPE_USIM:
 ....

Comment 36

[:gerard-majax]

Attachment: 0001-Bug-955946-HTC-Vision-Request-FCP-Template.patch

Edgar, this new version should address the appType checking you spotted :)

Comment 37

[Edgar Chen [:edgar]]

Comment on attachment 8482130 [details] [diff] [review]
0001-Bug-955946-HTC-Vision-Request-FCP-Template.patch

Review of attachment 8482130 [details] [diff] [review]:
-----------------------------------------------------------------

I tested flame and dolphin, both of them works good with this patch.
Thank you, lissyx. :)

Comment 38

[Carsten Book [:Tomcat]]

Hi, could you provide a try link for this patch thanks!

Comment 39

[:gerard-majax]

Try link: https://tbpl.mozilla.org/?tree=Try&rev=25acc069c8fa

I'll just amend the current patch to make the title more descriptive

Comment 40

[:gerard-majax]

Attachment: Distinguish SIM/USIM getResponse p1/p2/p3 parameters r=echen

Performing proper ICC_IO operations depends on the type of the SIM card.
For example, the Desire Z will work with FCP Template, so we need to
follow TS 102.221 table 11.2 to perform the proper request and get a FCP
Template. This is done when we write the ICC_IO command: we need to set
the p2/p3 fields depending on the SIM card type (SIM or USIM) so that
the proper behavior is triggered.

Comment 41

[:gerard-majax]

Comment on attachment 8483355 [details] [diff] [review]
Distinguish SIM/USIM getResponse p1/p2/p3 parameters r=echen

Carrying r+ from previous patch: just changing title and description.

Comment 42

[:gerard-majax]

(In reply to Carsten Book [:Tomcat] from comment #38)
> Hi, could you provide a try link for this patch thanks!

https://tbpl.mozilla.org/?tree=Try&rev=25acc069c8fa Mostly green

Comment 43

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #42)
> (In reply to Carsten Book [:Tomcat] from comment #38)
> > Hi, could you provide a try link for this patch thanks!
> 
> https://tbpl.mozilla.org/?tree=Try&rev=25acc069c8fa Mostly green

Green after the retriggers completed

Comment 44

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/2b64789c274e

Comment 45

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2b64789c274e