[PATCH v3 0/6] Extra verbose/perf-list details

[PATCH v3 0/6] Extra verbose/perf-list details

[Ian Rogers]

Add more encoding detail and raw event details in perf list. Add PMU
name and reverse lookup from config to event name to
perf_event_attr_fprintf. This makes the verbose output easier to read,
and the perf list information more specific.

v3. Fix to reverse lookup to ensure or aliases are loaded and if
    getting the config value fails for an event/alias just continue to
    the next one.
v2. Address feedback from Kan Liang, "Raw hardware event descriptor"
    becomes "Raw event descriptor" add assert to keep term numbers in
    sync, fix a commit message.

Ian Rogers (6):
  perf list: Add tracepoint encoding to detailed output
  perf pmu: Drop "default_core" from alias names
  perf list: Allow wordwrap to wrap on commas
  perf list: Give more details about raw event encodings
  perf tools: Use pmus to describe type from attribute
  perf tools: Add/use PMU reverse lookup from config to name

 tools/perf/builtin-list.c                 | 21 ++++-
 tools/perf/util/perf_event_attr_fprintf.c | 26 +++++--
 tools/perf/util/pmu.c                     | 82 +++++++++++++++++++-
 tools/perf/util/pmu.h                     |  4 +
 tools/perf/util/pmus.c                    | 94 +++++++++++++++++++++++
 tools/perf/util/pmus.h                    |  1 +
 tools/perf/util/print-events.c            | 55 +++++++------
 7 files changed, 242 insertions(+), 41 deletions(-)

-- 
2.44.0.278.ge034bb2e1d-goog

[PATCH v3 1/6] perf list: Add tracepoint encoding to detailed output

[Ian Rogers]

The tracepoint id holds the config value and is probed in determining
what an event is. Add reading of the id so that we can display the
event encoding as:

```
$ perf list --details
...
  alarmtimer:alarmtimer_cancel                       [Tracepoint event]
        tracepoint/config=0x18c/
...
```

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/print-events.c | 35 ++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c
index 7b54e9385442..e0d2b49bab66 100644
--- a/tools/perf/util/print-events.c
+++ b/tools/perf/util/print-events.c
@@ -9,6 +9,7 @@
 #include <unistd.h>
 
 #include <api/fs/tracing_path.h>
+#include <api/io.h>
 #include <linux/stddef.h>
 #include <linux/perf_event.h>
 #include <linux/zalloc.h>
