All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/5] perf/amd: Zen4 IBS extensions support (tool changes)
@ 2022-05-19  5:43 Ravi Bangoria
  2022-05-19  5:43 ` [PATCH v3 1/5] perf record ibs: Warn about sampling period skew Ravi Bangoria
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Ravi Bangoria @ 2022-05-19  5:43 UTC (permalink / raw)
  To: acme
  Cc: ravi.bangoria, peterz, rrichter, mingo, mark.rutland, jolsa,
	namhyung, tglx, bp, irogers, james.clark, leo.yan, kan.liang, ak,
	eranian, like.xu.linux, x86, linux-perf-users, linux-kernel,
	sandipan.das, ananth.narayan, kim.phillips, santosh.shukla

Kernel side of changes have already been applied to tip/perf/core.
This series contains only perf tool changes.

v2: https://lore.kernel.org/lkml/20220509044914.1473-1-ravi.bangoria@amd.com
v2->v3:
  - Rename arch_evsel__warn_ambiguity() to arch__post_evsel_config()
  - Optimize arch__post_evsel_config() for non-AMD and non-IBS cases
  - Split header changes into separate patches
  - Rebase on tip/perf/core (841b51e4a3590)

Original cover letter:

IBS support has been enhanced with two new features in upcoming uarch:
1. DataSrc extension and 2. L3 Miss Filtering capability. Both are
indicated by CPUID_Fn8000001B_EAX bit 11.

DataSrc extension provides additional data source details for tagged
load/store operations. Add support for these new bits in perf report/
script raw-dump.

IBS L3 miss filtering works by tagging an instruction on IBS counter
overflow and generating an NMI if the tagged instruction causes an L3
miss. Samples without an L3 miss are discarded and counter is reset
with random value (between 1-15 for fetch pmu and 1-127 for op pmu).
This helps in reducing sampling overhead when user is interested only
in such samples. One of the use case of such filtered samples is to
feed data to page-migration daemon in tiered memory systems.

Add support for L3 miss filtering in IBS driver via new pmu attribute
"l3missonly". Example usage:

  # perf record -a -e ibs_op/l3missonly=1/ --raw-samples sleep 5
  # perf report -D

Some important points to keep in mind while using L3 miss filtering:
1. Hw internally reset sampling period when tagged instruction does
   not cause L3 miss. But there is no way to reconstruct aggregated
   sampling period when this happens.
2. L3 miss is not the actual event being counted. Rather, IBS will
   count fetch, cycles or uOps depending on the configuration. Thus
   sampling period have no direct connection to L3 misses.

1st causes sampling period skew. Thus, I've added warning message at
perf record:

  # perf record -c 10000 -C 0 -e ibs_op/l3missonly=1/
  WARNING: Hw internally resets sampling period when L3 Miss Filtering is enabled
  and tagged operation does not cause L3 Miss. This causes sampling period skew.

User can configure smaller sampling period to get more samples while
using l3missonly.

Ravi Bangoria (5):
  perf record ibs: Warn about sampling period skew
  perf header: Parse non-cpu pmu capabilities
  perf/x86/ibs: Add new IBS register bits into header
  perf tool ibs: Sync amd ibs header file
  perf script ibs: Support new IBS bits in raw trace dump

 arch/x86/include/asm/amd-ibs.h                |  16 +-
 tools/arch/x86/include/asm/amd-ibs.h          |  16 +-
 .../Documentation/perf.data-file-format.txt   |  18 ++
 tools/perf/arch/x86/util/evsel.c              |  50 +++++
 tools/perf/util/amd-sample-raw.c              |  68 +++++-
 tools/perf/util/env.c                         |  48 +++++
 tools/perf/util/env.h                         |  11 +
 tools/perf/util/evsel.c                       |   7 +
 tools/perf/util/evsel.h                       |   1 +
 tools/perf/util/header.c                      | 198 ++++++++++++++++++
 tools/perf/util/header.h                      |   1 +
 tools/perf/util/pmu.c                         |  15 +-
 tools/perf/util/pmu.h                         |   2 +
 13 files changed, 427 insertions(+), 24 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3 1/5] perf record ibs: Warn about sampling period skew
  2022-05-19  5:43 [PATCH v3 0/5] perf/amd: Zen4 IBS extensions support (tool changes) Ravi Bangoria
@ 2022-05-19  5:43 ` Ravi Bangoria
  2022-05-19 22:14   ` Ian Rogers
  2022-05-19  5:43 ` [PATCH v3 2/5] perf header: Parse non-cpu pmu capabilities Ravi Bangoria
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Ravi Bangoria @ 2022-05-19  5:43 UTC (permalink / raw)
  To: acme
  Cc: ravi.bangoria, peterz, rrichter, mingo, mark.rutland, jolsa,
	namhyung, tglx, bp, irogers, james.clark, leo.yan, kan.liang, ak,
	eranian, like.xu.linux, x86, linux-perf-users, linux-kernel,
	sandipan.das, ananth.narayan, kim.phillips, santosh.shukla

Samples without an L3 miss are discarded and counter is reset with
random value (between 1-15 for fetch pmu and 1-127 for op pmu) when
IBS L3 miss filtering is enabled. This causes a sampling period skew
but there is no way to reconstruct aggregated sampling period. So
print a warning at perf record if user sets l3missonly=1.

Ex:
  # perf record -c 10000 -C 0 -e ibs_op/l3missonly=1/
  WARNING: Hw internally resets sampling period when L3 Miss Filtering is enabled
  and tagged operation does not cause L3 Miss. This causes sampling period skew.

Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
---
 tools/perf/arch/x86/util/evsel.c | 50 ++++++++++++++++++++++++++++++++
 tools/perf/util/evsel.c          |  7 +++++
 tools/perf/util/evsel.h          |  1 +
 3 files changed, 58 insertions(+)

diff --git a/tools/perf/arch/x86/util/evsel.c b/tools/perf/arch/x86/util/evsel.c
index ac2899a25b7a..e8ff4ddb53c9 100644
--- a/tools/perf/arch/x86/util/evsel.c
+++ b/tools/perf/arch/x86/util/evsel.c
@@ -4,6 +4,8 @@
 #include "util/evsel.h"
 #include "util/env.h"
 #include "linux/string.h"
+#include "util/pmu.h"
+#include "util/debug.h"
 
 void arch_evsel__set_sample_weight(struct evsel *evsel)
 {
@@ -29,3 +31,51 @@ void arch_evsel__fixup_new_cycles(struct perf_event_attr *attr)
 
 	free(env.cpuid);
 }
+
+static void ibs_l3miss_warn(void)
+{
+	pr_warning(
+"WARNING: Hw internally resets sampling period when L3 Miss Filtering is enabled\n"
+"and tagged operation does not cause L3 Miss. This causes sampling period skew.\n");
+}
+
+void arch__post_evsel_config(struct evsel *evsel, struct perf_event_attr *attr)
+{
+	struct perf_pmu *evsel_pmu, *ibs_fetch_pmu, *ibs_op_pmu;
+	static int warned_once;
+	/* 0: Uninitialized, 1: Yes, -1: No */
+	static int is_amd;
+
+	if (warned_once || is_amd == -1)
+		return;
+
+	if (!is_amd) {
+		struct perf_env *env = evsel__env(evsel);
+
+		if (!perf_env__cpuid(env) || !env->cpuid ||
+		    !strstarts(env->cpuid, "AuthenticAMD")) {
+			is_amd = -1;
+			return;
+		}
+		is_amd = 1;
+	}
+
+	evsel_pmu = evsel__find_pmu(evsel);
+	if (!evsel_pmu)
+		return;
+
+	ibs_fetch_pmu = perf_pmu__find("ibs_fetch");
+	ibs_op_pmu = perf_pmu__find("ibs_op");
+
+	if (ibs_fetch_pmu && ibs_fetch_pmu->type == evsel_pmu->type) {
+		if (attr->config & (1ULL << 59)) {
+			ibs_l3miss_warn();
+			warned_once = 1;
+		}
+	} else if (ibs_op_pmu && ibs_op_pmu->type == evsel_pmu->type) {
+		if (attr->config & (1ULL << 16)) {
+			ibs_l3miss_warn();
+			warned_once = 1;
+		}
+	}
+}
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 2a1729e7aee4..e9be9b94a062 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1064,6 +1064,11 @@ void __weak arch_evsel__fixup_new_cycles(struct perf_event_attr *attr __maybe_un
 {
 }
 
+void __weak arch__post_evsel_config(struct evsel *evsel __maybe_unused,
+				    struct perf_event_attr *attr __maybe_unused)
+{
+}
+
 static void evsel__set_default_freq_period(struct record_opts *opts,
 					   struct perf_event_attr *attr)
 {
@@ -1339,6 +1344,8 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts,
 	 */
 	if (evsel__is_dummy_event(evsel))
 		evsel__reset_sample_bit(evsel, BRANCH_STACK);
+
+	arch__post_evsel_config(evsel, attr);
 }
 
 int evsel__set_filter(struct evsel *evsel, const char *filter)
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 041b42d33bf5..207de5082ee9 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -281,6 +281,7 @@ void evsel__set_sample_id(struct evsel *evsel, bool use_sample_identifier);
 
 void arch_evsel__set_sample_weight(struct evsel *evsel);
 void arch_evsel__fixup_new_cycles(struct perf_event_attr *attr);
+void arch__post_evsel_config(struct evsel *evsel, struct perf_event_attr *attr);
 
 int evsel__set_filter(struct evsel *evsel, const char *filter);
 int evsel__append_tp_filter(struct evsel *evsel, const char *filter);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 2/5] perf header: Parse non-cpu pmu capabilities
  2022-05-19  5:43 [PATCH v3 0/5] perf/amd: Zen4 IBS extensions support (tool changes) Ravi Bangoria
  2022-05-19  5:43 ` [PATCH v3 1/5] perf record ibs: Warn about sampling period skew Ravi Bangoria
@ 2022-05-19  5:43 ` Ravi Bangoria
  2022-05-19 22:27   ` Ian Rogers
  2022-05-19  5:43 ` [PATCH v3 3/5] perf/x86/ibs: Add new IBS register bits into header Ravi Bangoria
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Ravi Bangoria @ 2022-05-19  5:43 UTC (permalink / raw)
  To: acme
  Cc: ravi.bangoria, peterz, rrichter, mingo, mark.rutland, jolsa,
	namhyung, tglx, bp, irogers, james.clark, leo.yan, kan.liang, ak,
	eranian, like.xu.linux, x86, linux-perf-users, linux-kernel,
	sandipan.das, ananth.narayan, kim.phillips, santosh.shukla

Pmus advertise their capabilities via sysfs attribute files but
perf tool currently parses only core(cpu) pmu capabilities. Add
support for parsing non-cpu pmu capabilities.

Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
---
 .../Documentation/perf.data-file-format.txt   |  18 ++
 tools/perf/util/env.c                         |  48 +++++
 tools/perf/util/env.h                         |  11 +
 tools/perf/util/header.c                      | 198 ++++++++++++++++++
 tools/perf/util/header.h                      |   1 +
 tools/perf/util/pmu.c                         |  15 +-
 tools/perf/util/pmu.h                         |   2 +
 7 files changed, 289 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
index f56d0e0fbff6..7f8341db9134 100644
--- a/tools/perf/Documentation/perf.data-file-format.txt
+++ b/tools/perf/Documentation/perf.data-file-format.txt
@@ -435,6 +435,24 @@ struct {
 	} [nr_pmu];
 };
 
+	HEADER_PMU_CAPS = 32,
+
+	List of pmu capabilities (except cpu pmu which is already
+	covered by HEADER_CPU_PMU_CAPS)
+
+struct {
+	u32 nr_pmus;
+	struct {
+		u32 core_type;	/* For hybrid topology */
+		char pmu_name[];
+		u32 nr_caps;
+		struct {
+			char name[];
+			char value[];
+		} [nr_caps];
+	} [nr_pmus];
+};
+
 	other bits are reserved and should ignored for now
 	HEADER_FEAT_BITS	= 256,
 
diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index 579e44c59914..69c7804b5640 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -180,6 +180,7 @@ static void perf_env__purge_bpf(struct perf_env *env __maybe_unused)
 void perf_env__exit(struct perf_env *env)
 {
 	int i;
+	u32 j;
 
 	perf_env__purge_bpf(env);
 	perf_env__purge_cgroups(env);
@@ -222,6 +223,14 @@ void perf_env__exit(struct perf_env *env)
 		zfree(&env->hybrid_cpc_nodes[i].pmu_name);
 	}
 	zfree(&env->hybrid_cpc_nodes);
+
+	for (i = 0; i < env->nr_pmus_with_caps; i++) {
+		zfree(&env->env_pmu_caps[i].pmu_name);
+		for (j = 0; j < env->env_pmu_caps[i].nr_caps; j++)
+			zfree(&env->env_pmu_caps[i].pmu_caps[j]);
+		zfree(&env->env_pmu_caps[i].pmu_caps);
+	}
+	zfree(&env->env_pmu_caps);
 }
 
 void perf_env__init(struct perf_env *env)
@@ -527,3 +536,42 @@ int perf_env__numa_node(struct perf_env *env, struct perf_cpu cpu)
 
 	return cpu.cpu >= 0 && cpu.cpu < env->nr_numa_map ? env->numa_map[cpu.cpu] : -1;
 }
+
+char *perf_env__find_pmu_cap(struct perf_env *env, u32 core_type,
+			     const char *pmu_name, const char *cap)
+{
+	struct env_pmu_caps *env_pmu_caps = env->env_pmu_caps;
+	char *cap_eq;
+	int cap_size;
+	char **ptr;
+	int i;
+	u32 j;
+
+	if (!pmu_name || !cap)
+		return NULL;
+
+	cap_size = strlen(cap);
+	cap_eq = zalloc(cap_size + 2);
+	if (!cap_eq)
+		return NULL;
+
+	memcpy(cap_eq, cap, cap_size);
+	cap_eq[cap_size] = '=';
+
+	for (i = 0; i < env->nr_pmus_with_caps; i++) {
+		if (env_pmu_caps[i].core_type != core_type ||
+		    strcmp(env_pmu_caps[i].pmu_name, pmu_name))
+			continue;
+
+		ptr = env_pmu_caps[i].pmu_caps;
+
+		for (j = 0; j < env_pmu_caps[i].nr_caps; j++) {
+			if (!strncmp(ptr[j], cap_eq, cap_size + 1)) {
+				free(cap_eq);
+				return &ptr[j][cap_size + 1];
+			}
+		}
+	}
+	free(cap_eq);
+	return NULL;
+}
diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
index a3541f98e1fc..c3ffc818661c 100644
--- a/tools/perf/util/env.h
+++ b/tools/perf/util/env.h
@@ -50,6 +50,13 @@ struct hybrid_cpc_node {
 	char            *pmu_name;
 };
 
+struct env_pmu_caps {
+	u32	core_type;
+	char	*pmu_name;
+	u32	nr_caps;
+	char	**pmu_caps;
+};
+
 struct perf_env {
 	char			*hostname;
 	char			*os_release;
@@ -75,6 +82,7 @@ struct perf_env {
 	int			nr_cpu_pmu_caps;
 	int			nr_hybrid_nodes;
 	int			nr_hybrid_cpc_nodes;
+	int			nr_pmus_with_caps;
 	char			*cmdline;
 	const char		**cmdline_argv;
 	char			*sibling_cores;
@@ -95,6 +103,7 @@ struct perf_env {
 	unsigned long long	 memory_bsize;
 	struct hybrid_node	*hybrid_nodes;
 	struct hybrid_cpc_node	*hybrid_cpc_nodes;
+	struct env_pmu_caps	*env_pmu_caps;
 #ifdef HAVE_LIBBPF_SUPPORT
 	/*
 	 * bpf_info_lock protects bpf rbtrees. This is needed because the
@@ -172,4 +181,6 @@ bool perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node);
 struct btf_node *perf_env__find_btf(struct perf_env *env, __u32 btf_id);
 
 int perf_env__numa_node(struct perf_env *env, struct perf_cpu cpu);
+char *perf_env__find_pmu_cap(struct perf_env *env, u32 core_type,
+			     const char *pmu_name, const char *cap);
 #endif /* __PERF_ENV_H */
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index a27132e5a5ef..b86951fa3d7a 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1580,6 +1580,77 @@ static int write_hybrid_cpu_pmu_caps(struct feat_fd *ff,
 	return 0;
 }
 
+/*
+ * File format:
+ *
+ * struct {
+ *	u32 nr_pmus;
+ *	struct {
+ *		u32 core_type;
+ *		char pmu_name[];
+ *		u32 nr_caps;
+ *		struct {
+ *			char name[];
+ *			char value[];
+ *		} [nr_caps];
+ *	} [nr_pmus];
+ * };
+ */
+static int write_pmu_caps(struct feat_fd *ff, struct evlist *evlist __maybe_unused)
+{
+	struct perf_pmu_caps *caps = NULL;
+	struct perf_pmu *pmu = NULL;
+	u32 core_type = 0;
+	u32 nr_pmus = 0;
+	int ret;
+
+	while ((pmu = perf_pmu__scan(pmu))) {
+		if (!pmu->name || !strncmp(pmu->name, "cpu", 3) ||
+		    perf_pmu__caps_parse(pmu) <= 0)
+			continue;
+		nr_pmus++;
+	}
+
+	ret = do_write(ff, &nr_pmus, sizeof(nr_pmus));
+	if (ret < 0)
+		return ret;
+
+	if (!nr_pmus)
+		return 0;
+
+	while ((pmu = perf_pmu__scan(pmu))) {
+		if (!pmu->name || !strncmp(pmu->name, "cpu", 3) || !pmu->nr_caps)
+			continue;
+
+		/*
+		 * Currently core_type is always set to 0. But it can be
+		 * used in future for hybrid topology pmus.
+		 */
+		ret = do_write(ff, &core_type, sizeof(core_type));
+		if (ret < 0)
+			return ret;
+
+		ret = do_write_string(ff, pmu->name);
+		if (ret < 0)
+			return ret;
+
+		ret = do_write(ff, &pmu->nr_caps, sizeof(pmu->nr_caps));
+		if (ret < 0)
+			return ret;
+
+		list_for_each_entry(caps, &pmu->caps, list) {
+			ret = do_write_string(ff, caps->name);
+			if (ret < 0)
+				return ret;
+
+			ret = do_write_string(ff, caps->value);
+			if (ret < 0)
+				return ret;
+		}
+	}
+	return 0;
+}
+
 static void print_hostname(struct feat_fd *ff, FILE *fp)
 {
 	fprintf(fp, "# hostname : %s\n", ff->ph->env.hostname);
@@ -2209,6 +2280,32 @@ static void print_mem_topology(struct feat_fd *ff, FILE *fp)
 	}
 }
 
+static void print_pmu_caps(struct feat_fd *ff, FILE *fp)
+{
+	struct env_pmu_caps *env_pmu_caps = ff->ph->env.env_pmu_caps;
+	int nr_pmus_with_caps = ff->ph->env.nr_pmus_with_caps;
+	const char *delimiter = "";
+	char **ptr;
+	int i;
+	u32 j;
+
+	if (!nr_pmus_with_caps)
+		return;
+
+	for (i = 0; i < nr_pmus_with_caps; i++) {
+		fprintf(fp, "# %s pmu capabilities: ", env_pmu_caps[i].pmu_name);
+
+		ptr = env_pmu_caps[i].pmu_caps;
+
+		delimiter = "";
+		for (j = 0; j < env_pmu_caps[i].nr_caps; j++) {
+			fprintf(fp, "%s%s", delimiter, ptr[j]);
+			delimiter = ", ";
+		}
+		fprintf(fp, "\n");
+	}
+}
+
 static int __event_process_build_id(struct perf_record_header_build_id *bev,
 				    char *filename,
 				    struct perf_session *session)
@@ -3319,6 +3416,106 @@ static int process_hybrid_cpu_pmu_caps(struct feat_fd *ff,
 	return ret;
 }
 
+static int __process_pmu_caps(struct feat_fd *ff, struct env_pmu_caps *env_pmu_caps)
+{
+	u32 nr_caps = env_pmu_caps->nr_caps;
+	int name_size, value_size;
+	char *name, *value, *ptr;
+	u32 i;
+
+	env_pmu_caps->pmu_caps = zalloc(sizeof(char *) * nr_caps);
+	if (!env_pmu_caps->pmu_caps)
+		return -1;
+
+	for (i = 0; i < nr_caps; i++) {
+		name = do_read_string(ff);
+		if (!name)
+			goto error;
+
+		value = do_read_string(ff);
+		if (!value)
+			goto free_name;
+
+		name_size = strlen(name);
+		value_size = strlen(value);
+		ptr = zalloc(sizeof(char) * (name_size + value_size + 2));
+		if (!ptr)
+			goto free_value;
+
+		memcpy(ptr, name, name_size);
+		ptr[name_size] = '=';
+		memcpy(ptr + name_size + 1, value, value_size);
+		env_pmu_caps->pmu_caps[i] = ptr;
+
+		free(value);
+		free(name);
+	}
+	return 0;
+
+free_value:
+	free(value);
+free_name:
+	free(name);
+error:
+	for (; i > 0; i--)
+		free(env_pmu_caps->pmu_caps[i - 1]);
+	free(env_pmu_caps->pmu_caps);
+	return -1;
+}
+
+static int process_pmu_caps(struct feat_fd *ff, void *data __maybe_unused)
+{
+	struct env_pmu_caps *env_pmu_caps;
+	u32 nr_pmus;
+	u32 i, j;
+
+	ff->ph->env.nr_pmus_with_caps = 0;
+	ff->ph->env.env_pmu_caps = NULL;
+
+	if (do_read_u32(ff, &nr_pmus))
+		return -1;
+
+	if (!nr_pmus)
+		return 0;
+
+	env_pmu_caps = zalloc(sizeof(struct env_pmu_caps) * nr_pmus);
+	if (!env_pmu_caps)
+		return -ENOMEM;
+
+	for (i = 0; i < nr_pmus; i++) {
+		if (do_read_u32(ff, &env_pmu_caps[i].core_type))
+			goto error;
+
+		env_pmu_caps[i].pmu_name = do_read_string(ff);
+		if (!env_pmu_caps[i].pmu_name)
+			goto error;
+
+		if (do_read_u32(ff, &env_pmu_caps[i].nr_caps))
+			goto free_pmu_name;
+
+		if (!__process_pmu_caps(ff, &env_pmu_caps[i]))
+			continue;
+
+free_pmu_name:
+		free(env_pmu_caps[i].pmu_name);
+		goto error;
+	}
+
+	ff->ph->env.nr_pmus_with_caps = nr_pmus;
+	ff->ph->env.env_pmu_caps = env_pmu_caps;
+	return 0;
+
+error:
+	for (; i > 0; i--) {
+		free(env_pmu_caps[i - 1].pmu_name);
+		for (j = 0; j < env_pmu_caps[i - 1].nr_caps; j++)
+			free(env_pmu_caps[i - 1].pmu_caps[j]);
+		free(env_pmu_caps[i - 1].pmu_caps);
+	}
+	free(env_pmu_caps);
+	return -1;
+}
+
 #define FEAT_OPR(n, func, __full_only) \
 	[HEADER_##n] = {					\
 		.name	    = __stringify(n),			\
@@ -3382,6 +3579,7 @@ const struct perf_header_feature_ops feat_ops[HEADER_LAST_FEATURE] = {
 	FEAT_OPR(CLOCK_DATA,	clock_data,	false),
 	FEAT_OPN(HYBRID_TOPOLOGY,	hybrid_topology,	true),
 	FEAT_OPR(HYBRID_CPU_PMU_CAPS,	hybrid_cpu_pmu_caps,	false),
+	FEAT_OPR(PMU_CAPS,	pmu_caps,	false),
 };
 
 struct header_print_data {
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 0eb4bc29a5a4..e9a067bb8b9e 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -47,6 +47,7 @@ enum {
 	HEADER_CLOCK_DATA,
 	HEADER_HYBRID_TOPOLOGY,
 	HEADER_HYBRID_CPU_PMU_CAPS,
+	HEADER_PMU_CAPS,
 	HEADER_LAST_FEATURE,
 	HEADER_FEAT_BITS	= 256,
 };
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 9a1c7e63e663..8d599acb7569 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1890,16 +1890,22 @@ int perf_pmu__caps_parse(struct perf_pmu *pmu)
 	const char *sysfs = sysfs__mountpoint();
 	DIR *caps_dir;
 	struct dirent *evt_ent;
-	int nr_caps = 0;
+
+	if (pmu->caps_initialized)
+		return pmu->nr_caps;
 
 	if (!sysfs)
 		return -1;
 
+	pmu->nr_caps = 0;
+
 	snprintf(caps_path, PATH_MAX,
 		 "%s" EVENT_SOURCE_DEVICE_PATH "%s/caps", sysfs, pmu->name);
 
-	if (stat(caps_path, &st) < 0)
+	if (stat(caps_path, &st) < 0) {
+		pmu->caps_initialized = true;
 		return 0;	/* no error if caps does not exist */
+	}
 
 	caps_dir = opendir(caps_path);
 	if (!caps_dir)
@@ -1926,13 +1932,14 @@ int perf_pmu__caps_parse(struct perf_pmu *pmu)
 			continue;
 		}
 
-		nr_caps++;
+		pmu->nr_caps++;
 		fclose(file);
 	}
 
 	closedir(caps_dir);
 
-	return nr_caps;
+	pmu->caps_initialized = true;
+	return pmu->nr_caps;
 }
 
 void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 541889fa9f9c..4b45fd8da5a3 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -46,6 +46,8 @@ struct perf_pmu {
 	struct perf_cpu_map *cpus;
 	struct list_head format;  /* HEAD struct perf_pmu_format -> list */
 	struct list_head aliases; /* HEAD struct perf_pmu_alias -> list */
+	bool caps_initialized;
+	u32 nr_caps;
 	struct list_head caps;    /* HEAD struct perf_pmu_caps -> list */
 	struct list_head list;    /* ELEM */
 	struct list_head hybrid_list;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 3/5] perf/x86/ibs: Add new IBS register bits into header
  2022-05-19  5:43 [PATCH v3 0/5] perf/amd: Zen4 IBS extensions support (tool changes) Ravi Bangoria
  2022-05-19  5:43 ` [PATCH v3 1/5] perf record ibs: Warn about sampling period skew Ravi Bangoria
  2022-05-19  5:43 ` [PATCH v3 2/5] perf header: Parse non-cpu pmu capabilities Ravi Bangoria
@ 2022-05-19  5:43 ` Ravi Bangoria
  2022-05-19 23:47   ` Ian Rogers
  2022-05-19  5:43 ` [PATCH v3 4/5] perf tool ibs: Sync amd ibs header file Ravi Bangoria
  2022-05-19  5:43 ` [PATCH v3 5/5] perf script ibs: Support new IBS bits in raw trace dump Ravi Bangoria
  4 siblings, 1 reply; 16+ messages in thread
From: Ravi Bangoria @ 2022-05-19  5:43 UTC (permalink / raw)
  To: acme
  Cc: ravi.bangoria, peterz, rrichter, mingo, mark.rutland, jolsa,
	namhyung, tglx, bp, irogers, james.clark, leo.yan, kan.liang, ak,
	eranian, like.xu.linux, x86, linux-perf-users, linux-kernel,
	sandipan.das, ananth.narayan, kim.phillips, santosh.shukla

IBS support has been enhanced with two new features in upcoming uarch:
1. DataSrc extension and 2. L3 miss filtering. Additional set of bits
has been introduced in IBS registers to exploit these features. Define
these new bits into arch/x86/ header.

Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
---
 arch/x86/include/asm/amd-ibs.h | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/amd-ibs.h b/arch/x86/include/asm/amd-ibs.h
index aabdbb5ab920..f3eb098d63d4 100644
--- a/arch/x86/include/asm/amd-ibs.h
+++ b/arch/x86/include/asm/amd-ibs.h
@@ -29,7 +29,10 @@ union ibs_fetch_ctl {
 			rand_en:1,	/* 57: random tagging enable */
 			fetch_l2_miss:1,/* 58: L2 miss for sampled fetch
 					 *      (needs IbsFetchComp) */
-			reserved:5;	/* 59-63: reserved */
+			l3_miss_only:1,	/* 59: Collect L3 miss samples only */
+			fetch_oc_miss:1,/* 60: Op cache miss for the sampled fetch */
+			fetch_l3_miss:1,/* 61: L3 cache miss for the sampled fetch */
+			reserved:2;	/* 62-63: reserved */
 	};
 };
 
@@ -38,14 +41,14 @@ union ibs_op_ctl {
 	__u64 val;
 	struct {
 		__u64	opmaxcnt:16,	/* 0-15: periodic op max. count */
-			reserved0:1,	/* 16: reserved */
+			l3_miss_only:1,	/* 16: Collect L3 miss samples only */
 			op_en:1,	/* 17: op sampling enable */
 			op_val:1,	/* 18: op sample valid */
 			cnt_ctl:1,	/* 19: periodic op counter control */
 			opmaxcnt_ext:7,	/* 20-26: upper 7 bits of periodic op maximum count */
-			reserved1:5,	/* 27-31: reserved */
+			reserved0:5,	/* 27-31: reserved */
 			opcurcnt:27,	/* 32-58: periodic op counter current count */
-			reserved2:5;	/* 59-63: reserved */
+			reserved1:5;	/* 59-63: reserved */
 	};
 };
 
@@ -71,11 +74,12 @@ union ibs_op_data {
 union ibs_op_data2 {
 	__u64 val;
 	struct {
-		__u64	data_src:3,	/* 0-2: data source */
+		__u64	data_src_lo:3,	/* 0-2: data source low */
 			reserved0:1,	/* 3: reserved */
 			rmt_node:1,	/* 4: destination node */
 			cache_hit_st:1,	/* 5: cache hit state */
-			reserved1:57;	/* 5-63: reserved */
+			data_src_hi:2,	/* 6-7: data source high */
+			reserved1:56;	/* 8-63: reserved */
 	};
 };
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 4/5] perf tool ibs: Sync amd ibs header file
  2022-05-19  5:43 [PATCH v3 0/5] perf/amd: Zen4 IBS extensions support (tool changes) Ravi Bangoria
                   ` (2 preceding siblings ...)
  2022-05-19  5:43 ` [PATCH v3 3/5] perf/x86/ibs: Add new IBS register bits into header Ravi Bangoria
@ 2022-05-19  5:43 ` Ravi Bangoria
  2022-05-19 23:48   ` Ian Rogers
  2022-05-19  5:43 ` [PATCH v3 5/5] perf script ibs: Support new IBS bits in raw trace dump Ravi Bangoria
  4 siblings, 1 reply; 16+ messages in thread
From: Ravi Bangoria @ 2022-05-19  5:43 UTC (permalink / raw)
  To: acme
  Cc: ravi.bangoria, peterz, rrichter, mingo, mark.rutland, jolsa,
	namhyung, tglx, bp, irogers, james.clark, leo.yan, kan.liang, ak,
	eranian, like.xu.linux, x86, linux-perf-users, linux-kernel,
	sandipan.das, ananth.narayan, kim.phillips, santosh.shukla

IBS support has been enhanced with two new features in upcoming uarch:
1. DataSrc extension and 2. L3 miss filtering. Additional set of bits
has been introduced in IBS registers to exploit these features.
New bits are already defining in arch/x86/ header. Sync it with tools
header file.

Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
---
 tools/arch/x86/include/asm/amd-ibs.h | 16 ++++++++++------
 tools/perf/util/amd-sample-raw.c     |  4 ++--
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/tools/arch/x86/include/asm/amd-ibs.h b/tools/arch/x86/include/asm/amd-ibs.h
index 765e9e752d03..9a3312e12e2e 100644
--- a/tools/arch/x86/include/asm/amd-ibs.h
+++ b/tools/arch/x86/include/asm/amd-ibs.h
@@ -29,7 +29,10 @@ union ibs_fetch_ctl {
 			rand_en:1,	/* 57: random tagging enable */
 			fetch_l2_miss:1,/* 58: L2 miss for sampled fetch
 					 *      (needs IbsFetchComp) */
-			reserved:5;	/* 59-63: reserved */
+			l3_miss_only:1,	/* 59: Collect L3 miss samples only */
+			fetch_oc_miss:1,/* 60: Op cache miss for the sampled fetch */
+			fetch_l3_miss:1,/* 61: L3 cache miss for the sampled fetch */
+			reserved:2;	/* 62-63: reserved */
 	};
 };
 
@@ -38,14 +41,14 @@ union ibs_op_ctl {
 	__u64 val;
 	struct {
 		__u64	opmaxcnt:16,	/* 0-15: periodic op max. count */
-			reserved0:1,	/* 16: reserved */
+			l3_miss_only:1,	/* 16: Collect L3 miss samples only */
 			op_en:1,	/* 17: op sampling enable */
 			op_val:1,	/* 18: op sample valid */
 			cnt_ctl:1,	/* 19: periodic op counter control */
 			opmaxcnt_ext:7,	/* 20-26: upper 7 bits of periodic op maximum count */
-			reserved1:5,	/* 27-31: reserved */
+			reserved0:5,	/* 27-31: reserved */
 			opcurcnt:27,	/* 32-58: periodic op counter current count */
-			reserved2:5;	/* 59-63: reserved */
+			reserved1:5;	/* 59-63: reserved */
 	};
 };
 
@@ -71,11 +74,12 @@ union ibs_op_data {
 union ibs_op_data2 {
 	__u64 val;
 	struct {
-		__u64	data_src:3,	/* 0-2: data source */
+		__u64	data_src_lo:3,	/* 0-2: data source low */
 			reserved0:1,	/* 3: reserved */
 			rmt_node:1,	/* 4: destination node */
 			cache_hit_st:1,	/* 5: cache hit state */
-			reserved1:57;	/* 5-63: reserved */
+			data_src_hi:2,	/* 6-7: data source high */
+			reserved1:56;	/* 8-63: reserved */
 	};
 };
 
diff --git a/tools/perf/util/amd-sample-raw.c b/tools/perf/util/amd-sample-raw.c
index d19d765195c5..3b623ea6ee7e 100644
--- a/tools/perf/util/amd-sample-raw.c
+++ b/tools/perf/util/amd-sample-raw.c
@@ -98,9 +98,9 @@ static void pr_ibs_op_data2(union ibs_op_data2 reg)
 	};
 
 	printf("ibs_op_data2:\t%016llx %sRmtNode %d%s\n", reg.val,
-	       reg.data_src == 2 ? (reg.cache_hit_st ? "CacheHitSt 1=O-State "
+	       reg.data_src_lo == 2 ? (reg.cache_hit_st ? "CacheHitSt 1=O-State "
 						     : "CacheHitSt 0=M-state ") : "",
-	       reg.rmt_node, data_src_str[reg.data_src]);
+	       reg.rmt_node, data_src_str[reg.data_src_lo]);
 }
 
 static void pr_ibs_op_data3(union ibs_op_data3 reg)
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 5/5] perf script ibs: Support new IBS bits in raw trace dump
  2022-05-19  5:43 [PATCH v3 0/5] perf/amd: Zen4 IBS extensions support (tool changes) Ravi Bangoria
                   ` (3 preceding siblings ...)
  2022-05-19  5:43 ` [PATCH v3 4/5] perf tool ibs: Sync amd ibs header file Ravi Bangoria
@ 2022-05-19  5:43 ` Ravi Bangoria
  2022-05-19 23:50   ` Ian Rogers
  4 siblings, 1 reply; 16+ messages in thread
From: Ravi Bangoria @ 2022-05-19  5:43 UTC (permalink / raw)
  To: acme
  Cc: ravi.bangoria, peterz, rrichter, mingo, mark.rutland, jolsa,
	namhyung, tglx, bp, irogers, james.clark, leo.yan, kan.liang, ak,
	eranian, like.xu.linux, x86, linux-perf-users, linux-kernel,
	sandipan.das, ananth.narayan, kim.phillips, santosh.shukla

Interpret Additional set of IBS register bits while doing
perf report/script raw dump.

IBS op pmu ex:
  $ sudo ./perf record -c 130 -a -e ibs_op/l3missonly=1/ --raw-samples
  $ sudo ./perf report -D
  ...
  ibs_op_ctl:     0000004500070008 MaxCnt       128 L3MissOnly 1 En 1
        Val 1 CntCtl 0=cycles CurCnt        69
  ibs_op_data:    0000000000710002 CompToRetCtr     2 TagToRetCtr   113
        BrnRet 0  RipInvalid 0 BrnFuse 0 Microcode 0
  ibs_op_data2:   0000000000000002 CacheHitSt 0=M-state RmtNode 0
        DataSrc 2=A peer cache in a near CCX
  ibs_op_data3:   000000681d1700a1 LdOp 1 StOp 0 DcL1TlbMiss 0
        DcL2TlbMiss 0 DcL1TlbHit2M 0 DcL1TlbHit1G 1 DcL2TlbHit2M 0
        DcMiss 1 DcMisAcc 0 DcWcMemAcc 0 DcUcMemAcc 0 DcLockedOp 0
        DcMissNoMabAlloc 1 DcLinAddrValid 1 DcPhyAddrValid 1
        DcL2TlbHit1G 0 L2Miss 1 SwPf 0 OpMemWidth  8 bytes
        OpDcMissOpenMemReqs  7 DcMissLat   104 TlbRefillLat     0

IBS Fetch pmu ex:
  $ sudo ./perf record -c 130 -a -e ibs_fetch/l3missonly=1/ --raw-samples
  $ sudo ./perf report -D
  ...
  ibs_fetch_ctl:  3c1f00c700080008 MaxCnt     128 Cnt     128 Lat   199
        En 1 Val 1 Comp 1 IcMiss 1 PhyAddrValid        1 L1TlbPgSz 4KB
        L1TlbMiss 0 L2TlbMiss 0 RandEn 0 L2Miss 1 L3MissOnly 1
        FetchOcMiss 1 FetchL3Miss 1

With the DataSrc extensions, the source of data can be decoded among:
 - Local L3 or other L1/L2 in CCX.
 - A peer cache in a near CCX.
 - Data returned from DRAM.
 - A peer cache in a far CCX.
 - DRAM address map with "long latency" bit set.
 - Data returned from MMIO/Config/PCI/APIC.
 - Extension Memory (S-Link, GenZ, etc - identified by the CS target
    and/or address map at DF's choice).
 - Peer Agent Memory.

Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
---
 tools/perf/util/amd-sample-raw.c | 64 +++++++++++++++++++++++++++++---
 1 file changed, 58 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/amd-sample-raw.c b/tools/perf/util/amd-sample-raw.c
index 3b623ea6ee7e..63303f583bc0 100644
--- a/tools/perf/util/amd-sample-raw.c
+++ b/tools/perf/util/amd-sample-raw.c
@@ -18,6 +18,7 @@
 #include "pmu-events/pmu-events.h"
 
 static u32 cpu_family, cpu_model, ibs_fetch_type, ibs_op_type;
+static bool zen4_ibs_extensions;
 
 static void pr_ibs_fetch_ctl(union ibs_fetch_ctl reg)
 {
@@ -39,6 +40,7 @@ static void pr_ibs_fetch_ctl(union ibs_fetch_ctl reg)
 	};
 	const char *ic_miss_str = NULL;
 	const char *l1tlb_pgsz_str = NULL;
+	char l3_miss_str[sizeof(" L3MissOnly _ FetchOcMiss _ FetchL3Miss _")] = "";
 
 	if (cpu_family == 0x19 && cpu_model < 0x10) {
 		/*
@@ -53,12 +55,19 @@ static void pr_ibs_fetch_ctl(union ibs_fetch_ctl reg)
 		ic_miss_str = ic_miss_strs[reg.ic_miss];
 	}
 
+	if (zen4_ibs_extensions) {
+		snprintf(l3_miss_str, sizeof(l3_miss_str),
+			 " L3MissOnly %d FetchOcMiss %d FetchL3Miss %d",
+			 reg.l3_miss_only, reg.fetch_oc_miss, reg.fetch_l3_miss);
+	}
+
 	printf("ibs_fetch_ctl:\t%016llx MaxCnt %7d Cnt %7d Lat %5d En %d Val %d Comp %d%s "
-	       "PhyAddrValid %d%s L1TlbMiss %d L2TlbMiss %d RandEn %d%s\n",
+		"PhyAddrValid %d%s L1TlbMiss %d L2TlbMiss %d RandEn %d%s%s\n",
 		reg.val, reg.fetch_maxcnt << 4, reg.fetch_cnt << 4, reg.fetch_lat,
 		reg.fetch_en, reg.fetch_val, reg.fetch_comp, ic_miss_str ? : "",
 		reg.phy_addr_valid, l1tlb_pgsz_str ? : "", reg.l1tlb_miss, reg.l2tlb_miss,
-		reg.rand_en, reg.fetch_comp ? (reg.fetch_l2_miss ? " L2Miss 1" : " L2Miss 0") : "");
+		reg.rand_en, reg.fetch_comp ? (reg.fetch_l2_miss ? " L2Miss 1" : " L2Miss 0") : "",
+		l3_miss_str);
 }
 
 static void pr_ic_ibs_extd_ctl(union ic_ibs_extd_ctl reg)
@@ -68,9 +77,15 @@ static void pr_ic_ibs_extd_ctl(union ic_ibs_extd_ctl reg)
 
 static void pr_ibs_op_ctl(union ibs_op_ctl reg)
 {
-	printf("ibs_op_ctl:\t%016llx MaxCnt %9d En %d Val %d CntCtl %d=%s CurCnt %9d\n",
-	       reg.val, ((reg.opmaxcnt_ext << 16) | reg.opmaxcnt) << 4, reg.op_en, reg.op_val,
-	       reg.cnt_ctl, reg.cnt_ctl ? "uOps" : "cycles", reg.opcurcnt);
+	char l3_miss_only[sizeof(" L3MissOnly _")] = "";
+
+	if (zen4_ibs_extensions)
+		snprintf(l3_miss_only, sizeof(l3_miss_only), " L3MissOnly %d", reg.l3_miss_only);
+
+	printf("ibs_op_ctl:\t%016llx MaxCnt %9d%s En %d Val %d CntCtl %d=%s CurCnt %9d\n",
+		reg.val, ((reg.opmaxcnt_ext << 16) | reg.opmaxcnt) << 4, l3_miss_only,
+		reg.op_en, reg.op_val, reg.cnt_ctl,
+		reg.cnt_ctl ? "uOps" : "cycles", reg.opcurcnt);
 }
 
 static void pr_ibs_op_data(union ibs_op_data reg)
@@ -84,7 +99,34 @@ static void pr_ibs_op_data(union ibs_op_data reg)
 		reg.op_brn_ret, reg.op_rip_invalid, reg.op_brn_fuse, reg.op_microcode);
 }
 
-static void pr_ibs_op_data2(union ibs_op_data2 reg)
+static void pr_ibs_op_data2_extended(union ibs_op_data2 reg)
+{
+	static const char * const data_src_str[] = {
+		"",
+		" DataSrc 1=Local L3 or other L1/L2 in CCX",
+		" DataSrc 2=A peer cache in a near CCX",
+		" DataSrc 3=Data returned from DRAM",
+		" DataSrc 4=(reserved)",
+		" DataSrc 5=A peer cache in a far CCX",
+		" DataSrc 6=DRAM address map with \"long latency\" bit set",
+		" DataSrc 7=Data returned from MMIO/Config/PCI/APIC",
+		" DataSrc 8=Extension Memory (S-Link, GenZ, etc)",
+		" DataSrc 9=(reserved)",
+		" DataSrc 10=(reserved)",
+		" DataSrc 11=(reserved)",
+		" DataSrc 12=Peer Agent Memory",
+		/* 13 to 31 are reserved. Avoid printing them. */
+	};
+	int data_src = (reg.data_src_hi << 3) | reg.data_src_lo;
+
+	printf("ibs_op_data2:\t%016llx %sRmtNode %d%s\n", reg.val,
+		(data_src == 1 || data_src == 2 || data_src == 5) ?
+			(reg.cache_hit_st ? "CacheHitSt 1=O-State " : "CacheHitSt 0=M-state ") : "",
+		reg.rmt_node,
+		data_src < (int)ARRAY_SIZE(data_src_str) ? data_src_str[data_src] : "");
+}
+
+static void pr_ibs_op_data2_default(union ibs_op_data2 reg)
 {
 	static const char * const data_src_str[] = {
 		"",
@@ -103,6 +145,13 @@ static void pr_ibs_op_data2(union ibs_op_data2 reg)
 	       reg.rmt_node, data_src_str[reg.data_src_lo]);
 }
 
+static void pr_ibs_op_data2(union ibs_op_data2 reg)
+{
+	if (zen4_ibs_extensions)
+		return pr_ibs_op_data2_extended(reg);
+	pr_ibs_op_data2_default(reg);
+}
+
 static void pr_ibs_op_data3(union ibs_op_data3 reg)
 {
 	char l2_miss_str[sizeof(" L2Miss _")] = "";
@@ -279,6 +328,9 @@ bool evlist__has_amd_ibs(struct evlist *evlist)
 		pmu_mapping += strlen(pmu_mapping) + 1 /* '\0' */;
 	}
 
+	if (perf_env__find_pmu_cap(env, 0, "ibs_op", "zen4_ibs_extensions"))
+		zen4_ibs_extensions = 1;
+
 	if (ibs_fetch_type || ibs_op_type) {
 		if (!cpu_family)
 			parse_cpuid(env);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/5] perf record ibs: Warn about sampling period skew
  2022-05-19  5:43 ` [PATCH v3 1/5] perf record ibs: Warn about sampling period skew Ravi Bangoria
