[PATCH 0/4] Enable PMU for ArrowLake-H

[PATCH 0/4] Enable PMU for ArrowLake-H

[Dapeng Mi]

ArrowLake-H is a specific variant of regular ArrowLake. It shares same
PMU features on lioncove P-cores and skymont E-cores with regular
ArrowLake except ArrowLake-H adds extra crestmont uarch E-cores.

Thus ArrowLake-H contains two different atom uarchs. This is totally
different with previous Intel hybrid platforms which contains only one
kind of atom uarchs. In this case, it's not enough to distinguish the
uarchs just by core type. So CPUID.1AH.EAX[23:0] provides an unique
native model ID for each uarch, this unique native model ID combining
the core type can be used to uniquely identity the uarch.

This patch series introduces PMU support for ArrowLake-H. Besides
inheriting the same PMU support from regular ArrowLake, it leverages
the native model ID to detect the 2nd kind of atom uarch core and
enables PMU support. To distinguish the two atom uarchs in sysfs, the
PMU of 2nd atom uarch is named to "cpu_atom2".

Run basic counting, PMI based sampling, PEBS based sampling, LBR and
topdown metrics tests on ArrowLake-H platform, no issue is found.

Dapeng Mi (4):
  perf/x86: Refine hybrid_pmu_type defination
  x86/cpu/intel: Define helper to get CPU core native ID
  perf/x86/intel: Support hybrid PMU with multiple atom uarchs
  perf/x86/intel: Add PMU support for ArrowLake-H

 arch/x86/events/intel/core.c | 129 ++++++++++++++++++++++++++++++++---
 arch/x86/events/intel/ds.c   |  21 ++++++
 arch/x86/events/perf_event.h |  33 ++++++---
 arch/x86/include/asm/cpu.h   |   6 ++
 arch/x86/kernel/cpu/intel.c  |  15 ++++
 5 files changed, 186 insertions(+), 18 deletions(-)


base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
-- 
2.40.1

[PATCH 1/4] perf/x86: Refine hybrid_pmu_type defination

[Dapeng Mi]

Use macros instead of magic number to define hybrid_pmu_type and remove
X86_HYBRID_NUM_PMUS since it's never used.

Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
Tested-by: Yongwei Ma <yongwei.ma@intel.com>
---
 arch/x86/events/perf_event.h | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index ac1182141bf6..5d1677844e04 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -674,19 +674,16 @@ enum hybrid_cpu_type {
 	HYBRID_INTEL_CORE	= 0x40,
 };
 
+#define X86_HYBRID_PMU_ATOM_IDX		0
+#define X86_HYBRID_PMU_CORE_IDX		1
 enum hybrid_pmu_type {
 	not_hybrid,
-	hybrid_small		= BIT(0),
-	hybrid_big		= BIT(1),
+	hybrid_small		= BIT(X86_HYBRID_PMU_ATOM_IDX),
+	hybrid_big		= BIT(X86_HYBRID_PMU_CORE_IDX),
 
 	hybrid_big_small	= hybrid_big | hybrid_small, /* only used for matching */
 };
 
-#define X86_HYBRID_PMU_ATOM_IDX		0
-#define X86_HYBRID_PMU_CORE_IDX		1
-
-#define X86_HYBRID_NUM_PMUS		2
-
 struct x86_hybrid_pmu {
 	struct pmu			pmu;
 	const char			*name;
-- 
2.40.1

[PATCH 2/4] x86/cpu/intel: Define helper to get CPU core native ID

[Dapeng Mi]

Define helper get_this_hybrid_cpu_native_id() to return the CPU core
native ID. This core native ID combining with core type can be used to
figure out the CPU core uarch uniquely.

Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
Tested-by: Yongwei Ma <yongwei.ma@intel.com>
---
 arch/x86/include/asm/cpu.h  |  6 ++++++
 arch/x86/kernel/cpu/intel.c | 15 +++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index aa30fd8cad7f..5af69b5be2fb 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -32,6 +32,7 @@ extern bool handle_user_split_lock(struct pt_regs *regs, long error_code);
 extern bool handle_guest_split_lock(unsigned long ip);
 extern void handle_bus_lock(struct pt_regs *regs);
 u8 get_this_hybrid_cpu_type(void);
+u32 get_this_hybrid_cpu_native_id(void);
 #else
 static inline void __init sld_setup(struct cpuinfo_x86 *c) {}
 static inline bool handle_user_split_lock(struct pt_regs *regs, long error_code)
@@ -50,6 +51,11 @@ static inline u8 get_this_hybrid_cpu_type(void)
 {
 	return 0;
 }
+
+static inline u32 get_this_hybrid_cpu_native_id(void)
+{
+	return 0;
+}
 #endif
 #ifdef CONFIG_IA32_FEAT_CTL
 void init_ia32_feat_ctl(struct cpuinfo_x86 *c);
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 08b95a35b5cb..dbc457626207 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -1297,3 +1297,18 @@ u8 get_this_hybrid_cpu_type(void)
 
 	return cpuid_eax(0x0000001a) >> X86_HYBRID_CPU_TYPE_ID_SHIFT;
 }
+
+/**
+ * get_this_hybrid_cpu_native_id() - Get the native id of this hybrid CPU
+ *
+ * Returns the uarch native ID [23:0] of a CPU in a hybrid processor.
+ * If the processor is not hybrid, returns 0.
+ */
+u32 get_this_hybrid_cpu_native_id(void)
+{
+	if (!cpu_feature_enabled(X86_FEATURE_HYBRID_CPU))
+		return 0;
+
+	return cpuid_eax(0x0000001a) &
+	       (BIT_ULL(X86_HYBRID_CPU_TYPE_ID_SHIFT) - 1);
+}
-- 
2.40.1

[PATCH 3/4] perf/x86/intel: Support hybrid PMU with multiple atom uarchs

[Dapeng Mi]

The upcoming ARL-H hybrid processor contains 2 different atom uarchs
which have different PMU capabilities. To distinguish these atom uarchs,
CPUID.1AH.EAX[23:0] defines a native model ID which can be used to
uniquely identify the uarch of the core by combining with core type.

Thus a 3rd hybrid pmu type "hybrid_small2" is defined to mark the 2nd
atom uarch. The helper find_hybrid_pmu_for_cpu() would compare the
hybrid pmu type and dynamically read core native id from cpu to identify
the corresponding hybrid pmu structure.

Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
Tested-by: Yongwei Ma <yongwei.ma@intel.com>
---
 arch/x86/events/intel/core.c | 24 +++++++++++++++++-------
 arch/x86/events/perf_event.h | 18 +++++++++++++++++-
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 0c9c2706d4ec..b6429bc009c0 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -4902,17 +4902,26 @@ static struct x86_hybrid_pmu *find_hybrid_pmu_for_cpu(void)
 
 	/*
 	 * This essentially just maps between the 'hybrid_cpu_type'
-	 * and 'hybrid_pmu_type' enums:
+	 * and 'hybrid_pmu_type' enums except for ARL-H processor
+	 * which needs to compare atom uarch native id since ARL-H
+	 * contains two different atom uarchs.
 	 */
 	for (i = 0; i < x86_pmu.num_hybrid_pmus; i++) {
 		enum hybrid_pmu_type pmu_type = x86_pmu.hybrid_pmu[i].pmu_type;
+		u32 native_id;
 
-		if (cpu_type == HYBRID_INTEL_CORE &&
-		    pmu_type == hybrid_big)
-			return &x86_pmu.hybrid_pmu[i];
-		if (cpu_type == HYBRID_INTEL_ATOM &&
-		    pmu_type == hybrid_small)
+		if (cpu_type == HYBRID_INTEL_CORE && pmu_type == hybrid_big)
 			return &x86_pmu.hybrid_pmu[i];
+		if (cpu_type == HYBRID_INTEL_ATOM) {
+			if (x86_pmu.num_hybrid_pmus == 2 && pmu_type == hybrid_small)
+				return &x86_pmu.hybrid_pmu[i];
+
+			native_id = get_this_hybrid_cpu_native_id();
+			if (native_id == skt_native_id && pmu_type == hybrid_small)
+				return &x86_pmu.hybrid_pmu[i];
+			if (native_id == cmt_native_id && pmu_type == hybrid_small2)
+				return &x86_pmu.hybrid_pmu[i];
+		}
 	}
 
 	return NULL;
@@ -6218,6 +6227,7 @@ static inline int intel_pmu_v6_addr_offset(int index, bool eventsel)
 static const struct { enum hybrid_pmu_type id; char *name; } intel_hybrid_pmu_type_map[] __initconst = {
 	{ hybrid_small, "cpu_atom" },
 	{ hybrid_big, "cpu_core" },
+	{ hybrid_small2, "cpu_atom2" },
 };
 
 static __always_inline int intel_pmu_init_hybrid(enum hybrid_pmu_type pmus)
@@ -6250,7 +6260,7 @@ static __always_inline int intel_pmu_init_hybrid(enum hybrid_pmu_type pmus)
 							0, x86_pmu_num_counters(&pmu->pmu), 0, 0);
 
 		pmu->intel_cap.capabilities = x86_pmu.intel_cap.capabilities;
