diff mbox series

[v2,24/26] dmaengine: dw-edma: Skip cleanup procedure if no private data found

Message ID 20220503225104.12108-25-Sergey.Semin@baikalelectronics.ru (mailing list archive)
State Superseded
Headers show
Series dmaengine: dw-edma: Add RP/EP local DMA controllers support | expand

Commit Message

Serge Semin May 3, 2022, 10:51 p.m. UTC
DW eDMA driver private data is preserved in the passed DW eDMA chip info
structure. If either probe procedure failed or for some reason the passed
info object doesn't have private data pointer initialized we need to halt
the DMA device cleanup procedure in order to prevent possible system
crashes.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/dma/dw-edma/dw-edma-core.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Manivannan Sadhasivam May 12, 2022, 3:23 p.m. UTC | #1
On Wed, May 04, 2022 at 01:51:02AM +0300, Serge Semin wrote:
> DW eDMA driver private data is preserved in the passed DW eDMA chip info
> structure. If either probe procedure failed or for some reason the passed
> info object doesn't have private data pointer initialized we need to halt
> the DMA device cleanup procedure in order to prevent possible system
> crashes.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>

As I said in the previous iteration, I'm not sure we will hit this issue.
But I don't object the patch. So,

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/dma/dw-edma/dw-edma-core.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 908607785401..561686b51915 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -1035,6 +1035,10 @@ int dw_edma_remove(struct dw_edma_chip *chip)
>  	struct dw_edma *dw = chip->dw;
>  	int i;
>  
> +	/* Skip removal if no private data found */
> +	if (!dw)
> +		return -ENODEV;
> +
>  	/* Disable eDMA */
>  	dw_edma_v0_core_off(dw);
>  
> -- 
> 2.35.1
>
Serge Semin May 12, 2022, 5:38 p.m. UTC | #2
On Thu, May 12, 2022 at 08:53:54PM +0530, Manivannan Sadhasivam wrote:
> On Wed, May 04, 2022 at 01:51:02AM +0300, Serge Semin wrote:
> > DW eDMA driver private data is preserved in the passed DW eDMA chip info
> > structure. If either probe procedure failed or for some reason the passed
> > info object doesn't have private data pointer initialized we need to halt
> > the DMA device cleanup procedure in order to prevent possible system
> > crashes.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 

> As I said in the previous iteration, I'm not sure we will hit this issue.

It's unlikely but still not impossible. Consider the case when your DW
PCIe controller has eDMA capability, but the platform code isn't ready
to have it properly utilized. In that case dw_edma_probe() will fail
at some point while the rest of the DW PCIe code will go on executing
ignoring that error. In that case calling the dw_edma_remove() will
cause the system crash since the dw_edma_chip.dw pointer will be left
uninitialized. For instance possible circumstances can be as follows:

dw_pcie_host_init()
+-> dw_pcie_edma_detect() FAIL ignored.
+-> pci_host_probe() FAIL goto err_stop_link
                      |
                      +-> dw_pcie_edma_remove() CRASH !!!

So to be on a safe side we need to make sure the DW eDMA descriptor
cleanup is skipped in case if it hasn't been properly initialized. The
proper initialization is indicated by having the dw_edma_chip.dw set
with the valid pointer to the private data.

> But I don't object the patch. So,
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks.

-Sergey

> 
> Thanks,
> Mani
> 
> > ---
> >  drivers/dma/dw-edma/dw-edma-core.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > index 908607785401..561686b51915 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -1035,6 +1035,10 @@ int dw_edma_remove(struct dw_edma_chip *chip)
> >  	struct dw_edma *dw = chip->dw;
> >  	int i;
> >  
> > +	/* Skip removal if no private data found */
> > +	if (!dw)
> > +		return -ENODEV;
> > +
> >  	/* Disable eDMA */
> >  	dw_edma_v0_core_off(dw);
> >  
> > -- 
> > 2.35.1
> > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்
diff mbox series

Patch

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 908607785401..561686b51915 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -1035,6 +1035,10 @@  int dw_edma_remove(struct dw_edma_chip *chip)
 	struct dw_edma *dw = chip->dw;
 	int i;
 
+	/* Skip removal if no private data found */
+	if (!dw)
+		return -ENODEV;
+
 	/* Disable eDMA */
 	dw_edma_v0_core_off(dw);