Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 22 Jan 2023 00:10:18 +0300
From:   Serge Semin <fancer.lancer@gmail.com>
To:     Bjorn Helgaas <helgaas@kernel.org>, Vinod Koul <vkoul@kernel.org>
Cc:     Serge Semin <Sergey.Semin@baikalelectronics.ru>,
        Gustavo Pimentel <gustavo.pimentel@synopsys.com>,
        Rob Herring <robh@kernel.org>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Cai Huoqing <cai.huoqing@linux.dev>,
        Robin Murphy <robin.murphy@arm.com>,
        Jingoo Han <jingoohan1@gmail.com>, Frank Li <Frank.Li@nxp.com>,
        Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>,
        Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>,
        Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kw@linux.com>,
        caihuoqing <caihuoqing@baidu.com>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
        linux-pci@vger.kernel.org, dmaengine@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v9 09/27] dmaengine: dw-edma: Drop chancnt initialization
Message-ID: <20230121211018.wn2zjxts7hzmetin@mobilestation>
References: <20230113171409.30470-10-Sergey.Semin@baikalelectronics.ru>
 <20230120235449.GA682361@bhelgaas>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230120235449.GA682361@bhelgaas>
Precedence: bulk

On Fri, Jan 20, 2023 at 05:54:49PM -0600, Bjorn Helgaas wrote:
> On Fri, Jan 13, 2023 at 08:13:51PM +0300, Serge Semin wrote:
> > DMA device drivers aren't supposed to initialize the dma_device.chancnt
> > field. It will be done by the DMA-engine core in accordance with number of
> > added virtual DMA-channels. Pre-initializing it with some value causes
> > having a wrong number of channels printed in the device summary.
> > 
> > Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Tested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Acked-by: Vinod Koul <vkoul@kernel.org>
> > ---
> >  drivers/dma/dw-edma/dw-edma-core.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > index 6c9f95a8e397..ecd3e8f7ac5d 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -817,7 +817,6 @@ static int dw_edma_channel_setup(struct dw_edma *dw, bool write,
> >  	dma->src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> >  	dma->dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> >  	dma->residue_granularity = DMA_RESIDUE_GRANULARITY_DESCRIPTOR;
> > -	dma->chancnt = cnt;
> 

> Did you look for other instances of this bug?  There are several other
> places that *look* like they might have the same issue:
> 
>   axi_dmac_probe
>   dw_probe               (dw-axi-dmac-platform.c)
>   ioat_pci_probe
>   plx_dma_create
>   ...
> 
> I hate to fix just one if there are other similar issues.

Indeed. The same problem can be spotted in:
drivers/dma/dma-axi-dmac.c
drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
drivers/dma/ioat/init.c
drivers/dma/plx_dma.c
drivers/dma/qcom/hidma.c
drivers/dma/sprd-dma.c
drivers/rapidio/devices/tsi721_dma.c

Moreover besides causing a wrong number of channels printed in the
device summary the problem may also lead to the dma_ida IDs leak. So
it's a bit more complex than I described in the patch log.

@Vinod you didn't say anything about that during the patch review. Do
you want me to provide a similar fix for the denoted drivers?

-Serge(y)

> 
> >  	/* Set DMA channel callbacks */
> >  	dma->dev = chip->dev;
> > -- 
> > 2.39.0
> > 
> >