@ 2022-05-19 22:14   ` Ian Rogers
  0 siblings, 0 replies; 16+ messages in thread
From: Ian Rogers @ 2022-05-19 22:14 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: acme, peterz, rrichter, mingo, mark.rutland, jolsa, namhyung,
	tglx, bp, james.clark, leo.yan, kan.liang, ak, eranian,
	like.xu.linux, x86, linux-perf-users, linux-kernel, sandipan.das,
	ananth.narayan, kim.phillips, santosh.shukla

On Wed, May 18, 2022 at 10:44 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>
> Samples without an L3 miss are discarded and counter is reset with
> random value (between 1-15 for fetch pmu and 1-127 for op pmu) when
> IBS L3 miss filtering is enabled. This causes a sampling period skew
> but there is no way to reconstruct aggregated sampling period. So
> print a warning at perf record if user sets l3missonly=1.
>
> Ex:
>   # perf record -c 10000 -C 0 -e ibs_op/l3missonly=1/
>   WARNING: Hw internally resets sampling period when L3 Miss Filtering is enabled
>   and tagged operation does not cause L3 Miss. This causes sampling period skew.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>

Acked-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/arch/x86/util/evsel.c | 50 ++++++++++++++++++++++++++++++++
>  tools/perf/util/evsel.c          |  7 +++++
>  tools/perf/util/evsel.h          |  1 +
>  3 files changed, 58 insertions(+)
>
> diff --git a/tools/perf/arch/x86/util/evsel.c b/tools/perf/arch/x86/util/evsel.c
> index ac2899a25b7a..e8ff4ddb53c9 100644
> --- a/tools/perf/arch/x86/util/evsel.c
> +++ b/tools/perf/arch/x86/util/evsel.c
> @@ -4,6 +4,8 @@
>  #include "util/evsel.h"
>  #include "util/env.h"
>  #include "linux/string.h"
> +#include "util/pmu.h"
> +#include "util/debug.h"
>
>  void arch_evsel__set_sample_weight(struct evsel *evsel)
>  {
> @@ -29,3 +31,51 @@ void arch_evsel__fixup_new_cycles(struct perf_event_attr *attr)
>
>         free(env.cpuid);
>  }
> +
> +static void ibs_l3miss_warn(void)
> +{
> +       pr_warning(
> +"WARNING: Hw internally resets sampling period when L3 Miss Filtering is enabled\n"
> +"and tagged operation does not cause L3 Miss. This causes sampling period skew.\n");
> +}
> +
> +void arch__post_evsel_config(struct evsel *evsel, struct perf_event_attr *attr)
> +{
> +       struct perf_pmu *evsel_pmu, *ibs_fetch_pmu, *ibs_op_pmu;
> +       static int warned_once;
> +       /* 0: Uninitialized, 1: Yes, -1: No */
> +       static int is_amd;
> +
> +       if (warned_once || is_amd == -1)
> +               return;
> +
> +       if (!is_amd) {
> +               struct perf_env *env = evsel__env(evsel);
> +
> +               if (!perf_env__cpuid(env) || !env->cpuid ||
> +                   !strstarts(env->cpuid, "AuthenticAMD")) {
> +                       is_amd = -1;
> +                       return;
> +               }
> +               is_amd = 1;
> +       }
> +
> +       evsel_pmu = evsel__find_pmu(evsel);
> +       if (!evsel_pmu)
> +               return;
> +
> +       ibs_fetch_pmu = perf_pmu__find("ibs_fetch");
> +       ibs_op_pmu = perf_pmu__find("ibs_op");
> +
> +       if (ibs_fetch_pmu && ibs_fetch_pmu->type == evsel_pmu->type) {
> +               if (attr->config & (1ULL << 59)) {
> +                       ibs_l3miss_warn();
> +                       warned_once = 1;
> +               }
> +       } else if (ibs_op_pmu && ibs_op_pmu->type == evsel_pmu->type) {
> +               if (attr->config & (1ULL << 16)) {
> +                       ibs_l3miss_warn();
> +                       warned_once = 1;
> +               }
> +       }
> +}
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 2a1729e7aee4..e9be9b94a062 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1064,6 +1064,11 @@ void __weak arch_evsel__fixup_new_cycles(struct perf_event_attr *attr __maybe_un
>  {
>  }
>
> +void __weak arch__post_evsel_config(struct evsel *evsel __maybe_unused,
> +                                   struct perf_event_attr *attr __maybe_unused)
> +{
> +}
> +
>  static void evsel__set_default_freq_period(struct record_opts *opts,
>                                            struct perf_event_attr *attr)
>  {
> @@ -1339,6 +1344,8 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts,
>          */
>         if (evsel__is_dummy_event(evsel))
>                 evsel__reset_sample_bit(evsel, BRANCH_STACK);
> +
> +       arch__post_evsel_config(evsel, attr);
>  }
>
>  int evsel__set_filter(struct evsel *evsel, const char *filter)
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 041b42d33bf5..207de5082ee9 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -281,6 +281,7 @@ void evsel__set_sample_id(struct evsel *evsel, bool use_sample_identifier);
>
>  void arch_evsel__set_sample_weight(struct evsel *evsel);
>  void arch_evsel__fixup_new_cycles(struct perf_event_attr *attr);
> +void arch__post_evsel_config(struct evsel *evsel, struct perf_event_attr *attr);
>
>  int evsel__set_filter(struct evsel *evsel, const char *filter);
>  int evsel__append_tp_filter(struct evsel *evsel, const char *filter);
> --
> 2.27.0
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/5] perf header: Parse non-cpu pmu capabilities
  2022-05-19  5:43 ` [PATCH v3 2/5] perf header: Parse non-cpu pmu capabilities Ravi Bangoria
@ 2022-05-19 22:27   ` Ian Rogers
  2022-05-20  3:49     ` Ravi Bangoria
  0 siblings, 1 reply; 16+ messages in thread
From: Ian Rogers @ 2022-05-19 22:27 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: acme, peterz, rrichter, mingo, mark.rutland, jolsa, namhyung,
	tglx, bp, james.clark, leo.yan, kan.liang, ak, eranian,
	like.xu.linux, x86, linux-perf-users, linux-kernel, sandipan.das,
	ananth.narayan, kim.phillips, santosh.shukla

On Wed, May 18, 2022 at 10:45 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>
> Pmus advertise their capabilities via sysfs attribute files but
> perf tool currently parses only core(cpu) pmu capabilities. Add
> support for parsing non-cpu pmu capabilities.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
> ---
>  .../Documentation/perf.data-file-format.txt   |  18 ++
>  tools/perf/util/env.c                         |  48 +++++
>  tools/perf/util/env.h                         |  11 +
>  tools/perf/util/header.c                      | 198 ++++++++++++++++++
>  tools/perf/util/header.h                      |   1 +
>  tools/perf/util/pmu.c                         |  15 +-
>  tools/perf/util/pmu.h                         |   2 +
>  7 files changed, 289 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
> index f56d0e0fbff6..7f8341db9134 100644
> --- a/tools/perf/Documentation/perf.data-file-format.txt
> +++ b/tools/perf/Documentation/perf.data-file-format.txt
> @@ -435,6 +435,24 @@ struct {
>         } [nr_pmu];
>  };
>
> +       HEADER_PMU_CAPS = 32,
> +
> +       List of pmu capabilities (except cpu pmu which is already
> +       covered by HEADER_CPU_PMU_CAPS)

Sorry for the ignorance, is this currently broken for hybrid then?
Will hybrid have a HEADER_CPU_PMU_CAPS? Presumably this varies between
ARM's big.little and Alderlake.

> +
> +struct {
> +       u32 nr_pmus;
> +       struct {
> +               u32 core_type;  /* For hybrid topology */

Could this be pmu_type as presumably we can have capabilities on any
kind of PMU?

Thanks,
Ian

> +               char pmu_name[];
> +               u32 nr_caps;
> +               struct {
> +                       char name[];
> +                       char value[];
> +               } [nr_caps];
> +       } [nr_pmus];
> +};
> +
>         other bits are reserved and should ignored for now
>         HEADER_FEAT_BITS        = 256,
>
> diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
> index 579e44c59914..69c7804b5640 100644
> --- a/tools/perf/util/env.c
> +++ b/tools/perf/util/env.c
> @@ -180,6 +180,7 @@ static void perf_env__purge_bpf(struct perf_env *env __maybe_unused)
>  void perf_env__exit(struct perf_env *env)
>  {
>         int i;
> +       u32 j;
>
>         perf_env__purge_bpf(env);
>         perf_env__purge_cgroups(env);
> @@ -222,6 +223,14 @@ void perf_env__exit(struct perf_env *env)
>                 zfree(&env->hybrid_cpc_nodes[i].pmu_name);
>         }
>         zfree(&env->hybrid_cpc_nodes);
> +
> +       for (i = 0; i < env->nr_pmus_with_caps; i++) {
> +               zfree(&env->env_pmu_caps[i].pmu_name);
> +               for (j = 0; j < env->env_pmu_caps[i].nr_caps; j++)
> +                       zfree(&env->env_pmu_caps[i].pmu_caps[j]);
> +               zfree(&env->env_pmu_caps[i].pmu_caps);
> +       }
> +       zfree(&env->env_pmu_caps);
>  }
>
>  void perf_env__init(struct perf_env *env)
> @@ -527,3 +536,42 @@ int perf_env__numa_node(struct perf_env *env, struct perf_cpu cpu)
>
>         return cpu.cpu >= 0 && cpu.cpu < env->nr_numa_map ? env->numa_map[cpu.cpu] : -1;
>  }
> +
> +char *perf_env__find_pmu_cap(struct perf_env *env, u32 core_type,
> +                            const char *pmu_name, const char *cap)
> +{
> +       struct env_pmu_caps *env_pmu_caps = env->env_pmu_caps;
> +       char *cap_eq;
> +       int cap_size;
> +       char **ptr;
> +       int i;
> +       u32 j;
> +
> +       if (!pmu_name || !cap)
> +               return NULL;
> +
> +       cap_size = strlen(cap);
> +       cap_eq = zalloc(cap_size + 2);
> +       if (!cap_eq)
> +               return NULL;
> +
> +       memcpy(cap_eq, cap, cap_size);
> +       cap_eq[cap_size] = '=';
> +
> +       for (i = 0; i < env->nr_pmus_with_caps; i++) {
> +               if (env_pmu_caps[i].core_type != core_type ||
> +                   strcmp(env_pmu_caps[i].pmu_name, pmu_name))
> +                       continue;
> +
> +               ptr = env_pmu_caps[i].pmu_caps;
> +
> +               for (j = 0; j < env_pmu_caps[i].nr_caps; j++) {
> +                       if (!strncmp(ptr[j], cap_eq, cap_size + 1)) {
> +                               free(cap_eq);
> +                               return &ptr[j][cap_size + 1];
> +                       }
> +               }
> +       }
> +       free(cap_eq);
> +       return NULL;
> +}
> diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
> index a3541f98e1fc..c3ffc818661c 100644
> --- a/tools/perf/util/env.h
> +++ b/tools/perf/util/env.h
> @@ -50,6 +50,13 @@ struct hybrid_cpc_node {
>         char            *pmu_name;
>  };
>
> +struct env_pmu_caps {
> +       u32     core_type;
> +       char    *pmu_name;
> +       u32     nr_caps;
> +       char    **pmu_caps;
> +};
> +
>  struct perf_env {
>         char                    *hostname;
>         char                    *os_release;
> @@ -75,6 +82,7 @@ struct perf_env {
>         int                     nr_cpu_pmu_caps;
>         int                     nr_hybrid_nodes;
>         int                     nr_hybrid_cpc_nodes;
> +       int                     nr_pmus_with_caps;
>         char                    *cmdline;
>         const char              **cmdline_argv;
>         char                    *sibling_cores;
> @@ -95,6 +103,7 @@ struct perf_env {
>         unsigned long long       memory_bsize;
>         struct hybrid_node      *hybrid_nodes;
>         struct hybrid_cpc_node  *hybrid_cpc_nodes;
> +       struct env_pmu_caps     *env_pmu_caps;
>  #ifdef HAVE_LIBBPF_SUPPORT
>         /*
>          * bpf_info_lock protects bpf rbtrees. This is needed because the
> @@ -172,4 +181,6 @@ bool perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node);
>  struct btf_node *perf_env__find_btf(struct perf_env *env, __u32 btf_id);
>
>  int perf_env__numa_node(struct perf_env *env, struct perf_cpu cpu);
> +char *perf_env__find_pmu_cap(struct perf_env *env, u32 core_type,
> +                            const char *pmu_name, const char *cap);
>  #endif /* __PERF_ENV_H */
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index a27132e5a5ef..b86951fa3d7a 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -1580,6 +1580,77 @@ static int write_hybrid_cpu_pmu_caps(struct feat_fd *ff,
>         return 0;
>  }
>
> +/*
> + * File format:
> + *
> + * struct {
> + *     u32 nr_pmus;
> + *     struct {
> + *             u32 core_type;
> + *             char pmu_name[];
> + *             u32 nr_caps;
> + *             struct {
> + *                     char name[];
> + *                     char value[];
> + *             } [nr_caps];
> + *     } [nr_pmus];
> + * };
> + */
> +static int write_pmu_caps(struct feat_fd *ff, struct evlist *evlist __maybe_unused)
> +{
> +       struct perf_pmu_caps *caps = NULL;
> +       struct perf_pmu *pmu = NULL;
> +       u32 core_type = 0;
> +       u32 nr_pmus = 0;
> +       int ret;
> +
> +       while ((pmu = perf_pmu__scan(pmu))) {
> +               if (!pmu->name || !strncmp(pmu->name, "cpu", 3) ||
> +                   perf_pmu__caps_parse(pmu) <= 0)
> +                       continue;
> +               nr_pmus++;
> +       }
> +
> +       ret = do_write(ff, &nr_pmus, sizeof(nr_pmus));
> +       if (ret < 0)
> +               return ret;
> +
> +       if (!nr_pmus)
> +               return 0;
> +
> +       while ((pmu = perf_pmu__scan(pmu))) {
> +               if (!pmu->name || !strncmp(pmu->name, "cpu", 3) || !pmu->nr_caps)
> +                       continue;
> +
> +               /*
> +                * Currently core_type is always set to 0. But it can be
> +                * used in future for hybrid topology pmus.
> +                */
> +               ret = do_write(ff, &core_type, sizeof(core_type));
> +               if (ret < 0)
> +                       return ret;
> +
> +               ret = do_write_string(ff, pmu->name);
> +               if (ret < 0)
> +                       return ret;
> +
> +               ret = do_write(ff, &pmu->nr_caps, sizeof(pmu->nr_caps));
> +               if (ret < 0)
> +                       return ret;
> +
> +               list_for_each_entry(caps, &pmu->caps, list) {
> +                       ret = do_write_string(ff, caps->name);
> +                       if (ret < 0)
> +                               return ret;
> +
> +                       ret = do_write_string(ff, caps->value);
> +                       if (ret < 0)
> +                               return ret;
> +               }
> +       }
> +       return 0;
> +}
> +
>  static void print_hostname(struct feat_fd *ff, FILE *fp)
>  {
>         fprintf(fp, "# hostname : %s\n", ff->ph->env.hostname);
> @@ -2209,6 +2280,32 @@ static void print_mem_topology(struct feat_fd *ff, FILE *fp)
>         }
>  }
>
> +static void print_pmu_caps(struct feat_fd *ff, FILE *fp)
> +{
> +       struct env_pmu_caps *env_pmu_caps = ff->ph->env.env_pmu_caps;
> +       int nr_pmus_with_caps = ff->ph->env.nr_pmus_with_caps;
> +       const char *delimiter = "";
> +       char **ptr;
> +       int i;
> +       u32 j;
> +
> +       if (!nr_pmus_with_caps)
> +               return;
> +
> +       for (i = 0; i < nr_pmus_with_caps; i++) {
> +               fprintf(fp, "# %s pmu capabilities: ", env_pmu_caps[i].pmu_name);
> +
> +               ptr = env_pmu_caps[i].pmu_caps;
> +
> +               delimiter = "";
> +               for (j = 0; j < env_pmu_caps[i].nr_caps; j++) {
> +                       fprintf(fp, "%s%s", delimiter, ptr[j]);
> +                       delimiter = ", ";
> +               }
> +               fprintf(fp, "\n");
> +       }
> +}
> +
>  static int __event_process_build_id(struct perf_record_header_build_id *bev,
>                                     char *filename,
>                                     struct perf_session *session)
> @@ -3319,6 +3416,106 @@ static int process_hybrid_cpu_pmu_caps(struct feat_fd *ff,
>         return ret;
>  }
>
> +static int __process_pmu_caps(struct feat_fd *ff, struct env_pmu_caps *env_pmu_caps)
> +{
> +       u32 nr_caps = env_pmu_caps->nr_caps;
> +       int name_size, value_size;
> +       char *name, *value, *ptr;
> +       u32 i;
> +
> +       env_pmu_caps->pmu_caps = zalloc(sizeof(char *) * nr_caps);
> +       if (!env_pmu_caps->pmu_caps)
> +               return -1;
> +
> +       for (i = 0; i < nr_caps; i++) {
> +               name = do_read_string(ff);
> +               if (!name)
> +                       goto error;
> +
> +               value = do_read_string(ff);
> +               if (!value)
> +                       goto free_name;
> +
> +               name_size = strlen(name);
> +               value_size = strlen(value);
> +               ptr = zalloc(sizeof(char) * (name_size + value_size + 2));
> +               if (!ptr)
> +                       goto free_value;
> +
> +               memcpy(ptr, name, name_size);
> +               ptr[name_size] = '=';
> +               memcpy(ptr + name_size + 1, value, value_size);
> +               env_pmu_caps->pmu_caps[i] = ptr;
> +
> +               free(value);
> +               free(name);
> +       }
> +       return 0;
> +
> +free_value:
> +       free(value);
> +free_name:
> +       free(name);
> +error:
> +       for (; i > 0; i--)
> +               free(env_pmu_caps->pmu_caps[i - 1]);
> +       free(env_pmu_caps->pmu_caps);
> +       return -1;
> +}
> +
> +static int process_pmu_caps(struct feat_fd *ff, void *data __maybe_unused)
> +{
> +       struct env_pmu_caps *env_pmu_caps;
> +       u32 nr_pmus;
> +       u32 i, j;
> +
> +       ff->ph->env.nr_pmus_with_caps = 0;
> +       ff->ph->env.env_pmu_caps = NULL;
> +
> +       if (do_read_u32(ff, &nr_pmus))
> +               return -1;
> +
> +       if (!nr_pmus)
> +               return 0;
> +
> +       env_pmu_caps = zalloc(sizeof(struct env_pmu_caps) * nr_pmus);
> +       if (!env_pmu_caps)
> +               return -ENOMEM;
> +
> +       for (i = 0; i < nr_pmus; i++) {
> +               if (do_read_u32(ff, &env_pmu_caps[i].core_type))
> +                       goto error;
> +
> +               env_pmu_caps[i].pmu_name = do_read_string(ff);
> +               if (!env_pmu_caps[i].pmu_name)
> +                       goto error;
> +
> +               if (do_read_u32(ff, &env_pmu_caps[i].nr_caps))
> +                       goto free_pmu_name;
> +
> +               if (!__process_pmu_caps(ff, &env_pmu_caps[i]))
> +                       continue;
> +
> +free_pmu_name:
> +               free(env_pmu_caps[i].pmu_name);
> +               goto error;
> +       }
> +
> +       ff->ph->env.nr_pmus_with_caps = nr_pmus;
> +       ff->ph->env.env_pmu_caps = env_pmu_caps;
> +       return 0;
> +
> +error:
> +       for (; i > 0; i--) {
> +               free(env_pmu_caps[i - 1].pmu_name);
> +               for (j = 0; j < env_pmu_caps[i - 1].nr_caps; j++)
> +                       free(env_pmu_caps[i - 1].pmu_caps[j]);
> +               free(env_pmu_caps[i - 1].pmu_caps);
> +       }
> +       free(env_pmu_caps);
> +       return -1;
> +}
> +
>  #define FEAT_OPR(n, func, __full_only) \
>         [HEADER_##n] = {                                        \
>                 .name       = __stringify(n),                   \
> @@ -3382,6 +3579,7 @@ const struct perf_header_feature_ops feat_ops[HEADER_LAST_FEATURE] = {
>         FEAT_OPR(CLOCK_DATA,    clock_data,     false),
>         FEAT_OPN(HYBRID_TOPOLOGY,       hybrid_topology,        true),
>         FEAT_OPR(HYBRID_CPU_PMU_CAPS,   hybrid_cpu_pmu_caps,    false),
> +       FEAT_OPR(PMU_CAPS,      pmu_caps,       false),
>  };
>
>  struct header_print_data {
> diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
> index 0eb4bc29a5a4..e9a067bb8b9e 100644
> --- a/tools/perf/util/header.h
> +++ b/tools/perf/util/header.h
> @@ -47,6 +47,7 @@ enum {
>         HEADER_CLOCK_DATA,
>         HEADER_HYBRID_TOPOLOGY,
>         HEADER_HYBRID_CPU_PMU_CAPS,
> +       HEADER_PMU_CAPS,
>         HEADER_LAST_FEATURE,
>         HEADER_FEAT_BITS        = 256,
>  };
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 9a1c7e63e663..8d599acb7569 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -1890,16 +1890,22 @@ int perf_pmu__caps_parse(struct perf_pmu *pmu)
>         const char *sysfs = sysfs__mountpoint();
>         DIR *caps_dir;
>         struct dirent *evt_ent;
> -       int nr_caps = 0;
> +
> +       if (pmu->caps_initialized)
> +               return pmu->nr_caps;
>
>         if (!sysfs)
>                 return -1;
>
> +       pmu->nr_caps = 0;
> +
>         snprintf(caps_path, PATH_MAX,
>                  "%s" EVENT_SOURCE_DEVICE_PATH "%s/caps", sysfs, pmu->name);
>
> -       if (stat(caps_path, &st) < 0)
> +       if (stat(caps_path, &st) < 0) {
> +               pmu->caps_initialized = true;
>                 return 0;       /* no error if caps does not exist */
> +       }
>
>         caps_dir = opendir(caps_path);
>         if (!caps_dir)
> @@ -1926,13 +1932,14 @@ int perf_pmu__caps_parse(struct perf_pmu *pmu)
>                         continue;
>                 }
>
> -               nr_caps++;
> +               pmu->nr_caps++;
>                 fclose(file);
>         }
>
>         closedir(caps_dir);
>
> -       return nr_caps;
> +       pmu->caps_initialized = true;
> +       return pmu->nr_caps;
>  }
>
>  void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index 541889fa9f9c..4b45fd8da5a3 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -46,6 +46,8 @@ struct perf_pmu {
>         struct perf_cpu_map *cpus;
>         struct list_head format;  /* HEAD struct perf_pmu_format -> list */
>         struct list_head aliases; /* HEAD struct perf_pmu_alias -> list */
> +       bool caps_initialized;
> +       u32 nr_caps;
>         struct list_head caps;    /* HEAD struct perf_pmu_caps -> list */
>         struct list_head list;    /* ELEM */
>         struct list_head hybrid_list;
> --
> 2.27.0
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/5] perf/x86/ibs: Add new IBS register bits into header
  2022-05-19  5:43 ` [PATCH v3 3/5] perf/x86/ibs: Add new IBS register bits into header Ravi Bangoria
@ 2022-05-19 23:47   ` Ian Rogers
  0 siblings, 0 replies; 16+ messages in thread
From: Ian Rogers @ 2022-05-19 23:47 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: acme, peterz, rrichter, mingo, mark.rutland, jolsa, namhyung,
	tglx, bp, james.clark, leo.yan, kan.liang, ak, eranian,
	like.xu.linux, x86, linux-perf-users, linux-kernel, sandipan.das,
	ananth.narayan, kim.phillips, santosh.shukla

On Wed, May 18, 2022 at 10:45 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>
> IBS support has been enhanced with two new features in upcoming uarch:
> 1. DataSrc extension and 2. L3 miss filtering. Additional set of bits
> has been introduced in IBS registers to exploit these features. Define
> these new bits into arch/x86/ header.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>

Acked-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  arch/x86/include/asm/amd-ibs.h | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/amd-ibs.h b/arch/x86/include/asm/amd-ibs.h
> index aabdbb5ab920..f3eb098d63d4 100644
> --- a/arch/x86/include/asm/amd-ibs.h
> +++ b/arch/x86/include/asm/amd-ibs.h
> @@ -29,7 +29,10 @@ union ibs_fetch_ctl {
>                         rand_en:1,      /* 57: random tagging enable */
>                         fetch_l2_miss:1,/* 58: L2 miss for sampled fetch
>                                          *      (needs IbsFetchComp) */
> -                       reserved:5;     /* 59-63: reserved */
> +                       l3_miss_only:1, /* 59: Collect L3 miss samples only */
> +                       fetch_oc_miss:1,/* 60: Op cache miss for the sampled fetch */
> +                       fetch_l3_miss:1,/* 61: L3 cache miss for the sampled fetch */
> +                       reserved:2;     /* 62-63: reserved */
>         };
>  };
>
> @@ -38,14 +41,14 @@ union ibs_op_ctl {
>         __u64 val;
>         struct {
>                 __u64   opmaxcnt:16,    /* 0-15: periodic op max. count */
> -                       reserved0:1,    /* 16: reserved */
> +                       l3_miss_only:1, /* 16: Collect L3 miss samples only */
>                         op_en:1,        /* 17: op sampling enable */
>                         op_val:1,       /* 18: op sample valid */
>                         cnt_ctl:1,      /* 19: periodic op counter control */
>                         opmaxcnt_ext:7, /* 20-26: upper 7 bits of periodic op maximum count */
> -                       reserved1:5,    /* 27-31: reserved */
> +                       reserved0:5,    /* 27-31: reserved */
>                         opcurcnt:27,    /* 32-58: periodic op counter current count */
> -                       reserved2:5;    /* 59-63: reserved */
> +                       reserved1:5;    /* 59-63: reserved */
>         };
>  };
>
> @@ -71,11 +74,12 @@ union ibs_op_data {
>  union ibs_op_data2 {
>         __u64 val;
>         struct {
> -               __u64   data_src:3,     /* 0-2: data source */
> +               __u64   data_src_lo:3,  /* 0-2: data source low */
>                         reserved0:1,    /* 3: reserved */
>                         rmt_node:1,     /* 4: destination node */
>                         cache_hit_st:1, /* 5: cache hit state */
> -                       reserved1:57;   /* 5-63: reserved */
> +                       data_src_hi:2,  /* 6-7: data source high */
> +                       reserved1:56;   /* 8-63: reserved */
>         };
>  };
>
> --
> 2.27.0
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 4/5] perf tool ibs: Sync amd ibs header file
  2022-05-19  5:43 ` [PATCH v3 4/5] perf tool ibs: Sync amd ibs header file Ravi Bangoria
@ 2022-05-19 23:48   ` Ian Rogers
  2022-05-20  3:55     ` Ravi Bangoria
  0 siblings, 1 reply; 16+ messages in thread
From: Ian Rogers @ 2022-05-19 23:48 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: acme, peterz, rrichter, mingo, mark.rutland, jolsa, namhyung,
	tglx, bp, james.clark, leo.yan, kan.liang, ak, eranian,
	like.xu.linux, x86, linux-perf-users, linux-kernel, sandipan.das,
	ananth.narayan, kim.phillips, santosh.shukla

On Wed, May 18, 2022 at 10:45 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>
> IBS support has been enhanced with two new features in upcoming uarch:
> 1. DataSrc extension and 2. L3 miss filtering. Additional set of bits
> has been introduced in IBS registers to exploit these features.
> New bits are already defining in arch/x86/ header. Sync it with tools
> header file.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
> ---
>  tools/arch/x86/include/asm/amd-ibs.h | 16 ++++++++++------
>  tools/perf/util/amd-sample-raw.c     |  4 ++--
>  2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/tools/arch/x86/include/asm/amd-ibs.h b/tools/arch/x86/include/asm/amd-ibs.h
> index 765e9e752d03..9a3312e12e2e 100644
> --- a/tools/arch/x86/include/asm/amd-ibs.h
> +++ b/tools/arch/x86/include/asm/amd-ibs.h
> @@ -29,7 +29,10 @@ union ibs_fetch_ctl {
>                         rand_en:1,      /* 57: random tagging enable */
>                         fetch_l2_miss:1,/* 58: L2 miss for sampled fetch
>                                          *      (needs IbsFetchComp) */
> -                       reserved:5;     /* 59-63: reserved */
> +                       l3_miss_only:1, /* 59: Collect L3 miss samples only */
> +                       fetch_oc_miss:1,/* 60: Op cache miss for the sampled fetch */
> +                       fetch_l3_miss:1,/* 61: L3 cache miss for the sampled fetch */
> +                       reserved:2;     /* 62-63: reserved */
>         };
>  };
>
> @@ -38,14 +41,14 @@ union ibs_op_ctl {
>         __u64 val;
>         struct {
>                 __u64   opmaxcnt:16,    /* 0-15: periodic op max. count */
> -                       reserved0:1,    /* 16: reserved */
> +                       l3_miss_only:1, /* 16: Collect L3 miss samples only */
>                         op_en:1,        /* 17: op sampling enable */
>                         op_val:1,       /* 18: op sample valid */
>                         cnt_ctl:1,      /* 19: periodic op counter control */
>                         opmaxcnt_ext:7, /* 20-26: upper 7 bits of periodic op maximum count */
> -                       reserved1:5,    /* 27-31: reserved */
> +                       reserved0:5,    /* 27-31: reserved */
>                         opcurcnt:27,    /* 32-58: periodic op counter current count */
> -                       reserved2:5;    /* 59-63: reserved */
> +                       reserved1:5;    /* 59-63: reserved */
>         };
>  };
>
> @@ -71,11 +74,12 @@ union ibs_op_data {
>  union ibs_op_data2 {
>         __u64 val;
>         struct {
> -               __u64   data_src:3,     /* 0-2: data source */
> +               __u64   data_src_lo:3,  /* 0-2: data source low */
>                         reserved0:1,    /* 3: reserved */
>                         rmt_node:1,     /* 4: destination node */
>                         cache_hit_st:1, /* 5: cache hit state */
> -                       reserved1:57;   /* 5-63: reserved */
> +                       data_src_hi:2,  /* 6-7: data source high */
> +                       reserved1:56;   /* 8-63: reserved */
>         };
>  };
>
> diff --git a/tools/perf/util/amd-sample-raw.c b/tools/perf/util/amd-sample-raw.c
> index d19d765195c5..3b623ea6ee7e 100644
> --- a/tools/perf/util/amd-sample-raw.c
> +++ b/tools/perf/util/amd-sample-raw.c

nit: given the commit message this should probably be a separate patch.

Thanks,
Ian

> @@ -98,9 +98,9 @@ static void pr_ibs_op_data2(union ibs_op_data2 reg)
>         };
>
>         printf("ibs_op_data2:\t%016llx %sRmtNode %d%s\n", reg.val,
> -              reg.data_src == 2 ? (reg.cache_hit_st ? "CacheHitSt 1=O-State "
> +              reg.data_src_lo == 2 ? (reg.cache_hit_st ? "CacheHitSt 1=O-State "
>                                                      : "CacheHitSt 0=M-state ") : "",
> -              reg.rmt_node, data_src_str[reg.data_src]);
> +              reg.rmt_node, data_src_str[reg.data_src_lo]);
>  }
>
>  static void pr_ibs_op_data3(union ibs_op_data3 reg)
> --
> 2.27.0
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 5/5] perf script ibs: Support new IBS bits in raw trace dump
  2022-05-19  5:43 ` [PATCH v3 5/5] perf script ibs: Support new IBS bits in raw trace dump Ravi Bangoria
@ 2022-05-19 23:50   ` Ian Rogers
  0 siblings, 0 replies; 16+ messages in thread
From: Ian Rogers @ 2022-05-19 23:50 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: acme, peterz, rrichter, mingo, mark.rutland, jolsa, namhyung,
	tglx, bp, james.clark, leo.yan, kan.liang, ak, eranian,
	like.xu.linux, x86, linux-perf-users, linux-kernel, sandipan.das,
	ananth.narayan, kim.phillips, santosh.shukla

On Wed, May 18, 2022 at 10:45 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>
> Interpret Additional set of IBS register bits while doing
> perf report/script raw dump.
>
> IBS op pmu ex:
>   $ sudo ./perf record -c 130 -a -e ibs_op/l3missonly=1/ --raw-samples
>   $ sudo ./perf report -D
>   ...
>   ibs_op_ctl:     0000004500070008 MaxCnt       128 L3MissOnly 1 En 1
>         Val 1 CntCtl 0=cycles CurCnt        69
>   ibs_op_data:    0000000000710002 CompToRetCtr     2 TagToRetCtr   113
>         BrnRet 0  RipInvalid 0 BrnFuse 0 Microcode 0
>   ibs_op_data2:   0000000000000002 CacheHitSt 0=M-state RmtNode 0
>         DataSrc 2=A peer cache in a near CCX
>   ibs_op_data3:   000000681d1700a1 LdOp 1 StOp 0 DcL1TlbMiss 0
>         DcL2TlbMiss 0 DcL1TlbHit2M 0 DcL1TlbHit1G 1 DcL2TlbHit2M 0
>         DcMiss 1 DcMisAcc 0 DcWcMemAcc 0 DcUcMemAcc 0 DcLockedOp 0
>         DcMissNoMabAlloc 1 DcLinAddrValid 1 DcPhyAddrValid 1
>         DcL2TlbHit1G 0 L2Miss 1 SwPf 0 OpMemWidth  8 bytes
>         OpDcMissOpenMemReqs  7 DcMissLat   104 TlbRefillLat     0
>
> IBS Fetch pmu ex:
>   $ sudo ./perf record -c 130 -a -e ibs_fetch/l3missonly=1/ --raw-samples
>   $ sudo ./perf report -D
>   ...
>   ibs_fetch_ctl:  3c1f00c700080008 MaxCnt     128 Cnt     128 Lat   199
>         En 1 Val 1 Comp 1 IcMiss 1 PhyAddrValid        1 L1TlbPgSz 4KB
>         L1TlbMiss 0 L2TlbMiss 0 RandEn 0 L2Miss 1 L3MissOnly 1
>         FetchOcMiss 1 FetchL3Miss 1
>
> With the DataSrc extensions, the source of data can be decoded among:
>  - Local L3 or other L1/L2 in CCX.
>  - A peer cache in a near CCX.
>  - Data returned from DRAM.
>  - A peer cache in a far CCX.
>  - DRAM address map with "long latency" bit set.
>  - Data returned from MMIO/Config/PCI/APIC.
>  - Extension Memory (S-Link, GenZ, etc - identified by the CS target
>     and/or address map at DF's choice).
>  - Peer Agent Memory.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>

Looks good, but I think part of the previous patch should also be in this one.

Thanks,
Ian
> ---
>  tools/perf/util/amd-sample-raw.c | 64 +++++++++++++++++++++++++++++---
>  1 file changed, 58 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/util/amd-sample-raw.c b/tools/perf/util/amd-sample-raw.c
> index 3b623ea6ee7e..63303f583bc0 100644
> --- a/tools/perf/util/amd-sample-raw.c
> +++ b/tools/perf/util/amd-sample-raw.c
> @@ -18,6 +18,7 @@
>  #include "pmu-events/pmu-events.h"
>
>  static u32 cpu_family, cpu_model, ibs_fetch_type, ibs_op_type;
> +static bool zen4_ibs_extensions;
>
>  static void pr_ibs_fetch_ctl(union ibs_fetch_ctl reg)
>  {
> @@ -39,6 +40,7 @@ static void pr_ibs_fetch_ctl(union ibs_fetch_ctl reg)
>         };
>         const char *ic_miss_str = NULL;
>         const char *l1tlb_pgsz_str = NULL;
> +       char l3_miss_str[sizeof(" L3MissOnly _ FetchOcMiss _ FetchL3Miss _")] = "";
>
>         if (cpu_family == 0x19 && cpu_model < 0x10) {
>                 /*
> @@ -53,12 +55,19 @@ static void pr_ibs_fetch_ctl(union ibs_fetch_ctl reg)
>                 ic_miss_str = ic_miss_strs[reg.ic_miss];
>         }
>
> +       if (zen4_ibs_extensions) {
> +               snprintf(l3_miss_str, sizeof(l3_miss_str),
> +                        " L3MissOnly %d FetchOcMiss %d FetchL3Miss %d",
> +                        reg.l3_miss_only, reg.fetch_oc_miss, reg.fetch_l3_miss);
> +       }
> +
>         printf("ibs_fetch_ctl:\t%016llx MaxCnt %7d Cnt %7d Lat %5d En %d Val %d Comp %d%s "
> -              "PhyAddrValid %d%s L1TlbMiss %d L2TlbMiss %d RandEn %d%s\n",
> +               "PhyAddrValid %d%s L1TlbMiss %d L2TlbMiss %d RandEn %d%s%s\n",
>                 reg.val, reg.fetch_maxcnt << 4, reg.fetch_cnt << 4, reg.fetch_lat,
>                 reg.fetch_en, reg.fetch_val, reg.fetch_comp, ic_miss_str ? : "",
>                 reg.phy_addr_valid, l1tlb_pgsz_str ? : "", reg.l1tlb_miss, reg.l2tlb_miss,
> -               reg.rand_en, reg.fetch_comp ? (reg.fetch_l2_miss ? " L2Miss 1" : " L2Miss 0") : "");
> +               reg.rand_en, reg.fetch_comp ? (reg.fetch_l2_miss ? " L2Miss 1" : " L2Miss 0") : "",
> +               l3_miss_str);
>  }
>
>  static void pr_ic_ibs_extd_ctl(union ic_ibs_extd_ctl reg)
> @@ -68,9 +77,15 @@ static void pr_ic_ibs_extd_ctl(union ic_ibs_extd_ctl reg)
>
>  static void pr_ibs_op_ctl(union ibs_op_ctl reg)
>  {
> -       printf("ibs_op_ctl:\t%016llx MaxCnt %9d En %d Val %d CntCtl %d=%s CurCnt %9d\n",
> -              reg.val, ((reg.opmaxcnt_ext << 16) | reg.opmaxcnt) << 4, reg.op_en, reg.op_val,
> -              reg.cnt_ctl, reg.cnt_ctl ? "uOps" : "cycles", reg.opcurcnt);
> +       char l3_miss_only[sizeof(" L3MissOnly _")] = "";
> +
> +       if (zen4_ibs_extensions)
> +               snprintf(l3_miss_only, sizeof(l3_miss_only), " L3MissOnly %d", reg.l3_miss_only);
> +
> +       printf("ibs_op_ctl:\t%016llx MaxCnt %9d%s En %d Val %d CntCtl %d=%s CurCnt %9d\n",
> +               reg.val, ((reg.opmaxcnt_ext << 16) | reg.opmaxcnt) << 4, l3_miss_only,
> +               reg.op_en, reg.op_val, reg.cnt_ctl,
> +               reg.cnt_ctl ? "uOps" : "cycles", reg.opcurcnt);
>  }
>
>  static void pr_ibs_op_data(union ibs_op_data reg)
> @@ -84,7 +99,34 @@ static void pr_ibs_op_data(union ibs_op_data reg)
>                 reg.op_brn_ret, reg.op_rip_invalid, reg.op_brn_fuse, reg.op_microcode);
>  }
>
> -static void pr_ibs_op_data2(union ibs_op_data2 reg)
> +static void pr_ibs_op_data2_extended(union ibs_op_data2 reg)
> +{
> +       static const char * const data_src_str[] = {
> +               "",
> +               " DataSrc 1=Local L3 or other L1/L2 in CCX",
> +               " DataSrc 2=A peer cache in a near CCX",
> +               " DataSrc 3=Data returned from DRAM",
> +               " DataSrc 4=(reserved)",
> +               " DataSrc 5=A peer cache in a far CCX",
> +               " DataSrc 6=DRAM address map with \"long latency\" bit set",
> +               " DataSrc 7=Data returned from MMIO/Config/PCI/APIC",
> +               " DataSrc 8=Extension Memory (S-Link, GenZ, etc)",
> +               " DataSrc 9=(reserved)",
> +               " DataSrc 10=(reserved)",
> +               " DataSrc 11=(reserved)",
> +               " DataSrc 12=Peer Agent Memory",
> +               /* 13 to 31 are reserved. Avoid printing them. */
> +       };
> +       int data_src = (reg.data_src_hi << 3) | reg.data_src_lo;
> +
> +       printf("ibs_op_data2:\t%016llx %sRmtNode %d%s\n", reg.val,
> +               (data_src == 1 || data_src == 2 || data_src == 5) ?
> +                       (reg.cache_hit_st ? "CacheHitSt 1=O-State " : "CacheHitSt 0=M-state ") : "",
> +               reg.rmt_node,
> +               data_src < (int)ARRAY_SIZE(data_src_str) ? data_src_str[data_src] : "");
> +}
> +
> +static void pr_ibs_op_data2_default(union ibs_op_data2 reg)
>  {
>         static const char * const data_src_str[] = {
>                 "",
> @@ -103,6 +145,13 @@ static void pr_ibs_op_data2(union ibs_op_data2 reg)
>                reg.rmt_node, data_src_str[reg.data_src_lo]);
>  }
>
> +static void pr_ibs_op_data2(union ibs_op_data2 reg)
> +{
> +       if (zen4_ibs_extensions)
> +               return pr_ibs_op_data2_extended(reg);
> +       pr_ibs_op_data2_default(reg);
> +}
> +
>  static void pr_ibs_op_data3(union ibs_op_data3 reg)
>  {
>         char l2_miss_str[sizeof(" L2Miss _")] = "";
> @@ -279,6 +328,9 @@ bool evlist__has_amd_ibs(struct evlist *evlist)
>                 pmu_mapping += strlen(pmu_mapping) + 1 /* '\0' */;
>         }
>
> +       if (perf_env__find_pmu_cap(env, 0, "ibs_op", "zen4_ibs_extensions"))
> +               zen4_ibs_extensions = 1;
> +
>         if (ibs_fetch_type || ibs_op_type) {
>                 if (!cpu_family)
>                         parse_cpuid(env);
> --
> 2.27.0
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/5] perf header: Parse non-cpu pmu capabilities
  2022-05-19 22:27   ` Ian Rogers
