[PATCH V3 0/6] tools/perf: Add powerpc HTM auxtrace support

[PATCH V3 0/6] tools/perf: Add powerpc HTM auxtrace support

[Athira Rajeev]

Overview:
This series adds perf tool support for capturing and processing Hardware
Trace Macro (HTM) trace data on POWER systems.  HTM exposes raw bus-trace
data via the H_HTM hypervisor call.  The kernel-side HTM PMU driver
(submitted separately) streams this data into perf AUX buffers and emits
memory configuration as PERF_SAMPLE_RAW records.

This series wires up the record and report sides in the perf tool so that
'perf record' collects HTM data and 'perf report' writes the trace and
memory configuration to files ready for post-processing with htmdecode.

Patch overview:
Patch 1 refactors the existing powerpc auxtrace dispatch.  All VPA-DTL
recording logic is moved from arch/powerpc/util/auxtrace.c into its own
file, arch/powerpc/util/vpa-dtl.c, leaving auxtrace_record__init() as a
thin per-PMU dispatcher.  This is a prerequisite for adding the HTM
recording path cleanly.

Patch 2 adds the HTM recording path (arch/powerpc/util/htm.c and
util/powerpc-htm.h).  htm_recording_init() sets up the auxtrace_record
callbacks.  htm_info_fill() stores the PMU type and a (cpu, attr.config)
pair for each htm evsel into the PERF_RECORD_AUXTRACE_INFO priv[] area.
The decode side reads these back to map each AUX buffer to its
(node, chip, core) target using event->auxtrace.cpu.  PERF_SAMPLE_RAW is
also enabled here so that memory configuration records are captured
alongside the AUX stream.

Patch 3 adds a generic weak arch_perf_record__need_read() hook in
builtin-record.c.  When the hook returns non-zero, perf record performs
an extra mmap-read pass before disabling events.  The drain loop runs
until the hook returns 0 or no forward progress is made, then the events
are disabled and closed normally.

Patch 4 implements arch_perf_record__need_read() for powerpc.  It reads
event->count for all open htm evsels via perf_evsel__read() and
accumulates the values.  The kernel driver sets this count to the number
of 128-byte HTM trace records written on a successful dump, 1 when the
AUX buffer is temporarily full but the hypervisor stream is intact, and
0 once the stream is exhausted or a hard error occurs.  A non-zero total
causes the recording loop to perform another read pass.

Patch 5 adds PERF_AUXTRACE_POWERPC_HTM to the auxtrace_type enum and
wires the dispatch in perf_event__process_auxtrace_info().  It also
amends htm_info_fill() to set auxtrace_info->type now that the constant
is available.

Patch 6 adds the full HTM decode path in util/powerpc-htm.c.
powerpc_htm_process_auxtrace_info() reads the (cpu, config) pairs from
priv[] and builds a cpu_configs[] lookup table.  As each
PERF_RECORD_AUXTRACE event arrives, process_auxtrace_event() maps it to
the correct target and writes the data to htm.bin.nX.pX.cX immediately.
PERF_RECORD_SAMPLE RAW records are written to translation.nX.pX.cX by
process_event().  Per-run first-write tracking (htm_target_seen) ensures
O_TRUNC on the first write and O_APPEND on subsequent writes.

Patches not included in V2:
Three patches present in V1 are intentionally dropped from this series
and will be submitted as a follow-on once the core infrastructure gets
integrated:

  - htmdecode integration: V1 patch 6 invoked the external htmdecode
    tool via fork/exec after writing htm.bin.* to decode the raw
    bus-trace data in-process.  This is deferred because the interface
    between perf and an external decode tool needs more discussion.

  - Physical-to-logical address mapping: V1 patch 7 read the current
    partition ID from /proc/powerpc/lparcfg, parsed the translation.*
    memory map entries, and mapped physical addresses in the decoded
    trace to logical addresses within the LPAR.

  - Synthetic sample generation: V1 patches 8 and 9 created a
    PERF_TYPE_SYNTH event named "htm", injected synthetic perf samples
    with the translated logical addresses as sample IP, and wrote a
    separate .out.l file with logical addresses alongside the decoded
    trace.  The appropriate abstraction for synthetic HTM samples in the
    perf tool requires further discussion.

Usage example:
Collect HTM trace data from a single chip target:

  # perf record -C 9 -m,256 \
      -e htm/nodalchipindex=2,nodeindex=0,htm_type=1/ sleep 3
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 256.277 MB perf.data ]

  # perf report

Output files written by perf report:

  htm.bin.n0.p2.c0       raw HTM bus-trace data
  translation.n0.p2.c0   memory configuration records

Multiple targets can be collected simultaneously:

  # perf record -m,256 \
      -e htm/nodalchipindex=2,nodeindex=0,htm_type=1,cpu=8/ \
      -e htm/nodalchipindex=1,nodeindex=0,htm_type=1,cpu=9/ \
      -a sleep 10

Testing:
Tested on POWER10.  Both htm.bin.* and translation.* files are produced
correctly for single and dual target collection.

Note: Link to kernel patches:
https://lore.kernel.org/linuxppc-dev/20260725065942.78839-2-atrajeev@linux.ibm.com/T/#t

Athira Rajeev (6):
  tools/perf: Move powerpc VPA-DTL auxtrace init into a separate file
  tools/perf: Add AUXTRACE recording support for powerpc HTM
  tools/perf: Add arch hook to drain remaining data before event close
  tools/perf: Add powerpc callback support for
    arch_perf_record__need_read
  tools/perf: Add powerpc HTM auxtrace event processing support
  tools/perf: Add perf tool support for processing powerpc HTM AUXTRACE
    records

 tools/perf/arch/powerpc/util/Build      |   2 +
 tools/perf/arch/powerpc/util/auxtrace.c |  93 +----
 tools/perf/arch/powerpc/util/evsel.c    |  55 +++
 tools/perf/arch/powerpc/util/htm.c      | 170 +++++++++
 tools/perf/arch/powerpc/util/vpa-dtl.c  |  96 ++++++
 tools/perf/builtin-record.c             |  68 ++++
 tools/perf/util/Build                   |   1 +
 tools/perf/util/auxtrace.c              |   4 +
 tools/perf/util/auxtrace.h              |   1 +
 tools/perf/util/powerpc-htm.c           | 441 ++++++++++++++++++++++++
 tools/perf/util/powerpc-htm.h           |  43 +++
 tools/perf/util/powerpc-vpadtl.h        |   1 +
 tools/perf/util/record.h                |   3 +
 13 files changed, 902 insertions(+), 76 deletions(-)
 create mode 100644 tools/perf/arch/powerpc/util/htm.c
 create mode 100644 tools/perf/arch/powerpc/util/vpa-dtl.c
 create mode 100644 tools/perf/util/powerpc-htm.c
 create mode 100644 tools/perf/util/powerpc-htm.h

-- 
2.43.0

[PATCH V3 1/6] tools/perf: Move powerpc VPA-DTL auxtrace init into a separate file

[Athira Rajeev]

The powerpc auxtrace dispatch lives entirely in
arch/powerpc/util/auxtrace.c.  As new PMUs such as HTM are added, this
file would grow to contain the recording logic for all of them.

Factor out the VPA-DTL recording initializer into its own file,
arch/powerpc/util/vpa-dtl.c, and reduce auxtrace.c to a thin dispatch
layer.  auxtrace_record__init() now detects the PMU by name and calls
the appropriate per-PMU init function:

  - vpa_dtl_recording_init() for VPA-DTL events (unchanged behaviour)
  - Further PMU entries will follow in subsequent patches

This makes room in auxtrace_record__init() for the HTM recording path
added in the next patch without growing a single monolithic file.

Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
---
Changes in V3:
Add #include <linux/zalloc.h> to vpa-dtl.c; without it the compiler
treats zalloc() as implicitly returning int, truncating the upper
32 bits of the returned pointer on 64-bit PowerPC.

Changes in V2:
- Renamed the destination file from arch/powerpc/util/vpa-dtl.c (same
  name, unchanged) but the subject and commit message are reworded to
  clearly state that the goal is to make auxtrace_record__init() a thin
  per-PMU dispatcher, not merely to "allow multiple PMUs to use auxtrace".
- Handle failure from memory allocation
- Included stdlib and limits.h
- No functional change to the VPA-DTL path itself.
- Patch is now 1/6 instead of 1/9.

 tools/perf/arch/powerpc/util/Build      |  1 +
 tools/perf/arch/powerpc/util/auxtrace.c | 84 +++-------------------
 tools/perf/arch/powerpc/util/vpa-dtl.c  | 96 +++++++++++++++++++++++++
 tools/perf/util/powerpc-vpadtl.h        |  1 +
 4 files changed, 106 insertions(+), 76 deletions(-)
 create mode 100644 tools/perf/arch/powerpc/util/vpa-dtl.c

diff --git a/tools/perf/arch/powerpc/util/Build b/tools/perf/arch/powerpc/util/Build
index ae928050e07a..7819c8f5af2d 100644
--- a/tools/perf/arch/powerpc/util/Build
+++ b/tools/perf/arch/powerpc/util/Build
@@ -7,3 +7,4 @@ perf-util-y += evsel.o
 perf-util-$(CONFIG_LIBDW) += skip-callchain-idx.o
 
 perf-util-y += auxtrace.o
+perf-util-y += vpa-dtl.o
diff --git a/tools/perf/arch/powerpc/util/auxtrace.c b/tools/perf/arch/powerpc/util/auxtrace.c
index 4600a1661b4f..e04a0bd61755 100644
--- a/tools/perf/arch/powerpc/util/auxtrace.c
+++ b/tools/perf/arch/powerpc/util/auxtrace.c
@@ -13,63 +13,12 @@
 #include "../../util/auxtrace.h"
 #include "../../util/powerpc-vpadtl.h"
 #include "../../util/record.h"
-#include <internal/lib.h> // page_size
-
-#define KiB(x) ((x) * 1024)
-
-static int
-powerpc_vpadtl_recording_options(struct auxtrace_record *ar __maybe_unused,
-			struct evlist *evlist __maybe_unused,
-			struct record_opts *opts)
-{
-	opts->full_auxtrace = true;
-
-	/*
-	 * Set auxtrace_mmap_pages to minimum
-	 * two pages
-	 */
-	if (!opts->auxtrace_mmap_pages) {
-		opts->auxtrace_mmap_pages = KiB(128) / page_size;
-		if (opts->mmap_pages == UINT_MAX)
-			opts->mmap_pages = KiB(256) / page_size;
-	}
-
-	return 0;
-}
-
-static size_t powerpc_vpadtl_info_priv_size(struct auxtrace_record *itr __maybe_unused,
-					struct evlist *evlist __maybe_unused)
-{
-	return VPADTL_AUXTRACE_PRIV_SIZE;
-}
-
-static int
-powerpc_vpadtl_info_fill(struct auxtrace_record *itr __maybe_unused,
-		struct perf_session *session __maybe_unused,
-		struct perf_record_auxtrace_info *auxtrace_info,
-		size_t priv_size __maybe_unused)
-{
-	auxtrace_info->type = PERF_AUXTRACE_VPA_DTL;
-
-	return 0;
-}
-
-static void powerpc_vpadtl_free(struct auxtrace_record *itr)
-{
-	free(itr);
-}
-
-static u64 powerpc_vpadtl_reference(struct auxtrace_record *itr __maybe_unused)
-{
-	return 0;
-}
 
 struct auxtrace_record *auxtrace_record__init(struct evlist *evlist,
 						int *err)
 {
-	struct auxtrace_record *aux;
 	struct evsel *pos;
-	int found = 0;
+	struct evsel *vpa_dtl_evsel = NULL;
 
 	/*
 	 * Set err value to zero here. Any fail later
@@ -78,33 +27,16 @@ struct auxtrace_record *auxtrace_record__init(struct evlist *evlist,
 	*err = 0;
 
 	evlist__for_each_entry(evlist, pos) {
-		if (strstarts(pos->name, "vpa_dtl")) {
-			found = 1;
+		if (pos->name && strstarts(pos->name, "vpa_dtl")) {
 			pos->needs_auxtrace_mmap = true;
-			break;
+			/* Remember the first matching VPA DTL event */
+			if (!vpa_dtl_evsel)
+				vpa_dtl_evsel = pos;
 		}
 	}
 
-	if (!found)
-		return NULL;
-
-	/*
-	 * To obtain the auxtrace buffer file descriptor, the auxtrace event
-	 * must come first.
-	 */
-	evlist__to_front(pos->evlist, pos);
-
-	aux = zalloc(sizeof(*aux));
-	if (aux == NULL) {
-		pr_debug("aux record is NULL\n");
-		*err = -ENOMEM;
-		return NULL;
-	}
+	if (vpa_dtl_evsel)
+		return vpa_dtl_recording_init(vpa_dtl_evsel, err);
 
-	aux->recording_options = powerpc_vpadtl_recording_options;
-	aux->info_priv_size = powerpc_vpadtl_info_priv_size;
-	aux->info_fill = powerpc_vpadtl_info_fill;
-	aux->free = powerpc_vpadtl_free;
-	aux->reference = powerpc_vpadtl_reference;
-	return aux;
+	return NULL;
 }
diff --git a/tools/perf/arch/powerpc/util/vpa-dtl.c b/tools/perf/arch/powerpc/util/vpa-dtl.c
new file mode 100644
index 000000000000..2609b88f61d8
--- /dev/null
+++ b/tools/perf/arch/powerpc/util/vpa-dtl.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * VPA DTL AUX tracing support
+ */
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/string.h>
+#include <linux/zalloc.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <limits.h>
+#include "../../util/cpumap.h"
+#include "../../util/evsel.h"
+#include "../../util/evlist.h"
+#include "../../util/session.h"
+#include "../../util/util.h"
+#include "../../util/debug.h"
+#include "../../util/auxtrace.h"
+#include "../../util/powerpc-vpadtl.h"
+#include "../../util/record.h"
+#include <internal/lib.h> // page_size
+
+#define KiB(x) ((x) * 1024)
+
+static int
+powerpc_vpadtl_recording_options(struct auxtrace_record *ar __maybe_unused,
+			struct evlist *evlist __maybe_unused,
+			struct record_opts *opts)
+{
+	opts->full_auxtrace = true;
+
+	/*
+	 * Set auxtrace_mmap_pages to minimum
+	 * two pages
+	 */
+	if (!opts->auxtrace_mmap_pages) {
+		opts->auxtrace_mmap_pages = KiB(128) / page_size;
+		if (opts->mmap_pages == UINT_MAX)
+			opts->mmap_pages = KiB(256) / page_size;
+	}
+
+	return 0;
+}
+
+static size_t powerpc_vpadtl_info_priv_size(struct auxtrace_record *itr __maybe_unused,
+					struct evlist *evlist __maybe_unused)
+{
+	return VPADTL_AUXTRACE_PRIV_SIZE;
+}
+
+static int
+powerpc_vpadtl_info_fill(struct auxtrace_record *itr __maybe_unused,
+		struct perf_session *session __maybe_unused,
+		struct perf_record_auxtrace_info *auxtrace_info,
+		size_t priv_size __maybe_unused)
+{
+	auxtrace_info->type = PERF_AUXTRACE_VPA_DTL;
+
+	return 0;
+}
+
+static void powerpc_vpadtl_free(struct auxtrace_record *itr)
+{
+	free(itr);
+}
+
+static u64 powerpc_vpadtl_reference(struct auxtrace_record *itr __maybe_unused)
+{
+	return 0;
+}
+
+struct auxtrace_record *vpa_dtl_recording_init(struct evsel *pos, int *err)
+{
+	struct auxtrace_record *aux;
+
+	/*
+	 * To obtain the auxtrace buffer file descriptor, the auxtrace event
+	 * must come first.
+	 */
+	evlist__to_front(pos->evlist, pos);
+
+	aux = zalloc(sizeof(*aux));
+	if (aux == NULL) {
+		pr_debug("aux record allocation failed (-ENOMEM)\n");
+		*err = -ENOMEM;
+		return NULL;
+	}
+
+	aux->recording_options = powerpc_vpadtl_recording_options;
+	aux->info_priv_size = powerpc_vpadtl_info_priv_size;
+	aux->info_fill = powerpc_vpadtl_info_fill;
+	aux->free = powerpc_vpadtl_free;
+	aux->reference = powerpc_vpadtl_reference;
+	return aux;
+}
diff --git a/tools/perf/util/powerpc-vpadtl.h b/tools/perf/util/powerpc-vpadtl.h
index ca809660b9bb..41e09b17a353 100644
--- a/tools/perf/util/powerpc-vpadtl.h
+++ b/tools/perf/util/powerpc-vpadtl.h
@@ -20,4 +20,5 @@ struct perf_pmu;
 int powerpc_vpadtl_process_auxtrace_info(union perf_event *event,
 				  struct perf_session *session);
 
+struct auxtrace_record *vpa_dtl_recording_init(struct evsel *pos, int *err);
 #endif
-- 
2.43.0

[PATCH V3 2/6] tools/perf: Add AUXTRACE recording support for powerpc HTM

[Athira Rajeev]

Add powerpc perf tool support for the HTM PMU AUXTRACE recording path.

Introduce htm_recording_init() and the associated auxtrace callbacks so
perf record can create PERF_RECORD_AUXTRACE records for HTM data.

htm_info_fill() stores the PMU type, number of HTM evsels, and a
(cpu, attr.config) pair for each evsel into PERF_RECORD_AUXTRACE_INFO
priv[].  The decode side reads these back to map each AUX buffer
(identified by event->auxtrace.cpu) to the correct (node, chip, core)
target when writing htm.bin.* output files.

Update auxtrace_record__init() to detect HTM events and dispatch to the
HTM-specific recording initializer.

Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
---
Changes in V3:
- Switch HTM event detection in auxtrace_record__init() from
strstarts(pos->name, "htm") to !strcmp(evsel__pmu_name(pos), "htm"),
matching the kernel-assigned PMU name and preventing false
matches on user-named events.
- Switch htm_recording_options(), htm_nr_events(), and the
loop in htm_info_fill() from strstarts(pos->name, "htm")
to evsel__pmu_name(), consistent with the above and with
patches 4 and 6.
- Add #include <linux/zalloc.h> to htm.c for zalloc().
- Set auxtrace_info->type = PERF_AUXTRACE_POWERPC_HTM directly
in htm_info_fill(), removing the placeholder comment. (V2
deferred this to patch 5; it is cleaner to set it in the same
patch that defines the enum.)
- Set aux_watermark to half the AUX ring-buffer size
(auxtrace_mmap_pages * page_size / 2) so there is always
headroom for hardware to write while userspace drains. V2 set
it to the full buffer size, risking data loss under back-pressure.
- Add PERF_AUXTRACE_POWERPC_HTM to the enum auxtrace_type in
util/auxtrace.h and add #include "powerpc-htm.h" plus a stub
case PERF_AUXTRACE_POWERPC_HTM: (falls through to
PERF_AUXTRACE_UNKNOWN) in util/auxtrace.c in this patch, so
the enum is defined before it is used in htm_info_fill().
(V2 did this in patch 5.)

Changes in V2:
- htm_info_fill() now stores a (cpu, attr.config) pair for every HTM
  evsel into PERF_RECORD_AUXTRACE_INFO priv[].  V1 stored only the PMU
  type and a single config value; there was no per-CPU mapping.
- The priv[] layout is formalised in util/powerpc-htm.h with enum
  constants POWERPC_HTM_PMU_TYPE, POWERPC_HTM_NUM_EVENTS, and
  POWERPC_HTM_EVENT_DATA and the helper macros HTM_AUXTRACE_PRIV_FIXED
  and HTM_AUXTRACE_PRIV_SIZE(n).  V1 used bare numeric offsets.
- PERF_SAMPLE_RAW is enabled in the recording options so that memory
  configuration records emitted by the kernel driver are captured
  alongside the AUX stream.  V1 added this in a later patch.
- PERF_AUXTRACE_POWERPC_HTM type constant is set in htm_info_fill()
  (wired up in patch 5 once the enum is defined).
- Patch is now 2/6 instead of 2/9.

 tools/perf/arch/powerpc/util/Build      |   1 +
 tools/perf/arch/powerpc/util/auxtrace.c |   9 ++
 tools/perf/arch/powerpc/util/htm.c      | 170 ++++++++++++++++++++++++
 tools/perf/util/auxtrace.c              |   2 +
 tools/perf/util/auxtrace.h              |   1 +
 tools/perf/util/powerpc-htm.h           |  43 ++++++
 6 files changed, 226 insertions(+)
 create mode 100644 tools/perf/arch/powerpc/util/htm.c
 create mode 100644 tools/perf/util/powerpc-htm.h

diff --git a/tools/perf/arch/powerpc/util/Build b/tools/perf/arch/powerpc/util/Build
index 7819c8f5af2d..297152591046 100644
--- a/tools/perf/arch/powerpc/util/Build
+++ b/tools/perf/arch/powerpc/util/Build
@@ -8,3 +8,4 @@ perf-util-$(CONFIG_LIBDW) += skip-callchain-idx.o
 
 perf-util-y += auxtrace.o
 perf-util-y += vpa-dtl.o
+perf-util-y += htm.o
diff --git a/tools/perf/arch/powerpc/util/auxtrace.c b/tools/perf/arch/powerpc/util/auxtrace.c
index e04a0bd61755..74f0a37236b8 100644
--- a/tools/perf/arch/powerpc/util/auxtrace.c
+++ b/tools/perf/arch/powerpc/util/auxtrace.c
@@ -12,6 +12,7 @@
 #include "../../util/debug.h"
 #include "../../util/auxtrace.h"
 #include "../../util/powerpc-vpadtl.h"
+#include "../../util/powerpc-htm.h"
 #include "../../util/record.h"
 
 struct auxtrace_record *auxtrace_record__init(struct evlist *evlist,
@@ -19,6 +20,7 @@ struct auxtrace_record *auxtrace_record__init(struct evlist *evlist,
 {
 	struct evsel *pos;
 	struct evsel *vpa_dtl_evsel = NULL;
+	struct evsel *htm_evsel = NULL;
 
 	/*
 	 * Set err value to zero here. Any fail later
@@ -32,11 +34,18 @@ struct auxtrace_record *auxtrace_record__init(struct evlist *evlist,
 			/* Remember the first matching VPA DTL event */
 			if (!vpa_dtl_evsel)
 				vpa_dtl_evsel = pos;
+		} else if (pos->name && (!strcmp(evsel__pmu_name(pos), "htm"))) {
+			pos->needs_auxtrace_mmap = true;
+			/* Remember the first matching HTM event */
+			if (!htm_evsel)
+				htm_evsel = pos;
 		}
 	}
 
 	if (vpa_dtl_evsel)
 		return vpa_dtl_recording_init(vpa_dtl_evsel, err);
+	else if (htm_evsel)
+		return htm_recording_init(htm_evsel, err);
 
 	return NULL;
 }
diff --git a/tools/perf/arch/powerpc/util/htm.c b/tools/perf/arch/powerpc/util/htm.c
new file mode 100644
index 000000000000..9a12a8fe8027
--- /dev/null
+++ b/tools/perf/arch/powerpc/util/htm.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HTM AUX tracing support
+ */
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/string.h>
+#include <linux/zalloc.h>
+#include <stdlib.h>
+#include <limits.h>
+#include "../../util/evsel.h"
+#include "../../util/evlist.h"
+#include "../../util/session.h"
+#include "../../util/debug.h"
+#include "../../util/auxtrace.h"
+#include "../../util/powerpc-htm.h"
+#include "../../util/record.h"
+#include <internal/lib.h> /* page_size */
+#include <errno.h>
+
+#define KiB(x) ((x) * 1024)
+
+struct htm_recording {
+	struct auxtrace_record	itr;
+	struct evsel		*evsel;
+};
+
+static int
+htm_recording_options(struct auxtrace_record *itr __maybe_unused,
+		      struct evlist *evlist,
+		      struct record_opts *opts)
+{
+	struct evsel *pos;
+
+	opts->full_auxtrace = true;
+
+	if (!opts->auxtrace_mmap_pages) {
+		opts->auxtrace_mmap_pages = KiB(128) / page_size;
+		if (opts->mmap_pages == UINT_MAX)
+			opts->mmap_pages = KiB(256) / page_size;
+	}
+
+	evlist__for_each_entry(evlist, pos) {
+		if (!pos->name || (strcmp(evsel__pmu_name(pos), "htm")))
+			continue;
+		pos->core.attr.aux_watermark =
+			opts->auxtrace_mmap_pages * (size_t)page_size / 2;
+		pos->core.attr.freq = 0;
+		pos->core.attr.sample_type |= PERF_SAMPLE_RAW;
+		pos->needs_auxtrace_mmap = true;
+	}
+
+	return 0;
+}
+
+/* Count htm evsels in the evlist */
+static int htm_nr_events(struct evlist *evlist)
+{
+	struct evsel *pos;
+	int n = 0;
+
+	evlist__for_each_entry(evlist, pos) {
+		if (pos->name && (!strcmp(evsel__pmu_name(pos), "htm")))
+			n++;
+	}
+	return n;
+}
+
+static size_t htm_info_priv_size(struct auxtrace_record *itr __maybe_unused,
+				 struct evlist *evlist)
+{
+	return HTM_AUXTRACE_PRIV_SIZE(htm_nr_events(evlist));
+}
+
+/*
+ * Fill the PERF_RECORD_AUXTRACE_INFO private data with:
+ *   priv[POWERPC_HTM_PMU_TYPE]   = pmu->type of the first htm evsel
+ *   priv[POWERPC_HTM_NUM_EVENTS] = number of htm evsels
+ *   priv[POWERPC_HTM_EVENT_DATA + n*2]     = CPU for nth htm evsel
+ *   priv[POWERPC_HTM_EVENT_DATA + n*2 + 1] = attr.config for nth htm evsel
+ *
+ * The CPU is the first CPU in the evsel's cpu map; for events opened with
+ * cpu=N there is exactly one CPU.  The decode side uses event->auxtrace.cpu
+ * to look up the matching config and derive (node, chip, core) for the
+ * output file name.
+ */
+static int
+htm_info_fill(struct auxtrace_record *itr,
+	      struct perf_session *session,
+	      struct perf_record_auxtrace_info *auxtrace_info,
+	      size_t priv_size)
+{
+	struct htm_recording *htm_r = container_of(itr, struct htm_recording, itr);
+	struct evlist *evlist = session->evlist;
+	struct evsel *pos;
+	int n = 0;
+	int expected_n = htm_nr_events(evlist);
+
+	if (priv_size != HTM_AUXTRACE_PRIV_SIZE(expected_n))
+		return -EINVAL;
+
+	auxtrace_info->type = PERF_AUXTRACE_POWERPC_HTM;
+	auxtrace_info->priv[POWERPC_HTM_PMU_TYPE] = htm_r->evsel->core.attr.type;
+	auxtrace_info->priv[POWERPC_HTM_NUM_EVENTS] = expected_n;
+
+	evlist__for_each_entry(evlist, pos) {
+		struct perf_cpu_map *cpus;
+		int cpu;
+
+		if (!pos->name || (strcmp(evsel__pmu_name(pos), "htm")))
+			continue;
+
+		/*
+		 * Get the CPU this evsel is pinned to.  For events opened
+		 * with cpu=N, evsel__cpus() returns a single-entry map {N}
+		 * at record time (not during replay).
+		 */
+		cpus = evsel__cpus(pos);
+		if (cpus && perf_cpu_map__nr(cpus) > 0)
+			cpu = perf_cpu_map__cpu(cpus, 0).cpu;
+		else
+			cpu = -1;
+
+		auxtrace_info->priv[POWERPC_HTM_EVENT_DATA + n * 2]     = cpu;
+		auxtrace_info->priv[POWERPC_HTM_EVENT_DATA + n * 2 + 1] =
+							pos->core.attr.config;
+		n++;
+	}
+
+	return 0;
+}
+
+static u64 htm_reference(struct auxtrace_record *itr __maybe_unused)
+{
+	return 0;
+}
+
+static void htm_free(struct auxtrace_record *itr)
+{
+	struct htm_recording *htm_r = container_of(itr, struct htm_recording, itr);
+
+	free(htm_r);
+}
+
+struct auxtrace_record *htm_recording_init(struct evsel *pos, int *err)
+{
+	struct htm_recording *htm_r;
+
+	/*
+	 * To obtain the auxtrace buffer file descriptor, the auxtrace event
+	 * must come first.
+	 */
+	evlist__to_front(pos->evlist, pos);
+
+	htm_r = zalloc(sizeof(*htm_r));
+	if (!htm_r) {
+		pr_debug("htm_recording allocation failed (-ENOMEM)\n");
+		*err = -ENOMEM;
+		return NULL;
+	}
+
+	htm_r->evsel = pos;
+	htm_r->itr.recording_options = htm_recording_options;
+	htm_r->itr.info_priv_size    = htm_info_priv_size;
+	htm_r->itr.info_fill         = htm_info_fill;
+	htm_r->itr.free              = htm_free;
+	htm_r->itr.reference         = htm_reference;
+	return &htm_r->itr;
+}
diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index 0b851f32e98c..31c0db933f61 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -56,6 +56,7 @@
 #include "s390-cpumsf.h"
 #include "util/mmap.h"
 #include "powerpc-vpadtl.h"
+#include "powerpc-htm.h"
 
 #include <linux/ctype.h>
 #include "symbol/kallsyms.h"
@@ -1427,6 +1428,7 @@ int perf_event__process_auxtrace_info(const struct perf_tool *tool __maybe_unuse
 	case PERF_AUXTRACE_VPA_DTL:
 		err = powerpc_vpadtl_process_auxtrace_info(event, session);
 		break;
+	case PERF_AUXTRACE_POWERPC_HTM:
 	case PERF_AUXTRACE_UNKNOWN:
 	default:
 		return -EINVAL;
diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
index 6947f3f284c0..68b17802a419 100644
--- a/tools/perf/util/auxtrace.h
+++ b/tools/perf/util/auxtrace.h
@@ -46,6 +46,7 @@ enum auxtrace_type {
 	PERF_AUXTRACE_S390_CPUMSF,
 	PERF_AUXTRACE_HISI_PTT,
 	PERF_AUXTRACE_VPA_DTL,
+	PERF_AUXTRACE_POWERPC_HTM,
 };
 
 enum itrace_period_type {
diff --git a/tools/perf/util/powerpc-htm.h b/tools/perf/util/powerpc-htm.h
new file mode 100644
index 000000000000..18e39417d556
--- /dev/null
+++ b/tools/perf/util/powerpc-htm.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __POWERPC_HTM_H
+#define __POWERPC_HTM_H
+
+#include <linux/types.h>
+
+/*
+ * Layout of the private data in PERF_RECORD_AUXTRACE_INFO for HTM.
+ *
+ * priv[POWERPC_HTM_PMU_TYPE]   = htm PMU type ID (pmu->type from kernel)
+ * priv[POWERPC_HTM_NUM_EVENTS] = number of htm evsels recorded (N)
+ *
+ * Followed by N pairs (2 u64 each):
+ *   priv[POWERPC_HTM_EVENT_DATA + n*2 + 0] = CPU number for nth htm evsel
+ *   priv[POWERPC_HTM_EVENT_DATA + n*2 + 1] = attr.config for nth htm evsel
+ *
+ * Total priv entries: POWERPC_HTM_EVENT_DATA + N * 2
+ */
+enum {
+	POWERPC_HTM_PMU_TYPE  = 0,
+	POWERPC_HTM_NUM_EVENTS,
+	POWERPC_HTM_EVENT_DATA,	/* variable-length: 2 u64 per event */
+};
+
+/* Fixed header size (everything before the per-event data) */
+#define HTM_AUXTRACE_PRIV_FIXED  (POWERPC_HTM_EVENT_DATA * sizeof(u64))
+
+/* Total priv size for N htm evsels */
+#define HTM_AUXTRACE_PRIV_SIZE(n) \
+	(HTM_AUXTRACE_PRIV_FIXED + (n) * 2 * sizeof(u64))
+
+struct evsel;
+struct evlist;
+union perf_event;
+struct perf_session;
+struct auxtrace_record;
+
+struct auxtrace_record *htm_recording_init(struct evsel *pos, int *err);
+
+int powerpc_htm_process_auxtrace_info(union perf_event *event,
+				      struct perf_session *session);
+
+#endif /* __POWERPC_HTM_H */
-- 
2.43.0

[PATCH V3 3/6] tools/perf: Add arch hook to drain remaining data before event close

[Athira Rajeev]

While collecting samples using perf record, __cmd_record() disables the
evlist once recording is complete. After that, event fds are no longer
read and any remaining PMU-specific data cannot be drained.

Add a weak arch_perf_record__need_read() hook so architecture code can
indicate that more data remains to be collected before events are
disabled and closed. When the hook reports pending data, perf record
performs another read pass via record__final_data().

A static volatile sig_atomic_t drain_interrupted flag is added.
sig_handler() sets it when a second signal (SIGINT/SIGTERM) arrives
while done is already set, allowing a second Ctrl+C during the drain
loop to abort immediately.  Without this, a user who presses Ctrl+C
twice while HTM data is being drained would have no way to interrupt
a stalled drain loop.

This allows architectures such as powerpc HTM to drain trace data and
associated metadata before the event is closed.

Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
---
Changes in V3:
- Add static volatile sig_atomic_t drain_interrupted.  Set it in
  sig_handler() when a second signal arrives while done is already set.
  This lets a second Ctrl+C abort the drain loop immediately.  V2
  tested done > 1 to detect a second signal, which is unreachable
  because done is only ever set to 1.
- Replace rec->bytes_written with record__bytes_written(rec)
  (which includes rec->thread_bytes_written) so the no-progress
  check accounts for data written by the AIO/thread path.
- Add a retry counter (FINAL_DATA_MAX_RETRIES 20, 20 x 1 ms = 20 ms
  maximum) instead of aborting on the first stall.  The no-progress
  sleep of 1 ms is enough for the AUX ring buffer consumer (perf's
  ~1 ms poll interval) to advance the tail pointer.
- Move record__final_data() into the in-loop disable block
  (before evlist__disable()) as well as into a post-loop if
  (!disabled) block that covers both the early-break and
  child-workload paths.  V2 only called it inside the loop
  with a final_data_drained guard, missing the early-break
  and child-workload cases.
- Remove the now-unnecessary bool final_data_drained local variable.

Changes in V2:
-  V1's callback was responsible for driving the read loop
  including evlist__enable cycling. Removed that logic
- Use bytes written to check if session needs to be continued.
- Patch is now 3/6 instead of 3/9.

 tools/perf/builtin-record.c | 68 +++++++++++++++++++++++++++++++++++++
 tools/perf/util/record.h    |  3 ++
 2 files changed, 71 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index f58d7e3c7879..370003f31e56 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -192,6 +192,7 @@ struct record {
 };
 
 static volatile int done;
+static volatile sig_atomic_t drain_interrupted;
 
 static volatile int auxtrace_record__snapshot_started;
 static DEFINE_TRIGGER(auxtrace_snapshot_trigger);
@@ -681,6 +682,8 @@ static void sig_handler(int sig)
 	else
 		signr = sig;
 
+	if (done)
+		drain_interrupted = 1;
 	done = 1;
 #ifdef HAVE_EVENTFD_SUPPORT
 	if (done_fd >= 0) {
@@ -2437,6 +2440,54 @@ static unsigned long record__waking(struct record *rec)
 	return waking;
 }
 
+/*
+ * Weak symbol - architecture can override to indicate if more
+ * data needs to be collected before finishing output.
+ *
+ * Returns: 1 if more data exists, 0 if collection is complete
+ */
+__weak int arch_perf_record__need_read(struct evlist *evlist __maybe_unused)
+{
+	return 0;  /* Default: no arch-specific data to collect */
+}
+
+static void record__final_data(struct record *rec)
+{
+	u64 last_bytes_written = 0;
+	int retries = 0;
+#define FINAL_DATA_MAX_RETRIES 20  /* 20 * 1 ms = 20 ms max wait */
+
+	/*
+	 * Collect any remaining architecture-specific data.
+	 * The arch code checks if more data exists, and we do the actual
+	 * reading here since we have access to record__mmap_read_all().
+	 * This code performs the additional read pass while events are
+	 * still live.  A second SIGINT during drain sets drain_interrupted
+	 * and aborts the loop immediately.
+	 */
+	while (arch_perf_record__need_read(rec->evlist)) {
+		if (drain_interrupted)
+			break;
+
+		last_bytes_written = record__bytes_written(rec);
+
+		if (record__mmap_read_all(rec, true) < 0)
+			break;
+
+		if (record__bytes_written(rec) == last_bytes_written) {
+			if (++retries >= FINAL_DATA_MAX_RETRIES) {
+				pr_warning("Final data drain made no forward progress after %d retries.\n",
+					   FINAL_DATA_MAX_RETRIES);
+				break;
+			}
+			usleep(1000); /* 1 ms: let AUX ring buffer consumer advance */
+		} else {
+			retries = 0;
+			usleep(100);
+		}
+	}
+}
+
 static int __cmd_record(struct record *rec, int argc, const char **argv)
 {
 	int err;
@@ -2864,11 +2915,28 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		 */
 		if (done && !disabled && !target__none(&opts->target)) {
 			trigger_off(&auxtrace_snapshot_trigger);
+			record__final_data(rec);
 			evlist__disable(rec->evlist);
 			disabled = true;
 		}
 	}
 
+	/*
+	 * If the loop exited via the early break (ring buffer empty when done
+	 * was set, so the in-loop disable block was never reached), or this is
+	 * a child workload (target__none, so the in-loop block is never entered
+	 * and disabled stays false), drain any remaining arch-specific data now.
+	 * Events are still live in both cases: for child workloads they die with
+	 * the process after this point; for non-child they are disabled below.
+	 */
+	if (!disabled) {
+		record__final_data(rec);
+		if (!target__none(&opts->target)) {
+			trigger_off(&auxtrace_snapshot_trigger);
+			evlist__disable(rec->evlist);
+		}
+	}
+
 	trigger_off(&auxtrace_snapshot_trigger);
 	trigger_off(&switch_output_trigger);
 
diff --git a/tools/perf/util/record.h b/tools/perf/util/record.h
index 93627c9a7338..56de4f95a836 100644
--- a/tools/perf/util/record.h
+++ b/tools/perf/util/record.h
@@ -10,6 +10,7 @@
 #include "util/target.h"
 
 struct option;
+struct evlist;
 
 struct record_opts {
 	struct target target;
@@ -95,4 +96,6 @@ static inline bool record_opts__no_switch_events(const struct record_opts *opts)
 	return opts->record_switch_events_set && !opts->record_switch_events;
 }
 
+int arch_perf_record__need_read(struct evlist *evlist);
+
 #endif // _PERF_RECORD_H
-- 
2.43.0

[PATCH V3 4/6] tools/perf: Add powerpc callback support for arch_perf_record__need_read

[Athira Rajeev]

Implement the arch_perf_record__need_read() architecture-specific hook
for powerpc in arch/powerpc/util/evsel.c.

The HTM kernel driver sets event->count to the number of records still
staged in its internal buffers (total_size / record_size), and to 0
once the stream is exhausted.  This hook reads that count for every open
htm evsel via perf_evsel__read() and accumulates the values into
total_pending_records.  A non-zero total means at least one HTM target
still has records pending; the recording loop added in the previous
patch will perform another mmap-read pass.

The drain uses a two-layer safety check: event->count detects records
staged by the driver, and record__bytes_written() in the drain loop
confirms data was actually moved into perf.data.  This combination
handles the case where the driver count is briefly stale while hardware
is still flushing.

The implementation scans the evlist using evsel__pmu_name() to identify
HTM events by their kernel-assigned PMU name rather than the
user-visible event name, preventing false matches.  It iterates the fd/
sample-id xyarray, and skips any evsel whose fd and sample-id arrays are
mismatched to avoid reading stale state.  When the accumulated record
count reaches zero the hook returns 0 and the recording loop proceeds to
disable and close the events.

Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
---
Changes in V3:
- Use evsel__pmu_name(evsel) instead of strstarts(evsel->name, "htm")
to identify HTM events, matching by kernel-assigned PMU name rather
than user-visible event name.
- Remove the redundant two-pass loop (first pass to set found_htm,
second to accumulate counts); a single pass with evsel__pmu_name()
is sufficient. if no HTM event exists total_pending_records stays 0
and the function returns 0.
- Remove the dead !strcmp(evsel->name, "dummy:u") check;
- evsel__pmu_name() will never return "htm" for a dummy:u software
event.
- Rename total_pending_bytes -> total_pending_records to match what
the driver actually reports (event->count = total_size / record_size,
a record count, not a byte count).
- Add #include <string.h> for musl compatibility (strcmp() without
it warns on some toolchains).

Changes in V2:
- Implements the renamed arch_perf_record__need_read() hook (V1
  implemented arch_record__collect_final_data()).
- Skips evsels whose fd and sample-id xyarrays are mismatched, avoiding
  stale-state reads.  V1 had no such guard.
- evlist__enable cycling is removed; that responsibility now belongs to
  the drain loop in builtin-record.c added in patch 3.
- File location changed to arch/powerpc/util/evsel.c (V1 used
  arch/powerpc/util/powerpc-htm.c).
- Patch is now 4/6 instead of 4/9.

 tools/perf/arch/powerpc/util/evsel.c | 55 ++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/tools/perf/arch/powerpc/util/evsel.c b/tools/perf/arch/powerpc/util/evsel.c
index 2f733cdc8dbb..3c7ccc955b0a 100644
--- a/tools/perf/arch/powerpc/util/evsel.c
+++ b/tools/perf/arch/powerpc/util/evsel.c
@@ -1,8 +1,63 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <stdio.h>
+#include <string.h>
+#include <linux/string.h>
 #include "util/evsel.h"
+#include "util/record.h"
+#include "util/evlist.h"
+#include "util/debug.h"
+#include <internal/xyarray.h>
 
 void arch_evsel__set_sample_weight(struct evsel *evsel)
 {
 	evsel__set_sample_bit(evsel, WEIGHT_STRUCT);
 }
+
+/*
+ * Check if HTM events have more data to collect.
+ *
+ * This function reads the HTM event counts. When the kernel driver
+ * has more data available, it returns a non-zero count. When all
+ * data has been collected, it returns zero.
+ *
+ * Returns: 1 if more data exists, 0 if collection is complete
+ */
+int arch_perf_record__need_read(struct evlist *evlist)
+{
+	struct evsel *evsel;
+	u64 total_pending_records = 0;
+	int x, y;
+
+	/* there was an error during record__open */
+	if (!evlist)
+		return 0;
+
+	/* Read HTM event counts to check if more data is available */
+	evlist__for_each_entry(evlist, evsel) {
+		struct xyarray *xy = evsel->core.sample_id;
+
+		if (strcmp(evsel__pmu_name(evsel), "htm"))
+			continue;
+
+		if (xy == NULL || evsel->core.fd == NULL)
+			continue;
+
+		if (xyarray__max_x(evsel->core.fd) != xyarray__max_x(xy) ||
+		    xyarray__max_y(evsel->core.fd) != xyarray__max_y(xy)) {
+			pr_debug("Unmatched FD vs sample ID array for HTM event\n");
+			continue;
+		}
+
+		for (x = 0; x < xyarray__max_x(xy); x++) {
+			for (y = 0; y < xyarray__max_y(xy); y++) {
+				struct perf_counts_values count = { .val = 0 };
+
+				if (perf_evsel__read(&evsel->core, x, y, &count) == 0)
+					total_pending_records += count.val;
+			}
+		}
+	}
+
+	/* Collection is complete only when ALL hardware queues have no pending records */
+	return (total_pending_records > 0) ? 1 : 0;
+}
-- 
2.43.0

[PATCH V3 5/6] tools/perf: Add powerpc HTM auxtrace event processing support

[Athira Rajeev]

Add the powerpc-htm.c decode stub and wire the dispatch in
perf_event__process_auxtrace_info() to call
powerpc_htm_process_auxtrace_info().

Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
---
Changes in V3:
- Change powerpc_htm_dump_event() parameter from size_t to u64 and
use %" PRIu64 ", matching the type of buffer->size and avoiding
truncation on 32-bit platforms.
- Add #include <inttypes.h> for PRIu64.
- Remove the htm->auxtrace_type = auxtrace_info->type assignment
that was added here in V2; this field is now set from
priv[POWERPC_HTM_PMU_TYPE] in patch 6 where the full processing
logic lives.
- Do not re-add PERF_AUXTRACE_POWERPC_HTM to auxtrace.h or the
stub case in auxtrace.c; those are now in patch 2. This patch only
adds the powerpc-htm.c decode stub and wires the dispatch to call
powerpc_htm_process_auxtrace_info().

Changes in V2:
- Scope narrowed: this patch now only adds the PERF_AUXTRACE_POWERPC_HTM
  enum constant to auxtrace.h and wires the dispatch in
  perf_event__process_auxtrace_info() to call
  powerpc_htm_process_auxtrace_info().  It also amends htm_info_fill()
  (from patch 2) to set auxtrace_info->type now that the constant is
  defined.
- All file-writing and decoding logic is moved to patch 6.
- Patch is now 5/6 instead of 5/9.

 tools/perf/util/Build         |   1 +
 tools/perf/util/auxtrace.c    |   2 +
 tools/perf/util/powerpc-htm.c | 116 ++++++++++++++++++++++++++++++++++
 3 files changed, 119 insertions(+)
 create mode 100644 tools/perf/util/powerpc-htm.c

diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 330311cac550..7fa354853d2a 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -141,6 +141,7 @@ perf-util-y += hisi-ptt.o
 perf-util-y += hisi-ptt-decoder/
 perf-util-y += s390-cpumsf.o
 perf-util-y += powerpc-vpadtl.o
+perf-util-y += powerpc-htm.o
 
 ifdef CONFIG_LIBOPENCSD
 perf-util-y += cs-etm.o
diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index 31c0db933f61..9f32f54fad43 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -1429,6 +1429,8 @@ int perf_event__process_auxtrace_info(const struct perf_tool *tool __maybe_unuse
 		err = powerpc_vpadtl_process_auxtrace_info(event, session);
 		break;
 	case PERF_AUXTRACE_POWERPC_HTM:
+		err = powerpc_htm_process_auxtrace_info(event, session);
+		break;
 	case PERF_AUXTRACE_UNKNOWN:
 	default:
 		return -EINVAL;
diff --git a/tools/perf/util/powerpc-htm.c b/tools/perf/util/powerpc-htm.c
new file mode 100644
index 000000000000..952ed32997d9
--- /dev/null
+++ b/tools/perf/util/powerpc-htm.c
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <stdio.h>
+#include <stdlib.h>
+#include <inttypes.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <string.h>
+#include <errno.h>
+#include <endian.h>
+#include "util/evsel.h"
+#include "util/evlist.h"
+#include "util/session.h"
+#include "color.h"
+#include "powerpc-htm.h"
+#include "debug.h"
+#include "sample.h"
+
+struct powerpc_htm {
+	struct auxtrace		auxtrace;
+	struct auxtrace_queues	queues;
+	struct auxtrace_heap	heap;
+	u32			auxtrace_type;
+	struct perf_session	*session;
+	struct machine		*machine;
+};
+
+static void powerpc_htm_dump_event(u64 len)
+{
+	const char *color = PERF_COLOR_BLUE;
+
+	if (dump_trace) {
+		color_fprintf(stdout, color,
+			". ... HTM PMU data: size %" PRIu64 " bytes\n", len);
+	}
+}
+
+static int powerpc_htm_process_event(struct perf_session *session __maybe_unused,
+				     union perf_event *event __maybe_unused,
+				     struct perf_sample *sample __maybe_unused,
+				     const struct perf_tool *tool __maybe_unused)
+{
+	return 0;
+}
+
+static int powerpc_htm_process_auxtrace_event(struct perf_session *session __maybe_unused,
+					      union perf_event *event,
+					      const struct perf_tool *tool __maybe_unused)
+{
+	if (dump_trace)
+		powerpc_htm_dump_event(event->auxtrace.size);
+
+	return 0;
+}
+
+static int powerpc_htm_flush(struct perf_session *session __maybe_unused,
+			     const struct perf_tool *tool __maybe_unused)
+{
+	return 0;
+}
+
+static void powerpc_htm_free_events(struct perf_session *session)
+{
+	struct powerpc_htm *htm;
+
+	if (!session || !session->auxtrace)
+		return;
+
+	htm = container_of(session->auxtrace, struct powerpc_htm, auxtrace);
+	auxtrace_queues__free(&htm->queues);
+}
+
+static void powerpc_htm_free(struct perf_session *session)
+{
+	struct powerpc_htm *htm;
+
+	if (!session || !session->auxtrace)
+		return;
+
+	htm = container_of(session->auxtrace, struct powerpc_htm, auxtrace);
+	powerpc_htm_free_events(session);
+	session->auxtrace = NULL;
+	free(htm);
+}
+
+int powerpc_htm_process_auxtrace_info(union perf_event *event,
+				      struct perf_session *session)
+{
+	struct perf_record_auxtrace_info *auxtrace_info = &event->auxtrace_info;
+	struct powerpc_htm *htm;
+	int err;
+
+	if (auxtrace_info->header.size < sizeof(struct perf_record_auxtrace_info) +
+					 HTM_AUXTRACE_PRIV_FIXED)
+		return -EINVAL;
+
+	htm = zalloc(sizeof(struct powerpc_htm));
+	if (!htm)
+		return -ENOMEM;
+
+	err = auxtrace_queues__init(&htm->queues);
+	if (err) {
+		free(htm);
+		return err;
+	}
+
+	htm->session = session;
+	htm->machine = &session->machines.host;
+	htm->auxtrace.process_event = powerpc_htm_process_event;
+	htm->auxtrace.process_auxtrace_event = powerpc_htm_process_auxtrace_event;
+	htm->auxtrace.flush_events = powerpc_htm_flush;
+	htm->auxtrace.free_events = powerpc_htm_free_events;
+	htm->auxtrace.free = powerpc_htm_free;
+	session->auxtrace = &htm->auxtrace;
+
+	return 0;
+}
-- 
2.43.0

[PATCH V3 6/6] tools/perf: Add perf tool support for processing powerpc HTM AUXTRACE records

[Athira Rajeev]

powerpc_htm_process_auxtrace_info() reads the (cpu, attr.config) pairs
stored by htm_info_fill() at record time and builds a cpu_configs[]
table.  This lets process_auxtrace_event() map each AUX buffer, which
carries event->auxtrace.cpu, to the correct (node, chip, core) target
and write the raw trace data to htm.bin.nX.pX.cX immediately.

PERF_RECORD_SAMPLE events carrying PERF_SAMPLE_RAW data are handled by
process_event(), which extracts memory configuration records and writes
them to translation.nX.pX.cX.

Per-run first-write tracking (htm_target_seen()) ensures each output
file is opened with O_TRUNC on the first write and O_APPEND on
subsequent writes, correctly handling multiple AUX chunks from the same
target and stale files from prior runs.

For perf report -D the AUX buffer size is printed.

Example:
  # perf record -C 9 -m,256 \
      -e htm/nodalchipindex=2,nodeindex=0,htm_type=1/ sleep 3
  # perf report
  # ls htm.bin.* translation.*
  htm.bin.n0.p2.c0  translation.n0.p2.c0

Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
---
Changes in V3:
- Add #include <linux/unaligned.h> (placed after the perf "..."
headers so <linux/compiler.h> is already in scope) and use
get_unaligned_be64(byte_ptr + 0x10) instead of be64_to_cpu(*(__be64 *)(byte_ptr + 0x10))
to avoid SIGBUS on strict-alignment architectures.
- Add raw_size underflow guard: skip silently if sample->raw_size <=
sizeof(uint32_t) before subtracting the 4-byte alignment padding.
- Reject duplicate PERF_RECORD_AUXTRACE_INFO:
check session->auxtrace != NULL before allocation to prevent
overwriting the pointer and leaking the first allocation.
- Validate num_events against the actual auxtrace_info->header.size
before using it to index priv[] or allocate arrays, preventing
out-of-bounds reads from a malformed record.
- Cap num_events at HTM_MAX_SAFE_TARGETS (1024) to prevent size_t
truncation on 32-bit platforms when passed to calloc().
- Fix htm_target_seen(): return true (use O_APPEND) when capacity is
exceeded, matching the warning message that already claimed append
behaviour. V2 returned false (would have used O_TRUNC), discarding
previously written data.
- Cast calloc() arguments to (size_t)num_events now that num_events
has been validated and capped.

Changes in V2:
- Consolidated: the file-writing, memory-configuration processing, and
  address-mapping logic that was spread across old patches 5 to 9 is
  rationalised into a single, focused implementation in
  tools/perf/util/powerpc-htm.c.
- Synthetic sample generation (old patch 8: PERF_TYPE_SYNTH "htm" event
  with logical addresses) and the separate logical-address mapping output
  file (old patch 9: .out.l file) are not included in V2.  These features
  require further discussion on the appropriate abstraction and will be
  posted as a follow-on series.
- Physical-to-logical address mapping (old patch 7) is not included in
  V2 for the same reason.
- Memory configuration records are now written via process_event()
  handling PERF_RECORD_SAMPLE with PERF_SAMPLE_RAW, keyed on the "htm"
  PMU name from evsel__pmu_name().  V1 used PERF_SAMPLE_RAW boundary
  markers inside the AUX buffer to locate the configuration data.
- AUX buffer processing uses htm_config_for_cpu() to map
  event->auxtrace.cpu to the correct (node, chip, core) target using
  the (cpu, attr.config) table populated from AUXTRACE_INFO priv[] in
  powerpc_htm_process_auxtrace_info().  V1 iterated the evlist at
  report time.
- Per-run first-write tracking (htm_target_seen()) uses O_TRUNC on the
  first write and O_APPEND on subsequent writes for both htm.bin.* and
  translation.* files, correctly handling multiple AUX chunks per target
  and stale files from prior runs.  V1 always opened with O_TRUNC.
- HTM_MAX_TARGETS compile-time constant is replaced by dynamic allocation
  sized to num_events read from AUXTRACE_INFO priv[POWERPC_HTM_NUM_EVENTS].
- Patch is now 6/6 instead of spanning patches 5 to 9.

 tools/perf/util/powerpc-htm.c | 337 +++++++++++++++++++++++++++++++++-
 1 file changed, 331 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/powerpc-htm.c b/tools/perf/util/powerpc-htm.c
index 952ed32997d9..1d1b4302e2e4 100644
--- a/tools/perf/util/powerpc-htm.c
+++ b/tools/perf/util/powerpc-htm.c
@@ -12,9 +12,14 @@
 #include "util/session.h"
 #include "color.h"
 #include "powerpc-htm.h"
+#include <errno.h>
 #include "debug.h"
 #include "sample.h"
 
+#include <linux/unaligned.h>
+
+struct perf_session;
+
 struct powerpc_htm {
 	struct auxtrace		auxtrace;
 	struct auxtrace_queues	queues;
@@ -22,8 +27,52 @@ struct powerpc_htm {
 	u32			auxtrace_type;
 	struct perf_session	*session;
 	struct machine		*machine;
+
+	/*
+	 * Capacity: number of distinct HTM targets (node/chip/core tuples)
+	 * recorded, read from auxtrace_info->priv[POWERPC_HTM_NUM_EVENTS] at
+	 * init time.  All three arrays below are allocated to this size.
+	 */
+	int	nr_targets;
+
+	/*
+	 * Per-run first-write tracking for htm.bin.* and translation.* files.
+	 * Each entry is a packed u32: node<<16 | chip<<8 | core.
+	 * First write for a given key -> O_TRUNC; subsequent writes -> O_APPEND.
+	 */
+	u32	*htm_bin_targets;
+	int	nr_htm_bin_targets;
+	u32	*translation_targets;
+	int	nr_translation_targets;
+
+	/*
+	 * CPU -> attr.config table, populated from auxtrace_info->priv[] at
+	 * init time.  htm_info_fill() (record side) stores the CPU and config
+	 * for each htm evsel; we read them back here to map each
+	 * event->auxtrace.cpu to the correct (node, chip, core) config.
+	 */
+	struct {
+		int	cpu;
+		u64	config;
+	} *cpu_configs;
+	int	nr_cpu_configs;
 };
 
+/*
+ * Look up attr.config by the CPU number carried in event->auxtrace.cpu.
+ * Returns 0 if not found.
+ */
+static u64 htm_config_for_cpu(struct powerpc_htm *htm, int cpu)
+{
+	int i;
+
+	for (i = 0; i < htm->nr_cpu_configs; i++) {
+		if (htm->cpu_configs[i].cpu == cpu)
+			return htm->cpu_configs[i].config;
+	}
+	return 0;
+}
+
 static void powerpc_htm_dump_event(u64 len)
 {
 	const char *color = PERF_COLOR_BLUE;
@@ -34,22 +83,237 @@ static void powerpc_htm_dump_event(u64 len)
 	}
 }
 
-static int powerpc_htm_process_event(struct perf_session *session __maybe_unused,
-				     union perf_event *event __maybe_unused,
-				     struct perf_sample *sample __maybe_unused,
+#define HTM_MEM_ENTRY_SIZE 32
+
+static inline u32 htm_pack_target(u32 node, u32 chip, u32 core)
+{
+	return (node << 16) | (chip << 8) | core;
+}
+
+static bool htm_target_seen(u32 *targets, int *nr, int capacity, u32 key)
+{
+	int i;
+
+	for (i = 0; i < *nr; i++) {
+		if (targets[i] == key)
+			return true;
+	}
+
+	if (*nr < capacity)
+		targets[(*nr)++] = key;
+	else {
+		pr_warning("htm: too many targets (max %d), appending to existing file\n",
+			   capacity);
+		return true;  /* treat as seen: use O_APPEND not O_TRUNC */
+	}
+
+	return false;
+}
+
+/*
+ * Write HTM data to a file.
+ *
+ * mem_maps == 0: AUX bus-trace path  -> htm.bin.nX.pX.cX
+ * mem_maps != 0: memory config path  -> translation.nX.pX.cX
+ *
+ * htm_target_seen() decides O_TRUNC (first write this run) vs O_APPEND
+ * (subsequent writes), keyed on what this process has already written --
+ * not on whether the file exists on disk.
+ */
+static int write_htm(struct powerpc_htm *htm, void *data, size_t size,
+		     u32 node, u32 chip, u32 core, int mem_maps)
+{
+	u32 target_key = htm_pack_target(node, chip, core);
+	char target_file[128];
+	size_t written;
+	int flags;
+	int fd;
+
+	if (!data || !size)
+		return -EINVAL;
+
+	flags = O_CREAT | O_WRONLY | O_NOFOLLOW | O_CLOEXEC;
+
+	if (mem_maps) {
+		uint8_t *byte_ptr = (uint8_t *)data;
+		size_t entries;
+		size_t payload;
+
+		if (size < HTM_MEM_ENTRY_SIZE) {
+			pr_err("Malformed memory mapping entry trace segment\n");
+			return -EINVAL;
+		}
+
+		/* Entry count is at offset 0x10; add 1 for the 32-byte header */
+		entries = get_unaligned_be64(byte_ptr + 0x10) + 1;
+		payload = entries * HTM_MEM_ENTRY_SIZE;
+
+		if (payload != size) {
+			pr_err("Bad memory mapping data, invalid number of entries\n");
+			return -EINVAL;
+		}
+
+		snprintf(target_file, sizeof(target_file),
+			 "translation.n%d.p%d.c%d", node, chip, core);
+		flags |= htm_target_seen(htm->translation_targets,
+					 &htm->nr_translation_targets,
+					 htm->nr_targets,
+					 target_key) ? O_APPEND : O_TRUNC;
+		fd = open(target_file, flags, 0644);
+		if (fd == -1) {
+			pr_err("Failed to open %s: %s\n", target_file, strerror(errno));
+			return -errno;
+		}
+
+		written = write(fd, data, payload);
+		close(fd);
+
+		if (written != payload) {
+			pr_err("Failed to write memory config: expected %zu bytes, wrote %zu\n",
+			       payload, written);
+			return -EIO;
+		}
+
+		return 0;
+	}
+
+	/* AUX bus-trace path */
+	snprintf(target_file, sizeof(target_file),
+		 "htm.bin.n%d.p%d.c%d", node, chip, core);
+	flags |= htm_target_seen(htm->htm_bin_targets,
+				 &htm->nr_htm_bin_targets,
+				 htm->nr_targets,
+				 target_key) ? O_APPEND : O_TRUNC;
+	fd = open(target_file, flags, 0644);
+	if (fd == -1) {
+		pr_err("Failed to open %s: %s\n", target_file, strerror(errno));
+		return -errno;
+	}
+
+	written = write(fd, data, size);
+	close(fd);
+
+	if (written != size) {
+		pr_err("Failed to write htm trace data: expected %zu bytes, wrote %zu\n",
+		       size, written);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int powerpc_htm_process_event(struct perf_session *session,
+				     union perf_event *event,
+				     struct perf_sample *sample,
 				     const struct perf_tool *tool __maybe_unused)
 {
+	struct powerpc_htm *htm;
+	struct evsel *evsel;
+	u32 node, chip, core;
+	u64 ev_config;
+
+	if (!session || !session->auxtrace || !event || !sample)
+		return 0;
+
+	if (event->header.type != PERF_RECORD_SAMPLE || !sample->raw_data)
+		return 0;
+
+	htm = container_of(session->auxtrace, struct powerpc_htm, auxtrace);
+	evsel = evlist__event2evsel(session->evlist, event);
+
+	if (!evsel || strcmp(evsel__pmu_name(evsel), "htm") != 0)
+		return 0;
+
+	ev_config = evsel->core.attr.config;
+	node = (ev_config >> 4)  & 0xff;
+	chip = (ev_config >> 12) & 0xff;
+	core = (ev_config >> 20) & 0xff;
+
+	/*
+	 * raw_size includes 4 bytes of u64 alignment padding added by the
+	 * kernel.  Subtract sizeof(u32) to recover the true payload byte count.
+	 * Guard against underflow: if raw_size is too small, skip silently.
+	 */
+	if (sample->raw_size <= sizeof(uint32_t))
+		return 0;
+	if (write_htm(htm, sample->raw_data,
+		      sample->raw_size - sizeof(uint32_t),
+		      node, chip, core, 1) < 0) {
+		pr_err("Failed to write memory translation block\n");
+		return -EIO;
+	}
+
 	return 0;
 }
 
-static int powerpc_htm_process_auxtrace_event(struct perf_session *session __maybe_unused,
+static int powerpc_htm_process_auxtrace_event(struct perf_session *session,
 					      union perf_event *event,
 					      const struct perf_tool *tool __maybe_unused)
 {
+	struct powerpc_htm *htm;
+	struct auxtrace_buffer *buffer;
+	off_t data_offset;
+	u32 node, chip, core;
+	u64 ev_config;
+	int fd;
+	int err;
+
+	if (!session || !session->auxtrace)
+		return 0;
+
+	htm = container_of(session->auxtrace, struct powerpc_htm, auxtrace);
+	fd = perf_data__fd(session->data);
+
+	if (perf_data__is_pipe(session->data)) {
+		data_offset = 0;
+	} else {
+		data_offset = lseek(fd, 0, SEEK_CUR);
+		if (data_offset == -1)
+			return -errno;
+	}
+
+	/*
+	 * Queue the buffer and get back a pointer to it.  We immediately load
+	 * and write the data so htm.bin.* exists on disk before subsequent
+	 * patches invoke htmdecode during the same session pass.
+	 */
+	err = auxtrace_queues__add_event(&htm->queues, session, event,
+					 data_offset, &buffer);
+	if (err)
+		return err;
+
+	if (!buffer)
+		return 0;
+
+	/*
+	 * Map event->auxtrace.cpu -> attr.config using the table built from
+	 * auxtrace_info->priv[] at init time.  This is reliable because
+	 * htm_info_fill() stored the exact (cpu, config) pair for each evsel
+	 * at record time -- no CPU map or evlist iteration needed here.
+	 */
+	ev_config = htm_config_for_cpu(htm, (int)event->auxtrace.cpu);
+	if (!ev_config) {
+		pr_err("htm: no config found for auxtrace cpu %u\n",
+		       event->auxtrace.cpu);
+		return 0;
+	}
+
+	node = (ev_config >> 4)  & 0xff;
+	chip = (ev_config >> 12) & 0xff;
+	core = (ev_config >> 20) & 0xff;
+
+	if (!auxtrace_buffer__get_data(buffer, fd)) {
+		pr_err("Failed to read AUX buffer data\n");
+		return -ENOMEM;
+	}
+
 	if (dump_trace)
-		powerpc_htm_dump_event(event->auxtrace.size);
+		powerpc_htm_dump_event(buffer->size);
 
-	return 0;
+	err = write_htm(htm, buffer->data, buffer->size, node, chip, core, 0);
+	auxtrace_buffer__put_data(buffer);
+
+	return err < 0 ? err : 0;
 }
 
 static int powerpc_htm_flush(struct perf_session *session __maybe_unused,
@@ -79,6 +343,9 @@ static void powerpc_htm_free(struct perf_session *session)
 	htm = container_of(session->auxtrace, struct powerpc_htm, auxtrace);
 	powerpc_htm_free_events(session);
 	session->auxtrace = NULL;
+	free(htm->cpu_configs);
+	free(htm->htm_bin_targets);
+	free(htm->translation_targets);
 	free(htm);
 }
 
@@ -87,6 +354,8 @@ int powerpc_htm_process_auxtrace_info(union perf_event *event,
 {
 	struct perf_record_auxtrace_info *auxtrace_info = &event->auxtrace_info;
 	struct powerpc_htm *htm;
+	u64 num_events;
+	u64 i;
 	int err;
 
 	if (auxtrace_info->header.size < sizeof(struct perf_record_auxtrace_info) +
@@ -97,12 +366,68 @@ int powerpc_htm_process_auxtrace_info(union perf_event *event,
 	if (!htm)
 		return -ENOMEM;
 
+	/* Reject duplicate AUXTRACE_INFO: would overwrite session->auxtrace and leak */
+	if (session->auxtrace) {
+		pr_err("htm: duplicate PERF_RECORD_AUXTRACE_INFO, ignoring\n");
+		free(htm);
+		return -EINVAL;
+	}
+
+	htm->auxtrace_type = auxtrace_info->priv[POWERPC_HTM_PMU_TYPE];
+	num_events = auxtrace_info->priv[POWERPC_HTM_NUM_EVENTS];
+
+	/*
+	 * Validate num_events against the actual header size before using it
+	 * to index priv[] or allocate arrays.  Each event contributes 2 u64
+	 * priv entries (cpu + config); reject if the header is too small.
+	 */
+	if (num_events > (auxtrace_info->header.size -
+			  sizeof(struct perf_record_auxtrace_info) -
+			  HTM_AUXTRACE_PRIV_FIXED) / (2 * sizeof(u64))) {
+		pr_err("htm: num_events %llu exceeds auxtrace_info payload\n",
+		       (unsigned long long)num_events);
+		free(htm);
+		return -EINVAL;
+	}
+
 	err = auxtrace_queues__init(&htm->queues);
 	if (err) {
 		free(htm);
 		return err;
 	}
 
+	/*
+	 * All three arrays are sized to num_events -- the exact count of HTM
+	 * targets written by htm_info_fill() at record time.  num_events has
+	 * been validated above so the cast to int and size_t are safe.
+	 */
+	htm->nr_targets   = (int)num_events;
+	htm->cpu_configs  = calloc((size_t)num_events, sizeof(*htm->cpu_configs));
+	htm->htm_bin_targets    = calloc((size_t)num_events, sizeof(*htm->htm_bin_targets));
+	htm->translation_targets = calloc((size_t)num_events, sizeof(*htm->translation_targets));
+	if (!htm->cpu_configs || !htm->htm_bin_targets || !htm->translation_targets) {
+		free(htm->cpu_configs);
+		free(htm->htm_bin_targets);
+		free(htm->translation_targets);
+		auxtrace_queues__free(&htm->queues);
+		free(htm);
+		return -ENOMEM;
+	}
+
+	/*
+	 * Read (cpu, config) pairs from priv[].  These were written by
+	 * htm_info_fill() at record time -- one pair per htm evsel in evlist
+	 * order.  Keying by CPU lets process_auxtrace_event() look up the
+	 * correct attr.config for each AUX buffer using event->auxtrace.cpu.
+	 */
+	for (i = 0; i < num_events; i++) {
+		htm->cpu_configs[i].cpu =
+			(int)auxtrace_info->priv[POWERPC_HTM_EVENT_DATA + i * 2];
+		htm->cpu_configs[i].config =
+			auxtrace_info->priv[POWERPC_HTM_EVENT_DATA + i * 2 + 1];
+		htm->nr_cpu_configs++;
+	}
+
 	htm->session = session;
 	htm->machine = &session->machines.host;
 	htm->auxtrace.process_event = powerpc_htm_process_event;
-- 
2.43.0

Re: [PATCH V3 3/6] tools/perf: Add arch hook to drain remaining data before event close

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] SIGCHLD incorrectly aborts the final data drain loop, bypassing the intended double Ctrl+C behavior for child workloads.
- [Medium] Final data drain fails to read worker threads' mmaps when --threads is enabled.
--

--- Patch [3]: [PATCH V3 3/6] tools/perf: Add arch hook to drain remaining data before event close ---
commit ed7d9849ca86c7b6e1369882d695adbc00355511
Author: Athira Rajeev <atrajeev@linux.ibm.com>

tools/perf: Add arch hook to drain remaining data before event close

While collecting samples using perf record, __cmd_record() disables the evlist
once recording is complete. After that, event fds are no longer read and any
remaining PMU-specific data cannot be drained. This patch adds a weak
arch_perf_record__need_read() hook so architecture code can indicate that
more data remains to be collected before events are disabled and closed.

> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index e915390556752..e158197c2d15c 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c

[ ... ]

> @@ -693,6 +694,8 @@ static void sig_handler(int sig)
>  	else
>  		signr = sig;
>  
> +	if (done)
> +		drain_interrupted = 1;

[Severity: Medium]
Will this unconditionally set drain_interrupted when a child workload
terminates and sends SIGCHLD?

If running perf record on a child workload and interrupting it with a single
Ctrl+C, the child workload terminates and sends SIGCHLD. If done was already
set by the SIGINT, would this cause the single SIGCHLD to prematurely abort
the drain loop intended for double Ctrl+C?

>  	done = 1;
>  #ifdef HAVE_EVENTFD_SUPPORT
>  	if (done_fd >= 0) {

[ ... ]

> @@ -2425,6 +2428,54 @@ static unsigned long record__waking(struct record *rec)
>  	return waking;
>  }
>  
> +/*
> + * Weak symbol - architecture can override to indicate if more
> + * data needs to be collected before finishing output.
> + *
> + * Returns: 1 if more data exists, 0 if collection is complete
> + */
> +__weak int arch_perf_record__need_read(struct evlist *evlist __maybe_unused)
> +{
> +	return 0;  /* Default: no arch-specific data to collect */
> +}
> +
> +static void record__final_data(struct record *rec)
> +{
> +	u64 last_bytes_written = 0;
> +	int retries = 0;
> +#define FINAL_DATA_MAX_RETRIES 20  /* 20 * 1 ms = 20 ms max wait */
> +
> +	/*
> +	 * Collect any remaining architecture-specific data.
> +	 * The arch code checks if more data exists, and we do the actual
> +	 * reading here since we have access to record__mmap_read_all().
> +	 * This code performs the additional read pass while events are
> +	 * still live.  A second SIGINT during drain sets drain_interrupted
> +	 * and aborts the loop immediately.
> +	 */
> +	while (arch_perf_record__need_read(rec->evlist)) {
> +		if (drain_interrupted)
> +			break;
> +
> +		last_bytes_written = record__bytes_written(rec);
> +
> +		if (record__mmap_read_all(rec, true) < 0)

[Severity: Medium]
When using the --threads option, will this fail to read worker threads' mmaps?

Looking at record__mmap_read_evlist(), the read target is mapped to
thread->maps (a __thread thread-local variable):

    nr_mmaps = thread->nr_mmaps;
    maps = overwrite ? thread->overwrite_maps : thread->maps;

Since record__final_data() is executed on the main thread, it seems it will
only access the main thread's mmaps and completely skip worker mmaps. Could
this lead to a 20ms spin loop and premature event disablement while worker
threads remain blocked in poll?

> +			break;
> +
> +		if (record__bytes_written(rec) == last_bytes_written) {
> +			if (++retries >= FINAL_DATA_MAX_RETRIES) {
> +				pr_warning("Final data drain made no forward progress after %d retries.\n",
> +					   FINAL_DATA_MAX_RETRIES);
> +				break;
> +			}
> +			usleep(1000); /* 1 ms: let AUX ring buffer consumer advance */
> +		} else {
> +			retries = 0;
> +			usleep(100);
> +		}
> +	}
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725070747.81435-1-atrajeev@linux.ibm.com?part=3

Re: [PATCH V3 5/6] tools/perf: Add powerpc HTM auxtrace event processing support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The file `powerpc-htm.c` calls `zalloc()` but fails to include `<linux/zalloc.h>`, resulting in an implicit function declaration.
- [Low] The file `powerpc-htm.c` embeds `struct auxtrace_queues` and calls its API but fails to directly include `"auxtrace.h"`, relying on brittle implicit inclusion.
--

--- Patch [5]: [PATCH V3 5/6] tools/perf: Add powerpc HTM auxtrace event processing support ---
commit 306ea78646ef6e5fe95dbad9977aff1241d1027a
Author: Athira Rajeev <atrajeev@linux.ibm.com>

tools/perf: Add powerpc HTM auxtrace event processing support

This commit adds the powerpc-htm.c decode stub for powerpc HTM auxtrace.
It wires the dispatch in perf_event__process_auxtrace_info() to call
powerpc_htm_process_auxtrace_info().

> diff --git a/tools/perf/util/powerpc-htm.c b/tools/perf/util/powerpc-htm.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/perf/util/powerpc-htm.c
> @@ -0,0 +1,116 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <inttypes.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <endian.h>
> +#include "util/evsel.h"
> +#include "util/evlist.h"
> +#include "util/session.h"
> +#include "color.h"
> +#include "powerpc-htm.h"
> +#include "debug.h"
> +#include "sample.h"
> +
> +struct powerpc_htm {
> +	struct auxtrace		auxtrace;
> +	struct auxtrace_queues	queues;

[Severity: Low]
Should this file explicitly include "auxtrace.h"?

The code embeds struct auxtrace_queues here, and later calls
auxtrace_queues__init(), both of which are defined in "auxtrace.h". Relying
on implicit inclusion (for example, through "util/evlist.h") currently works
but could break the build if intermediate headers are refactored in the
future.

[ ... ]

> +int powerpc_htm_process_auxtrace_info(union perf_event *event,
> +				      struct perf_session *session)
> +{
> +	struct perf_record_auxtrace_info *auxtrace_info = &event->auxtrace_info;
> +	struct powerpc_htm *htm;
> +	int err;
> +
> +	if (auxtrace_info->header.size < sizeof(struct perf_record_auxtrace_info) +
> +					 HTM_AUXTRACE_PRIV_FIXED)
> +		return -EINVAL;
> +
> +	htm = zalloc(sizeof(struct powerpc_htm));

[Severity: High]
Does this implicit function declaration cause a build failure or pointer
truncation?

The function zalloc() is used here, but <linux/zalloc.h> is not included in
this file. Because the kernel build system uses -Werror, this missing header
could trigger a build failure.

If -Werror were disabled, the compiler would assume zalloc() returns a 32-bit
integer. On 64-bit architectures like PowerPC, this truncates the 64-bit
pointer. When extended back to 64 bits during assignment, the higher bits are
lost, which could result in a segmentation fault upon dereference.

Should <linux/zalloc.h> be included to ensure the return type is properly
defined?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725070747.81435-1-atrajeev@linux.ibm.com?part=5

Re: [PATCH V3 2/6] tools/perf: Add AUXTRACE recording support for powerpc HTM

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Concurrent use of VPA DTL and HTM auxtrace events silently drops HTM initialization, producing corrupted trace data.
- [High] System-wide and multi-CPU profiles fail to record CPU mappings for all but the first CPU, causing decoding failures.
- [Medium] Missing explicit `<string.h>` inclusion for `strcmp()` breaks musl libc compilation.
- [Medium] Redundant `pos->name` NULL checks incorrectly skip valid unnamed HTM events.
--

--- Patch [2]: [PATCH V3 2/6] tools/perf: Add AUXTRACE recording support for powerpc HTM ---
commit 00980957c6d482ec39e6aa18c32409446efcd20e
Author: Athira Rajeev <atrajeev@linux.ibm.com>

tools/perf: Add AUXTRACE recording support for powerpc HTM

This commit adds powerpc perf tool support for the HTM PMU AUXTRACE
recording path. It introduces initialization and callbacks to create
PERF_RECORD_AUXTRACE records for HTM data, populating mapping information
for decoding.

> diff --git a/tools/perf/arch/powerpc/util/auxtrace.c b/tools/perf/arch/powerpc/util/auxtrace.c
> index e04a0bd617557..74f0a37236b8f 100644
> --- a/tools/perf/arch/powerpc/util/auxtrace.c
> +++ b/tools/perf/arch/powerpc/util/auxtrace.c

[ ... ]

> @@ -32,11 +34,18 @@ struct auxtrace_record *auxtrace_record__init(struct evlist *evlist,
>  			/* Remember the first matching VPA DTL event */
>  			if (!vpa_dtl_evsel)
>  				vpa_dtl_evsel = pos;
> +		} else if (pos->name && (!strcmp(evsel__pmu_name(pos), "htm"))) {

[Severity: Medium]
Since evsel__pmu_name() accurately identifies the PMU name regardless of
pos->name, does the pos->name check silently ignore valid HTM evsels that
might be created dynamically and lack a name?

Also, this code calls strcmp() but the file doesn't explicitly include
<string.h>. Will this break musl libc builds that strictly enforce POSIX
header inclusions without implicit dependencies?

> +			pos->needs_auxtrace_mmap = true;
> +			/* Remember the first matching HTM event */
> +			if (!htm_evsel)
> +				htm_evsel = pos;
>  		}
>  	}
>  
>  	if (vpa_dtl_evsel)
>  		return vpa_dtl_recording_init(vpa_dtl_evsel, err);
> +	else if (htm_evsel)
> +		return htm_recording_init(htm_evsel, err);

[Severity: High]
What happens if a user requests both VPA DTL and HTM auxtrace PMUs
concurrently?

It looks like the code will initialize VPA DTL and return immediately,
completely skipping HTM initialization. Since both had needs_auxtrace_mmap
set to true, will this result in HTM aux buffers being collected without
writing the required PERF_RECORD_AUXTRACE_INFO metadata, causing decoding
failures?

Should this explicitly reject concurrent auxtrace PMUs instead?

>  	return NULL;
>  }

> diff --git a/tools/perf/arch/powerpc/util/htm.c b/tools/perf/arch/powerpc/util/htm.c
> new file mode 100644
> index 0000000000000..9a12a8fe8027f
> --- /dev/null
> +++ b/tools/perf/arch/powerpc/util/htm.c

[ ... ]

> +static int
> +htm_recording_options(struct auxtrace_record *itr __maybe_unused,
> +		      struct evlist *evlist,
> +		      struct record_opts *opts)
> +{
> +	struct evsel *pos;
> +
> +	opts->full_auxtrace = true;
> +
> +	if (!opts->auxtrace_mmap_pages) {
> +		opts->auxtrace_mmap_pages = KiB(128) / page_size;
> +		if (opts->mmap_pages == UINT_MAX)
> +			opts->mmap_pages = KiB(256) / page_size;
> +	}
> +
> +	evlist__for_each_entry(evlist, pos) {
> +		if (!pos->name || (strcmp(evsel__pmu_name(pos), "htm")))
> +			continue;

[Severity: Medium]
Similar to auxtrace_record__init(), does the pos->name check unnecessarily
skip unnamed events?

Additionally, strcmp() is used here without explicitly including <string.h>.
Could this cause compilation failures on musl libc systems?

> +		pos->core.attr.aux_watermark =
> +			opts->auxtrace_mmap_pages * (size_t)page_size / 2;
> +		pos->core.attr.freq = 0;
> +		pos->core.attr.sample_type |= PERF_SAMPLE_RAW;
> +		pos->needs_auxtrace_mmap = true;
> +	}
> +
> +	return 0;
> +}

[ ... ]

> +/* Count htm evsels in the evlist */
> +static int htm_nr_events(struct evlist *evlist)
> +{
> +	struct evsel *pos;
> +	int n = 0;
> +
> +	evlist__for_each_entry(evlist, pos) {
> +		if (pos->name && (!strcmp(evsel__pmu_name(pos), "htm")))
> +			n++;
> +	}
> +	return n;
> +}

[ ... ]

> +static int
> +htm_info_fill(struct auxtrace_record *itr,
> +	      struct perf_session *session,
> +	      struct perf_record_auxtrace_info *auxtrace_info,
> +	      size_t priv_size)
> +{

[ ... ]

> +	evlist__for_each_entry(evlist, pos) {
> +		struct perf_cpu_map *cpus;
> +		int cpu;
> +
> +		if (!pos->name || (strcmp(evsel__pmu_name(pos), "htm")))
> +			continue;
> +
> +		/*
> +		 * Get the CPU this evsel is pinned to.  For events opened
> +		 * with cpu=N, evsel__cpus() returns a single-entry map {N}
> +		 * at record time (not during replay).
> +		 */
> +		cpus = evsel__cpus(pos);
> +		if (cpus && perf_cpu_map__nr(cpus) > 0)
> +			cpu = perf_cpu_map__cpu(cpus, 0).cpu;

[Severity: High]
When an event is opened system-wide or with multiple CPUs, will this loop
miss CPU mappings for all but the first CPU?

By extracting only index 0 from the perf_cpu_map, the resulting metadata
will lack mappings for CPU 1 and beyond. When the auxtrace decoder tries to
decode data from these additional CPUs, will it fail to find the required
event->auxtrace.cpu mapping?

Also, since htm_nr_events() only counts the number of evsels rather than
the total number of CPUs across the maps, would the priv array even be
large enough to hold metadata for all CPUs in a multi-CPU profile?

> +		else
> +			cpu = -1;
> +
> +		auxtrace_info->priv[POWERPC_HTM_EVENT_DATA + n * 2]     = cpu;
> +		auxtrace_info->priv[POWERPC_HTM_EVENT_DATA + n * 2 + 1] =
> +							pos->core.attr.config;
> +		n++;
> +	}
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725070747.81435-1-atrajeev@linux.ibm.com?part=2

Re: [PATCH V3 6/6] tools/perf: Add perf tool support for processing powerpc HTM AUXTRACE records

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] The implementation is missing the HTM_MAX_SAFE_TARGETS (1024) cap for num_events that was explicitly promised in the commit message.
- [Medium] htm_config_for_cpu conflates a lookup failure with a valid 0 PMU configuration, causing valid auxtrace events to be dropped.
- [Low] Assigning write() return value to a size_t variable masks the actual error code and produces misleading log messages.
- [Low] Missing explicit inclusion of POSIX standard headers required for musl libc compatibility.
--

--- Patch [6]: [PATCH V3 6/6] tools/perf: Add perf tool support for processing powerpc HTM AUXTRACE records ---
commit b319ae58a675d493de8d8d9853fc6d781771f6b8
Author: Athira Rajeev <atrajeev@linux.ibm.com>

tools/perf: Add perf tool support for processing powerpc HTM AUXTRACE records

powerpc_htm_process_auxtrace_info() reads the (cpu, attr.config) pairs
stored by htm_info_fill() at record time and builds a cpu_configs[]
table.  This lets process_auxtrace_event() map each AUX buffer, which
carries event->auxtrace.cpu, to the correct (node, chip, core) target
and write the raw trace data to htm.bin.nX.pX.cX immediately.

[ ... ]
> - Cap num_events at HTM_MAX_SAFE_TARGETS (1024) to prevent size_t
> truncation on 32-bit platforms when passed to calloc().

[Severity: Medium]
This isn't a bug, but the commit message mentions capping num_events at 1024
(HTM_MAX_SAFE_TARGETS).

Looking at powerpc_htm_process_auxtrace_info(), the arrays are allocated
using num_events directly:

tools/perf/util/powerpc-htm.c:powerpc_htm_process_auxtrace_info() {
    ...
    htm->nr_targets   = (int)num_events;
    htm->cpu_configs  = calloc((size_t)num_events, sizeof(*htm->cpu_configs));
    ...
}

Has the 1024 maximum cap and truncation prevention logic been omitted here?

> diff --git a/tools/perf/util/powerpc-htm.c b/tools/perf/util/powerpc-htm.c
> index 952ed32997d9a..1d1b4302e2e4a 100644
> --- a/tools/perf/util/powerpc-htm.c
> +++ b/tools/perf/util/powerpc-htm.c
> @@ -12,9 +12,14 @@
>  #include "util/session.h"
>  #include "color.h"
>  #include "powerpc-htm.h"
> +#include <errno.h>
>  #include "debug.h"
>  #include "sample.h"
>  
> +#include <linux/unaligned.h>
> +
> +struct perf_session;

[Severity: Low]
Are the standard POSIX headers missing here?

The code uses functions like open(), write(), lseek(), snprintf(), strerror(),
calloc(), and free().

Without explicitly including fcntl.h, unistd.h, stdio.h, string.h, and
stdlib.h, could this fail to compile on systems using musl libc due to
relying on implicit header inclusions?

[ ... ]
> +/*
> + * Look up attr.config by the CPU number carried in event->auxtrace.cpu.
> + * Returns 0 if not found.
> + */
> +static u64 htm_config_for_cpu(struct powerpc_htm *htm, int cpu)
> +{
> +	int i;
> +
> +	for (i = 0; i < htm->nr_cpu_configs; i++) {
> +		if (htm->cpu_configs[i].cpu == cpu)
> +			return htm->cpu_configs[i].config;
> +	}
> +	return 0;
> +}

[Severity: Medium]
Can a valid configuration evaluate to 0?

If the HTM PMU is configured with node=0, chip=0, core=0 and no other flags,
attr.config could legitimately be 0.

If it returns 0, the caller powerpc_htm_process_auxtrace_event() checks:

tools/perf/util/powerpc-htm.c:powerpc_htm_process_auxtrace_event() {
    ...
    ev_config = htm_config_for_cpu(htm, (int)event->auxtrace.cpu);
    if (!ev_config) {
        ...
        return 0;
    }
    ...
}

Would this cause valid auxtrace events to be dropped by conflating a lookup
failure with a valid 0 PMU configuration?

[ ... ]
> +static int write_htm(struct powerpc_htm *htm, void *data, size_t size,
> +		     u32 node, u32 chip, u32 core, int mem_maps)
> +{
> +	u32 target_key = htm_pack_target(node, chip, core);
> +	char target_file[128];
> +	size_t written;
> +	int flags;
> +	int fd;
[ ... ]
> +		written = write(fd, data, payload);
> +		close(fd);
> +
> +		if (written != payload) {
> +			pr_err("Failed to write memory config: expected %zu bytes, wrote %zu\n",
> +			       payload, written);
> +			return -EIO;
> +		}
> +
> +		return 0;
> +	}
> +
> +	/* AUX bus-trace path */
> +	snprintf(target_file, sizeof(target_file),
> +		 "htm.bin.n%d.p%d.c%d", node, chip, core);
> +	flags |= htm_target_seen(htm->htm_bin_targets,
> +				 &htm->nr_htm_bin_targets,
> +				 htm->nr_targets,
> +				 target_key) ? O_APPEND : O_TRUNC;
> +	fd = open(target_file, flags, 0644);
> +	if (fd == -1) {
> +		pr_err("Failed to open %s: %s\n", target_file, strerror(errno));
> +		return -errno;
> +	}
> +
> +	written = write(fd, data, size);
> +	close(fd);
> +
> +	if (written != size) {
> +		pr_err("Failed to write htm trace data: expected %zu bytes, wrote %zu\n",
> +		       size, written);
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}

[Severity: Low]
Does this code assign the signed return value of write() to a size_t variable?

If write() fails and returns -1, it will be assigned to size_t written,
wrapping it to SIZE_MAX.

While the error is still caught by the written != payload and written != size
checks, will this result in logging an impossibly large value for bytes written
and mask the actual error code?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725070747.81435-1-atrajeev@linux.ibm.com?part=6