From c798f72c7a67a4c089142d4b16c8b9587e6db5e6 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 1 Oct 2024 20:20:05 -0700 Subject: [PATCH] perf pmu: Allow hardcoded terms to be applied to attributes Hard coded terms like "config=10" are skipped by perf_pmu__config assuming they were already applied to a perf_event_attr by parse event's config_attr function. When doing a reverse number to name lookup in perf_pmu__name_from_config, as the hardcoded terms aren't applied the config value is incorrect leading to misses or false matches. Fix this by adding a parameter to have perf_pmu__config apply hardcoded terms too (not just in parse event's config_term_common). Signed-off-by: Ian Rogers Acked-by: Namhyung Kim Link: https://lore.kernel.org/r/20241002032016.333748-3-irogers@google.com Signed-off-by: Namhyung Kim --- tools/perf/arch/x86/util/intel-pt.c | 3 +- tools/perf/tests/pmu.c | 3 +- tools/perf/util/parse-events.c | 4 ++- tools/perf/util/pmu.c | 56 ++++++++++++++++++++++++----- tools/perf/util/pmu.h | 4 ++- 5 files changed, 58 insertions(+), 12 deletions(-) diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c index ea510a7486b1..8f235d8b67b6 100644 --- a/tools/perf/arch/x86/util/intel-pt.c +++ b/tools/perf/arch/x86/util/intel-pt.c @@ -75,7 +75,8 @@ static int intel_pt_parse_terms_with_default(const struct perf_pmu *pmu, goto out_free; attr.config = *config; - err = perf_pmu__config_terms(pmu, &attr, &terms, /*zero=*/true, /*err=*/NULL); + err = perf_pmu__config_terms(pmu, &attr, &terms, /*zero=*/true, /*apply_hardcoded=*/false, + /*err=*/NULL); if (err) goto out_free; diff --git a/tools/perf/tests/pmu.c b/tools/perf/tests/pmu.c index be18506f6a24..6a681e3fb552 100644 --- a/tools/perf/tests/pmu.c +++ b/tools/perf/tests/pmu.c @@ -176,7 +176,8 @@ static int test__pmu_format(struct test_suite *test __maybe_unused, int subtest } memset(&attr, 0, sizeof(attr)); - ret = perf_pmu__config_terms(pmu, &attr, &terms, /*zero=*/false, /*err=*/NULL); + ret = perf_pmu__config_terms(pmu, &attr, &terms, /*zero=*/false, + /*apply_hardcoded=*/false, /*err=*/NULL); if (ret) { pr_err("perf_pmu__config_terms failed"); goto err_out; diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index e96cf13dc396..6ae611e70fae 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -1546,7 +1546,9 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state, return -ENOMEM; } - if (perf_pmu__config(pmu, &attr, &parsed_terms, parse_state->error)) { + /* Skip configuring hard coded terms that were applied by config_attr. */ + if (perf_pmu__config(pmu, &attr, &parsed_terms, /*apply_hardcoded=*/false, + parse_state->error)) { free_config_terms(&config_terms); parse_events_terms__exit(&parsed_terms); return -EINVAL; diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index cb2eb2407de5..a1c7e879cb2f 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -1366,7 +1366,8 @@ static int pmu_config_term(const struct perf_pmu *pmu, struct perf_event_attr *attr, struct parse_events_term *term, struct parse_events_terms *head_terms, - bool zero, struct parse_events_error *err) + bool zero, bool apply_hardcoded, + struct parse_events_error *err) { struct perf_pmu_format *format; __u64 *vp; @@ -1380,11 +1381,46 @@ static int pmu_config_term(const struct perf_pmu *pmu, return 0; /* - * Hardcoded terms should be already in, so nothing - * to be done for them. + * Hardcoded terms are generally handled in event parsing, which + * traditionally have had to handle not having a PMU. An alias may + * have hard coded config values, optionally apply them below. */ - if (parse_events__is_hardcoded_term(term)) + if (parse_events__is_hardcoded_term(term)) { + /* Config terms set all bits in the config. */ + DECLARE_BITMAP(bits, PERF_PMU_FORMAT_BITS); + + if (!apply_hardcoded) + return 0; + + bitmap_fill(bits, PERF_PMU_FORMAT_BITS); + + 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); + 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); + 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); + 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); + break; + case PARSE_EVENTS__TERM_TYPE_USER: /* Not hardcoded. */ + return -EINVAL; + case PARSE_EVENTS__TERM_TYPE_NAME ... PARSE_EVENTS__TERM_TYPE_HARDWARE: + /* Skip non-config terms. */ + break; + default: + break; + } return 0; + } format = pmu_find_format(&pmu->format, term->config); if (!format) { @@ -1487,12 +1523,13 @@ static int pmu_config_term(const struct perf_pmu *pmu, int perf_pmu__config_terms(const struct perf_pmu *pmu, struct perf_event_attr *attr, struct parse_events_terms *terms, - bool zero, struct parse_events_error *err) + bool zero, bool apply_hardcoded, + struct parse_events_error *err) { struct parse_events_term *term; list_for_each_entry(term, &terms->terms, list) { - if (pmu_config_term(pmu, attr, term, terms, zero, err)) + if (pmu_config_term(pmu, attr, term, terms, zero, apply_hardcoded, err)) return -EINVAL; } @@ -1506,6 +1543,7 @@ int perf_pmu__config_terms(const struct perf_pmu *pmu, */ int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr, struct parse_events_terms *head_terms, + bool apply_hardcoded, struct parse_events_error *err) { bool zero = !!pmu->perf_event_attr_init_default; @@ -1514,7 +1552,7 @@ int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr, if (perf_pmu__is_fake(pmu)) return 0; - return perf_pmu__config_terms(pmu, attr, head_terms, zero, err); + return perf_pmu__config_terms(pmu, attr, head_terms, zero, apply_hardcoded, err); } static struct perf_pmu_alias *pmu_find_alias(struct perf_pmu *pmu, @@ -2283,7 +2321,9 @@ const char *perf_pmu__name_from_config(struct perf_pmu *pmu, u64 config) pmu_add_cpu_aliases(pmu); list_for_each_entry(event, &pmu->aliases, list) { struct perf_event_attr attr = {.config = 0,}; - int ret = perf_pmu__config(pmu, &attr, &event->terms, NULL); + + int ret = perf_pmu__config(pmu, &attr, &event->terms, /*apply_hardcoded=*/true, + /*err=*/NULL); if (ret == 0 && config == attr.config) return event->name; diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h index d352d53b8d55..c70317d3fb04 100644 --- a/tools/perf/util/pmu.h +++ b/tools/perf/util/pmu.h @@ -206,11 +206,13 @@ typedef int (*pmu_format_callback)(void *state, const char *name, int config, void pmu_add_sys_aliases(struct perf_pmu *pmu); int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr, struct parse_events_terms *head_terms, + bool apply_hardcoded, struct parse_events_error *error); int perf_pmu__config_terms(const struct perf_pmu *pmu, struct perf_event_attr *attr, struct parse_events_terms *terms, - bool zero, struct parse_events_error *error); + 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); int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_terms, -- 2.50.1