[dpdk-dev] [PATCH v2] eal/ppc64: improve rte_rdtsc with ppc_get_timebase

[Thinh Tran]

Hi David, Thanks for your response.

We were worry about breaking the DPDK/FreeBSD on power, may hit the 
similar bug:
   https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=241146

 From my understanding, and also from the documentation
https://doc.dpdk.org/guides/freebsd_gsg/install_from_ports.html
DPDK is supported, glibc may not be required by default, and it is not 
clear what the supported architectures are.

So if FreeBSD does not support DPDK on powerpc or
FreeBSD supports DPDK on powerpc and requires glibc installed,
then the GLIBC guard can be removed

This is unknown to me.

Agreed. __powerpc__ guard is not necessary in this path and will be removed.

Thanks,
Thinh Tran

On 3/10/2020 7:55 AM, David Marchand wrote:
> On Fri, Jan 31, 2020 at 11:04 PM Thinh Tran <thinhtr at linux.vnet.ibm.com> wrote:
>>
>>    __ppc_get_timebase() is GNU extension and is more efficient
>>
>>    v2: Advoid breaking other ppc_64 flatforms. The __ppc_get_timebase()
>>        seems to be specific to powerpc platform and with GLIBC.
> 
> dpdk only supports glibc at the moment.
> https://doc.dpdk.org/guides/linux_gsg/sys_reqs.html#system-software
> 
> Now the 1M$ question is which C library are you using and on which platform :-).
> 
>>
>> Signed-off-by: Thinh Tran <thinhtr at linux.vnet.ibm.com>
>> ---
>>   lib/librte_eal/common/include/arch/ppc_64/rte_cycles.h | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/lib/librte_eal/common/include/arch/ppc_64/rte_cycles.h b/lib/librte_eal/common/include/arch/ppc_64/rte_cycles.h
>> index 8f2e98642..1c3fd556e 100644
>> --- a/lib/librte_eal/common/include/arch/ppc_64/rte_cycles.h
>> +++ b/lib/librte_eal/common/include/arch/ppc_64/rte_cycles.h
>> @@ -14,6 +14,9 @@ extern "C" {
>>
>>   #include <rte_byteorder.h>
>>   #include <rte_common.h>
>> +#if defined(__powerpc__) && defined(__GLIBC__)
>> +#include <sys/platform/ppc.h>
>> +#endif
> 
> libc headers must come first:
> https://doc.dpdk.org/guides/contributing/coding_style.html#header-includes
> 
> Why do you need the __powerpc__ guard?
> 
>>
>>   /**
>>    * Read the time base register.
>> @@ -24,6 +27,9 @@ extern "C" {
>>   static inline uint64_t
>>   rte_rdtsc(void)
>>   {
>> +#if defined(__powerpc__) && defined(__GLIBC__)
>> +       return __ppc_get_timebase();
>> +#else
>>          union {
>>                  uint64_t tsc_64;
>>                  RTE_STD_C11
>> @@ -50,6 +56,7 @@ rte_rdtsc(void)
>>                          [tmp] "=r"(tmp)
>>                      );
>>          return tsc.tsc_64;
>> +#endif /* __powerpc__ && __GLIBC__ */
>>   }
>>
>>   static inline uint64_t
>> --
>> 2.17.1
>>
> 
>