[PATCH] perf/x86/intel/p4: Fix unused variable warning in p4_pmu_init()

[PATCH] perf/x86/intel/p4: Fix unused variable warning in p4_pmu_init()

[Aldo Conte]

Build the kernel with make W=1 generates the following warning:

  arch/x86/events/intel/p4.c: In function ‘p4_pmu_init’:
  arch/x86/events/intel/p4.c:1370:27: error: variable ‘high’ set but not used [-Werror=unused-but-set-variable]
   1370 |         unsigned int low, high;
        |                           ^~~~

This happens because, although both variables are declared and
initialized by rdmsr, only `low` is used in the subsequent if statement.

This patch prints the full content of Model-Specific Register
via `pr_cont` and so both the low and high part. It is also
very useful to have the contents of MSR_IA32_MISC_ENABLE
in dmesg for debugging purposes.

Running `make W=1` again resolves the error.
I was unable to test the patch because
i do not have the hardware.
Signed-off-by: Aldo Conte <aldocontelk@gmail.com>
---
 arch/x86/events/intel/p4.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/events/intel/p4.c b/arch/x86/events/intel/p4.c
index e5fd7367e45d..dc2866ed2495 100644
--- a/arch/x86/events/intel/p4.c
+++ b/arch/x86/events/intel/p4.c
@@ -1383,7 +1383,7 @@ __init int p4_pmu_init(void)
 	memcpy(hw_cache_event_ids, p4_hw_cache_event_ids,
 		sizeof(hw_cache_event_ids));
 
-	pr_cont("Netburst events, ");
+	pr_cont("Netburst events,  (misc_enable: %08x:%08x), ", high, low);
 
 	x86_pmu = p4_pmu;
 
-- 
2.53.0

Re: [PATCH] perf/x86/intel/p4: Fix unused variable warning in p4_pmu_init()

[Dave Hansen]

On 3/19/26 08:22, Aldo Conte wrote:
> This patch prints the full content of Model-Specific Register
> via `pr_cont` and so both the low and high part. It is also
> very useful to have the contents of MSR_IA32_MISC_ENABLE
> in dmesg for debugging purposes.

