linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mike Leach <mike.leach@linaro.org>
To: Leo Yan <leo.yan@linaro.org>
Cc: James Clark <james.clark@arm.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Coresight ML <coresight@lists.linaro.org>,
	Al Grant <al.grant@arm.com>,
	"Suzuki K. Poulose" <suzuki.poulose@arm.com>,
	Anshuman Khandual <anshuman.khandual@arm.com>,
	John Garry <john.garry@huawei.com>, Will Deacon <will@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-perf-users@vger.kernel.org
Subject: Re: [PATCH 2/6] perf cs-etm: Initialise architecture based on TRCIDR1
Date: Mon, 2 Aug 2021 15:04:14 +0100	[thread overview]
Message-ID: <CAJ9a7VhdLaP3rkXwscAQpVe+jSgcx02WjY=RmiBkaCB6ObGTjQ@mail.gmail.com> (raw)
In-Reply-To: <20210731060312.GB7437@leoy-ThinkPad-X240s>

Hi Leo,

On Sat, 31 Jul 2021 at 07:03, Leo Yan <leo.yan@linaro.org> wrote:
>
> On Thu, Jul 22, 2021 at 12:10:35PM +0100, Mike Leach wrote:
> > HI James
> >
> > On Wed, 21 Jul 2021 at 10:07, James Clark <james.clark@arm.com> wrote:
> > >
> > > Currently the architecture is hard coded as ARCH_V8, but with the
> > > introduction of ETE we want to pick ARCH_AA64. And this change is also
> > > applicable to ETM v4.4 onwards as well.
> > >
> > > Signed-off-by: James Clark <james.clark@arm.com>
> > > ---
> > >  tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 14 +++++++++++++-
> > >  1 file changed, 13 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> > > index 30889a9d0165..5972a8afcc6b 100644
> > > --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> > > +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> > > @@ -126,6 +126,18 @@ static int cs_etm_decoder__gen_etmv3_config(struct cs_etm_trace_params *params,
> > >         return 0;
> > >  }
> > >
> > > +#define TRCIDR1_TRCARCHMIN_SHIFT 4
> > > +#define TRCIDR1_TRCARCHMIN_MASK  GENMASK(7, 4)
> > > +#define TRCIDR1_TRCARCHMIN(x)    (((x) & TRCIDR1_TRCARCHMIN_MASK) >> TRCIDR1_TRCARCHMIN_SHIFT)
> > > +static enum _ocsd_arch_version cs_etm_decoder__get_arch_ver(u32 reg_idr1)
> > > +{
> > > +       /*
> > > +        * If the ETM trace minor version is 4 or more then we can assume
> > > +        * the architecture is ARCH_AA64 rather than just V8
> > > +        */
> > > +       return TRCIDR1_TRCARCHMIN(reg_idr1) >= 4 ? ARCH_AA64 : ARCH_V8;
> > > +}
> >
> > This is true for ETM4.x & ETE 1.x (arch 5.x) but not ETM 3.x
> > Probably need to beef up this comment or the function name to emphasise this.
>
> Yeah, I think it's good to change the function name.  Eventually, this
> function should only be used for ETM4.x and ETE.
>
> Another minor comment is: can we refine the arch version number, e.g.
> change the OpenCSD's macro "ARCH_AA64" to "ARCH_V8R4", (or
> "ARCH_V8R3_AA64"), this can give more clear clue what's the ETM version.
>

The purpose of these macros is to inform the decoder of the
architecture of the PE - not the version of the ETM.

These OpenCSD macros are defined by the library headers
(ocsd_if_types.h) and not the perf headers.
These have been published as the API / ABI for OpenCSD and as such
changing them affects all OpenCSD clients, not just perf.

This PE architecture version is used along with the core profile to
determine which instructions are valid waypoint instructions to
associate with atom elements when walking the program image during
trace decode.

From v8.3  onwards we moved away from filtering on specific
architecture versions. This was due to two factors:-
1. The architectural rules now allow architectural features for one
increment e.g. Arch 8.4, to be backported into  the previous increment
- e,g, 8.3, which made this filtering more difficult to track.
2. After discussion with the PE architects it was clear that
instructions in a later architect version would not re-use older
opcodes from a previous one and  be nop / invalid in the earlier
architectures. (certainly in the scope of AA64). Therefore
the policy in the decoder is to check for all the instructions we know
about for the latest version of architecture, even if we could be
decoding an earlier architecture version. This means we may check for
a few more opcodes than necessary for earlier version of the
architecture, but the overall decode is more robust and easier to
maintain.

Therefore for any AA64 core beyond v8.3 - it is safe to use the
ARCH_AA64 PE architecture version and the decoder will handle it.

Regards

Mike

> And a nitpick: how about to change OpenCSD macro "ARCH_V8r3" to
> "ARCH_V8R3" and assign it for ETMv4.3 IPs.
>
> Thanks,
> Leo



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

  reply	other threads:[~2021-08-02 14:21 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-21  9:06 [PATCH 0/6] Support ETE decoding James Clark
2021-07-21  9:07 ` [PATCH 1/6] perf cs-etm: Refactor initialisation of decoder params James Clark
2021-07-31  5:48   ` Leo Yan
2021-07-21  9:07 ` [PATCH 2/6] perf cs-etm: Initialise architecture based on TRCIDR1 James Clark
2021-07-22 11:10   ` Mike Leach
2021-07-31  6:03     ` Leo Yan
2021-08-02 14:04       ` Mike Leach [this message]
2021-08-02 15:03         ` Leo Yan
2021-08-02 15:43           ` Mike Leach
2021-07-21  9:07 ` [PATCH 3/6] perf cs-etm: Save TRCDEVARCH register James Clark
2021-07-21  9:48   ` Mike Leach
2021-07-23 12:09     ` James Clark
2021-07-31  6:37     ` Leo Yan
2021-08-03 12:33       ` James Clark
2021-08-03 12:34       ` James Clark
2021-08-05  9:40         ` Leo Yan
2021-08-03 12:36       ` James Clark
2021-07-31  7:43   ` Leo Yan
2021-08-02 11:21     ` Mike Leach
2021-08-02 12:05       ` Leo Yan
2021-08-02 12:48         ` Mike Leach
2021-08-03 12:29         ` James Clark
2021-07-21  9:07 ` [PATCH 4/6] perf cs-etm: Update OpenCSD decoder for ETE James Clark
2021-07-31  6:50   ` Leo Yan
2021-07-21  9:07 ` [PATCH 5/6] perf cs-etm: Create ETE decoder James Clark
2021-07-31  7:23   ` Leo Yan
2021-08-03 13:09     ` James Clark
2021-08-05 10:59       ` Leo Yan
2021-07-21  9:07 ` [PATCH 6/6] perf cs-etm: Print the decoder name James Clark
2021-07-31  7:30   ` Leo Yan
2021-08-06  9:43     ` James Clark
2021-08-06 11:52       ` Leo Yan
2021-07-21 14:59 ` [PATCH 0/6] Support ETE decoding Mathieu Poirier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJ9a7VhdLaP3rkXwscAQpVe+jSgcx02WjY=RmiBkaCB6ObGTjQ@mail.gmail.com' \
    --to=mike.leach@linaro.org \
    --cc=acme@kernel.org \
    --cc=al.grant@arm.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=anshuman.khandual@arm.com \
    --cc=coresight@lists.linaro.org \
    --cc=james.clark@arm.com \
    --cc=john.garry@huawei.com \
    --cc=jolsa@redhat.com \
    --cc=leo.yan@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mathieu.poirier@linaro.org \
    --cc=namhyung@kernel.org \
    --cc=suzuki.poulose@arm.com \
    --cc=will@kernel.org \
    --subject='Re: [PATCH 2/6] perf cs-etm: Initialise architecture based on TRCIDR1' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).