linux-kernel - Re: [PATCH v3 14/15] perf/x86: Simplify Intel PMU initialization

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <49c59a81-015f-46a1-88b9-f043ca2710d0@linux.intel.com>
Date: Wed, 19 Feb 2025 15:10:42 -0500
From: "Liang, Kan" <kan.liang@...ux.intel.com>
To: Sohil Mehta <sohil.mehta@...el.com>, x86@...nel.org,
 Dave Hansen <dave.hansen@...ux.intel.com>, Tony Luck <tony.luck@...el.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Arnaldo Carvalho de Melo <acme@...nel.org>,
 Namhyung Kim <namhyung@...nel.org>, Mark Rutland <mark.rutland@....com>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
 Adrian Hunter <adrian.hunter@...el.com>, Thomas Gleixner
 <tglx@...utronix.de>, Borislav Petkov <bp@...en8.de>,
 "H . Peter Anvin" <hpa@...or.com>, "Rafael J . Wysocki" <rafael@...nel.org>,
 Len Brown <lenb@...nel.org>, Andy Lutomirski <luto@...nel.org>,
 Viresh Kumar <viresh.kumar@...aro.org>, Jean Delvare <jdelvare@...e.com>,
 Guenter Roeck <linux@...ck-us.net>, Zhang Rui <rui.zhang@...el.com>,
 Andrew Cooper <andrew.cooper3@...rix.com>,
 David Laight <david.laight.linux@...il.com>,
 Dapeng Mi <dapeng1.mi@...ux.intel.com>, linux-perf-users@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org,
 linux-pm@...r.kernel.org
Subject: Re: [PATCH v3 14/15] perf/x86: Simplify Intel PMU initialization



On 2025-02-19 1:41 p.m., Sohil Mehta wrote:
> Architectural Perfmon was introduced on the Family 6 "Core" processors
> starting with Yonah. Processors before Yonah need their own customized
> PMU initialization.
> 
> p6_pmu_init() is expected to provide that initialization for early
> Family 6 processors. But, due to the unrestricted call to p6_pmu_init(),
> it could get called for any Family 6 processor if the architectural
> perfmon feature is disabled on that processor.
> 
> To simplify, restrict the call to p6_pmu_init() to early Family 6
> processors that do not have architectural perfmon support. As a result,
> the "unsupported" console print becomes practically unreachable because
> all the released P6 processors are covered by the switch cases.
> 
> Move the console print to a common location where it can cover all
> modern processors that do not have architectural perfmon support.
> 
> Also, use this opportunity to get rid of the unnecessary switch cases in
> p6_pmu_init().  Only the Pentium Pro processor needs a quirk, and the
> rest of the processors do not need any special handling. The gaps in the
> case numbers are only due to no processor with those model numbers being
> released.
> 
> Converting to a VFM based check gets rid of one last few Intel x86_model
> comparisons.
> 
> Signed-off-by: Sohil Mehta <sohil.mehta@...el.com>
> ---
> v3: Restrict calling p6_pmu_init() to only when needed.
>     Move the console print to a common location.
> 
> v2: No change.
> ---
>  arch/x86/events/intel/core.c | 16 +++++++++++-----
>  arch/x86/events/intel/p6.c   | 26 +++-----------------------
>  2 files changed, 14 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index 7601196d1d18..c645d8c8ab87 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -6466,16 +6466,22 @@ __init int intel_pmu_init(void)
>  	char *name;
>  	struct x86_hybrid_pmu *pmu;
>  
> +	/* Architectural Perfmon was introduced starting with INTEL_CORE_YONAH */
>  	if (!cpu_has(&boot_cpu_data, X86_FEATURE_ARCH_PERFMON)) {
>  		switch (boot_cpu_data.x86) {
> -		case 0x6:
> -			return p6_pmu_init();
> -		case 0xb:
> +		case 6:
> +			if (boot_cpu_data.x86_vfm < INTEL_CORE_YONAH)
> +				return p6_pmu_init();
> +			break;

We may need a return -ENODEV here.

I think it's possible that some weird hypervisor doesn't enumerate the
ARCH_PERFMON for a modern CPU. Perf should not touch the leaf 10 if the
ARCH_PERFMON is not supported.

Thanks,
Kan

> +		case 11:
>  			return knc_pmu_init();
> -		case 0xf:
> +		case 15:
>  			return p4_pmu_init();
> +		default:
> +			pr_cont("unsupported CPU family %d model %d ",
> +				boot_cpu_data.x86, boot_cpu_data.x86_model);
> +			return -ENODEV;
>  		}
> -		return -ENODEV;
>  	}
>  
>  	/*
> diff --git a/arch/x86/events/intel/p6.c b/arch/x86/events/intel/p6.c
> index a6cffb4f4ef5..65b45e9d7016 100644
> --- a/arch/x86/events/intel/p6.c
> +++ b/arch/x86/events/intel/p6.c
> @@ -2,6 +2,8 @@
>  #include <linux/perf_event.h>
>  #include <linux/types.h>
>  
> +#include <asm/cpu_device_id.h>
> +
>  #include "../perf_event.h"
>  
>  /*
> @@ -248,30 +250,8 @@ __init int p6_pmu_init(void)
>  {
>  	x86_pmu = p6_pmu;
>  
> -	switch (boot_cpu_data.x86_model) {
> -	case  1: /* Pentium Pro */
> +	if (boot_cpu_data.x86_vfm == INTEL_PENTIUM_PRO)
>  		x86_add_quirk(p6_pmu_rdpmc_quirk);
> -		break;
> -
> -	case  3: /* Pentium II - Klamath */
> -	case  5: /* Pentium II - Deschutes */
> -	case  6: /* Pentium II - Mendocino */
> -		break;
> -
> -	case  7: /* Pentium III - Katmai */
> -	case  8: /* Pentium III - Coppermine */
> -	case 10: /* Pentium III Xeon */
> -	case 11: /* Pentium III - Tualatin */
> -		break;
> -
> -	case  9: /* Pentium M - Banias */
> -	case 13: /* Pentium M - Dothan */
> -		break;
> -
> -	default:
> -		pr_cont("unsupported p6 CPU model %d ", boot_cpu_data.x86_model);
> -		return -ENODEV;
> -	}
>  
>  	memcpy(hw_cache_event_ids, p6_hw_cache_event_ids,
>  		sizeof(hw_cache_event_ids));