I'd probably just switch it over to rdmsrq():

	unsigned int misc;

	rdmsr(MSR_IA32_MISC_ENABLE, misc);

	if (!(misc & MSR_IA32_MISC_ENABLE_EMON)) {
		...

I'm kinda surprised the compiler is complaining, though. I suspect we've
got a ton of these around where one of the registers isn't used.

Re: [PATCH] perf/x86/intel/p4: Fix unused variable warning in p4_pmu_init()

[Aldo Conte]

On 3/19/26 4:30 PM, Dave Hansen wrote:
> On 3/19/26 08:22, Aldo Conte wrote:
>> This patch prints the full content of Model-Specific Register
>> via `pr_cont` and so both the low and high part. It is also
>> very useful to have the contents of MSR_IA32_MISC_ENABLE
>> in dmesg for debugging purposes.
> 
> I'd probably just switch it over to rdmsrq():
> 
> 	unsigned int misc;
> 
> 	rdmsr(MSR_IA32_MISC_ENABLE, misc);
> 
> 	if (!(misc & MSR_IA32_MISC_ENABLE_EMON)) {
> 		...
> 
> I'm kinda surprised the compiler is complaining, though. I suspect we've
> got a ton of these around where one of the registers isn't used.
Okay, your comment actually caught my attention. I looked into it 
further and noticed that `rdmsr` is defined as a macro in both
`arch/x86/include/asm/msr.h` and `/arch/x86/include/asm/paravirt.h`. In 
the first header, however, the expansion shows that high and low are 
evaluated as follows:
(void)((low) = (u32)__val);                \
(void)((high) = (u32)(__val >> 32));            \
so with a sort of warning suppressor, whereas in the second header, the 
(void) is not used.

Now, the file `arch/x86/events/intel/p4.c` includes `msr.h` and thus the 
expansion of `rdmsr`, which in theory contains `void`.

However, scrolling through the msr.h file, you can see that if the 
CONFIG_PARAVIRT_XXL configuration parameter is defined, then the second 
header (paravirt.h) is included, which then causes rdmsr to expand to

#define rdmsr(msr, val1, val2)            \
do {                        \
	u64 _l = paravirt_read_msr(msr);    \
     val1 = (u32)_l;				\
     val2 = _l >> 32;            \
} while (0)

Since there is no (void) in front of val1 and val2, this will trigger a 
warning.

I admit I'm a novice in this field, so what do you suggest is the best 
fix at this point?
Should I use the “warning suppressor" with something like
     (void) val1 = (u32)_l;                \
     (void) val2 = _l >> 32;            \
or implement the solution you suggested?

Re: [PATCH] perf/x86/intel/p4: Fix unused variable warning in p4_pmu_init()

[Dave Hansen]

On 3/19/26 15:20, Aldo Conte wrote:
> I admit I'm a novice in this field, so what do you suggest is the best
> fix at this point?
> Should I use the “warning suppressor" with something like
>     (void) val1 = (u32)_l;                \
>     (void) val2 = _l >> 32;            \
> or implement the solution you suggested?

Ideally, someone should go consolidate the two copies.

But, for now, for the p4.c issue, I'm still happy with transitioning
over to rdmsrq().

[PATCH v2] perf/x86/intel/p4: Fix unused variable warning in p4_pmu_init()

[Aldo Conte]

Build the kernel with make W=1 generates the following warning:

  arch/x86/events/intel/p4.c: In function ‘p4_pmu_init’:
  arch/x86/events/intel/p4.c:1370:27: error: variable ‘high’ set but not used [-Werror=unused-but-set-variable]
   1370 |         unsigned int low, high;
        |                           ^~~~

This happens because, although both variables are declared and
initialized by rdmsr, only `low` is used in the subsequent if statement.

This patch uses the rdmsrq() macro instead of the rdmsr() macro.
The rdmsrq() macro avoids the use of high and low variables
because it reads the msr value in a single u64 variable.
Also, replace (1 << 7) with the proper macro.

Running `make W=1` again resolves the error.
I was unable to test the patch because
i do not have the hardware.

Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Aldo Conte <aldocontelk@gmail.com>
---
v2: 
 - Use of rdmsrq() instead of rdmsr() to read the full 64-bit MSR
   in one single read avoiding the use of high and low.
 - Replace (1 << 7) with the MSR_IA32_MISC_ENABLE_EMON
 - Remove pr_cont() with MSR printout added in the previous patch

v1: https://lore.kernel.org/all/20260319152210.210854-1-aldocontelk@gmail.com/
 arch/x86/events/intel/p4.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/intel/p4.c b/arch/x86/events/intel/p4.c
index e5fd7367e45d..02bfdb77158b 100644
--- a/arch/x86/events/intel/p4.c
+++ b/arch/x86/events/intel/p4.c
@@ -1367,14 +1367,14 @@ static __initconst const struct x86_pmu p4_pmu = {
 
 __init int p4_pmu_init(void)
 {
-	unsigned int low, high;
+	unsigned int misc;
 	int i, reg;
 
 	/* If we get stripped -- indexing fails */
 	BUILD_BUG_ON(ARCH_P4_MAX_CCCR > INTEL_PMC_MAX_GENERIC);
 
-	rdmsr(MSR_IA32_MISC_ENABLE, low, high);
-	if (!(low & (1 << 7))) {
+	rdmsrq(MSR_IA32_MISC_ENABLE, misc);
+	if (!(misc & MSR_IA32_MISC_ENABLE_EMON)) {
 		pr_cont("unsupported Netburst CPU model %d ",
 			boot_cpu_data.x86_model);
 		return -ENODEV;
-- 
2.53.0

[PATCH v2] perf/x86/intel/p4: Fix unused variable warning in p4_pmu_init()

[Aldo Conte]

Build the kernel with make W=1 generates the following warning:

  arch/x86/events/intel/p4.c: In function ‘p4_pmu_init’:
  arch/x86/events/intel/p4.c:1370:27: error: variable ‘high’ set but not used [-Werror=unused-but-set-variable]
   1370 |         unsigned int low, high;
        |                           ^~~~

This happens because, although both variables are declared and
initialized by rdmsr, only `low` is used in the subsequent if statement.

This patch uses the rdmsrq() macro instead of the rdmsr() macro.
The rdmsrq() macro avoids the use of high and low variables
because it reads the msr value in a single u64 variable.
Also, replace (1 << 7) with the proper macro.

Running `make W=1` again resolves the error.
I was unable to test the patch because
i do not have the hardware.

Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Aldo Conte <aldocontelk@gmail.com>
---
v2: 
 - Use of rdmsrq() instead of rdmsr() to read the full 64-bit MSR
   in one single read avoiding the use of high and low.
 - Replace (1 << 7) with the MSR_IA32_MISC_ENABLE_EMON
 - Remove pr_cont() with MSR printout added in the previous patch

v1: https://lore.kernel.org/all/20260319152210.210854-1-aldocontelk@gmail.com/
 arch/x86/events/intel/p4.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/intel/p4.c b/arch/x86/events/intel/p4.c
index e5fd7367e45d..02bfdb77158b 100644
--- a/arch/x86/events/intel/p4.c
+++ b/arch/x86/events/intel/p4.c
@@ -1367,14 +1367,14 @@ static __initconst const struct x86_pmu p4_pmu = {
 
 __init int p4_pmu_init(void)
 {
-	unsigned int low, high;
+	unsigned int misc;
 	int i, reg;
 
 	/* If we get stripped -- indexing fails */
 	BUILD_BUG_ON(ARCH_P4_MAX_CCCR > INTEL_PMC_MAX_GENERIC);
 
-	rdmsr(MSR_IA32_MISC_ENABLE, low, high);
-	if (!(low & (1 << 7))) {
+	rdmsrq(MSR_IA32_MISC_ENABLE, misc);
+	if (!(misc & MSR_IA32_MISC_ENABLE_EMON)) {
 		pr_cont("unsupported Netburst CPU model %d ",
 			boot_cpu_data.x86_model);
 		return -ENODEV;
-- 
2.53.0

Re: [PATCH v2] perf/x86/intel/p4: Fix unused variable warning in p4_pmu_init()

[Peter Zijlstra]

On Fri, Mar 20, 2026 at 12:23:02PM +0100, Aldo Conte wrote:
> Build the kernel with make W=1 generates the following warning:
> 
>   arch/x86/events/intel/p4.c: In function ‘p4_pmu_init’:
>   arch/x86/events/intel/p4.c:1370:27: error: variable ‘high’ set but not used [-Werror=unused-but-set-variable]
>    1370 |         unsigned int low, high;
>         |                           ^~~~
> 
> This happens because, although both variables are declared and
> initialized by rdmsr, only `low` is used in the subsequent if statement.
> 
> This patch uses the rdmsrq() macro instead of the rdmsr() macro.
> The rdmsrq() macro avoids the use of high and low variables
> because it reads the msr value in a single u64 variable.
> Also, replace (1 << 7) with the proper macro.
> 
> Running `make W=1` again resolves the error.
> I was unable to test the patch because
> i do not have the hardware.
> 
> Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Aldo Conte <aldocontelk@gmail.com>
> ---
> v2: 
>  - Use of rdmsrq() instead of rdmsr() to read the full 64-bit MSR
>    in one single read avoiding the use of high and low.
>  - Replace (1 << 7) with the MSR_IA32_MISC_ENABLE_EMON
>  - Remove pr_cont() with MSR printout added in the previous patch
> 
> v1: https://lore.kernel.org/all/20260319152210.210854-1-aldocontelk@gmail.com/
>  arch/x86/events/intel/p4.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 

In general I figure people using W=1 can keep the pieces, but the split
msr interface is a relic, so yeah, lets clean this up.

Thanks!