[PATCH v13 0/3] Add perf support for HiSilicon PCIe Tune and Trace device

[PATCH v13 0/3] Add perf support for HiSilicon PCIe Tune and Trace device

[Yicong Yang]

From: Yicong Yang <yangyicong@hisilicon.com>

This patchset adds the perf tool support for HiSilicon PCIe Tune and Trace
device [1]. The device driver makes use of perf AUX trace for tracing
TLP (Transaction Layer Packet) headers of PCIe. The trace can be used by
`perf record` and the traced data can be decoded by `perf report` with
this patchset. The detailed usage is documented in [1].

This is split from the v11 series as suggested to send driver part and
perf tool part separately. The perf tool part has no change since v11.
The whole changelog can be found at [2].

Since the driver has been accepted, wish to have perf support along with
the PTT driver to enable the full support.

[1] https://lore.kernel.org/lkml/20220816114414.4092-5-yangyicong@huawei.com/
[2] https://lore.kernel.org/lkml/20220816114414.4092-1-yangyicong@huawei.com/

Change since v12:
- Remove unused headers and some minor fixes, Per John. Thanks!
Link: https://lore.kernel.org/lkml/20220914075925.48549-1-yangyicong@huawei.com/

Qi Liu (3):
  perf tool: arm: Refactor event list iteration in
    auxtrace_record__init()
  perf tool: Add support for HiSilicon PCIe Tune and Trace device driver
  perf tool: Add support for parsing HiSilicon PCIe Trace packet

 tools/perf/arch/arm/util/auxtrace.c           | 116 +++++++++--
 tools/perf/arch/arm/util/pmu.c                |   3 +
 tools/perf/arch/arm64/util/Build              |   2 +-
 tools/perf/arch/arm64/util/hisi-ptt.c         | 188 +++++++++++++++++
 tools/perf/util/Build                         |   2 +
 tools/perf/util/auxtrace.c                    |   4 +
 tools/perf/util/auxtrace.h                    |   1 +
 tools/perf/util/hisi-ptt-decoder/Build        |   1 +
 .../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c   | 164 +++++++++++++++
 .../hisi-ptt-decoder/hisi-ptt-pkt-decoder.h   |  31 +++
 tools/perf/util/hisi-ptt.c                    | 192 ++++++++++++++++++
 tools/perf/util/hisi-ptt.h                    |  19 ++
 12 files changed, 703 insertions(+), 20 deletions(-)
 create mode 100644 tools/perf/arch/arm64/util/hisi-ptt.c
 create mode 100644 tools/perf/util/hisi-ptt-decoder/Build
 create mode 100644 tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
 create mode 100644 tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
 create mode 100644 tools/perf/util/hisi-ptt.c
 create mode 100644 tools/perf/util/hisi-ptt.h

-- 
2.24.0

[PATCH v13 1/3] perf tool: arm: Refactor event list iteration in auxtrace_record__init()

[Yicong Yang]

From: Qi Liu <liuqi115@huawei.com>

Add find_pmu_for_event() and use to simplify logic in
auxtrace_record_init(). find_pmu_for_event() will be
reused in subsequent patches.

Reviewed-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Qi Liu <liuqi115@huawei.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 tools/perf/arch/arm/util/auxtrace.c | 53 ++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/tools/perf/arch/arm/util/auxtrace.c b/tools/perf/arch/arm/util/auxtrace.c
index 5fc6a2a3dbc5..384c7cfda0fd 100644
--- a/tools/perf/arch/arm/util/auxtrace.c
+++ b/tools/perf/arch/arm/util/auxtrace.c
@@ -50,16 +50,32 @@ static struct perf_pmu **find_all_arm_spe_pmus(int *nr_spes, int *err)
 	return arm_spe_pmus;
 }
 
