Re: [PATCH 5/9] dmaengine: dw-edma: HDMA: Fix possible race condition in remote setup

["Köry Maincent" <kory.maincent@bootlin.com>]

On Mon, 19 Jun 2023 20:15:50 +0300
Serge Semin <fancer.lancer@gmail.com> wrote:

> On Fri, Jun 09, 2023 at 10:16:50AM +0200, Köry Maincent wrote:
> > From: Kory Maincent <kory.maincent@bootlin.com>
> > 
> > When writing the linked list elements and pointer the control need to be
> > written at the end. If the control is written and the SAR and DAR not
> > stored we could face a race condition.
> > 
> > Fixes: e74c39573d35 ("dmaengine: dw-edma: Add support for native HDMA")
> > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>  
> 
> Once again. Is this a hypothetical bug or have you actually
> experienced the denoted problem? If you do please describe the
> circumstances, give more details. Otherwise it sounds weird. Here is
> why.
> 
> DW eDMA HW manual states that the control LL DWORD is indeed supposed
> to be written after the rest of the descriptor DWORDs are written. But
> AFAICS it's only relevant for the LL tree entries recycling. Current
> DW eDMA driver design doesn't truly implement that pattern. Instead
> the DMA transfer is halted at the end of the chunk. Then the engine is
> recharged with the next chunk and execution is started over. So the
> runtime recycling isn't implemented (alas) for which the CB flag of
> the control DWORD needs to be updated only after the rest of the LLI
> fields.

This one is only hypothetical. It appears to me that writing the control
after the configuration of sar and dar is more relevant to prevent race issues
and should be the usual coding choice. Also you are right saying that it will
be relevant only for the LL tree entries recycling.
Simple question from non DMA expert: isn't cyclic DMA mode recycle the LL tree
entries? 

> 
> If you described a hypothetical problem then it would be ok to accept
> the change for the sake of consistency but I would have dropped the
> Fixes tag and updated the patch description with more details of the
> race condition you are talking about.

Alright, I will do that.

Köry