Re: [PATCH v2 08/13] perf: cs-etm: Move mapping of Trace ID and cpu into helper function

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Wed, Jul 20, 2022 at 11:22:37AM +0100, Mike Leach escreveu:
> On Tue, 19 Jul 2022 at 15:54, James Clark <james.clark@arm.com> wrote:
> > I'm starting to look at this set now.

> > Am I right in thinking that this hard coded value means that new versions
> > of Perf won't work with older drivers? Does this need to be highlighted
> > somewhere in a warning that it's not the Perf version that's the issue but
> > both the Perf and driver version together?
 
> Need to differentiate here between perf record, and perf report.
 
> My understanding is that perf record must always match the version of
> your kernel. If you use an old version of perf record on a newer

No, that is not what is intended, one should be able to use whatever
perf (record or otherwise) with whatever kernel version.

perf tries to cope with, and if it is not possible to record the way the
user asks to then it should emit a helpful error message stating why it
is not possible, see:

  evsel__disable_missing_features()
  evsel__detect_missing_features()

Used during a evsel__open()

- Arnaldo

> kernel then you are asking for trouble.
> Indeed, if I run perf on my x86 dev machine at the moment it whinges:
> WARNING: perf not found for kernel 5.4.0-122
> because the last version of perf I have is for 5.4.0-120.
> 
> The new perf report will differentiate between the new and old
> versions of the perf.data file and act accordingly. For version 1 it
> will take the IDs from the metadata, for version 2 it will search for
> the IDs in the packet data.
> An older perf report will not be able to decode the newer files -
> though that has always been the case.
> 
> Were we to permit and old version of perf report to be used to
> generate a file using the new drivers, and then attempt to process
> that file with and older perf report, it would fail miserably.
> 
> Regards
> 
> Mike
> 
> 
> > I thought the idea was to search through the file to look for
> > PERF_RECORD_AUX_OUTPUT_HW_ID records (or lack of) and then choose the appropriate
> > decode method. But maybe that's too complicated and there is no requirement
> > for backwards compatibility?
> >
> > From experience it can be inconvenient when you can't just throw
> > any build of Perf on a system and it supports everything that it knows
> > about. Now we will have Perf builds that know about Coresight but don't
> > work with older drivers.
> >
> > But then as you say the ID allocation is already broken for some people.
> > It's hard to decide.
> >
> > James
> >
> > >
> > >  /* Beginning of header common to both ETMv3 and V4 */
> > >  enum {
> > > @@ -85,6 +89,12 @@ enum {
> > >       CS_ETE_PRIV_MAX
> > >  };
> > >
> > > +/*
> > > + * Check for valid CoreSight trace ID. If an invalid value is present in the metadata,
> > > + * then IDs are present in the hardware ID packet in the data file.
> > > + */
> > > +#define CS_IS_VALID_TRACE_ID(id) ((id > 0) && (id < 0x70))
> > > +
> > >  /*
> > >   * ETMv3 exception encoding number:
> > >   * See Embedded Trace Macrocell specification (ARM IHI 0014Q)
> 
> 
> 
> -- 
> Mike Leach
> Principal Engineer, ARM Ltd.
> Manchester Design Centre. UK

-- 

- Arnaldo