| Message ID | 20220507211546.2363770-1-dunk@denkimushi.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | David Marchand |
| Headers | show |
| Series | [v6] lib/eal/ppc fix compilation for musl | expand |
| Context | Check | Description |
|---|---|---|
| ci/iol-x86_64-compile-testing | success | Testing PASS |
| ci/iol-abi-testing | success | Testing PASS |
| ci/iol-x86_64-unit-testing | success | Testing PASS |
| ci/iol-aarch64-compile-testing | success | Testing PASS |
| ci/iol-aarch64-unit-testing | success | Testing PASS |
| ci/github-robot: build | success | github build: passed |
| ci/iol-intel-Functional | success | Functional Testing PASS |
| ci/iol-intel-Performance | success | Performance Testing PASS |
| ci/intel-Testing | success | Testing PASS |
| ci/iol-mellanox-Performance | success | Performance Testing PASS |
| ci/Intel-compilation | success | Compilation OK |
| ci/checkpatch | warning | coding style issues |
On Sat, May 7, 2022 at 11:16 PM Duncan Bellamy <dunk@denkimushi.com> wrote: > > musl lacks __ppc_get_timebase() but has __builtin_ppc_get_timebase() > > the __ppc_get_timebase_freq() is taken from: > https://git.alpinelinux.org/aports/commit/?id=06b03f70fb94972286c0c9f6278df89e53903833 > > Signed-off-by: Duncan Bellamy <dunk@denkimushi.com> - A patch title does not need lib/ prefix. Here, "eal/ppc: " is enough. - Code in lib/eal/linux won't be used for FreeBSD/Windows. On the other hand, arch-specific code (here, lib/eal/ppc/) can be used for the various OS. Besides, as far as I can see in the Linux kernel sources, powerpc is the only architecture that exports a "timebase" entry in /proc/cpuinfo. So, I see no reason to put any code out of lib/eal/ppc. - In the end, unless I missed some point, the patch could probably look like (untested): diff --git a/lib/eal/ppc/include/rte_cycles.h b/lib/eal/ppc/include/rte_cycles.h index 5585f9273c..666fc9b0bf 100644 --- a/lib/eal/ppc/include/rte_cycles.h +++ b/lib/eal/ppc/include/rte_cycles.h @@ -10,7 +10,10 @@ extern "C" { #endif +#include <features.h> +#ifdef __GLIBC__ #include <sys/platform/ppc.h> +#endif #include "generic/rte_cycles.h" @@ -26,7 +29,11 @@ extern "C" { static inline uint64_t rte_rdtsc(void) { +#ifdef __GLIBC__ return __ppc_get_timebase(); +#else + return __builtin_ppc_get_timebase(); +#endif } static inline uint64_t diff --git a/lib/eal/ppc/rte_cycles.c b/lib/eal/ppc/rte_cycles.c index 3180adb0ff..99d36b2f7e 100644 --- a/lib/eal/ppc/rte_cycles.c +++ b/lib/eal/ppc/rte_cycles.c @@ -2,12 +2,50 @@ * Copyright (C) IBM Corporation 2019. */ +#include <features.h> +#ifdef __GLIBC__ #include <sys/platform/ppc.h> +#elif RTE_EXEC_ENV_LINUX +#include <string.h> +#include <stdio.h> +#endif #include "eal_private.h" uint64_t get_tsc_freq_arch(void) { +#ifdef __GLIBC__ return __ppc_get_timebase_freq(); +#elif RTE_EXEC_ENV_LINUX + static unsigned long base; + char buf[512]; + ssize_t nr; + FILE *f; + + if (base != 0) + goto out; + + f = fopen("/proc/cpuinfo", "rb"); + if (f == NULL) + goto out; + + while (fgets(buf, sizeof(buf), f) != NULL) { + char *ret = strstr(buf, "timebase"); + + if (ret == NULL) + continue; + ret += sizeof("timebase") - 1; + ret = strchr(ret, ':'); + if (ret == NULL) + continue; + base = strtoul(ret + 1, NULL, 10); + break; + } + fclose(f); +out: + return (uint64_t) base; +#else + return 0; +#endif }
> On 9 May 2022, at 13:06, David Marchand <david.marchand@redhat.com> wrote: > > On Sat, May 7, 2022 at 11:16 PM Duncan Bellamy <dunk@denkimushi.com> wrote: >> >> musl lacks __ppc_get_timebase() but has __builtin_ppc_get_timebase() >> >> the __ppc_get_timebase_freq() is taken from: >> https://git.alpinelinux.org/aports/commit/?id=06b03f70fb94972286c0c9f6278df89e53903833 >> >> Signed-off-by: Duncan Bellamy <dunk@denkimushi.com> > > - A patch title does not need lib/ prefix. > Here, "eal/ppc: " is enough. > > > - Code in lib/eal/linux won't be used for FreeBSD/Windows. > On the other hand, arch-specific code (here, lib/eal/ppc/) can be used > for the various OS. > Besides, as far as I can see in the Linux kernel sources, powerpc is > the only architecture that exports a "timebase" entry in > /proc/cpuinfo. > So, I see no reason to put any code out of lib/eal/ppc. > > > - In the end, unless I missed some point, the patch could probably > look like (untested): > > diff --git a/lib/eal/ppc/include/rte_cycles.h b/lib/eal/ppc/include/rte_cycles.h > index 5585f9273c..666fc9b0bf 100644 > --- a/lib/eal/ppc/include/rte_cycles.h > +++ b/lib/eal/ppc/include/rte_cycles.h > @@ -10,7 +10,10 @@ > extern "C" { > #endif > > +#include <features.h> > +#ifdef __GLIBC__ > #include <sys/platform/ppc.h> > +#endif > > #include "generic/rte_cycles.h" > > @@ -26,7 +29,11 @@ extern "C" { > static inline uint64_t > rte_rdtsc(void) > { > +#ifdef __GLIBC__ > return __ppc_get_timebase(); > +#else > + return __builtin_ppc_get_timebase(); > +#endif > } > > static inline uint64_t > diff --git a/lib/eal/ppc/rte_cycles.c b/lib/eal/ppc/rte_cycles.c > index 3180adb0ff..99d36b2f7e 100644 > --- a/lib/eal/ppc/rte_cycles.c > +++ b/lib/eal/ppc/rte_cycles.c > @@ -2,12 +2,50 @@ > * Copyright (C) IBM Corporation 2019. > */ > > +#include <features.h> > +#ifdef __GLIBC__ > #include <sys/platform/ppc.h> > +#elif RTE_EXEC_ENV_LINUX > +#include <string.h> > +#include <stdio.h> > +#endif > > #include "eal_private.h" > > uint64_t > get_tsc_freq_arch(void) > { > +#ifdef __GLIBC__ > return __ppc_get_timebase_freq(); > +#elif RTE_EXEC_ENV_LINUX > + static unsigned long base; > + char buf[512]; > + ssize_t nr; > + FILE *f; > + > + if (base != 0) > + goto out; > + > + f = fopen("/proc/cpuinfo", "rb"); > + if (f == NULL) > + goto out; > + > + while (fgets(buf, sizeof(buf), f) != NULL) { > + char *ret = strstr(buf, "timebase"); > + > + if (ret == NULL) > + continue; > + ret += sizeof("timebase") - 1; > + ret = strchr(ret, ':'); > + if (ret == NULL) > + continue; > + base = strtoul(ret + 1, NULL, 10); > + break; > + } > + fclose(f); > +out: > + return (uint64_t) base; > +#else > + return 0; > +#endif > } > > > -- > David Marchand > Thanks, that looks the same thing. Will run through alpine CI and change commit title
diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h index 44d14241f0..9ac1949e1c 100644 --- a/lib/eal/common/eal_private.h +++ b/lib/eal/common/eal_private.h @@ -743,4 +743,11 @@ int eal_asprintf(char **buffer, const char *format, ...); eal_asprintf(buffer, format, ##__VA_ARGS__) #endif +/** + * Function for systems with no __ppc_get_timebase_freq function. + */ +#ifndef RTE_ARCH_PPC_64 +uint64_t no_ppc_get_timebase_freq(void); +#endif + #endif /* _EAL_PRIVATE_H_ */ diff --git a/lib/eal/linux/eal_timer.c b/lib/eal/linux/eal_timer.c index 620baf038d..4f46f5411d 100644 --- a/lib/eal/linux/eal_timer.c +++ b/lib/eal/linux/eal_timer.c @@ -5,6 +5,8 @@ #include <stdio.h> #include <stdint.h> +#include <string.h> +#include <features.h> #include <rte_common.h> #include <rte_cycles.h> @@ -222,3 +224,38 @@ rte_eal_timer_init(void) set_tsc_freq(); return 0; } + +#ifndef RTE_ARCH_PPC_64 +uint64_t +no_ppc_get_timebase_freq(void) +{ + static uint64_t base; + #if defined(__LINUX__) + if (!base) { + FILE *f = fopen("/proc/cpuinfo", "rb"); + if (f) { + ssize_t nr; + /* virtually always big enough to hold the line */ + char buf[512]; + while (fgets(buf, sizeof(buf), f)) { + char *ret = strstr(buf, "timebase"); + if (!ret) + continue; + ret += sizeof("timebase") - 1; + ret = strchr(ret, ':'); + if (!ret) + continue; + base = strtoul(ret + 1, 0, 10); + break; + } + fclose(f); + } + } + #elif defined(_WIN32) || defined(_WIN64) + /* windows code here */ + #elif defined(RTE_EXEC_ENV_FREEBSD) + /* freebsd code here */ + #endif + return base; +} +#endif diff --git a/lib/eal/ppc/include/rte_cycles.h b/lib/eal/ppc/include/rte_cycles.h index 5585f9273c..666fc9b0bf 100644 --- a/lib/eal/ppc/include/rte_cycles.h +++ b/lib/eal/ppc/include/rte_cycles.h @@ -10,7 +10,10 @@ extern "C" { #endif +#include <features.h> +#ifdef __GLIBC__ #include <sys/platform/ppc.h> +#endif #include "generic/rte_cycles.h" @@ -26,7 +29,11 @@ extern "C" { static inline uint64_t rte_rdtsc(void) { +#ifdef __GLIBC__ return __ppc_get_timebase(); +#else + return __builtin_ppc_get_timebase(); +#endif } static inline uint64_t diff --git a/lib/eal/ppc/rte_cycles.c b/lib/eal/ppc/rte_cycles.c index 3180adb0ff..b69fc701a6 100644 --- a/lib/eal/ppc/rte_cycles.c +++ b/lib/eal/ppc/rte_cycles.c @@ -2,12 +2,22 @@ * Copyright (C) IBM Corporation 2019. */ +#include <features.h> +#ifdef __GLIBC__ #include <sys/platform/ppc.h> +#else +#include <string.h> +#include <stdio.h> +#endif #include "eal_private.h" uint64_t get_tsc_freq_arch(void) { +#ifdef __GLIBC__ return __ppc_get_timebase_freq(); +#else + return no_ppc_get_timebase_freq(); +#endif }
musl lacks __ppc_get_timebase() but has __builtin_ppc_get_timebase() the __ppc_get_timebase_freq() is taken from: https://git.alpinelinux.org/aports/commit/?id=06b03f70fb94972286c0c9f6278df89e53903833 Signed-off-by: Duncan Bellamy <dunk@denkimushi.com> --- lib/eal/common/eal_private.h | 7 ++++++ lib/eal/linux/eal_timer.c | 37 ++++++++++++++++++++++++++++++++ lib/eal/ppc/include/rte_cycles.h | 7 ++++++ lib/eal/ppc/rte_cycles.c | 10 +++++++++ 4 files changed, 61 insertions(+)