linux-kernel - Re: [RESEND PATCH] perf/x86/intel: Fix unchecked MSR access error for Alder Lake N

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]

Date:   Fri, 19 Aug 2022 10:33:07 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     kan.liang@...ux.intel.com
Cc:     acme@...hat.com, linux-kernel@...r.kernel.org,
        alexander.shishkin@...ux.intel.com, ak@...ux.intel.com,
        Jianfeng Gao <jianfeng.gao@...el.com>
Subject: Re: [RESEND PATCH] perf/x86/intel: Fix unchecked MSR access error
 for Alder Lake N

On Fri, Aug 19, 2022 at 10:05:40AM +0200, Peter Zijlstra wrote:
> On Thu, Aug 18, 2022 at 11:15:30AM -0700, kan.liang@...ux.intel.com wrote:
> 
> > The Alder Lake N only has e-cores. The X86_FEATURE_HYBRID_CPU flag is
> > not set. The perf cannot retrieve the correct CPU type via
> > get_this_hybrid_cpu_type(). The model specific get_hybrid_cpu_type() is
> > hardcode to p-core. The wrong CPU type is given to the PMU of the
> > Alder Lake N.
> 
> If ADL-N isn't in fact a hybrid CPU, then *WHY* are we running
> init_hybrid_pmu() and setting up all that nonsense?
> 
> That is, wouldn't the right thing be to remove ALDERLAKE_N from the rest
> of {ALDER,RAPTOP}LAKE and create a non-hybrid PMU setup for it?

Something like the *completely* untested below.. which adds it like a
regular atom chip (which it is).

(I basically did copy/paste of tremont and added bits from the cpu_atom
thing from alderlake -- but might well have missed something)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 2db93498ff71..e509f1033a2d 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -5974,6 +5974,38 @@ __init int intel_pmu_init(void)
 		name = "Tremont";
 		break;
 
+	case INTEL_FAM6_ALDERLAKE_N:
+		x86_pmu.mid_ack = true;
+		memcpy(hw_cache_event_ids, glp_hw_cache_event_ids,
+		       sizeof(hw_cache_event_ids));
+		memcpy(hw_cache_extra_regs, tnt_hw_cache_extra_regs,
+		       sizeof(hw_cache_extra_regs));
+		hw_cache_event_ids[C(ITLB)][C(OP_READ)][C(RESULT_ACCESS)] = -1;
+
+		intel_pmu_lbr_init_skl();
+
+		x86_pmu.event_constraints = intel_slm_event_constraints;
+		x86_pmu.pebs_constraints = intel_grt_pebs_event_constraints;
+		x86_pmu.extra_regs = intel_grt_extra_regs;
+		/*
+		 * It's recommended to use CPU_CLK_UNHALTED.CORE_P + NPEBS
+		 * for precise cycles.
+		 */
+		x86_pmu.pebs_aliases = NULL;
+		x86_pmu.pebs_prec_dist = true;
+		x86_pmu.lbr_pt_coexist = true;
+		x86_pmu.flags |= PMU_FL_HAS_RSP_1;
+		x86_pmu.flags |= PMU_FL_NO_HT_SHARING;
+		x86_pmu.flags |= PMU_FL_PEBS_ALL;
+		x86_pmu.flags |= PMU_FL_INSTR_LATENCY;
+		x86_pmu.flags |= PMU_FL_MEM_LOADS_AUX;
+		x86_pmu.get_event_constraints = tnt_get_event_constraints;
+		td_attr = tnt_events_attrs;
+		extra_attr = slm_format_attr;
+		pr_cont("Gracemont events, ");
+		name = "Gracemont";
+		break;
+
 	case INTEL_FAM6_WESTMERE:
 	case INTEL_FAM6_WESTMERE_EP:
 	case INTEL_FAM6_WESTMERE_EX:
@@ -6318,7 +6350,6 @@ __init int intel_pmu_init(void)
 
 	case INTEL_FAM6_ALDERLAKE:
 	case INTEL_FAM6_ALDERLAKE_L:
-	case INTEL_FAM6_ALDERLAKE_N:
 	case INTEL_FAM6_RAPTORLAKE:
 	case INTEL_FAM6_RAPTORLAKE_P:
 		/*