[v1] Enable PMU for ArrowLake-H

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

Posted by Dapeng Mi 1 year, 6 months ago

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

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

Posted by Peter Zijlstra 1 year, 6 months ago

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 3/4] perf/x86/intel: Support hybrid PMU with multiple atom uarchs

Posted by Mi, Dapeng 1 year, 5 months ago

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

Posted by Zhenyu Wang 1 year, 5 months ago

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
> >>

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

Posted by Mi, Dapeng 1 year, 5 months ago

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
>>>>

[PATCH 1/4] perf/x86: Refine hybrid_pmu_type defination
[PATCH 2/4] x86/cpu/intel: Define helper to get CPU core native ID
[PATCH 3/4] perf/x86/intel: Support hybrid PMU with multiple atom uarchs
[PATCH 4/4] perf/x86/intel: Add PMU support for ArrowLake-H