@ 2022-05-20  3:49     ` Ravi Bangoria
  2022-05-20  4:31       ` Ian Rogers
  0 siblings, 1 reply; 16+ messages in thread
From: Ravi Bangoria @ 2022-05-20  3:49 UTC (permalink / raw)
  To: Ian Rogers
  Cc: acme, peterz, rrichter, mingo, mark.rutland, jolsa, namhyung,
	tglx, bp, james.clark, leo.yan, kan.liang, ak, eranian,
	like.xu.linux, x86, linux-perf-users, linux-kernel, sandipan.das,
	ananth.narayan, kim.phillips, santosh.shukla, Ravi Bangoria

Hi Ian,

On 20-May-22 3:57 AM, Ian Rogers wrote:
> On Wed, May 18, 2022 at 10:45 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>>
>> Pmus advertise their capabilities via sysfs attribute files but
>> perf tool currently parses only core(cpu) pmu capabilities. Add
>> support for parsing non-cpu pmu capabilities.
>>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
>> ---
>>  .../Documentation/perf.data-file-format.txt   |  18 ++
>>  tools/perf/util/env.c                         |  48 +++++
>>  tools/perf/util/env.h                         |  11 +
>>  tools/perf/util/header.c                      | 198 ++++++++++++++++++
>>  tools/perf/util/header.h                      |   1 +
>>  tools/perf/util/pmu.c                         |  15 +-
>>  tools/perf/util/pmu.h                         |   2 +
>>  7 files changed, 289 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
>> index f56d0e0fbff6..7f8341db9134 100644
>> --- a/tools/perf/Documentation/perf.data-file-format.txt
>> +++ b/tools/perf/Documentation/perf.data-file-format.txt
>> @@ -435,6 +435,24 @@ struct {
>>         } [nr_pmu];
>>  };
>>
>> +       HEADER_PMU_CAPS = 32,
>> +
>> +       List of pmu capabilities (except cpu pmu which is already
>> +       covered by HEADER_CPU_PMU_CAPS)
> 
> Sorry for the ignorance, is this currently broken for hybrid then?
> Will hybrid have a HEADER_CPU_PMU_CAPS? Presumably this varies between
> ARM's big.little and Alderlake.

It's covered by HEADER_HYBRID_CPU_PMU_CAPS, but that too covers only
cpu pmu. I think I should update the above comment to:

	List of pmu capabilities (except cpu pmu which is already
	covered by HEADER_CPU_PMU_CAPS / HEADER_HYBRID_CPU_PMU_CAPS)

