[PATCH] perf/x86/intel: Clean up PEBS-via-PT on hybrid

[PATCH] perf/x86/intel: Clean up PEBS-via-PT on hybrid

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

The PEBS-via-PT feature is exposed for the e-core of some hybrid
platforms, e.g., ADL and MTL. But it never works.

$ dmesg | grep PEBS
[    1.793888] core: cpu_atom PMU driver: PEBS-via-PT

$ perf record -c 1000 -e '{intel_pt/branch=0/,
cpu_atom/cpu-cycles,aux-output/pp}' -C8
Error:
The sys_perf_event_open() syscall returned with 22 (Invalid argument)
for event (cpu_atom/cpu-cycles,aux-output/pp).
"dmesg | grep -i perf" may provide additional information.

The "PEBS-via-PT" is printed if the corresponding bit of per-PMU
capabilities is set. Since the feature is supported by the e-core HW,
perf sets the bit for e-core. However, for Intel PT, if a feature is not
supported on all CPUs, it is not supported at all. The PEBS-via-PT event
cannot be created successfully.

The PEBS-via-PT is no longer enumerated on the latest hybrid platform. It
will be deprecated on future platforms with Arch PEBS. Let's remove it
from the existing hybrid platforms.

Fixes: d9977c43bff8 ("perf/x86: Register hybrid PMUs")
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 arch/x86/events/intel/core.c | 10 ----------
 arch/x86/events/intel/ds.c   |  2 +-
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 7f1b6b90a5fb..0a1030eb6db8 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -4981,11 +4981,6 @@ static void intel_pmu_check_hybrid_pmus(struct x86_hybrid_pmu *pmu)
 	else
 		pmu->intel_ctrl &= ~(1ULL << GLOBAL_CTRL_EN_PERF_METRICS);
 
-	if (pmu->intel_cap.pebs_output_pt_available)
-		pmu->pmu.capabilities |= PERF_PMU_CAP_AUX_OUTPUT;
-	else
-		pmu->pmu.capabilities &= ~PERF_PMU_CAP_AUX_OUTPUT;
-
 	intel_pmu_check_event_constraints(pmu->event_constraints,
 					  pmu->cntr_mask64,
 					  pmu->fixed_cntr_mask64,
@@ -5063,9 +5058,6 @@ static bool init_hybrid_pmu(int cpu)
 
 	pr_info("%s PMU driver: ", pmu->name);
 
-	if (pmu->intel_cap.pebs_output_pt_available)
-		pr_cont("PEBS-via-PT ");
-
 	pr_cont("\n");
 
 	x86_pmu_show_pmu_cap(&pmu->pmu);
@@ -6420,11 +6412,9 @@ static __always_inline int intel_pmu_init_hybrid(enum hybrid_pmu_type pmus)
 		pmu->intel_cap.capabilities = x86_pmu.intel_cap.capabilities;
 		if (pmu->pmu_type & hybrid_small_tiny) {
 			pmu->intel_cap.perf_metrics = 0;
-			pmu->intel_cap.pebs_output_pt_available = 1;
 			pmu->mid_ack = true;
 		} else if (pmu->pmu_type & hybrid_big) {
 			pmu->intel_cap.perf_metrics = 1;
-			pmu->intel_cap.pebs_output_pt_available = 0;
 			pmu->late_ack = true;
 		}
 	}
diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index 13a78a8a2780..123b0e4392b4 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -2742,7 +2742,7 @@ void __init intel_ds_init(void)
 			}
 			pr_cont("PEBS fmt%d%c%s, ", format, pebs_type, pebs_qual);
 
