Skip to contentSkip to conversation

eal/x86: fix segfaults in waitpkg power intrinsics

PATCH Applied Bruce Richardson 3 participants 3 messages Nov 7, 2023 – Nov 8, 2023

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 eal/x86: fix segfaults in waitpkg power intrinsics

From: David Hunt <[email protected]>

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")
---
 lib/eal/x86/rte_power_intrinsics.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
lib/eal/x86/rte_power_intrinsics.c +7 -7
index 483395dcd5..532a2e646b 100644
@@ -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 */
Signature · 1 line
2.39.2
Reviewed-by on v1 1/1
#1 lore raw
Quoted 9 lines · Bruce Richardson

From: David Hunt <[email protected]>

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.

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

---

#2 lore raw

08/11/2023 04:19, Tyler Retzlaff:

Quoted 10 lines · Bruce Richardson
Quoted 9 lines

From: David Hunt <[email protected]>

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.

Quoted 6 lines
Quoted 4 lines
Fixes 60943c04f3bc ("eal/x86: use intrinsics for power management")

Applied, thanks.