On Thu, Jan 09, 2025 at 02:21:09PM -0800, Ian Rogers wrote: > Originally posted and merged from: > https://lore.kernel.org/r/20240416061533.921723-10-irogers@xxxxxxxxxx > This reverts commit 4f1b067359ac8364cdb7f9fda41085fa85789d0f although > the patch is now smaller due to related fixes being applied in commit > 22a4db3c3603 ("perf evsel: Add alternate_hw_config and use in > evsel__match"). > The original commit message was: > > It was requested that RISC-V be able to add events to the perf tool so > the PMU driver didn't need to map legacy events to config encodings: > https://lore.kernel.org/lkml/20240217005738.3744121-1-atishp@xxxxxxxxxxxx/ > > This change makes the priority of events specified without a PMU the > same as those specified with a PMU, namely sysfs and JSON events are > checked first before using the legacy encoding. I'm still not convinced why we need this change despite of these troubles. If it's because RISC-V cannot define the lagacy hardware events in the kernel driver, why not using a different name in JSON and ask users to use the name specifically? Something like: $ perf record -e riscv-cycles ... Thanks, Namhyung > > The hw_term is made more generic as a hardware_event that encodes a > pair of string and int value, allowing parse_events_multi_pmu_add to > fall back on a known encoding when the sysfs/JSON adding fails for > core events. As this covers PE_VALUE_SYM_HW, that token is removed and > related code simplified. > > Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx> > Reviewed-by: Kan Liang <kan.liang@xxxxxxxxxxxxxxx> > Tested-by: Atish Patra <atishp@xxxxxxxxxxxx> > Tested-by: James Clark <james.clark@xxxxxxxxxx> > Tested-by: Leo Yan <leo.yan@xxxxxxx> > Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx> > Cc: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx> > Cc: Beeman Strong <beeman@xxxxxxxxxxxx> > Cc: Ingo Molnar <mingo@xxxxxxxxxx> > Cc: Jiri Olsa <jolsa@xxxxxxxxxx> > Cc: Mark Rutland <mark.rutland@xxxxxxx> > Cc: Namhyung Kim <namhyung@xxxxxxxxxx> > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx> > Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> > --- > tools/perf/util/parse-events.c | 26 +++++++++--- > tools/perf/util/parse-events.l | 76 +++++++++++++++++----------------- > tools/perf/util/parse-events.y | 60 ++++++++++++++++++--------- > 3 files changed, 98 insertions(+), 64 deletions(-) > > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c > index 1e23faa364b1..3a60fca53cfa 100644 > --- a/tools/perf/util/parse-events.c > +++ b/tools/perf/util/parse-events.c > @@ -1545,8 +1545,8 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state, > struct list_head *list = NULL; > struct perf_pmu *pmu = NULL; > YYLTYPE *loc = loc_; > - int ok = 0; > - const char *config; > + int ok = 0, core_ok = 0; > + const char *tmp; > struct parse_events_terms parsed_terms; > > *listp = NULL; > @@ -1559,15 +1559,15 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state, > return ret; > } > > - config = strdup(event_name); > - if (!config) > + tmp = strdup(event_name); > + if (!tmp) > goto out_err; > > if (parse_events_term__num(&term, > PARSE_EVENTS__TERM_TYPE_USER, > - config, /*num=*/1, /*novalue=*/true, > + tmp, /*num=*/1, /*novalue=*/true, > loc, /*loc_val=*/NULL) < 0) { > - zfree(&config); > + zfree(&tmp); > goto out_err; > } > list_add_tail(&term->list, &parsed_terms.terms); > @@ -1598,6 +1598,8 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state, > pr_debug("%s -> %s/%s/\n", event_name, pmu->name, sb.buf); > strbuf_release(&sb); > ok++; > + if (pmu->is_core) > + core_ok++; > } > } > > @@ -1614,6 +1616,18 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state, > } > } > > + if (hw_config != PERF_COUNT_HW_MAX && !core_ok) { > + /* > + * The event wasn't found on core PMUs but it has a hardware > + * config version to try. > + */ > + if (!parse_events_add_numeric(parse_state, list, > + PERF_TYPE_HARDWARE, hw_config, > + const_parsed_terms, > + /*wildcard=*/true)) > + ok++; > + } > + > out_err: > parse_events_terms__exit(&parsed_terms); > if (ok) > diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l > index bf7f73548605..29082a22ccc9 100644 > --- a/tools/perf/util/parse-events.l > +++ b/tools/perf/util/parse-events.l > @@ -113,12 +113,12 @@ do { \ > yyless(0); \ > } while (0) > > -static int sym(yyscan_t scanner, int type, int config) > +static int sym(yyscan_t scanner, int config) > { > YYSTYPE *yylval = parse_events_get_lval(scanner); > > - yylval->num = (type << 16) + config; > - return type == PERF_TYPE_HARDWARE ? PE_VALUE_SYM_HW : PE_VALUE_SYM_SW; > + yylval->num = config; > + return PE_VALUE_SYM_SW; > } > > static int term(yyscan_t scanner, enum parse_events__term_type type) > @@ -129,13 +129,13 @@ static int term(yyscan_t scanner, enum parse_events__term_type type) > return PE_TERM; > } > > -static int hw_term(yyscan_t scanner, int config) > +static int hw(yyscan_t scanner, int config) > { > YYSTYPE *yylval = parse_events_get_lval(scanner); > char *text = parse_events_get_text(scanner); > > - yylval->hardware_term.str = strdup(text); > - yylval->hardware_term.num = PERF_TYPE_HARDWARE + config; > + yylval->hardware_event.str = strdup(text); > + yylval->hardware_event.num = config; > return PE_TERM_HW; > } > > @@ -324,16 +324,16 @@ aux-output { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_AUX_OUTPUT); } > aux-action { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_AUX_ACTION); } > aux-sample-size { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE); } > metric-id { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_METRIC_ID); } > -cpu-cycles|cycles { return hw_term(yyscanner, PERF_COUNT_HW_CPU_CYCLES); } > -stalled-cycles-frontend|idle-cycles-frontend { return hw_term(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_FRONTEND); } > -stalled-cycles-backend|idle-cycles-backend { return hw_term(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_BACKEND); } > -instructions { return hw_term(yyscanner, PERF_COUNT_HW_INSTRUCTIONS); } > -cache-references { return hw_term(yyscanner, PERF_COUNT_HW_CACHE_REFERENCES); } > -cache-misses { return hw_term(yyscanner, PERF_COUNT_HW_CACHE_MISSES); } > -branch-instructions|branches { return hw_term(yyscanner, PERF_COUNT_HW_BRANCH_INSTRUCTIONS); } > -branch-misses { return hw_term(yyscanner, PERF_COUNT_HW_BRANCH_MISSES); } > -bus-cycles { return hw_term(yyscanner, PERF_COUNT_HW_BUS_CYCLES); } > -ref-cycles { return hw_term(yyscanner, PERF_COUNT_HW_REF_CPU_CYCLES); } > +cpu-cycles|cycles { return hw(yyscanner, PERF_COUNT_HW_CPU_CYCLES); } > +stalled-cycles-frontend|idle-cycles-frontend { return hw(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_FRONTEND); } > +stalled-cycles-backend|idle-cycles-backend { return hw(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_BACKEND); } > +instructions { return hw(yyscanner, PERF_COUNT_HW_INSTRUCTIONS); } > +cache-references { return hw(yyscanner, PERF_COUNT_HW_CACHE_REFERENCES); } > +cache-misses { return hw(yyscanner, PERF_COUNT_HW_CACHE_MISSES); } > +branch-instructions|branches { return hw(yyscanner, PERF_COUNT_HW_BRANCH_INSTRUCTIONS); } > +branch-misses { return hw(yyscanner, PERF_COUNT_HW_BRANCH_MISSES); } > +bus-cycles { return hw(yyscanner, PERF_COUNT_HW_BUS_CYCLES); } > +ref-cycles { return hw(yyscanner, PERF_COUNT_HW_REF_CPU_CYCLES); } > r{num_raw_hex} { return str(yyscanner, PE_RAW); } > r0x{num_raw_hex} { return str(yyscanner, PE_RAW); } > , { return ','; } > @@ -377,28 +377,28 @@ r0x{num_raw_hex} { return str(yyscanner, PE_RAW); } > <<EOF>> { BEGIN(INITIAL); } > } > > -cpu-cycles|cycles { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_CPU_CYCLES); } > -stalled-cycles-frontend|idle-cycles-frontend { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_STALLED_CYCLES_FRONTEND); } > -stalled-cycles-backend|idle-cycles-backend { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_STALLED_CYCLES_BACKEND); } > -instructions { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_INSTRUCTIONS); } > -cache-references { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_CACHE_REFERENCES); } > -cache-misses { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_CACHE_MISSES); } > -branch-instructions|branches { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_BRANCH_INSTRUCTIONS); } > -branch-misses { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_BRANCH_MISSES); } > -bus-cycles { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_BUS_CYCLES); } > -ref-cycles { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_REF_CPU_CYCLES); } > -cpu-clock { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CPU_CLOCK); } > -task-clock { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_TASK_CLOCK); } > -page-faults|faults { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_PAGE_FAULTS); } > -minor-faults { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_PAGE_FAULTS_MIN); } > -major-faults { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_PAGE_FAULTS_MAJ); } > -context-switches|cs { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CONTEXT_SWITCHES); } > -cpu-migrations|migrations { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CPU_MIGRATIONS); } > -alignment-faults { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_ALIGNMENT_FAULTS); } > -emulation-faults { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_EMULATION_FAULTS); } > -dummy { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY); } > -bpf-output { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_BPF_OUTPUT); } > -cgroup-switches { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CGROUP_SWITCHES); } > +cpu-cycles|cycles { return hw(yyscanner, PERF_COUNT_HW_CPU_CYCLES); } > +stalled-cycles-frontend|idle-cycles-frontend { return hw(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_FRONTEND); } > +stalled-cycles-backend|idle-cycles-backend { return hw(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_BACKEND); } > +instructions { return hw(yyscanner, PERF_COUNT_HW_INSTRUCTIONS); } > +cache-references { return hw(yyscanner, PERF_COUNT_HW_CACHE_REFERENCES); } > +cache-misses { return hw(yyscanner, PERF_COUNT_HW_CACHE_MISSES); } > +branch-instructions|branches { return hw(yyscanner, PERF_COUNT_HW_BRANCH_INSTRUCTIONS); } > +branch-misses { return hw(yyscanner, PERF_COUNT_HW_BRANCH_MISSES); } > +bus-cycles { return hw(yyscanner, PERF_COUNT_HW_BUS_CYCLES); } > +ref-cycles { return hw(yyscanner, PERF_COUNT_HW_REF_CPU_CYCLES); } > +cpu-clock { return sym(yyscanner, PERF_COUNT_SW_CPU_CLOCK); } > +task-clock { return sym(yyscanner, PERF_COUNT_SW_TASK_CLOCK); } > +page-faults|faults { return sym(yyscanner, PERF_COUNT_SW_PAGE_FAULTS); } > +minor-faults { return sym(yyscanner, PERF_COUNT_SW_PAGE_FAULTS_MIN); } > +major-faults { return sym(yyscanner, PERF_COUNT_SW_PAGE_FAULTS_MAJ); } > +context-switches|cs { return sym(yyscanner, PERF_COUNT_SW_CONTEXT_SWITCHES); } > +cpu-migrations|migrations { return sym(yyscanner, PERF_COUNT_SW_CPU_MIGRATIONS); } > +alignment-faults { return sym(yyscanner, PERF_COUNT_SW_ALIGNMENT_FAULTS); } > +emulation-faults { return sym(yyscanner, PERF_COUNT_SW_EMULATION_FAULTS); } > +dummy { return sym(yyscanner, PERF_COUNT_SW_DUMMY); } > +bpf-output { return sym(yyscanner, PERF_COUNT_SW_BPF_OUTPUT); } > +cgroup-switches { return sym(yyscanner, PERF_COUNT_SW_CGROUP_SWITCHES); } > > {lc_type} { return str(yyscanner, PE_LEGACY_CACHE); } > {lc_type}-{lc_op_result} { return str(yyscanner, PE_LEGACY_CACHE); } > diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y > index f888cbb076d6..d2ef1890007e 100644 > --- a/tools/perf/util/parse-events.y > +++ b/tools/perf/util/parse-events.y > @@ -55,7 +55,7 @@ static void free_list_evsel(struct list_head* list_evsel) > %} > > %token PE_START_EVENTS PE_START_TERMS > -%token PE_VALUE PE_VALUE_SYM_HW PE_VALUE_SYM_SW PE_TERM > +%token PE_VALUE PE_VALUE_SYM_SW PE_TERM > %token PE_EVENT_NAME > %token PE_RAW PE_NAME > %token PE_MODIFIER_EVENT PE_MODIFIER_BP PE_BP_COLON PE_BP_SLASH > @@ -65,11 +65,9 @@ static void free_list_evsel(struct list_head* list_evsel) > %token PE_DRV_CFG_TERM > %token PE_TERM_HW > %type <num> PE_VALUE > -%type <num> PE_VALUE_SYM_HW > %type <num> PE_VALUE_SYM_SW > %type <mod> PE_MODIFIER_EVENT > %type <term_type> PE_TERM > -%type <num> value_sym > %type <str> PE_RAW > %type <str> PE_NAME > %type <str> PE_LEGACY_CACHE > @@ -85,6 +83,7 @@ static void free_list_evsel(struct list_head* list_evsel) > %type <list_terms> opt_pmu_config > %destructor { parse_events_terms__delete ($$); } <list_terms> > %type <list_evsel> event_pmu > +%type <list_evsel> event_legacy_hardware > %type <list_evsel> event_legacy_symbol > %type <list_evsel> event_legacy_cache > %type <list_evsel> event_legacy_mem > @@ -102,8 +101,8 @@ static void free_list_evsel(struct list_head* list_evsel) > %destructor { free_list_evsel ($$); } <list_evsel> > %type <tracepoint_name> tracepoint_name > %destructor { free ($$.sys); free ($$.event); } <tracepoint_name> > -%type <hardware_term> PE_TERM_HW > -%destructor { free ($$.str); } <hardware_term> > +%type <hardware_event> PE_TERM_HW > +%destructor { free ($$.str); } <hardware_event> > > %union > { > @@ -118,10 +117,10 @@ static void free_list_evsel(struct list_head* list_evsel) > char *sys; > char *event; > } tracepoint_name; > - struct hardware_term { > + struct hardware_event { > char *str; > u64 num; > - } hardware_term; > + } hardware_event; > } > %% > > @@ -264,6 +263,7 @@ PE_EVENT_NAME event_def > event_def > > event_def: event_pmu | > + event_legacy_hardware | > event_legacy_symbol | > event_legacy_cache sep_dc | > event_legacy_mem sep_dc | > @@ -306,24 +306,45 @@ PE_NAME sep_dc > $$ = list; > } > > -value_sym: > -PE_VALUE_SYM_HW > +event_legacy_hardware: > +PE_TERM_HW opt_pmu_config > +{ > + /* List of created evsels. */ > + struct list_head *list = NULL; > + int err = parse_events_multi_pmu_add(_parse_state, $1.str, $1.num, $2, &list, &@1); > + > + free($1.str); > + parse_events_terms__delete($2); > + if (err) > + PE_ABORT(err); > + > + $$ = list; > +} > | > -PE_VALUE_SYM_SW > +PE_TERM_HW sep_dc > +{ > + struct list_head *list; > + int err; > + > + err = parse_events_multi_pmu_add(_parse_state, $1.str, $1.num, NULL, &list, &@1); > + free($1.str); > + if (err) > + PE_ABORT(err); > + $$ = list; > +} > > event_legacy_symbol: > -value_sym '/' event_config '/' > +PE_VALUE_SYM_SW '/' event_config '/' > { > struct list_head *list; > - int type = $1 >> 16; > - int config = $1 & 255; > int err; > - bool wildcard = (type == PERF_TYPE_HARDWARE || type == PERF_TYPE_HW_CACHE); > > list = alloc_list(); > if (!list) > YYNOMEM; > - err = parse_events_add_numeric(_parse_state, list, type, config, $3, wildcard); > + err = parse_events_add_numeric(_parse_state, list, > + /*type=*/PERF_TYPE_SOFTWARE, /*config=*/$1, > + $3, /*wildcard=*/false); > parse_events_terms__delete($3); > if (err) { > free_list_evsel(list); > @@ -332,18 +353,17 @@ value_sym '/' event_config '/' > $$ = list; > } > | > -value_sym sep_slash_slash_dc > +PE_VALUE_SYM_SW sep_slash_slash_dc > { > struct list_head *list; > - int type = $1 >> 16; > - int config = $1 & 255; > - bool wildcard = (type == PERF_TYPE_HARDWARE || type == PERF_TYPE_HW_CACHE); > int err; > > list = alloc_list(); > if (!list) > YYNOMEM; > - err = parse_events_add_numeric(_parse_state, list, type, config, /*head_config=*/NULL, wildcard); > + err = parse_events_add_numeric(_parse_state, list, > + /*type=*/PERF_TYPE_SOFTWARE, /*config=*/$1, > + /*head_config=*/NULL, /*wildcard=*/false); > if (err) > PE_ABORT(err); > $$ = list; > -- > 2.47.1.613.gc27f4b7a9f-goog >
