eal/x86: fix segfaults in waitpkg power intrinsics

Message ID 20231107161900.46058-1-bruce.richardson@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series eal/x86: fix segfaults in waitpkg power intrinsics |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS

Commit Message

Bruce Richardson Nov. 7, 2023, 4:19 p.m. UTC
  From: David Hunt <david.hunt@intel.com>

The code was recently enhanced to allow the use of the waitpkg
intrinsics rather than the raw assembly in the rte_power functions.
However, the parameters to the intrinsics, while compiling fine, were
incorrect, and would segfault when run on the appropriate hardware.
This patch fixes the intrinsic parameters. Tested on a system with
tpause and umonitor/umwait instructions.

Fixes: 60943c04f3bc ("eal/x86: use intrinsics for power management")

Signed-off-by: David Hunt <david.hunt@intel.com>
Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/eal/x86/rte_power_intrinsics.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
  

Comments

Tyler Retzlaff Nov. 8, 2023, 3:19 a.m. UTC | #1
On Tue, Nov 07, 2023 at 04:19:01PM +0000, Bruce Richardson wrote:
> From: David Hunt <david.hunt@intel.com>
> 
> The code was recently enhanced to allow the use of the waitpkg
> intrinsics rather than the raw assembly in the rte_power functions.
> However, the parameters to the intrinsics, while compiling fine, were
> incorrect, and would segfault when run on the appropriate hardware.
> This patch fixes the intrinsic parameters. Tested on a system with
> tpause and umonitor/umwait instructions.
> 
> Fixes: 60943c04f3bc ("eal/x86: use intrinsics for power management")
> 
> Signed-off-by: David Hunt <david.hunt@intel.com>
> Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

Reviewed-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
  
Thomas Monjalon Nov. 8, 2023, 3:15 p.m. UTC | #2
08/11/2023 04:19, Tyler Retzlaff:
> On Tue, Nov 07, 2023 at 04:19:01PM +0000, Bruce Richardson wrote:
> > From: David Hunt <david.hunt@intel.com>
> > 
> > The code was recently enhanced to allow the use of the waitpkg
> > intrinsics rather than the raw assembly in the rte_power functions.
> > However, the parameters to the intrinsics, while compiling fine, were
> > incorrect, and would segfault when run on the appropriate hardware.
> > This patch fixes the intrinsic parameters. Tested on a system with
> > tpause and umonitor/umwait instructions.
> > 
> > Fixes: 60943c04f3bc ("eal/x86: use intrinsics for power management")
> > 
> > Signed-off-by: David Hunt <david.hunt@intel.com>
> > Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> Reviewed-by: Tyler Retzlaff <roretzla@linux.microsoft.com>

Applied, thanks.
  

Patch

diff --git a/lib/eal/x86/rte_power_intrinsics.c b/lib/eal/x86/rte_power_intrinsics.c
index 483395dcd5..532a2e646b 100644
--- a/lib/eal/x86/rte_power_intrinsics.c
+++ b/lib/eal/x86/rte_power_intrinsics.c
@@ -40,12 +40,12 @@  static void intel_umonitor(volatile void *addr)
 
 static void intel_umwait(const uint64_t timeout)
 {
+#if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)
+	_umwait(0, timeout);
+#else
 	const uint32_t tsc_l = (uint32_t)timeout;
 	const uint32_t tsc_h = (uint32_t)(timeout >> 32);
 
-#if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)
-	_umwait(tsc_l, tsc_h);
-#else
 	asm volatile(".byte 0xf2, 0x0f, 0xae, 0xf7;"
 			: /* ignore rflags */
 			: "D"(0), /* enter C0.2 */
@@ -208,17 +208,17 @@  rte_power_monitor(const struct rte_power_monitor_cond *pmc,
 int
 rte_power_pause(const uint64_t tsc_timestamp)
 {
-	const uint32_t tsc_l = (uint32_t)tsc_timestamp;
-	const uint32_t tsc_h = (uint32_t)(tsc_timestamp >> 32);
-
 	/* prevent user from running this instruction if it's not supported */
 	if (!wait_supported)
 		return -ENOTSUP;
 
 	/* execute TPAUSE */
 #if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)
-	_tpause(tsc_l, tsc_h);
+	_tpause(0, tsc_timestamp);
 #else
+	const uint32_t tsc_l = (uint32_t)tsc_timestamp;
+	const uint32_t tsc_h = (uint32_t)(tsc_timestamp >> 32);
+
 	asm volatile(".byte 0x66, 0x0f, 0xae, 0xf7;"
 			: /* ignore rflags */
 			: "D"(0), /* enter C0.2 */