-		if (pmu->pmu_type & hybrid_small) {
+		if (pmu->pmu_type & hybrid_small_all) {
 			pmu->intel_cap.perf_metrics = 0;
 			pmu->intel_cap.pebs_output_pt_available = 1;
 			pmu->mid_ack = true;
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index 5d1677844e04..f7b55c909eff 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -668,6 +668,13 @@ enum {
 #define PERF_PEBS_DATA_SOURCE_GRT_MAX	0x10
 #define PERF_PEBS_DATA_SOURCE_GRT_MASK	(PERF_PEBS_DATA_SOURCE_GRT_MAX - 1)
 
+
+/*
+ * CPUID.1AH.EAX[31:0] uniquely identifies the microarchitecture
+ * of the core. Bits 31-24 indicates its core type (Core or Atom)
+ * and Bits [23:0] indicates the native model ID of the core.
+ * Core type and native model ID are defined in below enumerations.
+ */
 enum hybrid_cpu_type {
 	HYBRID_INTEL_NONE,
 	HYBRID_INTEL_ATOM	= 0x20,
@@ -676,12 +683,21 @@ enum hybrid_cpu_type {
 
 #define X86_HYBRID_PMU_ATOM_IDX		0
 #define X86_HYBRID_PMU_CORE_IDX		1
+#define X86_HYBRID_PMU_ATOM2_IDX	2
 enum hybrid_pmu_type {
 	not_hybrid,
 	hybrid_small		= BIT(X86_HYBRID_PMU_ATOM_IDX),
 	hybrid_big		= BIT(X86_HYBRID_PMU_CORE_IDX),
+	hybrid_small2		= BIT(X86_HYBRID_PMU_ATOM2_IDX),
+	/* The belows are only used for matching */
+	hybrid_big_small	= hybrid_big | hybrid_small,
+	hybrid_small_all	= hybrid_small | hybrid_small2,
+	hybrid_big_small_arl_h	= hybrid_big | hybrid_small_all,
+};
 
-	hybrid_big_small	= hybrid_big | hybrid_small, /* only used for matching */
+enum atom_native_id {
+	cmt_native_id           = 0x2,  /* Crestmont */
+	skt_native_id           = 0x3,  /* Skymont */
 };
 
 struct x86_hybrid_pmu {
-- 
2.40.1

[PATCH 4/4] perf/x86/intel: Add PMU support for ArrowLake-H

[Dapeng Mi]

ArrowLake-H contains 3 different uarchs, LionCove, Skymont and Crestmont.
It is different with previous hybrid processors which only contains two
kinds of uarchs.

This patch adds PMU support for ArrowLake-H processor, adds ARL-H
specific events which supports the 3 kinds of uarchs, such as
td_retiring_arl_h, and extends some existed format attributes like
offcore_rsp to make them be available to support ARL-H as well. Althrough
these format attributes like offcore_rsp have been extended to support
ARL-H, they can still support the regular hybrid platforms with 2 kinds
of uarchs since the helper hybrid_format_is_visible() would filter PMU
types and only show the format attribute for available PMUs.

Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
Tested-by: Yongwei Ma <yongwei.ma@intel.com>
---
 arch/x86/events/intel/core.c | 105 ++++++++++++++++++++++++++++++++++-
 arch/x86/events/intel/ds.c   |  21 +++++++
 arch/x86/events/perf_event.h |   4 ++
 3 files changed, 127 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index b6429bc009c0..72836bb05387 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -4589,6 +4589,28 @@ static enum hybrid_cpu_type adl_get_hybrid_cpu_type(void)
 	return HYBRID_INTEL_CORE;
 }
 
+static struct event_constraint *
+arl_h_get_event_constraints(struct cpu_hw_events *cpuc, int idx,
+			  struct perf_event *event)
+{
+	struct x86_hybrid_pmu *pmu = hybrid_pmu(event->pmu);
+
+	if (pmu->pmu_type == hybrid_small2)
+		return cmt_get_event_constraints(cpuc, idx, event);
+
+	return mtl_get_event_constraints(cpuc, idx, event);
+}
+
+static int arl_h_hw_config(struct perf_event *event)
+{
+	struct x86_hybrid_pmu *pmu = hybrid_pmu(event->pmu);
+
+	if (pmu->pmu_type == hybrid_small2)
+		return intel_pmu_hw_config(event);
+
+	return adl_hw_config(event);
+}
+
 /*
  * Broadwell:
  *
@@ -5952,6 +5974,37 @@ static struct attribute *lnl_hybrid_events_attrs[] = {
 	NULL
 };
 
+/* The event string must be in PMU IDX order. */
+EVENT_ATTR_STR_HYBRID(topdown-retiring,
+		      td_retiring_arl_h,
+		      "event=0xc2,umask=0x02;event=0x00,umask=0x80;event=0xc2,umask=0x0",
+		      hybrid_big_small_arl_h);
+EVENT_ATTR_STR_HYBRID(topdown-bad-spec,
+		      td_bad_spec_arl_h,
+		      "event=0x73,umask=0x0;event=0x00,umask=0x81;event=0x73,umask=0x0",
+		      hybrid_big_small_arl_h);
+EVENT_ATTR_STR_HYBRID(topdown-fe-bound,
+		      td_fe_bound_arl_h,
+		      "event=0x9c,umask=0x01;event=0x00,umask=0x82;event=0x71,umask=0x0",
+		      hybrid_big_small_arl_h);
+EVENT_ATTR_STR_HYBRID(topdown-be-bound,
+		      td_be_bound_arl_h,
+		      "event=0xa4,umask=0x02;event=0x00,umask=0x83;event=0x74,umask=0x0",
+		      hybrid_big_small_arl_h);
+
+static struct attribute *arl_h_hybrid_events_attrs[] = {
+	EVENT_PTR(slots_adl),
+	EVENT_PTR(td_retiring_arl_h),
+	EVENT_PTR(td_bad_spec_arl_h),
+	EVENT_PTR(td_fe_bound_arl_h),
+	EVENT_PTR(td_be_bound_arl_h),
+	EVENT_PTR(td_heavy_ops_adl),
+	EVENT_PTR(td_br_mis_adl),
+	EVENT_PTR(td_fetch_lat_adl),
+	EVENT_PTR(td_mem_bound_adl),
+	NULL,
+};
+
 /* Must be in IDX order */
 EVENT_ATTR_STR_HYBRID(mem-loads,     mem_ld_adl,     "event=0xd0,umask=0x5,ldlat=3;event=0xcd,umask=0x1,ldlat=3", hybrid_big_small);
 EVENT_ATTR_STR_HYBRID(mem-stores,    mem_st_adl,     "event=0xd0,umask=0x6;event=0xcd,umask=0x2",                 hybrid_big_small);
@@ -5970,6 +6023,21 @@ static struct attribute *mtl_hybrid_mem_attrs[] = {
 	NULL
 };
 
+EVENT_ATTR_STR_HYBRID(mem-loads,
+		      mem_ld_arl_h,
+		      "event=0xd0,umask=0x5,ldlat=3;event=0xcd,umask=0x1,ldlat=3;event=0xd0,umask=0x5,ldlat=3",
+		      hybrid_big_small_arl_h);
+EVENT_ATTR_STR_HYBRID(mem-stores,
+		      mem_st_arl_h,
+		      "event=0xd0,umask=0x6;event=0xcd,umask=0x2;event=0xd0,umask=0x6",
+		      hybrid_big_small_arl_h);
+
+static struct attribute *arl_h_hybrid_mem_attrs[] = {
+	EVENT_PTR(mem_ld_arl_h),
+	EVENT_PTR(mem_st_arl_h),
+	NULL,
+};
+
 EVENT_ATTR_STR_HYBRID(tx-start,          tx_start_adl,          "event=0xc9,umask=0x1",          hybrid_big);
 EVENT_ATTR_STR_HYBRID(tx-commit,         tx_commit_adl,         "event=0xc9,umask=0x2",          hybrid_big);
 EVENT_ATTR_STR_HYBRID(tx-abort,          tx_abort_adl,          "event=0xc9,umask=0x4",          hybrid_big);
@@ -5993,8 +6061,8 @@ static struct attribute *adl_hybrid_tsx_attrs[] = {
 
 FORMAT_ATTR_HYBRID(in_tx,       hybrid_big);
 FORMAT_ATTR_HYBRID(in_tx_cp,    hybrid_big);
-FORMAT_ATTR_HYBRID(offcore_rsp, hybrid_big_small);
-FORMAT_ATTR_HYBRID(ldlat,       hybrid_big_small);
+FORMAT_ATTR_HYBRID(offcore_rsp, hybrid_big_small_arl_h);
+FORMAT_ATTR_HYBRID(ldlat,       hybrid_big_small_arl_h);
 FORMAT_ATTR_HYBRID(frontend,    hybrid_big);
 
 #define ADL_HYBRID_RTM_FORMAT_ATTR	\
@@ -6017,7 +6085,7 @@ static struct attribute *adl_hybrid_extra_attr[] = {
 	NULL
 };
 
-FORMAT_ATTR_HYBRID(snoop_rsp,	hybrid_small);
+FORMAT_ATTR_HYBRID(snoop_rsp,	hybrid_small_all);
 
 static struct attribute *mtl_hybrid_extra_attr_rtm[] = {
 	ADL_HYBRID_RTM_FORMAT_ATTR,
@@ -7098,6 +7166,37 @@ __init int intel_pmu_init(void)
 		name = "lunarlake_hybrid";
 		break;
 
+	case INTEL_ARROWLAKE_H:
+		intel_pmu_init_hybrid(hybrid_big_small_arl_h);
+
+		x86_pmu.pebs_latency_data = arl_h_latency_data;
+		x86_pmu.get_event_constraints = arl_h_get_event_constraints;
+		x86_pmu.hw_config = arl_h_hw_config;
+
+		td_attr = arl_h_hybrid_events_attrs;
+		mem_attr = arl_h_hybrid_mem_attrs;
+		tsx_attr = adl_hybrid_tsx_attrs;
+		extra_attr = boot_cpu_has(X86_FEATURE_RTM) ?
+			mtl_hybrid_extra_attr_rtm : mtl_hybrid_extra_attr;
+
+		/* Initialize big core specific PerfMon capabilities.*/
+		pmu = &x86_pmu.hybrid_pmu[X86_HYBRID_PMU_CORE_IDX];
+		intel_pmu_init_lnc(&pmu->pmu);
+
+		/* Initialize Atom core specific PerfMon capabilities.*/
+		pmu = &x86_pmu.hybrid_pmu[X86_HYBRID_PMU_ATOM_IDX];
+		intel_pmu_init_skt(&pmu->pmu);
+
+		/* Initialize Atom2 core specific PerfMon capabilities.*/
+		pmu = &x86_pmu.hybrid_pmu[X86_HYBRID_PMU_ATOM2_IDX];
+		intel_pmu_init_grt(&pmu->pmu);
+		pmu->extra_regs = intel_cmt_extra_regs;
+
+		intel_pmu_pebs_data_source_arl_h();
+		pr_cont("ArrowLake-H Hybrid events, ");
+		name = "arrowlake_h_hybrid";
+		break;
+
 	default:
 		switch (x86_pmu.version) {
 		case 1:
diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index fa5ea65de0d0..64242a8ffaf1 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -177,6 +177,17 @@ void __init intel_pmu_pebs_data_source_mtl(void)
 	__intel_pmu_pebs_data_source_cmt(data_source);
 }
 
+void __init intel_pmu_pebs_data_source_arl_h(void)
+{
+	u64 *data_source;
+
+	intel_pmu_pebs_data_source_lnl();
+
+	data_source = x86_pmu.hybrid_pmu[X86_HYBRID_PMU_ATOM2_IDX].pebs_data_source;
+	memcpy(data_source, pebs_data_source, sizeof(pebs_data_source));
+	__intel_pmu_pebs_data_source_cmt(data_source);
+}
+
 void __init intel_pmu_pebs_data_source_cmt(void)
 {
 	__intel_pmu_pebs_data_source_cmt(pebs_data_source);
@@ -388,6 +399,16 @@ u64 lnl_latency_data(struct perf_event *event, u64 status)
 	return lnc_latency_data(event, status);
 }
 
+u64 arl_h_latency_data(struct perf_event *event, u64 status)
+{
+	struct x86_hybrid_pmu *pmu = hybrid_pmu(event->pmu);
+
+	if (pmu->pmu_type == hybrid_small2)
+		return cmt_latency_data(event, status);
+
+	return lnl_latency_data(event, status);
+}
+
 static u64 load_latency_data(struct perf_event *event, u64 status)
 {
 	union intel_x86_pebs_dse dse;
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index f7b55c909eff..32fcbdb464e2 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -1591,6 +1591,8 @@ u64 cmt_latency_data(struct perf_event *event, u64 status);
 
 u64 lnl_latency_data(struct perf_event *event, u64 status);
 
+u64 arl_h_latency_data(struct perf_event *event, u64 status);
+
 extern struct event_constraint intel_core2_pebs_event_constraints[];
 
 extern struct event_constraint intel_atom_pebs_event_constraints[];
@@ -1710,6 +1712,8 @@ void intel_pmu_pebs_data_source_grt(void);
 
 void intel_pmu_pebs_data_source_mtl(void);
 
+void intel_pmu_pebs_data_source_arl_h(void);
+
 void intel_pmu_pebs_data_source_cmt(void);
 
 void intel_pmu_pebs_data_source_lnl(void);
-- 
2.40.1

Re: [PATCH 0/4] Enable PMU for ArrowLake-H

[Liang, Kan]

On 2024-08-08 10:02 a.m., Dapeng Mi wrote:
> ArrowLake-H is a specific variant of regular ArrowLake. It shares same
> PMU features on lioncove P-cores and skymont E-cores with regular
> ArrowLake except ArrowLake-H adds extra crestmont uarch E-cores.
> 
> Thus ArrowLake-H contains two different atom uarchs. This is totally
> different with previous Intel hybrid platforms which contains only one
> kind of atom uarchs. In this case, it's not enough to distinguish the
> uarchs just by core type. So CPUID.1AH.EAX[23:0] provides an unique
> native model ID for each uarch, this unique native model ID combining
> the core type can be used to uniquely identity the uarch.
> 
> This patch series introduces PMU support for ArrowLake-H. Besides
> inheriting the same PMU support from regular ArrowLake, it leverages
> the native model ID to detect the 2nd kind of atom uarch core and
> enables PMU support. To distinguish the two atom uarchs in sysfs, the
> PMU of 2nd atom uarch is named to "cpu_atom2".
> 
> Run basic counting, PMI based sampling, PEBS based sampling, LBR and
> topdown metrics tests on ArrowLake-H platform, no issue is found.
> 
> Dapeng Mi (4):
>   perf/x86: Refine hybrid_pmu_type defination
>   x86/cpu/intel: Define helper to get CPU core native ID
>   perf/x86/intel: Support hybrid PMU with multiple atom uarchs
>   perf/x86/intel: Add PMU support for ArrowLake-H

For the series,

Reviewed-by: Kan Liang <kan.liang@linux.intel.com>

Thanks,
Kan
> 
>  arch/x86/events/intel/core.c | 129 ++++++++++++++++++++++++++++++++---
>  arch/x86/events/intel/ds.c   |  21 ++++++
>  arch/x86/events/perf_event.h |  33 ++++++---
>  arch/x86/include/asm/cpu.h   |   6 ++
>  arch/x86/kernel/cpu/intel.c  |  15 ++++
>  5 files changed, 186 insertions(+), 18 deletions(-)
> 
> 
> base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b

Re: [PATCH 1/4] perf/x86: Refine hybrid_pmu_type defination

[Peter Zijlstra]

On Thu, Aug 08, 2024 at 02:02:07PM +0000, Dapeng Mi wrote:
> Use macros instead of magic number to define hybrid_pmu_type and remove
> X86_HYBRID_NUM_PMUS since it's never used.
> 
> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
> Tested-by: Yongwei Ma <yongwei.ma@intel.com>
> ---
>  arch/x86/events/perf_event.h | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
> index ac1182141bf6..5d1677844e04 100644
> --- a/arch/x86/events/perf_event.h
> +++ b/arch/x86/events/perf_event.h
> @@ -674,19 +674,16 @@ enum hybrid_cpu_type {
>  	HYBRID_INTEL_CORE	= 0x40,
>  };
>  
> +#define X86_HYBRID_PMU_ATOM_IDX		0
> +#define X86_HYBRID_PMU_CORE_IDX		1

There wants to be some whitespace here..

>  enum hybrid_pmu_type {
>  	not_hybrid,
> -	hybrid_small		= BIT(0),
> -	hybrid_big		= BIT(1),
> +	hybrid_small		= BIT(X86_HYBRID_PMU_ATOM_IDX),
> +	hybrid_big		= BIT(X86_HYBRID_PMU_CORE_IDX),
>  
>  	hybrid_big_small	= hybrid_big | hybrid_small, /* only used for matching */
>  };
>  
> -#define X86_HYBRID_PMU_ATOM_IDX		0
> -#define X86_HYBRID_PMU_CORE_IDX		1
> -
> -#define X86_HYBRID_NUM_PMUS		2
> -
>  struct x86_hybrid_pmu {
>  	struct pmu			pmu;
>  	const char			*name;
> -- 
> 2.40.1
> 

Re: [PATCH 3/4] perf/x86/intel: Support hybrid PMU with multiple atom uarchs

[Peter Zijlstra]

On Thu, Aug 08, 2024 at 02:02:09PM +0000, Dapeng Mi wrote:
>  arch/x86/events/intel/core.c | 24 +++++++++++++++++-------
>  arch/x86/events/perf_event.h | 18 +++++++++++++++++-
>  2 files changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index 0c9c2706d4ec..b6429bc009c0 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c

> @@ -6218,6 +6227,7 @@ static inline int intel_pmu_v6_addr_offset(int index, bool eventsel)
>  static const struct { enum hybrid_pmu_type id; char *name; } intel_hybrid_pmu_type_map[] __initconst = {
>  	{ hybrid_small, "cpu_atom" },
>  	{ hybrid_big, "cpu_core" },
> +	{ hybrid_small2, "cpu_atom2" },

This is awfully uninspired and quite terrible. How is one supposed to
know which is which? A possibly better naming might be: hybrid_tiny,
"cpu_lowpower" or whatever.

>  };
>  
>  static __always_inline int intel_pmu_init_hybrid(enum hybrid_pmu_type pmus)
> @@ -6250,7 +6260,7 @@ static __always_inline int intel_pmu_init_hybrid(enum hybrid_pmu_type pmus)
>  							0, x86_pmu_num_counters(&pmu->pmu), 0, 0);
>  
>  		pmu->intel_cap.capabilities = x86_pmu.intel_cap.capabilities;
> -		if (pmu->pmu_type & hybrid_small) {
> +		if (pmu->pmu_type & hybrid_small_all) {
>  			pmu->intel_cap.perf_metrics = 0;
>  			pmu->intel_cap.pebs_output_pt_available = 1;
>  			pmu->mid_ack = true;
> diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
> index 5d1677844e04..f7b55c909eff 100644
> --- a/arch/x86/events/perf_event.h
> +++ b/arch/x86/events/perf_event.h
> @@ -668,6 +668,13 @@ enum {
>  #define PERF_PEBS_DATA_SOURCE_GRT_MAX	0x10
>  #define PERF_PEBS_DATA_SOURCE_GRT_MASK	(PERF_PEBS_DATA_SOURCE_GRT_MAX - 1)
>  
> +
> +/*
> + * CPUID.1AH.EAX[31:0] uniquely identifies the microarchitecture
> + * of the core. Bits 31-24 indicates its core type (Core or Atom)
> + * and Bits [23:0] indicates the native model ID of the core.
> + * Core type and native model ID are defined in below enumerations.
> + */
>  enum hybrid_cpu_type {
>  	HYBRID_INTEL_NONE,
>  	HYBRID_INTEL_ATOM	= 0x20,
> @@ -676,12 +683,21 @@ enum hybrid_cpu_type {
>  
>  #define X86_HYBRID_PMU_ATOM_IDX		0
>  #define X86_HYBRID_PMU_CORE_IDX		1
> +#define X86_HYBRID_PMU_ATOM2_IDX	2
>  enum hybrid_pmu_type {
>  	not_hybrid,
>  	hybrid_small		= BIT(X86_HYBRID_PMU_ATOM_IDX),
>  	hybrid_big		= BIT(X86_HYBRID_PMU_CORE_IDX),
> +	hybrid_small2		= BIT(X86_HYBRID_PMU_ATOM2_IDX),
> +	/* The belows are only used for matching */
> +	hybrid_big_small	= hybrid_big | hybrid_small,
> +	hybrid_small_all	= hybrid_small | hybrid_small2,
> +	hybrid_big_small_arl_h	= hybrid_big | hybrid_small_all,

Same complaint, how about:

+	hybrid_tiny		= BIT(X86_HYBRID_PMU_TINY_IDX),
	hybrid_big_small	= hybrid_big | hybrid_small,
+	hybrid_small_tiny	= hybrid_small | hybrid_tiny,
+	hybrid_big_small_tiny	= hybrid_big_small | hybrid_tiny,


> +};
>  
> -	hybrid_big_small	= hybrid_big | hybrid_small, /* only used for matching */
> +enum atom_native_id {
> +	cmt_native_id           = 0x2,  /* Crestmont */
> +	skt_native_id           = 0x3,  /* Skymont */
>  };
>  
>  struct x86_hybrid_pmu {
> -- 
> 2.40.1
> 

Re: [PATCH 1/4] perf/x86: Refine hybrid_pmu_type defination

[Mi, Dapeng]

On 8/11/2024 5:35 AM, Peter Zijlstra wrote:
> On Thu, Aug 08, 2024 at 02:02:07PM +0000, Dapeng Mi wrote:
>> Use macros instead of magic number to define hybrid_pmu_type and remove
>> X86_HYBRID_NUM_PMUS since it's never used.
>>
>> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
>> Tested-by: Yongwei Ma <yongwei.ma@intel.com>
>> ---
>>  arch/x86/events/perf_event.h | 11 ++++-------
>>  1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
>> index ac1182141bf6..5d1677844e04 100644
>> --- a/arch/x86/events/perf_event.h
>> +++ b/arch/x86/events/perf_event.h
>> @@ -674,19 +674,16 @@ enum hybrid_cpu_type {
>>  	HYBRID_INTEL_CORE	= 0x40,
>>  };
>>  
>> +#define X86_HYBRID_PMU_ATOM_IDX		0
>> +#define X86_HYBRID_PMU_CORE_IDX		1
> There wants to be some whitespace here..

Sure. Thanks.


>
>>  enum hybrid_pmu_type {
>>  	not_hybrid,
>> -	hybrid_small		= BIT(0),
>> -	hybrid_big		= BIT(1),
>> +	hybrid_small		= BIT(X86_HYBRID_PMU_ATOM_IDX),
>> +	hybrid_big		= BIT(X86_HYBRID_PMU_CORE_IDX),
>>  
>>  	hybrid_big_small	= hybrid_big | hybrid_small, /* only used for matching */
>>  };
>>  
>> -#define X86_HYBRID_PMU_ATOM_IDX		0
>> -#define X86_HYBRID_PMU_CORE_IDX		1
>> -
>> -#define X86_HYBRID_NUM_PMUS		2
>> -
>>  struct x86_hybrid_pmu {
>>  	struct pmu			pmu;
>>  	const char			*name;
>> -- 
>> 2.40.1
>>

Re: [PATCH 3/4] perf/x86/intel: Support hybrid PMU with multiple atom uarchs

[Mi, Dapeng]

On 8/11/2024 5:55 AM, Peter Zijlstra wrote:
> On Thu, Aug 08, 2024 at 02:02:09PM +0000, Dapeng Mi wrote:
>>  arch/x86/events/intel/core.c | 24 +++++++++++++++++-------
>>  arch/x86/events/perf_event.h | 18 +++++++++++++++++-
>>  2 files changed, 34 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
>> index 0c9c2706d4ec..b6429bc009c0 100644
>> --- a/arch/x86/events/intel/core.c
>> +++ b/arch/x86/events/intel/core.c
>> @@ -6218,6 +6227,7 @@ static inline int intel_pmu_v6_addr_offset(int index, bool eventsel)
>>  static const struct { enum hybrid_pmu_type id; char *name; } intel_hybrid_pmu_type_map[] __initconst = {
>>  	{ hybrid_small, "cpu_atom" },
>>  	{ hybrid_big, "cpu_core" },
>> +	{ hybrid_small2, "cpu_atom2" },
> This is awfully uninspired and quite terrible. How is one supposed to
> know which is which? A possibly better naming might be: hybrid_tiny,
> "cpu_lowpower" or whatever.

We have lots of discussion internally about the naming, but unfortunately
we can't come to a conclusion. The reason that we select "cpu_atom2" is
that it's generic enough and won't expose too much model specific
information, we can reuse it if there are similar platforms in the future.
But of course I admit the name is indeed uninspired and easy to cause
confusion.

The other names which I ever discussed are "cpu_lp_soc", "cpu_soc" and
"cpu_atom_soc", but this name would expose some model specific architecture
information more or less, not sure which one is better. How is your opinion
on this?


>
>>  };
>>  
>>  static __always_inline int intel_pmu_init_hybrid(enum hybrid_pmu_type pmus)
>> @@ -6250,7 +6260,7 @@ static __always_inline int intel_pmu_init_hybrid(enum hybrid_pmu_type pmus)
>>  							0, x86_pmu_num_counters(&pmu->pmu), 0, 0);
>>  
>>  		pmu->intel_cap.capabilities = x86_pmu.intel_cap.capabilities;
>> -		if (pmu->pmu_type & hybrid_small) {
>> +		if (pmu->pmu_type & hybrid_small_all) {
>>  			pmu->intel_cap.perf_metrics = 0;
>>  			pmu->intel_cap.pebs_output_pt_available = 1;
>>  			pmu->mid_ack = true;
>> diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
>> index 5d1677844e04..f7b55c909eff 100644
>> --- a/arch/x86/events/perf_event.h
>> +++ b/arch/x86/events/perf_event.h
>> @@ -668,6 +668,13 @@ enum {
>>  #define PERF_PEBS_DATA_SOURCE_GRT_MAX	0x10
>>  #define PERF_PEBS_DATA_SOURCE_GRT_MASK	(PERF_PEBS_DATA_SOURCE_GRT_MAX - 1)
>>  
>> +
>> +/*
>> + * CPUID.1AH.EAX[31:0] uniquely identifies the microarchitecture
>> + * of the core. Bits 31-24 indicates its core type (Core or Atom)
>> + * and Bits [23:0] indicates the native model ID of the core.
>> + * Core type and native model ID are defined in below enumerations.
>> + */
>>  enum hybrid_cpu_type {
>>  	HYBRID_INTEL_NONE,
>>  	HYBRID_INTEL_ATOM	= 0x20,
>> @@ -676,12 +683,21 @@ enum hybrid_cpu_type {
>>  
>>  #define X86_HYBRID_PMU_ATOM_IDX		0
>>  #define X86_HYBRID_PMU_CORE_IDX		1
>> +#define X86_HYBRID_PMU_ATOM2_IDX	2
>>  enum hybrid_pmu_type {
>>  	not_hybrid,
>>  	hybrid_small		= BIT(X86_HYBRID_PMU_ATOM_IDX),
>>  	hybrid_big		= BIT(X86_HYBRID_PMU_CORE_IDX),
>> +	hybrid_small2		= BIT(X86_HYBRID_PMU_ATOM2_IDX),
>> +	/* The belows are only used for matching */
>> +	hybrid_big_small	= hybrid_big | hybrid_small,
>> +	hybrid_small_all	= hybrid_small | hybrid_small2,
>> +	hybrid_big_small_arl_h	= hybrid_big | hybrid_small_all,
> Same complaint, how about:
>
> +	hybrid_tiny		= BIT(X86_HYBRID_PMU_TINY_IDX),
> 	hybrid_big_small	= hybrid_big | hybrid_small,
> +	hybrid_small_tiny	= hybrid_small | hybrid_tiny,
> +	hybrid_big_small_tiny	= hybrid_big_small | hybrid_tiny,

Sure. I would adjust the macro name base on the above discussed final name.
Thanks.


>
>
>> +};
>>  
>> -	hybrid_big_small	= hybrid_big | hybrid_small, /* only used for matching */
>> +enum atom_native_id {
>> +	cmt_native_id           = 0x2,  /* Crestmont */
>> +	skt_native_id           = 0x3,  /* Skymont */
>>  };
>>  
>>  struct x86_hybrid_pmu {
>> -- 
>> 2.40.1
>>

Re: [PATCH 3/4] perf/x86/intel: Support hybrid PMU with multiple atom uarchs

[Zhenyu Wang]

[-- Attachment #1: Type: text/plain, Size: 4555 bytes --]

On 2024.08.12 11:18:34 +0800, Mi, Dapeng wrote:
> 
> On 8/11/2024 5:55 AM, Peter Zijlstra wrote:
> > On Thu, Aug 08, 2024 at 02:02:09PM +0000, Dapeng Mi wrote:
> >>  arch/x86/events/intel/core.c | 24 +++++++++++++++++-------
> >>  arch/x86/events/perf_event.h | 18 +++++++++++++++++-
> >>  2 files changed, 34 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> >> index 0c9c2706d4ec..b6429bc009c0 100644
> >> --- a/arch/x86/events/intel/core.c
> >> +++ b/arch/x86/events/intel/core.c
> >> @@ -6218,6 +6227,7 @@ static inline int intel_pmu_v6_addr_offset(int index, bool eventsel)
> >>  static const struct { enum hybrid_pmu_type id; char *name; } intel_hybrid_pmu_type_map[] __initconst = {
> >>  	{ hybrid_small, "cpu_atom" },
> >>  	{ hybrid_big, "cpu_core" },
> >> +	{ hybrid_small2, "cpu_atom2" },
> > This is awfully uninspired and quite terrible. How is one supposed to
> > know which is which? A possibly better naming might be: hybrid_tiny,
> > "cpu_lowpower" or whatever.
> 
> We have lots of discussion internally about the naming, but unfortunately
> we can't come to a conclusion. The reason that we select "cpu_atom2" is
> that it's generic enough and won't expose too much model specific
> information, we can reuse it if there are similar platforms in the future.
> But of course I admit the name is indeed uninspired and easy to cause
> confusion.
> 
> The other names which I ever discussed are "cpu_lp_soc", "cpu_soc" and
> "cpu_atom_soc", but this name would expose some model specific architecture
> information more or less, not sure which one is better. How is your opinion
> on this?
>

Now I don't like to put 'soc' in name as it's specific for platform design
e.g ARL-H, but pmu actually only cares about cpu type. Maybe "cpu_atom_lp"
is good enough.

> 
> >
> >>  };
> >>  
> >>  static __always_inline int intel_pmu_init_hybrid(enum hybrid_pmu_type pmus)
> >> @@ -6250,7 +6260,7 @@ static __always_inline int intel_pmu_init_hybrid(enum hybrid_pmu_type pmus)
> >>  							0, x86_pmu_num_counters(&pmu->pmu), 0, 0);
> >>  
> >>  		pmu->intel_cap.capabilities = x86_pmu.intel_cap.capabilities;
> >> -		if (pmu->pmu_type & hybrid_small) {
> >> +		if (pmu->pmu_type & hybrid_small_all) {
> >>  			pmu->intel_cap.perf_metrics = 0;
> >>  			pmu->intel_cap.pebs_output_pt_available = 1;
> >>  			pmu->mid_ack = true;
> >> diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
> >> index 5d1677844e04..f7b55c909eff 100644
> >> --- a/arch/x86/events/perf_event.h
> >> +++ b/arch/x86/events/perf_event.h
> >> @@ -668,6 +668,13 @@ enum {
> >>  #define PERF_PEBS_DATA_SOURCE_GRT_MAX	0x10
> >>  #define PERF_PEBS_DATA_SOURCE_GRT_MASK	(PERF_PEBS_DATA_SOURCE_GRT_MAX - 1)
> >>  
> >> +
> >> +/*
> >> + * CPUID.1AH.EAX[31:0] uniquely identifies the microarchitecture
> >> + * of the core. Bits 31-24 indicates its core type (Core or Atom)
> >> + * and Bits [23:0] indicates the native model ID of the core.
> >> + * Core type and native model ID are defined in below enumerations.
> >> + */
> >>  enum hybrid_cpu_type {
> >>  	HYBRID_INTEL_NONE,
> >>  	HYBRID_INTEL_ATOM	= 0x20,
> >> @@ -676,12 +683,21 @@ enum hybrid_cpu_type {
> >>  
> >>  #define X86_HYBRID_PMU_ATOM_IDX		0
> >>  #define X86_HYBRID_PMU_CORE_IDX		1
> >> +#define X86_HYBRID_PMU_ATOM2_IDX	2
> >>  enum hybrid_pmu_type {
> >>  	not_hybrid,
> >>  	hybrid_small		= BIT(X86_HYBRID_PMU_ATOM_IDX),
> >>  	hybrid_big		= BIT(X86_HYBRID_PMU_CORE_IDX),
> >> +	hybrid_small2		= BIT(X86_HYBRID_PMU_ATOM2_IDX),
> >> +	/* The belows are only used for matching */
> >> +	hybrid_big_small	= hybrid_big | hybrid_small,
> >> +	hybrid_small_all	= hybrid_small | hybrid_small2,
> >> +	hybrid_big_small_arl_h	= hybrid_big | hybrid_small_all,
> > Same complaint, how about:
> >
> > +	hybrid_tiny		= BIT(X86_HYBRID_PMU_TINY_IDX),
> > 	hybrid_big_small	= hybrid_big | hybrid_small,
> > +	hybrid_small_tiny	= hybrid_small | hybrid_tiny,
> > +	hybrid_big_small_tiny	= hybrid_big_small | hybrid_tiny,
> 
> Sure. I would adjust the macro name base on the above discussed final name.
> Thanks.
> 
> 
> >
> >
> >> +};
> >>  
> >> -	hybrid_big_small	= hybrid_big | hybrid_small, /* only used for matching */
> >> +enum atom_native_id {
> >> +	cmt_native_id           = 0x2,  /* Crestmont */
> >> +	skt_native_id           = 0x3,  /* Skymont */
> >>  };
> >>  
> >>  struct x86_hybrid_pmu {
> >> -- 
> >> 2.40.1
> >>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH 3/4] perf/x86/intel: Support hybrid PMU with multiple atom uarchs

[Mi, Dapeng]

On 8/12/2024 11:27 AM, Zhenyu Wang wrote:
> On 2024.08.12 11:18:34 +0800, Mi, Dapeng wrote:
>> On 8/11/2024 5:55 AM, Peter Zijlstra wrote:
>>> On Thu, Aug 08, 2024 at 02:02:09PM +0000, Dapeng Mi wrote:
>>>>  arch/x86/events/intel/core.c | 24 +++++++++++++++++-------
>>>>  arch/x86/events/perf_event.h | 18 +++++++++++++++++-
>>>>  2 files changed, 34 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
>>>> index 0c9c2706d4ec..b6429bc009c0 100644
>>>> --- a/arch/x86/events/intel/core.c
>>>> +++ b/arch/x86/events/intel/core.c
>>>> @@ -6218,6 +6227,7 @@ static inline int intel_pmu_v6_addr_offset(int index, bool eventsel)
>>>>  static const struct { enum hybrid_pmu_type id; char *name; } intel_hybrid_pmu_type_map[] __initconst = {
>>>>  	{ hybrid_small, "cpu_atom" },
>>>>  	{ hybrid_big, "cpu_core" },
>>>> +	{ hybrid_small2, "cpu_atom2" },
>>> This is awfully uninspired and quite terrible. How is one supposed to
>>> know which is which? A possibly better naming might be: hybrid_tiny,
>>> "cpu_lowpower" or whatever.
>> We have lots of discussion internally about the naming, but unfortunately
>> we can't come to a conclusion. The reason that we select "cpu_atom2" is
>> that it's generic enough and won't expose too much model specific
>> information, we can reuse it if there are similar platforms in the future.
>> But of course I admit the name is indeed uninspired and easy to cause
>> confusion.
>>
>> The other names which I ever discussed are "cpu_lp_soc", "cpu_soc" and
>> "cpu_atom_soc", but this name would expose some model specific architecture
>> information more or less, not sure which one is better. How is your opinion
>> on this?
>>
> Now I don't like to put 'soc' in name as it's specific for platform design
> e.g ARL-H, but pmu actually only cares about cpu type. Maybe "cpu_atom_lp"
> is good enough.

Synced with Kan, Andi and Zhenyu, all prefer to use the name
"cpu_lowpower". If no one objects it, it would be used as official name.


>
>>>>  };
>>>>  
>>>>  static __always_inline int intel_pmu_init_hybrid(enum hybrid_pmu_type pmus)
>>>> @@ -6250,7 +6260,7 @@ static __always_inline int intel_pmu_init_hybrid(enum hybrid_pmu_type pmus)
>>>>  							0, x86_pmu_num_counters(&pmu->pmu), 0, 0);
>>>>  
>>>>  		pmu->intel_cap.capabilities = x86_pmu.intel_cap.capabilities;
>>>> -		if (pmu->pmu_type & hybrid_small) {
>>>> +		if (pmu->pmu_type & hybrid_small_all) {
>>>>  			pmu->intel_cap.perf_metrics = 0;
>>>>  			pmu->intel_cap.pebs_output_pt_available = 1;
>>>>  			pmu->mid_ack = true;
>>>> diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
>>>> index 5d1677844e04..f7b55c909eff 100644
>>>> --- a/arch/x86/events/perf_event.h
>>>> +++ b/arch/x86/events/perf_event.h
>>>> @@ -668,6 +668,13 @@ enum {
>>>>  #define PERF_PEBS_DATA_SOURCE_GRT_MAX	0x10
>>>>  #define PERF_PEBS_DATA_SOURCE_GRT_MASK	(PERF_PEBS_DATA_SOURCE_GRT_MAX - 1)
>>>>  
>>>> +
>>>> +/*
>>>> + * CPUID.1AH.EAX[31:0] uniquely identifies the microarchitecture
>>>> + * of the core. Bits 31-24 indicates its core type (Core or Atom)
>>>> + * and Bits [23:0] indicates the native model ID of the core.
>>>> + * Core type and native model ID are defined in below enumerations.
>>>> + */
>>>>  enum hybrid_cpu_type {
>>>>  	HYBRID_INTEL_NONE,
>>>>  	HYBRID_INTEL_ATOM	= 0x20,
>>>> @@ -676,12 +683,21 @@ enum hybrid_cpu_type {
>>>>  
>>>>  #define X86_HYBRID_PMU_ATOM_IDX		0
>>>>  #define X86_HYBRID_PMU_CORE_IDX		1
>>>> +#define X86_HYBRID_PMU_ATOM2_IDX	2
>>>>  enum hybrid_pmu_type {
>>>>  	not_hybrid,
>>>>  	hybrid_small		= BIT(X86_HYBRID_PMU_ATOM_IDX),
>>>>  	hybrid_big		= BIT(X86_HYBRID_PMU_CORE_IDX),
>>>> +	hybrid_small2		= BIT(X86_HYBRID_PMU_ATOM2_IDX),
>>>> +	/* The belows are only used for matching */
>>>> +	hybrid_big_small	= hybrid_big | hybrid_small,
>>>> +	hybrid_small_all	= hybrid_small | hybrid_small2,
>>>> +	hybrid_big_small_arl_h	= hybrid_big | hybrid_small_all,
>>> Same complaint, how about:
>>>
>>> +	hybrid_tiny		= BIT(X86_HYBRID_PMU_TINY_IDX),
>>> 	hybrid_big_small	= hybrid_big | hybrid_small,
>>> +	hybrid_small_tiny	= hybrid_small | hybrid_tiny,
>>> +	hybrid_big_small_tiny	= hybrid_big_small | hybrid_tiny,
>> Sure. I would adjust the macro name base on the above discussed final name.
>> Thanks.
>>
>>
>>>
>>>> +};
>>>>  
>>>> -	hybrid_big_small	= hybrid_big | hybrid_small, /* only used for matching */
>>>> +enum atom_native_id {
>>>> +	cmt_native_id           = 0x2,  /* Crestmont */
>>>> +	skt_native_id           = 0x3,  /* Skymont */
>>>>  };
>>>>  
>>>>  struct x86_hybrid_pmu {
>>>> -- 
>>>> 2.40.1
>>>>