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: <20230424134748.228137-5-james.clark@arm.com>
Date:   Mon, 24 Apr 2023 14:47:44 +0100
From:   James Clark <james.clark@....com>
To:     linux-perf-users@...r.kernel.org, coresight@...ts.linaro.org,
        shy828301@...il.com
Cc:     denik@...gle.com, James Clark <james.clark@....com>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        Mike Leach <mike.leach@...aro.org>,
        Leo Yan <leo.yan@...aro.org>,
        John Garry <john.g.garry@...cle.com>,
        Will Deacon <will@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Ian Rogers <irogers@...gle.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: [PATCH 4/7] perf: cs-etm: Validate options after applying them

Currently the cs_etm_set_option() function both validates and applies
the config options. Because it's only called when they are added
automatically, there are some paths where the user can apply the option
on the command line and skip the validation. By moving it to the end it
covers both cases.

Also, options don't need to be re-applied anyway, Perf handles parsing
and applying the config terms automatically.

Signed-off-by: James Clark <james.clark@....com>
---
 tools/perf/arch/arm/util/cs-etm.c | 152 +++++++++++++-----------------
 1 file changed, 68 insertions(+), 84 deletions(-)

diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index f9b9ebf7fffc..af0a2400c655 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -69,21 +69,29 @@ static const char * const metadata_ete_ro[] = {
 static bool cs_etm_is_etmv4(struct auxtrace_record *itr, int cpu);
 static bool cs_etm_is_ete(struct auxtrace_record *itr, int cpu);
 
-static int cs_etm_set_context_id(struct auxtrace_record *itr,
-				 struct evsel *evsel, int cpu)
+static int cs_etm_validate_context_id(struct auxtrace_record *itr,
+				      struct evsel *evsel, int cpu)
 {
-	struct cs_etm_recording *ptr;
-	struct perf_pmu *cs_etm_pmu;
+	struct cs_etm_recording *ptr =
+		container_of(itr, struct cs_etm_recording, itr);
+	struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
 	char path[PATH_MAX];
-	int err = -EINVAL;
+	int err;
 	u32 val;
-	u64 contextid;
+	u64 contextid =
+		evsel->core.attr.config &
+		(perf_pmu__format_bits(&cs_etm_pmu->format, "contextid1") |
+		 perf_pmu__format_bits(&cs_etm_pmu->format, "contextid2"));
 
-	ptr = container_of(itr, struct cs_etm_recording, itr);
-	cs_etm_pmu = ptr->cs_etm_pmu;
+	if (!contextid)
+		return 0;
 
-	if (!cs_etm_is_etmv4(itr, cpu))
-		goto out;
+	/* Not supported in etmv3 */
+	if (!cs_etm_is_etmv4(itr, cpu)) {
+		pr_err("%s: contextid not supported in ETMv3, disable with %s/contextid=0/\n",
+		       CORESIGHT_ETM_PMU_NAME, CORESIGHT_ETM_PMU_NAME);
+		return -EINVAL;
+	}
 
 	/* Get a handle on TRCIDR2 */
 	snprintf(path, PATH_MAX, "cpu%d/%s",
@@ -92,27 +100,13 @@ static int cs_etm_set_context_id(struct auxtrace_record *itr,
 
 	/* There was a problem reading the file, bailing out */
 	if (err != 1) {
-		pr_err("%s: can't read file %s\n",
-		       CORESIGHT_ETM_PMU_NAME, path);
-		goto out;
+		pr_err("%s: can't read file %s\n", CORESIGHT_ETM_PMU_NAME,
+		       path);
+		return err;
 	}
 
-	/* User has configured for PID tracing, respects it. */
-	contextid = evsel->core.attr.config &
-			(BIT(ETM_OPT_CTXTID) | BIT(ETM_OPT_CTXTID2));
-
-	/*
-	 * If user doesn't configure the contextid format, parse PMU format and
-	 * enable PID tracing according to the "contextid" format bits:
-	 *
-	 *   If bit ETM_OPT_CTXTID is set, trace CONTEXTIDR_EL1;
-	 *   If bit ETM_OPT_CTXTID2 is set, trace CONTEXTIDR_EL2.
-	 */
-	if (!contextid)
-		contextid = perf_pmu__format_bits(&cs_etm_pmu->format,
-						  "contextid");
-
-	if (contextid & BIT(ETM_OPT_CTXTID)) {
+	if (contextid &
+	    perf_pmu__format_bits(&cs_etm_pmu->format, "contextid1")) {
 		/*
 		 * TRCIDR2.CIDSIZE, bit [9-5], indicates whether contextID
 		 * tracing is supported:
@@ -122,14 +116,14 @@ static int cs_etm_set_context_id(struct auxtrace_record *itr,
 		 */
 		val = BMVAL(val, 5, 9);
 		if (!val || val != 0x4) {
-			pr_err("%s: CONTEXTIDR_EL1 isn't supported\n",
-			       CORESIGHT_ETM_PMU_NAME);
-			err = -EINVAL;
-			goto out;
+			pr_err("%s: CONTEXTIDR_EL1 isn't supported, disable with %s/contextid1=0/\n",
+			       CORESIGHT_ETM_PMU_NAME, CORESIGHT_ETM_PMU_NAME);
+			return -EINVAL;
 		}
 	}
 
-	if (contextid & BIT(ETM_OPT_CTXTID2)) {
+	if (contextid &
+	    perf_pmu__format_bits(&cs_etm_pmu->format, "contextid2")) {
 		/*
 		 * TRCIDR2.VMIDOPT[30:29] != 0 and
 		 * TRCIDR2.VMIDSIZE[14:10] == 0b00100 (32bit virtual contextid)
@@ -138,35 +132,34 @@ static int cs_etm_set_context_id(struct auxtrace_record *itr,
 		 * Any value of VMIDSIZE >= 4 (i.e, > 32bit) is fine for us.
 		 */
 		if (!BMVAL(val, 29, 30) || BMVAL(val, 10, 14) < 4) {
-			pr_err("%s: CONTEXTIDR_EL2 isn't supported\n",
-			       CORESIGHT_ETM_PMU_NAME);
-			err = -EINVAL;
-			goto out;
+			pr_err("%s: CONTEXTIDR_EL2 isn't supported, disable with %s/contextid2=0/\n",
+			       CORESIGHT_ETM_PMU_NAME, CORESIGHT_ETM_PMU_NAME);
+			return -EINVAL;
 		}
 	}
 
-	/* All good, let the kernel know */
-	evsel->core.attr.config |= contextid;
-	err = 0;
-
-out:
-	return err;
+	return 0;
 }
 
-static int cs_etm_set_timestamp(struct auxtrace_record *itr,
-				struct evsel *evsel, int cpu)
+static int cs_etm_validate_timestamp(struct auxtrace_record *itr,
+				     struct evsel *evsel, int cpu)
 {
-	struct cs_etm_recording *ptr;
-	struct perf_pmu *cs_etm_pmu;
+	struct cs_etm_recording *ptr =
+		container_of(itr, struct cs_etm_recording, itr);
+	struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
 	char path[PATH_MAX];
-	int err = -EINVAL;
+	int err;
 	u32 val;
 
-	ptr = container_of(itr, struct cs_etm_recording, itr);
-	cs_etm_pmu = ptr->cs_etm_pmu;
+	if (!(evsel->core.attr.config &
+	      perf_pmu__format_bits(&cs_etm_pmu->format, "timestamp")))
+		return 0;
 
-	if (!cs_etm_is_etmv4(itr, cpu))
-		goto out;
+	if (!cs_etm_is_etmv4(itr, cpu)) {
+		pr_err("%s: timestamp not supported in ETMv3, disable with %s/timestamp=0/\n",
+		       CORESIGHT_ETM_PMU_NAME, CORESIGHT_ETM_PMU_NAME);
+		return -EINVAL;
+	}
 
 	/* Get a handle on TRCIRD0 */
 	snprintf(path, PATH_MAX, "cpu%d/%s",
@@ -177,7 +170,7 @@ static int cs_etm_set_timestamp(struct auxtrace_record *itr,
 	if (err != 1) {
 		pr_err("%s: can't read file %s\n",
 		       CORESIGHT_ETM_PMU_NAME, path);
-		goto out;
+		return err;
 	}
 
 	/*
@@ -189,24 +182,21 @@ static int cs_etm_set_timestamp(struct auxtrace_record *itr,
 	 */
 	val &= GENMASK(28, 24);
 	if (!val) {
-		err = -EINVAL;
-		goto out;
+		return -EINVAL;
 	}
 
-	/* All good, let the kernel know */
-	evsel->core.attr.config |= (1 << ETM_OPT_TS);
-	err = 0;
-
-out:
-	return err;
+	return 0;
 }
 
-#define ETM_SET_OPT_CTXTID	(1 << 0)
-#define ETM_SET_OPT_TS		(1 << 1)
-#define ETM_SET_OPT_MASK	(ETM_SET_OPT_CTXTID | ETM_SET_OPT_TS)
-
-static int cs_etm_set_option(struct auxtrace_record *itr,
-			     struct evsel *evsel, u32 option)
+/*
+ * Check whether the requested timestamp and contextid options should be
+ * available on all requested CPUs and if not, tell the user how to override.
+ * The kernel will silently disable any unavailable options so a warning here
+ * first is better. In theory the kernel could still disable the option for
+ * some other reason so this is best effort only.
+ */
+static int cs_etm_validate_config(struct auxtrace_record *itr,
+				  struct evsel *evsel)
 {
 	int i, err = -EINVAL;
 	struct perf_cpu_map *event_cpus = evsel->evlist->core.user_requested_cpus;
@@ -220,18 +210,11 @@ static int cs_etm_set_option(struct auxtrace_record *itr,
 		    !perf_cpu_map__has(online_cpus, cpu))
 			continue;
 
-		if (option & BIT(ETM_OPT_CTXTID)) {
-			err = cs_etm_set_context_id(itr, evsel, i);
-			if (err)
-				goto out;
-		}
-		if (option & BIT(ETM_OPT_TS)) {
-			err = cs_etm_set_timestamp(itr, evsel, i);
-			if (err)
-				goto out;
-		}
-		if (option & ~(BIT(ETM_OPT_CTXTID) | BIT(ETM_OPT_TS)))
-			/* Nothing else is currently supported */
+		err = cs_etm_validate_context_id(itr, evsel, i);
+		if (err)
+			goto out;
+		err = cs_etm_validate_timestamp(itr, evsel, i);
+		if (err)
 			goto out;
 	}
 
@@ -447,10 +430,10 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
 	 * when a context switch happened.
 	 */
 	if (!perf_cpu_map__empty(cpus)) {
-		err = cs_etm_set_option(itr, cs_etm_evsel,
-					BIT(ETM_OPT_CTXTID) | BIT(ETM_OPT_TS));
-		if (err)
-			goto out;
+		cs_etm_evsel->core.attr.config |=
+			perf_pmu__format_bits(&cs_etm_pmu->format, "timestamp");
+		cs_etm_evsel->core.attr.config |=
+			perf_pmu__format_bits(&cs_etm_pmu->format, "contextid");
 	}
 
 	/* Add dummy event to keep tracking */
@@ -466,6 +449,7 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
 	if (!perf_cpu_map__empty(cpus))
 		evsel__set_sample_bit(evsel, TIME);
 
+	err = cs_etm_validate_config(itr, cs_etm_evsel);
 out:
 	return err;
 }
-- 
2.34.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