[PATCH V5] arm64: perf: Make exporting of pmu events configurable

Srinivasarao Pathipati posted 1 patch 1 year, 10 months ago
There is a newer version of this series
Documentation/admin-guide/kernel-parameters.txt |  5 +++++
Documentation/admin-guide/sysctl/kernel.rst     |  9 +++++++++
arch/arm64/kernel/perf_event.c                  | 24 ++++++++++++++++++++++++
3 files changed, 38 insertions(+)
[PATCH V5] arm64: perf: Make exporting of pmu events configurable
Posted by Srinivasarao Pathipati 1 year, 10 months ago
The PMU export bit (PMCR_EL0.X) is getting reset during pmu reset,
Make is configurable using sysctls to enable/disable at runtime.
It can also be enabled at early bootup with kernel arguments.

Signed-off-by: Srinivasarao Pathipati <quic_c_spathi@quicinc.com>
---
Changes since V4:
	- Registering sysctls dynamically for only arm64 as suggested by Will
	- Not removed the code to configure with kernel parameters 
	  as the sysctl's kernel parameter(sysctl.kernel.export_pmu_events)
	  is not working at early bootup. pmu_reset() getting called before 
	  sysctl's kernel parameter is set.
Changes since V3:
	- export bit is now configurable with sysctl
	- enabling export bit on reset instead of retaining

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 |  5 +++++
 Documentation/admin-guide/sysctl/kernel.rst     |  9 +++++++++
 arch/arm64/kernel/perf_event.c                  | 24 ++++++++++++++++++++++++
 3 files changed, 38 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index de3da15..2bf1187 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5150,6 +5150,11 @@
 			Useful for devices that are detected asynchronously
 			(e.g. USB and MMC devices).
 
+	export_pmu_events
+			[KNL,ARM64] Sets the PMU export bit (PMCR_EL0.X), which enables
+			the exporting of events over an IMPLEMENTATION DEFINED PMU event
+			export bus to another device.
+
 	retain_initrd	[RAM] Keep initrd memory after extraction
 
 	rfkill.default_state=
diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
index ddccd10..db42d4c 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -892,6 +892,15 @@ The default value is 0 (access disabled).
 
 See Documentation/arm64/perf.rst for more information.
 
+export_pmu_events (arm64 only)
+==============================
+Controls the PMU export bit (PMCR_EL0.X), which enables the exporting of
+events over an IMPLEMENTATION DEFINED PMU event export bus to another device.
+
+0: disables exporting of events (default).
+
+1: enables exporting of events.
+
 
 pid_max
 =======
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index cb69ff1..d93e7c4 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -298,6 +298,7 @@ PMU_FORMAT_ATTR(long, "config1:0");
 PMU_FORMAT_ATTR(rdpmc, "config1:1");
 
 static int sysctl_perf_user_access __read_mostly;
+static int sysctl_export_pmu_events __read_mostly;
 
 static inline bool armv8pmu_event_is_64bit(struct perf_event *event)
 {
@@ -1025,6 +1026,17 @@ static int armv8pmu_filter_match(struct perf_event *event)
 	return evtype != ARMV8_PMUV3_PERFCTR_CHAIN;
 }
 
+static int __init export_pmu_events(char *str)
+{
+	/* Enable exporting of pmu events at early bootup with kernel
+	 * arguments.
+	 */
+	sysctl_export_pmu_events = 1;
+	return 0;
+}
+
+early_param("export_pmu_events", export_pmu_events);
+
 static void armv8pmu_reset(void *info)
 {
 	struct arm_pmu *cpu_pmu = (struct arm_pmu *)info;
@@ -1047,6 +1059,9 @@ static void armv8pmu_reset(void *info)
 	if (armv8pmu_has_long_event(cpu_pmu))
 		pmcr |= ARMV8_PMU_PMCR_LP;
 
+	if (sysctl_export_pmu_events)
+		pmcr |= ARMV8_PMU_PMCR_X;
+
 	armv8pmu_pmcr_write(pmcr);
 }
 
@@ -1221,6 +1236,15 @@ static struct ctl_table armv8_pmu_sysctl_table[] = {
 		.extra1		= SYSCTL_ZERO,
 		.extra2		= SYSCTL_ONE,
 	},
+	{
+		.procname       = "export_pmu_events",
+		.data           = &sysctl_export_pmu_events,
+		.maxlen         = sizeof(unsigned int),
+		.mode           = 0644,
+		.proc_handler   = proc_dointvec_minmax,
+		.extra1         = SYSCTL_ZERO,
+		.extra2         = SYSCTL_ONE,
+	},
 	{ }
 };
 
-- 
2.7.4
Re: [PATCH V5] arm64: perf: Make exporting of pmu events configurable
Posted by Will Deacon 1 year, 9 months ago
On Mon, May 23, 2022 at 05:19:34PM +0530, Srinivasarao Pathipati wrote:
> The PMU export bit (PMCR_EL0.X) is getting reset during pmu reset,
> Make is configurable using sysctls to enable/disable at runtime.
> It can also be enabled at early bootup with kernel arguments.
> 
> Signed-off-by: Srinivasarao Pathipati <quic_c_spathi@quicinc.com>
> ---
> Changes since V4:
> 	- Registering sysctls dynamically for only arm64 as suggested by Will
> 	- Not removed the code to configure with kernel parameters 
> 	  as the sysctl's kernel parameter(sysctl.kernel.export_pmu_events)
> 	  is not working at early bootup. pmu_reset() getting called before 
> 	  sysctl's kernel parameter is set.

Why do you need this during early bootup? Perf won't program any events
until much later and if somebody else is configuring the PMU before
entering Linux then they can also set that X bit in the PMCR.

Will
Re: [PATCH V5] arm64: perf: Make exporting of pmu events configurable
Posted by Srinivasarao Pathipati 1 year, 9 months ago
On our Qualcomm platforms, The X bit is getting set by firmware at early 
bootup for Qualcomm use cases
and non-secure world is resetting it, that causing issue.

On 6/9/2022 3:32 PM, Will Deacon wrote:
> On Mon, May 23, 2022 at 05:19:34PM +0530, Srinivasarao Pathipati wrote:
>> The PMU export bit (PMCR_EL0.X) is getting reset during pmu reset,
>> Make is configurable using sysctls to enable/disable at runtime.
>> It can also be enabled at early bootup with kernel arguments.
>>
>> Signed-off-by: Srinivasarao Pathipati <quic_c_spathi@quicinc.com>
>> ---
>> Changes since V4:
>> 	- Registering sysctls dynamically for only arm64 as suggested by Will
>> 	- Not removed the code to configure with kernel parameters
>> 	  as the sysctl's kernel parameter(sysctl.kernel.export_pmu_events)
>> 	  is not working at early bootup. pmu_reset() getting called before
>> 	  sysctl's kernel parameter is set.
> Why do you need this during early bootup? Perf won't program any events
> until much later and if somebody else is configuring the PMU before
> entering Linux then they can also set that X bit in the PMCR.
>
> Will
Re: [PATCH V5] arm64: perf: Make exporting of pmu events configurable
Posted by Robin Murphy 1 year, 9 months ago
On 2022-06-09 11:35, Srinivasarao Pathipati wrote:
> On our Qualcomm platforms, The X bit is getting set by firmware at early 
> bootup for Qualcomm use cases
> and non-secure world is resetting it, that causing issue.

I think you're going to have to clarify what exactly this "issue" is if 
we're ever going to make sense of it...

I can't imagine that export from a disabled PMU would matter much, so my 
best guess is that EL2 firmware has reserved some counters via 
MDCR_EL2.HPMN which it's using to monitor the Non-Secure boot; if that 
also depends on PMCR.X remaining set, then as far as I can see it's 
really the firmware's own stupid fault for not using MDCR_EL2.TPMCR to 
prevent Linux from messing with its configuration. Or maybe something in 
the Secure world is trying to use the PMU independently and it's an EL3 
bug where PMCR_EL0 isn't being context-switched properly?

Robin.

