Re: [PATCH 6/6] perf/x86/intel/uncore: Implement global init callback for GNR uncore

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The return value of `wrmsrq_on_cpu()` is ignored in `uncore_msr_global_init()`, masking hardware initialization failures caused by a CPU hotplug TOCTOU race.
--

commit d795e124f297e8f2c0cfee9efd29cdf4ec1993ce
Author: Zide Chen <zide.chen@intel.com>

perf/x86/intel/uncore: Implement global init callback for GNR uncore

This commit adds a generic uncore_msr_global_init() callback for GNR, GRR,
SRF, and CWF. It writes 0 to the global control register during boot to
ensure the UBOX PMON units are properly initialized.

> diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c
> index 19056514b0812..a7780c5cd419a 100644
> --- a/arch/x86/events/intel/uncore.c
> +++ b/arch/x86/events/intel/uncore.c

[ ... ]

> @@ -1731,6 +1731,17 @@ static int uncore_mmio_global_init(u64 ctl)
>  	return 0;
>  }
>  
> +static int uncore_msr_global_init(int die, u64 msr)
> +{
> +	int cpu = uncore_die_to_cpu(die);
> +
> +	if (cpu == -1)
> +		return -ENODEV;
> +
> +	wrmsrq_on_cpu(cpu, msr, 0);
> +	return 0;
> +}

uncore_die_to_cpu() acquires and releases cpus_read_lock() to find an
online CPU, meaning the selected CPU could be hot-unplugged concurrently
before wrmsrq_on_cpu() executes.

If the CPU goes offline, the underlying smp_call_function_single() will
fail and wrmsrq_on_cpu() will return an error.

Because this function unconditionally returns 0, the caller will assume
success and proceed to register the PMON units. As noted in the commit
message, the units will not work unless the MSR is written, which could
lead to a silent functional failure.

Does this code need to check and propagate the return value of
wrmsrq_on_cpu() to prevent this regression?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511230527.26096-1-zide.chen@intel.com?part=6