-			if (!is_hybrid() && x86_pmu.intel_cap.pebs_output_pt_available) {
+			if (x86_pmu.intel_cap.pebs_output_pt_available) {
 				pr_cont("PEBS-via-PT, ");
 				x86_get_pmu(smp_processor_id())->capabilities |= PERF_PMU_CAP_AUX_OUTPUT;
 			}
-- 
2.38.1

Re: [PATCH] perf/x86/intel: Clean up PEBS-via-PT on hybrid

[Adrian Hunter]

On 24/01/25 20:34, kan.liang@linux.intel.com wrote:
> From: Kan Liang <kan.liang@linux.intel.com>
> 
> The PEBS-via-PT feature is exposed for the e-core of some hybrid
> platforms, e.g., ADL and MTL. But it never works.
> 
> $ dmesg | grep PEBS
> [    1.793888] core: cpu_atom PMU driver: PEBS-via-PT
> 
> $ perf record -c 1000 -e '{intel_pt/branch=0/,
> cpu_atom/cpu-cycles,aux-output/pp}' -C8
> Error:
> The sys_perf_event_open() syscall returned with 22 (Invalid argument)
> for event (cpu_atom/cpu-cycles,aux-output/pp).
> "dmesg | grep -i perf" may provide additional information.
> 
> The "PEBS-via-PT" is printed if the corresponding bit of per-PMU
> capabilities is set. Since the feature is supported by the e-core HW,
> perf sets the bit for e-core. However, for Intel PT, if a feature is not
> supported on all CPUs, it is not supported at all. The PEBS-via-PT event
> cannot be created successfully.
> 
> The PEBS-via-PT is no longer enumerated on the latest hybrid platform. It
> will be deprecated on future platforms with Arch PEBS. Let's remove it
> from the existing hybrid platforms.
> 
> Fixes: d9977c43bff8 ("perf/x86: Register hybrid PMUs")
> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> ---
>  arch/x86/events/intel/core.c | 10 ----------
>  arch/x86/events/intel/ds.c   |  2 +-
>  2 files changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index 7f1b6b90a5fb..0a1030eb6db8 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -4981,11 +4981,6 @@ static void intel_pmu_check_hybrid_pmus(struct x86_hybrid_pmu *pmu)
>  	else
>  		pmu->intel_ctrl &= ~(1ULL << GLOBAL_CTRL_EN_PERF_METRICS);
>  
> -	if (pmu->intel_cap.pebs_output_pt_available)
> -		pmu->pmu.capabilities |= PERF_PMU_CAP_AUX_OUTPUT;
> -	else
> -		pmu->pmu.capabilities &= ~PERF_PMU_CAP_AUX_OUTPUT;
> -
>  	intel_pmu_check_event_constraints(pmu->event_constraints,
>  					  pmu->cntr_mask64,
>  					  pmu->fixed_cntr_mask64,
> @@ -5063,9 +5058,6 @@ static bool init_hybrid_pmu(int cpu)
>  
>  	pr_info("%s PMU driver: ", pmu->name);
>  
> -	if (pmu->intel_cap.pebs_output_pt_available)
> -		pr_cont("PEBS-via-PT ");
> -
>  	pr_cont("\n");
>  
>  	x86_pmu_show_pmu_cap(&pmu->pmu);
> @@ -6420,11 +6412,9 @@ static __always_inline int intel_pmu_init_hybrid(enum hybrid_pmu_type pmus)
>  		pmu->intel_cap.capabilities = x86_pmu.intel_cap.capabilities;
>  		if (pmu->pmu_type & hybrid_small_tiny) {
>  			pmu->intel_cap.perf_metrics = 0;
> -			pmu->intel_cap.pebs_output_pt_available = 1;
>  			pmu->mid_ack = true;
>  		} else if (pmu->pmu_type & hybrid_big) {
>  			pmu->intel_cap.perf_metrics = 1;
> -			pmu->intel_cap.pebs_output_pt_available = 0;
>  			pmu->late_ack = true;
>  		}
>  	}
> diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
> index 13a78a8a2780..123b0e4392b4 100644
> --- a/arch/x86/events/intel/ds.c
> +++ b/arch/x86/events/intel/ds.c
> @@ -2742,7 +2742,7 @@ void __init intel_ds_init(void)
>  			}
>  			pr_cont("PEBS fmt%d%c%s, ", format, pebs_type, pebs_qual);
>  
> -			if (!is_hybrid() && x86_pmu.intel_cap.pebs_output_pt_available) {
> +			if (x86_pmu.intel_cap.pebs_output_pt_available) {

I'd suggest keeping the !is_hybrid() check here
since it documents and enforces the effective policy.

>  				pr_cont("PEBS-via-PT, ");
>  				x86_get_pmu(smp_processor_id())->capabilities |= PERF_PMU_CAP_AUX_OUTPUT;
>  			}

Re: [PATCH] perf/x86/intel: Clean up PEBS-via-PT on hybrid

[Alexander Shishkin]

[sending again, because smtp is hard, for some reason]
kan.liang@linux.intel.com writes:

> From: Kan Liang <kan.liang@linux.intel.com>
>
> The PEBS-via-PT feature is exposed for the e-core of some hybrid
> platforms, e.g., ADL and MTL. But it never works.
>
> $ dmesg | grep PEBS
> [    1.793888] core: cpu_atom PMU driver: PEBS-via-PT
>
> $ perf record -c 1000 -e '{intel_pt/branch=0/,
> cpu_atom/cpu-cycles,aux-output/pp}' -C8
> Error:
> The sys_perf_event_open() syscall returned with 22 (Invalid argument)
> for event (cpu_atom/cpu-cycles,aux-output/pp).
> "dmesg | grep -i perf" may provide additional information.
>
> The "PEBS-via-PT" is printed if the corresponding bit of per-PMU
> capabilities is set. Since the feature is supported by the e-core HW,
> perf sets the bit for e-core. However, for Intel PT, if a feature is not
> supported on all CPUs, it is not supported at all. The PEBS-via-PT event
> cannot be created successfully.
>
> The PEBS-via-PT is no longer enumerated on the latest hybrid platform. It
> will be deprecated on future platforms with Arch PEBS. Let's remove it
> from the existing hybrid platforms.

