From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id DAF25C433F5 for ; Tue, 17 May 2022 13:24:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=kcstm52jh9lpPG5sVzhQgoj+/Ft1ghRxCGw8JNlNKGo=; b=alnSfVAF0F9Oxz ijCDV+hcNi2WBhGD6FbywCcmII/GBZL5ZqduDvpwzJ4bbqE+IAyBXyCQhyJercbTFnzE6vDKeW9g9 AdxzCCMFYoplFqg0JzEpr4JlHd0pLyZuu+kvFaTOhiIAcXpa0C0QKK8yRpPiVXqqgQwqQOYV52mqt rDrGnu0Xqw7eTGYld2EXg5fdB57BCdU2KUYcGUIo1o9qcSZqtwMsnk2++Zd+D6iklw6T97ih7nj3u M+5xKcOwefxb862T0z53TdP6CoVMPKUjg3WoGmTApwd5gBTC9YSLnB7Dhn5zd7z/WlywIuypM19LK OODEP5EPX6brHqTZMapw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nqxAI-00DsjT-0M; Tue, 17 May 2022 13:23:06 +0000 Received: from sin.source.kernel.org ([145.40.73.55]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nqwwE-00Dnb5-6d for linux-arm-kernel@lists.infradead.org; Tue, 17 May 2022 13:08:36 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sin.source.kernel.org (Postfix) with ESMTPS id 67BD0CE19FF; Tue, 17 May 2022 13:08:30 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 922AEC385B8; Tue, 17 May 2022 13:08:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1652792908; bh=8Cyf4GoLJYIxjStrlVirMdV2e3KN0kCdHlgFZMnClfc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=KNkvxPWsgEj6I8hGcmVi1y/dFSIrjLT5rJ4/qQH2mQDomf/Lcm9aOSL9KGuA68AYA 39bZCNbFe4TkNDVNbM7BkzyCFBSztrbmSC7ST4H8YvAcWPLCoRyMJQjz1wPnyZa9cY rU1jG6Z0mV6sBwtDR2rsbbYi4yF8Ura1oNdPC2r+ZBrl0HEIpZ7a4+MRTQr4+G32Mm 9DUcwJrnxb194U6Z+l7itzO0p3tZfI3WxPYziSAut36etUmLB1Vtbtv2hBndrGFkyI r99BOcj2d39U3cidcnf1L6KLCay226aE9H9ppLTMjWfg5iSYE59LWlsvxg3Vn+dKdQ l3OuK/muTRw0A== Date: Tue, 17 May 2022 14:08:22 +0100 From: Will Deacon To: Srinivasarao Pathipati Cc: mark.rutland@arm.com, peterz@infradead.org, mingo@redhat.com, acme@kernel.org, alexander.shishkin@linux.intel.com, jolsa@kernel.org, namhyung@kernel.org, catalin.marinas@arm.com, linux-arm-kernel@lists.infradead.org, linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH V4] arm64: perf: Set PMCR.X of PMCR_EL0 during pmu reset Message-ID: <20220517130822.GA3369@willie-the-truck> References: <1652705738-1628-1-git-send-email-quic_c_spathi@quicinc.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1652705738-1628-1-git-send-email-quic_c_spathi@quicinc.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220517_060834_654170_02761EB0 X-CRM114-Status: GOOD ( 39.31 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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 > --- > 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel