Re: [PATCH v9 5/8] perf tool: Add support for HiSilicon PCIe Tune and Trace device driver

2022-06-27 Thread Yicong Yang via iommu
On 2022/6/27 10:02, Leo Yan wrote:
> On Mon, Jun 06, 2022 at 07:55:52PM +0800, Yicong Yang wrote:
>> From: Qi Liu 
>>
>> 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.
>>
>> Signed-off-by: Qi Liu 
>> Signed-off-by: Yicong Yang 
> 
> Just one minor comment.
> 
> [...]
> 
>> diff --git a/tools/perf/util/hisi-ptt.h b/tools/perf/util/hisi-ptt.h
>> new file mode 100644
>> index ..2db9b4056214
>> --- /dev/null
>> +++ b/tools/perf/util/hisi-ptt.h
>> @@ -0,0 +1,19 @@
>> +/* 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);
>> +
>> +int hisi_ptt_process_auxtrace_info(union perf_event *event,
>> +   struct perf_session *session);
> 
> The function hisi_ptt_process_auxtrace_info() is introduced by next
> patch for support PTT decoding, for good practice (e.g. keep
> bisection), it's good to introduce function declaration and definition
> in one patch.
> 

Thanks for catching this. There's something wrong when doing the patch 
splitting.
Will fix this!

> With this fixing, this patch looks good to me:
> 
> Reviewed-by: Leo Yan 
> 

Thanks.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v9 5/8] perf tool: Add support for HiSilicon PCIe Tune and Trace device driver

2022-06-26 Thread Leo Yan
On Mon, Jun 06, 2022 at 07:55:52PM +0800, Yicong Yang wrote:
> From: Qi Liu 
> 
> 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.
> 
> Signed-off-by: Qi Liu 
> Signed-off-by: Yicong Yang 

Just one minor comment.

[...]

> diff --git a/tools/perf/util/hisi-ptt.h b/tools/perf/util/hisi-ptt.h
> new file mode 100644
> index ..2db9b4056214
> --- /dev/null
> +++ b/tools/perf/util/hisi-ptt.h
> @@ -0,0 +1,19 @@
> +/* 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);
> +
> +int hisi_ptt_process_auxtrace_info(union perf_event *event,
> +struct perf_session *session);

The function hisi_ptt_process_auxtrace_info() is introduced by next
patch for support PTT decoding, for good practice (e.g. keep
bisection), it's good to introduce function declaration and definition
in one patch.

With this fixing, this patch looks good to me:

Reviewed-by: Leo Yan 

> +
> +#endif
> -- 
> 2.24.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v9 5/8] perf tool: Add support for HiSilicon PCIe Tune and Trace device driver

2022-06-06 Thread Yicong Yang via iommu
From: Qi Liu 

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.

Signed-off-by: Qi Liu 
Signed-off-by: Yicong Yang 
---
 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 | 187 ++
 tools/perf/util/auxtrace.c|   1 +
 tools/perf/util/auxtrace.h|   1 +
 tools/perf/util/hisi-ptt.h|  19 +++
 7 files changed, 275 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 
  */
 
+#include 
 #include 
 #include 
 #include 
+#include 
 
 #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(_spes, err);
+   hisi_ptt_pmus = find_all_hisi_ptt_pmus(_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
---