2015-02-28 06:00:50

by yjin

[permalink] [raw]
Subject: [PATCH 1/4] crypto: caam: fix some compile warnings

From: Yanjiang Jin <[email protected]>

This commit is to avoid the below warnings:

drivers/crypto/caam/sg_sw_sec4.h:88:12: warning:
'dma_map_sg_chained' defined but not used [-Wunused-function]
static int dma_map_sg_chained(struct device *dev, struct scatterlist *sg,
^
drivers/crypto/caam/sg_sw_sec4.h:104:12: warning:
'dma_unmap_sg_chained' defined but not used [-Wunused-function]
static int dma_unmap_sg_chained(struct device *dev,
^

Signed-off-by: Yanjiang Jin <[email protected]>
---
drivers/crypto/caam/sg_sw_sec4.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/caam/sg_sw_sec4.h b/drivers/crypto/caam/sg_sw_sec4.h
index 3b91821..a6276eb 100644
--- a/drivers/crypto/caam/sg_sw_sec4.h
+++ b/drivers/crypto/caam/sg_sw_sec4.h
@@ -85,7 +85,7 @@ static inline int sg_count(struct scatterlist *sg_list, int nbytes,
return sg_nents;
}

-static int dma_map_sg_chained(struct device *dev, struct scatterlist *sg,
+static inline int dma_map_sg_chained(struct device *dev, struct scatterlist *sg,
unsigned int nents, enum dma_data_direction dir,
bool chained)
{
@@ -101,9 +101,9 @@ static int dma_map_sg_chained(struct device *dev, struct scatterlist *sg,
return nents;
}

-static int dma_unmap_sg_chained(struct device *dev, struct scatterlist *sg,
- unsigned int nents, enum dma_data_direction dir,
- bool chained)
+static inline int dma_unmap_sg_chained(struct device *dev,
+ struct scatterlist *sg, unsigned int nents,
+ enum dma_data_direction dir, bool chained)
{
if (unlikely(chained)) {
int i;
--
1.9.1


2015-02-28 06:00:53

by yjin

[permalink] [raw]
Subject: [PATCH 3/4] crypto: caamhash: add two missed dma_mapping_error

From: Yanjiang Jin <[email protected]>

Add two missed dma_mapping_error() after dma_map_single().

Signed-off-by: Yanjiang Jin <[email protected]>
---
drivers/crypto/caam/caamhash.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index f347ab7..f6ad322 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -160,6 +160,10 @@ static inline dma_addr_t map_seq_out_ptr_result(u32 *desc, struct device *jrdev,
dma_addr_t dst_dma;

dst_dma = dma_map_single(jrdev, result, digestsize, DMA_FROM_DEVICE);
+ if (dma_mapping_error(jrdev, dst_dma)) {
+ dev_err(jrdev, "unable to map dst dma\n");
+ return -ENOMEM;
+ }
append_seq_out_ptr(desc, dst_dma, digestsize, 0);

return dst_dma;
@@ -173,6 +177,10 @@ static inline dma_addr_t buf_map_to_sec4_sg(struct device *jrdev,
dma_addr_t buf_dma;

buf_dma = dma_map_single(jrdev, buf, buflen, DMA_TO_DEVICE);
+ if (dma_mapping_error(jrdev, buf_dma)) {
+ dev_err(jrdev, "unable to map buf dma\n");
+ return 0;
+ }
dma_to_sec4_sg_one(sec4_sg, buf_dma, buflen, 0);

return buf_dma;
--
1.9.1

2015-02-28 06:02:09

by yjin

[permalink] [raw]
Subject: [PATCH 2/4] crypto: caam_rng: fix rng_unmap_ctx's DMA_UNMAP size problem

From: Yanjiang Jin <[email protected]>

Fix rng_unmap_ctx's DMA_UNMAP size problem for caam_rng, else system would
report the below calltrace during kexec boot:

caam_jr ffe301000.jr: DMA-API: device driver frees DMA memory with different size [device address=0x000000007f080010] [map size=16 bytes] [unmap size=40 bytes]
------------[ cut here ]------------
WARNING: at lib/dma-debug.c:887
Modules linked in:
CPU: 1 PID: 730 Comm: kexec Not tainted 3.10.62-ltsi-WR6.0.0.0_standard #188
task: c0000000f7cdaa80 ti: c0000000e5340000 task.ti: c0000000e5340000
NIP: c0000000004f5bc8 LR: c0000000004f5bc4 CTR: c0000000005f69b0
REGS: c0000000e53433c0 TRAP: 0700 Not tainted (3.10.62-ltsi-WR6.0.0.0_standard)
MSR: 0000000080029000 <CE,EE,ME> CR: 24088482 XER: 00000000
SOFTE: 0

GPR00: c0000000004f5bc4 c0000000e5343640 c0000000012af360 000000000000009f
GPR04: 0000000000000000 00000000000000a0 c000000000d02070 c000000015980660
GPR08: c000000000cff360 0000000000000000 0000000000000000 c0000000012da018
GPR12: 00000000000001e3 c000000001fff780 00000000100f0000 0000000000000001
GPR16: 0000000000000002 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 ffffffffffffffff 0000000000000001
GPR24: 0000000000000001 0000000000000001 0000000000000000 0000000000000001
GPR28: c000000001556b90 c000000001565b80 c0000000e5343750 c0000000f9427480
NIP [c0000000004f5bc8] .check_unmap+0x538/0x9c0
LR [c0000000004f5bc4] .check_unmap+0x534/0x9c0
Call Trace:
[c0000000e5343640] [c0000000004f5bc4] .check_unmap+0x534/0x9c0 (unreliable)
[c0000000e53436e0] [c0000000004f60d4] .debug_dma_unmap_page+0x84/0xb0
[c0000000e5343810] [c00000000082f9d4] .caam_cleanup+0x1d4/0x240
[c0000000e53438a0] [c00000000056cc88] .hwrng_unregister+0xd8/0x1c0
[c0000000e5343930] [c00000000082fa74] .caam_rng_shutdown+0x34/0x60
[c0000000e53439a0] [c000000000817354] .caam_remove+0x54/0x420
[c0000000e5343a70] [c0000000005791ac] .platform_drv_shutdown+0x3c/0x60
[c0000000e5343af0] [c000000000573728] .device_shutdown+0x128/0x240
[c0000000e5343b90] [c0000000000880b4] .kernel_restart_prepare+0x54/0x70
[c0000000e5343c10] [c0000000000e5cac] .kernel_kexec+0x9c/0xd0
[c0000000e5343c90] [c000000000088404] .SyS_reboot+0x244/0x2d0
[c0000000e5343e30] [c000000000000718] syscall_exit+0x0/0x8c
Instruction dump:
7c641b78 41de0410 e8a90050 2fa50000 419e0484 e8de0028 e8ff0030 3c62ff90
e91e0030 38638388 48546ed9 60000000 <0fe00000> 3c62ff8f 38637fc8 48546ec5
---[ end trace e43fd1734d6600df ]---

Signed-off-by: Yanjiang Jin <[email protected]>
---
drivers/crypto/caam/caamrng.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c
index ae31e55..314f73d 100644
--- a/drivers/crypto/caam/caamrng.c
+++ b/drivers/crypto/caam/caamrng.c
@@ -90,8 +90,8 @@ static inline void rng_unmap_ctx(struct caam_rng_ctx *ctx)
struct device *jrdev = ctx->jrdev;

if (ctx->sh_desc_dma)
- dma_unmap_single(jrdev, ctx->sh_desc_dma, DESC_RNG_LEN,
- DMA_TO_DEVICE);
+ dma_unmap_single(jrdev, ctx->sh_desc_dma,
+ desc_bytes(ctx->sh_desc), DMA_TO_DEVICE);
rng_unmap_buf(jrdev, &ctx->bufs[0]);
rng_unmap_buf(jrdev, &ctx->bufs[1]);
}
--
1.9.1

2015-02-28 06:00:34

by yjin

[permalink] [raw]
Subject: [PATCH 4/4] crypto: caamhash: replace kmalloc with kzalloc

From: Yanjiang Jin <[email protected]>

This can make sure we get a clean memory, else system would report
the below warning:

caam_jr ffe301000.jr: DMA-API: device driver tries to free DMA memory it has not allocated [device address=0xdeadbeefdeadbeef] [size=18446744073150512879 bytes]
------------[ cut here ]------------
WARNING: at lib/dma-debug.c:877
Modules linked in:
CPU: 1 PID: 98 Comm: cryptomgr_test Not tainted 3.10.62-ltsi-WR6.0.0.0_standard #175
task: c0000000f74bc400 ti: c0000000fffd0000 task.ti: c0000000f775c000
NIP: c0000000004f5ed8 LR: c0000000004f5ed4 CTR: c00000000055a160
REGS: c0000000fffd3650 TRAP: 0700 Not tainted (3.10.62-ltsi-WR6.0.0.0_standard)
MSR: 0000000080029000 <CE,EE,ME> CR: 24a48e84 XER: 00000000
SOFTE: 1

004f5ed4 c0000000fffd38d0 c0000000012af348 00000000000000a0
24a48e84 0000000000000000 c00000000125f1c8 00000000000001eb
00000060 0000000000000001 0000000010187373 0000000000000020
000001eb c000000001fff780 c0000000011ac928 c00000007f003028
00000097 0000000000000098 0000000000000098 c0000000f7758800
f7098c00 0000000000000001 0000000000000001 000000000000003f
f7098c00 0000000000000014 c00000007f003000 c0000000011b0e98
00000000 c000000001565b80 c0000000fffd39e0 c0000000f72f2410
NIP [c0000000004f5ed8] .check_unmap+0x848/0x9c0
LR [c0000000004f5ed4] .check_unmap+0x844/0x9c0
Call Trace:
[c0000000fffd38d0] [c0000000004f5ed4] .check_unmap+0x844/0x9c0 (unreliable)
[c0000000fffd3970] [c0000000004f60d4] .debug_dma_unmap_page+0x84/0xb0
[c0000000fffd3aa0] [c0000000008295cc] .ahash_done+0x1dc/0x360
[c0000000fffd3ca0] [c00000000081b7ec] .caam_jr_dequeue+0x26c/0x3a0
[c0000000fffd3da0] [c0000000008be50c] .net_rx_action+0x1cc/0x330
[c0000000fffd3e80] [c00000000007276c] .__do_softirq+0x19c/0x3d0
[c0000000fffd3f90] [c000000000017054] .call_do_softirq+0x14/0x24
[c0000000f775ef10] [c000000000005fe8] .do_softirq+0x118/0x150
sda: sda1 sda2 sda3
[c0000000f775efa0] [c000000000072c54] .irq_exit+0x124/0x140
[c0000000f775f020] [c000000000005ac4] .do_IRQ+0x184/0x370
[c0000000f775f0d0] [c00000000001b93c] exc_0x500_common+0xfc/0x100
--- Exception: 501 at .rcu_note_context_switch+0x0/0x370
edule+0xbc/0x7f0
[c0000000f775f3c0] [c000000000a29944] .__schedule+0xa4/0x7f0 (unreliable)
[c0000000f775f620] [c000000000a277f4] .schedule_timeout+0x1b4/0x2e0
[c0000000f775f700] [c000000000a29428] .wait_for_common+0xf8/0x1d0
[c0000000f775f7c0] [c000000000a295ac] .wait_for_completion_interruptible+0x2c/0x50
[c0000000f775f840] [c000000000494b64] .do_one_async_hash_op.isra.1.part.2+0x24/0x50
[c0000000f775f8c0] [c0000000004951a8] .test_hash+0x618/0x7d0
[c0000000f775fb30] [c000000000495424] .alg_test_hash+0xc4/0xf0
[c0000000f775fbc0] [c000000000494928] .alg_test+0xa8/0x2c0
[c0000000f775fcb0] [c000000000491164] .cryptomgr_test+0x64/0x80
[c0000000f775fd30] [c00000000009a8d0] .kthread+0xf0/0x100
[c0000000f775fe30] [c000000000000a08] .ret_from_kernel_thread+0x5c/0xd4
Instruction dump:
7c641b78 419e0160 e8a90050 2fa50000 409e0008 e8a90010 e8de0028 e8fe0030
3c62ff90 38638320 48546b69 60000000 <0fe00000> 4bffff34 e87e0010 2fa30000
---[ end trace 52825d316d569f00 ]---

Signed-off-by: Yanjiang Jin <[email protected]>
---
drivers/crypto/caam/caamhash.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index f6ad322..a6ba9f7 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -451,7 +451,7 @@ static int hash_digest_key(struct caam_hash_ctx *ctx, const u8 *key_in,
dma_addr_t src_dma, dst_dma;
int ret = 0;

- desc = kmalloc(CAAM_CMD_SZ * 8 + CAAM_PTR_SZ * 2, GFP_KERNEL | GFP_DMA);
+ desc = kzalloc(CAAM_CMD_SZ * 8 + CAAM_PTR_SZ * 2, GFP_KERNEL | GFP_DMA);
if (!desc) {
dev_err(jrdev, "unable to allocate key input memory\n");
return -ENOMEM;
@@ -815,7 +815,7 @@ static int ahash_update_ctx(struct ahash_request *req)
* allocate space for base edesc and hw desc commands,
* link tables
*/
- edesc = kmalloc(sizeof(struct ahash_edesc) + DESC_JOB_IO_LEN +
+ edesc = kzalloc(sizeof(struct ahash_edesc) + DESC_JOB_IO_LEN +
sec4_sg_bytes, GFP_DMA | flags);
if (!edesc) {
dev_err(jrdev,
@@ -926,7 +926,7 @@ static int ahash_final_ctx(struct ahash_request *req)
sec4_sg_bytes = (1 + (buflen ? 1 : 0)) * sizeof(struct sec4_sg_entry);

/* allocate space for base edesc and hw desc commands, link tables */
- edesc = kmalloc(sizeof(struct ahash_edesc) + DESC_JOB_IO_LEN +
+ edesc = kzalloc(sizeof(struct ahash_edesc) + DESC_JOB_IO_LEN +
sec4_sg_bytes, GFP_DMA | flags);
if (!edesc) {
dev_err(jrdev, "could not allocate extended descriptor\n");
@@ -1013,7 +1013,7 @@ static int ahash_finup_ctx(struct ahash_request *req)
sizeof(struct sec4_sg_entry);

/* allocate space for base edesc and hw desc commands, link tables */
- edesc = kmalloc(sizeof(struct ahash_edesc) + DESC_JOB_IO_LEN +
+ edesc = kzalloc(sizeof(struct ahash_edesc) + DESC_JOB_IO_LEN +
sec4_sg_bytes, GFP_DMA | flags);
if (!edesc) {
dev_err(jrdev, "could not allocate extended descriptor\n");
@@ -1099,7 +1099,7 @@ static int ahash_digest(struct ahash_request *req)
sec4_sg_bytes = src_nents * sizeof(struct sec4_sg_entry);

/* allocate space for base edesc and hw desc commands, link tables */
- edesc = kmalloc(sizeof(struct ahash_edesc) + sec4_sg_bytes +
+ edesc = kzalloc(sizeof(struct ahash_edesc) + sec4_sg_bytes +
DESC_JOB_IO_LEN, GFP_DMA | flags);
if (!edesc) {
dev_err(jrdev, "could not allocate extended descriptor\n");
@@ -1173,7 +1173,7 @@ static int ahash_final_no_ctx(struct ahash_request *req)
int sh_len;

/* allocate space for base edesc and hw desc commands, link tables */
- edesc = kmalloc(sizeof(struct ahash_edesc) + DESC_JOB_IO_LEN,
+ edesc = kzalloc(sizeof(struct ahash_edesc) + DESC_JOB_IO_LEN,
GFP_DMA | flags);
if (!edesc) {
dev_err(jrdev, "could not allocate extended descriptor\n");
@@ -1252,7 +1252,7 @@ static int ahash_update_no_ctx(struct ahash_request *req)
* allocate space for base edesc and hw desc commands,
* link tables
*/
- edesc = kmalloc(sizeof(struct ahash_edesc) + DESC_JOB_IO_LEN +
+ edesc = kzalloc(sizeof(struct ahash_edesc) + DESC_JOB_IO_LEN +
sec4_sg_bytes, GFP_DMA | flags);
if (!edesc) {
dev_err(jrdev,
@@ -1359,7 +1359,7 @@ static int ahash_finup_no_ctx(struct ahash_request *req)
sizeof(struct sec4_sg_entry);

/* allocate space for base edesc and hw desc commands, link tables */
- edesc = kmalloc(sizeof(struct ahash_edesc) + DESC_JOB_IO_LEN +
+ edesc = kzalloc(sizeof(struct ahash_edesc) + DESC_JOB_IO_LEN +
sec4_sg_bytes, GFP_DMA | flags);
if (!edesc) {
dev_err(jrdev, "could not allocate extended descriptor\n");
@@ -1454,7 +1454,7 @@ static int ahash_update_first(struct ahash_request *req)
* allocate space for base edesc and hw desc commands,
* link tables
*/
- edesc = kmalloc(sizeof(struct ahash_edesc) + DESC_JOB_IO_LEN +
+ edesc = kzalloc(sizeof(struct ahash_edesc) + DESC_JOB_IO_LEN +
sec4_sg_bytes, GFP_DMA | flags);
if (!edesc) {
dev_err(jrdev,
--
1.9.1

2015-03-02 11:08:51

by Horia Geantă

[permalink] [raw]
Subject: Re: [PATCH 1/4] crypto: caam: fix some compile warnings

On 2/28/2015 8:00 AM, [email protected] wrote:
> From: Yanjiang Jin <[email protected]>
>
> This commit is to avoid the below warnings:
>
> drivers/crypto/caam/sg_sw_sec4.h:88:12: warning:
> 'dma_map_sg_chained' defined but not used [-Wunused-function]
> static int dma_map_sg_chained(struct device *dev, struct scatterlist *sg,
> ^
> drivers/crypto/caam/sg_sw_sec4.h:104:12: warning:
> 'dma_unmap_sg_chained' defined but not used [-Wunused-function]
> static int dma_unmap_sg_chained(struct device *dev,
> ^
>
> Signed-off-by: Yanjiang Jin <[email protected]>

Reviewed-by: Horia Geanta <[email protected]>

> ---
> drivers/crypto/caam/sg_sw_sec4.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/crypto/caam/sg_sw_sec4.h b/drivers/crypto/caam/sg_sw_sec4.h
> index 3b91821..a6276eb 100644
> --- a/drivers/crypto/caam/sg_sw_sec4.h
> +++ b/drivers/crypto/caam/sg_sw_sec4.h
> @@ -85,7 +85,7 @@ static inline int sg_count(struct scatterlist *sg_list, int nbytes,
> return sg_nents;
> }
>
> -static int dma_map_sg_chained(struct device *dev, struct scatterlist *sg,
> +static inline int dma_map_sg_chained(struct device *dev, struct scatterlist *sg,
> unsigned int nents, enum dma_data_direction dir,
> bool chained)
> {
> @@ -101,9 +101,9 @@ static int dma_map_sg_chained(struct device *dev, struct scatterlist *sg,
> return nents;
> }
>
> -static int dma_unmap_sg_chained(struct device *dev, struct scatterlist *sg,
> - unsigned int nents, enum dma_data_direction dir,
> - bool chained)
> +static inline int dma_unmap_sg_chained(struct device *dev,
> + struct scatterlist *sg, unsigned int nents,
> + enum dma_data_direction dir, bool chained)
> {
> if (unlikely(chained)) {
> int i;
>

2015-03-02 11:19:53

by Horia Geantă

[permalink] [raw]
Subject: Re: [PATCH 4/4] crypto: caamhash: replace kmalloc with kzalloc

On 2/28/2015 8:00 AM, [email protected] wrote:
> From: Yanjiang Jin <[email protected]>
>
> This can make sure we get a clean memory, else system would report
> the below warning:

I'd avoid using kzalloc, it's an overhead on the hot path. kmalloc can
be used with a bit of attention to detail, i.e. what params to
explicitly initialize.

I see that the stack trace reports using WR Linux and a modified caam
driver - it uses NAPI (net_rx_action) instead of tasklet.

Have you actually reproduced the problem on upstream linux?
There are some fixes that already address similar (if not exact) problem:
76b99080ccc9 "crypto: caam - fix uninitialized edesc->dst_dma fiel"
45e9af78b1ab "crypto: caam - fix uninitialized S/G table size in
ahash_digest"

Thanks,
Horia

>
> caam_jr ffe301000.jr: DMA-API: device driver tries to free DMA memory it has not allocated [device address=0xdeadbeefdeadbeef] [size=18446744073150512879 bytes]
> ------------[ cut here ]------------
> WARNING: at lib/dma-debug.c:877
> Modules linked in:
> CPU: 1 PID: 98 Comm: cryptomgr_test Not tainted 3.10.62-ltsi-WR6.0.0.0_standard #175
> task: c0000000f74bc400 ti: c0000000fffd0000 task.ti: c0000000f775c000
> NIP: c0000000004f5ed8 LR: c0000000004f5ed4 CTR: c00000000055a160
> REGS: c0000000fffd3650 TRAP: 0700 Not tainted (3.10.62-ltsi-WR6.0.0.0_standard)
> MSR: 0000000080029000 <CE,EE,ME> CR: 24a48e84 XER: 00000000
> SOFTE: 1
>
> 004f5ed4 c0000000fffd38d0 c0000000012af348 00000000000000a0
> 24a48e84 0000000000000000 c00000000125f1c8 00000000000001eb
> 00000060 0000000000000001 0000000010187373 0000000000000020
> 000001eb c000000001fff780 c0000000011ac928 c00000007f003028
> 00000097 0000000000000098 0000000000000098 c0000000f7758800
> f7098c00 0000000000000001 0000000000000001 000000000000003f
> f7098c00 0000000000000014 c00000007f003000 c0000000011b0e98
> 00000000 c000000001565b80 c0000000fffd39e0 c0000000f72f2410
> NIP [c0000000004f5ed8] .check_unmap+0x848/0x9c0
> LR [c0000000004f5ed4] .check_unmap+0x844/0x9c0
> Call Trace:
> [c0000000fffd38d0] [c0000000004f5ed4] .check_unmap+0x844/0x9c0 (unreliable)
> [c0000000fffd3970] [c0000000004f60d4] .debug_dma_unmap_page+0x84/0xb0
> [c0000000fffd3aa0] [c0000000008295cc] .ahash_done+0x1dc/0x360
> [c0000000fffd3ca0] [c00000000081b7ec] .caam_jr_dequeue+0x26c/0x3a0
> [c0000000fffd3da0] [c0000000008be50c] .net_rx_action+0x1cc/0x330
> [c0000000fffd3e80] [c00000000007276c] .__do_softirq+0x19c/0x3d0
> [c0000000fffd3f90] [c000000000017054] .call_do_softirq+0x14/0x24
> [c0000000f775ef10] [c000000000005fe8] .do_softirq+0x118/0x150
> sda: sda1 sda2 sda3
> [c0000000f775efa0] [c000000000072c54] .irq_exit+0x124/0x140
> [c0000000f775f020] [c000000000005ac4] .do_IRQ+0x184/0x370
> [c0000000f775f0d0] [c00000000001b93c] exc_0x500_common+0xfc/0x100
> --- Exception: 501 at .rcu_note_context_switch+0x0/0x370
> edule+0xbc/0x7f0
> [c0000000f775f3c0] [c000000000a29944] .__schedule+0xa4/0x7f0 (unreliable)
> [c0000000f775f620] [c000000000a277f4] .schedule_timeout+0x1b4/0x2e0
> [c0000000f775f700] [c000000000a29428] .wait_for_common+0xf8/0x1d0
> [c0000000f775f7c0] [c000000000a295ac] .wait_for_completion_interruptible+0x2c/0x50
> [c0000000f775f840] [c000000000494b64] .do_one_async_hash_op.isra.1.part.2+0x24/0x50
> [c0000000f775f8c0] [c0000000004951a8] .test_hash+0x618/0x7d0
> [c0000000f775fb30] [c000000000495424] .alg_test_hash+0xc4/0xf0
> [c0000000f775fbc0] [c000000000494928] .alg_test+0xa8/0x2c0
> [c0000000f775fcb0] [c000000000491164] .cryptomgr_test+0x64/0x80
> [c0000000f775fd30] [c00000000009a8d0] .kthread+0xf0/0x100
> [c0000000f775fe30] [c000000000000a08] .ret_from_kernel_thread+0x5c/0xd4
> Instruction dump:
> 7c641b78 419e0160 e8a90050 2fa50000 409e0008 e8a90010 e8de0028 e8fe0030
> 3c62ff90 38638320 48546b69 60000000 <0fe00000> 4bffff34 e87e0010 2fa30000
> ---[ end trace 52825d316d569f00 ]---
>
> Signed-off-by: Yanjiang Jin <[email protected]>
> ---

2015-03-02 11:24:57

by Horia Geantă

[permalink] [raw]
Subject: Re: [PATCH 2/4] crypto: caam_rng: fix rng_unmap_ctx's DMA_UNMAP size problem

On 2/28/2015 8:00 AM, [email protected] wrote:
> From: Yanjiang Jin <[email protected]>
>
> Fix rng_unmap_ctx's DMA_UNMAP size problem for caam_rng, else system would
> report the below calltrace during kexec boot:
>
> caam_jr ffe301000.jr: DMA-API: device driver frees DMA memory with different size [device address=0x000000007f080010] [map size=16 bytes] [unmap size=40 bytes]
> ------------[ cut here ]------------
> WARNING: at lib/dma-debug.c:887
> Modules linked in:
> CPU: 1 PID: 730 Comm: kexec Not tainted 3.10.62-ltsi-WR6.0.0.0_standard #188
> task: c0000000f7cdaa80 ti: c0000000e5340000 task.ti: c0000000e5340000
> NIP: c0000000004f5bc8 LR: c0000000004f5bc4 CTR: c0000000005f69b0
> REGS: c0000000e53433c0 TRAP: 0700 Not tainted (3.10.62-ltsi-WR6.0.0.0_standard)
> MSR: 0000000080029000 <CE,EE,ME> CR: 24088482 XER: 00000000
> SOFTE: 0
>
> GPR00: c0000000004f5bc4 c0000000e5343640 c0000000012af360 000000000000009f
> GPR04: 0000000000000000 00000000000000a0 c000000000d02070 c000000015980660
> GPR08: c000000000cff360 0000000000000000 0000000000000000 c0000000012da018
> GPR12: 00000000000001e3 c000000001fff780 00000000100f0000 0000000000000001
> GPR16: 0000000000000002 0000000000000000 0000000000000000 0000000000000000
> GPR20: 0000000000000000 0000000000000000 ffffffffffffffff 0000000000000001
> GPR24: 0000000000000001 0000000000000001 0000000000000000 0000000000000001
> GPR28: c000000001556b90 c000000001565b80 c0000000e5343750 c0000000f9427480
> NIP [c0000000004f5bc8] .check_unmap+0x538/0x9c0
> LR [c0000000004f5bc4] .check_unmap+0x534/0x9c0
> Call Trace:
> [c0000000e5343640] [c0000000004f5bc4] .check_unmap+0x534/0x9c0 (unreliable)
> [c0000000e53436e0] [c0000000004f60d4] .debug_dma_unmap_page+0x84/0xb0
> [c0000000e5343810] [c00000000082f9d4] .caam_cleanup+0x1d4/0x240
> [c0000000e53438a0] [c00000000056cc88] .hwrng_unregister+0xd8/0x1c0
> [c0000000e5343930] [c00000000082fa74] .caam_rng_shutdown+0x34/0x60
> [c0000000e53439a0] [c000000000817354] .caam_remove+0x54/0x420
> [c0000000e5343a70] [c0000000005791ac] .platform_drv_shutdown+0x3c/0x60
> [c0000000e5343af0] [c000000000573728] .device_shutdown+0x128/0x240
> [c0000000e5343b90] [c0000000000880b4] .kernel_restart_prepare+0x54/0x70
> [c0000000e5343c10] [c0000000000e5cac] .kernel_kexec+0x9c/0xd0
> [c0000000e5343c90] [c000000000088404] .SyS_reboot+0x244/0x2d0
> [c0000000e5343e30] [c000000000000718] syscall_exit+0x0/0x8c
> Instruction dump:
> 7c641b78 41de0410 e8a90050 2fa50000 419e0484 e8de0028 e8ff0030 3c62ff90
> e91e0030 38638388 48546ed9 60000000 <0fe00000> 3c62ff8f 38637fc8 48546ec5
> ---[ end trace e43fd1734d6600df ]---
>
> Signed-off-by: Yanjiang Jin <[email protected]>

Reviewed-by: Horia Geanta <[email protected]>

> ---
> drivers/crypto/caam/caamrng.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c
> index ae31e55..314f73d 100644
> --- a/drivers/crypto/caam/caamrng.c
> +++ b/drivers/crypto/caam/caamrng.c
> @@ -90,8 +90,8 @@ static inline void rng_unmap_ctx(struct caam_rng_ctx *ctx)
> struct device *jrdev = ctx->jrdev;
>
> if (ctx->sh_desc_dma)
> - dma_unmap_single(jrdev, ctx->sh_desc_dma, DESC_RNG_LEN,
> - DMA_TO_DEVICE);
> + dma_unmap_single(jrdev, ctx->sh_desc_dma,
> + desc_bytes(ctx->sh_desc), DMA_TO_DEVICE);
> rng_unmap_buf(jrdev, &ctx->bufs[0]);
> rng_unmap_buf(jrdev, &ctx->bufs[1]);
> }
>

2015-03-02 11:54:00

by Horia Geantă

[permalink] [raw]
Subject: Re: [PATCH 3/4] crypto: caamhash: add two missed dma_mapping_error

On 2/28/2015 8:00 AM, [email protected] wrote:
> From: Yanjiang Jin <[email protected]>
>
> Add two missed dma_mapping_error() after dma_map_single().
>
> Signed-off-by: Yanjiang Jin <[email protected]>
> ---
> drivers/crypto/caam/caamhash.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
> index f347ab7..f6ad322 100644
> --- a/drivers/crypto/caam/caamhash.c
> +++ b/drivers/crypto/caam/caamhash.c
> @@ -160,6 +160,10 @@ static inline dma_addr_t map_seq_out_ptr_result(u32 *desc, struct device *jrdev,
> dma_addr_t dst_dma;
>
> dst_dma = dma_map_single(jrdev, result, digestsize, DMA_FROM_DEVICE);
> + if (dma_mapping_error(jrdev, dst_dma)) {
> + dev_err(jrdev, "unable to map dst dma\n");
> + return -ENOMEM;
> + }
> append_seq_out_ptr(desc, dst_dma, digestsize, 0);
>
> return dst_dma;

Value returned by map_seq_out_ptr_result() - dst_dma - is always fed to
dma_mapping_error().
Note that using an invalid dst_dma in append_seq_out_ptr() doesn't break
anything, so it's ok to check dst_dma later.

> @@ -173,6 +177,10 @@ static inline dma_addr_t buf_map_to_sec4_sg(struct device *jrdev,
> dma_addr_t buf_dma;
>
> buf_dma = dma_map_single(jrdev, buf, buflen, DMA_TO_DEVICE);
> + if (dma_mapping_error(jrdev, buf_dma)) {
> + dev_err(jrdev, "unable to map buf dma\n");
> + return 0;
> + }
> dma_to_sec4_sg_one(sec4_sg, buf_dma, buflen, 0);
>
> return buf_dma;
>

These functions are expected to return dma_addr_t, not an error code.
If dma_mapping_error() is needed within their scope, the return type
will have to change. And return value will need to be checked by their
callers.

2015-03-03 06:54:59

by yjin

[permalink] [raw]
Subject: Re: [PATCH 4/4] crypto: caamhash: replace kmalloc with kzalloc


On 2015年03月02日 19:03, Horia Geantă wrote:
> On 2/28/2015 8:00 AM, [email protected] wrote:
>> From: Yanjiang Jin <[email protected]>
>>
>> This can make sure we get a clean memory, else system would report
>> the below warning:
> I'd avoid using kzalloc, it's an overhead on the hot path. kmalloc can
> be used with a bit of attention to detail, i.e. what params to
> explicitly initialize.
Got it. Just zeroing edesc->sec4_sg_bytes in V2.

Thanks!
Yanjiang
>
> I see that the stack trace reports using WR Linux and a modified caam
> driver - it uses NAPI (net_rx_action) instead of tasklet.
>
> Have you actually reproduced the problem on upstream linux?
> There are some fixes that already address similar (if not exact) problem:
> 76b99080ccc9 "crypto: caam - fix uninitialized edesc->dst_dma fiel"
> 45e9af78b1ab "crypto: caam - fix uninitialized S/G table size in
> ahash_digest"
>
> Thanks,
> Horia
>
>> caam_jr ffe301000.jr: DMA-API: device driver tries to free DMA memory it has not allocated [device address=0xdeadbeefdeadbeef] [size=18446744073150512879 bytes]
>> ------------[ cut here ]------------
>> WARNING: at lib/dma-debug.c:877
>> Modules linked in:
>> CPU: 1 PID: 98 Comm: cryptomgr_test Not tainted 3.10.62-ltsi-WR6.0.0.0_standard #175
>> task: c0000000f74bc400 ti: c0000000fffd0000 task.ti: c0000000f775c000
>> NIP: c0000000004f5ed8 LR: c0000000004f5ed4 CTR: c00000000055a160
>> REGS: c0000000fffd3650 TRAP: 0700 Not tainted (3.10.62-ltsi-WR6.0.0.0_standard)
>> MSR: 0000000080029000 <CE,EE,ME> CR: 24a48e84 XER: 00000000
>> SOFTE: 1
>>
>> 004f5ed4 c0000000fffd38d0 c0000000012af348 00000000000000a0
>> 24a48e84 0000000000000000 c00000000125f1c8 00000000000001eb
>> 00000060 0000000000000001 0000000010187373 0000000000000020
>> 000001eb c000000001fff780 c0000000011ac928 c00000007f003028
>> 00000097 0000000000000098 0000000000000098 c0000000f7758800
>> f7098c00 0000000000000001 0000000000000001 000000000000003f
>> f7098c00 0000000000000014 c00000007f003000 c0000000011b0e98
>> 00000000 c000000001565b80 c0000000fffd39e0 c0000000f72f2410
>> NIP [c0000000004f5ed8] .check_unmap+0x848/0x9c0
>> LR [c0000000004f5ed4] .check_unmap+0x844/0x9c0
>> Call Trace:
>> [c0000000fffd38d0] [c0000000004f5ed4] .check_unmap+0x844/0x9c0 (unreliable)
>> [c0000000fffd3970] [c0000000004f60d4] .debug_dma_unmap_page+0x84/0xb0
>> [c0000000fffd3aa0] [c0000000008295cc] .ahash_done+0x1dc/0x360
>> [c0000000fffd3ca0] [c00000000081b7ec] .caam_jr_dequeue+0x26c/0x3a0
>> [c0000000fffd3da0] [c0000000008be50c] .net_rx_action+0x1cc/0x330
>> [c0000000fffd3e80] [c00000000007276c] .__do_softirq+0x19c/0x3d0
>> [c0000000fffd3f90] [c000000000017054] .call_do_softirq+0x14/0x24
>> [c0000000f775ef10] [c000000000005fe8] .do_softirq+0x118/0x150
>> sda: sda1 sda2 sda3
>> [c0000000f775efa0] [c000000000072c54] .irq_exit+0x124/0x140
>> [c0000000f775f020] [c000000000005ac4] .do_IRQ+0x184/0x370
>> [c0000000f775f0d0] [c00000000001b93c] exc_0x500_common+0xfc/0x100
>> --- Exception: 501 at .rcu_note_context_switch+0x0/0x370
>> edule+0xbc/0x7f0
>> [c0000000f775f3c0] [c000000000a29944] .__schedule+0xa4/0x7f0 (unreliable)
>> [c0000000f775f620] [c000000000a277f4] .schedule_timeout+0x1b4/0x2e0
>> [c0000000f775f700] [c000000000a29428] .wait_for_common+0xf8/0x1d0
>> [c0000000f775f7c0] [c000000000a295ac] .wait_for_completion_interruptible+0x2c/0x50
>> [c0000000f775f840] [c000000000494b64] .do_one_async_hash_op.isra.1.part.2+0x24/0x50
>> [c0000000f775f8c0] [c0000000004951a8] .test_hash+0x618/0x7d0
>> [c0000000f775fb30] [c000000000495424] .alg_test_hash+0xc4/0xf0
>> [c0000000f775fbc0] [c000000000494928] .alg_test+0xa8/0x2c0
>> [c0000000f775fcb0] [c000000000491164] .cryptomgr_test+0x64/0x80
>> [c0000000f775fd30] [c00000000009a8d0] .kthread+0xf0/0x100
>> [c0000000f775fe30] [c000000000000a08] .ret_from_kernel_thread+0x5c/0xd4
>> Instruction dump:
>> 7c641b78 419e0160 e8a90050 2fa50000 409e0008 e8a90010 e8de0028 e8fe0030
>> 3c62ff90 38638320 48546b69 60000000 <0fe00000> 4bffff34 e87e0010 2fa30000
>> ---[ end trace 52825d316d569f00 ]---
>>
>> Signed-off-by: Yanjiang Jin <[email protected]>
>> ---
>
>
>

2015-03-03 06:57:22

by yjin

[permalink] [raw]
Subject: Re: [PATCH 3/4] crypto: caamhash: add two missed dma_mapping_error


On 2015年03月02日 19:53, Horia Geantă wrote:
> On 2/28/2015 8:00 AM, [email protected] wrote:
>> From: Yanjiang Jin <[email protected]>
>>
>> Add two missed dma_mapping_error() after dma_map_single().
>>
>> Signed-off-by: Yanjiang Jin <[email protected]>
>> ---
>> drivers/crypto/caam/caamhash.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
>> index f347ab7..f6ad322 100644
>> --- a/drivers/crypto/caam/caamhash.c
>> +++ b/drivers/crypto/caam/caamhash.c
>> @@ -160,6 +160,10 @@ static inline dma_addr_t map_seq_out_ptr_result(u32 *desc, struct device *jrdev,
>> dma_addr_t dst_dma;
>>
>> dst_dma = dma_map_single(jrdev, result, digestsize, DMA_FROM_DEVICE);
>> + if (dma_mapping_error(jrdev, dst_dma)) {
>> + dev_err(jrdev, "unable to map dst dma\n");
>> + return -ENOMEM;
>> + }
>> append_seq_out_ptr(desc, dst_dma, digestsize, 0);
>>
>> return dst_dma;
> Value returned by map_seq_out_ptr_result() - dst_dma - is always fed to
> dma_mapping_error().
> Note that using an invalid dst_dma in append_seq_out_ptr() doesn't break
> anything, so it's ok to check dst_dma later.
>
>> @@ -173,6 +177,10 @@ static inline dma_addr_t buf_map_to_sec4_sg(struct device *jrdev,
>> dma_addr_t buf_dma;
>>
>> buf_dma = dma_map_single(jrdev, buf, buflen, DMA_TO_DEVICE);
>> + if (dma_mapping_error(jrdev, buf_dma)) {
>> + dev_err(jrdev, "unable to map buf dma\n");
>> + return 0;
>> + }
>> dma_to_sec4_sg_one(sec4_sg, buf_dma, buflen, 0);
>>
>> return buf_dma;
>>
> These functions are expected to return dma_addr_t, not an error code.
> If dma_mapping_error() is needed within their scope, the return type
> will have to change. And return value will need to be checked by their
> callers.
>
System run well without the above changes, so abandoned this patch in
V2. Will do more test in the future.

Thanks!
Yanjiang
>
>