Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933906AbbHJKQS (ORCPT ); Mon, 10 Aug 2015 06:16:18 -0400 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:43919 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933833AbbHJKQF (ORCPT ); Mon, 10 Aug 2015 06:16:05 -0400 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit MIME-Version: 1.0 From: Ben Hutchings To: linux-kernel@vger.kernel.org, stable@vger.kernel.org CC: akpm@linux-foundation.org, "Alexey Brodkin" , "David Miller" , arc-linux-dev@synopsys.com, "Giuseppe Cavallaro" , "Alexey Brodkin" Date: Mon, 10 Aug 2015 12:12:31 +0200 Message-ID: X-Mailer: LinuxStableQueue (scripts by bwh) Subject: [PATCH 3.2 047/110] stmmac: troubleshoot unexpected bits in des0 & des1 In-Reply-To: X-SA-Exim-Connect-IP: 2.173.94.72 X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4497 Lines: 133 3.2.71-rc1 review patch. If anyone has any objections, please let me know. ------------------ From: Alexey Brodkin commit f1590670ce069eefeb93916391a67643e6ad1630 upstream. Current implementation of descriptor init procedure only takes care about setting/clearing ownership flag in "des0"/"des1" fields while it is perfectly possible to get unexpected bits set because of the following factors: [1] On driver probe underlying memory allocated with dma_alloc_coherent() might not be zeroed and so it will be filled with garbage. [2] During driver operation some bits could be set by SD/MMC controller (for example error flags etc). And unexpected and/or randomly set flags in "des0"/"des1" fields may lead to unpredictable behavior of GMAC DMA block. This change addresses both items above with: [1] Use of dma_zalloc_coherent() instead of simple dma_alloc_coherent() to make sure allocated memory is zeroed. That shouldn't affect performance because this allocation only happens once on driver probe. [2] Do explicit zeroing of both "des0" and "des1" fields of all buffer descriptors during initialization of DMA transfer. And while at it fixed identation of dma_free_coherent() counterpart as well. Signed-off-by: Alexey Brodkin Cc: Giuseppe Cavallaro Cc: arc-linux-dev@synopsys.com Cc: linux-kernel@vger.kernel.org Cc: David Miller Signed-off-by: David S. Miller [bwh: Backported to 3.2: - Adjust context, indentation - Normal and extended descriptors are allocated in the same place here] Signed-off-by: Ben Hutchings --- --- a/drivers/net/ethernet/stmicro/stmmac/descs.h +++ b/drivers/net/ethernet/stmicro/stmmac/descs.h @@ -153,6 +153,8 @@ struct dma_desc { u32 buffer2_size:13; u32 reserved4:3; } etx; /* -- enhanced -- */ + + u64 all_flags; } des01; unsigned int des2; unsigned int des3; --- a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c +++ b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c @@ -232,6 +232,7 @@ static void enh_desc_init_rx_desc(struct { int i; for (i = 0; i < ring_size; i++) { + p->des01.all_flags = 0; p->des01.erx.own = 1; p->des01.erx.buffer1_size = BUF_SIZE_8KiB - 1; @@ -248,7 +249,7 @@ static void enh_desc_init_tx_desc(struct int i; for (i = 0; i < ring_size; i++) { - p->des01.etx.own = 0; + p->des01.all_flags = 0; ehn_desc_tx_set_on_ring_chain(p, (i == ring_size - 1)); p++; } --- a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c +++ b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c @@ -126,6 +126,7 @@ static void ndesc_init_rx_desc(struct dm { int i; for (i = 0; i < ring_size; i++) { + p->des01.all_flags = 0; p->des01.rx.own = 1; p->des01.rx.buffer1_size = BUF_SIZE_2KiB - 1; @@ -141,7 +142,7 @@ static void ndesc_init_tx_desc(struct dm { int i; for (i = 0; i < ring_size; i++) { - p->des01.tx.own = 0; + p->des01.all_flags = 0; ndesc_tx_set_on_ring_chain(p, (i == (ring_size - 1))); p++; } --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -441,19 +441,17 @@ static void init_dma_desc_rings(struct n priv->rx_skbuff = kmalloc(sizeof(struct sk_buff *) * rxsize, GFP_KERNEL); priv->dma_rx = - (struct dma_desc *)dma_alloc_coherent(priv->device, - rxsize * - sizeof(struct dma_desc), - &priv->dma_rx_phy, - GFP_KERNEL); + (struct dma_desc *)dma_zalloc_coherent(priv->device, rxsize * + sizeof(struct dma_desc), + &priv->dma_rx_phy, + GFP_KERNEL); priv->tx_skbuff = kmalloc(sizeof(struct sk_buff *) * txsize, GFP_KERNEL); priv->dma_tx = - (struct dma_desc *)dma_alloc_coherent(priv->device, - txsize * - sizeof(struct dma_desc), - &priv->dma_tx_phy, - GFP_KERNEL); + (struct dma_desc *)dma_zalloc_coherent(priv->device, txsize * + sizeof(struct dma_desc), + &priv->dma_tx_phy, + GFP_KERNEL); if ((priv->dma_rx == NULL) || (priv->dma_tx == NULL)) { pr_err("%s:ERROR allocating the DMA Tx/Rx desc\n", __func__); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/