Question about Talitos Linux driver for MPC885

Question about Talitos Linux driver for MPC885

[leroy christophe]

Dear Kim,

I'm trying to use the Talitos crypto driver with the MPC885 
microcontroller. For the time being, it doesn't work.
The kernel startup blocks at the test of the DES function.

I have added the following definition in the DTS file:

         crypto@20000 {
             compatible = "fsl,sec2.0";
             reg = <0x20000 0x8000>;
             interrupts = <1 1>;
             interrupt-parent = <&PIC>;
             fsl,num-channels = <1>;
             fsl,channel-fifo-len = <24>;
             fsl,exec-units-mask = <0x4c>;
             fsl,descriptor-types-mask = <0x301f>;
         };

I have this defined in the config file

CONFIG_CRYPTO_HW=y
# CONFIG_CRYPTO_DEV_FSL_CAAM is not set
CONFIG_CRYPTO_DEV_TALITOS=y


Is there anything wrong in what I did ? Or is there something else I 
should do ?

Regards
Christophe

Re: Question about Talitos Linux driver for MPC885

[Kim Phillips]

On Tue, 25 Sep 2012 10:45:17 +0200
leroy christophe <christophe.leroy@c-s.fr> wrote:

> I'm trying to use the Talitos crypto driver with the MPC885 
> microcontroller. For the time being, it doesn't work.

yes, they're not exactly compatible...

