[PATCH V3] arm64: perf: Retain PMCR.X of PMCR_EL0 during reset

[PATCH V3] arm64: perf: Retain PMCR.X of PMCR_EL0 during reset

[Srinivasarao Pathipati]

Preserve the bit PMCR.X of PMCR_EL0 during PMU reset to export
PMU events. as it could be set before PMU initialization.

Exporting events could result in additional power consumption
so making it configurable.

Signed-off-by: Srinivasarao Pathipati <quic_c_spathi@quicinc.com>
---
Changes since V2:
	Done below changes as per Will's comments
	- enabling pmcr_x now configurable with kernel parameters and
	  by default it is disabled.
	
Changes since V1:
	- Preserving only PMCR_X bit as per Robin Murphy's comment.
---
 Documentation/admin-guide/kernel-parameters.txt |  4 ++++
 arch/arm64/kernel/perf_event.c                  | 13 +++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index de3da15..cf3410e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5150,6 +5150,10 @@
 			Useful for devices that are detected asynchronously
 			(e.g. USB and MMC devices).
 
+	retain_pmcr_x
+			[KNL] Retain export bit of PMCR_EL0 that is getting reset
+			during pmu reset.
+
 	retain_initrd	[RAM] Keep initrd memory after extraction
 
 	rfkill.default_state=
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index cb69ff1..1ac251f 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -34,6 +34,7 @@
 #define ARMV8_THUNDER_PERFCTR_L1I_CACHE_PREF_ACCESS		0xEC
 #define ARMV8_THUNDER_PERFCTR_L1I_CACHE_PREF_MISS		0xED
 
+static int export_pmu_events  __read_mostly;
 /*
  * ARMv8 Architectural defined events, not all of these may
  * be supported on any given implementation. Unsupported events will
@@ -1025,6 +1026,14 @@ static int armv8pmu_filter_match(struct perf_event *event)
 	return evtype != ARMV8_PMUV3_PERFCTR_CHAIN;
 }
 
+static int __init retain_pmcr_x(char *str)
+{
+	export_pmu_events = 1;
+	return 0;
+}
+
+early_param("retain_pmcr_x", retain_pmcr_x);
+
 static void armv8pmu_reset(void *info)
 {
 	struct arm_pmu *cpu_pmu = (struct arm_pmu *)info;
@@ -1047,6 +1056,10 @@ static void armv8pmu_reset(void *info)
 	if (armv8pmu_has_long_event(cpu_pmu))
 		pmcr |= ARMV8_PMU_PMCR_LP;
 
+	/* Preserve PMCR_X to export PMU events */
+	if (export_pmu_events)
+		pmcr |= (armv8pmu_pmcr_read() & ARMV8_PMU_PMCR_X);
+
 	armv8pmu_pmcr_write(pmcr);
 }
 
-- 
2.7.4

Re: [PATCH V3] arm64: perf: Retain PMCR.X of PMCR_EL0 during reset

[Robin Murphy]

On 2022-05-09 18:28, Srinivasarao Pathipati wrote:
> Preserve the bit PMCR.X of PMCR_EL0 during PMU reset to export
> PMU events. as it could be set before PMU initialization.

Reiterating my previous comment, if there are use-cases for enabling the 
event export bus under Linux, then it makes most sense to have an option 
to enable the event export bus. It shouldn't have to depend on whatever 
the bootloader may or may not have done.

What would it even mean to preserve a value of 0 instead of resetting it 
to 0?

Thanks,
Robin.

> Exporting events could result in additional power consumption
> so making it configurable.
> 
> Signed-off-by: Srinivasarao Pathipati <quic_c_spathi@quicinc.com>
> ---
> Changes since V2:
> 	Done below changes as per Will's comments
> 	- enabling pmcr_x now configurable with kernel parameters and
> 	  by default it is disabled.
> 	
> Changes since V1:
> 	- Preserving only PMCR_X bit as per Robin Murphy's comment.
> ---
>   Documentation/admin-guide/kernel-parameters.txt |  4 ++++
>   arch/arm64/kernel/perf_event.c                  | 13 +++++++++++++
>   2 files changed, 17 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index de3da15..cf3410e 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5150,6 +5150,10 @@
>   			Useful for devices that are detected asynchronously
>   			(e.g. USB and MMC devices).
>   
> +	retain_pmcr_x
> +			[KNL] Retain export bit of PMCR_EL0 that is getting reset
> +			during pmu reset.
> +
>   	retain_initrd	[RAM] Keep initrd memory after extraction
>   
>   	rfkill.default_state=
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index cb69ff1..1ac251f 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -34,6 +34,7 @@
>   #define ARMV8_THUNDER_PERFCTR_L1I_CACHE_PREF_ACCESS		0xEC
>   #define ARMV8_THUNDER_PERFCTR_L1I_CACHE_PREF_MISS		0xED
>   
> +static int export_pmu_events  __read_mostly;
>   /*
>    * ARMv8 Architectural defined events, not all of these may
>    * be supported on any given implementation. Unsupported events will
> @@ -1025,6 +1026,14 @@ static int armv8pmu_filter_match(struct perf_event *event)
>   	return evtype != ARMV8_PMUV3_PERFCTR_CHAIN;
>   }
>   
> +static int __init retain_pmcr_x(char *str)
> +{
> +	export_pmu_events = 1;
> +	return 0;
> +}
> +
> +early_param("retain_pmcr_x", retain_pmcr_x);
> +
>   static void armv8pmu_reset(void *info)
>   {
>   	struct arm_pmu *cpu_pmu = (struct arm_pmu *)info;
> @@ -1047,6 +1056,10 @@ static void armv8pmu_reset(void *info)
>   	if (armv8pmu_has_long_event(cpu_pmu))
>   		pmcr |= ARMV8_PMU_PMCR_LP;
>   
> +	/* Preserve PMCR_X to export PMU events */
> +	if (export_pmu_events)
> +		pmcr |= (armv8pmu_pmcr_read() & ARMV8_PMU_PMCR_X);
> +
>   	armv8pmu_pmcr_write(pmcr);
>   }
>   

Re: [PATCH V3] arm64: perf: Retain PMCR.X of PMCR_EL0 during reset

[Will Deacon]

On Mon, May 09, 2022 at 10:58:50PM +0530, Srinivasarao Pathipati wrote:
> Preserve the bit PMCR.X of PMCR_EL0 during PMU reset to export
> PMU events. as it could be set before PMU initialization.
> 
> Exporting events could result in additional power consumption
> so making it configurable.
> 
> Signed-off-by: Srinivasarao Pathipati <quic_c_spathi@quicinc.com>
> ---
> Changes since V2:
> 	Done below changes as per Will's comments
> 	- enabling pmcr_x now configurable with kernel parameters and
> 	  by default it is disabled.
> 	
> Changes since V1:
> 	- Preserving only PMCR_X bit as per Robin Murphy's comment.
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  4 ++++
>  arch/arm64/kernel/perf_event.c                  | 13 +++++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index de3da15..cf3410e 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5150,6 +5150,10 @@
>  			Useful for devices that are detected asynchronously
>  			(e.g. USB and MMC devices).
>  
> +	retain_pmcr_x
> +			[KNL] Retain export bit of PMCR_EL0 that is getting reset
> +			during pmu reset.

I think it would be better to build this as a sysctl so that it can be
toggled at runtime as well.

Will