* [PATCH v2 1/3] perf cs-etm: Use struct perf_cpu as much as possible
2024-05-01 13:57 [PATCH v2 0/3] perf cs-etm: Improve version detection and error reporting James Clark
@ 2024-05-01 13:57 ` James Clark
2024-05-01 13:57 ` [PATCH v2 2/3] perf cs-etm: Remove repeated fetches of the ETM PMU James Clark
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: James Clark @ 2024-05-01 13:57 UTC (permalink / raw)
To: linux-perf-users, coresight, leo.yan, irogers
Cc: James Clark, Suzuki K Poulose, Mike Leach, John Garry,
Will Deacon, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Liang, Kan,
linux-arm-kernel, linux-kernel
The perf_cpu struct makes some iterators simpler and avoids some
mistakes with interchanging CPU IDs with indexes etc. At the moment in
this file the conversion to an integer is done somewhere in the middle
of the call tree. Change it to delay the conversion to an int until the
leaf functions.
Some of the usage patterns are duplicated, so instead of changing them
all, make cs_etm_get_ro() more reusable and use that everywhere.
cs_etm_get_ro() didn't return an error before, but return one now so
that it can also be used where an error is needed. Continue to ignore
the error where it was already ignored.
Use cs_etm_pmu_path_exists() instead of cs_etm_get_ro() in
cs_etm_is_etmv4() because cs_etm_get_ro() prints a warning, but path
exists is sufficient for this use case.
Signed-off-by: James Clark <james.clark@arm.com>
---
tools/perf/arch/arm/util/cs-etm.c | 204 +++++++++++++-----------------
1 file changed, 88 insertions(+), 116 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index 07be32d99805..a1fa711dc41a 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -66,18 +66,19 @@ static const char * const metadata_ete_ro[] = {
[CS_ETE_TS_SOURCE] = "ts_source",
};
-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 bool cs_etm_is_etmv4(struct auxtrace_record *itr, struct perf_cpu cpu);
+static bool cs_etm_is_ete(struct auxtrace_record *itr, struct perf_cpu cpu);
+static int cs_etm_get_ro(struct perf_pmu *pmu, struct perf_cpu cpu, const char *path, __u64 *val);
+static bool cs_etm_pmu_path_exists(struct perf_pmu *pmu, struct perf_cpu cpu, const char *path);
-static int cs_etm_validate_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,
+ struct perf_cpu cpu)
{
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;
- u32 val;
+ __u64 val;
u64 contextid = evsel->core.attr.config &
(perf_pmu__format_bits(cs_etm_pmu, "contextid") |
perf_pmu__format_bits(cs_etm_pmu, "contextid1") |
@@ -94,16 +95,9 @@ static int cs_etm_validate_context_id(struct auxtrace_record *itr,
}
/* Get a handle on TRCIDR2 */
- snprintf(path, PATH_MAX, "cpu%d/%s",
- cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR2]);
- err = perf_pmu__scan_file(cs_etm_pmu, path, "%x", &val);
-
- /* 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);
+ err = cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR2], &val);
+ if (err)
return err;
- }
if (contextid &
perf_pmu__format_bits(cs_etm_pmu, "contextid1")) {
@@ -140,15 +134,14 @@ static int cs_etm_validate_context_id(struct auxtrace_record *itr,
return 0;
}
-static int cs_etm_validate_timestamp(struct auxtrace_record *itr,
- struct evsel *evsel, int cpu)
+static int cs_etm_validate_timestamp(struct auxtrace_record *itr, struct evsel *evsel,
+ struct perf_cpu cpu)
{
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;
- u32 val;
+ __u64 val;
if (!(evsel->core.attr.config &
perf_pmu__format_bits(cs_etm_pmu, "timestamp")))
@@ -161,16 +154,9 @@ static int cs_etm_validate_timestamp(struct auxtrace_record *itr,
}
/* Get a handle on TRCIRD0 */
- snprintf(path, PATH_MAX, "cpu%d/%s",
- cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR0]);
- err = perf_pmu__scan_file(cs_etm_pmu, path, "%x", &val);
-
- /* 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);
+ err = cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR0], &val);
+ if (err)
return err;
- }
/*
* TRCIDR0.TSSIZE, bit [28-24], indicates whether global timestamping
@@ -218,11 +204,11 @@ static int cs_etm_validate_config(struct auxtrace_record *itr,
}
perf_cpu_map__for_each_cpu_skip_any(cpu, idx, intersect_cpus) {
- err = cs_etm_validate_context_id(itr, evsel, cpu.cpu);
+ err = cs_etm_validate_context_id(itr, evsel, cpu);
if (err)
break;
- err = cs_etm_validate_timestamp(itr, evsel, cpu.cpu);
+ err = cs_etm_validate_timestamp(itr, evsel, cpu);
if (err)
break;
}
@@ -549,9 +535,9 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused,
intersect_cpus = perf_cpu_map__new_online_cpus();
}
perf_cpu_map__for_each_cpu_skip_any(cpu, idx, intersect_cpus) {
- if (cs_etm_is_ete(itr, cpu.cpu))
+ if (cs_etm_is_ete(itr, cpu))
ete++;
- else if (cs_etm_is_etmv4(itr, cpu.cpu))
+ else if (cs_etm_is_etmv4(itr, cpu))
etmv4++;
else
etmv3++;
@@ -564,66 +550,59 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused,
(etmv3 * CS_ETMV3_PRIV_SIZE));
}
-static bool cs_etm_is_etmv4(struct auxtrace_record *itr, int cpu)
+static bool cs_etm_is_etmv4(struct auxtrace_record *itr, struct perf_cpu cpu)
{
- bool ret = false;
- char path[PATH_MAX];
- int scan;
- unsigned int val;
struct cs_etm_recording *ptr =
container_of(itr, struct cs_etm_recording, itr);
struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
/* Take any of the RO files for ETMv4 and see if it present */
- snprintf(path, PATH_MAX, "cpu%d/%s",
- cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR0]);
- scan = perf_pmu__scan_file(cs_etm_pmu, path, "%x", &val);
-
- /* The file was read successfully, we have a winner */
- if (scan == 1)
- ret = true;
-
- return ret;
+ return cs_etm_pmu_path_exists(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR0]);
}
-static int cs_etm_get_ro(struct perf_pmu *pmu, int cpu, const char *path)
+static int cs_etm_get_ro(struct perf_pmu *pmu, struct perf_cpu cpu, const char *path, __u64 *val)
{
char pmu_path[PATH_MAX];
int scan;
- unsigned int val = 0;
/* Get RO metadata from sysfs */
- snprintf(pmu_path, PATH_MAX, "cpu%d/%s", cpu, path);
+ snprintf(pmu_path, PATH_MAX, "cpu%d/%s", cpu.cpu, path);
- scan = perf_pmu__scan_file(pmu, pmu_path, "%x", &val);
- if (scan != 1)
+ scan = perf_pmu__scan_file(pmu, pmu_path, "%llx", val);
+ if (scan != 1) {
pr_err("%s: error reading: %s\n", __func__, pmu_path);
+ return -EINVAL;
+ }
- return val;
+ return 0;
}
-static int cs_etm_get_ro_signed(struct perf_pmu *pmu, int cpu, const char *path)
+static int cs_etm_get_ro_signed(struct perf_pmu *pmu, struct perf_cpu cpu, const char *path,
+ __u64 *out_val)
{
char pmu_path[PATH_MAX];
int scan;
int val = 0;
/* Get RO metadata from sysfs */
- snprintf(pmu_path, PATH_MAX, "cpu%d/%s", cpu, path);
+ snprintf(pmu_path, PATH_MAX, "cpu%d/%s", cpu.cpu, path);
scan = perf_pmu__scan_file(pmu, pmu_path, "%d", &val);
- if (scan != 1)
+ if (scan != 1) {
pr_err("%s: error reading: %s\n", __func__, pmu_path);
+ return -EINVAL;
+ }
- return val;
+ *out_val = (__u64) val;
+ return 0;
}
-static bool cs_etm_pmu_path_exists(struct perf_pmu *pmu, int cpu, const char *path)
+static bool cs_etm_pmu_path_exists(struct perf_pmu *pmu, struct perf_cpu cpu, const char *path)
{
char pmu_path[PATH_MAX];
/* Get RO metadata from sysfs */
- snprintf(pmu_path, PATH_MAX, "cpu%d/%s", cpu, path);
+ snprintf(pmu_path, PATH_MAX, "cpu%d/%s", cpu.cpu, path);
return perf_pmu__file_exists(pmu, pmu_path);
}
@@ -636,16 +615,16 @@ static bool cs_etm_pmu_path_exists(struct perf_pmu *pmu, int cpu, const char *pa
#define TRCDEVARCH_ARCHVER_MASK GENMASK(15, 12)
#define TRCDEVARCH_ARCHVER(x) (((x) & TRCDEVARCH_ARCHVER_MASK) >> TRCDEVARCH_ARCHVER_SHIFT)
-static bool cs_etm_is_ete(struct auxtrace_record *itr, int cpu)
+static bool cs_etm_is_ete(struct auxtrace_record *itr, struct perf_cpu cpu)
{
struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr);
struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
- int trcdevarch;
+ __u64 trcdevarch;
if (!cs_etm_pmu_path_exists(cs_etm_pmu, cpu, metadata_ete_ro[CS_ETE_TRCDEVARCH]))
return false;
- trcdevarch = cs_etm_get_ro(cs_etm_pmu, cpu, metadata_ete_ro[CS_ETE_TRCDEVARCH]);
+ cs_etm_get_ro(cs_etm_pmu, cpu, metadata_ete_ro[CS_ETE_TRCDEVARCH], &trcdevarch);
/*
* ETE if ARCHVER is 5 (ARCHVER is 4 for ETM) and ARCHPART is 0xA13.
* See ETM_DEVARCH_ETE_ARCH in coresight-etm4x.h
@@ -653,7 +632,12 @@ static bool cs_etm_is_ete(struct auxtrace_record *itr, int cpu)
return TRCDEVARCH_ARCHVER(trcdevarch) == 5 && TRCDEVARCH_ARCHPART(trcdevarch) == 0xA13;
}
-static void cs_etm_save_etmv4_header(__u64 data[], struct auxtrace_record *itr, int cpu)
+static __u64 cs_etm_get_legacy_trace_id(struct perf_cpu cpu)
+{
+ return CORESIGHT_LEGACY_CPU_TRACE_ID(cpu.cpu);
+}
+
+static void cs_etm_save_etmv4_header(__u64 data[], struct auxtrace_record *itr, struct perf_cpu cpu)
{
struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr);
struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
@@ -661,33 +645,32 @@ static void cs_etm_save_etmv4_header(__u64 data[], struct auxtrace_record *itr,
/* Get trace configuration register */
data[CS_ETMV4_TRCCONFIGR] = cs_etmv4_get_config(itr);
/* traceID set to legacy version, in case new perf running on older system */
- data[CS_ETMV4_TRCTRACEIDR] =
- CORESIGHT_LEGACY_CPU_TRACE_ID(cpu) | CORESIGHT_TRACE_ID_UNUSED_FLAG;
+ data[CS_ETMV4_TRCTRACEIDR] = cs_etm_get_legacy_trace_id(cpu) |
+ CORESIGHT_TRACE_ID_UNUSED_FLAG;
/* Get read-only information from sysFS */
- data[CS_ETMV4_TRCIDR0] = cs_etm_get_ro(cs_etm_pmu, cpu,
- metadata_etmv4_ro[CS_ETMV4_TRCIDR0]);
- data[CS_ETMV4_TRCIDR1] = cs_etm_get_ro(cs_etm_pmu, cpu,
- metadata_etmv4_ro[CS_ETMV4_TRCIDR1]);
- data[CS_ETMV4_TRCIDR2] = cs_etm_get_ro(cs_etm_pmu, cpu,
- metadata_etmv4_ro[CS_ETMV4_TRCIDR2]);
- data[CS_ETMV4_TRCIDR8] = cs_etm_get_ro(cs_etm_pmu, cpu,
- metadata_etmv4_ro[CS_ETMV4_TRCIDR8]);
- data[CS_ETMV4_TRCAUTHSTATUS] = cs_etm_get_ro(cs_etm_pmu, cpu,
- metadata_etmv4_ro[CS_ETMV4_TRCAUTHSTATUS]);
+ cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR0],
+ &data[CS_ETMV4_TRCIDR0]);
+ cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR1],
+ &data[CS_ETMV4_TRCIDR1]);
+ cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR2],
+ &data[CS_ETMV4_TRCIDR2]);
+ cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR8],
+ &data[CS_ETMV4_TRCIDR8]);
+ cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TRCAUTHSTATUS],
+ &data[CS_ETMV4_TRCAUTHSTATUS]);
/* Kernels older than 5.19 may not expose ts_source */
- if (cs_etm_pmu_path_exists(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TS_SOURCE]))
- data[CS_ETMV4_TS_SOURCE] = (__u64) cs_etm_get_ro_signed(cs_etm_pmu, cpu,
- metadata_etmv4_ro[CS_ETMV4_TS_SOURCE]);
- else {
+ if (!cs_etm_pmu_path_exists(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TS_SOURCE]) ||
+ cs_etm_get_ro_signed(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TS_SOURCE],
+ &data[CS_ETMV4_TS_SOURCE])) {
pr_debug3("[%03d] pmu file 'ts_source' not found. Fallback to safe value (-1)\n",
- cpu);
+ cpu.cpu);
data[CS_ETMV4_TS_SOURCE] = (__u64) -1;
}
}
-static void cs_etm_save_ete_header(__u64 data[], struct auxtrace_record *itr, int cpu)
+static void cs_etm_save_ete_header(__u64 data[], struct auxtrace_record *itr, struct perf_cpu cpu)
{
struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr);
struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
@@ -695,36 +678,30 @@ static void cs_etm_save_ete_header(__u64 data[], struct auxtrace_record *itr, in
/* Get trace configuration register */
data[CS_ETE_TRCCONFIGR] = cs_etmv4_get_config(itr);
/* traceID set to legacy version, in case new perf running on older system */
- data[CS_ETE_TRCTRACEIDR] =
- CORESIGHT_LEGACY_CPU_TRACE_ID(cpu) | CORESIGHT_TRACE_ID_UNUSED_FLAG;
+ data[CS_ETE_TRCTRACEIDR] = cs_etm_get_legacy_trace_id(cpu) | CORESIGHT_TRACE_ID_UNUSED_FLAG;
/* Get read-only information from sysFS */
- data[CS_ETE_TRCIDR0] = cs_etm_get_ro(cs_etm_pmu, cpu,
- metadata_ete_ro[CS_ETE_TRCIDR0]);
- data[CS_ETE_TRCIDR1] = cs_etm_get_ro(cs_etm_pmu, cpu,
- metadata_ete_ro[CS_ETE_TRCIDR1]);
- data[CS_ETE_TRCIDR2] = cs_etm_get_ro(cs_etm_pmu, cpu,
- metadata_ete_ro[CS_ETE_TRCIDR2]);
- data[CS_ETE_TRCIDR8] = cs_etm_get_ro(cs_etm_pmu, cpu,
- metadata_ete_ro[CS_ETE_TRCIDR8]);
- data[CS_ETE_TRCAUTHSTATUS] = cs_etm_get_ro(cs_etm_pmu, cpu,
- metadata_ete_ro[CS_ETE_TRCAUTHSTATUS]);
+ cs_etm_get_ro(cs_etm_pmu, cpu, metadata_ete_ro[CS_ETE_TRCIDR0], &data[CS_ETE_TRCIDR0]);
+ cs_etm_get_ro(cs_etm_pmu, cpu, metadata_ete_ro[CS_ETE_TRCIDR1], &data[CS_ETE_TRCIDR1]);
+ cs_etm_get_ro(cs_etm_pmu, cpu, metadata_ete_ro[CS_ETE_TRCIDR2], &data[CS_ETE_TRCIDR2]);
+ cs_etm_get_ro(cs_etm_pmu, cpu, metadata_ete_ro[CS_ETE_TRCIDR8], &data[CS_ETE_TRCIDR8]);
+ cs_etm_get_ro(cs_etm_pmu, cpu, metadata_ete_ro[CS_ETE_TRCAUTHSTATUS],
+ &data[CS_ETE_TRCAUTHSTATUS]);
/* ETE uses the same registers as ETMv4 plus TRCDEVARCH */
- data[CS_ETE_TRCDEVARCH] = cs_etm_get_ro(cs_etm_pmu, cpu,
- metadata_ete_ro[CS_ETE_TRCDEVARCH]);
+ cs_etm_get_ro(cs_etm_pmu, cpu, metadata_ete_ro[CS_ETE_TRCDEVARCH],
+ &data[CS_ETE_TRCDEVARCH]);
/* Kernels older than 5.19 may not expose ts_source */
- if (cs_etm_pmu_path_exists(cs_etm_pmu, cpu, metadata_ete_ro[CS_ETE_TS_SOURCE]))
- data[CS_ETE_TS_SOURCE] = (__u64) cs_etm_get_ro_signed(cs_etm_pmu, cpu,
- metadata_ete_ro[CS_ETE_TS_SOURCE]);
- else {
+ if (!cs_etm_pmu_path_exists(cs_etm_pmu, cpu, metadata_ete_ro[CS_ETE_TS_SOURCE]) ||
+ cs_etm_get_ro_signed(cs_etm_pmu, cpu, metadata_ete_ro[CS_ETE_TS_SOURCE],
+ &data[CS_ETE_TS_SOURCE])) {
pr_debug3("[%03d] pmu file 'ts_source' not found. Fallback to safe value (-1)\n",
- cpu);
+ cpu.cpu);
data[CS_ETE_TS_SOURCE] = (__u64) -1;
}
}
-static void cs_etm_get_metadata(int cpu, u32 *offset,
+static void cs_etm_get_metadata(struct perf_cpu cpu, u32 *offset,
struct auxtrace_record *itr,
struct perf_record_auxtrace_info *info)
{
@@ -754,15 +731,13 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,
/* Get configuration register */
info->priv[*offset + CS_ETM_ETMCR] = cs_etm_get_config(itr);
/* traceID set to legacy value in case new perf running on old system */
- info->priv[*offset + CS_ETM_ETMTRACEIDR] =
- CORESIGHT_LEGACY_CPU_TRACE_ID(cpu) | CORESIGHT_TRACE_ID_UNUSED_FLAG;
+ info->priv[*offset + CS_ETM_ETMTRACEIDR] = cs_etm_get_legacy_trace_id(cpu) |
+ CORESIGHT_TRACE_ID_UNUSED_FLAG;
/* Get read-only information from sysFS */
- info->priv[*offset + CS_ETM_ETMCCER] =
- cs_etm_get_ro(cs_etm_pmu, cpu,
- metadata_etmv3_ro[CS_ETM_ETMCCER]);
- info->priv[*offset + CS_ETM_ETMIDR] =
- cs_etm_get_ro(cs_etm_pmu, cpu,
- metadata_etmv3_ro[CS_ETM_ETMIDR]);
+ cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv3_ro[CS_ETM_ETMCCER],
+ &info->priv[*offset + CS_ETM_ETMCCER]);
+ cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv3_ro[CS_ETM_ETMIDR],
+ &info->priv[*offset + CS_ETM_ETMIDR]);
/* How much space was used */
increment = CS_ETM_PRIV_MAX;
@@ -771,7 +746,7 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,
/* Build generic header portion */
info->priv[*offset + CS_ETM_MAGIC] = magic;
- info->priv[*offset + CS_ETM_CPU] = cpu;
+ info->priv[*offset + CS_ETM_CPU] = cpu.cpu;
info->priv[*offset + CS_ETM_NR_TRC_PARAMS] = nr_trc_params;
/* Where the next CPU entry should start from */
*offset += increment;
@@ -791,6 +766,7 @@ static int cs_etm_info_fill(struct auxtrace_record *itr,
struct cs_etm_recording *ptr =
container_of(itr, struct cs_etm_recording, itr);
struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
+ struct perf_cpu cpu;
if (priv_size != cs_etm_info_priv_size(itr, session->evlist))
return -EINVAL;
@@ -803,8 +779,6 @@ static int cs_etm_info_fill(struct auxtrace_record *itr,
cpu_map = online_cpus;
} else {
/* Make sure all specified CPUs are online */
- struct perf_cpu cpu;
-
perf_cpu_map__for_each_cpu(cpu, i, event_cpus) {
if (!perf_cpu_map__has(online_cpus, cpu))
return -EINVAL;
@@ -826,11 +800,9 @@ static int cs_etm_info_fill(struct auxtrace_record *itr,
offset = CS_ETM_SNAPSHOT + 1;
- for (i = 0; i < cpu__max_cpu().cpu && offset < priv_size; i++) {
- struct perf_cpu cpu = { .cpu = i, };
-
- if (perf_cpu_map__has(cpu_map, cpu))
- cs_etm_get_metadata(i, &offset, itr, info);
+ perf_cpu_map__for_each_cpu(cpu, i, cpu_map) {
+ assert(offset < priv_size);
+ cs_etm_get_metadata(cpu, &offset, itr, info);
}
perf_cpu_map__put(online_cpus);
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2 2/3] perf cs-etm: Remove repeated fetches of the ETM PMU
2024-05-01 13:57 [PATCH v2 0/3] perf cs-etm: Improve version detection and error reporting James Clark
2024-05-01 13:57 ` [PATCH v2 1/3] perf cs-etm: Use struct perf_cpu as much as possible James Clark
@ 2024-05-01 13:57 ` James Clark
2024-05-01 13:57 ` [PATCH v2 3/3] perf cs-etm: Improve version detection and error reporting James Clark
2024-05-02 14:35 ` [PATCH v2 0/3] " Arnaldo Carvalho de Melo
3 siblings, 0 replies; 5+ messages in thread
From: James Clark @ 2024-05-01 13:57 UTC (permalink / raw)
To: linux-perf-users, coresight, leo.yan, irogers
Cc: James Clark, Suzuki K Poulose, Mike Leach, John Garry,
Will Deacon, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Liang, Kan,
linux-arm-kernel, linux-kernel
Most functions already have cs_etm_pmu, so it's a bit neater to pass
it through rather than itr only to convert it again.
Signed-off-by: James Clark <james.clark@arm.com>
---
tools/perf/arch/arm/util/cs-etm.c | 60 ++++++++++++++-----------------
1 file changed, 27 insertions(+), 33 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index a1fa711dc41a..2fc4b41daea1 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -66,17 +66,14 @@ static const char * const metadata_ete_ro[] = {
[CS_ETE_TS_SOURCE] = "ts_source",
};
-static bool cs_etm_is_etmv4(struct auxtrace_record *itr, struct perf_cpu cpu);
-static bool cs_etm_is_ete(struct auxtrace_record *itr, struct perf_cpu cpu);
+static bool cs_etm_is_etmv4(struct perf_pmu *cs_etm_pmu, struct perf_cpu cpu);
+static bool cs_etm_is_ete(struct perf_pmu *cs_etm_pmu, struct perf_cpu cpu);
static int cs_etm_get_ro(struct perf_pmu *pmu, struct perf_cpu cpu, const char *path, __u64 *val);
static bool cs_etm_pmu_path_exists(struct perf_pmu *pmu, struct perf_cpu cpu, const char *path);
-static int cs_etm_validate_context_id(struct auxtrace_record *itr, struct evsel *evsel,
+static int cs_etm_validate_context_id(struct perf_pmu *cs_etm_pmu, struct evsel *evsel,
struct perf_cpu cpu)
{
- struct cs_etm_recording *ptr =
- container_of(itr, struct cs_etm_recording, itr);
- struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
int err;
__u64 val;
u64 contextid = evsel->core.attr.config &
@@ -88,7 +85,7 @@ static int cs_etm_validate_context_id(struct auxtrace_record *itr, struct evsel
return 0;
/* Not supported in etmv3 */
- if (!cs_etm_is_etmv4(itr, cpu)) {
+ if (!cs_etm_is_etmv4(cs_etm_pmu, 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;
@@ -134,12 +131,9 @@ static int cs_etm_validate_context_id(struct auxtrace_record *itr, struct evsel
return 0;
}
-static int cs_etm_validate_timestamp(struct auxtrace_record *itr, struct evsel *evsel,
+static int cs_etm_validate_timestamp(struct perf_pmu *cs_etm_pmu, struct evsel *evsel,
struct perf_cpu cpu)
{
- struct cs_etm_recording *ptr =
- container_of(itr, struct cs_etm_recording, itr);
- struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
int err;
__u64 val;
@@ -147,7 +141,7 @@ static int cs_etm_validate_timestamp(struct auxtrace_record *itr, struct evsel *
perf_pmu__format_bits(cs_etm_pmu, "timestamp")))
return 0;
- if (!cs_etm_is_etmv4(itr, cpu)) {
+ if (!cs_etm_is_etmv4(cs_etm_pmu, 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;
@@ -173,6 +167,13 @@ static int cs_etm_validate_timestamp(struct auxtrace_record *itr, struct evsel *
return 0;
}
+static struct perf_pmu *cs_etm_get_pmu(struct auxtrace_record *itr)
+{
+ struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr);
+
+ return ptr->cs_etm_pmu;
+}
+
/*
* Check whether the requested timestamp and contextid options should be
* available on all requested CPUs and if not, tell the user how to override.
@@ -180,7 +181,7 @@ static int cs_etm_validate_timestamp(struct auxtrace_record *itr, struct evsel *
* 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,
+static int cs_etm_validate_config(struct perf_pmu *cs_etm_pmu,
struct evsel *evsel)
{
int idx, err = 0;
@@ -204,11 +205,11 @@ static int cs_etm_validate_config(struct auxtrace_record *itr,
}
perf_cpu_map__for_each_cpu_skip_any(cpu, idx, intersect_cpus) {
- err = cs_etm_validate_context_id(itr, evsel, cpu);
+ err = cs_etm_validate_context_id(cs_etm_pmu, evsel, cpu);
if (err)
break;
- err = cs_etm_validate_timestamp(itr, evsel, cpu);
+ err = cs_etm_validate_timestamp(cs_etm_pmu, evsel, cpu);
if (err)
break;
}
@@ -449,7 +450,7 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
if (!perf_cpu_map__is_any_cpu_or_is_empty(cpus))
evsel__set_sample_bit(evsel, TIME);
- err = cs_etm_validate_config(itr, cs_etm_evsel);
+ err = cs_etm_validate_config(cs_etm_pmu, cs_etm_evsel);
out:
return err;
}
@@ -515,14 +516,15 @@ static u64 cs_etmv4_get_config(struct auxtrace_record *itr)
}
static size_t
-cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused,
- struct evlist *evlist __maybe_unused)
+cs_etm_info_priv_size(struct auxtrace_record *itr,
+ struct evlist *evlist)
{
int idx;
int etmv3 = 0, etmv4 = 0, ete = 0;
struct perf_cpu_map *event_cpus = evlist->core.user_requested_cpus;
struct perf_cpu_map *intersect_cpus;
struct perf_cpu cpu;
+ struct perf_pmu *cs_etm_pmu = cs_etm_get_pmu(itr);
if (!perf_cpu_map__has_any_cpu(event_cpus)) {
/* cpu map is not "any" CPU , we have specific CPUs to work with */
@@ -535,9 +537,9 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused,
intersect_cpus = perf_cpu_map__new_online_cpus();
}
perf_cpu_map__for_each_cpu_skip_any(cpu, idx, intersect_cpus) {
- if (cs_etm_is_ete(itr, cpu))
+ if (cs_etm_is_ete(cs_etm_pmu, cpu))
ete++;
- else if (cs_etm_is_etmv4(itr, cpu))
+ else if (cs_etm_is_etmv4(cs_etm_pmu, cpu))
etmv4++;
else
etmv3++;
@@ -550,12 +552,8 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused,
(etmv3 * CS_ETMV3_PRIV_SIZE));
}
-static bool cs_etm_is_etmv4(struct auxtrace_record *itr, struct perf_cpu cpu)
+static bool cs_etm_is_etmv4(struct perf_pmu *cs_etm_pmu, struct perf_cpu cpu)
{
- struct cs_etm_recording *ptr =
- container_of(itr, struct cs_etm_recording, itr);
- struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
-
/* Take any of the RO files for ETMv4 and see if it present */
return cs_etm_pmu_path_exists(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR0]);
}
@@ -615,10 +613,8 @@ static bool cs_etm_pmu_path_exists(struct perf_pmu *pmu, struct perf_cpu cpu, co
#define TRCDEVARCH_ARCHVER_MASK GENMASK(15, 12)
#define TRCDEVARCH_ARCHVER(x) (((x) & TRCDEVARCH_ARCHVER_MASK) >> TRCDEVARCH_ARCHVER_SHIFT)
-static bool cs_etm_is_ete(struct auxtrace_record *itr, struct perf_cpu cpu)
+static bool cs_etm_is_ete(struct perf_pmu *cs_etm_pmu, struct perf_cpu cpu)
{
- struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr);
- struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
__u64 trcdevarch;
if (!cs_etm_pmu_path_exists(cs_etm_pmu, cpu, metadata_ete_ro[CS_ETE_TRCDEVARCH]))
@@ -707,19 +703,17 @@ static void cs_etm_get_metadata(struct perf_cpu cpu, u32 *offset,
{
u32 increment, nr_trc_params;
u64 magic;
- struct cs_etm_recording *ptr =
- container_of(itr, struct cs_etm_recording, itr);
- struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
+ struct perf_pmu *cs_etm_pmu = cs_etm_get_pmu(itr);
/* first see what kind of tracer this cpu is affined to */
- if (cs_etm_is_ete(itr, cpu)) {
+ if (cs_etm_is_ete(cs_etm_pmu, cpu)) {
magic = __perf_cs_ete_magic;
cs_etm_save_ete_header(&info->priv[*offset], itr, cpu);
/* How much space was used */
increment = CS_ETE_PRIV_MAX;
nr_trc_params = CS_ETE_PRIV_MAX - CS_ETM_COMMON_BLK_MAX_V1;
- } else if (cs_etm_is_etmv4(itr, cpu)) {
+ } else if (cs_etm_is_etmv4(cs_etm_pmu, cpu)) {
magic = __perf_cs_etmv4_magic;
cs_etm_save_etmv4_header(&info->priv[*offset], itr, cpu);
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2 3/3] perf cs-etm: Improve version detection and error reporting
2024-05-01 13:57 [PATCH v2 0/3] perf cs-etm: Improve version detection and error reporting James Clark
2024-05-01 13:57 ` [PATCH v2 1/3] perf cs-etm: Use struct perf_cpu as much as possible James Clark
2024-05-01 13:57 ` [PATCH v2 2/3] perf cs-etm: Remove repeated fetches of the ETM PMU James Clark
@ 2024-05-01 13:57 ` James Clark
2024-05-02 14:35 ` [PATCH v2 0/3] " Arnaldo Carvalho de Melo
3 siblings, 0 replies; 5+ messages in thread
From: James Clark @ 2024-05-01 13:57 UTC (permalink / raw)
To: linux-perf-users, coresight, leo.yan, irogers
Cc: James Clark, Suzuki K Poulose, Mike Leach, John Garry,
Will Deacon, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Liang, Kan,
linux-arm-kernel, linux-kernel
When the config validation functions are warning about ETMv3, they do it
based on "not ETMv4". If the drivers aren't all loaded or the hardware
doesn't support Coresight it will appear as "not ETMv4" and then Perf
will print the error message "... not supported in ETMv3 ..." which is
wrong and confusing.
cs_etm_is_etmv4() is also misnamed because it also returns true for
ETE because ETE has a superset of the ETMv4 metadata files. Although
this was always done in the correct order so it wasn't a bug.
Improve all this by making a single get version function which also
handles not present as a separate case. Change the ETMv3 error message
to only print when ETMv3 is detected, and add a new error message for
the not present case.
Reviewed-by: Leo Yan <leo.yan@linux.dev>
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: James Clark <james.clark@arm.com>
---
tools/perf/arch/arm/util/cs-etm.c | 61 ++++++++++++++++++++++---------
1 file changed, 43 insertions(+), 18 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index 2fc4b41daea1..da6231367993 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -66,11 +66,25 @@ static const char * const metadata_ete_ro[] = {
[CS_ETE_TS_SOURCE] = "ts_source",
};
-static bool cs_etm_is_etmv4(struct perf_pmu *cs_etm_pmu, struct perf_cpu cpu);
+enum cs_etm_version { CS_NOT_PRESENT, CS_ETMV3, CS_ETMV4, CS_ETE };
+
static bool cs_etm_is_ete(struct perf_pmu *cs_etm_pmu, struct perf_cpu cpu);
static int cs_etm_get_ro(struct perf_pmu *pmu, struct perf_cpu cpu, const char *path, __u64 *val);
static bool cs_etm_pmu_path_exists(struct perf_pmu *pmu, struct perf_cpu cpu, const char *path);
+static enum cs_etm_version cs_etm_get_version(struct perf_pmu *cs_etm_pmu,
+ struct perf_cpu cpu)
+{
+ if (cs_etm_is_ete(cs_etm_pmu, cpu))
+ return CS_ETE;
+ else if (cs_etm_pmu_path_exists(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR0]))
+ return CS_ETMV4;
+ else if (cs_etm_pmu_path_exists(cs_etm_pmu, cpu, metadata_etmv3_ro[CS_ETM_ETMCCER]))
+ return CS_ETMV3;
+
+ return CS_NOT_PRESENT;
+}
+
static int cs_etm_validate_context_id(struct perf_pmu *cs_etm_pmu, struct evsel *evsel,
struct perf_cpu cpu)
{
@@ -85,7 +99,7 @@ static int cs_etm_validate_context_id(struct perf_pmu *cs_etm_pmu, struct evsel
return 0;
/* Not supported in etmv3 */
- if (!cs_etm_is_etmv4(cs_etm_pmu, cpu)) {
+ if (cs_etm_get_version(cs_etm_pmu, cpu) == CS_ETMV3) {
pr_err("%s: contextid not supported in ETMv3, disable with %s/contextid=0/\n",
CORESIGHT_ETM_PMU_NAME, CORESIGHT_ETM_PMU_NAME);
return -EINVAL;
@@ -141,7 +155,7 @@ static int cs_etm_validate_timestamp(struct perf_pmu *cs_etm_pmu, struct evsel *
perf_pmu__format_bits(cs_etm_pmu, "timestamp")))
return 0;
- if (!cs_etm_is_etmv4(cs_etm_pmu, cpu)) {
+ if (cs_etm_get_version(cs_etm_pmu, cpu) == CS_ETMV3) {
pr_err("%s: timestamp not supported in ETMv3, disable with %s/timestamp=0/\n",
CORESIGHT_ETM_PMU_NAME, CORESIGHT_ETM_PMU_NAME);
return -EINVAL;
@@ -205,6 +219,11 @@ static int cs_etm_validate_config(struct perf_pmu *cs_etm_pmu,
}
perf_cpu_map__for_each_cpu_skip_any(cpu, idx, intersect_cpus) {
+ if (cs_etm_get_version(cs_etm_pmu, cpu) == CS_NOT_PRESENT) {
+ pr_err("%s: Not found on CPU %d. Check hardware and firmware support and that all Coresight drivers are loaded\n",
+ CORESIGHT_ETM_PMU_NAME, cpu.cpu);
+ return -EINVAL;
+ }
err = cs_etm_validate_context_id(cs_etm_pmu, evsel, cpu);
if (err)
break;
@@ -536,13 +555,13 @@ cs_etm_info_priv_size(struct auxtrace_record *itr,
/* Event can be "any" CPU so count all online CPUs. */
intersect_cpus = perf_cpu_map__new_online_cpus();
}
+ /* Count number of each type of ETM. Don't count if that CPU has CS_NOT_PRESENT. */
perf_cpu_map__for_each_cpu_skip_any(cpu, idx, intersect_cpus) {
- if (cs_etm_is_ete(cs_etm_pmu, cpu))
- ete++;
- else if (cs_etm_is_etmv4(cs_etm_pmu, cpu))
- etmv4++;
- else
- etmv3++;
+ enum cs_etm_version v = cs_etm_get_version(cs_etm_pmu, cpu);
+
+ ete += v == CS_ETE;
+ etmv4 += v == CS_ETMV4;
+ etmv3 += v == CS_ETMV3;
}
perf_cpu_map__put(intersect_cpus);
@@ -552,12 +571,6 @@ cs_etm_info_priv_size(struct auxtrace_record *itr,
(etmv3 * CS_ETMV3_PRIV_SIZE));
}
-static bool cs_etm_is_etmv4(struct perf_pmu *cs_etm_pmu, struct perf_cpu cpu)
-{
- /* Take any of the RO files for ETMv4 and see if it present */
- return cs_etm_pmu_path_exists(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR0]);
-}
-
static int cs_etm_get_ro(struct perf_pmu *pmu, struct perf_cpu cpu, const char *path, __u64 *val)
{
char pmu_path[PATH_MAX];
@@ -706,21 +719,26 @@ static void cs_etm_get_metadata(struct perf_cpu cpu, u32 *offset,
struct perf_pmu *cs_etm_pmu = cs_etm_get_pmu(itr);
/* first see what kind of tracer this cpu is affined to */
- if (cs_etm_is_ete(cs_etm_pmu, cpu)) {
+ switch (cs_etm_get_version(cs_etm_pmu, cpu)) {
+ case CS_ETE:
magic = __perf_cs_ete_magic;
cs_etm_save_ete_header(&info->priv[*offset], itr, cpu);
/* How much space was used */
increment = CS_ETE_PRIV_MAX;
nr_trc_params = CS_ETE_PRIV_MAX - CS_ETM_COMMON_BLK_MAX_V1;
- } else if (cs_etm_is_etmv4(cs_etm_pmu, cpu)) {
+ break;
+
+ case CS_ETMV4:
magic = __perf_cs_etmv4_magic;
cs_etm_save_etmv4_header(&info->priv[*offset], itr, cpu);
/* How much space was used */
increment = CS_ETMV4_PRIV_MAX;
nr_trc_params = CS_ETMV4_PRIV_MAX - CS_ETMV4_TRCCONFIGR;
- } else {
+ break;
+
+ case CS_ETMV3:
magic = __perf_cs_etmv3_magic;
/* Get configuration register */
info->priv[*offset + CS_ETM_ETMCR] = cs_etm_get_config(itr);
@@ -736,6 +754,13 @@ static void cs_etm_get_metadata(struct perf_cpu cpu, u32 *offset,
/* How much space was used */
increment = CS_ETM_PRIV_MAX;
nr_trc_params = CS_ETM_PRIV_MAX - CS_ETM_ETMCR;
+ break;
+
+ default:
+ case CS_NOT_PRESENT:
+ /* Unreachable, CPUs already validated in cs_etm_validate_config() */
+ assert(true);
+ return;
}
/* Build generic header portion */
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread