[PATCH v3 0/5] perf arm-spe: Introduce metadata version 2

[PATCH v3 0/5] perf arm-spe: Introduce metadata version 2

[Leo Yan]

This patch series enhances Arm SPE metadata in the Perf file to a
version 2 format and maintains backward compatibility for metadata v1.

The version 2 metadata contains a global structure with fields for
metadata header version number, header size, CPU numbers. It also
expands to store per-CPU metadata, which includes the CPU logical
number in the kernel, MIDR, and capacity values associated with Arm SPE.

This patch set has been tested the perf to decode the Arm SPE metadata
v1 and v2.

Changes from v2:
- Added comment for arm_spe_find_cpus (Namhyung)
- Corrected resource releasing in arm_spe_save_cpu_header() and
  arm_spe_info_fill() (Namhyung)
- Changed to use calloc() for allocating metadata pointer array
  (Namhyung)

Changes from v1:
- Dropped LDS bit exposing from Arm SPE driver (Will Deacon).
- To simplify the change, this series did not include multiple AUX event
  support.


Leo Yan (5):
  perf arm-spe: Define metadata header version 2
  perf arm-spe: Calculate meta data size
  perf arm-spe: Save per CPU information in metadata
  perf arm-spe: Support metadata version 2
  perf arm-spe: Dump metadata with version 2

 tools/perf/arch/arm64/util/arm-spe.c | 122 +++++++++++++++++++++-
 tools/perf/util/arm-spe.c            | 151 +++++++++++++++++++++++++--
 tools/perf/util/arm-spe.h            |  38 ++++++-
 3 files changed, 296 insertions(+), 15 deletions(-)

-- 
2.34.1

[PATCH v3 1/5] perf arm-spe: Define metadata header version 2

[Leo Yan]

The first version's metadata header structure doesn't include a field to
indicate a header version, which is not friendly for extension.

