From patchwork Tue Jan 16 17:03:48 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Rutland X-Patchwork-Id: 13520962 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 1FC4DC47077 for ; Tue, 16 Jan 2024 17:04:55 +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:MIME-Version:Message-Id:Date:Subject:Cc :To:From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=hzrnYPlPHakj2UoCxpsYufWj29QPhmWhjH9h6hhZSTo=; b=pTwWzEuphi0s56 04Xy922zszzwLbUjwppte9CfEGyY2/VzamMw8EaK5Y1dh4XzB+e7Ehs7+F8wlkoOm1W7WTTgLo514 Im9d4QFLzAJzNHF6Oc4DewltMVLnl2x3kJqcpbsHI/pXLsJixKPSTzdpFqVNp1GMDbFtN87FPgWi0 TR/vjW+JkPpcqc72vLHqhhyeh7c36GRuciV05IJNLHI/Qgf+Srpvg1GJvNLDnLrbyPITNaoPvDKNa 7t7j4C2aUYBTd6pOWrsdoPxo7rCS/DGjDuJPGsyV42VcLeMzfq69CxTMHazAi7I+ggpDTA022G2cO S2DzkXi1OLMYBvbfdtXQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1rPmrQ-00ChJK-28; Tue, 16 Jan 2024 17:04:24 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1rPmrL-00ChIS-2i for linux-arm-kernel@lists.infradead.org; Tue, 16 Jan 2024 17:04:22 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 158092F4; Tue, 16 Jan 2024 09:05:00 -0800 (PST) Received: from lakrids.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id B1BC03F73F; Tue, 16 Jan 2024 09:04:11 -0800 (PST) From: Mark Rutland To: linux-kernel@vger.kernel.org, Hector Martin , Marc Zyngier , Ian Rogers Cc: acme@redhat.com, james.clark@arm.com, john.g.garry@oracle.com, leo.yan@linaro.org, linux-arm-kernel@lists.infradead.org, linux-perf-users@vger.kernel.org, mark.rutland@arm.com, mike.leach@linaro.org, namhyung@kernel.org, suzuki.poulose@arm.com, tmricht@linux.ibm.com, will@kernel.org Subject: [PATCH] perf print-events: make is_event_supported() more robust Date: Tue, 16 Jan 2024 17:03:48 +0000 Message-Id: <20240116170348.463479-1-mark.rutland@arm.com> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240116_090419_971789_051D681B X-CRM114-Status: GOOD ( 34.25 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Currently the perf tool doesn't deteect support for extneded event types on Apple M1/M2 systems, and will not auto-expand plain PERF_EVENT_TYPE hardware events into per-PMU events. This is due to the detection of extended event types not handling mandatory filters required by the M1/M2 PMU driver. PMU drivers and the core perf_events code can require that perf_event_attr::exclude_* filters are configured in a specific way and may reject certain configurations of filters, for example: (a) Many PMUs lack support for any event filtering, and require all perf_event_attr::exclude_* bits to be clear. This includes Alpha's CPU PMU, and ARM CPU PMUs prior to the introduction of PMUv2 in ARMv7, (b) When /proc/sys/kernel/perf_event_paranoid >= 2, the perf core requires that perf_event_attr::exclude_kernel is set. (c) The Apple M1/M2 PMU requires that perf_event_attr::exclude_guest is set as the hardware PMU does not count while a guest is running (but might be extended in future to do so). In is_event_supported(), we try to account for cases (a) and (b), first attempting to open an event without any filters, and if this fails, retrying with perf_event_attr::exclude_kernel set. We do not account for case (c), or any other filters that drivers could theoretically require to be set. Thus is_event_supported() will fail to detect support for any events targetting an Apple M1/M2 PMU, even where events would be supported with perf_event_attr:::exclude_guest set. Since commit: 82fe2e45cdb00de4 ("perf pmus: Check if we can encode the PMU number in perf_event_attr.type") ... we use is_event_supported() to detect support for extended types, with the PMU ID encoded into the perf_event_attr::type. As above, on an Apple M1/M2 system this will always fail to detect that the event is supported, and consequently we fail to detect support for extended types even when these are supported, as they have been since commit: 5c816728651ae425 ("arm_pmu: Add PERF_PMU_CAP_EXTENDED_HW_TYPE capability") Due to this, the perf tool will not automatically expand plain PERF_TYPE_HARDWARE events into per-PMU events, even when all the necessary kernel support is present. This patch updates is_event_supported() to additionally try opening events with perf_event_attr::exclude_guest set, allowing support for events to be detected on Apple M1/M2 systems. I beleive that this is sufficient for all contemporary CPU PMU drivers, though in future it may be necessary to check for other combinations of filter bits. I've deliberately changed the check to not expect a specific error code for missing filters, as today ;the kernel may return a number of different error codes for missing filters (e.g. -EACCESS, -EINVAL, or -EOPNOTSUPP) depending on why and where the filter configuration is rejected, and retrying for any error is more robust. Note that this does not remove the need for commit: a24d9d9dc096fc0d ("perf parse-events: Make legacy events lower priority than sysfs/JSON") ... which is still necessary so that named-pmu/event/ events work on kernels without extended type support, even if the event name happens to be the same as a PERF_EVENT_TYPE_HARDWARE event (e.g. as is the case for the M1/M2 PMU's 'cycles' and 'instructions' events). Fixes: 82fe2e45cdb00de4 ("perf pmus: Check if we can encode the PMU number in perf_event_attr.type") Signed-off-by: Mark Rutland Cc: Arnaldo Carvalho de Melo Cc: Hector Martin Cc: Ian Rogers Cc: James Clark Cc: John Garry Cc: Leo Yan Cc: Marc Zyngier Cc: Mike Leach Cc: Namhyung Kim Cc: Suzuki K Poulose Cc: Thomas Richter Cc: Will Deacon Tested-by: Ian Rogers Tested-by: James Clark --- tools/perf/util/print-events.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) Hector, Marc, I'd appreciate if either of you could give this a spin on your M1/M2 machines. I've given it local testing with the arm_pmuv3 driver modified to behave the same as the apple_m1_pmu driver (requiring exclude_guest, having a 'cycles' event in sysfs), but that might not perfectly replicate your setup. The patch is based on the 'perf-tools-for-v6.8-1-2024-01-09' tag in the perf-tools tree: https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools.git/ ... and I've pushed it out to the 'perf-tools/event-supported-filters' branch in my tree: https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/ This patch *should* make it possible to do: perf stat -e cycles ./workload perf stat -e instructions ./workload ... with those 'cycles' and 'instructions' events being automatically expanded and reported as separate events per-PMU, which is a nice quality-of-life improvement. Comparing before and after this patch: | # ./perf-before stat -e cycles true | | Performance counter stats for 'true': | | cycles (0.00%) | | 0.000990250 seconds time elapsed | | 0.000934000 seconds user | 0.000000000 seconds sys | | # ./perf-after stat -e cycles true | | Performance counter stats for 'true': | | 965175 armv8_pmuv3_0/cycles/ | armv8_pmuv3_1/cycles/ (0.00%) | armv8_pmuv3_2/cycles/ (0.00%) | armv8_pmuv3_3/cycles/ (0.00%) | | 0.000836555 seconds time elapsed | | 0.000884000 seconds user | 0.000000000 seconds sys This *shouldn't* change the interpetation of named-pmu events, e.g. perf stat -e apple_whichever_pmu/cycles/ ./workload ... should behave the same as without this patch Comparing before and after this patch: | # ./perf-before stat -e armv8_pmuv3_0/cycles/ -e armv8_pmuv3_1/cycles/ -e armv8_pmuv3_2/cycles/ -e armv8_pmuv3_3/cycles/ true | | Performance counter stats for 'true': | | armv8_pmuv3_0/cycles/ (0.00%) | armv8_pmuv3_1/cycles/ (0.00%) | armv8_pmuv3_2/cycles/ (0.00%) | 901415 armv8_pmuv3_3/cycles/ | | 0.000756590 seconds time elapsed | | 0.000811000 seconds user | 0.000000000 seconds sys | | # ./perf-after stat -e armv8_pmuv3_0/cycles/ -e armv8_pmuv3_1/cycles/ -e armv8_pmuv3_2/cycles/ -e armv8_pmuv3_3/cycles/ true | | Performance counter stats for 'true': | | 923314 armv8_pmuv3_0/cycles/ | armv8_pmuv3_1/cycles/ (0.00%) | armv8_pmuv3_2/cycles/ (0.00%) | armv8_pmuv3_3/cycles/ (0.00%) | | 0.000782420 seconds time elapsed | | 0.000836000 seconds user | 0.000000000 seconds sys One thing I'm still looing into is that this doesn't seem to do anything for a default perf stat session, e.g. perf stat ./workload ... doesn't automatically expand the implicitly-created events into per-pmu events. Comparing before and after this patch: | # ./perf-before stat true | | Performance counter stats for 'true': | | 0.42 msec task-clock # 0.569 CPUs utilized | 0 context-switches # 0.000 /sec | 0 cpu-migrations # 0.000 /sec | 38 page-faults # 89.796 K/sec | cycles (0.00%) | instructions (0.00%) | branches (0.00%) | branch-misses (0.00%) | | 0.000744185 seconds time elapsed | | 0.000795000 seconds user | 0.000000000 seconds sys | | # ./perf-after stat true | | Performance counter stats for 'true': | | 0.43 msec task-clock # 0.582 CPUs utilized | 0 context-switches # 0.000 /sec | 0 cpu-migrations # 0.000 /sec | 38 page-faults # 88.960 K/sec | cycles (0.00%) | instructions (0.00%) | branches (0.00%) | branch-misses (0.00%) | | 0.000734120 seconds time elapsed | | 0.000786000 seconds user | 0.000000000 seconds sys Ian, how does that behave on x86? Is that the same, or do the default events get expanded? Thanks, Mark. diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c index b0fc48be623f3..4f67e8f00a4d6 100644 --- a/tools/perf/util/print-events.c +++ b/tools/perf/util/print-events.c @@ -232,7 +232,6 @@ void print_sdt_events(const struct print_callbacks *print_cb, void *print_state) bool is_event_supported(u8 type, u64 config) { bool ret = true; - int open_return; struct evsel *evsel; struct perf_event_attr attr = { .type = type, @@ -246,20 +245,32 @@ bool is_event_supported(u8 type, u64 config) evsel = evsel__new(&attr); if (evsel) { - open_return = evsel__open(evsel, NULL, tmap); - ret = open_return >= 0; + ret = evsel__open(evsel, NULL, tmap) >= 0; - if (open_return == -EACCES) { + if (!ret) { /* - * This happens if the paranoid value + * The event may fail to open if the paranoid value * /proc/sys/kernel/perf_event_paranoid is set to 2 - * Re-run with exclude_kernel set; we don't do that - * by default as some ARM machines do not support it. - * + * Re-run with exclude_kernel set; we don't do that by + * default as some ARM machines do not support it. */ evsel->core.attr.exclude_kernel = 1; ret = evsel__open(evsel, NULL, tmap) >= 0; } + + if (!ret) { + /* + * The event may fail to open if the PMU requires + * exclude_guest to be set (e.g. as the Apple M1 PMU + * requires). + * Re-run with exclude_guest set; we don't do that by + * default as it's equally legitimate for another PMU + * driver to require that exclude_guest is clear. + */ + evsel->core.attr.exclude_guest = 1; + ret = evsel__open(evsel, NULL, tmap) >= 0; + } + evsel__delete(evsel); }