* [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields
@ 2025-12-22 15:14 James Clark
2025-12-22 15:14 ` [PATCH v4 01/14] perf parse-events: Refactor get_config_terms() to remove macros James Clark
` (15 more replies)
0 siblings, 16 replies; 24+ messages in thread
From: James Clark @ 2025-12-22 15:14 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan
Cc: linux-perf-users, linux-kernel, coresight, linux-arm-kernel,
James Clark, Leo Yan
The specific config field that an event format attribute is in is
consistently hard coded, even though the API is supposed to be that the
driver publishes the config field name. To stop this pattern from being
copy pasted and causing problems in the future, replace them all with
calls to a new helper that returns the value that a user set.
This reveals some issues in evsel__set_config_if_unset(). It doesn't
work with sparse bitfields, which are an unused but documented feature.
And it also only writes to the attr.config field. To fix it we need to
start tracking user changes for all config fields and then use existing
helper functions that support sparse bitfields. Some other refactoring
was also required and a test was added.
---
Changes in v4:
- Constify some function args (Ian)
- Move some evsel__* functions to evsel.c and make some pmu.c functions
public to support this (Ian)
- Drop pmu arg where it can be fetched from evsel->pmu (Ian)
- Link to v3: https://lore.kernel.org/r/20251212-james-perf-config-bits-v3-0-aa36a4846776@linaro.org
Changes in v3:
- Fix uninitialized variable warning on GCC
- Fix leak of evlist in test
- Confirm no type punning issues with ubsan (Ian)
- Link to v2: https://lore.kernel.org/r/20251208-james-perf-config-bits-v2-0-4ac0281993b0@linaro.org
Changes in v2:
- Remove macros in get_config_chgs() and some other refactoring.
- Support sparse bitfields in evsel__set_config_if_unset().
- Always track user changes instead of only when
'pmu->perf_event_attr_init_default' is set.
- Add a test.
- Don't bail out in cs-etm.c if any format fields are missing (Leo).
- Rename 'guess' to 'synth' (Mike).
- Link to v1: https://lore.kernel.org/r/20251201-james-perf-config-bits-v1-0-22ecbbf8007c@linaro.org
---
James Clark (14):
perf parse-events: Refactor get_config_terms() to remove macros
perf evsel: Refactor evsel__set_config_if_unset() arguments
perf evsel: Move evsel__* functions to evsel.c
perf evsel: Support sparse fields in evsel__set_config_if_unset()
perf parse-events: Track all user changed config bits
perf evsel: apply evsel__set_config_if_unset() to all config fields
perf evsel: Add a helper to get the value of a config field
perf parse-events: Always track user config changes
perf tests: Test evsel__set_config_if_unset() and config change tracking
perf cs-etm: Make a helper to find the Coresight evsel
perf cs-etm: Don't use hard coded config bits when setting up ETMCR
perf cs-etm: Don't use hard coded config bits when setting up TRCCONFIGR
perf cs-etm: Don't hard code config attribute when configuring the event
perf arm-spe: Don't hard code config attribute
tools/perf/arch/arm/util/cs-etm.c | 201 +++++++++++++++-------------
tools/perf/arch/arm64/util/arm-spe.c | 17 +--
tools/perf/arch/x86/util/intel-pt.c | 3 +-
tools/perf/tests/pmu.c | 91 +++++++++++++
tools/perf/util/evsel.c | 112 +++++++++++++++-
tools/perf/util/evsel.h | 6 +-
tools/perf/util/evsel_config.h | 7 +-
tools/perf/util/parse-events.c | 248 ++++++++++++++++++++---------------
tools/perf/util/pmu.c | 95 ++++----------
tools/perf/util/pmu.h | 34 ++++-
10 files changed, 529 insertions(+), 285 deletions(-)
---
base-commit: cbd41c6d4c26c161a2b0e70ad411d3885ff13507
change-id: 20251112-james-perf-config-bits-bee7106f0f00
Best regards,
--
James Clark <james.clark@linaro.org>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v4 01/14] perf parse-events: Refactor get_config_terms() to remove macros
2025-12-22 15:14 [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
@ 2025-12-22 15:14 ` James Clark
2026-01-13 20:53 ` Arnaldo Carvalho de Melo
2025-12-22 15:14 ` [PATCH v4 02/14] perf evsel: Refactor evsel__set_config_if_unset() arguments James Clark
` (14 subsequent siblings)
15 siblings, 1 reply; 24+ messages in thread
From: James Clark @ 2025-12-22 15:14 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan
Cc: linux-perf-users, linux-kernel, coresight, linux-arm-kernel,
James Clark
The ADD_CONFIG_TERM() macros build the __type argument out of a partial
EVSEL__CONFIG_TERM_x enum name. This means that they can't be called
from a function where __type is a variable and it's also impossible to
grep the codebase to find usages of these enums as they're never typed
in full.
Fix this by removing the macros and replacing them with an
add_config_term() function. It seems the main reason these existed in
the first place was to avoid type punning and to write to a specific
field in the union, but the same thing can be achieved with a single
write to a u64 'val' field.
Running the Perf tests with "-fsanitize=undefined -fno-sanitize-recover"
results in no new issues as a result of this change.
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/perf/util/evsel_config.h | 1 +
tools/perf/util/parse-events.c | 146 ++++++++++++++++++++++++-----------------
2 files changed, 86 insertions(+), 61 deletions(-)
diff --git a/tools/perf/util/evsel_config.h b/tools/perf/util/evsel_config.h
index bcd3a978f0c4..685fd8d5c4a8 100644
--- a/tools/perf/util/evsel_config.h
+++ b/tools/perf/util/evsel_config.h
@@ -50,6 +50,7 @@ struct evsel_config_term {
u64 cfg_chg;
char *str;
int cpu;
+ u64 val;
} val;
bool weak;
};
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 17c1c36a7bf9..46422286380f 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1116,105 +1116,107 @@ static int config_attr(struct perf_event_attr *attr,
return 0;
}
-static int get_config_terms(const struct parse_events_terms *head_config,
- struct list_head *head_terms)
+static struct evsel_config_term *add_config_term(enum evsel_term_type type,
+ struct list_head *head_terms,
+ bool weak)
{
-#define ADD_CONFIG_TERM(__type, __weak) \
- struct evsel_config_term *__t; \
- \
- __t = zalloc(sizeof(*__t)); \
- if (!__t) \
- return -ENOMEM; \
- \
- INIT_LIST_HEAD(&__t->list); \
- __t->type = EVSEL__CONFIG_TERM_ ## __type; \
- __t->weak = __weak; \
- list_add_tail(&__t->list, head_terms)
-
-#define ADD_CONFIG_TERM_VAL(__type, __name, __val, __weak) \
-do { \
- ADD_CONFIG_TERM(__type, __weak); \
- __t->val.__name = __val; \
-} while (0)
+ struct evsel_config_term *t;
-#define ADD_CONFIG_TERM_STR(__type, __val, __weak) \
-do { \
- ADD_CONFIG_TERM(__type, __weak); \
- __t->val.str = strdup(__val); \
- if (!__t->val.str) { \
- zfree(&__t); \
- return -ENOMEM; \
- } \
- __t->free_str = true; \
-} while (0)
+ t = zalloc(sizeof(*t));
+ if (!t)
+ return NULL;
+
+ INIT_LIST_HEAD(&t->list);
+ t->type = type;
+ t->weak = weak;
+ list_add_tail(&t->list, head_terms);
+ return t;
+}
+
+static int get_config_terms(const struct parse_events_terms *head_config,
+ struct list_head *head_terms)
+{
struct parse_events_term *term;
list_for_each_entry(term, &head_config->terms, list) {
+ struct evsel_config_term *new_term;
+ enum evsel_term_type new_type;
+ bool str_type = false;
+ u64 val;
+
switch (term->type_term) {
case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
- ADD_CONFIG_TERM_VAL(PERIOD, period, term->val.num, term->weak);
+ new_type = EVSEL__CONFIG_TERM_PERIOD;
+ val = term->val.num;
break;
case PARSE_EVENTS__TERM_TYPE_SAMPLE_FREQ:
- ADD_CONFIG_TERM_VAL(FREQ, freq, term->val.num, term->weak);
+ new_type = EVSEL__CONFIG_TERM_FREQ;
+ val = term->val.num;
break;
case PARSE_EVENTS__TERM_TYPE_TIME:
- ADD_CONFIG_TERM_VAL(TIME, time, term->val.num, term->weak);
+ new_type = EVSEL__CONFIG_TERM_TIME;
+ val = term->val.num;
break;
case PARSE_EVENTS__TERM_TYPE_CALLGRAPH:
- ADD_CONFIG_TERM_STR(CALLGRAPH, term->val.str, term->weak);
+ new_type = EVSEL__CONFIG_TERM_CALLGRAPH;
+ str_type = true;
break;
case PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE:
- ADD_CONFIG_TERM_STR(BRANCH, term->val.str, term->weak);
+ new_type = EVSEL__CONFIG_TERM_BRANCH;
+ str_type = true;
break;
case PARSE_EVENTS__TERM_TYPE_STACKSIZE:
- ADD_CONFIG_TERM_VAL(STACK_USER, stack_user,
- term->val.num, term->weak);
+ new_type = EVSEL__CONFIG_TERM_STACK_USER;
+ val = term->val.num;
break;
case PARSE_EVENTS__TERM_TYPE_INHERIT:
- ADD_CONFIG_TERM_VAL(INHERIT, inherit,
- term->val.num ? 1 : 0, term->weak);
+ new_type = EVSEL__CONFIG_TERM_INHERIT;
+ val = term->val.num ? 1 : 0;
break;
case PARSE_EVENTS__TERM_TYPE_NOINHERIT:
- ADD_CONFIG_TERM_VAL(INHERIT, inherit,
- term->val.num ? 0 : 1, term->weak);
+ new_type = EVSEL__CONFIG_TERM_INHERIT;
+ val = term->val.num ? 0 : 1;
break;
case PARSE_EVENTS__TERM_TYPE_MAX_STACK:
- ADD_CONFIG_TERM_VAL(MAX_STACK, max_stack,
- term->val.num, term->weak);
+ new_type = EVSEL__CONFIG_TERM_MAX_STACK;
+ val = term->val.num;
break;
case PARSE_EVENTS__TERM_TYPE_MAX_EVENTS:
- ADD_CONFIG_TERM_VAL(MAX_EVENTS, max_events,
- term->val.num, term->weak);
+ new_type = EVSEL__CONFIG_TERM_MAX_EVENTS;
+ val = term->val.num;
break;
case PARSE_EVENTS__TERM_TYPE_OVERWRITE:
- ADD_CONFIG_TERM_VAL(OVERWRITE, overwrite,
- term->val.num ? 1 : 0, term->weak);
+ new_type = EVSEL__CONFIG_TERM_OVERWRITE;
+ val = term->val.num ? 1 : 0;
break;
case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:
- ADD_CONFIG_TERM_VAL(OVERWRITE, overwrite,
- term->val.num ? 0 : 1, term->weak);
+ new_type = EVSEL__CONFIG_TERM_OVERWRITE;
+ val = term->val.num ? 0 : 1;
break;
case PARSE_EVENTS__TERM_TYPE_DRV_CFG:
- ADD_CONFIG_TERM_STR(DRV_CFG, term->val.str, term->weak);
+ new_type = EVSEL__CONFIG_TERM_DRV_CFG;
+ str_type = true;
break;
case PARSE_EVENTS__TERM_TYPE_PERCORE:
- ADD_CONFIG_TERM_VAL(PERCORE, percore,
- term->val.num ? true : false, term->weak);
+ new_type = EVSEL__CONFIG_TERM_PERCORE;
+ val = term->val.num ? true : false;
break;
case PARSE_EVENTS__TERM_TYPE_AUX_OUTPUT:
- ADD_CONFIG_TERM_VAL(AUX_OUTPUT, aux_output,
- term->val.num ? 1 : 0, term->weak);
+ new_type = EVSEL__CONFIG_TERM_AUX_OUTPUT;
+ val = term->val.num ? 1 : 0;
break;
case PARSE_EVENTS__TERM_TYPE_AUX_ACTION:
- ADD_CONFIG_TERM_STR(AUX_ACTION, term->val.str, term->weak);
+ new_type = EVSEL__CONFIG_TERM_AUX_ACTION;
+ str_type = true;
break;
case PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE:
- ADD_CONFIG_TERM_VAL(AUX_SAMPLE_SIZE, aux_sample_size,
- term->val.num, term->weak);
+ new_type = EVSEL__CONFIG_TERM_AUX_SAMPLE_SIZE;
+ val = term->val.num;
break;
case PARSE_EVENTS__TERM_TYPE_RATIO_TO_PREV:
- ADD_CONFIG_TERM_STR(RATIO_TO_PREV, term->val.str, term->weak);
+ new_type = EVSEL__CONFIG_TERM_RATIO_TO_PREV;
+ str_type = true;
break;
case PARSE_EVENTS__TERM_TYPE_USER:
case PARSE_EVENTS__TERM_TYPE_CONFIG:
@@ -1229,7 +1231,23 @@ do { \
case PARSE_EVENTS__TERM_TYPE_RAW:
case PARSE_EVENTS__TERM_TYPE_CPU:
default:
- break;
+ /* Don't add a new term for these ones */
+ continue;
+ }
+
+ new_term = add_config_term(new_type, head_terms, term->weak);
+ if (!new_term)
+ return -ENOMEM;
+
+ if (str_type) {
+ new_term->val.str = strdup(term->val.str);
+ if (!new_term->val.str) {
+ zfree(&new_term);
+ return -ENOMEM;
+ }
+ new_term->free_str = true;
+ } else {
+ new_term->val.val = val;
}
}
return 0;
@@ -1290,10 +1308,16 @@ static int get_config_chgs(struct perf_pmu *pmu, struct parse_events_terms *head
}
}
- if (bits)
- ADD_CONFIG_TERM_VAL(CFG_CHG, cfg_chg, bits, false);
+ if (bits) {
+ struct evsel_config_term *new_term;
+
+ new_term = add_config_term(EVSEL__CONFIG_TERM_CFG_CHG,
+ head_terms, false);
+ if (!new_term)
+ return -ENOMEM;
+ new_term->val.cfg_chg = bits;
+ }
-#undef ADD_CONFIG_TERM
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v4 02/14] perf evsel: Refactor evsel__set_config_if_unset() arguments
2025-12-22 15:14 [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
2025-12-22 15:14 ` [PATCH v4 01/14] perf parse-events: Refactor get_config_terms() to remove macros James Clark
@ 2025-12-22 15:14 ` James Clark
2026-01-13 22:13 ` Arnaldo Carvalho de Melo
2025-12-22 15:14 ` [PATCH v4 03/14] perf evsel: Move evsel__* functions to evsel.c James Clark
` (13 subsequent siblings)
15 siblings, 1 reply; 24+ messages in thread
From: James Clark @ 2025-12-22 15:14 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan
Cc: linux-perf-users, linux-kernel, coresight, linux-arm-kernel,
James Clark
Make the evsel argument first to match the other evsel__* functions
and remove the redundant pmu argument, which can be accessed via evsel.
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/perf/arch/arm/util/cs-etm.c | 9 +++------
tools/perf/arch/arm64/util/arm-spe.c | 2 +-
tools/perf/arch/x86/util/intel-pt.c | 3 +--
tools/perf/util/evsel.h | 4 ++--
tools/perf/util/pmu.c | 6 +++---
5 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index ea891d12f8f4..c28208361d91 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -441,10 +441,8 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
* when a context switch happened.
*/
if (!perf_cpu_map__is_any_cpu_or_is_empty(cpus)) {
- evsel__set_config_if_unset(cs_etm_pmu, cs_etm_evsel,
- "timestamp", 1);
- evsel__set_config_if_unset(cs_etm_pmu, cs_etm_evsel,
- "contextid", 1);
+ evsel__set_config_if_unset(cs_etm_evsel, "timestamp", 1);
+ evsel__set_config_if_unset(cs_etm_evsel, "contextid", 1);
}
/*
@@ -453,8 +451,7 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
* timestamp tracing.
*/
if (opts->sample_time_set)
- evsel__set_config_if_unset(cs_etm_pmu, cs_etm_evsel,
- "timestamp", 1);
+ evsel__set_config_if_unset(cs_etm_evsel, "timestamp", 1);
/* Add dummy event to keep tracking */
err = parse_event(evlist, "dummy:u");
diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index d5ec1408d0ae..51014f8bff97 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -274,7 +274,7 @@ static void arm_spe_setup_evsel(struct evsel *evsel, struct perf_cpu_map *cpus)
*/
if (!perf_cpu_map__is_any_cpu_or_is_empty(cpus)) {
evsel__set_sample_bit(evsel, CPU);
- evsel__set_config_if_unset(evsel->pmu, evsel, "ts_enable", 1);
+ evsel__set_config_if_unset(evsel, "ts_enable", 1);
}
/*
diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
index b394ad9cc635..c131a727774f 100644
--- a/tools/perf/arch/x86/util/intel-pt.c
+++ b/tools/perf/arch/x86/util/intel-pt.c
@@ -664,8 +664,7 @@ static int intel_pt_recording_options(struct auxtrace_record *itr,
return 0;
if (opts->auxtrace_sample_mode)
- evsel__set_config_if_unset(intel_pt_pmu, intel_pt_evsel,
- "psb_period", 0);
+ evsel__set_config_if_unset(intel_pt_evsel, "psb_period", 0);
err = intel_pt_validate_config(intel_pt_pmu, intel_pt_evsel);
if (err)
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index a08130ff2e47..2cf87bc67df7 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -575,8 +575,8 @@ void evsel__uniquify_counter(struct evsel *counter);
((((src) >> (pos)) & ((1ull << (size)) - 1)) << (63 - ((pos) + (size) - 1)))
u64 evsel__bitfield_swap_branch_flags(u64 value);
-void evsel__set_config_if_unset(struct perf_pmu *pmu, struct evsel *evsel,
- const char *config_name, u64 val);
+void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,
+ u64 val);
bool evsel__is_offcpu_event(struct evsel *evsel);
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 956ea273c2c7..e87c12946d71 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1382,8 +1382,8 @@ bool evsel__is_aux_event(const struct evsel *evsel)
* something to true, pass 1 for val rather than a pre shifted value.
*/
#define field_prep(_mask, _val) (((_val) << (ffsll(_mask) - 1)) & (_mask))
-void evsel__set_config_if_unset(struct perf_pmu *pmu, struct evsel *evsel,
- const char *config_name, u64 val)
+void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,
+ u64 val)
{
u64 user_bits = 0, bits;
struct evsel_config_term *term = evsel__get_config_term(evsel, CFG_CHG);
@@ -1391,7 +1391,7 @@ void evsel__set_config_if_unset(struct perf_pmu *pmu, struct evsel *evsel,
if (term)
user_bits = term->val.cfg_chg;
- bits = perf_pmu__format_bits(pmu, config_name);
+ bits = perf_pmu__format_bits(evsel->pmu, config_name);
/* Do nothing if the user changed the value */
if (bits & user_bits)
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v4 03/14] perf evsel: Move evsel__* functions to evsel.c
2025-12-22 15:14 [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
2025-12-22 15:14 ` [PATCH v4 01/14] perf parse-events: Refactor get_config_terms() to remove macros James Clark
2025-12-22 15:14 ` [PATCH v4 02/14] perf evsel: Refactor evsel__set_config_if_unset() arguments James Clark
@ 2025-12-22 15:14 ` James Clark
2025-12-22 15:14 ` [PATCH v4 04/14] perf evsel: Support sparse fields in evsel__set_config_if_unset() James Clark
` (12 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: James Clark @ 2025-12-22 15:14 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan
Cc: linux-perf-users, linux-kernel, coresight, linux-arm-kernel,
James Clark
At least one of these were put here to avoid a Python binding linking
issue which is no longer present. Put them back in their correct
location to avoid confusion about which file to add a new evsel__*
function to later.
Link: https://lore.kernel.org/all/ZEbAS2yx2fguW60w@kernel.org/
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/perf/util/evsel.c | 40 ++++++++++++++++++++++++++++++++++++++++
tools/perf/util/pmu.c | 40 ----------------------------------------
2 files changed, 40 insertions(+), 40 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index ec6552a6f667..21878e9bd029 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1314,6 +1314,35 @@ struct evsel_config_term *__evsel__get_config_term(struct evsel *evsel, enum evs
return found_term;
}
+/*
+ * Set @config_name to @val as long as the user hasn't already set or cleared it
+ * by passing a config term on the command line.
+ *
+ * @val is the value to put into the bits specified by @config_name rather than
+ * the bit pattern. It is shifted into position by this function, so to set
+ * something to true, pass 1 for val rather than a pre shifted value.
+ */
+#define field_prep(_mask, _val) (((_val) << (ffsll(_mask) - 1)) & (_mask))
+void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,
+ u64 val)
+{
+ u64 user_bits = 0, bits;
+ struct evsel_config_term *term = evsel__get_config_term(evsel, CFG_CHG);
+
+ if (term)
+ user_bits = term->val.cfg_chg;
+
+ bits = perf_pmu__format_bits(evsel->pmu, config_name);
+
+ /* Do nothing if the user changed the value */
+ if (bits & user_bits)
+ return;
+
+ /* Otherwise replace it */
+ evsel->core.attr.config &= ~bits;
+ evsel->core.attr.config |= field_prep(bits, val);
+}
+
void __weak arch_evsel__set_sample_weight(struct evsel *evsel)
{
evsel__set_sample_bit(evsel, WEIGHT);
@@ -4098,6 +4127,17 @@ void evsel__set_leader(struct evsel *evsel, struct evsel *leader)
evsel->core.leader = &leader->core;
}
+bool evsel__is_aux_event(const struct evsel *evsel)
+{
+ struct perf_pmu *pmu;
+
+ if (evsel->needs_auxtrace_mmap)
+ return true;
+
+ pmu = evsel__find_pmu(evsel);
+ return pmu && pmu->auxtrace;
+}
+
int evsel__source_count(const struct evsel *evsel)
{
struct evsel *pos;
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index e87c12946d71..e3a1f26213ec 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1362,46 +1362,6 @@ void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu)
}
}
-bool evsel__is_aux_event(const struct evsel *evsel)
-{
- struct perf_pmu *pmu;
-
- if (evsel->needs_auxtrace_mmap)
- return true;
-
- pmu = evsel__find_pmu(evsel);
- return pmu && pmu->auxtrace;
-}
-
-/*
- * Set @config_name to @val as long as the user hasn't already set or cleared it
- * by passing a config term on the command line.
- *
- * @val is the value to put into the bits specified by @config_name rather than
- * the bit pattern. It is shifted into position by this function, so to set
- * something to true, pass 1 for val rather than a pre shifted value.
- */
-#define field_prep(_mask, _val) (((_val) << (ffsll(_mask) - 1)) & (_mask))
-void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,
- u64 val)
-{
- u64 user_bits = 0, bits;
- struct evsel_config_term *term = evsel__get_config_term(evsel, CFG_CHG);
-
- if (term)
- user_bits = term->val.cfg_chg;
-
- bits = perf_pmu__format_bits(evsel->pmu, config_name);
-
- /* Do nothing if the user changed the value */
- if (bits & user_bits)
- return;
-
- /* Otherwise replace it */
- evsel->core.attr.config &= ~bits;
- evsel->core.attr.config |= field_prep(bits, val);
-}
-
static struct perf_pmu_format *
pmu_find_format(const struct list_head *formats, const char *name)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v4 04/14] perf evsel: Support sparse fields in evsel__set_config_if_unset()
2025-12-22 15:14 [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
` (2 preceding siblings ...)
2025-12-22 15:14 ` [PATCH v4 03/14] perf evsel: Move evsel__* functions to evsel.c James Clark
@ 2025-12-22 15:14 ` James Clark
2025-12-22 15:14 ` [PATCH v4 05/14] perf parse-events: Track all user changed config bits James Clark
` (11 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: James Clark @ 2025-12-22 15:14 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan
Cc: linux-perf-users, linux-kernel, coresight, linux-arm-kernel,
James Clark
Sparse config fields are technically supported although currently
unused. field_prep() only works for contiguous bitfields so replace it
with pmu_format_value().
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/perf/util/evsel.c | 4 +---
tools/perf/util/pmu.c | 3 +--
tools/perf/util/pmu.h | 1 +
3 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 21878e9bd029..45ab97beeb34 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1322,7 +1322,6 @@ struct evsel_config_term *__evsel__get_config_term(struct evsel *evsel, enum evs
* the bit pattern. It is shifted into position by this function, so to set
* something to true, pass 1 for val rather than a pre shifted value.
*/
-#define field_prep(_mask, _val) (((_val) << (ffsll(_mask) - 1)) & (_mask))
void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,
u64 val)
{
@@ -1339,8 +1338,7 @@ void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,
return;
/* Otherwise replace it */
- evsel->core.attr.config &= ~bits;
- evsel->core.attr.config |= field_prep(bits, val);
+ pmu_format_value(&bits, val, &evsel->core.attr.config, /*zero=*/true);
}
void __weak arch_evsel__set_sample_weight(struct evsel *evsel)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index e3a1f26213ec..599fdf5e28ba 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1404,8 +1404,7 @@ int perf_pmu__format_type(struct perf_pmu *pmu, const char *name)
* Sets value based on the format definition (format parameter)
* and unformatted value (value parameter).
*/
-static void pmu_format_value(unsigned long *format, __u64 value, __u64 *v,
- bool zero)
+void pmu_format_value(unsigned long *format, __u64 value, __u64 *v, bool zero)
{
unsigned long fbit, vbit;
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 8f11bfe8ed6d..e34db47ae5ef 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -254,6 +254,7 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_
u64 *alternate_hw_config, struct parse_events_error *err);
int perf_pmu__find_event(struct perf_pmu *pmu, const char *event, void *state, pmu_event_callback cb);
+void pmu_format_value(unsigned long *format, __u64 value, __u64 *v, bool zero);
void perf_pmu_format__set_value(void *format, int config, unsigned long *bits);
bool perf_pmu__has_format(const struct perf_pmu *pmu, const char *name);
int perf_pmu__for_each_format(struct perf_pmu *pmu, void *state, pmu_format_callback cb);
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v4 05/14] perf parse-events: Track all user changed config bits
2025-12-22 15:14 [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
` (3 preceding siblings ...)
2025-12-22 15:14 ` [PATCH v4 04/14] perf evsel: Support sparse fields in evsel__set_config_if_unset() James Clark
@ 2025-12-22 15:14 ` James Clark
2025-12-22 15:14 ` [PATCH v4 06/14] perf evsel: apply evsel__set_config_if_unset() to all config fields James Clark
` (10 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: James Clark @ 2025-12-22 15:14 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan
Cc: linux-perf-users, linux-kernel, coresight, linux-arm-kernel,
James Clark
Currently we only track which bits were set by the user in attr->config.
But all configN fields should be treated equally as they can all have
default and user overridden values.
Track them all by making get_config_chgs() generic and calling it once
for each config value.
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/perf/util/evsel.c | 9 +++-
tools/perf/util/evsel_config.h | 6 ++-
tools/perf/util/parse-events.c | 98 +++++++++++++++++++++++-------------------
tools/perf/util/pmu.c | 4 +-
tools/perf/util/pmu.h | 4 +-
5 files changed, 70 insertions(+), 51 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 45ab97beeb34..354e28e94523 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1242,7 +1242,11 @@ static void evsel__apply_config_terms(struct evsel *evsel,
case EVSEL__CONFIG_TERM_AUX_SAMPLE_SIZE:
/* Already applied by auxtrace */
break;
- case EVSEL__CONFIG_TERM_CFG_CHG:
+ case EVSEL__CONFIG_TERM_USR_CHG_CONFIG:
+ case EVSEL__CONFIG_TERM_USR_CHG_CONFIG1:
+ case EVSEL__CONFIG_TERM_USR_CHG_CONFIG2:
+ case EVSEL__CONFIG_TERM_USR_CHG_CONFIG3:
+ case EVSEL__CONFIG_TERM_USR_CHG_CONFIG4:
break;
case EVSEL__CONFIG_TERM_RATIO_TO_PREV:
rtp_buf = term->val.str;
@@ -1326,7 +1330,8 @@ void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,
u64 val)
{
u64 user_bits = 0, bits;
- struct evsel_config_term *term = evsel__get_config_term(evsel, CFG_CHG);
+ struct evsel_config_term *term = evsel__get_config_term(evsel,
+ USR_CHG_CONFIG);
if (term)
user_bits = term->val.cfg_chg;
diff --git a/tools/perf/util/evsel_config.h b/tools/perf/util/evsel_config.h
index 685fd8d5c4a8..7b565d76c0bc 100644
--- a/tools/perf/util/evsel_config.h
+++ b/tools/perf/util/evsel_config.h
@@ -27,7 +27,11 @@ enum evsel_term_type {
EVSEL__CONFIG_TERM_AUX_OUTPUT,
EVSEL__CONFIG_TERM_AUX_ACTION,
EVSEL__CONFIG_TERM_AUX_SAMPLE_SIZE,
- EVSEL__CONFIG_TERM_CFG_CHG,
+ EVSEL__CONFIG_TERM_USR_CHG_CONFIG,
+ EVSEL__CONFIG_TERM_USR_CHG_CONFIG1,
+ EVSEL__CONFIG_TERM_USR_CHG_CONFIG2,
+ EVSEL__CONFIG_TERM_USR_CHG_CONFIG3,
+ EVSEL__CONFIG_TERM_USR_CHG_CONFIG4,
EVSEL__CONFIG_TERM_RATIO_TO_PREV,
};
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 46422286380f..1f6e2213326d 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1253,66 +1253,32 @@ static int get_config_terms(const struct parse_events_terms *head_config,
return 0;
}
-/*
- * Add EVSEL__CONFIG_TERM_CFG_CHG where cfg_chg will have a bit set for
- * each bit of attr->config that the user has changed.
- */
-static int get_config_chgs(struct perf_pmu *pmu, struct parse_events_terms *head_config,
- struct list_head *head_terms)
+static int add_cfg_chg(const struct perf_pmu *pmu,
+ const struct parse_events_terms *head_config,
+ struct list_head *head_terms,
+ int format_type,
+ enum parse_events__term_type term_type,
+ enum evsel_term_type new_term_type)
{
struct parse_events_term *term;
u64 bits = 0;
int type;
list_for_each_entry(term, &head_config->terms, list) {
- switch (term->type_term) {
- case PARSE_EVENTS__TERM_TYPE_USER:
+ if (term->type_term == PARSE_EVENTS__TERM_TYPE_USER) {
type = perf_pmu__format_type(pmu, term->config);
- if (type != PERF_PMU_FORMAT_VALUE_CONFIG)
+ if (type != format_type)
continue;
bits |= perf_pmu__format_bits(pmu, term->config);
- break;
- case PARSE_EVENTS__TERM_TYPE_CONFIG:
+ } else if (term->type_term == term_type) {
bits = ~(u64)0;
- break;
- case PARSE_EVENTS__TERM_TYPE_CONFIG1:
- case PARSE_EVENTS__TERM_TYPE_CONFIG2:
- case PARSE_EVENTS__TERM_TYPE_CONFIG3:
- case PARSE_EVENTS__TERM_TYPE_CONFIG4:
- case PARSE_EVENTS__TERM_TYPE_LEGACY_HARDWARE_CONFIG:
- case PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE_CONFIG:
- case PARSE_EVENTS__TERM_TYPE_NAME:
- case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
- case PARSE_EVENTS__TERM_TYPE_SAMPLE_FREQ:
- case PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE:
- case PARSE_EVENTS__TERM_TYPE_TIME:
- case PARSE_EVENTS__TERM_TYPE_CALLGRAPH:
- case PARSE_EVENTS__TERM_TYPE_STACKSIZE:
- case PARSE_EVENTS__TERM_TYPE_NOINHERIT:
- case PARSE_EVENTS__TERM_TYPE_INHERIT:
- case PARSE_EVENTS__TERM_TYPE_MAX_STACK:
- case PARSE_EVENTS__TERM_TYPE_MAX_EVENTS:
- case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:
- case PARSE_EVENTS__TERM_TYPE_OVERWRITE:
- case PARSE_EVENTS__TERM_TYPE_DRV_CFG:
- case PARSE_EVENTS__TERM_TYPE_PERCORE:
- case PARSE_EVENTS__TERM_TYPE_AUX_OUTPUT:
- case PARSE_EVENTS__TERM_TYPE_AUX_ACTION:
- case PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE:
- case PARSE_EVENTS__TERM_TYPE_METRIC_ID:
- case PARSE_EVENTS__TERM_TYPE_RAW:
- case PARSE_EVENTS__TERM_TYPE_CPU:
- case PARSE_EVENTS__TERM_TYPE_RATIO_TO_PREV:
- default:
- break;
}
}
if (bits) {
struct evsel_config_term *new_term;
- new_term = add_config_term(EVSEL__CONFIG_TERM_CFG_CHG,
- head_terms, false);
+ new_term = add_config_term(new_term_type, head_terms, false);
if (!new_term)
return -ENOMEM;
new_term->val.cfg_chg = bits;
@@ -1321,6 +1287,50 @@ static int get_config_chgs(struct perf_pmu *pmu, struct parse_events_terms *head
return 0;
}
+/*
+ * Add EVSEL__CONFIG_TERM_USR_CFG_CONFIGn where cfg_chg will have a bit set for
+ * each bit of attr->configN that the user has changed.
+ */
+static int get_config_chgs(const struct perf_pmu *pmu,
+ const struct parse_events_terms *head_config,
+ struct list_head *head_terms)
+{
+ int ret;
+
+ ret = add_cfg_chg(pmu, head_config, head_terms,
+ PERF_PMU_FORMAT_VALUE_CONFIG,
+ PARSE_EVENTS__TERM_TYPE_CONFIG,
+ EVSEL__CONFIG_TERM_USR_CHG_CONFIG);
+ if (ret)
+ return ret;
+
+ ret = add_cfg_chg(pmu, head_config, head_terms,
+ PERF_PMU_FORMAT_VALUE_CONFIG1,
+ PARSE_EVENTS__TERM_TYPE_CONFIG1,
+ EVSEL__CONFIG_TERM_USR_CHG_CONFIG1);
+ if (ret)
+ return ret;
+
+ ret = add_cfg_chg(pmu, head_config, head_terms,
+ PERF_PMU_FORMAT_VALUE_CONFIG2,
+ PARSE_EVENTS__TERM_TYPE_CONFIG2,
+ EVSEL__CONFIG_TERM_USR_CHG_CONFIG2);
+ if (ret)
+ return ret;
+
+ ret = add_cfg_chg(pmu, head_config, head_terms,
+ PERF_PMU_FORMAT_VALUE_CONFIG3,
+ PARSE_EVENTS__TERM_TYPE_CONFIG3,
+ EVSEL__CONFIG_TERM_USR_CHG_CONFIG3);
+ if (ret)
+ return ret;
+
+ return add_cfg_chg(pmu, head_config, head_terms,
+ PERF_PMU_FORMAT_VALUE_CONFIG4,
+ PARSE_EVENTS__TERM_TYPE_CONFIG4,
+ EVSEL__CONFIG_TERM_USR_CHG_CONFIG4);
+}
+
int parse_events_add_tracepoint(struct parse_events_state *parse_state,
struct list_head *list,
const char *sys, const char *event,
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 599fdf5e28ba..2fe92b4b6e02 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1374,7 +1374,7 @@ pmu_find_format(const struct list_head *formats, const char *name)
return NULL;
}
-__u64 perf_pmu__format_bits(struct perf_pmu *pmu, const char *name)
+__u64 perf_pmu__format_bits(const struct perf_pmu *pmu, const char *name)
{
struct perf_pmu_format *format = pmu_find_format(&pmu->format, name);
__u64 bits = 0;
@@ -1389,7 +1389,7 @@ __u64 perf_pmu__format_bits(struct perf_pmu *pmu, const char *name)
return bits;
}
-int perf_pmu__format_type(struct perf_pmu *pmu, const char *name)
+int perf_pmu__format_type(const struct perf_pmu *pmu, const char *name)
{
struct perf_pmu_format *format = pmu_find_format(&pmu->format, name);
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index e34db47ae5ef..7bf5b23d0ea1 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -247,8 +247,8 @@ int perf_pmu__config_terms(const struct perf_pmu *pmu,
struct parse_events_terms *terms,
bool zero, bool apply_hardcoded,
struct parse_events_error *error);
-__u64 perf_pmu__format_bits(struct perf_pmu *pmu, const char *name);
-int perf_pmu__format_type(struct perf_pmu *pmu, const char *name);
+__u64 perf_pmu__format_bits(const struct perf_pmu *pmu, const char *name);
+int perf_pmu__format_type(const struct perf_pmu *pmu, const char *name);
int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_terms,
struct perf_pmu_info *info, bool *rewrote_terms,
u64 *alternate_hw_config, struct parse_events_error *err);
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v4 06/14] perf evsel: apply evsel__set_config_if_unset() to all config fields
2025-12-22 15:14 [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
` (4 preceding siblings ...)
2025-12-22 15:14 ` [PATCH v4 05/14] perf parse-events: Track all user changed config bits James Clark
@ 2025-12-22 15:14 ` James Clark
2025-12-22 15:14 ` [PATCH v4 07/14] perf evsel: Add a helper to get the value of a config field James Clark
` (9 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: James Clark @ 2025-12-22 15:14 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan
Cc: linux-perf-users, linux-kernel, coresight, linux-arm-kernel,
James Clark
Misleadingly, evsel__set_config_if_unset() only works with the config
field and not config1, config2, etc. This is fine at the moment because
all users of it happen to operate on bits that are in that config field.
Fix it before there are any new users of the function which operate on
bits in different config fields.
In theory it's also possible for a driver to move an existing bit to
another config field and this fixes that scenario too, although this
hasn't happened yet either.
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/perf/util/evsel.c | 37 ++++++++++++++++++++++++++++++++++---
tools/perf/util/pmu.c | 29 ++---------------------------
tools/perf/util/pmu.h | 27 +++++++++++++++++++++++++++
3 files changed, 63 insertions(+), 30 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 354e28e94523..b863096ed568 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1330,8 +1330,39 @@ void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,
u64 val)
{
u64 user_bits = 0, bits;
- struct evsel_config_term *term = evsel__get_config_term(evsel,
- USR_CHG_CONFIG);
+ struct evsel_config_term *term;
+ struct perf_pmu_format *format = pmu_find_format(&evsel->pmu->format,
+ config_name);
+ __u64 *vp;
+
+ if (!format)
+ return;
+
+ switch (format->value) {
+ case PERF_PMU_FORMAT_VALUE_CONFIG:
+ term = evsel__get_config_term(evsel, USR_CHG_CONFIG);
+ vp = &evsel->core.attr.config;
+ break;
+ case PERF_PMU_FORMAT_VALUE_CONFIG1:
+ term = evsel__get_config_term(evsel, USR_CHG_CONFIG1);
+ vp = &evsel->core.attr.config1;
+ break;
+ case PERF_PMU_FORMAT_VALUE_CONFIG2:
+ term = evsel__get_config_term(evsel, USR_CHG_CONFIG2);
+ vp = &evsel->core.attr.config2;
+ break;
+ case PERF_PMU_FORMAT_VALUE_CONFIG3:
+ term = evsel__get_config_term(evsel, USR_CHG_CONFIG3);
+ vp = &evsel->core.attr.config3;
+ break;
+ case PERF_PMU_FORMAT_VALUE_CONFIG4:
+ term = evsel__get_config_term(evsel, USR_CHG_CONFIG4);
+ vp = &evsel->core.attr.config4;
+ break;
+ default:
+ pr_err("Unknown format value: %d\n", format->value);
+ return;
+ }
if (term)
user_bits = term->val.cfg_chg;
@@ -1343,7 +1374,7 @@ void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,
return;
/* Otherwise replace it */
- pmu_format_value(&bits, val, &evsel->core.attr.config, /*zero=*/true);
+ pmu_format_value(&bits, val, vp, /*zero=*/true);
}
void __weak arch_evsel__set_sample_weight(struct evsel *evsel)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 2fe92b4b6e02..dc5dab69151f 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -118,31 +118,6 @@ struct perf_pmu_alias {
bool info_loaded;
};
-/**
- * struct perf_pmu_format - Values from a format file read from
- * <sysfs>/devices/cpu/format/ held in struct perf_pmu.
- *
- * For example, the contents of <sysfs>/devices/cpu/format/event may be
- * "config:0-7" and will be represented here as name="event",
- * value=PERF_PMU_FORMAT_VALUE_CONFIG and bits 0 to 7 will be set.
- */
-struct perf_pmu_format {
- /** @list: Element on list within struct perf_pmu. */
- struct list_head list;
- /** @bits: Which config bits are set by this format value. */
- DECLARE_BITMAP(bits, PERF_PMU_FORMAT_BITS);
- /** @name: The modifier/file name. */
- char *name;
- /**
- * @value : Which config value the format relates to. Supported values
- * are from PERF_PMU_FORMAT_VALUE_CONFIG to
- * PERF_PMU_FORMAT_VALUE_CONFIG_END.
- */
- u16 value;
- /** @loaded: Has the contents been loaded/parsed. */
- bool loaded;
-};
-
static int pmu_aliases_parse(struct perf_pmu *pmu);
static struct perf_pmu_format *perf_pmu__new_format(struct list_head *list, char *name)
@@ -1362,8 +1337,8 @@ void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu)
}
}
-static struct perf_pmu_format *
-pmu_find_format(const struct list_head *formats, const char *name)
+struct perf_pmu_format *pmu_find_format(const struct list_head *formats,
+ const char *name)
{
struct perf_pmu_format *format;
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 7bf5b23d0ea1..7655d996090a 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -233,6 +233,31 @@ struct pmu_event_info {
bool deprecated;
};
+/**
+ * struct perf_pmu_format - Values from a format file read from
+ * <sysfs>/devices/cpu/format/ held in struct perf_pmu.
+ *
+ * For example, the contents of <sysfs>/devices/cpu/format/event may be
+ * "config:0-7" and will be represented here as name="event",
+ * value=PERF_PMU_FORMAT_VALUE_CONFIG and bits 0 to 7 will be set.
+ */
+struct perf_pmu_format {
+ /** @list: Element on list within struct perf_pmu. */
+ struct list_head list;
+ /** @bits: Which config bits are set by this format value. */
+ DECLARE_BITMAP(bits, PERF_PMU_FORMAT_BITS);
+ /** @name: The modifier/file name. */
+ char *name;
+ /**
+ * @value : Which config value the format relates to. Supported values
+ * are from PERF_PMU_FORMAT_VALUE_CONFIG to
+ * PERF_PMU_FORMAT_VALUE_CONFIG_END.
+ */
+ u16 value;
+ /** @loaded: Has the contents been loaded/parsed. */
+ bool loaded;
+};
+
typedef int (*pmu_event_callback)(void *state, struct pmu_event_info *info);
typedef int (*pmu_format_callback)(void *state, const char *name, int config,
const unsigned long *bits);
@@ -255,6 +280,8 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_
int perf_pmu__find_event(struct perf_pmu *pmu, const char *event, void *state, pmu_event_callback cb);
void pmu_format_value(unsigned long *format, __u64 value, __u64 *v, bool zero);
+struct perf_pmu_format *pmu_find_format(const struct list_head *formats,
+ const char *name);
void perf_pmu_format__set_value(void *format, int config, unsigned long *bits);
bool perf_pmu__has_format(const struct perf_pmu *pmu, const char *name);
int perf_pmu__for_each_format(struct perf_pmu *pmu, void *state, pmu_format_callback cb);
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v4 07/14] perf evsel: Add a helper to get the value of a config field
2025-12-22 15:14 [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
` (5 preceding siblings ...)
2025-12-22 15:14 ` [PATCH v4 06/14] perf evsel: apply evsel__set_config_if_unset() to all config fields James Clark
@ 2025-12-22 15:14 ` James Clark
2025-12-22 15:14 ` [PATCH v4 08/14] perf parse-events: Always track user config changes James Clark
` (8 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: James Clark @ 2025-12-22 15:14 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan
Cc: linux-perf-users, linux-kernel, coresight, linux-arm-kernel,
James Clark
This will be used by aux PMUs to read an already written value for
configuring their events and for also testing.
Its helper perf_pmu__format_unpack() does the opposite of the existing
pmu_format_value() so rename that one to perf_pmu__format_pack() so it's
clear how they are related.
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/perf/util/evsel.c | 38 +++++++++++++++++++++++++++++++++++++-
tools/perf/util/evsel.h | 2 ++
tools/perf/util/pmu.c | 35 ++++++++++++++++++++++++++++-------
tools/perf/util/pmu.h | 4 +++-
4 files changed, 70 insertions(+), 9 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index b863096ed568..703a6f73d468 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1374,7 +1374,43 @@ void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,
return;
/* Otherwise replace it */
- pmu_format_value(&bits, val, vp, /*zero=*/true);
+ perf_pmu__format_pack(&bits, val, vp, /*zero=*/true);
+}
+
+
+int evsel__get_config_val(const struct evsel *evsel, const char *config_name,
+ u64 *val)
+{
+ struct perf_pmu_format *format = pmu_find_format(&evsel->pmu->format, config_name);
+ u64 bits = perf_pmu__format_bits(evsel->pmu, config_name);
+
+ if (!format || !bits) {
+ pr_err("Unknown/empty format name: %s\n", config_name);
+ *val = 0;
+ return -EINVAL;
+ }
+
+ switch (format->value) {
+ case PERF_PMU_FORMAT_VALUE_CONFIG:
+ *val = perf_pmu__format_unpack(bits, evsel->core.attr.config);
+ return 0;
+ case PERF_PMU_FORMAT_VALUE_CONFIG1:
+ *val = perf_pmu__format_unpack(bits, evsel->core.attr.config1);
+ return 0;
+ case PERF_PMU_FORMAT_VALUE_CONFIG2:
+ *val = perf_pmu__format_unpack(bits, evsel->core.attr.config2);
+ return 0;
+ case PERF_PMU_FORMAT_VALUE_CONFIG3:
+ *val = perf_pmu__format_unpack(bits, evsel->core.attr.config3);
+ return 0;
+ case PERF_PMU_FORMAT_VALUE_CONFIG4:
+ *val = perf_pmu__format_unpack(bits, evsel->core.attr.config4);
+ return 0;
+ default:
+ pr_err("Unknown format value: %d\n", format->value);
+ *val = 0;
+ return -EINVAL;
+ }
}
void __weak arch_evsel__set_sample_weight(struct evsel *evsel)
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 2cf87bc67df7..95c4bd0f0f2e 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -575,6 +575,8 @@ void evsel__uniquify_counter(struct evsel *counter);
((((src) >> (pos)) & ((1ull << (size)) - 1)) << (63 - ((pos) + (size) - 1)))
u64 evsel__bitfield_swap_branch_flags(u64 value);
+int evsel__get_config_val(const struct evsel *evsel, const char *config_name,
+ u64 *val);
void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,
u64 val);
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index dc5dab69151f..626c7307c8a3 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1337,6 +1337,26 @@ void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu)
}
}
+/*
+ * Unpacks a raw config[n] value using the sparse bitfield that defines a
+ * format attr. For example "config1:1,6-7,44" defines a 4 bit value across non
+ * contiguous bits and this function returns those 4 bits as a value.
+ */
+u64 perf_pmu__format_unpack(u64 format, u64 config_val)
+{
+ int val_bit = 0;
+ u64 res = 0;
+ int fmt_bit;
+
+ for_each_set_bit(fmt_bit, &format, PERF_PMU_FORMAT_BITS) {
+ if (test_bit(fmt_bit, &config_val))
+ res |= BIT_ULL(val_bit);
+
+ val_bit++;
+ }
+ return res;
+}
+
struct perf_pmu_format *pmu_find_format(const struct list_head *formats,
const char *name)
{
@@ -1379,7 +1399,8 @@ int perf_pmu__format_type(const struct perf_pmu *pmu, const char *name)
* Sets value based on the format definition (format parameter)
* and unformatted value (value parameter).
*/
-void pmu_format_value(unsigned long *format, __u64 value, __u64 *v, bool zero)
+void perf_pmu__format_pack(unsigned long *format, __u64 value, __u64 *v,
+ bool zero)
{
unsigned long fbit, vbit;
@@ -1496,23 +1517,23 @@ static int pmu_config_term(const struct perf_pmu *pmu,
switch (term->type_term) {
case PARSE_EVENTS__TERM_TYPE_CONFIG:
assert(term->type_val == PARSE_EVENTS__TERM_TYPE_NUM);
- pmu_format_value(bits, term->val.num, &attr->config, zero);
+ perf_pmu__format_pack(bits, term->val.num, &attr->config, zero);
break;
case PARSE_EVENTS__TERM_TYPE_CONFIG1:
assert(term->type_val == PARSE_EVENTS__TERM_TYPE_NUM);
- pmu_format_value(bits, term->val.num, &attr->config1, zero);
+ perf_pmu__format_pack(bits, term->val.num, &attr->config1, zero);
break;
case PARSE_EVENTS__TERM_TYPE_CONFIG2:
assert(term->type_val == PARSE_EVENTS__TERM_TYPE_NUM);
- pmu_format_value(bits, term->val.num, &attr->config2, zero);
+ perf_pmu__format_pack(bits, term->val.num, &attr->config2, zero);
break;
case PARSE_EVENTS__TERM_TYPE_CONFIG3:
assert(term->type_val == PARSE_EVENTS__TERM_TYPE_NUM);
- pmu_format_value(bits, term->val.num, &attr->config3, zero);
+ perf_pmu__format_pack(bits, term->val.num, &attr->config3, zero);
break;
case PARSE_EVENTS__TERM_TYPE_CONFIG4:
assert(term->type_val == PARSE_EVENTS__TERM_TYPE_NUM);
- pmu_format_value(bits, term->val.num, &attr->config4, zero);
+ perf_pmu__format_pack(bits, term->val.num, &attr->config4, zero);
break;
case PARSE_EVENTS__TERM_TYPE_LEGACY_HARDWARE_CONFIG:
assert(term->type_val == PARSE_EVENTS__TERM_TYPE_NUM);
@@ -1650,7 +1671,7 @@ static int pmu_config_term(const struct perf_pmu *pmu,
*/
}
- pmu_format_value(format->bits, val, vp, zero);
+ perf_pmu__format_pack(format->bits, val, vp, zero);
return 0;
}
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 7655d996090a..a1255b3a8f91 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -279,12 +279,14 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_
u64 *alternate_hw_config, struct parse_events_error *err);
int perf_pmu__find_event(struct perf_pmu *pmu, const char *event, void *state, pmu_event_callback cb);
-void pmu_format_value(unsigned long *format, __u64 value, __u64 *v, bool zero);
+void perf_pmu__format_pack(unsigned long *format, __u64 value, __u64 *v,
+ bool zero);
struct perf_pmu_format *pmu_find_format(const struct list_head *formats,
const char *name);
void perf_pmu_format__set_value(void *format, int config, unsigned long *bits);
bool perf_pmu__has_format(const struct perf_pmu *pmu, const char *name);
int perf_pmu__for_each_format(struct perf_pmu *pmu, void *state, pmu_format_callback cb);
+u64 perf_pmu__format_unpack(u64 format, u64 config_val);
bool is_pmu_core(const char *name);
bool perf_pmu__supports_legacy_cache(const struct perf_pmu *pmu);
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v4 08/14] perf parse-events: Always track user config changes
2025-12-22 15:14 [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
` (6 preceding siblings ...)
2025-12-22 15:14 ` [PATCH v4 07/14] perf evsel: Add a helper to get the value of a config field James Clark
@ 2025-12-22 15:14 ` James Clark
2025-12-22 15:14 ` [PATCH v4 09/14] perf tests: Test evsel__set_config_if_unset() and config change tracking James Clark
` (7 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: James Clark @ 2025-12-22 15:14 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan
Cc: linux-perf-users, linux-kernel, coresight, linux-arm-kernel,
James Clark
Requiring the 'pmu->perf_event_attr_init_default' callback to be set to
track user changes is a bit of a trap to fall in. It's hard to see that
this is required when depending on the user change tracking.
It's possible to want all 0 defaults so not set it, but at the same time
still do some programmatic setting of configs with
evsel__set_config_if_unset(). Also if a PMU reverts to 0 defaults and
deletes its existing callback, it will silently break existing uses of
evsel__set_config_if_unset().
One way to fix this would be to assert in evsel__set_config_if_unset()
if the changes weren't tracked, but that would be a possibly untested
runtime failure. Instead, always track it as it's harmless and
simplifies testing too.
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/perf/util/parse-events.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1f6e2213326d..c8f2962a06c7 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1528,12 +1528,8 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
return -ENOMEM;
}
- /*
- * When using default config, record which bits of attr->config were
- * changed by the user.
- */
- if (pmu->perf_event_attr_init_default &&
- get_config_chgs(pmu, &parsed_terms, &config_terms)) {
+ /* Record which bits of attr->config were changed by the user. */
+ if (get_config_chgs(pmu, &parsed_terms, &config_terms)) {
parse_events_terms__exit(&parsed_terms);
return -ENOMEM;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v4 09/14] perf tests: Test evsel__set_config_if_unset() and config change tracking
2025-12-22 15:14 [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
` (7 preceding siblings ...)
2025-12-22 15:14 ` [PATCH v4 08/14] perf parse-events: Always track user config changes James Clark
@ 2025-12-22 15:14 ` James Clark
2025-12-22 15:14 ` [PATCH v4 10/14] perf cs-etm: Make a helper to find the Coresight evsel James Clark
` (6 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: James Clark @ 2025-12-22 15:14 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan
Cc: linux-perf-users, linux-kernel, coresight, linux-arm-kernel,
James Clark
Test that evsel__set_config_if_unset() behaves as expected. This also
tests the user config change tracking mechanism as it depends on it.
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/perf/tests/pmu.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 91 insertions(+)
diff --git a/tools/perf/tests/pmu.c b/tools/perf/tests/pmu.c
index cbded2c6faa4..0ebf2d7b2cb4 100644
--- a/tools/perf/tests/pmu.c
+++ b/tools/perf/tests/pmu.c
@@ -192,12 +192,102 @@ static int test__pmu_format(struct test_suite *test __maybe_unused, int subtest
}
if (attr.config2 != 0x0400000020041d07) {
pr_err("Unexpected config2 value %llx\n", attr.config2);
+ }
+
+ ret = TEST_OK;
+err_out:
+ parse_events_terms__exit(&terms);
+ test_pmu_put(dir, pmu);
+ return ret;
+}
+
+static int test__pmu_usr_chgs(struct test_suite *test __maybe_unused, int subtest __maybe_unused)
+{
+ const char *event = "perf-pmu-test/config=15,config1=4,krava02=170,"
+ "krava03=1,krava11=27,krava12=1/";
+ struct parse_events_terms terms;
+ struct parse_events_error err;
+ LIST_HEAD(config_terms);
+ struct evlist *evlist;
+ struct perf_pmu *pmu;
+ struct evsel *evsel;
+ int ret = TEST_FAIL;
+ char dir[PATH_MAX];
+ u64 val;
+
+ pmu = test_pmu_get(dir, sizeof(dir));
+ if (!pmu)
+ return TEST_FAIL;
+
+ evlist = evlist__new();
+ if (evlist == NULL) {
+ pr_err("Failed allocation");
+ goto err_out;
+ }
+
+ parse_events_terms__init(&terms);
+ ret = parse_events(evlist, event, &err);
+ if (ret) {
+ pr_debug("failed to parse event '%s', err %d\n", event, ret);
+ parse_events_error__print(&err, event);
+ if (parse_events_error__contains(&err, "can't access trace events"))
+ ret = TEST_SKIP;
goto err_out;
}
+ evsel = evlist__first(evlist);
+
+ /*
+ * Set via config=15, krava01 bits 0-1
+ * Set via config1=4, krava11 bit 1
+ * Set values: krava02=170, krava03=1, krava11=27, krava12=1
+ *
+ * Test that already set values aren't overwritten.
+ */
+ evsel__set_config_if_unset(evsel, "krava01", 16);
+ evsel__get_config_val(evsel, "krava01", &val);
+ TEST_ASSERT_EQUAL("krava01 overwritten", (int) val, (15 & 0b11));
+
+ evsel__set_config_if_unset(evsel, "krava11", 45);
+ evsel__get_config_val(evsel, "krava11", &val);
+ TEST_ASSERT_EQUAL("krava11 overwritten", (int) val, (27 | (4 << 1)));
+
+ evsel__set_config_if_unset(evsel, "krava02", 32);
+ evsel__get_config_val(evsel, "krava02", &val);
+ TEST_ASSERT_EQUAL("krava02 overwritten", (int) val, 170);
+
+ evsel__set_config_if_unset(evsel, "krava03", 0);
+ evsel__get_config_val(evsel, "krava03", &val);
+ TEST_ASSERT_EQUAL("krava03 overwritten", (int) val, 1);
+
+ /*
+ * krava13 doesn't have any bits set by either krava13= or config1=
+ * but setting _any_ raw value for config1 implies that krava13
+ * shouldn't be overwritten. So it's value should remain as 0.
+ */
+ evsel__set_config_if_unset(evsel, "krava13", 5);
+ evsel__get_config_val(evsel, "krava13", &val);
+ TEST_ASSERT_EQUAL("krava13 overwritten", (int) val, 0);
+
+ /*
+ * Unset values: krava21, krava22, krava23
+ *
+ * Test that unset values are overwritten.
+ */
+ evsel__set_config_if_unset(evsel, "krava21", 13905);
+ evsel__get_config_val(evsel, "krava21", &val);
+ TEST_ASSERT_EQUAL("krava21 not overwritten", (int) val, 13905);
+
+ evsel__set_config_if_unset(evsel, "krava22", 11);
+ evsel__get_config_val(evsel, "krava22", &val);
+ TEST_ASSERT_EQUAL("krava22 not overwritten", (int) val, 11);
+ evsel__set_config_if_unset(evsel, "krava23", 0);
+ evsel__get_config_val(evsel, "krava23", &val);
+ TEST_ASSERT_EQUAL("krava23 not overwritten", (int) val, 0);
ret = TEST_OK;
err_out:
parse_events_terms__exit(&terms);
+ evlist__delete(evlist);
test_pmu_put(dir, pmu);
return ret;
}
@@ -539,6 +629,7 @@ static struct test_case tests__pmu[] = {
TEST_CASE("PMU name combining", name_len),
TEST_CASE("PMU name comparison", name_cmp),
TEST_CASE("PMU cmdline match", pmu_match),
+ TEST_CASE("PMU user config changes", pmu_usr_chgs),
{ .name = NULL, }
};
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v4 10/14] perf cs-etm: Make a helper to find the Coresight evsel
2025-12-22 15:14 [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
` (8 preceding siblings ...)
2025-12-22 15:14 ` [PATCH v4 09/14] perf tests: Test evsel__set_config_if_unset() and config change tracking James Clark
@ 2025-12-22 15:14 ` James Clark
2025-12-22 15:14 ` [PATCH v4 11/14] perf cs-etm: Don't use hard coded config bits when setting up ETMCR James Clark
` (5 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: James Clark @ 2025-12-22 15:14 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan
Cc: linux-perf-users, linux-kernel, coresight, linux-arm-kernel,
Leo Yan, James Clark
This pattern occurs a few times and we'll add another one later, so add
a helper function for it.
Reviewed-by: Leo Yan <leo.yan@arm.com>
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/perf/arch/arm/util/cs-etm.c | 50 +++++++++++++++++++++------------------
1 file changed, 27 insertions(+), 23 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index c28208361d91..a49753f0d20f 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -302,6 +302,19 @@ static int cs_etm_set_sink_attr(struct perf_pmu *pmu,
return 0;
}
+static struct evsel *cs_etm_get_evsel(struct evlist *evlist,
+ struct perf_pmu *cs_etm_pmu)
+{
+ struct evsel *evsel;
+
+ evlist__for_each_entry(evlist, evsel) {
+ if (evsel->core.attr.type == cs_etm_pmu->type)
+ return evsel;
+ }
+
+ return NULL;
+}
+
static int cs_etm_recording_options(struct auxtrace_record *itr,
struct evlist *evlist,
struct record_opts *opts)
@@ -473,29 +486,21 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
static u64 cs_etm_get_config(struct auxtrace_record *itr)
{
- u64 config = 0;
struct cs_etm_recording *ptr =
container_of(itr, struct cs_etm_recording, itr);
struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
struct evlist *evlist = ptr->evlist;
- struct evsel *evsel;
+ struct evsel *evsel = cs_etm_get_evsel(evlist, cs_etm_pmu);
- evlist__for_each_entry(evlist, evsel) {
- if (evsel->core.attr.type == cs_etm_pmu->type) {
- /*
- * Variable perf_event_attr::config is assigned to
- * ETMv3/PTM. The bit fields have been made to match
- * the ETMv3.5 ETRMCR register specification. See the
- * PMU_FORMAT_ATTR() declarations in
- * drivers/hwtracing/coresight/coresight-perf.c for
- * details.
- */
- config = evsel->core.attr.config;
- break;
- }
- }
-
- return config;
+ /*
+ * Variable perf_event_attr::config is assigned to
+ * ETMv3/PTM. The bit fields have been made to match
+ * the ETMv3.5 ETRMCR register specification. See the
+ * PMU_FORMAT_ATTR() declarations in
+ * drivers/hwtracing/coresight/coresight-perf.c for
+ * details.
+ */
+ return evsel ? evsel->core.attr.config : 0;
}
#ifndef BIT
@@ -829,12 +834,11 @@ static int cs_etm_snapshot_start(struct auxtrace_record *itr)
{
struct cs_etm_recording *ptr =
container_of(itr, struct cs_etm_recording, itr);
- struct evsel *evsel;
+ struct evsel *evsel = cs_etm_get_evsel(ptr->evlist, ptr->cs_etm_pmu);
+
+ if (evsel)
+ return evsel__disable(evsel);
- evlist__for_each_entry(ptr->evlist, evsel) {
- if (evsel->core.attr.type == ptr->cs_etm_pmu->type)
- return evsel__disable(evsel);
- }
return -EINVAL;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v4 11/14] perf cs-etm: Don't use hard coded config bits when setting up ETMCR
2025-12-22 15:14 [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
` (9 preceding siblings ...)
2025-12-22 15:14 ` [PATCH v4 10/14] perf cs-etm: Make a helper to find the Coresight evsel James Clark
@ 2025-12-22 15:14 ` James Clark
2025-12-22 15:14 ` [PATCH v4 12/14] perf cs-etm: Don't use hard coded config bits when setting up TRCCONFIGR James Clark
` (4 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: James Clark @ 2025-12-22 15:14 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan
Cc: linux-perf-users, linux-kernel, coresight, linux-arm-kernel,
James Clark
Perf only looks at attr.config when determining what was programmed into
ETMCR. These bits could theoretically be in any of the config fields.
Add a generic helper to find the value of any named format field in any
config field and then use it to get the attributes relevant to ETMCR.
The kernel will also stop publishing the ETMCR register bits in a header
[1] so preempt that by defining them here.
Move field_prep() to util.h so we can define it along side field_get().
Unfortunately FIELD_PREP() and FIELD_GET() from the kernel can't be used
as they require the mask to be a compile time constant.
[1]: https://lore.kernel.org/linux-arm-kernel/20251128-james-cs-syncfreq-v8-10-4d319764cc58@linaro.org/
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/perf/arch/arm/util/cs-etm.c | 35 ++++++++++++++++++++++++++++++++++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index a49753f0d20f..f535027ce862 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -68,6 +68,12 @@ static const char * const metadata_ete_ro[] = {
enum cs_etm_version { CS_NOT_PRESENT, CS_ETMV3, CS_ETMV4, CS_ETE };
+
+/* ETMv3 ETMCR register bits */
+#define ETMCR_CYC_ACC BIT(12)
+#define ETMCR_TIMESTAMP_EN BIT(28)
+#define ETMCR_RETURN_STACK BIT(29)
+
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);
@@ -484,6 +490,33 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
return err;
}
+static u64 cs_etm_synth_etmcr(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 evsel *evsel = cs_etm_get_evsel(ptr->evlist, cs_etm_pmu);
+ u64 etmcr = 0;
+ u64 val;
+
+ if (!evsel)
+ return 0;
+
+ /*
+ * Synthesize what the kernel programmed into ETMCR based on
+ * what options the event was opened with. This doesn't have to be
+ * complete or 100% accurate, not all bits used by OpenCSD anyway.
+ */
+ if (!evsel__get_config_val(evsel, "cycacc", &val) && val)
+ etmcr |= ETMCR_CYC_ACC;
+ if (!evsel__get_config_val(evsel, "timestamp", &val) && val)
+ etmcr |= ETMCR_TIMESTAMP_EN;
+ if (!evsel__get_config_val(evsel, "retstack", &val) && val)
+ etmcr |= ETMCR_RETURN_STACK;
+
+ return etmcr;
+}
+
static u64 cs_etm_get_config(struct auxtrace_record *itr)
{
struct cs_etm_recording *ptr =
@@ -743,7 +776,7 @@ static void cs_etm_get_metadata(struct perf_cpu cpu, u32 *offset,
case CS_ETMV3:
magic = __perf_cs_etmv3_magic;
/* Get configuration register */
- info->priv[*offset + CS_ETM_ETMCR] = cs_etm_get_config(itr);
+ info->priv[*offset + CS_ETM_ETMCR] = cs_etm_synth_etmcr(itr);
/* traceID set to legacy value in case new perf running on old system */
info->priv[*offset + CS_ETM_ETMTRACEIDR] = cs_etm_get_legacy_trace_id(cpu);
/* Get read-only information from sysFS */
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v4 12/14] perf cs-etm: Don't use hard coded config bits when setting up TRCCONFIGR
2025-12-22 15:14 [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
` (10 preceding siblings ...)
2025-12-22 15:14 ` [PATCH v4 11/14] perf cs-etm: Don't use hard coded config bits when setting up ETMCR James Clark
@ 2025-12-22 15:14 ` James Clark
2025-12-22 15:14 ` [PATCH v4 13/14] perf cs-etm: Don't hard code config attribute when configuring the event James Clark
` (3 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: James Clark @ 2025-12-22 15:14 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan
Cc: linux-perf-users, linux-kernel, coresight, linux-arm-kernel,
Leo Yan, James Clark
Perf only looks at attr.config when determining what was programmed into
TRCCONFIGR. These bits could theoretically be in any of the config
fields. Use the evsel__get_config_val() helper so it's agnostic to
which config field they are in.
The kernel will also stop publishing the TRCCONFIGR register bits in a
header [1] so preempt that by defining them here.
[1]: https://lore.kernel.org/linux-arm-kernel/20251128-james-cs-syncfreq-v8-10-4d319764cc58@linaro.org/
Reviewed-by: Leo Yan <leo.yan@arm.com>
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/perf/arch/arm/util/cs-etm.c | 79 +++++++++++++++++----------------------
1 file changed, 34 insertions(+), 45 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index f535027ce862..12b28562c2f3 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -68,6 +68,14 @@ static const char * const metadata_ete_ro[] = {
enum cs_etm_version { CS_NOT_PRESENT, CS_ETMV3, CS_ETMV4, CS_ETE };
+/* ETMv4 CONFIGR register bits */
+#define TRCCONFIGR_BB BIT(3)
+#define TRCCONFIGR_CCI BIT(4)
+#define TRCCONFIGR_CID BIT(6)
+#define TRCCONFIGR_VMID BIT(7)
+#define TRCCONFIGR_TS BIT(11)
+#define TRCCONFIGR_RS BIT(12)
+#define TRCCONFIGR_VMIDOPT BIT(15)
/* ETMv3 ETMCR register bits */
#define ETMCR_CYC_ACC BIT(12)
@@ -517,56 +525,37 @@ static u64 cs_etm_synth_etmcr(struct auxtrace_record *itr)
return etmcr;
}
-static u64 cs_etm_get_config(struct auxtrace_record *itr)
+static u64 cs_etmv4_synth_trcconfigr(struct auxtrace_record *itr)
{
+ u64 trcconfigr = 0;
struct cs_etm_recording *ptr =
- container_of(itr, struct cs_etm_recording, itr);
+ container_of(itr, struct cs_etm_recording, itr);
struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
- struct evlist *evlist = ptr->evlist;
- struct evsel *evsel = cs_etm_get_evsel(evlist, cs_etm_pmu);
-
- /*
- * Variable perf_event_attr::config is assigned to
- * ETMv3/PTM. The bit fields have been made to match
- * the ETMv3.5 ETRMCR register specification. See the
- * PMU_FORMAT_ATTR() declarations in
- * drivers/hwtracing/coresight/coresight-perf.c for
- * details.
- */
- return evsel ? evsel->core.attr.config : 0;
-}
-
-#ifndef BIT
-#define BIT(N) (1UL << (N))
-#endif
+ struct evsel *evsel = cs_etm_get_evsel(ptr->evlist, cs_etm_pmu);
+ u64 val;
-static u64 cs_etmv4_get_config(struct auxtrace_record *itr)
-{
- u64 config = 0;
- u64 config_opts = 0;
+ if (!evsel)
+ return 0;
/*
- * The perf event variable config bits represent both
- * the command line options and register programming
- * bits in ETMv3/PTM. For ETMv4 we must remap options
- * to real bits
+ * Synthesize what the kernel programmed into TRCCONFIGR based on
+ * what options the event was opened with. This doesn't have to be
+ * complete or 100% accurate, not all bits used by OpenCSD anyway.
*/
- config_opts = cs_etm_get_config(itr);
- if (config_opts & BIT(ETM_OPT_CYCACC))
- config |= BIT(ETM4_CFG_BIT_CYCACC);
- if (config_opts & BIT(ETM_OPT_CTXTID))
- config |= BIT(ETM4_CFG_BIT_CTXTID);
- if (config_opts & BIT(ETM_OPT_TS))
- config |= BIT(ETM4_CFG_BIT_TS);
- if (config_opts & BIT(ETM_OPT_RETSTK))
- config |= BIT(ETM4_CFG_BIT_RETSTK);
- if (config_opts & BIT(ETM_OPT_CTXTID2))
- config |= BIT(ETM4_CFG_BIT_VMID) |
- BIT(ETM4_CFG_BIT_VMID_OPT);
- if (config_opts & BIT(ETM_OPT_BRANCH_BROADCAST))
- config |= BIT(ETM4_CFG_BIT_BB);
-
- return config;
+ if (!evsel__get_config_val(evsel, "cycacc", &val) && val)
+ trcconfigr |= TRCCONFIGR_CCI;
+ if (!evsel__get_config_val(evsel, "contextid1", &val) && val)
+ trcconfigr |= TRCCONFIGR_CID;
+ if (!evsel__get_config_val(evsel, "timestamp", &val) && val)
+ trcconfigr |= TRCCONFIGR_TS;
+ if (!evsel__get_config_val(evsel, "retstack", &val) && val)
+ trcconfigr |= TRCCONFIGR_RS;
+ if (!evsel__get_config_val(evsel, "contextid2", &val) && val)
+ trcconfigr |= TRCCONFIGR_VMID | TRCCONFIGR_VMIDOPT;
+ if (!evsel__get_config_val(evsel, "branch_broadcast", &val) && val)
+ trcconfigr |= TRCCONFIGR_BB;
+
+ return trcconfigr;
}
static size_t
@@ -688,7 +677,7 @@ static void cs_etm_save_etmv4_header(__u64 data[], struct auxtrace_record *itr,
struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
/* Get trace configuration register */
- data[CS_ETMV4_TRCCONFIGR] = cs_etmv4_get_config(itr);
+ data[CS_ETMV4_TRCCONFIGR] = cs_etmv4_synth_trcconfigr(itr);
/* traceID set to legacy version, in case new perf running on older system */
data[CS_ETMV4_TRCTRACEIDR] = cs_etm_get_legacy_trace_id(cpu);
@@ -720,7 +709,7 @@ static void cs_etm_save_ete_header(__u64 data[], struct auxtrace_record *itr, st
struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
/* Get trace configuration register */
- data[CS_ETE_TRCCONFIGR] = cs_etmv4_get_config(itr);
+ data[CS_ETE_TRCCONFIGR] = cs_etmv4_synth_trcconfigr(itr);
/* traceID set to legacy version, in case new perf running on older system */
data[CS_ETE_TRCTRACEIDR] = cs_etm_get_legacy_trace_id(cpu);
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v4 13/14] perf cs-etm: Don't hard code config attribute when configuring the event
2025-12-22 15:14 [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
` (11 preceding siblings ...)
2025-12-22 15:14 ` [PATCH v4 12/14] perf cs-etm: Don't use hard coded config bits when setting up TRCCONFIGR James Clark
@ 2025-12-22 15:14 ` James Clark
2025-12-22 15:14 ` [PATCH v4 14/14] perf arm-spe: Don't hard code config attribute James Clark
` (2 subsequent siblings)
15 siblings, 0 replies; 24+ messages in thread
From: James Clark @ 2025-12-22 15:14 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan
Cc: linux-perf-users, linux-kernel, coresight, linux-arm-kernel,
James Clark
These instances of hard coded config attributes are used for configuring
and validating the event options. Use the config attribute that's
published by the driver by replacing the open coded operations with
evsel__get_config_val() and evsel__set_config_if_unset().
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/perf/arch/arm/util/cs-etm.c | 56 +++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 29 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index 12b28562c2f3..dc3f4e86b075 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -103,13 +103,14 @@ static int cs_etm_validate_context_id(struct perf_pmu *cs_etm_pmu, struct evsel
struct perf_cpu cpu)
{
int err;
- __u64 val;
- u64 contextid = evsel->core.attr.config &
- (perf_pmu__format_bits(cs_etm_pmu, "contextid") |
- perf_pmu__format_bits(cs_etm_pmu, "contextid1") |
- perf_pmu__format_bits(cs_etm_pmu, "contextid2"));
+ u64 ctxt, ctxt1, ctxt2;
+ __u64 trcidr2;
- if (!contextid)
+ evsel__get_config_val(evsel, "contextid", &ctxt);
+ evsel__get_config_val(evsel, "contextid1", &ctxt1);
+ evsel__get_config_val(evsel, "contextid2", &ctxt2);
+
+ if (!ctxt && !ctxt1 && !ctxt2)
return 0;
/* Not supported in etmv3 */
@@ -120,12 +121,11 @@ static int cs_etm_validate_context_id(struct perf_pmu *cs_etm_pmu, struct evsel
}
/* Get a handle on TRCIDR2 */
- err = cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR2], &val);
+ err = cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR2], &trcidr2);
if (err)
return err;
- if (contextid &
- perf_pmu__format_bits(cs_etm_pmu, "contextid1")) {
+ if (ctxt1) {
/*
* TRCIDR2.CIDSIZE, bit [9-5], indicates whether contextID
* tracing is supported:
@@ -133,15 +133,14 @@ static int cs_etm_validate_context_id(struct perf_pmu *cs_etm_pmu, struct evsel
* 0b00100 Maximum of 32-bit Context ID size.
* All other values are reserved.
*/
- if (BMVAL(val, 5, 9) != 0x4) {
+ if (BMVAL(trcidr2, 5, 9) != 0x4) {
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 &
- perf_pmu__format_bits(cs_etm_pmu, "contextid2")) {
+ if (ctxt2) {
/*
* TRCIDR2.VMIDOPT[30:29] != 0 and
* TRCIDR2.VMIDSIZE[14:10] == 0b00100 (32bit virtual contextid)
@@ -149,7 +148,7 @@ static int cs_etm_validate_context_id(struct perf_pmu *cs_etm_pmu, struct evsel
* virtual context id is < 32bit.
* Any value of VMIDSIZE >= 4 (i.e, > 32bit) is fine for us.
*/
- if (!BMVAL(val, 29, 30) || BMVAL(val, 10, 14) < 4) {
+ if (!BMVAL(trcidr2, 29, 30) || BMVAL(trcidr2, 10, 14) < 4) {
pr_err("%s: CONTEXTIDR_EL2 isn't supported, disable with %s/contextid2=0/\n",
CORESIGHT_ETM_PMU_NAME, CORESIGHT_ETM_PMU_NAME);
return -EINVAL;
@@ -163,10 +162,11 @@ static int cs_etm_validate_timestamp(struct perf_pmu *cs_etm_pmu, struct evsel *
struct perf_cpu cpu)
{
int err;
- __u64 val;
+ u64 val;
+ __u64 trcidr0;
- if (!(evsel->core.attr.config &
- perf_pmu__format_bits(cs_etm_pmu, "timestamp")))
+ evsel__get_config_val(evsel, "timestamp", &val);
+ if (!val)
return 0;
if (cs_etm_get_version(cs_etm_pmu, cpu) == CS_ETMV3) {
@@ -176,7 +176,7 @@ static int cs_etm_validate_timestamp(struct perf_pmu *cs_etm_pmu, struct evsel *
}
/* Get a handle on TRCIRD0 */
- err = cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR0], &val);
+ err = cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR0], &trcidr0);
if (err)
return err;
@@ -187,10 +187,9 @@ static int cs_etm_validate_timestamp(struct perf_pmu *cs_etm_pmu, struct evsel *
* 0b00110 Implementation supports a maximum timestamp of 48bits.
* 0b01000 Implementation supports a maximum timestamp of 64bits.
*/
- val &= GENMASK(28, 24);
- if (!val) {
+ trcidr0 &= GENMASK(28, 24);
+ if (!trcidr0)
return -EINVAL;
- }
return 0;
}
@@ -273,16 +272,19 @@ static int cs_etm_parse_snapshot_options(struct auxtrace_record *itr,
return 0;
}
+/*
+ * If the sink name format "@sink_name" is used, lookup the sink by name to convert to
+ * "sinkid=sink_hash" format. If the user has already manually provided a hash then
+ * "sinkid" isn't overwritten. If neither are provided then the driver will pick the best
+ * sink.
+ */
static int cs_etm_set_sink_attr(struct perf_pmu *pmu,
struct evsel *evsel)
{
char msg[BUFSIZ], path[PATH_MAX], *sink;
struct evsel_config_term *term;
- int ret = -EINVAL;
u32 hash;
-
- if (evsel->core.attr.config2 & GENMASK(31, 0))
- return 0;
+ int ret;
list_for_each_entry(term, &evsel->config_terms, list) {
if (term->type != EVSEL__CONFIG_TERM_DRV_CFG)
@@ -305,14 +307,10 @@ static int cs_etm_set_sink_attr(struct perf_pmu *pmu,
return ret;
}
- evsel->core.attr.config2 |= hash;
+ evsel__set_config_if_unset(evsel, "sinkid", hash);
return 0;
}
- /*
- * No sink was provided on the command line - allow the CoreSight
- * system to look for a default
- */
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v4 14/14] perf arm-spe: Don't hard code config attribute
2025-12-22 15:14 [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
` (12 preceding siblings ...)
2025-12-22 15:14 ` [PATCH v4 13/14] perf cs-etm: Don't hard code config attribute when configuring the event James Clark
@ 2025-12-22 15:14 ` James Clark
2026-01-13 20:58 ` [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields Ian Rogers
2026-01-13 21:01 ` Arnaldo Carvalho de Melo
15 siblings, 0 replies; 24+ messages in thread
From: James Clark @ 2025-12-22 15:14 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan
Cc: linux-perf-users, linux-kernel, coresight, linux-arm-kernel,
James Clark
Use the config attribute that's published by the driver instead of
hard coding "attr.config".
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/perf/arch/arm64/util/arm-spe.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index 51014f8bff97..17ced7bbbdda 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -256,7 +256,7 @@ static __u64 arm_spe_pmu__sample_period(const struct perf_pmu *arm_spe_pmu)
static void arm_spe_setup_evsel(struct evsel *evsel, struct perf_cpu_map *cpus)
{
- u64 bit;
+ u64 pa_enable_bit;
evsel->core.attr.freq = 0;
evsel->core.attr.sample_period = arm_spe_pmu__sample_period(evsel->pmu);
@@ -288,9 +288,10 @@ static void arm_spe_setup_evsel(struct evsel *evsel, struct perf_cpu_map *cpus)
* inform that the resulting output's SPE samples contain physical addresses
* where applicable.
*/
- bit = perf_pmu__format_bits(evsel->pmu, "pa_enable");
- if (evsel->core.attr.config & bit)
- evsel__set_sample_bit(evsel, PHYS_ADDR);
+
+ if (!evsel__get_config_val(evsel, "pa_enable", &pa_enable_bit))
+ if (pa_enable_bit)
+ evsel__set_sample_bit(evsel, PHYS_ADDR);
}
static int arm_spe_setup_aux_buffer(struct record_opts *opts)
@@ -397,6 +398,7 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
struct perf_cpu_map *cpus = evlist->core.user_requested_cpus;
bool discard = false;
int err;
+ u64 discard_bit;
sper->evlist = evlist;
@@ -425,9 +427,8 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
evlist__for_each_entry_safe(evlist, tmp, evsel) {
if (evsel__is_aux_event(evsel)) {
arm_spe_setup_evsel(evsel, cpus);
- if (evsel->core.attr.config &
- perf_pmu__format_bits(evsel->pmu, "discard"))
- discard = true;
+ if (!evsel__get_config_val(evsel, "discard", &discard_bit))
+ discard = !!discard_bit;
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v4 01/14] perf parse-events: Refactor get_config_terms() to remove macros
2025-12-22 15:14 ` [PATCH v4 01/14] perf parse-events: Refactor get_config_terms() to remove macros James Clark
@ 2026-01-13 20:53 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 24+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-01-13 20:53 UTC (permalink / raw)
To: James Clark
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Suzuki K Poulose, Mike Leach, John Garry, Will Deacon, Leo Yan,
linux-perf-users, linux-kernel, coresight, linux-arm-kernel
On Mon, Dec 22, 2025 at 03:14:26PM +0000, James Clark wrote:
> The ADD_CONFIG_TERM() macros build the __type argument out of a partial
> EVSEL__CONFIG_TERM_x enum name. This means that they can't be called
> from a function where __type is a variable and it's also impossible to
> grep the codebase to find usages of these enums as they're never typed
> in full.
>
> Fix this by removing the macros and replacing them with an
> add_config_term() function. It seems the main reason these existed in
> the first place was to avoid type punning and to write to a specific
> field in the union, but the same thing can be achieved with a single
> write to a u64 'val' field.
>
> Running the Perf tests with "-fsanitize=undefined -fno-sanitize-recover"
> results in no new issues as a result of this change.
>
> Reviewed-by: Ian Rogers <irogers@google.com>
So I see the tags here, will try applying...
- Arnaldo
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
> tools/perf/util/evsel_config.h | 1 +
> tools/perf/util/parse-events.c | 146 ++++++++++++++++++++++++-----------------
> 2 files changed, 86 insertions(+), 61 deletions(-)
>
> diff --git a/tools/perf/util/evsel_config.h b/tools/perf/util/evsel_config.h
> index bcd3a978f0c4..685fd8d5c4a8 100644
> --- a/tools/perf/util/evsel_config.h
> +++ b/tools/perf/util/evsel_config.h
> @@ -50,6 +50,7 @@ struct evsel_config_term {
> u64 cfg_chg;
> char *str;
> int cpu;
> + u64 val;
> } val;
> bool weak;
> };
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 17c1c36a7bf9..46422286380f 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1116,105 +1116,107 @@ static int config_attr(struct perf_event_attr *attr,
> return 0;
> }
>
> -static int get_config_terms(const struct parse_events_terms *head_config,
> - struct list_head *head_terms)
> +static struct evsel_config_term *add_config_term(enum evsel_term_type type,
> + struct list_head *head_terms,
> + bool weak)
> {
> -#define ADD_CONFIG_TERM(__type, __weak) \
> - struct evsel_config_term *__t; \
> - \
> - __t = zalloc(sizeof(*__t)); \
> - if (!__t) \
> - return -ENOMEM; \
> - \
> - INIT_LIST_HEAD(&__t->list); \
> - __t->type = EVSEL__CONFIG_TERM_ ## __type; \
> - __t->weak = __weak; \
> - list_add_tail(&__t->list, head_terms)
> -
> -#define ADD_CONFIG_TERM_VAL(__type, __name, __val, __weak) \
> -do { \
> - ADD_CONFIG_TERM(__type, __weak); \
> - __t->val.__name = __val; \
> -} while (0)
> + struct evsel_config_term *t;
>
> -#define ADD_CONFIG_TERM_STR(__type, __val, __weak) \
> -do { \
> - ADD_CONFIG_TERM(__type, __weak); \
> - __t->val.str = strdup(__val); \
> - if (!__t->val.str) { \
> - zfree(&__t); \
> - return -ENOMEM; \
> - } \
> - __t->free_str = true; \
> -} while (0)
> + t = zalloc(sizeof(*t));
> + if (!t)
> + return NULL;
> +
> + INIT_LIST_HEAD(&t->list);
> + t->type = type;
> + t->weak = weak;
> + list_add_tail(&t->list, head_terms);
>
> + return t;
> +}
> +
> +static int get_config_terms(const struct parse_events_terms *head_config,
> + struct list_head *head_terms)
> +{
> struct parse_events_term *term;
>
> list_for_each_entry(term, &head_config->terms, list) {
> + struct evsel_config_term *new_term;
> + enum evsel_term_type new_type;
> + bool str_type = false;
> + u64 val;
> +
> switch (term->type_term) {
> case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
> - ADD_CONFIG_TERM_VAL(PERIOD, period, term->val.num, term->weak);
> + new_type = EVSEL__CONFIG_TERM_PERIOD;
> + val = term->val.num;
> break;
> case PARSE_EVENTS__TERM_TYPE_SAMPLE_FREQ:
> - ADD_CONFIG_TERM_VAL(FREQ, freq, term->val.num, term->weak);
> + new_type = EVSEL__CONFIG_TERM_FREQ;
> + val = term->val.num;
> break;
> case PARSE_EVENTS__TERM_TYPE_TIME:
> - ADD_CONFIG_TERM_VAL(TIME, time, term->val.num, term->weak);
> + new_type = EVSEL__CONFIG_TERM_TIME;
> + val = term->val.num;
> break;
> case PARSE_EVENTS__TERM_TYPE_CALLGRAPH:
> - ADD_CONFIG_TERM_STR(CALLGRAPH, term->val.str, term->weak);
> + new_type = EVSEL__CONFIG_TERM_CALLGRAPH;
> + str_type = true;
> break;
> case PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE:
> - ADD_CONFIG_TERM_STR(BRANCH, term->val.str, term->weak);
> + new_type = EVSEL__CONFIG_TERM_BRANCH;
> + str_type = true;
> break;
> case PARSE_EVENTS__TERM_TYPE_STACKSIZE:
> - ADD_CONFIG_TERM_VAL(STACK_USER, stack_user,
> - term->val.num, term->weak);
> + new_type = EVSEL__CONFIG_TERM_STACK_USER;
> + val = term->val.num;
> break;
> case PARSE_EVENTS__TERM_TYPE_INHERIT:
> - ADD_CONFIG_TERM_VAL(INHERIT, inherit,
> - term->val.num ? 1 : 0, term->weak);
> + new_type = EVSEL__CONFIG_TERM_INHERIT;
> + val = term->val.num ? 1 : 0;
> break;
> case PARSE_EVENTS__TERM_TYPE_NOINHERIT:
> - ADD_CONFIG_TERM_VAL(INHERIT, inherit,
> - term->val.num ? 0 : 1, term->weak);
> + new_type = EVSEL__CONFIG_TERM_INHERIT;
> + val = term->val.num ? 0 : 1;
> break;
> case PARSE_EVENTS__TERM_TYPE_MAX_STACK:
> - ADD_CONFIG_TERM_VAL(MAX_STACK, max_stack,
> - term->val.num, term->weak);
> + new_type = EVSEL__CONFIG_TERM_MAX_STACK;
> + val = term->val.num;
> break;
> case PARSE_EVENTS__TERM_TYPE_MAX_EVENTS:
> - ADD_CONFIG_TERM_VAL(MAX_EVENTS, max_events,
> - term->val.num, term->weak);
> + new_type = EVSEL__CONFIG_TERM_MAX_EVENTS;
> + val = term->val.num;
> break;
> case PARSE_EVENTS__TERM_TYPE_OVERWRITE:
> - ADD_CONFIG_TERM_VAL(OVERWRITE, overwrite,
> - term->val.num ? 1 : 0, term->weak);
> + new_type = EVSEL__CONFIG_TERM_OVERWRITE;
> + val = term->val.num ? 1 : 0;
> break;
> case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:
> - ADD_CONFIG_TERM_VAL(OVERWRITE, overwrite,
> - term->val.num ? 0 : 1, term->weak);
> + new_type = EVSEL__CONFIG_TERM_OVERWRITE;
> + val = term->val.num ? 0 : 1;
> break;
> case PARSE_EVENTS__TERM_TYPE_DRV_CFG:
> - ADD_CONFIG_TERM_STR(DRV_CFG, term->val.str, term->weak);
> + new_type = EVSEL__CONFIG_TERM_DRV_CFG;
> + str_type = true;
> break;
> case PARSE_EVENTS__TERM_TYPE_PERCORE:
> - ADD_CONFIG_TERM_VAL(PERCORE, percore,
> - term->val.num ? true : false, term->weak);
> + new_type = EVSEL__CONFIG_TERM_PERCORE;
> + val = term->val.num ? true : false;
> break;
> case PARSE_EVENTS__TERM_TYPE_AUX_OUTPUT:
> - ADD_CONFIG_TERM_VAL(AUX_OUTPUT, aux_output,
> - term->val.num ? 1 : 0, term->weak);
> + new_type = EVSEL__CONFIG_TERM_AUX_OUTPUT;
> + val = term->val.num ? 1 : 0;
> break;
> case PARSE_EVENTS__TERM_TYPE_AUX_ACTION:
> - ADD_CONFIG_TERM_STR(AUX_ACTION, term->val.str, term->weak);
> + new_type = EVSEL__CONFIG_TERM_AUX_ACTION;
> + str_type = true;
> break;
> case PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE:
> - ADD_CONFIG_TERM_VAL(AUX_SAMPLE_SIZE, aux_sample_size,
> - term->val.num, term->weak);
> + new_type = EVSEL__CONFIG_TERM_AUX_SAMPLE_SIZE;
> + val = term->val.num;
> break;
> case PARSE_EVENTS__TERM_TYPE_RATIO_TO_PREV:
> - ADD_CONFIG_TERM_STR(RATIO_TO_PREV, term->val.str, term->weak);
> + new_type = EVSEL__CONFIG_TERM_RATIO_TO_PREV;
> + str_type = true;
> break;
> case PARSE_EVENTS__TERM_TYPE_USER:
> case PARSE_EVENTS__TERM_TYPE_CONFIG:
> @@ -1229,7 +1231,23 @@ do { \
> case PARSE_EVENTS__TERM_TYPE_RAW:
> case PARSE_EVENTS__TERM_TYPE_CPU:
> default:
> - break;
> + /* Don't add a new term for these ones */
> + continue;
> + }
> +
> + new_term = add_config_term(new_type, head_terms, term->weak);
> + if (!new_term)
> + return -ENOMEM;
> +
> + if (str_type) {
> + new_term->val.str = strdup(term->val.str);
> + if (!new_term->val.str) {
> + zfree(&new_term);
> + return -ENOMEM;
> + }
> + new_term->free_str = true;
> + } else {
> + new_term->val.val = val;
> }
> }
> return 0;
> @@ -1290,10 +1308,16 @@ static int get_config_chgs(struct perf_pmu *pmu, struct parse_events_terms *head
> }
> }
>
> - if (bits)
> - ADD_CONFIG_TERM_VAL(CFG_CHG, cfg_chg, bits, false);
> + if (bits) {
> + struct evsel_config_term *new_term;
> +
> + new_term = add_config_term(EVSEL__CONFIG_TERM_CFG_CHG,
> + head_terms, false);
> + if (!new_term)
> + return -ENOMEM;
> + new_term->val.cfg_chg = bits;
> + }
>
> -#undef ADD_CONFIG_TERM
> return 0;
> }
>
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields
2025-12-22 15:14 [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
` (13 preceding siblings ...)
2025-12-22 15:14 ` [PATCH v4 14/14] perf arm-spe: Don't hard code config attribute James Clark
@ 2026-01-13 20:58 ` Ian Rogers
2026-01-13 21:03 ` Arnaldo Carvalho de Melo
2026-01-13 21:01 ` Arnaldo Carvalho de Melo
15 siblings, 1 reply; 24+ messages in thread
From: Ian Rogers @ 2026-01-13 20:58 UTC (permalink / raw)
To: James Clark
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Adrian Hunter, Suzuki K Poulose, Mike Leach, John Garry,
Will Deacon, Leo Yan, linux-perf-users, linux-kernel, coresight,
linux-arm-kernel, Leo Yan
On Mon, Dec 22, 2025 at 7:14 AM James Clark <james.clark@linaro.org> wrote:
>
> The specific config field that an event format attribute is in is
> consistently hard coded, even though the API is supposed to be that the
> driver publishes the config field name. To stop this pattern from being
> copy pasted and causing problems in the future, replace them all with
> calls to a new helper that returns the value that a user set.
>
> This reveals some issues in evsel__set_config_if_unset(). It doesn't
> work with sparse bitfields, which are an unused but documented feature.
> And it also only writes to the attr.config field. To fix it we need to
> start tracking user changes for all config fields and then use existing
> helper functions that support sparse bitfields. Some other refactoring
> was also required and a test was added.
>
> ---
> Changes in v4:
> - Constify some function args (Ian)
> - Move some evsel__* functions to evsel.c and make some pmu.c functions
> public to support this (Ian)
> - Drop pmu arg where it can be fetched from evsel->pmu (Ian)
> - Link to v3: https://lore.kernel.org/r/20251212-james-perf-config-bits-v3-0-aa36a4846776@linaro.org
>
> Changes in v3:
> - Fix uninitialized variable warning on GCC
> - Fix leak of evlist in test
> - Confirm no type punning issues with ubsan (Ian)
> - Link to v2: https://lore.kernel.org/r/20251208-james-perf-config-bits-v2-0-4ac0281993b0@linaro.org
>
> Changes in v2:
> - Remove macros in get_config_chgs() and some other refactoring.
> - Support sparse bitfields in evsel__set_config_if_unset().
> - Always track user changes instead of only when
> 'pmu->perf_event_attr_init_default' is set.
> - Add a test.
> - Don't bail out in cs-etm.c if any format fields are missing (Leo).
> - Rename 'guess' to 'synth' (Mike).
> - Link to v1: https://lore.kernel.org/r/20251201-james-perf-config-bits-v1-0-22ecbbf8007c@linaro.org
>
> ---
> James Clark (14):
> perf parse-events: Refactor get_config_terms() to remove macros
> perf evsel: Refactor evsel__set_config_if_unset() arguments
> perf evsel: Move evsel__* functions to evsel.c
> perf evsel: Support sparse fields in evsel__set_config_if_unset()
> perf parse-events: Track all user changed config bits
> perf evsel: apply evsel__set_config_if_unset() to all config fields
> perf evsel: Add a helper to get the value of a config field
> perf parse-events: Always track user config changes
> perf tests: Test evsel__set_config_if_unset() and config change tracking
> perf cs-etm: Make a helper to find the Coresight evsel
> perf cs-etm: Don't use hard coded config bits when setting up ETMCR
> perf cs-etm: Don't use hard coded config bits when setting up TRCCONFIGR
> perf cs-etm: Don't hard code config attribute when configuring the event
> perf arm-spe: Don't hard code config attribute
Reviewed-by: Ian Rogers <irogers@google.com>
Most patches already have my tag, sending this for the remainder.
Thanks,
Ian
> tools/perf/arch/arm/util/cs-etm.c | 201 +++++++++++++++-------------
> tools/perf/arch/arm64/util/arm-spe.c | 17 +--
> tools/perf/arch/x86/util/intel-pt.c | 3 +-
> tools/perf/tests/pmu.c | 91 +++++++++++++
> tools/perf/util/evsel.c | 112 +++++++++++++++-
> tools/perf/util/evsel.h | 6 +-
> tools/perf/util/evsel_config.h | 7 +-
> tools/perf/util/parse-events.c | 248 ++++++++++++++++++++---------------
> tools/perf/util/pmu.c | 95 ++++----------
> tools/perf/util/pmu.h | 34 ++++-
> 10 files changed, 529 insertions(+), 285 deletions(-)
> ---
> base-commit: cbd41c6d4c26c161a2b0e70ad411d3885ff13507
> change-id: 20251112-james-perf-config-bits-bee7106f0f00
>
> Best regards,
> --
> James Clark <james.clark@linaro.org>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields
2025-12-22 15:14 [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
` (14 preceding siblings ...)
2026-01-13 20:58 ` [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields Ian Rogers
@ 2026-01-13 21:01 ` Arnaldo Carvalho de Melo
15 siblings, 0 replies; 24+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-01-13 21:01 UTC (permalink / raw)
To: James Clark
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Suzuki K Poulose, Mike Leach, John Garry, Will Deacon, Leo Yan,
linux-perf-users, linux-kernel, coresight, linux-arm-kernel,
Leo Yan
On Mon, Dec 22, 2025 at 03:14:25PM +0000, James Clark wrote:
> The specific config field that an event format attribute is in is
> consistently hard coded, even though the API is supposed to be that the
> driver publishes the config field name. To stop this pattern from being
> copy pasted and causing problems in the future, replace them all with
> calls to a new helper that returns the value that a user set.
>
> This reveals some issues in evsel__set_config_if_unset(). It doesn't
> work with sparse bitfields, which are an unused but documented feature.
> And it also only writes to the attr.config field. To fix it we need to
> start tracking user changes for all config fields and then use existing
> helper functions that support sparse bitfields. Some other refactoring
> was also required and a test was added.
Thanks, applied to perf-tools-next,
- Arnaldo
> ---
> Changes in v4:
> - Constify some function args (Ian)
> - Move some evsel__* functions to evsel.c and make some pmu.c functions
> public to support this (Ian)
> - Drop pmu arg where it can be fetched from evsel->pmu (Ian)
> - Link to v3: https://lore.kernel.org/r/20251212-james-perf-config-bits-v3-0-aa36a4846776@linaro.org
>
> Changes in v3:
> - Fix uninitialized variable warning on GCC
> - Fix leak of evlist in test
> - Confirm no type punning issues with ubsan (Ian)
> - Link to v2: https://lore.kernel.org/r/20251208-james-perf-config-bits-v2-0-4ac0281993b0@linaro.org
>
> Changes in v2:
> - Remove macros in get_config_chgs() and some other refactoring.
> - Support sparse bitfields in evsel__set_config_if_unset().
> - Always track user changes instead of only when
> 'pmu->perf_event_attr_init_default' is set.
> - Add a test.
> - Don't bail out in cs-etm.c if any format fields are missing (Leo).
> - Rename 'guess' to 'synth' (Mike).
> - Link to v1: https://lore.kernel.org/r/20251201-james-perf-config-bits-v1-0-22ecbbf8007c@linaro.org
>
> ---
> James Clark (14):
> perf parse-events: Refactor get_config_terms() to remove macros
> perf evsel: Refactor evsel__set_config_if_unset() arguments
> perf evsel: Move evsel__* functions to evsel.c
> perf evsel: Support sparse fields in evsel__set_config_if_unset()
> perf parse-events: Track all user changed config bits
> perf evsel: apply evsel__set_config_if_unset() to all config fields
> perf evsel: Add a helper to get the value of a config field
> perf parse-events: Always track user config changes
> perf tests: Test evsel__set_config_if_unset() and config change tracking
> perf cs-etm: Make a helper to find the Coresight evsel
> perf cs-etm: Don't use hard coded config bits when setting up ETMCR
> perf cs-etm: Don't use hard coded config bits when setting up TRCCONFIGR
> perf cs-etm: Don't hard code config attribute when configuring the event
> perf arm-spe: Don't hard code config attribute
>
> tools/perf/arch/arm/util/cs-etm.c | 201 +++++++++++++++-------------
> tools/perf/arch/arm64/util/arm-spe.c | 17 +--
> tools/perf/arch/x86/util/intel-pt.c | 3 +-
> tools/perf/tests/pmu.c | 91 +++++++++++++
> tools/perf/util/evsel.c | 112 +++++++++++++++-
> tools/perf/util/evsel.h | 6 +-
> tools/perf/util/evsel_config.h | 7 +-
> tools/perf/util/parse-events.c | 248 ++++++++++++++++++++---------------
> tools/perf/util/pmu.c | 95 ++++----------
> tools/perf/util/pmu.h | 34 ++++-
> 10 files changed, 529 insertions(+), 285 deletions(-)
> ---
> base-commit: cbd41c6d4c26c161a2b0e70ad411d3885ff13507
> change-id: 20251112-james-perf-config-bits-bee7106f0f00
>
> Best regards,
> --
> James Clark <james.clark@linaro.org>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields
2026-01-13 20:58 ` [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields Ian Rogers
@ 2026-01-13 21:03 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 24+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-01-13 21:03 UTC (permalink / raw)
To: Ian Rogers
Cc: James Clark, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
Suzuki K Poulose, Mike Leach, John Garry, Will Deacon, Leo Yan,
linux-perf-users, linux-kernel, coresight, linux-arm-kernel,
Leo Yan
On Tue, Jan 13, 2026 at 12:58:16PM -0800, Ian Rogers wrote:
> > perf arm-spe: Don't hard code config attribute
>
> Reviewed-by: Ian Rogers <irogers@google.com>
>
> Most patches already have my tag, sending this for the remainder.
Thanks, will add to the ones missing it.
- Arnaldo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4 02/14] perf evsel: Refactor evsel__set_config_if_unset() arguments
2025-12-22 15:14 ` [PATCH v4 02/14] perf evsel: Refactor evsel__set_config_if_unset() arguments James Clark
@ 2026-01-13 22:13 ` Arnaldo Carvalho de Melo
2026-01-14 12:14 ` James Clark
0 siblings, 1 reply; 24+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-01-13 22:13 UTC (permalink / raw)
To: James Clark
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Suzuki K Poulose, Mike Leach, John Garry, Will Deacon, Leo Yan,
linux-perf-users, linux-kernel, coresight, linux-arm-kernel
On Mon, Dec 22, 2025 at 03:14:27PM +0000, James Clark wrote:
> Make the evsel argument first to match the other evsel__* functions
> and remove the redundant pmu argument, which can be accessed via evsel.
I haven't checked if this is the exactly where this takes place but
should be in this series, 32-bit build is broken:
3: almalinux:9-i386WARNING: image platform (linux/386) does not match the expected platform (linux/amd64)
WARNING: image platform (linux/386) does not match the expected platform (linux/amd64)
21.72 almalinux:9-i386 : FAIL gcc version 11.4.1 20231218 (Red Hat 11.4.1-3) (GCC)
1378 | perf_pmu__format_pack(&bits, val, vp, /*zero=*/true);
| ^~~~~
| |
| u64 * {aka long long unsigned int *}
In file included from util/evsel.h:14,
from util/evsel.c:38:
util/pmu.h:282:43: note: expected ‘long unsigned int *’ but argument is of type ‘u64 *’ {aka ‘long long unsigned int *’}
282 | void perf_pmu__format_pack(unsigned long *format, __u64 value, __u64 *v,
| ~~~~~~~~~~~~~~~^~~~~~
What I have is in perf-tools-next/tmp.perf-tools-next BTW, I'll try and
fix this tomorrow if you don't do it first. :-)
There are some more build problems in other containers/distros, I'll be
reporting as replies to the patches that looks related
- Arnaldo
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
> tools/perf/arch/arm/util/cs-etm.c | 9 +++------
> tools/perf/arch/arm64/util/arm-spe.c | 2 +-
> tools/perf/arch/x86/util/intel-pt.c | 3 +--
> tools/perf/util/evsel.h | 4 ++--
> tools/perf/util/pmu.c | 6 +++---
> 5 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
> index ea891d12f8f4..c28208361d91 100644
> --- a/tools/perf/arch/arm/util/cs-etm.c
> +++ b/tools/perf/arch/arm/util/cs-etm.c
> @@ -441,10 +441,8 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
> * when a context switch happened.
> */
> if (!perf_cpu_map__is_any_cpu_or_is_empty(cpus)) {
> - evsel__set_config_if_unset(cs_etm_pmu, cs_etm_evsel,
> - "timestamp", 1);
> - evsel__set_config_if_unset(cs_etm_pmu, cs_etm_evsel,
> - "contextid", 1);
> + evsel__set_config_if_unset(cs_etm_evsel, "timestamp", 1);
> + evsel__set_config_if_unset(cs_etm_evsel, "contextid", 1);
> }
>
> /*
> @@ -453,8 +451,7 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
> * timestamp tracing.
> */
> if (opts->sample_time_set)
> - evsel__set_config_if_unset(cs_etm_pmu, cs_etm_evsel,
> - "timestamp", 1);
> + evsel__set_config_if_unset(cs_etm_evsel, "timestamp", 1);
>
> /* Add dummy event to keep tracking */
> err = parse_event(evlist, "dummy:u");
> diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
> index d5ec1408d0ae..51014f8bff97 100644
> --- a/tools/perf/arch/arm64/util/arm-spe.c
> +++ b/tools/perf/arch/arm64/util/arm-spe.c
> @@ -274,7 +274,7 @@ static void arm_spe_setup_evsel(struct evsel *evsel, struct perf_cpu_map *cpus)
> */
> if (!perf_cpu_map__is_any_cpu_or_is_empty(cpus)) {
> evsel__set_sample_bit(evsel, CPU);
> - evsel__set_config_if_unset(evsel->pmu, evsel, "ts_enable", 1);
> + evsel__set_config_if_unset(evsel, "ts_enable", 1);
> }
>
> /*
> diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
> index b394ad9cc635..c131a727774f 100644
> --- a/tools/perf/arch/x86/util/intel-pt.c
> +++ b/tools/perf/arch/x86/util/intel-pt.c
> @@ -664,8 +664,7 @@ static int intel_pt_recording_options(struct auxtrace_record *itr,
> return 0;
>
> if (opts->auxtrace_sample_mode)
> - evsel__set_config_if_unset(intel_pt_pmu, intel_pt_evsel,
> - "psb_period", 0);
> + evsel__set_config_if_unset(intel_pt_evsel, "psb_period", 0);
>
> err = intel_pt_validate_config(intel_pt_pmu, intel_pt_evsel);
> if (err)
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index a08130ff2e47..2cf87bc67df7 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -575,8 +575,8 @@ void evsel__uniquify_counter(struct evsel *counter);
> ((((src) >> (pos)) & ((1ull << (size)) - 1)) << (63 - ((pos) + (size) - 1)))
>
> u64 evsel__bitfield_swap_branch_flags(u64 value);
> -void evsel__set_config_if_unset(struct perf_pmu *pmu, struct evsel *evsel,
> - const char *config_name, u64 val);
> +void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,
> + u64 val);
>
> bool evsel__is_offcpu_event(struct evsel *evsel);
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 956ea273c2c7..e87c12946d71 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -1382,8 +1382,8 @@ bool evsel__is_aux_event(const struct evsel *evsel)
> * something to true, pass 1 for val rather than a pre shifted value.
> */
> #define field_prep(_mask, _val) (((_val) << (ffsll(_mask) - 1)) & (_mask))
> -void evsel__set_config_if_unset(struct perf_pmu *pmu, struct evsel *evsel,
> - const char *config_name, u64 val)
> +void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,
> + u64 val)
> {
> u64 user_bits = 0, bits;
> struct evsel_config_term *term = evsel__get_config_term(evsel, CFG_CHG);
> @@ -1391,7 +1391,7 @@ void evsel__set_config_if_unset(struct perf_pmu *pmu, struct evsel *evsel,
> if (term)
> user_bits = term->val.cfg_chg;
>
> - bits = perf_pmu__format_bits(pmu, config_name);
> + bits = perf_pmu__format_bits(evsel->pmu, config_name);
>
> /* Do nothing if the user changed the value */
> if (bits & user_bits)
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4 02/14] perf evsel: Refactor evsel__set_config_if_unset() arguments
2026-01-13 22:13 ` Arnaldo Carvalho de Melo
@ 2026-01-14 12:14 ` James Clark
2026-01-14 13:33 ` James Clark
2026-01-14 15:47 ` Arnaldo Carvalho de Melo
0 siblings, 2 replies; 24+ messages in thread
From: James Clark @ 2026-01-14 12:14 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Suzuki K Poulose, Mike Leach, John Garry, Will Deacon, Leo Yan,
linux-perf-users, linux-kernel, coresight, linux-arm-kernel
On 13/01/2026 10:13 pm, Arnaldo Carvalho de Melo wrote:
> On Mon, Dec 22, 2025 at 03:14:27PM +0000, James Clark wrote:
>> Make the evsel argument first to match the other evsel__* functions
>> and remove the redundant pmu argument, which can be accessed via evsel.
>
> I haven't checked if this is the exactly where this takes place but
> should be in this series, 32-bit build is broken:
>
> 3: almalinux:9-i386WARNING: image platform (linux/386) does not match the expected platform (linux/amd64)
> WARNING: image platform (linux/386) does not match the expected platform (linux/amd64)
> 21.72 almalinux:9-i386 : FAIL gcc version 11.4.1 20231218 (Red Hat 11.4.1-3) (GCC)
> 1378 | perf_pmu__format_pack(&bits, val, vp, /*zero=*/true);
> | ^~~~~
> | |
> | u64 * {aka long long unsigned int *}
> In file included from util/evsel.h:14,
> from util/evsel.c:38:
> util/pmu.h:282:43: note: expected ‘long unsigned int *’ but argument is of type ‘u64 *’ {aka ‘long long unsigned int *’}
> 282 | void perf_pmu__format_pack(unsigned long *format, __u64 value, __u64 *v,
> | ~~~~~~~~~~~~~~~^~~~~~
>
>
> What I have is in perf-tools-next/tmp.perf-tools-next BTW, I'll try and
> fix this tomorrow if you don't do it first. :-)
Taking a look, but I'm wondering if this is already not working
properly. There are existing "unsigned long"s in pmu.c that operate on
the config bits which is what I copied.
On this target an unsigned long is 32bits but struct
perf_event_attr->configs are __u64. So it looks like it might leave the
top bits unset sometimes.
I'll look at a fix for that which should fix the compilation error at
the same time.
Another question is, do we actually care about this platform?
>
> There are some more build problems in other containers/distros, I'll be
> reporting as replies to the patches that looks related
>
> - Arnaldo
>
>> Signed-off-by: James Clark <james.clark@linaro.org>
>> ---
>> tools/perf/arch/arm/util/cs-etm.c | 9 +++------
>> tools/perf/arch/arm64/util/arm-spe.c | 2 +-
>> tools/perf/arch/x86/util/intel-pt.c | 3 +--
>> tools/perf/util/evsel.h | 4 ++--
>> tools/perf/util/pmu.c | 6 +++---
>> 5 files changed, 10 insertions(+), 14 deletions(-)
>>
>> diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
>> index ea891d12f8f4..c28208361d91 100644
>> --- a/tools/perf/arch/arm/util/cs-etm.c
>> +++ b/tools/perf/arch/arm/util/cs-etm.c
>> @@ -441,10 +441,8 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
>> * when a context switch happened.
>> */
>> if (!perf_cpu_map__is_any_cpu_or_is_empty(cpus)) {
>> - evsel__set_config_if_unset(cs_etm_pmu, cs_etm_evsel,
>> - "timestamp", 1);
>> - evsel__set_config_if_unset(cs_etm_pmu, cs_etm_evsel,
>> - "contextid", 1);
>> + evsel__set_config_if_unset(cs_etm_evsel, "timestamp", 1);
>> + evsel__set_config_if_unset(cs_etm_evsel, "contextid", 1);
>> }
>>
>> /*
>> @@ -453,8 +451,7 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
>> * timestamp tracing.
>> */
>> if (opts->sample_time_set)
>> - evsel__set_config_if_unset(cs_etm_pmu, cs_etm_evsel,
>> - "timestamp", 1);
>> + evsel__set_config_if_unset(cs_etm_evsel, "timestamp", 1);
>>
>> /* Add dummy event to keep tracking */
>> err = parse_event(evlist, "dummy:u");
>> diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
>> index d5ec1408d0ae..51014f8bff97 100644
>> --- a/tools/perf/arch/arm64/util/arm-spe.c
>> +++ b/tools/perf/arch/arm64/util/arm-spe.c
>> @@ -274,7 +274,7 @@ static void arm_spe_setup_evsel(struct evsel *evsel, struct perf_cpu_map *cpus)
>> */
>> if (!perf_cpu_map__is_any_cpu_or_is_empty(cpus)) {
>> evsel__set_sample_bit(evsel, CPU);
>> - evsel__set_config_if_unset(evsel->pmu, evsel, "ts_enable", 1);
>> + evsel__set_config_if_unset(evsel, "ts_enable", 1);
>> }
>>
>> /*
>> diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
>> index b394ad9cc635..c131a727774f 100644
>> --- a/tools/perf/arch/x86/util/intel-pt.c
>> +++ b/tools/perf/arch/x86/util/intel-pt.c
>> @@ -664,8 +664,7 @@ static int intel_pt_recording_options(struct auxtrace_record *itr,
>> return 0;
>>
>> if (opts->auxtrace_sample_mode)
>> - evsel__set_config_if_unset(intel_pt_pmu, intel_pt_evsel,
>> - "psb_period", 0);
>> + evsel__set_config_if_unset(intel_pt_evsel, "psb_period", 0);
>>
>> err = intel_pt_validate_config(intel_pt_pmu, intel_pt_evsel);
>> if (err)
>> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
>> index a08130ff2e47..2cf87bc67df7 100644
>> --- a/tools/perf/util/evsel.h
>> +++ b/tools/perf/util/evsel.h
>> @@ -575,8 +575,8 @@ void evsel__uniquify_counter(struct evsel *counter);
>> ((((src) >> (pos)) & ((1ull << (size)) - 1)) << (63 - ((pos) + (size) - 1)))
>>
>> u64 evsel__bitfield_swap_branch_flags(u64 value);
>> -void evsel__set_config_if_unset(struct perf_pmu *pmu, struct evsel *evsel,
>> - const char *config_name, u64 val);
>> +void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,
>> + u64 val);
>>
>> bool evsel__is_offcpu_event(struct evsel *evsel);
>>
>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
>> index 956ea273c2c7..e87c12946d71 100644
>> --- a/tools/perf/util/pmu.c
>> +++ b/tools/perf/util/pmu.c
>> @@ -1382,8 +1382,8 @@ bool evsel__is_aux_event(const struct evsel *evsel)
>> * something to true, pass 1 for val rather than a pre shifted value.
>> */
>> #define field_prep(_mask, _val) (((_val) << (ffsll(_mask) - 1)) & (_mask))
>> -void evsel__set_config_if_unset(struct perf_pmu *pmu, struct evsel *evsel,
>> - const char *config_name, u64 val)
>> +void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,
>> + u64 val)
>> {
>> u64 user_bits = 0, bits;
>> struct evsel_config_term *term = evsel__get_config_term(evsel, CFG_CHG);
>> @@ -1391,7 +1391,7 @@ void evsel__set_config_if_unset(struct perf_pmu *pmu, struct evsel *evsel,
>> if (term)
>> user_bits = term->val.cfg_chg;
>>
>> - bits = perf_pmu__format_bits(pmu, config_name);
>> + bits = perf_pmu__format_bits(evsel->pmu, config_name);
>>
>> /* Do nothing if the user changed the value */
>> if (bits & user_bits)
>>
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4 02/14] perf evsel: Refactor evsel__set_config_if_unset() arguments
2026-01-14 12:14 ` James Clark
@ 2026-01-14 13:33 ` James Clark
2026-01-14 15:47 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 24+ messages in thread
From: James Clark @ 2026-01-14 13:33 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Suzuki K Poulose, Mike Leach, John Garry, Will Deacon, Leo Yan,
linux-perf-users, linux-kernel, coresight, linux-arm-kernel
On 14/01/2026 12:14 pm, James Clark wrote:
>
>
> On 13/01/2026 10:13 pm, Arnaldo Carvalho de Melo wrote:
>> On Mon, Dec 22, 2025 at 03:14:27PM +0000, James Clark wrote:
>>> Make the evsel argument first to match the other evsel__* functions
>>> and remove the redundant pmu argument, which can be accessed via evsel.
>>
>> I haven't checked if this is the exactly where this takes place but
>> should be in this series, 32-bit build is broken:
>>
>> 3: almalinux:9-i386WARNING: image platform (linux/386) does not
>> match the expected platform (linux/amd64)
>> WARNING: image platform (linux/386) does not match the expected
>> platform (linux/amd64)
>> 21.72 almalinux:9-i386 : FAIL gcc version 11.4.1
>> 20231218 (Red Hat 11.4.1-3) (GCC)
>> 1378 | perf_pmu__format_pack(&bits, val, vp, /*zero=*/
>> true);
>> | ^~~~~
>> | |
>> | u64 * {aka long long
>> unsigned int *}
>> In file included from util/evsel.h:14,
>> from util/evsel.c:38:
>> util/pmu.h:282:43: note: expected ‘long unsigned int *’ but
>> argument is of type ‘u64 *’ {aka ‘long long unsigned int *’}
>> 282 | void perf_pmu__format_pack(unsigned long *format, __u64
>> value, __u64 *v,
>> | ~~~~~~~~~~~~~~~^~~~~~
>>
>>
>> What I have is in perf-tools-next/tmp.perf-tools-next BTW, I'll try and
>> fix this tomorrow if you don't do it first. :-)
>
> Taking a look, but I'm wondering if this is already not working
> properly. There are existing "unsigned long"s in pmu.c that operate on
> the config bits which is what I copied.
>
> On this target an unsigned long is 32bits but struct perf_event_attr-
> >configs are __u64. So it looks like it might leave the top bits unset
> sometimes.
>
> I'll look at a fix for that which should fix the compilation error at
> the same time.
False alarm, I was confusing a pointer to a single unsigned long with
what are actually DECLARE_BITMAP()s. So they're two longs in this case
for 64 bits.
I'll send a fix for the compilation issue and double check the patches
to make sure that misunderstanding hasn't caused any issues.
>
> Another question is, do we actually care about this platform?
>
>>
>> There are some more build problems in other containers/distros, I'll be
>> reporting as replies to the patches that looks related
>>
>> - Arnaldo
>>> Signed-off-by: James Clark <james.clark@linaro.org>
>>> ---
>>> tools/perf/arch/arm/util/cs-etm.c | 9 +++------
>>> tools/perf/arch/arm64/util/arm-spe.c | 2 +-
>>> tools/perf/arch/x86/util/intel-pt.c | 3 +--
>>> tools/perf/util/evsel.h | 4 ++--
>>> tools/perf/util/pmu.c | 6 +++---
>>> 5 files changed, 10 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/
>>> util/cs-etm.c
>>> index ea891d12f8f4..c28208361d91 100644
>>> --- a/tools/perf/arch/arm/util/cs-etm.c
>>> +++ b/tools/perf/arch/arm/util/cs-etm.c
>>> @@ -441,10 +441,8 @@ static int cs_etm_recording_options(struct
>>> auxtrace_record *itr,
>>> * when a context switch happened.
>>> */
>>> if (!perf_cpu_map__is_any_cpu_or_is_empty(cpus)) {
>>> - evsel__set_config_if_unset(cs_etm_pmu, cs_etm_evsel,
>>> - "timestamp", 1);
>>> - evsel__set_config_if_unset(cs_etm_pmu, cs_etm_evsel,
>>> - "contextid", 1);
>>> + evsel__set_config_if_unset(cs_etm_evsel, "timestamp", 1);
>>> + evsel__set_config_if_unset(cs_etm_evsel, "contextid", 1);
>>> }
>>> /*
>>> @@ -453,8 +451,7 @@ static int cs_etm_recording_options(struct
>>> auxtrace_record *itr,
>>> * timestamp tracing.
>>> */
>>> if (opts->sample_time_set)
>>> - evsel__set_config_if_unset(cs_etm_pmu, cs_etm_evsel,
>>> - "timestamp", 1);
>>> + evsel__set_config_if_unset(cs_etm_evsel, "timestamp", 1);
>>> /* Add dummy event to keep tracking */
>>> err = parse_event(evlist, "dummy:u");
>>> diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/
>>> arm64/util/arm-spe.c
>>> index d5ec1408d0ae..51014f8bff97 100644
>>> --- a/tools/perf/arch/arm64/util/arm-spe.c
>>> +++ b/tools/perf/arch/arm64/util/arm-spe.c
>>> @@ -274,7 +274,7 @@ static void arm_spe_setup_evsel(struct evsel
>>> *evsel, struct perf_cpu_map *cpus)
>>> */
>>> if (!perf_cpu_map__is_any_cpu_or_is_empty(cpus)) {
>>> evsel__set_sample_bit(evsel, CPU);
>>> - evsel__set_config_if_unset(evsel->pmu, evsel, "ts_enable", 1);
>>> + evsel__set_config_if_unset(evsel, "ts_enable", 1);
>>> }
>>> /*
>>> diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/
>>> x86/util/intel-pt.c
>>> index b394ad9cc635..c131a727774f 100644
>>> --- a/tools/perf/arch/x86/util/intel-pt.c
>>> +++ b/tools/perf/arch/x86/util/intel-pt.c
>>> @@ -664,8 +664,7 @@ static int intel_pt_recording_options(struct
>>> auxtrace_record *itr,
>>> return 0;
>>> if (opts->auxtrace_sample_mode)
>>> - evsel__set_config_if_unset(intel_pt_pmu, intel_pt_evsel,
>>> - "psb_period", 0);
>>> + evsel__set_config_if_unset(intel_pt_evsel, "psb_period", 0);
>>> err = intel_pt_validate_config(intel_pt_pmu, intel_pt_evsel);
>>> if (err)
>>> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
>>> index a08130ff2e47..2cf87bc67df7 100644
>>> --- a/tools/perf/util/evsel.h
>>> +++ b/tools/perf/util/evsel.h
>>> @@ -575,8 +575,8 @@ void evsel__uniquify_counter(struct evsel *counter);
>>> ((((src) >> (pos)) & ((1ull << (size)) - 1)) << (63 - ((pos) +
>>> (size) - 1)))
>>> u64 evsel__bitfield_swap_branch_flags(u64 value);
>>> -void evsel__set_config_if_unset(struct perf_pmu *pmu, struct evsel
>>> *evsel,
>>> - const char *config_name, u64 val);
>>> +void evsel__set_config_if_unset(struct evsel *evsel, const char
>>> *config_name,
>>> + u64 val);
>>> bool evsel__is_offcpu_event(struct evsel *evsel);
>>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
>>> index 956ea273c2c7..e87c12946d71 100644
>>> --- a/tools/perf/util/pmu.c
>>> +++ b/tools/perf/util/pmu.c
>>> @@ -1382,8 +1382,8 @@ bool evsel__is_aux_event(const struct evsel
>>> *evsel)
>>> * something to true, pass 1 for val rather than a pre shifted value.
>>> */
>>> #define field_prep(_mask, _val) (((_val) << (ffsll(_mask) - 1)) &
>>> (_mask))
>>> -void evsel__set_config_if_unset(struct perf_pmu *pmu, struct evsel
>>> *evsel,
>>> - const char *config_name, u64 val)
>>> +void evsel__set_config_if_unset(struct evsel *evsel, const char
>>> *config_name,
>>> + u64 val)
>>> {
>>> u64 user_bits = 0, bits;
>>> struct evsel_config_term *term = evsel__get_config_term(evsel,
>>> CFG_CHG);
>>> @@ -1391,7 +1391,7 @@ void evsel__set_config_if_unset(struct perf_pmu
>>> *pmu, struct evsel *evsel,
>>> if (term)
>>> user_bits = term->val.cfg_chg;
>>> - bits = perf_pmu__format_bits(pmu, config_name);
>>> + bits = perf_pmu__format_bits(evsel->pmu, config_name);
>>> /* Do nothing if the user changed the value */
>>> if (bits & user_bits)
>>>
>>> --
>>> 2.34.1
>>>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4 02/14] perf evsel: Refactor evsel__set_config_if_unset() arguments
2026-01-14 12:14 ` James Clark
2026-01-14 13:33 ` James Clark
@ 2026-01-14 15:47 ` Arnaldo Carvalho de Melo
2026-01-14 15:58 ` James Clark
1 sibling, 1 reply; 24+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-01-14 15:47 UTC (permalink / raw)
To: James Clark
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Suzuki K Poulose, Mike Leach, John Garry, Will Deacon, Leo Yan,
linux-perf-users, linux-kernel, coresight, linux-arm-kernel
On Wed, Jan 14, 2026 at 12:14:43PM +0000, James Clark wrote:
> On 13/01/2026 10:13 pm, Arnaldo Carvalho de Melo wrote:
> > On Mon, Dec 22, 2025 at 03:14:27PM +0000, James Clark wrote:
> > > Make the evsel argument first to match the other evsel__* functions
> > > and remove the redundant pmu argument, which can be accessed via evsel.
> > I haven't checked if this is the exactly where this takes place but
> > should be in this series, 32-bit build is broken:
> > 3: almalinux:9-i386WARNING: image platform (linux/386) does not match the expected platform (linux/amd64)
> > WARNING: image platform (linux/386) does not match the expected platform (linux/amd64)
> > 21.72 almalinux:9-i386 : FAIL gcc version 11.4.1 20231218 (Red Hat 11.4.1-3) (GCC)
> > 1378 | perf_pmu__format_pack(&bits, val, vp, /*zero=*/true);
> > | ^~~~~
> > | |
> > | u64 * {aka long long unsigned int *}
> > In file included from util/evsel.h:14,
> > from util/evsel.c:38:
> > util/pmu.h:282:43: note: expected ‘long unsigned int *’ but argument is of type ‘u64 *’ {aka ‘long long unsigned int *’}
> > 282 | void perf_pmu__format_pack(unsigned long *format, __u64 value, __u64 *v,
> > | ~~~~~~~~~~~~~~~^~~~~~
> > What I have is in perf-tools-next/tmp.perf-tools-next BTW, I'll try and
> > fix this tomorrow if you don't do it first. :-)
> Taking a look, but I'm wondering if this is already not working properly.
> There are existing "unsigned long"s in pmu.c that operate on the config bits
> which is what I copied.
> On this target an unsigned long is 32bits but struct
> perf_event_attr->configs are __u64. So it looks like it might leave the top
> bits unset sometimes.
> I'll look at a fix for that which should fix the compilation error at the
> same time.
> Another question is, do we actually care about this platform?
It failed for other 32-bit platforms too, so the question is if we care
about 32-bit at all.
- Arnaldo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4 02/14] perf evsel: Refactor evsel__set_config_if_unset() arguments
2026-01-14 15:47 ` Arnaldo Carvalho de Melo
@ 2026-01-14 15:58 ` James Clark
0 siblings, 0 replies; 24+ messages in thread
From: James Clark @ 2026-01-14 15:58 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Suzuki K Poulose, Mike Leach, John Garry, Will Deacon, Leo Yan,
linux-perf-users, linux-kernel, coresight, linux-arm-kernel
On 14/01/2026 3:47 pm, Arnaldo Carvalho de Melo wrote:
> On Wed, Jan 14, 2026 at 12:14:43PM +0000, James Clark wrote:
>> On 13/01/2026 10:13 pm, Arnaldo Carvalho de Melo wrote:
>>> On Mon, Dec 22, 2025 at 03:14:27PM +0000, James Clark wrote:
>>>> Make the evsel argument first to match the other evsel__* functions
>>>> and remove the redundant pmu argument, which can be accessed via evsel.
>
>>> I haven't checked if this is the exactly where this takes place but
>>> should be in this series, 32-bit build is broken:
>
>>> 3: almalinux:9-i386WARNING: image platform (linux/386) does not match the expected platform (linux/amd64)
>>> WARNING: image platform (linux/386) does not match the expected platform (linux/amd64)
>>> 21.72 almalinux:9-i386 : FAIL gcc version 11.4.1 20231218 (Red Hat 11.4.1-3) (GCC)
>>> 1378 | perf_pmu__format_pack(&bits, val, vp, /*zero=*/true);
>>> | ^~~~~
>>> | |
>>> | u64 * {aka long long unsigned int *}
>>> In file included from util/evsel.h:14,
>>> from util/evsel.c:38:
>>> util/pmu.h:282:43: note: expected ‘long unsigned int *’ but argument is of type ‘u64 *’ {aka ‘long long unsigned int *’}
>>> 282 | void perf_pmu__format_pack(unsigned long *format, __u64 value, __u64 *v,
>>> | ~~~~~~~~~~~~~~~^~~~~~
>
>>> What I have is in perf-tools-next/tmp.perf-tools-next BTW, I'll try and
>>> fix this tomorrow if you don't do it first. :-)
>
>> Taking a look, but I'm wondering if this is already not working properly.
>> There are existing "unsigned long"s in pmu.c that operate on the config bits
>> which is what I copied.
>
>> On this target an unsigned long is 32bits but struct
>> perf_event_attr->configs are __u64. So it looks like it might leave the top
>> bits unset sometimes.
>
>> I'll look at a fix for that which should fix the compilation error at the
>> same time.
>
>> Another question is, do we actually care about this platform?
>
> It failed for other 32-bit platforms too, so the question is if we care
> about 32-bit at all.
>
> - Arnaldo
I suppose the answer is we still do then.
I sent another version. A couple of patches were changed a bit where I
used more bitfields instead of converting to u64s which was probably the
right thing to do regardless of the build issue.
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2026-01-14 15:59 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-22 15:14 [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
2025-12-22 15:14 ` [PATCH v4 01/14] perf parse-events: Refactor get_config_terms() to remove macros James Clark
2026-01-13 20:53 ` Arnaldo Carvalho de Melo
2025-12-22 15:14 ` [PATCH v4 02/14] perf evsel: Refactor evsel__set_config_if_unset() arguments James Clark
2026-01-13 22:13 ` Arnaldo Carvalho de Melo
2026-01-14 12:14 ` James Clark
2026-01-14 13:33 ` James Clark
2026-01-14 15:47 ` Arnaldo Carvalho de Melo
2026-01-14 15:58 ` James Clark
2025-12-22 15:14 ` [PATCH v4 03/14] perf evsel: Move evsel__* functions to evsel.c James Clark
2025-12-22 15:14 ` [PATCH v4 04/14] perf evsel: Support sparse fields in evsel__set_config_if_unset() James Clark
2025-12-22 15:14 ` [PATCH v4 05/14] perf parse-events: Track all user changed config bits James Clark
2025-12-22 15:14 ` [PATCH v4 06/14] perf evsel: apply evsel__set_config_if_unset() to all config fields James Clark
2025-12-22 15:14 ` [PATCH v4 07/14] perf evsel: Add a helper to get the value of a config field James Clark
2025-12-22 15:14 ` [PATCH v4 08/14] perf parse-events: Always track user config changes James Clark
2025-12-22 15:14 ` [PATCH v4 09/14] perf tests: Test evsel__set_config_if_unset() and config change tracking James Clark
2025-12-22 15:14 ` [PATCH v4 10/14] perf cs-etm: Make a helper to find the Coresight evsel James Clark
2025-12-22 15:14 ` [PATCH v4 11/14] perf cs-etm: Don't use hard coded config bits when setting up ETMCR James Clark
2025-12-22 15:14 ` [PATCH v4 12/14] perf cs-etm: Don't use hard coded config bits when setting up TRCCONFIGR James Clark
2025-12-22 15:14 ` [PATCH v4 13/14] perf cs-etm: Don't hard code config attribute when configuring the event James Clark
2025-12-22 15:14 ` [PATCH v4 14/14] perf arm-spe: Don't hard code config attribute James Clark
2026-01-13 20:58 ` [PATCH v4 00/14] perf cs-etm/arm-spe: Remove hard coded config fields Ian Rogers
2026-01-13 21:03 ` Arnaldo Carvalho de Melo
2026-01-13 21:01 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox