[RFT PATCH v2 0/7] perf tool: Support iostat for multiple platform

[RFT PATCH v2 0/7] perf tool: Support iostat for multiple platform

[Yushan Wang]

Currently, platform-specific iostat code for PMUs is implemented as a
common iostat callback interface and invoked based on what is being
built. This approach limits support for iostat across different types of
PMUs.

Support of HiSilicon PCIe PMU iostat was raised at [1], which uses the
similar approach.

To extend support of iostat across platforms, change common iostat
interface to framework to allow perf to probe PMU capabilities during
runtime and route iostat request to the correct PMU-specific functions.
Then HiSilicon PCIe PMU iostat is supported with the new framework.

Request For Test:
Refactors has been made to x86 iostat to adapt the iostat framework, the
probe function that checks if there's any PMU's name contains
'uncore_iio' may not work properly, tests of that would be appreciated.

[1] https://lore.kernel.org/all/20240208032518.25830-1-yangyicong@huawei.com/

Changes:

RFT v2:
- Replace typedefs with plain function pointer declaration.
- Replace _Bool with bool with the help of stdbool.h.
- Moved architecture-specific iostat implementation code to utils
  directory per Ian.
- Dropped PMU wildcard matching variation, and changed pmu_name to
  pmu_name_wildcard for wildcard PMU name matching.
- Changed PMU name pattern of x86 iostat to "uncore_iio".
- Minor refactors to iostat framework.
Link: https://lore.kernel.org/all/20260126123514.3238425-1-wangyushan12@huawei.com

Shiju Jose (2):
  perf-iostat: Extend iostat interface to support different iostat PMUs
  perf-iostat: Make x86 iostat compatible with new iostat framework

Yicong Yang (1):
  perf-iostat: Enable iostat mode for HiSilicon PCIe PMU

Yushan Wang (4):
  perf stat: Check color's length instead of the pointer
  perf stat: Save unnecessary print_metric() call
  perf-x86: iostat: Move iostat arch-specific implementation to util
  perf-x86: iostat: Change iostat_prefix() to static

 tools/perf/arch/x86/util/Build                |   1 -
 tools/perf/builtin-script.c                   |   2 +-
 tools/perf/util/Build                         |   2 +
 tools/perf/util/hisi-iostat.c                 | 478 ++++++++++++++++++
 tools/perf/util/iostat.c                      |  89 +++-
 tools/perf/util/iostat.h                      |  40 +-
 tools/perf/util/stat-display.c                |   4 +-
 tools/perf/util/stat-shadow.c                 |   4 +-
 .../x86/util/iostat.c => util/x86-iostat.c}   | 100 ++--
 9 files changed, 650 insertions(+), 70 deletions(-)
 create mode 100644 tools/perf/util/hisi-iostat.c
 rename tools/perf/{arch/x86/util/iostat.c => util/x86-iostat.c} (85%)

-- 
2.33.0

[RFT PATCH v2 1/7] perf stat: Check color's length instead of the pointer

[Yushan Wang]

Color string returned by metric_threshold_classify__color() is never
NULL, check the presence of *color will always return true.

Fix this by change the checks against length of *color.

Fixes: 37b77ae95416 ("perf stat: Change color to threshold in print_metric")
Signed-off-by: Yushan Wang <wangyushan12@huawei.com>
---
 tools/perf/builtin-script.c    | 2 +-
 tools/perf/util/stat-display.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index c8ac9f01a36b..58cd5cd377a6 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -2111,7 +2111,7 @@ static void script_print_metric(struct perf_stat_config *config __maybe_unused,
 	perf_sample__fprintf_start(NULL, mctx->sample, mctx->thread, mctx->evsel,
 				   PERF_RECORD_SAMPLE, mctx->fp);
 	fputs("\tmetric: ", mctx->fp);
-	if (color)
+	if (strlen(color))
 		color_fprintf(mctx->fp, color, fmt, val);
 	else
 		printf(fmt, val);
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 993f4c4b8f44..74cba80c24b0 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -474,7 +474,7 @@ static void print_metric_std(struct perf_stat_config *config,
 		do_new_line_std(config, os);
 
 	n = fprintf(out, " # ");
-	if (color)
+	if (strlen(color))
 		n += color_fprintf(out, color, fmt, val);
 	else
 		n += fprintf(out, fmt, val);
@@ -607,7 +607,7 @@ static void print_metric_only(struct perf_stat_config *config,
 	if (mlen < strlen(unit))
 		mlen = strlen(unit) + 1;
 
-	if (color)
+	if (strlen(color))
 		mlen += strlen(color) + sizeof(PERF_COLOR_RESET) - 1;
 
 	color_snprintf(str, sizeof(str), color ?: "", fmt ?: "", val);
-- 
2.33.0

Re: [RFT PATCH v2 1/7] perf stat: Check color's length instead of the pointer

[Ian Rogers]

On Wed, May 6, 2026 at 11:37 PM Yushan Wang <wangyushan12@huawei.com> wrote:
>
> Color string returned by metric_threshold_classify__color() is never
> NULL, check the presence of *color will always return true.
>
> Fix this by change the checks against length of *color.
>
> Fixes: 37b77ae95416 ("perf stat: Change color to threshold in print_metric")
> Signed-off-by: Yushan Wang <wangyushan12@huawei.com>

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

Thanks,
Ian

> ---
>  tools/perf/builtin-script.c    | 2 +-
>  tools/perf/util/stat-display.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index c8ac9f01a36b..58cd5cd377a6 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -2111,7 +2111,7 @@ static void script_print_metric(struct perf_stat_config *config __maybe_unused,
>         perf_sample__fprintf_start(NULL, mctx->sample, mctx->thread, mctx->evsel,
>                                    PERF_RECORD_SAMPLE, mctx->fp);
>         fputs("\tmetric: ", mctx->fp);
> -       if (color)
> +       if (strlen(color))
>                 color_fprintf(mctx->fp, color, fmt, val);
>         else
>                 printf(fmt, val);
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index 993f4c4b8f44..74cba80c24b0 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -474,7 +474,7 @@ static void print_metric_std(struct perf_stat_config *config,
>                 do_new_line_std(config, os);
>
>         n = fprintf(out, " # ");
> -       if (color)
> +       if (strlen(color))
>                 n += color_fprintf(out, color, fmt, val);
>         else
>                 n += fprintf(out, fmt, val);
> @@ -607,7 +607,7 @@ static void print_metric_only(struct perf_stat_config *config,
>         if (mlen < strlen(unit))
>                 mlen = strlen(unit) + 1;
>
> -       if (color)
> +       if (strlen(color))
>                 mlen += strlen(color) + sizeof(PERF_COLOR_RESET) - 1;
>
>         color_snprintf(str, sizeof(str), color ?: "", fmt ?: "", val);
> --
> 2.33.0
>

[RFT PATCH v2 2/7] perf stat: Save unnecessary print_metric() call

[Yushan Wang]

The patch listed under Fixes removed the other branch of iostat_run, and
changed num to 0 since it is the default behavior of it. But during
iostat_run, default value 1 of num is required to avoid print_metric()
call later.

Set num as 1 to avoid redundant print_metric() call that causes
unaligned blank printed.

Before this patch:

root@localhost$ ./perf stat --iostat=0000:20:0f.0 --timeout 100

 Performance counter stats for 'system wide':

    port              Inbound MWR(MB)      Inbound MRD(MB)      Inbound CPL(MB)     Outbound MWR(MB)     Outbound MRD(MB)     Outbound CPL(MB)
0000:20:0f.0                 0.00                                      0.00                                      0.00                                      0.00                                      0.00
                            0.01

       0.100138030 seconds time elapsed

After this patch:

root@localhost$ ./perf stat --iostat=0000:20:0f.0 --timeout 100

 Performance counter stats for 'system wide':

    port              Inbound MWR(MB)      Inbound MRD(MB)      Inbound CPL(MB)     Outbound MWR(MB)     Outbound MRD(MB)     Outbound CPL(MB)
0000:20:0f.0                 0.00                 0.00                 0.00                 0.00                 0.00                 0.01

       0.100127590 seconds time elapsed

Fixes: b71f46a6a708 ("perf stat: Remove hard coded shadow metrics")
Signed-off-by: Yushan Wang <wangyushan12@huawei.com>
---
 tools/perf/util/stat-shadow.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index bc2d44df7baf..0056444523d6 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -326,8 +326,10 @@ void perf_stat__print_shadow_stats(struct perf_stat_config *config,
 	void *ctxp = out->ctx;
 	int num = 0;
 
-	if (config->iostat_run)
+	if (config->iostat_run) {
 		iostat_print_metric(config, evsel, out);
+		num = 1;
+	}
 
 	perf_stat__print_shadow_stats_metricgroup(config, evsel, aggr_idx,
 						  &num, NULL, out);
-- 
2.33.0

Re: [RFT PATCH v2 2/7] perf stat: Save unnecessary print_metric() call

[Ian Rogers]

On Wed, May 6, 2026 at 11:37 PM Yushan Wang <wangyushan12@huawei.com> wrote:
>
> The patch listed under Fixes removed the other branch of iostat_run, and
> changed num to 0 since it is the default behavior of it. But during
> iostat_run, default value 1 of num is required to avoid print_metric()
> call later.
>
> Set num as 1 to avoid redundant print_metric() call that causes
> unaligned blank printed.
>
> Before this patch:
>
> root@localhost$ ./perf stat --iostat=0000:20:0f.0 --timeout 100
>
>  Performance counter stats for 'system wide':
>
>     port              Inbound MWR(MB)      Inbound MRD(MB)      Inbound CPL(MB)     Outbound MWR(MB)     Outbound MRD(MB)     Outbound CPL(MB)
> 0000:20:0f.0                 0.00                                      0.00                                      0.00                                      0.00                                      0.00
>                             0.01
>
>        0.100138030 seconds time elapsed
>
> After this patch:
>
> root@localhost$ ./perf stat --iostat=0000:20:0f.0 --timeout 100
>
>  Performance counter stats for 'system wide':
>
>     port              Inbound MWR(MB)      Inbound MRD(MB)      Inbound CPL(MB)     Outbound MWR(MB)     Outbound MRD(MB)     Outbound CPL(MB)
> 0000:20:0f.0                 0.00                 0.00                 0.00                 0.00                 0.00                 0.01
>
>        0.100127590 seconds time elapsed
>
> Fixes: b71f46a6a708 ("perf stat: Remove hard coded shadow metrics")
> Signed-off-by: Yushan Wang <wangyushan12@huawei.com>

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

Thanks,
Ian

> ---
>  tools/perf/util/stat-shadow.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
> index bc2d44df7baf..0056444523d6 100644
> --- a/tools/perf/util/stat-shadow.c
> +++ b/tools/perf/util/stat-shadow.c
> @@ -326,8 +326,10 @@ void perf_stat__print_shadow_stats(struct perf_stat_config *config,
>         void *ctxp = out->ctx;
>         int num = 0;
>
> -       if (config->iostat_run)
> +       if (config->iostat_run) {
>                 iostat_print_metric(config, evsel, out);
> +               num = 1;
> +       }
>
>         perf_stat__print_shadow_stats_metricgroup(config, evsel, aggr_idx,
>                                                   &num, NULL, out);
> --
> 2.33.0
>

[RFT PATCH v2 3/7] perf-x86: iostat: Move iostat arch-specific implementation to util

[Yushan Wang]

To support cross-platform iostat capabilities, like record/report on
different platforms, architecture-specific implementation of iostat
should be moved out of arch directory to get built. Build scripts are
changed accordingly.

Signed-off-by: Yushan Wang <wangyushan12@huawei.com>
---
 tools/perf/arch/x86/util/Build                           | 1 -
 tools/perf/util/Build                                    | 1 +
 tools/perf/{arch/x86/util/iostat.c => util/x86-iostat.c} | 0
 3 files changed, 1 insertion(+), 1 deletion(-)
 rename tools/perf/{arch/x86/util/iostat.c => util/x86-iostat.c} (100%)

diff --git a/tools/perf/arch/x86/util/Build b/tools/perf/arch/x86/util/Build
index b94c91984c66..358541beaa0f 100644
--- a/tools/perf/arch/x86/util/Build
+++ b/tools/perf/arch/x86/util/Build
@@ -7,7 +7,6 @@ perf-util-y += event.o
 perf-util-y += evlist.o
 perf-util-y += mem-events.o
 perf-util-y += evsel.o
-perf-util-y += iostat.o
 
 perf-util-$(CONFIG_LOCAL_LIBUNWIND)    += unwind-libunwind.o
 
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 70cc91d00804..c7e5ada3800d 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -127,6 +127,7 @@ perf-util-y += thread-stack.o
 perf-util-y += spark.o
 perf-util-y += topdown.o
 perf-util-y += iostat.o
+perf-util-y += x86-iostat.o
 perf-util-y += stream.o
 perf-util-$(CONFIG_LIBTRACEEVENT) += kvm-stat.o
 perf-util-y += kvm-stat-arch/
diff --git a/tools/perf/arch/x86/util/iostat.c b/tools/perf/util/x86-iostat.c
similarity index 100%
rename from tools/perf/arch/x86/util/iostat.c
rename to tools/perf/util/x86-iostat.c
-- 
2.33.0

Re: [RFT PATCH v2 3/7] perf-x86: iostat: Move iostat arch-specific implementation to util

[Ian Rogers]

On Wed, May 6, 2026 at 11:37 PM Yushan Wang <wangyushan12@huawei.com> wrote:
>
> To support cross-platform iostat capabilities, like record/report on
> different platforms, architecture-specific implementation of iostat
> should be moved out of arch directory to get built. Build scripts are
> changed accordingly.
>
> Signed-off-by: Yushan Wang <wangyushan12@huawei.com>
> ---
>  tools/perf/arch/x86/util/Build                           | 1 -
>  tools/perf/util/Build                                    | 1 +
>  tools/perf/{arch/x86/util/iostat.c => util/x86-iostat.c} | 0
>  3 files changed, 1 insertion(+), 1 deletion(-)
>  rename tools/perf/{arch/x86/util/iostat.c => util/x86-iostat.c} (100%)

To avoid cluttering up tools/perf/util, would it make sense to create
a "tools/perf/util/iostat-arch" directory similar to  annotate-arch,
dwarf-regs-arch, kvm-stat-arch and perf-regs-arch?

Thanks,
Ian

> diff --git a/tools/perf/arch/x86/util/Build b/tools/perf/arch/x86/util/Build
> index b94c91984c66..358541beaa0f 100644
> --- a/tools/perf/arch/x86/util/Build
> +++ b/tools/perf/arch/x86/util/Build
> @@ -7,7 +7,6 @@ perf-util-y += event.o
>  perf-util-y += evlist.o
>  perf-util-y += mem-events.o
>  perf-util-y += evsel.o
> -perf-util-y += iostat.o
>
>  perf-util-$(CONFIG_LOCAL_LIBUNWIND)    += unwind-libunwind.o
>
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 70cc91d00804..c7e5ada3800d 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -127,6 +127,7 @@ perf-util-y += thread-stack.o
>  perf-util-y += spark.o
>  perf-util-y += topdown.o
>  perf-util-y += iostat.o
> +perf-util-y += x86-iostat.o
>  perf-util-y += stream.o
>  perf-util-$(CONFIG_LIBTRACEEVENT) += kvm-stat.o
>  perf-util-y += kvm-stat-arch/
> diff --git a/tools/perf/arch/x86/util/iostat.c b/tools/perf/util/x86-iostat.c
> similarity index 100%
> rename from tools/perf/arch/x86/util/iostat.c
> rename to tools/perf/util/x86-iostat.c
> --
> 2.33.0
>

Re: [RFT PATCH v2 3/7] perf-x86: iostat: Move iostat arch-specific implementation to util

[Yushan Wang]

 On 5/7/2026 11:35 PM, Ian Rogers wrote:
> On Wed, May 6, 2026 at 11:37 PM Yushan Wang <wangyushan12@huawei.com> wrote:
>>
>> To support cross-platform iostat capabilities, like record/report on
>> different platforms, architecture-specific implementation of iostat
>> should be moved out of arch directory to get built. Build scripts are
>> changed accordingly.
>>
>> Signed-off-by: Yushan Wang <wangyushan12@huawei.com>
>> ---
>>  tools/perf/arch/x86/util/Build                           | 1 -
>>  tools/perf/util/Build                                    | 1 +
>>  tools/perf/{arch/x86/util/iostat.c => util/x86-iostat.c} | 0
>>  3 files changed, 1 insertion(+), 1 deletion(-)
>>  rename tools/perf/{arch/x86/util/iostat.c => util/x86-iostat.c} (100%)
>
> To avoid cluttering up tools/perf/util, would it make sense to create
> a "tools/perf/util/iostat-arch" directory similar to  annotate-arch,
> dwarf-regs-arch, kvm-stat-arch and perf-regs-arch?
>
> Thanks,
> Ian

Yes, I can do that in the next version.

In fact, as what latter comments addressed, I think merging these iostat
handlers into one source file to avoid redundancy may be better.

Thanks,
Yushan

>

[RFT PATCH v2 4/7] perf-x86: iostat: Change iostat_prefix() to static

[Yushan Wang]

Change iostat_prefix() to static function, since it is not used outside.

Signed-off-by: Yushan Wang <wangyushan12@huawei.com>
---
 tools/perf/util/iostat.c     |  7 ------
 tools/perf/util/iostat.h     |  2 --
 tools/perf/util/x86-iostat.c | 44 ++++++++++++++++++------------------
 3 files changed, 22 insertions(+), 31 deletions(-)

diff --git a/tools/perf/util/iostat.c b/tools/perf/util/iostat.c
index b770bd473af7..a68ab100780d 100644
--- a/tools/perf/util/iostat.c
+++ b/tools/perf/util/iostat.c
@@ -37,13 +37,6 @@ __weak void iostat_print_metric(struct perf_stat_config *config __maybe_unused,
 {
 }
 
-__weak void iostat_prefix(struct evlist *evlist __maybe_unused,
-			  struct perf_stat_config *config __maybe_unused,
-			  char *prefix __maybe_unused,
-			  struct timespec *ts __maybe_unused)
-{
-}
-
 __weak void iostat_print_counters(struct evlist *evlist __maybe_unused,
 				  struct perf_stat_config *config __maybe_unused,
 				  struct timespec *ts __maybe_unused,
diff --git a/tools/perf/util/iostat.h b/tools/perf/util/iostat.h
index a4e7299c5c2f..820930a096d9 100644
--- a/tools/perf/util/iostat.h
+++ b/tools/perf/util/iostat.h
@@ -35,8 +35,6 @@ int iostat_parse(const struct option *opt, const char *str,
 		 int unset __maybe_unused);
 void iostat_list(struct evlist *evlist, struct perf_stat_config *config);
 void iostat_release(struct evlist *evlist);
-void iostat_prefix(struct evlist *evlist, struct perf_stat_config *config,
-		   char *prefix, struct timespec *ts);
 void iostat_print_header_prefix(struct perf_stat_config *config);
 void iostat_print_metric(struct perf_stat_config *config, struct evsel *evsel,
 			 struct perf_stat_output_ctx *out);
diff --git a/tools/perf/util/x86-iostat.c b/tools/perf/util/x86-iostat.c
index 7442a2cd87ed..f8d4c9718594 100644
--- a/tools/perf/util/x86-iostat.c
+++ b/tools/perf/util/x86-iostat.c
@@ -332,6 +332,28 @@ static int iostat_event_group(struct evlist *evl,
 	return ret;
 }
 
+static void iostat_prefix(struct evlist *evlist,
+			  struct perf_stat_config *config,
+			  char *prefix, struct timespec *ts)
+{
+	struct iio_root_port *rp = evlist->selected->priv;
+
+	if (rp) {
+		/*
+		 * TODO: This is the incorrect format in JSON mode.
+		 *       See prepare_timestamp()
+		 */
+		if (ts)
+			sprintf(prefix, "%6lu.%09lu%s%04x:%02x%s",
+				ts->tv_sec, ts->tv_nsec,
+				config->csv_sep, rp->domain, rp->bus,
+				config->csv_sep);
+		else
+			sprintf(prefix, "%04x:%02x%s", rp->domain, rp->bus,
+				config->csv_sep);
+	}
+}
+
 int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)
 {
 	if (evlist->core.nr_entries > 0) {
@@ -396,28 +418,6 @@ void iostat_release(struct evlist *evlist)
 	}
 }
 
-void iostat_prefix(struct evlist *evlist,
-		   struct perf_stat_config *config,
-		   char *prefix, struct timespec *ts)
-{
-	struct iio_root_port *rp = evlist->selected->priv;
-
-	if (rp) {
-		/*
-		 * TODO: This is the incorrect format in JSON mode.
-		 *       See prepare_timestamp()
-		 */
-		if (ts)
-			sprintf(prefix, "%6lu.%09lu%s%04x:%02x%s",
-				ts->tv_sec, ts->tv_nsec,
-				config->csv_sep, rp->domain, rp->bus,
-				config->csv_sep);
-		else
-			sprintf(prefix, "%04x:%02x%s", rp->domain, rp->bus,
-				config->csv_sep);
-	}
-}
-
 void iostat_print_header_prefix(struct perf_stat_config *config)
 {
 	if (config->csv_output)
-- 
2.33.0

Re: [RFT PATCH v2 4/7] perf-x86: iostat: Change iostat_prefix() to static

[Ian Rogers]

On Wed, May 6, 2026 at 11:37 PM Yushan Wang <wangyushan12@huawei.com> wrote:
>
> Change iostat_prefix() to static function, since it is not used outside.
>
> Signed-off-by: Yushan Wang <wangyushan12@huawei.com>

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

> ---
>  tools/perf/util/iostat.c     |  7 ------
>  tools/perf/util/iostat.h     |  2 --
>  tools/perf/util/x86-iostat.c | 44 ++++++++++++++++++------------------
>  3 files changed, 22 insertions(+), 31 deletions(-)
>
> diff --git a/tools/perf/util/iostat.c b/tools/perf/util/iostat.c
> index b770bd473af7..a68ab100780d 100644
> --- a/tools/perf/util/iostat.c
> +++ b/tools/perf/util/iostat.c
> @@ -37,13 +37,6 @@ __weak void iostat_print_metric(struct perf_stat_config *config __maybe_unused,
>  {
>  }
>
> -__weak void iostat_prefix(struct evlist *evlist __maybe_unused,
> -                         struct perf_stat_config *config __maybe_unused,
> -                         char *prefix __maybe_unused,
> -                         struct timespec *ts __maybe_unused)
> -{
> -}
> -
>  __weak void iostat_print_counters(struct evlist *evlist __maybe_unused,
>                                   struct perf_stat_config *config __maybe_unused,
>                                   struct timespec *ts __maybe_unused,

This change doesn't remove the weak symbols, but they should go. We
should use the ELF machine to determine which architecture version to
use.

Thanks,
Ian

> diff --git a/tools/perf/util/iostat.h b/tools/perf/util/iostat.h
> index a4e7299c5c2f..820930a096d9 100644
> --- a/tools/perf/util/iostat.h
> +++ b/tools/perf/util/iostat.h
> @@ -35,8 +35,6 @@ int iostat_parse(const struct option *opt, const char *str,
>                  int unset __maybe_unused);
>  void iostat_list(struct evlist *evlist, struct perf_stat_config *config);
>  void iostat_release(struct evlist *evlist);
> -void iostat_prefix(struct evlist *evlist, struct perf_stat_config *config,
> -                  char *prefix, struct timespec *ts);
>  void iostat_print_header_prefix(struct perf_stat_config *config);
>  void iostat_print_metric(struct perf_stat_config *config, struct evsel *evsel,
>                          struct perf_stat_output_ctx *out);
> diff --git a/tools/perf/util/x86-iostat.c b/tools/perf/util/x86-iostat.c
> index 7442a2cd87ed..f8d4c9718594 100644
> --- a/tools/perf/util/x86-iostat.c
> +++ b/tools/perf/util/x86-iostat.c
> @@ -332,6 +332,28 @@ static int iostat_event_group(struct evlist *evl,
>         return ret;
>  }
>
> +static void iostat_prefix(struct evlist *evlist,
> +                         struct perf_stat_config *config,
> +                         char *prefix, struct timespec *ts)
> +{
> +       struct iio_root_port *rp = evlist->selected->priv;
> +
> +       if (rp) {
> +               /*
> +                * TODO: This is the incorrect format in JSON mode.
> +                *       See prepare_timestamp()
> +                */
> +               if (ts)
> +                       sprintf(prefix, "%6lu.%09lu%s%04x:%02x%s",
> +                               ts->tv_sec, ts->tv_nsec,
> +                               config->csv_sep, rp->domain, rp->bus,
> +                               config->csv_sep);
> +               else
> +                       sprintf(prefix, "%04x:%02x%s", rp->domain, rp->bus,
> +                               config->csv_sep);
> +       }
> +}
> +
>  int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)
>  {
>         if (evlist->core.nr_entries > 0) {
> @@ -396,28 +418,6 @@ void iostat_release(struct evlist *evlist)
>         }
>  }
>
> -void iostat_prefix(struct evlist *evlist,
> -                  struct perf_stat_config *config,
> -                  char *prefix, struct timespec *ts)
> -{
> -       struct iio_root_port *rp = evlist->selected->priv;
> -
> -       if (rp) {
> -               /*
> -                * TODO: This is the incorrect format in JSON mode.
> -                *       See prepare_timestamp()
> -                */
> -               if (ts)
> -                       sprintf(prefix, "%6lu.%09lu%s%04x:%02x%s",
> -                               ts->tv_sec, ts->tv_nsec,
> -                               config->csv_sep, rp->domain, rp->bus,
> -                               config->csv_sep);
> -               else
> -                       sprintf(prefix, "%04x:%02x%s", rp->domain, rp->bus,
> -                               config->csv_sep);
> -       }
> -}
> -
>  void iostat_print_header_prefix(struct perf_stat_config *config)
>  {
>         if (config->csv_output)
> --
> 2.33.0
>

Re: [RFT PATCH v2 4/7] perf-x86: iostat: Change iostat_prefix() to static

[Yushan Wang]

On 5/7/2026 11:39 PM, Ian Rogers wrote:
> On Wed, May 6, 2026 at 11:37 PM Yushan Wang <wangyushan12@huawei.com> wrote:
>>
>> Change iostat_prefix() to static function, since it is not used outside.
>>
>> Signed-off-by: Yushan Wang <wangyushan12@huawei.com>
>
> Reviewed-by: Ian Rogers <irogers@google.com>
>
>> ---
>>  tools/perf/util/iostat.c     |  7 ------
>>  tools/perf/util/iostat.h     |  2 --
>>  tools/perf/util/x86-iostat.c | 44 ++++++++++++++++++------------------
>>  3 files changed, 22 insertions(+), 31 deletions(-)
>>
>> diff --git a/tools/perf/util/iostat.c b/tools/perf/util/iostat.c
>> index b770bd473af7..a68ab100780d 100644
>> --- a/tools/perf/util/iostat.c
>> +++ b/tools/perf/util/iostat.c
>> @@ -37,13 +37,6 @@ __weak void iostat_print_metric(struct perf_stat_config *config __maybe_unused,
>>  {
>>  }
>>
>> -__weak void iostat_prefix(struct evlist *evlist __maybe_unused,
>> -                         struct perf_stat_config *config __maybe_unused,
>> -                         char *prefix __maybe_unused,
>> -                         struct timespec *ts __maybe_unused)
>> -{
>> -}
>> -
>>  __weak void iostat_print_counters(struct evlist *evlist __maybe_unused,
>>                                   struct perf_stat_config *config __maybe_unused,
>>                                   struct timespec *ts __maybe_unused,
>
> This change doesn't remove the weak symbols, but they should go. We
> should use the ELF machine to determine which architecture version to
> use.

The weak symbols are removed in patch 6. Reserving them here keeps this patch
from failing perf build :)

Thanks,
Yushan

>
> Thanks,
> Ian
>

[...]

[RFT PATCH v2 5/7] perf-iostat: Extend iostat interface to support different iostat PMUs

[Yushan Wang]

From: Shiju Jose <shiju.jose@huawei.com>

Currently, platform-specific iostat code for PMUs is implemented as a
common iostat callback interface and linked during build. This approach
limits support for iostat across different implementations of PMU of the
same architecture.

To address this, extend common iostat interface to provide support for
different PMUs by allowing each PMU to register itself and receive
callbacks to its PMU-specific functions through the unified iostat
framework.

Signed-off-by: Shiju Jose  <shiju.jose@huawei.com>
Co-developed-by: Yushan Wang <wangyushan12@huawei.com>
Signed-off-by: Yushan Wang <wangyushan12@huawei.com>
---
 tools/perf/util/iostat.c | 88 ++++++++++++++++++++++++++++++----------
 tools/perf/util/iostat.h | 38 ++++++++++++++++-
 2 files changed, 102 insertions(+), 24 deletions(-)

diff --git a/tools/perf/util/iostat.c b/tools/perf/util/iostat.c
index a68ab100780d..90607d1cf3fa 100644
--- a/tools/perf/util/iostat.c
+++ b/tools/perf/util/iostat.c
@@ -1,47 +1,91 @@
 // SPDX-License-Identifier: GPL-2.0
 #include "util/iostat.h"
-#include "util/debug.h"
+
+/*
+ * Below iostat_* function calls are scattered through out perf stat process,
+ * allowing multiple iostat PMUs and iterated them in following functions may
+ * violate calling conventions or cause incorrect display.
+ *
+ * Default to register the first PMU device that matches any of the specified
+ * iostat pmu name wildcards.
+ */
+static struct iostat_pmu *iostat_pmu;
 
 enum iostat_mode_t iostat_mode = IOSTAT_NONE;
 
-__weak int iostat_prepare(struct evlist *evlist __maybe_unused,
-			  struct perf_stat_config *config __maybe_unused)
+__weak int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)
+{
+	if (!iostat_pmu)
+		return -1;
+
+	return iostat_pmu->prepare(evlist, config);
+}
+
+__weak int iostat_parse(const struct option *opt, const char *str, int unset)
 {
-	return -1;
+	if (!iostat_pmu)
+		return -1;
+
+	return iostat_pmu->parse(opt, str, unset);
 }
 
-__weak int iostat_parse(const struct option *opt __maybe_unused,
-			 const char *str __maybe_unused,
-			 int unset __maybe_unused)
+__weak void iostat_list(struct evlist *evlist, struct perf_stat_config *config)
 {
-	pr_err("iostat mode is not supported on current platform\n");
-	return -1;
+	iostat_pmu->list(evlist, config);
 }
 
-__weak void iostat_list(struct evlist *evlist __maybe_unused,
-		       struct perf_stat_config *config __maybe_unused)
+__weak void iostat_release(struct evlist *evlist)
 {
+	iostat_pmu->release(evlist);
 }
 
-__weak void iostat_release(struct evlist *evlist __maybe_unused)
+__weak void iostat_print_header_prefix(struct perf_stat_config *config)
 {
+	iostat_pmu->print_header_prefix(config);
 }
 
-__weak void iostat_print_header_prefix(struct perf_stat_config *config __maybe_unused)
+__weak void iostat_print_metric(struct perf_stat_config *config,
+				struct evsel *evsel,
+				struct perf_stat_output_ctx *out)
 {
+	iostat_pmu->print_metric(config, evsel, out);
 }
 
-__weak void iostat_print_metric(struct perf_stat_config *config __maybe_unused,
-				struct evsel *evsel __maybe_unused,
-				struct perf_stat_output_ctx *out __maybe_unused)
+__weak void iostat_print_counters(struct evlist *evlist,
+				  struct perf_stat_config *config,
+				  struct timespec *ts, char *prefix,
+				  iostat_print_counter_t print_cnt_cb,
+				  void *arg)
 {
+	iostat_pmu->print_counters(evlist, config, ts, prefix,
+				   print_cnt_cb, arg);
+}
+
+void register_iostat_pmu(struct iostat_pmu *pmu)
+{
+	if (!pmu || !pmu->match)
+		return;
+
+	if (iostat_pmu || !pmu->match(pmu))
+		return;
+
+	iostat_pmu = pmu;
+}
+
+static void unregister_iostat_pmu(void)
+{
+	if (!iostat_pmu)
+		return;
+
+	/*
+	 * Release function of iostat_pmu is called on the exit of cmd_stat, we
+	 * don't need to call release function here.
+	 */
+	iostat_pmu = NULL;
 }
 
-__weak void iostat_print_counters(struct evlist *evlist __maybe_unused,
-				  struct perf_stat_config *config __maybe_unused,
-				  struct timespec *ts __maybe_unused,
-				  char *prefix __maybe_unused,
-				  iostat_print_counter_t print_cnt_cb __maybe_unused,
-				  void *arg __maybe_unused)
+__attribute__((destructor))
+static void iostat_exit(void)
 {
+	unregister_iostat_pmu();
 }
diff --git a/tools/perf/util/iostat.h b/tools/perf/util/iostat.h
index 820930a096d9..5cc8963c6122 100644
--- a/tools/perf/util/iostat.h
+++ b/tools/perf/util/iostat.h
@@ -10,6 +10,7 @@
 #ifndef _IOSTAT_H
 #define _IOSTAT_H
 
+#include <stdbool.h>
 #include <subcmd/parse-options.h>
 #include "util/stat.h"
 #include "util/parse-events.h"
@@ -31,8 +32,7 @@ extern enum iostat_mode_t iostat_mode;
 typedef void (*iostat_print_counter_t)(struct perf_stat_config *, struct evsel *, void *);
 
 int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config);
-int iostat_parse(const struct option *opt, const char *str,
-		 int unset __maybe_unused);
+int iostat_parse(const struct option *opt, const char *str, int unset);
 void iostat_list(struct evlist *evlist, struct perf_stat_config *config);
 void iostat_release(struct evlist *evlist);
 void iostat_print_header_prefix(struct perf_stat_config *config);
@@ -42,4 +42,38 @@ void iostat_print_counters(struct evlist *evlist,
 			   struct perf_stat_config *config, struct timespec *ts,
 			   char *prefix, iostat_print_counter_t print_cnt_cb, void *arg);
 
+/**
+ * struct iostat_pmu - Callbacks for an iostat-capable PMU backend.
+ * @pmu_name_wildcard: Glob pattern to identify the PMU (e.g. "uncore_iio*").
+ * @match: Detect whether matching PMUs exist on this system.
+ * @prepare: Set up events and config for iostat collection.
+ * @parse: Parse the --iostat option argument.
+ * @list: Display available iostat PMU instances.
+ * @print_header_prefix: Print the column header prefix.
+ * @print_metric: Format and print one metric value.
+ * @print_counters:  Iterate over counters and print per-port results.
+ * @release: Clean up PMU-specific resources.
+ */
+struct iostat_pmu {
+	const char *pmu_name_wildcard;
+	bool (*match)(struct iostat_pmu *iostat_pmu);
+	int (*prepare)(struct evlist *evlist, struct perf_stat_config *config);
+	int (*parse)(const struct option *opt, const char *str, int unset);
+	void (*list)(struct evlist *evlist, struct perf_stat_config *config);
+	void (*print_header_prefix)(struct perf_stat_config *config);
+	void (*print_metric)(struct perf_stat_config *config, struct evsel *evsel,
+			     struct perf_stat_output_ctx *out);
+	void (*print_counters)(struct evlist *evlist,
+			       struct perf_stat_config *config, struct timespec *ts,
+			       char *prefix, iostat_print_counter_t print_cnt_cb, void *arg);
+	void (*release)(struct evlist *evlist __maybe_unused);
+};
+
+/*
+ * Register an iostat PMU handler. Called from __attribute__((constructor))
+ * functions in each backend's translation unit.
+ *
+ * Only the first matched backend is activated.
+ */
+void register_iostat_pmu(struct iostat_pmu *iostat_pmu);
 #endif /* _IOSTAT_H */
-- 
2.33.0

Re: [RFT PATCH v2 5/7] perf-iostat: Extend iostat interface to support different iostat PMUs

[Ian Rogers]

On Wed, May 6, 2026 at 11:37 PM Yushan Wang <wangyushan12@huawei.com> wrote:
>
> From: Shiju Jose <shiju.jose@huawei.com>
>
> Currently, platform-specific iostat code for PMUs is implemented as a
> common iostat callback interface and linked during build. This approach
> limits support for iostat across different implementations of PMU of the
> same architecture.
>
> To address this, extend common iostat interface to provide support for
> different PMUs by allowing each PMU to register itself and receive
> callbacks to its PMU-specific functions through the unified iostat
> framework.
>
> Signed-off-by: Shiju Jose  <shiju.jose@huawei.com>
> Co-developed-by: Yushan Wang <wangyushan12@huawei.com>
> Signed-off-by: Yushan Wang <wangyushan12@huawei.com>
> ---
>  tools/perf/util/iostat.c | 88 ++++++++++++++++++++++++++++++----------
>  tools/perf/util/iostat.h | 38 ++++++++++++++++-
>  2 files changed, 102 insertions(+), 24 deletions(-)
>
> diff --git a/tools/perf/util/iostat.c b/tools/perf/util/iostat.c
> index a68ab100780d..90607d1cf3fa 100644
> --- a/tools/perf/util/iostat.c
> +++ b/tools/perf/util/iostat.c
> @@ -1,47 +1,91 @@
>  // SPDX-License-Identifier: GPL-2.0
>  #include "util/iostat.h"
> -#include "util/debug.h"
> +
> +/*
> + * Below iostat_* function calls are scattered through out perf stat process,
> + * allowing multiple iostat PMUs and iterated them in following functions may
> + * violate calling conventions or cause incorrect display.
> + *
> + * Default to register the first PMU device that matches any of the specified
> + * iostat pmu name wildcards.
> + */
> +static struct iostat_pmu *iostat_pmu;

Could there be more than set of printing functions?

>  enum iostat_mode_t iostat_mode = IOSTAT_NONE;
>
> -__weak int iostat_prepare(struct evlist *evlist __maybe_unused,
> -                         struct perf_stat_config *config __maybe_unused)
> +__weak int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)

Eww.. weak.

> +{
> +       if (!iostat_pmu)
> +               return -1;
> +
> +       return iostat_pmu->prepare(evlist, config);
> +}
> +
> +__weak int iostat_parse(const struct option *opt, const char *str, int unset)
>  {
> -       return -1;
> +       if (!iostat_pmu)
> +               return -1;
> +
> +       return iostat_pmu->parse(opt, str, unset);
>  }
>
> -__weak int iostat_parse(const struct option *opt __maybe_unused,
> -                        const char *str __maybe_unused,
> -                        int unset __maybe_unused)
> +__weak void iostat_list(struct evlist *evlist, struct perf_stat_config *config)
>  {
> -       pr_err("iostat mode is not supported on current platform\n");
> -       return -1;
> +       iostat_pmu->list(evlist, config);
>  }
>
> -__weak void iostat_list(struct evlist *evlist __maybe_unused,
> -                      struct perf_stat_config *config __maybe_unused)
> +__weak void iostat_release(struct evlist *evlist)
>  {
> +       iostat_pmu->release(evlist);
>  }
>
> -__weak void iostat_release(struct evlist *evlist __maybe_unused)
> +__weak void iostat_print_header_prefix(struct perf_stat_config *config)
>  {
> +       iostat_pmu->print_header_prefix(config);
>  }
>
> -__weak void iostat_print_header_prefix(struct perf_stat_config *config __maybe_unused)
> +__weak void iostat_print_metric(struct perf_stat_config *config,
> +                               struct evsel *evsel,
> +                               struct perf_stat_output_ctx *out)
>  {
> +       iostat_pmu->print_metric(config, evsel, out);
>  }
>
> -__weak void iostat_print_metric(struct perf_stat_config *config __maybe_unused,
> -                               struct evsel *evsel __maybe_unused,
> -                               struct perf_stat_output_ctx *out __maybe_unused)
> +__weak void iostat_print_counters(struct evlist *evlist,
> +                                 struct perf_stat_config *config,
> +                                 struct timespec *ts, char *prefix,
> +                                 iostat_print_counter_t print_cnt_cb,
> +                                 void *arg)
>  {
> +       iostat_pmu->print_counters(evlist, config, ts, prefix,
> +                                  print_cnt_cb, arg);
> +}
> +
> +void register_iostat_pmu(struct iostat_pmu *pmu)
> +{
> +       if (!pmu || !pmu->match)
> +               return;
> +
> +       if (iostat_pmu || !pmu->match(pmu))
> +               return;
> +
> +       iostat_pmu = pmu;
> +}
> +
> +static void unregister_iostat_pmu(void)
> +{
> +       if (!iostat_pmu)
> +               return;
> +
> +       /*
> +        * Release function of iostat_pmu is called on the exit of cmd_stat, we
> +        * don't need to call release function here.
> +        */
> +       iostat_pmu = NULL;
>  }
>
> -__weak void iostat_print_counters(struct evlist *evlist __maybe_unused,
> -                                 struct perf_stat_config *config __maybe_unused,
> -                                 struct timespec *ts __maybe_unused,
> -                                 char *prefix __maybe_unused,
> -                                 iostat_print_counter_t print_cnt_cb __maybe_unused,
> -                                 void *arg __maybe_unused)
> +__attribute__((destructor))

I don't believe using this attribute is standard in either perf or the kernel.

> +static void iostat_exit(void)
>  {
> +       unregister_iostat_pmu();
>  }
> diff --git a/tools/perf/util/iostat.h b/tools/perf/util/iostat.h
> index 820930a096d9..5cc8963c6122 100644
> --- a/tools/perf/util/iostat.h
> +++ b/tools/perf/util/iostat.h
> @@ -10,6 +10,7 @@
>  #ifndef _IOSTAT_H
>  #define _IOSTAT_H
>
> +#include <stdbool.h>
>  #include <subcmd/parse-options.h>
>  #include "util/stat.h"
>  #include "util/parse-events.h"
> @@ -31,8 +32,7 @@ extern enum iostat_mode_t iostat_mode;
>  typedef void (*iostat_print_counter_t)(struct perf_stat_config *, struct evsel *, void *);
>
>  int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config);
> -int iostat_parse(const struct option *opt, const char *str,
> -                int unset __maybe_unused);
> +int iostat_parse(const struct option *opt, const char *str, int unset);
>  void iostat_list(struct evlist *evlist, struct perf_stat_config *config);
>  void iostat_release(struct evlist *evlist);
>  void iostat_print_header_prefix(struct perf_stat_config *config);
> @@ -42,4 +42,38 @@ void iostat_print_counters(struct evlist *evlist,
>                            struct perf_stat_config *config, struct timespec *ts,
>                            char *prefix, iostat_print_counter_t print_cnt_cb, void *arg);
>
> +/**
> + * struct iostat_pmu - Callbacks for an iostat-capable PMU backend.
> + * @pmu_name_wildcard: Glob pattern to identify the PMU (e.g. "uncore_iio*").
> + * @match: Detect whether matching PMUs exist on this system.
> + * @prepare: Set up events and config for iostat collection.
> + * @parse: Parse the --iostat option argument.
> + * @list: Display available iostat PMU instances.
> + * @print_header_prefix: Print the column header prefix.
> + * @print_metric: Format and print one metric value.
> + * @print_counters:  Iterate over counters and print per-port results.
> + * @release: Clean up PMU-specific resources.
> + */
> +struct iostat_pmu {

I wonder if iostat_pmu is the best name here, perhaps
iostat_callbacks? It would be nice not to overload the term PMU in the
code with regular PMUs and the iostat PMUs.

Thanks,
Ian

> +       const char *pmu_name_wildcard;
> +       bool (*match)(struct iostat_pmu *iostat_pmu);
> +       int (*prepare)(struct evlist *evlist, struct perf_stat_config *config);
> +       int (*parse)(const struct option *opt, const char *str, int unset);
> +       void (*list)(struct evlist *evlist, struct perf_stat_config *config);
> +       void (*print_header_prefix)(struct perf_stat_config *config);
> +       void (*print_metric)(struct perf_stat_config *config, struct evsel *evsel,
> +                            struct perf_stat_output_ctx *out);
> +       void (*print_counters)(struct evlist *evlist,
> +                              struct perf_stat_config *config, struct timespec *ts,
> +                              char *prefix, iostat_print_counter_t print_cnt_cb, void *arg);
> +       void (*release)(struct evlist *evlist __maybe_unused);
> +};
> +
> +/*
> + * Register an iostat PMU handler. Called from __attribute__((constructor))
> + * functions in each backend's translation unit.
> + *
> + * Only the first matched backend is activated.
> + */
> +void register_iostat_pmu(struct iostat_pmu *iostat_pmu);
>  #endif /* _IOSTAT_H */
> --
> 2.33.0
>

Re: [RFT PATCH v2 5/7] perf-iostat: Extend iostat interface to support different iostat PMUs

[Yushan Wang]

On 5/7/2026 11:47 PM, Ian Rogers wrote:
> On Wed, May 6, 2026 at 11:37 PM Yushan Wang <wangyushan12@huawei.com> wrote:
>>
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>> Currently, platform-specific iostat code for PMUs is implemented as a
>> common iostat callback interface and linked during build. This approach
>> limits support for iostat across different implementations of PMU of the
>> same architecture.
>>
>> To address this, extend common iostat interface to provide support for
>> different PMUs by allowing each PMU to register itself and receive
>> callbacks to its PMU-specific functions through the unified iostat
>> framework.
>>
>> Signed-off-by: Shiju Jose  <shiju.jose@huawei.com>
>> Co-developed-by: Yushan Wang <wangyushan12@huawei.com>
>> Signed-off-by: Yushan Wang <wangyushan12@huawei.com>
>> ---
>>  tools/perf/util/iostat.c | 88 ++++++++++++++++++++++++++++++----------
>>  tools/perf/util/iostat.h | 38 ++++++++++++++++-
>>  2 files changed, 102 insertions(+), 24 deletions(-)
>>
>> diff --git a/tools/perf/util/iostat.c b/tools/perf/util/iostat.c
>> index a68ab100780d..90607d1cf3fa 100644
>> --- a/tools/perf/util/iostat.c
>> +++ b/tools/perf/util/iostat.c
>> @@ -1,47 +1,91 @@
>>  // SPDX-License-Identifier: GPL-2.0
>>  #include "util/iostat.h"
>> -#include "util/debug.h"
>> +
>> +/*
>> + * Below iostat_* function calls are scattered through out perf stat process,
>> + * allowing multiple iostat PMUs and iterated them in following functions may
>> + * violate calling conventions or cause incorrect display.
>> + *
>> + * Default to register the first PMU device that matches any of the specified
>> + * iostat pmu name wildcards.
>> + */
>> +static struct iostat_pmu *iostat_pmu;
>
> Could there be more than set of printing functions?

At the second thought, to support cross-platform iostat, multiple printing
function should be supported, otherwise interpreting dumped perf.data from
another architecture may just not work.

This may require some more changes to the embedded iostat function calls, I can
try to add that support in the next version, thanks for pointing out!

>
>>  enum iostat_mode_t iostat_mode = IOSTAT_NONE;
>>
>> -__weak int iostat_prepare(struct evlist *evlist __maybe_unused,
>> -                         struct perf_stat_config *config __maybe_unused)
>> +__weak int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)
>
> Eww.. weak.

These __weak symbols are added to avoid confliction with existing x86 iostat
implementations, and are removed in patch 6.

>

[...]

>>
>> -__weak void iostat_print_counters(struct evlist *evlist __maybe_unused,
>> -                                 struct perf_stat_config *config __maybe_unused,
>> -                                 struct timespec *ts __maybe_unused,
>> -                                 char *prefix __maybe_unused,
>> -                                 iostat_print_counter_t print_cnt_cb __maybe_unused,
>> -                                 void *arg __maybe_unused)
>> +__attribute__((destructor))
>
> I don't believe using this attribute is standard in either perf or the kernel.

Sorry, I will fix that with explict calls to constructors and destructors.

>
>> +static void iostat_exit(void)
>>  {
>> +       unregister_iostat_pmu();
>>  }
>> diff --git a/tools/perf/util/iostat.h b/tools/perf/util/iostat.h
>> index 820930a096d9..5cc8963c6122 100644
>> --- a/tools/perf/util/iostat.h
>> +++ b/tools/perf/util/iostat.h
>> @@ -10,6 +10,7 @@
>>  #ifndef _IOSTAT_H
>>  #define _IOSTAT_H
>>
>> +#include <stdbool.h>
>>  #include <subcmd/parse-options.h>
>>  #include "util/stat.h"
>>  #include "util/parse-events.h"
>> @@ -31,8 +32,7 @@ extern enum iostat_mode_t iostat_mode;
>>  typedef void (*iostat_print_counter_t)(struct perf_stat_config *, struct evsel *, void *);
>>
>>  int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config);
>> -int iostat_parse(const struct option *opt, const char *str,
>> -                int unset __maybe_unused);
>> +int iostat_parse(const struct option *opt, const char *str, int unset);
>>  void iostat_list(struct evlist *evlist, struct perf_stat_config *config);
>>  void iostat_release(struct evlist *evlist);
>>  void iostat_print_header_prefix(struct perf_stat_config *config);
>> @@ -42,4 +42,38 @@ void iostat_print_counters(struct evlist *evlist,
>>                            struct perf_stat_config *config, struct timespec *ts,
>>                            char *prefix, iostat_print_counter_t print_cnt_cb, void *arg);
>>
>> +/**
>> + * struct iostat_pmu - Callbacks for an iostat-capable PMU backend.
>> + * @pmu_name_wildcard: Glob pattern to identify the PMU (e.g. "uncore_iio*").
>> + * @match: Detect whether matching PMUs exist on this system.
>> + * @prepare: Set up events and config for iostat collection.
>> + * @parse: Parse the --iostat option argument.
>> + * @list: Display available iostat PMU instances.
>> + * @print_header_prefix: Print the column header prefix.
>> + * @print_metric: Format and print one metric value.
>> + * @print_counters:  Iterate over counters and print per-port results.
>> + * @release: Clean up PMU-specific resources.
>> + */
>> +struct iostat_pmu {
>
> I wonder if iostat_pmu is the best name here, perhaps
> iostat_callbacks? It would be nice not to overload the term PMU in the
> code with regular PMUs and the iostat PMUs.

Yes. iostat_backend or iostat_handlers may be more specific. I'll change that in
the next version

>
> Thanks,
> Ian

Thank you for review!
Yushan

>

[...]

[RFT PATCH v2 6/7] perf-iostat: Make x86 iostat compatible with new iostat framework

[Yushan Wang]

From: Shiju Jose <shiju.jose@huawei.com>

Change the original x86 iio iostat supporter to be compatible with the
set of iostat frameworks.

The matching function of x86 iio may not be correct.

Signed-off-by: Shiju Jose  <shiju.jose@huawei.com>
Co-developed-by: Yushan Wang <wangyushan12@huawei.com>
Signed-off-by: Yushan Wang <wangyushan12@huawei.com>
---
 tools/perf/util/iostat.c     | 26 +++++++--------
 tools/perf/util/x86-iostat.c | 62 ++++++++++++++++++++++++++----------
 2 files changed, 59 insertions(+), 29 deletions(-)

diff --git a/tools/perf/util/iostat.c b/tools/perf/util/iostat.c
index 90607d1cf3fa..6c51f4a521af 100644
--- a/tools/perf/util/iostat.c
+++ b/tools/perf/util/iostat.c
@@ -13,7 +13,7 @@ static struct iostat_pmu *iostat_pmu;
 
 enum iostat_mode_t iostat_mode = IOSTAT_NONE;
 
-__weak int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)
+int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)
 {
 	if (!iostat_pmu)
 		return -1;
@@ -21,7 +21,7 @@ __weak int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config
 	return iostat_pmu->prepare(evlist, config);
 }
 
-__weak int iostat_parse(const struct option *opt, const char *str, int unset)
+int iostat_parse(const struct option *opt, const char *str, int unset)
 {
 	if (!iostat_pmu)
 		return -1;
@@ -29,33 +29,33 @@ __weak int iostat_parse(const struct option *opt, const char *str, int unset)
 	return iostat_pmu->parse(opt, str, unset);
 }
 
-__weak void iostat_list(struct evlist *evlist, struct perf_stat_config *config)
+void iostat_list(struct evlist *evlist, struct perf_stat_config *config)
 {
 	iostat_pmu->list(evlist, config);
 }
 
-__weak void iostat_release(struct evlist *evlist)
+void iostat_release(struct evlist *evlist)
 {
 	iostat_pmu->release(evlist);
 }
 
-__weak void iostat_print_header_prefix(struct perf_stat_config *config)
+void iostat_print_header_prefix(struct perf_stat_config *config)
 {
 	iostat_pmu->print_header_prefix(config);
 }
 
-__weak void iostat_print_metric(struct perf_stat_config *config,
-				struct evsel *evsel,
-				struct perf_stat_output_ctx *out)
+void iostat_print_metric(struct perf_stat_config *config,
+			 struct evsel *evsel,
+			 struct perf_stat_output_ctx *out)
 {
 	iostat_pmu->print_metric(config, evsel, out);
 }
 
-__weak void iostat_print_counters(struct evlist *evlist,
-				  struct perf_stat_config *config,
-				  struct timespec *ts, char *prefix,
-				  iostat_print_counter_t print_cnt_cb,
-				  void *arg)
+void iostat_print_counters(struct evlist *evlist,
+			   struct perf_stat_config *config,
+			   struct timespec *ts, char *prefix,
+			   iostat_print_counter_t print_cnt_cb,
+			   void *arg)
 {
 	iostat_pmu->print_counters(evlist, config, ts, prefix,
 				   print_cnt_cb, arg);
diff --git a/tools/perf/util/x86-iostat.c b/tools/perf/util/x86-iostat.c
index f8d4c9718594..778655cbc2a0 100644
--- a/tools/perf/util/x86-iostat.c
+++ b/tools/perf/util/x86-iostat.c
@@ -332,9 +332,9 @@ static int iostat_event_group(struct evlist *evl,
 	return ret;
 }
 
-static void iostat_prefix(struct evlist *evlist,
-			  struct perf_stat_config *config,
-			  char *prefix, struct timespec *ts)
+static void iio_iostat_prefix(struct evlist *evlist,
+			      struct perf_stat_config *config,
+			      char *prefix, struct timespec *ts)
 {
 	struct iio_root_port *rp = evlist->selected->priv;
 
@@ -354,7 +354,7 @@ static void iostat_prefix(struct evlist *evlist,
 	}
 }
 
-int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)
+static int iio_iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)
 {
 	if (evlist->core.nr_entries > 0) {
 		pr_warning("The -e and -M options are not supported."
@@ -371,8 +371,8 @@ int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)
 	return iostat_event_group(evlist, root_ports);
 }
 
-int iostat_parse(const struct option *opt, const char *str,
-		 int unset __maybe_unused)
+static int iio_iostat_parse(const struct option *opt, const char *str,
+			    int unset __maybe_unused)
 {
 	int ret;
 	struct perf_stat_config *config = (struct perf_stat_config *)opt->data;
@@ -392,7 +392,7 @@ int iostat_parse(const struct option *opt, const char *str,
 	return ret;
 }
 
-void iostat_list(struct evlist *evlist, struct perf_stat_config *config)
+static void iio_iostat_list(struct evlist *evlist, struct perf_stat_config *config)
 {
 	struct evsel *evsel;
 	struct iio_root_port *rp = NULL;
@@ -405,7 +405,7 @@ void iostat_list(struct evlist *evlist, struct perf_stat_config *config)
 	}
 }
 
-void iostat_release(struct evlist *evlist)
+static void iio_iostat_release(struct evlist *evlist)
 {
 	struct evsel *evsel;
 	struct iio_root_port *rp = NULL;
@@ -418,7 +418,7 @@ void iostat_release(struct evlist *evlist)
 	}
 }
 
-void iostat_print_header_prefix(struct perf_stat_config *config)
+static void iio_iostat_print_header_prefix(struct perf_stat_config *config)
 {
 	if (config->csv_output)
 		fputs("port,", config->output);
@@ -428,8 +428,8 @@ void iostat_print_header_prefix(struct perf_stat_config *config)
 		fprintf(config->output, "   port         ");
 }
 
-void iostat_print_metric(struct perf_stat_config *config, struct evsel *evsel,
-			 struct perf_stat_output_ctx *out)
+static void iio_iostat_print_metric(struct perf_stat_config *config, struct evsel *evsel,
+				    struct perf_stat_output_ctx *out)
 {
 	double iostat_value = 0;
 	u64 prev_count_val = 0;
@@ -452,24 +452,54 @@ void iostat_print_metric(struct perf_stat_config *config, struct evsel *evsel,
 			  iostat_value / (256 * 1024));
 }
 
-void iostat_print_counters(struct evlist *evlist,
-			   struct perf_stat_config *config, struct timespec *ts,
-			   char *prefix, iostat_print_counter_t print_cnt_cb, void *arg)
+static void iio_iostat_print_counters(struct evlist *evlist,
+				      struct perf_stat_config *config, struct timespec *ts,
+				      char *prefix, iostat_print_counter_t print_cnt_cb, void *arg)
 {
 	void *perf_device = NULL;
 	struct evsel *counter = evlist__first(evlist);
 
 	evlist__set_selected(evlist, counter);
-	iostat_prefix(evlist, config, prefix, ts);
+	iio_iostat_prefix(evlist, config, prefix, ts);
 	fprintf(config->output, "%s", prefix);
 	evlist__for_each_entry(evlist, counter) {
 		perf_device = evlist->selected->priv;
 		if (perf_device && perf_device != counter->priv) {
 			evlist__set_selected(evlist, counter);
-			iostat_prefix(evlist, config, prefix, ts);
+			iio_iostat_prefix(evlist, config, prefix, ts);
 			fprintf(config->output, "\n%s", prefix);
 		}
 		print_cnt_cb(config, counter, arg);
 	}
 	fputc('\n', config->output);
 }
+
+/*
+ * FIXME: pmu name prefix match might not work for x86 iio.
+ */
+static bool iio_iostat_probe(struct iostat_pmu *iostat_pmu)
+{
+	return perf_pmus__scan_matching_wildcard(NULL, iostat_pmu->pmu_name_wildcard);
+}
+
+static struct iostat_pmu x86_iio_iostat_pmu_list[]  = {
+	{
+		.pmu_name_wildcard = "uncore_iio*",
+		.match = iio_iostat_probe,
+		.prepare = iio_iostat_prepare,
+		.parse = iio_iostat_parse,
+		.list = iio_iostat_list,
+		.print_header_prefix = iio_iostat_print_header_prefix,
+		.print_metric = iio_iostat_print_metric,
+		.print_counters = iio_iostat_print_counters,
+		.release = iio_iostat_release,
+	},
+};
+
+static void __attribute__((constructor)) x86_iio_iostat_pmu_init(void)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(x86_iio_iostat_pmu_list); i++)
+		register_iostat_pmu(&x86_iio_iostat_pmu_list[i]);
+}
-- 
2.33.0

Re: [RFT PATCH v2 6/7] perf-iostat: Make x86 iostat compatible with new iostat framework

[Ian Rogers]

On Wed, May 6, 2026 at 11:37 PM Yushan Wang <wangyushan12@huawei.com> wrote:
>
> From: Shiju Jose <shiju.jose@huawei.com>
>
> Change the original x86 iio iostat supporter to be compatible with the
> set of iostat frameworks.
>
> The matching function of x86 iio may not be correct.
>
> Signed-off-by: Shiju Jose  <shiju.jose@huawei.com>
> Co-developed-by: Yushan Wang <wangyushan12@huawei.com>
> Signed-off-by: Yushan Wang <wangyushan12@huawei.com>
> ---
>  tools/perf/util/iostat.c     | 26 +++++++--------
>  tools/perf/util/x86-iostat.c | 62 ++++++++++++++++++++++++++----------
>  2 files changed, 59 insertions(+), 29 deletions(-)
>
> diff --git a/tools/perf/util/iostat.c b/tools/perf/util/iostat.c
> index 90607d1cf3fa..6c51f4a521af 100644
> --- a/tools/perf/util/iostat.c
> +++ b/tools/perf/util/iostat.c
> @@ -13,7 +13,7 @@ static struct iostat_pmu *iostat_pmu;
>
>  enum iostat_mode_t iostat_mode = IOSTAT_NONE;
>
> -__weak int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)
> +int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)

Less "weak", beautiful!

>  {
>         if (!iostat_pmu)
>                 return -1;
> @@ -21,7 +21,7 @@ __weak int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config
>         return iostat_pmu->prepare(evlist, config);
>  }
>
> -__weak int iostat_parse(const struct option *opt, const char *str, int unset)
> +int iostat_parse(const struct option *opt, const char *str, int unset)
>  {
>         if (!iostat_pmu)
>                 return -1;
> @@ -29,33 +29,33 @@ __weak int iostat_parse(const struct option *opt, const char *str, int unset)
>         return iostat_pmu->parse(opt, str, unset);
>  }
>
> -__weak void iostat_list(struct evlist *evlist, struct perf_stat_config *config)
> +void iostat_list(struct evlist *evlist, struct perf_stat_config *config)
>  {
>         iostat_pmu->list(evlist, config);
>  }
>
> -__weak void iostat_release(struct evlist *evlist)
> +void iostat_release(struct evlist *evlist)
>  {
>         iostat_pmu->release(evlist);
>  }
>
> -__weak void iostat_print_header_prefix(struct perf_stat_config *config)
> +void iostat_print_header_prefix(struct perf_stat_config *config)
>  {
>         iostat_pmu->print_header_prefix(config);
>  }
>
> -__weak void iostat_print_metric(struct perf_stat_config *config,
> -                               struct evsel *evsel,
> -                               struct perf_stat_output_ctx *out)
> +void iostat_print_metric(struct perf_stat_config *config,
> +                        struct evsel *evsel,
> +                        struct perf_stat_output_ctx *out)
>  {
>         iostat_pmu->print_metric(config, evsel, out);
>  }
>
> -__weak void iostat_print_counters(struct evlist *evlist,
> -                                 struct perf_stat_config *config,
> -                                 struct timespec *ts, char *prefix,
> -                                 iostat_print_counter_t print_cnt_cb,
> -                                 void *arg)
> +void iostat_print_counters(struct evlist *evlist,
> +                          struct perf_stat_config *config,
> +                          struct timespec *ts, char *prefix,
> +                          iostat_print_counter_t print_cnt_cb,
> +                          void *arg)
>  {
>         iostat_pmu->print_counters(evlist, config, ts, prefix,
>                                    print_cnt_cb, arg);
> diff --git a/tools/perf/util/x86-iostat.c b/tools/perf/util/x86-iostat.c
> index f8d4c9718594..778655cbc2a0 100644
> --- a/tools/perf/util/x86-iostat.c
> +++ b/tools/perf/util/x86-iostat.c
> @@ -332,9 +332,9 @@ static int iostat_event_group(struct evlist *evl,
>         return ret;
>  }
>
> -static void iostat_prefix(struct evlist *evlist,
> -                         struct perf_stat_config *config,
> -                         char *prefix, struct timespec *ts)
> +static void iio_iostat_prefix(struct evlist *evlist,
> +                             struct perf_stat_config *config,
> +                             char *prefix, struct timespec *ts)
>  {
>         struct iio_root_port *rp = evlist->selected->priv;
>
> @@ -354,7 +354,7 @@ static void iostat_prefix(struct evlist *evlist,
>         }
>  }
>
> -int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)
> +static int iio_iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)
>  {
>         if (evlist->core.nr_entries > 0) {
>                 pr_warning("The -e and -M options are not supported."
> @@ -371,8 +371,8 @@ int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)
>         return iostat_event_group(evlist, root_ports);
>  }
>
> -int iostat_parse(const struct option *opt, const char *str,
> -                int unset __maybe_unused)
> +static int iio_iostat_parse(const struct option *opt, const char *str,
> +                           int unset __maybe_unused)
>  {
>         int ret;
>         struct perf_stat_config *config = (struct perf_stat_config *)opt->data;
> @@ -392,7 +392,7 @@ int iostat_parse(const struct option *opt, const char *str,
>         return ret;
>  }
>
> -void iostat_list(struct evlist *evlist, struct perf_stat_config *config)
> +static void iio_iostat_list(struct evlist *evlist, struct perf_stat_config *config)
>  {
>         struct evsel *evsel;
>         struct iio_root_port *rp = NULL;
> @@ -405,7 +405,7 @@ void iostat_list(struct evlist *evlist, struct perf_stat_config *config)
>         }
>  }
>
> -void iostat_release(struct evlist *evlist)
> +static void iio_iostat_release(struct evlist *evlist)
>  {
>         struct evsel *evsel;
>         struct iio_root_port *rp = NULL;
> @@ -418,7 +418,7 @@ void iostat_release(struct evlist *evlist)
>         }
>  }
>
> -void iostat_print_header_prefix(struct perf_stat_config *config)
> +static void iio_iostat_print_header_prefix(struct perf_stat_config *config)
>  {
>         if (config->csv_output)
>                 fputs("port,", config->output);
> @@ -428,8 +428,8 @@ void iostat_print_header_prefix(struct perf_stat_config *config)
>                 fprintf(config->output, "   port         ");
>  }
>
> -void iostat_print_metric(struct perf_stat_config *config, struct evsel *evsel,
> -                        struct perf_stat_output_ctx *out)
> +static void iio_iostat_print_metric(struct perf_stat_config *config, struct evsel *evsel,
> +                                   struct perf_stat_output_ctx *out)
>  {
>         double iostat_value = 0;
>         u64 prev_count_val = 0;
> @@ -452,24 +452,54 @@ void iostat_print_metric(struct perf_stat_config *config, struct evsel *evsel,
>                           iostat_value / (256 * 1024));
>  }
>
> -void iostat_print_counters(struct evlist *evlist,
> -                          struct perf_stat_config *config, struct timespec *ts,
> -                          char *prefix, iostat_print_counter_t print_cnt_cb, void *arg)
> +static void iio_iostat_print_counters(struct evlist *evlist,
> +                                     struct perf_stat_config *config, struct timespec *ts,
> +                                     char *prefix, iostat_print_counter_t print_cnt_cb, void *arg)
>  {
>         void *perf_device = NULL;
>         struct evsel *counter = evlist__first(evlist);
>
>         evlist__set_selected(evlist, counter);
> -       iostat_prefix(evlist, config, prefix, ts);
> +       iio_iostat_prefix(evlist, config, prefix, ts);
>         fprintf(config->output, "%s", prefix);
>         evlist__for_each_entry(evlist, counter) {
>                 perf_device = evlist->selected->priv;
>                 if (perf_device && perf_device != counter->priv) {
>                         evlist__set_selected(evlist, counter);
> -                       iostat_prefix(evlist, config, prefix, ts);
> +                       iio_iostat_prefix(evlist, config, prefix, ts);
>                         fprintf(config->output, "\n%s", prefix);
>                 }
>                 print_cnt_cb(config, counter, arg);
>         }
>         fputc('\n', config->output);
>  }
> +
> +/*
> + * FIXME: pmu name prefix match might not work for x86 iio.
> + */
> +static bool iio_iostat_probe(struct iostat_pmu *iostat_pmu)
> +{
> +       return perf_pmus__scan_matching_wildcard(NULL, iostat_pmu->pmu_name_wildcard);

It feels more readable to me as:
```
  return perf_pmus__scan_matching_wildcard(NULL, "uncore_iio") != NULL;
```


> +}
> +
> +static struct iostat_pmu x86_iio_iostat_pmu_list[]  = {
> +       {
> +               .pmu_name_wildcard = "uncore_iio*",
> +               .match = iio_iostat_probe,
> +               .prepare = iio_iostat_prepare,
> +               .parse = iio_iostat_parse,
> +               .list = iio_iostat_list,
> +               .print_header_prefix = iio_iostat_print_header_prefix,
> +               .print_metric = iio_iostat_print_metric,

Is there scope to push the metric printing into json? For example, you
can have PMUs with json:
tools/perf/pmu-events/arch/x86/alderlake/adl-metrics.json
```
    {
       "BriefDescription": "Uncore frequency per die [GHZ]",
       "MetricExpr": "tma_info_system_socket_clks / #num_dies /
duration_time / 1e9",
       "MetricGroup": "SoC",
       "MetricName": "UNCORE_FREQ",
       "Unit": "cpu_core"
   },
```
and do uncore type things:
tools/perf/pmu-events/arch/x86/amdzen6/recommended.json
```
  {
   "MetricName": "umc_activate_cmd_rate",
   "BriefDescription": "Memory controller ACTIVATE command rate.",
   "MetricExpr": "d_ratio(umc_act_cmd.all * 1000, umc_mem_clk)",
   "MetricGroup": "memory_controller",
   "PerPkg": "1",
   "ScaleUnit": "1per_memclk"
 },
```

> +               .print_counters = iio_iostat_print_counters,
> +               .release = iio_iostat_release,
> +       },
> +};
> +
> +static void __attribute__((constructor)) x86_iio_iostat_pmu_init(void)

I don't believe these attributes are what we normally do.

Thanks,
Ian


> +{
> +       unsigned int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(x86_iio_iostat_pmu_list); i++)
> +               register_iostat_pmu(&x86_iio_iostat_pmu_list[i]);
> +}
> --
> 2.33.0
>

Re: [RFT PATCH v2 6/7] perf-iostat: Make x86 iostat compatible with new iostat framework

[Yushan Wang]

On 5/8/2026 12:17 AM, Ian Rogers wrote:
> On Wed, May 6, 2026 at 11:37 PM Yushan Wang <wangyushan12@huawei.com> wrote:
>>
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>> Change the original x86 iio iostat supporter to be compatible with the
>> set of iostat frameworks.
>>
>> The matching function of x86 iio may not be correct.
>>
>> Signed-off-by: Shiju Jose  <shiju.jose@huawei.com>
>> Co-developed-by: Yushan Wang <wangyushan12@huawei.com>
>> Signed-off-by: Yushan Wang <wangyushan12@huawei.com>
>> ---
>>  tools/perf/util/iostat.c     | 26 +++++++--------
>>  tools/perf/util/x86-iostat.c | 62 ++++++++++++++++++++++++++----------
>>  2 files changed, 59 insertions(+), 29 deletions(-)
>>
>> diff --git a/tools/perf/util/x86-iostat.c b/tools/perf/util/x86-iostat.c
>> index f8d4c9718594..778655cbc2a0 100644
>> --- a/tools/perf/util/x86-iostat.c
>> +++ b/tools/perf/util/x86-iostat.c
>> @@ -332,9 +332,9 @@ static int iostat_event_group(struct evlist *evl,
>>         return ret;
>>  }
>>

[...]

>> +
>> +/*
>> + * FIXME: pmu name prefix match might not work for x86 iio.
>> + */
>> +static bool iio_iostat_probe(struct iostat_pmu *iostat_pmu)
>> +{
>> +       return perf_pmus__scan_matching_wildcard(NULL, iostat_pmu->pmu_name_wildcard);
>
> It feels more readable to me as:
> ```
>   return perf_pmus__scan_matching_wildcard(NULL, "uncore_iio") != NULL;
> ```

Yes, I can do that refactor.

>
>
>> +}
>> +
>> +static struct iostat_pmu x86_iio_iostat_pmu_list[]  = {
>> +       {
>> +               .pmu_name_wildcard = "uncore_iio*",
>> +               .match = iio_iostat_probe,
>> +               .prepare = iio_iostat_prepare,
>> +               .parse = iio_iostat_parse,
>> +               .list = iio_iostat_list,
>> +               .print_header_prefix = iio_iostat_print_header_prefix,
>> +               .print_metric = iio_iostat_print_metric,
>
> Is there scope to push the metric printing into json? For example, you
> can have PMUs with json:
> tools/perf/pmu-events/arch/x86/alderlake/adl-metrics.json
> ```
>     {
>        "BriefDescription": "Uncore frequency per die [GHZ]",
>        "MetricExpr": "tma_info_system_socket_clks / #num_dies /
> duration_time / 1e9",
>        "MetricGroup": "SoC",
>        "MetricName": "UNCORE_FREQ",
>        "Unit": "cpu_core"
>    },
> ```
> and do uncore type things:
> tools/perf/pmu-events/arch/x86/amdzen6/recommended.json
> ```
>   {
>    "MetricName": "umc_activate_cmd_rate",
>    "BriefDescription": "Memory controller ACTIVATE command rate.",
>    "MetricExpr": "d_ratio(umc_act_cmd.all * 1000, umc_mem_clk)",
>    "MetricGroup": "memory_controller",
>    "PerPkg": "1",
>    "ScaleUnit": "1per_memclk"
>  },
> ```

Currently it's a bit far to reach json configurable PMU iostat structural
display, since the PMUs are vendor specific and may have different meaningful
combinations of events, and they may be hard to be handled with generalized
handler.

Taking currently availble uncore_iio* and hisi_pcie* pmu as example, they have
different sets of events to display, different ways of calculation, and
different PMU to PCIe root port mapping topology, which requires dedicated
handler.

But admittedly there are a lot of redundant code that can be optimized, I can
see if those iostat_* callbacks can be merged or generalized so that variated
functions can be minimized.

Thanks,
Yushan

>
>> +               .print_counters = iio_iostat_print_counters,
>> +               .release = iio_iostat_release,
>> +       },
>> +};
>> +
>> +static void __attribute__((constructor)) x86_iio_iostat_pmu_init(void)
>
> I don't believe these attributes are what we normally do.
>
> Thanks,
> Ian
>
>
>> +{
>> +       unsigned int i;
>> +
>> +       for (i = 0; i < ARRAY_SIZE(x86_iio_iostat_pmu_list); i++)
>> +               register_iostat_pmu(&x86_iio_iostat_pmu_list[i]);
>> +}
>> --
>> 2.33.0
>>

[RFT PATCH v2 7/7] perf-iostat: Enable iostat mode for HiSilicon PCIe PMU

[Yushan Wang]

From: Yicong Yang <yangyicong@hisilicon.com>

Some HiSilicon platforms provide PCIe PMU devices for monitoring the
throughput and latency of PCIe traffic. With the support of PCIe PMU
we can enable the perf iostat mode.

The HiSilicon PCIe PMU can support measuring the throughput of certain
TLP types and of certain root port. Totally 6 metrics are provided in
the unit of MB:

- Inbound MWR: Memory write TLPs from downstream devices to root port
- Inbound MRD: Memory read TLPs from downstream devices to root port
- Inbound CPL: Completion TLPs from downstream devices to root port
- Outbound MWR: Memory write TLPs from CPU to downstream devices
- Outbound MRD: Memory read TLPs from CPU to downstream devices
- Outbound CPL: Completions TLPs from CPU to downstream devices

Since the PMU measures the throughput in DWords, we need to calculate
the throughput in MB like:
  Count * 4B / 1024 / 1024

Some of the display of the `perf iostat` will be like:
[root@localhost tmp]# ./perf iostat list
hisi_pcie0_core2<0000:40:00.0>
hisi_pcie2_core2<0000:5f:00.0>
hisi_pcie0_core1<0000:16:00.0>
hisi_pcie0_core1<0000:16:04.0>
[root@localhost tmp]# ./perf iostat --timeout 10000

 Performance counter stats for 'system wide':

    port              Inbound MWR(MB)      Inbound MRD(MB)      Inbound CPL(MB)     Outbound MWR(MB)     Outbound MRD(MB)     Outbound CPL(MB)
0000:40:00.0                    0                    0                    0                    0                    0                    0
0000:5f:00.0                    0                    0                    0                    0                    0                    0
0000:16:00.0             16272.99               366.58                    0                15.09                    0             16156.85
0000:16:04.0                    0                    0                    0                    0                    0                    0

      10.008227512 seconds time elapsed

[root@localhost tmp]# ./perf iostat 0000:16:00.0 -- fio -name=rw -numjobs=30 -filename=/dev/nvme0n1 -rw=rw -iodepth=128 -direct=1 -sync=0 -norandommap -group_reporting -runtime=10 -time_based -bs=64k 2>&1 > /dev/null

 Performance counter stats for 'system wide':

    port              Inbound MWR(MB)      Inbound MRD(MB)      Inbound CPL(MB)     Outbound MWR(MB)     Outbound MRD(MB)     Outbound CPL(MB)
0000:16:00.0                16614                  379                    0                   16                    0                16721

      10.180349717 seconds time elapsed

       0.558810000 seconds user
       2.495016000 seconds sys

More information of the HiSilicon PCIe PMU can be found at
Documentation/admin-guide/perf/hisi-pcie-pmu.rst.

Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
Signed-off-by: Shiju Jose  <shiju.jose@huawei.com>
Signed-off-by: Yushan Wang <wangyushan12@huawei.com>
---
 tools/perf/util/Build         |   1 +
 tools/perf/util/hisi-iostat.c | 478 ++++++++++++++++++++++++++++++++++
 2 files changed, 479 insertions(+)
 create mode 100644 tools/perf/util/hisi-iostat.c

diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index c7e5ada3800d..fee9980cee77 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -128,6 +128,7 @@ perf-util-y += spark.o
 perf-util-y += topdown.o
 perf-util-y += iostat.o
 perf-util-y += x86-iostat.o
+perf-util-y += hisi-iostat.o
 perf-util-y += stream.o
 perf-util-$(CONFIG_LIBTRACEEVENT) += kvm-stat.o
 perf-util-y += kvm-stat-arch/
diff --git a/tools/perf/util/hisi-iostat.c b/tools/perf/util/hisi-iostat.c
new file mode 100644
index 000000000000..30041fd08614
--- /dev/null
+++ b/tools/perf/util/hisi-iostat.c
@@ -0,0 +1,478 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * perf iostat support for HiSilicon PCIe PMU.
+ * Partly derived from tools/perf/arch/x86/util/iostat.c.
+ *
+ * Copyright (c) 2024 HiSilicon Technologies Co., Ltd.
+ * Author: Yicong Yang <yangyicong@hisilicon.com>
+ * Author: Yushan Wang <wangyushan12@huawei.com>
+ */
+
+#include <linux/err.h>
+#include <linux/limits.h>
+#include <linux/zalloc.h>
+
+#include <api/fs/fs.h>
+#include <dirent.h>
+#include <errno.h>
+#include <stdio.h>
+
+#include "util/counts.h"
+#include "util/debug.h"
+#include "util/iostat.h"
+#include "util/pmu.h"
+
+/* From include/uapi/linux/pci.h */
+#define PCI_SLOT(devfn)		(((devfn) >> 3) & 0x1f)
+#define PCI_DEVFN(slot, func)	((((slot) & 0x1f) << 3) | ((func) & 0x07))
+
+#define PCI_DEVICE_NAME_PATTERN		"%04x:%02hhx:%02hhx.%hhu"
+#define PCI_ROOT_BUS_DEVICES_PATH	"bus/pci/devices"
+
+static const char * const hisi_iostat_metrics[] = {
+	"Inbound MWR(MB)",
+	"Inbound MRD(MB)",
+	"Inbound CPL(MB)",
+	"Outbound MWR(MB)",
+	"Outbound MRD(MB)",
+	"Outbound CPL(MB)",
+};
+
+static const char * const hisi_iostat_cmd_template[] = {
+	/* Inbound Memory Write */
+	"hisi_pcie%hu_core%hu/event=0x0104,port=0x%hx/",
+	/* Inbound Memory Read */
+	"hisi_pcie%hu_core%hu/event=0x0804,port=0x%hx/",
+	/* Inbound Memory Completion */
+	"hisi_pcie%hu_core%hu/event=0x2004,port=0x%hx/",
+	/* Outbound Memory Write */
+	"hisi_pcie%hu_core%hu/event=0x0105,port=0x%hx/",
+	/* Outbound Memory Read */
+	"hisi_pcie%hu_core%hu/event=0x0405,port=0x%hx/",
+	/* Outbound Memory Completion */
+	"hisi_pcie%hu_core%hu/event=0x1005,port=0x%hx/",
+};
+
+struct hisi_pcie_root_port {
+	struct list_head list;
+	/* Is this Root Port selected for monitoring */
+	bool selected;
+	/* IDs to locate the PMU */
+	u16 sicl_id;
+	u16 core_id;
+	/* Filter mask for this Root Port */
+	u16 mask;
+	/* PCIe Root Port's <domain>:<bus>:<device>.<function> */
+	u32 domain;
+	u8 bus;
+	u8 dev;
+	u8 fn;
+};
+
+static LIST_HEAD(hisi_pcie_root_ports_list);
+
+/*
+ * Select specific Root Port to monitor. Return 0 if successfully find the
+ * Root Port, Otherwise -EINVAL.
+ */
+static int hisi_pcie_root_ports_select_one(u32 domain, u8 bus, u8 dev, u8 fn)
+{
+	struct hisi_pcie_root_port *rp;
+
+	list_for_each_entry(rp, &hisi_pcie_root_ports_list, list)
+		if (domain == rp->domain && bus == rp->bus &&
+		    dev == rp->dev && fn == rp->fn) {
+			rp->selected = true;
+			return 0;
+		}
+
+	return -EINVAL;
+}
+
+static void hisi_pcie_root_ports_select_all(void)
+{
+	struct hisi_pcie_root_port *rp;
+
+	list_for_each_entry(rp, &hisi_pcie_root_ports_list, list)
+		rp->selected = true;
+}
+
+static void hisi_pcie_root_ports_add(u16 sicl_id, u16 core_id, u8 target_bus,
+				     u16 bdf_min, u16 bdf_max)
+{
+	const char *sysfs = sysfs__mountpoint();
+	struct hisi_pcie_root_port *rp;
+	unsigned long path_len;
+	struct dirent *dent;
+	char path[PATH_MAX];
+	u8 bus, dev, fn;
+	u32 domain;
+	DIR *dir;
+	u16 bdf;
+	int ret;
+
+	path_len = snprintf(path, PATH_MAX, "%s/%s", sysfs, PCI_ROOT_BUS_DEVICES_PATH);
+	if (path_len > PATH_MAX)
+		return;
+
+	dir = opendir(path);
+	if (!dir)
+		return;
+
+	/* Scan the PCI root bus to find the match root port on @target_bus */
+	while ((dent = readdir(dir))) {
+		ret = sscanf(dent->d_name, PCI_DEVICE_NAME_PATTERN,
+			     &domain, &bus, &dev, &fn);
+		if (ret != 4 || bus != target_bus)
+			continue;
+
+		bdf = (bus << 8) | PCI_DEVFN(dev, fn);
+		if (bdf < bdf_min || bdf > bdf_max)
+			continue;
+
+		rp = zalloc(sizeof(*rp));
+		if (!rp)
+			continue;
+
+		rp->selected = false;
+		rp->sicl_id = sicl_id;
+		rp->core_id = core_id;
+		rp->domain = domain;
+		rp->bus = bus;
+		rp->dev = dev;
+		rp->fn = fn;
+
+		rp->mask = BIT((rp->dev - PCI_SLOT(bdf_min)) << 1);
+
+		list_add(&rp->list, &hisi_pcie_root_ports_list);
+
+		pr_debug3("Found root port %s\n", dent->d_name);
+	}
+
+	closedir(dir);
+}
+
+/* Scan the PMUs and build the mapping of the Root Ports to the PMU */
+static int hisi_pcie_root_ports_init(void)
+{
+	char event_source[PATH_MAX], bus_path[PATH_MAX];
+	unsigned long long bus, bdf_max, bdf_min;
+	u16 sicl_id, core_id;
+	struct dirent *dent;
+	DIR *dir;
+
+	perf_pmu__event_source_devices_scnprintf(event_source, sizeof(event_source));
+	dir = opendir(event_source);
+	if (!dir)
+		return -ENOENT;
+
+	while ((dent = readdir(dir))) {
+		/*
+		 * This HiSilicon PCIe PMU will be named as:
+		 *   hisi_pcie<sicl_id>_core<core_id>
+		 */
+		if ((sscanf(dent->d_name, "hisi_pcie%hu_core%hu", &sicl_id, &core_id)) != 2)
+			continue;
+
+		/*
+		 * Driver will export the root port it can monitor through
+		 * the "bus" sysfs attribute.
+		 */
+		scnprintf(bus_path, sizeof(bus_path), "%s/hisi_pcie%hu_core%hu/bus",
+			  event_source, sicl_id, core_id);
+
+		/*
+		 * Per PCIe spec the bus should be 8bit, use unsigned long long
+		 * for the convenience of the library function.
+		 */
+		if (filename__read_ull(bus_path, &bus))
+			continue;
+
+		scnprintf(bus_path, sizeof(bus_path), "%s/hisi_pcie%hu_core%hu/bdf_max",
+			  event_source, sicl_id, core_id);
+		if (filename__read_xll(bus_path, &bdf_max))
+			bdf_max = -1;
+
+		scnprintf(bus_path, sizeof(bus_path), "%s/hisi_pcie%hu_core%hu/bdf_min",
+			  event_source, sicl_id, core_id);
+		if (filename__read_xll(bus_path, &bdf_min))
+			bdf_min = 0;
+
+		pr_debug3("Found pmu %s bus 0x%llx\n", dent->d_name, bus);
+
+		hisi_pcie_root_ports_add(sicl_id, core_id, (u8)bus, (u16)bdf_min, (u16)bdf_max);
+	}
+
+	closedir(dir);
+	return !list_empty(&hisi_pcie_root_ports_list) ? 0 : -ENOENT;
+}
+
+static void hisi_pcie_root_ports_free(void)
+{
+	struct hisi_pcie_root_port *rp, *tmp;
+
+	if (list_empty(&hisi_pcie_root_ports_list))
+		return;
+
+	list_for_each_entry_safe(rp, tmp, &hisi_pcie_root_ports_list, list) {
+		list_del(&rp->list);
+		zfree(&rp);
+	}
+}
+
+static int hisi_iostat_add_events(struct evlist *evl)
+{
+	struct hisi_pcie_root_port *rp;
+	struct evsel *evsel;
+	unsigned int i, j;
+	char *iostat_cmd;
+	int pos = 0;
+	int ret;
+
+	if (list_empty(&hisi_pcie_root_ports_list))
+		return -ENOENT;
+
+	iostat_cmd = zalloc(PATH_MAX);
+	if (!iostat_cmd)
+		return -ENOMEM;
+
+	list_for_each_entry(rp, &hisi_pcie_root_ports_list, list) {
+		if (!rp->selected)
+			continue;
+
+		iostat_cmd[pos++] = '{';
+		for (j = 0; j < ARRAY_SIZE(hisi_iostat_cmd_template); j++) {
+			pos += snprintf(iostat_cmd + pos, ARG_MAX - pos - 1,
+					hisi_iostat_cmd_template[j],
+					rp->sicl_id, rp->core_id, rp->mask);
+
+			if (j == ARRAY_SIZE(hisi_iostat_cmd_template) - 1)
+				iostat_cmd[pos++] = '}';
+			else
+				iostat_cmd[pos++] = ',';
+		}
+
+		ret = parse_event(evl, iostat_cmd);
+		if (ret)
+			break;
+
+		i = 0;
+		evlist__for_each_entry_reverse(evl, evsel) {
+			if (i == ARRAY_SIZE(hisi_iostat_cmd_template))
+				break;
+
+			evsel->priv = rp;
+			i++;
+		}
+
+		memset(iostat_cmd, 0, PATH_MAX);
+		pos = 0;
+	}
+
+	zfree(&iostat_cmd);
+	return ret;
+}
+
+static int hisi_iostat_prepare(struct evlist *evlist,
+			       struct perf_stat_config *config)
+{
+	if (evlist->core.nr_entries > 0) {
+		pr_warning("The -e and -M options are not supported."
+			   "All chosen events/metrics will be dropped\n");
+		evlist__delete(evlist);
+		evlist = evlist__new();
+		if (!evlist)
+			return -ENOMEM;
+	}
+
+	config->metric_only = true;
+	config->aggr_mode = AGGR_GLOBAL;
+
+	return hisi_iostat_add_events(evlist);
+}
+
+static int hisi_pcie_root_ports_list_filter(const char *str)
+{
+	char *tok, *tmp, *copy = NULL;
+	u8 bus, dev, fn;
+	u32 domain;
+	int ret;
+
+	copy = strdup(str);
+	if (!copy)
+		return -ENOMEM;
+
+	for (tok = strtok_r(copy, ",", &tmp); tok; tok = strtok_r(NULL, ",", &tmp)) {
+		ret = sscanf(tok, PCI_DEVICE_NAME_PATTERN, &domain, &bus, &dev, &fn);
+		if (ret != 4) {
+			ret = -EINVAL;
+			break;
+		}
+
+		ret = hisi_pcie_root_ports_select_one(domain, bus, dev, fn);
+		if (ret)
+			break;
+	}
+
+	zfree(&copy);
+	return ret;
+}
+
+static int hisi_iostat_parse(const struct option *opt, const char *str, int unset __maybe_unused)
+{
+	struct perf_stat_config *config = (struct perf_stat_config *)opt->data;
+	int ret;
+
+	ret = hisi_pcie_root_ports_init();
+	if (ret)
+		return ret;
+
+	config->iostat_run = true;
+
+	if (!str) {
+		iostat_mode = IOSTAT_RUN;
+		hisi_pcie_root_ports_select_all();
+	} else if (!strcmp(str, "list")) {
+		iostat_mode = IOSTAT_LIST;
+		hisi_pcie_root_ports_select_all();
+	} else {
+		iostat_mode = IOSTAT_RUN;
+		ret = hisi_pcie_root_ports_list_filter(str);
+	}
+
+	if (ret)
+		hisi_pcie_root_ports_free();
+
+	return ret;
+}
+
+static void hisi_pcie_root_port_show(FILE *output,
+				     const struct hisi_pcie_root_port * const rp)
+{
+	if (output && rp)
+		fprintf(output, "hisi_pcie%hu_core%hu<" PCI_DEVICE_NAME_PATTERN ">\n",
+			rp->sicl_id, rp->core_id, rp->domain, rp->bus, rp->dev, rp->fn);
+}
+
+static void hisi_iostat_list(struct evlist *evlist __maybe_unused, struct perf_stat_config *config)
+{
+	struct hisi_pcie_root_port *rp = NULL;
+	struct evsel *evsel;
+
+	evlist__for_each_entry(evlist, evsel) {
+		if (rp != evsel->priv) {
+			hisi_pcie_root_port_show(config->output, evsel->priv);
+			rp = evsel->priv;
+		}
+	}
+}
+
+static void hisi_iostat_release(struct evlist *evlist)
+{
+	struct evsel *evsel;
+
+	evlist__for_each_entry(evlist, evsel)
+		evsel->priv = NULL;
+
+	hisi_pcie_root_ports_free();
+}
+
+static void hisi_iostat_print_header_prefix(struct perf_stat_config *config)
+{
+	if (config->csv_output)
+		fputs("port,", config->output);
+	else if (config->interval)
+		fprintf(config->output, "#          time    port         ");
+	else
+		fprintf(config->output, "   port         ");
+}
+
+static void hisi_iostat_print_metric(struct perf_stat_config *config, struct evsel *evsel,
+				     struct perf_stat_output_ctx *out)
+{
+	const char *iostat_metric = hisi_iostat_metrics[evsel->core.idx % ARRAY_SIZE(hisi_iostat_metrics)];
+	struct perf_counts_values *count;
+	double iostat_value;
+
+	/* We're using AGGR_GLOBAL so there's only one aggr counts aggr[0]. */
+	count = &evsel->stats->aggr[0].counts;
+
+	/* The counts has been scaled, we can use it directly. */
+	iostat_value = (double)count->val;
+
+	/*
+	 * Display two digits after decimal point for better accuracy if the
+	 * value is non-zero.
+	 */
+	out->print_metric(config, out->ctx, METRIC_THRESHOLD_UNKNOWN,
+			  iostat_value > 0 ? "%8.2f" : "%8.0f",
+			  iostat_metric, iostat_value / (256 * 1024));
+}
+
+static void hisi_iostat_prefix(struct evlist *evlist, struct perf_stat_config *config,
+			       char *prefix, struct timespec *ts)
+{
+	struct hisi_pcie_root_port *rp = evlist->selected->priv;
+
+	if (!rp)
+		return;
+
+	if (ts)
+		sprintf(prefix, "%6lu.%09lu%s" PCI_DEVICE_NAME_PATTERN "%s",
+			ts->tv_sec, ts->tv_nsec, config->csv_sep,
+			rp->domain, rp->bus, rp->dev, rp->fn,
+			config->csv_sep);
+	else
+		sprintf(prefix, PCI_DEVICE_NAME_PATTERN "%s",
+			rp->domain, rp->bus, rp->dev, rp->fn,
+			config->csv_sep);
+}
+
+static void hisi_iostat_print_counters(struct evlist *evlist, struct perf_stat_config *config,
+				       struct timespec *ts, char *prefix,
+				       iostat_print_counter_t print_cnt_cb, void *arg)
+{
+	struct evsel *counter = evlist__first(evlist);
+	void *perf_device;
+
+	evlist__set_selected(evlist, counter);
+	hisi_iostat_prefix(evlist, config, prefix, ts);
+	fprintf(config->output, "%s", prefix);
+	evlist__for_each_entry(evlist, counter) {
+		perf_device = evlist->selected->priv;
+		if (perf_device && perf_device != counter->priv) {
+			evlist__set_selected(evlist, counter);
+			hisi_iostat_prefix(evlist, config, prefix, ts);
+			fprintf(config->output, "\n%s", prefix);
+		}
+		print_cnt_cb(config, counter, arg);
+	}
+	fputc('\n', config->output);
+}
+
+static bool hisi_iostat_match(struct iostat_pmu *iostat_pmu)
+{
+	return perf_pmus__scan_matching_wildcard(NULL, iostat_pmu->pmu_name_wildcard);
+}
+
+static struct iostat_pmu hisi_iostat_pmu_list[] = {
+	{
+		.pmu_name_wildcard = "hisi_pcie*",
+		.match = hisi_iostat_match,
+		.prepare = hisi_iostat_prepare,
+		.parse = hisi_iostat_parse,
+		.list = hisi_iostat_list,
+		.print_header_prefix = hisi_iostat_print_header_prefix,
+		.print_metric = hisi_iostat_print_metric,
+		.print_counters = hisi_iostat_print_counters,
+		.release = hisi_iostat_release,
+	},
+};
+
+static void __attribute__((constructor)) hisi_iostat_pmu_init(void)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(hisi_iostat_pmu_list); i++)
+		register_iostat_pmu(&hisi_iostat_pmu_list[i]);
+}
-- 
2.33.0

Re: [RFT PATCH v2 7/7] perf-iostat: Enable iostat mode for HiSilicon PCIe PMU

[Ian Rogers]

On Wed, May 6, 2026 at 11:37 PM Yushan Wang <wangyushan12@huawei.com> wrote:
>
> From: Yicong Yang <yangyicong@hisilicon.com>
>
> Some HiSilicon platforms provide PCIe PMU devices for monitoring the
> throughput and latency of PCIe traffic. With the support of PCIe PMU
> we can enable the perf iostat mode.
>
> The HiSilicon PCIe PMU can support measuring the throughput of certain
> TLP types and of certain root port. Totally 6 metrics are provided in
> the unit of MB:
>
> - Inbound MWR: Memory write TLPs from downstream devices to root port
> - Inbound MRD: Memory read TLPs from downstream devices to root port
> - Inbound CPL: Completion TLPs from downstream devices to root port
> - Outbound MWR: Memory write TLPs from CPU to downstream devices
> - Outbound MRD: Memory read TLPs from CPU to downstream devices
> - Outbound CPL: Completions TLPs from CPU to downstream devices
>
> Since the PMU measures the throughput in DWords, we need to calculate
> the throughput in MB like:
>   Count * 4B / 1024 / 1024
>
> Some of the display of the `perf iostat` will be like:
> [root@localhost tmp]# ./perf iostat list
> hisi_pcie0_core2<0000:40:00.0>
> hisi_pcie2_core2<0000:5f:00.0>
> hisi_pcie0_core1<0000:16:00.0>
> hisi_pcie0_core1<0000:16:04.0>
> [root@localhost tmp]# ./perf iostat --timeout 10000
>
>  Performance counter stats for 'system wide':
>
>     port              Inbound MWR(MB)      Inbound MRD(MB)      Inbound CPL(MB)     Outbound MWR(MB)     Outbound MRD(MB)     Outbound CPL(MB)
> 0000:40:00.0                    0                    0                    0                    0                    0                    0
> 0000:5f:00.0                    0                    0                    0                    0                    0                    0
> 0000:16:00.0             16272.99               366.58                    0                15.09                    0             16156.85
> 0000:16:04.0                    0                    0                    0                    0                    0                    0
>
>       10.008227512 seconds time elapsed
>
> [root@localhost tmp]# ./perf iostat 0000:16:00.0 -- fio -name=rw -numjobs=30 -filename=/dev/nvme0n1 -rw=rw -iodepth=128 -direct=1 -sync=0 -norandommap -group_reporting -runtime=10 -time_based -bs=64k 2>&1 > /dev/null
>
>  Performance counter stats for 'system wide':
>
>     port              Inbound MWR(MB)      Inbound MRD(MB)      Inbound CPL(MB)     Outbound MWR(MB)     Outbound MRD(MB)     Outbound CPL(MB)
> 0000:16:00.0                16614                  379                    0                   16                    0                16721
>
>       10.180349717 seconds time elapsed
>
>        0.558810000 seconds user
>        2.495016000 seconds sys
>
> More information of the HiSilicon PCIe PMU can be found at
> Documentation/admin-guide/perf/hisi-pcie-pmu.rst.
>
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> Signed-off-by: Shiju Jose  <shiju.jose@huawei.com>
> Signed-off-by: Yushan Wang <wangyushan12@huawei.com>
> ---
>  tools/perf/util/Build         |   1 +
>  tools/perf/util/hisi-iostat.c | 478 ++++++++++++++++++++++++++++++++++
>  2 files changed, 479 insertions(+)
>  create mode 100644 tools/perf/util/hisi-iostat.c
>
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index c7e5ada3800d..fee9980cee77 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -128,6 +128,7 @@ perf-util-y += spark.o
>  perf-util-y += topdown.o
>  perf-util-y += iostat.o
>  perf-util-y += x86-iostat.o
> +perf-util-y += hisi-iostat.o
>  perf-util-y += stream.o
>  perf-util-$(CONFIG_LIBTRACEEVENT) += kvm-stat.o
>  perf-util-y += kvm-stat-arch/
> diff --git a/tools/perf/util/hisi-iostat.c b/tools/perf/util/hisi-iostat.c
> new file mode 100644
> index 000000000000..30041fd08614
> --- /dev/null
> +++ b/tools/perf/util/hisi-iostat.c
> @@ -0,0 +1,478 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * perf iostat support for HiSilicon PCIe PMU.
> + * Partly derived from tools/perf/arch/x86/util/iostat.c.
> + *
> + * Copyright (c) 2024 HiSilicon Technologies Co., Ltd.
> + * Author: Yicong Yang <yangyicong@hisilicon.com>
> + * Author: Yushan Wang <wangyushan12@huawei.com>
> + */
> +
> +#include <linux/err.h>
> +#include <linux/limits.h>
> +#include <linux/zalloc.h>
> +
> +#include <api/fs/fs.h>
> +#include <dirent.h>
> +#include <errno.h>
> +#include <stdio.h>
> +
> +#include "util/counts.h"
> +#include "util/debug.h"
> +#include "util/iostat.h"
> +#include "util/pmu.h"
> +
> +/* From include/uapi/linux/pci.h */
> +#define PCI_SLOT(devfn)                (((devfn) >> 3) & 0x1f)
> +#define PCI_DEVFN(slot, func)  ((((slot) & 0x1f) << 3) | ((func) & 0x07))
> +
> +#define PCI_DEVICE_NAME_PATTERN                "%04x:%02hhx:%02hhx.%hhu"
> +#define PCI_ROOT_BUS_DEVICES_PATH      "bus/pci/devices"
> +
> +static const char * const hisi_iostat_metrics[] = {
> +       "Inbound MWR(MB)",
> +       "Inbound MRD(MB)",
> +       "Inbound CPL(MB)",
> +       "Outbound MWR(MB)",
> +       "Outbound MRD(MB)",
> +       "Outbound CPL(MB)",
> +};
> +
> +static const char * const hisi_iostat_cmd_template[] = {
> +       /* Inbound Memory Write */
> +       "hisi_pcie%hu_core%hu/event=0x0104,port=0x%hx/",
> +       /* Inbound Memory Read */
> +       "hisi_pcie%hu_core%hu/event=0x0804,port=0x%hx/",
> +       /* Inbound Memory Completion */
> +       "hisi_pcie%hu_core%hu/event=0x2004,port=0x%hx/",
> +       /* Outbound Memory Write */
> +       "hisi_pcie%hu_core%hu/event=0x0105,port=0x%hx/",
> +       /* Outbound Memory Read */
> +       "hisi_pcie%hu_core%hu/event=0x0405,port=0x%hx/",
> +       /* Outbound Memory Completion */
> +       "hisi_pcie%hu_core%hu/event=0x1005,port=0x%hx/",
> +};
> +
> +struct hisi_pcie_root_port {
> +       struct list_head list;
> +       /* Is this Root Port selected for monitoring */
> +       bool selected;
> +       /* IDs to locate the PMU */
> +       u16 sicl_id;
> +       u16 core_id;
> +       /* Filter mask for this Root Port */
> +       u16 mask;
> +       /* PCIe Root Port's <domain>:<bus>:<device>.<function> */
> +       u32 domain;
> +       u8 bus;
> +       u8 dev;
> +       u8 fn;
> +};
> +
> +static LIST_HEAD(hisi_pcie_root_ports_list);
> +
> +/*
> + * Select specific Root Port to monitor. Return 0 if successfully find the
> + * Root Port, Otherwise -EINVAL.
> + */
> +static int hisi_pcie_root_ports_select_one(u32 domain, u8 bus, u8 dev, u8 fn)
> +{
> +       struct hisi_pcie_root_port *rp;
> +
> +       list_for_each_entry(rp, &hisi_pcie_root_ports_list, list)
> +               if (domain == rp->domain && bus == rp->bus &&
> +                   dev == rp->dev && fn == rp->fn) {
> +                       rp->selected = true;
> +                       return 0;
> +               }
> +
> +       return -EINVAL;
> +}
> +
> +static void hisi_pcie_root_ports_select_all(void)
> +{
> +       struct hisi_pcie_root_port *rp;
> +
> +       list_for_each_entry(rp, &hisi_pcie_root_ports_list, list)
> +               rp->selected = true;
> +}
> +
> +static void hisi_pcie_root_ports_add(u16 sicl_id, u16 core_id, u8 target_bus,
> +                                    u16 bdf_min, u16 bdf_max)
> +{
> +       const char *sysfs = sysfs__mountpoint();
> +       struct hisi_pcie_root_port *rp;
> +       unsigned long path_len;
> +       struct dirent *dent;
> +       char path[PATH_MAX];
> +       u8 bus, dev, fn;
> +       u32 domain;
> +       DIR *dir;
> +       u16 bdf;
> +       int ret;
> +
> +       path_len = snprintf(path, PATH_MAX, "%s/%s", sysfs, PCI_ROOT_BUS_DEVICES_PATH);
> +       if (path_len > PATH_MAX)
> +               return;
> +
> +       dir = opendir(path);
> +       if (!dir)
> +               return;
> +
> +       /* Scan the PCI root bus to find the match root port on @target_bus */
> +       while ((dent = readdir(dir))) {
> +               ret = sscanf(dent->d_name, PCI_DEVICE_NAME_PATTERN,
> +                            &domain, &bus, &dev, &fn);
> +               if (ret != 4 || bus != target_bus)
> +                       continue;
> +
> +               bdf = (bus << 8) | PCI_DEVFN(dev, fn);
> +               if (bdf < bdf_min || bdf > bdf_max)
> +                       continue;
> +
> +               rp = zalloc(sizeof(*rp));
> +               if (!rp)
> +                       continue;
> +
> +               rp->selected = false;
> +               rp->sicl_id = sicl_id;
> +               rp->core_id = core_id;
> +               rp->domain = domain;
> +               rp->bus = bus;
> +               rp->dev = dev;
> +               rp->fn = fn;
> +
> +               rp->mask = BIT((rp->dev - PCI_SLOT(bdf_min)) << 1);
> +
> +               list_add(&rp->list, &hisi_pcie_root_ports_list);
> +
> +               pr_debug3("Found root port %s\n", dent->d_name);
> +       }
> +
> +       closedir(dir);
> +}
> +
> +/* Scan the PMUs and build the mapping of the Root Ports to the PMU */
> +static int hisi_pcie_root_ports_init(void)
> +{
> +       char event_source[PATH_MAX], bus_path[PATH_MAX];
> +       unsigned long long bus, bdf_max, bdf_min;
> +       u16 sicl_id, core_id;
> +       struct dirent *dent;
> +       DIR *dir;
> +
> +       perf_pmu__event_source_devices_scnprintf(event_source, sizeof(event_source));
> +       dir = opendir(event_source);
> +       if (!dir)
> +               return -ENOENT;
> +
> +       while ((dent = readdir(dir))) {
> +               /*
> +                * This HiSilicon PCIe PMU will be named as:
> +                *   hisi_pcie<sicl_id>_core<core_id>
> +                */
> +               if ((sscanf(dent->d_name, "hisi_pcie%hu_core%hu", &sicl_id, &core_id)) != 2)
> +                       continue;
> +
> +               /*
> +                * Driver will export the root port it can monitor through
> +                * the "bus" sysfs attribute.
> +                */
> +               scnprintf(bus_path, sizeof(bus_path), "%s/hisi_pcie%hu_core%hu/bus",
> +                         event_source, sicl_id, core_id);
> +
> +               /*
> +                * Per PCIe spec the bus should be 8bit, use unsigned long long
> +                * for the convenience of the library function.
> +                */
> +               if (filename__read_ull(bus_path, &bus))
> +                       continue;
> +
> +               scnprintf(bus_path, sizeof(bus_path), "%s/hisi_pcie%hu_core%hu/bdf_max",
> +                         event_source, sicl_id, core_id);
> +               if (filename__read_xll(bus_path, &bdf_max))
> +                       bdf_max = -1;
> +
> +               scnprintf(bus_path, sizeof(bus_path), "%s/hisi_pcie%hu_core%hu/bdf_min",
> +                         event_source, sicl_id, core_id);
> +               if (filename__read_xll(bus_path, &bdf_min))
> +                       bdf_min = 0;
> +
> +               pr_debug3("Found pmu %s bus 0x%llx\n", dent->d_name, bus);
> +
> +               hisi_pcie_root_ports_add(sicl_id, core_id, (u8)bus, (u16)bdf_min, (u16)bdf_max);
> +       }
> +
> +       closedir(dir);
> +       return !list_empty(&hisi_pcie_root_ports_list) ? 0 : -ENOENT;
> +}
> +
> +static void hisi_pcie_root_ports_free(void)
> +{
> +       struct hisi_pcie_root_port *rp, *tmp;
> +
> +       if (list_empty(&hisi_pcie_root_ports_list))
> +               return;
> +
> +       list_for_each_entry_safe(rp, tmp, &hisi_pcie_root_ports_list, list) {
> +               list_del(&rp->list);
> +               zfree(&rp);
> +       }
> +}
> +
> +static int hisi_iostat_add_events(struct evlist *evl)
> +{
> +       struct hisi_pcie_root_port *rp;
> +       struct evsel *evsel;
> +       unsigned int i, j;
> +       char *iostat_cmd;
> +       int pos = 0;
> +       int ret;
> +
> +       if (list_empty(&hisi_pcie_root_ports_list))
> +               return -ENOENT;
> +
> +       iostat_cmd = zalloc(PATH_MAX);
> +       if (!iostat_cmd)
> +               return -ENOMEM;
> +
> +       list_for_each_entry(rp, &hisi_pcie_root_ports_list, list) {
> +               if (!rp->selected)
> +                       continue;
> +
> +               iostat_cmd[pos++] = '{';
> +               for (j = 0; j < ARRAY_SIZE(hisi_iostat_cmd_template); j++) {
> +                       pos += snprintf(iostat_cmd + pos, ARG_MAX - pos - 1,
> +                                       hisi_iostat_cmd_template[j],
> +                                       rp->sicl_id, rp->core_id, rp->mask);
> +
> +                       if (j == ARRAY_SIZE(hisi_iostat_cmd_template) - 1)
> +                               iostat_cmd[pos++] = '}';
> +                       else
> +                               iostat_cmd[pos++] = ',';
> +               }
> +
> +               ret = parse_event(evl, iostat_cmd);
> +               if (ret)
> +                       break;
> +
> +               i = 0;
> +               evlist__for_each_entry_reverse(evl, evsel) {
> +                       if (i == ARRAY_SIZE(hisi_iostat_cmd_template))
> +                               break;
> +
> +                       evsel->priv = rp;
> +                       i++;
> +               }
> +
> +               memset(iostat_cmd, 0, PATH_MAX);
> +               pos = 0;
> +       }
> +
> +       zfree(&iostat_cmd);
> +       return ret;
> +}
> +
> +static int hisi_iostat_prepare(struct evlist *evlist,
> +                              struct perf_stat_config *config)
> +{
> +       if (evlist->core.nr_entries > 0) {
> +               pr_warning("The -e and -M options are not supported."
> +                          "All chosen events/metrics will be dropped\n");
> +               evlist__delete(evlist);
> +               evlist = evlist__new();
> +               if (!evlist)
> +                       return -ENOMEM;
> +       }
> +
> +       config->metric_only = true;
> +       config->aggr_mode = AGGR_GLOBAL;
> +
> +       return hisi_iostat_add_events(evlist);
> +}
> +
> +static int hisi_pcie_root_ports_list_filter(const char *str)
> +{
> +       char *tok, *tmp, *copy = NULL;
> +       u8 bus, dev, fn;
> +       u32 domain;
> +       int ret;
> +
> +       copy = strdup(str);
> +       if (!copy)
> +               return -ENOMEM;
> +
> +       for (tok = strtok_r(copy, ",", &tmp); tok; tok = strtok_r(NULL, ",", &tmp)) {
> +               ret = sscanf(tok, PCI_DEVICE_NAME_PATTERN, &domain, &bus, &dev, &fn);
> +               if (ret != 4) {
> +                       ret = -EINVAL;
> +                       break;
> +               }
> +
> +               ret = hisi_pcie_root_ports_select_one(domain, bus, dev, fn);
> +               if (ret)
> +                       break;
> +       }
> +
> +       zfree(&copy);
> +       return ret;
> +}
> +
> +static int hisi_iostat_parse(const struct option *opt, const char *str, int unset __maybe_unused)
> +{
> +       struct perf_stat_config *config = (struct perf_stat_config *)opt->data;
> +       int ret;
> +
> +       ret = hisi_pcie_root_ports_init();
> +       if (ret)
> +               return ret;
> +
> +       config->iostat_run = true;
> +
> +       if (!str) {
> +               iostat_mode = IOSTAT_RUN;
> +               hisi_pcie_root_ports_select_all();
> +       } else if (!strcmp(str, "list")) {
> +               iostat_mode = IOSTAT_LIST;
> +               hisi_pcie_root_ports_select_all();
> +       } else {
> +               iostat_mode = IOSTAT_RUN;
> +               ret = hisi_pcie_root_ports_list_filter(str);
> +       }
> +
> +       if (ret)
> +               hisi_pcie_root_ports_free();
> +
> +       return ret;
> +}
> +
> +static void hisi_pcie_root_port_show(FILE *output,
> +                                    const struct hisi_pcie_root_port * const rp)
> +{
> +       if (output && rp)
> +               fprintf(output, "hisi_pcie%hu_core%hu<" PCI_DEVICE_NAME_PATTERN ">\n",
> +                       rp->sicl_id, rp->core_id, rp->domain, rp->bus, rp->dev, rp->fn);
> +}
> +
> +static void hisi_iostat_list(struct evlist *evlist __maybe_unused, struct perf_stat_config *config)
> +{
> +       struct hisi_pcie_root_port *rp = NULL;
> +       struct evsel *evsel;
> +
> +       evlist__for_each_entry(evlist, evsel) {
> +               if (rp != evsel->priv) {
> +                       hisi_pcie_root_port_show(config->output, evsel->priv);
> +                       rp = evsel->priv;
> +               }
> +       }
> +}
> +
> +static void hisi_iostat_release(struct evlist *evlist)
> +{
> +       struct evsel *evsel;
> +
> +       evlist__for_each_entry(evlist, evsel)
> +               evsel->priv = NULL;
> +
> +       hisi_pcie_root_ports_free();
> +}
> +
> +static void hisi_iostat_print_header_prefix(struct perf_stat_config *config)
> +{
> +       if (config->csv_output)
> +               fputs("port,", config->output);
> +       else if (config->interval)
> +               fprintf(config->output, "#          time    port         ");
> +       else
> +               fprintf(config->output, "   port         ");
> +}
> +
> +static void hisi_iostat_print_metric(struct perf_stat_config *config, struct evsel *evsel,
> +                                    struct perf_stat_output_ctx *out)
> +{
> +       const char *iostat_metric = hisi_iostat_metrics[evsel->core.idx % ARRAY_SIZE(hisi_iostat_metrics)];
> +       struct perf_counts_values *count;
> +       double iostat_value;
> +
> +       /* We're using AGGR_GLOBAL so there's only one aggr counts aggr[0]. */
> +       count = &evsel->stats->aggr[0].counts;
> +
> +       /* The counts has been scaled, we can use it directly. */
> +       iostat_value = (double)count->val;
> +
> +       /*
> +        * Display two digits after decimal point for better accuracy if the
> +        * value is non-zero.
> +        */
> +       out->print_metric(config, out->ctx, METRIC_THRESHOLD_UNKNOWN,
> +                         iostat_value > 0 ? "%8.2f" : "%8.0f",
> +                         iostat_metric, iostat_value / (256 * 1024));
> +}
> +
> +static void hisi_iostat_prefix(struct evlist *evlist, struct perf_stat_config *config,
> +                              char *prefix, struct timespec *ts)
> +{
> +       struct hisi_pcie_root_port *rp = evlist->selected->priv;
> +
> +       if (!rp)
> +               return;
> +
> +       if (ts)
> +               sprintf(prefix, "%6lu.%09lu%s" PCI_DEVICE_NAME_PATTERN "%s",
> +                       ts->tv_sec, ts->tv_nsec, config->csv_sep,
> +                       rp->domain, rp->bus, rp->dev, rp->fn,
> +                       config->csv_sep);
> +       else
> +               sprintf(prefix, PCI_DEVICE_NAME_PATTERN "%s",
> +                       rp->domain, rp->bus, rp->dev, rp->fn,
> +                       config->csv_sep);
> +}
> +
> +static void hisi_iostat_print_counters(struct evlist *evlist, struct perf_stat_config *config,
> +                                      struct timespec *ts, char *prefix,
> +                                      iostat_print_counter_t print_cnt_cb, void *arg)
> +{
> +       struct evsel *counter = evlist__first(evlist);
> +       void *perf_device;
> +
> +       evlist__set_selected(evlist, counter);
> +       hisi_iostat_prefix(evlist, config, prefix, ts);
> +       fprintf(config->output, "%s", prefix);
> +       evlist__for_each_entry(evlist, counter) {
> +               perf_device = evlist->selected->priv;
> +               if (perf_device && perf_device != counter->priv) {
> +                       evlist__set_selected(evlist, counter);
> +                       hisi_iostat_prefix(evlist, config, prefix, ts);
> +                       fprintf(config->output, "\n%s", prefix);
> +               }
> +               print_cnt_cb(config, counter, arg);
> +       }
> +       fputc('\n', config->output);
> +}
> +
> +static bool hisi_iostat_match(struct iostat_pmu *iostat_pmu)
> +{
> +       return perf_pmus__scan_matching_wildcard(NULL, iostat_pmu->pmu_name_wildcard);
> +}
> +
> +static struct iostat_pmu hisi_iostat_pmu_list[] = {
> +       {
> +               .pmu_name_wildcard = "hisi_pcie*",
> +               .match = hisi_iostat_match,
> +               .prepare = hisi_iostat_prepare,
> +               .parse = hisi_iostat_parse,
> +               .list = hisi_iostat_list,
> +               .print_header_prefix = hisi_iostat_print_header_prefix,
> +               .print_metric = hisi_iostat_print_metric,
> +               .print_counters = hisi_iostat_print_counters,
> +               .release = hisi_iostat_release,
> +       },
> +};
> +
> +static void __attribute__((constructor)) hisi_iostat_pmu_init(void)

This looks good. My concerns are over:
 - the use of the constructor attribute,
 - the naming of iostat_pmu,
 - we probably don't want a per PMU C file in tools/perf/util as there
could be many,
 - we might be able to move things into json and better share code, etc.

Thanks,
Ian

> +{
> +       unsigned int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(hisi_iostat_pmu_list); i++)
> +               register_iostat_pmu(&hisi_iostat_pmu_list[i]);
> +}
> --
> 2.33.0
>

Re: [RFT PATCH v2 7/7] perf-iostat: Enable iostat mode for HiSilicon PCIe PMU

[Yushan Wang]

On 5/8/2026 12:20 AM, Ian Rogers wrote:
> On Wed, May 6, 2026 at 11:37 PM Yushan Wang <wangyushan12@huawei.com> wrote:

[...]

>> +
>> +static void __attribute__((constructor)) hisi_iostat_pmu_init(void)
>
> This looks good. My concerns are over:
>  - the use of the constructor attribute,
>  - the naming of iostat_pmu,
>  - we probably don't want a per PMU C file in tools/perf/util as there
> could be many,
>  - we might be able to move things into json and better share code, etc.
>
> Thanks,
> Ian

Hi Ian, thanks for the fast comments and reviewed-by tags, and sorry for the
delay of v2 patches.

I will try to merge those *_iostat.c into one file to avoid redundancy,
currently replacing iostat with json configs as a whole may be a huge work, but
eliminating redundant codes may not.

I think iostat can start by supporting variant platforms by providing structs
and callbacks, and migrate to totally json configurable for convenience of
extensibility in the future. After all, json metrics are firstly converted to a
big static string before being compiled into binary, and rebuild is necessary to
make any change of metric effective :)

Thanks,
Yushan

>
>> +{
>> +       unsigned int i;
>> +
>> +       for (i = 0; i < ARRAY_SIZE(hisi_iostat_pmu_list); i++)
>> +               register_iostat_pmu(&hisi_iostat_pmu_list[i]);
>> +}
>> --
>> 2.33.0
>>