From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AD93A26F476; Thu, 14 May 2026 00:38:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778719126; cv=none; b=CWc7R5HpIrm4wy5dUoO3urzFP8tSOy+ArjNm9cvP5NsYm493xabtmnZ4OymUkIy/vjD1IxoPuq236E8q83QY8yjcT8kJF5I/T1vgsSx1t90xRJdV8eKqdNOID2DI4Et/YW17lGF6bKaJAk2XUCyTXxkWK9+jsc621nOFdL3dEEA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778719126; c=relaxed/simple; bh=0pm2so/gT8FBwtcqO3Wc625VGbJi1uk65jlu5tB71k0=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=cHEkIaBJfLmrnF3LjCMuBgCTiY+++C212h52jA/HeLCjnhAPCEHcGpxjrUvaYlCjQikD8xIznv8zvv+2sqLVTyN2AYiWMFexQlz2lLfJvj5HLO7xRNFKGFaogNudFwG1tIEUB3u7eaLpHDbuWcXlh9RD6e8K8UJppctBmfBABFc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=dmUVjk0V; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="dmUVjk0V" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0CBB2C19425; Thu, 14 May 2026 00:38:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1778719126; bh=0pm2so/gT8FBwtcqO3Wc625VGbJi1uk65jlu5tB71k0=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From; b=dmUVjk0V6c3cIOQmVYOme1LOWxhrUKp5FhM1cWIovvW2z2o9QnlNrLHCsJeZf1J6N IRjjW16wKRp0F/+3PtCDEYkQa4llwobDFZZs0PIwGD8YgkexjZ9EXFtKIaRojr/CC8 ITZgYUaTJxG6+HSFfGabOSsp92U4ewqvZiljTSUjDroYd6c2+Azu6VbLvKesBNEQRM 5LakxrBYShlmh65Kx2zCosafj4AV8DlV0FjQBniRRvNEtrWHGIxLfJZqyEvMfsEeBJ uORTBsbuOrq7kGogBxd/AbTwQS/5e+a1Tu8ypbxIjueshqb1Mir4jAyeIgSgHjS1zx mwM1J/VaUjTtQ== From: sashiko-bot@kernel.org Subject: Re: [PATCH v5 5/9] dmaengine: dw-edma: Pass dma_slave_config to dw_edma_device_transfer() Reply-To: sashiko-reviews@lists.linux.dev To: "Frank Li" Cc: vkoul@kernel.org, imx@lists.linux.dev, dmaengine@vger.kernel.org, linux-pci@vger.kernel.org, Frank.Li@kernel.org In-Reply-To: <20260512-dma_prep_config-v5-5-26865bf7d935@nxp.com> References: <20260512-dma_prep_config-v5-5-26865bf7d935@nxp.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Thu, 14 May 2026 00:38:45 +0000 Message-Id: <20260514003846.0CBB2C19425@smtp.kernel.org> Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Thank you for your contribution! Sashiko AI review found 2 potential issue(= s) to consider: - [High] The `dw_edma_device_prep_config_sg` function ignores the return va= lue of `dw_edma_device_config` and proceeds to use an explicitly rejected c= onfiguration for DMA transfers. - [High] The implementation fails to achieve the claimed "atomic configurat= ion" because it locklessly mutates shared channel state, corrupting concurr= ently active DMA transfers. -- commit 8f2944930122fd18a6a04f03176bb9c6eddcd9ba Author: Frank Li dmaengine: dw-edma: Pass dma_slave_config to dw_edma_device_transfer() This commit passes dma_slave_config to dw_edma_device_transfer() to support atomic configuration and descriptor preparation when a non-NULL configuration is provided to device_prep_config_sg(). > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-= edma-core.c > index f7f58b0010e26..ec6f6b1e48256 100644 > --- a/drivers/dma/dw-edma/dw-edma-core.c > +++ b/drivers/dma/dw-edma/dw-edma-core.c [ ... ] > @@ -595,7 +610,7 @@ dw_edma_device_prep_config_sg(struct dma_chan *dchan,= struct scatterlist *sgl, > if (config) > dw_edma_device_config(dchan, config); Should we check the return value of dw_edma_device_config() here? If an invalid configuration is passed, dw_edma_device_config() returns -EINVAL. By ignoring the return value, the code proceeds to extract the source and destination addresses from the explicitly rejected configuration via dw_edma_device_get_config(), potentially resulting in descriptors prepared with unvalidated hardware parameters. Additionally, the commit message states this change supports atomic configuration, but dw_edma_device_config() mutates shared channel state: dw_edma_device_config() { ... if (cfg_non_ll || non_ll) chan->non_ll =3D true; ... memcpy(&chan->config, config, sizeof(*config)); } Since the dmaengine API allows concurrent descriptor preparations when device_prep_config_sg is provided, could this cause a data race on chan->config and chan->non_ll? If chan->non_ll is actively read by the interrupt handler to dictate hardware execution mode, could modifying it while preparing a new descriptor dynamically corrupt the execution mode of currently active transfers? > - return dw_edma_device_transfer(&xfer); > + return dw_edma_device_transfer(&xfer, dw_edma_device_get_config(dchan, = config)); > } --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260512-dma_prep_c= onfig-v5-0-26865bf7d935@nxp.com?part=3D5