linux-kernel - [PATCH 2/2] perf stat: Disable HW watchdog around a perf stat session

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170207011028.hj54i2tjqh5yaz6i@pd.tnic>
Date:   Tue, 7 Feb 2017 02:10:28 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Ingo Molnar <mingo@...nel.org>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Robert Richter <rric@...nel.org>,
        Vince Weaver <vince@...ter.net>,
        lkml <linux-kernel@...r.kernel.org>
Subject: [PATCH 2/2] perf stat: Disable HW watchdog around a perf stat session

From: Borislav Petkov <bp@...e.de>

When using perf stat on an AMD F15h system with the default hw events
attributes, some of the events don't get counted:

 Performance counter stats for 'sleep 1':

          0.749208      task-clock (msec)         #    0.001 CPUs utilized
                 1      context-switches          #    0.001 M/sec
                 0      cpu-migrations            #    0.000 K/sec
                54      page-faults               #    0.072 M/sec
         1,122,815      cycles                    #    1.499 GHz
           286,740      stalled-cycles-frontend   #   25.54% frontend cycles idle
     <not counted>      stalled-cycles-backend                                        (0.00%)
     ^^^^^^^^^^^^
     <not counted>      instructions                                                  (0.00%)
     ^^^^^^^^^^^^
     <not counted>      branches                                                      (0.00%)
     <not counted>      branch-misses                                                 (0.00%)

       1.001550070 seconds time elapsed

The reason is that we have the HW watchdog consume one PMU counter
and when perf tries to schedule 6 events on 6 counters and some of
those counters are constrained to only a specific subset of PMCs by the
hardware, the event scheduling fails.

So let's disable the HW watchdog around a perf stat session running as
root and restore it after it to its previous state. This frees up the
one counter and the scheduling of the default events succeeds:

 Performance counter stats for 'sleep 1':

          0.806902      task-clock (msec)         #    0.001 CPUs utilized
                 1      context-switches          #    0.001 M/sec
                 0      cpu-migrations            #    0.000 K/sec
                55      page-faults               #    0.068 M/sec
         1,200,677      cycles                    #    1.488 GHz
           308,044      stalled-cycles-frontend   #   25.66% frontend cycles idle
           424,292      stalled-cycles-backend    #   35.34% backend cycles idle
           672,694      instructions              #    0.56  insn per cycle
                                                  #    0.63  stalled cycles per insn
           132,965      branches                  #  164.785 M/sec
             7,300      branch-misses             #    5.49% of all branches

       1.001689739 seconds time elapsed

There's a --dont-disable-hwdt option which preserves the old behavior.

Signed-off-by: Borislav Petkov <bp@...e.de>
---
 tools/perf/builtin-stat.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index a02f2e965628..b2aa2ed3161c 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -146,6 +146,8 @@ static aggr_get_id_t		aggr_get_id;
 static bool			append_file;
 static const char		*output_name;
 static int			output_fd;
+static bool			keep_hwdt;
+static int			prev_hwdt;	/* previous HW watchdog state */
 
 struct perf_stat {
 	bool			 record;
@@ -1539,6 +1541,39 @@ static void print_counters(struct timespec *ts, int argc, const char **argv)
 	fflush(stat_config.output);
 }
 
+static void perf_stat_toggle_hwdt(int on)
+{
+	static const char *p = "sys/kernel/nmi_watchdog";
+	int val;
+
+	if (keep_hwdt)
+		return;
+
+	if (geteuid())
+		return;
+
+	if (procfs__read_int(p, &val) < 0)
+		return;
+
+	/* Reenable only when it was enabled before. */
+	if (on) {
+		if (prev_hwdt)
+			goto write;
+	/* Disable HWDT only when it is enabled. */
+	} else {
+		prev_hwdt = val;
+
+		if (val)
+			goto write;
+	}
+
+	return;
+
+write:
+	if (procfs__write_int(p, on) < 0)
+		return;
+}
+
 static volatile int signr = -1;
 
 static void skip_signal(int signo)
@@ -1575,6 +1610,8 @@ static void sig_atexit(void)
 
 	sigprocmask(SIG_SETMASK, &oset, NULL);
 
+	perf_stat_toggle_hwdt(1);
+
 	if (signr == -1)
 		return;
 
@@ -1659,6 +1696,8 @@ static const struct option stat_options[] = {
 			"Only print computed metrics. No raw values", enable_metric_only),
 	OPT_BOOLEAN(0, "topdown", &topdown_run,
 			"measure topdown level 1 statistics"),
+	OPT_BOOLEAN(0, "dont-disable-hwdt", &keep_hwdt,
+			"Do not disable HW NMI watchdog during the current session"),
 	OPT_END()
 };
 
@@ -2523,6 +2562,8 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (perf_stat_init_aggr_mode())
 		goto out;
 
+	perf_stat_toggle_hwdt(0);
+
 	/*
 	 * We dont want to block the signals - that would cause
 	 * child tasks to inherit that and Ctrl-C would not work.
-- 
2.11.0

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.