From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0E720D15D89 for ; Mon, 21 Oct 2024 12:44:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=0pzpyDGRpt+sC1iXvAhak25Z94QVf6C5wA+hEogYXLI=; b=lE1H5HoEzDkPJlQqNg85DIH7Ym Sjfp2JLjUCy05urZTR/fR03sMShWU+qGZbnkgkOpWMOPPYmIikZcEvn/atb4fbVwKygyjiIwjMBl8 23wNuAJSaQJXPVfO5Lz0tvFSrA8+bfrU/0UN3LywEoqRQMF3fBEl9+fwuw2c7EjiSm6Ds1m+T3eAi 7BtjvabkdQt+pw92MxlyU88slM888fNEaAu4+2TQ8ktZOIMUt2iCuNMFxKSeE91s3lJFoOeV8S/44 FdKWo82Dnjo+TVbTH6u4OSaqubNV0E8CIwLoWAj6FryXRKKEoqOtPDYK/kwfVZh92o+NikwPh8df+ hydrDllQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1t2rlJ-00000007IwS-3Prt; Mon, 21 Oct 2024 12:43:53 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1t2rU6-00000007Ef0-3Zqs for linux-arm-kernel@lists.infradead.org; Mon, 21 Oct 2024 12:26:08 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id C2AE85C5561; Mon, 21 Oct 2024 12:26:01 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 15F5EC4CEC3; Mon, 21 Oct 2024 12:26:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1729513565; bh=cx/tLvH9CPRgHxEav9sPf4o/gHFa4lb5PR3fe7KBDA0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=HlF6z4iSE4m4KO6vx5hubfGjBlUBCwj1PXsxIH+/Xcop+vfX86LaDkpRtod2cq+MH xH+iCQ7icStQOSTZEWdc7fwJ2/vdFJWRYkY8zGc56jciyDi9wjqf3ULXVX0bpDynPR TABkMjgXNxkDAXPztzvpPj4Y0rCfErFGxe9c9p5bryS3zWbd8G4u9ot6FCw+FIbdfv Pi3m1RlJ4bGU9Z8TSdjIx1fidU6H8IECafN4z0VFaBT+I+dzNoeq1cC5K/+tD2XSYa CY3zwTqSII+OWs2FChK3B0e9JOcWROv1/xtstboToEqnH7FR+5JCEpEFWZpZ1FpbJG deqz1OZtUifHg== Date: Mon, 21 Oct 2024 13:26:01 +0100 From: Simon Horman To: Furong Xu <0x1207@gmail.com> Cc: netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Alexandre Torgue , Jose Abreu , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Maxime Coquelin , xfr@outlook.com, Suraj Jaiswal Subject: Re: [PATCH net v1] net: stmmac: TSO: Fix unbalanced DMA map/unmap for non-paged SKB data Message-ID: <20241021122601.GI402847@kernel.org> References: <20241021061023.2162701-1-0x1207@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20241021061023.2162701-1-0x1207@gmail.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241021_052607_004609_44929E93 X-CRM114-Status: GOOD ( 23.14 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Oct 21, 2024 at 02:10:23PM +0800, Furong Xu wrote: > In case the non-paged data of a SKB carries protocol header and protocol > payload to be transmitted on a certain platform that the DMA AXI address > width is configured to 40-bit/48-bit, or the size of the non-paged data > is bigger than TSO_MAX_BUFF_SIZE on a certain platform that the DMA AXI > address width is configured to 32-bit, then this SKB requires at least > two DMA transmit descriptors to serve it. > > For example, three descriptors are allocated to split one DMA buffer > mapped from one piece of non-paged data: > dma_desc[N + 0], > dma_desc[N + 1], > dma_desc[N + 2]. > Then three elements of tx_q->tx_skbuff_dma[] will be allocated to hold > extra information to be reused in stmmac_tx_clean(): > tx_q->tx_skbuff_dma[N + 0], > tx_q->tx_skbuff_dma[N + 1], > tx_q->tx_skbuff_dma[N + 2]. > Now we focus on tx_q->tx_skbuff_dma[entry].buf, which is the DMA buffer > address returned by DMA mapping call. stmmac_tx_clean() will try to > unmap the DMA buffer _ONLY_IF_ tx_q->tx_skbuff_dma[entry].buf > is a valid buffer address. > > The expected behavior that saves DMA buffer address of this non-paged > data to tx_q->tx_skbuff_dma[entry].buf is: > tx_q->tx_skbuff_dma[N + 0].buf = NULL; > tx_q->tx_skbuff_dma[N + 1].buf = NULL; > tx_q->tx_skbuff_dma[N + 2].buf = dma_map_single(); > Unfortunately, the current code misbehaves like this: > tx_q->tx_skbuff_dma[N + 0].buf = dma_map_single(); > tx_q->tx_skbuff_dma[N + 1].buf = NULL; > tx_q->tx_skbuff_dma[N + 2].buf = NULL; > > On the stmmac_tx_clean() side, when dma_desc[N + 0] is closed by the > DMA engine, tx_q->tx_skbuff_dma[N + 0].buf is a valid buffer address > obviously, then the DMA buffer will be unmapped immediately. > There may be a rare case that the DMA engine does not finish the > pending dma_desc[N + 1], dma_desc[N + 2] yet. Now things will go > horribly wrong, DMA is going to access a unmapped/unreferenced memory > region, corrupted data will be transmited or iommu fault will be > triggered :( > > In contrast, the for-loop that maps SKB fragments behaves perfectly > as expected, and that is how the driver should do for both non-paged > data and paged frags actually. > > This patch corrects DMA map/unmap sequences by fixing the array index > for tx_q->tx_skbuff_dma[entry].buf when assigning DMA buffer address. > > Tested and verified on DWXGMAC CORE 3.20a > > Reported-by: Suraj Jaiswal > Fixes: f748be531d70 ("stmmac: support new GMAC4") > Signed-off-by: Furong Xu <0x1207@gmail.com> Thanks for the very thorough explanation, much appreciated. Reviewed-by: Simon Horman