| Message ID | 20160726025625.7343-12-brian.brooks@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 07/26/16 05:56, Brian Brooks wrote: > --- a/platform/linux-generic/arch/mips64/cpu_arch.h > +++ b/platform/linux-generic/arch/mips64/cpu_arch.h > @@ -7,6 +7,8 @@ > #ifndef ODP_PLAT_CPU_ARCH_H_ > #define ODP_PLAT_CPU_ARCH_H_ > > +#include <stdint.h> > + > #ifdef __cplusplus > extern "C" { > #endif > @@ -17,6 +19,18 @@ extern "C" { > #error Please add support for your arch in cpu_arch.h > #endif > > +static inline uint64_t _odp_cpu_cycles(void) > +{ > + #define CVMX_TMP_STR(x) CVMX_TMP_STR2(x) > + #define CVMX_TMP_STR2(x) #x > + uint64_t cycle; > + > + __asm__ __volatile__ ("rdhwr %[rt],$" CVMX_TMP_STR(31) : > + [rt] "=d" (cycle) : : "memory"); > + > + return cycle; > +} > + I know it's simple code move. But as I remember CVMX_ are Octeon internal functions, but generic mips64. So probably we missed some #ifdef OCTEON here... Maxim.
On 07/26 10:44:17, Maxim Uvarov wrote: > On 07/26/16 05:56, Brian Brooks wrote: > > --- a/platform/linux-generic/arch/mips64/cpu_arch.h > > +++ b/platform/linux-generic/arch/mips64/cpu_arch.h > > @@ -7,6 +7,8 @@ > > #ifndef ODP_PLAT_CPU_ARCH_H_ > > #define ODP_PLAT_CPU_ARCH_H_ > > +#include <stdint.h> > > + > > #ifdef __cplusplus > > extern "C" { > > #endif > > @@ -17,6 +19,18 @@ extern "C" { > > #error Please add support for your arch in cpu_arch.h > > #endif > > +static inline uint64_t _odp_cpu_cycles(void) > > +{ > > + #define CVMX_TMP_STR(x) CVMX_TMP_STR2(x) > > + #define CVMX_TMP_STR2(x) #x > > + uint64_t cycle; > > + > > + __asm__ __volatile__ ("rdhwr %[rt],$" CVMX_TMP_STR(31) : > > + [rt] "=d" (cycle) : : "memory"); > > + > > + return cycle; > > +} > > + > I know it's simple code move. But as I remember CVMX_ are Octeon internal > functions, but generic mips64. So > probably we missed some #ifdef OCTEON here... > > Maxim. Bala, can we wrap this in __OCTEON__ and use "31" instead of stringification macros?
On 26 July 2016 at 22:26, Brian Brooks <brian.brooks@linaro.org> wrote: > On 07/26 10:44:17, Maxim Uvarov wrote: >> On 07/26/16 05:56, Brian Brooks wrote: >> > --- a/platform/linux-generic/arch/mips64/cpu_arch.h >> > +++ b/platform/linux-generic/arch/mips64/cpu_arch.h >> > @@ -7,6 +7,8 @@ >> > #ifndef ODP_PLAT_CPU_ARCH_H_ >> > #define ODP_PLAT_CPU_ARCH_H_ >> > +#include <stdint.h> >> > + >> > #ifdef __cplusplus >> > extern "C" { >> > #endif >> > @@ -17,6 +19,18 @@ extern "C" { >> > #error Please add support for your arch in cpu_arch.h >> > #endif >> > +static inline uint64_t _odp_cpu_cycles(void) >> > +{ >> > + #define CVMX_TMP_STR(x) CVMX_TMP_STR2(x) >> > + #define CVMX_TMP_STR2(x) #x >> > + uint64_t cycle; >> > + >> > + __asm__ __volatile__ ("rdhwr %[rt],$" CVMX_TMP_STR(31) : >> > + [rt] "=d" (cycle) : : "memory"); >> > + >> > + return cycle; >> > +} >> > + >> I know it's simple code move. But as I remember CVMX_ are Octeon internal >> functions, but generic mips64. So >> probably we missed some #ifdef OCTEON here... >> >> Maxim. > > Bala, can we wrap this in __OCTEON__ and use "31" instead of stringification > macros? CVMX is octeon macro and I would prefer not to remove these. Do you have any specific reason for this proposal? Regards, Bala
On 07/26 22:38:20, Bala Manoharan wrote: > On 26 July 2016 at 22:26, Brian Brooks <brian.brooks@linaro.org> wrote: > > On 07/26 10:44:17, Maxim Uvarov wrote: > >> On 07/26/16 05:56, Brian Brooks wrote: > >> > --- a/platform/linux-generic/arch/mips64/cpu_arch.h > >> > +++ b/platform/linux-generic/arch/mips64/cpu_arch.h > >> > @@ -7,6 +7,8 @@ > >> > #ifndef ODP_PLAT_CPU_ARCH_H_ > >> > #define ODP_PLAT_CPU_ARCH_H_ > >> > +#include <stdint.h> > >> > + > >> > #ifdef __cplusplus > >> > extern "C" { > >> > #endif > >> > @@ -17,6 +19,18 @@ extern "C" { > >> > #error Please add support for your arch in cpu_arch.h > >> > #endif > >> > +static inline uint64_t _odp_cpu_cycles(void) > >> > +{ > >> > + #define CVMX_TMP_STR(x) CVMX_TMP_STR2(x) > >> > + #define CVMX_TMP_STR2(x) #x > >> > + uint64_t cycle; > >> > + > >> > + __asm__ __volatile__ ("rdhwr %[rt],$" CVMX_TMP_STR(31) : > >> > + [rt] "=d" (cycle) : : "memory"); > >> > + > >> > + return cycle; > >> > +} > >> > + > >> I know it's simple code move. But as I remember CVMX_ are Octeon internal > >> functions, but generic mips64. So > >> probably we missed some #ifdef OCTEON here... > >> > >> Maxim. > > > > Bala, can we wrap this in __OCTEON__ and use "31" instead of stringification > > macros? > > CVMX is octeon macro and I would prefer not to remove these. OK. > Do you have any specific reason for this proposal? > > Regards, > Bala I think the question is whether reading hardware register 31 to get a value for odp_cpu_cycles() is specific to Octeon? If so, the proposal is to wrap in __OCTEON__ so that this code works for other mips builds.
On 26 July 2016 at 23:09, Brian Brooks <brian.brooks@linaro.org> wrote: > On 07/26 22:38:20, Bala Manoharan wrote: >> On 26 July 2016 at 22:26, Brian Brooks <brian.brooks@linaro.org> wrote: >> > On 07/26 10:44:17, Maxim Uvarov wrote: >> >> On 07/26/16 05:56, Brian Brooks wrote: >> >> > --- a/platform/linux-generic/arch/mips64/cpu_arch.h >> >> > +++ b/platform/linux-generic/arch/mips64/cpu_arch.h >> >> > @@ -7,6 +7,8 @@ >> >> > #ifndef ODP_PLAT_CPU_ARCH_H_ >> >> > #define ODP_PLAT_CPU_ARCH_H_ >> >> > +#include <stdint.h> >> >> > + >> >> > #ifdef __cplusplus >> >> > extern "C" { >> >> > #endif >> >> > @@ -17,6 +19,18 @@ extern "C" { >> >> > #error Please add support for your arch in cpu_arch.h >> >> > #endif >> >> > +static inline uint64_t _odp_cpu_cycles(void) >> >> > +{ >> >> > + #define CVMX_TMP_STR(x) CVMX_TMP_STR2(x) >> >> > + #define CVMX_TMP_STR2(x) #x >> >> > + uint64_t cycle; >> >> > + >> >> > + __asm__ __volatile__ ("rdhwr %[rt],$" CVMX_TMP_STR(31) : >> >> > + [rt] "=d" (cycle) : : "memory"); >> >> > + >> >> > + return cycle; >> >> > +} >> >> > + >> >> I know it's simple code move. But as I remember CVMX_ are Octeon internal >> >> functions, but generic mips64. So >> >> probably we missed some #ifdef OCTEON here... >> >> >> >> Maxim. >> > >> > Bala, can we wrap this in __OCTEON__ and use "31" instead of stringification >> > macros? >> >> CVMX is octeon macro and I would prefer not to remove these. > > OK. > >> Do you have any specific reason for this proposal? >> >> Regards, >> Bala > > I think the question is whether reading hardware register 31 to get a value > for odp_cpu_cycles() is specific to Octeon? > > If so, the proposal is to wrap in __OCTEON__ so that this code works for other > mips builds. Yes. This is an octeon specific instruction and it should be wrapped using __OCTEON__. Regards, Bala
diff --git a/platform/Makefile.inc b/platform/Makefile.inc index b45c955..fae6094 100644 --- a/platform/Makefile.inc +++ b/platform/Makefile.inc @@ -63,14 +63,10 @@ odpapispecinclude_HEADERS = \ EXTRA_DIST = \ arch/arm/cpu_arch.h \ - arch/arm/odp_cpu_arch.c \ arch/arm/odp_sysinfo_parse.c \ arch/mips64/cpu_arch.h \ - arch/mips64/odp_cpu_arch.c \ arch/mips64/odp_sysinfo_parse.c \ arch/powerpc/cpu_arch.h \ - arch/powerpc/odp_cpu_arch.c \ arch/powerpc/odp_sysinfo_parse.c \ arch/x86/cpu_arch.h \ - arch/x86/odp_cpu_arch.c \ arch/x86/odp_sysinfo_parse.c diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am index 8dc2764..fef2109 100644 --- a/platform/linux-generic/Makefile.am +++ b/platform/linux-generic/Makefile.am @@ -183,7 +183,6 @@ __LIB__libodp_linux_la_SOURCES = \ odp_traffic_mngr.c \ odp_version.c \ odp_weak.c \ - arch/@ARCH_DIR@/odp_cpu_arch.c \ arch/@ARCH_DIR@/odp_sysinfo_parse.c if HAVE_PCAP diff --git a/platform/linux-generic/arch/arm/cpu_arch.h b/platform/linux-generic/arch/arm/cpu_arch.h index bdd3335..19ee349 100644 --- a/platform/linux-generic/arch/arm/cpu_arch.h +++ b/platform/linux-generic/arch/arm/cpu_arch.h @@ -7,12 +7,24 @@ #ifndef ODP_PLAT_CPU_ARCH_H_ #define ODP_PLAT_CPU_ARCH_H_ +#include <stdint.h> + #ifdef __cplusplus extern "C" { #endif #define _ODP_CACHE_LINE_SIZE 64 +static inline uint64_t _odp_cpu_cycles(void) +{ +#if defined(__aarch64__) + uint64_t vct; + __asm__ volatile("isb" : : : "memory"); + __asm__ volatile("mrs %0, cntvct_el0" : "=r" (vct)); + return vct; +#endif +} + static inline void _odp_cpu_pause(void) { #if defined(__aarch64__) diff --git a/platform/linux-generic/arch/arm/odp_cpu_arch.c b/platform/linux-generic/arch/arm/odp_cpu_arch.c deleted file mode 100644 index 0ae3e0e..0000000 --- a/platform/linux-generic/arch/arm/odp_cpu_arch.c +++ /dev/null @@ -1,25 +0,0 @@ -/* Copyright (c) 2015, Linaro Limited - * All rights reserved. - * - * SPDX-License-Identifier: BSD-3-Clause - */ - -#include <odp_posix_extensions.h> - -#include <stdlib.h> -#include <time.h> - -#include <odp/api/cpu.h> -#include <odp/api/hints.h> -#include <odp/api/system_info.h> -#include <odp_debug_internal.h> - -uint64_t odp_cpu_cycles(void) -{ -#if defined(__aarch64__) - uint64_t vct; - __asm__ volatile("isb" : : : "memory"); - __asm__ volatile("mrs %0, cntvct_el0" : "=r" (vct)); - return vct; -#endif -} diff --git a/platform/linux-generic/arch/mips64/cpu_arch.h b/platform/linux-generic/arch/mips64/cpu_arch.h index d984255..e615e26 100644 --- a/platform/linux-generic/arch/mips64/cpu_arch.h +++ b/platform/linux-generic/arch/mips64/cpu_arch.h @@ -7,6 +7,8 @@ #ifndef ODP_PLAT_CPU_ARCH_H_ #define ODP_PLAT_CPU_ARCH_H_ +#include <stdint.h> + #ifdef __cplusplus extern "C" { #endif @@ -17,6 +19,18 @@ extern "C" { #error Please add support for your arch in cpu_arch.h #endif +static inline uint64_t _odp_cpu_cycles(void) +{ + #define CVMX_TMP_STR(x) CVMX_TMP_STR2(x) + #define CVMX_TMP_STR2(x) #x + uint64_t cycle; + + __asm__ __volatile__ ("rdhwr %[rt],$" CVMX_TMP_STR(31) : + [rt] "=d" (cycle) : : "memory"); + + return cycle; +} + static inline void _odp_cpu_pause(void) { __asm__ __volatile__ ("nop"); diff --git a/platform/linux-generic/arch/mips64/odp_cpu_arch.c b/platform/linux-generic/arch/mips64/odp_cpu_arch.c deleted file mode 100644 index 6dc8f86..0000000 --- a/platform/linux-generic/arch/mips64/odp_cpu_arch.c +++ /dev/null @@ -1,21 +0,0 @@ -/* Copyright (c) 2015, Linaro Limited - * All rights reserved. - * - * SPDX-License-Identifier: BSD-3-Clause - */ - -#include <odp/api/cpu.h> -#include <odp/api/hints.h> -#include <odp/api/system_info.h> - -uint64_t odp_cpu_cycles(void) -{ - #define CVMX_TMP_STR(x) CVMX_TMP_STR2(x) - #define CVMX_TMP_STR2(x) #x - uint64_t cycle; - - __asm__ __volatile__ ("rdhwr %[rt],$" CVMX_TMP_STR(31) : - [rt] "=d" (cycle) : : "memory"); - - return cycle; -} diff --git a/platform/linux-generic/arch/powerpc/cpu_arch.h b/platform/linux-generic/arch/powerpc/cpu_arch.h index 0d52616..3387444 100644 --- a/platform/linux-generic/arch/powerpc/cpu_arch.h +++ b/platform/linux-generic/arch/powerpc/cpu_arch.h @@ -7,12 +7,29 @@ #ifndef ODP_PLAT_CPU_ARCH_H_ #define ODP_PLAT_CPU_ARCH_H_ +#include <stdint.h> + #ifdef __cplusplus extern "C" { #endif #define _ODP_CACHE_LINE_SIZE 64 +static inline uint64_t _odp_cpu_cycles(void) +{ +#if defined(__powerpc__) || defined(__ppc__) + uint64_t tbl, tbu0, tbu1; + + do { + __asm__ volatile("mftbu %0" : "=r"(tbu0)); + __asm__ volatile("mftb %0" : "=r"(tbl)); + __asm__ volatile("mftbu %0" : "=r"(tbu1)); + } while (tbu0 != tbu1); + + return (tbu0 << 32) | tbl; +#endif +} + static inline void _odp_cpu_pause(void) { } diff --git a/platform/linux-generic/arch/powerpc/odp_cpu_arch.c b/platform/linux-generic/arch/powerpc/odp_cpu_arch.c deleted file mode 100644 index 346c170..0000000 --- a/platform/linux-generic/arch/powerpc/odp_cpu_arch.c +++ /dev/null @@ -1,30 +0,0 @@ -/* Copyright (c) 2015, Linaro Limited - * All rights reserved. - * - * SPDX-License-Identifier: BSD-3-Clause - */ - -#include <odp_posix_extensions.h> - -#include <stdlib.h> -#include <time.h> - -#include <odp/api/cpu.h> -#include <odp/api/hints.h> -#include <odp/api/system_info.h> -#include <odp_debug_internal.h> - -uint64_t odp_cpu_cycles(void) -{ -#if defined(__powerpc__) || defined(__ppc__) - uint64_t tbl, tbu0, tbu1; - - do { - __asm__ volatile("mftbu %0" : "=r"(tbu0)); - __asm__ volatile("mftb %0" : "=r"(tbl)); - __asm__ volatile("mftbu %0" : "=r"(tbu1)); - } while (tbu0 != tbu1); - - return (tbu0 << 32) | tbl; -#endif -} diff --git a/platform/linux-generic/arch/x86/cpu_arch.h b/platform/linux-generic/arch/x86/cpu_arch.h index 4f4dbff..6aaf80e 100644 --- a/platform/linux-generic/arch/x86/cpu_arch.h +++ b/platform/linux-generic/arch/x86/cpu_arch.h @@ -7,12 +7,24 @@ #ifndef ODP_PLAT_CPU_ARCH_H_ #define ODP_PLAT_CPU_ARCH_H_ +#include <stdint.h> + #ifdef __cplusplus extern "C" { #endif #define _ODP_CACHE_LINE_SIZE 64 +static inline uint64_t _odp_cpu_cycles(void) +{ +#if defined(__x86_64__) || defined(__amd64__) + uint64_t hi, lo; + __asm__ volatile("mfence" : : : "memory"); + __asm__ volatile("rdtsc" : "=a" (lo), "=d" (hi)); + return (hi << 32) | lo; +#endif +} + static inline void _odp_cpu_pause(void) { #ifdef __SSE2__ diff --git a/platform/linux-generic/arch/x86/odp_cpu_arch.c b/platform/linux-generic/arch/x86/odp_cpu_arch.c deleted file mode 100644 index a9e819d..0000000 --- a/platform/linux-generic/arch/x86/odp_cpu_arch.c +++ /dev/null @@ -1,16 +0,0 @@ -/* Copyright (c) 2015, Linaro Limited - * All rights reserved. - * - * SPDX-License-Identifier: BSD-3-Clause - */ -#include <odp/api/cpu.h> - -uint64_t odp_cpu_cycles(void) -{ -#if defined(__x86_64__) || defined(__amd64__) - uint64_t hi, lo; - __asm__ volatile("mfence" : : : "memory"); - __asm__ volatile("rdtsc" : "=a" (lo), "=d" (hi)); - return (hi << 32) | lo; -#endif -} diff --git a/platform/linux-generic/include/odp/api/cpu.h b/platform/linux-generic/include/odp/api/cpu.h index 8dd978e..2e59ea8 100644 --- a/platform/linux-generic/include/odp/api/cpu.h +++ b/platform/linux-generic/include/odp/api/cpu.h @@ -18,7 +18,11 @@ extern "C" { #endif #include "cpu_arch.h" -#include <odp/api/std_types.h> + +static inline uint64_t odp_cpu_cycles(void) +{ + return _odp_cpu_cycles(); +} static inline uint64_t odp_cpu_cycles_max(void) {
Signed-off-by: Brian Brooks <brian.brooks@linaro.org> --- platform/Makefile.inc | 4 --- platform/linux-generic/Makefile.am | 1 - platform/linux-generic/arch/arm/cpu_arch.h | 12 +++++++++ platform/linux-generic/arch/arm/odp_cpu_arch.c | 25 ------------------ platform/linux-generic/arch/mips64/cpu_arch.h | 14 ++++++++++ platform/linux-generic/arch/mips64/odp_cpu_arch.c | 21 --------------- platform/linux-generic/arch/powerpc/cpu_arch.h | 17 ++++++++++++ platform/linux-generic/arch/powerpc/odp_cpu_arch.c | 30 ---------------------- platform/linux-generic/arch/x86/cpu_arch.h | 12 +++++++++ platform/linux-generic/arch/x86/odp_cpu_arch.c | 16 ------------ platform/linux-generic/include/odp/api/cpu.h | 6 ++++- 11 files changed, 60 insertions(+), 98 deletions(-) delete mode 100644 platform/linux-generic/arch/arm/odp_cpu_arch.c delete mode 100644 platform/linux-generic/arch/mips64/odp_cpu_arch.c delete mode 100644 platform/linux-generic/arch/powerpc/odp_cpu_arch.c delete mode 100644 platform/linux-generic/arch/x86/odp_cpu_arch.c -- 2.9.0