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

[Adrian Hunter <adrian.hunter@intel.com>]

On 07/08/2026 17:41, Athira Rajeev wrote:
> 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>

A couple of very minor comments.

Nevertheless:

Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
> Changes in V5:
> - htm_recording_options(): clamp aux_watermark with min_t(size_t, wm,
>   UINT_MAX) before assigning to pos->core.attr.aux_watermark (u32).
>   On a system with large auxtrace_mmap_pages the unclamped product can
>   exceed UINT_MAX and silently truncate on 32-bit platforms.
> - htm_info_fill(): iterate all CPUs in each evsel's CPU map and write
>   one (cpu, config) entry per CPU instead of only the first CPU.  An
>   evsel opened with -C 0,1,2 now contributes three priv[] entries, one
>   per CPU, each carrying the same attr.config.  The decode side
>   (powerpc_htm_process_auxtrace_event) uses event->auxtrace.cpu to look
>   up the matching entry, so every CPU's AUX buffer is correctly mapped
>   to its (node, chip, core, type) target and written to the right output
>   file.  An evsel with no CPU map records cpu=-1 as before.
> 
> Changes in V4:
> - Add explicit mutual-exclusion check in auxtrace_record__init(): if
>   both vpa_dtl_evsel and htm_evsel are set after scanning the evlist,
>   print an error, set *err = -EINVAL, and return NULL.  V3 silently
>   fell through to prefer VPA-DTL when both PMU types were present,
>   leaving HTM AUX buffers collected without a
>   PERF_RECORD_AUXTRACE_INFO record and the trace undecodable.
> - Drop the superfluous pos->name && null-guard from the HTM detection
>   condition in auxtrace_record__init(): the condition becomes
>   !strcmp(evsel__pmu_name(pos), "htm") without the leading check.
>   evsel__pmu_name() handles a NULL name internally so the guard is
>   redundant.
> - Apply the same simplification in htm_recording_options() and
>   htm_nr_events() in htm.c: remove the !pos->name || prefix from
>   the evsel__pmu_name() comparisons, consistent with the dispatcher
>   change above.
> - Set pos->core.attr.sample_period = 1 for each HTM evsel inside
>   htm_recording_options().  V3 set freq=0 and aux_watermark but did
>   not set sample_period, leaving the kernel to use whatever default
>   value was in the attr, which could cause the HTM event to fire at
>   an unintended rate.
> 
> 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 |  22 +++
>  tools/perf/arch/powerpc/util/htm.c      | 185 ++++++++++++++++++++++++
>  tools/perf/util/auxtrace.c              |   2 +
>  tools/perf/util/auxtrace.h              |   1 +
>  tools/perf/util/powerpc-htm.h           |  43 ++++++
>  6 files changed, 254 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..9936d1910256 100644
> --- a/tools/perf/arch/powerpc/util/auxtrace.c
> +++ b/tools/perf/arch/powerpc/util/auxtrace.c
> @@ -12,13 +12,16 @@
>  #include "../../util/debug.h"
>  #include "../../util/auxtrace.h"
>  #include "../../util/powerpc-vpadtl.h"
> +#include "../../util/powerpc-htm.h"
>  #include "../../util/record.h"
> +#include <string.h>

There is "#include <errno.h>" further up.  Maybe nicer to put them
together as standard library includes

>  
>  struct auxtrace_record *auxtrace_record__init(struct evlist *evlist,
>  						int *err)
>  {
>  	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 +35,30 @@ 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 (!strcmp(evsel__pmu_name(pos), "htm")) {
> +			pos->needs_auxtrace_mmap = true;
> +			/* Remember the first matching HTM event */
> +			if (!htm_evsel)
> +				htm_evsel = pos;
>  		}
>  	}
>  
> +	/*
> +	 * Only one auxtrace PMU can be initialised per session.  Reject
> +	 * concurrent VPA DTL and HTM events: HTM AUX buffers would be
> +	 * collected without a PERF_RECORD_AUXTRACE_INFO record, making
> +	 * the trace undecodable.
> +	 */
> +	if (vpa_dtl_evsel && htm_evsel) {
> +		pr_err("Cannot record VPA DTL and HTM auxtrace events simultaneously\n");
> +		*err = -EINVAL;
> +		return NULL;
> +	}
> +
>  	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..d2fdef488032
> --- /dev/null
> +++ b/tools/perf/arch/powerpc/util/htm.c
> @@ -0,0 +1,185 @@
> +// 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) {
> +		size_t wm;
> +
> +		if (strcmp(evsel__pmu_name(pos), "htm"))
> +			continue;
> +		wm = opts->auxtrace_mmap_pages * (size_t)page_size / 2;
> +		pos->core.attr.aux_watermark = min_t(size_t, wm, UINT_MAX);
> +		pos->core.attr.sample_type |= PERF_SAMPLE_RAW;
> +		pos->core.attr.freq = 0;
> +		pos->core.attr.sample_period = 1;
> +		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 (!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] = total number of HTM CPU entries
> + *   priv[POWERPC_HTM_EVENT_DATA + n*2]     = CPU number for nth entry
> + *   priv[POWERPC_HTM_EVENT_DATA + n*2 + 1] = attr.config for nth entry
> + *
> + * One entry is written per CPU in each evsel's cpu map.  An evsel opened
> + * with -C 0,1,2 contributes three entries (one per CPU), each carrying
> + * the same attr.config.  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 i, nr;
> +
> +		if (strcmp(evsel__pmu_name(pos), "htm"))
> +			continue;
> +
> +		/*
> +		 * Emit one (cpu, config) entry for every CPU in this evsel's
> +		 * map.  perf record -C 0,1,2 creates one evsel with a
> +		 * three-entry cpu map; each CPU gets its own AUX buffer and
> +		 * must be individually mapped so the decoder can match
> +		 * event->auxtrace.cpu to the correct (node, chip, core).
> +		 */
> +		cpus = evsel__cpus(pos);
> +		nr = cpus ? perf_cpu_map__nr(cpus) : 0;
> +
> +		if (nr > 0) {
> +			for (i = 0; i < nr; i++) {
> +				int cpu = perf_cpu_map__cpu(cpus, i).cpu;
> +
> +				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++;
> +			}
> +		} else {
> +			/* cpu-agnostic evsel: record cpu = -1 */
> +			auxtrace_info->priv[POWERPC_HTM_EVENT_DATA + n * 2]     = (u64)-1;
> +			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 aa749e1c3036..bf08f41d623f 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"
> @@ -1432,6 +1433,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);

Ideally, should only add a declaration for powerpc_htm_process_auxtrace_info()
when it actually exists

> +
> +#endif /* __POWERPC_HTM_H */