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

[Aldo Conte <aldocontelk@gmail.com>]

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?