@@ -92,34 +93,48 @@ void print_tracepoint_events(const struct print_callbacks *print_cb __maybe_unus
 
 		evt_items = scandirat(events_fd, sys_dirent->d_name, &evt_namelist, NULL, alphasort);
 		for (int j = 0; j < evt_items; j++) {
+			/*
+			 * Buffer sized at twice the max filename length + 1
+			 * separator + 1 \0 terminator.
+			 */
+			char buf[NAME_MAX * 2 + 2];
+			/* 16 possible hex digits and 22 other characters and \0. */
+			char encoding[16 + 22];
 			struct dirent *evt_dirent = evt_namelist[j];
-			char evt_path[MAXPATHLEN];
-			int evt_fd;
+			struct io id;
+			__u64 config;
 
 			if (evt_dirent->d_type != DT_DIR ||
 			    !strcmp(evt_dirent->d_name, ".") ||
 			    !strcmp(evt_dirent->d_name, ".."))
 				goto next_evt;
 
-			snprintf(evt_path, sizeof(evt_path), "%s/id", evt_dirent->d_name);
-			evt_fd = openat(dir_fd, evt_path, O_RDONLY);
-			if (evt_fd < 0)
+			snprintf(buf, sizeof(buf), "%s/id", evt_dirent->d_name);
+			io__init(&id, openat(dir_fd, buf, O_RDONLY), buf, sizeof(buf));
+
+			if (id.fd < 0)
+				goto next_evt;
+
+			if (io__get_dec(&id, &config) < 0) {
+				close(id.fd);
 				goto next_evt;
-			close(evt_fd);
+			}
+			close(id.fd);
 
-			snprintf(evt_path, MAXPATHLEN, "%s:%s",
+			snprintf(buf, sizeof(buf), "%s:%s",
 				 sys_dirent->d_name, evt_dirent->d_name);
+			snprintf(encoding, sizeof(encoding), "tracepoint/config=0x%llx/", config);
 			print_cb->print_event(print_state,
 					/*topic=*/NULL,
-					/*pmu_name=*/NULL,
-					evt_path,
+					/*pmu_name=*/NULL, /* really "tracepoint" */
+					/*event_name=*/buf,
 					/*event_alias=*/NULL,
 					/*scale_unit=*/NULL,
 					/*deprecated=*/false,
 					"Tracepoint event",
 					/*desc=*/NULL,
 					/*long_desc=*/NULL,
-					/*encoding_desc=*/NULL);
+					encoding);
 next_evt:
 			free(evt_namelist[j]);
 		}
-- 
2.44.0.278.ge034bb2e1d-goog

[PATCH v3 2/6] perf pmu: Drop "default_core" from alias names

[Ian Rogers]

"default_core" is used by jevents.py for json events' PMU name when
none is specified. On x86 the "default_core" is typically the PMU
"cpu". When creating an alias see if the event's PMU name is
"default_core" in which case don't record it. This means in places
like "perf list" the PMU's name will be used in its place.

Before:
```
$ perf list --details
...
cache:
  l1d.replacement
       [Counts the number of cache lines replaced in L1 data cache]
        default_core/event=0x51,period=0x186a3,umask=0x1/
...
```

After:
```
$ perf list --details
...
cache:
  l1d.replacement
       [Counts the number of cache lines replaced in L1 data cache. Unit: cpu]
        cpu/event=0x51,period=0x186a3,umask=0x1/
...
```

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/pmu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index f39cbbc1a7ec..24be587e3537 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -518,7 +518,8 @@ static int perf_pmu__new_alias(struct perf_pmu *pmu, const char *name,
 		unit = pe->unit;
 		perpkg = pe->perpkg;
 		deprecated = pe->deprecated;
-		pmu_name = pe->pmu;
+		if (pe->pmu && strcmp(pe->pmu, "default_core"))
+			pmu_name = pe->pmu;
 	}
 
 	alias = zalloc(sizeof(*alias));
-- 
2.44.0.278.ge034bb2e1d-goog

[PATCH v3 3/6] perf list: Allow wordwrap to wrap on commas

[Ian Rogers]

A raw event encoding may be a block with terms separated by commas. If
wrapping such a string it would be useful to break at the commas, so
add this ability to wordwrap.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-list.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 02bf608d585e..e0fe3d178d63 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -22,6 +22,7 @@
 #include <subcmd/pager.h>
 #include <subcmd/parse-options.h>
 #include <linux/zalloc.h>
+#include <ctype.h>
 #include <stdarg.h>
 #include <stdio.h>
 
@@ -76,26 +77,38 @@ static void default_print_start(void *ps)
 
 static void default_print_end(void *print_state __maybe_unused) {}
 
+static const char *skip_spaces_or_commas(const char *str)
+{
+	while (isspace(*str) || *str == ',')
+		++str;
+	return str;
+}
+
 static void wordwrap(FILE *fp, const char *s, int start, int max, int corr)
 {
 	int column = start;
 	int n;
 	bool saw_newline = false;
+	bool comma = false;
 
 	while (*s) {
-		int wlen = strcspn(s, " \t\n");
+		int wlen = strcspn(s, " ,\t\n");
+		const char *sep = comma ? "," : " ";
 
 		if ((column + wlen >= max && column > start) || saw_newline) {
-			fprintf(fp, "\n%*s", start, "");
+			fprintf(fp, comma ? ",\n%*s" : "\n%*s", start, "");
 			column = start + corr;
 		}
-		n = fprintf(fp, "%s%.*s", column > start ? " " : "", wlen, s);
+		if (column <= start)
+			sep = "";
+		n = fprintf(fp, "%s%.*s", sep, wlen, s);
 		if (n <= 0)
 			break;
 		saw_newline = s[wlen] == '\n';
 		s += wlen;
+		comma = s[0] == ',';
 		column += n;
-		s = skip_spaces(s);
+		s = skip_spaces_or_commas(s);
 	}
 }
 
-- 
2.44.0.278.ge034bb2e1d-goog

[PATCH v3 4/6] perf list: Give more details about raw event encodings

[Ian Rogers]

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 event descriptor]
  cpu/event=0..255,pc,edge,.../modifier              [Raw event descriptor]
       [(see 'man perf-list' or 'man perf-record' on how to encode it)]
  breakpoint//modifier                               [Raw event descriptor]
  cstate_core/event=0..0xffffffffffffffff/modifier   [Raw event descriptor]
  cstate_pkg/event=0..0xffffffffffffffff/modifier    [Raw event descriptor]
  i915/i915_eventid=0..0x1fffff/modifier             [Raw event descriptor]
  intel_bts//modifier                                [Raw event descriptor]
  intel_pt/ptw,event,cyc_thresh=0..15,.../modifier   [Raw event descriptor]
  kprobe/retprobe/modifier                           [Raw event descriptor]
  msr/event=0..0xffffffffffffffff/modifier           [Raw event descriptor]
  power/event=0..255/modifier                        [Raw event descriptor]
  software//modifier                                 [Raw event descriptor]
  tracepoint//modifier                               [Raw event descriptor]
  uncore_arb/event=0..255,edge,inv,.../modifier      [Raw event descriptor]
  uncore_cbox/event=0..255,edge,inv,.../modifier     [Raw event descriptor]
  uncore_clock/event=0..255/modifier                 [Raw event descriptor]
  uncore_imc_free_running/event=0..255,umask=0..255/modifier[Raw event descriptor]
  uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier[Raw 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 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 event descriptor]
        breakpoint//modifier
  cstate_core/event=0..0xffffffffffffffff/modifier   [Raw event descriptor]
        cstate_core/event=0..0xffffffffffffffff/modifier
  cstate_pkg/event=0..0xffffffffffffffff/modifier    [Raw event descriptor]
        cstate_pkg/event=0..0xffffffffffffffff/modifier
  i915/i915_eventid=0..0x1fffff/modifier             [Raw event descriptor]
        i915/i915_eventid=0..0x1fffff/modifier
  intel_bts//modifier                                [Raw event descriptor]
        intel_bts//modifier
  intel_pt/ptw,event,cyc_thresh=0..15,.../modifier   [Raw 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 event descriptor]
        kprobe/retprobe/modifier
  msr/event=0..0xffffffffffffffff/modifier           [Raw event descriptor]
        msr/event=0..0xffffffffffffffff/modifier
  power/event=0..255/modifier                        [Raw event descriptor]
        power/event=0..255/modifier
  software//modifier                                 [Raw event descriptor]
        software//modifier
  tracepoint//modifier                               [Raw event descriptor]
        tracepoint//modifier
  uncore_arb/event=0..255,edge,inv,.../modifier      [Raw 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 event descriptor]
        uncore_cbox/event=0..255,edge,inv,umask=0..255,cmask=0..31/modifier
  uncore_clock/event=0..255/modifier                 [Raw event descriptor]
        uncore_clock/event=0..255/modifier
  uncore_imc_free_running/event=0..255,umask=0..255/modifier[Raw event descriptor]
        uncore_imc_free_running/event=0..255,umask=0..255/modifier
  uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier[Raw event descriptor]
        uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier
```

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/pmu.c          | 61 +++++++++++++++++++++-
 tools/perf/util/pmu.h          |  3 ++
 tools/perf/util/pmus.c         | 94 ++++++++++++++++++++++++++++++++++
 tools/perf/util/pmus.h         |  1 +
 tools/perf/util/print-events.c | 20 +-------
 5 files changed, 160 insertions(+), 19 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 24be587e3537..e24bc3b8f696 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1603,6 +1603,61 @@ 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",
+	};
+	struct perf_pmu_format *format;
+	int ret;
+
+	/*
+	 * max-events and driver-config are missing above as are the internal
+	 * types user, metric-id, raw, legacy cache and hardware. Assert against
+	 * the enum parse_events__term_type so they are kept in sync.
+	 */
+	_Static_assert(ARRAY_SIZE(terms) == __PARSE_EVENTS__TERM_TYPE_NR - 6);
+	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 +1752,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 +1782,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..2fd369e45832 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 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..3f38c27f0157 100644
--- a/tools/perf/util/print-events.c
+++ b/tools/perf/util/print-events.c
@@ -39,7 +39,7 @@ static const char * const event_type_descriptors[] = {
 	"Software event",
 	"Tracepoint event",
 	"Hardware cache event",
-	"Raw hardware event descriptor",
+	"Raw event descriptor",
 	"Hardware breakpoint",
 };
 
@@ -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,
-- 
2.44.0.278.ge034bb2e1d-goog

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

[Ian Rogers]

On Thu, Mar 7, 2024 at 4:19 PM Ian Rogers <irogers@google.com> 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 event descriptor]
>   cpu/event=0..255,pc,edge,.../modifier              [Raw event descriptor]
>        [(see 'man perf-list' or 'man perf-record' on how to encode it)]
>   breakpoint//modifier                               [Raw event descriptor]
>   cstate_core/event=0..0xffffffffffffffff/modifier   [Raw event descriptor]
>   cstate_pkg/event=0..0xffffffffffffffff/modifier    [Raw event descriptor]
>   i915/i915_eventid=0..0x1fffff/modifier             [Raw event descriptor]
>   intel_bts//modifier                                [Raw event descriptor]
>   intel_pt/ptw,event,cyc_thresh=0..15,.../modifier   [Raw event descriptor]
>   kprobe/retprobe/modifier                           [Raw event descriptor]
>   msr/event=0..0xffffffffffffffff/modifier           [Raw event descriptor]
>   power/event=0..255/modifier                        [Raw event descriptor]
>   software//modifier                                 [Raw event descriptor]
>   tracepoint//modifier                               [Raw event descriptor]
>   uncore_arb/event=0..255,edge,inv,.../modifier      [Raw event descriptor]
>   uncore_cbox/event=0..255,edge,inv,.../modifier     [Raw event descriptor]
>   uncore_clock/event=0..255/modifier                 [Raw event descriptor]
>   uncore_imc_free_running/event=0..255,umask=0..255/modifier[Raw event descriptor]
>   uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier[Raw 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 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 event descriptor]
>         breakpoint//modifier
>   cstate_core/event=0..0xffffffffffffffff/modifier   [Raw event descriptor]
>         cstate_core/event=0..0xffffffffffffffff/modifier
>   cstate_pkg/event=0..0xffffffffffffffff/modifier    [Raw event descriptor]
>         cstate_pkg/event=0..0xffffffffffffffff/modifier
>   i915/i915_eventid=0..0x1fffff/modifier             [Raw event descriptor]
>         i915/i915_eventid=0..0x1fffff/modifier
>   intel_bts//modifier                                [Raw event descriptor]
>         intel_bts//modifier
>   intel_pt/ptw,event,cyc_thresh=0..15,.../modifier   [Raw 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 event descriptor]
>         kprobe/retprobe/modifier
>   msr/event=0..0xffffffffffffffff/modifier           [Raw event descriptor]
>         msr/event=0..0xffffffffffffffff/modifier
>   power/event=0..255/modifier                        [Raw event descriptor]
>         power/event=0..255/modifier
>   software//modifier                                 [Raw event descriptor]
>         software//modifier
>   tracepoint//modifier                               [Raw event descriptor]
>         tracepoint//modifier
>   uncore_arb/event=0..255,edge,inv,.../modifier      [Raw 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 event descriptor]
>         uncore_cbox/event=0..255,edge,inv,umask=0..255,cmask=0..31/modifier
>   uncore_clock/event=0..255/modifier                 [Raw event descriptor]
>         uncore_clock/event=0..255/modifier
>   uncore_imc_free_running/event=0..255,umask=0..255/modifier[Raw event descriptor]
>         uncore_imc_free_running/event=0..255,umask=0..255/modifier
>   uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier[Raw event descriptor]
>         uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier
> ```
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/util/pmu.c          | 61 +++++++++++++++++++++-
>  tools/perf/util/pmu.h          |  3 ++
>  tools/perf/util/pmus.c         | 94 ++++++++++++++++++++++++++++++++++
>  tools/perf/util/pmus.h         |  1 +
>  tools/perf/util/print-events.c | 20 +-------
>  5 files changed, 160 insertions(+), 19 deletions(-)
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 24be587e3537..e24bc3b8f696 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -1603,6 +1603,61 @@ 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",
> +       };
> +       struct perf_pmu_format *format;
> +       int ret;
> +
> +       /*
> +        * max-events and driver-config are missing above as are the internal
> +        * types user, metric-id, raw, legacy cache and hardware. Assert against
> +        * the enum parse_events__term_type so they are kept in sync.
> +        */
> +       _Static_assert(ARRAY_SIZE(terms) == __PARSE_EVENTS__TERM_TYPE_NR - 6);

For C11 it is required that a 2nd message argument be passed:
https://en.cppreference.com/w/c/language/_Static_assert

This line needs to be something like:
_Static_assert(ARRAY_SIZE(terms) == __PARSE_EVENTS__TERM_TYPE_NR - 6,
"Unexpected array size");

Let me know if we I should send a fix, resend all the patches or if
you can fix in tmp.perf-tools-next.

Thanks,
Ian

> +       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 +1752,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 +1782,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..2fd369e45832 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 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..3f38c27f0157 100644
> --- a/tools/perf/util/print-events.c
> +++ b/tools/perf/util/print-events.c
> @@ -39,7 +39,7 @@ static const char * const event_type_descriptors[] = {
>         "Software event",
>         "Tracepoint event",
>         "Hardware cache event",
> -       "Raw hardware event descriptor",
> +       "Raw event descriptor",
>         "Hardware breakpoint",
>  };
>
> @@ -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,
> --
> 2.44.0.278.ge034bb2e1d-goog
>

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

[Arnaldo Carvalho de Melo]

On Wed, Mar 20, 2024 at 07:59:06PM -0700, Ian Rogers wrote:
> On Thu, Mar 7, 2024 at 4:19 PM Ian Rogers <irogers@google.com> 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 event descriptor]
> >   cpu/event=0..255,pc,edge,.../modifier              [Raw event descriptor]
> >        [(see 'man perf-list' or 'man perf-record' on how to encode it)]
> >   breakpoint//modifier                               [Raw event descriptor]
> >   cstate_core/event=0..0xffffffffffffffff/modifier   [Raw event descriptor]
> >   cstate_pkg/event=0..0xffffffffffffffff/modifier    [Raw event descriptor]
> >   i915/i915_eventid=0..0x1fffff/modifier             [Raw event descriptor]
> >   intel_bts//modifier                                [Raw event descriptor]
> >   intel_pt/ptw,event,cyc_thresh=0..15,.../modifier   [Raw event descriptor]
> >   kprobe/retprobe/modifier                           [Raw event descriptor]
> >   msr/event=0..0xffffffffffffffff/modifier           [Raw event descriptor]
> >   power/event=0..255/modifier                        [Raw event descriptor]
> >   software//modifier                                 [Raw event descriptor]
> >   tracepoint//modifier                               [Raw event descriptor]
> >   uncore_arb/event=0..255,edge,inv,.../modifier      [Raw event descriptor]
> >   uncore_cbox/event=0..255,edge,inv,.../modifier     [Raw event descriptor]
> >   uncore_clock/event=0..255/modifier                 [Raw event descriptor]
> >   uncore_imc_free_running/event=0..255,umask=0..255/modifier[Raw event descriptor]
> >   uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier[Raw 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 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 event descriptor]
> >         breakpoint//modifier
> >   cstate_core/event=0..0xffffffffffffffff/modifier   [Raw event descriptor]
> >         cstate_core/event=0..0xffffffffffffffff/modifier
> >   cstate_pkg/event=0..0xffffffffffffffff/modifier    [Raw event descriptor]
> >         cstate_pkg/event=0..0xffffffffffffffff/modifier
> >   i915/i915_eventid=0..0x1fffff/modifier             [Raw event descriptor]
> >         i915/i915_eventid=0..0x1fffff/modifier
> >   intel_bts//modifier                                [Raw event descriptor]
> >         intel_bts//modifier
> >   intel_pt/ptw,event,cyc_thresh=0..15,.../modifier   [Raw 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 event descriptor]
> >         kprobe/retprobe/modifier
> >   msr/event=0..0xffffffffffffffff/modifier           [Raw event descriptor]
> >         msr/event=0..0xffffffffffffffff/modifier
> >   power/event=0..255/modifier                        [Raw event descriptor]
> >         power/event=0..255/modifier
> >   software//modifier                                 [Raw event descriptor]
> >         software//modifier
> >   tracepoint//modifier                               [Raw event descriptor]
> >         tracepoint//modifier
> >   uncore_arb/event=0..255,edge,inv,.../modifier      [Raw 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 event descriptor]
> >         uncore_cbox/event=0..255,edge,inv,umask=0..255,cmask=0..31/modifier
> >   uncore_clock/event=0..255/modifier                 [Raw event descriptor]
> >         uncore_clock/event=0..255/modifier
> >   uncore_imc_free_running/event=0..255,umask=0..255/modifier[Raw event descriptor]
> >         uncore_imc_free_running/event=0..255,umask=0..255/modifier
> >   uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier[Raw event descriptor]
> >         uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier
> > ```
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/util/pmu.c          | 61 +++++++++++++++++++++-
> >  tools/perf/util/pmu.h          |  3 ++
> >  tools/perf/util/pmus.c         | 94 ++++++++++++++++++++++++++++++++++
> >  tools/perf/util/pmus.h         |  1 +
> >  tools/perf/util/print-events.c | 20 +-------
> >  5 files changed, 160 insertions(+), 19 deletions(-)
> >
> > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> > index 24be587e3537..e24bc3b8f696 100644
> > --- a/tools/perf/util/pmu.c
> > +++ b/tools/perf/util/pmu.c
> > @@ -1603,6 +1603,61 @@ 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",
> > +       };
> > +       struct perf_pmu_format *format;
> > +       int ret;
> > +
> > +       /*
> > +        * max-events and driver-config are missing above as are the internal
> > +        * types user, metric-id, raw, legacy cache and hardware. Assert against
> > +        * the enum parse_events__term_type so they are kept in sync.
> > +        */
> > +       _Static_assert(ARRAY_SIZE(terms) == __PARSE_EVENTS__TERM_TYPE_NR - 6);
> 
> For C11 it is required that a 2nd message argument be passed:
> https://en.cppreference.com/w/c/language/_Static_assert
> 
> This line needs to be something like:
> _Static_assert(ARRAY_SIZE(terms) == __PARSE_EVENTS__TERM_TYPE_NR - 6,
> "Unexpected array size");
> 
> Let me know if we I should send a fix, resend all the patches or if
> you can fix in tmp.perf-tools-next.

I added this, no need to resend.

- Arnaldo

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index e24bc3b8f696a9f5..81952c6cd3c6f5e8 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1635,7 +1635,8 @@ int perf_pmu__for_each_format(struct perf_pmu *pmu, void *state, pmu_format_call
 	 * types user, metric-id, raw, legacy cache and hardware. Assert against
 	 * the enum parse_events__term_type so they are kept in sync.
 	 */
-	_Static_assert(ARRAY_SIZE(terms) == __PARSE_EVENTS__TERM_TYPE_NR - 6);
+	_Static_assert(ARRAY_SIZE(terms) == __PARSE_EVENTS__TERM_TYPE_NR - 6,
+		       "perf_pmu__for_each_format()'s terms must be kept in sync with enum parse_events__term_type");
 	list_for_each_entry(format, &pmu->format, list) {
 		perf_pmu_format__load(pmu, format);
 		ret = cb(state, format->name, (int)format->value, format->bits);

[PATCH v3 5/6] perf tools: Use pmus to describe type from attribute

[Ian Rogers]

When dumping a perf_event_attr, use pmus to find the PMU and its name
by the type number. This allows dynamically added PMUs to be described.

Before:
```
$ perf stat -vv -e data_read true
...
perf_event_attr:
  type                             24
  size                             136
  config                           0x20ff
  sample_type                      IDENTIFIER
  read_format                      TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
  disabled                         1
  inherit                          1
  exclude_guest                    1