> On 6/9/2022 3:32 PM, Will Deacon wrote:
>> On Mon, May 23, 2022 at 05:19:34PM +0530, Srinivasarao Pathipati wrote:
>>> The PMU export bit (PMCR_EL0.X) is getting reset during pmu reset,
>>> Make is configurable using sysctls to enable/disable at runtime.
>>> It can also be enabled at early bootup with kernel arguments.
>>>
>>> Signed-off-by: Srinivasarao Pathipati <quic_c_spathi@quicinc.com>
>>> ---
>>> Changes since V4:
>>>     - Registering sysctls dynamically for only arm64 as suggested by 
>>> Will
>>>     - Not removed the code to configure with kernel parameters
>>>       as the sysctl's kernel parameter(sysctl.kernel.export_pmu_events)
>>>       is not working at early bootup. pmu_reset() getting called before
>>>       sysctl's kernel parameter is set.
>> Why do you need this during early bootup? Perf won't program any events
>> until much later and if somebody else is configuring the PMU before
>> entering Linux then they can also set that X bit in the PMCR.
>>
>> Will
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH V5] arm64: perf: Make exporting of pmu events configurable
Posted by Will Deacon 1 year, 9 months ago
[Please don't top-post]

On Thu, Jun 09, 2022 at 04:05:20PM +0530, Srinivasarao Pathipati wrote:
> On 6/9/2022 3:32 PM, Will Deacon wrote:
> > On Mon, May 23, 2022 at 05:19:34PM +0530, Srinivasarao Pathipati wrote:
> > > The PMU export bit (PMCR_EL0.X) is getting reset during pmu reset,
> > > Make is configurable using sysctls to enable/disable at runtime.
> > > It can also be enabled at early bootup with kernel arguments.
> > > 
> > > Signed-off-by: Srinivasarao Pathipati <quic_c_spathi@quicinc.com>
> > > ---
> > > Changes since V4:
> > > 	- Registering sysctls dynamically for only arm64 as suggested by Will
> > > 	- Not removed the code to configure with kernel parameters
> > > 	  as the sysctl's kernel parameter(sysctl.kernel.export_pmu_events)
> > > 	  is not working at early bootup. pmu_reset() getting called before
> > > 	  sysctl's kernel parameter is set.
> > Why do you need this during early bootup? Perf won't program any events
> > until much later and if somebody else is configuring the PMU before
> > entering Linux then they can also set that X bit in the PMCR.
>
> On our Qualcomm platforms, The X bit is getting set by firmware at early
> bootup for Qualcomm use cases
> and non-secure world is resetting it, that causing issue.

What "Qualcomm use cases" and why should we care about them upstream?

Will
Re: [PATCH V5] arm64: perf: Make exporting of pmu events configurable
Posted by Srinivasarao Pathipati 1 year, 9 months ago
On 6/9/2022 5:02 PM, Will Deacon wrote:
> [Please don't top-post]
>
> On Thu, Jun 09, 2022 at 04:05:20PM +0530, Srinivasarao Pathipati wrote:
>> On 6/9/2022 3:32 PM, Will Deacon wrote:
>>> On Mon, May 23, 2022 at 05:19:34PM +0530, Srinivasarao Pathipati wrote:
>>>> The PMU export bit (PMCR_EL0.X) is getting reset during pmu reset,
>>>> Make is configurable using sysctls to enable/disable at runtime.
>>>> It can also be enabled at early bootup with kernel arguments.
>>>>
>>>> Signed-off-by: Srinivasarao Pathipati <quic_c_spathi@quicinc.com>
>>>> ---
>>>> Changes since V4:
>>>> 	- Registering sysctls dynamically for only arm64 as suggested by Will
>>>> 	- Not removed the code to configure with kernel parameters
>>>> 	  as the sysctl's kernel parameter(sysctl.kernel.export_pmu_events)
>>>> 	  is not working at early bootup. pmu_reset() getting called before
>>>> 	  sysctl's kernel parameter is set.
>>> Why do you need this during early bootup? Perf won't program any events
>>> until much later and if somebody else is configuring the PMU before
>>> entering Linux then they can also set that X bit in the PMCR.
>> On our Qualcomm platforms, The X bit is getting set by firmware at early
>> bootup for Qualcomm use cases
>> and non-secure world is resetting it, that causing issue.
> What "Qualcomm use cases" and why should we care about them upstream?

Thanks Will & Robin for your inputs .

Understood your point ,  pushed next version [V6] after removing kernel 
parameters code.

>
> Will
Re: [PATCH V5] arm64: perf: Make exporting of pmu events configurable
Posted by Srinivasarao Pathipati 1 year, 9 months ago
Hi Will / Robin ,

Could you please review this change.

On 5/23/2022 5:19 PM, Srinivasarao Pathipati wrote:
> The PMU export bit (PMCR_EL0.X) is getting reset during pmu reset,
> Make is configurable using sysctls to enable/disable at runtime.
> It can also be enabled at early bootup with kernel arguments.
>
> Signed-off-by: Srinivasarao Pathipati <quic_c_spathi@quicinc.com>
> ---
> Changes since V4:
> 	- Registering sysctls dynamically for only arm64 as suggested by Will
> 	- Not removed the code to configure with kernel parameters
> 	  as the sysctl's kernel parameter(sysctl.kernel.export_pmu_events)
> 	  is not working at early bootup. pmu_reset() getting called before
> 	  sysctl's kernel parameter is set.
> Changes since V3:
> 	- export bit is now configurable with sysctl
> 	- enabling export bit on reset instead of retaining
>
> 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 |  5 +++++
>   Documentation/admin-guide/sysctl/kernel.rst     |  9 +++++++++
>   arch/arm64/kernel/perf_event.c                  | 24 ++++++++++++++++++++++++
>   3 files changed, 38 insertions(+)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index de3da15..2bf1187 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5150,6 +5150,11 @@
>   			Useful for devices that are detected asynchronously
>   			(e.g. USB and MMC devices).
>   
> +	export_pmu_events
> +			[KNL,ARM64] Sets the PMU export bit (PMCR_EL0.X), which enables
> +			the exporting of events over an IMPLEMENTATION DEFINED PMU event
> +			export bus to another device.
> +
>   	retain_initrd	[RAM] Keep initrd memory after extraction
>   
>   	rfkill.default_state=
> diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
> index ddccd10..db42d4c 100644
> --- a/Documentation/admin-guide/sysctl/kernel.rst
> +++ b/Documentation/admin-guide/sysctl/kernel.rst
> @@ -892,6 +892,15 @@ The default value is 0 (access disabled).
>   
>   See Documentation/arm64/perf.rst for more information.
>   
> +export_pmu_events (arm64 only)
> +==============================
> +Controls the PMU export bit (PMCR_EL0.X), which enables the exporting of
> +events over an IMPLEMENTATION DEFINED PMU event export bus to another device.
> +
> +0: disables exporting of events (default).
> +
> +1: enables exporting of events.
> +
>   
>   pid_max
>   =======
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index cb69ff1..d93e7c4 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -298,6 +298,7 @@ PMU_FORMAT_ATTR(long, "config1:0");
>   PMU_FORMAT_ATTR(rdpmc, "config1:1");
>   
>   static int sysctl_perf_user_access __read_mostly;
> +static int sysctl_export_pmu_events __read_mostly;
>   
>   static inline bool armv8pmu_event_is_64bit(struct perf_event *event)
>   {
> @@ -1025,6 +1026,17 @@ static int armv8pmu_filter_match(struct perf_event *event)
>   	return evtype != ARMV8_PMUV3_PERFCTR_CHAIN;
>   }
>   
> +static int __init export_pmu_events(char *str)
> +{
> +	/* Enable exporting of pmu events at early bootup with kernel
> +	 * arguments.
> +	 */
> +	sysctl_export_pmu_events = 1;
> +	return 0;
> +}
> +
> +early_param("export_pmu_events", export_pmu_events);
> +
>   static void armv8pmu_reset(void *info)
>   {
>   	struct arm_pmu *cpu_pmu = (struct arm_pmu *)info;
> @@ -1047,6 +1059,9 @@ static void armv8pmu_reset(void *info)
>   	if (armv8pmu_has_long_event(cpu_pmu))
>   		pmcr |= ARMV8_PMU_PMCR_LP;
>   
> +	if (sysctl_export_pmu_events)
> +		pmcr |= ARMV8_PMU_PMCR_X;
> +
>   	armv8pmu_pmcr_write(pmcr);
>   }
>   
> @@ -1221,6 +1236,15 @@ static struct ctl_table armv8_pmu_sysctl_table[] = {
>   		.extra1		= SYSCTL_ZERO,
>   		.extra2		= SYSCTL_ONE,
>   	},
> +	{
> +		.procname       = "export_pmu_events",
> +		.data           = &sysctl_export_pmu_events,
> +		.maxlen         = sizeof(unsigned int),
> +		.mode           = 0644,
> +		.proc_handler   = proc_dointvec_minmax,
> +		.extra1         = SYSCTL_ZERO,
> +		.extra2         = SYSCTL_ONE,
> +	},
>   	{ }
>   };
>