On 31/07/2021 08:23, Leo Yan wrote:
On Wed, Jul 21, 2021 at 10:07:04AM +0100, James Clark wrote:
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 james.clark@arm.com
.../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;
[...]
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?
[...]
+#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