> The kernel startup blocks at the test of the DES function.
> 
> I have added the following definition in the DTS file:
> 
>          crypto@20000 {
>              compatible = "fsl,sec2.0";

interesting, its called "SEC Lite" and its version register does
indeed say 2.  I see it has a single channel FIFO instead of a ring,
that the SEC v1.x (MPC185) used, so you probably don't have to
rewrite talitos_submit.

>              reg = <0x20000 0x8000>;
>              interrupts = <1 1>;

I couldn't find the IRQ line in the MPC855RM - if there's no IRQ
line, then that's a problem.

>              interrupt-parent = <&PIC>;
>              fsl,num-channels = <1>;
>              fsl,channel-fifo-len = <24>;
>              fsl,exec-units-mask = <0x4c>;
>              fsl,descriptor-types-mask = <0x301f>;

the descriptor type enumeration isn't uniform across into the mpc8xx
SEC version, e.g., the SEC Lite doesn't support the ipsec_esp
descriptor type, represented in mpc8xxx SEC versions as the second
bit, so this descriptor-types-mask setting should be fixed to at
least omit that since the driver checks for, and uses it if
available.

> Is there anything wrong in what I did ? Or is there something else I 
> should do ?

might want to go through the defines in talitos.h, e.g,
TALITOS_MCR_SWR is 0x1 on mpc8xxx vs. 0x10000000 on
mpc8xx (I suppose CONFIG_PPC_8xx can be used as the ifdef, btw).

Descriptor header and pointer formats, along with field locations,
sizes, and enumerations may also be different.

It also appears the SEC Lite doesn't support scatter-gather tables,
which will make performance hurt for fragmented (large) packet sizes.

Kim

Re: Question about Talitos Linux driver for MPC885

[leroy christophe]

Dear Kim,

Thank you for you quick answer.
I didn't have much time to look at it until this week unfortunatly.

I have some more questions/observations below

Le 26/09/2012 02:47, Kim Phillips a écrit :
> On Tue, 25 Sep 2012 10:45:17 +0200
> leroy christophe <christophe.leroy@c-s.fr> wrote:
>
>> I'm trying to use the Talitos crypto driver with the MPC885
>> microcontroller. For the time being, it doesn't work.
> yes, they're not exactly compatible...
>
>> The kernel startup blocks at the test of the DES function.
>>
>> I have added the following definition in the DTS file:
>>
>>           crypto@20000 {
>>               compatible = "fsl,sec2.0";
> interesting, its called "SEC Lite" and its version register does
> indeed say 2.  I see it has a single channel FIFO instead of a ring,
> that the SEC v1.x (MPC185) used, so you probably don't have to
> rewrite talitos_submit.
Good news, it was also my understanding.
>
>>               reg = <0x20000 0x8000>;
>>               interrupts = <1 1>;
> I couldn't find the IRQ line in the MPC855RM - if there's no IRQ
> line, then that's a problem.
Neither do I on the drawing, however in Table 52-1, there are 3 bits in 
the CPTR register for defining the interrupt level of the SEC lite, just 
like you do for the CPM and for the FEC.
So I believe this should be ok ?
>
>>               interrupt-parent = <&PIC>;
>>               fsl,num-channels = <1>;
>>               fsl,channel-fifo-len = <24>;
>>               fsl,exec-units-mask = <0x4c>;
>>               fsl,descriptor-types-mask = <0x301f>;
> the descriptor type enumeration isn't uniform across into the mpc8xx
> SEC version, e.g., the SEC Lite doesn't support the ipsec_esp
> descriptor type, represented in mpc8xxx SEC versions as the second
> bit, so this descriptor-types-mask setting should be fixed to at
> least omit that since the driver checks for, and uses it if
> available.
>
>> Is there anything wrong in what I did ? Or is there something else I
>> should do ?
> might want to go through the defines in talitos.h, e.g,
> TALITOS_MCR_SWR is 0x1 on mpc8xxx vs. 0x10000000 on
> mpc8xx (I suppose CONFIG_PPC_8xx can be used as the ifdef, btw).
I'm surprised about this, I didn't check the talitos.h file, but had 
checked the Reference Manual of the MPC 8272.
I rechecked yesterday and the SWR bit is at the same place as on the 
MPC885 which is different from what is defined in talitos.h
>
> Descriptor header and pointer formats, along with field locations,
> sizes, and enumerations may also be different.
>
> It also appears the SEC Lite doesn't support scatter-gather tables,
> which will make performance hurt for fragmented (large) packet sizes.
Does it mean something has to be modified if the SW ?
>
> Kim
>
Thanks,
Christophe

Re: Question about Talitos Linux driver for MPC885

[leroy christophe]

Kim,

Looking once more at talitos.h file, it looks like none of the values 
corresponds to what is in the reference manuals.
But it looks like in the talitos.h it is encoded as if it was with LSB 
first. Hence for instance the TALITOS_MCR_SWR which is 0x01 instead of  
0x01000000.
Then I'm wondering what/where the problem is.

Christophe


Le 23/11/2012 14:51, leroy christophe a écrit :
> Dear Kim,
>
> Thank you for you quick answer.
> I didn't have much time to look at it until this week unfortunatly.
>
> I have some more questions/observations below
>
> Le 26/09/2012 02:47, Kim Phillips a écrit :
>> On Tue, 25 Sep 2012 10:45:17 +0200
>> leroy christophe <christophe.leroy@c-s.fr> wrote:
>>
>>> I'm trying to use the Talitos crypto driver with the MPC885
>>> microcontroller. For the time being, it doesn't work.
>> yes, they're not exactly compatible...
>>
>>> The kernel startup blocks at the test of the DES function.
>>>
>>> I have added the following definition in the DTS file:
>>>
>>>           crypto@20000 {
>>>               compatible = "fsl,sec2.0";
>> interesting, its called "SEC Lite" and its version register does
>> indeed say 2.  I see it has a single channel FIFO instead of a ring,
>> that the SEC v1.x (MPC185) used, so you probably don't have to
>> rewrite talitos_submit.
> Good news, it was also my understanding.
>>
>>>               reg = <0x20000 0x8000>;
>>>               interrupts = <1 1>;
>> I couldn't find the IRQ line in the MPC855RM - if there's no IRQ
>> line, then that's a problem.
> Neither do I on the drawing, however in Table 52-1, there are 3 bits 
> in the CPTR register for defining the interrupt level of the SEC lite, 
> just like you do for the CPM and for the FEC.
> So I believe this should be ok ?
>>
>>>               interrupt-parent = <&PIC>;
>>>               fsl,num-channels = <1>;
>>>               fsl,channel-fifo-len = <24>;
>>>               fsl,exec-units-mask = <0x4c>;
>>>               fsl,descriptor-types-mask = <0x301f>;
>> the descriptor type enumeration isn't uniform across into the mpc8xx
>> SEC version, e.g., the SEC Lite doesn't support the ipsec_esp
>> descriptor type, represented in mpc8xxx SEC versions as the second
>> bit, so this descriptor-types-mask setting should be fixed to at
>> least omit that since the driver checks for, and uses it if
>> available.
>>
>>> Is there anything wrong in what I did ? Or is there something else I
>>> should do ?
>> might want to go through the defines in talitos.h, e.g,
>> TALITOS_MCR_SWR is 0x1 on mpc8xxx vs. 0x10000000 on
>> mpc8xx (I suppose CONFIG_PPC_8xx can be used as the ifdef, btw).
> I'm surprised about this, I didn't check the talitos.h file, but had 
> checked the Reference Manual of the MPC 8272.
> I rechecked yesterday and the SWR bit is at the same place as on the 
> MPC885 which is different from what is defined in talitos.h
>>
>> Descriptor header and pointer formats, along with field locations,
>> sizes, and enumerations may also be different.
>>
>> It also appears the SEC Lite doesn't support scatter-gather tables,
>> which will make performance hurt for fragmented (large) packet sizes.
> Does it mean something has to be modified if the SW ?
>>
>> Kim
>>
> Thanks,
> Christophe

Re: Question about Talitos Linux driver for MPC885

[Kim Phillips]

On Fri, 23 Nov 2012 17:03:15 +0100
leroy christophe <christophe.leroy@c-s.fr> wrote:

> Kim,
> 
> Looking once more at talitos.h file, it looks like none of the values 
> corresponds to what is in the reference manuals.
> But it looks like in the talitos.h it is encoded as if it was with LSB 
> first. Hence for instance the TALITOS_MCR_SWR which is 0x01 instead of  
> 0x01000000.
> Then I'm wondering what/where the problem is.

the problem is they're not the same on all SECs - some SECs just
have different bitfield encodings.  One solution is to #ifdef
CONFIG_PPC_8xx for the values correct for the 885, and #else the
existing values.

Kim

> Christophe
> 
> 
> Le 23/11/2012 14:51, leroy christophe a écrit :
> > Dear Kim,
> >
> > Thank you for you quick answer.
> > I didn't have much time to look at it until this week unfortunatly.
> >
> > I have some more questions/observations below
> >
> > Le 26/09/2012 02:47, Kim Phillips a écrit :
> >> On Tue, 25 Sep 2012 10:45:17 +0200
> >> leroy christophe <christophe.leroy@c-s.fr> wrote:
> >>
> >>> I'm trying to use the Talitos crypto driver with the MPC885
> >>> microcontroller. For the time being, it doesn't work.
> >> yes, they're not exactly compatible...
> >>
> >>> The kernel startup blocks at the test of the DES function.
> >>>
> >>> I have added the following definition in the DTS file:
> >>>
> >>>           crypto@20000 {
> >>>               compatible = "fsl,sec2.0";
> >> interesting, its called "SEC Lite" and its version register does
> >> indeed say 2.  I see it has a single channel FIFO instead of a ring,
> >> that the SEC v1.x (MPC185) used, so you probably don't have to
> >> rewrite talitos_submit.
> > Good news, it was also my understanding.
> >>
> >>>               reg = <0x20000 0x8000>;
> >>>               interrupts = <1 1>;
> >> I couldn't find the IRQ line in the MPC855RM - if there's no IRQ
> >> line, then that's a problem.
> > Neither do I on the drawing, however in Table 52-1, there are 3 bits 
> > in the CPTR register for defining the interrupt level of the SEC lite, 
> > just like you do for the CPM and for the FEC.
> > So I believe this should be ok ?
> >>
> >>>               interrupt-parent = <&PIC>;
> >>>               fsl,num-channels = <1>;
> >>>               fsl,channel-fifo-len = <24>;
> >>>               fsl,exec-units-mask = <0x4c>;
> >>>               fsl,descriptor-types-mask = <0x301f>;
> >> the descriptor type enumeration isn't uniform across into the mpc8xx
> >> SEC version, e.g., the SEC Lite doesn't support the ipsec_esp
> >> descriptor type, represented in mpc8xxx SEC versions as the second
> >> bit, so this descriptor-types-mask setting should be fixed to at
> >> least omit that since the driver checks for, and uses it if
> >> available.
> >>
> >>> Is there anything wrong in what I did ? Or is there something else I
> >>> should do ?
> >> might want to go through the defines in talitos.h, e.g,
> >> TALITOS_MCR_SWR is 0x1 on mpc8xxx vs. 0x10000000 on
> >> mpc8xx (I suppose CONFIG_PPC_8xx can be used as the ifdef, btw).
> > I'm surprised about this, I didn't check the talitos.h file, but had 
> > checked the Reference Manual of the MPC 8272.
> > I rechecked yesterday and the SWR bit is at the same place as on the 
> > MPC885 which is different from what is defined in talitos.h
> >>
> >> Descriptor header and pointer formats, along with field locations,
> >> sizes, and enumerations may also be different.
> >>
> >> It also appears the SEC Lite doesn't support scatter-gather tables,
> >> which will make performance hurt for fragmented (large) packet sizes.
> > Does it mean something has to be modified if the SW ?
> >>
> >> Kim
> >>
> > Thanks,
> > Christophe
> 
> 

Re: Question about Talitos Linux driver for MPC885

[Kim Phillips]

On Fri, 23 Nov 2012 14:51:08 +0100
leroy christophe <christophe.leroy@c-s.fr> wrote:

> Le 26/09/2012 02:47, Kim Phillips a écrit :
> > On Tue, 25 Sep 2012 10:45:17 +0200
> > leroy christophe <christophe.leroy@c-s.fr> wrote:
> >
> >> I'm trying to use the Talitos crypto driver with the MPC885
> >> microcontroller. For the time being, it doesn't work.
> > yes, they're not exactly compatible...
> >
> >> The kernel startup blocks at the test of the DES function.
> >>
> >> I have added the following definition in the DTS file:
> >>
> >>           crypto@20000 {
> >>               compatible = "fsl,sec2.0";
> > interesting, its called "SEC Lite" and its version register does
> > indeed say 2.  I see it has a single channel FIFO instead of a ring,
> > that the SEC v1.x (MPC185) used, so you probably don't have to
> > rewrite talitos_submit.
> Good news, it was also my understanding.
> >
> >>               reg = <0x20000 0x8000>;
> >>               interrupts = <1 1>;
> > I couldn't find the IRQ line in the MPC855RM - if there's no IRQ
> > line, then that's a problem.
> Neither do I on the drawing, however in Table 52-1, there are 3 bits in 
> the CPTR register for defining the interrupt level of the SEC lite, just 
> like you do for the CPM and for the FEC.

not sure how interrupt _level_ is relevant, but I'm going to assume
you're saying the SEC's IRQ line is integrated into that of the CPM
and/or FEC.

> So I believe this should be ok ?

Assuming the above, the 'interrupts' property above should
equal that of the CPM and/or FEC, and the IRQ_SHARED flag set in
request_irq (in both talitos and the CPM and/or FEC drivers).  The
talitos IRQ handler will return IRQ_NONE if none of the channel
done/error bits are set in the SEC.

> >>               interrupt-parent = <&PIC>;
> >>               fsl,num-channels = <1>;
> >>               fsl,channel-fifo-len = <24>;
> >>               fsl,exec-units-mask = <0x4c>;
> >>               fsl,descriptor-types-mask = <0x301f>;
> > the descriptor type enumeration isn't uniform across into the mpc8xx
> > SEC version, e.g., the SEC Lite doesn't support the ipsec_esp
> > descriptor type, represented in mpc8xxx SEC versions as the second
> > bit, so this descriptor-types-mask setting should be fixed to at
> > least omit that since the driver checks for, and uses it if
> > available.
> >
> >> Is there anything wrong in what I did ? Or is there something else I
> >> should do ?
> > might want to go through the defines in talitos.h, e.g,
> > TALITOS_MCR_SWR is 0x1 on mpc8xxx vs. 0x10000000 on
> > mpc8xx (I suppose CONFIG_PPC_8xx can be used as the ifdef, btw).
> I'm surprised about this, I didn't check the talitos.h file, but had 
> checked the Reference Manual of the MPC 8272.
> I rechecked yesterday and the SWR bit is at the same place as on the 
> MPC885 which is different from what is defined in talitos.h

right, for some odd reason the h/w designers decided to scramble
the bitfield definitions.

> > Descriptor header and pointer formats, along with field locations,
> > sizes, and enumerations may also be different.
> >
> > It also appears the SEC Lite doesn't support scatter-gather tables,
> > which will make performance hurt for fragmented (large) packet sizes.
> Does it mean something has to be modified if the SW ?

yes, s/w will have to allocate a new contiguous memory buffer and
sg_copy_to_buffer() prior to crypto request submission to the h/w,
and sg_copy_from_buffer() after h/w completion.

Kim

Re: Question about Talitos Linux driver for MPC885

[leroy christophe]

Le 28/11/2012 00:11, Kim Phillips a écrit :
> On Fri, 23 Nov 2012 14:51:08 +0100
> leroy christophe <christophe.leroy@c-s.fr> wrote:
>
>> Le 26/09/2012 02:47, Kim Phillips a écrit :
>>> On Tue, 25 Sep 2012 10:45:17 +0200
>>> leroy christophe <christophe.leroy@c-s.fr> wrote:
>>>
>>>> I'm trying to use the Talitos crypto driver with the MPC885
>>>> microcontroller. For the time being, it doesn't work.
>>> yes, they're not exactly compatible...
>>>
>>>> The kernel startup blocks at the test of the DES function.
>>>>
>>>> I have added the following definition in the DTS file:
>>>>
>>>>            crypto@20000 {
>>>>                compatible = "fsl,sec2.0";
>>> interesting, its called "SEC Lite" and its version register does
>>> indeed say 2.  I see it has a single channel FIFO instead of a ring,
>>> that the SEC v1.x (MPC185) used, so you probably don't have to
>>> rewrite talitos_submit.
>> Good news, it was also my understanding.
>>>>                reg = <0x20000 0x8000>;
>>>>                interrupts = <1 1>;
>>> I couldn't find the IRQ line in the MPC855RM - if there's no IRQ
>>> line, then that's a problem.
>> Neither do I on the drawing, however in Table 52-1, there are 3 bits in
>> the CPTR register for defining the interrupt level of the SEC lite, just
>> like you do for the CPM and for the FEC.
> not sure how interrupt _level_ is relevant, but I'm going to assume
> you're saying the SEC's IRQ line is integrated into that of the CPM
> and/or FEC.
>
>> So I believe this should be ok ?
> Assuming the above, the 'interrupts' property above should
> equal that of the CPM and/or FEC, and the IRQ_SHARED flag set in
> request_irq (in both talitos and the CPM and/or FEC drivers).  The
> talitos IRQ handler will return IRQ_NONE if none of the channel
> done/error bits are set in the SEC.
Indeed no. It shares the same registrer (CPTR) as the FEC, but has its 
own bits and its own IRQ line. So it works without having to share the IRQ.
>>>>                interrupt-parent = <&PIC>;
>>>>                fsl,num-channels = <1>;
>>>>                fsl,channel-fifo-len = <24>;
>>>>                fsl,exec-units-mask = <0x4c>;
>>>>                fsl,descriptor-types-mask = <0x301f>;
>>> the descriptor type enumeration isn't uniform across into the mpc8xx
>>> SEC version, e.g., the SEC Lite doesn't support the ipsec_esp
>>> descriptor type, represented in mpc8xxx SEC versions as the second
>>> bit, so this descriptor-types-mask setting should be fixed to at
>>> least omit that since the driver checks for, and uses it if
>>> available.
In fact it looks like the enumeration is uniform as far as what I was 
seen, the different is that on SEC1 it is encoded on 4 bits, while in 
SEC2 it is encoded on 5 bits, and unfortunately it is the least 
significant bit that is missing on SEC1.
So by shifting one bit to the left in matches.
>>>
>>>> Is there anything wrong in what I did ? Or is there something else I
>>>> should do ?
>>> might want to go through the defines in talitos.h, e.g,
>>> TALITOS_MCR_SWR is 0x1 on mpc8xxx vs. 0x10000000 on
>>> mpc8xx (I suppose CONFIG_PPC_8xx can be used as the ifdef, btw).
>> I'm surprised about this, I didn't check the talitos.h file, but had
>> checked the Reference Manual of the MPC 8272.
>> I rechecked yesterday and the SWR bit is at the same place as on the
>> MPC885 which is different from what is defined in talitos.h
> right, for some odd reason the h/w designers decided to scramble
> the bitfield definitions.
Right. In the begining in fact I was wrongly thinking the MPC8282 had 
SEC2 while MPC885 had SEC1, whereas indeed both have SEC1.
Therefore as you suggested I started altering the needed defines inside 
CONFIG_PPC_8xx

I'm now having a driver which starts bubbling, I'm having two issues:
1) One with the interrupt routing of the existing talitos driver. The 
drivers considers that when ISR_LO is non zero, it is an error, whereas 
according to the reference manuels, on both SEC1 (MPC885) and SEC2 
(MPC8323), ISR_LO includes ERR and DONE for the EUs. So when the 
interrupt fires after completion, at least on MPC885, both the Channel 
and the EUs have their DONE bit set.
It there any reason why a non zero ISR_LO is considered as a fault ?
2) I'm having an issue with the MD5 runtime test at startup. The first 
test on MD5 seems to the the hashing of a zero length data. It looks 
like the SEC1 doesn't like this, the kernel remains stuck and nothing 
happens. Looking at the status registers of the crypto channel, status 
is 0x10 meaning Processing Data Pairs, and status2 is 0x00280005.
First, is that correct to try and hash an empty data packet ?
Second, is there a way to workaround this issue ?

Herebelow is the content of /dev/crypto (I have faked the MD5 by setting 
data length to 1 instead of 0, hence the failure)

name         : sha256
driver       : sha256-talitos
module       : kernel
priority     : 3000
refcnt       : 1
selftest     : passed
type         : ahash
async        : yes
blocksize    : 64
digestsize   : 32

name         : sha224
driver       : sha224-talitos
module       : kernel
priority     : 3000
refcnt       : 1
selftest     : passed
type         : ahash
async        : yes
blocksize    : 64
digestsize   : 28

name         : sha1
driver       : sha1-talitos
module       : kernel
priority     : 3000
refcnt       : 1
selftest     : passed
type         : ahash
async        : yes
blocksize    : 64
digestsize   : 20

name         : md5
driver       : md5-talitos
module       : kernel
priority     : 3000
refcnt       : 1
selftest     : unknown
type         : ahash
async        : yes
blocksize    : 64
digestsize   : 16

name         : cbc(des3_ede)
driver       : cbc-3des-talitos
module       : kernel
priority     : 3000
refcnt       : 1
selftest     : passed
type         : ablkcipher
async        : yes
blocksize    : 8
min keysize  : 24
max keysize  : 24
ivsize       : 8
geniv        : eseqiv

name         : cbc(aes)
driver       : cbc-aes-talitos
module       : kernel
priority     : 3000
refcnt       : 1
selftest     : passed
type         : ablkcipher
async        : yes
blocksize    : 16
min keysize  : 16
max keysize  : 32
ivsize       : 16
geniv        : eseqiv

name         : stdrng
driver       : krng
module       : kernel
priority     : 200
refcnt       : 1
selftest     : passed
type         : rng
seedsize     : 0

name         : lzo
driver       : lzo-generic
module       : kernel
priority     : 0
refcnt       : 2
selftest     : passed
type         : compression

name         : aes
driver       : aes-generic
module       : kernel
priority     : 100
refcnt       : 2
selftest     : passed
type         : cipher
blocksize    : 16
min keysize  : 16
max keysize  : 32


Christophe

>
>>> Descriptor header and pointer formats, along with field locations,
>>> sizes, and enumerations may also be different.
>>>
>>> It also appears the SEC Lite doesn't support scatter-gather tables,
>>> which will make performance hurt for fragmented (large) packet sizes.
>> Does it mean something has to be modified if the SW ?
> yes, s/w will have to allocate a new contiguous memory buffer and
> sg_copy_to_buffer() prior to crypto request submission to the h/w,
> and sg_copy_from_buffer() after h/w completion.
>
> Kim
>