Re: [PATCH v2 3/3] PCI: epf-mhi: Implement mhi_cntrl->flush_async() to flush DMA read/write

[sashiko-bot@kernel.org]

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

New issues:
- [High] Deadlock / Infinite wait in pci_epf_mhi_edma_flush_async() when synchronizing globally shared DMA channels.

Pre-existing issues:
- [High] Pre-existing NULL pointer dereference on PCI link flap exacerbated by unconditional cleanup dereference.
--

--- Patch [3]: [PATCH v2 3/3] PCI: epf-mhi: Implement mhi_cntrl->flush_async() to flush DMA read/write ---
commit c83c3a93b470add22e521d4fb56e38b2a49dd98b
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

PCI: epf-mhi: Implement mhi_cntrl->flush_async() to flush DMA read/write

The MHI core needs to make sure that all the current DMA transactions are
completed before removing the channels. So implement the
mhi_cntrl->flush_async() callback by first making sure all the in-flight
DMA operations are completed and then flushing the DMA workqueue.

> diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c
> --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
[ ... ]
> @@ -644,6 +644,15 @@ static int pci_epf_mhi_edma_write_async(struct mhi_ep_cntrl *mhi_cntrl,
>  	return ret;
>  }
>  
> +static void pci_epf_mhi_edma_flush_async(struct mhi_ep_cntrl *mhi_cntrl)
> +{
> +	struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl);
> +
> +	dmaengine_synchronize(epf_mhi->dma_chan_rx);
> +	dmaengine_synchronize(epf_mhi->dma_chan_tx);
> +	flush_workqueue(epf_mhi->dma_wq);

[Severity: High]
Can this cause an indefinite hang if other channels on the same controller
are active?

If a user unloads an MHI client module (invoking mhi_ep_remove), it calls
this function to drain the DMA channel. However, because
epf_mhi->dma_chan_rx and dma_chan_tx are globally shared across the entire
PCI endpoint, and dmaengine_synchronize() polls until the channel is no
longer busy, continuous data traffic from a different independent MHI
client might prevent this from ever returning.

[Severity: High]
This is a pre-existing issue, but could this also lead to a NULL pointer
dereference after a PCI link flap?

When the PCI link goes down, pci_epf_mhi_link_down() calls
pci_epf_mhi_dma_deinit(), which frees the DMA channels and workqueue and
sets them to NULL.

When the link comes back up, pci_epf_mhi_link_up() unconditionally assigns
these EDMA callbacks but fails to call pci_epf_mhi_dma_init() to reallocate
the resources.

If the link goes down again (or a client device is removed), invoking this
new flush_async() callback will pass NULL pointers to
dmaengine_synchronize() and flush_workqueue(), causing a crash.

> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714-mhi-ep-flush-v2-0-b6a9db011e85@oss.qualcomm.com?part=3