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@xxxxxxxxx>
> ---
> 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));
