linux-kernel - Re: [PATCH v2 1/2] dmaengine: dw-edma: Fix unmasking STOP and ABORT interrupts for HDMA

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: <6faa27be-54eb-ca00-f2a8-de3eb6fa7547@quicinc.com>
Date: Mon, 5 Aug 2024 19:30:14 +0530
From: Mrinmay Sarkar <quic_msarkar@...cinc.com>
To: Serge Semin <fancer.lancer@...il.com>
CC: <manivannan.sadhasivam@...aro.org>, <vkoul@...nel.org>,
        <quic_shazhuss@...cinc.com>, <quic_nitegupt@...cinc.com>,
        <quic_ramkri@...cinc.com>, <quic_nayiluri@...cinc.com>,
        <quic_krichai@...cinc.com>, <quic_vbadigan@...cinc.com>,
        <stable@...r.kernel.org>, Cai Huoqing <cai.huoqing@...ux.dev>,
        <dmaengine@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] dmaengine: dw-edma: Fix unmasking STOP and ABORT
 interrupts for HDMA


On 8/2/2024 5:12 AM, Serge Semin wrote:
> On Tue, Jul 23, 2024 at 06:49:31PM +0530, Mrinmay Sarkar wrote:
>> The current logic is enabling both STOP_INT_MASK and ABORT_INT_MASK
>> bit. This is apparently masking those particular interrupts rather than
>> unmasking the same. If the interrupts are masked, they would never get
>> triggered.
>>
>> So fix the issue by unmasking the STOP and ABORT interrupts properly.
>>
>> Fixes: e74c39573d35 ("dmaengine: dw-edma: Add support for native HDMA")
>> cc: stable@...r.kernel.org
>> Signed-off-by: Mrinmay Sarkar <quic_msarkar@...cinc.com>
>> ---
>>   drivers/dma/dw-edma/dw-hdma-v0-core.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
>> index 10e8f07..fa89b3a 100644
>> --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
>> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
>> @@ -247,10 +247,11 @@ static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
>>   	if (first) {
>>   		/* Enable engine */
>>   		SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
>> -		/* Interrupt enable&unmask - done, abort */
>> -		tmp = GET_CH_32(dw, chan->dir, chan->id, int_setup) |
>> -		      HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK |
>> -		      HDMA_V0_LOCAL_STOP_INT_EN | HDMA_V0_LOCAL_ABORT_INT_EN;
> Just curious, if all the interrupts were actually masked, how has this
> been even working?.. In other words if it affected both local and
> remote interrupts, then the HDMA driver has never actually worked,
> right?

Agreed, it should not work as interrupts were masked.

But as we are enabling LIE/RIE bits (LWIE/RWIE) that eventually enabling 
watermark
interrupt in HDMA case and somehow on device I could see interrupt was
generating with watermark and stop bit set and it was working.
Since we were not clearing watermark interrupt, it was also causing 
storm of interrupt.

>> +		/* Interrupt unmask - STOP, ABORT */
>> +		tmp = GET_CH_32(dw, chan->dir, chan->id, int_setup) &
>> +		      ~HDMA_V0_STOP_INT_MASK & ~HDMA_V0_ABORT_INT_MASK;
> Please convert this to:
> +		/* Interrupt unmask - stop, abort */
> +		tmp = GET_CH_32(dw, chan->dir, chan->id, int_setup);
> +		tmp &= ~(HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
>
> -Serge(y)

Sure. Will do

Thanks,
Mrinmay

>> +		/* Interrupt enable - STOP, ABORT */
>> +		tmp |= HDMA_V0_LOCAL_STOP_INT_EN | HDMA_V0_LOCAL_ABORT_INT_EN;
>>   		if (!(dw->chip->flags & DW_EDMA_CHIP_LOCAL))
>>   			tmp |= HDMA_V0_REMOTE_STOP_INT_EN | HDMA_V0_REMOTE_ABORT_INT_EN;
>>   		SET_CH_32(dw, chan->dir, chan->id, int_setup, tmp);
>> -- 
>> 2.7.4
>>