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;
Date:   Sat, 31 Jul 2021 15:43:43 +0800
From:   Leo Yan <leo.yan@linaro.org>
To:     James Clark <james.clark@arm.com>
Cc:     acme@kernel.org, mathieu.poirier@linaro.org,
        coresight@lists.linaro.org, al.grant@arm.com,
        suzuki.poulose@arm.com, anshuman.khandual@arm.com,
        mike.leach@linaro.org, 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@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-perf-users@vger.kernel.org
Subject: Re: [PATCH 3/6] perf cs-etm: Save TRCDEVARCH register
Message-ID: <20210731074343.GG7437@leoy-ThinkPad-X240s>
References: <20210721090706.21523-1-james.clark@arm.com>
 <20210721090706.21523-4-james.clark@arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210721090706.21523-4-james.clark@arm.com>
Precedence: bulk

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.

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.

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

Thanks,
Leo