lkml.org 
[lkml]   [2022]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH V4] arm64: perf: Set PMCR.X of PMCR_EL0 during pmu reset
From

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

\
 
 \ /
  Last update: 2022-05-18 16:20    [W:0.083 / U:0.844 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site