2024-03-07 14:34:57

by Liang, Kan

[permalink] [raw]
Subject: Re: [PATCH v1 4/6] perf list: Give more details about raw event encodings



On 2024-03-07 3:14 a.m., Ian Rogers wrote:
> List all the PMUs, not just the first core one, and list real format
> specifiers with value ranges.
>
> Before:
> ```
> $ perf list
> ...
> rNNN [Raw hardware event descriptor]
> cpu/t1=v1[,t2=v2,t3 ...]/modifier [Raw hardware event descriptor]
> [(see 'man perf-list' on how to encode it)]
> mem:<addr>[/len][:access] [Hardware breakpoint]
> ...
> ```
>
> After:
> ```
> $ perf list
> ...
> rNNN [Raw hardware event descriptor]
> cpu/event=0..255,pc,edge,.../modifier [Raw hardware event descriptor]
> [(see 'man perf-list' or 'man perf-record' on how to encode it)]
> breakpoint//modifier [Raw hardware event descriptor]
> cstate_core/event=0..0xffffffffffffffff/modifier [Raw hardware event descriptor]
> cstate_pkg/event=0..0xffffffffffffffff/modifier [Raw hardware event descriptor]
> i915/i915_eventid=0..0x1fffff/modifier [Raw hardware event descriptor]
> intel_bts//modifier [Raw hardware event descriptor]
> intel_pt/ptw,event,cyc_thresh=0..15,.../modifier [Raw hardware event descriptor]
> kprobe/retprobe/modifier [Raw hardware event descriptor]
> msr/event=0..0xffffffffffffffff/modifier [Raw hardware event descriptor]
> power/event=0..255/modifier [Raw hardware event descriptor]
> software//modifier [Raw hardware event descriptor]

Software apparently is not a raw hardware event. Ideally, we should have
a consist name. E.g.,
software//modifier [Raw Software event descriptor]
tracepoint//modifier [Raw Tracepoint event descriptor]

If it's too complex, I guess using [event descriptor] or just dropping
it should be OK for me as well.

> tracepoint//modifier [Raw hardware event descriptor]
> uncore_arb/event=0..255,edge,inv,.../modifier [Raw hardware event descriptor]
> uncore_cbox/event=0..255,edge,inv,.../modifier [Raw hardware event descriptor]
> uncore_clock/event=0..255/modifier [Raw hardware event descriptor]
> uncore_imc_free_running/event=0..255,umask=0..255/modifier[Raw hardware event descriptor]
> uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier[Raw hardware event descriptor]
> mem:<addr>[/len][:access] [Hardware breakpoint]
> ...
> ```
>
> With '--details' provide more details on the formats encoding:
> ```
> cpu/event=0..255,pc,edge,.../modifier [Raw hardware event descriptor]
> [(see 'man perf-list' or 'man perf-record' on how to encode it)]
> cpu/event=0..255,pc,edge,offcore_rsp=0..0xffffffffffffffff,ldlat=0..0xffff,inv,
> umask=0..255,frontend=0..0xffffff,cmask=0..255,config=0..0xffffffffffffffff,
> config1=0..0xffffffffffffffff,config2=0..0xffffffffffffffff,config3=0..0xffffffffffffffff,
> name=string,period=number,freq=number,branch_type=(u|k|hv|any|...),time,
> call-graph=(fp|dwarf|lbr),stack-size=number,max-stack=number,nr=number,inherit,no-inherit,
> overwrite,no-overwrite,percore,aux-output,aux-sample-size=number/modifier
> breakpoint//modifier [Raw hardware event descriptor]
> breakpoint//modifier
> cstate_core/event=0..0xffffffffffffffff/modifier [Raw hardware event descriptor]
> cstate_core/event=0..0xffffffffffffffff/modifier
> cstate_pkg/event=0..0xffffffffffffffff/modifier [Raw hardware event descriptor]
> cstate_pkg/event=0..0xffffffffffffffff/modifier
> i915/i915_eventid=0..0x1fffff/modifier [Raw hardware event descriptor]
> i915/i915_eventid=0..0x1fffff/modifier
> intel_bts//modifier [Raw hardware event descriptor]
> intel_bts//modifier
> intel_pt/ptw,event,cyc_thresh=0..15,.../modifier [Raw hardware event descriptor]
> intel_pt/ptw,event,cyc_thresh=0..15,pt,notnt,branch,tsc,pwr_evt,fup_on_ptw,cyc,noretcomp,
> mtc,psb_period=0..15,mtc_period=0..15/modifier
> kprobe/retprobe/modifier [Raw hardware event descriptor]
> kprobe/retprobe/modifier
> msr/event=0..0xffffffffffffffff/modifier [Raw hardware event descriptor]
> msr/event=0..0xffffffffffffffff/modifier
> power/event=0..255/modifier [Raw hardware event descriptor]
> power/event=0..255/modifier
> software//modifier [Raw hardware event descriptor]
> software//modifier
> tracepoint//modifier [Raw hardware event descriptor]
> tracepoint//modifier
> uncore_arb/event=0..255,edge,inv,.../modifier [Raw hardware event descriptor]
> uncore_arb/event=0..255,edge,inv,umask=0..255,cmask=0..31/modifier
> uncore_cbox/event=0..255,edge,inv,.../modifier [Raw hardware event descriptor]
> uncore_cbox/event=0..255,edge,inv,umask=0..255,cmask=0..31/modifier
> uncore_clock/event=0..255/modifier [Raw hardware event descriptor]
> uncore_clock/event=0..255/modifier
> uncore_imc_free_running/event=0..255,umask=0..255/modifier[Raw hardware event descriptor]
> uncore_imc_free_running/event=0..255,umask=0..255/modifier
> uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier[Raw hardware event descriptor]
> uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier
> ```
>
> Signed-off-by: Ian Rogers <[email protected]>
> ---
> tools/perf/util/pmu.c | 55 +++++++++++++++++++-
> tools/perf/util/pmu.h | 3 ++
> tools/perf/util/pmus.c | 94 ++++++++++++++++++++++++++++++++++
> tools/perf/util/pmus.h | 1 +
> tools/perf/util/print-events.c | 18 +------
> 5 files changed, 153 insertions(+), 18 deletions(-)
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 24be587e3537..904725f03d29 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -1603,6 +1603,55 @@ bool perf_pmu__has_format(const struct perf_pmu *pmu, const char *name)
> return false;
> }
>
> +int perf_pmu__for_each_format(struct perf_pmu *pmu, void *state, pmu_format_callback cb)
> +{
> + static const char *const terms[] = {
> + "config=0..0xffffffffffffffff",
> + "config1=0..0xffffffffffffffff",
> + "config2=0..0xffffffffffffffff",
> + "config3=0..0xffffffffffffffff",
> + "name=string",
> + "period=number",
> + "freq=number",
> + "branch_type=(u|k|hv|any|...)",
> + "time",
> + "call-graph=(fp|dwarf|lbr)",
> + "stack-size=number",
> + "max-stack=number",
> + "nr=number",
> + "inherit",
> + "no-inherit",
> + "overwrite",
> + "no-overwrite",
> + "percore",
> + "aux-output",
> + "aux-sample-size=number",
> + };

I think it's very likely we forget to update the const table when
introducing a new term. In the parse-events.c, there is
config_term_names[] to restore the name of terms. Can it be shared here?
So every time a new term is introduced, the perf list can be updated
automatically.

Thanks,
Kan
> + struct perf_pmu_format *format;
> + int ret;
> +
> + list_for_each_entry(format, &pmu->format, list) {
> + perf_pmu_format__load(pmu, format);
> + ret = cb(state, format->name, (int)format->value, format->bits);
> + if (ret)
> + return ret;
> + }
> + if (!pmu->is_core)
> + return 0;
> +
> + for (size_t i = 0; i < ARRAY_SIZE(terms); i++) {
> + int config = PERF_PMU_FORMAT_VALUE_CONFIG;
> +
> + if (i < PERF_PMU_FORMAT_VALUE_CONFIG_END)
> + config = i;
> +
> + ret = cb(state, terms[i], config, /*bits=*/NULL);
> + if (ret)
> + return ret;
> + }
> + return 0;
> +}
> +
> bool is_pmu_core(const char *name)
> {
> return !strcmp(name, "cpu") || !strcmp(name, "cpum_cf") || is_sysfs_pmu_core(name);
> @@ -1697,8 +1746,12 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
> pmu_add_cpu_aliases(pmu);
> list_for_each_entry(event, &pmu->aliases, list) {
> size_t buf_used;
> + int pmu_name_len;
>
> info.pmu_name = event->pmu_name ?: pmu->name;
> + pmu_name_len = skip_duplicate_pmus
> + ? pmu_name_len_no_suffix(info.pmu_name, /*num=*/NULL)
> + : (int)strlen(info.pmu_name);
> info.alias = NULL;
> if (event->desc) {
> info.name = event->name;
> @@ -1723,7 +1776,7 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
> info.encoding_desc = buf + buf_used;
> parse_events_terms__to_strbuf(&event->terms, &sb);
> buf_used += snprintf(buf + buf_used, sizeof(buf) - buf_used,
> - "%s/%s/", info.pmu_name, sb.buf) + 1;
> + "%.*s/%s/", pmu_name_len, info.pmu_name, sb.buf) + 1;
> info.topic = event->topic;
> info.str = sb.buf;
> info.deprecated = event->deprecated;
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index e35d985206db..9f5284b29ecf 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -196,6 +196,8 @@ struct pmu_event_info {
> };
>
> 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);
>
> void pmu_add_sys_aliases(struct perf_pmu *pmu);
> int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr,
> @@ -215,6 +217,7 @@ int perf_pmu__find_event(struct perf_pmu *pmu, const char *event, void *state, p
> int perf_pmu__format_parse(struct perf_pmu *pmu, int dirfd, bool eager_load);
> 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);
>
> bool is_pmu_core(const char *name);
> bool perf_pmu__supports_legacy_cache(const struct perf_pmu *pmu);
> diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
> index 16505071d362..89b15ddeb24e 100644
> --- a/tools/perf/util/pmus.c
> +++ b/tools/perf/util/pmus.c
> @@ -16,6 +16,7 @@
> #include "pmus.h"
> #include "pmu.h"
> #include "print-events.h"
> +#include "strbuf.h"
>
> /*
> * core_pmus: A PMU belongs to core_pmus if it's name is "cpu" or it's sysfs
> @@ -503,6 +504,99 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
> zfree(&aliases);
> }
>
> +struct build_format_string_args {
> + struct strbuf short_string;
> + struct strbuf long_string;
> + int num_formats;
> +};
> +
> +static int build_format_string(void *state, const char *name, int config,
> + const unsigned long *bits)
> +{
> + struct build_format_string_args *args = state;
> + unsigned int num_bits;
> + int ret1, ret2 = 0;
> +
> + (void)config;
> + args->num_formats++;
> + if (args->num_formats > 1) {
> + strbuf_addch(&args->long_string, ',');
> + if (args->num_formats < 4)
> + strbuf_addch(&args->short_string, ',');
> + }
> + num_bits = bits ? bitmap_weight(bits, PERF_PMU_FORMAT_BITS) : 0;
> + if (num_bits <= 1) {
> + ret1 = strbuf_addf(&args->long_string, "%s", name);
> + if (args->num_formats < 4)
> + ret2 = strbuf_addf(&args->short_string, "%s", name);
> + } else if (num_bits > 8) {
> + ret1 = strbuf_addf(&args->long_string, "%s=0..0x%llx", name,
> + ULLONG_MAX >> (64 - num_bits));
> + if (args->num_formats < 4) {
> + ret2 = strbuf_addf(&args->short_string, "%s=0..0x%llx", name,
> + ULLONG_MAX >> (64 - num_bits));
> + }
> + } else {
> + ret1 = strbuf_addf(&args->long_string, "%s=0..%llu", name,
> + ULLONG_MAX >> (64 - num_bits));
> + if (args->num_formats < 4) {
> + ret2 = strbuf_addf(&args->short_string, "%s=0..%llu", name,
> + ULLONG_MAX >> (64 - num_bits));
> + }
> + }
> + return ret1 < 0 ? ret1 : (ret2 < 0 ? ret2 : 0);
> +}
> +
> +void perf_pmus__print_raw_pmu_events(const struct print_callbacks *print_cb, void *print_state)
> +{
> + bool skip_duplicate_pmus = print_cb->skip_duplicate_pmus(print_state);
> + struct perf_pmu *(*scan_fn)(struct perf_pmu *);
> + struct perf_pmu *pmu = NULL;
> +
> + if (skip_duplicate_pmus)
> + scan_fn = perf_pmus__scan_skip_duplicates;
> + else
> + scan_fn = perf_pmus__scan;
> +
> + while ((pmu = scan_fn(pmu)) != NULL) {
> + struct build_format_string_args format_args = {
> + .short_string = STRBUF_INIT,
> + .long_string = STRBUF_INIT,
> + .num_formats = 0,
> + };
> + int len = pmu_name_len_no_suffix(pmu->name, /*num=*/NULL);
> + const char *desc = "(see 'man perf-list' or 'man perf-record' on how to encode it)";
> +
> + if (!pmu->is_core)
> + desc = NULL;
> +
> + strbuf_addf(&format_args.short_string, "%.*s/", len, pmu->name);
> + strbuf_addf(&format_args.long_string, "%.*s/", len, pmu->name);
> + perf_pmu__for_each_format(pmu, &format_args, build_format_string);
> +
> + if (format_args.num_formats > 3)
> + strbuf_addf(&format_args.short_string, ",.../modifier");
> + else
> + strbuf_addf(&format_args.short_string, "/modifier");
> +
> + strbuf_addf(&format_args.long_string, "/modifier");
> + print_cb->print_event(print_state,
> + /*topic=*/NULL,
> + /*pmu_name=*/NULL,
> + format_args.short_string.buf,
> + /*event_alias=*/NULL,
> + /*scale_unit=*/NULL,
> + /*deprecated=*/false,
> + "Raw hardware event descriptor",
> + desc,
> + /*long_desc=*/NULL,
> + format_args.long_string.buf);
> +
> + strbuf_release(&format_args.short_string);
> + strbuf_release(&format_args.long_string);
> + }
> +}
> +
> bool perf_pmus__have_event(const char *pname, const char *name)
> {
> struct perf_pmu *pmu = perf_pmus__find(pname);
> diff --git a/tools/perf/util/pmus.h b/tools/perf/util/pmus.h
> index 94d2a08d894b..eec599d8aebd 100644
> --- a/tools/perf/util/pmus.h
> +++ b/tools/perf/util/pmus.h
> @@ -18,6 +18,7 @@ struct perf_pmu *perf_pmus__scan_core(struct perf_pmu *pmu);
> const struct perf_pmu *perf_pmus__pmu_for_pmu_filter(const char *str);
>
> void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *print_state);
> +void perf_pmus__print_raw_pmu_events(const struct print_callbacks *print_cb, void *print_state);
> bool perf_pmus__have_event(const char *pname, const char *name);
> int perf_pmus__num_core_pmus(void);
> bool perf_pmus__supports_extended_type(void);
> diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c
> index e0d2b49bab66..3a7f14fe2390 100644
> --- a/tools/perf/util/print-events.c
> +++ b/tools/perf/util/print-events.c
> @@ -416,8 +416,6 @@ void print_symbol_events(const struct print_callbacks *print_cb, void *print_sta
> */
> void print_events(const struct print_callbacks *print_cb, void *print_state)
> {
> - char *tmp;
> -
> print_symbol_events(print_cb, print_state, PERF_TYPE_HARDWARE,
> event_symbols_hw, PERF_COUNT_HW_MAX);
> print_symbol_events(print_cb, print_state, PERF_TYPE_SOFTWARE,
> @@ -441,21 +439,7 @@ void print_events(const struct print_callbacks *print_cb, void *print_state)
> /*long_desc=*/NULL,
> /*encoding_desc=*/NULL);
>
> - if (asprintf(&tmp, "%s/t1=v1[,t2=v2,t3 ...]/modifier",
> - perf_pmus__scan_core(/*pmu=*/NULL)->name) > 0) {
> - print_cb->print_event(print_state,
> - /*topic=*/NULL,
> - /*pmu_name=*/NULL,
> - tmp,
> - /*event_alias=*/NULL,
> - /*scale_unit=*/NULL,
> - /*deprecated=*/false,
> - event_type_descriptors[PERF_TYPE_RAW],
> - "(see 'man perf-list' on how to encode it)",
> - /*long_desc=*/NULL,
> - /*encoding_desc=*/NULL);
> - free(tmp);
> - }
> + perf_pmus__print_raw_pmu_events(print_cb, print_state);
>
> print_cb->print_event(print_state,
> /*topic=*/NULL,


2024-03-07 16:42:19

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH v1 4/6] perf list: Give more details about raw event encodings

On Thu, Mar 7, 2024 at 6:34 AM Liang, Kan <[email protected]> wrote:
>
>
>
> On 2024-03-07 3:14 a.m., Ian Rogers wrote:
> > List all the PMUs, not just the first core one, and list real format
> > specifiers with value ranges.
> >
> > Before:
> > ```
> > $ perf list
> > ...
> > rNNN [Raw hardware event descriptor]
> > cpu/t1=v1[,t2=v2,t3 ...]/modifier [Raw hardware event descriptor]
> > [(see 'man perf-list' on how to encode it)]
> > mem:<addr>[/len][:access] [Hardware breakpoint]
> > ...
> > ```
> >
> > After:
> > ```
> > $ perf list
> > ...
> > rNNN [Raw hardware event descriptor]
> > cpu/event=0..255,pc,edge,.../modifier [Raw hardware event descriptor]
> > [(see 'man perf-list' or 'man perf-record' on how to encode it)]
> > breakpoint//modifier [Raw hardware event descriptor]
> > cstate_core/event=0..0xffffffffffffffff/modifier [Raw hardware event descriptor]
> > cstate_pkg/event=0..0xffffffffffffffff/modifier [Raw hardware event descriptor]
> > i915/i915_eventid=0..0x1fffff/modifier [Raw hardware event descriptor]
> > intel_bts//modifier [Raw hardware event descriptor]
> > intel_pt/ptw,event,cyc_thresh=0..15,.../modifier [Raw hardware event descriptor]
> > kprobe/retprobe/modifier [Raw hardware event descriptor]
> > msr/event=0..0xffffffffffffffff/modifier [Raw hardware event descriptor]
> > power/event=0..255/modifier [Raw hardware event descriptor]
> > software//modifier [Raw hardware event descriptor]
>
> Software apparently is not a raw hardware event. Ideally, we should have
> a consist name. E.g.,
> software//modifier [Raw Software event descriptor]
> tracepoint//modifier [Raw Tracepoint event descriptor]
>
> If it's too complex, I guess using [event descriptor] or just dropping
> it should be OK for me as well.

Thanks Kan! I think just "[Raw event descriptor]" so I can avoid
having a bunch of special case logic.

>
> > tracepoint//modifier [Raw hardware event descriptor]
> > uncore_arb/event=0..255,edge,inv,.../modifier [Raw hardware event descriptor]
> > uncore_cbox/event=0..255,edge,inv,.../modifier [Raw hardware event descriptor]
> > uncore_clock/event=0..255/modifier [Raw hardware event descriptor]
> > uncore_imc_free_running/event=0..255,umask=0..255/modifier[Raw hardware event descriptor]
> > uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier[Raw hardware event descriptor]
> > mem:<addr>[/len][:access] [Hardware breakpoint]
> > ...
> > ```
> >
> > With '--details' provide more details on the formats encoding:
> > ```
> > cpu/event=0..255,pc,edge,.../modifier [Raw hardware event descriptor]
> > [(see 'man perf-list' or 'man perf-record' on how to encode it)]
> > cpu/event=0..255,pc,edge,offcore_rsp=0..0xffffffffffffffff,ldlat=0..0xffff,inv,
> > umask=0..255,frontend=0..0xffffff,cmask=0..255,config=0.0xffffffffffffffff,
> > config1=0..0xffffffffffffffff,config2=0..0xffffffffffffffff,config3=0..0xffffffffffffffff,
> > name=string,period=number,freq=number,branch_type=(u|k|hv|any|...),time,
> > call-graph=(fp|dwarf|lbr),stack-size=number,max-stack=number,nr=number,inherit,no-inherit,
> > overwrite,no-overwrite,percore,aux-output,aux-sample-size=number/modifier
> > breakpoint//modifier [Raw hardware event descriptor]
> > breakpoint//modifier
> > cstate_core/event=0..0xffffffffffffffff/modifier [Raw hardware event descriptor]
> > cstate_core/event=0..0xffffffffffffffff/modifier
> > cstate_pkg/event=0..0xffffffffffffffff/modifier [Raw hardware event descriptor]
> > cstate_pkg/event=0..0xffffffffffffffff/modifier
> > i915/i915_eventid=0..0x1fffff/modifier [Raw hardware event descriptor]
> > i915/i915_eventid=0..0x1fffff/modifier
> > intel_bts//modifier [Raw hardware event descriptor]
> > intel_bts//modifier
> > intel_pt/ptw,event,cyc_thresh=0..15,.../modifier [Raw hardware event descriptor]
> > intel_pt/ptw,event,cyc_thresh=0..15,pt,notnt,branch,tsc,pwr_evt,fup_on_ptw,cyc,noretcomp,
> > mtc,psb_period=0..15,mtc_period=0..15/modifier
> > kprobe/retprobe/modifier [Raw hardware event descriptor]
> > kprobe/retprobe/modifier
> > msr/event=0..0xffffffffffffffff/modifier [Raw hardware event descriptor]
> > msr/event=0..0xffffffffffffffff/modifier
> > power/event=0..255/modifier [Raw hardware event descriptor]
> > power/event=0..255/modifier
> > software//modifier [Raw hardware event descriptor]
> > software//modifier
> > tracepoint//modifier [Raw hardware event descriptor]
> > tracepoint//modifier
> > uncore_arb/event=0..255,edge,inv,.../modifier [Raw hardware event descriptor]
> > uncore_arb/event=0..255,edge,inv,umask=0..255,cmask=0..31/modifier
> > uncore_cbox/event=0..255,edge,inv,.../modifier [Raw hardware event descriptor]
> > uncore_cbox/event=0..255,edge,inv,umask=0..255,cmask=0..31/modifier
> > uncore_clock/event=0..255/modifier [Raw hardware event descriptor]
> > uncore_clock/event=0..255/modifier
> > uncore_imc_free_running/event=0..255,umask=0..255/modifier[Raw hardware event descriptor]
> > uncore_imc_free_running/event=0..255,umask=0..255/modifier
> > uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier[Raw hardware event descriptor]
> > uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier
> > ```
> >
> > Signed-off-by: Ian Rogers <[email protected]>
> > ---
> > tools/perf/util/pmu.c | 55 +++++++++++++++++++-
> > tools/perf/util/pmu.h | 3 ++
> > tools/perf/util/pmus.c | 94 ++++++++++++++++++++++++++++++++++
> > tools/perf/util/pmus.h | 1 +
> > tools/perf/util/print-events.c | 18 +------
> > 5 files changed, 153 insertions(+), 18 deletions(-)
> >
> > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> > index 24be587e3537..904725f03d29 100644
> > --- a/tools/perf/util/pmu.c
> > +++ b/tools/perf/util/pmu.c
> > @@ -1603,6 +1603,55 @@ bool perf_pmu__has_format(const struct perf_pmu *pmu, const char *name)
> > return false;
> > }
> >
> > +int perf_pmu__for_each_format(struct perf_pmu *pmu, void *state, pmu_format_callback cb)
> > +{
> > + static const char *const terms[] = {
> > + "config=0..0xffffffffffffffff",
> > + "config1=0..0xffffffffffffffff",
> > + "config2=0..0xffffffffffffffff",
> > + "config3=0..0xffffffffffffffff",
> > + "name=string",
> > + "period=number",
> > + "freq=number",
> > + "branch_type=(u|k|hv|any|...)",
> > + "time",
> > + "call-graph=(fp|dwarf|lbr)",
> > + "stack-size=number",
> > + "max-stack=number",
> > + "nr=number",
> > + "inherit",
> > + "no-inherit",
> > + "overwrite",
> > + "no-overwrite",
> > + "percore",
> > + "aux-output",
> > + "aux-sample-size=number",
> > + };
>
> I think it's very likely we forget to update the const table when
> introducing a new term. In the parse-events.c, there is
> config_term_names[] to restore the name of terms. Can it be shared here?
> So every time a new term is introduced, the perf list can be updated
> automatically.

Makes sense, I'll add in v2. I was originally pondering making the
list specific in some way. Perhaps in the future we can associate
valid generic terms with PMUs, like the config_term_common,
config_term_pmu, etc. code in parse-events.c.

Thanks,
Ian

> Thanks,
> Kan
> > + struct perf_pmu_format *format;
> > + int ret;
> > +
> > + list_for_each_entry(format, &pmu->format, list) {
> > + perf_pmu_format__load(pmu, format);
> > + ret = cb(state, format->name, (int)format->value, format->bits);
> > + if (ret)
> > + return ret;
> > + }
> > + if (!pmu->is_core)
> > + return 0;
> > +
> > + for (size_t i = 0; i < ARRAY_SIZE(terms); i++) {
> > + int config = PERF_PMU_FORMAT_VALUE_CONFIG;
> > +
> > + if (i < PERF_PMU_FORMAT_VALUE_CONFIG_END)
> > + config = i;
> > +
> > + ret = cb(state, terms[i], config, /*bits=*/NULL);
> > + if (ret)
> > + return ret;
> > + }
> > + return 0;
> > +}
> > +
> > bool is_pmu_core(const char *name)
> > {
> > return !strcmp(name, "cpu") || !strcmp(name, "cpum_cf") || is_sysfs_pmu_core(name);
> > @@ -1697,8 +1746,12 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
> > pmu_add_cpu_aliases(pmu);
> > list_for_each_entry(event, &pmu->aliases, list) {
> > size_t buf_used;
> > + int pmu_name_len;
> >
> > info.pmu_name = event->pmu_name ?: pmu->name;
> > + pmu_name_len = skip_duplicate_pmus
> > + ? pmu_name_len_no_suffix(info.pmu_name, /*num=*/NULL)
> > + : (int)strlen(info.pmu_name);
> > info.alias = NULL;
> > if (event->desc) {
> > info.name = event->name;
> > @@ -1723,7 +1776,7 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
> > info.encoding_desc = buf + buf_used;
> > parse_events_terms__to_strbuf(&event->terms, &sb);
> > buf_used += snprintf(buf + buf_used, sizeof(buf) - buf_used,
> > - "%s/%s/", info.pmu_name, sb.buf) + 1;
> > + "%.*s/%s/", pmu_name_len, info.pmu_name, sb.buf) + 1;
> > info.topic = event->topic;
> > info.str = sb.buf;
> > info.deprecated = event->deprecated;
> > diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> > index e35d985206db..9f5284b29ecf 100644
> > --- a/tools/perf/util/pmu.h
> > +++ b/tools/perf/util/pmu.h
> > @@ -196,6 +196,8 @@ struct pmu_event_info {
> > };
> >
> > 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);
> >
> > void pmu_add_sys_aliases(struct perf_pmu *pmu);
> > int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr,
> > @@ -215,6 +217,7 @@ int perf_pmu__find_event(struct perf_pmu *pmu, const char *event, void *state, p
> > int perf_pmu__format_parse(struct perf_pmu *pmu, int dirfd, bool eager_load);
> > 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);
> >
> > bool is_pmu_core(const char *name);
> > bool perf_pmu__supports_legacy_cache(const struct perf_pmu *pmu);
> > diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
> > index 16505071d362..89b15ddeb24e 100644
> > --- a/tools/perf/util/pmus.c
> > +++ b/tools/perf/util/pmus.c
> > @@ -16,6 +16,7 @@
> > #include "pmus.h"
> > #include "pmu.h"
> > #include "print-events.h"
> > +#include "strbuf.h"
> >
> > /*
> > * core_pmus: A PMU belongs to core_pmus if it's name is "cpu" or it's sysfs
> > @@ -503,6 +504,99 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
> > zfree(&aliases);
> > }
> >
> > +struct build_format_string_args {
> > + struct strbuf short_string;
> > + struct strbuf long_string;
> > + int num_formats;
> > +};
> > +
> > +static int build_format_string(void *state, const char *name, int config,
> > + const unsigned long *bits)
> > +{
> > + struct build_format_string_args *args = state;
> > + unsigned int num_bits;
> > + int ret1, ret2 = 0;
> > +
> > + (void)config;
> > + args->num_formats++;
> > + if (args->num_formats > 1) {
> > + strbuf_addch(&args->long_string, ',');
> > + if (args->num_formats < 4)
> > + strbuf_addch(&args->short_string, ',');
> > + }
> > + num_bits = bits ? bitmap_weight(bits, PERF_PMU_FORMAT_BITS) : 0;
> > + if (num_bits <= 1) {
> > + ret1 = strbuf_addf(&args->long_string, "%s", name);
> > + if (args->num_formats < 4)
> > + ret2 = strbuf_addf(&args->short_string, "%s", name);
> > + } else if (num_bits > 8) {
> > + ret1 = strbuf_addf(&args->long_string, "%s=0..0x%llx", name,
> > + ULLONG_MAX >> (64 - num_bits));
> > + if (args->num_formats < 4) {
> > + ret2 = strbuf_addf(&args->short_string, "%s=0.0x%llx", name,
> > + ULLONG_MAX >> (64 - num_bits));
> > + }
> > + } else {
> > + ret1 = strbuf_addf(&args->long_string, "%s=0..%llu", name,
> > + ULLONG_MAX >> (64 - num_bits));
> > + if (args->num_formats < 4) {
> > + ret2 = strbuf_addf(&args->short_string, "%s=0.%llu", name,
> > + ULLONG_MAX >> (64 - num_bits));
> > + }
> > + }
> > + return ret1 < 0 ? ret1 : (ret2 < 0 ? ret2 : 0);
> > +}
> > +
> > +void perf_pmus__print_raw_pmu_events(const struct print_callbacks *print_cb, void *print_state)
> > +{
> > + bool skip_duplicate_pmus = print_cb->skip_duplicate_pmus(print_state);
> > + struct perf_pmu *(*scan_fn)(struct perf_pmu *);
> > + struct perf_pmu *pmu = NULL;
> > +
> > + if (skip_duplicate_pmus)
> > + scan_fn = perf_pmus__scan_skip_duplicates;
> > + else
> > + scan_fn = perf_pmus__scan;
> > +
> > + while ((pmu = scan_fn(pmu)) != NULL) {
> > + struct build_format_string_args format_args = {
> > + .short_string = STRBUF_INIT,
> > + .long_string = STRBUF_INIT,
> > + .num_formats = 0,
> > + };
> > + int len = pmu_name_len_no_suffix(pmu->name, /*num=*/NULL);
> > + const char *desc = "(see 'man perf-list' or 'man perf-record' on how to encode it)";
> > +
> > + if (!pmu->is_core)
> > + desc = NULL;
> > +
> > + strbuf_addf(&format_args.short_string, "%.*s/", len, pmu->name);
> > + strbuf_addf(&format_args.long_string, "%.*s/", len, pmu->name);
> > + perf_pmu__for_each_format(pmu, &format_args, build_format_string);
> > +
> > + if (format_args.num_formats > 3)
> > + strbuf_addf(&format_args.short_string, ",.../modifier");
> > + else
> > + strbuf_addf(&format_args.short_string, "/modifier");
> > +
> > + strbuf_addf(&format_args.long_string, "/modifier");
> > + print_cb->print_event(print_state,
> > + /*topic=*/NULL,
> > + /*pmu_name=*/NULL,
> > + format_args.short_string.buf,
> > + /*event_alias=*/NULL,
> > + /*scale_unit=*/NULL,
> > + /*deprecated=*/false,
> > + "Raw hardware event descriptor",
> > + desc,
> > + /*long_desc=*/NULL,
> > + format_args.long_string.buf);
> > +
> > + strbuf_release(&format_args.short_string);
> > + strbuf_release(&format_args.long_string);
> > + }
> > +}
> > +
> > bool perf_pmus__have_event(const char *pname, const char *name)
> > {
> > struct perf_pmu *pmu = perf_pmus__find(pname);
> > diff --git a/tools/perf/util/pmus.h b/tools/perf/util/pmus.h
> > index 94d2a08d894b..eec599d8aebd 100644
> > --- a/tools/perf/util/pmus.h
> > +++ b/tools/perf/util/pmus.h
> > @@ -18,6 +18,7 @@ struct perf_pmu *perf_pmus__scan_core(struct perf_pmu *pmu);
> > const struct perf_pmu *perf_pmus__pmu_for_pmu_filter(const char *str);
> >
> > void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *print_state);
> > +void perf_pmus__print_raw_pmu_events(const struct print_callbacks *print_cb, void *print_state);
> > bool perf_pmus__have_event(const char *pname, const char *name);
> > int perf_pmus__num_core_pmus(void);
> > bool perf_pmus__supports_extended_type(void);
> > diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c
> > index e0d2b49bab66..3a7f14fe2390 100644
> > --- a/tools/perf/util/print-events.c
> > +++ b/tools/perf/util/print-events.c
> > @@ -416,8 +416,6 @@ void print_symbol_events(const struct print_callbacks *print_cb, void *print_sta
> > */
> > void print_events(const struct print_callbacks *print_cb, void *print_state)
> > {
> > - char *tmp;
> > -
> > print_symbol_events(print_cb, print_state, PERF_TYPE_HARDWARE,
> > event_symbols_hw, PERF_COUNT_HW_MAX);
> > print_symbol_events(print_cb, print_state, PERF_TYPE_SOFTWARE,
> > @@ -441,21 +439,7 @@ void print_events(const struct print_callbacks *print_cb, void *print_state)
> > /*long_desc=*/NULL,
> > /*encoding_desc=*/NULL);
> >
> > - if (asprintf(&tmp, "%s/t1=v1[,t2=v2,t3 ...]/modifier",
> > - perf_pmus__scan_core(/*pmu=*/NULL)->name) > 0) {
> > - print_cb->print_event(print_state,
> > - /*topic=*/NULL,
> > - /*pmu_name=*/NULL,
> > - tmp,
> > - /*event_alias=*/NULL,
> > - /*scale_unit=*/NULL,
> > - /*deprecated=*/false,
> > - event_type_descriptors[PERF_TYPE_RAW],
> > - "(see 'man perf-list' on how to encode it)",
> > - /*long_desc=*/NULL,
> > - /*encoding_desc=*/NULL);
> > - free(tmp);
> > - }
> > + perf_pmus__print_raw_pmu_events(print_cb, print_state);
> >
> > print_cb->print_event(print_state,
> > /*topic=*/NULL,