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 3/6] perf cs-etm: Save TRCDEVARCH register
Date: Mon, 2 Aug 2021 12:21:31 +0100	[thread overview]
Message-ID: <CAJ9a7Vj_KhV+v6VU+EQN5t818VS9jvf3v3-2JbwVMOHZbi3gcg@mail.gmail.com> (raw)
In-Reply-To: <20210731074343.GG7437@leoy-ThinkPad-X240s>

Hi Leo.

On Sat, 31 Jul 2021 at 08:43, Leo Yan <leo.yan@linaro.org> wrote:
>
> On Wed, Jul 21, 2021 at 10:07:02AM +0100, James Clark wrote:
> > Now that the metadata has a length field we can add extra registers
> > without breaking any previous versions of perf.
> >
> > Save the TRCDEVARCH register so that it can be used to configure the ETE
> > decoder in the next commit. If the sysfs file doesn't exist then 0 will
> > be saved which is an impossible register value and can also be used to
> > signify that the file couldn't be read.
>
> After reviewed the whole patch set, come back to highlight one thing:
> seems to me ETE is only a feature introduced by new ETMv4 revisions; in
> other words, if we support ETMv4.5 or any later revisions, it will
> support ETE feature.
>

The ETE hardware and trace protocol is introduced to support new
architectural features in the PE - principally those associated with
v9 architecture and beyond. It has has additional packet types that
will never appear in any ETM4.x trace sequence.
ETE (and TRBE) are defined as architectural features of the PE - i.e.
FEAT_ETE and FEAT_TRBE read from feature registers on the core.

You are correct in saying that some features in ETM v4.x beyond ETM
4.5 will also appear in ETE, but the reverse is not true - ETE is a
superset of ETMv4.

> Here I think the right thing to do is to support newer revisions for
> ETMv4, and then based on the revision it creates a decoder with
> supporting ETE feature.  For a more neat solution, if the perf tool
> passes the "correct" revision number to the OpenCSD decoder, it should
> can decode trace data with ETE packets.  In this way, the ETE decoding
> can be transparent for perf cs-etm code.
>

The OpenCSD decoder separates the ETMv4 decoder from the ETE decoder -
for the reasons given above.

Additionally the ETE decoder and the ETMv4 decoder required different
sets of ID registers to correctly set up the decoder.  For example,
for ETMv4 the version is extracted form TRCIDR1, for ETE the version
in TRCIDR1 is set 0xFF, and thus needs TRCDEVARCH to extract the
revision. It is likely that later updates to ETE will require an
additional TRCIDR register to be saved.

Choosing the base type of decoder in this way is how the library can
support ETMv3, EMTv4, ETE, STM, PTM etc - and while some of those
protocols use TRCIDR1 and TRCDEVARCH - not all do.

It would in theory be possible to have the OpenCSD library
"autodetect" the type of decoder needed based purely on a set of ID
registers. But this set of ID registers would be far larger than the
ones currently used, and would require modifcation to a lot of the
existing device drivers to ensure they were accessible via sysfs. This
register set includes the ID registers that are currently used to
identify the component on the AMBA bus and match to the correct
driver, plus additional CoreSight management registers. This would
also create a dependency between decoder creation and ID numbers - in
the same way that each new ETM4.x part number has to be added to the
ETM4.x device driver.

Such a system would require a significant update to the OpenCSD
infrastructure, and is not planned at this time.

Regards

Mike


> How about you think for this?  Sorry if I introduce noise due to my
> lack knowledge (and platform) for ETE.
>
> Thanks,
> Leo



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

  reply	other threads:[~2021-08-02 11: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
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 [this message]
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=CAJ9a7Vj_KhV+v6VU+EQN5t818VS9jvf3v3-2JbwVMOHZbi3gcg@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 3/6] perf cs-etm: Save TRCDEVARCH register' \
    /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).