Messages in this thread | | | Date | Wed, 8 Mar 2023 12:29:57 +0530 | Subject | Re: [PATCH V6] arm64: perf: Make exporting of pmu events configurable | From | Srinivasarao Pathipati <> |
| |
On 7/13/2022 9:35 AM, Srinivasarao Pathipati wrote: > > On 6/24/2022 5:48 PM, Will Deacon wrote: >> On Thu, Jun 09, 2022 at 06:29:49PM +0530, Srinivasarao Pathipati wrote: >>> The PMU export bit (PMCR_EL0.X) is getting reset during pmu reset, >>> Make it configurable using sysctls to enable/disable at runtime. >>> >>> Signed-off-by: Srinivasarao Pathipati <quic_c_spathi@quicinc.com> >>> --- >>> Changes since V5: >>> - removed configuring with kernel parameters. >>> 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/sysctl/kernel.rst | 11 +++++++++++ >>> arch/arm64/kernel/perf_event.c | 13 +++++++++++++ >>> 2 files changed, 24 insertions(+) >>> >>> diff --git a/Documentation/admin-guide/sysctl/kernel.rst >>> b/Documentation/admin-guide/sysctl/kernel.rst >>> index ddccd10..c2ecd84 100644 >>> --- a/Documentation/admin-guide/sysctl/kernel.rst >>> +++ b/Documentation/admin-guide/sysctl/kernel.rst >>> @@ -267,6 +267,17 @@ domain names are in general different. For a >>> detailed discussion >>> see the ``hostname(1)`` man page. >>> +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. >>> + >>> + >>> firmware_config >>> =============== >>> diff --git a/arch/arm64/kernel/perf_event.c >>> b/arch/arm64/kernel/perf_event.c >>> index cb69ff1..a8c32a0 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) >>> { >>> @@ -1047,6 +1048,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; >> I think we need to do this in armv8pmu_start() rather than >> armv8pmu_reset(), >> otherwise any changes to the sysctl at runtime won't take effect >> unless you >> do something like re-online the CPU. >> >> Will > > Hi Will , > > We think it may not work. Say for example pmu->reset is called which > disables the exporting of events, > and pmu->start not called. This would lead to missing of events. > > How about enabling it in both reset and start functions? do you see > any issue? > > --Srinivas
Hi Will ,
Could you please suggest ,better way to fix this.
| |