Re: [PATCH v4 03/12] dmaengine: switchtec-dma: always clear DMA base registers on chan_stop()

On 2026-08-14 13:45, Frank Li wrote:
> On Tue, Jul 28, 2026 at 11:15:14AM -0600, Logan Gunthorpe wrote:
>> switchtec_dma_chan_stop() returned early if halt_channel() timed out,
>> skipping the writes that clear sq_base/cq_base on the channel, and
>> gave its caller no way to tell the halt hadn't been confirmed.
>> switchtec_dma_free_chan_resources() unconditionally frees the
>> descriptor rings right after calling this function, so if the
>> hardware failed to halt, it could keep writing into memory that had
>> already been freed.
>>
>> Attempt the register clear regardless of whether the halt was successful
>> and have switchtec_dma_chan_stop() return the halt result so callers can
>> tell when it wasn't confirmed. switchtec_dma_free_chan_resources() now
>> skips freeing the descriptor rings (leaking them instead) in case the
>> hardware continues to write into that memory.
>>
>> But all this is hardening that is pretty unlikely to be hit in the real
>> world.
>>
>> Reported-by: Sashiko <[email protected]>
>> Link: https://lore.kernel.org/dmaengine/[email protected]
>> Signed-off-by: Logan Gunthorpe <[email protected]>
>> ---
> 
> This common problem when timeout happen. Before we have good method to
> handle it, I suggest leave it as it for now.
> 
> I don't want to introduce new problem by fix an unlikely happen problem.

I have to push back at this. I *really* think this patch, and the next
one, are worth applying.

While I definitely agree that this is an unlikely problem to hit in the
real world, I think it is the correct and best approach to fix the
problem. I don't think there is some good common method that the
dmaengine layer can implement to improve the situation, and if there is
it can always be applied on top of this change.

The problem it is trying to fix is a hypothetical hardware failure where
the hardware was given a job to do and never returned a completion. In
this scenario the hardware could theoretically wake up and write to that
memory at any time after the driver is completely removed. And the only
conceivable solution (absent an IOMMU) to preventing that memory from
being reused and then scribbled on by the buggy hardware, is to leak it.

But we don't know if the firmware in our hardware has bugs that could
ever trigger the problem so it is a bit of a moot point. However, for me
personally, I'd rather apply the fix in order to silence Sashiko. I'm
fairly confident it is correct and isn't going to cause another issue.
I'd much rather have this fix, as it is, than to have Sashiko
complaining about it every time I send a patch set.

Thanks,

Logan