There are still lots of them in use in the wild, I'd still like to fix
this properly instead of chopping it off. Unless there's a reason not to?

Thanks,
--
Alex

Re: [PATCH] perf/x86/intel: Clean up PEBS-via-PT on hybrid

[Adrian Hunter]

On 27/01/25 11:42, Alexander Shishkin wrote:
> [sending again, because smtp is hard, for some reason]
> kan.liang@linux.intel.com writes:
> 
>> From: Kan Liang <kan.liang@linux.intel.com>
>>
>> The PEBS-via-PT feature is exposed for the e-core of some hybrid
>> platforms, e.g., ADL and MTL. But it never works.
>>
>> $ dmesg | grep PEBS
>> [    1.793888] core: cpu_atom PMU driver: PEBS-via-PT
>>
>> $ perf record -c 1000 -e '{intel_pt/branch=0/,
>> cpu_atom/cpu-cycles,aux-output/pp}' -C8
>> Error:
>> The sys_perf_event_open() syscall returned with 22 (Invalid argument)
>> for event (cpu_atom/cpu-cycles,aux-output/pp).
>> "dmesg | grep -i perf" may provide additional information.
>>
>> The "PEBS-via-PT" is printed if the corresponding bit of per-PMU
>> capabilities is set. Since the feature is supported by the e-core HW,
>> perf sets the bit for e-core. However, for Intel PT, if a feature is not
>> supported on all CPUs, it is not supported at all. The PEBS-via-PT event
>> cannot be created successfully.
>>
>> The PEBS-via-PT is no longer enumerated on the latest hybrid platform. It
>> will be deprecated on future platforms with Arch PEBS. Let's remove it
>> from the existing hybrid platforms.
> 
> There are still lots of them in use in the wild, I'd still like to fix
> this properly instead of chopping it off. Unless there's a reason not to?

The kernel has never supported PEBS-via-PT on hybrids before.
AFAIK no one is asking for it on hybrids.

Note, it will continue to work on non-hybrid atom/e-core systems
like ADL-N etc

Re: [PATCH] perf/x86/intel: Clean up PEBS-via-PT on hybrid

[Liang, Kan]

On 2025-01-27 3:19 a.m., Adrian Hunter wrote:
> On 24/01/25 20:34, kan.liang@linux.intel.com wrote:
>> From: Kan Liang <kan.liang@linux.intel.com>
>>
>> The PEBS-via-PT feature is exposed for the e-core of some hybrid
>> platforms, e.g., ADL and MTL. But it never works.
>>
>> $ dmesg | grep PEBS
>> [    1.793888] core: cpu_atom PMU driver: PEBS-via-PT
>>
>> $ perf record -c 1000 -e '{intel_pt/branch=0/,
>> cpu_atom/cpu-cycles,aux-output/pp}' -C8
>> Error:
>> The sys_perf_event_open() syscall returned with 22 (Invalid argument)
>> for event (cpu_atom/cpu-cycles,aux-output/pp).
>> "dmesg | grep -i perf" may provide additional information.
>>
>> The "PEBS-via-PT" is printed if the corresponding bit of per-PMU
>> capabilities is set. Since the feature is supported by the e-core HW,
>> perf sets the bit for e-core. However, for Intel PT, if a feature is not
>> supported on all CPUs, it is not supported at all. The PEBS-via-PT event
>> cannot be created successfully.
>>
>> The PEBS-via-PT is no longer enumerated on the latest hybrid platform. It
>> will be deprecated on future platforms with Arch PEBS. Let's remove it
>> from the existing hybrid platforms.
>>
>> Fixes: d9977c43bff8 ("perf/x86: Register hybrid PMUs")
>> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
>> Cc: Adrian Hunter <adrian.hunter@intel.com>
>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> ---
>>  arch/x86/events/intel/core.c | 10 ----------
>>  arch/x86/events/intel/ds.c   |  2 +-
>>  2 files changed, 1 insertion(+), 11 deletions(-)
>>
>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
>> index 7f1b6b90a5fb..0a1030eb6db8 100644
>> --- a/arch/x86/events/intel/core.c
>> +++ b/arch/x86/events/intel/core.c
>> @@ -4981,11 +4981,6 @@ static void intel_pmu_check_hybrid_pmus(struct x86_hybrid_pmu *pmu)
>>  	else
>>  		pmu->intel_ctrl &= ~(1ULL << GLOBAL_CTRL_EN_PERF_METRICS);
>>  
>> -	if (pmu->intel_cap.pebs_output_pt_available)
>> -		pmu->pmu.capabilities |= PERF_PMU_CAP_AUX_OUTPUT;
>> -	else
>> -		pmu->pmu.capabilities &= ~PERF_PMU_CAP_AUX_OUTPUT;
>> -
>>  	intel_pmu_check_event_constraints(pmu->event_constraints,
>>  					  pmu->cntr_mask64,
>>  					  pmu->fixed_cntr_mask64,
>> @@ -5063,9 +5058,6 @@ static bool init_hybrid_pmu(int cpu)
>>  
>>  	pr_info("%s PMU driver: ", pmu->name);
>>  
>> -	if (pmu->intel_cap.pebs_output_pt_available)
>> -		pr_cont("PEBS-via-PT ");
>> -
>>  	pr_cont("\n");
>>  
>>  	x86_pmu_show_pmu_cap(&pmu->pmu);
>> @@ -6420,11 +6412,9 @@ static __always_inline int intel_pmu_init_hybrid(enum hybrid_pmu_type pmus)
>>  		pmu->intel_cap.capabilities = x86_pmu.intel_cap.capabilities;
>>  		if (pmu->pmu_type & hybrid_small_tiny) {
>>  			pmu->intel_cap.perf_metrics = 0;
>> -			pmu->intel_cap.pebs_output_pt_available = 1;
>>  			pmu->mid_ack = true;
>>  		} else if (pmu->pmu_type & hybrid_big) {
>>  			pmu->intel_cap.perf_metrics = 1;
>> -			pmu->intel_cap.pebs_output_pt_available = 0;
>>  			pmu->late_ack = true;
>>  		}
>>  	}
>> diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
>> index 13a78a8a2780..123b0e4392b4 100644
>> --- a/arch/x86/events/intel/ds.c
>> +++ b/arch/x86/events/intel/ds.c
>> @@ -2742,7 +2742,7 @@ void __init intel_ds_init(void)
>>  			}
>>  			pr_cont("PEBS fmt%d%c%s, ", format, pebs_type, pebs_qual);
>>  
>> -			if (!is_hybrid() && x86_pmu.intel_cap.pebs_output_pt_available) {
>> +			if (x86_pmu.intel_cap.pebs_output_pt_available) {
> 
> I'd suggest keeping the !is_hybrid() check here
> since it documents and enforces the effective policy.

Sigh, the !is_hybrid() should never work here either. Because the
intel_ds_init() is invoked before intel_pmu_init_hybrid(), which enable
perf_is_hybrid.

I think we'd better remove it, but I will add some comments to explain
the status of PEBS-via-PT on hybrid instead. For example,

/*
 * The PEBS-via-PT is not supported on hybrid platforms, because not all
 * CPUs of a hybrid machine support the feature. The global
 * x86_pmu.intel_cap, which only contains the common capabilities, is
 * used to check the availability of the feature. The per-PMU
 * pebs_output_pt_available in a hybrid machine will be ignored.
 */

Thanks,
Kan
> 
>>  				pr_cont("PEBS-via-PT, ");
>>  				x86_get_pmu(smp_processor_id())->capabilities |= PERF_PMU_CAP_AUX_OUTPUT;
>>  			}
> 
> 

Re: [PATCH] perf/x86/intel: Clean up PEBS-via-PT on hybrid

[Liang, Kan]

On 2025-01-27 6:42 a.m., Adrian Hunter wrote:
> On 27/01/25 11:42, Alexander Shishkin wrote:
>> [sending again, because smtp is hard, for some reason]
>> kan.liang@linux.intel.com writes:
>>
>>> From: Kan Liang <kan.liang@linux.intel.com>
>>>
>>> The PEBS-via-PT feature is exposed for the e-core of some hybrid
>>> platforms, e.g., ADL and MTL. But it never works.
>>>
>>> $ dmesg | grep PEBS
>>> [    1.793888] core: cpu_atom PMU driver: PEBS-via-PT
>>>
>>> $ perf record -c 1000 -e '{intel_pt/branch=0/,
>>> cpu_atom/cpu-cycles,aux-output/pp}' -C8
>>> Error:
>>> The sys_perf_event_open() syscall returned with 22 (Invalid argument)
>>> for event (cpu_atom/cpu-cycles,aux-output/pp).
>>> "dmesg | grep -i perf" may provide additional information.
>>>
>>> The "PEBS-via-PT" is printed if the corresponding bit of per-PMU
>>> capabilities is set. Since the feature is supported by the e-core HW,
>>> perf sets the bit for e-core. However, for Intel PT, if a feature is not
>>> supported on all CPUs, it is not supported at all. The PEBS-via-PT event
>>> cannot be created successfully.
>>>
>>> The PEBS-via-PT is no longer enumerated on the latest hybrid platform. It
>>> will be deprecated on future platforms with Arch PEBS. Let's remove it
>>> from the existing hybrid platforms.
>>
>> There are still lots of them in use in the wild, I'd still like to fix
>> this properly instead of chopping it off. Unless there's a reason not to?
> 
> The kernel has never supported PEBS-via-PT on hybrids before.
> AFAIK no one is asking for it on hybrids.

Right, I didn't get the request on hybrid either.

> 
> Note, it will continue to work on non-hybrid atom/e-core systems
> like ADL-N etc
> 

Right, ADL-N and all the non-hybrid still works.
It removes the false claim on the hybrid ADL, MTL, RPL, LNL, and Arrow Lake.

Thanks,
Kan