* [PATCH V4] arm64: perf: Set PMCR.X of PMCR_EL0 during pmu reset
@ 2022-05-16 12:55 Srinivasarao Pathipati
2022-05-17 13:08 ` Will Deacon
0 siblings, 1 reply; 3+ messages in thread
From: Srinivasarao Pathipati @ 2022-05-16 12:55 UTC (permalink / raw)
To: will, mark.rutland, peterz, mingo, acme, alexander.shishkin,
jolsa, namhyung, catalin.marinas, linux-arm-kernel,
linux-perf-users, linux-kernel
Cc: Srinivasarao Pathipati
Enable exporting of events over PMU event export bus by setting
PMCR.X of PMCR_EL0 during pmu reset.
As it impacts power consumption make it configurable at bootup
with kernel arguments and at runtime with sysctl.
Signed-off-by: Srinivasarao Pathipati <quic_c_spathi@quicinc.com>
---
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 | 4 ++++
Documentation/admin-guide/sysctl/kernel.rst | 8 ++++++++
arch/arm64/kernel/perf_event.c | 15 +++++++++++++++
include/linux/perf_event.h | 1 +
kernel/sysctl.c | 12 ++++++++++++
5 files changed, 40 insertions(+)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index de3da15..2139b81 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).
+ export_pmu_events
+ [KNL] Sets export bit of PMCR_EL0 to enable the exporting of
+ events over PMU event export bus.
+
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..8fbc3a0 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -892,6 +892,14 @@ The default value is 0 (access disabled).
See Documentation/arm64/perf.rst for more information.
+export_pmu_events
+=================
+Controls the export bit(4th bit) of PMCR_EL0 which enables the exporting of
+events over an IMPLEMENTATION DEFINED PMU event export bus to another device.
+
+0: disables exporting of events
+
+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..271a8c6 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
+int sysctl_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,17 @@ static int armv8pmu_filter_match(struct perf_event *event)
return evtype != ARMV8_PMUV3_PERFCTR_CHAIN;
}
+static int __init export_pmu_events(char *str)
+{
+ /* Exporting of events can be enabled at runtime with sysctl or
+ * statically at bootup with kernel parameters.
+ */
+ 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);
}
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index da75956..7790328 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1311,6 +1311,7 @@ extern void put_callchain_entry(int rctx);
extern int sysctl_perf_event_max_stack;
extern int sysctl_perf_event_max_contexts_per_stack;
+extern int sysctl_export_pmu_events;
static inline int perf_callchain_store_context(struct perf_callchain_entry_ctx *ctx, u64 ip)
{
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index e52b6e3..3b751a2e 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2008,6 +2008,18 @@ static struct ctl_table kern_table[] = {
.extra2 = SYSCTL_ONE_THOUSAND,
},
#endif
+#ifdef CONFIG_HW_PERF_EVENTS
+ {
+ .procname = "export_pmu_events",
+ .data = &sysctl_export_pmu_events,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
+
+ },
+#endif
{
.procname = "panic_on_warn",
.data = &panic_on_warn,
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH V4] arm64: perf: Set PMCR.X of PMCR_EL0 during pmu reset
2022-05-16 12:55 [PATCH V4] arm64: perf: Set PMCR.X of PMCR_EL0 during pmu reset Srinivasarao Pathipati
@ 2022-05-17 13:08 ` Will Deacon
2022-05-18 14:17 ` Srinivasarao Pathipati
0 siblings, 1 reply; 3+ messages in thread
From: Will Deacon @ 2022-05-17 13:08 UTC (permalink / raw)
To: Srinivasarao Pathipati
Cc: mark.rutland, peterz, mingo, acme, alexander.shishkin, jolsa,
namhyung, catalin.marinas, linux-arm-kernel, linux-perf-users,
linux-kernel
On Mon, May 16, 2022 at 06:25:38PM +0530, Srinivasarao Pathipati wrote:
> Enable exporting of events over PMU event export bus by setting
> PMCR.X of PMCR_EL0 during pmu reset.
>
> As it impacts power consumption make it configurable at bootup
> with kernel arguments and at runtime with sysctl.
>
> Signed-off-by: Srinivasarao Pathipati <quic_c_spathi@quicinc.com>
> ---
> 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 | 4 ++++
> Documentation/admin-guide/sysctl/kernel.rst | 8 ++++++++
> arch/arm64/kernel/perf_event.c | 15 +++++++++++++++
> include/linux/perf_event.h | 1 +
> kernel/sysctl.c | 12 ++++++++++++
> 5 files changed, 40 insertions(+)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index de3da15..2139b81 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).
>
> + export_pmu_events
> + [KNL] Sets export bit of PMCR_EL0 to enable the exporting of
> + events over PMU event export bus.
Sorry, I should've been clearer ahbout this before: if you add a sysctl,
then you get the kernel cmdline option for free via something like
"sysctl.kernel.export_pmu_events=foo", so I think you can drop this and
the early_param().
> +
> 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..8fbc3a0 100644
> --- a/Documentation/admin-guide/sysctl/kernel.rst
> +++ b/Documentation/admin-guide/sysctl/kernel.rst
> @@ -892,6 +892,14 @@ The default value is 0 (access disabled).
>
> See Documentation/arm64/perf.rst for more information.
>
> +export_pmu_events
> +=================
You should add something like "(arm64 only)" to the title.
> +Controls the export bit(4th bit) of PMCR_EL0 which enables the exporting of
Just say "Controls the PMU export bit (PMCR_EL0.X), which enables ...".
> +events over an IMPLEMENTATION DEFINED PMU event export bus to another device.
> +
> +0: disables exporting of events
> +
> +1: enables exporting of events
Please state that the default value is 0.
> pid_max
> =======
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index cb69ff1..271a8c6 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
>
> +int sysctl_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,17 @@ static int armv8pmu_filter_match(struct perf_event *event)
> return evtype != ARMV8_PMUV3_PERFCTR_CHAIN;
> }
>
> +static int __init export_pmu_events(char *str)
> +{
> + /* Exporting of events can be enabled at runtime with sysctl or
> + * statically at bootup with kernel parameters.
> + */
> + 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);
Hmm, I think this reset path only runs when initialising/onlining a CPU,
so it's not a great user interface where the sysctl is concerned. It's
probably better to hook armv8pmu_start() for this.
> }
>
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index da75956..7790328 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -1311,6 +1311,7 @@ extern void put_callchain_entry(int rctx);
>
> extern int sysctl_perf_event_max_stack;
> extern int sysctl_perf_event_max_contexts_per_stack;
> +extern int sysctl_export_pmu_events;
>
> static inline int perf_callchain_store_context(struct perf_callchain_entry_ctx *ctx, u64 ip)
> {
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index e52b6e3..3b751a2e 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2008,6 +2008,18 @@ static struct ctl_table kern_table[] = {
> .extra2 = SYSCTL_ONE_THOUSAND,
> },
> #endif
> +#ifdef CONFIG_HW_PERF_EVENTS
> + {
> + .procname = "export_pmu_events",
> + .data = &sysctl_export_pmu_events,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = SYSCTL_ZERO,
> + .extra2 = SYSCTL_ONE,
> +
> + },
> +#endif
Since this is arm64-specific, it should live in the arm64 code and not
here. See how we already register 'armv8_pmu_sysctl_table' for the ARMv8
PMU.
Will
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH V4] arm64: perf: Set PMCR.X of PMCR_EL0 during pmu reset
2022-05-17 13:08 ` Will Deacon
@ 2022-05-18 14:17 ` Srinivasarao Pathipati
0 siblings, 0 replies; 3+ messages in thread
From: Srinivasarao Pathipati @ 2022-05-18 14:17 UTC (permalink / raw)
To: Will Deacon
Cc: mark.rutland, peterz, mingo, acme, alexander.shishkin, jolsa,
namhyung, catalin.marinas, linux-arm-kernel, linux-perf-users,
linux-kernel
On 5/17/2022 6:38 PM, Will Deacon wrote:
> On Mon, May 16, 2022 at 06:25:38PM +0530, Srinivasarao Pathipati wrote:
>> Enable exporting of events over PMU event export bus by setting
>> PMCR.X of PMCR_EL0 during pmu reset.
>>
>> As it impacts power consumption make it configurable at bootup
>> with kernel arguments and at runtime with sysctl.
>>
>> Signed-off-by: Srinivasarao Pathipati <quic_c_spathi@quicinc.com>
>> ---
>> 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 | 4 ++++
>> Documentation/admin-guide/sysctl/kernel.rst | 8 ++++++++
>> arch/arm64/kernel/perf_event.c | 15 +++++++++++++++
>> include/linux/perf_event.h | 1 +
>> kernel/sysctl.c | 12 ++++++++++++
>> 5 files changed, 40 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index de3da15..2139b81 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).
>>
>> + export_pmu_events
>> + [KNL] Sets export bit of PMCR_EL0 to enable the exporting of
>> + events over PMU event export bus.
> Sorry, I should've been clearer ahbout this before: if you add a sysctl,
> then you get the kernel cmdline option for free via something like
> "sysctl.kernel.export_pmu_events=foo", so I think you can drop this and
> the early_param().
Thanks for your suggestion , I tried this method and observed that
pmu_reset is getting called even
before kernel argument updated the sysctl variable. So exporting is
not happening at early boot.
>> +
>> 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..8fbc3a0 100644
>> --- a/Documentation/admin-guide/sysctl/kernel.rst
>> +++ b/Documentation/admin-guide/sysctl/kernel.rst
>> @@ -892,6 +892,14 @@ The default value is 0 (access disabled).
>>
>> See Documentation/arm64/perf.rst for more information.
>>
>> +export_pmu_events
>> +=================
> You should add something like "(arm64 only)" to the title.
>
>> +Controls the export bit(4th bit) of PMCR_EL0 which enables the exporting of
> Just say "Controls the PMU export bit (PMCR_EL0.X), which enables ...".
>
>> +events over an IMPLEMENTATION DEFINED PMU event export bus to another device.
>> +
>> +0: disables exporting of events
>> +
>> +1: enables exporting of events
> Please state that the default value is 0.
>
>> pid_max
>> =======
>> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
>> index cb69ff1..271a8c6 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
>>
>> +int sysctl_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,17 @@ static int armv8pmu_filter_match(struct perf_event *event)
>> return evtype != ARMV8_PMUV3_PERFCTR_CHAIN;
>> }
>>
>> +static int __init export_pmu_events(char *str)
>> +{
>> + /* Exporting of events can be enabled at runtime with sysctl or
>> + * statically at bootup with kernel parameters.
>> + */
>> + 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);
>
> Hmm, I think this reset path only runs when initialising/onlining a CPU,
> so it's not a great user interface where the sysctl is concerned. It's
> probably better to hook armv8pmu_start() for this.
>
>> }
>>
>> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
>> index da75956..7790328 100644
>> --- a/include/linux/perf_event.h
>> +++ b/include/linux/perf_event.h
>> @@ -1311,6 +1311,7 @@ extern void put_callchain_entry(int rctx);
>>
>> extern int sysctl_perf_event_max_stack;
>> extern int sysctl_perf_event_max_contexts_per_stack;
>> +extern int sysctl_export_pmu_events;
>>
>> static inline int perf_callchain_store_context(struct perf_callchain_entry_ctx *ctx, u64 ip)
>> {
>> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
>> index e52b6e3..3b751a2e 100644
>> --- a/kernel/sysctl.c
>> +++ b/kernel/sysctl.c
>> @@ -2008,6 +2008,18 @@ static struct ctl_table kern_table[] = {
>> .extra2 = SYSCTL_ONE_THOUSAND,
>> },
>> #endif
>> +#ifdef CONFIG_HW_PERF_EVENTS
>> + {
>> + .procname = "export_pmu_events",
>> + .data = &sysctl_export_pmu_events,
>> + .maxlen = sizeof(int),
>> + .mode = 0644,
>> + .proc_handler = proc_dointvec_minmax,
>> + .extra1 = SYSCTL_ZERO,
>> + .extra2 = SYSCTL_ONE,
>> +
>> + },
>> +#endif
> Since this is arm64-specific, it should live in the arm64 code and not
> here. See how we already register 'armv8_pmu_sysctl_table' for the ARMv8
> PMU.
>
> Will
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-05-18 14:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-16 12:55 [PATCH V4] arm64: perf: Set PMCR.X of PMCR_EL0 during pmu reset Srinivasarao Pathipati
2022-05-17 13:08 ` Will Deacon
2022-05-18 14:17 ` Srinivasarao Pathipati
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).