Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210127175350.7943-1-mike.leach@linaro.org> <20210129184004.GA1236509@xps15>
 <20210129185717.GB1236509@xps15>
In-Reply-To: <20210129185717.GB1236509@xps15>
From:   Mike Leach <mike.leach@linaro.org>
Date:   Tue, 2 Feb 2021 21:38:32 +0000
Message-ID: <CAJ9a7VjTmwcgA7zNXDgUM-jjMyHkZ8WuD7Tmr5Hcv+vxGrgoFA@mail.gmail.com>
Subject: Re: [PATCH v4] perf: cs-etm: update ETM metadata format
To:     Mathieu Poirier <mathieu.poirier@linaro.org>
Cc:     linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        Coresight ML <coresight@lists.linaro.org>,
        Leo Yan <leo.yan@linaro.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "Suzuki K. Poulose" <suzuki.poulose@arm.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Will Deacon <will@kernel.org>,
        John Garry <john.garry@huawei.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@redhat.com>,
        Namhyung Kim <namhyung@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Mathieu,

On Fri, 29 Jan 2021 at 18:57, Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
>
> It just dawned on me that this file doesn't go through my tree, Arnaldo handles
> perf tools modifications.  Please see below for further instructions.
>
> On Fri, Jan 29, 2021 at 11:40:04AM -0700, Mathieu Poirier wrote:
> > Good morning,
> >
> > On Wed, Jan 27, 2021 at 05:53:50PM +0000, Mike Leach wrote:
> > > The current fixed metadata version format (version 0), means that adding
> > > metadata parameter items renders files from a previous version of perf
> > > unreadable. Per CPU parameters appear in a fixed order, but there is no
> > > field to indicate the number of ETM parameters per CPU.
> > >
> > > This patch updates the per CPU parameter blocks to include a NR_PARAMs
> > > value which indicates the number of parameters in the block.
> > >
> > > The header version is incremented to 1. Fixed ordering is retained,
> > > new ETM parameters are added to the end of the list.
> > >
> > > The reader code is updated to be able to read current version 0 files,
> > > For version 1, the reader will read the number of parameters in the
> > > per CPU block. This allows the reader to process older or newer files
> > > that may have different numbers of parameters than in use at the
> > > time perf was built.
> > >
> > > Changes since v3
> > > 1. Fixed index bug (Leo)
> > >
> > > Changes since v2
> > > 1. Add error path print to improve --dump logging
> > > 2. Replace some hardcoded values with enum consts (Mathieu).
> > >
> > > Changes since v1 (from Review by Leo):
> > > 1. Split printing routine into sub functions per version
> > > 2. Renamed NR_PARAMs to NR_TRC_PARAMs to emphasise use as count of trace
> > > related parameters, not total block parameter.
> > > 3. Misc other fixes.
> > >
> > > Signed-off-by: Mike Leach <mike.leach@linaro.org>
> >
> > Leo added his RB and TB tags to this patch for V3.  When that happens the author
> > is expected to carry them in the next revision.  I added them.
>

Yes sorry - oversight on my part. Fixed in v5


> Version logs needs to go below the "---".  Otherwise they will be added
> to the changelog and we don't want that.
>

OK

> >
> > > ---
> > >  tools/perf/arch/arm/util/cs-etm.c |   7 +-
> > >  tools/perf/util/cs-etm.c          | 235 ++++++++++++++++++++++++------
> > >  tools/perf/util/cs-etm.h          |  30 +++-
> > >  3 files changed, 223 insertions(+), 49 deletions(-)
> > >
> > > diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
> > > index bd446aba64f7..b0470f2a955a 100644
> > > --- a/tools/perf/arch/arm/util/cs-etm.c
> > > +++ b/tools/perf/arch/arm/util/cs-etm.c
> > > @@ -572,7 +572,7 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,
> > >                             struct auxtrace_record *itr,
> > >                             struct perf_record_auxtrace_info *info)
> > >  {
> > > -   u32 increment;
> > > +   u32 increment, nr_trc_params;
> > >     u64 magic;
> > >     struct cs_etm_recording *ptr =
> > >                     container_of(itr, struct cs_etm_recording, itr);
> > > @@ -607,6 +607,7 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,
> > >
> > >             /* How much space was used */
> > >             increment = CS_ETMV4_PRIV_MAX;
> > > +           nr_trc_params = CS_ETMV4_PRIV_MAX - CS_ETMV4_TRCCONFIGR;
> > >     } else {
> > >             magic = __perf_cs_etmv3_magic;
> > >             /* Get configuration register */
> > > @@ -624,11 +625,13 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,
> > >
> > >             /* How much space was used */
> > >             increment = CS_ETM_PRIV_MAX;
> > > +           nr_trc_params = CS_ETM_PRIV_MAX - CS_ETM_ETMCR;
> > >     }
> > >
> > >     /* Build generic header portion */
> > >     info->priv[*offset + CS_ETM_MAGIC] = magic;
> > >     info->priv[*offset + CS_ETM_CPU] = cpu;
> > > +   info->priv[*offset + CS_ETM_NR_TRC_PARAMS] = nr_trc_params;
> > >     /* Where the next CPU entry should start from */
> > >     *offset += increment;
> > >  }
> > > @@ -674,7 +677,7 @@ static int cs_etm_info_fill(struct auxtrace_record *itr,
> > >
> > >     /* First fill out the session header */
> > >     info->type = PERF_AUXTRACE_CS_ETM;
> > > -   info->priv[CS_HEADER_VERSION_0] = 0;
> > > +   info->priv[CS_HEADER_VERSION] = CS_HEADER_CURRENT_VERSION;
> > >     info->priv[CS_PMU_TYPE_CPUS] = type << 32;
> > >     info->priv[CS_PMU_TYPE_CPUS] |= nr_cpu;
> > >     info->priv[CS_ETM_SNAPSHOT] = ptr->snapshot_mode;
> > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > > index a2a369e2fbb6..f9af3fe0ed83 100644
> > > --- a/tools/perf/util/cs-etm.c
> > > +++ b/tools/perf/util/cs-etm.c
> > > @@ -2435,7 +2435,7 @@ static bool cs_etm__is_timeless_decoding(struct cs_etm_auxtrace *etm)
> > >  }
> > >
> > >  static const char * const cs_etm_global_header_fmts[] = {
> > > -   [CS_HEADER_VERSION_0]   = "     Header version                 %llx\n",
> > > +   [CS_HEADER_VERSION]     = "     Header version                 %llx\n",
> > >     [CS_PMU_TYPE_CPUS]      = "     PMU type/num cpus              %llx\n",
> > >     [CS_ETM_SNAPSHOT]       = "     Snapshot                       %llx\n",
> > >  };
> > > @@ -2443,6 +2443,7 @@ static const char * const cs_etm_global_header_fmts[] = {
> > >  static const char * const cs_etm_priv_fmts[] = {
> > >     [CS_ETM_MAGIC]          = "     Magic number                   %llx\n",
> > >     [CS_ETM_CPU]            = "     CPU                            %lld\n",
> > > +   [CS_ETM_NR_TRC_PARAMS]  = "     NR_TRC_PARAMS                  %llx\n",
> > >     [CS_ETM_ETMCR]          = "     ETMCR                          %llx\n",
> > >     [CS_ETM_ETMTRACEIDR]    = "     ETMTRACEIDR                    %llx\n",
> > >     [CS_ETM_ETMCCER]        = "     ETMCCER                        %llx\n",
> > > @@ -2452,6 +2453,7 @@ static const char * const cs_etm_priv_fmts[] = {
> > >  static const char * const cs_etmv4_priv_fmts[] = {
> > >     [CS_ETM_MAGIC]          = "     Magic number                   %llx\n",
> > >     [CS_ETM_CPU]            = "     CPU                            %lld\n",
> > > +   [CS_ETM_NR_TRC_PARAMS]  = "     NR_TRC_PARAMS                  %llx\n",
> > >     [CS_ETMV4_TRCCONFIGR]   = "     TRCCONFIGR                     %llx\n",
> > >     [CS_ETMV4_TRCTRACEIDR]  = "     TRCTRACEIDR                    %llx\n",
> > >     [CS_ETMV4_TRCIDR0]      = "     TRCIDR0                        %llx\n",
> > > @@ -2461,26 +2463,167 @@ static const char * const cs_etmv4_priv_fmts[] = {
> > >     [CS_ETMV4_TRCAUTHSTATUS] = "    TRCAUTHSTATUS                  %llx\n",
> > >  };
> > >
> > > -static void cs_etm__print_auxtrace_info(__u64 *val, int num)
> > > +static const char * const param_unk_fmt =
> > > +   "       Unknown parameter [%d]         %llx\n";
> > > +static const char * const magic_unk_fmt =
> > > +   "       Magic number Unknown           %llx\n";
> > > +
> > > +static int cs_etm__print_cpu_metadata_v0(__u64 *val, int *offset)
> > >  {
> > > -   int i, j, cpu = 0;
> > > +   int i = *offset, j, nr_params = 0, fmt_offset;
> > > +   __u64 magic;
> > >
> > > -   for (i = 0; i < CS_HEADER_VERSION_0_MAX; i++)
> > > -           fprintf(stdout, cs_etm_global_header_fmts[i], val[i]);
> > > +   /* check magic value */
> > > +   magic = val[i + CS_ETM_MAGIC];
> > > +   if ((magic != __perf_cs_etmv3_magic) &&
> > > +       (magic != __perf_cs_etmv4_magic)) {
> > > +           /* failure - note bad magic value */
> > > +           fprintf(stdout, magic_unk_fmt, magic);
> > > +           return -EINVAL;
> > > +   }
> > > +
> > > +   /* print common header block */
> > > +   fprintf(stdout, cs_etm_priv_fmts[CS_ETM_MAGIC], val[i++]);
> > > +   fprintf(stdout, cs_etm_priv_fmts[CS_ETM_CPU], val[i++]);
> > > +
> > > +   if (magic == __perf_cs_etmv3_magic) {
> > > +           nr_params = CS_ETM_NR_TRC_PARAMS_V0;
> > > +           fmt_offset = CS_ETM_ETMCR;
> > > +           /* after common block, offset format index past NR_PARAMS */
> > > +           for (j = fmt_offset; j < nr_params + fmt_offset; j++, i++)
> > > +                   fprintf(stdout, cs_etm_priv_fmts[j], val[i]);
> > > +   } else if (magic == __perf_cs_etmv4_magic) {
> > > +           nr_params = CS_ETMV4_NR_TRC_PARAMS_V0;
> > > +           fmt_offset = CS_ETMV4_TRCCONFIGR;
> > > +           /* after common block, offset format index past NR_PARAMS */
> > > +           for (j = fmt_offset; j < nr_params + fmt_offset; j++, i++)
> > > +                   fprintf(stdout, cs_etmv4_priv_fmts[j], val[i]);
> > > +   }
> > > +   *offset = i;
> > > +   return 0;
> > > +}
> > > +
> > > +static int cs_etm__print_cpu_metadata_v1(__u64 *val, int *offset)
> > > +{
> > > +   int i = *offset, j, total_params = 0;
> > > +   __u64 magic;
> > > +
> > > +   magic = val[i + CS_ETM_MAGIC];
> > > +   /* total params to print is NR_PARAMS + common block size for v1 */
> > > +   total_params = val[i + CS_ETM_NR_TRC_PARAMS] + CS_ETM_COMMON_BLK_MAX_V1;
> > >
> > > -   for (i = CS_HEADER_VERSION_0_MAX; cpu < num; cpu++) {
> > > -           if (val[i] == __perf_cs_etmv3_magic)
> > > -                   for (j = 0; j < CS_ETM_PRIV_MAX; j++, i++)
> > > +   if (magic == __perf_cs_etmv3_magic) {
> > > +           for (j = 0; j < total_params; j++, i++) {
> > > +                   /* if newer record - could be excess params */
> > > +                   if (j >= CS_ETM_PRIV_MAX)
> > > +                           fprintf(stdout, param_unk_fmt, j, val[i]);
> > > +                   else
> > >                             fprintf(stdout, cs_etm_priv_fmts[j], val[i]);
> > > -           else if (val[i] == __perf_cs_etmv4_magic)
> > > -                   for (j = 0; j < CS_ETMV4_PRIV_MAX; j++, i++)
> > > +           }
> > > +   } else if (magic == __perf_cs_etmv4_magic) {
> > > +           for (j = 0; j < total_params; j++, i++) {
> > > +                   /* if newer record - could be excess params */
> > > +                   if (j >= CS_ETMV4_PRIV_MAX)
> > > +                           fprintf(stdout, param_unk_fmt, j, val[i]);
> > > +                   else
> > >                             fprintf(stdout, cs_etmv4_priv_fmts[j], val[i]);
> > > -           else
> > > -                   /* failure.. return */
> > > +           }
> > > +   } else {
> > > +           /* failure - note bad magic value and error out */
> > > +           fprintf(stdout, magic_unk_fmt, magic);
> > > +           return -EINVAL;
> > > +   }
> > > +   *offset = i;
> > > +   return 0;
> > > +}
> > > +
> > > +static void cs_etm__print_auxtrace_info(__u64 *val, int num)
> > > +{
> > > +   int i, cpu = 0, version, err;
> > > +
> > > +   /* bail out early on bad header version */
> > > +   version = val[0];
> > > +   if (version > CS_HEADER_CURRENT_VERSION) {
> > > +           /* failure.. return */
> > > +           fprintf(stdout, "       Unknown Header Version = %x, ", version);
> > > +           fprintf(stdout, "Version supported <= %x\n", CS_HEADER_CURRENT_VERSION);
> > > +           return;
> > > +   }
> > > +
> > > +   for (i = 0; i < CS_HEADER_VERSION_MAX; i++)
> > > +           fprintf(stdout, cs_etm_global_header_fmts[i], val[i]);
> > > +
> > > +   for (i = CS_HEADER_VERSION_MAX; cpu < num; cpu++) {
> > > +           if (version == 0)
> > > +                   err = cs_etm__print_cpu_metadata_v0(val, &i);
> > > +           else if (version == 1)
> > > +                   err = cs_etm__print_cpu_metadata_v1(val, &i);
> > > +           if (err)
> > >                     return;
> > >     }
> > >  }
> > >
> > > +/*
> > > + * Read a single cpu parameter block from the auxtrace_info priv block.
> > > + *
> > > + * For version 1 there is a per cpu nr_params entry. If we are handling
> > > + * version 1 file, then there may be less, the same, or more params
> > > + * indicated by this value than the compile time number we understand.
> > > + *
> > > + * For a version 0 info block, there are a fixed number, and we need to
> > > + * fill out the nr_param value in the metadata we create.
> > > + */
> > > +static u64 *cs_etm__create_meta_blk(u64 *buff_in, int *buff_in_offset,
> > > +                               int out_blk_size, int nr_params_v0)
> > > +{
> > > +   u64 *metadata = NULL;
> > > +   int hdr_version;
> > > +   int nr_in_params, nr_out_params, nr_cmn_params;
> > > +   int i, k;
> > > +
> > > +   metadata = zalloc(sizeof(*metadata) * out_blk_size);
> > > +   if (!metadata)
> > > +           return NULL;
> > > +
> > > +   /* read block current index & version */
> > > +   i = *buff_in_offset;
> > > +   hdr_version = buff_in[CS_HEADER_VERSION];
> > > +
> > > +   if (!hdr_version) {
> > > +   /* read version 0 info block into a version 1 metadata block  */
> > > +           nr_in_params = nr_params_v0;
> > > +           metadata[CS_ETM_MAGIC] = buff_in[i + CS_ETM_MAGIC];
> > > +           metadata[CS_ETM_CPU] = buff_in[i + CS_ETM_CPU];
> > > +           metadata[CS_ETM_NR_TRC_PARAMS] = nr_in_params;
> > > +           /* remaining block params at offset +1 from source */
> > > +           for (k = CS_ETM_ETMCR - 1; k < nr_in_params; k++)
> >
> > Function cs_etm__create_meta_blk() is called for both etmv3 and etmv4 but above
> > an etmv3 specific constant is used.  Wouldn't it be better to change
> > CS_ETM_ETMCR for CS_ETM_COMMON_BLK_MAX_V1?
> >

Yes.

> > > +                   metadata[k+1] = buff_in[i + k];
> >
> > This needs to be "metadata[k + 1] = ... ".
> >

Agreed - but suprised that checkpatch didn't complain!

> > You can either send me another patch or I can make the changes - let me know
> > what you want to do.
>
> As stated above this file goes through Arnaldo's tree and as such you will have
> to spin off a new revision.
>

WIll do.

Mike

> >


> > Thanks,
> > Mathieu
> >
> > > +           /* version 0 has 2 common params */
> > > +           nr_cmn_params = 2;
> > > +   } else {
> > > +   /* read version 1 info block - input and output nr_params may differ */
> > > +           /* version 1 has 3 common params */
> > > +           nr_cmn_params = 3;
> > > +           nr_in_params = buff_in[i + CS_ETM_NR_TRC_PARAMS];
> > > +
> > > +           /* if input has more params than output - skip excess */
> > > +           nr_out_params = nr_in_params + nr_cmn_params;
> > > +           if (nr_out_params > out_blk_size)
> > > +                   nr_out_params = out_blk_size;
> > > +
> > > +           for (k = CS_ETM_MAGIC; k < nr_out_params; k++)
> > > +                   metadata[k] = buff_in[i + k];
> > > +
> > > +           /* record the actual nr params we copied */
> > > +           metadata[CS_ETM_NR_TRC_PARAMS] = nr_out_params - nr_cmn_params;
> > > +   }
> > > +
> > > +   /* adjust in offset by number of in params used */
> > > +   i += nr_in_params + nr_cmn_params;
> > > +   *buff_in_offset = i;
> > > +   return metadata;
> > > +}
> > > +
> > >  int cs_etm__process_auxtrace_info(union perf_event *event,
> > >                               struct perf_session *session)
> > >  {
> > > @@ -2492,11 +2635,12 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
> > >     int info_header_size;
> > >     int total_size = auxtrace_info->header.size;
> > >     int priv_size = 0;
> > > -   int num_cpu;
> > > -   int err = 0, idx = -1;
> > > -   int i, j, k;
> > > +   int num_cpu, trcidr_idx;
> > > +   int err = 0;
> > > +   int i, j;
> > >     u64 *ptr, *hdr = NULL;
> > >     u64 **metadata = NULL;
> > > +   u64 hdr_version;
> > >
> > >     /*
> > >      * sizeof(auxtrace_info_event::type) +
> > > @@ -2512,16 +2656,21 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
> > >     /* First the global part */
> > >     ptr = (u64 *) auxtrace_info->priv;
> > >
> > > -   /* Look for version '0' of the header */
> > > -   if (ptr[0] != 0)
> > > +   /* Look for version of the header */
> > > +   hdr_version = ptr[0];
> > > +   if (hdr_version > CS_HEADER_CURRENT_VERSION) {
> > > +           /* print routine will print an error on bad version */
> > > +           if (dump_trace)
> > > +                   cs_etm__print_auxtrace_info(auxtrace_info->priv, 0);
> > >             return -EINVAL;
> > > +   }
> > >
> > > -   hdr = zalloc(sizeof(*hdr) * CS_HEADER_VERSION_0_MAX);
> > > +   hdr = zalloc(sizeof(*hdr) * CS_HEADER_VERSION_MAX);
> > >     if (!hdr)
> > >             return -ENOMEM;
> > >
> > >     /* Extract header information - see cs-etm.h for format */
> > > -   for (i = 0; i < CS_HEADER_VERSION_0_MAX; i++)
> > > +   for (i = 0; i < CS_HEADER_VERSION_MAX; i++)
> > >             hdr[i] = ptr[i];
> > >     num_cpu = hdr[CS_PMU_TYPE_CPUS] & 0xffffffff;
> > >     pmu_type = (unsigned int) ((hdr[CS_PMU_TYPE_CPUS] >> 32) &
> > > @@ -2552,35 +2701,31 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
> > >      */
> > >     for (j = 0; j < num_cpu; j++) {
> > >             if (ptr[i] == __perf_cs_etmv3_magic) {
> > > -                   metadata[j] = zalloc(sizeof(*metadata[j]) *
> > > -                                        CS_ETM_PRIV_MAX);
> > > -                   if (!metadata[j]) {
> > > -                           err = -ENOMEM;
> > > -                           goto err_free_metadata;
> > > -                   }
> > > -                   for (k = 0; k < CS_ETM_PRIV_MAX; k++)
> > > -                           metadata[j][k] = ptr[i + k];
> > > +                   metadata[j] =
> > > +                           cs_etm__create_meta_blk(ptr, &i,
> > > +                                                   CS_ETM_PRIV_MAX,
> > > +                                                   CS_ETM_NR_TRC_PARAMS_V0);
> > >
> > >                     /* The traceID is our handle */
> > > -                   idx = metadata[j][CS_ETM_ETMTRACEIDR];
> > > -                   i += CS_ETM_PRIV_MAX;
> > > +                   trcidr_idx = CS_ETM_ETMTRACEIDR;
> > > +
> > >             } else if (ptr[i] == __perf_cs_etmv4_magic) {
> > > -                   metadata[j] = zalloc(sizeof(*metadata[j]) *
> > > -                                        CS_ETMV4_PRIV_MAX);
> > > -                   if (!metadata[j]) {
> > > -                           err = -ENOMEM;
> > > -                           goto err_free_metadata;
> > > -                   }
> > > -                   for (k = 0; k < CS_ETMV4_PRIV_MAX; k++)
> > > -                           metadata[j][k] = ptr[i + k];
> > > +                   metadata[j] =
> > > +                           cs_etm__create_meta_blk(ptr, &i,
> > > +                                                   CS_ETMV4_PRIV_MAX,
> > > +                                                   CS_ETMV4_NR_TRC_PARAMS_V0);
> > >
> > >                     /* The traceID is our handle */
> > > -                   idx = metadata[j][CS_ETMV4_TRCTRACEIDR];
> > > -                   i += CS_ETMV4_PRIV_MAX;
> > > +                   trcidr_idx = CS_ETMV4_TRCTRACEIDR;
> > > +           }
> > > +
> > > +           if (!metadata[j]) {
> > > +                   err = -ENOMEM;
> > > +                   goto err_free_metadata;
> > >             }
> > >
> > >             /* Get an RB node for this CPU */
> > > -           inode = intlist__findnew(traceid_list, idx);
> > > +           inode = intlist__findnew(traceid_list, metadata[j][trcidr_idx]);
> > >
> > >             /* Something went wrong, no need to continue */
> > >             if (!inode) {
> > > @@ -2601,7 +2746,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
> > >     }
> > >
> > >     /*
> > > -    * Each of CS_HEADER_VERSION_0_MAX, CS_ETM_PRIV_MAX and
> > > +    * Each of CS_HEADER_VERSION_MAX, CS_ETM_PRIV_MAX and
> > >      * CS_ETMV4_PRIV_MAX mark how many double words are in the
> > >      * global metadata, and each cpu's metadata respectively.
> > >      * The following tests if the correct number of double words was
> > > @@ -2703,6 +2848,12 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
> > >     intlist__delete(traceid_list);
> > >  err_free_hdr:
> > >     zfree(&hdr);
> > > -
> > > +   /*
> > > +    * At this point, as a minimum we have valid header. Dump the rest of
> > > +    * the info section - the print routines will error out on structural
> > > +    * issues.
> > > +    */
> > > +   if (dump_trace)
> > > +           cs_etm__print_auxtrace_info(auxtrace_info->priv, num_cpu);
> > >     return err;
> > >  }
> > > diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h
> > > index 4ad925d6d799..e153d02df0de 100644
> > > --- a/tools/perf/util/cs-etm.h
> > > +++ b/tools/perf/util/cs-etm.h
> > > @@ -17,23 +17,37 @@ struct perf_session;
> > >   */
> > >  enum {
> > >     /* Starting with 0x0 */
> > > -   CS_HEADER_VERSION_0,
> > > +   CS_HEADER_VERSION,
> > >     /* PMU->type (32 bit), total # of CPUs (32 bit) */
> > >     CS_PMU_TYPE_CPUS,
> > >     CS_ETM_SNAPSHOT,
> > > -   CS_HEADER_VERSION_0_MAX,
> > > +   CS_HEADER_VERSION_MAX,
> > >  };
> > >
> > > +/*
> > > + * Update the version for new format.
> > > + *
> > > + * New version 1 format adds a param count to the per cpu metadata.
> > > + * This allows easy adding of new metadata parameters.
> > > + * Requires that new params always added after current ones.
> > > + * Also allows client reader to handle file versions that are different by
> > > + * checking the number of params in the file vs the number expected.
> > > + */
> > > +#define CS_HEADER_CURRENT_VERSION 1
> > > +
> > >  /* Beginning of header common to both ETMv3 and V4 */
> > >  enum {
> > >     CS_ETM_MAGIC,
> > >     CS_ETM_CPU,
> > > +   /* Number of trace config params in following ETM specific block */
> > > +   CS_ETM_NR_TRC_PARAMS,
> > > +   CS_ETM_COMMON_BLK_MAX_V1,
> > >  };
> > >
> > >  /* ETMv3/PTM metadata */
> > >  enum {
> > >     /* Dynamic, configurable parameters */
> > > -   CS_ETM_ETMCR = CS_ETM_CPU + 1,
> > > +   CS_ETM_ETMCR = CS_ETM_COMMON_BLK_MAX_V1,
> > >     CS_ETM_ETMTRACEIDR,
> > >     /* RO, taken from sysFS */
> > >     CS_ETM_ETMCCER,
> > > @@ -41,10 +55,13 @@ enum {
> > >     CS_ETM_PRIV_MAX,
> > >  };
> > >
> > > +/* define fixed version 0 length - allow new format reader to read old files. */
> > > +#define CS_ETM_NR_TRC_PARAMS_V0 (CS_ETM_ETMIDR - CS_ETM_ETMCR + 1)
> > > +
> > >  /* ETMv4 metadata */
> > >  enum {
> > >     /* Dynamic, configurable parameters */
> > > -   CS_ETMV4_TRCCONFIGR = CS_ETM_CPU + 1,
> > > +   CS_ETMV4_TRCCONFIGR = CS_ETM_COMMON_BLK_MAX_V1,
> > >     CS_ETMV4_TRCTRACEIDR,
> > >     /* RO, taken from sysFS */
> > >     CS_ETMV4_TRCIDR0,
> > > @@ -55,6 +72,9 @@ enum {
> > >     CS_ETMV4_PRIV_MAX,
> > >  };
> > >
> > > +/* define fixed version 0 length - allow new format reader to read old files. */
> > > +#define CS_ETMV4_NR_TRC_PARAMS_V0 (CS_ETMV4_TRCAUTHSTATUS - CS_ETMV4_TRCCONFIGR + 1)
> > > +
> > >  /*
> > >   * ETMv3 exception encoding number:
> > >   * See Embedded Trace Macrocell spcification (ARM IHI 0014Q)
> > > @@ -162,7 +182,7 @@ struct cs_etm_packet_queue {
> > >
> > >  #define BMVAL(val, lsb, msb)       ((val & GENMASK(msb, lsb)) >> lsb)
> > >
> > > -#define CS_ETM_HEADER_SIZE (CS_HEADER_VERSION_0_MAX * sizeof(u64))
> > > +#define CS_ETM_HEADER_SIZE (CS_HEADER_VERSION_MAX * sizeof(u64))
> > >
> > >  #define __perf_cs_etmv3_magic 0x3030303030303030ULL
> > >  #define __perf_cs_etmv4_magic 0x4040404040404040ULL
> > > --
> > > 2.17.1
> > >


-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK