linux-kernel - [PATCH] perf stat: Add arch-specific TopdownL1 check for the default mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 27 Apr 2023 11:29:06 -0700
From:   kan.liang@...ux.intel.com
To:     acme@...nel.org, irogers@...gle.com
Cc:     peterz@...radead.org, mingo@...hat.com, mark.rutland@....com,
        alexander.shishkin@...ux.intel.com, jolsa@...nel.org,
        namhyung@...nel.org, adrian.hunter@...el.com, eranian@...gle.com,
        ahmad.yasin@...el.com, ak@...ux.intel.com, perry.taylor@...el.com,
        samantha.alt@...el.com, caleb.biggers@...el.com,
        weilin.wang@...el.com, linux-kernel@...r.kernel.org,
        linux-perf-users@...r.kernel.org,
        Kan Liang <kan.liang@...ux.intel.com>
Subject: [PATCH] perf stat: Add arch-specific TopdownL1 check for the default mode

From: Kan Liang <kan.liang@...ux.intel.com>

The default of perf stat fails on several Intel platforms.
Skylake:

$ perf stat true
Error:
Access to performance monitoring and observability operations is limited.
Consider adjusting /proc/sys/kernel/perf_event_paranoid setting to open
access to performance monitoring and observability operations for processes
without CAP_PERFMON, CAP_SYS_PTRACE or CAP_SYS_ADMIN Linux capability.
More information can be found at 'Perf events and tool security' document:
https://www.kernel.org/doc/html/latest/admin-guide/perf-security.html
perf_event_paranoid setting is 2:
  -1: Allow use of (almost) all events by all users
      Ignore mlock limit after perf_event_mlock_kb without CAP_IPC_LOCK
> = 0: Disallow raw and ftrace function tracepoint access
> = 1: Disallow CPU event access
> = 2: Disallow kernel profiling

ADL (hybrid):

./perf stat
Segmentation fault (core dumped)

The default of perf stat was switched to TopdownL1 Json metric since
commit 94b1a603fca7("perf stat: Add TopdownL1 metric as a default if
present"). But the patch only checks whether the TopdownL1 is present
in the event list. It doesn't check whether the hardware has the
capability to provide a clean output for the default mode.

Add arch_has_topdown_metric() to check the hardware capability as well.
Drop the TopdownL1 support in the defalut mode for pre-ICL and hybrid
platforms. Users can still use -M TopdownL1 to access the TopdownL1
on pre-ICL platforms.

Signed-off-by: Kan Liang <kan.liang@...ux.intel.com>
---

The patch tries to workaround the serious issues on pre-ICL and hybrid
platforms with the default mode of perf stat. It could be a temporary
fix for the upcoming 6.4. So we have more time to look for a proper fix
for all metrics issues and output issues with 6.5.

Thanks,
Kan

 tools/perf/arch/x86/util/topdown.c | 14 ++++++++++++++
 tools/perf/builtin-stat.c          |  2 +-
 tools/perf/util/stat-display.c     |  2 +-
 tools/perf/util/topdown.c          |  6 ++++++
 tools/perf/util/topdown.h          |  2 ++
 5 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/tools/perf/arch/x86/util/topdown.c b/tools/perf/arch/x86/util/topdown.c
index 9ad5e5c7bd27..5d861e851619 100644
--- a/tools/perf/arch/x86/util/topdown.c
+++ b/tools/perf/arch/x86/util/topdown.c
@@ -3,6 +3,7 @@
 #include "util/evsel.h"
 #include "util/pmu.h"
 #include "util/topdown.h"
+#include "util/metricgroup.h"
 #include "topdown.h"
 #include "evsel.h"
 
@@ -48,3 +49,16 @@ bool arch_topdown_sample_read(struct evsel *leader)
 
 	return false;
 }
+
+bool arch_has_topdown_metric(const char *name)
+{
+	/*
+	 * Disable the Topdown events in the default mode
+	 * for hybrid platforms and old platform which
+	 * doesn't support the Topdown metric feature.
+	 */
+	if (!pmu_have_event("cpu", "slots"))
+		return false;
+
+	return metricgroup__has_metric(name);
+}
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index efda63f6bf32..0b865155656d 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1885,7 +1885,7 @@ static int add_default_attributes(void)
 		 * Add TopdownL1 metrics if they exist. To minimize
 		 * multiplexing, don't request threshold computation.
 		 */
-		if (metricgroup__has_metric("TopdownL1") &&
+		if (arch_has_topdown_metric("TopdownL1") &&
 		    metricgroup__parse_groups(evsel_list, "TopdownL1",
 					    /*metric_no_group=*/false,
 					    /*metric_no_merge=*/false,
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index e6035ecbeee8..73b2ff2ddf29 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -747,7 +747,7 @@ static void uniquify_event_name(struct evsel *counter)
 	int ret = 0;
 
 	if (counter->uniquified_name || counter->use_config_name ||
-	    !counter->pmu_name || !strncmp(counter->name, counter->pmu_name,
+	    !counter->pmu_name || !strncmp(evsel__name(counter), counter->pmu_name,
 					   strlen(counter->pmu_name)))
 		return;
 
diff --git a/tools/perf/util/topdown.c b/tools/perf/util/topdown.c
index 18fd5fed5d1a..f3a9ebc52f8b 100644
--- a/tools/perf/util/topdown.c
+++ b/tools/perf/util/topdown.c
@@ -1,8 +1,14 @@
 // SPDX-License-Identifier: GPL-2.0
 #include "topdown.h"
+#include "metricgroup.h"
 #include <linux/kernel.h>
 
 __weak bool arch_topdown_sample_read(struct evsel *leader __maybe_unused)
 {
 	return false;
 }
+
+__weak bool arch_has_topdown_metric(const char *name)
+{
+	return metricgroup__has_metric(name);
+}
diff --git a/tools/perf/util/topdown.h b/tools/perf/util/topdown.h
index 1996c5fedcd7..7e83c8b247f2 100644
--- a/tools/perf/util/topdown.h
+++ b/tools/perf/util/topdown.h
@@ -8,4 +8,6 @@ struct evsel;
 
 bool arch_topdown_sample_read(struct evsel *leader);
 
+bool arch_has_topdown_metric(const char *name);
+
 #endif
-- 
2.35.1