Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dma: pl330: ensure DMA descriptors are zero-initialised
@ 2013-11-29 11:50 Will Deacon
  2013-11-29 12:57 ` Shevchenko, Andriy
  2013-11-29 15:18 ` Jassi Brar
  0 siblings, 2 replies; 4+ messages in thread
From: Will Deacon @ 2013-11-29 11:50 UTC (permalink / raw)
  To: linux-arm-kernel

I see the following splat with 3.13-rc1 when attempting to perform DMA:

[  253.004516] Alignment trap: not handling instruction e1902f9f at [<c0204b40>]
[  253.004583] Unhandled fault: alignment exception (0x221) at 0xdfdfdfd7
[  253.004646] Internal error: : 221 [#1] PREEMPT SMP ARM
[  253.004691] Modules linked in: dmatest(+) [last unloaded: dmatest]
[  253.004798] CPU: 0 PID: 671 Comm: kthreadd Not tainted 3.13.0-rc1+ #2
[  253.004864] task: df9b0900 ti: df03e000 task.ti: df03e000
[  253.004937] PC is at dmaengine_unmap_put+0x14/0x34
[  253.005010] LR is at pl330_tasklet+0x3c8/0x550
[  253.005087] pc : [<c0204b44>]    lr : [<c0207478>]    psr: a00e0193
[  253.005087] sp : df03fe48  ip : 00000000  fp : df03bf18
[  253.005178] r10: bf00e108  r9 : 00000001  r8 : 00000000
[  253.005245] r7 : df837040  r6 : dfb41800  r5 : df837048  r4 : df837000
[  253.005316] r3 : dfdfdfcf  r2 : dfb41f80  r1 : df837048  r0 : dfdfdfd7
[  253.005384] Flags: NzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
[  253.005459] Control: 30c5387d  Table: 9fb9ba80  DAC: fffffffd
[  253.005520] Process kthreadd (pid: 671, stack limit = 0xdf03e248)

This is due to desc->txd.unmap containing garbage (uninitialised memory).

Rather than add another dummy initialisation to _init_desc, instead
ensure that the descriptors are zero-initialised during allocation and
remove the dummy, per-field initialisation.

Cc: Jassi Brar <jaswinder.singh@linaro.org>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 drivers/dma/pl330.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 98641eaca080..79e52a94f054 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -2492,14 +2492,8 @@ static dma_cookie_t pl330_tx_submit(struct dma_async_tx_descriptor *tx)
 
 static inline void _init_desc(struct dma_pl330_desc *desc)
 {
-	desc->pchan = NULL;
 	desc->req.x = &desc->px;
 	desc->req.token = desc;
-	desc->rqcfg.swap = SWAP_NO;
-	desc->rqcfg.privileged = 0;
-	desc->rqcfg.insnaccess = 0;
-	desc->rqcfg.scctl = SCCTRL0;
-	desc->rqcfg.dcctl = DCCTRL0;
 	desc->req.cfg = &desc->rqcfg;
 	desc->req.xfer_cb = dma_pl330_rqcb;
 	desc->txd.tx_submit = pl330_tx_submit;
@@ -2517,7 +2511,7 @@ static int add_desc(struct dma_pl330_dmac *pdmac, gfp_t flg, int count)
 	if (!pdmac)
 		return 0;
 
-	desc = kmalloc(count * sizeof(*desc), flg);
+	desc = kzalloc(count * sizeof(*desc), flg);
 	if (!desc)
 		return 0;
 
-- 
1.8.2.2

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH] dma: pl330: ensure DMA descriptors are zero-initialised
  2013-11-29 11:50 [PATCH] dma: pl330: ensure DMA descriptors are zero-initialised Will Deacon
@ 2013-11-29 12:57 ` Shevchenko, Andriy
  2013-11-29 14:41   ` Will Deacon
  2013-11-29 15:18 ` Jassi Brar
  1 sibling, 1 reply; 4+ messages in thread
From: Shevchenko, Andriy @ 2013-11-29 12:57 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 2013-11-29 at 11:50 +0000, Will Deacon wrote:
> I see the following splat with 3.13-rc1 when attempting to perform DMA:
> 
> [  253.004516] Alignment trap: not handling instruction e1902f9f at [<c0204b40>]
> [  253.004583] Unhandled fault: alignment exception (0x221) at 0xdfdfdfd7
> [  253.004646] Internal error: : 221 [#1] PREEMPT SMP ARM
> [  253.004691] Modules linked in: dmatest(+) [last unloaded: dmatest]
> [  253.004798] CPU: 0 PID: 671 Comm: kthreadd Not tainted 3.13.0-rc1+ #2
> [  253.004864] task: df9b0900 ti: df03e000 task.ti: df03e000
> [  253.004937] PC is at dmaengine_unmap_put+0x14/0x34
> [  253.005010] LR is at pl330_tasklet+0x3c8/0x550
> [  253.005087] pc : [<c0204b44>]    lr : [<c0207478>]    psr: a00e0193
> [  253.005087] sp : df03fe48  ip : 00000000  fp : df03bf18
> [  253.005178] r10: bf00e108  r9 : 00000001  r8 : 00000000
> [  253.005245] r7 : df837040  r6 : dfb41800  r5 : df837048  r4 : df837000
> [  253.005316] r3 : dfdfdfcf  r2 : dfb41f80  r1 : df837048  r0 : dfdfdfd7
> [  253.005384] Flags: NzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> [  253.005459] Control: 30c5387d  Table: 9fb9ba80  DAC: fffffffd
> [  253.005520] Process kthreadd (pid: 671, stack limit = 0xdf03e248)
> 
> This is due to desc->txd.unmap containing garbage (uninitialised memory).
> 
> Rather than add another dummy initialisation to _init_desc, instead
> ensure that the descriptors are zero-initialised during allocation and
> remove the dummy, per-field initialisation.
> 
> Cc: Jassi Brar <jaswinder.singh@linaro.org>
> Cc: Vinod Koul <vinod.koul@intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  drivers/dma/pl330.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index 98641eaca080..79e52a94f054 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -2492,14 +2492,8 @@ static dma_cookie_t pl330_tx_submit(struct dma_async_tx_descriptor *tx)
>  
>  static inline void _init_desc(struct dma_pl330_desc *desc)
>  {
> -	desc->pchan = NULL;
>  	desc->req.x = &desc->px;
>  	desc->req.token = desc;
> -	desc->rqcfg.swap = SWAP_NO;
> -	desc->rqcfg.privileged = 0;
> -	desc->rqcfg.insnaccess = 0;
> -	desc->rqcfg.scctl = SCCTRL0;
> -	desc->rqcfg.dcctl = DCCTRL0;
>  	desc->req.cfg = &desc->rqcfg;
>  	desc->req.xfer_cb = dma_pl330_rqcb;
>  	desc->txd.tx_submit = pl330_tx_submit;
> @@ -2517,7 +2511,7 @@ static int add_desc(struct dma_pl330_dmac *pdmac, gfp_t flg, int count)
>  	if (!pdmac)
>  		return 0;
>  
> -	desc = kmalloc(count * sizeof(*desc), flg);
> +	desc = kzalloc(count * sizeof(*desc), flg);

Maybe kcalloc() ?

>  	if (!desc)
>  		return 0;
>  

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] dma: pl330: ensure DMA descriptors are zero-initialised
  2013-11-29 12:57 ` Shevchenko, Andriy
@ 2013-11-29 14:41   ` Will Deacon
  0 siblings, 0 replies; 4+ messages in thread
From: Will Deacon @ 2013-11-29 14:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Nov 29, 2013 at 12:57:50PM +0000, Shevchenko, Andriy wrote:
> On Fri, 2013-11-29 at 11:50 +0000, Will Deacon wrote:
> > diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> > index 98641eaca080..79e52a94f054 100644
> > --- a/drivers/dma/pl330.c
> > +++ b/drivers/dma/pl330.c
> > @@ -2492,14 +2492,8 @@ static dma_cookie_t pl330_tx_submit(struct dma_async_tx_descriptor *tx)
> >  
> >  static inline void _init_desc(struct dma_pl330_desc *desc)
> >  {
> > -	desc->pchan = NULL;
> >  	desc->req.x = &desc->px;
> >  	desc->req.token = desc;
> > -	desc->rqcfg.swap = SWAP_NO;
> > -	desc->rqcfg.privileged = 0;
> > -	desc->rqcfg.insnaccess = 0;
> > -	desc->rqcfg.scctl = SCCTRL0;
> > -	desc->rqcfg.dcctl = DCCTRL0;
> >  	desc->req.cfg = &desc->rqcfg;
> >  	desc->req.xfer_cb = dma_pl330_rqcb;
> >  	desc->txd.tx_submit = pl330_tx_submit;
> > @@ -2517,7 +2511,7 @@ static int add_desc(struct dma_pl330_dmac *pdmac, gfp_t flg, int count)
> >  	if (!pdmac)
> >  		return 0;
> >  
> > -	desc = kmalloc(count * sizeof(*desc), flg);
> > +	desc = kzalloc(count * sizeof(*desc), flg);
> 
> Maybe kcalloc() ?

Yup, that could work. I'll include that in v2 pending any other comments.

Cheers,

Will

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] dma: pl330: ensure DMA descriptors are zero-initialised
  2013-11-29 11:50 [PATCH] dma: pl330: ensure DMA descriptors are zero-initialised Will Deacon
  2013-11-29 12:57 ` Shevchenko, Andriy
@ 2013-11-29 15:18 ` Jassi Brar
  1 sibling, 0 replies; 4+ messages in thread
From: Jassi Brar @ 2013-11-29 15:18 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Will,

On Fri, Nov 29, 2013 at 5:20 PM, Will Deacon <will.deacon@arm.com> wrote:
>
> I see the following splat with 3.13-rc1 when attempting to perform DMA:
>
> [  253.004516] Alignment trap: not handling instruction e1902f9f at [<c0204b40>]
> [  253.004583] Unhandled fault: alignment exception (0x221) at 0xdfdfdfd7
> [  253.004646] Internal error: : 221 [#1] PREEMPT SMP ARM
> [  253.004691] Modules linked in: dmatest(+) [last unloaded: dmatest]
> [  253.004798] CPU: 0 PID: 671 Comm: kthreadd Not tainted 3.13.0-rc1+ #2
> [  253.004864] task: df9b0900 ti: df03e000 task.ti: df03e000
> [  253.004937] PC is at dmaengine_unmap_put+0x14/0x34
> [  253.005010] LR is at pl330_tasklet+0x3c8/0x550
> [  253.005087] pc : [<c0204b44>]    lr : [<c0207478>]    psr: a00e0193
> [  253.005087] sp : df03fe48  ip : 00000000  fp : df03bf18
> [  253.005178] r10: bf00e108  r9 : 00000001  r8 : 00000000
> [  253.005245] r7 : df837040  r6 : dfb41800  r5 : df837048  r4 : df837000
> [  253.005316] r3 : dfdfdfcf  r2 : dfb41f80  r1 : df837048  r0 : dfdfdfd7
> [  253.005384] Flags: NzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> [  253.005459] Control: 30c5387d  Table: 9fb9ba80  DAC: fffffffd
> [  253.005520] Process kthreadd (pid: 671, stack limit = 0xdf03e248)
>
> This is due to desc->txd.unmap containing garbage (uninitialised memory).
>
> Rather than add another dummy initialisation to _init_desc, instead
> ensure that the descriptors are zero-initialised during allocation and
> remove the dummy, per-field initialisation.
>
> Cc: Jassi Brar <jaswinder.singh@linaro.org>
> Cc: Vinod Koul <vinod.koul@intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  drivers/dma/pl330.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index 98641eaca080..79e52a94f054 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -2492,14 +2492,8 @@ static dma_cookie_t pl330_tx_submit(struct dma_async_tx_descriptor *tx)
>
>  static inline void _init_desc(struct dma_pl330_desc *desc)
>  {
> -       desc->pchan = NULL;
>         desc->req.x = &desc->px;
>         desc->req.token = desc;
> -       desc->rqcfg.swap = SWAP_NO;
> -       desc->rqcfg.privileged = 0;
> -       desc->rqcfg.insnaccess = 0;
> -       desc->rqcfg.scctl = SCCTRL0;
> -       desc->rqcfg.dcctl = DCCTRL0;
>
These happen to evaluate to 0, but for completeness sake they ought to
be initialized somewhere. Otherwise we might as well drop these
members.

However I don't strongly feel about either way. So whatever comes in v2...

Acked-by: Jassi Brar <jassisinghbrar@gmail.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-11-29 15:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-29 11:50 [PATCH] dma: pl330: ensure DMA descriptors are zero-initialised Will Deacon
2013-11-29 12:57 ` Shevchenko, Andriy
2013-11-29 14:41   ` Will Deacon
2013-11-29 15:18 ` Jassi Brar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox