From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934959AbcKWKQ2 (ORCPT ); Wed, 23 Nov 2016 05:16:28 -0500 Received: from mout.kundenserver.de ([217.72.192.73]:56131 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933417AbcKWKQU (ORCPT ); Wed, 23 Nov 2016 05:16:20 -0500 From: Arnd Bergmann To: Jisheng Zhang Cc: linux-arm-kernel@lists.infradead.org, Thomas Petazzoni , Andrew Lunn , Jason Cooper , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Gregory CLEMENT , Marcin Wojtas , "David S. Miller" , Sebastian Hesselbarth Subject: Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible Date: Wed, 23 Nov 2016 11:15:32 +0100 Message-ID: <9432400.S1OrxC027t@wuerfel> User-Agent: KMail/5.1.3 (Linux/4.4.0-34-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: <20161123175341.4777595f@xhacker> References: <20161122164844.19566-1-gregory.clement@free-electrons.com> <2948812.F3se4ieqO6@wuerfel> <20161123175341.4777595f@xhacker> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:oQce9c/u9C7V2UDhYBeXuEbR/ATs3km+KNl5N9uEYXuVFwYte1x tgeDZGaZatvuaGIAPEjg7HzrUnfGgMOsnRAh/rE+HS+fqQydX4Tax8dUEOhPHFfzkjEWX6E givo6mACsa/Gu/vu6ao4x1zhfGej+DFhXbhKhfHUe+pqrq63COmnmX2J0YJ/G2BMweW71p5 FQXwNlqiP8o4irxAKL2AA== X-UI-Out-Filterresults: notjunk:1;V01:K0:Po4nlkfxRbA=:LZxq/iqv+pcoCmk/9kAq/j fhaln6za+F690iPQahfsvv9+6iRjB3zDyCD/AxthZzo6S4fqVObhLb3HlSWgTLJJIFqoSMeWF iRyaNphjBURyIVkR8I6oCfZevp7N7CKya6z6il24YrEIVxb6jTKjhzl41Qn1EMF2MsRBbOBEM hr8mRYFaIa0HUPQrQPq+G9YC4PldC8zK5UADMTYQ2tu3044ZwDA9eObbmBqkRoJtaSknEj7oz gHdxabVlpCrrgdWwHkd92gizsneBtgNW2JBES3Y/6W2loDUanYoBv1KX/JXsM9Y+Iaj2SWKtg xCZYp1XQS2S3JDj+/jro0wz+l5FLot7b4KdF4FIHsr2bcV3yieKRKxv7SO4SGnBDJSRuQuHKO zIiW++Rn0hkcdWcyMnWJp8Li7nrvA3PsjCufgI2m8RvoAGra309Skef9wqA7BKxiGgTmzT+xZ GkuGeMY728MO12PeqNuiK3EqmRCYK+DsM2Ypz0h53mjBwz9tsqsaVhM2SpfjwAVUOP0g4/pkT +Ykx+MqWRdJOBwD4Py3JDxkgYQaLQeXURR/KryVstiZDu7ps3vvHewwgecG3Mi0pyALjjWaQu xh9ILPp9foRNzj94fXSM2ec/LsVgflIQJAtTan9yoy8dEbFoML2tyndOhPyLXOjpE6wEOEEEF j8cDlNTUufbJBHhAy7uPFTAML2DY2MfItYRlRHmDGq1qZVHrycCw1tuaSwYHPG5POxnBQO9Qk UpaxKv21ALXjuHJL Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday, November 23, 2016 5:53:41 PM CET Jisheng Zhang wrote: > On Tue, 22 Nov 2016 22:04:12 +0100 Arnd Bergmann wrote: > > > On Tuesday, November 22, 2016 5:48:41 PM CET Gregory CLEMENT wrote: > > > +#ifdef CONFIG_64BIT > > > + void *data_tmp; > > > + > > > + /* In Neta HW only 32 bits data is supported, so in order to > > > + * obtain whole 64 bits address from RX descriptor, we store > > > + * the upper 32 bits when allocating buffer, and put it back > > > + * when using buffer cookie for accessing packet in memory. > > > + * Frags should be allocated from single 'memory' region, > > > + * hence common upper address half should be sufficient. > > > + */ > > > + data_tmp = mvneta_frag_alloc(pp->frag_size); > > > + if (data_tmp) { > > > + pp->data_high = (u64)upper_32_bits((u64)data_tmp) << 32; > > > + mvneta_frag_free(pp->frag_size, data_tmp); > > > + } > > > > > > > How does this work when the region spans a n*4GB address boundary? > > indeed. We also make use of this driver on 64bit platforms. We use > different solution to make the driver 64bit safe. > > solA: make use of the reserved field in the mvneta_rx_desc, such > as reserved2 etc. Yes, the field is marked as "for future use, PnC", but > now it's not used at all. This is one possible solution however. Right, this sounds like the most straightforward choice. > solB: allocate a shadow buf cookie during init, e.g > > rxq->descs_bufcookie = kmalloc(rxq->size * sizeof(void*), GFP_KERNEL); > > then modify mvneta_rx_desc_fill a bit to save the 64bit pointer in > the shadow buf cookie, e.g > static void mvneta_rx_desc_fill(struct mvneta_rx_desc *rx_desc, > u32 phys_addr, u32 cookie, > struct mvneta_rx_queue *rxq) > > { > int i; > > rx_desc->buf_cookie = cookie; > rx_desc->buf_phys_addr = phys_addr; > i = rx_desc - rxq->descs; > rxq->descs_bufcookie[i] = cookie; > } > > then fetch the desc from the shadow buf cookie in all code path, such > as mvneta_rx() etc. > > Both solutions should not have the problems pointed out by Arnd. Wait, since you compute an index 'i' here, can't you just store 'i' directly in the descriptor instead of the pointer? Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 From: arnd@arndb.de (Arnd Bergmann) Date: Wed, 23 Nov 2016 11:15:32 +0100 Subject: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible In-Reply-To: <20161123175341.4777595f@xhacker> References: <20161122164844.19566-1-gregory.clement@free-electrons.com> <2948812.F3se4ieqO6@wuerfel> <20161123175341.4777595f@xhacker> Message-ID: <9432400.S1OrxC027t@wuerfel> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wednesday, November 23, 2016 5:53:41 PM CET Jisheng Zhang wrote: > On Tue, 22 Nov 2016 22:04:12 +0100 Arnd Bergmann wrote: > > > On Tuesday, November 22, 2016 5:48:41 PM CET Gregory CLEMENT wrote: > > > +#ifdef CONFIG_64BIT > > > + void *data_tmp; > > > + > > > + /* In Neta HW only 32 bits data is supported, so in order to > > > + * obtain whole 64 bits address from RX descriptor, we store > > > + * the upper 32 bits when allocating buffer, and put it back > > > + * when using buffer cookie for accessing packet in memory. > > > + * Frags should be allocated from single 'memory' region, > > > + * hence common upper address half should be sufficient. > > > + */ > > > + data_tmp = mvneta_frag_alloc(pp->frag_size); > > > + if (data_tmp) { > > > + pp->data_high = (u64)upper_32_bits((u64)data_tmp) << 32; > > > + mvneta_frag_free(pp->frag_size, data_tmp); > > > + } > > > > > > > How does this work when the region spans a n*4GB address boundary? > > indeed. We also make use of this driver on 64bit platforms. We use > different solution to make the driver 64bit safe. > > solA: make use of the reserved field in the mvneta_rx_desc, such > as reserved2 etc. Yes, the field is marked as "for future use, PnC", but > now it's not used at all. This is one possible solution however. Right, this sounds like the most straightforward choice. > solB: allocate a shadow buf cookie during init, e.g > > rxq->descs_bufcookie = kmalloc(rxq->size * sizeof(void*), GFP_KERNEL); > > then modify mvneta_rx_desc_fill a bit to save the 64bit pointer in > the shadow buf cookie, e.g > static void mvneta_rx_desc_fill(struct mvneta_rx_desc *rx_desc, > u32 phys_addr, u32 cookie, > struct mvneta_rx_queue *rxq) > > { > int i; > > rx_desc->buf_cookie = cookie; > rx_desc->buf_phys_addr = phys_addr; > i = rx_desc - rxq->descs; > rxq->descs_bufcookie[i] = cookie; > } > > then fetch the desc from the shadow buf cookie in all code path, such > as mvneta_rx() etc. > > Both solutions should not have the problems pointed out by Arnd. Wait, since you compute an index 'i' here, can't you just store 'i' directly in the descriptor instead of the pointer? Arnd