Re: [Patch v2 6/9] perf/x86/intel: Validate return value of intel_pmu_init_hybrid()

["Mi, Dapeng" <dapeng1.mi@linux.intel.com>]

On 6/10/2026 4:16 PM, Peter Zijlstra wrote:
> Would not something like so work?
>
> I could not find a reason we *have* to init arch lbr that early -- but
> perhaps I didn't look hard enough?
>
> ---
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index d9488ade0f8e..4e551f240b2b 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -7534,14 +7534,14 @@ __init int intel_pmu_init(void)
>  	struct attribute **td_attr    = &empty_attrs;
>  	struct attribute **mem_attr   = &empty_attrs;
>  	struct attribute **tsx_attr   = &empty_attrs;
> +	struct x86_hybrid_pmu *pmu;
> +	unsigned int fixed_mask;
>  	union cpuid10_edx edx;
>  	union cpuid10_eax eax;
>  	union cpuid10_ebx ebx;
> -	unsigned int fixed_mask;
> +	int version, i, ret;
>  	bool pmem = false;
> -	int version, i;
>  	char *name;
> -	struct x86_hybrid_pmu *pmu;
>  
>  	/* Architectural Perfmon was introduced starting with Core "Yonah" */
>  	if (!cpu_has(&boot_cpu_data, X86_FEATURE_ARCH_PERFMON)) {
> @@ -7611,9 +7611,6 @@ __init int intel_pmu_init(void)
>  		x86_pmu.lbr_read = intel_pmu_lbr_read_32;
>  	}
>  
> -	if (boot_cpu_has(X86_FEATURE_ARCH_LBR))
> -		intel_pmu_arch_lbr_init();
> -
>  	intel_pebs_init();
>  
>  	x86_add_quirk(intel_arch_events_quirk); /* Install first, so it runs last */
> @@ -8216,7 +8213,9 @@ __init int intel_pmu_init(void)
>  		 *
>  		 * Initialize the common PerfMon capabilities here.
>  		 */
> -		intel_pmu_init_hybrid(hybrid_big_small);
> +		ret = intel_pmu_init_hybrid(hybrid_big_small);
> +		if (ret)
> +			return ret;
>  
>  		x86_pmu.pebs_latency_data = grt_latency_data;
>  		x86_pmu.get_event_constraints = adl_get_event_constraints;
> @@ -8274,7 +8273,9 @@ __init int intel_pmu_init(void)
>  	case INTEL_METEORLAKE:
>  	case INTEL_METEORLAKE_L:
>  	case INTEL_ARROWLAKE_U:
> -		intel_pmu_init_hybrid(hybrid_big_small);
> +		ret = intel_pmu_init_hybrid(hybrid_big_small);
> +		if (ret)
> +			return ret;
>  
>  		x86_pmu.pebs_latency_data = cmt_latency_data;
>  		x86_pmu.get_event_constraints = mtl_get_event_constraints;
> @@ -8313,7 +8314,9 @@ __init int intel_pmu_init(void)
>  		name = "lunarlake_hybrid";
>  
>  	lnl_common:
> -		intel_pmu_init_hybrid(hybrid_big_small);
> +		ret = intel_pmu_init_hybrid(hybrid_big_small);
> +		if (ret)
> +			return ret;
>  
>  		x86_pmu.pebs_latency_data = lnl_latency_data;
>  		x86_pmu.get_event_constraints = mtl_get_event_constraints;
> @@ -8337,7 +8340,9 @@ __init int intel_pmu_init(void)
>  		break;
>  
>  	case INTEL_ARROWLAKE_H:
> -		intel_pmu_init_hybrid(hybrid_big_small_tiny);
> +		ret = intel_pmu_init_hybrid(hybrid_big_small_tiny);
> +		if (ret)
> +			return ret;
>  
>  		x86_pmu.pebs_latency_data = arl_h_latency_data;
>  		x86_pmu.get_event_constraints = arl_h_get_event_constraints;
> @@ -8371,7 +8376,9 @@ __init int intel_pmu_init(void)
>  	case INTEL_NOVALAKE_L:
>  		pr_cont("Novalake Hybrid events, ");
>  		name = "novalake_hybrid";
> -		intel_pmu_init_hybrid(hybrid_big_small);
> +		ret  = intel_pmu_init_hybrid(hybrid_big_small);
> +		if (ret)
> +			return ret;
>  
>  		x86_pmu.pebs_latency_data = nvl_latency_data;
>  		x86_pmu.get_event_constraints = mtl_get_event_constraints;
> @@ -8478,6 +8485,9 @@ __init int intel_pmu_init(void)
>  
>  	intel_pmu_check_event_constraints_all(NULL);
>  
> +	if (boot_cpu_has(X86_FEATURE_ARCH_LBR))
> +		intel_pmu_arch_lbr_init();

It looks fine to move the arch_lbr_init() after the model-specific
initialization, but need a double check. Let me run some tests on SPR and
NVL. Thanks.


> +
>  	/*
>  	 * Access LBR MSR may cause #GP under certain circumstances.
>  	 * Check all LBR MSR here.