Re: [PATCH v2 5/6] mtd: rawnand: marvell: stop DMA before unmapping on timeout

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mtd: rawnand: marvell: sleep in atomic context in marvell_nfc_xfer_data_dma()
--

commit cbbe6eae8df4a34b02e625e64676a65024331f76
Author: Samarth Narang <samarth.colleges@gmail.com>

mtd: rawnand: marvell: stop DMA before unmapping on timeout

This commit replaces dmaengine_terminate_all() with
dmaengine_terminate_sync() and moves it before dma_unmap_sg() on the
timeout path in marvell_nfc_xfer_data_dma() so the DMA transfer is
stopped before unmapping the buffer.

> diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
> index 38b7eb5b992c8..a342a48e2a08c 100644
> --- a/drivers/mtd/nand/raw/marvell_nand.c
> +++ b/drivers/mtd/nand/raw/marvell_nand.c
> @@ -900,14 +900,15 @@ static int marvell_nfc_xfer_data_dma(struct marvell_nfc *nfc,
>  
>  	dma_async_issue_pending(nfc->dma_chan);
>  	ret = marvell_nfc_wait_cmdd(nfc->selected_chip);
> -	dma_unmap_sg(nfc->dma_chan->device->dev, &sg, 1, direction);
> -	marvell_nfc_disable_dma(nfc);
>  	if (ret) {
>  		dev_err(nfc->dev, "Timeout waiting for DMA (status: %d)\n",
>  			dmaengine_tx_status(nfc->dma_chan, cookie, NULL));
> -		dmaengine_terminate_all(nfc->dma_chan);
> -		return -ETIMEDOUT;
> +		dmaengine_terminate_sync(nfc->dma_chan);

[Severity: Medium]
Can calling dmaengine_terminate_sync() here sleep in atomic context during
a panic write?

This driver supports mtd->oops_panic_write by polling in
marvell_nfc_wait_op() instead of sleeping:

drivers/mtd/nand/raw/marvell_nand.c:marvell_nfc_wait_op() {
	if (mtd->oops_panic_write) {
		ret = marvell_nfc_poll_status(nfc, NDSR_RDY(0),
					      NDSR_RDY(0),
					      timeout_ms);
	} else {
		init_completion(&nfc->complete);
		...
}

When nfc->use_dma is enabled on NFCv1, panic writes still go through
marvell_nfc_xfer_data_dma():

mtd_panic_write()
  -> panic_nand_write()
    -> nand_do_write_ops()
      -> nand_write_page()
        -> marvell_nfc_hw_ecc_hmg_write_page()
          -> marvell_nfc_hw_ecc_hmg_do_write_page()
            -> marvell_nfc_xfer_data_dma()

Because mtd_panic_write() can be invoked with interrupts disabled or under
a raw spinlock (such as pstore_dump()), and dmaengine_terminate_sync()
calls dmaengine_synchronize() (which calls might_sleep(), wait_event() in
pxad_synchronize(), and tasklet_kill() in vchan_synchronize()), wouldn't a
DMA timeout during a panic write trigger a sleep-in-atomic bug or hang
waiting for the DMA tasklet while softirqs are disabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030162041.2998170-1-samarth.colleges@gmail.com?part=5