From: Arnd Bergmann <arnd@arndb.de> To: Jisheng Zhang <jszhang@marvell.com> Cc: linux-arm-kernel@lists.infradead.org, Thomas Petazzoni <thomas.petazzoni@free-electrons.com>, Andrew Lunn <andrew@lunn.ch>, Jason Cooper <jason@lakedaemon.net>, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Gregory CLEMENT <gregory.clement@free-electrons.com>, Marcin Wojtas <mw@semihalf.com>, "David S. Miller" <davem@davemloft.net>, Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> Subject: Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible Date: Wed, 23 Nov 2016 11:15:32 +0100 [thread overview] Message-ID: <9432400.S1OrxC027t@wuerfel> (raw) In-Reply-To: <20161123175341.4777595f@xhacker> 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
WARNING: multiple messages have this Message-ID (diff)
From: arnd@arndb.de (Arnd Bergmann) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible Date: Wed, 23 Nov 2016 11:15:32 +0100 [thread overview] Message-ID: <9432400.S1OrxC027t@wuerfel> (raw) In-Reply-To: <20161123175341.4777595f@xhacker> 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
next prev parent reply other threads:[~2016-11-23 10:16 UTC|newest] Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-11-22 16:48 [PATCH net-next 0/4] Extend mvneta to support Armada 3700 (ARM 64) Gregory CLEMENT 2016-11-22 16:48 ` Gregory CLEMENT 2016-11-22 16:48 ` Gregory CLEMENT 2016-11-22 16:48 ` [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible Gregory CLEMENT 2016-11-22 16:48 ` Gregory CLEMENT 2016-11-22 21:04 ` Arnd Bergmann 2016-11-22 21:04 ` Arnd Bergmann 2016-11-23 9:53 ` Jisheng Zhang 2016-11-23 9:53 ` Jisheng Zhang 2016-11-23 9:53 ` Jisheng Zhang 2016-11-23 10:15 ` Arnd Bergmann [this message] 2016-11-23 10:15 ` Arnd Bergmann 2016-11-23 11:03 ` Jisheng Zhang 2016-11-23 11:03 ` Jisheng Zhang 2016-11-23 13:07 ` Gregory CLEMENT 2016-11-23 13:07 ` Gregory CLEMENT 2016-11-23 16:02 ` Marcin Wojtas 2016-11-23 16:02 ` Marcin Wojtas 2016-11-23 16:02 ` Marcin Wojtas 2016-11-24 8:37 ` Jisheng Zhang 2016-11-24 8:37 ` Jisheng Zhang 2016-11-24 8:37 ` Jisheng Zhang 2016-11-24 9:00 ` Arnd Bergmann 2016-11-24 9:00 ` Arnd Bergmann 2016-11-24 9:11 ` Jisheng Zhang 2016-11-24 9:11 ` Jisheng Zhang 2016-11-24 15:01 ` Gregory CLEMENT 2016-11-24 15:01 ` Gregory CLEMENT 2016-11-24 15:09 ` Marcin Wojtas 2016-11-24 15:09 ` Marcin Wojtas 2016-11-24 15:09 ` Marcin Wojtas 2016-11-24 19:04 ` Florian Fainelli 2016-11-24 19:04 ` Florian Fainelli 2016-11-22 16:48 ` [PATCH net-next 2/4] net: mvneta: Only disable mvneta_bm for 64-bits Gregory CLEMENT 2016-11-22 16:48 ` Gregory CLEMENT 2016-11-22 16:48 ` Gregory CLEMENT 2016-11-22 16:48 ` [PATCH net-next 3/4] net: mvneta: Add network support for Armada 3700 SoC Gregory CLEMENT 2016-11-22 16:48 ` Gregory CLEMENT 2016-11-22 16:48 ` [PATCH net-next 4/4] ARM64: dts: marvell: Add network support for Armada 3700 Gregory CLEMENT 2016-11-22 16:48 ` Gregory CLEMENT 2016-11-22 16:48 ` Gregory CLEMENT
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=9432400.S1OrxC027t@wuerfel \ --to=arnd@arndb.de \ --cc=andrew@lunn.ch \ --cc=davem@davemloft.net \ --cc=gregory.clement@free-electrons.com \ --cc=jason@lakedaemon.net \ --cc=jszhang@marvell.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mw@semihalf.com \ --cc=netdev@vger.kernel.org \ --cc=sebastian.hesselbarth@gmail.com \ --cc=thomas.petazzoni@free-electrons.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.