Re: [PATCH v3 20/34] dmaengine: xilinx-dpdma: kill vchan BH on remove

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The device AXI clock is disabled before tasklets and BHs are explicitly killed in the remove path. This allows pending tasklets or BH work to execute and access hardware registers while the clock is off, causing fatal AXI bus errors.
--

--- Patch [20]: [PATCH v3 20/34] dmaengine: xilinx-dpdma: kill vchan BH on remove ---
commit cd30e3f8d0e3620da3deb8ffdbd41901ce171ab7
Author: Allen Pais <[email protected]>
dmaengine: xilinx-dpdma: kill vchan BH on remove

This commit modifies the xilinx-dpdma driver to cancel the virt-dma
per-channel BH work when removing a channel. It adds a call to
dmaengine_kill_bh() to ensure the BH is killed before the channel is
removed, alongside the existing tasklet cleanup.

> diff --git a/drivers/dma/xilinx/xilinx_dpdma.c b/drivers/dma/xilinx/xilinx_dpdma.c
> index d9a3542c4531c..7e147ea11368d 100644
> --- a/drivers/dma/xilinx/xilinx_dpdma.c
> +++ b/drivers/dma/xilinx/xilinx_dpdma.c
> @@ -1685,6 +1685,7 @@ static void xilinx_dpdma_chan_remove(struct xilinx_dpdma_chan *chan)
>  		return;
>  
>  	tasklet_kill(&chan->err_task);
> +	dmaengine_kill_bh(&chan->vchan.chan);

[Severity: High]
This is a pre-existing issue, but could waiting for the BH or tasklet to
finish here trigger an AXI bus fault?

In xilinx_dpdma_remove(), the AXI clock is disabled before the driver iterates
over the channels to remove them:

drivers/dma/xilinx/xilinx_dpdma.c:xilinx_dpdma_remove() {
	...
	clk_disable_unprepare(xdev->axi_clk);

	for (i = 0; i < ARRAY_SIZE(xdev->chan); i++)
		xilinx_dpdma_chan_remove(xdev->chan[i]);
	...
}

If a tasklet like chan->err_task or a vchan BH was scheduled by the hardirq
just before the IRQ is freed, it could execute while the clock is already
disabled.

Functions like xilinx_dpdma_chan_err_task() read hardware registers via
xilinx_dpdma_chan_poll_no_ostand(). Does accessing these memory-mapped
registers without the required clock cause a system hang or panic?

Would it make sense to disable the AXI clock only after all channels have
been removed and their tasklets and BHs killed?

>  	list_del(&chan->vchan.chan.device_node);
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=20