...
```

After:
```
$ perf stat -vv -e data_read true
...
perf_event_attr:
  type                             24 (uncore_imc_free_running_0)
  size                             136
  config                           0x20ff
  sample_type                      IDENTIFIER
  read_format                      TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
  disabled                         1
  inherit                          1
  exclude_guest                    1
...
```

However, it also means that when we have a PMU name we prefer it to a
hard coded name:

Before:
```
$ perf stat -vv -e faults true
...
perf_event_attr:
  type                             1 (PERF_TYPE_SOFTWARE)
  size                             136
  config                           0x2 (PERF_COUNT_SW_PAGE_FAULTS)
  sample_type                      IDENTIFIER
  read_format                      TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
  disabled                         1
  inherit                          1
  enable_on_exec                   1
  exclude_guest                    1
...
```

After:
```
$ perf stat -vv -e faults true
...
perf_event_attr:
  type                             1 (software)
  size                             136
  config                           0x2 (PERF_COUNT_SW_PAGE_FAULTS)
  sample_type                      IDENTIFIER
  read_format                      TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
  disabled                         1
  inherit                          1
  enable_on_exec                   1
  exclude_guest                    1
...
```

It feels more consistent to do this, rather than only prefer a PMU
name when a hard coded name isn't available.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/perf_event_attr_fprintf.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/perf_event_attr_fprintf.c b/tools/perf/util/perf_event_attr_fprintf.c
index 8f04d3b7f3ec..29e66835da3a 100644
--- a/tools/perf/util/perf_event_attr_fprintf.c
+++ b/tools/perf/util/perf_event_attr_fprintf.c
@@ -7,6 +7,8 @@
 #include <linux/types.h>
 #include <linux/perf_event.h>
 #include "util/evsel_fprintf.h"
+#include "util/pmu.h"
+#include "util/pmus.h"
 #include "trace-event.h"
 
 struct bit_names {
@@ -75,9 +77,12 @@ static void __p_read_format(char *buf, size_t size, u64 value)
 }
 
 #define ENUM_ID_TO_STR_CASE(x) case x: return (#x);
-static const char *stringify_perf_type_id(u64 value)
+static const char *stringify_perf_type_id(struct perf_pmu *pmu, u32 type)
 {
-	switch (value) {
+	if (pmu)
+		return pmu->name;
+
+	switch (type) {
 	ENUM_ID_TO_STR_CASE(PERF_TYPE_HARDWARE)
 	ENUM_ID_TO_STR_CASE(PERF_TYPE_SOFTWARE)
 	ENUM_ID_TO_STR_CASE(PERF_TYPE_TRACEPOINT)
@@ -175,9 +180,9 @@ do {								\
 #define print_id_unsigned(_s)	PRINT_ID(_s, "%"PRIu64)
 #define print_id_hex(_s)	PRINT_ID(_s, "%#"PRIx64)
 
-static void __p_type_id(char *buf, size_t size, u64 value)
+static void __p_type_id(struct perf_pmu *pmu, char *buf, size_t size, u64 value)
 {
-	print_id_unsigned(stringify_perf_type_id(value));
+	print_id_unsigned(stringify_perf_type_id(pmu, value));
 }
 
 static void __p_config_hw_id(char *buf, size_t size, u64 value)
@@ -246,7 +251,7 @@ static void __p_config_id(char *buf, size_t size, u32 type, u64 value)
 #define p_sample_type(val)	__p_sample_type(buf, BUF_SIZE, val)
 #define p_branch_sample_type(val) __p_branch_sample_type(buf, BUF_SIZE, val)
 #define p_read_format(val)	__p_read_format(buf, BUF_SIZE, val)
-#define p_type_id(val)		__p_type_id(buf, BUF_SIZE, val)
+#define p_type_id(val)		__p_type_id(pmu, buf, BUF_SIZE, val)
 #define p_config_id(val)	__p_config_id(buf, BUF_SIZE, attr->type, val)
 
 #define PRINT_ATTRn(_n, _f, _p, _a)			\
@@ -262,6 +267,7 @@ do {							\
 int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
 			     attr__fprintf_f attr__fprintf, void *priv)
 {
+	struct perf_pmu *pmu = perf_pmus__find_by_type(attr->type);
 	char buf[BUF_SIZE];
 	int ret = 0;
 
-- 
2.44.0.278.ge034bb2e1d-goog

[PATCH v3 6/6] perf tools: Add/use PMU reverse lookup from config to name

[Ian Rogers]

Add perf_pmu__name_from_config that does a reverse lookup from a
config number to an alias name. The lookup is expensive as the config
is computed for every alias by filling in a perf_event_attr, but this
is only done when verbose output is enabled. The lookup also only
considers config, and not config1, config2 or config3.

An example of the output:
```
$ perf stat -vv -e data_read true
...
perf_event_attr:
  type                             24 (uncore_imc_free_running_0)
  size                             136
  config                           0x20ff (data_read)
  sample_type                      IDENTIFIER
  read_format                      TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
  disabled                         1
  inherit                          1
  exclude_guest                    1
