[v3] eal/ppc64: improve rte_rdtsc() with __ppc_get_timebase()

Message ID 20200325001356.147049-1-thinhtr@linux.vnet.ibm.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series [v3] eal/ppc64: improve rte_rdtsc() with __ppc_get_timebase() |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/iol-intel-Performance fail Performance Testing issues
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS

Commit Message

Thinh Tran March 25, 2020, 12:13 a.m. UTC
  __ppc_get_timebase() reads and returns the current value of the Time 
  Base Register. It's more efficient as it uses the processor’s time  
  base facility directly 

  the DPDK on FreeBSD currently is not supported on Powerpc64, it should
  be safe to include the sys/platform/ppc.h


Signed-off-by: Thinh Tran <thinhtr@linux.vnet.ibm.com>
---
 .../common/include/arch/ppc_64/rte_cycles.h   | 28 ++-----------------
 1 file changed, 2 insertions(+), 26 deletions(-)
  

Comments

Thomas Monjalon April 16, 2020, 10:35 p.m. UTC | #1
Any review please?

25/03/2020 01:13, Thinh Tran:
>   __ppc_get_timebase() reads and returns the current value of the Time 
>   Base Register. It's more efficient as it uses the processor’s time  
>   base facility directly 
> 
>   the DPDK on FreeBSD currently is not supported on Powerpc64, it should
>   be safe to include the sys/platform/ppc.h
> 
> 
> Signed-off-by: Thinh Tran <thinhtr@linux.vnet.ibm.com>
> ---
>  .../common/include/arch/ppc_64/rte_cycles.h   | 28 ++-----------------
>  1 file changed, 2 insertions(+), 26 deletions(-)
> 
> 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..871f9b6e4 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,7 @@ extern "C" {
>  
>  #include <rte_byteorder.h>
>  #include <rte_common.h>
> +#include <sys/platform/ppc.h>
>  
>  /**
>   * Read the time base register.
> @@ -24,32 +25,7 @@ extern "C" {
>  static inline uint64_t
>  rte_rdtsc(void)
>  {
> -	union {
> -		uint64_t tsc_64;
> -		RTE_STD_C11
> -		struct {
> -#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> -			uint32_t hi_32;
> -			uint32_t lo_32;
> -#else
> -			uint32_t lo_32;
> -			uint32_t hi_32;
> -#endif
> -		};
> -	} tsc;
> -	uint32_t tmp;
> -
> -	asm volatile(
> -			"0:\n"
> -			"mftbu   %[hi32]\n"
> -			"mftb    %[lo32]\n"
> -			"mftbu   %[tmp]\n"
> -			"cmpw    %[tmp],%[hi32]\n"
> -			"bne     0b\n"
> -			: [hi32] "=r"(tsc.hi_32), [lo32] "=r"(tsc.lo_32),
> -			[tmp] "=r"(tmp)
> -		    );
> -	return tsc.tsc_64;
> +	return __ppc_get_timebase();
>  }
>  
>  static inline uint64_t
  
David Christensen April 20, 2020, 4:42 p.m. UTC | #2
On 3/24/20 5:13 PM, Thinh Tran wrote:
>    __ppc_get_timebase() reads and returns the current value of the Time
>    Base Register. It's more efficient as it uses the processor’s time
>    base facility directly
> 
>    the DPDK on FreeBSD currently is not supported on Powerpc64, it should
>    be safe to include the sys/platform/ppc.h
> 
> 
> Signed-off-by: Thinh Tran <thinhtr@linux.vnet.ibm.com>
> ---
>   .../common/include/arch/ppc_64/rte_cycles.h   | 28 ++-----------------
>   1 file changed, 2 insertions(+), 26 deletions(-)
> 
> 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..871f9b6e4 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

Patched file location has moved, now lib/librte_eal/ppc/include/* as a 
result of commit a1b6cda16a07, otherwise it's good to go.

Reviewed-by: David Christensen (drc@linux.vnet.ibm.com)
  
David Marchand April 21, 2020, 4:17 p.m. UTC | #3
On Wed, Mar 25, 2020 at 1:17 AM Thinh Tran <thinhtr@linux.vnet.ibm.com> wrote:
>
>   __ppc_get_timebase() reads and returns the current value of the Time
>   Base Register. It's more efficient as it uses the processor’s time
>   base facility directly
>
>   the DPDK on FreeBSD currently is not supported on Powerpc64, it should
>   be safe to include the sys/platform/ppc.h
>
>
> Signed-off-by: Thinh Tran <thinhtr@linux.vnet.ibm.com>

Reviewed-by: David Christensen <drc@linux.vnet.ibm.com>

Applied, thanks.
  

Patch

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..871f9b6e4 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,7 @@  extern "C" {
 
 #include <rte_byteorder.h>
 #include <rte_common.h>
+#include <sys/platform/ppc.h>
 
 /**
  * Read the time base register.
@@ -24,32 +25,7 @@  extern "C" {
 static inline uint64_t
 rte_rdtsc(void)
 {
-	union {
-		uint64_t tsc_64;
-		RTE_STD_C11
-		struct {
-#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
-			uint32_t hi_32;
-			uint32_t lo_32;
-#else
-			uint32_t lo_32;
-			uint32_t hi_32;
-#endif
-		};
-	} tsc;
-	uint32_t tmp;
-
-	asm volatile(
-			"0:\n"
-			"mftbu   %[hi32]\n"
-			"mftb    %[lo32]\n"
-			"mftbu   %[tmp]\n"
-			"cmpw    %[tmp],%[hi32]\n"
-			"bne     0b\n"
-			: [hi32] "=r"(tsc.hi_32), [lo32] "=r"(tsc.lo_32),
-			[tmp] "=r"(tmp)
-		    );
-	return tsc.tsc_64;
+	return __ppc_get_timebase();
 }
 
 static inline uint64_t