Re: [PATCH 6/7] perf/x86/intel/uncore: Fix uncore_box ref/unref ordering on CPU hotplug

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Swapping the order of `uncore_box_unref()` and `uncore_change_context()` unmaps MMIO memory while perf events are still active on the offlining CPU, leading to a kernel panic.
--

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

perf/x86/intel/uncore: Fix uncore_box ref/unref ordering on CPU hotplug

This commit reorders the setup and teardown sequence in the uncore CPU
hotplug callbacks. It swaps the order of uncore_box_ref/unref and
uncore_change_context to ensure that the uncore box references the correct
context.

> diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c
> index 4d31dcaf5405b..b9b901b789d15 100644
> --- a/arch/x86/events/intel/uncore.c
> +++ b/arch/x86/events/intel/uncore.c
> @@ -1555,9 +1555,15 @@ static int uncore_event_cpu_offline(unsigned int cpu)
>  {
>  	int die, target;
>  
> +	/* Clear the references */
> +	die = topology_logical_die_id(cpu);
> +	uncore_box_unref(uncore_msr_uncores, die);
> +	uncore_box_unref(uncore_mmio_uncores, die);
> +

When uncore_box_unref() executes before uncore_change_context(), can this
cause a page fault if there are active events?

During CPU offline, the uncore teardown callback runs at state
CPUHP_AP_PERF_X86_UNCORE_ONLINE, which executes before the generic perf core
teardown at CPUHP_AP_PERF_ONLINE.

If the offlining CPU is the last CPU in the die, cpu_refcnt reaches 0 and
uncore_box_exit() is called immediately. For MMIO PMUs, this calls
uncore_mmio_exit_box(), which executes iounmap(box->io_addr).

Since uncore_change_context() is called later with target = -1, no event
migration happens, and the active uncore perf events stay attached to the
offlining CPU's context.

When teardown reaches CPUHP_AP_PERF_ONLINE, perf_event_exit_cpu_context()
runs to clean up remaining events on the dying CPU. It calls pmu->stop()
to disable the events.

Will uncore_pmu_event_stop() dereference the unmapped box->io_addr via
uncore_perf_event_update() and disable_event(), leading to a kernel panic?

Even if a later patch adds uncore_pmu_unregister() to uncore_box_unref(),
this registration is only skipped if die_refcnt > 0. On a multi-die system,
offlining all CPUs in a single die leaves die_refcnt > 0, so the PMU
unregistration is skipped and the panic could still occur.

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