linux-kernel - Re: [PATCH v2 1/4] soc: fsl: qmc: Only set completion interrupt when needed

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 for Android: free password hash cracker in your pocket

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

Message-ID: <06c3012d-96f1-434a-af17-a1705eede7d6@csgroup.eu>
Date: Mon, 18 Aug 2025 10:16:16 +0200
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Herve Codina <herve.codina@...tlin.com>
Cc: Qiang Zhao <qiang.zhao@....com>, Shengjiu Wang <shengjiu.wang@...il.com>,
 Xiubo Li <Xiubo.Lee@...il.com>, Fabio Estevam <festevam@...il.com>,
 Nicolin Chen <nicoleotsuka@...il.com>, Liam Girdwood <lgirdwood@...il.com>,
 Mark Brown <broonie@...nel.org>, Jaroslav Kysela <perex@...ex.cz>,
 Takashi Iwai <tiwai@...e.com>, linuxppc-dev@...ts.ozlabs.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-sound@...r.kernel.org
Subject: Re: [PATCH v2 1/4] soc: fsl: qmc: Only set completion interrupt when
 needed

Hi Hervé,

Le 14/08/2025 à 09:34, Herve Codina a écrit :
> Hi Christophe,
> 
> On Wed, 13 Aug 2025 12:06:51 +0200
> Herve Codina <herve.codina@...tlin.com> wrote:
> 
>> Hi Christophe,
>>
>> On Tue, 12 Aug 2025 12:50:55 +0200
>> Christophe Leroy <christophe.leroy@...roup.eu> wrote:
>>
>>> When no post-completion processing is expected, don't waste time
>>> handling useless interrupts.
>>>
>>> Only set QMC_BD_[R/T]X_I when a completion function is passed in,
>>> and perform seamless completion on submit for interruptless buffers.
>>>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
>>> ---
>>> v2: Keep the UB flag to mark not completed buffers and seamlessly flag them as completed during next submit.
>>> ---
>>>   drivers/soc/fsl/qe/qmc.c | 44 ++++++++++++++++++++++++++++++----------
>>>   1 file changed, 33 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
>>> index 36c0ccc06151f..8f76b9a5e385d 100644
>>> --- a/drivers/soc/fsl/qe/qmc.c
>>> +++ b/drivers/soc/fsl/qe/qmc.c
>>> @@ -461,9 +461,16 @@ int qmc_chan_write_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,
>>>   
>>>   	ctrl = qmc_read16(&bd->cbd_sc);
>>>   	if (ctrl & (QMC_BD_TX_R | QMC_BD_TX_UB)) {
>>> -		/* We are full ... */
>>> -		ret = -EBUSY;
>>> -		goto end;
>>> +		if (!(ctrl & QMC_BD_TX_I) && bd == chan->txbd_done) {
>>> +			if (ctrl & QMC_BD_TX_W)
>>> +				chan->txbd_done = chan->txbds;
>>> +			else
>>> +				chan->txbd_done++;
>>> +		} else {
>>> +			/* We are full ... */
>>> +			ret = -EBUSY;
>>> +			goto end;
>>> +		}
>>>   	}
>>>   
>>>   	qmc_write16(&bd->cbd_datlen, length);
>>> @@ -475,6 +482,10 @@ int qmc_chan_write_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,
>>>   
>>>   	/* Activate the descriptor */
>>>   	ctrl |= (QMC_BD_TX_R | QMC_BD_TX_UB);
>>> +	if (complete)
>>> +		ctrl |= QMC_BD_TX_I;
>>> +	else
>>> +		ctrl &= ~QMC_BD_TX_I;
>>>   	wmb(); /* Be sure to flush the descriptor before control update */
>>>   	qmc_write16(&bd->cbd_sc, ctrl);
>>>     
>>
>> You try to purge one descriptor for which the transfer is done but you do that
>> when you have no more free descriptors.
>>
>> You end up with all descriptor "used". I think a better way to do that is
>> to purge all "done" descriptor configured to work without interrupts until a
>> descriptor with interrupt is found.
> 
> I have looked again at your code and looking for a free descriptor only when it
> is needed is sufficient. You can forget my previous proposal.
> 
> Back to your code, I think you need to be sure that the descriptor you want to
> re-use is really available and so you need to check the 'R' bit to be sure
> that we are not with 'R' = 1 and 'UB' = 1 which means "BD is used, waiting for
> a transfer".
> 

Fixed in v3.

Thanks
Christophe