>> +
>> +struct {
>> +       u32 nr_pmus;
>> +       struct {
>> +               u32 core_type;  /* For hybrid topology */
> 
> Could this be pmu_type as presumably we can have capabilities on any
> kind of PMU?

Not sure I follow that question but let me just put my thoughts here.

{core_type, pmu_name} is the unique key here. Considering a hypothetical
scenario: A system has two types of cores P-core and E-core. Certain pmu
inside P-core has some capabilities which are missing in the identical
pmu belonging to E-core. The header will look something like:

struct {
	.nr_pmus = 2,
	[0] = struct {
		.core_type = 0, /* P-core */
		.pmu_name = xyz_pmu,
		.nr_caps = 2,
		[0] = { .name = cap1, .value = value1 },
		[1] = { .name = cap2, .value = value2 },
	},
	[1] = struct {
		.core_type = 1; /* E-core */
		.pmu_name = xyz_pmu;
		.nr_caps = 1;
		[0] = { .name = cap1, .value = value1 };
	},
};

Does that answer your question?

Thanks for the review,
Ravi

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 4/5] perf tool ibs: Sync amd ibs header file
  2022-05-19 23:48   ` Ian Rogers
@ 2022-05-20  3:55     ` Ravi Bangoria
  2022-05-20  4:32       ` Ian Rogers
  0 siblings, 1 reply; 16+ messages in thread
From: Ravi Bangoria @ 2022-05-20  3:55 UTC (permalink / raw)
  To: Ian Rogers
  Cc: acme, peterz, rrichter, mingo, mark.rutland, jolsa, namhyung,
	tglx, bp, james.clark, leo.yan, kan.liang, ak, eranian,
	like.xu.linux, x86, linux-perf-users, linux-kernel, sandipan.das,
	ananth.narayan, kim.phillips, santosh.shukla, Ravi Bangoria

Hi Ian,

On 20-May-22 5:18 AM, Ian Rogers wrote:
> On Wed, May 18, 2022 at 10:45 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>>
>> IBS support has been enhanced with two new features in upcoming uarch:
>> 1. DataSrc extension and 2. L3 miss filtering. Additional set of bits
>> has been introduced in IBS registers to exploit these features.
>> New bits are already defining in arch/x86/ header. Sync it with tools
>> header file.
>>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
>> ---
>>  tools/arch/x86/include/asm/amd-ibs.h | 16 ++++++++++------
>>  tools/perf/util/amd-sample-raw.c     |  4 ++--
>>  2 files changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/tools/arch/x86/include/asm/amd-ibs.h b/tools/arch/x86/include/asm/amd-ibs.h
>> index 765e9e752d03..9a3312e12e2e 100644
>> --- a/tools/arch/x86/include/asm/amd-ibs.h
>> +++ b/tools/arch/x86/include/asm/amd-ibs.h
>> @@ -29,7 +29,10 @@ union ibs_fetch_ctl {
>>                         rand_en:1,      /* 57: random tagging enable */
>>                         fetch_l2_miss:1,/* 58: L2 miss for sampled fetch
>>                                          *      (needs IbsFetchComp) */
>> -                       reserved:5;     /* 59-63: reserved */
>> +                       l3_miss_only:1, /* 59: Collect L3 miss samples only */
>> +                       fetch_oc_miss:1,/* 60: Op cache miss for the sampled fetch */
>> +                       fetch_l3_miss:1,/* 61: L3 cache miss for the sampled fetch */
>> +                       reserved:2;     /* 62-63: reserved */
>>         };
>>  };
>>
>> @@ -38,14 +41,14 @@ union ibs_op_ctl {
>>         __u64 val;
>>         struct {
>>                 __u64   opmaxcnt:16,    /* 0-15: periodic op max. count */
>> -                       reserved0:1,    /* 16: reserved */
>> +                       l3_miss_only:1, /* 16: Collect L3 miss samples only */
>>                         op_en:1,        /* 17: op sampling enable */
>>                         op_val:1,       /* 18: op sample valid */
>>                         cnt_ctl:1,      /* 19: periodic op counter control */
>>                         opmaxcnt_ext:7, /* 20-26: upper 7 bits of periodic op maximum count */
>> -                       reserved1:5,    /* 27-31: reserved */
>> +                       reserved0:5,    /* 27-31: reserved */
>>                         opcurcnt:27,    /* 32-58: periodic op counter current count */
>> -                       reserved2:5;    /* 59-63: reserved */
>> +                       reserved1:5;    /* 59-63: reserved */
>>         };
>>  };
>>
>> @@ -71,11 +74,12 @@ union ibs_op_data {
>>  union ibs_op_data2 {
>>         __u64 val;
>>         struct {
>> -               __u64   data_src:3,     /* 0-2: data source */
>> +               __u64   data_src_lo:3,  /* 0-2: data source low */
>>                         reserved0:1,    /* 3: reserved */
>>                         rmt_node:1,     /* 4: destination node */
>>                         cache_hit_st:1, /* 5: cache hit state */
>> -                       reserved1:57;   /* 5-63: reserved */
>> +                       data_src_hi:2,  /* 6-7: data source high */
>> +                       reserved1:56;   /* 8-63: reserved */
>>         };
>>  };
>>
>> diff --git a/tools/perf/util/amd-sample-raw.c b/tools/perf/util/amd-sample-raw.c
>> index d19d765195c5..3b623ea6ee7e 100644
>> --- a/tools/perf/util/amd-sample-raw.c
>> +++ b/tools/perf/util/amd-sample-raw.c
> 
> nit: given the commit message this should probably be a separate patch.

Renaming of an existing field in only header file creates a build issue.
So I had to include it in this patch. Would documenting it in the patch
description help?

  Also rename existing ibs_op_data field "data_src" to "data_src_lo".

Thanks for the review,
Ravi

> 
> Thanks,
> Ian
> 
>> @@ -98,9 +98,9 @@ static void pr_ibs_op_data2(union ibs_op_data2 reg)
>>         };
>>
>>         printf("ibs_op_data2:\t%016llx %sRmtNode %d%s\n", reg.val,
>> -              reg.data_src == 2 ? (reg.cache_hit_st ? "CacheHitSt 1=O-State "
>> +              reg.data_src_lo == 2 ? (reg.cache_hit_st ? "CacheHitSt 1=O-State "
>>                                                      : "CacheHitSt 0=M-state ") : "",
>> -              reg.rmt_node, data_src_str[reg.data_src]);
>> +              reg.rmt_node, data_src_str[reg.data_src_lo]);
>>  }
>>
>>  static void pr_ibs_op_data3(union ibs_op_data3 reg)
>> --
>> 2.27.0
>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/5] perf header: Parse non-cpu pmu capabilities
  2022-05-20  3:49     ` Ravi Bangoria
@ 2022-05-20  4:31       ` Ian Rogers
  2022-05-20  5:20         ` Ravi Bangoria
  0 siblings, 1 reply; 16+ messages in thread
From: Ian Rogers @ 2022-05-20  4:31 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: acme, peterz, rrichter, mingo, mark.rutland, jolsa, namhyung,
	tglx, bp, james.clark, leo.yan, kan.liang, ak, eranian,
	like.xu.linux, x86, linux-perf-users, linux-kernel, sandipan.das,
	ananth.narayan, kim.phillips, santosh.shukla

On Thu, May 19, 2022 at 8:49 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>
> Hi Ian,
>
> On 20-May-22 3:57 AM, Ian Rogers wrote:
> > On Wed, May 18, 2022 at 10:45 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
> >>
> >> Pmus advertise their capabilities via sysfs attribute files but
> >> perf tool currently parses only core(cpu) pmu capabilities. Add
> >> support for parsing non-cpu pmu capabilities.
> >>
> >> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
> >> ---
> >>  .../Documentation/perf.data-file-format.txt   |  18 ++
> >>  tools/perf/util/env.c                         |  48 +++++
> >>  tools/perf/util/env.h                         |  11 +
> >>  tools/perf/util/header.c                      | 198 ++++++++++++++++++
> >>  tools/perf/util/header.h                      |   1 +
> >>  tools/perf/util/pmu.c                         |  15 +-
> >>  tools/perf/util/pmu.h                         |   2 +
> >>  7 files changed, 289 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
> >> index f56d0e0fbff6..7f8341db9134 100644
> >> --- a/tools/perf/Documentation/perf.data-file-format.txt
> >> +++ b/tools/perf/Documentation/perf.data-file-format.txt
> >> @@ -435,6 +435,24 @@ struct {
> >>         } [nr_pmu];
> >>  };
> >>
> >> +       HEADER_PMU_CAPS = 32,
> >> +
> >> +       List of pmu capabilities (except cpu pmu which is already
> >> +       covered by HEADER_CPU_PMU_CAPS)
> >
> > Sorry for the ignorance, is this currently broken for hybrid then?
> > Will hybrid have a HEADER_CPU_PMU_CAPS? Presumably this varies between
> > ARM's big.little and Alderlake.
>
> It's covered by HEADER_HYBRID_CPU_PMU_CAPS, but that too covers only
> cpu pmu. I think I should update the above comment to:
>
>         List of pmu capabilities (except cpu pmu which is already
>         covered by HEADER_CPU_PMU_CAPS / HEADER_HYBRID_CPU_PMU_CAPS)
>
> >> +
> >> +struct {
> >> +       u32 nr_pmus;
> >> +       struct {
> >> +               u32 core_type;  /* For hybrid topology */
> >
> > Could this be pmu_type as presumably we can have capabilities on any
> > kind of PMU?
>
> Not sure I follow that question but let me just put my thoughts here.
>
> {core_type, pmu_name} is the unique key here. Considering a hypothetical
> scenario: A system has two types of cores P-core and E-core. Certain pmu
> inside P-core has some capabilities which are missing in the identical
> pmu belonging to E-core. The header will look something like:
>
> struct {
>         .nr_pmus = 2,
>         [0] = struct {
>                 .core_type = 0, /* P-core */
>                 .pmu_name = xyz_pmu,
>                 .nr_caps = 2,
>                 [0] = { .name = cap1, .value = value1 },
>                 [1] = { .name = cap2, .value = value2 },
>         },
>         [1] = struct {
>                 .core_type = 1; /* E-core */
>                 .pmu_name = xyz_pmu;
>                 .nr_caps = 1;
>                 [0] = { .name = cap1, .value = value1 };
>         },
> };
>
> Does that answer your question?
>
> Thanks for the review,
> Ravi

I may be being a little ahead of the current code as I'm wondering
about heterogeneous systems with many non-CPU PMUs. It seems such a
scenario just wouldn't touch the core_type field here. Could the p or
e core-ness of a PMU be implied by the name? Is there something
similar to core_type in sysfs or would we use the name in that case?

Thanks,
Ian

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 4/5] perf tool ibs: Sync amd ibs header file
  2022-05-20  3:55     ` Ravi Bangoria
@ 2022-05-20  4:32       ` Ian Rogers
  0 siblings, 0 replies; 16+ messages in thread
From: Ian Rogers @ 2022-05-20  4:32 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: acme, peterz, rrichter, mingo, mark.rutland, jolsa, namhyung,
	tglx, bp, james.clark, leo.yan, kan.liang, ak, eranian,
	like.xu.linux, x86, linux-perf-users, linux-kernel, sandipan.das,
	ananth.narayan, kim.phillips, santosh.shukla

On Thu, May 19, 2022 at 8:56 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>
> Hi Ian,
>
> On 20-May-22 5:18 AM, Ian Rogers wrote:
> > On Wed, May 18, 2022 at 10:45 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
> >>
> >> IBS support has been enhanced with two new features in upcoming uarch:
> >> 1. DataSrc extension and 2. L3 miss filtering. Additional set of bits
> >> has been introduced in IBS registers to exploit these features.
> >> New bits are already defining in arch/x86/ header. Sync it with tools
> >> header file.
> >>
> >> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
> >> ---
> >>  tools/arch/x86/include/asm/amd-ibs.h | 16 ++++++++++------
> >>  tools/perf/util/amd-sample-raw.c     |  4 ++--
> >>  2 files changed, 12 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/tools/arch/x86/include/asm/amd-ibs.h b/tools/arch/x86/include/asm/amd-ibs.h
> >> index 765e9e752d03..9a3312e12e2e 100644
> >> --- a/tools/arch/x86/include/asm/amd-ibs.h
> >> +++ b/tools/arch/x86/include/asm/amd-ibs.h
> >> @@ -29,7 +29,10 @@ union ibs_fetch_ctl {
> >>                         rand_en:1,      /* 57: random tagging enable */
> >>                         fetch_l2_miss:1,/* 58: L2 miss for sampled fetch
> >>                                          *      (needs IbsFetchComp) */
> >> -                       reserved:5;     /* 59-63: reserved */
> >> +                       l3_miss_only:1, /* 59: Collect L3 miss samples only */
> >> +                       fetch_oc_miss:1,/* 60: Op cache miss for the sampled fetch */
> >> +                       fetch_l3_miss:1,/* 61: L3 cache miss for the sampled fetch */
> >> +                       reserved:2;     /* 62-63: reserved */
> >>         };
> >>  };
> >>
> >> @@ -38,14 +41,14 @@ union ibs_op_ctl {
> >>         __u64 val;
> >>         struct {
> >>                 __u64   opmaxcnt:16,    /* 0-15: periodic op max. count */
> >> -                       reserved0:1,    /* 16: reserved */
> >> +                       l3_miss_only:1, /* 16: Collect L3 miss samples only */
> >>                         op_en:1,        /* 17: op sampling enable */
> >>                         op_val:1,       /* 18: op sample valid */
> >>                         cnt_ctl:1,      /* 19: periodic op counter control */
> >>                         opmaxcnt_ext:7, /* 20-26: upper 7 bits of periodic op maximum count */
> >> -                       reserved1:5,    /* 27-31: reserved */
> >> +                       reserved0:5,    /* 27-31: reserved */
> >>                         opcurcnt:27,    /* 32-58: periodic op counter current count */
> >> -                       reserved2:5;    /* 59-63: reserved */
> >> +                       reserved1:5;    /* 59-63: reserved */
> >>         };
> >>  };
> >>
> >> @@ -71,11 +74,12 @@ union ibs_op_data {
> >>  union ibs_op_data2 {
> >>         __u64 val;
> >>         struct {
> >> -               __u64   data_src:3,     /* 0-2: data source */
> >> +               __u64   data_src_lo:3,  /* 0-2: data source low */
> >>                         reserved0:1,    /* 3: reserved */
> >>                         rmt_node:1,     /* 4: destination node */
> >>                         cache_hit_st:1, /* 5: cache hit state */
> >> -                       reserved1:57;   /* 5-63: reserved */
> >> +                       data_src_hi:2,  /* 6-7: data source high */
> >> +                       reserved1:56;   /* 8-63: reserved */
> >>         };
> >>  };
> >>
> >> diff --git a/tools/perf/util/amd-sample-raw.c b/tools/perf/util/amd-sample-raw.c
> >> index d19d765195c5..3b623ea6ee7e 100644
> >> --- a/tools/perf/util/amd-sample-raw.c
> >> +++ b/tools/perf/util/amd-sample-raw.c
> >
> > nit: given the commit message this should probably be a separate patch.
>
> Renaming of an existing field in only header file creates a build issue.
> So I had to include it in this patch. Would documenting it in the patch
> description help?
>
>   Also rename existing ibs_op_data field "data_src" to "data_src_lo".
>
> Thanks for the review,
> Ravi

Perfect fix. Thanks!
Ian

> >
> > Thanks,
> > Ian
> >
> >> @@ -98,9 +98,9 @@ static void pr_ibs_op_data2(union ibs_op_data2 reg)
> >>         };
> >>
> >>         printf("ibs_op_data2:\t%016llx %sRmtNode %d%s\n", reg.val,
> >> -              reg.data_src == 2 ? (reg.cache_hit_st ? "CacheHitSt 1=O-State "
> >> +              reg.data_src_lo == 2 ? (reg.cache_hit_st ? "CacheHitSt 1=O-State "
> >>                                                      : "CacheHitSt 0=M-state ") : "",
> >> -              reg.rmt_node, data_src_str[reg.data_src]);
> >> +              reg.rmt_node, data_src_str[reg.data_src_lo]);
> >>  }
> >>
> >>  static void pr_ibs_op_data3(union ibs_op_data3 reg)
> >> --
> >> 2.27.0
> >>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/5] perf header: Parse non-cpu pmu capabilities
  2022-05-20  4:31       ` Ian Rogers
@ 2022-05-20  5:20         ` Ravi Bangoria
  0 siblings, 0 replies; 16+ messages in thread
From: Ravi Bangoria @ 2022-05-20  5:20 UTC (permalink / raw)
  To: Ian Rogers
  Cc: acme, peterz, rrichter, mingo, mark.rutland, jolsa, namhyung,
	tglx, bp, james.clark, leo.yan, kan.liang, ak, eranian,
	like.xu.linux, x86, linux-perf-users, linux-kernel, sandipan.das,
	ananth.narayan, kim.phillips, santosh.shukla, Ravi Bangoria


On 20-May-22 10:01 AM, Ian Rogers wrote:
> On Thu, May 19, 2022 at 8:49 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>>
>> Hi Ian,
>>
>> On 20-May-22 3:57 AM, Ian Rogers wrote:
>>> On Wed, May 18, 2022 at 10:45 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>>>>
>>>> Pmus advertise their capabilities via sysfs attribute files but
>>>> perf tool currently parses only core(cpu) pmu capabilities. Add
>>>> support for parsing non-cpu pmu capabilities.
>>>>
>>>> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
>>>> ---
>>>>  .../Documentation/perf.data-file-format.txt   |  18 ++
>>>>  tools/perf/util/env.c                         |  48 +++++
>>>>  tools/perf/util/env.h                         |  11 +
>>>>  tools/perf/util/header.c                      | 198 ++++++++++++++++++
>>>>  tools/perf/util/header.h                      |   1 +
>>>>  tools/perf/util/pmu.c                         |  15 +-
>>>>  tools/perf/util/pmu.h                         |   2 +
>>>>  7 files changed, 289 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
>>>> index f56d0e0fbff6..7f8341db9134 100644
>>>> --- a/tools/perf/Documentation/perf.data-file-format.txt
>>>> +++ b/tools/perf/Documentation/perf.data-file-format.txt
>>>> @@ -435,6 +435,24 @@ struct {
>>>>         } [nr_pmu];
>>>>  };
>>>>
>>>> +       HEADER_PMU_CAPS = 32,
>>>> +
>>>> +       List of pmu capabilities (except cpu pmu which is already
>>>> +       covered by HEADER_CPU_PMU_CAPS)
>>>
>>> Sorry for the ignorance, is this currently broken for hybrid then?
>>> Will hybrid have a HEADER_CPU_PMU_CAPS? Presumably this varies between
>>> ARM's big.little and Alderlake.
>>
>> It's covered by HEADER_HYBRID_CPU_PMU_CAPS, but that too covers only
>> cpu pmu. I think I should update the above comment to:
>>
>>         List of pmu capabilities (except cpu pmu which is already
>>         covered by HEADER_CPU_PMU_CAPS / HEADER_HYBRID_CPU_PMU_CAPS)
>>
>>>> +
>>>> +struct {
>>>> +       u32 nr_pmus;
>>>> +       struct {
>>>> +               u32 core_type;  /* For hybrid topology */
>>>
>>> Could this be pmu_type as presumably we can have capabilities on any
>>> kind of PMU?
>>
>> Not sure I follow that question but let me just put my thoughts here.
>>
>> {core_type, pmu_name} is the unique key here. Considering a hypothetical
>> scenario: A system has two types of cores P-core and E-core. Certain pmu
>> inside P-core has some capabilities which are missing in the identical
>> pmu belonging to E-core. The header will look something like:
>>
>> struct {
>>         .nr_pmus = 2,
>>         [0] = struct {
>>                 .core_type = 0, /* P-core */
>>                 .pmu_name = xyz_pmu,
>>                 .nr_caps = 2,
>>                 [0] = { .name = cap1, .value = value1 },
>>                 [1] = { .name = cap2, .value = value2 },
>>         },
>>         [1] = struct {
>>                 .core_type = 1; /* E-core */
>>                 .pmu_name = xyz_pmu;
>>                 .nr_caps = 1;
>>                 [0] = { .name = cap1, .value = value1 };
>>         },
>> };
>>
>> Does that answer your question?
>>
>> Thanks for the review,
>> Ravi
> 
> I may be being a little ahead of the current code as I'm wondering
> about heterogeneous systems with many non-CPU PMUs. It seems such a
> scenario just wouldn't touch the core_type field here. Could the p or
> e core-ness of a PMU be implied by the name?

Using just pmu_name to identify the type of core it belongs to; yeah
that might work assuming perf_pmu_register() doesn't allow registering
multiple pmus with the same name. I'll remove 'core_type'.

> Is there something similar to core_type in sysfs

I don't think so. I don't have any Intel ADL or ARM big.LITTLE system
to try. But that's not required now.

Thanks,
Ravi

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-05-20  5:21 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-19  5:43 [PATCH v3 0/5] perf/amd: Zen4 IBS extensions support (tool changes) Ravi Bangoria
2022-05-19  5:43 ` [PATCH v3 1/5] perf record ibs: Warn about sampling period skew Ravi Bangoria
2022-05-19 22:14   ` Ian Rogers
2022-05-19  5:43 ` [PATCH v3 2/5] perf header: Parse non-cpu pmu capabilities Ravi Bangoria
2022-05-19 22:27   ` Ian Rogers
2022-05-20  3:49     ` Ravi Bangoria
2022-05-20  4:31       ` Ian Rogers
2022-05-20  5:20         ` Ravi Bangoria
2022-05-19  5:43 ` [PATCH v3 3/5] perf/x86/ibs: Add new IBS register bits into header Ravi Bangoria
2022-05-19 23:47   ` Ian Rogers
2022-05-19  5:43 ` [PATCH v3 4/5] perf tool ibs: Sync amd ibs header file Ravi Bangoria
2022-05-19 23:48   ` Ian Rogers
2022-05-20  3:55     ` Ravi Bangoria
2022-05-20  4:32       ` Ian Rogers
2022-05-19  5:43 ` [PATCH v3 5/5] perf script ibs: Support new IBS bits in raw trace dump Ravi Bangoria
2022-05-19 23:50   ` Ian Rogers

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.