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

[PATCH v1 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.

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                     | 77 ++++++++++++++++++-
 tools/perf/util/pmu.h                     |  4 +
 tools/perf/util/pmus.c                    | 94 +++++++++++++++++++++++
 tools/perf/util/pmus.h                    |  1 +
 tools/perf/util/print-events.c            | 53 +++++++------
 7 files changed, 236 insertions(+), 40 deletions(-)

-- 
2.44.0.278.ge034bb2e1d-goog

[PATCH v1 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 v1 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 v1 3/6] perf list: Allow wordwrap to wrap on commas

[Ian Rogers]

A raw event encoding may be lock 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 v1 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 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]
  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 <irogers@google.com>
---
 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",
+	};
+	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,
-- 
2.44.0.278.ge034bb2e1d-goog

[PATCH v1 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 cycles 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 v1 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                     | 19 +++++++++++++++++++
 tools/perf/util/pmu.h                     |  1 +
 3 files changed, 28 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 904725f03d29..f26b06912a8a 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -2139,3 +2139,22 @@ 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;
+
+	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)
+			return NULL;
+		if (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 v1 4/6] perf list: Give more details about raw event encodings

[Liang, Kan]

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 <irogers@google.com>
> ---
>  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,

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

[Liang, Kan]

On 2024-03-07 3:14 a.m., Ian Rogers wrote:
> 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 cycles true

faults?

Thanks,
Kan
> ...
> 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;
>  

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

[Ian Rogers]

On Thu, Mar 7, 2024 at 6:34 AM Liang, Kan <kan.liang@linux.intel.com> 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 <irogers@google.com>
> > ---
> >  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,

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

[Ian Rogers]

On Thu, Mar 7, 2024 at 6:36 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>
>
>
> On 2024-03-07 3:14 a.m., Ian Rogers wrote:
> > 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 cycles true
>
> faults?

Thanks, will fix in v2. It does highlight we lack a struct pmu in pmus
for legacy types like hardware, hence switching to faults as a
software event that has a PMU. Perhaps we should just make legacy PMUs
anyway, to avoid special case logic. Anyway, not in these changes.

Thanks,
Ian

> Thanks,
> Kan
> > ...
> > 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;
> >