On 31/07/2021 08:23, Leo Yan wrote:

On Wed, Jul 21, 2021 at 10:07:04AM +0100, James Clark wrote:

quoted

If the TRCDEVARCH register was saved, and it shows that ETE is present,
then instantiate an OCSD_BUILTIN_DCD_ETE decoder instead of
OCSD_BUILTIN_DCD_ETMV4I. ETE is the new trace feature for Armv9.

Testing performed
=================

* Old files with v0 headers still open correctly
* Old files with v1 headers with no TRCDEVARCH saved still open
* New files with TRCDEVARCH open using an old version of perf that
  supports v1 headers
* Coresight decoding results in the same output if there are no new ETE
  packet types

Signed-off-by: James Clark <redacted>
---
 .../perf/util/cs-etm-decoder/cs-etm-decoder.c | 29 ++++++++++-
 .../perf/util/cs-etm-decoder/cs-etm-decoder.h |  7 +++
 tools/perf/util/cs-etm.c                      | 49 ++++++++++++++++++-
 tools/perf/util/cs-etm.h                      |  1 +
 4 files changed, 82 insertions(+), 4 deletions(-)

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 60147c908425..37bc9d6a7677 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c

@@ -127,8 +127,12 @@ static int cs_etm_decoder__gen_etmv3_config(struct cs_etm_trace_params *params,
 #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)
+static enum _ocsd_arch_version cs_etm_decoder__get_arch_ver(u32 reg_idr1, u32 reg_devarch)
 {
+	/* ETE has to be v9 so set arch version to v8.3+ (ARCH__AA64) */
+	if (cs_etm__is_ete(reg_devarch))
+		return ARCH_AA64;
+

Based on values used in below change, I think we can unify the ETM
versio number like:

  ARCH_V8R3 : REVISION, bits[19:16] is 0x3
  ARCH_V8R4 : REVISION, bits[19:16] is 0x4
  ARCH_V8R5 : REVISION, bits[19:16] is 0x5

Do you mean make this change in OpenCSD? At the moment it understands these
values so I'm not sure if the extra ones would be useful:

	/** Core Architecture Version */
	typedef enum _ocsd_arch_version {
	    ARCH_UNKNOWN = 0x0000,   /**< unknown architecture */
	    ARCH_CUSTOM = 0x0001,    /**< None ARM, custom architecture */
	    ARCH_V7 = 0x0700,        /**< V7 architecture */
	    ARCH_V8 = 0x0800,        /**< V8 architecture */
	    ARCH_V8r3 = 0x0803,      /**< V8.3 architecture */
	    ARCH_AA64 = 0x0864,      /**< Min v8r3 plus additional AA64 PE features */
	    ARCH_V8_max = ARCH_AA64,
	} ocsd_arch_version_t;

[...]

quoted

diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
index 11f3391d06f2..9137796fe3c5 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h

@@ -37,11 +37,17 @@ struct cs_etmv4_trace_params {
 	u32 reg_traceidr;
 };
 
+struct cs_ete_trace_params {
+	struct cs_etmv4_trace_params base_params;
+	u32 reg_devarch;

As we have said, can we directly support ETMv4.5, so that it can
smoothly support ETE features?  If so, we don't need to add a new
structure "cs_ete_trace_params" at here.

I think with the new magic number change this is more likely to stay,
what are your thoughts?

[...]

quoted

+
+#define TRCDEVARCH_ARCHPART_SHIFT 0
+#define TRCDEVARCH_ARCHPART_MASK  GENMASK(11, 0)
+#define TRCDEVARCH_ARCHPART(x)    (((x) & TRCDEVARCH_ARCHPART_MASK) >> TRCDEVARCH_ARCHPART_SHIFT)
+
+#define TRCDEVARCH_ARCHVER_SHIFT 12
+#define TRCDEVARCH_ARCHVER_MASK  GENMASK(15, 12)
+#define TRCDEVARCH_ARCHVER(x)    (((x) & TRCDEVARCH_ARCHVER_MASK) >> TRCDEVARCH_ARCHVER_SHIFT)
+
+bool cs_etm__is_ete(u32 trcdevarch)
+{
+	/*
+	 * ETE if ARCHVER is 5 (ARCHVER is 4 for ETM) and ARCHPART is 0xA13.
+	 * See ETM_DEVARCH_ETE_ARCH in coresight-etm4x.h
+	 */
+	return TRCDEVARCH_ARCHVER(trcdevarch) == 5 && TRCDEVARCH_ARCHPART(trcdevarch) == 0xA13;

I think this is incorrect.

Here should check the bit field "REVISION, bits[19:16]".  If it's
field value is >= 5, then we can say it supports ETE.  I checked the
spec for ETMv4.4 and ETMv4.6, both use the same values for the
Bits[15:12] = 0x4, so the architecture ID is same for ETMv4.x IPs.

I tried to copy this as closely as possible from the ETE driver. See in coresight-etm4x.h

	#define ETM_DEVARCH_ETE_ARCH						\
		(ETM_DEVARCH_ARCHITECT_ARM | ETM_DEVARCH_ARCHID_ETE | ETM_DEVARCH_PRESENT) 

Where ETM_DEVARCH_ARCHID_ETE is ARCHVER == 5 and ARCHPART == 0xA13. I didn't check 
ETM_DEVARCH_ARCHITECT_ARM because I thought that wouldn't be necessary. If we want to make
the change do detect >= 5 then I think this should be made in the driver first. @Suzuki,
what do you think?

Thanks
James