linux-kernel - Re: [PATCH V3] dmaengine: dw-axi-dmac: add spinlock for multi-channel DMA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZC+82NwoumBLYnml@chenyu5-mobl1>
Date:   Fri, 7 Apr 2023 14:48:56 +0800
From:   Chen Yu <yu.c.chen@...el.com>
To:     Libing Lei <libing.lei@...uarmicro.com>
CC:     <Eugeniy.Paltsev@...opsys.com>, <vkoul@...nel.org>,
        <dmaengine@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V3] dmaengine: dw-axi-dmac: add spinlock for
 multi-channel DMA

On 2023-04-04 at 23:39:45 +0800, Libing Lei wrote:
> When multiple channels operate simultaneously, the common register shared
> by all channels could be set to an unexpected value due to race conditions
Does the channel mean the "chip" in the following patch? I mean, each channel
has 1 chip plugged in? I asked this question because I'm trying to figure out
the protection scope of the introduced spinlock.
> on SMP machines, resulting in a disaster that DMAC cannot work from then
> on:
> 
> cpu0                            cpu1
>  dw_axi_dma_interrupt()
>   axi_dma_irq_disable(chip)
>   ...
>                                 axi_chan_block_xfer_start()
>                                  axi_dma_enable()
>                                   val = axi_dma_ioread32(chip, DMAC_CFG)
>   axi_dma_irq_enable(chip)
>                                   axi_dma_iowrite32(chip, DMAC_CFG, val)
> 
> As a result, the global interrupt enable bit INT_EN in the DMAC_CFG
> register is eventually cleared and the DMAC will no longer generates
> interrupts.
> 
> The error scenario is as follows:
> 
> [ 63.483688] dmatest: dma0chan1-copy: result #18: 'test timed out' with
> src_off=0xc2 dst_off=0x27b len=0x3a54 (0)
> [ 63.483693] dmatest: dma0chan2-copy: result #18: 'test timed out' with
> src_off=0x239 dst_off=0xfc9 len=0x213a (0)
> [ 63.483696] dmatest: dma0chan0-copy: result #19: 'test timed out' with
> src_off=0x5d1 dst_off=0x231 len=0x395e (0)
> 
> a spinlock is added to fix it up.
>
Although it is unlikely to introduce ABBA lock, maybe enabling the
CONFIG_LOCKDEP to have a double check on this. 
> Signed-off-by: Libing Lei <libing.lei@...uarmicro.com>
The patch looks good to me.

thanks,
Chenyu