Re: [PATCH 2/7] perf/x86/intel/uncore: Let init_box() callback report failures

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The adl_uncore_msr_init_box callback was missed during the return type conversion and still returns void, causing a build failure and a runtime functional regression.
--

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

perf/x86/intel/uncore: Let init_box() callback report failures

The init_box() callback currently returns void, so initialization
failures are silently ignored and the box is still marked initialized.
Change the callback to return int so platform code can report errors
back to the common uncore layer.

> diff --git a/arch/x86/events/intel/uncore.h b/arch/x86/events/intel/uncore.h
> index 2e41a66f00551..25a44d3de6e7d 100644
> --- a/arch/x86/events/intel/uncore.h
> +++ b/arch/x86/events/intel/uncore.h
> @@ -129,7 +129,7 @@ struct intel_uncore_type {
>  #define events_group attr_groups[2]
>  
>  struct intel_uncore_ops {
> -	void (*init_box)(struct intel_uncore_box *);
> +	int (*init_box)(struct intel_uncore_box *);

It appears the return type for the init_box callback was changed here, but
adl_uncore_msr_init_box() in arch/x86/events/intel/uncore_snb.c was missed
and still returns void:

static void adl_uncore_msr_init_box(struct intel_uncore_box *box)
{
	if (box->pmu->pmu_idx == 0)
		wrmsrq(ADL_UNC_PERF_GLOBAL_CTL, SNB_UNC_GLOBAL_CTL_EN);
}

Could this cause a build warning or failure (e.g., incompatible pointer
type)?

Also, at runtime, uncore_box_init() expects an int return value. Would it
end up reading an uninitialized return value from the eax register?

If the last operation in adl_uncore_msr_init_box() is wrmsrq(), which
loads SNB_UNC_GLOBAL_CTL_EN into eax, could this residual non-zero value
cause uncore_box_init() to incorrectly treat the initialization as a failure?

If the initialization failure marks the PMU as broken, would this render the
Alder Lake (ADL) MSR uncore PMU inaccessible to userspace?

>  	void (*exit_box)(struct intel_uncore_box *);
>  	void (*disable_box)(struct intel_uncore_box *);
>  	void (*enable_box)(struct intel_uncore_box *);

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