Re: [PATCH V2 1/7] perf/x86/intel/uncore: Fix discovery unit lookup for multi-die systems

[Mi, Dapeng]

LGTM.

Reviewed-by: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>

On 5/27/2026 11:11 PM, Zide Chen wrote:
> In uncore_find_add_unit(), PMON units with the same unit ID may be
> added to the uncore discovery RB-tree for different dies. These units
> are distinguished by node->die.
>
> However, intel_generic_uncore_box_ctl() uses a fixed die ID of -1 when
> looking up the discovery unit, which may retrieve the wrong node on
> multi-die systems.
>
> Use box->dieid instead so the correct discovery unit is selected.
>
> No functional issue has been observed so far because currently supported
> platforms happen to use the same unit control register for such units.
>
> Remove WARN_ON_ONCE() because with the above change a NULL unit can be
> expected, e.g. when a CPU die is offline during uncore enumeration and
> the unit is not added to the RB-tree. In this case,
> intel_uncore_find_discovery_unit() returns NULL once the die becomes
> online, and it is expected that the PMU box is not functional for that
> die.
>
> Fixes: b1d9ea2e1ca4 ("perf/x86/uncore: Apply the unit control RB tree to MSR uncore units")
> Signed-off-by: Zide Chen <zide.chen@xxxxxxxxx>
> ---
> v2:
> - Remove WARN_ON_ONCE().
> ---
>  arch/x86/events/intel/uncore_discovery.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/events/intel/uncore_discovery.c b/arch/x86/events/intel/uncore_discovery.c
> index 583cbd06b9b8..60e1200c4691 100644
> --- a/arch/x86/events/intel/uncore_discovery.c
> +++ b/arch/x86/events/intel/uncore_discovery.c
> @@ -481,8 +481,8 @@ static u64 intel_generic_uncore_box_ctl(struct intel_uncore_box *box)
>  	struct intel_uncore_discovery_unit *unit;
>  
>  	unit = intel_uncore_find_discovery_unit(box->pmu->type->boxes,
> -						-1, box->pmu->pmu_idx);
> -	if (WARN_ON_ONCE(!unit))
> +						box->dieid, box->pmu->pmu_idx);
> +	if (!unit)
>  		return 0;
>  
>  	return unit->addr;