...
```

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/perf_event_attr_fprintf.c | 10 ++++++++--
 tools/perf/util/pmu.c                     | 18 ++++++++++++++++++
 tools/perf/util/pmu.h                     |  1 +
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/perf_event_attr_fprintf.c b/tools/perf/util/perf_event_attr_fprintf.c
index 29e66835da3a..59fbbba79697 100644
--- a/tools/perf/util/perf_event_attr_fprintf.c
+++ b/tools/perf/util/perf_event_attr_fprintf.c
@@ -222,8 +222,14 @@ static void __p_config_tracepoint_id(char *buf, size_t size, u64 value)
 }
 #endif
 
-static void __p_config_id(char *buf, size_t size, u32 type, u64 value)
+static void __p_config_id(struct perf_pmu *pmu, char *buf, size_t size, u32 type, u64 value)
 {
+	const char *name = perf_pmu__name_from_config(pmu, value);
+
+	if (name) {
+		print_id_hex(name);
+		return;
+	}
 	switch (type) {
 	case PERF_TYPE_HARDWARE:
 		return __p_config_hw_id(buf, size, value);
@@ -252,7 +258,7 @@ static void __p_config_id(char *buf, size_t size, u32 type, u64 value)
 #define p_branch_sample_type(val) __p_branch_sample_type(buf, BUF_SIZE, val)
 #define p_read_format(val)	__p_read_format(buf, BUF_SIZE, val)
 #define p_type_id(val)		__p_type_id(pmu, buf, BUF_SIZE, val)
-#define p_config_id(val)	__p_config_id(buf, BUF_SIZE, attr->type, val)
+#define p_config_id(val)	__p_config_id(pmu, buf, BUF_SIZE, attr->type, val)
 
 #define PRINT_ATTRn(_n, _f, _p, _a)			\
 do {							\
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index e24bc3b8f696..97ad299f463f 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -2145,3 +2145,21 @@ void perf_pmu__delete(struct perf_pmu *pmu)
 	zfree(&pmu->id);
 	free(pmu);
 }
+
+const char *perf_pmu__name_from_config(struct perf_pmu *pmu, u64 config)
+{
+	struct perf_pmu_alias *event;
+
+	if (!pmu)
+		return NULL;
+
+	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);
+
+		if (ret == 0 && config == attr.config)
+			return event->name;
+	}
+	return NULL;
+}
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 9f5284b29ecf..152700f78455 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -276,5 +276,6 @@ struct perf_pmu *perf_pmu__lookup(struct list_head *pmus, int dirfd, const char
 struct perf_pmu *perf_pmu__create_placeholder_core_pmu(struct list_head *core_pmus);
 void perf_pmu__delete(struct perf_pmu *pmu);
 struct perf_pmu *perf_pmus__find_core_pmu(void);
+const char *perf_pmu__name_from_config(struct perf_pmu *pmu, u64 config);
 
 #endif /* __PMU_H */
-- 
2.44.0.278.ge034bb2e1d-goog

Re: [PATCH v3 0/6] Extra verbose/perf-list details

[Liang, Kan]

On 2024-03-07 7:19 p.m., Ian Rogers wrote:
> Add more encoding detail and raw event details in perf list. Add PMU
> name and reverse lookup from config to event name to
> perf_event_attr_fprintf. This makes the verbose output easier to read,
> and the perf list information more specific.
> 
> v3. Fix to reverse lookup to ensure or aliases are loaded and if
>     getting the config value fails for an event/alias just continue to
>     the next one.
> v2. Address feedback from Kan Liang, "Raw hardware event descriptor"
>     becomes "Raw event descriptor" add assert to keep term numbers in
>     sync, fix a commit message.
> 
> Ian Rogers (6):
>   perf list: Add tracepoint encoding to detailed output
>   perf pmu: Drop "default_core" from alias names
>   perf list: Allow wordwrap to wrap on commas
>   perf list: Give more details about raw event encodings
>   perf tools: Use pmus to describe type from attribute
>   perf tools: Add/use PMU reverse lookup from config to name

The patch series look good to me.
I verified it on a hybrid machine. The new format is the same as the
advertise.

Tested-by: Kan Liang <kan.liang@linux.intel.com>

Thanks,
Kan
> 
>  tools/perf/builtin-list.c                 | 21 ++++-
>  tools/perf/util/perf_event_attr_fprintf.c | 26 +++++--
>  tools/perf/util/pmu.c                     | 82 +++++++++++++++++++-
>  tools/perf/util/pmu.h                     |  4 +
>  tools/perf/util/pmus.c                    | 94 +++++++++++++++++++++++
>  tools/perf/util/pmus.h                    |  1 +
>  tools/perf/util/print-events.c            | 55 +++++++------
>  7 files changed, 242 insertions(+), 41 deletions(-)
> 

Re: [PATCH v3 0/6] Extra verbose/perf-list details

[Ian Rogers]

On Fri, Mar 8, 2024 at 8:34 AM Ian Rogers <irogers@google.com> wrote:
>
> On Fri, Mar 8, 2024 at 7:39 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>>
>>
>>
>> On 2024-03-07 7:19 p.m., Ian Rogers wrote:
>> > Add more encoding detail and raw event details in perf list. Add PMU
>> > name and reverse lookup from config to event name to
>> > perf_event_attr_fprintf. This makes the verbose output easier to read,
>> > and the perf list information more specific.
>> >
>> > v3. Fix to reverse lookup to ensure or aliases are loaded and if
>> >     getting the config value fails for an event/alias just continue to
>> >     the next one.
>> > v2. Address feedback from Kan Liang, "Raw hardware event descriptor"
>> >     becomes "Raw event descriptor" add assert to keep term numbers in
>> >     sync, fix a commit message.
>> >
>> > Ian Rogers (6):
>> >   perf list: Add tracepoint encoding to detailed output
>> >   perf pmu: Drop "default_core" from alias names
>> >   perf list: Allow wordwrap to wrap on commas
>> >   perf list: Give more details about raw event encodings
>> >   perf tools: Use pmus to describe type from attribute
>> >   perf tools: Add/use PMU reverse lookup from config to name
>>
>> The patch series look good to me.
>> I verified it on a hybrid machine. The new format is the same as the
>> advertise.
>>
>> Tested-by: Kan Liang <kan.liang@linux.intel.com>

Ping.

Thanks,
Ian

>
> Thanks Kan! Just to add another feature of perf_event_attr_fprintf changes is that it shows up in perf trace output:
>
> ```
> $ perf trace -e perf_event_open perf stat -e data_read true
>      0.000 ( 0.011 ms): :36857/36857 perf_event_open(attr_uptr: { type: 24 (uncore_imc_free_running_0), size: 136, config: 0x20ff (unc_mc0_rdcas_count_freerun), sample_type: IDENTIFIER, read_format: TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING, disabled: 1, inherit: 1, exclude_guest: 1 }, pid: -1, group_fd: -1, flags: FD_CLOEXEC) = -1 EINVAL (Invalid argument)
>      0.014 ( 0.002 ms): :36857/36857 perf_event_open(attr_uptr: { type: 24 (uncore_imc_free_running_0), size: 136, config: 0x20ff (unc_mc0_rdcas_count_freerun), sample_type: IDENTIFIER, read_format: TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING, disabled: 1, inherit: 1, exclude_guest: 1 }, pid: -1, group_fd: -1) = -1 EINVAL (Invalid argument)
>      0.020 ( 0.008 ms): :36857/36857 perf_event_open(attr_uptr: { type: 24 (uncore_imc_free_running_0), size: 136, config: 0x20ff (unc_mc0_rdcas_count_freerun), sample_type: IDENTIFIER, read_format: TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING, disabled: 1, inherit: 1 }, pid: -1, group_fd: -1) = 8
>      0.031 ( 0.006 ms): :36857/36857 perf_event_open(attr_uptr: { type: 25 (uncore_imc_free_running_1), size: 136, config: 0x20ff (unc_mc1_rdcas_count_freerun), sample_type: IDENTIFIER, read_format: TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING, disabled: 1, inherit: 1, exclude_guest: 1 }, pid: -1, group_fd: -1) = -1 EINVAL (Invalid argument)
>      0.044 ( 0.004 ms): :36857/36857 perf_event_open(attr_uptr: { type: 25 (uncore_imc_free_running_1), size: 136, config: 0x20ff (unc_mc1_rdcas_count_freerun), sample_type: IDENTIFIER, read_format: TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING, disabled: 1, inherit: 1 }, pid: -1, group_fd: -1) = 9
>
>  Performance counter stats for 'system wide':
>
>               5.07 MiB  data_read
>
>        0.001178187 seconds time elapsed
> ```
>
> Thanks,
> Ian
>
>>
>> Thanks,
>> Kan
>> >
>> >  tools/perf/builtin-list.c                 | 21 ++++-
>> >  tools/perf/util/perf_event_attr_fprintf.c | 26 +++++--
>> >  tools/perf/util/pmu.c                     | 82 +++++++++++++++++++-
>> >  tools/perf/util/pmu.h                     |  4 +
>> >  tools/perf/util/pmus.c                    | 94 +++++++++++++++++++++++
>> >  tools/perf/util/pmus.h                    |  1 +
>> >  tools/perf/util/print-events.c            | 55 +++++++------
>> >  7 files changed, 242 insertions(+), 41 deletions(-)
>> >

Re: [PATCH v3 0/6] Extra verbose/perf-list details

[Arnaldo Carvalho de Melo]

On Tue, Mar 19, 2024 at 06:01:35PM -0700, Ian Rogers wrote:
> On Fri, Mar 8, 2024 at 8:34 AM Ian Rogers <irogers@google.com> wrote:
> > On Fri, Mar 8, 2024 at 7:39 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
> >> On 2024-03-07 7:19 p.m., Ian Rogers wrote:
> >> > Add more encoding detail and raw event details in perf list. Add PMU
> >> > name and reverse lookup from config to event name to
> >> > perf_event_attr_fprintf. This makes the verbose output easier to read,
> >> > and the perf list information more specific.
> >> >
> >> > v3. Fix to reverse lookup to ensure or aliases are loaded and if
> >> >     getting the config value fails for an event/alias just continue to
> >> >     the next one.
> >> > v2. Address feedback from Kan Liang, "Raw hardware event descriptor"
> >> >     becomes "Raw event descriptor" add assert to keep term numbers in
> >> >     sync, fix a commit message.
> >> >
> >> > Ian Rogers (6):
> >> >   perf list: Add tracepoint encoding to detailed output
> >> >   perf pmu: Drop "default_core" from alias names
> >> >   perf list: Allow wordwrap to wrap on commas
> >> >   perf list: Give more details about raw event encodings
> >> >   perf tools: Use pmus to describe type from attribute
> >> >   perf tools: Add/use PMU reverse lookup from config to name
> >>
> >> The patch series look good to me.
> >> I verified it on a hybrid machine. The new format is the same as the
> >> advertise.
> >>
> >> Tested-by: Kan Liang <kan.liang@linux.intel.com>
> 
> Ping.

Thanks, applied to perf-tools-next,

- Arnaldo