Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932646AbcLAMWE (ORCPT ); Thu, 1 Dec 2016 07:22:04 -0500 Received: from mx0a-0016f401.pphosted.com ([67.231.148.174]:37395 "EHLO mx0b-0016f401.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932506AbcLAMWC (ORCPT ); Thu, 1 Dec 2016 07:22:02 -0500 Date: Thu, 1 Dec 2016 20:16:48 +0800 From: Jisheng Zhang To: Marcin Wojtas CC: Thomas Petazzoni , Andrew Lunn , Jason Cooper , Arnd Bergmann , , , Dmitri Epshtein , Nadav Haklai , "Yelena Krivosheev" , Gregory CLEMENT , "David S. Miller" , "linux-arm-kernel@lists.infradead.org" , Sebastian Hesselbarth Subject: Re: [PATCH v5 net-next 4/7] net: mvneta: Convert to be 64 bits compatible Message-ID: <20161201201648.6a50b442@xhacker> In-Reply-To: <20161201200205.46dec339@xhacker> References: <6b4d20a85b05e93d254e265eb07569dd1285d303.1480542157.git-series.gregory.clement@free-electrons.com> <20161201192604.07ed9516@xhacker> <20161201200205.46dec339@xhacker> X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-12-01_07:,, signatures=0 X-Proofpoint-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=2 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1609300000 definitions=main-1612010216 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5670 Lines: 154 On Thu, 1 Dec 2016 20:02:05 +0800 Jisheng Zhang wrote: > Hi Marcin, > > On Thu, 1 Dec 2016 12:48:39 +0100 Marcin Wojtas wrote: > > > Hi Jisheng, > > > > Which baseline do you use? > > > > It took me really lot of time to catch why RX broke after rebase from > > LKv4.1 to LKv4.4. Between those two, in commit: > > 97303480753e ("arm64: Increase the max granular size") > > L1_CACHE_BYTES for all ARMv8 platforms was increased to 128B and so > > did NET_SKB_PAD. > > > > And 128 is more than the maximum that can fit into packet offset > > [11:8]@0x1400. In such case this correction is needed. Did it answer > > your doubts? > > That's key! Thanks a lot. In my repo, we don't have commit 97303480753e > ("arm64: Increase the max granular size") > > I think it would be great if this information can be added into the commit > msg. > > IIRC, arm64 maintainers considered to let L1_CACHE_BYTES the _minimum_ of > cache line sizes of arm64. If that's implemented and merged, then we can I just searched and found the email. "We may have to revisit this logic and consider L1_CACHE_BYTES the _minimum_ of cache line sizes in arm64 systems supported by the kernel. Do you have any benchmarks on Cavium boards that would show significant degradation with 64-byte L1_CACHE_BYTES vs 128?" https://patchwork.kernel.org/patch/8634481/ > revert this patch later. > > Thanks, > Jisheng > > > > > Best regards, > > Marcin > > > > > > > > 2016-12-01 12:26 GMT+01:00 Jisheng Zhang : > > > Hi Gregory, Marcin, > > > > > > On Wed, 30 Nov 2016 22:42:49 +0100 Gregory CLEMENT wrote: > > > > > >> From: Marcin Wojtas > > >> > > >> Prepare the mvneta driver in order to be usable on the 64 bits platform > > >> such as the Armada 3700. > > >> > > >> [gregory.clement@free-electrons.com]: this patch was extract from a larger > > >> one to ease review and maintenance. > > >> > > >> Signed-off-by: Marcin Wojtas > > >> Signed-off-by: Gregory CLEMENT > > >> --- > > >> drivers/net/ethernet/marvell/mvneta.c | 17 ++++++++++++++++- > > >> 1 file changed, 16 insertions(+), 1 deletion(-) > > >> > > >> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c > > >> index 92b9af14c352..8ef03fb69bcd 100644 > > >> --- a/drivers/net/ethernet/marvell/mvneta.c > > >> +++ b/drivers/net/ethernet/marvell/mvneta.c > > >> @@ -296,6 +296,12 @@ > > >> /* descriptor aligned size */ > > >> #define MVNETA_DESC_ALIGNED_SIZE 32 > > >> > > >> +/* Number of bytes to be taken into account by HW when putting incoming data > > >> + * to the buffers. It is needed in case NET_SKB_PAD exceeds maximum packet > > >> + * offset supported in MVNETA_RXQ_CONFIG_REG(q) registers. > > > > > > We also brought up this driver on 64bit platforms, we doesn't have this > > > patch. Maybe I'm wrong, I'm trying to understand why we need this > > > modification. Let's assume the NET_SKB_PAD is 64B, we call > > > mvneta_rxq_offset_set(pp, rxq, 64), > > > > > > { > > > u32 val; > > > > > > val = mvreg_read(pp, MVNETA_RXQ_CONFIG_REG(rxq->id)); > > > val &= ~MVNETA_RXQ_PKT_OFFSET_ALL_MASK; > > > > > > /* Offset is in */ > > > val |= MVNETA_RXQ_PKT_OFFSET_MASK(offset >> 3); > > > // then this will be "val |= 8;" it doesn't exceeds the max offset of > > > MVNETA_RXQ_CONFIG_REG(q) register. > > > > > > Could you please kindly point out where I am wrong? > > > > > >> + */ > > >> +#define MVNETA_RX_PKT_OFFSET_CORRECTION 64 > > >> + > > >> #define MVNETA_RX_PKT_SIZE(mtu) \ > > >> ALIGN((mtu) + MVNETA_MH_SIZE + MVNETA_VLAN_TAG_LEN + \ > > >> ETH_HLEN + ETH_FCS_LEN, \ > > >> @@ -416,6 +422,7 @@ struct mvneta_port { > > >> u64 ethtool_stats[ARRAY_SIZE(mvneta_statistics)]; > > >> > > >> u32 indir[MVNETA_RSS_LU_TABLE_SIZE]; > > >> + u16 rx_offset_correction; > > >> }; > > >> > > >> /* The mvneta_tx_desc and mvneta_rx_desc structures describe the > > >> @@ -1807,6 +1814,7 @@ static int mvneta_rx_refill(struct mvneta_port *pp, > > >> return -ENOMEM; > > >> } > > >> > > >> + phys_addr += pp->rx_offset_correction; > > >> mvneta_rx_desc_fill(rx_desc, phys_addr, data, rxq); > > >> return 0; > > >> } > > >> @@ -2782,7 +2790,7 @@ static int mvneta_rxq_init(struct mvneta_port *pp, > > >> mvreg_write(pp, MVNETA_RXQ_SIZE_REG(rxq->id), rxq->size); > > >> > > >> /* Set Offset */ > > >> - mvneta_rxq_offset_set(pp, rxq, NET_SKB_PAD); > > >> + mvneta_rxq_offset_set(pp, rxq, NET_SKB_PAD - pp->rx_offset_correction); > > >> > > >> /* Set coalescing pkts and time */ > > >> mvneta_rx_pkts_coal_set(pp, rxq, rxq->pkts_coal); > > >> @@ -4033,6 +4041,13 @@ static int mvneta_probe(struct platform_device *pdev) > > >> > > >> pp->rxq_def = rxq_def; > > >> > > >> + /* Set RX packet offset correction for platforms, whose > > >> + * NET_SKB_PAD, exceeds 64B. It should be 64B for 64-bit > > >> + * platforms and 0B for 32-bit ones. > > > > > > Even we need this patch, I'm not sure this last comment is correct or not. > > > NET_SKB_PAD is defined as: > > > > > > #define NET_SKB_PAD max(32, L1_CACHE_BYTES) > > > > > > we have 64B cacheline 32bit platforms, on this platforms, the NET_SKB_PAD > > > should be 64B as well. > > > > > > Thanks, > > > Jisheng > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel