[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZvZRiTYrTOdXUVUk@google.com>
Date: Thu, 26 Sep 2024 23:32:41 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Leo Yan <leo.yan@....com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
James Clark <james.clark@...aro.org>,
Mike Leach <mike.leach@...aro.org>, Will Deacon <will@...nel.org>,
Mark Rutland <mark.rutland@....com>, Jiri Olsa <jolsa@...nel.org>,
Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
"Liang, Kan" <kan.liang@...ux.intel.com>,
linux-arm-kernel@...ts.infradead.org,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/5] perf arm-spe: Define metadata header version 2
On Sat, Sep 14, 2024 at 10:54:54PM +0100, Leo Yan wrote:
> 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@....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,
> };
Why don't you define something like struct arm_spe_header_v2 ?
Thanks,
Namhyung
>
> -#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
>
Powered by blists - more mailing lists