RE: [7/7] crypto: caam/qi - add ablkcipher and authenc algorithms

2017-04-07 Thread Laurentiu Tudor 


-Original Message-
From: Michael Ellerman [mailto:m...@ellerman.id.au] 
Sent: Friday, April 07, 2017 4:22 PM
To: Laurentiu Tudor <laurentiu.tu...@nxp.com>; Horia Geantă 
<horia.gea...@nxp.com>; Herbert Xu <herb...@gondor.apana.org.au>; Scott Wood 
<o...@buserror.net>; Roy Pledge <roy.ple...@nxp.com>
Cc: Claudiu Manoil <claudiu.man...@nxp.com>; Cristian Stoica 
<cristian.sto...@nxp.com>; Dan Douglass <dan.dougl...@nxp.com>; 
linux-arm-ker...@lists.infradead.org; Vakul Garg <vakul.g...@nxp.com>; 
linuxppc-...@lists.ozlabs.org; David S. Miller <da...@davemloft.net>; Alexandru 
Porosanu <alexandru.poros...@nxp.com>; linux-crypto@vger.kernel.org
Subject: Re: [7/7] crypto: caam/qi - add ablkcipher and authenc algorithms
Importance: High

Laurentiu Tudor <laurentiu.tu...@nxp.com> writes:

> On 04/05/2017 01:06 PM, Michael Ellerman wrote:
>> Laurentiu Tudor <laurentiu.tu...@nxp.com> writes:
>>
>>> Hi Michael,
>>>
>>> Just a couple of basic things to check:
>>>- was the dtb updated to the newest?
>>
>> Possibly not, it's an automated build/boot, I'll have to check what 
>> it does with the dtb.
>>
>>>- is the qman node present? This should be easily visible in 
>>> /proc/device-tree/soc@ffe00/qman@318000.
>>
>> No it's not there.
>>
>> That's running linux-next with:
>>
>> CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI=n
>>
>>
>> Does that mean I didn't update the device tree?
>
> I think so. Also, I just checked that the node is actually there by 
> compiling p5020ds.dts and then decompiling the dtb.

> OK, I'll make sure I update the DTB.
> 
> It will still be good if the code was a bit more robust about the qman being 
> missing.

Totally agree. We should handle this error condition.

---
Thanks & Best Regards, Laurentiu

Re: [7/7] crypto: caam/qi - add ablkcipher and authenc algorithms

2017-04-07 Thread Michael Ellerman 
Laurentiu Tudor  writes:

> On 04/05/2017 01:06 PM, Michael Ellerman wrote:
>> Laurentiu Tudor  writes:
>>
>>> Hi Michael,
>>>
>>> Just a couple of basic things to check:
>>>- was the dtb updated to the newest?
>>
>> Possibly not, it's an automated build/boot, I'll have to check what it
>> does with the dtb.
>>
>>>- is the qman node present? This should be easily visible in
>>> /proc/device-tree/soc@ffe00/qman@318000.
>>
>> No it's not there.
>>
>> That's running linux-next with:
>>
>> CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI=n
>>
>>
>> Does that mean I didn't update the device tree?
>
> I think so. Also, I just checked that the node is actually there by 
> compiling p5020ds.dts and then decompiling the dtb.

OK, I'll make sure I update the DTB.

It will still be good if the code was a bit more robust about the qman
being missing.

cheers

Re: [7/7] crypto: caam/qi - add ablkcipher and authenc algorithms

2017-04-05 Thread Laurentiu Tudor 


On 04/05/2017 01:06 PM, Michael Ellerman wrote:
> Laurentiu Tudor  writes:
>
>> Hi Michael,
>>
>> Just a couple of basic things to check:
>>- was the dtb updated to the newest?
>
> Possibly not, it's an automated build/boot, I'll have to check what it
> does with the dtb.
>
>>- is the qman node present? This should be easily visible in
>> /proc/device-tree/soc@ffe00/qman@318000.
>
> No it's not there.
>
> That's running linux-next with:
>
> CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI=n
>
>
> Does that mean I didn't update the device tree?
>

I think so. Also, I just checked that the node is actually there by 
compiling p5020ds.dts and then decompiling the dtb.

---
Best Regards, Laurentiu

Re: [7/7] crypto: caam/qi - add ablkcipher and authenc algorithms

