improve tsc frequency calibration
v2 cover improve tsc frequency calibration
From: Gowrishankar Muthukrishnan <[email protected]>
Some architecture like armv8 provides an architecture specific function to get the rdtsc frequency. The existing rdtsc calibration scheme uses OS serivce like sleep(1) to calibrate the frequency which may not produce the accurate result. Introducing an architecture specific hook to get the rdtsc frequency if architecture provides it. If not, use the exiting the calibrate scheme to get the rdtsc frequency.
Jerin Jacob (5):
eal/x86: define architecture specific rdtsc hz eal/ppc64: define architecture specific rdtsc hz eal/armv7: define architecture specific rdtsc hz eal/armv8: define architecture specific rdtsc hz eal/timer: honor architecture specific rdtsc hz function
lib/librte_eal/common/eal_common_timer.c | 5 +++- .../common/include/arch/arm/rte_cycles_32.h | 13 ++++++++++ .../common/include/arch/arm/rte_cycles_64.h | 30 ++++++++++++++++++++++ .../common/include/arch/ppc_64/rte_cycles.h | 24 +++++++++++++++++ .../common/include/arch/x86/rte_cycles.h | 13 ++++++++++ 5 files changed, 84 insertions(+), 1 deletion(-)
Signature · 1 line
1.9.1
PATCH v2 2/5 eal/ppc64: define architecture specific rdtsc hz
From: Jerin Jacob <[email protected]>
In ppc_64, rte_rdtsc() returns timebase register value which increments at independent timebase frequency and hence not related to lcore cpu frequency to derive TSC hz. Hence, we stick with master lcore frequency.
---
v2:
* add ppc_64 specific implementation
.../common/include/arch/ppc_64/rte_cycles.h | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+)
lib/librte_eal/common/include/arch/ppc_64/rte_cycles.h +24 -0
@@ -38,9 +38,13 @@#endif #include "generic/rte_cycles.h" +#include "../../lib/librte_eal/common/eal_filesystem.h" #include <rte_byteorder.h> #include <rte_common.h> +#include <rte_lcore.h> + +static const char sys_cpu_dir[] ="/sys/devices/system/cpu" ;/** * Read the time base register.
@@ -79,6 +83,26 @@return tsc.tsc_64; } +/** +* Get the number of rdtsc cycles in one second. +* +* @return +* The number of rdtsc cycles in one second. +*/ +static inline uint64_t +rte_rdtsc_arch_hz (void ) +{ +unsigned long cpu_hz; +char path[PATH_MAX ]; + +snprintf (path,sizeof (path),"%s/cpu%d/cpufreq/cpuinfo_cur_freq" , + sys_cpu_dir,rte_get_master_lcore ()); +if (eal_parse_sysfs_value (path, &cpu_hz) <0 ) +RTE_LOG (WARNING ,EAL ,"Unable to parse %s\n" , path); + +return cpu_hz*1000 ; +} +static inline uint64_t rte_rdtsc_precise (void ) {
Signature · 1 line
1.9.1
PATCH v2 1/5 eal/x86: define architecture specific rdtsc hz
In reply to GowrishankarFrom: Jerin Jacob <[email protected]>
--- lib/librte_eal/common/include/arch/x86/rte_cycles.h | 13 +++++++++++++ 1 file changed, 13 insertions(+)
lib/librte_eal/common/include/arch/x86/rte_cycles.h +13 -0
@@ -77,6 +77,19 @@return tsc.tsc_64; } +/** +* Get the number of rdtsc cycles in one second if the architecture supports. +* +* @return +* The number of rdtsc cycles in one second. Return zero if the architecture +* support is not available. +*/ +static inline uint64_t +rte_rdtsc_arch_hz (void ) +{ +return 0 ; +} +static inline uint64_t rte_rdtsc_precise (void ) {
Signature · 1 line
1.9.1
PATCH v2 3/5 eal/armv7: define architecture specific rdtsc hz
In reply to GowrishankarFrom: Jerin Jacob <[email protected]>
--- lib/librte_eal/common/include/arch/arm/rte_cycles_32.h | 13 +++++++++++++ 1 file changed, 13 insertions(+)
lib/librte_eal/common/include/arch/arm/rte_cycles_32.h +13 -0
@@ -104,6 +104,19 @@#endif /* RTE_ARM_EAL_RDTSC_USE_PMU */ +/** +* Get the number of rdtsc cycles in one second if the architecture supports. +* +* @return +* The number of rdtsc cycles in one second. Return zero if the architecture +* support is not available. +*/ +static inline uint64_t +rte_rdtsc_arch_hz (void ) +{ +return 0 ; +} +static inline uint64_t rte_rdtsc_precise (void ) {
Signature · 1 line
1.9.1
PATCH v2 4/5 eal/armv8: define architecture specific rdtsc hz
In reply to GowrishankarFrom: Jerin Jacob <[email protected]>
Use cntvct_el0 system register to get the system counter frequency.
If the system is configured with RTE_ARM_EAL_RDTSC_USE_PMU then return 0(let the common code calibrate the tsc frequency).
--- .../common/include/arch/arm/rte_cycles_64.h | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+)
lib/librte_eal/common/include/arch/arm/rte_cycles_64.h +30 -0
@@ -58,6 +58,23 @@ asmvolatile ("mrs %0, cntvct_el0" :"=r" (tsc)); return tsc; } + +/** +* Get the number of rdtsc cycles in one second if the architecture supports. +* +* @return +* The number of rdtsc cycles in one second. Return zero if the architecture +* support is not available. +*/ +static inline uint64_t +rte_rdtsc_arch_hz (void ) +{ +uint64_t freq; + + asmvolatile ("mrs %0, cntfrq_el0" :"=r" (freq)); +return freq; +} +#else /** * This is an alternative method to enable rte_rdtsc() with high resolution
@@ -85,6 +102,19 @@ asmvolatile ("mrs %0, pmccntr_el0" :"=r" (tsc)); return tsc; } + +/** +* Get the number of rdtsc cycles in one second if the architecture supports. +* +* @return +* The number of rdtsc cycles in one second. Return zero if the architecture +* support is not available. +*/ +static inline uint64_t +rte_rdtsc_arch_hz (void ) +{ +return 0 ; +}#endif static inline uint64_t
Signature · 1 line
1.9.1
PATCH v2 5/5 eal/timer: honor architecture specific rdtsc hz function
In reply to GowrishankarFrom: Jerin Jacob <[email protected]>
When calibrating the tsc frequency, first, probe the architecture specific rdtsc hz function. if not available, use the existing calibrate scheme to calibrate the tsc frequency.
--- lib/librte_eal/common/eal_common_timer.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
lib/librte_eal/common/eal_common_timer.c +4 -1
@@ -80,8 +80,11 @@void set_tsc_freq (void ) { -uint64_t freq =get_tsc_freq (); +uint64_t freq; + freq =rte_rdtsc_arch_hz (); +if (!freq) + freq =get_tsc_freq ();if (!freq) freq =estimate_tsc_freq ();
Signature · 1 line
1.9.1
Re: [PATCH v2 5/5] eal/timer: honor architecture specific rdtsc hz function
22/09/2017 10:25, Gowrishankar:
Quoted 7 lines
From: Jerin Jacob <[email protected]>
When calibrating the tsc frequency, first, probe the architecture specific rdtsc hz function. if not available, use the existing calibrate scheme to calibrate the tsc frequency.
Signed-off-by Jerin Jacob <[email protected]>
I agree on the idea.
The namespace of cycles related function in DPDK is a real mess. I think we can choose better names in this series as a first step to tidy this mess.
I will explain below.
At first, we should avoid TSC and RDTSC which are Intel-only wording. The generic word could be "cycles" (the word used in arch headers), or "ticks".
We should also name the timer sources or their function in a generic way.
Examples: CPU cycles? fast counter? precise counter?
Sometimes we use "hz", sometimes "freq".
It would better to keep one of them.
lib/librte_eal/common/eal_common_timer.c line 85
Quoted 10 lines
lib/librte_eal/common/eal_common_timer.c +2
-1@@ -80,8 +80,11 @@void set_tsc_freq (void ) { -uint64_t freq =get_tsc_freq (); +uint64_t freq; + freq =rte_rdtsc_arch_hz ();
This new function is arch-specific and exported as a new API.
lib/librte_eal/common/eal_common_timer.c line 87
Quoted 2 lines
lib/librte_eal/common/eal_common_timer.c +2
-0+if (!freq) + freq =get_tsc_freq ();
The function get_tsc_freq is guessing the freq with OS-specific method.
lib/librte_eal/common/eal_common_timer.c line 89
Quoted 2 lines
lib/librte_eal/common/eal_common_timer.c +0
-0if (!freq) freq =estimate_tsc_freq ();
The function estimate_tsc_freq is doing an estimation based on sleep().
At the end, the most accurate frequency is saved in eal_tsc_resolution_hz and can be retrieved with rte_get_tsc_hz().
I don't understand why rte_rdtsc_arch_hz() is also exported to the apps.
TSC and HPET timer sources are wrapped in rte_get_timer_hz() in the generic code despite HPET is Intel specific.
Similarly we can get the current timer with rte_get_timer_cycles(). In the case of TSC, it calls rte_get_tsc_cycles() which is an alias of rte_rdtsc().
Some code is still using directly rte_rdtsc().
There is also rte_rdtsc_precise which adds a memory barrier.
The real question is what is the right abstraction for the application? Do we want the fastest timer? the CPU timer? a precise timer?
I would like to see a real discussion on this topic, in order of building a new timer API which would alias the old one for some time.
If you don't want to bother with all these questions, I suggest to not export the new function rte_rdtsc_arch_hz() and rename it to tsc_arch_hz.
Re: [PATCH v2 5/5] eal/timer: honor architecture specific rdtsc hz function
-----Original Message-----
Quoted 14 lines
Date: Wed, 11 Oct 2017 19:36:11 +0200
From: Thomas Monjalon <[email protected]>
To: Gowrishankar <[email protected]>, Jerin Jacob
<[email protected]>
Cc: [email protected], Chao Zhu <[email protected]>, Bruce Richardson
<[email protected]>, Konstantin Ananyev <[email protected]>, [email protected], [email protected]
Subject: Re: [dpdk-dev] [PATCH v2 5/5] eal/timer: honor architecture
specific rdtsc hz function22/09/2017 10:25, Gowrishankar:
Quoted 2 lines
From: Jerin Jacob <[email protected]>
Quoted 7 lines
Quoted 5 lines
When calibrating the tsc frequency, first, probe the architecture specific rdtsc hz function. if not available, use the existing calibrate scheme to calibrate the tsc frequency.
Signed-off-by Jerin Jacob <[email protected]>I agree on the idea.
OK
Quoted 2 lines
The namespace of cycles related function in DPDK is a real mess.
Absolutely!!
Quoted 22 lines
I think we can choose better names in this series as a first step to tidy this mess.
I will explain below.At first, we should avoid TSC and RDTSC which are Intel-only wording. The generic word could be "cycles" (the word used in arch headers), or "ticks".
We should also name the timer sources or their function in a generic way.
Examples: CPU cycles? fast counter? precise counter?Sometimes we use "hz", sometimes "freq".
It would better to keep one of them.Quoted 9 lines
lib/librte_eal/common/eal_common_timer.c +1
-1@@ -80,8 +80,11 @@void set_tsc_freq (void ) { -uint64_t freq =get_tsc_freq (); +uint64_t freq;
Quoted 3 lines
lib/librte_eal/common/eal_common_timer.c line 85
Quoted 1 line
lib/librte_eal/common/eal_common_timer.c +1
-0+ freq =rte_rdtsc_arch_hz ();This new function is arch-specific and exported as a new API.
I thought of avoid exporting it. But then if the function is in lib/librte_eal/common/include/arch/../rte_cycles.h it is anyway exposed to application. i.e whatever files in lib/librte_eal/common/include/arch/../ anyway exposed to application.
See last comment.
Quoted 22 lines
lib/librte_eal/common/eal_common_timer.c line 87
Quoted 3 lines
lib/librte_eal/common/eal_common_timer.c +2
-0+if (!freq) + freq =get_tsc_freq ();The function
get_tsc_freqis guessing the freq with OS-specific method.lib/librte_eal/common/eal_common_timer.c line 89
Quoted 2 lines
lib/librte_eal/common/eal_common_timer.c +0
-0if (!freq) freq =estimate_tsc_freq ();The function
estimate_tsc_freqis doing an estimation based onsleep().At the end, the most accurate frequency is saved in
eal_tsc_resolution_hzand can be retrieved withrte_get_tsc_hz().
I don't understand whyrte_rdtsc_arch_hz()is also exported to the apps.TSC and HPET timer sources are wrapped in
rte_get_timer_hz()in the Similarly we can get the current timer withrte_get_timer_cycles(). In the case of TSC, it callsrte_get_tsc_cycles()which is an alias ofrte_rdtsc().
Some code is still using directlyrte_rdtsc().
There is alsorte_rdtsc_precisewhich adds a memory barrier.
Quoted 5 lines
The real question is what is the right abstraction for the application? Do we want the fastest timer? the CPU timer? a precise timer?
I would like to see a real discussion on this topic, in order of building a new timer API which would alias the old one for some time.
I guess, we may need to see to how abstract vmware TSC support also in proper way
Quoted 3 lines
If you don't want to bother with all these questions, I suggest to not export the new function
rte_rdtsc_arch_hz()and rename it totsc_arch_hz.
If I understand it correctly, You would like to create a header file in lib/librte_eal/common/include/arch/../ which should not be exported and change the name to tsc_arch_hz.
Re: [PATCH v2 5/5] eal/timer: honor architecture specific rdtsc hz function
11/10/2017 20:57, Jerin Jacob:
Quoted 40 lines
From: Thomas Monjalon <[email protected]>
Quoted 10 lines
22/09/2017 10:25, Gowrishankar:
Quoted 7 lines
From: Jerin Jacob <[email protected]>
When calibrating the tsc frequency, first, probe the architecture specific rdtsc hz function. if not available, use the existing calibrate scheme to calibrate the tsc frequency.
Signed-off-by Jerin Jacob <[email protected]>I agree on the idea.
OK
Quoted 1 line
The namespace of cycles related function in DPDK is a real mess.
Absolutely!!
Quoted 22 lines
I think we can choose better names in this series as a first step to tidy this mess.
I will explain below.At first, we should avoid TSC and RDTSC which are Intel-only wording. The generic word could be "cycles" (the word used in arch headers), or "ticks".
We should also name the timer sources or their function in a generic way.
Examples: CPU cycles? fast counter? precise counter?Sometimes we use "hz", sometimes "freq".
It would better to keep one of them.Quoted 9 lines
lib/librte_eal/common/eal_common_timer.c +1
-1@@ -80,8 +80,11 @@void set_tsc_freq (void ) { -uint64_t freq =get_tsc_freq (); +uint64_t freq;
Quoted 8 lines
Quoted 3 lines
lib/librte_eal/common/eal_common_timer.c line 85
Quoted 1 line
lib/librte_eal/common/eal_common_timer.c +1
-0+ freq =rte_rdtsc_arch_hz ();This new function is arch-specific and exported as a new API.
I thought of avoid exporting it. But then if the function is in
lib/librte_eal/common/include/arch/../rte_cycles.hit is anyway exposed to application. i.e whatever files inlib/librte_eal/common/include/arch/../anyway exposed to application.
Ah yes, you are right!
Quoted 24 lines
See last comment.
Quoted 22 lines
lib/librte_eal/common/eal_common_timer.c line 87
Quoted 3 lines
lib/librte_eal/common/eal_common_timer.c +2
-0+if (!freq) + freq =get_tsc_freq ();The function
get_tsc_freqis guessing the freq with OS-specific method.lib/librte_eal/common/eal_common_timer.c line 89
Quoted 2 lines
lib/librte_eal/common/eal_common_timer.c +0
-0if (!freq) freq =estimate_tsc_freq ();The function
estimate_tsc_freqis doing an estimation based onsleep().At the end, the most accurate frequency is saved in
eal_tsc_resolution_hzand can be retrieved withrte_get_tsc_hz().
I don't understand whyrte_rdtsc_arch_hz()is also exported to the apps.TSC and HPET timer sources are wrapped in
rte_get_timer_hz()in the Similarly we can get the current timer withrte_get_timer_cycles(). In the case of TSC, it callsrte_get_tsc_cycles()which is an alias ofrte_rdtsc().
Some code is still using directlyrte_rdtsc().
There is alsorte_rdtsc_precisewhich adds a memory barrier.
Quoted 8 lines
Quoted 5 lines
The real question is what is the right abstraction for the application? Do we want the fastest timer? the CPU timer? a precise timer?
I would like to see a real discussion on this topic, in order of building a new timer API which would alias the old one for some time.
I guess, we may need to see to how abstract vmware TSC support also in proper way
Yes
Quoted 6 lines
Quoted 2 lines
If you don't want to bother with all these questions, I suggest to not export the new function
rte_rdtsc_arch_hz()and rename it totsc_arch_hz.If I understand it correctly, You would like to create a header file in
lib/librte_eal/common/include/arch/../which should not be exported and change the name totsc_arch_hz.
I had not think about the way to do this.
What about having internal headers in lib/librte_eal/common/arch/ ?
Re: [PATCH v2 5/5] eal/timer: honor architecture specific rdtsc hz function
Quoted 90 lines · Thomas Monjalon
11/10/2017 20:57, Jerin Jacob:
Quoted 40 lines
From: Thomas Monjalon <[email protected]>
Quoted 10 lines
22/09/2017 10:25, Gowrishankar:
Quoted 7 lines
From: Jerin Jacob <[email protected]>
When calibrating the tsc frequency, first, probe the architecture specific rdtsc hz function. if not available, use the existing calibrate scheme to calibrate the tsc frequency.
Signed-off-by Jerin Jacob <[email protected]>I agree on the idea.
OK
Quoted 1 line
The namespace of cycles related function in DPDK is a real mess.
Absolutely!!
Quoted 22 lines
I think we can choose better names in this series as a first step to tidy this mess.
I will explain below.At first, we should avoid TSC and RDTSC which are Intel-only wording. The generic word could be "cycles" (the word used in arch headers), or "ticks".
We should also name the timer sources or their function in a generic way.
Examples: CPU cycles? fast counter? precise counter?Sometimes we use "hz", sometimes "freq".
It would better to keep one of them.Quoted 9 lines
lib/librte_eal/common/eal_common_timer.c +1
-1@@ -80,8 +80,11 @@void set_tsc_freq (void ) { -uint64_t freq =get_tsc_freq (); +uint64_t freq;Quoted 8 lines
Quoted 3 lines
lib/librte_eal/common/eal_common_timer.c line 85
Quoted 1 line
lib/librte_eal/common/eal_common_timer.c +1
-0+ freq =rte_rdtsc_arch_hz ();This new function is arch-specific and exported as a new API.
I thought of avoid exporting it. But then if the function is in
lib/librte_eal/common/include/arch/../rte_cycles.hit is anyway exposed to application. i.e whatever files inlib/librte_eal/common/include/arch/../anyway exposed to application.Ah yes, you are right!
Quoted 24 lines
See last comment.
Quoted 22 lines
lib/librte_eal/common/eal_common_timer.c line 87
Quoted 3 lines
lib/librte_eal/common/eal_common_timer.c +2
-0+if (!freq) + freq =get_tsc_freq ();The function
get_tsc_freqis guessing the freq with OS-specific method.lib/librte_eal/common/eal_common_timer.c line 89
Quoted 2 lines
lib/librte_eal/common/eal_common_timer.c +0
-0if (!freq) freq =estimate_tsc_freq ();The function
estimate_tsc_freqis doing an estimation based onsleep().At the end, the most accurate frequency is saved in
eal_tsc_resolution_hzand can be retrieved withrte_get_tsc_hz().
I don't understand whyrte_rdtsc_arch_hz()is also exported to the apps.TSC and HPET timer sources are wrapped in
rte_get_timer_hz()in the Similarly we can get the current timer withrte_get_timer_cycles(). In the case of TSC, it callsrte_get_tsc_cycles()which is an alias ofrte_rdtsc().
Some code is still using directlyrte_rdtsc().
There is alsorte_rdtsc_precisewhich adds a memory barrier.Quoted 8 lines
Quoted 5 lines
The real question is what is the right abstraction for the application? Do we want the fastest timer? the CPU timer? a precise timer?
I would like to see a real discussion on this topic, in order of building a new timer API which would alias the old one for some time.
I guess, we may need to see to how abstract vmware TSC support also in proper way
Yes
Quoted 3 lines
Quoted 2 lines
If you don't want to bother with all these questions, I suggest to not export the new function
rte_rdtsc_arch_hz()and rename it totsc_arch_hz.
Quoted 7 lines
Quoted 3 lines
If I understand it correctly, You would like to create a header file in
lib/librte_eal/common/include/arch/../which should not be exported and change the name totsc_arch_hz.I had not think about the way to do this.
What about having internal headers inlib/librte_eal/common/arch/?
Yes, this area needs cleanup, but I also think that this patchset (and follow-on patch for x86-specific implementation) does not make things significantly worse than they are now, while also giving significant benefits for users both in terms of improved clock accuracy and reduced startup time (due to not needing sleep). Therefore I'd like to see this merged into 17.11 as a definite improvement, even if it does not "fix" the bigger issues of poor naming etc. I think this is important enough an improvement that we need to see it in the LTS release.
/Bruce
Re: [PATCH v2 5/5] eal/timer: honor architecture specific rdtsc hz function
12/10/2017 10:48, Bruce Richardson:
Quoted 99 lines · Thomas Monjalon
Quoted 90 lines
11/10/2017 20:57, Jerin Jacob:
Quoted 40 lines
From: Thomas Monjalon <[email protected]>
Quoted 10 lines
22/09/2017 10:25, Gowrishankar:
Quoted 7 lines
From: Jerin Jacob <[email protected]>
When calibrating the tsc frequency, first, probe the architecture specific rdtsc hz function. if not available, use the existing calibrate scheme to calibrate the tsc frequency.
Signed-off-by Jerin Jacob <[email protected]>I agree on the idea.
OK
Quoted 1 line
The namespace of cycles related function in DPDK is a real mess.
Absolutely!!
Quoted 22 lines
I think we can choose better names in this series as a first step to tidy this mess.
I will explain below.At first, we should avoid TSC and RDTSC which are Intel-only wording. The generic word could be "cycles" (the word used in arch headers), or "ticks".
We should also name the timer sources or their function in a generic way.
Examples: CPU cycles? fast counter? precise counter?Sometimes we use "hz", sometimes "freq".
It would better to keep one of them.Quoted 9 lines
lib/librte_eal/common/eal_common_timer.c +1
-1@@ -80,8 +80,11 @@void set_tsc_freq (void ) { -uint64_t freq =get_tsc_freq (); +uint64_t freq;Quoted 8 lines
Quoted 3 lines
lib/librte_eal/common/eal_common_timer.c line 85
Quoted 1 line
lib/librte_eal/common/eal_common_timer.c +1
-0+ freq =rte_rdtsc_arch_hz ();This new function is arch-specific and exported as a new API.
I thought of avoid exporting it. But then if the function is in
lib/librte_eal/common/include/arch/../rte_cycles.hit is anyway exposed to application. i.e whatever files inlib/librte_eal/common/include/arch/../anyway exposed to application.Ah yes, you are right!
Quoted 24 lines
See last comment.
Quoted 22 lines
lib/librte_eal/common/eal_common_timer.c line 87
Quoted 3 lines
lib/librte_eal/common/eal_common_timer.c +2
-0+if (!freq) + freq =get_tsc_freq ();The function
get_tsc_freqis guessing the freq with OS-specific method.lib/librte_eal/common/eal_common_timer.c line 89
Quoted 2 lines
lib/librte_eal/common/eal_common_timer.c +0
-0if (!freq) freq =estimate_tsc_freq ();The function
estimate_tsc_freqis doing an estimation based onsleep().At the end, the most accurate frequency is saved in
eal_tsc_resolution_hzand can be retrieved withrte_get_tsc_hz().
I don't understand whyrte_rdtsc_arch_hz()is also exported to the apps.TSC and HPET timer sources are wrapped in
rte_get_timer_hz()in the Similarly we can get the current timer withrte_get_timer_cycles(). In the case of TSC, it callsrte_get_tsc_cycles()which is an alias ofrte_rdtsc().
Some code is still using directlyrte_rdtsc().
There is alsorte_rdtsc_precisewhich adds a memory barrier.Quoted 8 lines
Quoted 5 lines
The real question is what is the right abstraction for the application? Do we want the fastest timer? the CPU timer? a precise timer?
I would like to see a real discussion on this topic, in order of building a new timer API which would alias the old one for some time.
I guess, we may need to see to how abstract vmware TSC support also in proper way
Yes
Quoted 3 lines
Quoted 2 lines
If you don't want to bother with all these questions, I suggest to not export the new function
rte_rdtsc_arch_hz()and rename it totsc_arch_hz.Quoted 7 lines
Quoted 3 lines
If I understand it correctly, You would like to create a header file in
lib/librte_eal/common/include/arch/../which should not be exported and change the name totsc_arch_hz.I had not think about the way to do this.
What about having internal headers inlib/librte_eal/common/arch/?
Quoted 8 lines
Yes, this area needs cleanup, but I also think that this patchset (and follow-on patch for x86-specific implementation) does not make things significantly worse than they are now, while also giving significant benefits for users both in terms of improved clock accuracy and reduced startup time (due to not needing sleep). Therefore I'd like to see this merged into 17.11 as a definite improvement, even if it does not "fix" the bigger issues of poor naming etc. I think this is important enough an improvement that we need to see it in the LTS release.
I agree Bruce.
My initial point was to avoid exporting a new function with a wrong name. It would be better to find a way to avoid this export.
If it cannot be done in 17.11 timeframe, we can at least add the experimental tag.
Re: [PATCH v2 5/5] eal/timer: honor architecture specific rdtsc hz function
In reply to Thomas Monjalon-----Original Message-----
Quoted 101 lines
Date: Wed, 11 Oct 2017 21:25:58 +0200
From: Thomas Monjalon <[email protected]>
To: Jerin Jacob <[email protected]>
Cc: Gowrishankar <[email protected]>, [email protected], Chao
Zhu <[email protected]>, Bruce Richardson <[email protected]>, Konstantin Ananyev <[email protected]>, [email protected], [email protected]
Subject: Re: [dpdk-dev] [PATCH v2 5/5] eal/timer: honor architecture
specific rdtsc hz function11/10/2017 20:57, Jerin Jacob:
Quoted 40 lines
From: Thomas Monjalon <[email protected]>
Quoted 10 lines
22/09/2017 10:25, Gowrishankar:
Quoted 7 lines
From: Jerin Jacob <[email protected]>
When calibrating the tsc frequency, first, probe the architecture specific rdtsc hz function. if not available, use the existing calibrate scheme to calibrate the tsc frequency.
Signed-off-by Jerin Jacob <[email protected]>I agree on the idea.
OK
Quoted 1 line
The namespace of cycles related function in DPDK is a real mess.
Absolutely!!
Quoted 22 lines
I think we can choose better names in this series as a first step to tidy this mess.
I will explain below.At first, we should avoid TSC and RDTSC which are Intel-only wording. The generic word could be "cycles" (the word used in arch headers), or "ticks".
We should also name the timer sources or their function in a generic way.
Examples: CPU cycles? fast counter? precise counter?Sometimes we use "hz", sometimes "freq".
It would better to keep one of them.Quoted 9 lines
lib/librte_eal/common/eal_common_timer.c +1
-1@@ -80,8 +80,11 @@void set_tsc_freq (void ) { -uint64_t freq =get_tsc_freq (); +uint64_t freq;Quoted 8 lines
Quoted 3 lines
lib/librte_eal/common/eal_common_timer.c line 85
Quoted 1 line
lib/librte_eal/common/eal_common_timer.c +1
-0+ freq =rte_rdtsc_arch_hz ();This new function is arch-specific and exported as a new API.
I thought of avoid exporting it. But then if the function is in
lib/librte_eal/common/include/arch/../rte_cycles.hit is anyway exposed to application. i.e whatever files inlib/librte_eal/common/include/arch/../anyway exposed to application.Ah yes, you are right!
Quoted 24 lines
See last comment.
Quoted 22 lines
lib/librte_eal/common/eal_common_timer.c line 87
Quoted 3 lines
lib/librte_eal/common/eal_common_timer.c +2
-0+if (!freq) + freq =get_tsc_freq ();The function
get_tsc_freqis guessing the freq with OS-specific method.lib/librte_eal/common/eal_common_timer.c line 89
Quoted 2 lines
lib/librte_eal/common/eal_common_timer.c +0
-0if (!freq) freq =estimate_tsc_freq ();The function
estimate_tsc_freqis doing an estimation based onsleep().At the end, the most accurate frequency is saved in
eal_tsc_resolution_hzand can be retrieved withrte_get_tsc_hz().
I don't understand whyrte_rdtsc_arch_hz()is also exported to the apps.TSC and HPET timer sources are wrapped in
rte_get_timer_hz()in the Similarly we can get the current timer withrte_get_timer_cycles(). In the case of TSC, it callsrte_get_tsc_cycles()which is an alias ofrte_rdtsc().
Some code is still using directlyrte_rdtsc().
There is alsorte_rdtsc_precisewhich adds a memory barrier.Quoted 8 lines
Quoted 5 lines
The real question is what is the right abstraction for the application? Do we want the fastest timer? the CPU timer? a precise timer?
I would like to see a real discussion on this topic, in order of building a new timer API which would alias the old one for some time.
I guess, we may need to see to how abstract vmware TSC support also in proper way
Yes
Quoted 3 lines
Quoted 2 lines
If you don't want to bother with all these questions, I suggest to not export the new function
rte_rdtsc_arch_hz()and rename it totsc_arch_hz.
Quoted 6 lines
Quoted 3 lines
If I understand it correctly, You would like to create a header file in
lib/librte_eal/common/include/arch/../which should not be exported and change the name totsc_arch_hz.I had not think about the way to do this.
What about having internal headers inlib/librte_eal/common/arch/?
Looks bit odd when compare with existing scheme of arch function management. Anyway the whole timer stuff needs to be cleanup.
I think, we can take that when do the cleanup.
On the other side, This API has real ARM64, PPC, x86 implementation now. I think,it is important to have this feature for v17.11
Quoted 1 line
Re: [PATCH v2 5/5] eal/timer: honor architecture specific rdtsc hz function
In reply to Thomas MonjalonQuoted 109 lines · Thomas Monjalon
12/10/2017 10:48, Bruce Richardson:
Quoted 99 lines · Thomas Monjalon
Quoted 90 lines
11/10/2017 20:57, Jerin Jacob:
Quoted 40 lines
From: Thomas Monjalon <[email protected]>
Quoted 10 lines
22/09/2017 10:25, Gowrishankar:
Quoted 7 lines
From: Jerin Jacob <[email protected]>
When calibrating the tsc frequency, first, probe the architecture specific rdtsc hz function. if not available, use the existing calibrate scheme to calibrate the tsc frequency.
Signed-off-by Jerin Jacob <[email protected]>I agree on the idea.
OK
Quoted 1 line
The namespace of cycles related function in DPDK is a real mess.
Absolutely!!
Quoted 22 lines
I think we can choose better names in this series as a first step to tidy this mess.
I will explain below.At first, we should avoid TSC and RDTSC which are Intel-only wording. The generic word could be "cycles" (the word used in arch headers), or "ticks".
We should also name the timer sources or their function in a generic way.
Examples: CPU cycles? fast counter? precise counter?Sometimes we use "hz", sometimes "freq".
It would better to keep one of them.Quoted 9 lines
lib/librte_eal/common/eal_common_timer.c +1
-1@@ -80,8 +80,11 @@void set_tsc_freq (void ) { -uint64_t freq =get_tsc_freq (); +uint64_t freq;Quoted 8 lines
Quoted 3 lines
lib/librte_eal/common/eal_common_timer.c line 85
Quoted 1 line
lib/librte_eal/common/eal_common_timer.c +1
-0+ freq =rte_rdtsc_arch_hz ();This new function is arch-specific and exported as a new API.
I thought of avoid exporting it. But then if the function is in
lib/librte_eal/common/include/arch/../rte_cycles.hit is anyway exposed to application. i.e whatever files inlib/librte_eal/common/include/arch/../anyway exposed to application.Ah yes, you are right!
Quoted 24 lines
See last comment.
Quoted 22 lines
lib/librte_eal/common/eal_common_timer.c line 87
Quoted 3 lines
lib/librte_eal/common/eal_common_timer.c +2
-0+if (!freq) + freq =get_tsc_freq ();The function
get_tsc_freqis guessing the freq with OS-specific method.lib/librte_eal/common/eal_common_timer.c line 89
Quoted 2 lines
lib/librte_eal/common/eal_common_timer.c +0
-0if (!freq) freq =estimate_tsc_freq ();The function
estimate_tsc_freqis doing an estimation based onsleep().At the end, the most accurate frequency is saved in
eal_tsc_resolution_hzand can be retrieved withrte_get_tsc_hz().
I don't understand whyrte_rdtsc_arch_hz()is also exported to the apps.TSC and HPET timer sources are wrapped in
rte_get_timer_hz()in the Similarly we can get the current timer withrte_get_timer_cycles(). In the case of TSC, it callsrte_get_tsc_cycles()which is an alias ofrte_rdtsc().
Some code is still using directlyrte_rdtsc().
There is alsorte_rdtsc_precisewhich adds a memory barrier.Quoted 8 lines
Quoted 5 lines
The real question is what is the right abstraction for the application? Do we want the fastest timer? the CPU timer? a precise timer?
I would like to see a real discussion on this topic, in order of building a new timer API which would alias the old one for some time.
I guess, we may need to see to how abstract vmware TSC support also in proper way
Yes
Quoted 3 lines
Quoted 2 lines
If you don't want to bother with all these questions, I suggest to not export the new function
rte_rdtsc_arch_hz()and rename it totsc_arch_hz.Quoted 7 lines
Quoted 3 lines
If I understand it correctly, You would like to create a header file in
lib/librte_eal/common/include/arch/../which should not be exported and change the name totsc_arch_hz.I had not think about the way to do this.
What about having internal headers inlib/librte_eal/common/arch/?Quoted 8 lines
Yes, this area needs cleanup, but I also think that this patchset (and follow-on patch for x86-specific implementation) does not make things significantly worse than they are now, while also giving significant benefits for users both in terms of improved clock accuracy and reduced startup time (due to not needing sleep). Therefore I'd like to see this merged into 17.11 as a definite improvement, even if it does not "fix" the bigger issues of poor naming etc. I think this is important enough an improvement that we need to see it in the LTS release.
Quoted 5 lines
I agree Bruce.
My initial point was to avoid exporting a new function with a wrong name. It would be better to find a way to avoid this export.
If it cannot be done in 17.11 timeframe, we can at least add the experimental tag.
Ah, ok. That sounds fine.
/Bruce
Re: [PATCH v2 2/5] eal/ppc64: define architecture specific rdtsc hz
In reply to Gowrishankar22/09/2017 10:25, Gowrishankar:
Quoted 9 lines
From: Jerin Jacob <[email protected]>
In
ppc_64,rte_rdtsc()returns timebase register value which increments at independent timebase frequency and hence not related to lcore cpu frequency to derive TSC hz. Hence, we stick with master lcore frequency.CC Chao Zhu <[email protected]>Signed-off-by Jerin Jacob <[email protected]>Signed-off-by Gowrishankar Muthukrishnan <[email protected]>
[...]
lib/librte_eal/common/include/arch/ppc_64/rte_cycles.h line 41
Quoted 7 lines
lib/librte_eal/common/include/arch/ppc_64/rte_cycles.h +1
-0@@ -38,9 +38,13 @@#endif #include "generic/rte_cycles.h" +#include "../../lib/librte_eal/common/eal_filesystem.h"
rte_cycles.h is an installed header.eal_filesystem.h is not exported.
It cannot work.
cover improve tsc frequency calibration
In reply to Gowrishankarv3 changes:
- implement in .c files instead of exporting an inline arch function
- rename arch function from rte_rdtsc_arch_hz to get_tsc_freq_arch
- integrate x86 implementation in the series
- fix private EAL include in PPC implementation (not tested)
From: Gowrishankar Muthukrishnan <[email protected]>
Some architecture like armv8 provides an architecture specific function to get the rdtsc frequency. The existing rdtsc calibration scheme uses OS serivce like sleep(1) to calibrate the frequency which may not produce the accurate result. Introducing an architecture specific hook to get the rdtsc frequency if architecture provides it. If not, use the exiting the calibrate scheme to get the rdtsc frequency.
Jerin Jacob (3):
timer: honor arch-specific TSC frequency query eal/armv8: implement arch-specific TSC freq query eal/ppc64: implement arch-specific TSC freq query
Sergio Gonzalez Monroy (1):
eal/x86: implement arch-specific TSC freq query
lib/librte_eal/bsdapp/eal/Makefile | 1 +
lib/librte_eal/common/arch/arm/rte_cycles.c | 45 +++++++
.../ppc_64/rte_cycles.c} | 78 ++---------
.../{eal_common_timer.c => arch/x86/rte_cycles.c} | 143 ++++++++++++++-------
lib/librte_eal/common/eal_common_timer.c | 5 +-
lib/librte_eal/common/eal_private.h | 11 ++
lib/librte_eal/linuxapp/eal/Makefile | 1 +
7 files changed, 170 insertions(+), 114 deletions(-)
create mode 100644 lib/librte_eal/common/arch/arm/rte_cycles.c
copy lib/librte_eal/common/{eal_common_timer.c => arch/ppc_64/rte_cycles.c} (50%)
copy lib/librte_eal/common/{eal_common_timer.c => arch/x86/rte_cycles.c} (50%)Signature · 1 line
2.14.1
PATCH 1/4 timer: honor arch-specific TSC frequency query
From: Jerin Jacob <[email protected]>
When calibrating the TSC frequency, first, probe the architecture specific function. If not available, use the existing calibrate scheme.
--- lib/librte_eal/bsdapp/eal/Makefile | 1 + lib/librte_eal/common/arch/arm/rte_cycles.c | 7 +++++++ lib/librte_eal/common/arch/ppc_64/rte_cycles.c | 7 +++++++ lib/librte_eal/common/arch/x86/rte_cycles.c | 7 +++++++ lib/librte_eal/common/eal_common_timer.c | 5 ++++- lib/librte_eal/common/eal_private.h | 11 +++++++++++ lib/librte_eal/linuxapp/eal/Makefile | 1 + 7 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 lib/librte_eal/common/arch/arm/rte_cycles.c create mode 100644 lib/librte_eal/common/arch/ppc_64/rte_cycles.c create mode 100644 lib/librte_eal/common/arch/x86/rte_cycles.c
lib/librte_eal/bsdapp/eal/Makefile +1 -0
@@ -92,6 +92,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += rte_service.c # from arch dir SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += rte_cpuflags.c SRCS-$(CONFIG_RTE_ARCH_X86) += rte_spinlock.c +SRCS-y += rte_cycles.c CFLAGS_eal_common_cpuflags.o := $(CPUFLAGS_LIST)
lib/librte_eal/common/arch/arm/rte_cycles.c new file +7 -0
@@ -0,0 +1,7 @@ +#include "eal_private.h" + +uint64_t +get_tsc_freq_arch (void ) +{ +return 0 ; +}
lib/librte_eal/common/arch/ppc_64/rte_cycles.c new file +7 -0
@@ -0,0 +1,7 @@ +#include "eal_private.h" + +uint64_t +get_tsc_freq_arch (void ) +{ +return 0 ; +}
lib/librte_eal/common/arch/x86/rte_cycles.c new file +7 -0
@@ -0,0 +1,7 @@ +#include "eal_private.h" + +uint64_t +get_tsc_freq_arch (void ) +{ +return 0 ; +}
lib/librte_eal/common/eal_common_timer.c +4 -1
@@ -80,8 +80,11 @@ estimate_tsc_freq(void)void set_tsc_freq (void ) { -uint64_t freq =get_tsc_freq (); +uint64_t freq; + freq =get_tsc_freq_arch (); +if (!freq) + freq =get_tsc_freq ();if (!freq) freq =estimate_tsc_freq ();
lib/librte_eal/common/eal_private.h +11 -0
@@ -314,6 +314,17 @@ void set_tsc_freq(void);*/ uint64_t get_tsc_freq (void ); +/** +* Get TSC frequency if the architecture supports. +* +* This function is private to the EAL. +* +* @return +* The number of TSC cycles in one second. +* Returns zero if the architecture support is not available. +*/ +uint64_t get_tsc_freq_arch (void ); +/** * Prepare physical memory mapping * i.e. hugepages on Linux and
lib/librte_eal/linuxapp/eal/Makefile +1 -0
@@ -101,6 +101,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += rte_service.c # from arch dir SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += rte_cpuflags.c SRCS-$(CONFIG_RTE_ARCH_X86) += rte_spinlock.c +SRCS-y += rte_cycles.c CFLAGS_eal_common_cpuflags.o := $(CPUFLAGS_LIST)
Signature · 1 line
2.14.1
PATCH 2/4 eal/armv8: implement arch-specific TSC freq query
In reply to Thomas MonjalonFrom: Jerin Jacob <[email protected]>
Use cntvct_el0 system register to get the system counter frequency.
If the system is configured with RTE_ARM_EAL_RDTSC_USE_PMU then return 0(let the common code calibrate the tsc frequency).
--- lib/librte_eal/common/arch/arm/rte_cycles.c | 38 +++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
lib/librte_eal/common/arch/arm/rte_cycles.c +38 -0
@@ -1,7 +1,45 @@ +/* +* BSD LICENSE +* +* Copyright (C) Cavium, Inc. 2015. +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions +* are met: +* +* * Redistributions of source code must retain the above copyright +* notice, this list of conditions and the following disclaimer. +* * Redistributions in binary form must reproduce the above copyright +* notice, this list of conditions and the following disclaimer in +* the documentation and/or other materials provided with the +* distribution. +* * Neither the name of Cavium, Inc nor the names of its +* contributors may be used to endorse or promote products derived +* from this software without specific prior written permission. +* +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*/ +#include "eal_private.h" uint64_t get_tsc_freq_arch (void ) { +#if definedRTE_ARCH_ARM64 && !definedRTE_ARM_EAL_RDTSC_USE_PMU +uint64_t freq; + asmvolatile ("mrs %0, cntfrq_el0" :"=r" (freq)); +return freq; +#else return 0 ; +#endif }
Signature · 1 line
2.14.1
PATCH 3/4 eal/ppc64: implement arch-specific TSC freq query
In reply to Thomas MonjalonFrom: Jerin Jacob <[email protected]>
In ppc_64, rte_rdtsc() returns timebase register value which increments at independent timebase frequency and hence not related to lcore cpu frequency to derive TSC hz. Hence, we stick with master lcore frequency.
--- lib/librte_eal/common/arch/ppc_64/rte_cycles.c | 47 +++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-)
lib/librte_eal/common/arch/ppc_64/rte_cycles.c +46 -1
@@ -1,7 +1,52 @@ +/* +* BSD LICENSE +* +* Copyright (C) IBM Corporation 2014. +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions +* are met: +* +* * Redistributions of source code must retain the above copyright +* notice, this list of conditions and the following disclaimer. +* * Redistributions in binary form must reproduce the above copyright +* notice, this list of conditions and the following disclaimer in +* the documentation and/or other materials provided with the +* distribution. +* * Neither the name of IBM Corporation nor the names of its +* contributors may be used to endorse or promote products derived +* from this software without specific prior written permission. +* +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*/ + +#include <rte_lcore.h> +#include <rte_log.h> +#include "eal_filesystem.h" #include "eal_private.h" +static const char sys_cpu_dir[] ="/sys/devices/system/cpu" ; +uint64_t get_tsc_freq_arch (void ) { -return 0 ; +unsigned long cpu_hz; +char path[PATH_MAX ]; + +snprintf (path,sizeof (path),"%s/cpu%d/cpufreq/cpuinfo_cur_freq" , + sys_cpu_dir,rte_get_master_lcore ()); +if (eal_parse_sysfs_value (path, &cpu_hz) <0 ) +RTE_LOG (WARNING ,EAL ,"Unable to parse %s\n" , path); + +return cpu_hz*1000 ; }
Signature · 1 line
2.14.1
PATCH 4/4 eal/x86: implement arch-specific TSC freq query
In reply to Thomas MonjalonFrom: Sergio Gonzalez Monroy <[email protected]>
First, try to use CPUID Time Stamp Counter and Nominal Core Crystal Clock Information Leaf to determine the tsc hz on platforms that supports it (does not require privileged user).
If the CPUID leaf is not available, then try to determine the tsc hz by reading the MSR 0xCE (requires privileged user).
Default to the tsc hz estimation if both methods fail.
--- lib/librte_eal/common/arch/x86/rte_cycles.c | 142 +++++++++++++++++++++++++++- 1 file changed, 141 insertions(+), 1 deletion(-)
lib/librte_eal/common/arch/x86/rte_cycles.c +141 -1
@@ -1,7 +1,147 @@ +/*- +* BSD LICENSE +* +* Copyright(c) 2017 Intel Corporation. All rights reserved. +* All rights reserved. +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions +* are met: +* +* * Redistributions of source code must retain the above copyright +* notice, this list of conditions and the following disclaimer. +* * Redistributions in binary form must reproduce the above copyright +* notice, this list of conditions and the following disclaimer in +* the documentation and/or other materials provided with the +* distribution. +* * Neither the name of Intel Corporation nor the names of its +* contributors may be used to endorse or promote products derived +* from this software without specific prior written permission. +* +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*/ + +#include <fcntl.h> +#include <unistd.h> +#include <cpuid.h> +#include "eal_private.h" +static unsigned int +rte_cpu_get_model (uint32_t fam_mod_step) +{ +uint32_t family, model, ext_model; + + family = (fam_mod_step >>8 ) &0xf ; + model = (fam_mod_step >>4 ) &0xf ; + +if (family ==6 || family ==15 ) { + ext_model = (fam_mod_step >>16 ) &0xf ; + model += (ext_model <<4 ); + } + +return model; +} + +static int32_t +rdmsr (int msr,uint64_t *val) +{ +#ifdef RTE_EXEC_ENV_LINUXAPP +int fd; +int ret; + + fd =open ("/dev/cpu/0/msr" ,O_RDONLY ); +if (fd <0 ) +return fd; + + ret =pread (fd, val,sizeof (uint64_t ), msr); + +close (fd); + +return ret; +#else +return -1 ; +#endif +} + +static uint32_t +check_model_wsm_nhm (uint8_t model) +{ +switch (model) { +/* Westmere */ +case 0x25 : +case 0x2C : +case 0x2F : +/* Nehalem */ +case 0x1E : +case 0x1F : +case 0x1A : +case 0x2E : +return 1 ; + } + +return 0 ; +} + +static uint32_t +check_model_gdm_dnv (uint8_t model) +{ +switch (model) { +/* Goldmont */ +case 0x5C : +/* Denverton */ +case 0x5F : +return 1 ; + } + +return 0 ; +} +uint64_t get_tsc_freq_arch (void ) { -return 0 ; +uint64_t tsc_hz =0 ; +uint32_t a, b, c, d, maxleaf; +uint8_t mult, model; +int32_t ret; + +/* +* Time Stamp Counter and Nominal Core Crystal Clock +* Information Leaf +*/ + maxleaf =__get_cpuid_max (0 ,NULL ); + +if (maxleaf >=0x15 ) { +__cpuid (0x15 , a, b, c, d); + +/* EBX : TSC/Crystal ratio, ECX : Crystal Hz */ +if (b && c) +return c * (b / a); + } + +__cpuid (0x1 , a, b, c, d); + model =rte_cpu_get_model (a); + +if (check_model_wsm_nhm (model)) + mult =133 ; +else if ((c & bit_AVX) ||check_model_gdm_dnv (model)) + mult =100 ; +else +return 0 ; + + ret =rdmsr (0xCE , &tsc_hz); +if (ret <0 ) +return 0 ; + +return ((tsc_hz >>8 ) &0xff ) * mult *1E6 ; }
Signature · 1 line
2.14.1
-----Original Message-----
Quoted 9 lines
Date: Fri, 13 Oct 2017 02:02:43 +0200
From: Thomas Monjalon <[email protected]>
To: [email protected], [email protected],
[email protected], [email protected], [email protected]
Cc: [email protected]
Subject: [PATCH 0/4] improve tsc frequency calibration
X-Mailer: git-send-email 2.14.1
Quoted 5 lines
v3 changes:
- implement in .c files instead of exporting an inline arch function
- rename arch function fromrte_rdtsc_arch_hztoget_tsc_freq_arch
- integrate x86 implementation in the series
- fix private EAL include in PPC implementation (not tested)
Thanks Thomas.
Tested on a arm64 machine.
Quoted 34 lines
From: Gowrishankar Muthukrishnan <[email protected]>
Some architecture like armv8 provides an architecture specific function to get the rdtsc frequency. The existing rdtsc calibration scheme uses OS serivce like sleep(1) to calibrate the frequency which may not produce the accurate result. Introducing an architecture specific hook to get the rdtsc frequency if architecture provides it. If not, use the exiting the calibrate scheme to get the rdtsc frequency.
Jerin Jacob (3):
timer: honor arch-specific TSC frequency query eal/armv8: implement arch-specific TSC freq query eal/ppc64: implement arch-specific TSC freq querySergio Gonzalez Monroy (1):
eal/x86: implement arch-specific TSC freq querylib/librte_eal/bsdapp/eal/Makefile | 1 + lib/librte_eal/common/arch/arm/rte_cycles.c | 45 +++++++ .../ppc_64/rte_cycles.c} | 78 ++--------- .../{eal_common_timer.c => arch/x86/rte_cycles.c} | 143 ++++++++++++++------- lib/librte_eal/common/eal_common_timer.c | 5 +- lib/librte_eal/common/eal_private.h | 11 ++ lib/librte_eal/linuxapp/eal/Makefile | 1 + 7 files changed, 170 insertions(+), 114 deletions(-) create mode 100644 lib/librte_eal/common/arch/arm/rte_cycles.c copy lib/librte_eal/common/{eal_common_timer.c => arch/ppc_64/rte_cycles.c} (50%) copy lib/librte_eal/common/{eal_common_timer.c => arch/x86/rte_cycles.c} (50%)Signature · 1 line
2.14.1
Re: [PATCH 4/4] eal/x86: implement arch-specific TSC freq query
In reply to Thomas MonjalonQuoted 8 lines · On Fri, 13 Oct 2017 02:02:47 +0200 Thomas Monjalon <[email protected]> wrote:
lib/librte_eal/common/arch/x86/rte_cycles.c +8
-0+static uint32_t +check_model_wsm_nhm (uint8_t model) +{ +switch (model) { +/* Westmere */ +case 0x25 : +case 0x2C : +case 0x2F :
lib/librte_eal/common/arch/x86/rte_cycles.c line 94
Quoted 10 lines
lib/librte_eal/common/arch/x86/rte_cycles.c +10
-0+/* Nehalem */ +case 0x1E : +case 0x1F : +case 0x1A : +case 0x2E : +return 1 ; + } + +return 0 ; +}
How about a table rather than switch?
Also bool rather than int?
Re: [PATCH 4/4] eal/x86: implement arch-specific TSC freq query
13/10/2017 05:47, Stephen Hemminger:
Quoted 16 lines
Quoted 13 lines · On Fri, 13 Oct 2017 02:02:47 +0200 Thomas Monjalon <[email protected]> wrote:
lib/librte_eal/common/arch/x86/rte_cycles.c +13
-0+static uint32_t +check_model_wsm_nhm (uint8_t model) +{ +switch (model) { +/* Westmere */ +case 0x25 : +case 0x2C : +case 0x2F : +/* Nehalem */ +case 0x1E : +case 0x1F : +case 0x1A : +case 0x2E :
Quoted 8 lines
lib/librte_eal/common/arch/x86/rte_cycles.c line 94
Quoted 5 lines
lib/librte_eal/common/arch/x86/rte_cycles.c +5
-0+return 1 ; + } + +return 0 ; +}How about a table rather than switch?
Also bool rather than int?
It is a matter of taste :)
I plan to push it as is. It can be changed later if desired.
Thomas Monjalon
AppliedQuoted 11 lines
Quoted 5 lines
v3 changes:
- implement in .c files instead of exporting an inline arch function
- rename arch function fromrte_rdtsc_arch_hztoget_tsc_freq_arch
- integrate x86 implementation in the series
- fix private EAL include in PPC implementation (not tested)Thanks Thomas.
Tested on a arm64 machine.
Tested-by Jerin Jacob <[email protected]>
Applied
Re: [PATCH 4/4] eal/x86: implement arch-specific TSC freq query
In reply to Thomas MonjalonQuoted 26 lines · On Fri, 13 Oct 2017 09:30:43 +0200 Thomas Monjalon <[email protected]> wrote:
13/10/2017 05:47, Stephen Hemminger:
Quoted 16 lines
Quoted 13 lines · On Fri, 13 Oct 2017 02:02:47 +0200 Thomas Monjalon <[email protected]> wrote:
lib/librte_eal/common/arch/x86/rte_cycles.c +13
-0+static uint32_t +check_model_wsm_nhm (uint8_t model) +{ +switch (model) { +/* Westmere */ +case 0x25 : +case 0x2C : +case 0x2F : +/* Nehalem */ +case 0x1E : +case 0x1F : +case 0x1A : +case 0x2E :Quoted 8 lines
lib/librte_eal/common/arch/x86/rte_cycles.c line 94
Quoted 5 lines
lib/librte_eal/common/arch/x86/rte_cycles.c +5
-0+return 1 ; + } + +return 0 ; +}How about a table rather than switch?
Also bool rather than int?
Quoted 2 lines
It is a matter of taste :)
I plan to push it as is. It can be changed later if desired.
Also using #define's rather than magic constants would help.
Re: [PATCH 4/4] eal/x86: implement arch-specific TSC freq query
Quoted 29 lines · Stephen Hemminger
Quoted 26 lines · On Fri, 13 Oct 2017 09:30:43 +0200 Thomas Monjalon <[email protected]> wrote:
13/10/2017 05:47, Stephen Hemminger:
Quoted 24 lines
Quoted 8 lines · On Fri, 13 Oct 2017 02:02:47 +0200 Thomas Monjalon <[email protected]> wrote:
lib/librte_eal/common/arch/x86/rte_cycles.c +8
-0+static uint32_t +check_model_wsm_nhm (uint8_t model) +{ +switch (model) { +/* Westmere */ +case 0x25 : +case 0x2C : +case 0x2F :lib/librte_eal/common/arch/x86/rte_cycles.c line 94
Quoted 10 lines
lib/librte_eal/common/arch/x86/rte_cycles.c +10
-0+/* Nehalem */ +case 0x1E : +case 0x1F : +case 0x1A : +case 0x2E : +return 1 ; + } + +return 0 ; +}How about a table rather than switch?
Also bool rather than int?
Quoted 4 lines
Quoted 2 lines
It is a matter of taste :)
I plan to push it as is. It can be changed later if desired.Also using #define's rather than magic constants would help.
Assuming that we could just come up with more meaningful names that help more than just have a comment indicating what uarch's each set of numbers is for. :-)
For me, I don't think #defines are needed to enhance readability here.