| Message ID | 20161115150800.GB11283@hmsreliant.think-freely.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 5C3F8377A; Tue, 15 Nov 2016 16:08:17 +0100 (CET) Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 7C1C2234; Tue, 15 Nov 2016 16:08:15 +0100 (CET) Received: from cpe-2606-a000-111b-40ed-7aac-c0ff-fec2-933b.dyn6.twc.com ([2606:a000:111b:40ed:7aac:c0ff:fec2:933b] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from <nhorman@tuxdriver.com>) id 1c6fLF-0007ho-0c; Tue, 15 Nov 2016 10:08:11 -0500 Date: Tue, 15 Nov 2016 10:08:00 -0500 From: Neil Horman <nhorman@tuxdriver.com> To: Hemant Agrawal <hemant.agrawal@nxp.com> Cc: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "dev@dpdk.org" <dev@dpdk.org>, "users@dpdk.org" <users@dpdk.org>, "Jacob, Jerin" <Jerin.Jacob@cavium.com>, Jan Viktorin <viktorin@rehivetech.com> Message-ID: <20161115150800.GB11283@hmsreliant.think-freely.org> References: <DB5PR04MB1605F64CF0D986E0B2B26AF089BB0@DB5PR04MB1605.eurprd04.prod.outlook.com> <20161113205924.GA11392@localhost.localdomain> <20161114144839.GA15570@hmsreliant.think-freely.org> <DB5PR04MB1605203F2297ED88A47270F189BF0@DB5PR04MB1605.eurprd04.prod.outlook.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <DB5PR04MB1605203F2297ED88A47270F189BF0@DB5PR04MB1605.eurprd04.prod.outlook.com> User-Agent: Mutt/1.7.1 (2016-10-04) X-Spam-Score: -2.9 (--) X-Spam-Status: No Subject: Re: [dpdk-dev] pmdinfogen issues: cross compilation for ARM fails with older host compiler X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Neil Horman
Nov. 15, 2016, 3:08 p.m. UTC
On Tue, Nov 15, 2016 at 09:34:16AM +0000, Hemant Agrawal wrote: > > On Mon, Nov 14, 2016 at 02:29:24AM +0530, Jerin Jacob wrote: > > > On Fri, Nov 11, 2016 at 10:34:39AM +0000, Hemant Agrawal wrote: > > > > Hi Neil, > > > > Pmdinfogen compiles with host compiler. It usages rte_byteorder.h > > of the target platform. > > > > However, if the host compiler is older than 4.8, it will be an issue during cross > > compilation for some platforms. > > > > e.g. if we are compiling on x86 host for ARM, x86 host compiler will not > > understand the arm asm instructions. > > > > > > > > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if > > > > !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) static > > > > inline uint16_t rte_arch_bswap16(uint16_t _x) { > > > > register uint16_t x = _x; > > > > asm volatile ("rev16 %0,%1" > > > > : "=r" (x) > > > > : "r" (x) > > > > ); > > > > return x; > > > > } > > > > #endif > > > > > > > > One easy solution is that we add compiler platform check in this > > > > code section of rte_byteorder.h e.g #if !(defined __arm__ || defined > > > > __aarch64__) static inline uint16_t rte_arch_bswap16(uint16_t _x) { > > > > return (_x >> 8) | ((_x << 8) & 0xff00); } #else …. > > > > > > > > Is there a better way to fix it? > > > > > > IMO, It is a HOST build infrastructure issue. If a host app is using > > > the dpdk service then it should compile and link against HOST > > > target(in this specific case, build/x86_64-native-linuxapp-gcc). I > > > think, introducing the HOSTTARGET kind of scheme is a clean solution. > > > > > > /Jerin > > > > > > > > That would be accurate. That is to say, pmdinfogen is a tool that should only be > > run on the host doing the build, by the host doing the build, and so should be > > compiled to run on the host, not on the target being built for. > > > > Yeah, so what we need is a way to get to the host version of rte_byteorder.h > > when building in a cross environment > > > +1 > > > Neil > Give this a try, I've tested it on linux, but not BSD. From what I read the functions are not posix compliant, though they should exist on all BSD and Linux systems in recent history. There may be some fiddling needed for Net and OpenBSD variants, but I think this is the right general direction.
Comments
> -----Original Message----- > From: Neil Horman [mailto:nhorman@tuxdriver.com] > On Tue, Nov 15, 2016 at 09:34:16AM +0000, Hemant Agrawal wrote: > > > On Mon, Nov 14, 2016 at 02:29:24AM +0530, Jerin Jacob wrote: > > > > On Fri, Nov 11, 2016 at 10:34:39AM +0000, Hemant Agrawal wrote: > > > > > Hi Neil, > > > > > Pmdinfogen compiles with host compiler. It usages > rte_byteorder.h > > > of the target platform. > > > > > However, if the host compiler is older than 4.8, it will be an issue during > cross > > > compilation for some platforms. > > > > > e.g. if we are compiling on x86 host for ARM, x86 host compiler will not > > > understand the arm asm instructions. > > > > > > > > > > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if > > > > > !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) static > > > > > inline uint16_t rte_arch_bswap16(uint16_t _x) { > > > > > register uint16_t x = _x; > > > > > asm volatile ("rev16 %0,%1" > > > > > : "=r" (x) > > > > > : "r" (x) > > > > > ); > > > > > return x; > > > > > } > > > > > #endif > > > > > > > > > > One easy solution is that we add compiler platform check in this > > > > > code section of rte_byteorder.h e.g #if !(defined __arm__ || defined > > > > > __aarch64__) static inline uint16_t rte_arch_bswap16(uint16_t _x) { > > > > > return (_x >> 8) | ((_x << 8) & 0xff00); } #else …. > > > > > > > > > > Is there a better way to fix it? > > > > > > > > IMO, It is a HOST build infrastructure issue. If a host app is using > > > > the dpdk service then it should compile and link against HOST > > > > target(in this specific case, build/x86_64-native-linuxapp-gcc). I > > > > think, introducing the HOSTTARGET kind of scheme is a clean solution. > > > > > > > > /Jerin > > > > > > > > > > > That would be accurate. That is to say, pmdinfogen is a tool that should only > be > > > run on the host doing the build, by the host doing the build, and so should be > > > compiled to run on the host, not on the target being built for. > > > > > > Yeah, so what we need is a way to get to the host version of rte_byteorder.h > > > when building in a cross environment > > > > > +1 > > > > > Neil > > > > Give this a try, I've tested it on linux, but not BSD. From what I read the > functions are not posix compliant, though they should exist on all BSD and Linux > systems in recent history. There may be some fiddling needed for Net and > OpenBSD variants, but I think this is the right general direction. + 1 This patch works good for Linux. > > > diff --git a/buildtools/pmdinfogen/pmdinfogen.h > b/buildtools/pmdinfogen/pmdinfogen.h > index 1da2966..c5ef89d 100644 > --- a/buildtools/pmdinfogen/pmdinfogen.h > +++ b/buildtools/pmdinfogen/pmdinfogen.h > @@ -21,7 +21,6 @@ > #include <elf.h> > #include <rte_config.h> > #include <rte_pci.h> > -#include <rte_byteorder.h> > > /* On BSD-alike OSes elf.h defines these according to host's word size */ > #undef ELF_ST_BIND > @@ -75,9 +74,9 @@ > #define CONVERT_NATIVE(fend, width, x) ({ \ > typeof(x) ___x; \ > if ((fend) == ELFDATA2LSB) \ > - ___x = rte_le_to_cpu_##width(x); \ > + ___x = le##width##toh(x); \ > else \ > - ___x = rte_be_to_cpu_##width(x); \ > + ___x = be##width##toh(x); \ > ___x; \ > }) >
On Fri, Nov 18, 2016 at 12:03:19PM +0000, Hemant Agrawal wrote: > > -----Original Message----- > > From: Neil Horman [mailto:nhorman@tuxdriver.com] > > On Tue, Nov 15, 2016 at 09:34:16AM +0000, Hemant Agrawal wrote: > > > > On Mon, Nov 14, 2016 at 02:29:24AM +0530, Jerin Jacob wrote: > > > > > On Fri, Nov 11, 2016 at 10:34:39AM +0000, Hemant Agrawal wrote: > > > > > > Hi Neil, > > > > > > Pmdinfogen compiles with host compiler. It usages > > rte_byteorder.h > > > > of the target platform. > > > > > > However, if the host compiler is older than 4.8, it will be an issue during > > cross > > > > compilation for some platforms. > > > > > > e.g. if we are compiling on x86 host for ARM, x86 host compiler will not > > > > understand the arm asm instructions. > > > > > > > > > > > > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if > > > > > > !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) static > > > > > > inline uint16_t rte_arch_bswap16(uint16_t _x) { > > > > > > register uint16_t x = _x; > > > > > > asm volatile ("rev16 %0,%1" > > > > > > : "=r" (x) > > > > > > : "r" (x) > > > > > > ); > > > > > > return x; > > > > > > } > > > > > > #endif > > > > > > > > > > > > One easy solution is that we add compiler platform check in this > > > > > > code section of rte_byteorder.h e.g #if !(defined __arm__ || defined > > > > > > __aarch64__) static inline uint16_t rte_arch_bswap16(uint16_t _x) { > > > > > > return (_x >> 8) | ((_x << 8) & 0xff00); } #else …. > > > > > > > > > > > > Is there a better way to fix it? > > > > > > > > > > IMO, It is a HOST build infrastructure issue. If a host app is using > > > > > the dpdk service then it should compile and link against HOST > > > > > target(in this specific case, build/x86_64-native-linuxapp-gcc). I > > > > > think, introducing the HOSTTARGET kind of scheme is a clean solution. > > > > > > > > > > /Jerin > > > > > > > > > > > > > > That would be accurate. That is to say, pmdinfogen is a tool that should only > > be > > > > run on the host doing the build, by the host doing the build, and so should be > > > > compiled to run on the host, not on the target being built for. > > > > > > > > Yeah, so what we need is a way to get to the host version of rte_byteorder.h > > > > when building in a cross environment > > > > > > > +1 > > > > > > > Neil > > > > > > > Give this a try, I've tested it on linux, but not BSD. From what I read the > > functions are not posix compliant, though they should exist on all BSD and Linux > > systems in recent history. There may be some fiddling needed for Net and > > OpenBSD variants, but I think this is the right general direction. > > + 1 > This patch works good for Linux. > Can someone test it on BSD? I'd like to ensure we don't need to modify it for that platform Neil > > > > > > diff --git a/buildtools/pmdinfogen/pmdinfogen.h > > b/buildtools/pmdinfogen/pmdinfogen.h > > index 1da2966..c5ef89d 100644 > > --- a/buildtools/pmdinfogen/pmdinfogen.h > > +++ b/buildtools/pmdinfogen/pmdinfogen.h > > @@ -21,7 +21,6 @@ > > #include <elf.h> > > #include <rte_config.h> > > #include <rte_pci.h> > > -#include <rte_byteorder.h> > > > > /* On BSD-alike OSes elf.h defines these according to host's word size */ > > #undef ELF_ST_BIND > > @@ -75,9 +74,9 @@ > > #define CONVERT_NATIVE(fend, width, x) ({ \ > > typeof(x) ___x; \ > > if ((fend) == ELFDATA2LSB) \ > > - ___x = rte_le_to_cpu_##width(x); \ > > + ___x = le##width##toh(x); \ > > else \ > > - ___x = rte_be_to_cpu_##width(x); \ > > + ___x = be##width##toh(x); \ > > ___x; \ > > }) > >
On Fri, Nov 18, 2016 at 08:50:52AM -0500, Neil Horman wrote: > On Fri, Nov 18, 2016 at 12:03:19PM +0000, Hemant Agrawal wrote: > > > -----Original Message----- > > > From: Neil Horman [mailto:nhorman@tuxdriver.com] > > > On Tue, Nov 15, 2016 at 09:34:16AM +0000, Hemant Agrawal wrote: > > > > > On Mon, Nov 14, 2016 at 02:29:24AM +0530, Jerin Jacob wrote: > > > > > > On Fri, Nov 11, 2016 at 10:34:39AM +0000, Hemant Agrawal wrote: > > > > > > > Hi Neil, > > > > > > > Pmdinfogen compiles with host compiler. It usages > > > rte_byteorder.h > > > > > of the target platform. > > > > > > > However, if the host compiler is older than 4.8, it will be an issue during > > > cross > > > > > compilation for some platforms. > > > > > > > e.g. if we are compiling on x86 host for ARM, x86 host compiler will not > > > > > understand the arm asm instructions. > > > > > > > > > > > > > > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if > > > > > > > !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) static > > > > > > > inline uint16_t rte_arch_bswap16(uint16_t _x) { > > > > > > > register uint16_t x = _x; > > > > > > > asm volatile ("rev16 %0,%1" > > > > > > > : "=r" (x) > > > > > > > : "r" (x) > > > > > > > ); > > > > > > > return x; > > > > > > > } > > > > > > > #endif > > > > > > > > > > > > > > One easy solution is that we add compiler platform check in this > > > > > > > code section of rte_byteorder.h e.g #if !(defined __arm__ || defined > > > > > > > __aarch64__) static inline uint16_t rte_arch_bswap16(uint16_t _x) { > > > > > > > return (_x >> 8) | ((_x << 8) & 0xff00); } #else …. > > > > > > > > > > > > > > Is there a better way to fix it? > > > > > > > > > > > > IMO, It is a HOST build infrastructure issue. If a host app is using > > > > > > the dpdk service then it should compile and link against HOST > > > > > > target(in this specific case, build/x86_64-native-linuxapp-gcc). I > > > > > > think, introducing the HOSTTARGET kind of scheme is a clean solution. > > > > > > > > > > > > /Jerin > > > > > > > > > > > > > > > > > That would be accurate. That is to say, pmdinfogen is a tool that should only > > > be > > > > > run on the host doing the build, by the host doing the build, and so should be > > > > > compiled to run on the host, not on the target being built for. > > > > > > > > > > Yeah, so what we need is a way to get to the host version of rte_byteorder.h > > > > > when building in a cross environment > > > > > > > > > +1 > > > > > > > > > Neil > > > > > > > > > > Give this a try, I've tested it on linux, but not BSD. From what I read the > > > functions are not posix compliant, though they should exist on all BSD and Linux > > > systems in recent history. There may be some fiddling needed for Net and > > > OpenBSD variants, but I think this is the right general direction. > > > > + 1 > > This patch works good for Linux. > > > Can someone test it on BSD? I'd like to ensure we don't need to modify it for > that platform > > Neil > > > > > > > > > > diff --git a/buildtools/pmdinfogen/pmdinfogen.h > > > b/buildtools/pmdinfogen/pmdinfogen.h > > > index 1da2966..c5ef89d 100644 > > > --- a/buildtools/pmdinfogen/pmdinfogen.h > > > +++ b/buildtools/pmdinfogen/pmdinfogen.h > > > @@ -21,7 +21,6 @@ > > > #include <elf.h> > > > #include <rte_config.h> > > > #include <rte_pci.h> > > > -#include <rte_byteorder.h> > > > > > > /* On BSD-alike OSes elf.h defines these according to host's word size */ > > > #undef ELF_ST_BIND > > > @@ -75,9 +74,9 @@ > > > #define CONVERT_NATIVE(fend, width, x) ({ \ > > > typeof(x) ___x; \ > > > if ((fend) == ELFDATA2LSB) \ > > > - ___x = rte_le_to_cpu_##width(x); \ > > > + ___x = le##width##toh(x); \ > > > else \ > > > - ___x = rte_be_to_cpu_##width(x); \ > > > + ___x = be##width##toh(x); \ > > > ___x; \ > > > }) > > > For compile on FreeBSD 10 we need "#include <sys/endian.h>" and this works. /Bruce
On Fri, Nov 18, 2016 at 04:37:38PM +0000, Bruce Richardson wrote: > On Fri, Nov 18, 2016 at 08:50:52AM -0500, Neil Horman wrote: > > On Fri, Nov 18, 2016 at 12:03:19PM +0000, Hemant Agrawal wrote: > > > > -----Original Message----- > > > > From: Neil Horman [mailto:nhorman@tuxdriver.com] > > > > On Tue, Nov 15, 2016 at 09:34:16AM +0000, Hemant Agrawal wrote: > > > > > > On Mon, Nov 14, 2016 at 02:29:24AM +0530, Jerin Jacob wrote: > > > > > > > On Fri, Nov 11, 2016 at 10:34:39AM +0000, Hemant Agrawal wrote: > > > > > > > > Hi Neil, > > > > > > > > Pmdinfogen compiles with host compiler. It usages > > > > rte_byteorder.h > > > > > > of the target platform. > > > > > > > > However, if the host compiler is older than 4.8, it will be an issue during > > > > cross > > > > > > compilation for some platforms. > > > > > > > > e.g. if we are compiling on x86 host for ARM, x86 host compiler will not > > > > > > understand the arm asm instructions. > > > > > > > > > > > > > > > > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if > > > > > > > > !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) static > > > > > > > > inline uint16_t rte_arch_bswap16(uint16_t _x) { > > > > > > > > register uint16_t x = _x; > > > > > > > > asm volatile ("rev16 %0,%1" > > > > > > > > : "=r" (x) > > > > > > > > : "r" (x) > > > > > > > > ); > > > > > > > > return x; > > > > > > > > } > > > > > > > > #endif > > > > > > > > > > > > > > > > One easy solution is that we add compiler platform check in this > > > > > > > > code section of rte_byteorder.h e.g #if !(defined __arm__ || defined > > > > > > > > __aarch64__) static inline uint16_t rte_arch_bswap16(uint16_t _x) { > > > > > > > > return (_x >> 8) | ((_x << 8) & 0xff00); } #else …. > > > > > > > > > > > > > > > > Is there a better way to fix it? > > > > > > > > > > > > > > IMO, It is a HOST build infrastructure issue. If a host app is using > > > > > > > the dpdk service then it should compile and link against HOST > > > > > > > target(in this specific case, build/x86_64-native-linuxapp-gcc). I > > > > > > > think, introducing the HOSTTARGET kind of scheme is a clean solution. > > > > > > > > > > > > > > /Jerin > > > > > > > > > > > > > > > > > > > > That would be accurate. That is to say, pmdinfogen is a tool that should only > > > > be > > > > > > run on the host doing the build, by the host doing the build, and so should be > > > > > > compiled to run on the host, not on the target being built for. > > > > > > > > > > > > Yeah, so what we need is a way to get to the host version of rte_byteorder.h > > > > > > when building in a cross environment > > > > > > > > > > > +1 > > > > > > > > > > > Neil > > > > > > > > > > > > > Give this a try, I've tested it on linux, but not BSD. From what I read the > > > > functions are not posix compliant, though they should exist on all BSD and Linux > > > > systems in recent history. There may be some fiddling needed for Net and > > > > OpenBSD variants, but I think this is the right general direction. > > > > > > + 1 > > > This patch works good for Linux. > > > > > Can someone test it on BSD? I'd like to ensure we don't need to modify it for > > that platform > > > > Neil > > > > > > > > > > > > > > diff --git a/buildtools/pmdinfogen/pmdinfogen.h > > > > b/buildtools/pmdinfogen/pmdinfogen.h > > > > index 1da2966..c5ef89d 100644 > > > > --- a/buildtools/pmdinfogen/pmdinfogen.h > > > > +++ b/buildtools/pmdinfogen/pmdinfogen.h > > > > @@ -21,7 +21,6 @@ > > > > #include <elf.h> > > > > #include <rte_config.h> > > > > #include <rte_pci.h> > > > > -#include <rte_byteorder.h> > > > > > > > > /* On BSD-alike OSes elf.h defines these according to host's word size */ > > > > #undef ELF_ST_BIND > > > > @@ -75,9 +74,9 @@ > > > > #define CONVERT_NATIVE(fend, width, x) ({ \ > > > > typeof(x) ___x; \ > > > > if ((fend) == ELFDATA2LSB) \ > > > > - ___x = rte_le_to_cpu_##width(x); \ > > > > + ___x = le##width##toh(x); \ > > > > else \ > > > > - ___x = rte_be_to_cpu_##width(x); \ > > > > + ___x = be##width##toh(x); \ > > > > ___x; \ > > > > }) > > > > > > For compile on FreeBSD 10 we need "#include <sys/endian.h>" and this > works. > Yeah, but that will break linux, because there endian.h is in the base include directory. I'll have to do some ifdeffing Neil > /Bruce >
diff --git a/buildtools/pmdinfogen/pmdinfogen.h b/buildtools/pmdinfogen/pmdinfogen.h index 1da2966..c5ef89d 100644 --- a/buildtools/pmdinfogen/pmdinfogen.h +++ b/buildtools/pmdinfogen/pmdinfogen.h @@ -21,7 +21,6 @@ #include <elf.h> #include <rte_config.h> #include <rte_pci.h> -#include <rte_byteorder.h> /* On BSD-alike OSes elf.h defines these according to host's word size */ #undef ELF_ST_BIND @@ -75,9 +74,9 @@ #define CONVERT_NATIVE(fend, width, x) ({ \ typeof(x) ___x; \ if ((fend) == ELFDATA2LSB) \ - ___x = rte_le_to_cpu_##width(x); \ + ___x = le##width##toh(x); \ else \ - ___x = rte_be_to_cpu_##width(x); \ + ___x = be##width##toh(x); \ ___x; \ })