+static struct perf_pmu *find_pmu_for_event(struct perf_pmu **pmus,
+					   int pmu_nr, struct evsel *evsel)
+{
+	int i;
+
+	if (!pmus)
+		return NULL;
+
+	for (i = 0; i < pmu_nr; i++) {
+		if (evsel->core.attr.type == pmus[i]->type)
+			return pmus[i];
+	}
+
+	return NULL;
+}
+
 struct auxtrace_record
 *auxtrace_record__init(struct evlist *evlist, int *err)
 {
-	struct perf_pmu	*cs_etm_pmu;
+	struct perf_pmu	*cs_etm_pmu = NULL;
+	struct perf_pmu **arm_spe_pmus = NULL;
 	struct evsel *evsel;
-	bool found_etm = false;
+	struct perf_pmu *found_etm = NULL;
 	struct perf_pmu *found_spe = NULL;
-	struct perf_pmu **arm_spe_pmus = NULL;
+	int auxtrace_event_cnt = 0;
 	int nr_spes = 0;
-	int i = 0;
 
 	if (!evlist)
 		return NULL;
@@ -68,24 +84,23 @@ struct auxtrace_record
 	arm_spe_pmus = find_all_arm_spe_pmus(&nr_spes, err);
 
 	evlist__for_each_entry(evlist, evsel) {
-		if (cs_etm_pmu &&
-		    evsel->core.attr.type == cs_etm_pmu->type)
-			found_etm = true;
-
-		if (!nr_spes || found_spe)
-			continue;
-
-		for (i = 0; i < nr_spes; i++) {
-			if (evsel->core.attr.type == arm_spe_pmus[i]->type) {
-				found_spe = arm_spe_pmus[i];
-				break;
-			}
-		}
+		if (cs_etm_pmu && !found_etm)
+			found_etm = find_pmu_for_event(&cs_etm_pmu, 1, evsel);
+
+		if (arm_spe_pmus && !found_spe)
+			found_spe = find_pmu_for_event(arm_spe_pmus, nr_spes, evsel);
 	}
+
 	free(arm_spe_pmus);
 
-	if (found_etm && found_spe) {
-		pr_err("Concurrent ARM Coresight ETM and SPE operation not currently supported\n");
+	if (found_etm)
+		auxtrace_event_cnt++;
+
+	if (found_spe)
+		auxtrace_event_cnt++;
+
+	if (auxtrace_event_cnt > 1) {
+		pr_err("Concurrent AUX trace operation not currently supported\n");
 		*err = -EOPNOTSUPP;
 		return NULL;
 	}
-- 
2.24.0

[PATCH v13 2/3] perf tool: Add support for HiSilicon PCIe Tune and Trace device driver

[Yicong Yang]

From: Qi Liu <liuqi115@huawei.com>

HiSilicon PCIe tune and trace device (PTT) could dynamically tune
the PCIe link's events, and trace the TLP headers).

This patch add support for PTT device in perf tool, so users could
use 'perf record' to get TLP headers trace data.

Reviewed-by: Leo Yan <leo.yan@linaro.org>
Signed-off-by: Qi Liu <liuqi115@huawei.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 tools/perf/arch/arm/util/auxtrace.c   |  63 +++++++++
 tools/perf/arch/arm/util/pmu.c        |   3 +
 tools/perf/arch/arm64/util/Build      |   2 +-
 tools/perf/arch/arm64/util/hisi-ptt.c | 188 ++++++++++++++++++++++++++
 tools/perf/util/auxtrace.c            |   1 +
 tools/perf/util/auxtrace.h            |   1 +
 tools/perf/util/hisi-ptt.h            |  16 +++
 7 files changed, 273 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/arch/arm64/util/hisi-ptt.c
 create mode 100644 tools/perf/util/hisi-ptt.h

diff --git a/tools/perf/arch/arm/util/auxtrace.c b/tools/perf/arch/arm/util/auxtrace.c
index 384c7cfda0fd..129ed72391a4 100644
--- a/tools/perf/arch/arm/util/auxtrace.c
+++ b/tools/perf/arch/arm/util/auxtrace.c
@@ -4,9 +4,11 @@
  * Author: Mathieu Poirier <mathieu.poirier@linaro.org>
  */
 
+#include <dirent.h>
 #include <stdbool.h>
 #include <linux/coresight-pmu.h>
 #include <linux/zalloc.h>
+#include <api/fs/fs.h>
 
 #include "../../../util/auxtrace.h"
 #include "../../../util/debug.h"
@@ -14,6 +16,7 @@
 #include "../../../util/pmu.h"
 #include "cs-etm.h"
 #include "arm-spe.h"
+#include "hisi-ptt.h"
 
 static struct perf_pmu **find_all_arm_spe_pmus(int *nr_spes, int *err)
 {
@@ -50,6 +53,52 @@ static struct perf_pmu **find_all_arm_spe_pmus(int *nr_spes, int *err)
 	return arm_spe_pmus;
 }
 
+static struct perf_pmu **find_all_hisi_ptt_pmus(int *nr_ptts, int *err)
+{
+	const char *sysfs = sysfs__mountpoint();
+	struct perf_pmu **hisi_ptt_pmus = NULL;
+	struct dirent *dent;
+	char path[PATH_MAX];
+	DIR *dir = NULL;
+	int idx = 0;
+
+	snprintf(path, PATH_MAX, "%s" EVENT_SOURCE_DEVICE_PATH, sysfs);
+	dir = opendir(path);
+	if (!dir) {
+		pr_err("can't read directory '%s'\n", EVENT_SOURCE_DEVICE_PATH);
+		*err = -EINVAL;
+		goto out;
+	}
+
+	while ((dent = readdir(dir))) {
+		if (strstr(dent->d_name, HISI_PTT_PMU_NAME))
+			(*nr_ptts)++;
+	}
+
+	if (!(*nr_ptts))
+		goto out;
+
+	hisi_ptt_pmus = zalloc(sizeof(struct perf_pmu *) * (*nr_ptts));
+	if (!hisi_ptt_pmus) {
+		pr_err("hisi_ptt alloc failed\n");
+		*err = -ENOMEM;
+		goto out;
+	}
+
+	rewinddir(dir);
+	while ((dent = readdir(dir))) {
+		if (strstr(dent->d_name, HISI_PTT_PMU_NAME) && idx < (*nr_ptts)) {
+			hisi_ptt_pmus[idx] = perf_pmu__find(dent->d_name);
+			if (hisi_ptt_pmus[idx])
+				idx++;
+		}
+	}
+
+out:
+	closedir(dir);
+	return hisi_ptt_pmus;
+}
+
 static struct perf_pmu *find_pmu_for_event(struct perf_pmu **pmus,
 					   int pmu_nr, struct evsel *evsel)
 {
@@ -71,17 +120,21 @@ struct auxtrace_record
 {
 	struct perf_pmu	*cs_etm_pmu = NULL;
 	struct perf_pmu **arm_spe_pmus = NULL;
+	struct perf_pmu **hisi_ptt_pmus = NULL;
 	struct evsel *evsel;
 	struct perf_pmu *found_etm = NULL;
 	struct perf_pmu *found_spe = NULL;
+	struct perf_pmu *found_ptt = NULL;
 	int auxtrace_event_cnt = 0;
 	int nr_spes = 0;
+	int nr_ptts = 0;
 
 	if (!evlist)
 		return NULL;
 
 	cs_etm_pmu = perf_pmu__find(CORESIGHT_ETM_PMU_NAME);
 	arm_spe_pmus = find_all_arm_spe_pmus(&nr_spes, err);
+	hisi_ptt_pmus = find_all_hisi_ptt_pmus(&nr_ptts, err);
 
 	evlist__for_each_entry(evlist, evsel) {
 		if (cs_etm_pmu && !found_etm)
@@ -89,9 +142,13 @@ struct auxtrace_record
 
 		if (arm_spe_pmus && !found_spe)
 			found_spe = find_pmu_for_event(arm_spe_pmus, nr_spes, evsel);
+
+		if (hisi_ptt_pmus && !found_ptt)
+			found_ptt = find_pmu_for_event(hisi_ptt_pmus, nr_ptts, evsel);
 	}
 
 	free(arm_spe_pmus);
+	free(hisi_ptt_pmus);
 
 	if (found_etm)
 		auxtrace_event_cnt++;
@@ -99,6 +156,9 @@ struct auxtrace_record
 	if (found_spe)
 		auxtrace_event_cnt++;
 
+	if (found_ptt)
+		auxtrace_event_cnt++;
+
 	if (auxtrace_event_cnt > 1) {
 		pr_err("Concurrent AUX trace operation not currently supported\n");
 		*err = -EOPNOTSUPP;
@@ -111,6 +171,9 @@ struct auxtrace_record
 #if defined(__aarch64__)
 	if (found_spe)
 		return arm_spe_recording_init(err, found_spe);
+
+	if (found_ptt)
+		return hisi_ptt_recording_init(err, found_ptt);
 #endif
 
 	/*
diff --git a/tools/perf/arch/arm/util/pmu.c b/tools/perf/arch/arm/util/pmu.c
index b8b23b9dc598..887c8addc491 100644
--- a/tools/perf/arch/arm/util/pmu.c
+++ b/tools/perf/arch/arm/util/pmu.c
@@ -10,6 +10,7 @@
 #include <linux/string.h>
 
 #include "arm-spe.h"
+#include "hisi-ptt.h"
 #include "../../../util/pmu.h"
 
 struct perf_event_attr
@@ -22,6 +23,8 @@ struct perf_event_attr
 #if defined(__aarch64__)
 	} else if (strstarts(pmu->name, ARM_SPE_PMU_NAME)) {
 		return arm_spe_pmu_default_config(pmu);
+	} else if (strstarts(pmu->name, HISI_PTT_PMU_NAME)) {
+		pmu->selectable = true;
 #endif
 	}
 
diff --git a/tools/perf/arch/arm64/util/Build b/tools/perf/arch/arm64/util/Build
index 9fcb4e68add9..337aa9bdf905 100644
--- a/tools/perf/arch/arm64/util/Build
+++ b/tools/perf/arch/arm64/util/Build
@@ -11,4 +11,4 @@ perf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
 perf-$(CONFIG_AUXTRACE) += ../../arm/util/pmu.o \
 			      ../../arm/util/auxtrace.o \
 			      ../../arm/util/cs-etm.o \
-			      arm-spe.o mem-events.o
+			      arm-spe.o mem-events.o hisi-ptt.o
diff --git a/tools/perf/arch/arm64/util/hisi-ptt.c b/tools/perf/arch/arm64/util/hisi-ptt.c
new file mode 100644
index 000000000000..ba97c8a562a0
--- /dev/null
+++ b/tools/perf/arch/arm64/util/hisi-ptt.c
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HiSilicon PCIe Trace and Tuning (PTT) support
+ * Copyright (c) 2022 HiSilicon Technologies Co., Ltd.
+ */
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/bitops.h>
+#include <linux/log2.h>
+#include <linux/zalloc.h>
+#include <time.h>
+
+#include <internal/lib.h> // page_size
+#include "../../../util/auxtrace.h"
+#include "../../../util/cpumap.h"
+#include "../../../util/debug.h"
+#include "../../../util/event.h"
+#include "../../../util/evlist.h"
+#include "../../../util/evsel.h"
+#include "../../../util/hisi-ptt.h"
+#include "../../../util/pmu.h"
+#include "../../../util/record.h"
+#include "../../../util/session.h"
+#include "../../../util/tsc.h"
+
+#define KiB(x) ((x) * 1024)
+#define MiB(x) ((x) * 1024 * 1024)
+
+struct hisi_ptt_recording {
+	struct auxtrace_record	itr;
+	struct perf_pmu *hisi_ptt_pmu;
+	struct evlist *evlist;
+};
+
+static size_t
+hisi_ptt_info_priv_size(struct auxtrace_record *itr __maybe_unused,
+			struct evlist *evlist __maybe_unused)
+{
+	return HISI_PTT_AUXTRACE_PRIV_SIZE;
+}
+
+static int hisi_ptt_info_fill(struct auxtrace_record *itr,
+			      struct perf_session *session,
+			      struct perf_record_auxtrace_info *auxtrace_info,
+			      size_t priv_size)
+{
+	struct hisi_ptt_recording *pttr =
+			container_of(itr, struct hisi_ptt_recording, itr);
+	struct perf_pmu *hisi_ptt_pmu = pttr->hisi_ptt_pmu;
+
+	if (priv_size != HISI_PTT_AUXTRACE_PRIV_SIZE)
+		return -EINVAL;
+
+	if (!session->evlist->core.nr_mmaps)
+		return -EINVAL;
+
+	auxtrace_info->type = PERF_AUXTRACE_HISI_PTT;
+	auxtrace_info->priv[0] = hisi_ptt_pmu->type;
+
+	return 0;
+}
+
+static int hisi_ptt_set_auxtrace_mmap_page(struct record_opts *opts)
+{
+	bool privileged = perf_event_paranoid_check(-1);
+
+	if (!opts->full_auxtrace)
+		return 0;
+
+	if (opts->full_auxtrace && !opts->auxtrace_mmap_pages) {
+		if (privileged) {
+			opts->auxtrace_mmap_pages = MiB(16) / page_size;
+		} else {
+			opts->auxtrace_mmap_pages = KiB(128) / page_size;
+			if (opts->mmap_pages == UINT_MAX)
+				opts->mmap_pages = KiB(256) / page_size;
+		}
+	}
+
+	/* Validate auxtrace_mmap_pages */
+	if (opts->auxtrace_mmap_pages) {
+		size_t sz = opts->auxtrace_mmap_pages * (size_t)page_size;
+		size_t min_sz = KiB(8);
+
+		if (sz < min_sz || !is_power_of_2(sz)) {
+			pr_err("Invalid mmap size for HISI PTT: must be at least %zuKiB and a power of 2\n",
+			       min_sz / 1024);
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static int hisi_ptt_recording_options(struct auxtrace_record *itr,
+				      struct evlist *evlist,
+				      struct record_opts *opts)
+{
+	struct hisi_ptt_recording *pttr =
+			container_of(itr, struct hisi_ptt_recording, itr);
+	struct perf_pmu *hisi_ptt_pmu = pttr->hisi_ptt_pmu;
+	struct evsel *evsel, *hisi_ptt_evsel = NULL;
+	struct evsel *tracking_evsel;
+	int err;
+
+	pttr->evlist = evlist;
+	evlist__for_each_entry(evlist, evsel) {
+		if (evsel->core.attr.type == hisi_ptt_pmu->type) {
+			if (hisi_ptt_evsel) {
+				pr_err("There may be only one " HISI_PTT_PMU_NAME "x event\n");
+				return -EINVAL;
+			}
+			evsel->core.attr.freq = 0;
+			evsel->core.attr.sample_period = 1;
+			evsel->needs_auxtrace_mmap = true;
+			hisi_ptt_evsel = evsel;
+			opts->full_auxtrace = true;
+		}
+	}
+
+	err = hisi_ptt_set_auxtrace_mmap_page(opts);
+	if (err)
+		return err;
+	/*
+	 * To obtain the auxtrace buffer file descriptor, the auxtrace event
+	 * must come first.
+	 */
+	evlist__to_front(evlist, hisi_ptt_evsel);
+	evsel__set_sample_bit(hisi_ptt_evsel, TIME);
+
+	/* Add dummy event to keep tracking */
+	err = parse_event(evlist, "dummy:u");
+	if (err)
+		return err;
+
+	tracking_evsel = evlist__last(evlist);
+	evlist__set_tracking_event(evlist, tracking_evsel);
+
+	tracking_evsel->core.attr.freq = 0;
+	tracking_evsel->core.attr.sample_period = 1;
+	evsel__set_sample_bit(tracking_evsel, TIME);
+
+	return 0;
+}
+
+static u64 hisi_ptt_reference(struct auxtrace_record *itr __maybe_unused)
+{
+	return rdtsc();
+}
+
+static void hisi_ptt_recording_free(struct auxtrace_record *itr)
+{
+	struct hisi_ptt_recording *pttr =
+			container_of(itr, struct hisi_ptt_recording, itr);
+
+	free(pttr);
+}
+
+struct auxtrace_record *hisi_ptt_recording_init(int *err,
+						struct perf_pmu *hisi_ptt_pmu)
+{
+	struct hisi_ptt_recording *pttr;
+
+	if (!hisi_ptt_pmu) {
+		*err = -ENODEV;
+		return NULL;
+	}
+
+	pttr = zalloc(sizeof(*pttr));
+	if (!pttr) {
+		*err = -ENOMEM;
+		return NULL;
+	}
+
+	pttr->hisi_ptt_pmu = hisi_ptt_pmu;
+	pttr->itr.pmu = hisi_ptt_pmu;
+	pttr->itr.recording_options = hisi_ptt_recording_options;
+	pttr->itr.info_priv_size = hisi_ptt_info_priv_size;
+	pttr->itr.info_fill = hisi_ptt_info_fill;
+	pttr->itr.free = hisi_ptt_recording_free;
+	pttr->itr.reference = hisi_ptt_reference;
+	pttr->itr.read_finish = auxtrace_record__read_finish;
+	pttr->itr.alignment = 0;
+
+	*err = 0;
+	return &pttr->itr;
+}
diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index 6edab8a16de6..c30611d9ee99 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -1319,6 +1319,7 @@ int perf_event__process_auxtrace_info(struct perf_session *session,
 	case PERF_AUXTRACE_S390_CPUMSF:
 		err = s390_cpumsf_process_auxtrace_info(event, session);
 		break;
+	case PERF_AUXTRACE_HISI_PTT:
 	case PERF_AUXTRACE_UNKNOWN:
 	default:
 		return -EINVAL;
diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
index 6a4fbfd34c6b..3a122fc01ccd 100644
--- a/tools/perf/util/auxtrace.h
+++ b/tools/perf/util/auxtrace.h
@@ -48,6 +48,7 @@ enum auxtrace_type {
 	PERF_AUXTRACE_CS_ETM,
 	PERF_AUXTRACE_ARM_SPE,
 	PERF_AUXTRACE_S390_CPUMSF,
+	PERF_AUXTRACE_HISI_PTT,
 };
 
 enum itrace_period_type {
diff --git a/tools/perf/util/hisi-ptt.h b/tools/perf/util/hisi-ptt.h
new file mode 100644
index 000000000000..82283c81b4c1
--- /dev/null
+++ b/tools/perf/util/hisi-ptt.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * HiSilicon PCIe Trace and Tuning (PTT) support
+ * Copyright (c) 2022 HiSilicon Technologies Co., Ltd.
+ */
+
+#ifndef INCLUDE__PERF_HISI_PTT_H__
+#define INCLUDE__PERF_HISI_PTT_H__
+
+#define HISI_PTT_PMU_NAME		"hisi_ptt"
+#define HISI_PTT_AUXTRACE_PRIV_SIZE	sizeof(u64)
+
+struct auxtrace_record *hisi_ptt_recording_init(int *err,
+						struct perf_pmu *hisi_ptt_pmu);
+
+#endif
-- 
2.24.0

[PATCH v13 3/3] perf tool: Add support for parsing HiSilicon PCIe Trace packet

[Yicong Yang]

From: Qi Liu <liuqi115@huawei.com>

Add support for using 'perf report --dump-raw-trace' to parse PTT packet.

Example usage:

Output will contain raw PTT data and its textual representation, such
as:

0 0 0x5810 [0x30]: PERF_RECORD_AUXTRACE size: 0x400000  offset: 0
ref: 0xa5d50c725  idx: 0  tid: -1  cpu: 0
.
. ... HISI PTT data: size 4194304 bytes
.  00000000: 00 00 00 00                                 Prefix
.  00000004: 08 20 00 60                                 Header DW0
.  00000008: ff 02 00 01                                 Header DW1
.  0000000c: 20 08 00 00                                 Header DW2
.  00000010: 10 e7 44 ab                                 Header DW3
.  00000014: 2a a8 1e 01                                 Time
.  00000020: 00 00 00 00                                 Prefix
.  00000024: 01 00 00 60                                 Header DW0
.  00000028: 0f 1e 00 01                                 Header DW1
.  0000002c: 04 00 00 00                                 Header DW2
.  00000030: 40 00 81 02                                 Header DW3
.  00000034: ee 02 00 00                                 Time
....

Signed-off-by: Qi Liu <liuqi115@huawei.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 tools/perf/util/Build                         |   2 +
 tools/perf/util/auxtrace.c                    |   3 +
 tools/perf/util/hisi-ptt-decoder/Build        |   1 +
 .../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c   | 164 +++++++++++++++
 .../hisi-ptt-decoder/hisi-ptt-pkt-decoder.h   |  31 +++
 tools/perf/util/hisi-ptt.c                    | 192 ++++++++++++++++++
 tools/perf/util/hisi-ptt.h                    |   3 +
 7 files changed, 396 insertions(+)
 create mode 100644 tools/perf/util/hisi-ptt-decoder/Build
 create mode 100644 tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
 create mode 100644 tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
 create mode 100644 tools/perf/util/hisi-ptt.c

diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 9dfae1bda9cc..9bad8568071d 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -118,6 +118,8 @@ perf-$(CONFIG_AUXTRACE) += intel-pt.o
 perf-$(CONFIG_AUXTRACE) += intel-bts.o
 perf-$(CONFIG_AUXTRACE) += arm-spe.o
 perf-$(CONFIG_AUXTRACE) += arm-spe-decoder/
+perf-$(CONFIG_AUXTRACE) += hisi-ptt.o
+perf-$(CONFIG_AUXTRACE) += hisi-ptt-decoder/
 perf-$(CONFIG_AUXTRACE) += s390-cpumsf.o
 
 ifdef CONFIG_LIBOPENCSD
diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index c30611d9ee99..e799c7314d39 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -51,6 +51,7 @@
 #include "intel-pt.h"
 #include "intel-bts.h"
 #include "arm-spe.h"
+#include "hisi-ptt.h"
 #include "s390-cpumsf.h"
 #include "util/mmap.h"
 
@@ -1320,6 +1321,8 @@ int perf_event__process_auxtrace_info(struct perf_session *session,
 		err = s390_cpumsf_process_auxtrace_info(event, session);
 		break;
 	case PERF_AUXTRACE_HISI_PTT:
+		err = hisi_ptt_process_auxtrace_info(event, session);
+		break;
 	case PERF_AUXTRACE_UNKNOWN:
 	default:
 		return -EINVAL;
diff --git a/tools/perf/util/hisi-ptt-decoder/Build b/tools/perf/util/hisi-ptt-decoder/Build
new file mode 100644
index 000000000000..db3db8b75033
--- /dev/null
+++ b/tools/perf/util/hisi-ptt-decoder/Build
@@ -0,0 +1 @@
+perf-$(CONFIG_AUXTRACE) += hisi-ptt-pkt-decoder.o
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
new file mode 100644
index 000000000000..a17c423a526d
--- /dev/null
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HiSilicon PCIe Trace and Tuning (PTT) support
+ * Copyright (c) 2022 HiSilicon Technologies Co., Ltd.
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <endian.h>
+#include <byteswap.h>
+#include <linux/bitops.h>
+#include <stdarg.h>
+
+#include "../color.h"
+#include "hisi-ptt-pkt-decoder.h"
+
+/*
+ * For 8DW format, the bit[31:11] of DW0 is always 0x1fffff, which can be
+ * used to distinguish the data format.
+ * 8DW format is like:
+ *   bits [                 31:11                 ][       10:0       ]
+ *        |---------------------------------------|-------------------|
+ *    DW0 [                0x1fffff               ][ Reserved (0x7ff) ]
+ *    DW1 [                       Prefix                              ]
+ *    DW2 [                     Header DW0                            ]
+ *    DW3 [                     Header DW1                            ]
+ *    DW4 [                     Header DW2                            ]
+ *    DW5 [                     Header DW3                            ]
+ *    DW6 [                   Reserved (0x0)                          ]
+ *    DW7 [                        Time                               ]
+ *
+ * 4DW format is like:
+ *   bits [31:30] [ 29:25 ][24][23][22][21][    20:11   ][    10:0    ]
+ *        |-----|---------|---|---|---|---|-------------|-------------|
+ *    DW0 [ Fmt ][  Type  ][T9][T8][TH][SO][   Length   ][    Time    ]
+ *    DW1 [                     Header DW1                            ]
+ *    DW2 [                     Header DW2                            ]
+ *    DW3 [                     Header DW3                            ]
+ */
+
+enum hisi_ptt_8dw_pkt_field_type {
+	HISI_PTT_8DW_CHK_AND_RSV0,
+	HISI_PTT_8DW_PREFIX,
+	HISI_PTT_8DW_HEAD0,
+	HISI_PTT_8DW_HEAD1,
+	HISI_PTT_8DW_HEAD2,
+	HISI_PTT_8DW_HEAD3,
+	HISI_PTT_8DW_RSV1,
+	HISI_PTT_8DW_TIME,
+	HISI_PTT_8DW_TYPE_MAX
+};
+
+enum hisi_ptt_4dw_pkt_field_type {
+	HISI_PTT_4DW_HEAD1,
+	HISI_PTT_4DW_HEAD2,
+	HISI_PTT_4DW_HEAD3,
+	HISI_PTT_4DW_TYPE_MAX
+};
+
+static const char * const hisi_ptt_8dw_pkt_field_name[] = {
+	[HISI_PTT_8DW_PREFIX]	= "Prefix",
+	[HISI_PTT_8DW_HEAD0]	= "Header DW0",
+	[HISI_PTT_8DW_HEAD1]	= "Header DW1",
+	[HISI_PTT_8DW_HEAD2]	= "Header DW2",
+	[HISI_PTT_8DW_HEAD3]	= "Header DW3",
+	[HISI_PTT_8DW_TIME]	= "Time"
+};
+
+static const char * const hisi_ptt_4dw_pkt_field_name[] = {
+	[HISI_PTT_4DW_HEAD1]	= "Header DW1",
+	[HISI_PTT_4DW_HEAD2]	= "Header DW2",
+	[HISI_PTT_4DW_HEAD3]	= "Header DW3",
+};
+
+union hisi_ptt_4dw {
+	struct {
+		uint32_t format : 2;
+		uint32_t type : 5;
+		uint32_t t9 : 1;
+		uint32_t t8 : 1;
+		uint32_t th : 1;
+		uint32_t so : 1;
+		uint32_t len : 10;
+		uint32_t time : 11;
+	};
+	uint32_t value;
+};
+
+static void hisi_ptt_print_pkt(const unsigned char *buf, int pos, const char *desc)
+{
+	const char *color = PERF_COLOR_BLUE;
+	int i;
+
+	printf(".");
+	color_fprintf(stdout, color, "  %08x: ", pos);
+	for (i = 0; i < HISI_PTT_FIELD_LENTH; i++)
+		color_fprintf(stdout, color, "%02x ", buf[pos + i]);
+	for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
+		color_fprintf(stdout, color, "   ");
+	color_fprintf(stdout, color, "  %s\n", desc);
+}
+
+static int hisi_ptt_8dw_kpt_desc(const unsigned char *buf, int pos)
+{
+	int i;
+
+	for (i = 0; i < HISI_PTT_8DW_TYPE_MAX; i++) {
+		/* Do not show 8DW check field and reserved fields */
+		if (i == HISI_PTT_8DW_CHK_AND_RSV0 || i == HISI_PTT_8DW_RSV1) {
+			pos += HISI_PTT_FIELD_LENTH;
+			continue;
+		}
+
+		hisi_ptt_print_pkt(buf, pos, hisi_ptt_8dw_pkt_field_name[i]);
+		pos += HISI_PTT_FIELD_LENTH;
+	}
+
+	return hisi_ptt_pkt_size[HISI_PTT_8DW_PKT];
+}
+
+static void hisi_ptt_4dw_print_dw0(const unsigned char *buf, int pos)
+{
+	const char *color = PERF_COLOR_BLUE;
+	union hisi_ptt_4dw dw0;
+	int i;
+
+	dw0.value = *(uint32_t *)(buf + pos);
+	printf(".");
+	color_fprintf(stdout, color, "  %08x: ", pos);
+	for (i = 0; i < HISI_PTT_FIELD_LENTH; i++)
+		color_fprintf(stdout, color, "%02x ", buf[pos + i]);
+	for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
+		color_fprintf(stdout, color, "   ");
+
+	color_fprintf(stdout, color,
+		      "  %s %x %s %x %s %x %s %x %s %x %s %x %s %x %s %x\n",
+		      "Format", dw0.format, "Type", dw0.type, "T9", dw0.t9,
+		      "T8", dw0.t8, "TH", dw0.th, "SO", dw0.so, "Length",
+		      dw0.len, "Time", dw0.time);
+}
+
+static int hisi_ptt_4dw_kpt_desc(const unsigned char *buf, int pos)
+{
+	int i;
+
+	hisi_ptt_4dw_print_dw0(buf, pos);
+	pos += HISI_PTT_FIELD_LENTH;
+
+	for (i = 0; i < HISI_PTT_4DW_TYPE_MAX; i++) {
+		hisi_ptt_print_pkt(buf, pos, hisi_ptt_4dw_pkt_field_name[i]);
+		pos += HISI_PTT_FIELD_LENTH;
+	}
+
+	return hisi_ptt_pkt_size[HISI_PTT_4DW_PKT];
+}
+
+int hisi_ptt_pkt_desc(const unsigned char *buf, int pos, enum hisi_ptt_pkt_type type)
+{
+	if (type == HISI_PTT_8DW_PKT)
+		return hisi_ptt_8dw_kpt_desc(buf, pos);
+
+	return hisi_ptt_4dw_kpt_desc(buf, pos);
+}
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
new file mode 100644
index 000000000000..e78f1b5bc836
--- /dev/null
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * HiSilicon PCIe Trace and Tuning (PTT) support
+ * Copyright (c) 2022 HiSilicon Technologies Co., Ltd.
+ */
+
+#ifndef INCLUDE__HISI_PTT_PKT_DECODER_H__
+#define INCLUDE__HISI_PTT_PKT_DECODER_H__
+
+#include <stddef.h>
+#include <stdint.h>
+
+#define HISI_PTT_8DW_CHECK_MASK		GENMASK(31, 11)
+#define HISI_PTT_IS_8DW_PKT		GENMASK(31, 11)
+#define HISI_PTT_MAX_SPACE_LEN		10
+#define HISI_PTT_FIELD_LENTH		4
+
+enum hisi_ptt_pkt_type {
+	HISI_PTT_4DW_PKT,
+	HISI_PTT_8DW_PKT,
+	HISI_PTT_PKT_MAX
+};
+
+static int hisi_ptt_pkt_size[] = {
+	[HISI_PTT_4DW_PKT]	= 16,
+	[HISI_PTT_8DW_PKT]	= 32,
+};
+
+int hisi_ptt_pkt_desc(const unsigned char *buf, int pos, enum hisi_ptt_pkt_type type);
+
+#endif
diff --git a/tools/perf/util/hisi-ptt.c b/tools/perf/util/hisi-ptt.c
new file mode 100644
index 000000000000..45b614bb73bf
--- /dev/null
+++ b/tools/perf/util/hisi-ptt.c
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HiSilicon PCIe Trace and Tuning (PTT) support
+ * Copyright (c) 2022 HiSilicon Technologies Co., Ltd.
+ */
+
+#include <byteswap.h>
+#include <endian.h>
+#include <errno.h>
+#include <inttypes.h>
+#include <linux/bitops.h>
+#include <linux/kernel.h>
+#include <linux/log2.h>
+#include <linux/types.h>
+#include <linux/zalloc.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include "auxtrace.h"
+#include "color.h"
+#include "debug.h"
+#include "evsel.h"
+#include "hisi-ptt.h"
+#include "hisi-ptt-decoder/hisi-ptt-pkt-decoder.h"
+#include "machine.h"
+#include "session.h"
+#include "tool.h"
+#include <internal/lib.h>
+
+struct hisi_ptt {
+	struct auxtrace auxtrace;
+	u32 auxtrace_type;
+	struct perf_session *session;
+	struct machine *machine;
+	u32 pmu_type;
+};
+
+struct hisi_ptt_queue {
+	struct hisi_ptt *ptt;
+	struct auxtrace_buffer *buffer;
+};
+
+static enum hisi_ptt_pkt_type hisi_ptt_check_packet_type(unsigned char *buf)
+{
+	uint32_t head = *(uint32_t *)buf;
+
+	if ((HISI_PTT_8DW_CHECK_MASK & head) == HISI_PTT_IS_8DW_PKT)
+		return HISI_PTT_8DW_PKT;
+
+	return HISI_PTT_4DW_PKT;
+}
+
+static void hisi_ptt_dump(struct hisi_ptt *ptt __maybe_unused,
+			  unsigned char *buf, size_t len)
+{
+	const char *color = PERF_COLOR_BLUE;
+	enum hisi_ptt_pkt_type type;
+	size_t pos = 0;
+	int pkt_len;
+
+	type = hisi_ptt_check_packet_type(buf);
+	len = round_down(len, hisi_ptt_pkt_size[type]);
+	color_fprintf(stdout, color, ". ... HISI PTT data: size %zu bytes\n",
+		      len);
+
+	while (len > 0) {
+		pkt_len = hisi_ptt_pkt_desc(buf, pos, type);
+		if (!pkt_len)
+			color_fprintf(stdout, color, " Bad packet!\n");
+
+		pos += pkt_len;
+		len -= pkt_len;
+	}
+}
+
+static void hisi_ptt_dump_event(struct hisi_ptt *ptt, unsigned char *buf,
+				size_t len)
+{
+	printf(".\n");
+
+	hisi_ptt_dump(ptt, buf, len);
+}
+
+static int hisi_ptt_process_event(struct perf_session *session __maybe_unused,
+				  union perf_event *event __maybe_unused,
+				  struct perf_sample *sample __maybe_unused,
+				  struct perf_tool *tool __maybe_unused)
+{
+	return 0;
+}
+
+static int hisi_ptt_process_auxtrace_event(struct perf_session *session,
+					   union perf_event *event,
+					   struct perf_tool *tool __maybe_unused)
+{
+	struct hisi_ptt *ptt = container_of(session->auxtrace, struct hisi_ptt,
+					    auxtrace);
+	int fd = perf_data__fd(session->data);
+	int size = event->auxtrace.size;
+	void *data = malloc(size);
+	off_t data_offset;
+	int err;
+
+	if (!data)
+		return -errno;
+
+	if (perf_data__is_pipe(session->data)) {
+		data_offset = 0;
+	} else {
+		data_offset = lseek(fd, 0, SEEK_CUR);
+		if (data_offset == -1)
+			return -errno;
+	}
+
+	err = readn(fd, data, size);
+	if (err != (ssize_t)size) {
+		free(data);
+		return -errno;
+	}
+
+	if (dump_trace)
+		hisi_ptt_dump_event(ptt, data, size);
+
+	return 0;
+}
+
+static int hisi_ptt_flush(struct perf_session *session __maybe_unused,
+			  struct perf_tool *tool __maybe_unused)
+{
+	return 0;
+}
+
+static void hisi_ptt_free_events(struct perf_session *session __maybe_unused)
+{
+}
+
+static void hisi_ptt_free(struct perf_session *session)
+{
+	struct hisi_ptt *ptt = container_of(session->auxtrace, struct hisi_ptt,
+					    auxtrace);
+
+	session->auxtrace = NULL;
+	free(ptt);
+}
+
+static bool hisi_ptt_evsel_is_auxtrace(struct perf_session *session,
+				       struct evsel *evsel)
+{
+	struct hisi_ptt *ptt = container_of(session->auxtrace, struct hisi_ptt, auxtrace);
+
+	return evsel->core.attr.type == ptt->pmu_type;
+}
+
+static void hisi_ptt_print_info(__u64 type)
+{
+	if (!dump_trace)
+		return;
+
+	fprintf(stdout, "  PMU Type           %" PRId64 "\n", (s64) type);
+}
+
+int hisi_ptt_process_auxtrace_info(union perf_event *event,
+				   struct perf_session *session)
+{
+	struct perf_record_auxtrace_info *auxtrace_info = &event->auxtrace_info;
+	struct hisi_ptt *ptt;
+
+	if (auxtrace_info->header.size < HISI_PTT_AUXTRACE_PRIV_SIZE +
+				sizeof(struct perf_record_auxtrace_info))
+		return -EINVAL;
+
+	ptt = zalloc(sizeof(*ptt));
+	if (!ptt)
+		return -ENOMEM;
+
+	ptt->session = session;
+	ptt->machine = &session->machines.host; /* No kvm support */
+	ptt->auxtrace_type = auxtrace_info->type;
+	ptt->pmu_type = auxtrace_info->priv[0];
+
+	ptt->auxtrace.process_event = hisi_ptt_process_event;
+	ptt->auxtrace.process_auxtrace_event = hisi_ptt_process_auxtrace_event;
+	ptt->auxtrace.flush_events = hisi_ptt_flush;
+	ptt->auxtrace.free_events = hisi_ptt_free_events;
+	ptt->auxtrace.free = hisi_ptt_free;
+	ptt->auxtrace.evsel_is_auxtrace = hisi_ptt_evsel_is_auxtrace;
+	session->auxtrace = &ptt->auxtrace;
+
+	hisi_ptt_print_info(auxtrace_info->priv[0]);
+
+	return 0;
+}
diff --git a/tools/perf/util/hisi-ptt.h b/tools/perf/util/hisi-ptt.h
index 82283c81b4c1..2db9b4056214 100644
--- a/tools/perf/util/hisi-ptt.h
+++ b/tools/perf/util/hisi-ptt.h
@@ -13,4 +13,7 @@
 struct auxtrace_record *hisi_ptt_recording_init(int *err,
 						struct perf_pmu *hisi_ptt_pmu);
 
+int hisi_ptt_process_auxtrace_info(union perf_event *event,
+				   struct perf_session *session);
+
 #endif
-- 
2.24.0

Re: [PATCH v13 3/3] perf tool: Add support for parsing HiSilicon PCIe Trace packet

[Bjorn Helgaas]

On Mon, Sep 19, 2022 at 05:00:45PM +0800, Yicong Yang wrote:
> From: Qi Liu <liuqi115@huawei.com>
> 
> Add support for using 'perf report --dump-raw-trace' to parse PTT packet.
> 
> Example usage:
> 
> Output will contain raw PTT data and its textual representation, such
> as:
> 
> 0 0 0x5810 [0x30]: PERF_RECORD_AUXTRACE size: 0x400000  offset: 0
> ref: 0xa5d50c725  idx: 0  tid: -1  cpu: 0
> .
> . ... HISI PTT data: size 4194304 bytes
> .  00000000: 00 00 00 00                                 Prefix
> .  00000004: 08 20 00 60                                 Header DW0
> .  00000008: ff 02 00 01                                 Header DW1
> .  0000000c: 20 08 00 00                                 Header DW2
> .  00000010: 10 e7 44 ab                                 Header DW3
> .  00000014: 2a a8 1e 01                                 Time
> .  00000020: 00 00 00 00                                 Prefix
> .  00000024: 01 00 00 60                                 Header DW0
> .  00000028: 0f 1e 00 01                                 Header DW1
> .  0000002c: 04 00 00 00                                 Header DW2
> .  00000030: 40 00 81 02                                 Header DW3
> .  00000034: ee 02 00 00                                 Time

This is great!  Is there a way to actually decode the TLP headers?
E.g., something along the lines of what this does?
https://github.com/NetTLP/wireshark-nettlp/blob/master/plugins/nettlp.lua

If there is, it might be nice if the commit log included a hint about
how to do more decoding.

Bjorn

Re: [PATCH v13 3/3] perf tool: Add support for parsing HiSilicon PCIe Trace packet

[Yicong Yang]

On 2022/9/20 1:32, Bjorn Helgaas wrote:
> On Mon, Sep 19, 2022 at 05:00:45PM +0800, Yicong Yang wrote:
>> From: Qi Liu <liuqi115@huawei.com>
>>
>> Add support for using 'perf report --dump-raw-trace' to parse PTT packet.
>>
>> Example usage:
>>
>> Output will contain raw PTT data and its textual representation, such
>> as:
>>
>> 0 0 0x5810 [0x30]: PERF_RECORD_AUXTRACE size: 0x400000  offset: 0
>> ref: 0xa5d50c725  idx: 0  tid: -1  cpu: 0
>> .
>> . ... HISI PTT data: size 4194304 bytes
>> .  00000000: 00 00 00 00                                 Prefix
>> .  00000004: 08 20 00 60                                 Header DW0
>> .  00000008: ff 02 00 01                                 Header DW1
>> .  0000000c: 20 08 00 00                                 Header DW2
>> .  00000010: 10 e7 44 ab                                 Header DW3
>> .  00000014: 2a a8 1e 01                                 Time
>> .  00000020: 00 00 00 00                                 Prefix
>> .  00000024: 01 00 00 60                                 Header DW0
>> .  00000028: 0f 1e 00 01                                 Header DW1
>> .  0000002c: 04 00 00 00                                 Header DW2
>> .  00000030: 40 00 81 02                                 Header DW3
>> .  00000034: ee 02 00 00                                 Time
> 
> This is great!  Is there a way to actually decode the TLP headers?
> E.g., something along the lines of what this does?
> https://github.com/NetTLP/wireshark-nettlp/blob/master/plugins/nettlp.lua

Thanks for the link, we'd like to implement something similar. This patch brings
fundamental support for the decoding in perf, which means decoding according to
the hardware packets format definition of each fields. 8DW will like above and
4DW will look like (more detailed):

. ... HISI PTT data: size 4194304 bytes
.  00000000: ff 0f 20 40                                 Format 3 Type 1f T9 1 T8 1 TH 1 SO 1 Length 1 Time 201
.  00000004: 0f 10 80 00                                 Header DW1
.  00000008: 00 04 00 00                                 Header DW2
.  0000000c: 48 01 01 00                                 Header DW3

In the next step we intend to do further decoding in the perf report, with granule
to each fields defined in the PCIe spec according to the different TLP's format
and type.

> If there is, it might be nice if the commit log included a hint about
> how to do more decoding.
> 

Since the definition of each Header DWx is exactly the same with the PCIe spec,
so I think user can certainly get the raw data and use the tools like nettlp.lua
to further decode for now.

Thanks,
Yicong

Re: [PATCH v13 2/3] perf tool: Add support for HiSilicon PCIe Tune and Trace device driver

[John Garry]

On 19/09/2022 10:00, Yicong Yang wrote:
> From: Qi Liu <liuqi115@huawei.com>
> 
> HiSilicon PCIe tune and trace device (PTT) could dynamically tune
> the PCIe link's events, and trace the TLP headers).
> 
> This patch add support for PTT device in perf tool, so users could
> use 'perf record' to get TLP headers trace data.
> 
> Reviewed-by: Leo Yan <leo.yan@linaro.org>
> Signed-off-by: Qi Liu <liuqi115@huawei.com>
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>

I'm not overly fimilar with the auxtrace code, but this patch looks ok 
apart from comments, so acked-by seems more appropiate than reviewed-by:

Acked-by: John Garry <john.garry@huawei.com>

> ---
>   tools/perf/arch/arm/util/auxtrace.c   |  63 +++++++++
>   tools/perf/arch/arm/util/pmu.c        |   3 +
>   tools/perf/arch/arm64/util/Build      |   2 +-
>   tools/perf/arch/arm64/util/hisi-ptt.c | 188 ++++++++++++++++++++++++++
>   tools/perf/util/auxtrace.c            |   1 +
>   tools/perf/util/auxtrace.h            |   1 +
>   tools/perf/util/hisi-ptt.h            |  16 +++
>   7 files changed, 273 insertions(+), 1 deletion(-)
>   create mode 100644 tools/perf/arch/arm64/util/hisi-ptt.c
>   create mode 100644 tools/perf/util/hisi-ptt.h
> 
> diff --git a/tools/perf/arch/arm/util/auxtrace.c b/tools/perf/arch/arm/util/auxtrace.c
> index 384c7cfda0fd..129ed72391a4 100644
> --- a/tools/perf/arch/arm/util/auxtrace.c
> +++ b/tools/perf/arch/arm/util/auxtrace.c
> @@ -4,9 +4,11 @@
>    * Author: Mathieu Poirier <mathieu.poirier@linaro.org>
>    */
>   
> +#include <dirent.h>
>   #include <stdbool.h>
>   #include <linux/coresight-pmu.h>
>   #include <linux/zalloc.h>
> +#include <api/fs/fs.h>
>   
>   #include "../../../util/auxtrace.h"
>   #include "../../../util/debug.h"
> @@ -14,6 +16,7 @@
>   #include "../../../util/pmu.h"
>   #include "cs-etm.h"
>   #include "arm-spe.h"
> +#include "hisi-ptt.h"
>   
>   static struct perf_pmu **find_all_arm_spe_pmus(int *nr_spes, int *err)
>   {
> @@ -50,6 +53,52 @@ static struct perf_pmu **find_all_arm_spe_pmus(int *nr_spes, int *err)
>   	return arm_spe_pmus;
>   }
>   
> +static struct perf_pmu **find_all_hisi_ptt_pmus(int *nr_ptts, int *err)
> +{
> +	const char *sysfs = sysfs__mountpoint();
> +	struct perf_pmu **hisi_ptt_pmus = NULL;
> +	struct dirent *dent;
> +	char path[PATH_MAX];
> +	DIR *dir = NULL;
> +	int idx = 0;
> +
> +	snprintf(path, PATH_MAX, "%s" EVENT_SOURCE_DEVICE_PATH, sysfs);
> +	dir = opendir(path);
> +	if (!dir) {
> +		pr_err("can't read directory '%s'\n", EVENT_SOURCE_DEVICE_PATH);
> +		*err = -EINVAL;
> +		goto out;

Do you really need to call closedir() in this scenario?

> +	}
> +
> +	while ((dent = readdir(dir))) {
> +		if (strstr(dent->d_name, HISI_PTT_PMU_NAME))
> +			(*nr_ptts)++;
> +	}
> +
> +	if (!(*nr_ptts))
> +		goto out;
> +
> +	hisi_ptt_pmus = zalloc(sizeof(struct perf_pmu *) * (*nr_ptts));
> +	if (!hisi_ptt_pmus) {
> +		pr_err("hisi_ptt alloc failed\n");
> +		*err = -ENOMEM;
> +		goto out;
> +	}
> +
> +	rewinddir(dir);
> +	while ((dent = readdir(dir))) {
> +		if (strstr(dent->d_name, HISI_PTT_PMU_NAME) && idx < (*nr_ptts)) {

idx < *nr_ptts

is ok

> +			hisi_ptt_pmus[idx] = perf_pmu__find(dent->d_name);
> +			if (hisi_ptt_pmus[idx])
> +				idx++;
> +		}
> +	}
> +
> +out:
> +	closedir(dir);
> +	return hisi_ptt_pmus;
> +}
> +
>   static struct perf_pmu *find_pmu_for_event(struct perf_pmu **pmus,
>   					   int pmu_nr, struct evsel *evsel)
>   {
> @@ -71,17 +120,21 @@ struct auxtrace_record
>   {
>   	struct perf_pmu	*cs_etm_pmu = NULL;
>   	struct perf_pmu **arm_spe_pmus = NULL;
> +	struct perf_pmu **hisi_ptt_pmus = NULL;
>   	struct evsel *evsel;
>   	struct perf_pmu *found_etm = NULL;
>   	struct perf_pmu *found_spe = NULL;
> +	struct perf_pmu *found_ptt = NULL;
>   	int auxtrace_event_cnt = 0;
>   	int nr_spes = 0;
> +	int nr_ptts = 0;
>   
>   	if (!evlist)
>   		return NULL;
>   
>   	cs_etm_pmu = perf_pmu__find(CORESIGHT_ETM_PMU_NAME);
>   	arm_spe_pmus = find_all_arm_spe_pmus(&nr_spes, err);
> +	hisi_ptt_pmus = find_all_hisi_ptt_pmus(&nr_ptts, err);
>   
>   	evlist__for_each_entry(evlist, evsel) {
>   		if (cs_etm_pmu && !found_etm)
> @@ -89,9 +142,13 @@ struct auxtrace_record
>   
>   		if (arm_spe_pmus && !found_spe)
>   			found_spe = find_pmu_for_event(arm_spe_pmus, nr_spes, evsel);
> +
> +		if (hisi_ptt_pmus && !found_ptt)
> +			found_ptt = find_pmu_for_event(hisi_ptt_pmus, nr_ptts, evsel);
>   	}
>   
>   	free(arm_spe_pmus);
> +	free(hisi_ptt_pmus);
>   
>   	if (found_etm)
>   		auxtrace_event_cnt++;
> @@ -99,6 +156,9 @@ struct auxtrace_record
>   	if (found_spe)
>   		auxtrace_event_cnt++;
>   
> +	if (found_ptt)
> +		auxtrace_event_cnt++;
> +
>   	if (auxtrace_event_cnt > 1) {
>   		pr_err("Concurrent AUX trace operation not currently supported\n");
>   		*err = -EOPNOTSUPP;
> @@ -111,6 +171,9 @@ struct auxtrace_record
>   #if defined(__aarch64__)
>   	if (found_spe)
>   		return arm_spe_recording_init(err, found_spe);
> +
> +	if (found_ptt)
> +		return hisi_ptt_recording_init(err, found_ptt);
>   #endif
>   
>   	/*
> diff --git a/tools/perf/arch/arm/util/pmu.c b/tools/perf/arch/arm/util/pmu.c
> index b8b23b9dc598..887c8addc491 100644
> --- a/tools/perf/arch/arm/util/pmu.c
> +++ b/tools/perf/arch/arm/util/pmu.c
> @@ -10,6 +10,7 @@
>   #include <linux/string.h>
>   
>   #include "arm-spe.h"
> +#include "hisi-ptt.h"
>   #include "../../../util/pmu.h"
>   
>   struct perf_event_attr
> @@ -22,6 +23,8 @@ struct perf_event_attr
>   #if defined(__aarch64__)
>   	} else if (strstarts(pmu->name, ARM_SPE_PMU_NAME)) {
>   		return arm_spe_pmu_default_config(pmu);
> +	} else if (strstarts(pmu->name, HISI_PTT_PMU_NAME)) {
> +		pmu->selectable = true;
>   #endif
>   	}
>   
> diff --git a/tools/perf/arch/arm64/util/Build b/tools/perf/arch/arm64/util/Build
> index 9fcb4e68add9..337aa9bdf905 100644
> --- a/tools/perf/arch/arm64/util/Build
> +++ b/tools/perf/arch/arm64/util/Build
> @@ -11,4 +11,4 @@ perf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
>   perf-$(CONFIG_AUXTRACE) += ../../arm/util/pmu.o \
>   			      ../../arm/util/auxtrace.o \
>   			      ../../arm/util/cs-etm.o \
> -			      arm-spe.o mem-events.o
> +			      arm-spe.o mem-events.o hisi-ptt.o
> diff --git a/tools/perf/arch/arm64/util/hisi-ptt.c b/tools/perf/arch/arm64/util/hisi-ptt.c
> new file mode 100644
> index 000000000000..ba97c8a562a0
> --- /dev/null
> +++ b/tools/perf/arch/arm64/util/hisi-ptt.c
> @@ -0,0 +1,188 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * HiSilicon PCIe Trace and Tuning (PTT) support
> + * Copyright (c) 2022 HiSilicon Technologies Co., Ltd.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/bitops.h>
> +#include <linux/log2.h>
> +#include <linux/zalloc.h>
> +#include <time.h>
> +
> +#include <internal/lib.h> // page_size
> +#include "../../../util/auxtrace.h"
> +#include "../../../util/cpumap.h"
> +#include "../../../util/debug.h"
> +#include "../../../util/event.h"
> +#include "../../../util/evlist.h"
> +#include "../../../util/evsel.h"
> +#include "../../../util/hisi-ptt.h"
> +#include "../../../util/pmu.h"
> +#include "../../../util/record.h"
> +#include "../../../util/session.h"
> +#include "../../../util/tsc.h"
> +
> +#define KiB(x) ((x) * 1024)
> +#define MiB(x) ((x) * 1024 * 1024)

surely we have defines for these available elsewhere

> +
> +struct hisi_ptt_recording {
> +	struct auxtrace_record	itr;
> +	struct perf_pmu *hisi_ptt_pmu;
> +	struct evlist *evlist;
> +};
> +
> +static size_t
> +hisi_ptt_info_priv_size(struct auxtrace_record *itr __maybe_unused,
> +			struct evlist *evlist __maybe_unused)
> +{
> +	return HISI_PTT_AUXTRACE_PRIV_SIZE;
> +}
> +
> +static int hisi_ptt_info_fill(struct auxtrace_record *itr,
> +			      struct perf_session *session,
> +			      struct perf_record_auxtrace_info *auxtrace_info,
> +			      size_t priv_size)
> +{
> +	struct hisi_ptt_recording *pttr =
> +			container_of(itr, struct hisi_ptt_recording, itr);
> +	struct perf_pmu *hisi_ptt_pmu = pttr->hisi_ptt_pmu;
> +
> +	if (priv_size != HISI_PTT_AUXTRACE_PRIV_SIZE)
> +		return -EINVAL;
> +
> +	if (!session->evlist->core.nr_mmaps)
> +		return -EINVAL;
> +
> +	auxtrace_info->type = PERF_AUXTRACE_HISI_PTT;
> +	auxtrace_info->priv[0] = hisi_ptt_pmu->type;
> +
> +	return 0;
> +}
> +
> +static int hisi_ptt_set_auxtrace_mmap_page(struct record_opts *opts)
> +{
> +	bool privileged = perf_event_paranoid_check(-1);
> +
> +	if (!opts->full_auxtrace)
> +		return 0;
> +
> +	if (opts->full_auxtrace && !opts->auxtrace_mmap_pages) {
> +		if (privileged) {
> +			opts->auxtrace_mmap_pages = MiB(16) / page_size;
> +		} else {
> +			opts->auxtrace_mmap_pages = KiB(128) / page_size;
> +			if (opts->mmap_pages == UINT_MAX)
> +				opts->mmap_pages = KiB(256) / page_size;
> +		}
> +	}
> +
> +	/* Validate auxtrace_mmap_pages */
> +	if (opts->auxtrace_mmap_pages) {
> +		size_t sz = opts->auxtrace_mmap_pages * (size_t)page_size;
> +		size_t min_sz = KiB(8);
> +
> +		if (sz < min_sz || !is_power_of_2(sz)) {
> +			pr_err("Invalid mmap size for HISI PTT: must be at least %zuKiB and a power of 2\n",
> +			       min_sz / 1024);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int hisi_ptt_recording_options(struct auxtrace_record *itr,
> +				      struct evlist *evlist,
> +				      struct record_opts *opts)
> +{
> +	struct hisi_ptt_recording *pttr =
> +			container_of(itr, struct hisi_ptt_recording, itr);
> +	struct perf_pmu *hisi_ptt_pmu = pttr->hisi_ptt_pmu;
> +	struct evsel *evsel, *hisi_ptt_evsel = NULL;
> +	struct evsel *tracking_evsel;
> +	int err;
> +
> +	pttr->evlist = evlist;
> +	evlist__for_each_entry(evlist, evsel) {
> +		if (evsel->core.attr.type == hisi_ptt_pmu->type) {
> +			if (hisi_ptt_evsel) {
> +				pr_err("There may be only one " HISI_PTT_PMU_NAME "x event\n");
> +				return -EINVAL;
> +			}
> +			evsel->core.attr.freq = 0;
> +			evsel->core.attr.sample_period = 1;
> +			evsel->needs_auxtrace_mmap = true;
> +			hisi_ptt_evsel = evsel;
> +			opts->full_auxtrace = true;
> +		}
> +	}
> +
> +	err = hisi_ptt_set_auxtrace_mmap_page(opts);
> +	if (err)
> +		return err;
> +	/*
> +	 * To obtain the auxtrace buffer file descriptor, the auxtrace event
> +	 * must come first.
> +	 */
> +	evlist__to_front(evlist, hisi_ptt_evsel);
> +	evsel__set_sample_bit(hisi_ptt_evsel, TIME);
> +
> +	/* Add dummy event to keep tracking */
> +	err = parse_event(evlist, "dummy:u");
> +	if (err)
> +		return err;
> +
> +	tracking_evsel = evlist__last(evlist);
> +	evlist__set_tracking_event(evlist, tracking_evsel);
> +
> +	tracking_evsel->core.attr.freq = 0;
> +	tracking_evsel->core.attr.sample_period = 1;
> +	evsel__set_sample_bit(tracking_evsel, TIME);
> +
> +	return 0;
> +}
> +
> +static u64 hisi_ptt_reference(struct auxtrace_record *itr __maybe_unused)
> +{
> +	return rdtsc();
> +}
> +
> +static void hisi_ptt_recording_free(struct auxtrace_record *itr)
> +{
> +	struct hisi_ptt_recording *pttr =
> +			container_of(itr, struct hisi_ptt_recording, itr);
> +
> +	free(pttr);
> +}
> +
> +struct auxtrace_record *hisi_ptt_recording_init(int *err,
> +						struct perf_pmu *hisi_ptt_pmu)
> +{
> +	struct hisi_ptt_recording *pttr;
> +
> +	if (!hisi_ptt_pmu) {
> +		*err = -ENODEV;
> +		return NULL;
> +	}
> +
> +	pttr = zalloc(sizeof(*pttr));
> +	if (!pttr) {
> +		*err = -ENOMEM;
> +		return NULL;
> +	}
> +
> +	pttr->hisi_ptt_pmu = hisi_ptt_pmu;
> +	pttr->itr.pmu = hisi_ptt_pmu;
> +	pttr->itr.recording_options = hisi_ptt_recording_options;
> +	pttr->itr.info_priv_size = hisi_ptt_info_priv_size;
> +	pttr->itr.info_fill = hisi_ptt_info_fill;
> +	pttr->itr.free = hisi_ptt_recording_free;
> +	pttr->itr.reference = hisi_ptt_reference;
> +	pttr->itr.read_finish = auxtrace_record__read_finish;
> +	pttr->itr.alignment = 0;
> +
> +	*err = 0;
> +	return &pttr->itr;
> +}
> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> index 6edab8a16de6..c30611d9ee99 100644
> --- a/tools/perf/util/auxtrace.c
> +++ b/tools/perf/util/auxtrace.c
> @@ -1319,6 +1319,7 @@ int perf_event__process_auxtrace_info(struct perf_session *session,
>   	case PERF_AUXTRACE_S390_CPUMSF:
>   		err = s390_cpumsf_process_auxtrace_info(event, session);
>   		break;
> +	case PERF_AUXTRACE_HISI_PTT:
>   	case PERF_AUXTRACE_UNKNOWN:
>   	default:
>   		return -EINVAL;
> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
> index 6a4fbfd34c6b..3a122fc01ccd 100644
> --- a/tools/perf/util/auxtrace.h
> +++ b/tools/perf/util/auxtrace.h
> @@ -48,6 +48,7 @@ enum auxtrace_type {
>   	PERF_AUXTRACE_CS_ETM,
>   	PERF_AUXTRACE_ARM_SPE,
>   	PERF_AUXTRACE_S390_CPUMSF,
> +	PERF_AUXTRACE_HISI_PTT,
>   };
>   
>   enum itrace_period_type {
> diff --git a/tools/perf/util/hisi-ptt.h b/tools/perf/util/hisi-ptt.h

I'm still not a fan of having a header file which is only included by 1x 
.c file, which this seems to be

> new file mode 100644
> index 000000000000..82283c81b4c1
> --- /dev/null
> +++ b/tools/perf/util/hisi-ptt.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * HiSilicon PCIe Trace and Tuning (PTT) support
> + * Copyright (c) 2022 HiSilicon Technologies Co., Ltd.
> + */
> +
> +#ifndef INCLUDE__PERF_HISI_PTT_H__
> +#define INCLUDE__PERF_HISI_PTT_H__
> +
> +#define HISI_PTT_PMU_NAME		"hisi_ptt"
> +#define HISI_PTT_AUXTRACE_PRIV_SIZE	sizeof(u64)
> +
> +struct auxtrace_record *hisi_ptt_recording_init(int *err,
> +						struct perf_pmu *hisi_ptt_pmu);
> +
> +#endif

Re: [PATCH v13 2/3] perf tool: Add support for HiSilicon PCIe Tune and Trace device driver

[Yicong Yang]

On 2022/9/23 18:49, John Garry wrote:
> On 19/09/2022 10:00, Yicong Yang wrote:
>> From: Qi Liu <liuqi115@huawei.com>
>>
>> HiSilicon PCIe tune and trace device (PTT) could dynamically tune
>> the PCIe link's events, and trace the TLP headers).
>>
>> This patch add support for PTT device in perf tool, so users could
>> use 'perf record' to get TLP headers trace data.
>>
>> Reviewed-by: Leo Yan <leo.yan@linaro.org>
>> Signed-off-by: Qi Liu <liuqi115@huawei.com>
>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> 
> I'm not overly fimilar with the auxtrace code, but this patch looks ok apart from comments, so acked-by seems more appropiate than reviewed-by:
> 
> Acked-by: John Garry <john.garry@huawei.com>

Thanks! May I have the ack for the other 2 patches as well?

Some replies below.

> 
>> ---
>>   tools/perf/arch/arm/util/auxtrace.c   |  63 +++++++++
>>   tools/perf/arch/arm/util/pmu.c        |   3 +
>>   tools/perf/arch/arm64/util/Build      |   2 +-
>>   tools/perf/arch/arm64/util/hisi-ptt.c | 188 ++++++++++++++++++++++++++
>>   tools/perf/util/auxtrace.c            |   1 +
>>   tools/perf/util/auxtrace.h            |   1 +
>>   tools/perf/util/hisi-ptt.h            |  16 +++
>>   7 files changed, 273 insertions(+), 1 deletion(-)
>>   create mode 100644 tools/perf/arch/arm64/util/hisi-ptt.c
>>   create mode 100644 tools/perf/util/hisi-ptt.h
>>
>> diff --git a/tools/perf/arch/arm/util/auxtrace.c b/tools/perf/arch/arm/util/auxtrace.c
>> index 384c7cfda0fd..129ed72391a4 100644
>> --- a/tools/perf/arch/arm/util/auxtrace.c
>> +++ b/tools/perf/arch/arm/util/auxtrace.c
>> @@ -4,9 +4,11 @@
>>    * Author: Mathieu Poirier <mathieu.poirier@linaro.org>
>>    */
>>   +#include <dirent.h>
>>   #include <stdbool.h>
>>   #include <linux/coresight-pmu.h>
>>   #include <linux/zalloc.h>
>> +#include <api/fs/fs.h>
>>     #include "../../../util/auxtrace.h"
>>   #include "../../../util/debug.h"
>> @@ -14,6 +16,7 @@
>>   #include "../../../util/pmu.h"
>>   #include "cs-etm.h"
>>   #include "arm-spe.h"
>> +#include "hisi-ptt.h"
>>     static struct perf_pmu **find_all_arm_spe_pmus(int *nr_spes, int *err)
>>   {
>> @@ -50,6 +53,52 @@ static struct perf_pmu **find_all_arm_spe_pmus(int *nr_spes, int *err)
>>       return arm_spe_pmus;
>>   }
>>   +static struct perf_pmu **find_all_hisi_ptt_pmus(int *nr_ptts, int *err)
>> +{
>> +    const char *sysfs = sysfs__mountpoint();
>> +    struct perf_pmu **hisi_ptt_pmus = NULL;
>> +    struct dirent *dent;
>> +    char path[PATH_MAX];
>> +    DIR *dir = NULL;
>> +    int idx = 0;
>> +
>> +    snprintf(path, PATH_MAX, "%s" EVENT_SOURCE_DEVICE_PATH, sysfs);
>> +    dir = opendir(path);
>> +    if (!dir) {
>> +        pr_err("can't read directory '%s'\n", EVENT_SOURCE_DEVICE_PATH);
>> +        *err = -EINVAL;
>> +        goto out;
> 
> Do you really need to call closedir() in this scenario?
> 

It's unnecessary here. will return NULL directly.

>> +    }
>> +
>> +    while ((dent = readdir(dir))) {
>> +        if (strstr(dent->d_name, HISI_PTT_PMU_NAME))
>> +            (*nr_ptts)++;
>> +    }
>> +
>> +    if (!(*nr_ptts))
>> +        goto out;
>> +
>> +    hisi_ptt_pmus = zalloc(sizeof(struct perf_pmu *) * (*nr_ptts));
>> +    if (!hisi_ptt_pmus) {
>> +        pr_err("hisi_ptt alloc failed\n");
>> +        *err = -ENOMEM;
>> +        goto out;
>> +    }
>> +
>> +    rewinddir(dir);
>> +    while ((dent = readdir(dir))) {
>> +        if (strstr(dent->d_name, HISI_PTT_PMU_NAME) && idx < (*nr_ptts)) {
> 
> idx < *nr_ptts
> 
> is ok
> 

ok.

>> +            hisi_ptt_pmus[idx] = perf_pmu__find(dent->d_name);
>> +            if (hisi_ptt_pmus[idx])
>> +                idx++;
>> +        }
>> +    }
>> +
>> +out:
>> +    closedir(dir);
>> +    return hisi_ptt_pmus;
>> +}
>> +

[...]

>> +
>> +#define KiB(x) ((x) * 1024)
>> +#define MiB(x) ((x) * 1024 * 1024)
> 
> surely we have defines for these available elsewhere
> 

Yes but we looks like have no public definition for these:

yangyicong@ubuntu:~/mainline_linux/linux$ egrep -rwn "#define KiB|#define MiB" tools/perf/
tools/perf/arch/arm64/util/hisi-ptt.c:27:#define KiB(x) ((x) * 1024)
tools/perf/arch/arm64/util/hisi-ptt.c:28:#define MiB(x) ((x) * 1024 * 1024)
tools/perf/arch/arm64/util/arm-spe.c:28:#define KiB(x) ((x) * 1024)
tools/perf/arch/arm64/util/arm-spe.c:29:#define MiB(x) ((x) * 1024 * 1024)
tools/perf/arch/x86/util/intel-bts.c:28:#define KiB(x) ((x) * 1024)
tools/perf/arch/x86/util/intel-bts.c:29:#define MiB(x) ((x) * 1024 * 1024)
tools/perf/arch/x86/util/intel-pt.c:35:#define KiB(x) ((x) * 1024)
tools/perf/arch/x86/util/intel-pt.c:36:#define MiB(x) ((x) * 1024 * 1024)
tools/perf/util/cs-etm.h:188:#define KiB(x) ((x) * 1024)
tools/perf/util/cs-etm.h:189:#define MiB(x) ((x) * 1024 * 1024)

We can centralize this definition after this series.

>> +
>> +struct hisi_ptt_recording {
>> +    struct auxtrace_record    itr;
>> +    struct perf_pmu *hisi_ptt_pmu;
>> +    struct evlist *evlist;
>> +};
>> +

[...]

>> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
>> index 6edab8a16de6..c30611d9ee99 100644
>> --- a/tools/perf/util/auxtrace.c
>> +++ b/tools/perf/util/auxtrace.c
>> @@ -1319,6 +1319,7 @@ int perf_event__process_auxtrace_info(struct perf_session *session,
>>       case PERF_AUXTRACE_S390_CPUMSF:
>>           err = s390_cpumsf_process_auxtrace_info(event, session);
>>           break;
>> +    case PERF_AUXTRACE_HISI_PTT:
>>       case PERF_AUXTRACE_UNKNOWN:
>>       default:
>>           return -EINVAL;
>> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
>> index 6a4fbfd34c6b..3a122fc01ccd 100644
>> --- a/tools/perf/util/auxtrace.h
>> +++ b/tools/perf/util/auxtrace.h
>> @@ -48,6 +48,7 @@ enum auxtrace_type {
>>       PERF_AUXTRACE_CS_ETM,
>>       PERF_AUXTRACE_ARM_SPE,
>>       PERF_AUXTRACE_S390_CPUMSF,
>> +    PERF_AUXTRACE_HISI_PTT,
>>   };
>>     enum itrace_period_type {
>> diff --git a/tools/perf/util/hisi-ptt.h b/tools/perf/util/hisi-ptt.h
> 
> I'm still not a fan of having a header file which is only included by 1x .c file, which this seems to be
> 

The stuff in hisi-ptt.h is shared across several source files:

yangyicong@ubuntu:~/mainline_linux/linux$ egrep -rn "hisi-ptt.h" tools/perf/
tools/perf/arch/arm64/util/hisi-ptt.c:21:#include "../../../util/hisi-ptt.h"
tools/perf/arch/arm/util/auxtrace.c:19:#include "hisi-ptt.h"
tools/perf/arch/arm/util/pmu.c:13:#include "hisi-ptt.h"

Thanks,
Yicong

>> new file mode 100644
>> index 000000000000..82283c81b4c1
>> --- /dev/null
>> +++ b/tools/perf/util/hisi-ptt.h
>> @@ -0,0 +1,16 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * HiSilicon PCIe Trace and Tuning (PTT) support
>> + * Copyright (c) 2022 HiSilicon Technologies Co., Ltd.
>> + */
>> +
>> +#ifndef INCLUDE__PERF_HISI_PTT_H__
>> +#define INCLUDE__PERF_HISI_PTT_H__
>> +
>> +#define HISI_PTT_PMU_NAME        "hisi_ptt"
>> +#define HISI_PTT_AUXTRACE_PRIV_SIZE    sizeof(u64)
>> +
>> +struct auxtrace_record *hisi_ptt_recording_init(int *err,
>> +                        struct perf_pmu *hisi_ptt_pmu);
>> +
>> +#endif
> 
> .

Re: [PATCH v13 1/3] perf tool: arm: Refactor event list iteration in auxtrace_record__init()

[John Garry]

On 19/09/2022 10:00, Yicong Yang wrote:
> From: Qi Liu<liuqi115@huawei.com>
> 
> Add find_pmu_for_event() and use to simplify logic in
> auxtrace_record_init(). find_pmu_for_event() will be
> reused in subsequent patches.
> 
> Reviewed-by: Leo Yan<leo.yan@linaro.org>
> Reviewed-by: Jonathan Cameron<Jonathan.Cameron@huawei.com>
> Signed-off-by: Qi Liu<liuqi115@huawei.com>
> Signed-off-by: Yicong Yang<yangyicong@hisilicon.com>
> ---


Reviewed-by: John Garry <john.garry@huawei.com>

Re: [PATCH v13 2/3] perf tool: Add support for HiSilicon PCIe Tune and Trace device driver

[Arnaldo Carvalho de Melo]

Em Mon, Sep 26, 2022 at 02:53:34PM +0800, Yicong Yang escreveu:
> On 2022/9/23 18:49, John Garry wrote:
> > On 19/09/2022 10:00, Yicong Yang wrote:
> >> From: Qi Liu <liuqi115@huawei.com>
> >>
> >> HiSilicon PCIe tune and trace device (PTT) could dynamically tune
> >> the PCIe link's events, and trace the TLP headers).
> >>
> >> This patch add support for PTT device in perf tool, so users could
> >> use 'perf record' to get TLP headers trace data.
> >>
> >> Reviewed-by: Leo Yan <leo.yan@linaro.org>
> >> Signed-off-by: Qi Liu <liuqi115@huawei.com>
> >> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> > 
> > I'm not overly fimilar with the auxtrace code, but this patch looks ok apart from comments, so acked-by seems more appropiate than reviewed-by:
> > 
> > Acked-by: John Garry <john.garry@huawei.com>

Ok, applied it now.

- Arnaldo
 
> Thanks! May I have the ack for the other 2 patches as well?
> 
> Some replies below.
> 
> > 
> >> ---
> >>   tools/perf/arch/arm/util/auxtrace.c   |  63 +++++++++
> >>   tools/perf/arch/arm/util/pmu.c        |   3 +
> >>   tools/perf/arch/arm64/util/Build      |   2 +-
> >>   tools/perf/arch/arm64/util/hisi-ptt.c | 188 ++++++++++++++++++++++++++
> >>   tools/perf/util/auxtrace.c            |   1 +
> >>   tools/perf/util/auxtrace.h            |   1 +
> >>   tools/perf/util/hisi-ptt.h            |  16 +++
> >>   7 files changed, 273 insertions(+), 1 deletion(-)
> >>   create mode 100644 tools/perf/arch/arm64/util/hisi-ptt.c
> >>   create mode 100644 tools/perf/util/hisi-ptt.h
> >>
> >> diff --git a/tools/perf/arch/arm/util/auxtrace.c b/tools/perf/arch/arm/util/auxtrace.c
> >> index 384c7cfda0fd..129ed72391a4 100644
> >> --- a/tools/perf/arch/arm/util/auxtrace.c
> >> +++ b/tools/perf/arch/arm/util/auxtrace.c
> >> @@ -4,9 +4,11 @@
> >>    * Author: Mathieu Poirier <mathieu.poirier@linaro.org>
> >>    */
> >>   +#include <dirent.h>
> >>   #include <stdbool.h>
> >>   #include <linux/coresight-pmu.h>
> >>   #include <linux/zalloc.h>
> >> +#include <api/fs/fs.h>
> >>     #include "../../../util/auxtrace.h"
> >>   #include "../../../util/debug.h"
> >> @@ -14,6 +16,7 @@
> >>   #include "../../../util/pmu.h"
> >>   #include "cs-etm.h"
> >>   #include "arm-spe.h"
> >> +#include "hisi-ptt.h"
> >>     static struct perf_pmu **find_all_arm_spe_pmus(int *nr_spes, int *err)
> >>   {
> >> @@ -50,6 +53,52 @@ static struct perf_pmu **find_all_arm_spe_pmus(int *nr_spes, int *err)
> >>       return arm_spe_pmus;
> >>   }
> >>   +static struct perf_pmu **find_all_hisi_ptt_pmus(int *nr_ptts, int *err)
> >> +{
> >> +    const char *sysfs = sysfs__mountpoint();
> >> +    struct perf_pmu **hisi_ptt_pmus = NULL;
> >> +    struct dirent *dent;
> >> +    char path[PATH_MAX];
> >> +    DIR *dir = NULL;
> >> +    int idx = 0;
> >> +
> >> +    snprintf(path, PATH_MAX, "%s" EVENT_SOURCE_DEVICE_PATH, sysfs);
> >> +    dir = opendir(path);
> >> +    if (!dir) {
> >> +        pr_err("can't read directory '%s'\n", EVENT_SOURCE_DEVICE_PATH);
> >> +        *err = -EINVAL;
> >> +        goto out;
> > 
> > Do you really need to call closedir() in this scenario?
> > 
> 
> It's unnecessary here. will return NULL directly.
> 
> >> +    }
> >> +
> >> +    while ((dent = readdir(dir))) {
> >> +        if (strstr(dent->d_name, HISI_PTT_PMU_NAME))
> >> +            (*nr_ptts)++;
> >> +    }
> >> +
> >> +    if (!(*nr_ptts))
> >> +        goto out;
> >> +
> >> +    hisi_ptt_pmus = zalloc(sizeof(struct perf_pmu *) * (*nr_ptts));
> >> +    if (!hisi_ptt_pmus) {
> >> +        pr_err("hisi_ptt alloc failed\n");
> >> +        *err = -ENOMEM;
> >> +        goto out;
> >> +    }
> >> +
> >> +    rewinddir(dir);
> >> +    while ((dent = readdir(dir))) {
> >> +        if (strstr(dent->d_name, HISI_PTT_PMU_NAME) && idx < (*nr_ptts)) {
> > 
> > idx < *nr_ptts
> > 
> > is ok
> > 
> 
> ok.
> 
> >> +            hisi_ptt_pmus[idx] = perf_pmu__find(dent->d_name);
> >> +            if (hisi_ptt_pmus[idx])
> >> +                idx++;
> >> +        }
> >> +    }
> >> +
> >> +out:
> >> +    closedir(dir);
> >> +    return hisi_ptt_pmus;
> >> +}
> >> +
> 
> [...]
> 
> >> +
> >> +#define KiB(x) ((x) * 1024)
> >> +#define MiB(x) ((x) * 1024 * 1024)
> > 
> > surely we have defines for these available elsewhere
> > 
> 
> Yes but we looks like have no public definition for these:
> 
> yangyicong@ubuntu:~/mainline_linux/linux$ egrep -rwn "#define KiB|#define MiB" tools/perf/
> tools/perf/arch/arm64/util/hisi-ptt.c:27:#define KiB(x) ((x) * 1024)
> tools/perf/arch/arm64/util/hisi-ptt.c:28:#define MiB(x) ((x) * 1024 * 1024)
> tools/perf/arch/arm64/util/arm-spe.c:28:#define KiB(x) ((x) * 1024)
> tools/perf/arch/arm64/util/arm-spe.c:29:#define MiB(x) ((x) * 1024 * 1024)
> tools/perf/arch/x86/util/intel-bts.c:28:#define KiB(x) ((x) * 1024)
> tools/perf/arch/x86/util/intel-bts.c:29:#define MiB(x) ((x) * 1024 * 1024)
> tools/perf/arch/x86/util/intel-pt.c:35:#define KiB(x) ((x) * 1024)
> tools/perf/arch/x86/util/intel-pt.c:36:#define MiB(x) ((x) * 1024 * 1024)
> tools/perf/util/cs-etm.h:188:#define KiB(x) ((x) * 1024)
> tools/perf/util/cs-etm.h:189:#define MiB(x) ((x) * 1024 * 1024)
> 
> We can centralize this definition after this series.
> 
> >> +
> >> +struct hisi_ptt_recording {
> >> +    struct auxtrace_record    itr;
> >> +    struct perf_pmu *hisi_ptt_pmu;
> >> +    struct evlist *evlist;
> >> +};
> >> +
> 
> [...]
> 
> >> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> >> index 6edab8a16de6..c30611d9ee99 100644
> >> --- a/tools/perf/util/auxtrace.c
> >> +++ b/tools/perf/util/auxtrace.c
> >> @@ -1319,6 +1319,7 @@ int perf_event__process_auxtrace_info(struct perf_session *session,
> >>       case PERF_AUXTRACE_S390_CPUMSF:
> >>           err = s390_cpumsf_process_auxtrace_info(event, session);
> >>           break;
> >> +    case PERF_AUXTRACE_HISI_PTT:
> >>       case PERF_AUXTRACE_UNKNOWN:
> >>       default:
> >>           return -EINVAL;
> >> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
> >> index 6a4fbfd34c6b..3a122fc01ccd 100644
> >> --- a/tools/perf/util/auxtrace.h
> >> +++ b/tools/perf/util/auxtrace.h
> >> @@ -48,6 +48,7 @@ enum auxtrace_type {
> >>       PERF_AUXTRACE_CS_ETM,
> >>       PERF_AUXTRACE_ARM_SPE,
> >>       PERF_AUXTRACE_S390_CPUMSF,
> >> +    PERF_AUXTRACE_HISI_PTT,
> >>   };
> >>     enum itrace_period_type {
> >> diff --git a/tools/perf/util/hisi-ptt.h b/tools/perf/util/hisi-ptt.h
> > 
> > I'm still not a fan of having a header file which is only included by 1x .c file, which this seems to be
> > 
> 
> The stuff in hisi-ptt.h is shared across several source files:
> 
> yangyicong@ubuntu:~/mainline_linux/linux$ egrep -rn "hisi-ptt.h" tools/perf/
> tools/perf/arch/arm64/util/hisi-ptt.c:21:#include "../../../util/hisi-ptt.h"
> tools/perf/arch/arm/util/auxtrace.c:19:#include "hisi-ptt.h"
> tools/perf/arch/arm/util/pmu.c:13:#include "hisi-ptt.h"
> 
> Thanks,
> Yicong
> 
> >> new file mode 100644
> >> index 000000000000..82283c81b4c1
> >> --- /dev/null
> >> +++ b/tools/perf/util/hisi-ptt.h
> >> @@ -0,0 +1,16 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 */
> >> +/*
> >> + * HiSilicon PCIe Trace and Tuning (PTT) support
> >> + * Copyright (c) 2022 HiSilicon Technologies Co., Ltd.
> >> + */
> >> +
> >> +#ifndef INCLUDE__PERF_HISI_PTT_H__
> >> +#define INCLUDE__PERF_HISI_PTT_H__
> >> +
> >> +#define HISI_PTT_PMU_NAME        "hisi_ptt"
> >> +#define HISI_PTT_AUXTRACE_PRIV_SIZE    sizeof(u64)
> >> +
> >> +struct auxtrace_record *hisi_ptt_recording_init(int *err,
> >> +                        struct perf_pmu *hisi_ptt_pmu);
> >> +
> >> +#endif
> > 
> > .

-- 

- Arnaldo