[v2] eal/x86: fix build on systems with WAITPKG support

Message ID 20230828103933.1047285-1-bruce.richardson@intel.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series [v2] eal/x86: fix build on systems with WAITPKG support |

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/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/iol-sample-apps-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/Intel-compilation warning apply issues

Commit Message

Bruce Richardson Aug. 28, 2023, 10:39 a.m. UTC
  When doing a build for a system with WAITPKG support and a modern
compiler, we get build errors for the "_umonitor" intrinsic, due to the
casting away of the "volatile" on the parameter.

../lib/eal/x86/rte_power_intrinsics.c: In function 'rte_power_monitor':
../lib/eal/x86/rte_power_intrinsics.c:113:22: error: passing argument 1
of '_umonitor' discards 'volatile' qualifier from pointer target type
[-Werror=discarded-qualifiers]
  113 |         _umonitor(pmc->addr);
        |                   ~~~^~~~~~

We can avoid this issue by casting through "uintptr_t" and thereby
remove the volatile without warning.  We also ensure comments are
correct for each leg of the ifdef..else..endif block.

Fixes: 60943c04f3bc ("eal/x86: use intrinsics for power management")
Cc: roretzla@linux.microsoft.com

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Tested-by: David Marchand <david.marchand@redhat.com>
---
 lib/eal/x86/rte_power_intrinsics.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
  

Comments

David Marchand Aug. 28, 2023, 2:39 p.m. UTC | #1
On Mon, Aug 28, 2023 at 12:40 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> When doing a build for a system with WAITPKG support and a modern
> compiler, we get build errors for the "_umonitor" intrinsic, due to the
> casting away of the "volatile" on the parameter.
>
> ../lib/eal/x86/rte_power_intrinsics.c: In function 'rte_power_monitor':
> ../lib/eal/x86/rte_power_intrinsics.c:113:22: error: passing argument 1
> of '_umonitor' discards 'volatile' qualifier from pointer target type
> [-Werror=discarded-qualifiers]
>   113 |         _umonitor(pmc->addr);
>         |                   ~~~^~~~~~
>
> We can avoid this issue by casting through "uintptr_t" and thereby
> remove the volatile without warning.  We also ensure comments are
> correct for each leg of the ifdef..else..endif block.
>
> Fixes: 60943c04f3bc ("eal/x86: use intrinsics for power management")
> Cc: roretzla@linux.microsoft.com
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> Tested-by: David Marchand <david.marchand@redhat.com>

Applied to fix build on the main branch, thanks.

We can look at the casting helper as a followup.
  

Patch

diff --git a/lib/eal/x86/rte_power_intrinsics.c b/lib/eal/x86/rte_power_intrinsics.c
index 4066d1392e..97202e42fc 100644
--- a/lib/eal/x86/rte_power_intrinsics.c
+++ b/lib/eal/x86/rte_power_intrinsics.c
@@ -103,15 +103,15 @@  rte_power_monitor(const struct rte_power_monitor_cond *pmc,
 	rte_spinlock_lock(&s->lock);
 	s->monitor_addr = pmc->addr;
 
-	/*
-	 * we're using raw byte codes for now as only the newest compiler
-	 * versions support this instruction natively.
-	 */
-
 	/* set address for UMONITOR */
 #if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)
-	_umonitor(pmc->addr);
+	/* cast away "volatile" when using the intrinsic */
+	_umonitor((void *)(uintptr_t)pmc->addr);
 #else
+	/*
+	 * we're using raw byte codes for compiler versions which
+	 * don't support this instruction natively.
+	 */
 	asm volatile(".byte 0xf3, 0x0f, 0xae, 0xf7;"
 			:
 			: "D"(pmc->addr));