Define the metadata version 2 format with a new header structure and
extend per CPU's metadata. In the meantime, the old metadata header will
still be supported for backward compatibility.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/arch/arm64/util/arm-spe.c |  4 +--
 tools/perf/util/arm-spe.c            |  2 +-
 tools/perf/util/arm-spe.h            | 38 +++++++++++++++++++++++++++-
 3 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index 2be99fdf997d..c2d5c8ca4900 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -41,7 +41,7 @@ static size_t
 arm_spe_info_priv_size(struct auxtrace_record *itr __maybe_unused,
 		       struct evlist *evlist __maybe_unused)
 {
-	return ARM_SPE_AUXTRACE_PRIV_SIZE;
+	return ARM_SPE_AUXTRACE_V1_PRIV_SIZE;
 }
 
 static int arm_spe_info_fill(struct auxtrace_record *itr,
@@ -53,7 +53,7 @@ static int arm_spe_info_fill(struct auxtrace_record *itr,
 			container_of(itr, struct arm_spe_recording, itr);
 	struct perf_pmu *arm_spe_pmu = sper->arm_spe_pmu;
 
-	if (priv_size != ARM_SPE_AUXTRACE_PRIV_SIZE)
+	if (priv_size != ARM_SPE_AUXTRACE_V1_PRIV_SIZE)
 		return -EINVAL;
 
 	if (!session->evlist->core.nr_mmaps)
diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
index 138ffc71b32d..70989b1bae47 100644
--- a/tools/perf/util/arm-spe.c
+++ b/tools/perf/util/arm-spe.c
@@ -1262,7 +1262,7 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
 				  struct perf_session *session)
 {
 	struct perf_record_auxtrace_info *auxtrace_info = &event->auxtrace_info;
-	size_t min_sz = sizeof(u64) * ARM_SPE_AUXTRACE_PRIV_MAX;
+	size_t min_sz = ARM_SPE_AUXTRACE_V1_PRIV_SIZE;
 	struct perf_record_time_conv *tc = &session->time_conv;
 	const char *cpuid = perf_env__cpuid(session->evlist->env);
 	u64 midr = strtol(cpuid, NULL, 16);
diff --git a/tools/perf/util/arm-spe.h b/tools/perf/util/arm-spe.h
index 4f4900c18f3e..5416d4e1d15f 100644
--- a/tools/perf/util/arm-spe.h
+++ b/tools/perf/util/arm-spe.h
@@ -12,10 +12,46 @@
 enum {
 	ARM_SPE_PMU_TYPE,
 	ARM_SPE_PER_CPU_MMAPS,
+	ARM_SPE_AUXTRACE_V1_PRIV_MAX,
+};
+
+#define ARM_SPE_AUXTRACE_V1_PRIV_SIZE	\
+	(ARM_SPE_AUXTRACE_V1_PRIV_MAX * sizeof(u64))
+
+enum {
+	/*
+	 * The old metadata format (defined above) does not include a
+	 * field for version number. Version 1 is reserved and starts
+	 * from version 2.
+	 */
+	ARM_SPE_HEADER_VERSION,
+	/* Number of sizeof(u64) */
+	ARM_SPE_HEADER_SIZE,
+	/* PMU type shared by CPUs */
+	ARM_SPE_SHARED_PMU_TYPE,
+	/* Number of CPUs */
+	ARM_SPE_CPUS_NUM,
 	ARM_SPE_AUXTRACE_PRIV_MAX,
 };
 
-#define ARM_SPE_AUXTRACE_PRIV_SIZE (ARM_SPE_AUXTRACE_PRIV_MAX * sizeof(u64))
+enum {
+	/* Magic number */
+	ARM_SPE_MAGIC,
+	/* CPU logical number in system */
+	ARM_SPE_CPU,
+	/* Number of parameters */
+	ARM_SPE_CPU_NR_PARAMS,
+	/* CPU MIDR */
+	ARM_SPE_CPU_MIDR,
+	/* Associated PMU type */
+	ARM_SPE_CPU_PMU_TYPE,
+	/* Minimal interval */
+	ARM_SPE_CAP_MIN_IVAL,
+	ARM_SPE_CPU_PRIV_MAX,
+};
+
+#define ARM_SPE_HEADER_CURRENT_VERSION	2
+
 
 union perf_event;
 struct perf_session;
-- 
2.34.1

[PATCH v3 2/5] perf arm-spe: Calculate meta data size

[Leo Yan]

The metadata is designed to contain a header and per CPU information.

The arm_spe_find_cpus() function is introduced to identify how many CPUs
support ARM SPE. Based on the CPU number, calculates the metadata size.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/arch/arm64/util/arm-spe.c | 39 +++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index c2d5c8ca4900..81d5c07380a4 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -37,11 +37,44 @@ struct arm_spe_recording {
 	bool			*wrapped;
 };
 
+/*
+ * arm_spe_find_cpus() returns a new cpu map, and the caller should invoke
+ * perf_cpu_map__put() to release the map after use.
+ */
+static struct perf_cpu_map *arm_spe_find_cpus(struct evlist *evlist)
+{
+	struct perf_cpu_map *event_cpus = evlist->core.user_requested_cpus;
+	struct perf_cpu_map *online_cpus = perf_cpu_map__new_online_cpus();
+	struct perf_cpu_map *intersect_cpus;
+
+	/* cpu map is not "any" CPU , we have specific CPUs to work with */
+	if (!perf_cpu_map__has_any_cpu(event_cpus)) {
+		intersect_cpus = perf_cpu_map__intersect(event_cpus, online_cpus);
+		perf_cpu_map__put(online_cpus);
+	/* Event can be "any" CPU so count all CPUs. */
+	} else {
+		intersect_cpus = online_cpus;
+	}
+
+	return intersect_cpus;
+}
+
 static size_t
 arm_spe_info_priv_size(struct auxtrace_record *itr __maybe_unused,
-		       struct evlist *evlist __maybe_unused)
+		       struct evlist *evlist)
 {
-	return ARM_SPE_AUXTRACE_V1_PRIV_SIZE;
+	struct perf_cpu_map *cpu_map = arm_spe_find_cpus(evlist);
+	size_t size;
+
+	if (!cpu_map)
+		return 0;
+
+	size = ARM_SPE_AUXTRACE_PRIV_MAX +
+	       ARM_SPE_CPU_PRIV_MAX * perf_cpu_map__nr(cpu_map);
+	size *= sizeof(u64);
+
+	perf_cpu_map__put(cpu_map);
+	return size;
 }
 
 static int arm_spe_info_fill(struct auxtrace_record *itr,
@@ -53,7 +86,7 @@ static int arm_spe_info_fill(struct auxtrace_record *itr,
 			container_of(itr, struct arm_spe_recording, itr);
 	struct perf_pmu *arm_spe_pmu = sper->arm_spe_pmu;
 
-	if (priv_size != ARM_SPE_AUXTRACE_V1_PRIV_SIZE)
+	if (priv_size != arm_spe_info_priv_size(itr, session->evlist))
 		return -EINVAL;
 
 	if (!session->evlist->core.nr_mmaps)
-- 
2.34.1

[PATCH v3 3/5] perf arm-spe: Save per CPU information in metadata

[Leo Yan]

Save the Arm SPE information on a per-CPU basis. This approach is easier
in the decoding phase for retrieving metadata based on the CPU number of
every Arm SPE record.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/arch/arm64/util/arm-spe.c | 83 +++++++++++++++++++++++++++-
 1 file changed, 81 insertions(+), 2 deletions(-)

diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index 81d5c07380a4..0864db70848f 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -26,6 +26,8 @@
 #include "../../../util/arm-spe.h"
 #include <tools/libc_compat.h> // reallocarray
 
+#define ARM_SPE_CPU_MAGIC		0x1010101010101010ULL
+
 #define KiB(x) ((x) * 1024)
 #define MiB(x) ((x) * 1024 * 1024)
 
@@ -77,14 +79,70 @@ arm_spe_info_priv_size(struct auxtrace_record *itr __maybe_unused,
 	return size;
 }
 
+static int arm_spe_save_cpu_header(struct auxtrace_record *itr,
+				   struct perf_cpu cpu, __u64 data[])
+{
+	struct arm_spe_recording *sper =
+			container_of(itr, struct arm_spe_recording, itr);
+	struct perf_pmu *pmu = NULL;
+	struct perf_pmu tmp_pmu;
+	char cpu_id_str[16];
+	char *cpuid = NULL;
+	u64 val;
+
+	snprintf(cpu_id_str, sizeof(cpu_id_str), "%d", cpu.cpu);
+	tmp_pmu.cpus = perf_cpu_map__new(cpu_id_str);
+	if (!tmp_pmu.cpus)
+		return -ENOMEM;
+
+	/* Read CPU MIDR */
+	cpuid = perf_pmu__getcpuid(&tmp_pmu);
+
+	/* The CPU map will not be used anymore, release it */
+	perf_cpu_map__put(tmp_pmu.cpus);
+
+	if (!cpuid)
+		return -ENOMEM;
+	val = strtol(cpuid, NULL, 16);
+
+	data[ARM_SPE_MAGIC] = ARM_SPE_CPU_MAGIC;
+	data[ARM_SPE_CPU] = cpu.cpu;
+	data[ARM_SPE_CPU_NR_PARAMS] = ARM_SPE_CPU_PRIV_MAX - ARM_SPE_CPU_MIDR;
+	data[ARM_SPE_CPU_MIDR] = val;
+
+	/* Find the associate Arm SPE PMU for the CPU */
+	if (perf_cpu_map__has(sper->arm_spe_pmu->cpus, cpu))
+		pmu = sper->arm_spe_pmu;
+
+	if (!pmu) {
+		/* No Arm SPE PMU is found */
+		data[ARM_SPE_CPU_PMU_TYPE] = ULLONG_MAX;
+		data[ARM_SPE_CAP_MIN_IVAL] = 0;
+	} else {
+		data[ARM_SPE_CPU_PMU_TYPE] = pmu->type;
+
+		if (perf_pmu__scan_file(pmu, "caps/min_interval", "%lu", &val) != 1)
+			val = 0;
+		data[ARM_SPE_CAP_MIN_IVAL] = val;
+	}
+
+	free(cpuid);
+	return ARM_SPE_CPU_PRIV_MAX;
+}
+
 static int arm_spe_info_fill(struct auxtrace_record *itr,
 			     struct perf_session *session,
 			     struct perf_record_auxtrace_info *auxtrace_info,
 			     size_t priv_size)
 {
+	int i, ret;
+	size_t offset;
 	struct arm_spe_recording *sper =
 			container_of(itr, struct arm_spe_recording, itr);
 	struct perf_pmu *arm_spe_pmu = sper->arm_spe_pmu;
+	struct perf_cpu_map *cpu_map;
+	struct perf_cpu cpu;
+	__u64 *data;
 
 	if (priv_size != arm_spe_info_priv_size(itr, session->evlist))
 		return -EINVAL;
@@ -92,10 +150,31 @@ static int arm_spe_info_fill(struct auxtrace_record *itr,
 	if (!session->evlist->core.nr_mmaps)
 		return -EINVAL;
 
+	cpu_map = arm_spe_find_cpus(session->evlist);
+	if (!cpu_map)
+		return -EINVAL;
+
 	auxtrace_info->type = PERF_AUXTRACE_ARM_SPE;
-	auxtrace_info->priv[ARM_SPE_PMU_TYPE] = arm_spe_pmu->type;
+	auxtrace_info->priv[ARM_SPE_HEADER_VERSION] = ARM_SPE_HEADER_CURRENT_VERSION;
+	auxtrace_info->priv[ARM_SPE_HEADER_SIZE] =
+		ARM_SPE_AUXTRACE_PRIV_MAX - ARM_SPE_HEADER_VERSION;
+	auxtrace_info->priv[ARM_SPE_SHARED_PMU_TYPE] = arm_spe_pmu->type;
+	auxtrace_info->priv[ARM_SPE_CPUS_NUM] = perf_cpu_map__nr(cpu_map);
+
+	offset = ARM_SPE_AUXTRACE_PRIV_MAX;
+	perf_cpu_map__for_each_cpu(cpu, i, cpu_map) {
+		assert(offset < priv_size);
+		data = &auxtrace_info->priv[offset];
+		ret = arm_spe_save_cpu_header(itr, cpu, data);
+		if (ret < 0)
+			goto out;
+		offset += ret;
+	}
 
-	return 0;
+	ret = 0;
+out:
+	perf_cpu_map__put(cpu_map);
+	return ret;
 }
 
 static void
-- 
2.34.1

[PATCH v3 4/5] perf arm-spe: Support metadata version 2

[Leo Yan]

This commit is to support metadata version 2 and at the meantime it is
backward compatible for version 1's format.

The metadata version 1 doesn't include the ARM_SPE_HEADER_VERSION field.
As version 1 is fixed with two u64 fields, by checking the metadata
size, it distinguishes the metadata is version 1 or version 2 (and any
new versions if later will have). For version 2, it reads out CPU number
and retrieves the metadata info for every CPU.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/util/arm-spe.c | 95 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 92 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
index 70989b1bae47..52cee7401ff4 100644
--- a/tools/perf/util/arm-spe.c
+++ b/tools/perf/util/arm-spe.c
@@ -78,6 +78,10 @@ struct arm_spe {
 
 	unsigned long			num_events;
 	u8				use_ctx_pkt_for_pid;
+
+	u64				**metadata;
+	u64				metadata_ver;
+	u64				metadata_nr_cpu;
 };
 
 struct arm_spe_queue {
@@ -1016,6 +1020,73 @@ static int arm_spe_flush(struct perf_session *session __maybe_unused,
 	return 0;
 }
 
+static u64 *arm_spe__alloc_per_cpu_metadata(u64 *buf, int per_cpu_size)
+{
+	u64 *metadata;
+
+	metadata = zalloc(per_cpu_size);
+	if (!metadata)
+		return NULL;
+
+	memcpy(metadata, buf, per_cpu_size);
+	return metadata;
+}
+
+static void arm_spe__free_metadata(u64 **metadata, int nr_cpu)
+{
+	int i;
+
+	for (i = 0; i < nr_cpu; i++)
+		zfree(&metadata[i]);
+	free(metadata);
+}
+
+static u64 **arm_spe__alloc_metadata(struct perf_record_auxtrace_info *info,
+				     u64 *ver, int *nr_cpu)
+{
+	u64 *ptr = (u64 *)info->priv;
+	u64 metadata_size;
+	u64 **metadata = NULL;
+	int hdr_sz, per_cpu_sz, i;
+
+	metadata_size = info->header.size -
+		sizeof(struct perf_record_auxtrace_info);
+
+	/* Metadata version 1 */
+	if (metadata_size == ARM_SPE_AUXTRACE_V1_PRIV_SIZE) {
+		*ver = 1;
+		*nr_cpu = 0;
+		/* No per CPU metadata */
+		return NULL;
+	}
+
+	*ver = ptr[ARM_SPE_HEADER_VERSION];
+	hdr_sz = ptr[ARM_SPE_HEADER_SIZE];
+	*nr_cpu = ptr[ARM_SPE_CPUS_NUM];
+
+	metadata = calloc(*nr_cpu, sizeof(*metadata));
+	if (!metadata)
+		return NULL;
+
+	/* Locate the start address of per CPU metadata */
+	ptr += hdr_sz;
+	per_cpu_sz = (metadata_size - (hdr_sz * sizeof(u64))) / (*nr_cpu);
+
+	for (i = 0; i < *nr_cpu; i++) {
+		metadata[i] = arm_spe__alloc_per_cpu_metadata(ptr, per_cpu_sz);
+		if (!metadata[i])
+			goto err_per_cpu_metadata;
+
+		ptr += per_cpu_sz / sizeof(u64);
+	}
+
+	return metadata;
+
+err_per_cpu_metadata:
+	arm_spe__free_metadata(metadata, *nr_cpu);
+	return NULL;
+}
+
 static void arm_spe_free_queue(void *priv)
 {
 	struct arm_spe_queue *speq = priv;
@@ -1050,6 +1121,7 @@ static void arm_spe_free(struct perf_session *session)
 	auxtrace_heap__free(&spe->heap);
 	arm_spe_free_events(session);
 	session->auxtrace = NULL;
+	arm_spe__free_metadata(spe->metadata, spe->metadata_nr_cpu);
 	free(spe);
 }
 
@@ -1267,15 +1339,24 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
 	const char *cpuid = perf_env__cpuid(session->evlist->env);
 	u64 midr = strtol(cpuid, NULL, 16);
 	struct arm_spe *spe;
-	int err;
+	u64 **metadata = NULL;
+	u64 metadata_ver;
+	int nr_cpu, err;
 
 	if (auxtrace_info->header.size < sizeof(struct perf_record_auxtrace_info) +
 					min_sz)
 		return -EINVAL;
 
+	metadata = arm_spe__alloc_metadata(auxtrace_info, &metadata_ver,
+					   &nr_cpu);
+	if (!metadata && metadata_ver != 1) {
+		pr_err("Failed to parse Arm SPE metadata.\n");
+		return -EINVAL;
+	}
+
 	spe = zalloc(sizeof(struct arm_spe));
 	if (!spe)
-		return -ENOMEM;
+		goto err_free_metadata;
 
 	err = auxtrace_queues__init(&spe->queues);
 	if (err)
@@ -1284,8 +1365,14 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
 	spe->session = session;
 	spe->machine = &session->machines.host; /* No kvm support */
 	spe->auxtrace_type = auxtrace_info->type;
-	spe->pmu_type = auxtrace_info->priv[ARM_SPE_PMU_TYPE];
+	if (metadata_ver == 1)
+		spe->pmu_type = auxtrace_info->priv[ARM_SPE_PMU_TYPE];
+	else
+		spe->pmu_type = auxtrace_info->priv[ARM_SPE_SHARED_PMU_TYPE];
 	spe->midr = midr;
+	spe->metadata = metadata;
+	spe->metadata_ver = metadata_ver;
+	spe->metadata_nr_cpu = nr_cpu;
 
 	spe->timeless_decoding = arm_spe__is_timeless_decoding(spe);
 
@@ -1346,5 +1433,7 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
 	session->auxtrace = NULL;
 err_free:
 	free(spe);
+err_free_metadata:
+	arm_spe__free_metadata(metadata, nr_cpu);
 	return err;
 }
-- 
2.34.1

Re: [PATCH v3 4/5] perf arm-spe: Support metadata version 2

[James Clark]

On 28/09/2024 8:55 pm, Leo Yan wrote:
> This commit is to support metadata version 2 and at the meantime it is
> backward compatible for version 1's format.
> 
> The metadata version 1 doesn't include the ARM_SPE_HEADER_VERSION field.
> As version 1 is fixed with two u64 fields, by checking the metadata
> size, it distinguishes the metadata is version 1 or version 2 (and any
> new versions if later will have). For version 2, it reads out CPU number
> and retrieves the metadata info for every CPU.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   tools/perf/util/arm-spe.c | 95 +++++++++++++++++++++++++++++++++++++--
>   1 file changed, 92 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
> index 70989b1bae47..52cee7401ff4 100644
> --- a/tools/perf/util/arm-spe.c
> +++ b/tools/perf/util/arm-spe.c
> @@ -78,6 +78,10 @@ struct arm_spe {
>   
>   	unsigned long			num_events;
>   	u8				use_ctx_pkt_for_pid;
> +
> +	u64				**metadata;
> +	u64				metadata_ver;
> +	u64				metadata_nr_cpu;
>   };
>   
>   struct arm_spe_queue {
> @@ -1016,6 +1020,73 @@ static int arm_spe_flush(struct perf_session *session __maybe_unused,
>   	return 0;
>   }
>   
> +static u64 *arm_spe__alloc_per_cpu_metadata(u64 *buf, int per_cpu_size)
> +{
> +	u64 *metadata;
> +
> +	metadata = zalloc(per_cpu_size);
> +	if (!metadata)
> +		return NULL;
> +
> +	memcpy(metadata, buf, per_cpu_size);
> +	return metadata;
> +}
> +
> +static void arm_spe__free_metadata(u64 **metadata, int nr_cpu)
> +{
> +	int i;
> +
> +	for (i = 0; i < nr_cpu; i++)
> +		zfree(&metadata[i]);
> +	free(metadata);
> +}
> +
> +static u64 **arm_spe__alloc_metadata(struct perf_record_auxtrace_info *info,
> +				     u64 *ver, int *nr_cpu)
> +{
> +	u64 *ptr = (u64 *)info->priv;
> +	u64 metadata_size;
> +	u64 **metadata = NULL;
> +	int hdr_sz, per_cpu_sz, i;
> +
> +	metadata_size = info->header.size -
> +		sizeof(struct perf_record_auxtrace_info);
> +
> +	/* Metadata version 1 */
> +	if (metadata_size == ARM_SPE_AUXTRACE_V1_PRIV_SIZE) {
> +		*ver = 1;
> +		*nr_cpu = 0;
> +		/* No per CPU metadata */
> +		return NULL;
> +	}
> +
> +	*ver = ptr[ARM_SPE_HEADER_VERSION];
> +	hdr_sz = ptr[ARM_SPE_HEADER_SIZE];
> +	*nr_cpu = ptr[ARM_SPE_CPUS_NUM];
> +
> +	metadata = calloc(*nr_cpu, sizeof(*metadata));
> +	if (!metadata)
> +		return NULL;
> +
> +	/* Locate the start address of per CPU metadata */
> +	ptr += hdr_sz;
> +	per_cpu_sz = (metadata_size - (hdr_sz * sizeof(u64))) / (*nr_cpu);
> +
> +	for (i = 0; i < *nr_cpu; i++) {
> +		metadata[i] = arm_spe__alloc_per_cpu_metadata(ptr, per_cpu_sz);
> +		if (!metadata[i])
> +			goto err_per_cpu_metadata;
> +
> +		ptr += per_cpu_sz / sizeof(u64);
> +	}
> +
> +	return metadata;
> +
> +err_per_cpu_metadata:
> +	arm_spe__free_metadata(metadata, *nr_cpu);
> +	return NULL;
> +}
> +
>   static void arm_spe_free_queue(void *priv)
>   {
>   	struct arm_spe_queue *speq = priv;
> @@ -1050,6 +1121,7 @@ static void arm_spe_free(struct perf_session *session)
>   	auxtrace_heap__free(&spe->heap);
>   	arm_spe_free_events(session);
>   	session->auxtrace = NULL;
> +	arm_spe__free_metadata(spe->metadata, spe->metadata_nr_cpu);
>   	free(spe);
>   }
>   
> @@ -1267,15 +1339,24 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
>   	const char *cpuid = perf_env__cpuid(session->evlist->env);
>   	u64 midr = strtol(cpuid, NULL, 16);
>   	struct arm_spe *spe;
> -	int err;
> +	u64 **metadata = NULL;
> +	u64 metadata_ver;
> +	int nr_cpu, err;
>   
>   	if (auxtrace_info->header.size < sizeof(struct perf_record_auxtrace_info) +
>   					min_sz)
>   		return -EINVAL;
>   
> +	metadata = arm_spe__alloc_metadata(auxtrace_info, &metadata_ver,
> +					   &nr_cpu);
> +	if (!metadata && metadata_ver != 1) {
> +		pr_err("Failed to parse Arm SPE metadata.\n");
> +		return -EINVAL;
> +	}
> +
>   	spe = zalloc(sizeof(struct arm_spe));
>   	if (!spe)
> -		return -ENOMEM;
> +		goto err_free_metadata;
>   

Hi Leo,

There's a build error here:

util/arm-spe.c:1402:6: error: variable 'err' is used uninitialized 
whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
         if (!spe)
             ^~~~


>   	err = auxtrace_queues__init(&spe->queues);
>   	if (err)
> @@ -1284,8 +1365,14 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
>   	spe->session = session;
>   	spe->machine = &session->machines.host; /* No kvm support */
>   	spe->auxtrace_type = auxtrace_info->type;
> -	spe->pmu_type = auxtrace_info->priv[ARM_SPE_PMU_TYPE];
> +	if (metadata_ver == 1)
> +		spe->pmu_type = auxtrace_info->priv[ARM_SPE_PMU_TYPE];
> +	else
> +		spe->pmu_type = auxtrace_info->priv[ARM_SPE_SHARED_PMU_TYPE];

I thought V2 saved the type per-CPU, so I wasn't that clear on the 
SHARED_PMU_TYPE vs ARM_SPE_PMU_TYPE use here. Maybe it's just a naming 
issue? If it's a placeholder value to generate events can it be the same 
name as the V1 enum, that way it's clearer that it's the same thing.

Like: ARM_SPE_PMU_TYPE_V2


Other than that, for the whole set:

Reviewed-by: James Clark <james.clark@linaro.org>

Re: [PATCH v3 4/5] perf arm-spe: Support metadata version 2

[Leo Yan]

On 10/1/2024 2:53 PM, James Clark wrote:

[...]

>> @@ -1267,15 +1339,24 @@ int arm_spe_process_auxtrace_info(union perf_event
>> *event,
>>       const char *cpuid = perf_env__cpuid(session->evlist->env);
>>       u64 midr = strtol(cpuid, NULL, 16);
>>       struct arm_spe *spe;
>> -     int err;
>> +     u64 **metadata = NULL;
>> +     u64 metadata_ver;
>> +     int nr_cpu, err;
>>
>>       if (auxtrace_info->header.size < sizeof(struct
>> perf_record_auxtrace_info) +
>>                                       min_sz)
>>               return -EINVAL;
>>
>> +     metadata = arm_spe__alloc_metadata(auxtrace_info, &metadata_ver,
>> +                                        &nr_cpu);
>> +     if (!metadata && metadata_ver != 1) {
>> +             pr_err("Failed to parse Arm SPE metadata.\n");
>> +             return -EINVAL;
>> +     }
>> +
>>       spe = zalloc(sizeof(struct arm_spe));
>>       if (!spe)
>> -             return -ENOMEM;
>> +             goto err_free_metadata;
>>
> 
> Hi Leo,
> 
> There's a build error here:
> 
> util/arm-spe.c:1402:6: error: variable 'err' is used uninitialized
> whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>         if (!spe)
>             ^~~~

Thanks for catching this issue.

It is interesting that Aarch64 cross compiler GCC-13 and GCC-14 both do not
report this issue, I reproduced the error until I changed to x86_64 GCC-11.

>>       err = auxtrace_queues__init(&spe->queues);
>>       if (err)
>> @@ -1284,8 +1365,14 @@ int arm_spe_process_auxtrace_info(union perf_event
>> *event,
>>       spe->session = session;
>>       spe->machine = &session->machines.host; /* No kvm support */
>>       spe->auxtrace_type = auxtrace_info->type;
>> -     spe->pmu_type = auxtrace_info->priv[ARM_SPE_PMU_TYPE];
>> +     if (metadata_ver == 1)
>> +             spe->pmu_type = auxtrace_info->priv[ARM_SPE_PMU_TYPE];
>> +     else
>> +             spe->pmu_type = auxtrace_info->priv[ARM_SPE_SHARED_PMU_TYPE];
> 
> I thought V2 saved the type per-CPU, so I wasn't that clear on the
> SHARED_PMU_TYPE vs ARM_SPE_PMU_TYPE use here. Maybe it's just a naming
> issue? If it's a placeholder value to generate events can it be the same
> name as the V1 enum, that way it's clearer that it's the same thing.
> 
> Like: ARM_SPE_PMU_TYPE_V2

Yes, in the end we will use the PMU type saved in per-CPU metadata which will
be updated in the multi AUX event patch series. Before we reach this point,
let us keep a PMU type in the metadata header.

I will rename to ARM_SPE_PMU_TYPE_V2.

Thanks for review.

Leo

> Other than that, for the whole set:
> 
> Reviewed-by: James Clark <james.clark@linaro.org>
> 

[PATCH v3 5/5] perf arm-spe: Dump metadata with version 2

[Leo Yan]

This commit dumps metadata with version 2. It dumps metadata for header
and per CPU data respectively in the arm_spe_print_info() function to
support metadata version 2 format.

After:

  0 0 0x3c0 [0x1b0]: PERF_RECORD_AUXTRACE_INFO type: 4
    Header version     :2
    Header size        :4
    Shared PMU type    :13
    CPU number         :8
      Magic            :0x1010101010101010
      CPU #            :0
      Num of params    :3
      MIDR             :0x410fd801
      PMU Type         :-1
      Min Interval     :0
      Magic            :0x1010101010101010
      CPU #            :1
      Num of params    :3
      MIDR             :0x410fd801
      PMU Type         :-1
      Min Interval     :0
      Magic            :0x1010101010101010
      CPU #            :2
      Num of params    :3
      MIDR             :0x410fd870
      PMU Type         :13
      Min Interval     :1024
      Magic            :0x1010101010101010
      CPU #            :3
      Num of params    :3
      MIDR             :0x410fd870
      PMU Type         :13
      Min Interval     :1024
      Magic            :0x1010101010101010
      CPU #            :4
      Num of params    :3
      MIDR             :0x410fd870
      PMU Type         :13
      Min Interval     :1024
      Magic            :0x1010101010101010
      CPU #            :5
      Num of params    :3
      MIDR             :0x410fd870
      PMU Type         :13
      Min Interval     :1024
      Magic            :0x1010101010101010
      CPU #            :6
      Num of params    :3
      MIDR             :0x410fd850
      PMU Type         :-1
      Min Interval     :0
      Magic            :0x1010101010101010
      CPU #            :7
      Num of params    :3
      MIDR             :0x410fd850
      PMU Type         :-1
      Min Interval     :0

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/util/arm-spe.c | 54 +++++++++++++++++++++++++++++++++++----
 1 file changed, 49 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
index 52cee7401ff4..8a2da725670f 100644
--- a/tools/perf/util/arm-spe.c
+++ b/tools/perf/util/arm-spe.c
@@ -1133,16 +1133,60 @@ static bool arm_spe_evsel_is_auxtrace(struct perf_session *session,
 	return evsel->core.attr.type == spe->pmu_type;
 }
 
-static const char * const arm_spe_info_fmts[] = {
-	[ARM_SPE_PMU_TYPE]		= "  PMU Type           %"PRId64"\n",
+static const char * const metadata_hdr_v1_fmts[] = {
+	[ARM_SPE_PMU_TYPE]		= "  PMU Type           :%"PRId64"\n",
+	[ARM_SPE_PER_CPU_MMAPS]		= "  Per CPU mmaps      :%"PRId64"\n",
 };
 
-static void arm_spe_print_info(__u64 *arr)
+static const char * const metadata_hdr_fmts[] = {
+	[ARM_SPE_HEADER_VERSION]	= "  Header version     :%"PRId64"\n",
+	[ARM_SPE_HEADER_SIZE]		= "  Header size        :%"PRId64"\n",
+	[ARM_SPE_SHARED_PMU_TYPE]	= "  Shared PMU type    :%"PRId64"\n",
+	[ARM_SPE_CPUS_NUM]		= "  CPU number         :%"PRId64"\n",
+};
+
+static const char * const metadata_per_cpu_fmts[] = {
+	[ARM_SPE_MAGIC]			= "    Magic            :0x%"PRIx64"\n",
+	[ARM_SPE_CPU]			= "    CPU #            :%"PRId64"\n",
+	[ARM_SPE_CPU_NR_PARAMS]		= "    Num of params    :%"PRId64"\n",
+	[ARM_SPE_CPU_MIDR]		= "    MIDR             :0x%"PRIx64"\n",
+	[ARM_SPE_CPU_PMU_TYPE]		= "    PMU Type         :%"PRId64"\n",
+	[ARM_SPE_CAP_MIN_IVAL]		= "    Min Interval     :%"PRId64"\n",
+};
+
+static void arm_spe_print_info(struct arm_spe *spe, __u64 *arr)
 {
+	unsigned int i, cpu, hdr_size, cpu_num, cpu_size;
+	const char * const *hdr_fmts;
+
 	if (!dump_trace)
 		return;
 
-	fprintf(stdout, arm_spe_info_fmts[ARM_SPE_PMU_TYPE], arr[ARM_SPE_PMU_TYPE]);
+	if (spe->metadata_ver == 1) {
+		cpu_num = 0;
+		hdr_size = ARM_SPE_AUXTRACE_V1_PRIV_MAX;
+		hdr_fmts = metadata_hdr_v1_fmts;
+	} else {
+		cpu_num = arr[ARM_SPE_CPUS_NUM];
+		hdr_size = arr[ARM_SPE_HEADER_SIZE];
+		hdr_fmts = metadata_hdr_fmts;
+	}
+
+	for (i = 0; i < hdr_size; i++)
+		fprintf(stdout, hdr_fmts[i], arr[i]);
+
+	arr += hdr_size;
+	for (cpu = 0; cpu < cpu_num; cpu++) {
+		/*
+		 * The parameters from ARM_SPE_MAGIC to ARM_SPE_CPU_NR_PARAMS
+		 * are fixed. The sequential parameter size is decided by the
+		 * field 'ARM_SPE_CPU_NR_PARAMS'.
+		 */
+		cpu_size = (ARM_SPE_CPU_NR_PARAMS + 1) + arr[ARM_SPE_CPU_NR_PARAMS];
+		for (i = 0; i < cpu_size; i++)
+			fprintf(stdout, metadata_per_cpu_fmts[i], arr[i]);
+		arr += cpu_size;
+	}
 }
 
 static void arm_spe_set_event_name(struct evlist *evlist, u64 id,
@@ -1405,7 +1449,7 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
 	spe->auxtrace.evsel_is_auxtrace = arm_spe_evsel_is_auxtrace;
 	session->auxtrace = &spe->auxtrace;
 
-	arm_spe_print_info(&auxtrace_info->priv[0]);
+	arm_spe_print_info(spe, &auxtrace_info->priv[0]);
 
 	if (dump_trace)
 		return 0;
-- 
2.34.1