2017-04-05 Thread Michael Ellerman 
Laurentiu Tudor  writes:

> Hi Michael,
>
> Just a couple of basic things to check:
>   - was the dtb updated to the newest?

Possibly not, it's an automated build/boot, I'll have to check what it
does with the dtb.

>   - is the qman node present? This should be easily visible in 
> /proc/device-tree/soc@ffe00/qman@318000.

No it's not there.

That's running linux-next with:

CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI=n


Does that mean I didn't update the device tree?

cheers

Re: [7/7] crypto: caam/qi - add ablkcipher and authenc algorithms

2017-04-04 Thread Laurentiu Tudor 
Hi Michael,

Just a couple of basic things to check:
  - was the dtb updated to the newest?
  - is the qman node present? This should be easily visible in 
/proc/device-tree/soc@ffe00/qman@318000.

---
Best Regards, Laurentiu

On 04/04/2017 08:03 AM, Michael Ellerman wrote:
> Horia Geantă  writes:
>
>> Add support to submit ablkcipher and authenc algorithms
>> via the QI backend:
>> -ablkcipher:
>> cbc({aes,des,des3_ede})
>> ctr(aes), rfc3686(ctr(aes))
>> xts(aes)
>> -authenc:
>> authenc(hmac(md5),cbc({aes,des,des3_ede}))
>> authenc(hmac(sha*),cbc({aes,des,des3_ede}))
>>
>> caam/qi being a new driver, let's wait some time to settle down without
>> interfering with existing caam/jr driver.
>> Accordingly, for now all caam/qi algorithms (caamalg_qi module) are
>> marked to be of lower priority than caam/jr ones (caamalg module).
>>
>> Signed-off-by: Vakul Garg 
>> Signed-off-by: Alex Porosanu 
>> Signed-off-by: Horia Geantă 
>> ---
>>   drivers/crypto/caam/Kconfig|   20 +-
>>   drivers/crypto/caam/Makefile   |1 +
>>   drivers/crypto/caam/caamalg.c  |9 +-
>>   drivers/crypto/caam/caamalg_desc.c |   77 +-
>>   drivers/crypto/caam/caamalg_desc.h |   15 +-
>>   drivers/crypto/caam/caamalg_qi.c   | 2387 
>> 
>>   drivers/crypto/caam/sg_sw_qm.h |  108 ++
>>   7 files changed, 2601 insertions(+), 16 deletions(-)
>>   create mode 100644 drivers/crypto/caam/caamalg_qi.c
>>   create mode 100644 drivers/crypto/caam/sg_sw_qm.h
>
>
> This appears to be blowing up my Freescale (NXP) P5020DS board:
>
>Unable to handle kernel paging request for data at address 0x0020
>Faulting instruction address: 0xc04393e4
>Oops: Kernel access of bad area, sig: 11 [#1]
>SMP NR_CPUS=24
>CoreNet Generic
>Modules linked in:
>CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
> 4.11.0-rc3-compiler_gcc-4.6.3-00046-gb189817cf789 #5
>task: c000f70c task.stack: c000f70c8000
>NIP: c04393e4 LR: c04aeba0 CTR: c04fa7d8
>REGS: c000f70cb160 TRAP: 0300   Not tainted  
> (4.11.0-rc3-compiler_gcc-4.6.3-00046-gb189817cf789)
>MSR: 80029000 
>  CR: 24adbe48  XER: 2000
>DEAR: 0020 ESR:  SOFTE: 1
>GPR00: c06feba0 c000f70cb3e0 c0e6 
>GPR04: 0001  c0e0b290 0003
>GPR08: 0004 c0ea5280 0004 0004
>GPR12: 88adbe22 c0003fff5000 c0ba3518 880088090fa8
>GPR16: 1000 c0ba3500 c000f72c68d8 0004
>GPR20: c0ea5280 c0ba34e8 0020 0004
>GPR24: c0eab7c0  c000f7fc8818 c0eb
>GPR28: c000f786cc00 c0eab780 f786cc00 c0eab7c0
>NIP [c04393e4] .gen_pool_alloc+0x0/0xc
>LR [c04aeba0] .qman_alloc_cgrid_range+0x24/0x54
>Call Trace:
>[c000f70cb3e0] [c0504054] 
> .platform_device_register_full+0x12c/0x150 (unreliable)
>[c000f70cb460] [c06feba0] .caam_qi_init+0x158/0x63c
>[c000f70cb5f0] [c06fc64c] .caam_probe+0x8b8/0x1830
>[c000f70cb740] [c0503288] .platform_drv_probe+0x60/0xac
>[c000f70cb7c0] [c0501194] .driver_probe_device+0x248/0x344
>[c000f70cb870] [c05013b4] .__driver_attach+0x124/0x128
>[c000f70cb900] [c04fed90] .bus_for_each_dev+0x80/0xcc
>[c000f70cb9a0] [c0500858] .driver_attach+0x24/0x38
>[c000f70cba10] [c050043c] .bus_add_driver+0x14c/0x29c
>[c000f70cbab0] [c0501d64] .driver_register+0x8c/0x154
>[c000f70cbb30] [c0503000] .__platform_driver_register+0x48/0x5c
>[c000f70cbba0] [c0c7f798] .caam_driver_init+0x1c/0x30
>[c000f70cbc10] [c0001904] .do_one_initcall+0x60/0x1a8
>[c000f70cbcf0] [c0c35f30] .kernel_init_freeable+0x248/0x334
>[c000f70cbdb0] [c00020fc] .kernel_init+0x1c/0xf20
>[c000f70cbe30] [c9bc] .ret_from_kernel_thread+0x58/0x9c
>Instruction dump:
>eb61ffd8 eb81ffe0 eba1ffe8 ebc1fff0 ebe1fff8 4e800020 3860 4bb0
>7ce53b78 4b0c 7f67db78 4b24  e8c30028 4bfffd30 fbe1fff8
>---[ end trace 9f61087068991b02 ]---
>
>
> home:linux-next(4)(I)> git bisect log
> ...
> git bisect bad b189817cf7894e03fd3700acd923221d3007259e
> # first bad commit: [b189817cf7894e03fd3700acd923221d3007259e] crypto: 
> caam/qi - add ablkcipher and authenc algorithms
>
>
> The oops is saying gen_pool_alloc() was called with a NULL pointer, so
> it seems qm_cgralloc is NULL:
>
> static int qman_alloc_range(struct gen_pool *p, u32 *result, u32 cnt)
> {
>   unsigned long addr;

Re: [7/7] crypto: caam/qi - add ablkcipher and authenc algorithms

2017-04-04 Thread Horia Geantă 
On 4/4/2017 8:03 AM, Michael Ellerman wrote:
> Horia Geantă  writes:
> 
>> Add support to submit ablkcipher and authenc algorithms
>> via the QI backend:
>> -ablkcipher:
>> cbc({aes,des,des3_ede})
>> ctr(aes), rfc3686(ctr(aes))
>> xts(aes)
>> -authenc:
>> authenc(hmac(md5),cbc({aes,des,des3_ede}))
>> authenc(hmac(sha*),cbc({aes,des,des3_ede}))
>>
>> caam/qi being a new driver, let's wait some time to settle down without
>> interfering with existing caam/jr driver.
>> Accordingly, for now all caam/qi algorithms (caamalg_qi module) are
>> marked to be of lower priority than caam/jr ones (caamalg module).
>>
>> Signed-off-by: Vakul Garg 
>> Signed-off-by: Alex Porosanu 
>> Signed-off-by: Horia Geantă 
>> ---
>>  drivers/crypto/caam/Kconfig|   20 +-
>>  drivers/crypto/caam/Makefile   |1 +
>>  drivers/crypto/caam/caamalg.c  |9 +-
>>  drivers/crypto/caam/caamalg_desc.c |   77 +-
>>  drivers/crypto/caam/caamalg_desc.h |   15 +-
>>  drivers/crypto/caam/caamalg_qi.c   | 2387 
>> 
>>  drivers/crypto/caam/sg_sw_qm.h |  108 ++
>>  7 files changed, 2601 insertions(+), 16 deletions(-)
>>  create mode 100644 drivers/crypto/caam/caamalg_qi.c
>>  create mode 100644 drivers/crypto/caam/sg_sw_qm.h
> 
> 
> This appears to be blowing up my Freescale (NXP) P5020DS board:
> 

Michael,

Unfortunately I was not able to reproduce the issue on my P5020DS using
the same SHA1 (b189817cf789). Assuming you've used
corenet64_smp_defconfig and the p5020ds dts file in this tree.

Roy, Madalin,

This might be a probing order issue b/w CAAM/QI and QMan
(drivers/soc/fsl/qbman/qman_ccsr.c) platform drivers.
AFAICT the same dependency exists b/w QMan and DPAA Ethernet, thus is
should be fixed there too.

CAAM and QMan are sibling devices, having "soc" DT node as parent.
Should device link mechanism (Documentation/driver-api/device_link.rst)
be used to register the dependency b/w QMan driver (supplier) and
CAAM/QI driver (consumer)?

Thanks,
Horia

>   Unable to handle kernel paging request for data at address 0x0020
>   Faulting instruction address: 0xc04393e4
>   Oops: Kernel access of bad area, sig: 11 [#1]
>   SMP NR_CPUS=24 
>   CoreNet Generic
>   Modules linked in:
>   CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
> 4.11.0-rc3-compiler_gcc-4.6.3-00046-gb189817cf789 #5
>   task: c000f70c task.stack: c000f70c8000
>   NIP: c04393e4 LR: c04aeba0 CTR: c04fa7d8
>   REGS: c000f70cb160 TRAP: 0300   Not tainted  
> (4.11.0-rc3-compiler_gcc-4.6.3-00046-gb189817cf789)
>   MSR: 80029000 
> CR: 24adbe48  XER: 2000
>   DEAR: 0020 ESR:  SOFTE: 1 
>   GPR00: c06feba0 c000f70cb3e0 c0e6  
>   GPR04: 0001  c0e0b290 0003 
>   GPR08: 0004 c0ea5280 0004 0004 
>   GPR12: 88adbe22 c0003fff5000 c0ba3518 880088090fa8 
>   GPR16: 1000 c0ba3500 c000f72c68d8 0004 
>   GPR20: c0ea5280 c0ba34e8 0020 0004 
>   GPR24: c0eab7c0  c000f7fc8818 c0eb 
>   GPR28: c000f786cc00 c0eab780 f786cc00 c0eab7c0 
>   NIP [c04393e4] .gen_pool_alloc+0x0/0xc
>   LR [c04aeba0] .qman_alloc_cgrid_range+0x24/0x54
>   Call Trace:
>   [c000f70cb3e0] [c0504054] 
> .platform_device_register_full+0x12c/0x150 (unreliable)
>   [c000f70cb460] [c06feba0] .caam_qi_init+0x158/0x63c
>   [c000f70cb5f0] [c06fc64c] .caam_probe+0x8b8/0x1830
>   [c000f70cb740] [c0503288] .platform_drv_probe+0x60/0xac
>   [c000f70cb7c0] [c0501194] .driver_probe_device+0x248/0x344
>   [c000f70cb870] [c05013b4] .__driver_attach+0x124/0x128
>   [c000f70cb900] [c04fed90] .bus_for_each_dev+0x80/0xcc
>   [c000f70cb9a0] [c0500858] .driver_attach+0x24/0x38
>   [c000f70cba10] [c050043c] .bus_add_driver+0x14c/0x29c
>   [c000f70cbab0] [c0501d64] .driver_register+0x8c/0x154
>   [c000f70cbb30] [c0503000] .__platform_driver_register+0x48/0x5c
>   [c000f70cbba0] [c0c7f798] .caam_driver_init+0x1c/0x30
>   [c000f70cbc10] [c0001904] .do_one_initcall+0x60/0x1a8
>   [c000f70cbcf0] [c0c35f30] .kernel_init_freeable+0x248/0x334
>   [c000f70cbdb0] [c00020fc] .kernel_init+0x1c/0xf20
>   [c000f70cbe30] [c9bc] .ret_from_kernel_thread+0x58/0x9c
>   Instruction dump:
>   eb61ffd8 eb81ffe0 eba1ffe8 ebc1fff0 ebe1fff8 4e800020 3860 4bb0 
>   7ce53b78 4b0c 7f67db78 4b24  e8c30028 4bfffd30 fbe1fff8 
>   ---[ end trace 9f61087068991b02 ]---
> 
> 
> home:linux-next(4)(I)> git bisect log
>